Re: [PATCH v5 1/2] Add rev 2 check for PRESERVE_BOOT_CONFIG function

[Rafael J. Wysocki]

On Thu, Mar 13, 2025 at 11:07 AM Zhou Shengqing
<zhoushengqing@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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 */
> --