| Message ID | 20210713004205.775386-6-jusual@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Use ACPI PCI hot-plug for Q35 | expand |
On Tue, Jul 13, 2021 at 02:42:04AM +0200, Julia Suvorova 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. > > 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> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > 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); >
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? > 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); >
Hi Igor, On Tue, Jul 13, 2021 at 10:59 AM Igor Mammedov <imammedo@redhat.com> 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? > > Following an offline discussion with Michael the understanding is PCIe devices should work correctly without IO, so it only affects PCI devices hot-plugged into PCIe ports, but those devices should be plugged into pcie-pci bridges. Another alternative is to drop this patch and let ACPI hotplug be an option and not the default until we find a solution, meaning delaying the flip until 6.2 since today is the soft-freeze. I will let Michael decide, I am fine one way or the other. Thanks, Marcel > > > 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); > > > >
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); > >
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);
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. 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(-)