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.sist...@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
