[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.10 v3 2/3] hw/acpi: Move acpi_set_pci_info
From: |
Anthony PERARD |
Subject: |
Re: [Qemu-devel] [PATCH for-2.10 v3 2/3] hw/acpi: Move acpi_set_pci_info to pcihp |
Date: |
Fri, 18 Aug 2017 14:31:07 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Fri, Aug 18, 2017 at 11:37:34AM +0200, Igor Mammedov wrote:
> On Fri, 18 Aug 2017 04:40:02 +0300
> "Michael S. Tsirkin" <address@hidden> wrote:
>
> > On Thu, Aug 17, 2017 at 05:23:46PM +0100, Anthony PERARD wrote:
> > > This means that the function will be call and the property
> > > acpi-pcihp-bsel will be set even if ACPI build is disable.
> > >
> > > To do PCI passthrough with Xen, the property acpi-pcihp-bsel needs to be
> > > set, but this was done only when ACPI tables are built which is not
> > > needed for a Xen guest. The need for the property starts with commit
> > > "pc: pcihp: avoid adding ACPI_PCIHP_PROP_BSEL twice"
> > > (f0c9d64a68b776374ec4732424a3e27753ce37b6).
> > >
> > > Reported-by: Sander Eikelenboom <address@hidden>
> > > Signed-off-by: Anthony PERARD <address@hidden>
> > >
> > > ---
> > > Changes in V3:
> > > - move acpi_set_pci_info to pcihp instead
> > >
> > > Changes in V2:
> > > - check for acpi_enabled before calling acpi_set_pci_info.
> > > - set the property on the root bus only.
> > >
> > > This patch would be a canditade to backport to 2.9, along with
> > > "hw/acpi: Limit hotplug to root bus on legacy mode"
> > >
> > > CC: Stefano Stabellini <address@hidden>
> > > CC: Bruce Rogers <address@hidden>
> > > ---
> > > hw/acpi/pcihp.c | 31 +++++++++++++++++++++++++++++++
> > > hw/i386/acpi-build.c | 32 --------------------------------
> > > 2 files changed, 31 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
> > > index 9db3c2eaf2..44e8842db8 100644
> > > --- a/hw/acpi/pcihp.c
> > > +++ b/hw/acpi/pcihp.c
> > > @@ -75,6 +75,36 @@ static int acpi_pcihp_get_bsel(PCIBus *bus)
> > > }
> > > }
> > >
> > > +/* Assign BSEL property to all buses. In the future, this can be changed
> > > + * to only assign to buses that support hotplug.
> > > + */
> > > +static void *acpi_set_bsel(PCIBus *bus, void *opaque)
> > > +{
> > > + unsigned *bsel_alloc = opaque;
> > > + unsigned *bus_bsel;
> > > +
> > > + if (qbus_is_hotpluggable(BUS(bus))) {
> > > + bus_bsel = g_malloc(sizeof *bus_bsel);
> > > +
> > > + *bus_bsel = (*bsel_alloc)++;
> > > + object_property_add_uint32_ptr(OBJECT(bus), ACPI_PCIHP_PROP_BSEL,
> > > + bus_bsel, &error_abort);
> > > + }
> > > +
> > > + return bsel_alloc;
> > > +}
> > > +
> > > +static void acpi_set_pci_info(void)
> > > +{
> > > + PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
> > > + unsigned bsel_alloc = ACPI_PCIHP_BSEL_DEFAULT;
> > > +
> > > + if (bus) {
> > > + /* Scan all PCI buses. Set property to enable acpi based
> > > hotplug. */
> > > + pci_for_each_bus_depth_first(bus, acpi_set_bsel, NULL,
> > > &bsel_alloc);
> > > + }
> > > +}
> > > +
> > > static void acpi_pcihp_test_hotplug_bus(PCIBus *bus, void *opaque)
> > > {
> > > AcpiPciHpFind *find = opaque;
> > > @@ -177,6 +207,7 @@ static void acpi_pcihp_update(AcpiPciHpState *s)
> > >
> > > void acpi_pcihp_reset(AcpiPciHpState *s)
> > > {
> > > + acpi_set_pci_info();
> > > acpi_pcihp_update(s);
> > > }
> >
> > IIUC doing this on reset will add property over and over again leaking
> > memory.
> in v2 I've explicitly suggested to call it once, like:
Sorry I misunderstood. I'll fix it.
> acpi_set_pci_info() {
>
> static bool bsel_is set;
>
> if (bsel_is set)
> return;
> bsel_is set = true;
>
> ...
> }
>
> not patch related:
> BTW bsel assignment is not stable in hotplug + migration use case,
> and we probably should fix it up in 2.11 (CCing Marcel)
>
> > I think that we need to do it on machine done.
> >
> > Igor, I think reordering acpi-build like earlier version did
> > is less intrusive and more appropriate for 2.10.
> >
> > For 2.10 I would like to see ideally some changes that
> > are all if (xen) making it obvious non xen is not
> > affected. I can then ack it and it will be merged in xen tree.
> it didn't work before so I'd just push fix to 2.11 without
> intermediate fix.
> But if you guys think it's worth to fix in 2.10, I'm fine with v2
> for it if Anthony will take care of it (rebase this series)
> in 2.11 merge window.
Yes, I can take care of this series for 2.11, and find out how to build
the mips-softmmu target which does not build because it's missing
find_i440fx.
> >
> > Clean it up after 2.10.
> >
So is the v2 good enough or do I need to resend it?
Thanks,
--
Anthony PERARD
Re: [Qemu-devel] [PATCH for-2.10 v3 2/3] hw/acpi: Move acpi_set_pci_info to pcihp, Igor Mammedov, 2017/08/18
Re: [Qemu-devel] [PATCH for-2.10 v3 0/3] Fix hotplug of PCI passthrought device on Xen, no-reply, 2017/08/17