Message ID | 20200624132625.27453-6-eric.auger@redhat.com |
---|---|
State | New |
Headers | show |
Series | VIRTIO-IOMMU probe request support and MSI bypass on ARM | expand |
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
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 >
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"); + } } }