> -----Original Message-----
> From: Jean-Philippe Brucker <jean-phili...@linaro.org>
> Sent: Thursday, July 14, 2022 5:54 PM
> To: eric.au...@redhat.com
> Cc: m...@redhat.com; qemu-devel@nongnu.org; Jean-Philippe Brucker
> <jean-phili...@linaro.org>; Zhang, Tina <tina.zh...@intel.com>
> Subject: [PATCH] hw/virtio/virtio-iommu: Enforce power-of-two notify for
> both MAP and UNMAP
>
> Currently we only enforce power-of-two mappings (required by the QEMU
> notifier) for UNMAP requests. A MAP request not aligned on a power-of-two
> may be successfully handled by VFIO, and then the corresponding UNMAP
> notify will fail because it will attempt to split that mapping. Ensure MAP and
> UNMAP notifications are consistent.
>
> Fixes: dde3f08b5cab ("virtio-iommu: Handle non power of 2 range
> invalidations")
> Reported-by: Tina Zhang <tina.zh...@intel.com>
> Signed-off-by: Jean-Philippe Brucker <jean-phili...@linaro.org>
> ---
> hw/virtio/virtio-iommu.c | 44 +++++++++++++++++++++++-----------------
> 1 file changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c index
> 281152d338..f3ecbc71af 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -197,6 +197,29 @@ static gint interval_cmp(gconstpointer a,
> gconstpointer b, gpointer user_data)
> }
> }
>
> +static void virtio_iommu_notify_map_unmap(IOMMUMemoryRegion *mr,
> + IOMMUTLBEvent *event,
> + hwaddr virt_start, hwaddr
> +virt_end) {
> + uint64_t delta = virt_end - virt_start;
> +
> + event->entry.iova = virt_start;
> + event->entry.addr_mask = delta;
> +
> + if (delta == UINT64_MAX) {
> + memory_region_notify_iommu(mr, 0, *event);
> + }
> +
> + while (virt_start != virt_end + 1) {
> + uint64_t mask = dma_aligned_pow2_mask(virt_start, virt_end,
> + 64);
> +
> + event->entry.addr_mask = mask;
> + event->entry.iova = virt_start;
> + memory_region_notify_iommu(mr, 0, *event);
> + virt_start += mask + 1;
Hi Jean,
We also need to increase the event->translated_addr for the map request here.
Thanks,
Tina
> + }
> +}
> +
> static void virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr
> virt_start,
> hwaddr virt_end, hwaddr paddr,
> uint32_t flags) @@ -215,19 +238,16 @@
> static void
> virtio_iommu_notify_map(IOMMUMemoryRegion *mr, hwaddr virt_start,
>
> event.type = IOMMU_NOTIFIER_MAP;
> event.entry.target_as = &address_space_memory;
> - event.entry.addr_mask = virt_end - virt_start;
> - event.entry.iova = virt_start;
> event.entry.perm = perm;
> event.entry.translated_addr = paddr;
>
> - memory_region_notify_iommu(mr, 0, event);
> + virtio_iommu_notify_map_unmap(mr, &event, virt_start, virt_end);
> }
>
> static void virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr,
> hwaddr virt_start,
> hwaddr virt_end) {
> IOMMUTLBEvent event;
> - uint64_t delta = virt_end - virt_start;
>
> if (!(mr->iommu_notify_flags & IOMMU_NOTIFIER_UNMAP)) {
> return;
> @@ -239,22 +259,8 @@ static void
> virtio_iommu_notify_unmap(IOMMUMemoryRegion *mr, hwaddr virt_start,
> event.entry.target_as = &address_space_memory;
> event.entry.perm = IOMMU_NONE;
> event.entry.translated_addr = 0;
> - event.entry.addr_mask = delta;
> - event.entry.iova = virt_start;
> -
> - if (delta == UINT64_MAX) {
> - memory_region_notify_iommu(mr, 0, event);
> - }
>
> -
> - while (virt_start != virt_end + 1) {
> - uint64_t mask = dma_aligned_pow2_mask(virt_start, virt_end, 64);
> -
> - event.entry.addr_mask = mask;
> - event.entry.iova = virt_start;
> - memory_region_notify_iommu(mr, 0, event);
> - virt_start += mask + 1;
> - }
> + virtio_iommu_notify_map_unmap(mr, &event, virt_start, virt_end);
> }
>
> static gboolean virtio_iommu_notify_unmap_cb(gpointer key, gpointer value,
> --
> 2.36.1
