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

["Michael S. Tsirkin" <mst@redhat.com>]

On Mon, Jun 29, 2020 at 09:04:04AM +0200, Eric Auger 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>

This needs ARM ACK.

> ---
> v5 -> v6:
> - do not hardcode doorbell base and size
> - removed Jean's R-b
> 
> 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         | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 37 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 cd0834ce7f..eb4344cf6e 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)
> @@ -2200,8 +2202,36 @@ 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)) {
> +        hwaddr db_start = 0, db_end = 0;
> +        char *resv_prop_str;
> +
> +        switch (vms->msi_controller) {
> +        case VIRT_MSI_CTRL_NONE:
> +            return;
> +        case VIRT_MSI_CTRL_ITS:
> +            /* GITS_TRANSLATER page */
> +            db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000;
> +            db_end = base_memmap[VIRT_GIC_ITS].base +
> +                     base_memmap[VIRT_GIC_ITS].size - 1;
> +            break;
> +        case VIRT_MSI_CTRL_GICV2M:
> +            /* MSI_SETSPI_NS page */
> +            db_start = base_memmap[VIRT_GIC_V2M].base;
> +            db_end = db_start + base_memmap[VIRT_GIC_V2M].size - 1;
> +            break;
> +        }
> +        resv_prop_str = g_strdup_printf("0x%"PRIx64":0x%"PRIx64":%u",
> +                                        db_start, db_end,
> +                                        VIRTIO_IOMMU_RESV_MEM_T_MSI);
> +
> +        qdev_prop_set_uint32(dev, "len-reserved-regions", 1);
> +        qdev_prop_set_string(dev, "reserved-regions[0]", resv_prop_str);
> +        g_free(resv_prop_str);
>      }
>  }
>  
> -- 
> 2.20.1