linux-kernel - Re: [PATCHv2] PCI/ACPI: _DSM PRESERVE_BOOT_CONFIG function rev id doesn't match with spec.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0g66WmqeouN6AWADE+J3e-f30wMGyLX-Upk+w7Y+i2OxA@mail.gmail.com>
Date: Tue, 10 Dec 2024 20:56:14 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Zhou Shengqing <zhoushengqing@...info.com>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Len Brown <lenb@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>, linux-acpi@...r.kernel.org, 
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCHv2] PCI/ACPI: _DSM PRESERVE_BOOT_CONFIG function rev id
 doesn't match with spec.

On Tue, Dec 10, 2024 at 8:50 PM Bjorn Helgaas <helgaas@...nel.org> wrote:
>
> [+cc Ben, original author of a78cf9657ba5]
>
> On Thu, Nov 14, 2024 at 03:04:24AM +0000, Zhou Shengqing wrote:
> > Per PCI Firmware Specification Revision 3.3 Table 4-7 _DSM Definitions
> > for PCI. Preserve PCI Boot Configuration Initial Revision ID is 2. But
> > the code is 1.
>
> This _DSM function 5 was added in PCI Firmware r3.1, released Dec 13,
> 2010.  It's listed in sec 4.6 with Revision 2 (as *all* the defined
> functions are, even functions 1-4, which were included in r3.0 with
> Revision 1).
>
> But the actual definition that was added in r3.1 is in sec 4.6.5,
> which specifies Revision ID 1.
>
> PCI Firmware r3.2, released Jan 26, 2015, was the newest available at
> the time Ben implemented a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot
> Configuration _DSM"), and sec 4.6.5 still specified Revision ID 1.
>
> So I think Ben's addition used the correct Revision ID (1).
>
> PCI Firmware r3.3, released Jan 20, 2021, changed sec 4.6.5 to say
> "lowest valid Revision ID value: 2"
>
> I think it's a mistake to make the kernel change below because
> platforms in the field implemented function 5 with revision 1 (per the
> r3.1 and r3.2 specs), and we have no idea whether they implement
> function 5 revision 2.
>
> It's quite likely that newer platforms following r3.3 will implement
> function 5 revision 2, but NOT revision 1, and the existing code won't
> work for them.
>
> I think the fix is to try revision 1 and, if that isn't implemented,
> we should try revision 2.  The semantics stayed the same, so they
> should both work the same.

Or call Function 0 with the new revision and check the result?

> > Fixes: 9d7d5db8e78e ("PCI: Move PRESERVE_BOOT_CONFIG _DSM evaluation to pci_register_host_bridge()")
> > Origin fixes: a78cf9657ba5 ("PCI/ACPI: Evaluate PCI Boot Configuration _DSM")
> >
> > Signed-off-by: Zhou Shengqing <zhoushengqing@...info.com>
> > ---
> >  drivers/pci/pci-acpi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
> > index af370628e583..7a4cad0c1f00 100644
> > --- a/drivers/pci/pci-acpi.c
> > +++ b/drivers/pci/pci-acpi.c
> > @@ -132,7 +132,7 @@ bool pci_acpi_preserve_config(struct pci_host_bridge *host_bridge)
> >                */
> >               obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev),
> >                                             &pci_acpi_dsm_guid,
> > -                                           1, DSM_PCI_PRESERVE_BOOT_CONFIG,
> > +                                           2, DSM_PCI_PRESERVE_BOOT_CONFIG,
> >                                             NULL, ACPI_TYPE_INTEGER);
> >               if (obj && obj->integer.value == 0)
> >                       return true;
> > --
> > 2.39.2
> >