diff mbox series

[v2,7/8] pc: Allow instantiating a virtio-iommu device

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

Commit Message

Jean-Philippe Brucker Sept. 3, 2021, 2:32 p.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 | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

-- 
2.33.0

Comments

Eric Auger Sept. 6, 2021, 2:57 p.m. UTC | #1 
Hi Jean,

On 9/3/21 4:32 PM, 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.

s/bypasses/bypass.
> 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>

Feel free to remove my SoB. I have done much here.
> Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---

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

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

>

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

> index 694fc9ce07..c1e1cffe16 100644

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

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

> @@ -797,6 +797,11 @@ void pc_machine_done(Notifier *notifier, void *data)

>                       "irqchip support.");

>          exit(EXIT_FAILURE);

>      }

> +

> +    if (pcms->virtio_iommu && x86_iommu_get_default()) {

> +        error_report("QEMU does not support multiple vIOMMUs for x86 yet.");

> +        exit(EXIT_FAILURE);

> +    }

>  }


I think you shall detect the case of dual instantiation of intel_iommu
and virtio-iommu. Maybe pc_hotplug_allowed() can be used for that. Note
that both devices can be refered to in either order.
>  

>  void pc_guest_info_init(PCMachineState *pcms)

> @@ -1376,6 +1381,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);

>      }

>  }

>  

> @@ -1393,6 +1406,11 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,

>          PCMachineState *pcms = PC_MACHINE(hotplug_dev);

>          PCIDevice *pdev = PCI_DEVICE(dev);

>  

> +        if (pcms->virtio_iommu) {

> +            error_setg(errp,

> +                       "QEMU does not support multiple vIOMMUs for x86 yet.");

> +            return;

> +        }

>          pcms->virtio_iommu = true;

>          pcms->virtio_iommu_bdf = pci_get_bdf(pdev);

>      }

> @@ -1436,7 +1454,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);

>      }

>  

Thanks

Eric
Jean-Philippe Brucker Sept. 8, 2021, 9:54 a.m. UTC | #2 
On Mon, Sep 06, 2021 at 04:57:39PM +0200, Eric Auger wrote:
> Hi Jean,

> 

> On 9/3/21 4:32 PM, 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.

> s/bypasses/bypass.

> > 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>

> Feel free to remove my SoB. I have done much here.

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

> > ---

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

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

> >

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

> > index 694fc9ce07..c1e1cffe16 100644

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

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

> > @@ -797,6 +797,11 @@ void pc_machine_done(Notifier *notifier, void *data)

> >                       "irqchip support.");

> >          exit(EXIT_FAILURE);

> >      }

> > +

> > +    if (pcms->virtio_iommu && x86_iommu_get_default()) {

> > +        error_report("QEMU does not support multiple vIOMMUs for x86 yet.");

> > +        exit(EXIT_FAILURE);

> > +    }

> >  }

> 

> I think you shall detect the case of dual instantiation of intel_iommu

> and virtio-iommu.


Those are handled by x86_iommu_set_default() and
pc_machine_device_plug_cb() (below) respectively. I considered factoring
it but I think it just adds complexity.

Thanks,
Jean

> Maybe pc_hotplug_allowed() can be used for that. Note

> that both devices can be refered to in either order.

> >  

> >  void pc_guest_info_init(PCMachineState *pcms)

> > @@ -1376,6 +1381,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);

> >      }

> >  }

> >  

> > @@ -1393,6 +1406,11 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,

> >          PCMachineState *pcms = PC_MACHINE(hotplug_dev);

> >          PCIDevice *pdev = PCI_DEVICE(dev);

> >  

> > +        if (pcms->virtio_iommu) {

> > +            error_setg(errp,

> > +                       "QEMU does not support multiple vIOMMUs for x86 yet.");

> > +            return;

> > +        }

> >          pcms->virtio_iommu = true;

> >          pcms->virtio_iommu_bdf = pci_get_bdf(pdev);

> >      }

> > @@ -1436,7 +1454,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);

> >      }

> >  

> Thanks

> 

> Eric

>
Eric Auger Sept. 8, 2021, 9:58 a.m. UTC | #3 
Hi jean,

On 9/8/21 11:54 AM, Jean-Philippe Brucker wrote:
> On Mon, Sep 06, 2021 at 04:57:39PM +0200, Eric Auger wrote:

>> Hi Jean,

>>

>> On 9/3/21 4:32 PM, 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.

>> s/bypasses/bypass.

>>> 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>

>> Feel free to remove my SoB. I have done much here.

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

>>> ---

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

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

>>>

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

>>> index 694fc9ce07..c1e1cffe16 100644

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

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

>>> @@ -797,6 +797,11 @@ void pc_machine_done(Notifier *notifier, void *data)

>>>                       "irqchip support.");

>>>          exit(EXIT_FAILURE);

>>>      }

>>> +

>>> +    if (pcms->virtio_iommu && x86_iommu_get_default()) {

>>> +        error_report("QEMU does not support multiple vIOMMUs for x86 yet.");

>>> +        exit(EXIT_FAILURE);

>>> +    }

>>>  }

>> I think you shall detect the case of dual instantiation of intel_iommu

>> and virtio-iommu.

> Those are handled by x86_iommu_set_default() and

Oh I did not notice the x86_iommu_get_default() stuff.

sorry for the noise

Eric


> pc_machine_device_plug_cb() (below) respectively. I considered factoring

> it but I think it just adds complexity.

>

> Thanks,

> Jean

>

>> Maybe pc_hotplug_allowed() can be used for that. Note

>> that both devices can be refered to in either order.

>>>  

>>>  void pc_guest_info_init(PCMachineState *pcms)

>>> @@ -1376,6 +1381,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);

>>>      }

>>>  }

>>>  

>>> @@ -1393,6 +1406,11 @@ static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,

>>>          PCMachineState *pcms = PC_MACHINE(hotplug_dev);

>>>          PCIDevice *pdev = PCI_DEVICE(dev);

>>>  

>>> +        if (pcms->virtio_iommu) {

>>> +            error_setg(errp,

>>> +                       "QEMU does not support multiple vIOMMUs for x86 yet.");

>>> +            return;

>>> +        }

>>>          pcms->virtio_iommu = true;

>>>          pcms->virtio_iommu_bdf = pci_get_bdf(pdev);

>>>      }

>>> @@ -1436,7 +1454,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);

>>>      }

>>>  

>> Thanks

>>

>> Eric

>>
diff mbox series

Patch

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 694fc9ce07..c1e1cffe16 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -797,6 +797,11 @@  void pc_machine_done(Notifier *notifier, void *data)
                      "irqchip support.");
         exit(EXIT_FAILURE);
     }
+
+    if (pcms->virtio_iommu && x86_iommu_get_default()) {
+        error_report("QEMU does not support multiple vIOMMUs for x86 yet.");
+        exit(EXIT_FAILURE);
+    }
 }
 
 void pc_guest_info_init(PCMachineState *pcms)
@@ -1376,6 +1381,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);
     }
 }
 
@@ -1393,6 +1406,11 @@  static void pc_machine_device_plug_cb(HotplugHandler *hotplug_dev,
         PCMachineState *pcms = PC_MACHINE(hotplug_dev);
         PCIDevice *pdev = PCI_DEVICE(dev);
 
+        if (pcms->virtio_iommu) {
+            error_setg(errp,
+                       "QEMU does not support multiple vIOMMUs for x86 yet.");
+            return;
+        }
         pcms->virtio_iommu = true;
         pcms->virtio_iommu_bdf = pci_get_bdf(pdev);
     }
@@ -1436,7 +1454,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);
     }