[v6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

[PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Maciej S. Szmigiero 2 years, 6 months ago

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

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by David Hildenbrand 2 years, 6 months ago

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".

-- 
Cheers,

David / dhildenb

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Maciej S. Szmigiero 2 years, 6 months ago

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

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by David Hildenbrand 2 years, 6 months ago

>> 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.

-- 
Cheers,

David / dhildenb

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Markus Armbruster 2 years, 6 months ago

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

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Markus Armbruster 2 years, 6 months ago

Doesn't apply to master.  Care to rebase?

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Maciej S. Szmigiero 2 years, 6 months ago

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

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Markus Armbruster 2 years, 6 months ago

"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

Re: [PATCH v6 0/6] Hyper-V Dynamic Memory Protocol driver (hv-balloon 🎈️)

Posted by Maciej S. Szmigiero 2 years, 6 months ago

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