[PATCH v4 16/27] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5

Shameer Kolothum posted 27 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 16/27] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5
Posted by Shameer Kolothum 1 month, 2 weeks ago
From: Eric Auger <eric.auger@redhat.com>

Add a 'preserve_config' field in struct GPEXConfig and if set, generate the
DSM #5 for preserving PCI boot configurations. For SMMUV3 accel=on support,
we are making use of IORT RMRs in a subsequent patch and that requires the
DSM #5.

At the moment the DSM generation is not yet enabled.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
[Shameer: Removed possible duplicate _DSM creations]
Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
Previously, QEMU reverted an attempt to enable DSM #5 because it caused a
regression,
https://lore.kernel.org/all/20210724185234.GA2265457@roeck-us.net/.

However, in this series, we enable it selectively, only when SMMUv3 is in
accelerator mode. The devices involved in the earlier regression are not
expected in accelerated SMMUv3 use cases.
---
 hw/pci-host/gpex-acpi.c    | 29 +++++++++++++++++++++++------
 include/hw/pci-host/gpex.h |  1 +
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
index 4587baeb78..e3825ed0b1 100644
--- a/hw/pci-host/gpex-acpi.c
+++ b/hw/pci-host/gpex-acpi.c
@@ -51,10 +51,11 @@ static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq,
     }
 }
 
-static Aml *build_pci_host_bridge_dsm_method(void)
+static Aml *build_pci_host_bridge_dsm_method(bool preserve_config)
 {
     Aml *method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
     Aml *UUID, *ifctx, *ifctx1, *buf;
+    uint8_t byte_list[1] = {0};
 
     /* PCI Firmware Specification 3.0
      * 4.6.1. _DSM for PCI Express Slot Information
@@ -64,10 +65,23 @@ static Aml *build_pci_host_bridge_dsm_method(void)
     UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
     ifctx = aml_if(aml_equal(aml_arg(0), UUID));
     ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
-    uint8_t byte_list[1] = {0};
+    if (preserve_config) {
+        /* support for function 0 and function 5 */
+        byte_list[0] = 0x21;
+    }
     buf = aml_buffer(1, byte_list);
     aml_append(ifctx1, aml_return(buf));
     aml_append(ifctx, ifctx1);
+    if (preserve_config) {
+        Aml *ifctx2 = aml_if(aml_equal(aml_arg(2), aml_int(5)));
+        /*
+         * 0 - The operating system must not ignore the PCI configuration that
+         *     firmware has done at boot time.
+         */
+        aml_append(ifctx2, aml_return(aml_int(0)));
+        aml_append(ifctx, ifctx2);
+    }
+
     aml_append(method, ifctx);
 
     byte_list[0] = 0;
@@ -77,12 +91,13 @@ static Aml *build_pci_host_bridge_dsm_method(void)
 }
 
 static void acpi_dsdt_add_host_bridge_methods(Aml *dev,
-                                              bool enable_native_pcie_hotplug)
+                                              bool enable_native_pcie_hotplug,
+                                              bool preserve_config)
 {
     /* Declare an _OSC (OS Control Handoff) method */
     aml_append(dev,
                build_pci_host_bridge_osc_method(enable_native_pcie_hotplug));
-    aml_append(dev, build_pci_host_bridge_dsm_method());
+    aml_append(dev, build_pci_host_bridge_dsm_method(preserve_config));
 }
 
 void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
@@ -152,7 +167,8 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
                 build_cxl_osc_method(dev);
             } else {
                 /* pxb bridges do not have ACPI PCI Hot-plug enabled */
-                acpi_dsdt_add_host_bridge_methods(dev, true);
+                acpi_dsdt_add_host_bridge_methods(dev, true,
+                                                  cfg->preserve_config);
             }
 
             aml_append(scope, dev);
@@ -227,7 +243,8 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
     }
     aml_append(dev, aml_name_decl("_CRS", rbuf));
 
-    acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
+    acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug,
+                                      cfg->preserve_config);
 
     Aml *dev_res0 = aml_device("%s", "RES0");
     aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
index feaf827474..7eea16e728 100644
--- a/include/hw/pci-host/gpex.h
+++ b/include/hw/pci-host/gpex.h
@@ -46,6 +46,7 @@ struct GPEXConfig {
     int         irq;
     PCIBus      *bus;
     bool        pci_native_hotplug;
+    bool        preserve_config;
 };
 
 typedef struct GPEXIrq GPEXIrq;
