[PATCH v2 0/3] VIRTIO-IOMMU: Introduce an aw-bits option

[PATCH v2 0/3] VIRTIO-IOMMU: Introduce an aw-bits option

[Eric Auger]

In [1] and [2] we attempted to fix a case where a VFIO-PCI device
protected with a virtio-iommu is assigned to an x86 guest. On x86
the physical IOMMU may have an address width (gaw) of 39 or 48 bits
whereas the virtio-iommu exposes a 64b input address space by default.
Hence the guest may try to use the full 64b space and DMA MAP
failures may be encountered. To work around this issue we endeavoured
to pass usable host IOVA regions (excluding the out of range space) from
VFIO to the virtio-iommu device so that the virtio-iommu driver can
query those latter during the probe request and let the guest iommu
kernel subsystem carve them out. 

However if there are several devices in the same iommu group,
only the reserved regions of the first one are taken into
account by the iommu subsystem of the guest. This generally
works on baremetal because devices are not going to
expose different reserved regions. However in our case, this
may prevent from taking into account the host iommu geometry.

So the simplest solution to this problem looks to introduce an
input address width option, aw-bits, which matches what is
done on the intel-iommu. By default, from now on it is set
to 39 bits with pc_q35 and 48 with arm virt. This replaces the
previous default value of 64b. So we need to introduce a compat
for machines older than 9.0 to behave similarly. We use
hw_compat_8_2 to acheive that goal.

Outstanding series [2] remains useful to let resv regions beeing
communicated on time before the probe request.

[1] [PATCH v4 00/12] VIRTIO-IOMMU/VFIO: Don't assume 64b IOVA space
    https://lore.kernel.org/all/20231019134651.842175-1-eric.auger@redhat.com/
    - This is merged -

[2] [RFC 0/7] VIRTIO-IOMMU/VFIO: Fix host iommu geometry handling for hotplugged devices
    https://lore.kernel.org/all/20240117080414.316890-1-eric.auger@redhat.com/
    - This is pending for review on the ML -

This series can be found at:
https://github.com/eauger/qemu/tree/virtio-iommu-aw-bits-v2

Applied on top of [3]
[PATCH v2] virtio-iommu: Use qemu_real_host_page_mask as default page_size_mask
https://lore.kernel.org/all/20240117132039.332273-1-eric.auger@redhat.com/

History:
v1 -> v2
- Limit aw to 48b on ARM
- Check aw is within [32,64]
- Use hw_compat_8_2

Eric Auger (3):
  virtio-iommu: Add an option to define the input range width
  virtio-iommu: Trace domain range limits as unsigned int
  hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

 include/hw/virtio/virtio-iommu.h | 1 +
 hw/arm/virt.c                    | 6 ++++++
 hw/core/machine.c                | 5 ++++-
 hw/i386/pc.c                     | 6 ++++++
 hw/virtio/virtio-iommu.c         | 7 ++++++-
 hw/virtio/trace-events           | 2 +-
 6 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.41.0

[PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Eric Auger]

aw-bits is a new option that allows to set the bit width of
the input address range. This value will be used as a default for
the device config input_range.end. By default it is set to 64 bits
which is the current value.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v1 -> v2:
- Check the aw-bits value is within [32,64]
---
 include/hw/virtio/virtio-iommu.h | 1 +
 hw/virtio/virtio-iommu.c         | 7 ++++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
index 781ebaea8f..5fbe4677c2 100644
--- a/include/hw/virtio/virtio-iommu.h
+++ b/include/hw/virtio/virtio-iommu.h
@@ -66,6 +66,7 @@ struct VirtIOIOMMU {
     bool boot_bypass;
     Notifier machine_done;
     bool granule_frozen;
+    uint8_t aw_bits;
 };
 
 #endif
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index ec2ba11d1d..7870bdbeee 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
      */
     s->config.bypass = s->boot_bypass;
     s->config.page_size_mask = qemu_real_host_page_mask();
-    s->config.input_range.end = UINT64_MAX;
+    if (s->aw_bits < 32 || s->aw_bits > 64) {
+        error_setg(errp, "aw-bits must be within [32,64]");
+    }
+    s->config.input_range.end =
+        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
     s->config.domain_range.end = UINT32_MAX;
     s->config.probe_size = VIOMMU_PROBE_SIZE;
 
@@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
     DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
                      TYPE_PCI_BUS, PCIBus *),
     DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
+    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.41.0

[PATCH v2 2/3] virtio-iommu: Trace domain range limits as unsigned int

[Eric Auger]

Use %u format to trace domain_range limits.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
---
 hw/virtio/trace-events | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
index 77905d1994..2350849fbd 100644
--- a/hw/virtio/trace-events
+++ b/hw/virtio/trace-events
@@ -111,7 +111,7 @@ virtio_iommu_device_reset(void) "reset!"
 virtio_iommu_system_reset(void) "system reset!"
 virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
 virtio_iommu_device_status(uint8_t status) "driver status = %d"
-virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d domain range end=%d probe_size=0x%x bypass=0x%x"
+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%u domain range end=%u probe_size=0x%x bypass=0x%x"
 virtio_iommu_set_config(uint8_t bypass) "bypass=0x%x"
 virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
 virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
-- 
2.41.0

[PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

[Eric Auger]

Currently the default input range can extend to 64 bits. On x86,
when the virtio-iommu protects vfio devices, the physical iommu
may support only 39 bits. Let's set the default to 39, as done
for the intel-iommu. On ARM we set 48b as a default (matching
SMMUv3 SMMU_IDR5.VAX == 0).

We use hw_compat_8_2 to handle the compatibility for machines
before 9.0 which used to have a virtio-iommu default input range
of 64 bits.

Of course if aw-bits is set from the command line, the default
is overriden.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

---

v1 -> v2:
- set aw-bits to 48b on ARM
- use hw_compat_8_2 to handle the compat for older machines
  which used 64b as a default
---
 hw/arm/virt.c            | 6 ++++++
 hw/core/machine.c        | 5 ++++-
 hw/i386/pc.c             | 6 ++++++
 hw/virtio/virtio-iommu.c | 2 +-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e6ead2c5c8..56539f2fc5 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2718,10 +2718,16 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
+                                                   "aw-bits", NULL);
         hwaddr db_start = 0, db_end = 0;
         QList *reserved_regions;
         char *resv_prop_str;
 
