On 06.02.19 14:01, Igor Mammedov wrote:
> On Wed, 23 Jan 2019 20:55:27 +0100
> David Hildenbrand <da...@redhat.com> wrote:
>
>> Override the device hotplug handler to properly handle the memory device
>> part via virtio-pmem-pci callbacks 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 PCI host
>> buses don't have a PCI hotplug handler at all yet, just to be sure that
>> we alway have a hotplug handler on x86, add a simple error check.
>>
>> Unlocking virtio-pmem will unlock virtio-pmem-pci.
>>
>> Signed-off-by: David Hildenbrand <da...@redhat.com>
>> ---
>> default-configs/i386-softmmu.mak | 1 +
>> hw/i386/pc.c | 70 +++++++++++++++++++++++++++++++-
>> 2 files changed, 70 insertions(+), 1 deletion(-)
>>
>> diff --git a/default-configs/i386-softmmu.mak
>> b/default-configs/i386-softmmu.mak
>> index 64c998c4c8..3f63e95a55 100644
>> --- a/default-configs/i386-softmmu.mak
>> +++ b/default-configs/i386-softmmu.mak
>> @@ -52,6 +52,7 @@ CONFIG_APIC=y
>> CONFIG_IOAPIC=y
>> CONFIG_PVPANIC=y
>> CONFIG_MEM_DEVICE=y
>> +CONFIG_VIRTIO_PMEM=y
>> CONFIG_DIMM=y
>> CONFIG_NVDIMM=y
>> CONFIG_ACPI_NVDIMM=y
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index fd0cb29ba9..6a176caeb9 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/virtio/virtio-pmem-pci.h"
>>
>> /* debug PC/ISA interrupts */
>> //#define DEBUG_IRQ
>> @@ -2224,6 +2225,64 @@ static void pc_cpu_pre_plug(HotplugHandler
>> *hotplug_dev,
>> numa_cpu_pre_plug(cpu_slot, dev, errp);
>> }
>>
>> +static void pc_virtio_pmem_pci_pre_plug(HotplugHandler *hotplug_dev,
>> + DeviceState *dev, Error **errp)
>> +{
>> + HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
>> + Error *local_err = NULL;
>> +
>> + if (!hotplug_dev2) {
>> + /*
>> + * Without a bus hotplug handler, we cannot control the plug/unplug
>> + * order. This should never be the case on x86, however better add
>> + * a safety net.
>> + */
>> + error_setg(errp, "virtio-pmem-pci not supported on this bus.");
>> + return;
>> + }
>> + virtio_pmem_pci_pre_plug(VIRTIO_PMEM_PCI(dev), MACHINE(hotplug_dev),
> ^^^
> doesn't do anything beside being dumb proxy to memory_device_pre_plug()
> I'd just use memory_device_pre_plug() here and avoid so far needles wrappers
Had that initially but considered it cleaner. But I can change that back.
>
>> + &local_err);
>> + if (!local_err) {
> since logic is not trivial I'd add comment somewhere in this function
> explaining why handlers are called in this particular order.
Good idea, will do that.
Thanks!
--
Thanks,
David / dhildenb