-- 
2.43.0
Re: [PATCH v4 16/27] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5
Posted by Eric Auger 2 weeks, 4 days ago
Hi Shameer,

On 9/29/25 3:36 PM, Shameer Kolothum wrote:
> From: Eric Auger <eric.auger@redhat.com>
>
> Add a 'preserve_config' field in struct GPEXConfig and if set, generate the
> DSM #5 for preserving PCI boot configurations. For SMMUV3 accel=on support,
> we are making use of IORT RMRs in a subsequent patch and that requires the
> DSM #5.
>
> At the moment the DSM generation is not yet enabled.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> [Shameer: Removed possible duplicate _DSM creations]
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
> Previously, QEMU reverted an attempt to enable DSM #5 because it caused a
> regression,
> https://lore.kernel.org/all/20210724185234.GA2265457@roeck-us.net/.
Looking at the above link again and especially the list of devices for
which it was failing, it looks reasonable to me to enable that config
along with accel smmu.

Eric
>
> However, in this series, we enable it selectively, only when SMMUv3 is in
> accelerator mode. The devices involved in the earlier regression are not
> expected in accelerated SMMUv3 use cases.
> ---
>  hw/pci-host/gpex-acpi.c    | 29 +++++++++++++++++++++++------
>  include/hw/pci-host/gpex.h |  1 +
>  2 files changed, 24 insertions(+), 6 deletions(-)
>
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 4587baeb78..e3825ed0b1 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -51,10 +51,11 @@ static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq,
>      }
>  }
>  
> -static Aml *build_pci_host_bridge_dsm_method(void)
> +static Aml *build_pci_host_bridge_dsm_method(bool preserve_config)
>  {
>      Aml *method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
>      Aml *UUID, *ifctx, *ifctx1, *buf;
> +    uint8_t byte_list[1] = {0};
>  
>      /* PCI Firmware Specification 3.0
>       * 4.6.1. _DSM for PCI Express Slot Information
> @@ -64,10 +65,23 @@ static Aml *build_pci_host_bridge_dsm_method(void)
>      UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
>      ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>      ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> -    uint8_t byte_list[1] = {0};
> +    if (preserve_config) {
> +        /* support for function 0 and function 5 */
> +        byte_list[0] = 0x21;
> +    }
>      buf = aml_buffer(1, byte_list);
>      aml_append(ifctx1, aml_return(buf));
>      aml_append(ifctx, ifctx1);
> +    if (preserve_config) {
> +        Aml *ifctx2 = aml_if(aml_equal(aml_arg(2), aml_int(5)));
> +        /*
> +         * 0 - The operating system must not ignore the PCI configuration that
> +         *     firmware has done at boot time.
> +         */
> +        aml_append(ifctx2, aml_return(aml_int(0)));
> +        aml_append(ifctx, ifctx2);
> +    }
> +
>      aml_append(method, ifctx);
>  
>      byte_list[0] = 0;
> @@ -77,12 +91,13 @@ static Aml *build_pci_host_bridge_dsm_method(void)
>  }
>  
>  static void acpi_dsdt_add_host_bridge_methods(Aml *dev,
> -                                              bool enable_native_pcie_hotplug)
> +                                              bool enable_native_pcie_hotplug,
> +                                              bool preserve_config)
>  {
>      /* Declare an _OSC (OS Control Handoff) method */
>      aml_append(dev,
>                 build_pci_host_bridge_osc_method(enable_native_pcie_hotplug));
> -    aml_append(dev, build_pci_host_bridge_dsm_method());
> +    aml_append(dev, build_pci_host_bridge_dsm_method(preserve_config));
>  }
>  
>  void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
> @@ -152,7 +167,8 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>                  build_cxl_osc_method(dev);
>              } else {
>                  /* pxb bridges do not have ACPI PCI Hot-plug enabled */
> -                acpi_dsdt_add_host_bridge_methods(dev, true);
> +                acpi_dsdt_add_host_bridge_methods(dev, true,
> +                                                  cfg->preserve_config);
>              }
>  
>              aml_append(scope, dev);
> @@ -227,7 +243,8 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>      }
>      aml_append(dev, aml_name_decl("_CRS", rbuf));
>  
> -    acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
> +    acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug,
> +                                      cfg->preserve_config);
>  
>      Aml *dev_res0 = aml_device("%s", "RES0");
>      aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index feaf827474..7eea16e728 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -46,6 +46,7 @@ struct GPEXConfig {
>      int         irq;
>      PCIBus      *bus;
>      bool        pci_native_hotplug;
> +    bool        preserve_config;
>  };
>  
>  typedef struct GPEXIrq GPEXIrq;
Re: [PATCH v4 16/27] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5
Posted by Jonathan Cameron via 1 month, 2 weeks ago
On Mon, 29 Sep 2025 14:36:32 +0100
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> From: Eric Auger <eric.auger@redhat.com>
> 
> Add a 'preserve_config' field in struct GPEXConfig and if set, generate the
> DSM #5 for preserving PCI boot configurations. For SMMUV3 accel=on support,
> we are making use of IORT RMRs in a subsequent patch and that requires the
> DSM #5.
> 
> At the moment the DSM generation is not yet enabled.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> [Shameer: Removed possible duplicate _DSM creations]
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

