diff mbox series

[RFC,v8,15/18] hw/arm/virt: Add virtio-iommu to the virt board

Message ID 20181109112957.10067-16-eric.auger@redhat.com
State New
Headers show
Series VIRTIO-IOMMU device | expand

Commit Message

Eric Auger Nov. 9, 2018, 11:29 a.m. UTC 
Both the virtio-iommu device and its dedicated mmio
transport get instantiated when requested.

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

---

v6 -> v7:
- align to the smmu instantiation code

v4 -> v5:
- VirtMachineClass no_iommu added in this patch
- Use object_resolve_path_type
---
 hw/arm/virt.c | 48 +++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 45 insertions(+), 3 deletions(-)

Comments

Jean-Philippe Brucker Nov. 14, 2018, 4:01 p.m. UTC | #1 
On 09/11/2018 11:29, Eric Auger wrote:
> +static void create_virtio_iommu(VirtMachineState *vms,
> +                                const char *pciehb_nodename, PCIBus *bus)
> +{
> +    const char compat[] = "virtio,pci-iommu";
> +    uint16_t bdf = 0x8; /* 00:01.0 */
> +    DeviceState *dev;
> +    char *node;
> +
> +    dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
> +    object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);

For the Arm virt platform, should msi_bypass default to false? Otherwise
I don't think pass-through to guest userpace will work.

Thanks,
Jean
Eric Auger Nov. 14, 2018, 4:41 p.m. UTC | #2 
Hi Jean,

