Re: [PATCH v6 5/6] hw/acpi/ich9: Set ACPI PCI hot-plug as default on Q35

[Michael S. Tsirkin]

On Tue, Jul 13, 2021 at 09:59:31AM +0200, Igor Mammedov wrote:
> On Tue, 13 Jul 2021 02:42:04 +0200
> Julia Suvorova <jusual@redhat.com> wrote:
> 
> > Q35 has three different types of PCI devices hot-plug: PCIe Native,
> > SHPC Native and ACPI hot-plug. This patch changes the default choice
> > for cold-plugged bridges from PCIe Native to ACPI Hot-plug with
> > ability to use SHPC and PCIe Native for hot-plugged bridges.
> 
> Before we flip the switch,
> has the issue about not hotplug ports not getting IO (Michael)
> been addressed, if not are there any plans to fix it?
> 

I think it's a guest bug frankly. We'll workaround it
by setting io-reserve to 4k for hotplugged bridges,
I think this is minor enough that it's better to just
merge now and fix on top.
I've added this note to the commit log though.

> > This is a list of the PCIe Native hot-plug issues that led to this
> > change:
> >     * no racy behavior during boot (see 110c477c2ed)
> >     * no delay during deleting - after the actual power off software
> >       must wait at least 1 second before indicating about it. This case
> >       is quite important for users, it even has its own bug:
> >           https://bugzilla.redhat.com/show_bug.cgi?id=1594168
> >     * no timer-based behavior - in addition to the previous example,
> >       the attention button has a 5-second waiting period, during which
> >       the operation can be canceled with a second press. While this
> >       looks fine for manual button control, automation will result in
> >       the need to queue or drop events, and the software receiving
> >       events in all sort of unspecified combinations of attention/power
> >       indicator states, which is racy and uppredictable.
> >     * fixes:
> >         * https://bugzilla.redhat.com/show_bug.cgi?id=1752465
> >         * https://bugzilla.redhat.com/show_bug.cgi?id=1690256
> > 
> > To return to PCIe Native hot-plug:
> >     -global ICH9-LPC.acpi-pci-hotplug-with-bridge-support=off
> > 
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/acpi/ich9.c | 2 +-
> >  hw/i386/pc.c   | 1 +
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 2f4eb453ac..778e27b659 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -427,7 +427,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
> > *pm)
> >      pm->disable_s3 = 0;
> >      pm->disable_s4 = 0;
> >      pm->s4_val = 2;
> > -    pm->use_acpi_hotplug_bridge = false;
> > +    pm->use_acpi_hotplug_bridge = true;
> >  
> >      object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
> >                                     &pm->pm_io_base, OBJ_PROP_FLAG_READ);
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 8e1220db72..7e03848792 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_0[] = {
> >      { "qemu64" "-" TYPE_X86_CPU, "family", "6" },
> >      { "qemu64" "-" TYPE_X86_CPU, "model", "6" },
> >      { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> > +    { "ICH9-LPC", "acpi-pci-hotplug-with-bridge-support", "off" },
> >  };
> >  const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0);
> >