[BUG] PVH ACPI XSDT table construction

[BUG] PVH ACPI XSDT table construction

[Daniel Smith]

[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]

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)));



Thanks!


V/r,

Daniel P. Smith

Apertus Solutions, LLC

[-- Attachment #2: Type: text/html, Size: 2553 bytes --]

Re: [BUG] PVH ACPI XSDT table construction

[Roger Pau Monné]

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.

Re: [BUG] PVH ACPI XSDT table construction

[Daniel P. Smith]

On 5/26/20 1:57 PM, Roger Pau Monné wrote:
> 
> 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.
> 

Ugh, your are right. Apologies for the noise.

V/r,
DPS