ACK: [PATCH] hpet: fix the false alarm of hpet configuration test

[Colin Ian King]

On 06/11/17 09:19, Ivan Hu wrote:
> Some platforms got the report,
> Invalid Vendor ID: ffff - this should be configured.
> Invalid clock period 4294967295, must be non-zero and less than 10^8.
> after the commit#7cfe1a41956294bd8062b7680e1778ed9ee147b2 applied.
> 
> It is because HPET spec defined those registers need to 32-bit or 64-bit
> accesses. 8-bit read causes the following checking fail.
> 
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  src/acpi/hpet/hpet.c            |  2 +-
>  src/lib/include/fwts_safe_mem.h |  1 +
>  src/lib/src/fwts_safe_mem.c     | 27 +++++++++++++++++++++++++++
>  3 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/src/acpi/hpet/hpet.c b/src/acpi/hpet/hpet.c
> index 15917d4..c858c3e 100644
> --- a/src/acpi/hpet/hpet.c
> +++ b/src/acpi/hpet/hpet.c
> @@ -428,7 +428,7 @@ static int hpet_check_test4(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	}
>  
> -	if (fwts_safe_memread(hpet_base_v, HPET_REG_SIZE) != FWTS_OK) {
> +	if (fwts_safe_memread32(hpet_base_v, HPET_REG_SIZE / 4) != FWTS_OK) {
>  		fwts_log_info(fw, "Test skipped because HPET region cannot be read.");
>  		(void)fwts_munmap(hpet_base_v, HPET_REG_SIZE);
>  		return FWTS_SKIP;
> diff --git a/src/lib/include/fwts_safe_mem.h b/src/lib/include/fwts_safe_mem.h
> index b9fecd3..e376019 100644
> --- a/src/lib/include/fwts_safe_mem.h
> +++ b/src/lib/include/fwts_safe_mem.h
> @@ -22,5 +22,6 @@
>  
>  int fwts_safe_memcpy(void *dst, const void *src, const size_t n);
>  int fwts_safe_memread(const void *src, const size_t n);
> +int fwts_safe_memread32(const void *src, const size_t n);
>  
>  #endif
> diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c
> index f3f639b..c6b09f9 100644
> --- a/src/lib/src/fwts_safe_mem.c
> +++ b/src/lib/src/fwts_safe_mem.c
> @@ -92,3 +92,30 @@ int OPTIMIZE0 fwts_safe_memread(const void *src, const size_t n)
>  
>  	return FWTS_OK;
>  }
> +
> +int OPTIMIZE0 fwts_safe_memread32(const void *src, const size_t n)
> +{
> +	static uint32_t buffer[256];
> +	const uint32_t *ptr, *end = src + n;
> +	uint32_t *bufptr;
> +	const uint32_t *bufend = buffer + sizeof(buffer);
> +
> +	if (sigsetjmp(jmpbuf, 1) != 0)
> +		return FWTS_ERROR;
> +
> +	fwts_sig_handler_set(SIGSEGV, sig_handler, &old_segv_action);
> +	fwts_sig_handler_set(SIGBUS, sig_handler, &old_bus_action);
> +
> +	for (bufptr = buffer, ptr = src; ptr < end; ptr++) {
> +		/* Force data to be read */
> +		__builtin_prefetch(ptr, 0, 3);
> +		*bufptr = *ptr;
> +		bufptr++;
> +		bufptr = (bufptr >= bufend) ? buffer : bufptr;
> +	}
> +
> +	fwts_sig_handler_restore(SIGSEGV, &old_segv_action);
> +	fwts_sig_handler_restore(SIGBUS, &old_bus_action);
> +
> +	return FWTS_OK;
> +}
> 

Good catch.

Acked-by: Colin Ian King <colin.king at canonical.com>