+        if (!aw_bits) {
+            qdev_prop_set_uint8(dev, "aw-bits", 48);
+        }
+
         if (vms->iommu != VIRT_IOMMU_NONE) {
             error_setg(errp, "virt machine does not support multiple IOMMUs");
             return;
diff --git a/hw/core/machine.c b/hw/core/machine.c
index fb5afdcae4..70ac96954c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -30,9 +30,12 @@
 #include "exec/confidential-guest-support.h"
 #include "hw/virtio/virtio-pci.h"
 #include "hw/virtio/virtio-net.h"
+#include "hw/virtio/virtio-iommu.h"
 #include "audio/audio.h"
 
-GlobalProperty hw_compat_8_2[] = {};
+GlobalProperty hw_compat_8_2[] = {
+    { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
+};
 const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
 
 GlobalProperty hw_compat_8_1[] = {
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 803244e5cc..0e2bcb4840 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1458,6 +1458,8 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
+                                                   "aw-bits", NULL);
         /* Declare the APIC range as the reserved MSI region */
         char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
                                               VIRTIO_IOMMU_RESV_MEM_T_MSI);
@@ -1466,6 +1468,10 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
         qlist_append_str(reserved_regions, resv_prop_str);
         qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
 
+        if (!aw_bits) {
+            qdev_prop_set_uint8(dev, "aw-bits", 39);
+        }
+
         g_free(resv_prop_str);
     }
 
diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
index 7870bdbeee..c468e9b13b 100644
--- a/hw/virtio/virtio-iommu.c
+++ b/hw/virtio/virtio-iommu.c
@@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
     DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
                      TYPE_PCI_BUS, PCIBus *),
     DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
-    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
+    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.41.0

RE: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Duan, Zhenzhong]

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: [PATCH v2 1/3] virtio-iommu: Add an option to define the input
>range width
>
>aw-bits is a new option that allows to set the bit width of
>the input address range. This value will be used as a default for
>the device config input_range.end. By default it is set to 64 bits
>which is the current value.
>
>Signed-off-by: Eric Auger <eric.auger@redhat.com>

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

Thanks
Zhenzhong

>
>---
>
>v1 -> v2:
>- Check the aw-bits value is within [32,64]
>---
> include/hw/virtio/virtio-iommu.h | 1 +
> hw/virtio/virtio-iommu.c         | 7 ++++++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-
>iommu.h
>index 781ebaea8f..5fbe4677c2 100644
>--- a/include/hw/virtio/virtio-iommu.h
>+++ b/include/hw/virtio/virtio-iommu.h
>@@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>     bool boot_bypass;
>     Notifier machine_done;
>     bool granule_frozen;
>+    uint8_t aw_bits;
> };
>
> #endif
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index ec2ba11d1d..7870bdbeee 100644
>--- a/hw/virtio/virtio-iommu.c
>+++ b/hw/virtio/virtio-iommu.c
>@@ -1314,7 +1314,11 @@ static void
>virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>      */
>     s->config.bypass = s->boot_bypass;
>     s->config.page_size_mask = qemu_real_host_page_mask();
>-    s->config.input_range.end = UINT64_MAX;
>+    if (s->aw_bits < 32 || s->aw_bits > 64) {
>+        error_setg(errp, "aw-bits must be within [32,64]");
>+    }
>+    s->config.input_range.end =
>+        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
>     s->config.domain_range.end = UINT32_MAX;
>     s->config.probe_size = VIOMMU_PROBE_SIZE;
>
>@@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
>     DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>                      TYPE_PCI_BUS, PCIBus *),
>     DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>+    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>     DEFINE_PROP_END_OF_LIST(),
> };
>
>--
>2.41.0

RE: [PATCH v2 2/3] virtio-iommu: Trace domain range limits as unsigned int

[Duan, Zhenzhong]

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: [PATCH v2 2/3] virtio-iommu: Trace domain range limits as
>unsigned int
>
>Use %u format to trace domain_range limits.
>
>Signed-off-by: Eric Auger <eric.auger@redhat.com>

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

Thanks
Zhenzhong

>---
> hw/virtio/trace-events | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>index 77905d1994..2350849fbd 100644
>--- a/hw/virtio/trace-events
>+++ b/hw/virtio/trace-events
>@@ -111,7 +111,7 @@ virtio_iommu_device_reset(void) "reset!"
> virtio_iommu_system_reset(void) "system reset!"
> virtio_iommu_get_features(uint64_t features) "device supports
>features=0x%"PRIx64
> virtio_iommu_device_status(uint8_t status) "driver status = %d"
>-virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start,
>uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t
>probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range
>start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d
>domain range end=%d probe_size=0x%x bypass=0x%x"
>+virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start,
>uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t
>probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range
>start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%u
>domain range end=%u probe_size=0x%x bypass=0x%x"
> virtio_iommu_set_config(uint8_t bypass) "bypass=0x%x"
> virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d
>endpoint=%d"
> virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d
>endpoint=%d"
>--
>2.41.0

RE: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

[Duan, Zhenzhong]

