| Message ID | 20200507143201.31080-6-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | VIRTIO-IOMMU probe request support and MSI bypass on ARM | expand |
On Thu, May 07, 2020 at 04:32:01PM +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. The ITS does MSI translation > anyway. Makes sense. I think we have to check that the ITS is used before declaring the resv region, though. When using gicv2m, the guest will see this MSI region and deduce that it doesn't need to create a SW mapping for MSIs (might be wrong, I haven't tested it yet). > Signed-off-by: Eric Auger <eric.auger@redhat.com> > --- > hw/arm/virt.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 171e6908ec..6a041e97a5 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -2138,6 +2138,11 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *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 */ > + qdev_prop_set_uint32(dev, "len-reserved-regions", 1); > + qdev_prop_set_string(dev, "reserved-regions[0]", > + "0x8090000, 0x809FFFF, 1"); Maybe worth adding a comment saying this is the GITS_TRANSLATER page? Thanks, Jean
Hi Jean-Philippe, On 5/8/20 10:21 AM, Jean-Philippe Brucker wrote: > On Thu, May 07, 2020 at 04:32:01PM +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. The ITS does MSI translation >> anyway. > > Makes sense. I think we have to check that the ITS is used before > declaring the resv region, though. When using gicv2m, the guest will see > this MSI region and deduce that it doesn't need to create a SW mapping for > MSIs (might be wrong, I haven't tested it yet). Yes you're right. I think any MSI region hides the SW MSI one. So I will test the GIC type beforehand. Also I will extend the series to handle the GICv2M backdoor To me the problem is similar and the iommu subsystem will map the GICV2M MSI doorbell as well. From a security pov, there is no difference inbetween the 2 solutions. Anyway the doorbell is reachable by any assigned device, would it be mapped or not. And given the GICv2M does not perform any interrupt translation, an assigned device can trigger MSIs on another userspace driver. Thanks Eric > >> Signed-off-by: Eric Auger <eric.auger@redhat.com> >> --- >> hw/arm/virt.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 171e6908ec..6a041e97a5 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -2138,6 +2138,11 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *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 */ >> + qdev_prop_set_uint32(dev, "len-reserved-regions", 1); >> + qdev_prop_set_string(dev, "reserved-regions[0]", >> + "0x8090000, 0x809FFFF, 1"); > > Maybe worth adding a comment saying this is the GITS_TRANSLATER page? > > Thanks, > Jean >
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 171e6908ec..6a041e97a5 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -2138,6 +2138,11 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *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 */ + qdev_prop_set_uint32(dev, "len-reserved-regions", 1); + qdev_prop_set_string(dev, "reserved-regions[0]", + "0x8090000, 0x809FFFF, 1"); } }
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. The ITS does MSI translation anyway. Signed-off-by: Eric Auger <eric.auger@redhat.com> --- hw/arm/virt.c | 5 +++++ 1 file changed, 5 insertions(+)