| Message ID | 20250407074939.18657-2-chenyi.qiang@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Enable shared device assignment | expand |
On 7/4/25 17:49, Chenyi Qiang wrote: > Rename the helper to memory_region_section_intersect_range() to make it > more generic. Meanwhile, define the @end as Int128 and replace the > related operations with Int128_* format since the helper is exported as > a wider API. > > Suggested-by: Alexey Kardashevskiy <aik@amd.com> > Reviewed-by: David Hildenbrand <david@redhat.com> > Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> ./scripts/checkpatch.pl complains "WARNING: line over 80 characters" with that fixed, Reviewed-by: Alexey Kardashevskiy <aik@amd.com> > --- > Changes in v4: > - No change. > > Changes in v3: > - No change > > Changes in v2: > - Make memory_region_section_intersect_range() an inline function. > - Add Reviewed-by from David > - Define the @end as Int128 and use the related Int128_* ops as a wilder > API (Alexey) > --- > hw/virtio/virtio-mem.c | 32 +++++--------------------------- > include/exec/memory.h | 27 +++++++++++++++++++++++++++ > 2 files changed, 32 insertions(+), 27 deletions(-) > > diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c > index b1a003736b..21f16e4912 100644 > --- a/hw/virtio/virtio-mem.c > +++ b/hw/virtio/virtio-mem.c > @@ -244,28 +244,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, > @@ -287,7 +265,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); > @@ -319,7 +297,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); > @@ -355,7 +333,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); > @@ -371,7 +349,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); > @@ -388,7 +366,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 3ee1901b52..3bebc43d59 100644 > --- a/include/exec/memory.h > +++ b/include/exec/memory.h > @@ -1202,6 +1202,33 @@ 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. > + */ > +static inline bool memory_region_section_intersect_range(MemoryRegionSection *s, > + uint64_t offset, uint64_t size) > +{ > + uint64_t start = MAX(s->offset_within_region, offset); > + Int128 end = int128_min(int128_add(int128_make64(s->offset_within_region), s->size), > + int128_add(int128_make64(offset), int128_make64(size))); > + > + if (int128_le(end, int128_make64(start))) { > + return false; > + } > + > + s->offset_within_address_space += start - s->offset_within_region; > + s->offset_within_region = start; > + s->size = int128_sub(end, int128_make64(start)); > + return true; > +} > + > /** > * memory_region_init: Initialize a memory region > *
On 4/9/2025 10:47 AM, Alexey Kardashevskiy wrote: > > On 7/4/25 17:49, Chenyi Qiang wrote: >> Rename the helper to memory_region_section_intersect_range() to make it >> more generic. Meanwhile, define the @end as Int128 and replace the >> related operations with Int128_* format since the helper is exported as >> a wider API. >> >> Suggested-by: Alexey Kardashevskiy <aik@amd.com> >> Reviewed-by: David Hildenbrand <david@redhat.com> >> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> > > ./scripts/checkpatch.pl complains "WARNING: line over 80 characters" > > with that fixed, I observed many places in QEMU ignore the WARNING for over 80 characters, so I also ignored them in my series. After checking the rule in docs/devel/style.rst, I think I should try best to make it not longer than 80. But if it is hard to do so due to long function or symbol names, it is acceptable to not wrap it. Then, I would modify the first warning code. For the latter two warnings, see code below > > Reviewed-by: Alexey Kardashevskiy <aik@amd.com> > >> --- >> Changes in v4: >> - No change. >> >> Changes in v3: >> - No change >> >> Changes in v2: >> - Make memory_region_section_intersect_range() an inline function. >> - Add Reviewed-by from David >> - Define the @end as Int128 and use the related Int128_* ops as a >> wilder >> API (Alexey) >> --- >> hw/virtio/virtio-mem.c | 32 +++++--------------------------- >> include/exec/memory.h | 27 +++++++++++++++++++++++++++ >> 2 files changed, 32 insertions(+), 27 deletions(-) >> >> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c >> index b1a003736b..21f16e4912 100644 >> --- a/hw/virtio/virtio-mem.c >> +++ b/hw/virtio/virtio-mem.c >> @@ -244,28 +244,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, >> @@ -287,7 +265,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); >> @@ -319,7 +297,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); >> @@ -355,7 +333,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); >> @@ -371,7 +349,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); >> @@ -388,7 +366,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 3ee1901b52..3bebc43d59 100644 >> --- a/include/exec/memory.h >> +++ b/include/exec/memory.h >> @@ -1202,6 +1202,33 @@ 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. >> + */ >> +static inline bool >> memory_region_section_intersect_range(MemoryRegionSection *s, >> + uint64_t >> offset, uint64_t size) >> +{ >> + uint64_t start = MAX(s->offset_within_region, offset); >> + Int128 end = int128_min(int128_add(int128_make64(s- >> >offset_within_region), s->size), >> + int128_add(int128_make64(offset), >> int128_make64(size))); The Int128_* format helper make the line over 80. I think it's better not wrap it for readability. >> + >> + if (int128_le(end, int128_make64(start))) { >> + return false; >> + } >> + >> + s->offset_within_address_space += start - s->offset_within_region; >> + s->offset_within_region = start; >> + s->size = int128_sub(end, int128_make64(start)); >> + return true; >> +} >> + >> /** >> * memory_region_init: Initialize a memory region >> * >
On 9/4/25 16:26, Chenyi Qiang wrote: > > > On 4/9/2025 10:47 AM, Alexey Kardashevskiy wrote: >> >> On 7/4/25 17:49, Chenyi Qiang wrote: >>> Rename the helper to memory_region_section_intersect_range() to make it >>> more generic. Meanwhile, define the @end as Int128 and replace the >>> related operations with Int128_* format since the helper is exported as >>> a wider API. >>> >>> Suggested-by: Alexey Kardashevskiy <aik@amd.com> >>> Reviewed-by: David Hildenbrand <david@redhat.com> >>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> >> >> ./scripts/checkpatch.pl complains "WARNING: line over 80 characters" >> >> with that fixed, > > I observed many places in QEMU ignore the WARNING for over 80 > characters, so I also ignored them in my series. > > After checking the rule in docs/devel/style.rst, I think I should try > best to make it not longer than 80. But if it is hard to do so due to > long function or symbol names, it is acceptable to not wrap it. > > Then, I would modify the first warning code. For the latter two > warnings, see code below > >> >> Reviewed-by: Alexey Kardashevskiy <aik@amd.com> >> >>> --- >>> Changes in v4: >>> - No change. >>> >>> Changes in v3: >>> - No change >>> >>> Changes in v2: >>> - Make memory_region_section_intersect_range() an inline function. >>> - Add Reviewed-by from David >>> - Define the @end as Int128 and use the related Int128_* ops as a >>> wilder >>> API (Alexey) >>> --- >>> hw/virtio/virtio-mem.c | 32 +++++--------------------------- >>> include/exec/memory.h | 27 +++++++++++++++++++++++++++ >>> 2 files changed, 32 insertions(+), 27 deletions(-) >>> >>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c >>> index b1a003736b..21f16e4912 100644 >>> --- a/hw/virtio/virtio-mem.c >>> +++ b/hw/virtio/virtio-mem.c >>> @@ -244,28 +244,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, >>> @@ -287,7 +265,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); >>> @@ -319,7 +297,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); >>> @@ -355,7 +333,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); >>> @@ -371,7 +349,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); >>> @@ -388,7 +366,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 3ee1901b52..3bebc43d59 100644 >>> --- a/include/exec/memory.h >>> +++ b/include/exec/memory.h >>> @@ -1202,6 +1202,33 @@ 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. >>> + */ >>> +static inline bool >>> memory_region_section_intersect_range(MemoryRegionSection *s, >>> + uint64_t >>> offset, uint64_t size) >>> +{ >>> + uint64_t start = MAX(s->offset_within_region, offset); >>> + Int128 end = int128_min(int128_add(int128_make64(s- >>>> offset_within_region), s->size), >>> + int128_add(int128_make64(offset), >>> int128_make64(size))); > > The Int128_* format helper make the line over 80. I think it's better > not wrap it for readability. I'd just reduce indent to previous line + 4 spaces vs the current "under the opening bracket" rule which I dislike anyway :) Thanks, >>> + >>> + if (int128_le(end, int128_make64(start))) { >>> + return false; >>> + } >>> + >>> + s->offset_within_address_space += start - s->offset_within_region; >>> + s->offset_within_region = start; >>> + s->size = int128_sub(end, int128_make64(start)); >>> + return true; >>> +} >>> + >>> /** >>> * memory_region_init: Initialize a memory region >>> * >> >
On 4/9/2025 2:45 PM, Alexey Kardashevskiy wrote: > > > On 9/4/25 16:26, Chenyi Qiang wrote: >> >> >> On 4/9/2025 10:47 AM, Alexey Kardashevskiy wrote: >>> >>> On 7/4/25 17:49, Chenyi Qiang wrote: >>>> Rename the helper to memory_region_section_intersect_range() to make it >>>> more generic. Meanwhile, define the @end as Int128 and replace the >>>> related operations with Int128_* format since the helper is exported as >>>> a wider API. >>>> >>>> Suggested-by: Alexey Kardashevskiy <aik@amd.com> >>>> Reviewed-by: David Hildenbrand <david@redhat.com> >>>> Signed-off-by: Chenyi Qiang <chenyi.qiang@intel.com> >>> >>> ./scripts/checkpatch.pl complains "WARNING: line over 80 characters" >>> >>> with that fixed, >> >> I observed many places in QEMU ignore the WARNING for over 80 >> characters, so I also ignored them in my series. >> >> After checking the rule in docs/devel/style.rst, I think I should try >> best to make it not longer than 80. But if it is hard to do so due to >> long function or symbol names, it is acceptable to not wrap it. >> >> Then, I would modify the first warning code. For the latter two >> warnings, see code below >> >>> >>> Reviewed-by: Alexey Kardashevskiy <aik@amd.com> >>> >>>> --- >>>> Changes in v4: >>>> - No change. >>>> >>>> Changes in v3: >>>> - No change >>>> >>>> Changes in v2: >>>> - Make memory_region_section_intersect_range() an inline >>>> function. >>>> - Add Reviewed-by from David >>>> - Define the @end as Int128 and use the related Int128_* ops as a >>>> wilder >>>> API (Alexey) >>>> --- >>>> hw/virtio/virtio-mem.c | 32 +++++--------------------------- >>>> include/exec/memory.h | 27 +++++++++++++++++++++++++++ >>>> 2 files changed, 32 insertions(+), 27 deletions(-) >>>> >>>> diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c >>>> index b1a003736b..21f16e4912 100644 >>>> --- a/hw/virtio/virtio-mem.c >>>> +++ b/hw/virtio/virtio-mem.c >>>> @@ -244,28 +244,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, >>>> @@ -287,7 +265,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); >>>> @@ -319,7 +297,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); >>>> @@ -355,7 +333,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); >>>> @@ -371,7 +349,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); >>>> @@ -388,7 +366,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 3ee1901b52..3bebc43d59 100644 >>>> --- a/include/exec/memory.h >>>> +++ b/include/exec/memory.h >>>> @@ -1202,6 +1202,33 @@ 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. >>>> + */ >>>> +static inline bool >>>> memory_region_section_intersect_range(MemoryRegionSection *s, >>>> + uint64_t >>>> offset, uint64_t size) >>>> +{ >>>> + uint64_t start = MAX(s->offset_within_region, offset); >>>> + Int128 end = int128_min(int128_add(int128_make64(s- >>>>> offset_within_region), s->size), [..] >>>> + int128_add(int128_make64(offset), >>>> int128_make64(size))); >> >> The Int128_* format helper make the line over 80. I think it's better >> not wrap it for readability. > > I'd just reduce indent to previous line + 4 spaces vs the current "under > the opening bracket" rule which I dislike anyway :) Thanks, I can make the adjustment for this line. As for the previous line which also reports a warning, do you think it needs to wrap? > >
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c index b1a003736b..21f16e4912 100644 --- a/hw/virtio/virtio-mem.c +++ b/hw/virtio/virtio-mem.c @@ -244,28 +244,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, @@ -287,7 +265,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); @@ -319,7 +297,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); @@ -355,7 +333,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); @@ -371,7 +349,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); @@ -388,7 +366,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 3ee1901b52..3bebc43d59 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1202,6 +1202,33 @@ 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. + */ +static inline bool memory_region_section_intersect_range(MemoryRegionSection *s, + uint64_t offset, uint64_t size) +{ + uint64_t start = MAX(s->offset_within_region, offset); + Int128 end = int128_min(int128_add(int128_make64(s->offset_within_region), s->size), + int128_add(int128_make64(offset), int128_make64(size))); + + if (int128_le(end, int128_make64(start))) { + return false; + } + + s->offset_within_address_space += start - s->offset_within_region; + s->offset_within_region = start; + s->size = int128_sub(end, int128_make64(start)); + return true; +} + /** * memory_region_init: Initialize a memory region *