Hi Eric,

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>pc_q35_9.0 and arm virt
>
>Currently the default input range can extend to 64 bits. On x86,
>when the virtio-iommu protects vfio devices, the physical iommu
>may support only 39 bits. Let's set the default to 39, as done
>for the intel-iommu. On ARM we set 48b as a default (matching
>SMMUv3 SMMU_IDR5.VAX == 0).
>
>We use hw_compat_8_2 to handle the compatibility for machines
>before 9.0 which used to have a virtio-iommu default input range
>of 64 bits.
>
>Of course if aw-bits is set from the command line, the default
>is overriden.
>
>Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
>---
>
>v1 -> v2:
>- set aw-bits to 48b on ARM
>- use hw_compat_8_2 to handle the compat for older machines
>  which used 64b as a default
>---
> hw/arm/virt.c            | 6 ++++++
> hw/core/machine.c        | 5 ++++-
> hw/i386/pc.c             | 6 ++++++
> hw/virtio/virtio-iommu.c | 2 +-
> 4 files changed, 17 insertions(+), 2 deletions(-)
>
>diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>index e6ead2c5c8..56539f2fc5 100644
>--- a/hw/arm/virt.c
>+++ b/hw/arm/virt.c
>@@ -2718,10 +2718,16 @@ static void
>virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>errp);
>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>+        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>+                                                   "aw-bits", NULL);
>         hwaddr db_start = 0, db_end = 0;
>         QList *reserved_regions;
>         char *resv_prop_str;
>
>+        if (!aw_bits) {
>+            qdev_prop_set_uint8(dev, "aw-bits", 48);
>+        }
>+
>         if (vms->iommu != VIRT_IOMMU_NONE) {
>             error_setg(errp, "virt machine does not support multiple IOMMUs");
>             return;
>diff --git a/hw/core/machine.c b/hw/core/machine.c
>index fb5afdcae4..70ac96954c 100644
>--- a/hw/core/machine.c
>+++ b/hw/core/machine.c
>@@ -30,9 +30,12 @@
> #include "exec/confidential-guest-support.h"
> #include "hw/virtio/virtio-pci.h"
> #include "hw/virtio/virtio-net.h"
>+#include "hw/virtio/virtio-iommu.h"
> #include "audio/audio.h"
>
>-GlobalProperty hw_compat_8_2[] = {};
>+GlobalProperty hw_compat_8_2[] = {
>+    { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>+};
> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>
> GlobalProperty hw_compat_8_1[] = {
>diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>index 803244e5cc..0e2bcb4840 100644
>--- a/hw/i386/pc.c
>+++ b/hw/i386/pc.c
>@@ -1458,6 +1458,8 @@ static void
>pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>errp);
>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>+        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>+                                                   "aw-bits", NULL);
>         /* Declare the APIC range as the reserved MSI region */
>         char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>                                               VIRTIO_IOMMU_RESV_MEM_T_MSI);
>@@ -1466,6 +1468,10 @@ static void
>pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>         qlist_append_str(reserved_regions, resv_prop_str);
>         qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>
>+        if (!aw_bits) {
>+            qdev_prop_set_uint8(dev, "aw-bits", 39);
>+        }
>+
>         g_free(resv_prop_str);
>     }
>
>diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>index 7870bdbeee..c468e9b13b 100644
>--- a/hw/virtio/virtio-iommu.c
>+++ b/hw/virtio/virtio-iommu.c
>@@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>     DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>                      TYPE_PCI_BUS, PCIBus *),
>     DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>-    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>+    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),

Not clear if virtio-iommu support other archs besides x86 and arm.
It looks on those archs, aw_bits is default 0 on machine 9.0 above
and will fails the check in realize?

Thanks
Zhenzhong

>     DEFINE_PROP_END_OF_LIST(),
> };
>
>--
>2.41.0

Re: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

[Eric Auger]

Hi Zhenzhong,

On 2/2/24 07:51, Duan, Zhenzhong wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> pc_q35_9.0 and arm virt
>>
>> Currently the default input range can extend to 64 bits. On x86,
>> when the virtio-iommu protects vfio devices, the physical iommu
>> may support only 39 bits. Let's set the default to 39, as done
>> for the intel-iommu. On ARM we set 48b as a default (matching
>> SMMUv3 SMMU_IDR5.VAX == 0).
>>
>> We use hw_compat_8_2 to handle the compatibility for machines
>> before 9.0 which used to have a virtio-iommu default input range
>> of 64 bits.
>>
>> Of course if aw-bits is set from the command line, the default
>> is overriden.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - set aw-bits to 48b on ARM
>> - use hw_compat_8_2 to handle the compat for older machines
>>  which used 64b as a default
>> ---
>> hw/arm/virt.c            | 6 ++++++
>> hw/core/machine.c        | 5 ++++-
>> hw/i386/pc.c             | 6 ++++++
>> hw/virtio/virtio-iommu.c | 2 +-
>> 4 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index e6ead2c5c8..56539f2fc5 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2718,10 +2718,16 @@ static void
>> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>> errp);
>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> +                                                   "aw-bits", NULL);
>>         hwaddr db_start = 0, db_end = 0;
>>         QList *reserved_regions;
>>         char *resv_prop_str;
>>
>> +        if (!aw_bits) {
>> +            qdev_prop_set_uint8(dev, "aw-bits", 48);
>> +        }
>> +
>>         if (vms->iommu != VIRT_IOMMU_NONE) {
>>             error_setg(errp, "virt machine does not support multiple IOMMUs");
>>             return;
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index fb5afdcae4..70ac96954c 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -30,9 +30,12 @@
>> #include "exec/confidential-guest-support.h"
>> #include "hw/virtio/virtio-pci.h"
>> #include "hw/virtio/virtio-net.h"
>> +#include "hw/virtio/virtio-iommu.h"
>> #include "audio/audio.h"
>>
>> -GlobalProperty hw_compat_8_2[] = {};
>> +GlobalProperty hw_compat_8_2[] = {
>> +    { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>> +};
>> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>>
>> GlobalProperty hw_compat_8_1[] = {
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 803244e5cc..0e2bcb4840 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1458,6 +1458,8 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev),
>> errp);
>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> +                                                   "aw-bits", NULL);
>>         /* Declare the APIC range as the reserved MSI region */
>>         char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>>                                               VIRTIO_IOMMU_RESV_MEM_T_MSI);
>> @@ -1466,6 +1468,10 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>         qlist_append_str(reserved_regions, resv_prop_str);
>>         qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>>
>> +        if (!aw_bits) {
>> +            qdev_prop_set_uint8(dev, "aw-bits", 39);
>> +        }
>> +
>>         g_free(resv_prop_str);
>>     }
>>
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 7870bdbeee..c468e9b13b 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>     DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>                      TYPE_PCI_BUS, PCIBus *),
>>     DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> -    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
> Not clear if virtio-iommu support other archs besides x86 and arm.
> It looks on those archs, aw_bits is default 0 on machine 9.0 above
> and will fails the check in realize?

At the moment the virtio-iommu only is supported along with q35 and
arm-virt.
Only those machines set the reserved-regions prop and the aw-bits, which
are both requested for a correct behavior.

Thank you for the review!

Eric

>
> Thanks
> Zhenzhong
>
>>     DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> --
>> 2.41.0

RE: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

[Duan, Zhenzhong]

