Propagate the type of pci hotplug mode downto the gpex
acpi code. In case machine acpi_pcihp is unset we configure
pci native hotplug on pci0. For expander bridges we keep
legacy pci native hotplug, as done on x86 q35.
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
---
include/hw/pci-host/gpex.h | 1 +
hw/arm/virt-acpi-build.c | 1 +
hw/pci-host/gpex-acpi.c | 3 ++-
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
index 84471533af..feaf827474 100644
--- a/include/hw/pci-host/gpex.h
+++ b/include/hw/pci-host/gpex.h
@@ -45,6 +45,7 @@ struct GPEXConfig {
MemMapEntry pio;
int irq;
PCIBus *bus;
+ bool pci_native_hotplug;
};
typedef struct GPEXIrq GPEXIrq;
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 7e8e0f0298..be5e00a56e 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -129,6 +129,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
.ecam = memmap[ecam_id],
.irq = irq,
.bus = vms->bus,
+ .pci_native_hotplug = !vms->acpi_pcihp,
};
if (vms->highmem_mmio) {
diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
index 1aa2d12026..f1ab30f3d5 100644
--- a/hw/pci-host/gpex-acpi.c
+++ b/hw/pci-host/gpex-acpi.c
@@ -204,6 +204,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
if (is_cxl) {
build_cxl_osc_method(dev);
} else {
+ /* pxb bridges do not have ACPI PCI Hot-plug enabled */
acpi_dsdt_add_host_bridge_methods(dev, true);
}
@@ -279,7 +280,7 @@ 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, true);
+ acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
Aml *dev_res0 = aml_device("%s", "RES0");
aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
--
2.49.0On Tue, 27 May 2025 09:40:08 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> Propagate the type of pci hotplug mode downto the gpex
> acpi code. In case machine acpi_pcihp is unset we configure
> pci native hotplug on pci0. For expander bridges we keep
> legacy pci native hotplug, as done on x86 q35.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> include/hw/pci-host/gpex.h | 1 +
> hw/arm/virt-acpi-build.c | 1 +
> hw/pci-host/gpex-acpi.c | 3 ++-
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index 84471533af..feaf827474 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -45,6 +45,7 @@ struct GPEXConfig {
> MemMapEntry pio;
> int irq;
> PCIBus *bus;
> + bool pci_native_hotplug;
> };
>
> typedef struct GPEXIrq GPEXIrq;
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7e8e0f0298..be5e00a56e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -129,6 +129,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> .ecam = memmap[ecam_id],
> .irq = irq,
> .bus = vms->bus,
> + .pci_native_hotplug = !vms->acpi_pcihp,
> };
>
> if (vms->highmem_mmio) {
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 1aa2d12026..f1ab30f3d5 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -204,6 +204,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
> if (is_cxl) {
> build_cxl_osc_method(dev);
> } else {
> + /* pxb bridges do not have ACPI PCI Hot-plug enabled */
> acpi_dsdt_add_host_bridge_methods(dev, true);
This is awkward but explains why my CXL cases weren't causing trouble.
A mixed config is counter to the recommendation in the PCI firmware spec
"It is recommended that a machine with multiple host bridge devices should
report the same capabilities for all host bridges of the same type and also
negotiate control of the features described in the Control Field in the
same way for all host bridges of the same type"
I guess if any OS isn't coping with the mix then they can request native
hotplug.
> }
>
> @@ -279,7 +280,7 @@ 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, true);
> + acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
>
> Aml *dev_res0 = aml_device("%s", "RES0");
> aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
Hi Jonathan,
On 5/30/25 12:14 PM, Jonathan Cameron wrote:
> On Tue, 27 May 2025 09:40:08 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Propagate the type of pci hotplug mode downto the gpex
>> acpi code. In case machine acpi_pcihp is unset we configure
>> pci native hotplug on pci0. For expander bridges we keep
>> legacy pci native hotplug, as done on x86 q35.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>> include/hw/pci-host/gpex.h | 1 +
>> hw/arm/virt-acpi-build.c | 1 +
>> hw/pci-host/gpex-acpi.c | 3 ++-
>> 3 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>> index 84471533af..feaf827474 100644
>> --- a/include/hw/pci-host/gpex.h
>> +++ b/include/hw/pci-host/gpex.h
>> @@ -45,6 +45,7 @@ struct GPEXConfig {
>> MemMapEntry pio;
>> int irq;
>> PCIBus *bus;
>> + bool pci_native_hotplug;
>> };
>>
>> typedef struct GPEXIrq GPEXIrq;
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 7e8e0f0298..be5e00a56e 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -129,6 +129,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>> .ecam = memmap[ecam_id],
>> .irq = irq,
>> .bus = vms->bus,
>> + .pci_native_hotplug = !vms->acpi_pcihp,
>> };
>>
>> if (vms->highmem_mmio) {
>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>> index 1aa2d12026..f1ab30f3d5 100644
>> --- a/hw/pci-host/gpex-acpi.c
>> +++ b/hw/pci-host/gpex-acpi.c
>> @@ -204,6 +204,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>> if (is_cxl) {
>> build_cxl_osc_method(dev);
>> } else {
>> + /* pxb bridges do not have ACPI PCI Hot-plug enabled */
>> acpi_dsdt_add_host_bridge_methods(dev, true);
> This is awkward but explains why my CXL cases weren't causing trouble.
> A mixed config is counter to the recommendation in the PCI firmware spec
>
> "It is recommended that a machine with multiple host bridge devices should
> report the same capabilities for all host bridges of the same type and also
> negotiate control of the features described in the Control Field in the
> same way for all host bridges of the same type"
Thank for pointing to this spec excerpt.
Maybe the nuance relates to "host bridges of the same type", ie. gpex
versus pxb?
refering to Igor's following reply i will leave it as is. This is
inherited from existing x86 code in
hw/i386/acpi-build.c build_dsdt():
/* Expander bridges do not have ACPI PCI Hot-plug enabled */
aml_append(dev, build_q35_osc_method(true));
Eric
>
> I guess if any OS isn't coping with the mix then they can request native
> hotplug.
>
>
>
>> }
>>
>> @@ -279,7 +280,7 @@ 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, true);
>> + acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
>>
>> Aml *dev_res0 = aml_device("%s", "RES0");
>> aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
On Fri, 30 May 2025 11:14:13 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
> On Tue, 27 May 2025 09:40:08 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
> > Propagate the type of pci hotplug mode downto the gpex
> > acpi code. In case machine acpi_pcihp is unset we configure
> > pci native hotplug on pci0. For expander bridges we keep
> > legacy pci native hotplug, as done on x86 q35.
> >
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
> > ---
> > include/hw/pci-host/gpex.h | 1 +
> > hw/arm/virt-acpi-build.c | 1 +
> > hw/pci-host/gpex-acpi.c | 3 ++-
> > 3 files changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> > index 84471533af..feaf827474 100644
> > --- a/include/hw/pci-host/gpex.h
> > +++ b/include/hw/pci-host/gpex.h
> > @@ -45,6 +45,7 @@ struct GPEXConfig {
> > MemMapEntry pio;
> > int irq;
> > PCIBus *bus;
> > + bool pci_native_hotplug;
> > };
> >
> > typedef struct GPEXIrq GPEXIrq;
> > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> > index 7e8e0f0298..be5e00a56e 100644
> > --- a/hw/arm/virt-acpi-build.c
> > +++ b/hw/arm/virt-acpi-build.c
> > @@ -129,6 +129,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> > .ecam = memmap[ecam_id],
> > .irq = irq,
> > .bus = vms->bus,
> > + .pci_native_hotplug = !vms->acpi_pcihp,
> > };
> >
> > if (vms->highmem_mmio) {
> > diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> > index 1aa2d12026..f1ab30f3d5 100644
> > --- a/hw/pci-host/gpex-acpi.c
> > +++ b/hw/pci-host/gpex-acpi.c
> > @@ -204,6 +204,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
> > if (is_cxl) {
> > build_cxl_osc_method(dev);
> > } else {
> > + /* pxb bridges do not have ACPI PCI Hot-plug enabled */
> > acpi_dsdt_add_host_bridge_methods(dev, true);
>
> This is awkward but explains why my CXL cases weren't causing trouble.
> A mixed config is counter to the recommendation in the PCI firmware spec
>
> "It is recommended that a machine with multiple host bridge devices should
> report the same capabilities for all host bridges of the same type and also
> negotiate control of the features described in the Control Field in the
> same way for all host bridges of the same type"
>
> I guess if any OS isn't coping with the mix then they can request native
> hotplug.
guest should be able to cope with mixed configs,
ACPI pcihp is a crutch and works only for coldplugged bridges,
while hotplugged bridges are handled by native hotplug.
>
>
>
> > }
> >
> > @@ -279,7 +280,7 @@ 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, true);
> > + acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
> >
> > Aml *dev_res0 = aml_device("%s", "RES0");
> > aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
>
On Tue, 27 May 2025 09:40:08 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> Propagate the type of pci hotplug mode downto the gpex
> acpi code. In case machine acpi_pcihp is unset we configure
> pci native hotplug on pci0. For expander bridges we keep
> legacy pci native hotplug, as done on x86 q35.
this is not needed if done as suggested in 2/25
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> include/hw/pci-host/gpex.h | 1 +
> hw/arm/virt-acpi-build.c | 1 +
> hw/pci-host/gpex-acpi.c | 3 ++-
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index 84471533af..feaf827474 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -45,6 +45,7 @@ struct GPEXConfig {
> MemMapEntry pio;
> int irq;
> PCIBus *bus;
> + bool pci_native_hotplug;
> };
>
> typedef struct GPEXIrq GPEXIrq;
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 7e8e0f0298..be5e00a56e 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -129,6 +129,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
> .ecam = memmap[ecam_id],
> .irq = irq,
> .bus = vms->bus,
> + .pci_native_hotplug = !vms->acpi_pcihp,
> };
>
> if (vms->highmem_mmio) {
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 1aa2d12026..f1ab30f3d5 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -204,6 +204,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
> if (is_cxl) {
> build_cxl_osc_method(dev);
> } else {
> + /* pxb bridges do not have ACPI PCI Hot-plug enabled */
> acpi_dsdt_add_host_bridge_methods(dev, true);
> }
>
> @@ -279,7 +280,7 @@ 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, true);
> + acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
>
> Aml *dev_res0 = aml_device("%s", "RES0");
> aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
Hi Igor,
On 5/27/25 2:33 PM, Igor Mammedov wrote:
> On Tue, 27 May 2025 09:40:08 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> Propagate the type of pci hotplug mode downto the gpex
>> acpi code. In case machine acpi_pcihp is unset we configure
>> pci native hotplug on pci0. For expander bridges we keep
>> legacy pci native hotplug, as done on x86 q35.
> this is not needed if done as suggested in 2/25
So since the property would be applied to the GED device we will still
need a patch that retrieves the GED setting and propagate it to the GPEX
device.
Thanks
Eric
>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>> include/hw/pci-host/gpex.h | 1 +
>> hw/arm/virt-acpi-build.c | 1 +
>> hw/pci-host/gpex-acpi.c | 3 ++-
>> 3 files changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
>> index 84471533af..feaf827474 100644
>> --- a/include/hw/pci-host/gpex.h
>> +++ b/include/hw/pci-host/gpex.h
>> @@ -45,6 +45,7 @@ struct GPEXConfig {
>> MemMapEntry pio;
>> int irq;
>> PCIBus *bus;
>> + bool pci_native_hotplug;
>> };
>>
>> typedef struct GPEXIrq GPEXIrq;
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 7e8e0f0298..be5e00a56e 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -129,6 +129,7 @@ static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap,
>> .ecam = memmap[ecam_id],
>> .irq = irq,
>> .bus = vms->bus,
>> + .pci_native_hotplug = !vms->acpi_pcihp,
>> };
>>
>> if (vms->highmem_mmio) {
>> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
>> index 1aa2d12026..f1ab30f3d5 100644
>> --- a/hw/pci-host/gpex-acpi.c
>> +++ b/hw/pci-host/gpex-acpi.c
>> @@ -204,6 +204,7 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg)
>> if (is_cxl) {
>> build_cxl_osc_method(dev);
>> } else {
>> + /* pxb bridges do not have ACPI PCI Hot-plug enabled */
>> acpi_dsdt_add_host_bridge_methods(dev, true);
>> }
>>
>> @@ -279,7 +280,7 @@ 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, true);
>> + acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
>>
>> Aml *dev_res0 = aml_device("%s", "RES0");
>> aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
>
On Wed, 11 Jun 2025 11:00:02 +0200
Eric Auger <eric.auger@redhat.com> wrote:
> Hi Igor,
>
> On 5/27/25 2:33 PM, Igor Mammedov wrote:
> > On Tue, 27 May 2025 09:40:08 +0200
> > Eric Auger <eric.auger@redhat.com> wrote:
> >
> >> Propagate the type of pci hotplug mode downto the gpex
> >> acpi code. In case machine acpi_pcihp is unset we configure
> >> pci native hotplug on pci0. For expander bridges we keep
> >> legacy pci native hotplug, as done on x86 q35.
> > this is not needed if done as suggested in 2/25
> So since the property would be applied to the GED device we will still
> need a patch that retrieves the GED setting and propagate it to the GPEX
> device.
I don't see a need for propagating it to GPEX device itself for ACPI sake.
For ACPI code, what we do in q35 case is fetching
pm->pcihp_bridge_en from acpi_dev(whatever it is) and then
passing it down trough call-chain
build_dsdt->build_q35_osc_method
the only hack that touches GPEX device model, is the need to
wire up root bus to GED, when GED is being created.
So that GED would be able to swap hotplug handler on
an acpi specific one when configured to do so.
> Thanks
>
> Eric
> >
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> Reviewed-by: Gustavo Romero <gustavo.romero@linaro.org>
[...]
> >> @@ -279,7 +280,7 @@ 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, true);
> >> + acpi_dsdt_add_host_bridge_methods(dev, cfg->pci_native_hotplug);
also, having sufficiently forgotten this series context.
It might be better to abandon idea of consolidating osc and dsm methods
in acpi_dsdt_add_host_bridge_methods().
Just looking at above is confusing and forces one to re-rear previous
commits to figure out what acpi_dsdt_add_host_bridge_methods() is doing.
having here separate sequence of
add_osc()
add_dsm()
would be much more readable.
Lets drop acpi_dsdt_add_host_bridge_methods() helper.
> >>
> >> Aml *dev_res0 = aml_device("%s", "RES0");
> >> aml_append(dev_res0, aml_name_decl("_HID", aml_string("PNP0C02")));
> >
>
© 2016 - 2025 Red Hat, Inc.