On 11/14/18 5:01 PM, Jean-Philippe Brucker wrote:
> On 09/11/2018 11:29, Eric Auger wrote:
>> +static void create_virtio_iommu(VirtMachineState *vms,
>> +                                const char *pciehb_nodename, PCIBus *bus)
>> +{
>> +    const char compat[] = "virtio,pci-iommu";
>> +    uint16_t bdf = 0x8; /* 00:01.0 */
>> +    DeviceState *dev;
>> +    char *node;
>> +
>> +    dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
>> +    object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
> 
> For the Arm virt platform, should msi_bypass default to false? Otherwise
> I don't think pass-through to guest userpace will work.
That's correct. It's a regression compared to v7. I will fix this soon
while doing the pc machine integration + resv regions flaws you reported
in the meantime.

Thanks!

Eric
> 
> Thanks,
> Jean
>
Eric Auger Nov. 16, 2018, 9:51 a.m. UTC | #3 
Hi Jean, Bharat,

On 11/14/18 5:41 PM, Auger Eric wrote:
> Hi Jean,
> 
> On 11/14/18 5:01 PM, Jean-Philippe Brucker wrote:
>> On 09/11/2018 11:29, Eric Auger wrote:
>>> +static void create_virtio_iommu(VirtMachineState *vms,
>>> +                                const char *pciehb_nodename, PCIBus *bus)
>>> +{
>>> +    const char compat[] = "virtio,pci-iommu";
>>> +    uint16_t bdf = 0x8; /* 00:01.0 */
>>> +    DeviceState *dev;
>>> +    char *node;
>>> +
>>> +    dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
>>> +    object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
>>
>> For the Arm virt platform, should msi_bypass default to false? Otherwise
>> I don't think pass-through to guest userpace will work.
> That's correct. It's a regression compared to v7. I will fix this soon
> while doing the pc machine integration + resv regions flaws you reported
> in the meantime.

The reported issues related to probe requests are fixed in the following
branch:
https://github.com/eauger/qemu/tree/v3.1.0-rc1-virtio-iommu-v0.8.1

This branch was used to test [PATCH v4 0/7] Add virtio-iommu driver

Thanks

Eric

> 
> Thanks!
> 
> Eric
>>
>> Thanks,
>> Jean
>>
>
Bharat Bhushan Nov. 22, 2018, 9:15 a.m. UTC | #4 
Hi Eric,


> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: Friday, November 9, 2018 5:00 PM
> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
> devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
> mst@redhat.com; jean-philippe.brucker@arm.com
> Cc: kevin.tian@intel.com; tn@semihalf.com; Bharat Bhushan
> <bharat.bhushan@nxp.com>; peterx@redhat.com
> Subject: [RFC v8 15/18] hw/arm/virt: Add virtio-iommu to the virt board
> 
> Both the virtio-iommu device and its dedicated mmio transport get
> instantiated when requested.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
> ---
> 
> v6 -> v7:
> - align to the smmu instantiation code
> 
> v4 -> v5:
> - VirtMachineClass no_iommu added in this patch
> - Use object_resolve_path_type
> ---
>  hw/arm/virt.c | 48
> +++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 45 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c index a2b8d8f7c2..f2994c4359
> 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -29,6 +29,7 @@
>   */
> 
>  #include "qemu/osdep.h"
> +#include "monitor/qdev.h"
>  #include "qapi/error.h"
>  #include "hw/sysbus.h"
>  #include "hw/arm/arm.h"
> @@ -49,6 +50,7 @@
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
>  #include "hw/pci-host/gpex.h"
> +#include "hw/virtio/virtio-pci.h"
>  #include "hw/arm/sysbus-fdt.h"
>  #include "hw/platform-bus.h"
>  #include "hw/arm/fdt.h"
> @@ -59,6 +61,7 @@
>  #include "qapi/visitor.h"
>  #include "standard-headers/linux/input.h"
>  #include "hw/arm/smmuv3.h"
> +#include "hw/virtio/virtio-iommu.h"
> 
>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \ @@ -
> 1085,6 +1088,33 @@ static void create_smmu(const VirtMachineState *vms,
> qemu_irq *pic,
>      g_free(node);
>  }
> 
> +static void create_virtio_iommu(VirtMachineState *vms,
> +                                const char *pciehb_nodename, PCIBus
> +*bus) {
> +    const char compat[] = "virtio,pci-iommu";
> +    uint16_t bdf = 0x8; /* 00:01.0 */

Why we hardcoded "bdf = 8" ?
When adding the VFIO support I see virtio-iommu PCI device have bdf = x010.

Thanks
-Bharat

> +    DeviceState *dev;
> +    char *node;
> +
> +    dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
> +    object_property_set_bool(OBJECT(dev), true, "realized",
> + &error_fatal);
> +
> +    node = g_strdup_printf("%s/virtio_iommu@%d", pciehb_nodename,
> bdf);
> +    qemu_fdt_add_subnode(vms->fdt, node);
> +    qemu_fdt_setprop(vms->fdt, node, "compatible", compat,
> sizeof(compat));
> +    qemu_fdt_setprop_sized_cells(vms->fdt, node, "reg",
> +                                 1, bdf << 8 /* phys.hi */,
> +                                 1, 0        /* phys.mid */,
> +                                 1, 0        /* phys.lo  */,
> +                                 1, 0        /* size.hi  */,
> +                                 1, 0        /* size.low */);
> +
> +    qemu_fdt_setprop_cell(vms->fdt, node, "#iommu-cells", 1);
> +    qemu_fdt_setprop_cell(vms->fdt, node, "phandle", vms-
> >iommu_phandle);
> +    g_free(node);
> +}
> +
> +
>  static void create_pcie(VirtMachineState *vms, qemu_irq *pic)  {
>      hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base; @@ -
> 1205,10 +1235,22 @@ static void create_pcie(VirtMachineState *vms,
> qemu_irq *pic)
>      if (vms->iommu) {
>          vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
> 
> -        create_smmu(vms, pic, pci->bus);
> +        switch (vms->iommu) {
> +        case VIRT_IOMMU_SMMUV3:
> +            create_smmu(vms, pic, pci->bus);
> +            qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
> +                                   0x0, vms->iommu_phandle, 0x0, 0x10000);
> +            break;
> +        case VIRT_IOMMU_VIRTIO:
> +            create_virtio_iommu(vms, nodename, pci->bus);
> +            qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
> +                                   0x0, vms->iommu_phandle, 0x0, 0x8,
> +                                   0x9, vms->iommu_phandle, 0x9, 0xfff7);
> +            break;
> +        default:
> +            g_assert_not_reached();
> +        }
> 
> -        qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
> -                               0x0, vms->iommu_phandle, 0x0, 0x10000);
>      }
> 
>      g_free(nodename);
> --
> 2.17.2
Eric Auger Nov. 22, 2018, 9:25 a.m. UTC | #5 
Hi Bharat,

