[PATCH 6/7] memory: remove IOMMU MR iommu_set_page_size_mask() callback

Eric Auger posted 7 patches 5 months ago
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Eric Auger <eric.auger@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Peter Xu <peterx@redhat.com>, David Hildenbrand <david@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yi Liu <yi.l.liu@intel.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>
There is a newer version of this series
[PATCH 6/7] memory: remove IOMMU MR iommu_set_page_size_mask() callback
Posted by Eric Auger 5 months ago
Everything is now in place to use the Host IOMMU Device callbacks
to retrieve the page size mask usable with a given assigned device.
This new method brings the advantage to pass the info much earlier
to the virtual IOMMU and before the IOMMU MR gets enabled. So let's
remove the call to memory_region_iommu_set_page_size_mask in
vfio common.c and remove the single implementation of the IOMMU MR
callback in the virtio-iommu.c

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 include/exec/memory.h    | 38 ---------------------------------
 hw/vfio/common.c         |  8 -------
 hw/virtio/virtio-iommu.c | 45 ----------------------------------------
 system/memory.c          | 13 ------------
 hw/virtio/trace-events   |  1 -
 5 files changed, 105 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 0903513d13..6f9c78cc14 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -504,32 +504,6 @@ struct IOMMUMemoryRegionClass {
      * @iommu: the IOMMUMemoryRegion
      */
     int (*num_indexes)(IOMMUMemoryRegion *iommu);
-
-    /**
-     * @iommu_set_page_size_mask:
-     *
-     * Restrict the page size mask that can be supported with a given IOMMU
-     * memory region. Used for example to propagate host physical IOMMU page
-     * size mask limitations to the virtual IOMMU.
-     *
-     * Optional method: if this method is not provided, then the default global
-     * page mask is used.
-     *
-     * @iommu: the IOMMUMemoryRegion
-     *
-     * @page_size_mask: a bitmask of supported page sizes. At least one bit,
-     * representing the smallest page size, must be set. Additional set bits
-     * represent supported block sizes. For example a host physical IOMMU that
-     * uses page tables with a page size of 4kB, and supports 2MB and 4GB
-     * blocks, will set mask 0x40201000. A granule of 4kB with indiscriminate
-     * block sizes is specified with mask 0xfffffffffffff000.
-     *
-     * Returns 0 on success, or a negative error. In case of failure, the error
-     * object must be created.
-     */
-     int (*iommu_set_page_size_mask)(IOMMUMemoryRegion *iommu,
-                                     uint64_t page_size_mask,
-                                     Error **errp);
 };
 
 typedef struct RamDiscardListener RamDiscardListener;
@@ -1919,18 +1893,6 @@ int memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
  */
 int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
 
