[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [BUG] PVH ACPI XSDT table construction


  • To: Daniel Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Date: Tue, 26 May 2020 19:57:34 +0200
  • Authentication-results: esa1.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none
  • Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Tue, 26 May 2020 17:57:50 +0000
  • Ironport-sdr: NjSlT6lCFbs4DsndjP2AVVVWYUbhekmkov3gNnsNRRqog9kA6FV7rTCzkPNduU+D82ECQIYEbm thbB9Yka+1OvqcN/hjPVEcs4I2pMp4/IG6Gfo+IlLahN2WX5GdPpoUt+7BdoXxkYm97zu0coLe PoCz2tekGqFt2GZkqP9f3jLOdugN4+0QOGIxLPI143bgDGiLDfBToeo6++nEiZPtHEpNMZZUju Ce/ueXEwn3Z90mkMzGwKA8erJIClHfBPOZYiRuwJ1IF2f19nEOyuXDwP3O1R2kGhkT7aq48Rfj Aj4=
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.