| Message ID | 20250313100658.15805-1-zhoushengqing@ttyinfo.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [v5,1/2] Add rev 2 check for PRESERVE_BOOT_CONFIG function | expand |
On Thu, Mar 13, 2025 at 11:07 AM Zhou Shengqing <zhoushengqing@ttyinfo.com> wrote: > > Per PCI Firmware Specification Revision 3.3 Table 4-7 _DSM Definitions > for PCI. Preserve PCI Boot Configuration Initial Revision ID changed to 2. > > And add acpi_check_dsm() for DSM_PCI_PRESERVE_BOOT_CONFIG. > > Signed-off-by: Zhou Shengqing <zhoushengqing@ttyinfo.com> > --- > v5:follow Bjorn advice, add acpi_check_dsm for PCI _DSM. > v4:Initialize *obj to NULL. > v3:try revision id 1 first, then try revision id 2. > v2:add Fixes tag. > > Fixes: 9d7d5db8e78e ("PCI: Move PRESERVE_BOOT_CONFIG _DSM evaluation to pci_re") > --- > drivers/pci/pci-acpi.c | 43 ++++++++++++++++++++++++++++++------------ > 1 file changed, 31 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > index af370628e583..4f9e0548c96d 100644 > --- a/drivers/pci/pci-acpi.c > +++ b/drivers/pci/pci-acpi.c > @@ -122,24 +122,43 @@ phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) > > bool pci_acpi_preserve_config(struct pci_host_bridge *host_bridge) > { > - if (ACPI_HANDLE(&host_bridge->dev)) { > - union acpi_object *obj; > + bool rc = false; > + union acpi_object *obj; + u64 rev; > > - /* > - * Evaluate the "PCI Boot Configuration" _DSM Function. If it > - * exists and returns 0, we must preserve any PCI resource > - * assignments made by firmware for this host bridge. > - */ > + if (!ACPI_HANDLE(&host_bridge->dev)) > + return false; > + > + /* > + * Evaluate the "PCI Boot Configuration" _DSM Function. If it > + * exists and returns 0, we must preserve any PCI resource > + * assignments made by firmware for this host bridge. > + * > + * Per PCI Firmware r3.2, released Jan 26, 2015, > + * DSM_PCI_PRESERVE_BOOT_CONFIG Revision ID is 1. But PCI Firmware r3.3, > + * released Jan 20, 2021, changed sec 4.6.5 to say > + * "lowest valid Revision ID value: 2". So check rev 1 first, then rev 2. > + */ + for (rev = 1; rev <= 2; rev++) { + if (!acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), &pci_acpi_dsm_guid, + rev, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) + continue; + + obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), , &pci_acpi_dsm_guid, + rev, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG)); + if (obj && obj->integer.value == 0) + rc = true; + + ACPI_FREE(obj); + + if (rc) + return true; + } + + return false; would achieve the same with fewer lines of code and less code duplication if I'm not mistaken. > + if (acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), > + &pci_acpi_dsm_guid, 1, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) { > obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), > - &pci_acpi_dsm_guid, > - 1, DSM_PCI_PRESERVE_BOOT_CONFIG, > - NULL, ACPI_TYPE_INTEGER); > + &pci_acpi_dsm_guid, > + 1, DSM_PCI_PRESERVE_BOOT_CONFIG, > + NULL, ACPI_TYPE_INTEGER); > if (obj && obj->integer.value == 0) > - return true; > + rc = true; > + ACPI_FREE(obj); > + } else if (acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), > + &pci_acpi_dsm_guid, 2, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) { > + obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), > + &pci_acpi_dsm_guid, > + 2, DSM_PCI_PRESERVE_BOOT_CONFIG, > + NULL, ACPI_TYPE_INTEGER); > + if (obj && obj->integer.value == 0) > + rc = true; > ACPI_FREE(obj); > } > > - return false; > + return rc; > } > > /* _HPX PCI Setting Record (Type 0); same as _HPP */ > --
On Thu, Mar 27, 2025 at 7:57 PM Rafael J. Wysocki <rafael@kernel.org> wrote: > > On Thu, Mar 13, 2025 at 11:07 AM Zhou Shengqing > <zhoushengqing@ttyinfo.com> wrote: > > > > Per PCI Firmware Specification Revision 3.3 Table 4-7 _DSM Definitions > > for PCI. Preserve PCI Boot Configuration Initial Revision ID changed to 2. > > > > And add acpi_check_dsm() for DSM_PCI_PRESERVE_BOOT_CONFIG. > > > > Signed-off-by: Zhou Shengqing <zhoushengqing@ttyinfo.com> > > --- > > v5:follow Bjorn advice, add acpi_check_dsm for PCI _DSM. > > v4:Initialize *obj to NULL. > > v3:try revision id 1 first, then try revision id 2. > > v2:add Fixes tag. > > > > Fixes: 9d7d5db8e78e ("PCI: Move PRESERVE_BOOT_CONFIG _DSM evaluation to pci_re") > > --- > > drivers/pci/pci-acpi.c | 43 ++++++++++++++++++++++++++++++------------ > > 1 file changed, 31 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > index af370628e583..4f9e0548c96d 100644 > > --- a/drivers/pci/pci-acpi.c > > +++ b/drivers/pci/pci-acpi.c > > @@ -122,24 +122,43 @@ phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) > > > > bool pci_acpi_preserve_config(struct pci_host_bridge *host_bridge) > > { > > - if (ACPI_HANDLE(&host_bridge->dev)) { > > - union acpi_object *obj; > > + bool rc = false; > > + union acpi_object *obj; > > + u64 rev; > > > > > - /* > > - * Evaluate the "PCI Boot Configuration" _DSM Function. If it > > - * exists and returns 0, we must preserve any PCI resource > > - * assignments made by firmware for this host bridge. > > - */ > > + if (!ACPI_HANDLE(&host_bridge->dev)) > > + return false; > > + > > + /* > > + * Evaluate the "PCI Boot Configuration" _DSM Function. If it > > + * exists and returns 0, we must preserve any PCI resource > > + * assignments made by firmware for this host bridge. > > + * > > + * Per PCI Firmware r3.2, released Jan 26, 2015, > > + * DSM_PCI_PRESERVE_BOOT_CONFIG Revision ID is 1. But PCI Firmware r3.3, > > + * released Jan 20, 2021, changed sec 4.6.5 to say > > + * "lowest valid Revision ID value: 2". So check rev 1 first, then rev 2. > > + */ > > + for (rev = 1; rev <= 2; rev++) { > + if (!acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), > &pci_acpi_dsm_guid, > + rev, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) > + continue; > + > + obj = > acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), , > &pci_acpi_dsm_guid, > + rev, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG)); Of course, the above acpi_evaluate_dsm_typed() call has been messed-up, it should be obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), &pci_acpi_dsm_guid, rev, DSM_PCI_PRESERVE_BOOT_CONFIG, NULL, ACPI_TYPE_INTEGER); but the idea should be clear nevertheless. > + if (obj && obj->integer.value == 0) > + rc = true; > + > + ACPI_FREE(obj); > + > + if (rc) > + return true; > + } > + > + return false; > > would achieve the same with fewer lines of code and less code > duplication if I'm not mistaken. > > > + if (acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), > > + &pci_acpi_dsm_guid, 1, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) { > > obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), > > - &pci_acpi_dsm_guid, > > - 1, DSM_PCI_PRESERVE_BOOT_CONFIG, > > - NULL, ACPI_TYPE_INTEGER); > > + &pci_acpi_dsm_guid, > > + 1, DSM_PCI_PRESERVE_BOOT_CONFIG, > > + NULL, ACPI_TYPE_INTEGER); > > if (obj && obj->integer.value == 0) > > - return true; > > + rc = true; > > + ACPI_FREE(obj); > > + } else if (acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), > > + &pci_acpi_dsm_guid, 2, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) { > > + obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), > > + &pci_acpi_dsm_guid, > > + 2, DSM_PCI_PRESERVE_BOOT_CONFIG, > > + NULL, ACPI_TYPE_INTEGER); > > + if (obj && obj->integer.value == 0) > > + rc = true; > > ACPI_FREE(obj); > > } > > > > - return false; > > + return rc; > > } > > > > /* _HPX PCI Setting Record (Type 0); same as _HPP */ > > --
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c index af370628e583..4f9e0548c96d 100644 --- a/drivers/pci/pci-acpi.c +++ b/drivers/pci/pci-acpi.c @@ -122,24 +122,43 @@ phys_addr_t acpi_pci_root_get_mcfg_addr(acpi_handle handle) bool pci_acpi_preserve_config(struct pci_host_bridge *host_bridge) { - if (ACPI_HANDLE(&host_bridge->dev)) { - union acpi_object *obj; + bool rc = false; + union acpi_object *obj; - /* - * Evaluate the "PCI Boot Configuration" _DSM Function. If it - * exists and returns 0, we must preserve any PCI resource - * assignments made by firmware for this host bridge. - */ + if (!ACPI_HANDLE(&host_bridge->dev)) + return false; + + /* + * Evaluate the "PCI Boot Configuration" _DSM Function. If it + * exists and returns 0, we must preserve any PCI resource + * assignments made by firmware for this host bridge. + * + * Per PCI Firmware r3.2, released Jan 26, 2015, + * DSM_PCI_PRESERVE_BOOT_CONFIG Revision ID is 1. But PCI Firmware r3.3, + * released Jan 20, 2021, changed sec 4.6.5 to say + * "lowest valid Revision ID value: 2". So check rev 1 first, then rev 2. + */ + if (acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), + &pci_acpi_dsm_guid, 1, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) { obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), - &pci_acpi_dsm_guid, - 1, DSM_PCI_PRESERVE_BOOT_CONFIG, - NULL, ACPI_TYPE_INTEGER); + &pci_acpi_dsm_guid, + 1, DSM_PCI_PRESERVE_BOOT_CONFIG, + NULL, ACPI_TYPE_INTEGER); if (obj && obj->integer.value == 0) - return true; + rc = true; + ACPI_FREE(obj); + } else if (acpi_check_dsm(ACPI_HANDLE(&host_bridge->dev), + &pci_acpi_dsm_guid, 2, BIT(DSM_PCI_PRESERVE_BOOT_CONFIG))) { + obj = acpi_evaluate_dsm_typed(ACPI_HANDLE(&host_bridge->dev), + &pci_acpi_dsm_guid, + 2, DSM_PCI_PRESERVE_BOOT_CONFIG, + NULL, ACPI_TYPE_INTEGER); + if (obj && obj->integer.value == 0) + rc = true; ACPI_FREE(obj); } - return false; + return rc; } /* _HPX PCI Setting Record (Type 0); same as _HPP */
Per PCI Firmware Specification Revision 3.3 Table 4-7 _DSM Definitions for PCI. Preserve PCI Boot Configuration Initial Revision ID changed to 2. And add acpi_check_dsm() for DSM_PCI_PRESERVE_BOOT_CONFIG. Signed-off-by: Zhou Shengqing <zhoushengqing@ttyinfo.com> --- v5:follow Bjorn advice, add acpi_check_dsm for PCI _DSM. v4:Initialize *obj to NULL. v3:try revision id 1 first, then try revision id 2. v2:add Fixes tag. Fixes: 9d7d5db8e78e ("PCI: Move PRESERVE_BOOT_CONFIG _DSM evaluation to pci_re") --- drivers/pci/pci-acpi.c | 43 ++++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 12 deletions(-)