qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for 6.2 v2 2/5] hw/acpi/ich9: Add compat prop to keep HPC bit


From: Ani Sinha
Subject: Re: [PATCH for 6.2 v2 2/5] hw/acpi/ich9: Add compat prop to keep HPC bit set for 6.1 machine type
Date: Tue, 16 Nov 2021 13:32:10 +0530

On Mon, Nov 15, 2021 at 3:35 PM Ani Sinha <ani@anisinha.ca> wrote:
>
>
>
> On Wed, 10 Nov 2021, Igor Mammedov wrote:
>
> > From: Julia Suvorova <jusual@redhat.com>
> >
> > To solve issues [1-2] the Hot Plug Capable bit in PCIe Slots will be
> > turned on, while the switch to ACPI Hot-plug will be done in the
> > DSDT table.
> >
> > Introducing 'x-keep-native-hpc' property disables the HPC bit only
> > in 6.1 and as a result keeps the forced 'reserve-io' on
> > pcie-root-ports in 6.1 too.
> >
> > [1] https://gitlab.com/qemu-project/qemu/-/issues/641
> > [2] https://bugzilla.redhat.com/show_bug.cgi?id=2006409
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>
> This patch is a little bit hard to read ... but ...
>
> > ---
> > v2:
> >    * s/native-hpc-bit/x-native-hotplug/ to fix conflict
> > ---
> >  include/hw/acpi/ich9.h |  1 +
> >  hw/acpi/ich9.c         | 18 ++++++++++++++++++
> >  hw/i386/pc.c           |  2 ++
> >  hw/i386/pc_q35.c       |  7 ++++++-
> >  4 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> > index f04f1791bd..7ca92843c6 100644
> > --- a/include/hw/acpi/ich9.h
> > +++ b/include/hw/acpi/ich9.h
> > @@ -56,6 +56,7 @@ typedef struct ICH9LPCPMRegs {
> >      AcpiCpuHotplug gpe_cpu;
> >      CPUHotplugState cpuhp_state;
> >
> > +    bool keep_pci_slot_hpc;
> >      bool use_acpi_hotplug_bridge;
> >      AcpiPciHpState acpi_pci_hotplug;
> >      MemHotplugState acpi_memory_hotplug;
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 1ee2ba2c50..ebe08ed831 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -419,6 +419,20 @@ static void ich9_pm_set_acpi_pci_hotplug(Object *obj, 
> > bool value, Error **errp)
> >      s->pm.use_acpi_hotplug_bridge = value;
> >  }
> >
> > +static bool ich9_pm_get_keep_pci_slot_hpc(Object *obj, Error **errp)
> > +{
> > +    ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> > +
> > +    return s->pm.keep_pci_slot_hpc;
> > +}
> > +
> > +static void ich9_pm_set_keep_pci_slot_hpc(Object *obj, bool value, Error 
> > **errp)
> > +{
> > +    ICH9LPCState *s = ICH9_LPC_DEVICE(obj);
> > +
> > +    s->pm.keep_pci_slot_hpc = value;
> > +}
> > +
> >  void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs *pm)
> >  {
> >      static const uint32_t gpe0_len = ICH9_PMIO_GPE0_LEN;
> > @@ -428,6 +442,7 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
> > *pm)
> >      pm->disable_s4 = 0;
> >      pm->s4_val = 2;
> >      pm->use_acpi_hotplug_bridge = true;
> > +    pm->keep_pci_slot_hpc = true;
> >
> >      object_property_add_uint32_ptr(obj, ACPI_PM_PROP_PM_IO_BASE,
> >                                     &pm->pm_io_base, OBJ_PROP_FLAG_READ);
> > @@ -454,6 +469,9 @@ void ich9_pm_add_properties(Object *obj, ICH9LPCPMRegs 
> > *pm)
> >      object_property_add_bool(obj, ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
> >                               ich9_pm_get_acpi_pci_hotplug,
> >                               ich9_pm_set_acpi_pci_hotplug);
> > +    object_property_add_bool(obj, "x-keep-pci-slot-hpc",
> > +                             ich9_pm_get_keep_pci_slot_hpc,
> > +                             ich9_pm_set_keep_pci_slot_hpc);
> >  }
> >
> >  void ich9_pm_device_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState 
> > *dev,
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 2592a82148..a2ef40ecbc 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -98,6 +98,7 @@ GlobalProperty pc_compat_6_1[] = {
> >      { TYPE_X86_CPU, "hv-version-id-build", "0x1bbc" },
> >      { TYPE_X86_CPU, "hv-version-id-major", "0x0006" },
> >      { TYPE_X86_CPU, "hv-version-id-minor", "0x0001" },
> > +    { "ICH9-LPC", "x-keep-pci-slot-hpc", "false" },
> >  };
> >  const size_t pc_compat_6_1_len = G_N_ELEMENTS(pc_compat_6_1);
> >
> > @@ -107,6 +108,7 @@ GlobalProperty pc_compat_6_0[] = {
> >      { "qemu64" "-" TYPE_X86_CPU, "stepping", "3" },
> >      { TYPE_X86_CPU, "x-vendor-cpuid-only", "off" },
> >      { "ICH9-LPC", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" },
> > +    { "ICH9-LPC", "x-keep-pci-slot-hpc", "true" },
> >  };
> >  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 fc34b905ee..e1e100316d 100644
> > --- a/hw/i386/pc_q35.c
> > +++ b/hw/i386/pc_q35.c
> > @@ -137,6 +137,7 @@ static void pc_q35_init(MachineState *machine)
> >      DriveInfo *hd[MAX_SATA_PORTS];
> >      MachineClass *mc = MACHINE_GET_CLASS(machine);
> >      bool acpi_pcihp;
> > +    bool keep_pci_slot_hpc;
> >
> >      /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
> >       * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
> > @@ -242,7 +243,11 @@ static void pc_q35_init(MachineState *machine)
> >                                            ACPI_PM_PROP_ACPI_PCIHP_BRIDGE,
> >                                            NULL);
> >
> > -    if (acpi_pcihp) {
> > +    keep_pci_slot_hpc = object_property_get_bool(OBJECT(lpc),
> > +                                                 "x-keep-pci-slot-hpc",
> > +                                                 NULL);
> > +
> > +    if (!keep_pci_slot_hpc && acpi_pcihp) {
>
> Does this mean we are adding "x-native-hotplug" property for pcie
> slots only for 6.1 by default unless users pass x-keep-pci-slot-hpc =
> false manually?

Ok I get this now. Previously, we were disabling Hot-plug capable
(HPC) bit for PCIE slots unconditionally when ACPI hotplug was
enabled. Now, we do this only for 6.1. For 6.0 and 6.2 and above, we
keep HPC bit ON *but* disable native hotplug advertisement from ACPI
OSC method. The OSC advertisement change is only for 6.2 and above. So
basically if I am not mistaken, we keep 6.1 behavior unchanged.

Am I getting all the parts right?


>
> >          object_register_sugar_prop(TYPE_PCIE_SLOT, "x-native-hotplug",
> >                                     "false", true);
> >      }
> > --
> > 2.27.0
> >
> >



reply via email to

[Prev in Thread] Current Thread [Next in Thread]