On Tue, Dec 10, 2024 at 8:50 PM Bjorn Helgaas <helgaas@xxxxxxxxxx> 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@xxxxxxxxxxx>
> > ---
> > 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
> >