Throw an AML blob in the patch description as easier to check that against
the spec.  Add a specific spec reference as well.

> ---
> Previously, QEMU reverted an attempt to enable DSM #5 because it caused a
> regression,
> https://lore.kernel.org/all/20210724185234.GA2265457@roeck-us.net/.
> 
> However, in this series, we enable it selectively, only when SMMUv3 is in
> accelerator mode. The devices involved in the earlier regression are not
> expected in accelerated SMMUv3 use cases.
> ---
>  hw/pci-host/gpex-acpi.c    | 29 +++++++++++++++++++++++------
>  include/hw/pci-host/gpex.h |  1 +
>  2 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 4587baeb78..e3825ed0b1 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -51,10 +51,11 @@ static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq,
>      }
>  }
>  
> -static Aml *build_pci_host_bridge_dsm_method(void)
> +static Aml *build_pci_host_bridge_dsm_method(bool preserve_config)
>  {
>      Aml *method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
>      Aml *UUID, *ifctx, *ifctx1, *buf;
> +    uint8_t byte_list[1] = {0};

The inline declaration is a bit odd, but I'm not seeing a specific reason to
change that here.  Perhaps call out the change as some 'other cleanup' in the
patch description if you want to make it anyway.

>  
>      /* PCI Firmware Specification 3.0
>       * 4.6.1. _DSM for PCI Express Slot Information
> @@ -64,10 +65,23 @@ static Aml *build_pci_host_bridge_dsm_method(void)
>      UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
>      ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>      ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
> -    uint8_t byte_list[1] = {0};
> +    if (preserve_config) {
> +        /* support for function 0 and function 5 */
> +        byte_list[0] = 0x21;

Change the comment to reflect the fix in previous patch as otherwise
it sounds like bit(0) means function 0 is supported.

       /* support functions other than 0, specifically function 5 */

> +    }
>      buf = aml_buffer(1, byte_list);
>      aml_append(ifctx1, aml_return(buf));
>      aml_append(ifctx, ifctx1);
> +    if (preserve_config) {
> +        Aml *ifctx2 = aml_if(aml_equal(aml_arg(2), aml_int(5)));
> +        /*
> +         * 0 - The operating system must not ignore the PCI configuration that
> +         *     firmware has done at boot time.
> +         */
> +        aml_append(ifctx2, aml_return(aml_int(0)));
> +        aml_append(ifctx, ifctx2);
> +    }
> +
>      aml_append(method, ifctx);
>  
>      byte_list[0] = 0;
> @@ -77,12 +91,13 @@ static Aml *build_pci_host_bridge_dsm_method(void)
>  }
Re: [PATCH v4 16/27] hw/pci-host/gpex: Allow to generate preserve boot config DSM #5
Posted by Eric Auger 2 weeks, 4 days ago

