| Message ID | 20211111095203.961122-1-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | q35: flip acpi-pci-hotplug-with-bridge-support default back to off | expand |
On Thu, Nov 11, 2021 at 10:52:03AM +0100, Gerd Hoffmann wrote: > Switch qemu 6.2 back to 6.0 behavior (aka native pcie hotplug) because > acpi hotplug for pcie ports caused all kinds of regressions and a fix > for those is not in sight. > > Add compat property for 6.1 to keep it enabled there. Use a separate > compat property list so we can apply it to 6.1 only. > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/641 > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2006409 > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Hmm fixes for these two have been actually posted yesterday. In fact both are due to the single issue of disabling HPC capability in the bridge. Are there more issues? If not I'm inclined to go with the fix that Igor posted earlier unless e.g. Igor acks this one. > --- > hw/acpi/ich9.c | 2 +- > hw/i386/pc.c | 1 - > hw/i386/pc_q35.c | 14 +++++++++++++- > 3 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 1ee2ba2c508c..6e7d4c9eb54a 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 = true; > + pm->use_acpi_hotplug_bridge = false; > > 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 2592a821486f..4fed82dafcf0 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -106,7 +106,6 @@ GlobalProperty pc_compat_6_0[] = { > { "qemu64" "-" TYPE_X86_CPU, "model", "6" }, > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" }, > { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" }, > - { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" }, > }; > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0); > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 797e09500b15..735dd3cff4ed 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -375,8 +375,20 @@ static void pc_q35_6_1_machine_options(MachineClass *m) > m->smp_props.prefer_sockets = true; > } > > +/* 6.1 only compat property (not applied to 6.0 + older) */ > +static GlobalProperty pc_compat_6_1_only[] = { > + { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "on" }, > +}; > +static const size_t pc_compat_6_1_only_len = G_N_ELEMENTS(pc_compat_6_1_only); > + > +static void pc_q35_6_1_only_machine_options(MachineClass *m) > +{ > + pc_q35_6_1_machine_options(m); > + compat_props_add(m->compat_props, pc_compat_6_1_only, pc_compat_6_1_only_len); > +} > + > DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL, > - pc_q35_6_1_machine_options); > + pc_q35_6_1_only_machine_options); > > static void pc_q35_6_0_machine_options(MachineClass *m) > { > -- > 2.33.1
On Thu, 11 Nov 2021 10:52:03 +0100 Gerd Hoffmann <kraxel@redhat.com> wrote: > Switch qemu 6.2 back to 6.0 behavior (aka native pcie hotplug) because > acpi hotplug for pcie ports caused all kinds of regressions and a fix > for those is not in sight. > > Add compat property for 6.1 to keep it enabled there. Use a separate > compat property list so we can apply it to 6.1 only. > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/641 > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2006409 > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/acpi/ich9.c | 2 +- > hw/i386/pc.c | 1 - > hw/i386/pc_q35.c | 14 +++++++++++++- > 3 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 1ee2ba2c508c..6e7d4c9eb54a 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 = true; > + pm->use_acpi_hotplug_bridge = false; > > 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 2592a821486f..4fed82dafcf0 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -106,7 +106,6 @@ GlobalProperty pc_compat_6_0[] = { > { "qemu64" "-" TYPE_X86_CPU, "model", "6" }, > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" }, > { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" }, > - { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" }, > }; > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0); > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 797e09500b15..735dd3cff4ed 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -375,8 +375,20 @@ static void pc_q35_6_1_machine_options(MachineClass *m) > m->smp_props.prefer_sockets = true; > } > > +/* 6.1 only compat property (not applied to 6.0 + older) */ > +static GlobalProperty pc_compat_6_1_only[] = { > + { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "on" }, > +}; > +static const size_t pc_compat_6_1_only_len = G_N_ELEMENTS(pc_compat_6_1_only); > + > +static void pc_q35_6_1_only_machine_options(MachineClass *m) > +{ > + pc_q35_6_1_machine_options(m); > + compat_props_add(m->compat_props, pc_compat_6_1_only, pc_compat_6_1_only_len); > +} it works, but not quite the pattern we usually use. I'd prefer enabling it in pc_compat_6_1[] and then turn it off in pc_compat_6_2. It also would be good for downstream as it's less chances to miss compat tweak in usual place than in here. > DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL, > - pc_q35_6_1_machine_options); > + pc_q35_6_1_only_machine_options); > > static void pc_q35_6_0_machine_options(MachineClass *m) > {
On Thu, 11 Nov 2021, Gerd Hoffmann wrote: > Switch qemu 6.2 back to 6.0 behavior (aka native pcie hotplug) because > acpi hotplug for pcie ports caused all kinds of regressions and a fix > for those is not in sight. > > Add compat property for 6.1 to keep it enabled there. Use a separate > compat property list so we can apply it to 6.1 only. I think we are not going this route anymore. ACPI will continue to be the default for 6.2 and future and we will continue to fix the issues on the ACPI hotplug side. I see Michael sent PRs for some of the fixes from Igor and Julia. > > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/641 > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2006409 > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/acpi/ich9.c | 2 +- > hw/i386/pc.c | 1 - > hw/i386/pc_q35.c | 14 +++++++++++++- > 3 files changed, 14 insertions(+), 3 deletions(-) > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c > index 1ee2ba2c508c..6e7d4c9eb54a 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 = true; > + pm->use_acpi_hotplug_bridge = false; > > 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 2592a821486f..4fed82dafcf0 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -106,7 +106,6 @@ GlobalProperty pc_compat_6_0[] = { > { "qemu64" "-" TYPE_X86_CPU, "model", "6" }, > { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" }, > { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" }, > - { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" }, > }; > const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0); > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > index 797e09500b15..735dd3cff4ed 100644 > --- a/hw/i386/pc_q35.c > +++ b/hw/i386/pc_q35.c > @@ -375,8 +375,20 @@ static void pc_q35_6_1_machine_options(MachineClass *m) > m->smp_props.prefer_sockets = true; > } > > +/* 6.1 only compat property (not applied to 6.0 + older) */ > +static GlobalProperty pc_compat_6_1_only[] = { > + { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "on" }, > +}; > +static const size_t pc_compat_6_1_only_len = G_N_ELEMENTS(pc_compat_6_1_only); > + > +static void pc_q35_6_1_only_machine_options(MachineClass *m) > +{ > + pc_q35_6_1_machine_options(m); > + compat_props_add(m->compat_props, pc_compat_6_1_only, pc_compat_6_1_only_len); > +} > + > DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL, > - pc_q35_6_1_machine_options); > + pc_q35_6_1_only_machine_options); > > static void pc_q35_6_0_machine_options(MachineClass *m) > { > -- > 2.33.1 > >
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c index 1ee2ba2c508c..6e7d4c9eb54a 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 = true; + pm->use_acpi_hotplug_bridge = false; 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 2592a821486f..4fed82dafcf0 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -106,7 +106,6 @@ GlobalProperty pc_compat_6_0[] = { { "qemu64" "-" TYPE_X86_CPU, "model", "6" }, { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" }, { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" }, - { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" }, }; const size_t pc_compat_6_0_len = G_N_ELEMENTS(pc_compat_6_0); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 797e09500b15..735dd3cff4ed 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -375,8 +375,20 @@ static void pc_q35_6_1_machine_options(MachineClass *m) m->smp_props.prefer_sockets = true; } +/* 6.1 only compat property (not applied to 6.0 + older) */ +static GlobalProperty pc_compat_6_1_only[] = { + { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "on" }, +}; +static const size_t pc_compat_6_1_only_len = G_N_ELEMENTS(pc_compat_6_1_only); + +static void pc_q35_6_1_only_machine_options(MachineClass *m) +{ + pc_q35_6_1_machine_options(m); + compat_props_add(m->compat_props, pc_compat_6_1_only, pc_compat_6_1_only_len); +} + DEFINE_Q35_MACHINE(v6_1, "pc-q35-6.1", NULL, - pc_q35_6_1_machine_options); + pc_q35_6_1_only_machine_options); static void pc_q35_6_0_machine_options(MachineClass *m) {
Switch qemu 6.2 back to 6.0 behavior (aka native pcie hotplug) because acpi hotplug for pcie ports caused all kinds of regressions and a fix for those is not in sight. Add compat property for 6.1 to keep it enabled there. Use a separate compat property list so we can apply it to 6.1 only. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/641 Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2006409 Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/acpi/ich9.c | 2 +- hw/i386/pc.c | 1 - hw/i386/pc_q35.c | 14 +++++++++++++- 3 files changed, 14 insertions(+), 3 deletions(-)