[Qemu-devel] [PATCH/RFC 0/5] R-Car Gen3 GPIO Pass-Through Prototype (QEMU)

[Qemu-devel] [PATCH/RFC 0/5] R-Car Gen3 GPIO Pass-Through Prototype (QEMU)

[Geert Uytterhoeven]

	Hi all,

This RFC patch series is the QEMU side of a GPIO Pass-Through prototype
for Renesas R-Car platforms using vfio-platform.  Together with its
counterpart for Linux, it provides direct access from a QEMU+KVM guest
to a GPIO controller in an R-Car Gen3 SoC.  This allows the guest to
control the LEDs on a Renesas Salvator-X(S) board.

This patch series is not meant to be upstreamed as-is.  Indeed, for
various reasons (e.g. security, as the different GPIOs on the same GPIO
controller may control different parts of the system) access to GPIOs is
better not implemented using Device Pass-Through, but by
paravirtualization.  Yet, this is still a simple and valuable
proof-of-concept, which can serve as a basis for the future development
of Pass-Through support for more complex platform devices on R-Car Gen3
SoCs.

  - Patches 1-2 (submitted before by Eric Auger) make the vfio-platform
    device non-abstract, incl. matching using a compatible string.
  - Patch 3 allows dynamic sysbus devices again, without needing to
    create device-specific vfio types for each and every new device.
  - Patch 4 (submitted before by Xiao Feng Ren) adds support for the
    VFIO No-IOMMU mode, which was added to Linux two years ago (in
    v4.4).
  - Patch 5 adds code to instantiate device nodes for Renesas R-Car Gen3
    GPIO controllers.

Several questions and TODOs are appended to the individual patches.

Please see https://elinux.org/R-Car/Virtualization/VFIO for full usage
instructions of this prototype.

Thanks for your comments!

Auger Eric (2):
  vfio/platform: make the vfio-platform device non abstract
  hw/arm/sysbus-fdt: Allow device matching with compat string

Geert Uytterhoeven (2):
  hw/arm/virt: Allow dynamic sysbus devices again
  hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic instatiation

Xiao Feng Ren (1):
  vfio: No-IOMMU mode support

 hw/arm/sysbus-fdt.c             | 108 +++++++++++++++++++++++++++++++++++-----
 hw/arm/virt.c                   |   1 +
 hw/vfio/common.c                |  61 ++++++++++++++++++-----
 hw/vfio/platform.c              |  20 +++++++-
 include/hw/vfio/vfio-common.h   |   2 +
 include/hw/vfio/vfio-platform.h |   2 +
 6 files changed, 166 insertions(+), 28 deletions(-)

-- 
2.7.4

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

[Qemu-devel] [PATCH/RFC 1/5] vfio/platform: make the vfio-platform device non abstract

[Geert Uytterhoeven]

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

Up to now the vfio-platform device has been abstract and could not be
instantiated. The integration of a new vfio platform device required
to create a dummy derived device which only set the compatibility
string.

Following the few vfio-platform device integration we have seen
the actual requested adaptation happens on device tree node creation
(sysbus-fdt).

So this patch removes the abstract setting and defines 2 new options,
manufacturer and model that are used to build a compatibility string.

This latter will be used to match the device tree node creation
function

sysbus-fdt does not support the instantiation of the vfio-platform
device yet.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
[geert: Rebase, Set user_creatable=true]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 hw/vfio/platform.c              | 20 ++++++++++++++++++--
 include/hw/vfio/vfio-platform.h |  2 ++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c
index 0d4bc0aae8891435..32bed1974e23c569 100644
--- a/hw/vfio/platform.c
+++ b/hw/vfio/platform.c
@@ -637,7 +637,20 @@ static void vfio_platform_realize(DeviceState *dev, Error **errp)
     VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(dev);
     SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
     VFIODevice *vbasedev = &vdev->vbasedev;
-    int i, ret;
+    int i, ret = -EINVAL;
+
+    if (!vdev->compat) {
+        if (!vdev->model) {
+            error_setg(errp, "no usable compatible string");
+            goto out;
+        }
+        if (!vdev->manufacturer) {
+            vdev->compat = g_strdup(vdev->model);
+        } else {
+            vdev->compat = g_strjoin(",", vdev->manufacturer,
+                                     vdev->model, NULL);
+        }
+    }
 
     vbasedev->type = VFIO_DEVICE_TYPE_PLATFORM;
     vbasedev->dev = dev;
