Re: [PATCH v2 15/16] memory,vhost: Allow for marking memory device memory regions unmergeable

2023-08-28 Thread Philippe Mathieu-Daudé

On 25/8/23 15:21, David Hildenbrand wrote:

Let's allow for marking memory regions unmergeable, to teach
flatview code and vhost to not merge adjacent aliases to the same memory
region into a larger memory section; instead, we want separate aliases to
stay separate such that we can atomically map/unmap aliases without
affecting other aliases.

This is desired for virtio-mem mapping device memory located on a RAM
memory region via multiple aliases into a memory region container,
resulting in separate memslots that can get (un)mapped atomically.

As an example with virtio-mem, the layout would look something like this:
   [...]
   00024000-0020bfff (prio 0, i/o): device-memory
 00024000-00043fff (prio 0, i/o): virtio-mem
   00024000-00027fff (prio 0, ram): alias memslot-0 @mem2 
-3fff
   00028000-0002bfff (prio 0, ram): alias memslot-1 @mem2 
4000-7fff
   0002c000-0002 (prio 0, ram): alias memslot-2 @mem2 
8000-bfff
   [...]

Without unmergable memory regions, all three memslots would get merged into
a single memory section. For example, when mapping another alias (e.g.,
virtio-mem-memslot-3) or when unmapping any of the mapped aliases,
memory listeners will first get notified about the removal of the big
memory section to then get notified about re-adding of the new
(differently merged) memory section(s).

In an ideal world, memory listeners would be able to deal with that
atomically, like KVM nowadays does. However, (a) supporting this for other
memory listeners (vhost-user, vfio) is fairly hard: temporary removal
can result in all kinds of issues on concurrent access to guest memory;
and (b) this handling is undesired, because temporarily removing+readding
can consume quite some time on bigger memslots and is not efficient
(e.g., vfio unpinning and repinning pages ...).

Let's allow for marking a memory region unmergeable, such that we
can atomically (un)map aliases to the same memory region, similar to
(un)mapping individual DIMMs.

Similarly, teach vhost code to not redo what flatview core stopped doing:
don't merge such sections. Merging in vhost code is really only relevant
for handling random holes in boot memory where; without this merging,
the vhost-user backend wouldn't be able to mmap() some boot memory
backed on hugetlb.

We'll use this for virtio-mem next.

Signed-off-by: David Hildenbrand 
---
  hw/virtio/vhost.c |  4 ++--
  include/exec/memory.h | 22 ++
  softmmu/memory.c  | 31 +--
  3 files changed, 49 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 





[PATCH v2 15/16] memory, vhost: Allow for marking memory device memory regions unmergeable

2023-08-25 Thread David Hildenbrand
Let's allow for marking memory regions unmergeable, to teach
flatview code and vhost to not merge adjacent aliases to the same memory
region into a larger memory section; instead, we want separate aliases to
stay separate such that we can atomically map/unmap aliases without
affecting other aliases.

This is desired for virtio-mem mapping device memory located on a RAM
memory region via multiple aliases into a memory region container,
resulting in separate memslots that can get (un)mapped atomically.

As an example with virtio-mem, the layout would look something like this:
  [...]
  00024000-0020bfff (prio 0, i/o): device-memory
00024000-00043fff (prio 0, i/o): virtio-mem
  00024000-00027fff (prio 0, ram): alias memslot-0 @mem2 
-3fff
  00028000-0002bfff (prio 0, ram): alias memslot-1 @mem2 
4000-7fff
  0002c000-0002 (prio 0, ram): alias memslot-2 @mem2 
8000-bfff
  [...]

Without unmergable memory regions, all three memslots would get merged into
a single memory section. For example, when mapping another alias (e.g.,
virtio-mem-memslot-3) or when unmapping any of the mapped aliases,
memory listeners will first get notified about the removal of the big
memory section to then get notified about re-adding of the new
(differently merged) memory section(s).

In an ideal world, memory listeners would be able to deal with that
atomically, like KVM nowadays does. However, (a) supporting this for other
memory listeners (vhost-user, vfio) is fairly hard: temporary removal
can result in all kinds of issues on concurrent access to guest memory;
and (b) this handling is undesired, because temporarily removing+readding
can consume quite some time on bigger memslots and is not efficient
(e.g., vfio unpinning and repinning pages ...).

Let's allow for marking a memory region unmergeable, such that we
can atomically (un)map aliases to the same memory region, similar to
(un)mapping individual DIMMs.

Similarly, teach vhost code to not redo what flatview core stopped doing:
don't merge such sections. Merging in vhost code is really only relevant
for handling random holes in boot memory where; without this merging,
the vhost-user backend wouldn't be able to mmap() some boot memory
backed on hugetlb.

We'll use this for virtio-mem next.

Signed-off-by: David Hildenbrand 
---
 hw/virtio/vhost.c |  4 ++--
 include/exec/memory.h | 22 ++
 softmmu/memory.c  | 31 +--
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 24013b39d6..503a160c96 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -707,7 +707,7 @@ static void vhost_region_add_section(struct vhost_dev *dev,
mrs_size, mrs_host);
 }
 
-if (dev->n_tmp_sections) {
+if (dev->n_tmp_sections && !section->unmergeable) {
 /* Since we already have at least one section, lets see if
  * this extends it; since we're scanning in order, we only
  * have to look at the last one, and the FlatView that calls
@@ -740,7 +740,7 @@ static void vhost_region_add_section(struct vhost_dev *dev,
 size_t offset = mrs_gpa - prev_gpa_start;
 
 if (prev_host_start + offset == mrs_host &&
-section->mr == prev_sec->mr) {
+section->mr == prev_sec->mr && !prev_sec->unmergeable) {
 uint64_t max_end = MAX(prev_host_end, mrs_host + mrs_size);
 need_add = false;
 prev_sec->offset_within_address_space =
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 5feb704585..916d565533 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -95,6 +95,7 @@ struct ReservedRegion {
  * relative to the region's address space
  * @readonly: writes to this section are ignored
  * @nonvolatile: this section is non-volatile
+ * @unmergeable: this section should not get merged with adjacent sections
  */
 struct MemoryRegionSection {
 Int128 size;
@@ -104,6 +105,7 @@ struct MemoryRegionSection {
 hwaddr offset_within_address_space;
 bool readonly;
 bool nonvolatile;
+bool unmergeable;
 };
 
 typedef struct IOMMUTLBEntry IOMMUTLBEntry;
@@ -767,6 +769,7 @@ struct MemoryRegion {
 bool nonvolatile;
 bool rom_device;
 bool flush_coalesced_mmio;
+bool unmergeable;
 uint8_t dirty_log_mask;
 bool is_iommu;
 RAMBlock *ram_block;
@@ -2344,6 +2347,25 @@ void memory_region_set_size(MemoryRegion *mr, uint64_t 
size);
 void memory_region_set_alias_offset(MemoryRegion *mr,
 hwaddr offset);
 
+/*
+ * memory_region_set_unmergeable: Set a memory region unmergeable
+ *
+ * Mark a memory region unmergeable, resulting in the memory region (or
+ * everything