On Fri, Feb 2, 2024 at 4:54 PM David Hildenbrand <da...@redhat.com> wrote:
>
> Memory regions cannot overlap, and if we ever hit that case something
> would be really flawed.
>
> For example, when vhost code in QEMU decides to increase the size of memory
> regions to cover full huge pages, it makes sure to never create overlaps,
> and if there would be overlaps, it would bail out.
>
> QEMU commits 48d7c9757749 ("vhost: Merge sections added to temporary
> list"), c1ece84e7c93 ("vhost: Huge page align and merge") and
> e7b94a84b6cb ("vhost: Allow adjoining regions") added and clarified that
> handling and how overlaps are impossible.
>
> Consequently, each GPA can belong to at most one memory region, and
> everything else doesn't make sense. Let's factor out our search to prepare
> for further changes.
>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
Reviewed-by: Raphael Norwitz <raph...@enfabrica.net>
> ---
> subprojects/libvhost-user/libvhost-user.c | 79 +++++++++++++----------
> 1 file changed, 45 insertions(+), 34 deletions(-)
>
> diff --git a/subprojects/libvhost-user/libvhost-user.c
> b/subprojects/libvhost-user/libvhost-user.c
> index 22154b217f..d036b54ed0 100644
> --- a/subprojects/libvhost-user/libvhost-user.c
> +++ b/subprojects/libvhost-user/libvhost-user.c
> @@ -195,30 +195,47 @@ vu_panic(VuDev *dev, const char *msg, ...)
> */
> }
>
> +/* Search for a memory region that covers this guest physical address. */
> +static VuDevRegion *
> +vu_gpa_to_mem_region(VuDev *dev, uint64_t guest_addr)
> +{
> + unsigned int i;
> +
> + /*
> + * Memory regions cannot overlap in guest physical address space. Each
> + * GPA belongs to exactly one memory region, so there can only be one
> + * match.
> + */
> + for (i = 0; i < dev->nregions; i++) {
> + VuDevRegion *cur = &dev->regions[i];
> +
> + if (guest_addr >= cur->gpa && guest_addr < cur->gpa + cur->size) {
> + return cur;
> + }
> + }
> + return NULL;
> +}
> +
> /* Translate guest physical address to our virtual address. */
> void *
> vu_gpa_to_va(VuDev *dev, uint64_t *plen, uint64_t guest_addr)
> {
> - unsigned int i;
> + VuDevRegion *r;
>
> if (*plen == 0) {
> return NULL;
> }
>
> - /* Find matching memory region. */
> - for (i = 0; i < dev->nregions; i++) {
> - VuDevRegion *r = &dev->regions[i];
> -
> - if ((guest_addr >= r->gpa) && (guest_addr < (r->gpa + r->size))) {
> - if ((guest_addr + *plen) > (r->gpa + r->size)) {
> - *plen = r->gpa + r->size - guest_addr;
> - }
> - return (void *)(uintptr_t)
> - guest_addr - r->gpa + r->mmap_addr + r->mmap_offset;
> - }
> + r = vu_gpa_to_mem_region(dev, guest_addr);
> + if (!r) {
> + return NULL;
> }
>
> - return NULL;
> + if ((guest_addr + *plen) > (r->gpa + r->size)) {
> + *plen = r->gpa + r->size - guest_addr;
> + }
> + return (void *)(uintptr_t)guest_addr - r->gpa + r->mmap_addr +
> + r->mmap_offset;
> }
>
> /* Translate qemu virtual address to our virtual address. */
> @@ -854,8 +871,8 @@ static inline bool reg_equal(VuDevRegion *vudev_reg,
> static bool
> vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
> VhostUserMemoryRegion m = vmsg->payload.memreg.region, *msg_region = &m;
> - unsigned int i;
> - bool found = false;
> + unsigned int idx;
> + VuDevRegion *r;
>
> if (vmsg->fd_num > 1) {
> vmsg_close_fds(vmsg);
> @@ -882,28 +899,22 @@ vu_rem_mem_reg(VuDev *dev, VhostUserMsg *vmsg) {
> DPRINT(" mmap_offset 0x%016"PRIx64"\n",
> msg_region->mmap_offset);
>
> - for (i = 0; i < dev->nregions; i++) {
> - if (reg_equal(&dev->regions[i], msg_region)) {
> - VuDevRegion *r = &dev->regions[i];
> -
> - munmap((void *)(uintptr_t)r->mmap_addr, r->size +
> r->mmap_offset);
> -
> - /* Shift all affected entries by 1 to close the hole at index. */
> - memmove(dev->regions + i, dev->regions + i + 1,
> - sizeof(VuDevRegion) * (dev->nregions - i - 1));
> - DPRINT("Successfully removed a region\n");
> - dev->nregions--;
> - i--;
> -
> - found = true;
> - break;
> - }
> - }
> -
> - if (!found) {
> + r = vu_gpa_to_mem_region(dev, msg_region->guest_phys_addr);
> + if (!r || !reg_equal(r, msg_region)) {
> + vmsg_close_fds(vmsg);
> vu_panic(dev, "Specified region not found\n");
> + return false;
> }
>
> + munmap((void *)(uintptr_t)r->mmap_addr, r->size + r->mmap_offset);
> +
> + idx = r - dev->regions;
> + assert(idx < dev->nregions);
> + /* Shift all affected entries by 1 to close the hole. */
> + memmove(r, r + 1, sizeof(VuDevRegion) * (dev->nregions - idx - 1));
> + DPRINT("Successfully removed a region\n");
> + dev->nregions--;
> +
> vmsg_close_fds(vmsg);
>
> return false;
> --
> 2.43.0
>
>
