| Message ID | 20180920103243.28474-23-david@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | memory-device: complete refactoring + virtio-pmem | expand |
* David Hildenbrand (david@redhat.com) wrote: > Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices > (memory-device part) from our pc machine hotplug handler. > > Signed-off-by: David Hildenbrand <david@redhat.com> I see a few other places in the pc subdir that have checks for TYPE_NVDIMM; can you just explain why they are untouched: i386/pc.c: pc_memory_unplug_request acpi/ich9.c: ich9_pm_device_plug_cb acpi/piix4.c:piix4_device_plug_cb (Not that I understand the detail of those paths, but I just followed where the existing nvdimm code goes) Dave > --- > default-configs/i386-softmmu.mak | 1 + > hw/i386/pc.c | 10 +++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak > index 4c1637338b..dfb1e6d63a 100644 > --- a/default-configs/i386-softmmu.mak > +++ b/default-configs/i386-softmmu.mak > @@ -51,6 +51,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 03148450c8..bea043fe23 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -74,6 +74,7 @@ > #include "hw/nmi.h" > #include "hw/i386/intel_iommu.h" > #include "hw/net/ne2000-isa.h" > +#include "hw/virtio/virtio-pmem.h" > > /* debug PC/ISA interrupts */ > //#define DEBUG_IRQ > @@ -2013,6 +2014,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_VIRTIO_PMEM)) { > + virtio_pmem_pre_plug(VIRTIO_PMEM(dev), MACHINE(hotplug_dev), errp); > } > } > > @@ -2023,6 +2026,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_VIRTIO_PMEM)) { > + virtio_pmem_plug(VIRTIO_PMEM(dev), MACHINE(hotplug_dev), errp); > } > } > > @@ -2046,6 +2051,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_VIRTIO_PMEM)) { > + virtio_pmem_unplug(VIRTIO_PMEM(dev), MACHINE(hotplug_dev), errp); > } else { > error_setg(errp, "acpi: device unplug for not supported device" > " type: %s", object_get_typename(OBJECT(dev))); > @@ -2056,7 +2063,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_VIRTIO_PMEM)) { > return HOTPLUG_HANDLER(machine); > } > > -- > 2.17.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 21/09/2018 18:00, Dr. David Alan Gilbert wrote: > * David Hildenbrand (david@redhat.com) wrote: >> Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices >> (memory-device part) from our pc machine hotplug handler. >> >> Signed-off-by: David Hildenbrand <david@redhat.com> > > I see a few other places in the pc subdir that have checks for > TYPE_NVDIMM; can you just explain why they are untouched: > i386/pc.c: pc_memory_unplug_request I think I had an answer why that isn't needed, but for unplug we might still be missing something. In general: unplug requests are always directed at the proxy device (via qdev_unplug()) (e.g. virtio-pmem-pci, see below). virtio-pmem is a child device of virtio-pmem-pci. Once virtio-pmem-pci is finally unrealized, the virtio-mem device will be unrealized. Now, there are no hotplug handlers getting called as far as I can see for that child device. there would have to be an proper call to a hotplug handler somewhere when setting the device to unrealized. (and that's the place where a unplug requests does not make really sense, but only a straight unplug - because we are half way through removing the hierarchy of devices - one of the reason why also failures of unrealize are usually ignored). I'll look into the details. Thanks for pointing that out, this stuff always messes with my head. > acpi/ich9.c: ich9_pm_device_plug_cb > acpi/piix4.c:piix4_device_plug_cb virtio-pmem is not an acpi device, that's why these don't apply. E.g. if virtio-pmem-pci is plugged, it will most probably trigger these hotplug handlers for the proxy virtio pci device. The proxy device creates and realized the virtio-mem (memory-device) - without any hotplug handlers involved. That#s the point where we can jump in and realize the device via the machine instead. > > (Not that I understand the detail of those paths, but I just > followed where the existing nvdimm code goes) > > Dave
* David Hildenbrand (david@redhat.com) wrote: > On 21/09/2018 18:00, Dr. David Alan Gilbert wrote: > > * David Hildenbrand (david@redhat.com) wrote: > >> Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices > >> (memory-device part) from our pc machine hotplug handler. > >> > >> Signed-off-by: David Hildenbrand <david@redhat.com> > > > > I see a few other places in the pc subdir that have checks for > > TYPE_NVDIMM; can you just explain why they are untouched: > > i386/pc.c: pc_memory_unplug_request > > I think I had an answer why that isn't needed, but for unplug we might > still be missing something. > > In general: unplug requests are always directed at the proxy device (via > qdev_unplug()) (e.g. virtio-pmem-pci, see below). virtio-pmem is a child > device of virtio-pmem-pci. Once virtio-pmem-pci is finally unrealized, > the virtio-mem device will be unrealized. > > Now, there are no hotplug handlers getting called as far as I can see > for that child device. there would have to be an proper call to a > hotplug handler somewhere when setting the device to unrealized. (and > that's the place where a unplug requests does not make really sense, but > only a straight unplug - because we are half way through removing the > hierarchy of devices - one of the reason why also failures of unrealize > are usually ignored). > > I'll look into the details. Thanks for pointing that out, this stuff > always messes with my head. > > > acpi/ich9.c: ich9_pm_device_plug_cb > > acpi/piix4.c:piix4_device_plug_cb > > virtio-pmem is not an acpi device, that's why these don't apply. > > E.g. if virtio-pmem-pci is plugged, it will most probably trigger these > hotplug handlers for the proxy virtio pci device. The proxy device > creates and realized the virtio-mem (memory-device) - without any > hotplug handlers involved. That#s the point where we can jump in and > realize the device via the machine instead. Does not being an acpi-device make life easier or harder? Again, not knowing anything about the pmem acpi; if it looked like a normal nvdimm through ACPI would OSs be able to boot off it? Dave > > > > (Not that I understand the detail of those paths, but I just > > followed where the existing nvdimm code goes) > > > > Dave > > > -- > > Thanks, > > David / dhildenb -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 21/09/2018 18:55, Dr. David Alan Gilbert wrote: > * David Hildenbrand (david@redhat.com) wrote: >> On 21/09/2018 18:00, Dr. David Alan Gilbert wrote: >>> * David Hildenbrand (david@redhat.com) wrote: >>>> Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices >>>> (memory-device part) from our pc machine hotplug handler. >>>> >>>> Signed-off-by: David Hildenbrand <david@redhat.com> >>> >>> I see a few other places in the pc subdir that have checks for >>> TYPE_NVDIMM; can you just explain why they are untouched: >>> i386/pc.c: pc_memory_unplug_request >> >> I think I had an answer why that isn't needed, but for unplug we might >> still be missing something. >> >> In general: unplug requests are always directed at the proxy device (via >> qdev_unplug()) (e.g. virtio-pmem-pci, see below). virtio-pmem is a child >> device of virtio-pmem-pci. Once virtio-pmem-pci is finally unrealized, >> the virtio-mem device will be unrealized. >> >> Now, there are no hotplug handlers getting called as far as I can see >> for that child device. there would have to be an proper call to a >> hotplug handler somewhere when setting the device to unrealized. (and >> that's the place where a unplug requests does not make really sense, but >> only a straight unplug - because we are half way through removing the >> hierarchy of devices - one of the reason why also failures of unrealize >> are usually ignored). >> >> I'll look into the details. Thanks for pointing that out, this stuff >> always messes with my head. >> >>> acpi/ich9.c: ich9_pm_device_plug_cb >>> acpi/piix4.c:piix4_device_plug_cb >> >> virtio-pmem is not an acpi device, that's why these don't apply. >> >> E.g. if virtio-pmem-pci is plugged, it will most probably trigger these >> hotplug handlers for the proxy virtio pci device. The proxy device >> creates and realized the virtio-mem (memory-device) - without any >> hotplug handlers involved. That#s the point where we can jump in and >> realize the device via the machine instead. > > Does not being an acpi-device make life easier or harder? My opinion is: easier. Being an acpi device, we would have devices that are both, ACPI and virtio. Or you have to create separate device and somehow link them together (both in QEMU and in Linux) I don't like gluing virtio devices to different, unrelated technologies. Especially, you could never support architectures that don't support ACPI. (with virtio-pmem, we could eventually even support architectures that don't support something like nvdimm or acpi). > Again, not > knowing anything about the pmem acpi; if it looked like a normal nvdimm > through ACPI would OSs be able to boot off it? As far as I remember, booting from NVDIMM is not implemented anywhere. I guess if it would be, we could also realize booting from virtio-pmem (e.g. via grub extension). > > Dave
* David Hildenbrand (david@redhat.com) wrote: > On 21/09/2018 18:55, Dr. David Alan Gilbert wrote: > > * David Hildenbrand (david@redhat.com) wrote: > >> On 21/09/2018 18:00, Dr. David Alan Gilbert wrote: > >>> * David Hildenbrand (david@redhat.com) wrote: > >>>> Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices > >>>> (memory-device part) from our pc machine hotplug handler. > >>>> > >>>> Signed-off-by: David Hildenbrand <david@redhat.com> > >>> > >>> I see a few other places in the pc subdir that have checks for > >>> TYPE_NVDIMM; can you just explain why they are untouched: > >>> i386/pc.c: pc_memory_unplug_request > >> > >> I think I had an answer why that isn't needed, but for unplug we might > >> still be missing something. > >> > >> In general: unplug requests are always directed at the proxy device (via > >> qdev_unplug()) (e.g. virtio-pmem-pci, see below). virtio-pmem is a child > >> device of virtio-pmem-pci. Once virtio-pmem-pci is finally unrealized, > >> the virtio-mem device will be unrealized. > >> > >> Now, there are no hotplug handlers getting called as far as I can see > >> for that child device. there would have to be an proper call to a > >> hotplug handler somewhere when setting the device to unrealized. (and > >> that's the place where a unplug requests does not make really sense, but > >> only a straight unplug - because we are half way through removing the > >> hierarchy of devices - one of the reason why also failures of unrealize > >> are usually ignored). > >> > >> I'll look into the details. Thanks for pointing that out, this stuff > >> always messes with my head. > >> > >>> acpi/ich9.c: ich9_pm_device_plug_cb > >>> acpi/piix4.c:piix4_device_plug_cb > >> > >> virtio-pmem is not an acpi device, that's why these don't apply. > >> > >> E.g. if virtio-pmem-pci is plugged, it will most probably trigger these > >> hotplug handlers for the proxy virtio pci device. The proxy device > >> creates and realized the virtio-mem (memory-device) - without any > >> hotplug handlers involved. That#s the point where we can jump in and > >> realize the device via the machine instead. > > > > Does not being an acpi-device make life easier or harder? > > My opinion is: easier. > > Being an acpi device, we would have devices that are both, ACPI and > virtio. Or you have to create separate device and somehow link them > together (both in QEMU and in Linux) > > I don't like gluing virtio devices to different, unrelated technologies. > Especially, you could never support architectures that don't support > ACPI. (with virtio-pmem, we could eventually even support architectures > that don't support something like nvdimm or acpi). > > > Again, not > > knowing anything about the pmem acpi; if it looked like a normal nvdimm > > through ACPI would OSs be able to boot off it? > > As far as I remember, booting from NVDIMM is not implemented anywhere. I > guess if it would be, we could also realize booting from virtio-pmem > (e.g. via grub extension). OK, that's fair enough; I was just curious about why there was the difference. Dave > > > > Dave > > > -- > > Thanks, > > David / dhildenb -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
diff --git a/default-configs/i386-softmmu.mak b/default-configs/i386-softmmu.mak index 4c1637338b..dfb1e6d63a 100644 --- a/default-configs/i386-softmmu.mak +++ b/default-configs/i386-softmmu.mak @@ -51,6 +51,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 03148450c8..bea043fe23 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -74,6 +74,7 @@ #include "hw/nmi.h" #include "hw/i386/intel_iommu.h" #include "hw/net/ne2000-isa.h" +#include "hw/virtio/virtio-pmem.h" /* debug PC/ISA interrupts */ //#define DEBUG_IRQ @@ -2013,6 +2014,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_VIRTIO_PMEM)) { + virtio_pmem_pre_plug(VIRTIO_PMEM(dev), MACHINE(hotplug_dev), errp); } } @@ -2023,6 +2026,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_VIRTIO_PMEM)) { + virtio_pmem_plug(VIRTIO_PMEM(dev), MACHINE(hotplug_dev), errp); } } @@ -2046,6 +2051,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_VIRTIO_PMEM)) { + virtio_pmem_unplug(VIRTIO_PMEM(dev), MACHINE(hotplug_dev), errp); } else { error_setg(errp, "acpi: device unplug for not supported device" " type: %s", object_get_typename(OBJECT(dev))); @@ -2056,7 +2063,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_VIRTIO_PMEM)) { return HOTPLUG_HANDLER(machine); }
Compile virtio-pmem for x86 and properly hotplug virtio-pmem devices (memory-device part) from our pc machine hotplug handler. Signed-off-by: David Hildenbrand <david@redhat.com> --- default-configs/i386-softmmu.mak | 1 + hw/i386/pc.c | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)