1. Patchew
  2. QEMU
  3. [PATCH v5 0/5] VIRTIO-IOMMU probe request support and MSI bypass on ARM
  4. View patch

[PATCH v5 5/5] hw/arm/virt: Let the virtio-iommu bypass MSIs

Eric Auger posted 5 patches 5 years, 7 months ago
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>
There is a newer version of this series
[PATCH v5 5/5] hw/arm/virt: Let the virtio-iommu bypass MSIs
Posted by Eric Auger 5 years, 7 months ago 
At the moment the virtio-iommu translates MSI transactions.
This behavior is inherited from ARM SMMU. The virt machine
code knows where the guest MSI doorbells are so we can easily
declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
setting the guest will not map MSIs through the IOMMU and those
transactions will be simply bypassed.

Depending on which MSI controller is in use (ITS or GICV2M),
we declare either:
- the ITS interrupt translation space (ITS_base + 0x10000),
  containing the GITS_TRANSLATOR or
- The GICV2M single frame, containing the MSI_SETSP_NS register.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

---
v3 -> v4:
- use ':' as separators

v2 -> v3:
- Add a new value to VirtMSIControllerType

v1 -> v2:
- Test which MSI controller is instantiated
- If GICV2M is in use, declare its doorbell as an MSI doorbell too
---
 include/hw/arm/virt.h |  7 +++++++
 hw/arm/virt.c         | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 31878ddc72..a18b6b397b 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -96,6 +96,12 @@ typedef enum VirtIOMMUType {
     VIRT_IOMMU_VIRTIO,
 } VirtIOMMUType;
 
+typedef enum VirtMSIControllerType {
+    VIRT_MSI_CTRL_NONE,
+    VIRT_MSI_CTRL_GICV2M,
+    VIRT_MSI_CTRL_ITS,
+} VirtMSIControllerType;
+
 typedef enum VirtGICType {
     VIRT_GIC_VERSION_MAX,
     VIRT_GIC_VERSION_HOST,
@@ -136,6 +142,7 @@ typedef struct {
     OnOffAuto acpi;
     VirtGICType gic_version;
     VirtIOMMUType iommu;
+    VirtMSIControllerType msi_controller;
     uint16_t virtio_iommu_bdf;
     struct arm_boot_info bootinfo;
     MemMapEntry *memmap;
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index caceb1e4a0..75cf31fcba 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -602,6 +602,7 @@ static void create_its(VirtMachineState *vms)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, vms->memmap[VIRT_GIC_ITS].base);
 
     fdt_add_its_gic_node(vms);
+    vms->msi_controller = VIRT_MSI_CTRL_ITS;
 }
 
 static void create_v2m(VirtMachineState *vms)
@@ -622,6 +623,7 @@ static void create_v2m(VirtMachineState *vms)
     }
 
     fdt_add_v2m_gic_node(vms);
+    vms->msi_controller = VIRT_MSI_CTRL_GICV2M;
 }
 
 static void create_gic(VirtMachineState *vms)
@@ -2149,8 +2151,24 @@ out:
 static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
                                             DeviceState *dev, Error **errp)
 {
+    VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
+
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
         virt_memory_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        /* we declare a VIRTIO_IOMMU_RESV_MEM_T_MSI region */
+
+        if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
+            /* GITS_TRANSLATER page */
+            qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
+            qdev_prop_set_string(dev, "reserved-regions[0]",
+                                 "0x8090000:0x809FFFF:1");
+        } else if (vms->msi_controller == VIRT_MSI_CTRL_GICV2M) {
+            /* MSI_SETSPI_NS page */
+            qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
+            qdev_prop_set_string(dev, "reserved-regions[0]",
+                                 "0x8020000:0x8020FFF:1");
+        }
     }
 }
 
