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.
