ACK: [PATCH] acpi: hpet: fix getting invalid vendor ID and clock period

[Colin Ian King]

On 21/05/18 10:59, Ivan Hu wrote:
> Some platforms get the report again,
> 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.
> 
> some platforms can be fixed by commit#
> ee769ccf294cff2c63769482e84866a979d42f44, but still got fail on
> some Intel platforms with latest Bios.
> 
> The root cause is commit#7cfe1a41956294bd8062b7680e1778ed9ee147b2
> adds safe memory read checking, so do two memory reads immediately, HW might
>  not able to get ready for registers to be read and get the false values.
> 
> Adding delay between two reads could solve this issue, and also change to 64
> bits safe memory read check.
> 
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  src/acpi/hpet/hpet.c            |  4 +++-
>  src/lib/include/fwts_safe_mem.h |  1 +
>  src/lib/src/fwts_safe_mem.c     | 35 +++++++++++++++++++++++++++++++++++
>  3 files changed, 39 insertions(+), 1 deletion(-)
> 
> diff --git a/src/acpi/hpet/hpet.c b/src/acpi/hpet/hpet.c
> index fca11e6..92a0c58 100644
> --- a/src/acpi/hpet/hpet.c
> +++ b/src/acpi/hpet/hpet.c
> @@ -428,12 +428,14 @@ static int hpet_check_test4(fwts_framework *fw)
>  		return FWTS_ERROR;
>  	}
>  
> -	if (fwts_safe_memread32(hpet_base_v, HPET_REG_SIZE / 4) != FWTS_OK) {
> +	if (fwts_safe_memread64(hpet_base_v, HPET_REG_SIZE / 8) != 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;
>  	}
>  
> +	usleep(10);
> +
>  	hpet_id = *(uint64_t*) hpet_base_v;
>  	vendor_id = (hpet_id & 0xffff0000) >> 16;
>  
> diff --git a/src/lib/include/fwts_safe_mem.h b/src/lib/include/fwts_safe_mem.h
> index 964cb8d..3ba9094 100644
> --- a/src/lib/include/fwts_safe_mem.h
> +++ b/src/lib/include/fwts_safe_mem.h
> @@ -23,5 +23,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);
> +int fwts_safe_memread64(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 199b27f..10d4f7c 100644
> --- a/src/lib/src/fwts_safe_mem.c
> +++ b/src/lib/src/fwts_safe_mem.c
> @@ -127,3 +127,38 @@ int OPTIMIZE0 fwts_safe_memread32(const void *src, const size_t n)
>  
>  	return FWTS_OK;
>  }
> +
> +/*
> + *  fwts_safe_memread64()
> + *	check we can safely read a region of memory. This catches
> + *	SIGSEGV/SIGBUS errors and returns FWTS_ERROR if it is not
> + *	readable or FWTS_OK if it's OK.
> + *
> + *	n = number of of 64 bit words to check
> + */
> +int OPTIMIZE0 fwts_safe_memread64(const void *src, const size_t n)
> +{
> +	static uint64_t buffer[256];
> +	const uint64_t *ptr, *end = (uint64_t *)src + n;
> +	uint64_t *bufptr;
> +	const uint64_t *bufend = buffer + (sizeof(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;
> +}
> 
Acked-by: Colin Ian King <colin.king at canonical.com>