-- 
2.20.1
Re: [PATCH v5 5/5] hw/arm/virt: Let the virtio-iommu bypass MSIs
Posted by Peter Maydell 5 years, 7 months ago 
On Wed, 24 Jun 2020 at 14:27, Eric Auger <eric.auger@redhat.com> wrote:
>
> At the moment the virtio-iommu translates MSI transactions.
> This behavior is inherited from ARM SMMU. The virt machine
> code knows where the guest MSI doorbells are so we can easily
> declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
> setting the guest will not map MSIs through the IOMMU and those
> transactions will be simply bypassed.
>
> Depending on which MSI controller is in use (ITS or GICV2M),
> we declare either:
> - the ITS interrupt translation space (ITS_base + 0x10000),
>   containing the GITS_TRANSLATOR or
> - The GICV2M single frame, containing the MSI_SETSP_NS register.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>

>  static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>                                              DeviceState *dev, Error **errp)
>  {
> +    VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
> +
>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>          virt_memory_pre_plug(hotplug_dev, dev, errp);
> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
> +        /* we declare a VIRTIO_IOMMU_RESV_MEM_T_MSI region */
> +
> +        if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
> +            /* GITS_TRANSLATER page */
> +            qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
> +            qdev_prop_set_string(dev, "reserved-regions[0]",
> +                                 "0x8090000:0x809FFFF:1");
> +        } else if (vms->msi_controller == VIRT_MSI_CTRL_GICV2M) {
> +            /* MSI_SETSPI_NS page */
> +            qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
> +            qdev_prop_set_string(dev, "reserved-regions[0]",
> +                                 "0x8020000:0x8020FFF:1");

This hardcodes addresses and lengths that are in the
base_memmap[] array for VIRT_GIC_ITS and VIRT_GIC_V2M,
so it's setting up a bear trap if we ever decide to
move those. Could we construct the string from the
base_memmap[] array entry values instead, please ?

thanks
-- PMM
Re: [PATCH v5 5/5] hw/arm/virt: Let the virtio-iommu bypass MSIs
Posted by Auger Eric 5 years, 7 months ago 
Hi Peter,

On 6/25/20 12:01 PM, Peter Maydell wrote:
> On Wed, 24 Jun 2020 at 14:27, Eric Auger <eric.auger@redhat.com> wrote:
>>
>> At the moment the virtio-iommu translates MSI transactions.
>> This behavior is inherited from ARM SMMU. The virt machine
>> code knows where the guest MSI doorbells are so we can easily
>> declare those regions as VIRTIO_IOMMU_RESV_MEM_T_MSI. With that
>> setting the guest will not map MSIs through the IOMMU and those
>> transactions will be simply bypassed.
>>
>> Depending on which MSI controller is in use (ITS or GICV2M),
>> we declare either:
>> - the ITS interrupt translation space (ITS_base + 0x10000),
>>   containing the GITS_TRANSLATOR or
>> - The GICV2M single frame, containing the MSI_SETSP_NS register.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>>
> 
>>  static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>>                                              DeviceState *dev, Error **errp)
>>  {
>> +    VirtMachineState *vms = VIRT_MACHINE(hotplug_dev);
>> +
>>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
>>          virt_memory_pre_plug(hotplug_dev, dev, errp);
>> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
>> +        /* we declare a VIRTIO_IOMMU_RESV_MEM_T_MSI region */
>> +
>> +        if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
>> +            /* GITS_TRANSLATER page */
>> +            qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
>> +            qdev_prop_set_string(dev, "reserved-regions[0]",
>> +                                 "0x8090000:0x809FFFF:1");
>> +        } else if (vms->msi_controller == VIRT_MSI_CTRL_GICV2M) {
>> +            /* MSI_SETSPI_NS page */
>> +            qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
>> +            qdev_prop_set_string(dev, "reserved-regions[0]",
>> +                                 "0x8020000:0x8020FFF:1");
> 
> This hardcodes addresses and lengths that are in the
> base_memmap[] array for VIRT_GIC_ITS and VIRT_GIC_V2M,
> so it's setting up a bear trap if we ever decide to
> move those. Could we construct the string from the
> base_memmap[] array entry values instead, please ?
Sure

Eric
> 
> thanks
> -- PMM
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.