On 11/22/18 10:15 AM, Bharat Bhushan wrote:
> Hi Eric,
> 
> 
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: Friday, November 9, 2018 5:00 PM
>> To: eric.auger.pro@gmail.com; eric.auger@redhat.com; qemu-
>> devel@nongnu.org; qemu-arm@nongnu.org; peter.maydell@linaro.org;
>> mst@redhat.com; jean-philippe.brucker@arm.com
>> Cc: kevin.tian@intel.com; tn@semihalf.com; Bharat Bhushan
>> <bharat.bhushan@nxp.com>; peterx@redhat.com
>> Subject: [RFC v8 15/18] hw/arm/virt: Add virtio-iommu to the virt board
>>
>> Both the virtio-iommu device and its dedicated mmio transport get
>> instantiated when requested.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>>
>> v6 -> v7:
>> - align to the smmu instantiation code
>>
>> v4 -> v5:
>> - VirtMachineClass no_iommu added in this patch
>> - Use object_resolve_path_type
>> ---
>>  hw/arm/virt.c | 48
>> +++++++++++++++++++++++++++++++++++++++++++++---
>>  1 file changed, 45 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c index a2b8d8f7c2..f2994c4359
>> 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -29,6 +29,7 @@
>>   */
>>
>>  #include "qemu/osdep.h"
>> +#include "monitor/qdev.h"
>>  #include "qapi/error.h"
>>  #include "hw/sysbus.h"
>>  #include "hw/arm/arm.h"
>> @@ -49,6 +50,7 @@
>>  #include "qemu/bitops.h"
>>  #include "qemu/error-report.h"
>>  #include "hw/pci-host/gpex.h"
>> +#include "hw/virtio/virtio-pci.h"
>>  #include "hw/arm/sysbus-fdt.h"
>>  #include "hw/platform-bus.h"
>>  #include "hw/arm/fdt.h"
>> @@ -59,6 +61,7 @@
>>  #include "qapi/visitor.h"
>>  #include "standard-headers/linux/input.h"
>>  #include "hw/arm/smmuv3.h"
>> +#include "hw/virtio/virtio-iommu.h"
>>
>>  #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
>>      static void virt_##major##_##minor##_class_init(ObjectClass *oc, \ @@ -
>> 1085,6 +1088,33 @@ static void create_smmu(const VirtMachineState *vms,
>> qemu_irq *pic,
>>      g_free(node);
>>  }
>>
>> +static void create_virtio_iommu(VirtMachineState *vms,
>> +                                const char *pciehb_nodename, PCIBus
>> +*bus) {
>> +    const char compat[] = "virtio,pci-iommu";
>> +    uint16_t bdf = 0x8; /* 00:01.0 */
> 
> Why we hardcoded "bdf = 8" ?
> When adding the VFIO support I see virtio-iommu PCI device have bdf = x010.

This hardcoding has been removed. I will send a new version today.

Thanks

Eric
> 
> Thanks
> -Bharat
> 
>> +    DeviceState *dev;
>> +    char *node;
>> +
>> +    dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
>> +    object_property_set_bool(OBJECT(dev), true, "realized",
>> + &error_fatal);
>> +
>> +    node = g_strdup_printf("%s/virtio_iommu@%d", pciehb_nodename,
>> bdf);
>> +    qemu_fdt_add_subnode(vms->fdt, node);
>> +    qemu_fdt_setprop(vms->fdt, node, "compatible", compat,
>> sizeof(compat));
>> +    qemu_fdt_setprop_sized_cells(vms->fdt, node, "reg",
>> +                                 1, bdf << 8 /* phys.hi */,
>> +                                 1, 0        /* phys.mid */,
>> +                                 1, 0        /* phys.lo  */,
>> +                                 1, 0        /* size.hi  */,
>> +                                 1, 0        /* size.low */);
>> +
>> +    qemu_fdt_setprop_cell(vms->fdt, node, "#iommu-cells", 1);
>> +    qemu_fdt_setprop_cell(vms->fdt, node, "phandle", vms-
>>> iommu_phandle);
>> +    g_free(node);
>> +}
>> +
>> +
>>  static void create_pcie(VirtMachineState *vms, qemu_irq *pic)  {
>>      hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base; @@ -
>> 1205,10 +1235,22 @@ static void create_pcie(VirtMachineState *vms,
>> qemu_irq *pic)
>>      if (vms->iommu) {
>>          vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
>>
>> -        create_smmu(vms, pic, pci->bus);
>> +        switch (vms->iommu) {
>> +        case VIRT_IOMMU_SMMUV3:
>> +            create_smmu(vms, pic, pci->bus);
>> +            qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
>> +                                   0x0, vms->iommu_phandle, 0x0, 0x10000);
>> +            break;
>> +        case VIRT_IOMMU_VIRTIO:
>> +            create_virtio_iommu(vms, nodename, pci->bus);
>> +            qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
>> +                                   0x0, vms->iommu_phandle, 0x0, 0x8,
>> +                                   0x9, vms->iommu_phandle, 0x9, 0xfff7);
>> +            break;
>> +        default:
>> +            g_assert_not_reached();
>> +        }
>>
>> -        qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
>> -                               0x0, vms->iommu_phandle, 0x0, 0x10000);
>>      }
>>
>>      g_free(nodename);
>> --
>> 2.17.2
>
diff mbox series

