On 1/8/2025 12:47 PM, Alexey Kardashevskiy wrote: > On 13/12/24 18:08, Chenyi Qiang wrote: >> Rename the helper to memory_region_section_intersect_range() to make it >> more generic. >> >> Signed-off-by: Chenyi Qiang <chenyi.qiang@xxxxxxxxx> >> --- >> hw/virtio/virtio-mem.c | 32 +++++--------------------------- >> include/exec/memory.h | 13 +++++++++++++ >> system/memory.c | 17 +++++++++++++++++ >> 3 files changed, 35 insertions(+), 27 deletions(-) >> >> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c >> index 80ada89551..e3d1ccaeeb 100644 >> --- a/hw/virtio/virtio-mem.c >> +++ b/hw/virtio/virtio-mem.c >> @@ -242,28 +242,6 @@ static int >> virtio_mem_for_each_plugged_range(VirtIOMEM *vmem, void *arg, >> return ret; >> } >> -/* >> - * Adjust the memory section to cover the intersection with the given >> range. >> - * >> - * Returns false if the intersection is empty, otherwise returns true. >> - */ >> -static bool virtio_mem_intersect_memory_section(MemoryRegionSection *s, >> - uint64_t offset, >> uint64_t size) >> -{ >> - uint64_t start = MAX(s->offset_within_region, offset); >> - uint64_t end = MIN(s->offset_within_region + int128_get64(s->size), >> - offset + size); >> - >> - if (end <= start) { >> - return false; >> - } >> - >> - s->offset_within_address_space += start - s->offset_within_region; >> - s->offset_within_region = start; >> - s->size = int128_make64(end - start); >> - return true; >> -} >> - >> typedef int (*virtio_mem_section_cb)(MemoryRegionSection *s, void >> *arg); >> static int virtio_mem_for_each_plugged_section(const VirtIOMEM *vmem, >> @@ -285,7 +263,7 @@ static int >> virtio_mem_for_each_plugged_section(const VirtIOMEM *vmem, >> first_bit + 1) - 1; >> size = (last_bit - first_bit + 1) * vmem->block_size; >> - if (!virtio_mem_intersect_memory_section(&tmp, offset, >> size)) { >> + if (!memory_region_section_intersect_range(&tmp, offset, >> size)) { >> break; >> } >> ret = cb(&tmp, arg); >> @@ -317,7 +295,7 @@ static int >> virtio_mem_for_each_unplugged_section(const VirtIOMEM *vmem, >> first_bit + 1) - 1; >> size = (last_bit - first_bit + 1) * vmem->block_size; >> - if (!virtio_mem_intersect_memory_section(&tmp, offset, >> size)) { >> + if (!memory_region_section_intersect_range(&tmp, offset, >> size)) { >> break; >> } >> ret = cb(&tmp, arg); >> @@ -353,7 +331,7 @@ static void virtio_mem_notify_unplug(VirtIOMEM >> *vmem, uint64_t offset, >> QLIST_FOREACH(rdl, &vmem->rdl_list, next) { >> MemoryRegionSection tmp = *rdl->section; >> - if (!virtio_mem_intersect_memory_section(&tmp, offset, >> size)) { >> + if (!memory_region_section_intersect_range(&tmp, offset, >> size)) { >> continue; >> } >> rdl->notify_discard(rdl, &tmp); >> @@ -369,7 +347,7 @@ static int virtio_mem_notify_plug(VirtIOMEM *vmem, >> uint64_t offset, >> QLIST_FOREACH(rdl, &vmem->rdl_list, next) { >> MemoryRegionSection tmp = *rdl->section; >> - if (!virtio_mem_intersect_memory_section(&tmp, offset, >> size)) { >> + if (!memory_region_section_intersect_range(&tmp, offset, >> size)) { >> continue; >> } >> ret = rdl->notify_populate(rdl, &tmp); >> @@ -386,7 +364,7 @@ static int virtio_mem_notify_plug(VirtIOMEM *vmem, >> uint64_t offset, >> if (rdl2 == rdl) { >> break; >> } >> - if (!virtio_mem_intersect_memory_section(&tmp, offset, >> size)) { >> + if (!memory_region_section_intersect_range(&tmp, offset, >> size)) { >> continue; >> } >> rdl2->notify_discard(rdl2, &tmp); >> diff --git a/include/exec/memory.h b/include/exec/memory.h >> index e5e865d1a9..ec7bc641e8 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -1196,6 +1196,19 @@ MemoryRegionSection >> *memory_region_section_new_copy(MemoryRegionSection *s); >> */ >> void memory_region_section_free_copy(MemoryRegionSection *s); >> +/** >> + * memory_region_section_intersect_range: Adjust the memory section >> to cover >> + * the intersection with the given range. >> + * >> + * @s: the #MemoryRegionSection to be adjusted >> + * @offset: the offset of the given range in the memory region >> + * @size: the size of the given range >> + * >> + * Returns false if the intersection is empty, otherwise returns true. >> + */ >> +bool memory_region_section_intersect_range(MemoryRegionSection *s, >> + uint64_t offset, uint64_t >> size); >> + >> /** >> * memory_region_init: Initialize a memory region >> * >> diff --git a/system/memory.c b/system/memory.c >> index 85f6834cb3..ddcec90f5e 100644 >> --- a/system/memory.c >> +++ b/system/memory.c >> @@ -2898,6 +2898,23 @@ void >> memory_region_section_free_copy(MemoryRegionSection *s) >> g_free(s); >> } >> +bool memory_region_section_intersect_range(MemoryRegionSection *s, >> + uint64_t offset, uint64_t >> size) >> +{ >> + uint64_t start = MAX(s->offset_within_region, offset); >> + uint64_t end = MIN(s->offset_within_region + int128_get64(s->size), >> + offset + size); > > imho @end needs to be Int128 and s/MIN/int128_min/, etc to be totally > correct (although it is going to look horrendous). May be it was alright > when it was just virtio but now it is a wider API. I understand this is > cut-n-paste and unlikely scenario of offset+size crossing 1<<64 but > still. Thanks, Make sense. I'll change it in next version. > > >> + >> + if (end <= start) { >> + return false; >> + } >> + >> + s->offset_within_address_space += start - s->offset_within_region; >> + s->offset_within_region = start; >> + s->size = int128_make64(end - start); >> + return true; >> +} >> + >> bool memory_region_present(MemoryRegion *container, hwaddr addr) >> { >> MemoryRegion *mr; >