ACK: [PATCH] acpi: madt: only inspect Local APIC/x2APIC/SAPIC tables if enabled

[Alex Hung]

On 2017-11-28 08:55 AM, Ricardo Neri wrote:
> The Advanced Configuration and Power Interface (ACPI) specification
> version 6.2 Errata A in sections 5.2.12.2, 5.2.12.10 and 5.2.12.12 states
> that "when a processor is not present, the Processor Local APIC/x2APIC/
> SAPIC information is either not reported or flagged as disabled."
> 
> This implies that it is possible for firmware implementations to include
> Local APIC/SAPIC/x2APIC tables for non-existent processors provided that
> they are disabled.
> 
> Thus, only test these tables if they have the Enable flag set. Otherwise,
> skip the test. Also, print a message to record the fact that these tables
> exist but are not tested.
> 
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon at linux.intel.com>
> Reported-by: Bin Song <binx.song at intel.com>
> ---
>   src/acpi/madt/madt.c | 25 +++++++++++++++++++++++++
>   1 file changed, 25 insertions(+)
> 
> diff --git a/src/acpi/madt/madt.c b/src/acpi/madt/madt.c
> index 3bbaf2f..f997a5f 100644
> --- a/src/acpi/madt/madt.c
> +++ b/src/acpi/madt/madt.c
> @@ -546,6 +546,13 @@ static int madt_local_apic(fwts_framework *fw,
>   	fwts_acpi_madt_processor_local_apic *lapic =
>   		(fwts_acpi_madt_processor_local_apic *)data;
>   
> +	/* only test table if enabled */
> +	if (!(lapic->flags & 0x1)) {
> +		fwts_skipped(fw, "MADT %s is disabled. Skipping test.",
> +			      madt_sub_names[hdr->type]);
> +		goto out;
> +	}
> +
>   	madt_find_processor_uid(fw, lapic->acpi_processor_id, "LAPIC");
>   
>   	if (lapic->flags & 0xfffffffe)
> @@ -561,6 +568,7 @@ static int madt_local_apic(fwts_framework *fw,
>   			    "reserved and properly set to zero.",
>   			    madt_sub_names[hdr->type]);
>   
> +out:
>   	return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
>   }
>   
> @@ -780,6 +788,7 @@ static int madt_local_sapic(fwts_framework *fw,
>   	 * initial boot state.
>   	 */
>   
> +
>   	if (hdr->length != (strlen(lsapic->uid_string) + 16))
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
>   			    "MADTLSAPICLength",
> @@ -792,6 +801,13 @@ static int madt_local_sapic(fwts_framework *fw,
>   			    "MADT %s table length (%d) is correct.",
>   			    madt_sub_names[hdr->type], hdr->length);
>   
> +	/* only continue testing table if enabled */
> +	if (!(lsapic->flags & 0x1)) {
> +		fwts_skipped(fw, "MADT %s is disabled. Skipping test.",
> +			      madt_sub_names[hdr->type]);
> +		goto out;
> +	}
> +
>   	/*
>   	 * There are three values that need to be checked for a valid
>   	 * processor UID: the ACPI processor ID, the UID value, and the UID
> @@ -878,6 +894,7 @@ static int madt_local_sapic(fwts_framework *fw,
>   			    "MADT %s UID string is an ASCII string.",
>   			    madt_sub_names[hdr->type]);
>   
> +out:
>   	return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
>   }
>   
> @@ -952,6 +969,13 @@ static int madt_local_x2apic(fwts_framework *fw,
>   	fwts_acpi_madt_local_x2apic *lx2apic =
>   					(fwts_acpi_madt_local_x2apic *)data;
>   
> +	/* only test table if enabled */
> +	if (!(lx2apic->flags & 0x1)) {
> +		fwts_skipped(fw, "MADT %s is disabled. Skipping test.",
> +			      madt_sub_names[hdr->type]);
> +		goto out;
> +	}
> +
>   	if (lx2apic->reserved)
>   		fwts_failed(fw, LOG_LEVEL_MEDIUM,
>   			    "MADTLX2APICReservedNonZero",
> @@ -980,6 +1004,7 @@ static int madt_local_x2apic(fwts_framework *fw,
>   
>   	madt_find_processor_uid(fw, lx2apic->processor_uid, "X2APIC");
>   
> +out:
>   	return (hdr->length - sizeof(fwts_acpi_madt_sub_table_header));
>   }
>   
> 


Acked-by: Alex Hung <alex.hung at canonical.com>