@@ -681,6 +694,8 @@ static const VMStateDescription vfio_platform_vmstate = {
 static Property vfio_platform_dev_properties[] = {
     DEFINE_PROP_STRING("host", VFIOPlatformDevice, vbasedev.name),
     DEFINE_PROP_STRING("sysfsdev", VFIOPlatformDevice, vbasedev.sysfsdev),
+    DEFINE_PROP_STRING("manufacturer", VFIOPlatformDevice, manufacturer),
+    DEFINE_PROP_STRING("model", VFIOPlatformDevice, model),
     DEFINE_PROP_BOOL("x-no-mmap", VFIOPlatformDevice, vbasedev.no_mmap, false),
     DEFINE_PROP_UINT32("mmap-timeout-ms", VFIOPlatformDevice,
                        mmap_timeout, 1100),
@@ -699,6 +714,8 @@ static void vfio_platform_class_init(ObjectClass *klass, void *data)
     dc->desc = "VFIO-based platform device assignment";
     sbc->connect_irq_notifier = vfio_start_irqfd_injection;
     set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+    /* Supported by TYPE_VIRT_MACHINE */
+    dc->user_creatable = true;
 }
 
 static const TypeInfo vfio_platform_dev_info = {
@@ -707,7 +724,6 @@ static const TypeInfo vfio_platform_dev_info = {
     .instance_size = sizeof(VFIOPlatformDevice),
     .class_init = vfio_platform_class_init,
     .class_size = sizeof(VFIOPlatformDeviceClass),
-    .abstract   = true,
 };
 
 static void register_vfio_platform_dev_type(void)
diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platform.h
index 9baaa2db09b84f3e..31b9a9800f7d9d00 100644
--- a/include/hw/vfio/vfio-platform.h
+++ b/include/hw/vfio/vfio-platform.h
@@ -55,6 +55,8 @@ typedef struct VFIOPlatformDevice {
     /* queue of pending IRQs */
     QSIMPLEQ_HEAD(pending_intp_queue, VFIOINTp) pending_intp_queue;
     char *compat; /* compatibility string */
+    char *manufacturer; /* manufacturer (1st part of the compatible property) */
+    char *model; /* model (2d part of the compatible property) */
     uint32_t mmap_timeout; /* delay to re-enable mmaps after interrupt */
     QEMUTimer *mmap_timer; /* allows fast-path resume after IRQ hit */
     QemuMutex intp_mutex; /* protect the intp_list IRQ state */
-- 
2.7.4

[Qemu-devel] [PATCH/RFC 2/5] hw/arm/sysbus-fdt: Allow device matching with compat string

[Geert Uytterhoeven]

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

Up to now we have relied on the device type to identify a device tree
node creation function. Since we would like the VFIO-PLATFORM
device to be instantiable with different compatibility strings
we introduce the capability to specialize the node creation depending
on a manufacturer/model combo.

NodeCreationPair is renamed into BindingEntry. The struct is enhanced
with manufacturer, model and match_fn() fields. We introduce a new
matching function adapted to vfio-platform generic device.

>From now on, the AMD XGBE can be instantiated with either manner, ie:
-device vfio-amd-xgbe,host=e0900000.xgmac
or new option line:
-device vfio-platform,host=e0900000.xgmac,manufacturer=amd,model=xgbe-seattle-v1a

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 hw/arm/sysbus-fdt.c | 61 +++++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 48 insertions(+), 13 deletions(-)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index d68e3dcdbd260895..c5d4fd5604c28118 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -58,11 +58,14 @@ typedef struct PlatformBusFDTNotifierParams {
     ARMPlatformBusFDTParams *fdt_params;
 } PlatformBusFDTNotifierParams;
 
-/* struct that associates a device type name and a node creation function */
-typedef struct NodeCreationPair {
+/* struct that allows to match a device and create its FDT node */
+typedef struct BindingEntry {
     const char *typename;
-    int (*add_fdt_node_fn)(SysBusDevice *sbdev, void *opaque);
-} NodeCreationPair;
+    const char *manufacturer;
+    const char *model;
+    int  (*add_fn)(SysBusDevice *sbdev, void *opaque);
+    bool (*match_fn)(SysBusDevice *sbdev, const struct BindingEntry *combo);
+} BindingEntry;
 
 /* helpers */
 
@@ -413,15 +416,46 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void *opaque)
     return 0;
 }
 
+/* manufacturer/model matching */
+static bool vfio_platform_match(SysBusDevice *sbdev,
+                                const BindingEntry *entry)
+{
+    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+
+    if (strcmp(entry->model, vdev->model)) {
+        return false;
+    }
+
+    if (entry->manufacturer) {
+        if (!vdev->manufacturer) {
+            return false;
+        }
+        return !strcmp(entry->manufacturer, vdev->manufacturer);
+    }
+    return true;
+}
+
+#define VFIO_PLATFORM_BINDING(manuf, model, add_fn) \
+    {TYPE_VFIO_PLATFORM, (manuf), (model), (add_fn), vfio_platform_match}
+
 #endif /* CONFIG_LINUX */
 
-/* list of supported dynamic sysbus devices */
-static const NodeCreationPair add_fdt_node_functions[] = {
+/* Device type based matching */
+static bool type_match(SysBusDevice *sbdev, const BindingEntry *entry)
+{
+    return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
+}
+
+#define TYPE_BINDING(type, add_fn) {(type), NULL, NULL, (add_fn), type_match}
+
+/* list of supported dynamic sysbus bindings */
+static const BindingEntry bindings[] = {
 #ifdef CONFIG_LINUX
-    {TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node},
-    {TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node},
+    TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
+    TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
+    VFIO_PLATFORM_BINDING("amd", "xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
 #endif
-    {"", NULL}, /* last element */
+    TYPE_BINDING("", NULL), /* last element */
 };
 
 /* Generic Code */
@@ -440,10 +474,11 @@ static void add_fdt_node(SysBusDevice *sbdev, void *opaque)
 {
     int i, ret;
 
-    for (i = 0; i < ARRAY_SIZE(add_fdt_node_functions); i++) {
-        if (!strcmp(object_get_typename(OBJECT(sbdev)),
-                    add_fdt_node_functions[i].typename)) {
-            ret = add_fdt_node_functions[i].add_fdt_node_fn(sbdev, opaque);
+    for (i = 0; i < ARRAY_SIZE(bindings); i++) {
+        const BindingEntry *iter = &bindings[i];
+
+        if (iter->match_fn(sbdev, iter)) {
+            ret = iter->add_fn(sbdev, opaque);
             assert(!ret);
             return;
         }
-- 
2.7.4

[Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Geert Uytterhoeven]

Allow the instantation of generic dynamic sysbus devices again, without
the need to create a new device-specific vfio type.

This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
Allow only supported dynamic sysbus devices").

Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 hw/arm/virt.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index b334c82edae9fb1f..fa83784fc08ed076 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
     mc->max_cpus = 255;
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
     machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
+    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
     mc->block_default_type = IF_VIRTIO;
     mc->no_cdrom = 1;
     mc->pci_allow_0_address = true;
-- 
2.7.4

[Qemu-devel] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Geert Uytterhoeven]

From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>

Add qemu support for the newly introduced VFIO No-IOMMU driver.

We need to add special handling for:
- Group character device is /dev/vfio/noiommu-$GROUP.
- No-IOMMU does not rely on a memory listener.
- No IOMMU will be set for its group, so no need to call
  vfio_kvm_device_add_group.

Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
[geert: Rebase]
Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
 hw/vfio/common.c              | 61 ++++++++++++++++++++++++++++++++++---------
 include/hw/vfio/vfio-common.h |  2 ++
 2 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index f895e3c3359af779..2ee94a3304202a74 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1019,6 +1019,33 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     container->fd = fd;
     QLIST_INIT(&container->giommu_list);
     QLIST_INIT(&container->hostwin_list);
+    container->noiommu = group->noiommu;
+
+    if (container->noiommu) {
+        ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
+        if (ret) {
+            error_report("vfio: failed to set group container: %m");
+            ret = -errno;
+            goto free_container_exit;
+        }
+
+        ret = ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_NOIOMMU_IOMMU);
+        if (!ret) {
+            error_report("vfio: No available IOMMU models");
+            ret = -EINVAL;
+            goto free_container_exit;
+        }
+
+        ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_NOIOMMU_IOMMU);
+        if (ret) {
+            error_report("vfio: failed to set iommu for container: %m");
+            ret = -errno;
+            goto free_container_exit;
+        }
+
+        goto listener_register;
+    }
+
     if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
         ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
         bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU);
@@ -1151,15 +1178,16 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     group->container = container;
     QLIST_INSERT_HEAD(&container->group_list, group, container_next);
 
-    container->listener = vfio_memory_listener;
-
-    memory_listener_register(&container->listener, container->space->as);
-
-    if (container->error) {
-        ret = container->error;
-        error_setg_errno(errp, -ret,
-                         "memory listener initialization failed for container");
-        goto listener_release_exit;
+listener_register:
+    if (!container->noiommu) {
+        container->listener = vfio_memory_listener;
+        memory_listener_register(&container->listener, container->space->as);
+        if (container->error) {
+            ret = container->error;
+            error_setg_errno(errp, -ret,
+                    "memory listener initialization failed for container");
+            goto listener_release_exit;
+        }
     }
 
     container->initialized = true;
@@ -1169,7 +1197,9 @@ listener_release_exit:
     QLIST_REMOVE(group, container_next);
     QLIST_REMOVE(container, next);
     vfio_kvm_device_del_group(group);
-    vfio_listener_release(container);
+    if (!container->noiommu) {
+        vfio_listener_release(container);
+    }
 
 free_container_exit:
     g_free(container);
@@ -1195,7 +1225,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
      * since unset may destroy the backend container if it's the last
      * group.
      */
-    if (QLIST_EMPTY(&container->group_list)) {
+    if (QLIST_EMPTY(&container->group_list) && !container->noiommu) {
         vfio_listener_release(container);
     }
 
@@ -1249,8 +1279,13 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
     snprintf(path, sizeof(path), "/dev/vfio/%d", groupid);
     group->fd = qemu_open(path, O_RDWR);
     if (group->fd < 0) {
-        error_setg_errno(errp, errno, "failed to open %s", path);
-        goto free_group_exit;
+        snprintf(path, sizeof(path), "/dev/vfio/noiommu-%d", groupid);
+        group->fd = qemu_open(path, O_RDWR);
+        if (group->fd < 0) {
+            error_setg_errno(errp, errno, "failed to open %s", path);
+            goto free_group_exit;
+        }
+        group->noiommu = 1;
     }
 
     if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) {
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index f3a2ac9fee02066f..aca2e33efb9b118c 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -77,6 +77,7 @@ struct VFIOGroup;
 typedef struct VFIOContainer {
     VFIOAddressSpace *space;
     int fd; /* /dev/vfio/vfio, empowered by the attached groups */
+    bool noiommu;
     MemoryListener listener;
     MemoryListener prereg_listener;
     unsigned iommu_type;
@@ -136,6 +137,7 @@ struct VFIODeviceOps {
 typedef struct VFIOGroup {
     int fd;
     int groupid;
+    bool noiommu;
     VFIOContainer *container;
     QLIST_HEAD(, VFIODevice) device_list;
     QLIST_ENTRY(VFIOGroup) next;
-- 
2.7.4

[Qemu-devel] [PATCH/RFC 5/5] hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic instantiation

[Geert Uytterhoeven]

Allow the instantiation of a Renesas R-Car Gen3 GPIO controller device
from the QEMU command line:

-device vfio-platform,host=<device>,manufacturer=renesas,model=rcar-gen3-gpio
-device vfio-platform,sysfsdev=<path>,manufacturer=renesas,model=rcar-gen3-gpio

A specialized device tree node is created for the guest, containing
compatible, reg, gpio-controller, and #gpio-cells properties.

Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Question:
  - Why do we need the manufacturer=foo,model=bar syntax? Can't this
    just be extracted from the host DT?

TODO:
  - Copy properties from the host DT, as add_amd_xgbe_fdt_node() does,
  - Make this more generic?
---
 hw/arm/sysbus-fdt.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index c5d4fd5604c28118..428175f343d9f3b9 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -416,6 +416,52 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void *opaque)
     return 0;
 }
 
+/**
+ * add_rcar_gpio_fdt_node
+ *
+ * Generates a simple node with following properties:
+ * compatible string, regs, #gpio-cells, gpio-controller
+ */
+static int add_rcar_gpio_fdt_node(SysBusDevice *sbdev, void *opaque)
+{
+    PlatformBusFDTData *data = opaque;
+    PlatformBusDevice *pbus = data->pbus;
+    void *fdt = data->fdt;
+    const char *parent_node = data->pbus_node_name;
+    int compat_str_len, i;
+    char *nodename;
+    uint32_t *reg_attr;
+    uint64_t mmio_base;
+    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
+    VFIODevice *vbasedev = &vdev->vbasedev;
+
+    mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
+    nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
+                               vbasedev->name, mmio_base);
+    qemu_fdt_add_subnode(fdt, nodename);
+
+    compat_str_len = strlen(vdev->compat) + 1;
+    qemu_fdt_setprop(fdt, nodename, "compatible",
+                          vdev->compat, compat_str_len);
+
+    qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
+    qemu_fdt_setprop_cells(fdt, nodename, "#gpio-cells", 2);
+
+    reg_attr = g_new(uint32_t, vbasedev->num_regions * 2);
+    for (i = 0; i < vbasedev->num_regions; i++) {
+        mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
+        reg_attr[2 * i] = cpu_to_be32(mmio_base);
+        reg_attr[2 * i + 1] = cpu_to_be32(
+                                memory_region_size(vdev->regions[i]->mem));
+    }
+    qemu_fdt_setprop(fdt, nodename, "reg", reg_attr,
+                     vbasedev->num_regions * 2 * sizeof(uint32_t));
+
+    g_free(reg_attr);
+    g_free(nodename);
+    return 0;
+}
+
 /* manufacturer/model matching */
 static bool vfio_platform_match(SysBusDevice *sbdev,
                                 const BindingEntry *entry)
@@ -454,6 +500,7 @@ static const BindingEntry bindings[] = {
     TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
     TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
     VFIO_PLATFORM_BINDING("amd", "xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
+    VFIO_PLATFORM_BINDING("renesas", "rcar-gen3-gpio", add_rcar_gpio_fdt_node),
 #endif
     TYPE_BINDING("", NULL), /* last element */
 };
-- 
2.7.4

Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Peter Maydell]

On 9 February 2018 at 15:17, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
> Allow the instantation of generic dynamic sysbus devices again, without
> the need to create a new device-specific vfio type.
>
> This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
> Allow only supported dynamic sysbus devices").
>
> Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b334c82edae9fb1f..fa83784fc08ed076 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->max_cpus = 255;
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
>      mc->block_default_type = IF_VIRTIO;
>      mc->no_cdrom = 1;
>      mc->pci_allow_0_address = true;

This needs a lot of justification. Dynamic sysbus is not supposed
to be for plugging any random sysbus device in, it's for vfio,
which needs special casing anyway to work right. (Overall it's
a terrible hack -- in an ideal world all vfio would use pci
or another probeable bus.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Geert Uytterhoeven]

Hi Peter,

On Fri, Feb 9, 2018 at 4:27 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 9 February 2018 at 15:17, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>> Allow the instantation of generic dynamic sysbus devices again, without
>> the need to create a new device-specific vfio type.
>>
>> This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
>> Allow only supported dynamic sysbus devices").
>>
>> Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  hw/arm/virt.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b334c82edae9fb1f..fa83784fc08ed076 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>      mc->max_cpus = 255;
>>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
>> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
>>      mc->block_default_type = IF_VIRTIO;
>>      mc->no_cdrom = 1;
>>      mc->pci_allow_0_address = true;
>
> This needs a lot of justification. Dynamic sysbus is not supposed
> to be for plugging any random sysbus device in, it's for vfio,
> which needs special casing anyway to work right. (Overall it's

Sure. Is there a way to limit this to vfio devices?

> a terrible hack -- in an ideal world all vfio would use pci
> or another probeable bus.)

What about vfio-platform, which is my use case?
On DT-based systems, platform devices are described very well in DT (even
better than what's provided by probing PCI IDs and BARs ;-)

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Peter Maydell]

On 9 February 2018 at 15:37, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Peter,
>
> On Fri, Feb 9, 2018 at 4:27 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 9 February 2018 at 15:17, Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>>> Allow the instantation of generic dynamic sysbus devices again, without
>>> the need to create a new device-specific vfio type.
>>>
>>> This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
>>> Allow only supported dynamic sysbus devices").
>>>
>>> Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>>  hw/arm/virt.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index b334c82edae9fb1f..fa83784fc08ed076 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>>      mc->max_cpus = 255;
>>>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>>>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
>>> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
>>>      mc->block_default_type = IF_VIRTIO;
>>>      mc->no_cdrom = 1;
>>>      mc->pci_allow_0_address = true;
>>
>> This needs a lot of justification. Dynamic sysbus is not supposed
>> to be for plugging any random sysbus device in, it's for vfio,
>> which needs special casing anyway to work right. (Overall it's
>
> Sure. Is there a way to limit this to vfio devices?

That would be the code here which implements the whitelist of
"only allow this for the vfio devices which we have
support for mangling the device tree for and know will work".

>> a terrible hack -- in an ideal world all vfio would use pci
>> or another probeable bus.)
>
> What about vfio-platform, which is my use case?
> On DT-based systems, platform devices are described very well in DT (even
> better than what's provided by probing PCI IDs and BARs ;-)

The TYPE_VFIO_* devices in the whitelist all use DT, yes:
the code to handle creating/mangling the dt nodes for the
passed-through device is in hw/arm/sysbus-fdt.c.

thanks
-- PMM

Re: [Qemu-devel] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Alex Williamson]

On Fri,  9 Feb 2018 16:17:35 +0100
Geert Uytterhoeven <geert+renesas@glider.be> wrote:

> From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
> 
> Add qemu support for the newly introduced VFIO No-IOMMU driver.
> 
> We need to add special handling for:
> - Group character device is /dev/vfio/noiommu-$GROUP.
> - No-IOMMU does not rely on a memory listener.
> - No IOMMU will be set for its group, so no need to call
>   vfio_kvm_device_add_group.
> 
> Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
> [geert: Rebase]
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  hw/vfio/common.c              | 61 ++++++++++++++++++++++++++++++++++---------
>  include/hw/vfio/vfio-common.h |  2 ++
>  2 files changed, 50 insertions(+), 13 deletions(-)

NAK.  I'm opposed to no-iommu support in QEMU in general, but accepting
vfio devices with no-iommu (which provide no DMA translation!!!)
transparently as if they might actually work like a regular vfio device
is absolutely unacceptable.  Without DMA translation and isolation, you
might want to think about another interface, I'm not keen on the idea
of corrupting vfio support in order to blink some LEDs.  Thanks,

Alex

> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index f895e3c3359af779..2ee94a3304202a74 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1019,6 +1019,33 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>      container->fd = fd;
>      QLIST_INIT(&container->giommu_list);
>      QLIST_INIT(&container->hostwin_list);
> +    container->noiommu = group->noiommu;
> +
> +    if (container->noiommu) {
> +        ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &fd);
> +        if (ret) {
> +            error_report("vfio: failed to set group container: %m");
> +            ret = -errno;
> +            goto free_container_exit;
> +        }
> +
> +        ret = ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_NOIOMMU_IOMMU);
> +        if (!ret) {
> +            error_report("vfio: No available IOMMU models");
> +            ret = -EINVAL;
> +            goto free_container_exit;
> +        }
> +
> +        ret = ioctl(fd, VFIO_SET_IOMMU, VFIO_NOIOMMU_IOMMU);
> +        if (ret) {
> +            error_report("vfio: failed to set iommu for container: %m");
> +            ret = -errno;
> +            goto free_container_exit;
> +        }
> +
> +        goto listener_register;
> +    }
> +
>      if (ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU) ||
>          ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU)) {
>          bool v2 = !!ioctl(fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1v2_IOMMU);
> @@ -1151,15 +1178,16 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>      group->container = container;
>      QLIST_INSERT_HEAD(&container->group_list, group, container_next);
>  
> -    container->listener = vfio_memory_listener;
> -
> -    memory_listener_register(&container->listener, container->space->as);
> -
> -    if (container->error) {
> -        ret = container->error;
> -        error_setg_errno(errp, -ret,
> -                         "memory listener initialization failed for container");
> -        goto listener_release_exit;
> +listener_register:
> +    if (!container->noiommu) {
> +        container->listener = vfio_memory_listener;
> +        memory_listener_register(&container->listener, container->space->as);
> +        if (container->error) {
> +            ret = container->error;
> +            error_setg_errno(errp, -ret,
> +                    "memory listener initialization failed for container");
> +            goto listener_release_exit;
> +        }
>      }
>  
>      container->initialized = true;
> @@ -1169,7 +1197,9 @@ listener_release_exit:
>      QLIST_REMOVE(group, container_next);
>      QLIST_REMOVE(container, next);
>      vfio_kvm_device_del_group(group);
> -    vfio_listener_release(container);
> +    if (!container->noiommu) {
> +        vfio_listener_release(container);
> +    }
>  
>  free_container_exit:
>      g_free(container);
> @@ -1195,7 +1225,7 @@ static void vfio_disconnect_container(VFIOGroup *group)
>       * since unset may destroy the backend container if it's the last
>       * group.
>       */
> -    if (QLIST_EMPTY(&container->group_list)) {
> +    if (QLIST_EMPTY(&container->group_list) && !container->noiommu) {
>          vfio_listener_release(container);
>      }
>  
> @@ -1249,8 +1279,13 @@ VFIOGroup *vfio_get_group(int groupid, AddressSpace *as, Error **errp)
>      snprintf(path, sizeof(path), "/dev/vfio/%d", groupid);
>      group->fd = qemu_open(path, O_RDWR);
>      if (group->fd < 0) {
> -        error_setg_errno(errp, errno, "failed to open %s", path);
> -        goto free_group_exit;
> +        snprintf(path, sizeof(path), "/dev/vfio/noiommu-%d", groupid);
> +        group->fd = qemu_open(path, O_RDWR);
> +        if (group->fd < 0) {
> +            error_setg_errno(errp, errno, "failed to open %s", path);
> +            goto free_group_exit;
> +        }
> +        group->noiommu = 1;
>      }
>  
>      if (ioctl(group->fd, VFIO_GROUP_GET_STATUS, &status)) {
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index f3a2ac9fee02066f..aca2e33efb9b118c 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -77,6 +77,7 @@ struct VFIOGroup;
>  typedef struct VFIOContainer {
>      VFIOAddressSpace *space;
>      int fd; /* /dev/vfio/vfio, empowered by the attached groups */
> +    bool noiommu;
>      MemoryListener listener;
>      MemoryListener prereg_listener;
>      unsigned iommu_type;
> @@ -136,6 +137,7 @@ struct VFIODeviceOps {
>  typedef struct VFIOGroup {
>      int fd;
>      int groupid;
> +    bool noiommu;
>      VFIOContainer *container;
>      QLIST_HEAD(, VFIODevice) device_list;
>      QLIST_ENTRY(VFIOGroup) next;

Re: [Qemu-devel] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Geert Uytterhoeven]

Hi Alex,

On Fri, Feb 9, 2018 at 4:50 PM, Alex Williamson
<alex.williamson@redhat.com> wrote:
> On Fri,  9 Feb 2018 16:17:35 +0100
> Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>> From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
>>
>> Add qemu support for the newly introduced VFIO No-IOMMU driver.
>>
>> We need to add special handling for:
>> - Group character device is /dev/vfio/noiommu-$GROUP.
>> - No-IOMMU does not rely on a memory listener.
>> - No IOMMU will be set for its group, so no need to call
>>   vfio_kvm_device_add_group.
>>
>> Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
>> [geert: Rebase]
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  hw/vfio/common.c              | 61 ++++++++++++++++++++++++++++++++++---------
>>  include/hw/vfio/vfio-common.h |  2 ++
>>  2 files changed, 50 insertions(+), 13 deletions(-)
>
> NAK.  I'm opposed to no-iommu support in QEMU in general, but accepting
> vfio devices with no-iommu (which provide no DMA translation!!!)
> transparently as if they might actually work like a regular vfio device
> is absolutely unacceptable.  Without DMA translation and isolation, you
> might want to think about another interface, I'm not keen on the idea

What if the device cannot do DMA?

There are other devices that cannot do DMA, but that can be useful to
access from a guest in an embedded system.
E.g. smart ADC monitor blocks that monitor and average several ADCs in an
automotive environment (drivers/iio/adc/rcar-gyroadc.c).

Should all such devices be paravirtualized?

> of corrupting vfio support in order to blink some LEDs.  Thanks,

This GPIO+LED prototype is just a proof-of-concept. There's no plan to
upstream it.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [Qemu-devel] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Alex Williamson]

On Fri, 9 Feb 2018 17:06:34 +0100
Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Hi Alex,
> 
> On Fri, Feb 9, 2018 at 4:50 PM, Alex Williamson
> <alex.williamson@redhat.com> wrote:
> > On Fri,  9 Feb 2018 16:17:35 +0100
> > Geert Uytterhoeven <geert+renesas@glider.be> wrote:  
> >> From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
> >>
> >> Add qemu support for the newly introduced VFIO No-IOMMU driver.
> >>
> >> We need to add special handling for:
> >> - Group character device is /dev/vfio/noiommu-$GROUP.
> >> - No-IOMMU does not rely on a memory listener.
> >> - No IOMMU will be set for its group, so no need to call
> >>   vfio_kvm_device_add_group.
> >>
> >> Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
> >> [geert: Rebase]
> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> >> ---
> >>  hw/vfio/common.c              | 61 ++++++++++++++++++++++++++++++++++---------
> >>  include/hw/vfio/vfio-common.h |  2 ++
> >>  2 files changed, 50 insertions(+), 13 deletions(-)  
> >
> > NAK.  I'm opposed to no-iommu support in QEMU in general, but accepting
> > vfio devices with no-iommu (which provide no DMA translation!!!)
> > transparently as if they might actually work like a regular vfio device
> > is absolutely unacceptable.  Without DMA translation and isolation, you
> > might want to think about another interface, I'm not keen on the idea  
> 
> What if the device cannot do DMA?
> 
> There are other devices that cannot do DMA, but that can be useful to
> access from a guest in an embedded system.
> E.g. smart ADC monitor blocks that monitor and average several ADCs in an
> automotive environment (drivers/iio/adc/rcar-gyroadc.c).
> 
> Should all such devices be paravirtualized?

How do we know that a device is not capable of DMA?  The moment we add
no-iommu support generically to QEMU, I get to deal with a swarm of
people trying to use it to assign DMA capable PCI devices to VMs and
failing miserably.  We added no-iommu support because the UIO PCI driver
was under pressure to add support for MSI/X interrupts and we figured
we could at least taint the kernel an give people a path to a more
secure setup with an IOMMU by allowing use of the vfio device
interface.  For PCI, this makes some sense because PCI has architected
interrupts and config space and standard layouts.  What's the value of
vfio over UIO for ad-hoc, non-DMA platform devices?  UIO is intended for
non-DMA devices.  vfio is intended for IOMMU protected, DMA capable
devices.  vfio no-IOMMU is a band-aide that hopefully goes away
eventually.

> > of corrupting vfio support in order to blink some LEDs.  Thanks,  
> 
> This GPIO+LED prototype is just a proof-of-concept. There's no plan to
> upstream it.

If vfio with no-IOMMU is to be used, it must not enable users to
casually assign DMA capable devices.  Certainly any PCI/e device needs
to be assumed to be DMA capable.  I don't know how you tell with
platform devices since there's no standard interface.  Thanks,

Alex

Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Auger Eric]

Hi Geert,
On 09/02/18 16:17, Geert Uytterhoeven wrote:
> Allow the instantation of generic dynamic sysbus devices again, without
> the need to create a new device-specific vfio type.
> 
> This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
> Allow only supported dynamic sysbus devices").
> 
> Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  hw/arm/virt.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b334c82edae9fb1f..fa83784fc08ed076 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>      mc->max_cpus = 255;
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
Couldn't you limit that to TYPE_VFIO_PLATFORM instead?

Thanks

Eric
>      mc->block_default_type = IF_VIRTIO;
>      mc->no_cdrom = 1;
>      mc->pci_allow_0_address = true;
> 

Re: [Qemu-devel] [PATCH/RFC 5/5] hw/arm/sysbus-fdt: Enable rcar-gen3-gpio dynamic instantiation

[Auger Eric]

Hi Geert,

On 09/02/18 16:17, Geert Uytterhoeven wrote:
> Allow the instantiation of a Renesas R-Car Gen3 GPIO controller device
> from the QEMU command line:
> 
> -device vfio-platform,host=<device>,manufacturer=renesas,model=rcar-gen3-gpio
> -device vfio-platform,sysfsdev=<path>,manufacturer=renesas,model=rcar-gen3-gpio
> 
> A specialized device tree node is created for the guest, containing
> compatible, reg, gpio-controller, and #gpio-cells properties.
> 
> Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Question:
>   - Why do we need the manufacturer=foo,model=bar syntax? Can't this
>     just be extracted from the host DT?
I think this could be achieved that way too. We just need to pay
attention to the fact the dt node creation function matches the exact
same compatible property value.

> 
> TODO:
>   - Copy properties from the host DT, as add_amd_xgbe_fdt_node() does,
>   - Make this more generic?

Yes I think devising helpers to generate regs/interrupts properties
could help reducing the amount of code to be written

Thanks

Eric
> ---
>  hw/arm/sysbus-fdt.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index c5d4fd5604c28118..428175f343d9f3b9 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -416,6 +416,52 @@ static int add_amd_xgbe_fdt_node(SysBusDevice *sbdev, void *opaque)
>      return 0;
>  }
>  
> +/**
> + * add_rcar_gpio_fdt_node
> + *
> + * Generates a simple node with following properties:
> + * compatible string, regs, #gpio-cells, gpio-controller
> + */
> +static int add_rcar_gpio_fdt_node(SysBusDevice *sbdev, void *opaque)
> +{
> +    PlatformBusFDTData *data = opaque;
> +    PlatformBusDevice *pbus = data->pbus;
> +    void *fdt = data->fdt;
> +    const char *parent_node = data->pbus_node_name;
> +    int compat_str_len, i;
> +    char *nodename;
> +    uint32_t *reg_attr;
> +    uint64_t mmio_base;
> +    VFIOPlatformDevice *vdev = VFIO_PLATFORM_DEVICE(sbdev);
> +    VFIODevice *vbasedev = &vdev->vbasedev;
> +
> +    mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, 0);
> +    nodename = g_strdup_printf("%s/%s@%" PRIx64, parent_node,
> +                               vbasedev->name, mmio_base);
> +    qemu_fdt_add_subnode(fdt, nodename);
> +
> +    compat_str_len = strlen(vdev->compat) + 1;
> +    qemu_fdt_setprop(fdt, nodename, "compatible",
> +                          vdev->compat, compat_str_len);
> +
> +    qemu_fdt_setprop(fdt, nodename, "gpio-controller", NULL, 0);
> +    qemu_fdt_setprop_cells(fdt, nodename, "#gpio-cells", 2);
> +
> +    reg_attr = g_new(uint32_t, vbasedev->num_regions * 2);
> +    for (i = 0; i < vbasedev->num_regions; i++) {
> +        mmio_base = platform_bus_get_mmio_addr(pbus, sbdev, i);
> +        reg_attr[2 * i] = cpu_to_be32(mmio_base);
> +        reg_attr[2 * i + 1] = cpu_to_be32(
> +                                memory_region_size(vdev->regions[i]->mem));
> +    }
> +    qemu_fdt_setprop(fdt, nodename, "reg", reg_attr,
> +                     vbasedev->num_regions * 2 * sizeof(uint32_t));
> +
> +    g_free(reg_attr);
> +    g_free(nodename);
> +    return 0;
> +}
> +
>  /* manufacturer/model matching */
>  static bool vfio_platform_match(SysBusDevice *sbdev,
>                                  const BindingEntry *entry)
> @@ -454,6 +500,7 @@ static const BindingEntry bindings[] = {
>      TYPE_BINDING(TYPE_VFIO_CALXEDA_XGMAC, add_calxeda_midway_xgmac_fdt_node),
>      TYPE_BINDING(TYPE_VFIO_AMD_XGBE, add_amd_xgbe_fdt_node),
>      VFIO_PLATFORM_BINDING("amd", "xgbe-seattle-v1a", add_amd_xgbe_fdt_node),
> +    VFIO_PLATFORM_BINDING("renesas", "rcar-gen3-gpio", add_rcar_gpio_fdt_node),
>  #endif
>      TYPE_BINDING("", NULL), /* last element */
>  };
> 

Re: [Qemu-devel] [PATCH/RFC 3/5] hw/arm/virt: Allow dynamic sysbus devices again

[Geert Uytterhoeven]

Hi Eric,

On Wed, Feb 14, 2018 at 11:37 AM, Auger Eric <eric.auger@redhat.com> wrote:
> On 09/02/18 16:17, Geert Uytterhoeven wrote:
>> Allow the instantation of generic dynamic sysbus devices again, without
>> the need to create a new device-specific vfio type.
>>
>> This is a partial revert of commit  6f2062b9758ebc64 ("hw/arm/virt:
>> Allow only supported dynamic sysbus devices").
>>
>> Not-Yet-Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  hw/arm/virt.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index b334c82edae9fb1f..fa83784fc08ed076 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -1596,6 +1596,7 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>      mc->max_cpus = 255;
>>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_CALXEDA_XGMAC);
>>      machine_class_allow_dynamic_sysbus_dev(mc, TYPE_VFIO_AMD_XGBE);
>> +    machine_class_allow_dynamic_sysbus_dev(mc, TYPE_SYS_BUS_DEVICE);
> Couldn't you limit that to TYPE_VFIO_PLATFORM instead?

Yep, that works. Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [Qemu-arm] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Landon Clipp]

On Fri Feb 9, 2018 at 9:50 AM CST, Alex Williamson wrote:
> On Fri,  9 Feb 2018 16:17:35 +0100
> Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>
>> From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
>> 
>> Add qemu support for the newly introduced VFIO No-IOMMU driver.
>> 
>> We need to add special handling for:
>> - Group character device is /dev/vfio/noiommu-$GROUP.
>> - No-IOMMU does not rely on a memory listener.
>> - No IOMMU will be set for its group, so no need to call
>>   vfio_kvm_device_add_group.
>> 
>> Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
>> [geert: Rebase]
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>> ---
>>  hw/vfio/common.c              | 61 ++++++++++++++++++++++++++++++++++---------
>>  include/hw/vfio/vfio-common.h |  2 ++
>>  2 files changed, 50 insertions(+), 13 deletions(-)
>
> NAK.  I'm opposed to no-iommu support in QEMU in general, but accepting
> vfio devices with no-iommu (which provide no DMA translation!!!)
> transparently as if they might actually work like a regular vfio device
> is absolutely unacceptable.  Without DMA translation and isolation, you
> might want to think about another interface, I'm not keen on the idea
> of corrupting vfio support in order to blink some LEDs.  Thanks,
>
> Alex
>

8 years later, I was wondering if we could revisit this. I found myself in a
need to use enable_unsafe_noiommu_mode due to some systems I'm using not having
a usable IOMMU. QEMU is being used as part of a single-tenant workload
sandboxing environment, and given the specific circumstances, both the users and
the administrators understand that enabling the noiommu mode is unsafe and
cannot be used where security matters. Regardless, it's still something we want.

As the administrator of my system, it's under my control whether VFIO is put
into this mode, and if it is, I would expect the remaning software components
to respect that decision. As is, the system is unusable because of a lack of
support in QEMU.

I'm happy to elucidate more of my use-case if it is helpful, but I first wanted to
gauge your willingness to reconsider this under a different context.

Regards,
Landon (CoreWeave)

Re: [Qemu-arm] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Landon Clipp]

I don't know whether the threading will work 8 years in the past, this is the
link to the original conversation:
https://lists.gnu.org/archive/html/qemu-arm/2018-02/msg00226.html.

Landon

Re: [Qemu-arm] [PATCH/RFC 4/5] vfio: No-IOMMU mode support

[Alex Williamson]

On Fri, Mar 20, 2026, at 5:38 PM, Landon Clipp wrote:
> On Fri Feb 9, 2018 at 9:50 AM CST, Alex Williamson wrote:
>> On Fri,  9 Feb 2018 16:17:35 +0100
>> Geert Uytterhoeven <geert+renesas@glider.be> wrote:
>>
>>> From: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
>>> 
>>> Add qemu support for the newly introduced VFIO No-IOMMU driver.
>>> 
>>> We need to add special handling for:
>>> - Group character device is /dev/vfio/noiommu-$GROUP.
>>> - No-IOMMU does not rely on a memory listener.
>>> - No IOMMU will be set for its group, so no need to call
>>>   vfio_kvm_device_add_group.
>>> 
>>> Signed-off-by: Xiao Feng Ren <renxiaof@linux.vnet.ibm.com>
>>> [geert: Rebase]
>>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>> ---
>>>  hw/vfio/common.c              | 61 ++++++++++++++++++++++++++++++++++---------
>>>  include/hw/vfio/vfio-common.h |  2 ++
>>>  2 files changed, 50 insertions(+), 13 deletions(-)
>>
>> NAK.  I'm opposed to no-iommu support in QEMU in general, but accepting
>> vfio devices with no-iommu (which provide no DMA translation!!!)
>> transparently as if they might actually work like a regular vfio device
>> is absolutely unacceptable.  Without DMA translation and isolation, you
>> might want to think about another interface, I'm not keen on the idea
>> of corrupting vfio support in order to blink some LEDs.  Thanks,
>>
>> Alex
>>
>
> 8 years later, I was wondering if we could revisit this. I found myself in a
> need to use enable_unsafe_noiommu_mode due to some systems I'm using not having
> a usable IOMMU. QEMU is being used as part of a single-tenant workload
> sandboxing environment, and given the specific circumstances, both the users and
> the administrators understand that enabling the noiommu mode is unsafe and
> cannot be used where security matters. Regardless, it's still something we want.
>
> As the administrator of my system, it's under my control whether VFIO is put
> into this mode, and if it is, I would expect the remaning software components
> to respect that decision. As is, the system is unusable because of a lack of
> support in QEMU.
>
> I'm happy to elucidate more of my use-case if it is helpful, but I 
> first wanted to
> gauge your willingness to reconsider this under a different context.

I'm not aware of anything that's improved the chances of accepting something like this.

In fact, a vfio-platform motivated feature is arguably on worse footing now.

More importantly, QEMU now has support for vfio-user, so I believe you can already do this.  The model would be a vfio-noiommu userspace driver that exposes a vfio-user interface for QEMU.

All of the previous issues with security and virtualization still remain with implementing this support directly in QEMU.  Providing a DMA capable no-iommu device to a QEMU sandbox negates any isolation it might otherwise provide.  With the vfio-user no-iommu driver model, you take on the security burden to use only non-DMA capable devices, or mediate the access, rather than asking the QEMU community to assume that risk.  Thanks,

Alex