On Wed, Sep 09, 2026 at 04:54:18PM +0300, Alex Fishman wrote: > It is not desirable to merge every coherent adjacent section. > Virtio-mem sections are marked unmergeable so that their lifetimes can > be observed independently.
what does this mean? > For example, if regions 1 and 2 are merged, unconditionally merging an > adjacent region 3 would change the vhost memory table from: > > [ region 1 + region 2 ] > > to: > > [ region 1 + region 2 + region 3 ] > > This requires removing the existing region and adding an enlarged one, > even though region 3 is unrelated to the vring. So what? > At the failing boundary, regions 1 and 2 must be represented as one > vhost region because a vring part crosses them; otherwise ring > verification fails. The patch limits coalescing to that necessary > boundary. Region 3 remains separate and can be added without reshaping > the region containing the vring. > > I'll fix the empty lines in the next version. > > Thanks, > Alex > You are still changing the "lifetimes" thing presumably? why is that not a problem here? > On Wed, Sep 9, 2026 at 4:31 PM Michael S. Tsirkin <[email protected]> wrote: > > On Wed, Sep 09, 2026 at 03:59:22PM +0300, Alex Fishman wrote: > > Virtio-mem dynamic memslots are marked unmergeable so listeners can > > track their lifetimes independently. A vring part crossing the boundary > > between two such slots consequently cannot be contained in a single > > vhost memory region. > > > > Coalesce adjacent unmergeable sections only when a descriptor table, > > available ring, or used ring spans their boundary and the sections > > preserve a coherent GPA-to-HVA translation. Keep unrelated slots > > separate so activating them does not reshape the region containing the > > vring. > > > I don't get what does it have to do with vrings. If merging them like > this is ok, then it's always ok? > > > > > Fixes: 533f5d667909 ("memory,vhost: Allow for marking memory device > memory regions unmergeable") > > > > Buglink: https://redhat.atlassian.net/browse/RHEL-146583 > > > > Signed-off-by: Alex Fishman <[email protected]> > > > No empty lines between trailers,please. > > > --- > > hw/virtio/vhost.c | 72 +++++++++++++++++++++++++++++++++++++++++++---- > > 1 file changed, 67 insertions(+), 5 deletions(-) > > > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > index 371dca17dd..76910e2628 100644 > > --- a/hw/virtio/vhost.c > > +++ b/hw/virtio/vhost.c > > @@ -796,6 +796,70 @@ out: > > g_free(old_sections); > > } > > > > +static bool vhost_vring_part_crosses_boundary(uint64_t ring_gpa, > > + uint64_t ring_size, > > + uint64_t boundary) > > +{ > > + return ring_size && ring_gpa < boundary && > > + range_get_last(ring_gpa, ring_size) >= boundary; > > +} > > + > > +static bool vhost_vring_crosses_boundary(struct vhost_dev *dev, > > + uint64_t boundary) > > +{ > > + int i; > > + > > + if (vhost_dev_has_iommu(dev)) { > > + return false; > > + } > > + > > + for (i = 0; i < dev->nvqs; i++) { > > + struct vhost_virtqueue *vq = &dev->vqs[i]; > > + > > + if (vhost_vring_part_crosses_boundary(vq->desc_phys, vq-> > desc_size, > > + boundary) || > > + vhost_vring_part_crosses_boundary(vq->avail_phys, vq-> > avail_size, > > + boundary) || > > + vhost_vring_part_crosses_boundary(vq->used_phys, vq-> > used_size, > > + boundary)) { > > + return true; > > + } > > + } > > + > > + return false; > > +} > > + > > +static bool vhost_sections_can_merge(struct vhost_dev *dev, > > + const MemoryRegionSection > *prev_sec, > > + const MemoryRegionSection > *section, > > + uint64_t section_gpa, > > + uintptr_t section_host) > > +{ > > + uint64_t prev_gpa_start = prev_sec->offset_within_address_space; > > + uintptr_t prev_host_start = > > + (uintptr_t)memory_region_get_ram_ptr(prev_sec->mr) + > > + prev_sec->offset_within_region; > > + uint64_t offset; > > + > > + if (section->mr != prev_sec->mr || section_gpa < prev_gpa_start) { > > + return false; > > + } > > + > > + offset = section_gpa - prev_gpa_start; > > + > > + if (prev_host_start + offset != section_host) { > > + return false; > > + } > > + > > + if (!prev_sec->unmergeable && !section->unmergeable) { > > + return true; > > + } > > + > > + /* Only override an unmergeable boundary when a ring part spans it. > */ > > + return vhost_vring_crosses_boundary( > > + dev, section->offset_within_address_space); > > +} > > + > > /* Adds the section data to the tmp_section structure. > > * It relies on the listener calling us in memory address order > > * and for each region (via the _add and _nop methods) to > > @@ -833,7 +897,7 @@ static void vhost_region_add_section(struct > vhost_dev > *dev, > > mrs_size, mrs_host); > > } > > > > - if (dev->n_tmp_sections && !section->unmergeable) { > > + if (dev->n_tmp_sections) { > > /* 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 > > @@ -862,11 +926,9 @@ static void vhost_region_add_section(struct > vhost_dev *dev, > > /* A way to cleanly fail here would be better */ > > return; > > } > > - /* Offset from the start of the previous GPA to this GPA */ > > - size_t offset = mrs_gpa - prev_gpa_start; > > > > - if (prev_host_start + offset == mrs_host && > > - section->mr == prev_sec->mr && !prev_sec->unmergeable) > { > > + if (vhost_sections_can_merge(dev, prev_sec, section, > > + mrs_gpa, mrs_host)) { > > uint64_t max_end = MAX(prev_host_end, mrs_host + > mrs_size); > > need_add = false; > > prev_sec->offset_within_address_space = > > > > > > > > -- > > 2.52.0 > >
