Re: [PATCH v1 1/2] vhost: coalesce unmergeable sections across vring boundaries

["Michael S. Tsirkin" <mst@redhat.com>]

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 <mst@redhat.com> 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 <afishman@redhat.com>
> 
> 
>     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
> 
>