From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37005) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gIDhL-0007uI-QR for qemu-devel@nongnu.org; Thu, 01 Nov 2018 10:11:48 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gIDhK-0005v0-Hy for qemu-devel@nongnu.org; Thu, 01 Nov 2018 10:11:47 -0400 Date: Thu, 1 Nov 2018 15:11:42 +0100 From: Igor Mammedov Message-ID: <20181101151142.1fa6c84e@redhat.com> In-Reply-To: <20181024101930.20674-2-david@redhat.com> References: <20181024101930.20674-1-david@redhat.com> <20181024101930.20674-2-david@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v1 1/7] pcihp: perform check for bus capability in pre_plug handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Hildenbrand Cc: qemu-devel@nongnu.org, "Michael S . Tsirkin" , Marcel Apfelbaum , Alexander Graf , David Gibson , Eduardo Habkost , "Dr . David Alan Gilbert" , qemu-ppc@nongnu.org On Wed, 24 Oct 2018 12:19:24 +0200 David Hildenbrand wrote: > Perform the check in the pre_plug handler. In addition, we need the > capability only if the device is actually hotplugged (and not created > during machine initialization). This is a preparation for coldplugging > devices via that hotplug handler. > > Signed-off-by: David Hildenbrand Reviewed-by: Igor Mammedov > --- > hw/acpi/pcihp.c | 21 +++++++++++++++------ > hw/acpi/piix4.c | 16 ++++++++++++++-- > include/hw/acpi/pcihp.h | 2 ++ > 3 files changed, 31 insertions(+), 8 deletions(-) > > diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c > index 80d42e12ff..5e7cef173c 100644 > --- a/hw/acpi/pcihp.c > +++ b/hw/acpi/pcihp.c > @@ -217,17 +217,24 @@ void acpi_pcihp_reset(AcpiPciHpState *s) > acpi_pcihp_update(s); > } > > -void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, > - DeviceState *dev, Error **errp) > +void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > { > - PCIDevice *pdev = PCI_DEVICE(dev); > - int slot = PCI_SLOT(pdev->devfn); > - int bsel = acpi_pcihp_get_bsel(pci_get_bus(pdev)); > - if (bsel < 0) { > + /* Only hotplugged devices need the hotplug capability. */ > + if (dev->hotplugged && > + acpi_pcihp_get_bsel(pci_get_bus(PCI_DEVICE(dev))) < 0) { > error_setg(errp, "Unsupported bus. Bus doesn't have property '" > ACPI_PCIHP_PROP_BSEL "' set"); > return; > } > +} > + > +void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, > + DeviceState *dev, Error **errp) > +{ > + PCIDevice *pdev = PCI_DEVICE(dev); > + int slot = PCI_SLOT(pdev->devfn); > + int bsel; > > /* Don't send event when device is enabled during qemu machine creation: > * it is present on boot, no hotplug event is necessary. We do send an > @@ -236,6 +243,8 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, > return; > } > > + bsel = acpi_pcihp_get_bsel(pci_get_bus(pdev)); > + g_assert(bsel >= 0); > s->acpi_pcihp_pci_status[bsel].up |= (1U << slot); > acpi_send_event(DEVICE(hotplug_dev), ACPI_PCI_HOTPLUG_STATUS); > } > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c > index e330f24c71..167afe2cea 100644 > --- a/hw/acpi/piix4.c > +++ b/hw/acpi/piix4.c > @@ -370,6 +370,18 @@ static void piix4_pm_powerdown_req(Notifier *n, void *opaque) > acpi_pm1_evt_power_down(&s->ar); > } > > +static void piix4_device_pre_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp) > +{ > + if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > + acpi_pcihp_device_pre_plug_cb(hotplug_dev, dev, errp); > + } else if (!object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) && > + !object_dynamic_cast(OBJECT(dev), TYPE_CPU)) { > + error_setg(errp, "acpi: device plug request for not supported device" > + " type: %s", object_get_typename(OBJECT(dev))); > + } > +} > + > static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, > DeviceState *dev, Error **errp) > { > @@ -392,8 +404,7 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, > acpi_cpu_plug_cb(hotplug_dev, &s->cpuhp_state, dev, errp); > } > } else { > - error_setg(errp, "acpi: device plug request for not supported device" > - " type: %s", object_get_typename(OBJECT(dev))); > + g_assert_not_reached(); > } > } > > @@ -702,6 +713,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) > */ > dc->user_creatable = false; > dc->hotpluggable = false; > + hc->pre_plug = piix4_device_pre_plug_cb; > hc->plug = piix4_device_plug_cb; > hc->unplug_request = piix4_device_unplug_request_cb; > hc->unplug = piix4_device_unplug_cb; > diff --git a/include/hw/acpi/pcihp.h b/include/hw/acpi/pcihp.h > index 8a65f99fc8..ce31625850 100644 > --- a/include/hw/acpi/pcihp.h > +++ b/include/hw/acpi/pcihp.h > @@ -56,6 +56,8 @@ typedef struct AcpiPciHpState { > void acpi_pcihp_init(Object *owner, AcpiPciHpState *, PCIBus *root, > MemoryRegion *address_space_io, bool bridges_enabled); > > +void acpi_pcihp_device_pre_plug_cb(HotplugHandler *hotplug_dev, > + DeviceState *dev, Error **errp); > void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s, > DeviceState *dev, Error **errp); > void acpi_pcihp_device_unplug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,