[PATCH v3 02/29] hw/acpi/ged: Add a acpi-pci-hotplug-with-bridge-support property

Eric Auger posted 29 patches 5 months ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Ani Sinha <anisinha@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Shannon Zhao <shannon.zhaosl@gmail.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Richard Henderson <richard.henderson@linaro.org>
There is a newer version of this series
[PATCH v3 02/29] hw/acpi/ged: Add a acpi-pci-hotplug-with-bridge-support property
Posted by Eric Auger 5 months ago
A new boolean property is introduced. This will be used to turn
ACPI PCI hotplug support. By default it is unset.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/hw/acpi/generic_event_device.h | 2 ++
 hw/acpi/generic_event_device.c         | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
index d2dac87b4a..f5ffa67a39 100644
--- a/include/hw/acpi/generic_event_device.h
+++ b/include/hw/acpi/generic_event_device.h
@@ -63,6 +63,7 @@
 #include "hw/acpi/memory_hotplug.h"
 #include "hw/acpi/ghes.h"
 #include "hw/acpi/cpu.h"
+#include "hw/acpi/pcihp.h"
 #include "qom/object.h"
 
 #define ACPI_POWER_BUTTON_DEVICE "PWRB"
@@ -114,6 +115,7 @@ struct AcpiGedState {
     MemoryRegion container_memhp;
     CPUHotplugState cpuhp_state;
     MemoryRegion container_cpuhp;
+    AcpiPciHpState pcihp_state;
     GEDState ged_state;
     uint32_t ged_event_bitmap;
     qemu_irq irq;
diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
index 7a62f8d5bc..7831db412b 100644
--- a/hw/acpi/generic_event_device.c
+++ b/hw/acpi/generic_event_device.c
@@ -318,6 +318,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
 
 static const Property acpi_ged_properties[] = {
     DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
+    DEFINE_PROP_BOOL(ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, AcpiGedState,
+                     pcihp_state.use_acpi_hotplug_bridge, 0),
 };
 
 static const VMStateDescription vmstate_memhp_state = {
-- 
2.49.0
Re: [PATCH v3 02/29] hw/acpi/ged: Add a acpi-pci-hotplug-with-bridge-support property
Posted by Jonathan Cameron via 4 months, 4 weeks ago
On Mon, 16 Jun 2025 11:46:31 +0200
Eric Auger <eric.auger@redhat.com> wrote:

> A new boolean property is introduced. This will be used to turn
> ACPI PCI hotplug support. By default it is unset.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>

I wonder a bit if it makes sense to do this so early rather than just
before it is first used in the series?  Doesn't really matter though.
Just meant I read on a bit before giving an RB on this.

Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  include/hw/acpi/generic_event_device.h | 2 ++
>  hw/acpi/generic_event_device.c         | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> index d2dac87b4a..f5ffa67a39 100644
> --- a/include/hw/acpi/generic_event_device.h
> +++ b/include/hw/acpi/generic_event_device.h
> @@ -63,6 +63,7 @@
>  #include "hw/acpi/memory_hotplug.h"
>  #include "hw/acpi/ghes.h"
>  #include "hw/acpi/cpu.h"
> +#include "hw/acpi/pcihp.h"
>  #include "qom/object.h"
>  
>  #define ACPI_POWER_BUTTON_DEVICE "PWRB"
> @@ -114,6 +115,7 @@ struct AcpiGedState {
>      MemoryRegion container_memhp;
>      CPUHotplugState cpuhp_state;
>      MemoryRegion container_cpuhp;
> +    AcpiPciHpState pcihp_state;
>      GEDState ged_state;
>      uint32_t ged_event_bitmap;
>      qemu_irq irq;
> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> index 7a62f8d5bc..7831db412b 100644
> --- a/hw/acpi/generic_event_device.c
> +++ b/hw/acpi/generic_event_device.c
> @@ -318,6 +318,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
>  
>  static const Property acpi_ged_properties[] = {
>      DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
> +    DEFINE_PROP_BOOL(ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, AcpiGedState,
> +                     pcihp_state.use_acpi_hotplug_bridge, 0),
>  };
>  
>  static const VMStateDescription vmstate_memhp_state = {
Re: [PATCH v3 02/29] hw/acpi/ged: Add a acpi-pci-hotplug-with-bridge-support property
Posted by Igor Mammedov 4 months, 4 weeks ago
On Fri, 20 Jun 2025 09:53:08 +0100
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> On Mon, 16 Jun 2025 11:46:31 +0200
> Eric Auger <eric.auger@redhat.com> wrote:
> 
> > A new boolean property is introduced. This will be used to turn
> > ACPI PCI hotplug support. By default it is unset.
> > 
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>  
> 
> I wonder a bit if it makes sense to do this so early rather than just
> before it is first used in the series?  Doesn't really matter though.
> Just meant I read on a bit before giving an RB on this.

Just before would be better, but it doesn't really matter.
I guess Eric can rearrange that if there would be need to respin.

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> 
> > ---
> >  include/hw/acpi/generic_event_device.h | 2 ++
> >  hw/acpi/generic_event_device.c         | 2 ++
> >  2 files changed, 4 insertions(+)
> > 
> > diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
> > index d2dac87b4a..f5ffa67a39 100644
> > --- a/include/hw/acpi/generic_event_device.h
> > +++ b/include/hw/acpi/generic_event_device.h
> > @@ -63,6 +63,7 @@
> >  #include "hw/acpi/memory_hotplug.h"
> >  #include "hw/acpi/ghes.h"
> >  #include "hw/acpi/cpu.h"
> > +#include "hw/acpi/pcihp.h"
> >  #include "qom/object.h"
> >  
> >  #define ACPI_POWER_BUTTON_DEVICE "PWRB"
> > @@ -114,6 +115,7 @@ struct AcpiGedState {
> >      MemoryRegion container_memhp;
> >      CPUHotplugState cpuhp_state;
> >      MemoryRegion container_cpuhp;
> > +    AcpiPciHpState pcihp_state;
> >      GEDState ged_state;
> >      uint32_t ged_event_bitmap;
> >      qemu_irq irq;
> > diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
> > index 7a62f8d5bc..7831db412b 100644
> > --- a/hw/acpi/generic_event_device.c
> > +++ b/hw/acpi/generic_event_device.c
> > @@ -318,6 +318,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
> >  
> >  static const Property acpi_ged_properties[] = {
> >      DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
> > +    DEFINE_PROP_BOOL(ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, AcpiGedState,
> > +                     pcihp_state.use_acpi_hotplug_bridge, 0),
> >  };
> >  
> >  static const VMStateDescription vmstate_memhp_state = {  
>
Re: [PATCH v3 02/29] hw/acpi/ged: Add a acpi-pci-hotplug-with-bridge-support property
Posted by Eric Auger 4 months, 4 weeks ago
Hi Jonathan, Igor,

On 6/20/25 1:09 PM, Igor Mammedov wrote:
> On Fri, 20 Jun 2025 09:53:08 +0100
> Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:
>
>> On Mon, 16 Jun 2025 11:46:31 +0200
>> Eric Auger <eric.auger@redhat.com> wrote:
>>
>>> A new boolean property is introduced. This will be used to turn
>>> ACPI PCI hotplug support. By default it is unset.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>  
>> I wonder a bit if it makes sense to do this so early rather than just
>> before it is first used in the series?  Doesn't really matter though.
>> Just meant I read on a bit before giving an RB on this.
> Just before would be better, but it doesn't really matter.
> I guess Eric can rearrange that if there would be need to respin.
Yes I can definitively rearrange the order.

Thank you for the review

Eric
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>>
>>> ---
>>>  include/hw/acpi/generic_event_device.h | 2 ++
>>>  hw/acpi/generic_event_device.c         | 2 ++
>>>  2 files changed, 4 insertions(+)
>>>
>>> diff --git a/include/hw/acpi/generic_event_device.h b/include/hw/acpi/generic_event_device.h
>>> index d2dac87b4a..f5ffa67a39 100644
>>> --- a/include/hw/acpi/generic_event_device.h
>>> +++ b/include/hw/acpi/generic_event_device.h
>>> @@ -63,6 +63,7 @@
>>>  #include "hw/acpi/memory_hotplug.h"
>>>  #include "hw/acpi/ghes.h"
>>>  #include "hw/acpi/cpu.h"
>>> +#include "hw/acpi/pcihp.h"
>>>  #include "qom/object.h"
>>>  
>>>  #define ACPI_POWER_BUTTON_DEVICE "PWRB"
>>> @@ -114,6 +115,7 @@ struct AcpiGedState {
>>>      MemoryRegion container_memhp;
>>>      CPUHotplugState cpuhp_state;
>>>      MemoryRegion container_cpuhp;
>>> +    AcpiPciHpState pcihp_state;
>>>      GEDState ged_state;
>>>      uint32_t ged_event_bitmap;
>>>      qemu_irq irq;
>>> diff --git a/hw/acpi/generic_event_device.c b/hw/acpi/generic_event_device.c
>>> index 7a62f8d5bc..7831db412b 100644
>>> --- a/hw/acpi/generic_event_device.c
>>> +++ b/hw/acpi/generic_event_device.c
>>> @@ -318,6 +318,8 @@ static void acpi_ged_send_event(AcpiDeviceIf *adev, AcpiEventStatusBits ev)
>>>  
>>>  static const Property acpi_ged_properties[] = {
>>>      DEFINE_PROP_UINT32("ged-event", AcpiGedState, ged_event_bitmap, 0),
>>> +    DEFINE_PROP_BOOL(ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, AcpiGedState,
>>> +                     pcihp_state.use_acpi_hotplug_bridge, 0),
>>>  };
>>>  
>>>  static const VMStateDescription vmstate_memhp_state = {