[prev in list] [next in list] [prev in thread] [next in thread] 

List:       fwts-devel
Subject:    ACK: [PATCH] acpi: hpet: fix getting invalid vendor ID and clock period
From:       Alex Hung <alex.hung () canonical ! com>
Date:       2018-05-21 18:39:20
Message-ID: 1a3cd54e-0636-50a7-d632-12794112e2c9 () canonical ! com
[Download RAW message or body]

On 2018-05-21 02:59 AM, 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@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: Alex Hung <alex.hung@canonical.com>

-- 
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel

[prev in list] [next in list] [prev in thread] [next in thread]