Hi Eric,
>-----Original Message-----
>From: Eric Auger <eric.au...@redhat.com>
>Subject: [PATCH 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
>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.au...@redhat.com>
>---
> hw/virtio/virtio-iommu.c | 55
>++++++++++++++++++++++++++++++++++++++--
> hw/virtio/trace-events | 1 +
> 2 files changed, 54 insertions(+), 2 deletions(-)
>
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index b8f75d2b1a..631589735a 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,26 @@ 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);
>+ viommu->config.page_size_mask &= new_mask;
This is a bit different from original logic, it may update page_size_mask after
frozen.
Will that make issue?
Except this question, for all other patches,
Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
Thanks
Zhenzhong
>+ } else {
>+ error_prepend(errp, "%s: ", hiod->name);
>+ goto error;
> }
> }
>
>@@ -637,6 +685,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
