Re: [PATCH] q35: flip acpi-pci-hotplug-with-bridge-support default back to off

[Igor Mammedov]

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)
>  {