Re: [PATCH V6 19/27] vfio-pci: cpr part 1 (fd and dma)

[Steven Sistare <steven.sistare@oracle.com>]

On 11/10/2021 2:48 AM, Zheng Chuan wrote:
> 
> Hi, steve
> 
> On 2021/8/11 1:06, Alex Williamson wrote:
>> On Fri,  6 Aug 2021 14:43:53 -0700
>> Steve Sistare <steven.sistare@oracle.com> wrote:
>> [...]
>>> +static int
>>> +vfio_region_remap(MemoryRegionSection *section, void *handle, Error **errp)
>>> +{
>>> +    MemoryRegion *mr = section->mr;
>>> +    VFIOContainer *container = handle;
>>> +    const char *name = memory_region_name(mr);
>>> +    ram_addr_t size = int128_get64(section->size);
>>> +    hwaddr offset, iova, roundup;
>>> +    void *vaddr;
>>> +
>>> +    if (vfio_listener_skipped_section(section) || memory_region_is_iommu(mr)) {
>>> +        return 0;
>>> +    }
>>> +
>>> +    offset = section->offset_within_address_space;
>>> +    iova = REAL_HOST_PAGE_ALIGN(offset);
> We should not do remap if it shares on host page with other structures.
> I think a judgement like int128_ge((int128_make64(iova), llend)) in vfio_listener_region_add() should be also added here to check it,
> otherwise it will remap no-exit dma which causes the live update failure.
> diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
> index 0981d31..d231841 100644
> --- a/hw/vfio/cpr.c
> +++ b/hw/vfio/cpr.c
> @@ -58,13 +58,21 @@ vfio_region_remap(MemoryRegionSection *section, void *handle, Error **errp)
>      ram_addr_t size = int128_get64(section->size);
>      hwaddr offset, iova, roundup;
>      void *vaddr;
> -
> +    Int128 llend;
> +
>      if (vfio_listener_skipped_section(section) || memory_region_is_iommu(mr)) {
>          return 0;
>      }
> 
>      offset = section->offset_within_address_space;
>      iova = REAL_HOST_PAGE_ALIGN(offset);
> +    llend = int128_make64(section->offset_within_address_space);
> +    llend = int128_add(llend, section->size);
> +    llend = int128_and(llend, int128_exts64(qemu_real_host_page_mask));
> +    if (int128_ge(int128_make64(iova), llend)) {
> +        return 0;
> +    }
> +
>      roundup = iova - offset;
>      size -= roundup;
>      size = REAL_HOST_PAGE_ALIGN(size);
> 
>>> +    roundup = iova - offset;
>>> +    size -= roundup;
>>> +    size = REAL_HOST_PAGE_ALIGN(size);
>>> +    vaddr = memory_region_get_ram_ptr(mr) +
>>> +            section->offset_within_region + roundup;
>>> +
>>> +    trace_vfio_region_remap(name, container->fd, iova, iova + size - 1, vaddr);
>>> +    return vfio_dma_map_vaddr(container, iova, size, vaddr, errp);
>>> +}

Thank you Zheng.  I intended to implement the logic you suggest, using 64-bit arithmetic,
but I botched it.  This should do the trick:

diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c
index df334d9..bbdeaea 100644
--- a/hw/vfio/cpr.c
+++ b/hw/vfio/cpr.c
@@ -66,8 +66,8 @@ vfio_region_remap(MemoryRegionSection *section, void *handle,
     offset = section->offset_within_address_space;
     iova = REAL_HOST_PAGE_ALIGN(offset);
     roundup = iova - offset;
-    size -= roundup;
-    size = REAL_HOST_PAGE_ALIGN(size);
+    size -= roundup;                    /* adjust for starting alignment */
+    size &= qemu_real_host_page_mask;   /* adjust for ending alignment */
     end = iova + size;
     if (iova >= end) {
         return 0;

- Steve