>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on
>pc_q35_9.0 and arm virt
>
>Hi Zhenzhong,
>
>On 2/2/24 07:51, Duan, Zhenzhong wrote:
>> Hi Eric,
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.auger@redhat.com>
>>> pc_q35_9.0 and arm virt
>>>
>>> Currently the default input range can extend to 64 bits. On x86,
>>> when the virtio-iommu protects vfio devices, the physical iommu
>>> may support only 39 bits. Let's set the default to 39, as done
>>> for the intel-iommu. On ARM we set 48b as a default (matching
>>> SMMUv3 SMMU_IDR5.VAX == 0).
>>>
>>> We use hw_compat_8_2 to handle the compatibility for machines
>>> before 9.0 which used to have a virtio-iommu default input range
>>> of 64 bits.
>>>
>>> Of course if aw-bits is set from the command line, the default
>>> is overriden.
>>>
>>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>>
>>> ---
>>>
>>> v1 -> v2:
>>> - set aw-bits to 48b on ARM
>>> - use hw_compat_8_2 to handle the compat for older machines
>>>  which used 64b as a default
>>> ---
>>> hw/arm/virt.c            | 6 ++++++
>>> hw/core/machine.c        | 5 ++++-
>>> hw/i386/pc.c             | 6 ++++++
>>> hw/virtio/virtio-iommu.c | 2 +-
>>> 4 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index e6ead2c5c8..56539f2fc5 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -2718,10 +2718,16 @@ static void
>>> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>>         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>MACHINE(hotplug_dev),
>>> errp);
>>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI))
>{
>>> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>>> +                                                   "aw-bits", NULL);
>>>         hwaddr db_start = 0, db_end = 0;
>>>         QList *reserved_regions;
>>>         char *resv_prop_str;
>>>
>>> +        if (!aw_bits) {
>>> +            qdev_prop_set_uint8(dev, "aw-bits", 48);
>>> +        }
>>> +
>>>         if (vms->iommu != VIRT_IOMMU_NONE) {
>>>             error_setg(errp, "virt machine does not support multiple
>IOMMUs");
>>>             return;
>>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>>> index fb5afdcae4..70ac96954c 100644
>>> --- a/hw/core/machine.c
>>> +++ b/hw/core/machine.c
>>> @@ -30,9 +30,12 @@
>>> #include "exec/confidential-guest-support.h"
>>> #include "hw/virtio/virtio-pci.h"
>>> #include "hw/virtio/virtio-net.h"
>>> +#include "hw/virtio/virtio-iommu.h"
>>> #include "audio/audio.h"
>>>
>>> -GlobalProperty hw_compat_8_2[] = {};
>>> +GlobalProperty hw_compat_8_2[] = {
>>> +    { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>>> +};
>>> const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>>>
>>> GlobalProperty hw_compat_8_1[] = {
>>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>>> index 803244e5cc..0e2bcb4840 100644
>>> --- a/hw/i386/pc.c
>>> +++ b/hw/i386/pc.c
>>> @@ -1458,6 +1458,8 @@ static void
>>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>>         virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>MACHINE(hotplug_dev),
>>> errp);
>>>     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI))
>{
>>> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>>> +                                                   "aw-bits", NULL);
>>>         /* Declare the APIC range as the reserved MSI region */
>>>         char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>>>                                               VIRTIO_IOMMU_RESV_MEM_T_MSI);
>>> @@ -1466,6 +1468,10 @@ static void
>>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>>         qlist_append_str(reserved_regions, resv_prop_str);
>>>         qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>>>
>>> +        if (!aw_bits) {
>>> +            qdev_prop_set_uint8(dev, "aw-bits", 39);
>>> +        }
>>> +
>>>         g_free(resv_prop_str);
>>>     }
>>>
>>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>>> index 7870bdbeee..c468e9b13b 100644
>>> --- a/hw/virtio/virtio-iommu.c
>>> +++ b/hw/virtio/virtio-iommu.c
>>> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>>     DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>>                      TYPE_PCI_BUS, PCIBus *),
>>>     DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>>> -    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>>> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
>> Not clear if virtio-iommu support other archs besides x86 and arm.
>> It looks on those archs, aw_bits is default 0 on machine 9.0 above
>> and will fails the check in realize?
>
>At the moment the virtio-iommu only is supported along with q35 and
>arm-virt.
>Only those machines set the reserved-regions prop and the aw-bits, which
>are both requested for a correct behavior.

Clear, thanks Eric.

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

BRs.
Zhenzhong

>
>Thank you for the review!
>
>Eric
>
>>
>> Thanks
>> Zhenzhong
>>
>>>     DEFINE_PROP_END_OF_LIST(),
>>> };
>>>
>>> --
>>> 2.41.0

Re: [PATCH v2 0/3] VIRTIO-IOMMU: Introduce an aw-bits option

[YangHang Liu]

There is no VFIO_MAP_DMA error when hotplug 2 E810 PFs into a VM which
has a virtio-iommu device.

In addition, 21 regression tests of Tier1 level were run and all got PASS.

Tested-by: Yanghang Liu<yanghliu@redhat.com>


