diff mbox series

[6/6] pc: Allow instantiating a virtio-iommu device

Message ID 20210810084505.2257983-7-jean-philippe@linaro.org
State New
Headers show
Series virtio-iommu: Add ACPI support | expand

Commit Message

Jean-Philippe Brucker Aug. 10, 2021, 8:45 a.m. UTC
From: Eric Auger <eric.auger@redhat.com>


Add a hotplug handler for virtio-iommu on x86 and set the necessary
reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA
region is reserved for MSIs. DMA transactions to this range either
trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.

Although virtio-iommu does not support IRQ remapping it must be informed
of the reserved region so that it can forward DMA transactions targeting
this region.

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

Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

---
 hw/i386/pc.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

-- 
2.32.0

Comments

Eric Auger Aug. 17, 2021, 2:11 p.m. UTC | #1
Hi Jean,

On 8/10/21 10:45 AM, Jean-Philippe Brucker wrote:
> From: Eric Auger <eric.auger@redhat.com>

>

> Add a hotplug handler for virtio-iommu on x86 and set the necessary

> reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA

> region is reserved for MSIs. DMA transactions to this range either

> trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.

>

> Although virtio-iommu does not support IRQ remapping it must be informed

> of the reserved region so that it can forward DMA transactions targeting

> this region.

>

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

> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>


I think we need to handle the case where the end-user gets lost with
iommu options and use an invalid combination such as

-M q35,iommu=on,int_remap=on,kernel_irqchip=off -device -device virtio-iommu-pci

We may also document somewhere that the virtio-iommu-pci
does not support irq remapping as this may be an important limitation on x86.

Thanks

Eric

> ---

>  hw/i386/pc.c | 11 ++++++++++-

>  1 file changed, 10 insertions(+), 1 deletion(-)

>

> diff --git a/hw/i386/pc.c b/hw/i386/pc.c

> index 694fc9ce07..fb24f000e7 100644

> --- a/hw/i386/pc.c

> +++ b/hw/i386/pc.c

> @@ -1376,6 +1376,14 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,

>      } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||

>                 object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {

>          pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);

> +    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {

> +        /* Declare the reserved MSI region */

> +        char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",

> +                                              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);

>      }

>  }

>  

> @@ -1436,7 +1444,8 @@ static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,

>      if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||

>          object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||

>          object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||

> -        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {

> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||

> +        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {

>          return HOTPLUG_HANDLER(machine);

>      }

>
Jean-Philippe Brucker Aug. 27, 2021, 1:26 p.m. UTC | #2
On Tue, Aug 17, 2021 at 04:11:49PM +0200, Eric Auger wrote:
> Hi Jean,

> 

> On 8/10/21 10:45 AM, Jean-Philippe Brucker wrote:

> > From: Eric Auger <eric.auger@redhat.com>

> >

> > Add a hotplug handler for virtio-iommu on x86 and set the necessary

> > reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA

> > region is reserved for MSIs. DMA transactions to this range either

> > trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.

> >

> > Although virtio-iommu does not support IRQ remapping it must be informed

> > of the reserved region so that it can forward DMA transactions targeting

> > this region.

> >

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

> > Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> 

> I think we need to handle the case where the end-user gets lost with

> iommu options and use an invalid combination such as

> 

> -M q35,iommu=on,int_remap=on,kernel_irqchip=off -device -device virtio-iommu-pci


I guess that would be
"-M q35,kernel_irqchip=off -device intel-iommu,intremap=on -device virtio-iommu-pci"

I'll add the checks, similar to the one in x86_iommu_set_default().

> We may also document somewhere that the virtio-iommu-pci

> does not support irq remapping as this may be an important limitation on x86.


I'll mention it in the commit message, unless you had another place in
mind?

Thanks,
Jean
Eric Auger Sept. 2, 2021, 9:36 a.m. UTC | #3
Hi Jean,

On 8/27/21 3:26 PM, Jean-Philippe Brucker wrote:
> On Tue, Aug 17, 2021 at 04:11:49PM +0200, Eric Auger wrote:

>> Hi Jean,

>>

>> On 8/10/21 10:45 AM, Jean-Philippe Brucker wrote:

>>> From: Eric Auger <eric.auger@redhat.com>

>>>

>>> Add a hotplug handler for virtio-iommu on x86 and set the necessary

>>> reserved region property. On x86, the [0xfee00000, 0xfeefffff] DMA

>>> region is reserved for MSIs. DMA transactions to this range either

>>> trigger IRQ remapping in the IOMMU or bypasses IOMMU translation.

>>>

>>> Although virtio-iommu does not support IRQ remapping it must be informed

>>> of the reserved region so that it can forward DMA transactions targeting

>>> this region.

>>>

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

>>> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

>> I think we need to handle the case where the end-user gets lost with

>> iommu options and use an invalid combination such as

>>

>> -M q35,iommu=on,int_remap=on,kernel_irqchip=off -device -device virtio-iommu-pci

> I guess that would be

> "-M q35,kernel_irqchip=off -device intel-iommu,intremap=on -device virtio-iommu-pci"

>

> I'll add the checks, similar to the one in x86_iommu_set_default().

yes that what I meant sorry.
>

>> We may also document somewhere that the virtio-iommu-pci

>> does not support irq remapping as this may be an important limitation on x86.

> I'll mention it in the commit message, unless you had another place in

> mind?


you may add an entry in qemu-options.hx too as Peter did for the intel
iommu in
7395b3e3e7  docs: Add '-device intel-iommu' entry (7 weeks ago) <Peter Xu>

Thanks

Eric
>

> Thanks,

> Jean

>
diff mbox series

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 694fc9ce07..fb24f000e7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1376,6 +1376,14 @@  static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
                object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
         pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
+        /* Declare the reserved MSI region */
+        char *resv_prop_str = g_strdup_printf("0xfee00000:0xfeefffff:%d",
+                                              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);
     }
 }
 
@@ -1436,7 +1444,8 @@  static HotplugHandler *pc_get_hotplug_handler(MachineState *machine,
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
         object_dynamic_cast(OBJECT(dev), TYPE_CPU) ||
         object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
-        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
         return HOTPLUG_HANDLER(machine);
     }