On 10/1/25 3:05 PM, Jonathan Cameron wrote:
> On Mon, 29 Sep 2025 14:36:32 +0100
> Shameer Kolothum <skolothumtho@nvidia.com> wrote:
>
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> Add a 'preserve_config' field in struct GPEXConfig and if set, generate the
>> DSM #5 for preserving PCI boot configurations. For SMMUV3 accel=on support,
>> we are making use of IORT RMRs in a subsequent patch and that requires the
>> DSM #5.
>>
>> At the moment the DSM generation is not yet enabled.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> [Shameer: Removed possible duplicate _DSM creations]
>> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Throw an AML blob in the patch description as easier to check that against
> the spec.  Add a specific spec reference as well.
>
>> ---
>> Previously, QEMU reverted an attempt to enable DSM #5 because it caused a
>> regression,
>> https://lore.kernel.org/all/20210724185234.GA2265457@roeck-us.net/.
>>
>> However, in this series, we enable it selectively, only when SMMUv3 is in
>> accelerator mode. The devices involved in the earlier regression are not
>> expected in accelerated SMMUv3 use cases.
>> ---
>>  hw/pci-host/gpex-acpi.c    | 29 +++++++++++++++++++++++------
>>  include/hw/pci-host/gpex.h |  1 +
>>  2 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>> index 4587baeb78..e3825ed0b1 100644
>> --- a/hw/pci-host/gpex-acpi.c
>> +++ b/hw/pci-host/gpex-acpi.c
>> @@ -51,10 +51,11 @@ static void acpi_dsdt_add_pci_route_table(Aml *dev, uint32_t irq,
>>      }
>>  }
>>  
>> -static Aml *build_pci_host_bridge_dsm_method(void)
>> +static Aml *build_pci_host_bridge_dsm_method(bool preserve_config)
>>  {
>>      Aml *method = aml_method("_DSM", 4, AML_NOTSERIALIZED);
>>      Aml *UUID, *ifctx, *ifctx1, *buf;
>> +    uint8_t byte_list[1] = {0};
> The inline declaration is a bit odd, but I'm not seeing a specific reason to
> change that here.  Perhaps call out the change as some 'other cleanup' in the
> patch description if you want to make it anyway.
I think this is a good cleanup anyway.
>
>>  
>>      /* PCI Firmware Specification 3.0
>>       * 4.6.1. _DSM for PCI Express Slot Information
>> @@ -64,10 +65,23 @@ static Aml *build_pci_host_bridge_dsm_method(void)
>>      UUID = aml_touuid("E5C937D0-3553-4D7A-9117-EA4D19C3434D");
>>      ifctx = aml_if(aml_equal(aml_arg(0), UUID));
>>      ifctx1 = aml_if(aml_equal(aml_arg(2), aml_int(0)));
>> -    uint8_t byte_list[1] = {0};
>> +    if (preserve_config) {
>> +        /* support for function 0 and function 5 */
>> +        byte_list[0] = 0x21;
> Change the comment to reflect the fix in previous patch as otherwise
> it sounds like bit(0) means function 0 is supported.
>
>        /* support functions other than 0, specifically function 5 */

yeah I was confused too after all this time. Maybe quote the ACPI spec
at least in the commit msg:

"
If Function Index is zero, the return is a buffer containing one bit for
each function index, starting with zero. Bit 0 indicates whether there
is support for any functions other than function 0 for the specified
UUID and Revision ID. If set to zero, no functions are supported (other
than function zero) for the specified UUID and Revision ID. If set to
one, at least one additional function is supported. For all other bits
in the buffer, a bit is set to zero to indicate if that function index
is not supported for the specific UUID and Revision ID. (For example,
bit 1 set to 0 indicates that function index 1 is not supported for the
specific UUID and Revision ID.)
"

Eric


>
>> +    }
>>      buf = aml_buffer(1, byte_list);
>>      aml_append(ifctx1, aml_return(buf));
>>      aml_append(ifctx, ifctx1);
>> +    if (preserve_config) {
>> +        Aml *ifctx2 = aml_if(aml_equal(aml_arg(2), aml_int(5)));
>> +        /*
>> +         * 0 - The operating system must not ignore the PCI configuration that
>> +         *     firmware has done at boot time.
>> +         */
>> +        aml_append(ifctx2, aml_return(aml_int(0)));
>> +        aml_append(ifctx, ifctx2);
>> +    }
>> +
>>      aml_append(method, ifctx);
>>  
>>      byte_list[0] = 0;
>> @@ -77,12 +91,13 @@ static Aml *build_pci_host_bridge_dsm_method(void)
>>  }