On 18.01.19 11:20, Igor Mammedov wrote:
> On Wed, 16 Jan 2019 12:35:22 +0100
> David Hildenbrand <da...@redhat.com> wrote:
>
>> Override the PCI device hotplug handler to properly handle the
>> memory device part from the machine hotplug handler and forward to the
>> actual PCI bus hotplug handler.
>>
>> As PCI hotplug has not been properly factored out into hotplug handlers,
>> most magic is performed in the (un)realize functions. Also some buses don't
> ^^^^^^^^ pls, be
> more specific
Last time I checked there were some TYPE_PCI_HOST_BRIDGE devices in QEMU
that would not have a hotplug handler. I *think* they are all unrelated
to x86/pc. But as I was not sure if I am missing something, I introduced
a simple error check.
>
>> have a PCI hotplug handler at all yet (not sure if they are actually used
>> on x86, but just to be sure).
>>
>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>> ---
>> hw/i386/pc.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 89 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index fd0cb29ba9..62f83859fa 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -75,6 +75,7 @@
>> #include "hw/usb.h"
>> #include "hw/i386/intel_iommu.h"
>> #include "hw/net/ne2000-isa.h"
>> +#include "hw/mem/memory-device.h"
>>
>> /* debug PC/ISA interrupts */
>> //#define DEBUG_IRQ
>> @@ -2224,6 +2225,84 @@ static void pc_cpu_pre_plug(HotplugHandler
>> *hotplug_dev,
>> numa_cpu_pre_plug(cpu_slot, dev, errp);
>> }
>>
>> +static void pc_pci_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>> + Error **errp)
>> +{
>> + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
>> + Error *local_err = NULL;
>> +
>> + if (object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE)) {
>> + if (!hotplug_dev2) {
>> + /*
>> + * Without a bus hotplug handler, we cannot control the
>> plug/unplug
>> + * order. Disallow memory devices on such buses.
>> + */
>> + error_setg(errp, "Memory devices not supported on this bus.");
>> + return;
>> + }
>> + memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev),
>> + NULL, errp);
>> + }
>> +
>> + if (hotplug_dev2) {
>> + hotplug_handler_pre_plug(hotplug_dev2, dev, errp);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> + }
>> + }
>> +}
>> +
>> +static void pc_pci_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>> + Error **errp)
>> +{
>> + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
>> + Error *local_err = NULL;
>> +
>> + if (object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE)) {
>> + memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
>> + }
>> +
>> + if (hotplug_dev2) {
>> + hotplug_handler_plug(hotplug_dev2, dev, &local_err);
>> + }
>> +
>> + if (local_err) {
>> + if (object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE)) {
>> + memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
>> + }
>> + }
>> + error_propagate(errp, local_err);
>> +}
>> +
>> +static void pc_pci_unplug_request(HotplugHandler *hotplug_dev, DeviceState
>> *dev,
>> + Error **errp)
>> +{
>> + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
>> +
>> + if (hotplug_dev2) {
>> + hotplug_handler_unplug_request(hotplug_dev2, dev, errp);
>> + }
>> +}
>> +
>> +static void pc_pci_device_unplug(HotplugHandler *hotplug_dev, DeviceState
>> *dev,
>> + Error **errp)
>> +{
>> + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
>> + Error *local_err = NULL;
>> +
>> + if (hotplug_dev2) {
>> + hotplug_handler_unplug(hotplug_dev2, dev, &local_err);
>> + }
>> +
>> + if (!local_err) {
>> + if (object_dynamic_cast(OBJECT(dev), TYPE_MEMORY_DEVICE)) {
>> + memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
>> + }
>> + }
>> + error_propagate(errp, local_err);
>> +}
>> +
>> static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> DeviceState *dev, Error **errp)
>> {
>> @@ -2231,6 +2310,8 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>> pc_memory_pre_plug(hotplug_dev, dev, errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> pc_cpu_pre_plug(hotplug_dev, dev, errp);
>> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> + pc_pci_pre_plug(hotplug_dev, dev, errp);
>> }
>> }
>>
>> @@ -2241,6 +2322,8 @@ static void pc_machine_device_plug_cb(HotplugHandler
>> *hotplug_dev,
>> pc_memory_plug(hotplug_dev, dev, errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> pc_cpu_plug(hotplug_dev, dev, errp);
>> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> + pc_pci_plug(hotplug_dev, dev, errp);
>> }
>> }
>>
>> @@ -2251,6 +2334,8 @@ static void
>> pc_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
>> pc_memory_unplug_request(hotplug_dev, dev, errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> pc_cpu_unplug_request_cb(hotplug_dev, dev, errp);
>> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> + pc_pci_unplug_request(hotplug_dev, dev, errp);
>> } else {
>> error_setg(errp, "acpi: device unplug request for not supported
>> device"
>> " type: %s", object_get_typename(OBJECT(dev)));
>> @@ -2264,6 +2349,8 @@ static void pc_machine_device_unplug_cb(HotplugHandler
>> *hotplug_dev,
>> pc_memory_unplug(hotplug_dev, dev, errp);
>> } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> pc_cpu_unplug_cb(hotplug_dev, dev, errp);
>> + } else if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
>> + pc_pci_device_unplug(hotplug_dev, dev, errp);
>> } else {
>> error_setg(errp, "acpi: device unplug for not supported device"
>> " type: %s", object_get_typename(OBJECT(dev)));
>> @@ -2274,7 +2361,8 @@ static HotplugHandler
>> *pc_get_hotpug_handler(MachineState *machine,
>> DeviceState *dev)
>> {
>> if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
>> - object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
>> + object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
>> + object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
> why all PCI devices instead of virtio-pmem-pci one?
>
Last time I tried to include "hw/virtio/virtio-pci.h" in pc.c it
resulted in an inclusion error madness. But this should be easy to fix I
guess. (the basic idea of this patch is the important bit)
And I just tried to include and it seems to result in no errors right now.
>> return HOTPLUG_HANDLER(machine);
>> }
>>
>
--
Thanks,
David / dhildenb
