Hi On Mon, Sep 14, 2026 at 7:35 PM Cam Miller <[email protected]> wrote: > > Fix bug inside replay_by_populated_state() that forgets to initialize > MemoryRegionSection field offset_within_address_space. Follow the > established pattern of calling memory_region_section_intersect_range() > to accomplish this task. > > Prior to commit cc9c77f4ddf0 ("system/memory: implement > RamDiscardManager multi-source aggregation"), > replay_by_populated_state() had called > memory_region_section_intersect_range() in order to initialize > interdependent fields offset_within_address_space, offset_within_region, > and size together, as shown below. > > s->offset_within_address_space += start - s->offset_within_region; > s->offset_within_region = start; > s->size = int128_sub(end, int128_make64(start)); > > cc9c77f4ddf0 reimplements replay_by_populated_state() initializing the > fields of the given MemoryRegionSection instance by hand instead of via > memory_region_section_intersect_range(). In doing so, it leaves > offset_within_address_space uninitialized for some reason, as you can > see below. > > MemoryRegionSection subsection = { > .mr = section->mr, > .offset_within_region = offset, > .size = int128_make64(MIN(granularity, end_offset - offset)), > }; > > Consequently offset_within_address_space defaults to GPA 0x0, which is > incorrect. For example, on s390x, base RAM begins at GPA 0x0 and it is > problematic to report that a virtio-iommu MR section lives there > instead. > > cc9c77f4ddf0 deliberately calls memory_region_section_intersect_range() > from other related code paths inside the same file, namely > replay_source_by_state() and rdl_populate_cb()/rdl_discard_cb(). It is > unclear why the new replay_by_populated_state() implementation does not > conform to this same pattern. > > The effects of the bug include qemu crashes on multiple architectures. > The following assertion failure occurs when driving the > guest_phys_blocks_append() code path, for guests with virtio-mem device > that has some memory plugged. > > DBG: guest_phys_block_add_section: predecessor->target_end=280000000 > target_start=0 > ** > ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: > assertion failed: (predecessor->target_end <= target_start) > Bail out! > ERROR:../system/memory_mapping.c:222:guest_phys_block_add_section: assertion > failed: (predecessor->target_end <= target_start) > 2026-09-11 16:03:57.405+0000: shutting down, reason=crashed > > This crash can be triggered on x86 via the dump-guest-memory QMP > command. The same crash can be triggered on s390x by restoring VM State > that has been migrated to a local file. (I used libvirt to manage this > migration restore operation, namely command virsh managedsave then virsh > start.) Applying the fix resolved the crash on both platforms. > > Fixes: cc9c77f4ddf0 ("system/memory: implement RamDiscardManager multi-source > aggregation") > Reported-by: Boris Fiuczynski <[email protected]> > Signed-off-by: Cam Miller <[email protected]>
Reviewed-by: Marc-André Lureau <[email protected]> thanks > --- > system/ram-discard-manager.c | 29 +++++++++++++++-------------- > 1 file changed, 15 insertions(+), 14 deletions(-) > > diff --git a/system/ram-discard-manager.c b/system/ram-discard-manager.c > index 4e8816e5a2..e9a609e5cd 100644 > --- a/system/ram-discard-manager.c > +++ b/system/ram-discard-manager.c > @@ -238,14 +238,15 @@ static int replay_by_populated_state(const > RamDiscardManager *rdm, > } > } else { > if (in_run) { > - MemoryRegionSection run_section = { > - .mr = section->mr, > - .offset_within_region = run_start, > - .size = int128_make64(offset - run_start), > - }; > - ret = replay_fn(&run_section, user_opaque); > - if (ret) { > - return ret; > + MemoryRegionSection run_section = *section; > + > + if (memory_region_section_intersect_range(&run_section, > + run_start, > + offset - > run_start)) { > + ret = replay_fn(&run_section, user_opaque); > + if (ret) { > + return ret; > + } > } > in_run = false; > } > @@ -257,12 +258,12 @@ static int replay_by_populated_state(const > RamDiscardManager *rdm, > } > > if (in_run) { > - MemoryRegionSection run_section = { > - .mr = section->mr, > - .offset_within_region = run_start, > - .size = int128_make64(end_offset - run_start), > - }; > - ret = replay_fn(&run_section, user_opaque); > + MemoryRegionSection run_section = *section; > + > + if (memory_region_section_intersect_range(&run_section, run_start, > + end_offset - run_start)) { > + ret = replay_fn(&run_section, user_opaque); > + } > } > > return ret; > > --- > base-commit: 2242ae1f7bdcf76e78cf8a987118952fc6c9a469 > change-id: 20260914-vmem_fix-c80d75571aa6 > > Best regards, > -- > Cam Miller <[email protected]> > > -- Marc-André Lureau
