| Message ID | cover.1689786474.git.maciej.szmigiero@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Hyper-V Dynamic Memory Protocol driver (hv-balloon | expand |
Doesn't apply to master. Care to rebase?
On 24.07.2023 12:56, Markus Armbruster wrote: > Doesn't apply to master. Care to rebase? > The series is now based on David's virtio-mem-memslots patches (specifically, commit 6769107d1a4f from [1]) since it depends on support for exposing device memory via multiple memslots provided by that series. I'm sorry if that wasn't clear from the cover letter. Thanks, Maciej [1]: https://github.com/davidhildenbrand/qemu/tree/virtio-mem-memslots
"Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes: > On 24.07.2023 12:56, Markus Armbruster wrote: >> Doesn't apply to master. Care to rebase? >> > > The series is now based on David's virtio-mem-memslots patches > (specifically, commit 6769107d1a4f from [1]) since it depends > on support for exposing device memory via multiple memslots > provided by that series. > > I'm sorry if that wasn't clear from the cover letter. Aha! I just fetched David's branch, applied your patches on top, and rebased to current master. I recommend to list dependencies like Based-on: <message-id> so Patchew applies them. > Thanks, > Maciej > > [1]: https://github.com/davidhildenbrand/qemu/tree/virtio-mem-memslots
On 24.07.2023 13:39, Markus Armbruster wrote: > "Maciej S. Szmigiero" <mail@maciej.szmigiero.name> writes: > >> On 24.07.2023 12:56, Markus Armbruster wrote: >>> Doesn't apply to master. Care to rebase? >>> >> >> The series is now based on David's virtio-mem-memslots patches >> (specifically, commit 6769107d1a4f from [1]) since it depends >> on support for exposing device memory via multiple memslots >> provided by that series. >> >> I'm sorry if that wasn't clear from the cover letter. > > Aha! I just fetched David's branch, applied your patches on top, and > rebased to current master. > > I recommend to list dependencies like > > Based-on: <message-id> > > so Patchew applies them. Didn't know that notation, thanks for point this out. Will use it in the future version(s). >> >> [1]: https://github.com/davidhildenbrand/qemu/tree/virtio-mem-memslots > Thanks, Maciej
On 20.07.23 12:12, Maciej S. Szmigiero wrote: > From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> > > This is a continuation of the v5 of the patch series located here: > https://lore.kernel.org/qemu-devel/cover.1686577753.git.maciej.szmigiero@oracle.com/ > We're now in QEMU soft-freeze, which means the memslot series might take a bit to land. I'm going to follow-up on that soonish. > > Changes from v5: > * Incorporate David's rework of the driver on top of his virtio-mem-memslots > patches (specifically, commit 6769107d1a4f), making use of a memory region > container created upfront to avoid calling memory_device{,_pre}_plug() > functions from the driver and introducing a driver-specific MemoryDeviceInfo > sub-type. > > * Include two additional David's memory-device patches necessary for the > aforementioned conversion in this patch set. > > * Use multiple memslots to cover the hot-add memory backend in order to > reduce metadata size for the not-yet-hot-added part of the memory backend. > > * Add David's "Co-developed-by:" to patches where he contributed some changes. > > * Use OBJECT_DEFINE_TYPE_WITH_INTERFACES() and OBJECT_DECLARE_SIMPLE_TYPE() > macros instead of open-coding the equivalent functionality. > > * Drop no longer necessary patch adding g_autoptr() cleanup function for the > Error type. > > > David Hildenbrand (2): > memory-device: Support empty memory devices > memory-device: Drop size alignment check > > Maciej S. Szmigiero (4): > Add Hyper-V Dynamic Memory Protocol definitions > qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo > qapi: Add HV_BALLOON_STATUS_REPORT event > Add a Hyper-V Dynamic Memory Protocol driver (hv-balloon) That is still a gigantic patch. Is there any way to split that into reasonable chunks? For example, move the whole hotplug/memslot part into a dedicated patch? See below on splitting off the PC changes. > > Kconfig.host | 3 + > hw/core/machine-hmp-cmds.c | 15 + > hw/hyperv/Kconfig | 5 + > hw/hyperv/hv-balloon.c | 2246 ++++++++++++++++++++++++++++++ > hw/hyperv/meson.build | 1 + > hw/hyperv/trace-events | 18 + > hw/i386/pc.c | 22 + > hw/mem/memory-device.c | 45 +- > include/hw/hyperv/dynmem-proto.h | 423 ++++++ > include/hw/hyperv/hv-balloon.h | 18 + > include/hw/mem/memory-device.h | 7 +- > meson.build | 28 +- > meson_options.txt | 2 + > qapi/machine.json | 64 +- > scripts/meson-buildoptions.sh | 3 + It's probably best to separate the actual device implementation from wiring up the machine. That is, have a HV_BALLOON_SUPPORTED kconfig (like VIRTIO_MEM_SUPPORTED), and activate that in a single commit for PC, where you also modify hw/i386/pc.c. That commit would be called something like "pc: Support hv-balloon".
David Hildenbrand <david@redhat.com> writes: > On 20.07.23 12:12, Maciej S. Szmigiero wrote: >> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> >> This is a continuation of the v5 of the patch series located here: >> https://lore.kernel.org/qemu-devel/cover.1686577753.git.maciej.szmigiero@oracle.com/ >> > > We're now in QEMU soft-freeze, which means the memslot series might take a bit to land. I'm going to follow-up on that soonish. > >> Changes from v5: >> * Incorporate David's rework of the driver on top of his virtio-mem-memslots >> patches (specifically, commit 6769107d1a4f), making use of a memory region >> container created upfront to avoid calling memory_device{,_pre}_plug() >> functions from the driver and introducing a driver-specific MemoryDeviceInfo >> sub-type. >> * Include two additional David's memory-device patches necessary for the >> aforementioned conversion in this patch set. >> * Use multiple memslots to cover the hot-add memory backend in order to >> reduce metadata size for the not-yet-hot-added part of the memory backend. >> * Add David's "Co-developed-by:" to patches where he contributed some changes. >> * Use OBJECT_DEFINE_TYPE_WITH_INTERFACES() and OBJECT_DECLARE_SIMPLE_TYPE() >> macros instead of open-coding the equivalent functionality. >> * Drop no longer necessary patch adding g_autoptr() cleanup function for the >> Error type. >> David Hildenbrand (2): >> memory-device: Support empty memory devices >> memory-device: Drop size alignment check >> Maciej S. Szmigiero (4): >> Add Hyper-V Dynamic Memory Protocol definitions >> qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo >> qapi: Add HV_BALLOON_STATUS_REPORT event >> Add a Hyper-V Dynamic Memory Protocol driver (hv-balloon) > > That is still a gigantic patch. Is there any way to split that into reasonable chunks? For example, move the whole hotplug/memslot part into > a dedicated patch? > > See below on splitting off the PC changes. > >> Kconfig.host | 3 + >> hw/core/machine-hmp-cmds.c | 15 + >> hw/hyperv/Kconfig | 5 + >> hw/hyperv/hv-balloon.c | 2246 ++++++++++++++++++++++++++++++ >> hw/hyperv/meson.build | 1 + >> hw/hyperv/trace-events | 18 + >> hw/i386/pc.c | 22 + >> hw/mem/memory-device.c | 45 +- >> include/hw/hyperv/dynmem-proto.h | 423 ++++++ >> include/hw/hyperv/hv-balloon.h | 18 + >> include/hw/mem/memory-device.h | 7 +- >> meson.build | 28 +- >> meson_options.txt | 2 + >> qapi/machine.json | 64 +- >> scripts/meson-buildoptions.sh | 3 + > > It's probably best to separate the actual device implementation from wiring up the machine. That is, have a HV_BALLOON_SUPPORTED kconfig > (like VIRTIO_MEM_SUPPORTED), and activate that in a single commit for > PC, where you also modify hw/i386/pc.c. > > That commit would be called something like "pc: Support hv-balloon". Furthermore, try to factor out stuff related to QMP: 0. The driver with QMP stuff omitted / stubbed out 1. Enable query-memory-devices 2. Add HV_BALLOON_STATUS_REPORT event
On 24.07.2023 16:42, David Hildenbrand wrote: > On 20.07.23 12:12, Maciej S. Szmigiero wrote: >> From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> >> >> This is a continuation of the v5 of the patch series located here: >> https://lore.kernel.org/qemu-devel/cover.1686577753.git.maciej.szmigiero@oracle.com/ >> > > We're now in QEMU soft-freeze, which means the memslot series might take a bit to land. I'm going to follow-up on that soonish. Ack, [1] even says that we're in a hard-freeze already. >> >> Changes from v5: >> * Incorporate David's rework of the driver on top of his virtio-mem-memslots >> patches (specifically, commit 6769107d1a4f), making use of a memory region >> container created upfront to avoid calling memory_device{,_pre}_plug() >> functions from the driver and introducing a driver-specific MemoryDeviceInfo >> sub-type. >> >> * Include two additional David's memory-device patches necessary for the >> aforementioned conversion in this patch set. >> >> * Use multiple memslots to cover the hot-add memory backend in order to >> reduce metadata size for the not-yet-hot-added part of the memory backend. >> >> * Add David's "Co-developed-by:" to patches where he contributed some changes. >> >> * Use OBJECT_DEFINE_TYPE_WITH_INTERFACES() and OBJECT_DECLARE_SIMPLE_TYPE() >> macros instead of open-coding the equivalent functionality. >> >> * Drop no longer necessary patch adding g_autoptr() cleanup function for the >> Error type. >> >> >> David Hildenbrand (2): >> memory-device: Support empty memory devices >> memory-device: Drop size alignment check >> >> Maciej S. Szmigiero (4): >> Add Hyper-V Dynamic Memory Protocol definitions >> qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo >> qapi: Add HV_BALLOON_STATUS_REPORT event >> Add a Hyper-V Dynamic Memory Protocol driver (hv-balloon) > > That is still a gigantic patch. Is there any way to split that into reasonable chunks? For example, move the whole hotplug/memslot part into > a dedicated patch? Will move hot-add support from the initial driver patch to a separate one. > See below on splitting off the PC changes. > >> >> Kconfig.host | 3 + >> hw/core/machine-hmp-cmds.c | 15 + >> hw/hyperv/Kconfig | 5 + >> hw/hyperv/hv-balloon.c | 2246 ++++++++++++++++++++++++++++++ >> hw/hyperv/meson.build | 1 + >> hw/hyperv/trace-events | 18 + >> hw/i386/pc.c | 22 + >> hw/mem/memory-device.c | 45 +- >> include/hw/hyperv/dynmem-proto.h | 423 ++++++ >> include/hw/hyperv/hv-balloon.h | 18 + >> include/hw/mem/memory-device.h | 7 +- >> meson.build | 28 +- >> meson_options.txt | 2 + >> qapi/machine.json | 64 +- >> scripts/meson-buildoptions.sh | 3 + > > It's probably best to separate the actual device implementation from wiring up the machine. That is, have a HV_BALLOON_SUPPORTED kconfig > (like VIRTIO_MEM_SUPPORTED), and activate that in a single commit for > PC, where you also modify hw/i386/pc.c. > > That commit would be called something like "pc: Support hv-balloon". If I remove the driver from Kconfig in the initial patch then AFAIK this initial patch will add a dead driver file that it is not possible to build yet, right? Or is there some configure-time override for lack of specific Kconfig option? Thanks, Maciej [1]: https://wiki.qemu.org/Planning/8.1
>> That commit would be called something like "pc: Support hv-balloon". > > If I remove the driver from Kconfig in the initial patch then AFAIK > this initial patch will add a dead driver file that it is not possible > to build yet, right? Yes, that's also what we did for virtio-mem: (bottom to top) 0ed48fd32e pc: Support for virtio-mem-pci 16647a8224 numa: Handle virtio-mem in NUMA stats 2e70874b16 hmp: Handle virtio-mem when printing memory device info 751c7bdd04 MAINTAINERS: Add myself as virtio-mem maintainer 0b9a2443a4 virtio-pci: Proxy for virtio-mem 910b25766b virtio-mem: Paravirtualized memory hot(un)plug And virtio-pmem: (bottom to top) a0a49813f7 pc: Support for virtio-pmem-pci cae02c3480 numa: Handle virtio-pmem in NUMA stats d766b22bbd hmp: Handle virtio-pmem when printing memory device infos adf0748a49 virtio-pci: Proxy for virtio-pmem 9f583bdd47 virtio-pmem: sync linux headers 5f503cd9f3 virtio-pmem: add virtio device As you're adding all in a single series, that's perfectly fine.
From: "Maciej S. Szmigiero" <maciej.szmigiero@oracle.com> This is a continuation of the v5 of the patch series located here: https://lore.kernel.org/qemu-devel/cover.1686577753.git.maciej.szmigiero@oracle.com/ Changes from v5: * Incorporate David's rework of the driver on top of his virtio-mem-memslots patches (specifically, commit 6769107d1a4f), making use of a memory region container created upfront to avoid calling memory_device{,_pre}_plug() functions from the driver and introducing a driver-specific MemoryDeviceInfo sub-type. * Include two additional David's memory-device patches necessary for the aforementioned conversion in this patch set. * Use multiple memslots to cover the hot-add memory backend in order to reduce metadata size for the not-yet-hot-added part of the memory backend. * Add David's "Co-developed-by:" to patches where he contributed some changes. * Use OBJECT_DEFINE_TYPE_WITH_INTERFACES() and OBJECT_DECLARE_SIMPLE_TYPE() macros instead of open-coding the equivalent functionality. * Drop no longer necessary patch adding g_autoptr() cleanup function for the Error type. David Hildenbrand (2): memory-device: Support empty memory devices memory-device: Drop size alignment check Maciej S. Szmigiero (4): Add Hyper-V Dynamic Memory Protocol definitions qapi: Add HvBalloonDeviceInfo sub-type to MemoryDeviceInfo qapi: Add HV_BALLOON_STATUS_REPORT event Add a Hyper-V Dynamic Memory Protocol driver (hv-balloon) Kconfig.host | 3 + hw/core/machine-hmp-cmds.c | 15 + hw/hyperv/Kconfig | 5 + hw/hyperv/hv-balloon.c | 2246 ++++++++++++++++++++++++++++++ hw/hyperv/meson.build | 1 + hw/hyperv/trace-events | 18 + hw/i386/pc.c | 22 + hw/mem/memory-device.c | 45 +- include/hw/hyperv/dynmem-proto.h | 423 ++++++ include/hw/hyperv/hv-balloon.h | 18 + include/hw/mem/memory-device.h | 7 +- meson.build | 28 +- meson_options.txt | 2 + qapi/machine.json | 64 +- scripts/meson-buildoptions.sh | 3 + 15 files changed, 2888 insertions(+), 12 deletions(-) create mode 100644 hw/hyperv/hv-balloon.c create mode 100644 include/hw/hyperv/dynmem-proto.h create mode 100644 include/hw/hyperv/hv-balloon.h