| Message ID | 20210810084505.2257983-7-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio-iommu: Add ACPI support | expand |
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); > } >
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
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 --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); }