On Fri, Feb 2, 2024 at 12:33 AM Eric Auger <eric.auger@redhat.com> wrote:
>
> In [1] and [2] we attempted to fix a case where a VFIO-PCI device
> protected with a virtio-iommu is assigned to an x86 guest. On x86
> the physical IOMMU may have an address width (gaw) of 39 or 48 bits
> whereas the virtio-iommu exposes a 64b input address space by default.
> Hence the guest may try to use the full 64b space and DMA MAP
> failures may be encountered. To work around this issue we endeavoured
> to pass usable host IOVA regions (excluding the out of range space) from
> VFIO to the virtio-iommu device so that the virtio-iommu driver can
> query those latter during the probe request and let the guest iommu
> kernel subsystem carve them out.
>
> However if there are several devices in the same iommu group,
> only the reserved regions of the first one are taken into
> account by the iommu subsystem of the guest. This generally
> works on baremetal because devices are not going to
> expose different reserved regions. However in our case, this
> may prevent from taking into account the host iommu geometry.
>
> So the simplest solution to this problem looks to introduce an
> input address width option, aw-bits, which matches what is
> done on the intel-iommu. By default, from now on it is set
> to 39 bits with pc_q35 and 48 with arm virt. This replaces the
> previous default value of 64b. So we need to introduce a compat
> for machines older than 9.0 to behave similarly. We use
> hw_compat_8_2 to acheive that goal.
>
> Outstanding series [2] remains useful to let resv regions beeing
> communicated on time before the probe request.
>
> [1] [PATCH v4 00/12] VIRTIO-IOMMU/VFIO: Don't assume 64b IOVA space
>     https://lore.kernel.org/all/20231019134651.842175-1-eric.auger@redhat.com/
>     - This is merged -
>
> [2] [RFC 0/7] VIRTIO-IOMMU/VFIO: Fix host iommu geometry handling for hotplugged devices
>     https://lore.kernel.org/all/20240117080414.316890-1-eric.auger@redhat.com/
>     - This is pending for review on the ML -
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/virtio-iommu-aw-bits-v2
>
> Applied on top of [3]
> [PATCH v2] virtio-iommu: Use qemu_real_host_page_mask as default page_size_mask
> https://lore.kernel.org/all/20240117132039.332273-1-eric.auger@redhat.com/
>
> History:
> v1 -> v2
> - Limit aw to 48b on ARM
> - Check aw is within [32,64]
> - Use hw_compat_8_2
>
> Eric Auger (3):
>   virtio-iommu: Add an option to define the input range width
>   virtio-iommu: Trace domain range limits as unsigned int
>   hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt
>
>  include/hw/virtio/virtio-iommu.h | 1 +
>  hw/arm/virt.c                    | 6 ++++++
>  hw/core/machine.c                | 5 ++++-
>  hw/i386/pc.c                     | 6 ++++++
>  hw/virtio/virtio-iommu.c         | 7 ++++++-
>  hw/virtio/trace-events           | 2 +-
>  6 files changed, 24 insertions(+), 3 deletions(-)
>
> --
> 2.41.0
>

Re: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Cédric Le Goater]

On 2/1/24 17:32, Eric Auger wrote:
> aw-bits is a new option that allows to set the bit width of
> the input address range. This value will be used as a default for
> the device config input_range.end. By default it is set to 64 bits
> which is the current value.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v1 -> v2:
> - Check the aw-bits value is within [32,64]
> ---
>   include/hw/virtio/virtio-iommu.h | 1 +
>   hw/virtio/virtio-iommu.c         | 7 ++++++-
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 781ebaea8f..5fbe4677c2 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>       bool boot_bypass;
>       Notifier machine_done;
>       bool granule_frozen;
> +    uint8_t aw_bits;
>   };
>   
>   #endif
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index ec2ba11d1d..7870bdbeee 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>        */
>       s->config.bypass = s->boot_bypass;
>       s->config.page_size_mask = qemu_real_host_page_mask();
> -    s->config.input_range.end = UINT64_MAX;
> +    if (s->aw_bits < 32 || s->aw_bits > 64) {
> +        error_setg(errp, "aw-bits must be within [32,64]");
> +    }
> +    s->config.input_range.end =
> +        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;


This could be simplified :

   s->config.input_range.end = BIT_ULL(s->aw_bits) - 1;

Anyhow,


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

Thanks,

C.



>       s->config.domain_range.end = UINT32_MAX;
>       s->config.probe_size = VIOMMU_PROBE_SIZE;
>   
> @@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
>       DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>                        TYPE_PCI_BUS, PCIBus *),
>       DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   

Re: [PATCH v2 2/3] virtio-iommu: Trace domain range limits as unsigned int

[Cédric Le Goater]

On 2/1/24 17:32, Eric Auger wrote:
> Use %u format to trace domain_range limits.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>


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

Thanks,

C.


> ---
>   hw/virtio/trace-events | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
> index 77905d1994..2350849fbd 100644
> --- a/hw/virtio/trace-events
> +++ b/hw/virtio/trace-events
> @@ -111,7 +111,7 @@ virtio_iommu_device_reset(void) "reset!"
>   virtio_iommu_system_reset(void) "system reset!"
>   virtio_iommu_get_features(uint64_t features) "device supports features=0x%"PRIx64
>   virtio_iommu_device_status(uint8_t status) "driver status = %d"
> -virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%d domain range end=%d probe_size=0x%x bypass=0x%x"
> +virtio_iommu_get_config(uint64_t page_size_mask, uint64_t start, uint64_t end, uint32_t domain_start, uint32_t domain_end, uint32_t probe_size, uint8_t bypass) "page_size_mask=0x%"PRIx64" input range start=0x%"PRIx64" input range end=0x%"PRIx64" domain range start=%u domain range end=%u probe_size=0x%x bypass=0x%x"
>   virtio_iommu_set_config(uint8_t bypass) "bypass=0x%x"
>   virtio_iommu_attach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"
>   virtio_iommu_detach(uint32_t domain_id, uint32_t ep_id) "domain=%d endpoint=%d"

Re: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

[Cédric Le Goater]

On 2/1/24 17:32, Eric Auger wrote:
> Currently the default input range can extend to 64 bits. On x86,
> when the virtio-iommu protects vfio devices, the physical iommu
> may support only 39 bits. Let's set the default to 39, as done
> for the intel-iommu. On ARM we set 48b as a default (matching
> SMMUv3 SMMU_IDR5.VAX == 0).
> 
> We use hw_compat_8_2 to handle the compatibility for machines
> before 9.0 which used to have a virtio-iommu default input range
> of 64 bits.
> 
> Of course if aw-bits is set from the command line, the default
> is overriden.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v1 -> v2:
> - set aw-bits to 48b on ARM
> - use hw_compat_8_2 to handle the compat for older machines
>    which used 64b as a default
> ---
>   hw/arm/virt.c            | 6 ++++++
>   hw/core/machine.c        | 5 ++++-
>   hw/i386/pc.c             | 6 ++++++
>   hw/virtio/virtio-iommu.c | 2 +-
>   4 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index e6ead2c5c8..56539f2fc5 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2718,10 +2718,16 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>           virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
> +                                                   "aw-bits", NULL);

object_property_get_uint() should not fail. Please use &error_abort.

