RE: [PATCH v2 5/7] virtio-iommu : Retrieve page size mask on virtio_iommu_set_iommu_device()

[Duan, Zhenzhong]

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: [PATCH v2 5/7] virtio-iommu : Retrieve page size mask on
>virtio_iommu_set_iommu_device()
>
>Retrieve the Host IOMMU Device page size mask when this latter is set.
>This allows to get the information much sooner than when relying on
>IOMMU MR set_page_size_mask() call, whcih happens when the IOMMU
s/whcih/which
Sorry, I missed it in last review.
>MR
>gets enabled. We introduce check_page_size_mask() helper whose code
>is inherited from current virtio_iommu_set_page_size_mask()
>implementation. This callback will be removed in a subsequent patch.
>
>Signed-off-by: Eric Auger <eric.auger@redhat.com>

Otherwise,
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Thanks
Zhenzhong

>
>---
>
>v1 -> v2:
>- do not update the mask if the granule is frozen (Zhenzhong)
>---
> hw/virtio/virtio-iommu.c | 57
>++++++++++++++++++++++++++++++++++++++--
> hw/virtio/trace-events   |  1 +
> 2 files changed, 56 insertions(+), 2 deletions(-)
>
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index b8f75d2b1a..7d5db554af 100644
>--- a/hw/virtio/virtio-iommu.c
>+++ b/hw/virtio/virtio-iommu.c
>@@ -598,9 +598,39 @@ out:
>     return ret;
> }
>
>+static bool check_page_size_mask(VirtIOIOMMU *viommu, uint64_t
>new_mask,
>+                                 Error **errp)
>+{
>+    uint64_t cur_mask = viommu->config.page_size_mask;
>+
>+    if ((cur_mask & new_mask) == 0) {
>+        error_setg(errp, "virtio-iommu reports a page size mask 0x%"PRIx64
>+                   " incompatible with currently supported mask 0x%"PRIx64,
>+                   new_mask, cur_mask);
>+        return false;
>+    }
>+    /*
>+     * Once the granule is frozen we can't change the mask anymore. If by
>+     * chance the hotplugged device supports the same granule, we can still
>+     * accept it.
>+     */
>+    if (viommu->granule_frozen) {
>+        int cur_granule = ctz64(cur_mask);
>+
>+        if (!(BIT_ULL(cur_granule) & new_mask)) {
>+            error_setg(errp,
>+                       "virtio-iommu does not support frozen granule 0x%llx",
>+                       BIT_ULL(cur_granule));
>+            return false;
>+        }
>+    }
>+    return true;
>+}
>+
> static bool virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque,
>int devfn,
>                                           HostIOMMUDevice *hiod, Error **errp)
> {
>+    ERRP_GUARD();
>     VirtIOIOMMU *viommu = opaque;
>     HostIOMMUDeviceClass *hiodc =
>HOST_IOMMU_DEVICE_GET_CLASS(hiod);
>     struct hiod_key *new_key;
>@@ -623,8 +653,28 @@ static bool
>virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>                                                 hiod->aliased_devfn,
>                                                 host_iova_ranges, errp);
>         if (ret) {
>-            g_list_free_full(host_iova_ranges, g_free);
>-            return false;
>+            goto error;
>+        }
>+    }
>+    if (hiodc->get_page_size_mask) {
>+        uint64_t new_mask = hiodc->get_page_size_mask(hiod);
>+
>+        if (check_page_size_mask(viommu, new_mask, errp)) {
>+            /*
>+             * The default mask depends on the "granule" property. For 
>example,
>+             * with 4k granule, it is -(4 * KiB). When an assigned device has
>+             * page size restrictions due to the hardware IOMMU configuration,
>+             * apply this restriction to the mask.
>+             */
>+            trace_virtio_iommu_update_page_size_mask(hiod->name,
>+                                                     
>viommu->config.page_size_mask,
>+                                                     new_mask);
>+            if (!viommu->granule_frozen) {
>+                viommu->config.page_size_mask &= new_mask;
>+            }
>+        } else {
>+            error_prepend(errp, "%s: ", hiod->name);
>+            goto error;
>         }
>     }
>
>@@ -637,6 +687,9 @@ static bool
>virtio_iommu_set_iommu_device(PCIBus *bus, void *opaque, int devfn,
>     g_list_free_full(host_iova_ranges, g_free);
>
>     return true;
>+error:
>+    g_list_free_full(host_iova_ranges, g_free);
>+    return false;
> }
>
> static void
>diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>index 3cf84e04a7..599d855ff6 100644
>--- a/hw/virtio/trace-events
>+++ b/hw/virtio/trace-events
>@@ -132,6 +132,7 @@ virtio_iommu_notify_map(const char *name,
>uint64_t virt_start, uint64_t virt_end
> virtio_iommu_notify_unmap(const char *name, uint64_t virt_start,
>uint64_t virt_end) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64
> virtio_iommu_remap(const char *name, uint64_t virt_start, uint64_t
>virt_end, uint64_t phys_start) "mr=%s virt_start=0x%"PRIx64"
>virt_end=0x%"PRIx64" phys_start=0x%"PRIx64
> virtio_iommu_set_page_size_mask(const char *name, uint64_t old,
>uint64_t new) "mr=%s old_mask=0x%"PRIx64" new_mask=0x%"PRIx64
>+virtio_iommu_update_page_size_mask(const char *name, uint64_t old,
>uint64_t new) "host iommu device=%s old_mask=0x%"PRIx64"
>new_mask=0x%"PRIx64
> virtio_iommu_notify_flag_add(const char *name) "add notifier to mr %s"
> virtio_iommu_notify_flag_del(const char *name) "del notifier from mr %s"
> virtio_iommu_switch_address_space(uint8_t bus, uint8_t slot, uint8_t fn,
>bool on) "Device %02x:%02x.%x switching address space (iommu
>enabled=%d)"
>--
>2.41.0