From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58027) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VstIu-0000bD-1y for qemu-devel@nongnu.org; Tue, 17 Dec 2013 06:59:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VstIp-0005CX-O4 for qemu-devel@nongnu.org; Tue, 17 Dec 2013 06:59:11 -0500 Received: from mx1.redhat.com ([209.132.183.28]:14168) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VstIp-0005CD-G5 for qemu-devel@nongnu.org; Tue, 17 Dec 2013 06:59:07 -0500 Date: Tue, 17 Dec 2013 12:58:56 +0100 From: Igor Mammedov Message-ID: <20131217125856.6ea54d5d@nial.usersys.redhat.com> In-Reply-To: <87r49c1ioi.fsf@codemonkey.ws> References: <1386938688-22433-1-git-send-email-imammedo@redhat.com> <87r49c1ioi.fsf@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 00/11 v3] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: kwolf@redhat.com, peter.maydell@linaro.org, peter.crosthwaite@xilinx.com, ehabkost@redhat.com, mst@redhat.com, marcel.a@redhat.com, qemu-devel@nongnu.org, blauwirbel@gmail.com, alex.williamson@redhat.com, kraxel@redhat.com, dkoch@verizon.co, pbonzini@redhat.com, afaerber@suse.de On Mon, 16 Dec 2013 15:26:37 -0800 Anthony Liguori wrote: > Igor Mammedov writes: >=20 > > changes since v2: > > * s/hotplugable/hotpluggable/ > > * move hotplug check to an earlier patch: > > "qdev: add "hotpluggable" property to Device" > > -- > > Refactor PCI specific hotplug API to a more generic/reusable one. > > Model it after SCSI-BUS like hotplug API replacing single hotplug > > callback with hotplug/hot_unplug pair of callbacks as suggested by > > Paolo. > > Difference between SCSI-BUS and this approach is that the former > > is BUS centric while the latter is device centred. Which is evolved > > from the fact that hotplug callbacks used by ACPI/SHPC/PCIE are > > implemented by devices rather than by bus and bus serves only as > > a proxy to forward event to hotplug device. > > Memory hotplug also exposes tha same usage pattern hence an attempt > > to generalize hotplug API. > > > > Refactoring also simplifies wiring of a hotplug device with a bus, > > all it needs is to set "hotplug-device" link on bus, which > > would potentially allow to do it from configuration file, > > there is not need to setup hotplug device callbacks on bus > > synce it can get them via HOTPLUG_DEVICE API of "hotplug-device" > > target. > > > > In addition device centred hotplug API may be used by bus-less > > hotplug implementations as well if it's decided to use > > link instead of bus. >=20 > I'm having a hard time parsing this description. >=20 > Sharing hot plug code is a good thing. Making hotplug a qdev-level > concept seems like a bad thing to me. we could add TYPE_BUS_DEVICE as Peter suggests but let me explain why I've chosen DEVICE for it. All current hotplug code depends on hotpluggable bus but periodically there were suggestions to use link<>s for hotplug instead of bus. Making hotplug the bus device only concept will not allow to move in=20 direction of hotplug with links which are/were supposed to replace buses in future. As side effect series lays ground work for link based hotplug, it's not complete but a step towards it. It still doesn't allow link based hotplug, since purpose was to have unified hotplug interface across PCI/SCSI/virtio devices/buses.=20 Maybe we need TYPE_HOTPLUGGABLE_DEVICE so it could be abstracted from bus as well and eventually it could be used with bussless devices? >=20 > The series is a net add of code so I don't think we're winning anything > by generalizing here. it will allow to remove different implementations of hotplug mechanism in SCSI/virtio and maybe USB buses in addition to converted PCI hotplug. > Is there a use-case this enables that isn't possible today? It was prompted by memory hotplug, alternative was that it can re-implement hotplug mechanism in its own way or duplicating code from scsi or pci buses/devices. Neither alternatives look nice when there is a common usage pattern. > Regards, >=20 > Anthony Liguori >=20 > > > > Patches 8-11 are should be merged as one and are split only for > > simplifying review (they compile fine but PCI hotplug is broken > > until the last patch is applyed). > > > > git tree for testing: > > https://github.com/imammedo/qemu/commits/hotplug_dev_inf_v3 > > > > tested only ACPI and PCIE hotplug. > > > > Herv=E9 Poussineau (1): > > qom: detect bad reentrance during object_class_foreach > > > > Igor Mammedov (9): > > define hotplug interface > > qdev: add to BusState "hotplug-handler" link > > qdev: add "hotpluggable" property to Device > > hw/acpi: move typeinfo to the file end > > qdev:pci: refactor PCIDevice to use generic "hotpluggable" property > > acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API > > pci/shpc: convert SHPC hotplug to use hotplug-handler API > > pci/pcie: convert PCIE hotplug to use hotplug-handler API > > hw/pci: switch to a generic hotplug handling for PCIDevice > > > > Paolo Bonzini (1): > > qom: do not register interface "types" in the type table > > > > hw/acpi/piix4.c | 151 ++++++++++++++++++++++-----------= -------- > > hw/core/Makefile.objs | 1 + > > hw/core/hotplug.c | 48 +++++++++++++ > > hw/core/qdev.c | 50 ++++++++++++-- > > hw/display/cirrus_vga.c | 2 +- > > hw/display/qxl.c | 2 +- > > hw/display/vga-pci.c | 2 +- > > hw/display/vmware_vga.c | 2 +- > > hw/i386/acpi-build.c | 6 +- > > hw/ide/piix.c | 4 +- > > hw/isa/piix4.c | 2 +- > > hw/pci-bridge/pci_bridge_dev.c | 9 +++ > > hw/pci-host/piix.c | 6 +- > > hw/pci/pci.c | 40 +---------- > > hw/pci/pcie.c | 73 +++++++++++++------- > > hw/pci/pcie_port.c | 8 +++ > > hw/pci/shpc.c | 133 +++++++++++++++++++++++----------= --- > > hw/usb/hcd-ehci-pci.c | 2 +- > > hw/usb/hcd-ohci.c | 2 +- > > hw/usb/hcd-uhci.c | 2 +- > > hw/usb/hcd-xhci.c | 2 +- > > include/hw/hotplug.h | 75 ++++++++++++++++++++ > > include/hw/pci/pci.h | 13 ---- > > include/hw/pci/pci_bus.h | 2 - > > include/hw/pci/pcie.h | 5 ++ > > include/hw/pci/shpc.h | 8 +++ > > include/hw/qdev-core.h | 8 +++ > > qom/object.c | 17 ++++- > > 28 files changed, 455 insertions(+), 220 deletions(-) > > create mode 100644 hw/core/hotplug.c > > create mode 100644 include/hw/hotplug.h > > > > --=20 > > 1.8.3.1