>           hwaddr db_start = 0, db_end = 0;
>           QList *reserved_regions;
>           char *resv_prop_str;
>   
> +        if (!aw_bits) {
> +            qdev_prop_set_uint8(dev, "aw-bits", 48);
> +        }
> +
>           if (vms->iommu != VIRT_IOMMU_NONE) {
>               error_setg(errp, "virt machine does not support multiple IOMMUs");
>               return;
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index fb5afdcae4..70ac96954c 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -30,9 +30,12 @@
>   #include "exec/confidential-guest-support.h"
>   #include "hw/virtio/virtio-pci.h"
>   #include "hw/virtio/virtio-net.h"
> +#include "hw/virtio/virtio-iommu.h"
>   #include "audio/audio.h"
>   
> -GlobalProperty hw_compat_8_2[] = {};
> +GlobalProperty hw_compat_8_2[] = {
> +    { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
> +};
>   const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>   
>   GlobalProperty hw_compat_8_1[] = {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 803244e5cc..0e2bcb4840 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1458,6 +1458,8 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>           virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
> +                                                   "aw-bits", NULL);
>           /* Declare the APIC range as the reserved MSI region */
>           char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
>                                                 VIRTIO_IOMMU_RESV_MEM_T_MSI);
> @@ -1466,6 +1468,10 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>           qlist_append_str(reserved_regions, resv_prop_str);
>           qdev_prop_set_array(dev, "reserved-regions", reserved_regions);
>   
> +        if (!aw_bits) {
> +            qdev_prop_set_uint8(dev, "aw-bits", 39);

May be use VTD_HOST_AW_39BIT instead of 39 ? This would make it
easier to find uses of certain defaults values and would clarify
that the default AW of virtio-iommu is set as intel-iommu.

Thanks,

C.



> +        }
> +
>           g_free(resv_prop_str);
>       }
>   
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index 7870bdbeee..c468e9b13b 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>       DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>                        TYPE_PCI_BUS, PCIBus *),
>       DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
> -    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
>       DEFINE_PROP_END_OF_LIST(),
>   };
>   

Re: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Jean-Philippe Brucker]

Hi Eric,

On Thu, Feb 01, 2024 at 05:32:22PM +0100, Eric Auger wrote:
> aw-bits is a new option that allows to set the bit width of
> the input address range. This value will be used as a default for
> the device config input_range.end. By default it is set to 64 bits
> which is the current value.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v1 -> v2:
> - Check the aw-bits value is within [32,64]
> ---
>  include/hw/virtio/virtio-iommu.h | 1 +
>  hw/virtio/virtio-iommu.c         | 7 ++++++-
>  2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
> index 781ebaea8f..5fbe4677c2 100644
> --- a/include/hw/virtio/virtio-iommu.h
> +++ b/include/hw/virtio/virtio-iommu.h
> @@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>      bool boot_bypass;
>      Notifier machine_done;
>      bool granule_frozen;
> +    uint8_t aw_bits;
>  };
>  
>  #endif
> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> index ec2ba11d1d..7870bdbeee 100644
> --- a/hw/virtio/virtio-iommu.c
> +++ b/hw/virtio/virtio-iommu.c
> @@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>       */
>      s->config.bypass = s->boot_bypass;
>      s->config.page_size_mask = qemu_real_host_page_mask();
> -    s->config.input_range.end = UINT64_MAX;
> +    if (s->aw_bits < 32 || s->aw_bits > 64) {

I'm wondering if we should lower this to 16 bits, just to support all
possible host SMMU configurations (the smallest address space configurable
with T0SZ is 25-bit, or 16-bit with the STT extension).

Thanks,
Jean

> +        error_setg(errp, "aw-bits must be within [32,64]");
> +    }
> +    s->config.input_range.end =
> +        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
>      s->config.domain_range.end = UINT32_MAX;
>      s->config.probe_size = VIOMMU_PROBE_SIZE;
>  
> @@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
>      DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>                       TYPE_PCI_BUS, PCIBus *),
>      DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> -- 
> 2.41.0
> 

Re: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Cédric Le Goater]

On 2/5/24 10:14, Cédric Le Goater wrote:
> On 2/1/24 17:32, Eric Auger wrote:
>> aw-bits is a new option that allows to set the bit width of
>> the input address range. This value will be used as a default for
>> the device config input_range.end. By default it is set to 64 bits
>> which is the current value.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - Check the aw-bits value is within [32,64]
>> ---
>>   include/hw/virtio/virtio-iommu.h | 1 +
>>   hw/virtio/virtio-iommu.c         | 7 ++++++-
>>   2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>> index 781ebaea8f..5fbe4677c2 100644
>> --- a/include/hw/virtio/virtio-iommu.h
>> +++ b/include/hw/virtio/virtio-iommu.h
>> @@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>>       bool boot_bypass;
>>       Notifier machine_done;
>>       bool granule_frozen;
>> +    uint8_t aw_bits;
>>   };
>>   #endif
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index ec2ba11d1d..7870bdbeee 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>        */
>>       s->config.bypass = s->boot_bypass;
>>       s->config.page_size_mask = qemu_real_host_page_mask();
>> -    s->config.input_range.end = UINT64_MAX;
>> +    if (s->aw_bits < 32 || s->aw_bits > 64) {
>> +        error_setg(errp, "aw-bits must be within [32,64]");
>> +    }
>> +    s->config.input_range.end =
>> +        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
> 
> 
> This could be simplified :
> 
>    s->config.input_range.end = BIT_ULL(s->aw_bits) - 1;

Forget that. We would need a int28.

Thanks,

C.



> 
> Anyhow,
> 
> 
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
> 
> Thanks,
> 
> C.
> 
> 
> 
>>       s->config.domain_range.end = UINT32_MAX;
>>       s->config.probe_size = VIOMMU_PROBE_SIZE;
>> @@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>       DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>                        TYPE_PCI_BUS, PCIBus *),
>>       DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
> 

Re: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Eric Auger]

Hi Jean,

On 2/5/24 11:13, Jean-Philippe Brucker wrote:
> Hi Eric,
>
> On Thu, Feb 01, 2024 at 05:32:22PM +0100, Eric Auger wrote:
>> aw-bits is a new option that allows to set the bit width of
>> the input address range. This value will be used as a default for
>> the device config input_range.end. By default it is set to 64 bits
>> which is the current value.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - Check the aw-bits value is within [32,64]
>> ---
>>  include/hw/virtio/virtio-iommu.h | 1 +
>>  hw/virtio/virtio-iommu.c         | 7 ++++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>> index 781ebaea8f..5fbe4677c2 100644
>> --- a/include/hw/virtio/virtio-iommu.h
>> +++ b/include/hw/virtio/virtio-iommu.h
>> @@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>>      bool boot_bypass;
>>      Notifier machine_done;
>>      bool granule_frozen;
>> +    uint8_t aw_bits;
>>  };
>>  
>>  #endif
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index ec2ba11d1d..7870bdbeee 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>       */
>>      s->config.bypass = s->boot_bypass;
>>      s->config.page_size_mask = qemu_real_host_page_mask();
>> -    s->config.input_range.end = UINT64_MAX;
>> +    if (s->aw_bits < 32 || s->aw_bits > 64) {
> I'm wondering if we should lower this to 16 bits, just to support all
> possible host SMMU configurations (the smallest address space configurable
> with T0SZ is 25-bit, or 16-bit with the STT extension).
I have no objection relaxing that check. Alex suggested this 32b low
limit with his knowledge of x86. I am a bit reluctant to add extra
machine specific checks though. Anyway I assume that some [low] aw-bits
will fail with some guests and this will be difficult to understand from
bug reports.

Thanks

Eric
>
> Thanks,
> Jean
>
>> +        error_setg(errp, "aw-bits must be within [32,64]");
>> +    }
>> +    s->config.input_range.end =
>> +        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
>>      s->config.domain_range.end = UINT32_MAX;
>>      s->config.probe_size = VIOMMU_PROBE_SIZE;
>>  
>> @@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>      DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>                       TYPE_PCI_BUS, PCIBus *),
>>      DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> -- 
>> 2.41.0
>>

Re: [PATCH v2 3/3] hw: Set virtio-iommu aw-bits default value on pc_q35_9.0 and arm virt

[Eric Auger]

On 2/5/24 10:33, Cédric Le Goater wrote:
> On 2/1/24 17:32, Eric Auger wrote:
>> Currently the default input range can extend to 64 bits. On x86,
>> when the virtio-iommu protects vfio devices, the physical iommu
>> may support only 39 bits. Let's set the default to 39, as done
>> for the intel-iommu. On ARM we set 48b as a default (matching
>> SMMUv3 SMMU_IDR5.VAX == 0).
>>
>> We use hw_compat_8_2 to handle the compatibility for machines
>> before 9.0 which used to have a virtio-iommu default input range
>> of 64 bits.
>>
>> Of course if aw-bits is set from the command line, the default
>> is overriden.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - set aw-bits to 48b on ARM
>> - use hw_compat_8_2 to handle the compat for older machines
>>    which used 64b as a default
>> ---
>>   hw/arm/virt.c            | 6 ++++++
>>   hw/core/machine.c        | 5 ++++-
>>   hw/i386/pc.c             | 6 ++++++
>>   hw/virtio/virtio-iommu.c | 2 +-
>>   4 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index e6ead2c5c8..56539f2fc5 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2718,10 +2718,16 @@ static void
>> virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>           virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>> MACHINE(hotplug_dev), errp);
>>       } else if (object_dynamic_cast(OBJECT(dev),
>> TYPE_VIRTIO_IOMMU_PCI)) {
>> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> +                                                   "aw-bits", NULL);
>
> object_property_get_uint() should not fail. Please use &error_abort.
sure
>
>>           hwaddr db_start = 0, db_end = 0;
>>           QList *reserved_regions;
>>           char *resv_prop_str;
>>   +        if (!aw_bits) {
>> +            qdev_prop_set_uint8(dev, "aw-bits", 48);
>> +        }
>> +
>>           if (vms->iommu != VIRT_IOMMU_NONE) {
>>               error_setg(errp, "virt machine does not support
>> multiple IOMMUs");
>>               return;
>> diff --git a/hw/core/machine.c b/hw/core/machine.c
>> index fb5afdcae4..70ac96954c 100644
>> --- a/hw/core/machine.c
>> +++ b/hw/core/machine.c
>> @@ -30,9 +30,12 @@
>>   #include "exec/confidential-guest-support.h"
>>   #include "hw/virtio/virtio-pci.h"
>>   #include "hw/virtio/virtio-net.h"
>> +#include "hw/virtio/virtio-iommu.h"
>>   #include "audio/audio.h"
>>   -GlobalProperty hw_compat_8_2[] = {};
>> +GlobalProperty hw_compat_8_2[] = {
>> +    { TYPE_VIRTIO_IOMMU_PCI, "aw-bits", "64" },
>> +};
>>   const size_t hw_compat_8_2_len = G_N_ELEMENTS(hw_compat_8_2);
>>     GlobalProperty hw_compat_8_1[] = {
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 803244e5cc..0e2bcb4840 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -1458,6 +1458,8 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>       } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
>>           virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev),
>> MACHINE(hotplug_dev), errp);
>>       } else if (object_dynamic_cast(OBJECT(dev),
>> TYPE_VIRTIO_IOMMU_PCI)) {
>> +        uint8_t aw_bits = object_property_get_uint(OBJECT(dev),
>> +                                                   "aw-bits", NULL);
>>           /* Declare the APIC range as the reserved MSI region */
>>           char *resv_prop_str =
>> g_strdup_printf("0xfee00000:0xfeefffff:%d",
>>                                                
>> VIRTIO_IOMMU_RESV_MEM_T_MSI);
>> @@ -1466,6 +1468,10 @@ static void
>> pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>           qlist_append_str(reserved_regions, resv_prop_str);
>>           qdev_prop_set_array(dev, "reserved-regions",
>> reserved_regions);
>>   +        if (!aw_bits) {
>> +            qdev_prop_set_uint8(dev, "aw-bits", 39);
>
> May be use VTD_HOST_AW_39BIT instead of 39 ? This would make it
> easier to find uses of certain defaults values and would clarify
> that the default AW of virtio-iommu is set as intel-iommu.

OK

Thanks!

Eric
>
> Thanks,
>
> C.
>
>
>
>> +        }
>> +
>>           g_free(resv_prop_str);
>>       }
>>   diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index 7870bdbeee..c468e9b13b 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1529,7 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>       DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>                        TYPE_PCI_BUS, PCIBus *),
>>       DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> -    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 0),
>>       DEFINE_PROP_END_OF_LIST(),
>>   };
>>   
>

