* Re: [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off
2021-11-11 9:52 [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off Gerd Hoffmann
@ 2021-11-11 18:48 ` Michael S. Tsirkin
2021-11-11 22:17 ` Igor Mammedov
2021-11-16 6:14 ` Ani Sinha
2 siblings, 0 replies; 4+ messages in thread
From: Michael S. Tsirkin @ 2021-11-11 18:48 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Richard Henderson, qemu-devel, Igor Mammedov,
Ani Sinha, Paolo Bonzini
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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off
2021-11-11 9:52 [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off Gerd Hoffmann
2021-11-11 18:48 ` Michael S. Tsirkin
@ 2021-11-11 22:17 ` Igor Mammedov
2021-11-16 6:14 ` Ani Sinha
2 siblings, 0 replies; 4+ messages in thread
From: Igor Mammedov @ 2021-11-11 22:17 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Michael S. Tsirkin, Richard Henderson,
qemu-devel, Ani Sinha, Paolo Bonzini
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)
> {
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off
2021-11-11 9:52 [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off Gerd Hoffmann
2021-11-11 18:48 ` Michael S. Tsirkin
2021-11-11 22:17 ` Igor Mammedov
@ 2021-11-16 6:14 ` Ani Sinha
2 siblings, 0 replies; 4+ messages in thread
From: Ani Sinha @ 2021-11-16 6:14 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Eduardo Habkost, Michael S. Tsirkin, Richard Henderson,
qemu-devel, Igor Mammedov, Ani Sinha, Paolo Bonzini
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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread