ACK: [PATCH 1/3] sbbr: add checks for mandatory tables

[Colin Ian King]

On 04/06/2021 06:11, Alex Hung wrote:
> This is based on Arm Server Base Boot Requirements (SBBR) 1.2
> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/sbbr/acpitables/acpitables.c | 31 ++++++++++++++++++++++++++++++-
>  1 file changed, 30 insertions(+), 1 deletion(-)
> 
> diff --git a/src/sbbr/acpitables/acpitables.c b/src/sbbr/acpitables/acpitables.c
> index fa63be64..0919250b 100644
> --- a/src/sbbr/acpitables/acpitables.c
> +++ b/src/sbbr/acpitables/acpitables.c
> @@ -195,6 +195,22 @@ static int acpi_table_sbbr_check_test2(fwts_framework *fw)
>  	return FWTS_OK;
>  }
>  
> +/* List of mandatory ACPI tables (SBBR 4.2.1) */
> +static const char *mandatory_acpi_tables[] = {
> +	"RSDP",
> +	"XSDT",
> +	"FACP",
> +	"DSDT",
> +	"SSDT",
> +	"MADT",
> +	"GTDT",
> +	"DBG2",
> +	"SPCR",
> +	"MCFG",
> +	"PPTT",
> +	NULL
> +};
> +
>  /* List of ACPI tables recommended by SBBR 4.2.2 */
>  static const char *recommended_acpi_tables[] = {
>  	"MCFG",
> @@ -235,6 +251,19 @@ static int acpi_table_sbbr_check_test3(fwts_framework *fw)
>  {
>  	uint32_t i;
>  
> +	for (i = 0; mandatory_acpi_tables[i] != NULL; i++) {
> +		fwts_acpi_table_info *info;

FYI, we could use:

	size_t i;

	for (i = 0; i < FWTS_ARRAY_SIZE(mandatory_acpi_tables); i++) {

..and remove the NULL from the mandatory_acpi_tables array to save a few
bytes.

I think this could be addressed with a subsequent patch rather than
nack'ing it here.

> +
> +		info = sbbr_search_acpi_tables(fw, mandatory_acpi_tables[i]);
> +		if (info == NULL) {
> +			fwts_failed(fw, LOG_LEVEL_CRITICAL, "SBBRTableNotFound",
> +				    "SBBR mandatory ACPI table \"%s\" not found.", mandatory_acpi_tables[i]);
> +		} else {
> +			fwts_passed(fw, "SBBR mandatory ACPI table \"%s\" found.",
> +			            mandatory_acpi_tables[i]);
> +		}
> +	}
> +
>  	for (i = 0; recommended_acpi_tables[i] != NULL; i++) {
>  		fwts_acpi_table_info *info;
>  
> @@ -253,7 +282,7 @@ static int acpi_table_sbbr_check_test3(fwts_framework *fw)
>  static fwts_framework_minor_test acpi_table_sbbr_check_tests[] = {
>  	{ acpi_table_sbbr_namespace_check_test1, "Test that processors only exist in the _SB namespace." },
>  	{ acpi_table_sbbr_check_test2, "Test DSDT and SSDT tables are implemented." },
> -	{ acpi_table_sbbr_check_test3, "Check for recommended ACPI tables." },
> +	{ acpi_table_sbbr_check_test3, "Check for mandatory and recommended ACPI tables." },
>  	{ NULL, NULL }
>  };
>  
> 


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