Re: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Eric Auger]

Hi Jean,

On 2/5/24 11:13, Jean-Philippe Brucker wrote:
> Hi Eric,
>
> On Thu, Feb 01, 2024 at 05:32:22PM +0100, Eric Auger wrote:
>> aw-bits is a new option that allows to set the bit width of
>> the input address range. This value will be used as a default for
>> the device config input_range.end. By default it is set to 64 bits
>> which is the current value.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v1 -> v2:
>> - Check the aw-bits value is within [32,64]
>> ---
>>  include/hw/virtio/virtio-iommu.h | 1 +
>>  hw/virtio/virtio-iommu.c         | 7 ++++++-
>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/virtio/virtio-iommu.h b/include/hw/virtio/virtio-iommu.h
>> index 781ebaea8f..5fbe4677c2 100644
>> --- a/include/hw/virtio/virtio-iommu.h
>> +++ b/include/hw/virtio/virtio-iommu.h
>> @@ -66,6 +66,7 @@ struct VirtIOIOMMU {
>>      bool boot_bypass;
>>      Notifier machine_done;
>>      bool granule_frozen;
>> +    uint8_t aw_bits;
>>  };
>>  
>>  #endif
>> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
>> index ec2ba11d1d..7870bdbeee 100644
>> --- a/hw/virtio/virtio-iommu.c
>> +++ b/hw/virtio/virtio-iommu.c
>> @@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
>>       */
>>      s->config.bypass = s->boot_bypass;
>>      s->config.page_size_mask = qemu_real_host_page_mask();
>> -    s->config.input_range.end = UINT64_MAX;
>> +    if (s->aw_bits < 32 || s->aw_bits > 64) {
> I'm wondering if we should lower this to 16 bits, just to support all
> possible host SMMU configurations (the smallest address space configurable
> with T0SZ is 25-bit, or 16-bit with the STT extension).
Is it a valid use case case to assign host devices protected by
virtio-iommu with a physical SMMU featuring Small Translation Table?
It leaves 64kB IOVA space only. Besides in the spec, it is wriiten the
min T0SZ can even be 12.

"The minimum valid value is 16 unless all of the following also hold, in
which case the minimum permitted
value is 12:
– SMMUv3.1 or later is supported.
– SMMU_IDR5.VAX indicates support for 52-bit Vas.
– The corresponding CD.TGx selects a 64KB granule.
"

At the moment I would prefer to stick to the limit suggested by Alex
which looks also sensible for other archs whereas 16 doesn't.

Thanks

Eric
>
> Thanks,
> Jean
>
>> +        error_setg(errp, "aw-bits must be within [32,64]");
>> +    }
>> +    s->config.input_range.end =
>> +        s->aw_bits == 64 ? UINT64_MAX : BIT_ULL(s->aw_bits) - 1;
>>      s->config.domain_range.end = UINT32_MAX;
>>      s->config.probe_size = VIOMMU_PROBE_SIZE;
>>  
>> @@ -1525,6 +1529,7 @@ static Property virtio_iommu_properties[] = {
>>      DEFINE_PROP_LINK("primary-bus", VirtIOIOMMU, primary_bus,
>>                       TYPE_PCI_BUS, PCIBus *),
>>      DEFINE_PROP_BOOL("boot-bypass", VirtIOIOMMU, boot_bypass, true),
>> +    DEFINE_PROP_UINT8("aw-bits", VirtIOIOMMU, aw_bits, 64),
>>      DEFINE_PROP_END_OF_LIST(),
>>  };
>>  
>> -- 
>> 2.41.0
>>

Re: [PATCH v2 1/3] virtio-iommu: Add an option to define the input range width

[Jean-Philippe Brucker]

On Thu, Feb 08, 2024 at 09:16:35AM +0100, Eric Auger wrote:
> >> diff --git a/hw/virtio/virtio-iommu.c b/hw/virtio/virtio-iommu.c
> >> index ec2ba11d1d..7870bdbeee 100644
> >> --- a/hw/virtio/virtio-iommu.c
> >> +++ b/hw/virtio/virtio-iommu.c
> >> @@ -1314,7 +1314,11 @@ static void virtio_iommu_device_realize(DeviceState *dev, Error **errp)
> >>       */
> >>      s->config.bypass = s->boot_bypass;
> >>      s->config.page_size_mask = qemu_real_host_page_mask();
> >> -    s->config.input_range.end = UINT64_MAX;
> >> +    if (s->aw_bits < 32 || s->aw_bits > 64) {
> > I'm wondering if we should lower this to 16 bits, just to support all
> > possible host SMMU configurations (the smallest address space configurable
> > with T0SZ is 25-bit, or 16-bit with the STT extension).
> Is it a valid use case case to assign host devices protected by
> virtio-iommu with a physical SMMU featuring Small Translation Table?

Probably not, I'm guessing STT is for tiny embedded implementations where
QEMU or even virtualization is not a use case. But because smaller mobile
platforms now implement SMMUv3, using smaller IOVA spaces and thus fewer
page tables can be beneficial. One use case I have in mind is android with
pKVM, each app has its own VM, and devices can be partitioned into lots of
address spaces with PASID, so you can save a lot of memory and table-walk
time by shrinking those address space. But that particular case will use
crosvm so isn't relevant here, it's only an example.

Mainly I was concerned that if the Linux driver decides to allow
configuring smaller address spaces (maybe a linux cmdline option), then
using a architectural limit here would be a safe bet that things can still
work. But we can always change it in a later version, or implement finer
controls (ideally the guest driver would configure the VA size in ATTACH).

> It leaves 64kB IOVA space only. Besides in the spec, it is wriiten the
> min T0SZ can even be 12.
> 
> "The minimum valid value is 16 unless all of the following also hold, in
> which case the minimum permitted
> value is 12:
> – SMMUv3.1 or later is supported.
> – SMMU_IDR5.VAX indicates support for 52-bit Vas.
> – The corresponding CD.TGx selects a 64KB granule.
> "

Yes that's confusing because va_size = 64 - T0SZ, so T0SZ=12 actually
describes the largest address size, 52.

> 
> At the moment I would prefer to stick to the limit suggested by Alex
> which looks also sensible for other archs whereas 16 doesn't.

Agreed, it should be sufficient.

Thanks,
Jean