Re: [BUG] PVH ACPI XSDT table construction

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 26, 2020 at 01:13:19PM -0400, Daniel Smith wrote:
> Greetings,
> 
> 
> 
> I was reviewing the ACPI construction for PVH and discovered what I believe 
> is a flaw in the logic for selecting the XSDT tables. The current logic is,
> 
> 
> 
> static bool __init pvh_acpi_xsdt_table_allowed(const char *sig,
> 
>                                                unsigned long address,
> 
>                                                unsigned long size)
> 
> {
> 
>     /*
> 
>      * DSDT and FACS are pointed to from FADT and thus don't belong
> 
>      * in XSDT.
> 
>      */
> 
>     return (pvh_acpi_table_allowed(sig, address, size) &&
> 
>             strncmp(sig, ACPI_SIG_DSDT, ACPI_NAME_SIZE) &&
> 
>             strncmp(sig, ACPI_SIG_FACS, ACPI_NAME_SIZE));
> 
> }
> 
> 
> 
> Unless I am mistaken, the boolean logic in the return statement will always 
> return false resulting in an empty XSDT table. I believe based on the comment 
> what was intended here was,
> 
> 
> 
>     return (pvh_acpi_table_allowed(sig, address, size) &&
> 
>             !(strncmp(sig, ACPI_SIG_DSDT, ACPI_NAME_SIZE) ||
> 
>               strncmp(sig, ACPI_SIG_FACS, ACPI_NAME_SIZE)));

Keep in mind that strncmp will return 0 if the signature matches, and
hence doing this won't allow any table, as it would require a
signature to match both the DSDT and the FACS one (you would require
strncmp to return 0 in both cases).

The code is correct AFAICT, as it won't add DSDT or FACS to the XSDT
(because strncmp will return 0 in that case).

Roger.