[PATCH 08/21] FADT: minor cleanup and initial compliance tests

Alex Hung alex.hung at canonical.com
Wed Feb 17 05:35:57 UTC 2016


On 2016-02-09 09:32 AM, Al Stone wrote:
> The primary purpose of this patch is to catch some very minor white
> space edits while adding in two very simple compliance tests -- is the
> table checksum correct, and is the revision number current?
>
> Signed-off-by: Al Stone <al.stone at linaro.org>
> ---
>   src/acpi/fadt/fadt.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 47 insertions(+), 4 deletions(-)
>
> diff --git a/src/acpi/fadt/fadt.c b/src/acpi/fadt/fadt.c
> index 5106e4e..6e5ee26 100644
> --- a/src/acpi/fadt/fadt.c
> +++ b/src/acpi/fadt/fadt.c
> @@ -20,8 +20,8 @@
>    */
>   #include "fwts.h"
>
> -#include <stdlib.h>
>   #include <stdio.h>
> +#include <stdlib.h>
>   #include <sys/types.h>
>   #include <sys/stat.h>
>   #ifdef FWTS_ARCH_INTEL
> @@ -53,7 +53,7 @@ static int fadt_init(fwts_framework *fw)
>   		fwts_log_error(fw, "ACPI table FACP does not exist!");
>   		return FWTS_ERROR;
>   	}
> -	fadt = (const fwts_acpi_table_fadt*)table->data;
> +	fadt = (const fwts_acpi_table_fadt *)table->data;
>   	fadt_size = table->length;
>
>   	/*  Not having a FADT is not a failure on x86 */
> @@ -152,6 +152,47 @@ static int fadt_info(fwts_framework *fw)
>   	return FWTS_OK;
>   }
>
> +static int fadt_checksum(fwts_framework *fw)
> +{
> +	const uint8_t *data = (const uint8_t *)fadt;
> +	ssize_t length = fadt->header.length;
> +	uint8_t checksum = 0;
> +
> +	/* verify the table checksum */
> +	checksum = fwts_checksum(data, length);
> +	if (checksum == 0)
> +		fwts_passed(fw, "FADT checksum is correct");
> +	else
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +			    "SPECMADTChecksum",
> +			    "FADT checksum is incorrect: 0x%x", checksum);
> +
> +	return FWTS_OK;
> +}
> +
> +static int fadt_revision(fwts_framework *fw)
> +{
> +	uint8_t major;
> +	uint8_t minor;
> +
> +	major = fadt->header.revision;
> +	minor = 0;
> +	if (major >= 5 && fadt->header.length >= 268)
> +		minor = fadt->minor_version;   /* field added ACPI 5.1 */
> +
> +	fwts_log_info(fw, "FADT revision: %d.%d", major, minor);
> +	fwts_log_info(fw, "FADT table length: %d", fadt->header.length);
> +
> +	if (major == 6)
> +		fwts_passed(fw, "FADT revision is up to date.");
> +	else
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM, "SPECFADTRevision",
> +			    "FADT revision is outdated: %d.%d", major, minor);

If fwts is run on older systems with FADT revision not equal to 6, this 
generates an error even though the systems were cutting-edge at their 
shipping time.  I am not very sure a medium failure is the best choice.

How about an info or a warning with an advice so either a firmware 
developer can update it or an user can check for a firmware update?

Any comments from anyone?

> +
> +	return FWTS_OK;
> +}
> +
> +
>   static void acpi_table_check_fadt_firmware_control(
>   	fwts_framework *fw,
>   	const fwts_acpi_table_fadt *fadt,
> @@ -635,8 +676,10 @@ static int fadt_test3(fwts_framework *fw)
>   }
>
>   static fwts_framework_minor_test fadt_tests[] = {
> -	{ fadt_info, "FADT ACPI Description Table flag info." },
> -	{ fadt_test1, "Test FADT ACPI Description Table tests." },
> +	{ fadt_info, "ACPI FADT Description Table flag info." },
> +	{ fadt_checksum, "FADT checksum test." },
> +	{ fadt_revision, "FADT revision test." },
> +	{ fadt_test1, "ACPI FADT Description Table tests." },
>   	{ fadt_test2, "Test FADT SCI_EN bit is enabled." },
>   	{ fadt_test3, "Test FADT reset register." },
>   	{ NULL, NULL }
>


-- 
Cheers,
Alex Hung



More information about the fwts-devel mailing list