Patch

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index a2b8d8f7c2..f2994c4359 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -29,6 +29,7 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "monitor/qdev.h"
 #include "qapi/error.h"
 #include "hw/sysbus.h"
 #include "hw/arm/arm.h"
@@ -49,6 +50,7 @@ 
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
 #include "hw/pci-host/gpex.h"
+#include "hw/virtio/virtio-pci.h"
 #include "hw/arm/sysbus-fdt.h"
 #include "hw/platform-bus.h"
 #include "hw/arm/fdt.h"
@@ -59,6 +61,7 @@ 
 #include "qapi/visitor.h"
 #include "standard-headers/linux/input.h"
 #include "hw/arm/smmuv3.h"
+#include "hw/virtio/virtio-iommu.h"
 
 #define DEFINE_VIRT_MACHINE_LATEST(major, minor, latest) \
     static void virt_##major##_##minor##_class_init(ObjectClass *oc, \
@@ -1085,6 +1088,33 @@  static void create_smmu(const VirtMachineState *vms, qemu_irq *pic,
     g_free(node);
 }
 
+static void create_virtio_iommu(VirtMachineState *vms,
+                                const char *pciehb_nodename, PCIBus *bus)
+{
+    const char compat[] = "virtio,pci-iommu";
+    uint16_t bdf = 0x8; /* 00:01.0 */
+    DeviceState *dev;
+    char *node;
+
+    dev = qdev_create(BUS(bus), TYPE_VIRTIO_IOMMU_PCI);
+    object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
+
+    node = g_strdup_printf("%s/virtio_iommu@%d", pciehb_nodename, bdf);
+    qemu_fdt_add_subnode(vms->fdt, node);
+    qemu_fdt_setprop(vms->fdt, node, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vms->fdt, node, "reg",
+                                 1, bdf << 8 /* phys.hi */,
+                                 1, 0        /* phys.mid */,
+                                 1, 0        /* phys.lo  */,
+                                 1, 0        /* size.hi  */,
+                                 1, 0        /* size.low */);
+
+    qemu_fdt_setprop_cell(vms->fdt, node, "#iommu-cells", 1);
+    qemu_fdt_setprop_cell(vms->fdt, node, "phandle", vms->iommu_phandle);
+    g_free(node);
+}
+
+
 static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
 {
     hwaddr base_mmio = vms->memmap[VIRT_PCIE_MMIO].base;
@@ -1205,10 +1235,22 @@  static void create_pcie(VirtMachineState *vms, qemu_irq *pic)
     if (vms->iommu) {
         vms->iommu_phandle = qemu_fdt_alloc_phandle(vms->fdt);
 
-        create_smmu(vms, pic, pci->bus);
+        switch (vms->iommu) {
+        case VIRT_IOMMU_SMMUV3:
+            create_smmu(vms, pic, pci->bus);
+            qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
+                                   0x0, vms->iommu_phandle, 0x0, 0x10000);
+            break;
+        case VIRT_IOMMU_VIRTIO:
+            create_virtio_iommu(vms, nodename, pci->bus);
+            qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
+                                   0x0, vms->iommu_phandle, 0x0, 0x8,
+                                   0x9, vms->iommu_phandle, 0x9, 0xfff7);
+            break;
+        default:
+            g_assert_not_reached();
+        }
 
-        qemu_fdt_setprop_cells(vms->fdt, nodename, "iommu-map",
-                               0x0, vms->iommu_phandle, 0x0, 0x10000);
     }
 
     g_free(nodename);