-/**
- * memory_region_iommu_set_page_size_mask: set the supported page
- * sizes for a given IOMMU memory region
- *
- * @iommu_mr: IOMMU memory region
- * @page_size_mask: supported page size mask
- * @errp: pointer to Error*, to store an error if it happens.
- */
-int memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
-                                           uint64_t page_size_mask,
-                                           Error **errp);
-
 /**
  * memory_region_name: get a memory region's name
  *
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7cdb969fd3..6d15b36e0b 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -622,14 +622,6 @@ static void vfio_listener_region_add(MemoryListener *listener,
                             int128_get64(llend),
                             iommu_idx);
 
-        ret = memory_region_iommu_set_page_size_mask(giommu->iommu_mr,
-                                                     bcontainer->pgsizes,
-                                                     &err);
-        if (ret) {
-            g_free(giommu);
-            goto fail;
-        }
-
         ret = memory_region_register_iommu_notifier(section->mr, &giommu->n,
                                                     &err);
         if (ret) {
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 631589735a..b24e10de81 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -1363,50 +1363,6 @@ static int virtio_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu_mr,
     return 0;
 }
 
-/*
- * 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.
- */
-static int virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
-                                           uint64_t new_mask,
-                                           Error **errp)
-{
-    IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
-    VirtIOIOMMU *s = sdev->viommu;
-    uint64_t cur_mask = s->config.page_size_mask;
-
-    trace_virtio_iommu_set_page_size_mask(mr->parent_obj.name, cur_mask,
-                                          new_mask);
-
-    if ((cur_mask & new_mask) == 0) {
-        error_setg(errp, "virtio-iommu %s reports a page size mask 0x%"PRIx64
-                   " incompatible with currently supported mask 0x%"PRIx64,
-                   mr->parent_obj.name, new_mask, cur_mask);
-        return -1;
-    }
-
-    /*
-     * 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 (s->granule_frozen) {
-        int cur_granule = ctz64(cur_mask);
-
-        if (!(BIT_ULL(cur_granule) & new_mask)) {
-            error_setg(errp, "virtio-iommu %s does not support frozen granule 0x%llx",
-                       mr->parent_obj.name, BIT_ULL(cur_granule));
-            return -1;
-        }
-        return 0;
-    }
-
-    s->config.page_size_mask &= new_mask;
-    return 0;
-}
-
 static void virtio_iommu_system_reset(void *opaque)
 {
     VirtIOIOMMU *s = opaque;
@@ -1731,7 +1687,6 @@ static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
     imrc->translate = virtio_iommu_translate;
     imrc->replay = virtio_iommu_replay;
     imrc->notify_flag_changed = virtio_iommu_notify_flag_changed;
-    imrc->iommu_set_page_size_mask = virtio_iommu_set_page_size_mask;
 }
 
 static const TypeInfo virtio_iommu_info = {
diff --git a/system/memory.c b/system/memory.c
index 2d69521360..5e6eb459d5 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -1901,19 +1901,6 @@ static int memory_region_update_iommu_notify_flags(IOMMUMemoryRegion *iommu_mr,
     return ret;
 }
 
-int memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
-                                           uint64_t page_size_mask,
-                                           Error **errp)
-{
-    IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
-    int ret = 0;
-
-    if (imrc->iommu_set_page_size_mask) {
-        ret = imrc->iommu_set_page_size_mask(iommu_mr, page_size_mask, errp);
-    }
-    return ret;
-}
-
 int memory_region_register_iommu_notifier(MemoryRegion *mr,
                                           IOMMUNotifier *n, Error **errp)
 {
diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 599d855ff6..b7c04f0856 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -131,7 +131,6 @@ virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start,
 virtio_iommu_notify_map(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64" flags=%d"
 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"
-- 
2.41.0
Re: [PATCH 6/7] memory: remove IOMMU MR iommu_set_page_size_mask() callback
Posted by Cédric Le Goater 5 months ago
On 6/26/24 10:26 AM, Eric Auger wrote:
> Everything is now in place to use the Host IOMMU Device callbacks
> to retrieve the page size mask usable with a given assigned device.
> This new method brings the advantage to pass the info much earlier
> to the virtual IOMMU and before the IOMMU MR gets enabled. So let's
> remove the call to memory_region_iommu_set_page_size_mask in
> vfio common.c and remove the single implementation of the IOMMU MR
> callback in the virtio-iommu.c
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/exec/memory.h    | 38 ---------------------------------
>   hw/vfio/common.c         |  8 -------
>   hw/virtio/virtio-iommu.c | 45 ----------------------------------------
>   system/memory.c          | 13 ------------
>   hw/virtio/trace-events   |  1 -
>   5 files changed, 105 deletions(-)
> 
> diff --git a/include/exec/memory.h b/include/exec/memory.h
> index 0903513d13..6f9c78cc14 100644
> --- a/include/exec/memory.h
> +++ b/include/exec/memory.h
> @@ -504,32 +504,6 @@ struct IOMMUMemoryRegionClass {
>        * @iommu: the IOMMUMemoryRegion
>        */
>       int (*num_indexes)(IOMMUMemoryRegion *iommu);
> -
> -    /**
> -     * @iommu_set_page_size_mask:
> -     *
> -     * Restrict the page size mask that can be supported with a given IOMMU
> -     * memory region. Used for example to propagate host physical IOMMU page
> -     * size mask limitations to the virtual IOMMU.
> -     *
> -     * Optional method: if this method is not provided, then the default global
> -     * page mask is used.
> -     *
> -     * @iommu: the IOMMUMemoryRegion
> -     *
> -     * @page_size_mask: a bitmask of supported page sizes. At least one bit,
> -     * representing the smallest page size, must be set. Additional set bits
> -     * represent supported block sizes. For example a host physical IOMMU that
> -     * uses page tables with a page size of 4kB, and supports 2MB and 4GB
> -     * blocks, will set mask 0x40201000. A granule of 4kB with indiscriminate
> -     * block sizes is specified with mask 0xfffffffffffff000.
> -     *
> -     * Returns 0 on success, or a negative error. In case of failure, the error
> -     * object must be created.
> -     */
> -     int (*iommu_set_page_size_mask)(IOMMUMemoryRegion *iommu,
> -                                     uint64_t page_size_mask,
> -                                     Error **errp);
>   };
>   
>   typedef struct RamDiscardListener RamDiscardListener;
> @@ -1919,18 +1893,6 @@ int memory_region_iommu_attrs_to_index(IOMMUMemoryRegion *iommu_mr,
>    */
>   int memory_region_iommu_num_indexes(IOMMUMemoryRegion *iommu_mr);
>   
> -/**
> - * memory_region_iommu_set_page_size_mask: set the supported page
> - * sizes for a given IOMMU memory region
> - *
> - * @iommu_mr: IOMMU memory region
> - * @page_size_mask: supported page size mask
> - * @errp: pointer to Error*, to store an error if it happens.
> - */
> -int memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
> -                                           uint64_t page_size_mask,
> -                                           Error **errp);
> -
>   /**
>    * memory_region_name: get a memory region's name
>    *
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 7cdb969fd3..6d15b36e0b 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -622,14 +622,6 @@ static void vfio_listener_region_add(MemoryListener *listener,
>                               int128_get64(llend),
>                               iommu_idx);
>   
> -        ret = memory_region_iommu_set_page_size_mask(giommu->iommu_mr,
> -                                                     bcontainer->pgsizes,
> -                                                     &err);
> -        if (ret) {
> -            g_free(giommu);
> -            goto fail;
> -        }
> -
>           ret = memory_region_register_iommu_notifier(section->mr, &giommu->n,
>                                                       &err);
>           if (ret) {
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 631589735a..b24e10de81 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -1363,50 +1363,6 @@ static int virtio_iommu_notify_flag_changed(IOMMUMemoryRegion *iommu_mr,
>       return 0;
>   }
>   
> -/*
> - * 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.
> - */
> -static int virtio_iommu_set_page_size_mask(IOMMUMemoryRegion *mr,
> -                                           uint64_t new_mask,
> -                                           Error **errp)
> -{
> -    IOMMUDevice *sdev = container_of(mr, IOMMUDevice, iommu_mr);
> -    VirtIOIOMMU *s = sdev->viommu;
> -    uint64_t cur_mask = s->config.page_size_mask;
> -
> -    trace_virtio_iommu_set_page_size_mask(mr->parent_obj.name, cur_mask,
> -                                          new_mask);
> -
> -    if ((cur_mask & new_mask) == 0) {
> -        error_setg(errp, "virtio-iommu %s reports a page size mask 0x%"PRIx64
> -                   " incompatible with currently supported mask 0x%"PRIx64,
> -                   mr->parent_obj.name, new_mask, cur_mask);
> -        return -1;
> -    }
> -
> -    /*
> -     * 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 (s->granule_frozen) {
> -        int cur_granule = ctz64(cur_mask);
> -
> -        if (!(BIT_ULL(cur_granule) & new_mask)) {
> -            error_setg(errp, "virtio-iommu %s does not support frozen granule 0x%llx",
> -                       mr->parent_obj.name, BIT_ULL(cur_granule));
> -            return -1;
> -        }
> -        return 0;
> -    }
> -
> -    s->config.page_size_mask &= new_mask;
> -    return 0;
> -}
> -
>   static void virtio_iommu_system_reset(void *opaque)
>   {
>       VirtIOIOMMU *s = opaque;
> @@ -1731,7 +1687,6 @@ static void virtio_iommu_memory_region_class_init(ObjectClass *klass,
>       imrc->translate = virtio_iommu_translate;
>       imrc->replay = virtio_iommu_replay;
>       imrc->notify_flag_changed = virtio_iommu_notify_flag_changed;
> -    imrc->iommu_set_page_size_mask = virtio_iommu_set_page_size_mask;
>   }
>   
>   static const TypeInfo virtio_iommu_info = {
> diff --git a/system/memory.c b/system/memory.c
> index 2d69521360..5e6eb459d5 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -1901,19 +1901,6 @@ static int memory_region_update_iommu_notify_flags(IOMMUMemoryRegion *iommu_mr,
>       return ret;
>   }
>   
> -int memory_region_iommu_set_page_size_mask(IOMMUMemoryRegion *iommu_mr,
> -                                           uint64_t page_size_mask,
> -                                           Error **errp)
> -{
> -    IOMMUMemoryRegionClass *imrc = IOMMU_MEMORY_REGION_GET_CLASS(iommu_mr);
> -    int ret = 0;
> -
> -    if (imrc->iommu_set_page_size_mask) {
> -        ret = imrc->iommu_set_page_size_mask(iommu_mr, page_size_mask, errp);
> -    }
> -    return ret;
> -}
> -
>   int memory_region_register_iommu_notifier(MemoryRegion *mr,
>                                             IOMMUNotifier *n, Error **errp)
>   {
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 599d855ff6..b7c04f0856 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -131,7 +131,6 @@ virtio_iommu_fill_resv_property(uint32_t devid, uint8_t subtype, uint64_t start,
>   virtio_iommu_notify_map(const char *name, uint64_t virt_start, uint64_t virt_end, uint64_t phys_start, uint32_t flags) "mr=%s virt_start=0x%"PRIx64" virt_end=0x%"PRIx64" phys_start=0x%"PRIx64" flags=%d"
>   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"