[Qemu-devel] [RFC 0/2] platform device passthrough

[Qemu-devel] [RFC 0/2] platform device passthrough

[Kim Phillips]

This is an RFC for the first step of many to enable platform device
passthrough, where a guest OS is able to directly access platform
device MMIO, DMA regions, and receive interrupts from platform devices.

The first RFC patch works around the limitation of QEMU not being able
to create platform devices from the command line, and should only
be used for testing the second patch.  A discussion of how to address
the limitation is encouraged, however, in the context of the first patch.

Whilst the second RFC patch achieves MMIO direct access to the device,
it lacks considerable work before it is complete, and is posted here early
to weed out any issues with its initial approach and general direction.

Here are the instructions to test on a Calxeda Midway:

https://wiki.linaro.org/LEG/Engineering/Virtualization/Platform_Device_Passthrough_on_Midway

The code is based on today's:

git://git.qemu.org/qemu.git

and available on the 'platdevpassthrough' branch of:

git://git.linaro.org/people/kim.phillips/qemu.git

Thanks,

Kim

Kim Phillips (2):
  hw/arm/virt: add a Calxeda XGMAC device
  hw/misc/vfio: add vfio-platform support

 hw/arm/virt.c  |  24 +++++++-
 hw/misc/vfio.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 168 insertions(+), 36 deletions(-)

-- 
1.9.0

[Qemu-devel] [RFC 1/2] hw/arm/virt: add a Calxeda XGMAC device

[Kim Phillips]

This is a hack and only serves as an example of what needs to be
done to make the next RFC - add vfio-platform support - work
for development purposes on a Calxeda Midway system.  We don't want
mach-virt to always create this ethernet device - DO NOT APPLY, etc.

Initial attempts to convince QEMU to create a memory mapped device
on the command line (e.g., -device vfio-platform,name=fff51000.ethernet)
would fail with "Parameter 'driver' expects pluggable device type".
Any guidance as to how to overcome this apparent design limitation
is welcome.

RAM is reduced from 30 to 1GiB such as to not overlap the xgmac device's
physical address.  Not sure if the 30GiB RAM (or whatever the user sets
it to with -m) could be set up above 0x1_0000_0000, but there is probably
extra work needed to resolve this type of conflict.

note: vfio-platform interrupt support development may want interrupt
property data filled; here it's omitted for the time being.

Not-signed-off-by: Kim Phillips <kim.phillips@linaro.org>
---
 hw/arm/virt.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 517f2fe..75edf33 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -65,6 +65,7 @@ enum {
     VIRT_GIC_CPU,
     VIRT_UART,
     VIRT_MMIO,
+    VIRT_ETHERNET,
 };
 
 typedef struct MemMapEntry {
@@ -106,7 +107,8 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_MMIO] = { 0xa000000, 0x200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     /* 0x10000000 .. 0x40000000 reserved for PCI */
-    [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
+    [VIRT_MEM] = { 0x40000000, 1ULL * 1024 * 1024 * 1024 },
+    [VIRT_ETHERNET] = { 0xfff51000, 0x1000 },
 };
 
 static const int a15irqmap[] = {
@@ -291,6 +293,25 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }
 
+static void create_ethernet(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vbi->memmap[VIRT_ETHERNET].base;
+    hwaddr size = vbi->memmap[VIRT_ETHERNET].size;
+    const char compat[] = "calxeda,hb-xgmac";
+
+    sysbus_create_simple("vfio-platform", base, NULL);
+
+    nodename = g_strdup_printf("/ethernet@%" PRIx64, base);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+
+    /* Note that we can't use setprop_string because of the embedded NUL */
+    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, base, 2, size);
+
+    g_free(nodename);
+}
+
 static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;
@@ -419,6 +440,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
     }
 
     create_uart(vbi, pic);
+    create_ethernet(vbi, pic);
 
     /* Create mmio transports, so the user can create virtio backends
      * (which will be automatically plugged in to the transports). If
-- 
1.9.0

[Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Kim Phillips]

We basically add support for the SysBusDevice type in addition to the
existing PCIDevice support.  This involves taking common code from the
existing vfio_initfn(), and putting it under a new vfio_find_get_group(),
that both vfio_initfn() and the new vfio_platform_realize() call.
Since realize() returns void, unlike PCIDevice's initfn(),
error codes are moved into the error message text with %m.
Some exceptions to the PCI path are added for platform devices,
mostly in the form of early returns, since less setup is needed.
I chose to reuse VFIODevice's config_size to determine whether
the device was a PCI device or a platform device, which might
need to change.

Currently only MMIO access is supported at this time.  It works
because of qemu's stage 1 translation, but needs to be in stage 2
in case other entities than the guest OS want to access it.
A KVM patch to address this is in the works.

The perceived path for future QEMU development is:

- support allocating a variable number of regions
  - VFIODevice's bars[] become dynamically allocated *regions
  - VFIOBAR's device fd replaced with parent VFIODevice ptr,
    to facilitate BAR r/w ops calling vfio_eoi()
- add support for interrupts
- verify and test platform dev unmap path
- test existing PCI path for any regressions
- add support for creating platform devices on the qemu command line
  - currently device address specification is hardcoded for test
    development on Calxeda Midway's fff51000.ethernet device
- reset is not supported and registration of reset functions is
  bypassed for platform devices.
  - there is no standard means of resetting a platform device,
    unsure if it suffices to be handled at device--VFIO binding time

Signed-off-by: Kim Phillips <kim.phillips@linaro.org>
---
 hw/misc/vfio.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 145 insertions(+), 35 deletions(-)

diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
index 8db182f..eed24db 100644
--- a/hw/misc/vfio.c
+++ b/hw/misc/vfio.c
@@ -32,6 +32,7 @@
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
 #include "hw/pci/pci.h"
+#include "hw/sysbus.h"
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "qemu/event_notifier.h"
@@ -166,7 +167,10 @@ typedef struct VFIOMSIXInfo {
 } VFIOMSIXInfo;
 
 typedef struct VFIODevice {
-    PCIDevice pdev;
+    union {
+        PCIDevice pdev;
+        SysBusDevice sbdev;
+    };
     int fd;
     VFIOINTx intx;
     unsigned int config_size;
@@ -180,6 +184,8 @@ typedef struct VFIODevice {
     VFIOMSIXInfo *msix;
     int nr_vectors; /* Number of MSI/MSIX vectors currently in use */
     int interrupt; /* Current interrupt type */
+    char *name;  /* platform device name, e.g., fff51000.ethernet */
+    int nr_regions; /* platform devices' number of regions */
     VFIOBAR bars[PCI_NUM_REGIONS - 1]; /* No ROM */
     VFIOVGA vga; /* 0xa0000, 0x3b0, 0x3c0 */
     PCIHostDeviceAddress host;
@@ -2497,8 +2503,6 @@ empty_region:
         memory_region_init(submem, OBJECT(vdev), name, 0);
     }
 
-    memory_region_add_subregion(mem, offset, submem);
-
     return ret;
 }
 
@@ -2552,6 +2556,7 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
                       &bar->mmap_mem, &bar->mmap, size, 0, name)) {
         error_report("%s unsupported. Performance may be slow", name);
     }
+    memory_region_add_subregion(&bar->mem, 0, &bar->mmap_mem);
 
     if (vdev->msix && vdev->msix->table_bar == nr) {
         unsigned start;
@@ -2566,15 +2571,38 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
                           &vdev->msix->mmap, size, start, name)) {
             error_report("%s unsupported. Performance may be slow", name);
         }
+        memory_region_add_subregion(&bar->mem, start, &vdev->msix->mmap_mem);
     }
 
     vfio_bar_quirk_setup(vdev, nr);
 }
 
+static void vfio_map_region(VFIODevice *vdev, int nr)
+{
+    VFIOBAR *bar = &vdev->bars[nr];
+    unsigned size = bar->size;
+    char name[64];
+
+    snprintf(name, sizeof(name), "VFIO %s region %d mmap", vdev->name, nr);
+
+    if (vfio_mmap_bar(vdev, bar, &bar->mem,
+                      &bar->mmap_mem, &bar->mmap, size, 0, name)) {
+        error_report("error mmapping %s: %m", name);
+    }
+}
+
 static void vfio_map_bars(VFIODevice *vdev)
 {
     int i;
 
+    if (!vdev->config_size) {
+        /* platform device */
+        for (i = 0; i < vdev->nr_regions; i++) {
+            vfio_map_region(vdev, i);
+        }
+        return;
+    }
+
     for (i = 0; i < PCI_ROM_SLOT; i++) {
         vfio_map_bar(vdev, i);
     }
@@ -3144,7 +3172,8 @@ static void vfio_pci_reset_handler(void *opaque)
 
     QLIST_FOREACH(group, &group_list, next) {
         QLIST_FOREACH(vdev, &group->device_list, next) {
-            if (vdev->needs_reset) {
+            /* HACK: restrict reset to PCI devices (have config_size) for now */
+            if (vdev->config_size && vdev->needs_reset) {
                 vfio_pci_hot_reset_multi(vdev);
             }
         }
@@ -3418,25 +3447,18 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev)
     DPRINTF("Device %s flags: %u, regions: %u, irgs: %u\n", name,
             dev_info.flags, dev_info.num_regions, dev_info.num_irqs);
 
-    if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI)) {
-        error_report("vfio: Um, this isn't a PCI device");
-        goto error;
-    }
-
+    vdev->nr_regions = dev_info.num_regions;
     vdev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
 
-    if (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
+    if (dev_info.num_regions > PCI_NUM_REGIONS) ||
+        ((dev_info.flags & VFIO_DEVICE_FLAGS_PCI) &&
+         (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1)) {
         error_report("vfio: unexpected number of io regions %u",
                      dev_info.num_regions);
         goto error;
     }
 
-    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
-        error_report("vfio: unexpected number of irqs %u", dev_info.num_irqs);
-        goto error;
-    }
-
-    for (i = VFIO_PCI_BAR0_REGION_INDEX; i < VFIO_PCI_ROM_REGION_INDEX; i++) {
+    for (i = 0; i < dev_info.num_regions; i++) {
         reg_info.index = i;
 
         ret = ioctl(vdev->fd, VFIO_DEVICE_GET_REGION_INFO, &reg_info);
@@ -3458,6 +3480,15 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev)
         QLIST_INIT(&vdev->bars[i].quirks);
     }
 
+    /* following is for PCI devices, at least for now */
+    if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI))
+        return 0;
+
+    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
+        error_report("vfio: unexpected number of irqs %u", dev_info.num_irqs);
+        goto error;
+    }
+
     reg_info.index = VFIO_PCI_CONFIG_REGION_INDEX;
 
     ret = ioctl(vdev->fd, VFIO_DEVICE_GET_REGION_INFO, &reg_info);
@@ -3659,32 +3690,25 @@ static void vfio_unregister_err_notifier(VFIODevice *vdev)
     event_notifier_cleanup(&vdev->err_notifier);
 }
 
-static int vfio_initfn(PCIDevice *pdev)
+static VFIOGroup *vfio_find_get_group(char *path)
 {
-    VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
     VFIOGroup *group;
-    char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name;
+    char iommu_group_path[PATH_MAX], *group_name;
     ssize_t len;
     struct stat st;
     int groupid;
-    int ret;
 
-    /* Check that the host device exists */
-    snprintf(path, sizeof(path),
-             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
-             vdev->host.domain, vdev->host.bus, vdev->host.slot,
-             vdev->host.function);
     if (stat(path, &st) < 0) {
-        error_report("vfio: error: no such host device: %s", path);
-        return -errno;
+        error_report("vfio: error: no such host device: %s: %m", path);
+        return NULL;
     }
 
     strncat(path, "iommu_group", sizeof(path) - strlen(path) - 1);
 
     len = readlink(path, iommu_group_path, PATH_MAX);
     if (len <= 0) {
-        error_report("vfio: error no iommu_group for device");
-        return -errno;
+        error_report("vfio: error no iommu_group for device: %m");
+        return NULL;
     }
 
     iommu_group_path[len] = 0;
@@ -3692,18 +3716,37 @@ static int vfio_initfn(PCIDevice *pdev)
 
     if (sscanf(group_name, "%d", &groupid) != 1) {
         error_report("vfio: error reading %s: %m", path);
-        return -errno;
+        return NULL;
     }
 
-    DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__, vdev->host.domain,
-            vdev->host.bus, vdev->host.slot, vdev->host.function, groupid);
-
     group = vfio_get_group(groupid);
     if (!group) {
-        error_report("vfio: failed to get group %d", groupid);
-        return -ENOENT;
+        error_report("vfio: failed to get group %d: %m", groupid);
+        return NULL;
     }
 
+    return group;
+}
+
+static int vfio_initfn(PCIDevice *pdev)
+{
+    VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
+    VFIOGroup *group;
+    char path[PATH_MAX];
+    int ret;
+
+    /* Check that the host device exists */
+    snprintf(path, sizeof(path),
+             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
+             vdev->host.domain, vdev->host.bus, vdev->host.slot,
+             vdev->host.function);
+
+    group = vfio_find_get_group(path);
+
+    DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__, vdev->host.domain,
+            vdev->host.bus, vdev->host.slot, vdev->host.function,
+            group->groupid);
+
     snprintf(path, sizeof(path), "%04x:%02x:%02x.%01x",
             vdev->host.domain, vdev->host.bus, vdev->host.slot,
             vdev->host.function);
@@ -3916,3 +3959,70 @@ static void register_vfio_pci_dev_type(void)
 }
 
 type_init(register_vfio_pci_dev_type)
+
+
+/*
+ * VFIO platform devices
+ */
+static void vfio_platform_realize(DeviceState *dev, Error **errp)
+{
+    SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
+    VFIODevice *vdev = DO_UPCAST(VFIODevice, sbdev, sbdev);
+    VFIOGroup *group;
+    char path[PATH_MAX];
+    int ret;
+
+    vdev->name = malloc(PATH_MAX);
+    strcpy(vdev->name, "fff51000.ethernet");
+
+    /* Check that the host device exists */
+    snprintf(path, sizeof(path),
+             "/sys/bus/platform/devices/%s/", vdev->name);
+
+    group = vfio_find_get_group(path);
+    if (!group) {
+        error_report("vfio: failed to get group for device %s", path);
+        return;
+    }
+
+    strcpy(path, vdev->name);
+    ret = vfio_get_device(group, path, vdev);
+    if (ret) {
+        error_report("vfio: failed to get device %s", path);
+        vfio_put_group(group);
+        return;
+    }
+
+    vfio_map_bars(vdev);
+
+    sysbus_init_mmio(sbdev, &vdev->bars[0].mmap_mem);
+}
+
+static const VMStateDescription vfio_platform_vmstate = {
+    .name = "vfio-platform",
+    .unmigratable = 1,
+};
+
+static void vfio_platform_dev_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = vfio_platform_realize;
+    dc->vmsd = &vfio_platform_vmstate;
+    dc->desc = "VFIO-based platform device assignment";
+    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
+}
+
+static const TypeInfo vfio_platform_dev_info = {
+    .name = "vfio-platform",
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VFIODevice),
+    .class_init = vfio_platform_dev_class_init,
+};
+
+static void register_vfio_platform_dev_type(void)
+{
+    type_register_static(&vfio_platform_dev_info);
+}
+
+type_init(register_vfio_platform_dev_type)
-- 
1.9.0

Re: [Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Alex Williamson]

On Tue, 2014-02-25 at 20:37 -0600, Kim Phillips wrote:
> We basically add support for the SysBusDevice type in addition to the
> existing PCIDevice support.  This involves taking common code from the
> existing vfio_initfn(), and putting it under a new vfio_find_get_group(),
> that both vfio_initfn() and the new vfio_platform_realize() call.
> Since realize() returns void, unlike PCIDevice's initfn(),
> error codes are moved into the error message text with %m.
> Some exceptions to the PCI path are added for platform devices,
> mostly in the form of early returns, since less setup is needed.
> I chose to reuse VFIODevice's config_size to determine whether
> the device was a PCI device or a platform device, which might
> need to change.
> 
> Currently only MMIO access is supported at this time.  It works
> because of qemu's stage 1 translation, but needs to be in stage 2
> in case other entities than the guest OS want to access it.
> A KVM patch to address this is in the works.
> 
> The perceived path for future QEMU development is:
> 
> - support allocating a variable number of regions
>   - VFIODevice's bars[] become dynamically allocated *regions
>   - VFIOBAR's device fd replaced with parent VFIODevice ptr,
>     to facilitate BAR r/w ops calling vfio_eoi()
> - add support for interrupts
> - verify and test platform dev unmap path
> - test existing PCI path for any regressions
> - add support for creating platform devices on the qemu command line
>   - currently device address specification is hardcoded for test
>     development on Calxeda Midway's fff51000.ethernet device
> - reset is not supported and registration of reset functions is
>   bypassed for platform devices.
>   - there is no standard means of resetting a platform device,
>     unsure if it suffices to be handled at device--VFIO binding time


On one hand, I had assumed we'd create a hw/misc/vfio/ directory and
perhaps split things into pci, common, vga, and platform.  We already
need the pci vs vga split as there's lots of irrelevant and complicated
vga quirks that most people don't want to see.  On the other hand, I'm
surprised how much of the PCI code you can re-use.  Still, I think it
would be cleaner to create a VFIOPCIDevice and a VFIOPlatformDevice.

> Signed-off-by: Kim Phillips <kim.phillips@linaro.org>
> ---
>  hw/misc/vfio.c | 180 ++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 145 insertions(+), 35 deletions(-)
> 
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 8db182f..eed24db 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -32,6 +32,7 @@
>  #include "hw/pci/msi.h"
>  #include "hw/pci/msix.h"
>  #include "hw/pci/pci.h"
> +#include "hw/sysbus.h"
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
>  #include "qemu/event_notifier.h"
> @@ -166,7 +167,10 @@ typedef struct VFIOMSIXInfo {
>  } VFIOMSIXInfo;
>  
>  typedef struct VFIODevice {
> -    PCIDevice pdev;
> +    union {
> +        PCIDevice pdev;
> +        SysBusDevice sbdev;
> +    };
>      int fd;
>      VFIOINTx intx;
>      unsigned int config_size;
> @@ -180,6 +184,8 @@ typedef struct VFIODevice {
>      VFIOMSIXInfo *msix;
>      int nr_vectors; /* Number of MSI/MSIX vectors currently in use */
>      int interrupt; /* Current interrupt type */
> +    char *name;  /* platform device name, e.g., fff51000.ethernet */
> +    int nr_regions; /* platform devices' number of regions */
>      VFIOBAR bars[PCI_NUM_REGIONS - 1]; /* No ROM */
>      VFIOVGA vga; /* 0xa0000, 0x3b0, 0x3c0 */
>      PCIHostDeviceAddress host;
> @@ -2497,8 +2503,6 @@ empty_region:
>          memory_region_init(submem, OBJECT(vdev), name, 0);
>      }
>  
> -    memory_region_add_subregion(mem, offset, submem);
> -

The "mem" parameter to this function is never used if we do this, but
I'm not sure I buy into this change anyway, see below.

>      return ret;
>  }
>  
> @@ -2552,6 +2556,7 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
>                        &bar->mmap_mem, &bar->mmap, size, 0, name)) {
>          error_report("%s unsupported. Performance may be slow", name);
>      }
> +    memory_region_add_subregion(&bar->mem, 0, &bar->mmap_mem);
>  
>      if (vdev->msix && vdev->msix->table_bar == nr) {
>          unsigned start;
> @@ -2566,15 +2571,38 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
>                            &vdev->msix->mmap, size, start, name)) {
>              error_report("%s unsupported. Performance may be slow", name);
>          }
> +        memory_region_add_subregion(&bar->mem, start, &vdev->msix->mmap_mem);
>      }
>  
>      vfio_bar_quirk_setup(vdev, nr);
>  }
>  
> +static void vfio_map_region(VFIODevice *vdev, int nr)
> +{
> +    VFIOBAR *bar = &vdev->bars[nr];
> +    unsigned size = bar->size;
> +    char name[64];
> +
> +    snprintf(name, sizeof(name), "VFIO %s region %d mmap", vdev->name, nr);
> +
> +    if (vfio_mmap_bar(vdev, bar, &bar->mem,
> +                      &bar->mmap_mem, &bar->mmap, size, 0, name)) {
> +        error_report("error mmapping %s: %m", name);
> +    }
> +}
> +
>  static void vfio_map_bars(VFIODevice *vdev)
>  {
>      int i;
>  
> +    if (!vdev->config_size) {
> +        /* platform device */
> +        for (i = 0; i < vdev->nr_regions; i++) {
> +            vfio_map_region(vdev, i);
> +        }
> +        return;
> +    }

I don't understand this, vfio_map_region() calls vfio_mmap_bar(), but
vfio_mmap_bar() has been gutted so that it only initializes the mmap
subregion, but never adds it.  vfio_map_bar() does both the
initialization of the slow, read/write, memory region as well as adding
the mmap sub-region.  I don't see where platform devices get a memory
region inserted anywhere into the guest address space.

> +
>      for (i = 0; i < PCI_ROM_SLOT; i++) {
>          vfio_map_bar(vdev, i);
>      }
> @@ -3144,7 +3172,8 @@ static void vfio_pci_reset_handler(void *opaque)
>  
>      QLIST_FOREACH(group, &group_list, next) {
>          QLIST_FOREACH(vdev, &group->device_list, next) {
> -            if (vdev->needs_reset) {
> +            /* HACK: restrict reset to PCI devices (have config_size) for now */
> +            if (vdev->config_size && vdev->needs_reset) {
>                  vfio_pci_hot_reset_multi(vdev);
>              }
>          }
> @@ -3418,25 +3447,18 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev)
>      DPRINTF("Device %s flags: %u, regions: %u, irgs: %u\n", name,
>              dev_info.flags, dev_info.num_regions, dev_info.num_irqs);
>  
> -    if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI)) {
> -        error_report("vfio: Um, this isn't a PCI device");
> -        goto error;
> -    }
> -
> +    vdev->nr_regions = dev_info.num_regions;
>      vdev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
>  
> -    if (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
> +    if (dev_info.num_regions > PCI_NUM_REGIONS) ||
> +        ((dev_info.flags & VFIO_DEVICE_FLAGS_PCI) &&
> +         (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1)) {

Now we start to have platform devices error out if they have more
regions than PCI knows about...  That doesn't make much sense.

>          error_report("vfio: unexpected number of io regions %u",
>                       dev_info.num_regions);
>          goto error;
>      }
>  
> -    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
> -        error_report("vfio: unexpected number of irqs %u", dev_info.num_irqs);
> -        goto error;
> -    }
> -
> -    for (i = VFIO_PCI_BAR0_REGION_INDEX; i < VFIO_PCI_ROM_REGION_INDEX; i++) {
> +    for (i = 0; i < dev_info.num_regions; i++) {
>          reg_info.index = i;

This would break PCI since there are region indexes beyond the BARs.
>  
>          ret = ioctl(vdev->fd, VFIO_DEVICE_GET_REGION_INFO, &reg_info);
> @@ -3458,6 +3480,15 @@ static int vfio_get_device(VFIOGroup *group, const char *name, VFIODevice *vdev)
>          QLIST_INIT(&vdev->bars[i].quirks);
>      }
>  
> +    /* following is for PCI devices, at least for now */
> +    if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI))
> +        return 0;
> +
> +    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
> +        error_report("vfio: unexpected number of irqs %u", dev_info.num_irqs);
> +        goto error;
> +    }
> +
>      reg_info.index = VFIO_PCI_CONFIG_REGION_INDEX;
>  
>      ret = ioctl(vdev->fd, VFIO_DEVICE_GET_REGION_INFO, &reg_info);
> @@ -3659,32 +3690,25 @@ static void vfio_unregister_err_notifier(VFIODevice *vdev)
>      event_notifier_cleanup(&vdev->err_notifier);
>  }
>  
> -static int vfio_initfn(PCIDevice *pdev)
> +static VFIOGroup *vfio_find_get_group(char *path)
>  {
> -    VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
>      VFIOGroup *group;
> -    char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name;
> +    char iommu_group_path[PATH_MAX], *group_name;
>      ssize_t len;
>      struct stat st;
>      int groupid;
> -    int ret;
>  
> -    /* Check that the host device exists */
> -    snprintf(path, sizeof(path),
> -             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
> -             vdev->host.domain, vdev->host.bus, vdev->host.slot,
> -             vdev->host.function);
>      if (stat(path, &st) < 0) {
> -        error_report("vfio: error: no such host device: %s", path);
> -        return -errno;
> +        error_report("vfio: error: no such host device: %s: %m", path);
> +        return NULL;
>      }
>  
>      strncat(path, "iommu_group", sizeof(path) - strlen(path) - 1);
>  
>      len = readlink(path, iommu_group_path, PATH_MAX);
>      if (len <= 0) {
> -        error_report("vfio: error no iommu_group for device");
> -        return -errno;
> +        error_report("vfio: error no iommu_group for device: %m");
> +        return NULL;
>      }
>  
>      iommu_group_path[len] = 0;
> @@ -3692,18 +3716,37 @@ static int vfio_initfn(PCIDevice *pdev)
>  
>      if (sscanf(group_name, "%d", &groupid) != 1) {
>          error_report("vfio: error reading %s: %m", path);
> -        return -errno;
> +        return NULL;
>      }
>  
> -    DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__, vdev->host.domain,
> -            vdev->host.bus, vdev->host.slot, vdev->host.function, groupid);
> -
>      group = vfio_get_group(groupid);
>      if (!group) {
> -        error_report("vfio: failed to get group %d", groupid);
> -        return -ENOENT;
> +        error_report("vfio: failed to get group %d: %m", groupid);
> +        return NULL;
>      }
>  
> +    return group;
> +}
> +
> +static int vfio_initfn(PCIDevice *pdev)
> +{
> +    VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
> +    VFIOGroup *group;
> +    char path[PATH_MAX];
> +    int ret;
> +
> +    /* Check that the host device exists */
> +    snprintf(path, sizeof(path),
> +             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
> +             vdev->host.domain, vdev->host.bus, vdev->host.slot,
> +             vdev->host.function);
> +
> +    group = vfio_find_get_group(path);
> +
> +    DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__, vdev->host.domain,
> +            vdev->host.bus, vdev->host.slot, vdev->host.function,
> +            group->groupid);
> +
>      snprintf(path, sizeof(path), "%04x:%02x:%02x.%01x",
>              vdev->host.domain, vdev->host.bus, vdev->host.slot,
>              vdev->host.function);
> @@ -3916,3 +3959,70 @@ static void register_vfio_pci_dev_type(void)
>  }
>  
>  type_init(register_vfio_pci_dev_type)
> +
> +
> +/*
> + * VFIO platform devices
> + */
> +static void vfio_platform_realize(DeviceState *dev, Error **errp)
> +{
> +    SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
> +    VFIODevice *vdev = DO_UPCAST(VFIODevice, sbdev, sbdev);
> +    VFIOGroup *group;
> +    char path[PATH_MAX];
> +    int ret;
> +
> +    vdev->name = malloc(PATH_MAX);
> +    strcpy(vdev->name, "fff51000.ethernet");
> +
> +    /* Check that the host device exists */
> +    snprintf(path, sizeof(path),
> +             "/sys/bus/platform/devices/%s/", vdev->name);
> +
> +    group = vfio_find_get_group(path);
> +    if (!group) {
> +        error_report("vfio: failed to get group for device %s", path);
> +        return;
> +    }
> +
> +    strcpy(path, vdev->name);
> +    ret = vfio_get_device(group, path, vdev);
> +    if (ret) {
> +        error_report("vfio: failed to get device %s", path);
> +        vfio_put_group(group);
> +        return;
> +    }
> +
> +    vfio_map_bars(vdev);
> +
> +    sysbus_init_mmio(sbdev, &vdev->bars[0].mmap_mem);
> +}
> +
> +static const VMStateDescription vfio_platform_vmstate = {
> +    .name = "vfio-platform",
> +    .unmigratable = 1,
> +};
> +
> +static void vfio_platform_dev_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vfio_platform_realize;
> +    dc->vmsd = &vfio_platform_vmstate;
> +    dc->desc = "VFIO-based platform device assignment";
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vfio_platform_dev_info = {
> +    .name = "vfio-platform",
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VFIODevice),
> +    .class_init = vfio_platform_dev_class_init,
> +};
> +
> +static void register_vfio_platform_dev_type(void)
> +{
> +    type_register_static(&vfio_platform_dev_info);
> +}
> +
> +type_init(register_vfio_platform_dev_type)

This all looks reasonable, but I suspect it would be cleaner if
vfio_find_get_group() was in a common file along with basic mmap and
read/write access functions.  Thanks,

Alex

Re: [Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Kim Phillips]

On Fri, 28 Feb 2014 11:03:18 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Tue, 2014-02-25 at 20:37 -0600, Kim Phillips wrote:
> > - support allocating a variable number of regions
> >   - VFIODevice's bars[] become dynamically allocated *regions
> >   - VFIOBAR's device fd replaced with parent VFIODevice ptr,
> >     to facilitate BAR r/w ops calling vfio_eoi()
> 
> On one hand, I had assumed we'd create a hw/misc/vfio/ directory and
> perhaps split things into pci, common, vga, and platform.  We already
> need the pci vs vga split as there's lots of irrelevant and complicated
> vga quirks that most people don't want to see.  On the other hand, I'm
> surprised how much of the PCI code you can re-use.  Still, I think it
> would be cleaner to create a VFIOPCIDevice and a VFIOPlatformDevice.

sounds good, had started down that path but reverted it because I too
realized how much code was indeed being reused.  I'm ok with
VFIOPCIDevice and a separate VFIOPlatformDevice for now, thanks.

> >  static void vfio_map_bars(VFIODevice *vdev)
> >  {
> >      int i;
> >  
> > +    if (!vdev->config_size) {
> > +        /* platform device */
> > +        for (i = 0; i < vdev->nr_regions; i++) {
> > +            vfio_map_region(vdev, i);
> > +        }
> > +        return;
> > +    }
> 
> I don't understand this, vfio_map_region() calls vfio_mmap_bar(), but
> vfio_mmap_bar() has been gutted so that it only initializes the mmap
> subregion, but never adds it.  vfio_map_bar() does both the
> initialization of the slow, read/write, memory region as well as adding
> the mmap sub-region.  I don't see where platform devices get a memory
> region inserted anywhere into the guest address space.

it's being done by the board code via the first RFC/patch ("hw/arm/virt:
add a Calxeda XGMAC device"), and vfio_mmap_bar()'s call to
memory_region_init_ram_ptr().

Yeah, the slow, r/w ops region path isn't used, and I'm not sure how to
make it work without addressing the problems the first RFC presents.

> > -    if (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
> > +    if (dev_info.num_regions > PCI_NUM_REGIONS) ||
> > +        ((dev_info.flags & VFIO_DEVICE_FLAGS_PCI) &&
> > +         (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1)) {
> 
> Now we start to have platform devices error out if they have more
> regions than PCI knows about...  That doesn't make much sense.

right, I should have made it more clear that the multiple-regions code
is sketchy in the "support allocating a variable number of regions"
blurb in the commit text of this RFC.

> > +static void register_vfio_platform_dev_type(void)
> > +{
> > +    type_register_static(&vfio_platform_dev_info);
> > +}
> > +
> > +type_init(register_vfio_platform_dev_type)
> 
> This all looks reasonable, but I suspect it would be cleaner if
> vfio_find_get_group() was in a common file along with basic mmap and
> read/write access functions.  Thanks,

so rename existing hw/misc/vfio.c to its original name vfio-pci.c, and
load all common functions back into a vfio.c?

Kim

Re: [Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Alex Williamson]

On Tue, 2014-03-04 at 18:24 -0600, Kim Phillips wrote:
> On Fri, 28 Feb 2014 11:03:18 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
> 
> > On Tue, 2014-02-25 at 20:37 -0600, Kim Phillips wrote:
> > > - support allocating a variable number of regions
> > >   - VFIODevice's bars[] become dynamically allocated *regions
> > >   - VFIOBAR's device fd replaced with parent VFIODevice ptr,
> > >     to facilitate BAR r/w ops calling vfio_eoi()
> > 
> > On one hand, I had assumed we'd create a hw/misc/vfio/ directory and
> > perhaps split things into pci, common, vga, and platform.  We already
> > need the pci vs vga split as there's lots of irrelevant and complicated
> > vga quirks that most people don't want to see.  On the other hand, I'm
> > surprised how much of the PCI code you can re-use.  Still, I think it
> > would be cleaner to create a VFIOPCIDevice and a VFIOPlatformDevice.
> 
> sounds good, had started down that path but reverted it because I too
> realized how much code was indeed being reused.  I'm ok with
> VFIOPCIDevice and a separate VFIOPlatformDevice for now, thanks.
> 
> > >  static void vfio_map_bars(VFIODevice *vdev)
> > >  {
> > >      int i;
> > >  
> > > +    if (!vdev->config_size) {
> > > +        /* platform device */
> > > +        for (i = 0; i < vdev->nr_regions; i++) {
> > > +            vfio_map_region(vdev, i);
> > > +        }
> > > +        return;
> > > +    }
> > 
> > I don't understand this, vfio_map_region() calls vfio_mmap_bar(), but
> > vfio_mmap_bar() has been gutted so that it only initializes the mmap
> > subregion, but never adds it.  vfio_map_bar() does both the
> > initialization of the slow, read/write, memory region as well as adding
> > the mmap sub-region.  I don't see where platform devices get a memory
> > region inserted anywhere into the guest address space.
> 
> it's being done by the board code via the first RFC/patch ("hw/arm/virt:
> add a Calxeda XGMAC device"), and vfio_mmap_bar()'s call to
> memory_region_init_ram_ptr().
> 
> Yeah, the slow, r/w ops region path isn't used, and I'm not sure how to
> make it work without addressing the problems the first RFC presents.
> 
> > > -    if (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
> > > +    if (dev_info.num_regions > PCI_NUM_REGIONS) ||
> > > +        ((dev_info.flags & VFIO_DEVICE_FLAGS_PCI) &&
> > > +         (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1)) {
> > 
> > Now we start to have platform devices error out if they have more
> > regions than PCI knows about...  That doesn't make much sense.
> 
> right, I should have made it more clear that the multiple-regions code
> is sketchy in the "support allocating a variable number of regions"
> blurb in the commit text of this RFC.
> 
> > > +static void register_vfio_platform_dev_type(void)
> > > +{
> > > +    type_register_static(&vfio_platform_dev_info);
> > > +}
> > > +
> > > +type_init(register_vfio_platform_dev_type)
> > 
> > This all looks reasonable, but I suspect it would be cleaner if
> > vfio_find_get_group() was in a common file along with basic mmap and
> > read/write access functions.  Thanks,
> 
> so rename existing hw/misc/vfio.c to its original name vfio-pci.c, and
> load all common functions back into a vfio.c?

I think hw/misc/vfio/{pci.c,common.c,platform.c,etc}  Thanks,

Alex

Re: [Qemu-devel] [RFC 1/2] hw/arm/virt: add a Calxeda XGMAC device

[Eric Auger]

[-- Attachment #1: Type: text/plain, Size: 3913 bytes --]

Hi Kim,

- Parameter 'driver' expects pluggable device type
this can be fixed setting dc->cannot_instantiate_with_device_add_yet to
false in vfio_platform_dev_class_init. You do not get the error anymore.
However I must aknowledge I have not studies all possible side effects of
this setting and maybe there is a cleaner way to achieve the same result.
Then you can add additional properties in dc->props.
- location of the device in gpa space and impact on RAM size: my
understanding is that device could sit in another location than the hpa's
one and you are not compelled to use the same address.
- we effectively need to work on fdt tree generation for VFIO devices

Best Regards

Eric


On 26 February 2014 03:37, Kim Phillips <kim.phillips@linaro.org> wrote:

> This is a hack and only serves as an example of what needs to be
> done to make the next RFC - add vfio-platform support - work
> for development purposes on a Calxeda Midway system.  We don't want
> mach-virt to always create this ethernet device - DO NOT APPLY, etc.
>
> Initial attempts to convince QEMU to create a memory mapped device
> on the command line (e.g., -device vfio-platform,name=fff51000.ethernet)
> would fail with "Parameter 'driver' expects pluggable device type".
> Any guidance as to how to overcome this apparent design limitation
> is welcome.
>
> RAM is reduced from 30 to 1GiB such as to not overlap the xgmac device's
> physical address.  Not sure if the 30GiB RAM (or whatever the user sets
> it to with -m) could be set up above 0x1_0000_0000, but there is probably
> extra work needed to resolve this type of conflict.
>
> note: vfio-platform interrupt support development may want interrupt
> property data filled; here it's omitted for the time being.
>
> Not-signed-off-by: Kim Phillips <kim.phillips@linaro.org>
> ---
>  hw/arm/virt.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 517f2fe..75edf33 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -65,6 +65,7 @@ enum {
>      VIRT_GIC_CPU,
>      VIRT_UART,
>      VIRT_MMIO,
> +    VIRT_ETHERNET,
>  };
>
>  typedef struct MemMapEntry {
> @@ -106,7 +107,8 @@ static const MemMapEntry a15memmap[] = {
>      [VIRT_MMIO] = { 0xa000000, 0x200 },
>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that
> size */
>      /* 0x10000000 .. 0x40000000 reserved for PCI */
> -    [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
> +    [VIRT_MEM] = { 0x40000000, 1ULL * 1024 * 1024 * 1024 },
> +    [VIRT_ETHERNET] = { 0xfff51000, 0x1000 },
>  };
>
>  static const int a15irqmap[] = {
> @@ -291,6 +293,25 @@ static void create_uart(const VirtBoardInfo *vbi,
> qemu_irq *pic)
>      g_free(nodename);
>  }
>
> +static void create_ethernet(const VirtBoardInfo *vbi, qemu_irq *pic)
> +{
> +    char *nodename;
> +    hwaddr base = vbi->memmap[VIRT_ETHERNET].base;
> +    hwaddr size = vbi->memmap[VIRT_ETHERNET].size;
> +    const char compat[] = "calxeda,hb-xgmac";
> +
> +    sysbus_create_simple("vfio-platform", base, NULL);
> +
> +    nodename = g_strdup_printf("/ethernet@%" PRIx64, base);
> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
> +
> +    /* Note that we can't use setprop_string because of the embedded NUL
> */
> +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat,
> sizeof(compat));
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2, base, 2,
> size);
> +
> +    g_free(nodename);
> +}
> +
>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>  {
>      int i;
> @@ -419,6 +440,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>      }
>
>      create_uart(vbi, pic);
> +    create_ethernet(vbi, pic);
>
>      /* Create mmio transports, so the user can create virtio backends
>       * (which will be automatically plugged in to the transports). If
> --
> 1.9.0
>
>

[-- Attachment #2: Type: text/html, Size: 4768 bytes --]

Re: [Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Eric Auger]

[-- Attachment #1: Type: text/plain, Size: 13497 bytes --]

Hi Kim,

1) I agree with Alex on the fact I would prefer to have the qemu-side
platform device code separated from PCI device one, both using a separate
generic helper code. Indeed calling functions referencing BAR in the
platform device does not look natural to me;  although I understand the
code reuse rationale. This is also how the kernel side code is laid out.

2) about nr_regions. What is the number of nr_regions do you expect for
platform_devices? bars[] possible overshoot as already mentionned if
nr_regions > 6.

3) I understand you build nr_regions MemoryRegion (each mmaped on the hpa).
However I understand you only attach a single one to the SysBusDevice in
vfio_platform_realize:
sysbus_init_mmio(sbdev, &vdev->bars[0].mmap_mem);  this MemRegion is
assigned a gpa in virt.c? is it the correct understanding? What is the mem
hierarchy that is targetted. In PCI there are subregions. In this device,
there would be a single layer or hierarchy?

4)  vfio_unmap_bars should destroy the platform device mmaped regions
(vdev->mmap_mem and munmap the correspond hva (currently on slow regions
are munmapped and destroyed)

Best Regards

Eric


On 26 February 2014 03:37, Kim Phillips <kim.phillips@linaro.org> wrote:

> We basically add support for the SysBusDevice type in addition to the
> existing PCIDevice support.  This involves taking common code from the
> existing vfio_initfn(), and putting it under a new vfio_find_get_group(),
> that both vfio_initfn() and the new vfio_platform_realize() call.
> Since realize() returns void, unlike PCIDevice's initfn(),
> error codes are moved into the error message text with %m.
> Some exceptions to the PCI path are added for platform devices,
> mostly in the form of early returns, since less setup is needed.
> I chose to reuse VFIODevice's config_size to determine whether
> the device was a PCI device or a platform device, which might
> need to change.
>
> Currently only MMIO access is supported at this time.  It works
> because of qemu's stage 1 translation, but needs to be in stage 2
> in case other entities than the guest OS want to access it.
> A KVM patch to address this is in the works.
>
> The perceived path for future QEMU development is:
>
> - support allocating a variable number of regions
>   - VFIODevice's bars[] become dynamically allocated *regions
>   - VFIOBAR's device fd replaced with parent VFIODevice ptr,
>     to facilitate BAR r/w ops calling vfio_eoi()
> - add support for interrupts
> - verify and test platform dev unmap path
> - test existing PCI path for any regressions
> - add support for creating platform devices on the qemu command line
>   - currently device address specification is hardcoded for test
>     development on Calxeda Midway's fff51000.ethernet device
> - reset is not supported and registration of reset functions is
>   bypassed for platform devices.
>   - there is no standard means of resetting a platform device,
>     unsure if it suffices to be handled at device--VFIO binding time
>
> Signed-off-by: Kim Phillips <kim.phillips@linaro.org>
> ---
>  hw/misc/vfio.c | 180
> ++++++++++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 145 insertions(+), 35 deletions(-)
>
> diff --git a/hw/misc/vfio.c b/hw/misc/vfio.c
> index 8db182f..eed24db 100644
> --- a/hw/misc/vfio.c
> +++ b/hw/misc/vfio.c
> @@ -32,6 +32,7 @@
>  #include "hw/pci/msi.h"
>  #include "hw/pci/msix.h"
>  #include "hw/pci/pci.h"
> +#include "hw/sysbus.h"
>  #include "qemu-common.h"
>  #include "qemu/error-report.h"
>  #include "qemu/event_notifier.h"
> @@ -166,7 +167,10 @@ typedef struct VFIOMSIXInfo {
>  } VFIOMSIXInfo;
>
>  typedef struct VFIODevice {
> -    PCIDevice pdev;
> +    union {
> +        PCIDevice pdev;
> +        SysBusDevice sbdev;
> +    };
>      int fd;
>      VFIOINTx intx;
>      unsigned int config_size;
> @@ -180,6 +184,8 @@ typedef struct VFIODevice {
>      VFIOMSIXInfo *msix;
>      int nr_vectors; /* Number of MSI/MSIX vectors currently in use */
>      int interrupt; /* Current interrupt type */
> +    char *name;  /* platform device name, e.g., fff51000.ethernet */
> +    int nr_regions; /* platform devices' number of regions */
>      VFIOBAR bars[PCI_NUM_REGIONS - 1]; /* No ROM */
>      VFIOVGA vga; /* 0xa0000, 0x3b0, 0x3c0 */
>      PCIHostDeviceAddress host;
> @@ -2497,8 +2503,6 @@ empty_region:
>          memory_region_init(submem, OBJECT(vdev), name, 0);
>      }
>
> -    memory_region_add_subregion(mem, offset, submem);
> -
>      return ret;
>  }
>
> @@ -2552,6 +2556,7 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
>                        &bar->mmap_mem, &bar->mmap, size, 0, name)) {
>          error_report("%s unsupported. Performance may be slow", name);
>      }
> +    memory_region_add_subregion(&bar->mem, 0, &bar->mmap_mem);
>
>      if (vdev->msix && vdev->msix->table_bar == nr) {
>          unsigned start;
> @@ -2566,15 +2571,38 @@ static void vfio_map_bar(VFIODevice *vdev, int nr)
>                            &vdev->msix->mmap, size, start, name)) {
>              error_report("%s unsupported. Performance may be slow", name);
>          }
> +        memory_region_add_subregion(&bar->mem, start,
> &vdev->msix->mmap_mem);
>      }
>
>      vfio_bar_quirk_setup(vdev, nr);
>  }
>
> +static void vfio_map_region(VFIODevice *vdev, int nr)
> +{
> +    VFIOBAR *bar = &vdev->bars[nr];
> +    unsigned size = bar->size;
> +    char name[64];
> +
> +    snprintf(name, sizeof(name), "VFIO %s region %d mmap", vdev->name,
> nr);
> +
> +    if (vfio_mmap_bar(vdev, bar, &bar->mem,
> +                      &bar->mmap_mem, &bar->mmap, size, 0, name)) {
> +        error_report("error mmapping %s: %m", name);
> +    }
> +}
> +
>  static void vfio_map_bars(VFIODevice *vdev)
>  {
>      int i;
>
> +    if (!vdev->config_size) {
> +        /* platform device */
> +        for (i = 0; i < vdev->nr_regions; i++) {
> +            vfio_map_region(vdev, i);
> +        }
> +        return;
> +    }
> +
>      for (i = 0; i < PCI_ROM_SLOT; i++) {
>          vfio_map_bar(vdev, i);
>      }
> @@ -3144,7 +3172,8 @@ static void vfio_pci_reset_handler(void *opaque)
>
>      QLIST_FOREACH(group, &group_list, next) {
>          QLIST_FOREACH(vdev, &group->device_list, next) {
> -            if (vdev->needs_reset) {
> +            /* HACK: restrict reset to PCI devices (have config_size) for
> now */
> +            if (vdev->config_size && vdev->needs_reset) {
>                  vfio_pci_hot_reset_multi(vdev);
>              }
>          }
> @@ -3418,25 +3447,18 @@ static int vfio_get_device(VFIOGroup *group, const
> char *name, VFIODevice *vdev)
>      DPRINTF("Device %s flags: %u, regions: %u, irgs: %u\n", name,
>              dev_info.flags, dev_info.num_regions, dev_info.num_irqs);
>
> -    if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI)) {
> -        error_report("vfio: Um, this isn't a PCI device");
> -        goto error;
> -    }
> -
> +    vdev->nr_regions = dev_info.num_regions;
>      vdev->reset_works = !!(dev_info.flags & VFIO_DEVICE_FLAGS_RESET);
>
> -    if (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1) {
> +    if (dev_info.num_regions > PCI_NUM_REGIONS) ||
> +        ((dev_info.flags & VFIO_DEVICE_FLAGS_PCI) &&
> +         (dev_info.num_regions < VFIO_PCI_CONFIG_REGION_INDEX + 1)) {
>          error_report("vfio: unexpected number of io regions %u",
>                       dev_info.num_regions);
>          goto error;
>      }
>
> -    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
> -        error_report("vfio: unexpected number of irqs %u",
> dev_info.num_irqs);
> -        goto error;
> -    }
> -
> -    for (i = VFIO_PCI_BAR0_REGION_INDEX; i < VFIO_PCI_ROM_REGION_INDEX;
> i++) {
> +    for (i = 0; i < dev_info.num_regions; i++) {
>          reg_info.index = i;
>
>          ret = ioctl(vdev->fd, VFIO_DEVICE_GET_REGION_INFO, &reg_info);
> @@ -3458,6 +3480,15 @@ static int vfio_get_device(VFIOGroup *group, const
> char *name, VFIODevice *vdev)
>          QLIST_INIT(&vdev->bars[i].quirks);
>      }
>
> +    /* following is for PCI devices, at least for now */
> +    if (!(dev_info.flags & VFIO_DEVICE_FLAGS_PCI))
> +        return 0;
> +
> +    if (dev_info.num_irqs < VFIO_PCI_MSIX_IRQ_INDEX + 1) {
> +        error_report("vfio: unexpected number of irqs %u",
> dev_info.num_irqs);
> +        goto error;
> +    }
> +
>      reg_info.index = VFIO_PCI_CONFIG_REGION_INDEX;
>
>      ret = ioctl(vdev->fd, VFIO_DEVICE_GET_REGION_INFO, &reg_info);
> @@ -3659,32 +3690,25 @@ static void
> vfio_unregister_err_notifier(VFIODevice *vdev)
>      event_notifier_cleanup(&vdev->err_notifier);
>  }
>
> -static int vfio_initfn(PCIDevice *pdev)
> +static VFIOGroup *vfio_find_get_group(char *path)
>  {
> -    VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
>      VFIOGroup *group;
> -    char path[PATH_MAX], iommu_group_path[PATH_MAX], *group_name;
> +    char iommu_group_path[PATH_MAX], *group_name;
>      ssize_t len;
>      struct stat st;
>      int groupid;
> -    int ret;
>
> -    /* Check that the host device exists */
> -    snprintf(path, sizeof(path),
> -             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
> -             vdev->host.domain, vdev->host.bus, vdev->host.slot,
> -             vdev->host.function);
>      if (stat(path, &st) < 0) {
> -        error_report("vfio: error: no such host device: %s", path);
> -        return -errno;
> +        error_report("vfio: error: no such host device: %s: %m", path);
> +        return NULL;
>      }
>
>      strncat(path, "iommu_group", sizeof(path) - strlen(path) - 1);
>
>      len = readlink(path, iommu_group_path, PATH_MAX);
>      if (len <= 0) {
> -        error_report("vfio: error no iommu_group for device");
> -        return -errno;
> +        error_report("vfio: error no iommu_group for device: %m");
> +        return NULL;
>      }
>
>      iommu_group_path[len] = 0;
> @@ -3692,18 +3716,37 @@ static int vfio_initfn(PCIDevice *pdev)
>
>      if (sscanf(group_name, "%d", &groupid) != 1) {
>          error_report("vfio: error reading %s: %m", path);
> -        return -errno;
> +        return NULL;
>      }
>
> -    DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__,
> vdev->host.domain,
> -            vdev->host.bus, vdev->host.slot, vdev->host.function,
> groupid);
> -
>      group = vfio_get_group(groupid);
>      if (!group) {
> -        error_report("vfio: failed to get group %d", groupid);
> -        return -ENOENT;
> +        error_report("vfio: failed to get group %d: %m", groupid);
> +        return NULL;
>      }
>
> +    return group;
> +}
> +
> +static int vfio_initfn(PCIDevice *pdev)
> +{
> +    VFIODevice *pvdev, *vdev = DO_UPCAST(VFIODevice, pdev, pdev);
> +    VFIOGroup *group;
> +    char path[PATH_MAX];
> +    int ret;
> +
> +    /* Check that the host device exists */
> +    snprintf(path, sizeof(path),
> +             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/",
> +             vdev->host.domain, vdev->host.bus, vdev->host.slot,
> +             vdev->host.function);
> +
> +    group = vfio_find_get_group(path);
> +
> +    DPRINTF("%s(%04x:%02x:%02x.%x) group %d\n", __func__,
> vdev->host.domain,
> +            vdev->host.bus, vdev->host.slot, vdev->host.function,
> +            group->groupid);
> +
>      snprintf(path, sizeof(path), "%04x:%02x:%02x.%01x",
>              vdev->host.domain, vdev->host.bus, vdev->host.slot,
>              vdev->host.function);
> @@ -3916,3 +3959,70 @@ static void register_vfio_pci_dev_type(void)
>  }
>
>  type_init(register_vfio_pci_dev_type)
> +
> +
> +/*
> + * VFIO platform devices
> + */
> +static void vfio_platform_realize(DeviceState *dev, Error **errp)
> +{
> +    SysBusDevice *sbdev = SYS_BUS_DEVICE(dev);
> +    VFIODevice *vdev = DO_UPCAST(VFIODevice, sbdev, sbdev);
> +    VFIOGroup *group;
> +    char path[PATH_MAX];
> +    int ret;
> +
> +    vdev->name = malloc(PATH_MAX);
> +    strcpy(vdev->name, "fff51000.ethernet");
> +
> +    /* Check that the host device exists */
> +    snprintf(path, sizeof(path),
> +             "/sys/bus/platform/devices/%s/", vdev->name);
> +
> +    group = vfio_find_get_group(path);
> +    if (!group) {
> +        error_report("vfio: failed to get group for device %s", path);
> +        return;
> +    }
> +
> +    strcpy(path, vdev->name);
> +    ret = vfio_get_device(group, path, vdev);
> +    if (ret) {
> +        error_report("vfio: failed to get device %s", path);
> +        vfio_put_group(group);
> +        return;
> +    }
> +
> +    vfio_map_bars(vdev);
> +
> +    sysbus_init_mmio(sbdev, &vdev->bars[0].mmap_mem);
> +}
> +
> +static const VMStateDescription vfio_platform_vmstate = {
> +    .name = "vfio-platform",
> +    .unmigratable = 1,
> +};
> +
> +static void vfio_platform_dev_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = vfio_platform_realize;
> +    dc->vmsd = &vfio_platform_vmstate;
> +    dc->desc = "VFIO-based platform device assignment";
> +    set_bit(DEVICE_CATEGORY_MISC, dc->categories);
> +}
> +
> +static const TypeInfo vfio_platform_dev_info = {
> +    .name = "vfio-platform",
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(VFIODevice),
> +    .class_init = vfio_platform_dev_class_init,
> +};
> +
> +static void register_vfio_platform_dev_type(void)
> +{
> +    type_register_static(&vfio_platform_dev_info);
> +}
> +
> +type_init(register_vfio_platform_dev_type)
> --
> 1.9.0
>
>

[-- Attachment #2: Type: text/html, Size: 15765 bytes --]

Re: [Qemu-devel] [RFC 1/2] hw/arm/virt: add a Calxeda XGMAC device

[Andreas Färber]

Hi,

Am 05.03.2014 05:27, schrieb Eric Auger:
> Hi Kim,
> 
> - Parameter 'driver' expects pluggable device type
> this can be fixed setting dc->cannot_instantiate_with_device_add_yet to
> false in vfio_platform_dev_class_init. You do not get the error anymore.
> However I must aknowledge I have not studies all possible side effects
> of this setting and maybe there is a cleaner way to achieve the same
> result.

If the realize/init function sets up MMIO mappings and IRQs itself
(rather than expecting the code instantiating the device to do so) then
you are free to set that field to false.

In this case that would be vbi->memmap[VIRT_ETHERNET].base in
create_ethernet(), so I have doubts.

Regards,
Andreas

> Then you can add additional properties in dc->props.
> - location of the device in gpa space and impact on RAM size: my
> understanding is that device could sit in another location than the
> hpa's one and you are not compelled to use the same address.
> - we effectively need to work on fdt tree generation for VFIO devices
> 
> Best Regards
> 
> Eric
> 
> 
> On 26 February 2014 03:37, Kim Phillips <kim.phillips@linaro.org
> <mailto:kim.phillips@linaro.org>> wrote:
> 
>     This is a hack and only serves as an example of what needs to be
>     done to make the next RFC - add vfio-platform support - work
>     for development purposes on a Calxeda Midway system.  We don't want
>     mach-virt to always create this ethernet device - DO NOT APPLY, etc.
> 
>     Initial attempts to convince QEMU to create a memory mapped device
>     on the command line (e.g., -device vfio-platform,name=fff51000.ethernet)
>     would fail with "Parameter 'driver' expects pluggable device type".
>     Any guidance as to how to overcome this apparent design limitation
>     is welcome.
> 
>     RAM is reduced from 30 to 1GiB such as to not overlap the xgmac device's
>     physical address.  Not sure if the 30GiB RAM (or whatever the user sets
>     it to with -m) could be set up above 0x1_0000_0000, but there is
>     probably
>     extra work needed to resolve this type of conflict.
> 
>     note: vfio-platform interrupt support development may want interrupt
>     property data filled; here it's omitted for the time being.
> 
>     Not-signed-off-by: Kim Phillips <kim.phillips@linaro.org
>     <mailto:kim.phillips@linaro.org>>
>     ---
>      hw/arm/virt.c | 24 +++++++++++++++++++++++-
>      1 file changed, 23 insertions(+), 1 deletion(-)
> 
>     diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>     index 517f2fe..75edf33 100644
>     --- a/hw/arm/virt.c
>     +++ b/hw/arm/virt.c
>     @@ -65,6 +65,7 @@ enum {
>          VIRT_GIC_CPU,
>          VIRT_UART,
>          VIRT_MMIO,
>     +    VIRT_ETHERNET,
>      };
> 
>      typedef struct MemMapEntry {
>     @@ -106,7 +107,8 @@ static const MemMapEntry a15memmap[] = {
>          [VIRT_MMIO] = { 0xa000000, 0x200 },
>          /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of
>     that size */
>          /* 0x10000000 .. 0x40000000 reserved for PCI */
>     -    [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>     +    [VIRT_MEM] = { 0x40000000, 1ULL * 1024 * 1024 * 1024 },
>     +    [VIRT_ETHERNET] = { 0xfff51000, 0x1000 },
>      };
> 
>      static const int a15irqmap[] = {
>     @@ -291,6 +293,25 @@ static void create_uart(const VirtBoardInfo
>     *vbi, qemu_irq *pic)
>          g_free(nodename);
>      }
> 
>     +static void create_ethernet(const VirtBoardInfo *vbi, qemu_irq *pic)
>     +{
>     +    char *nodename;
>     +    hwaddr base = vbi->memmap[VIRT_ETHERNET].base;
>     +    hwaddr size = vbi->memmap[VIRT_ETHERNET].size;
>     +    const char compat[] = "calxeda,hb-xgmac";
>     +
>     +    sysbus_create_simple("vfio-platform", base, NULL);
>     +
>     +    nodename = g_strdup_printf("/ethernet@%" PRIx64, base);
>     +    qemu_fdt_add_subnode(vbi->fdt, nodename);
>     +
>     +    /* Note that we can't use setprop_string because of the
>     embedded NUL */
>     +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat,
>     sizeof(compat));
>     +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2,
>     base, 2, size);
>     +
>     +    g_free(nodename);
>     +}
>     +
>      static void create_virtio_devices(const VirtBoardInfo *vbi,
>     qemu_irq *pic)
>      {
>          int i;
>     @@ -419,6 +440,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>          }
> 
>          create_uart(vbi, pic);
>     +    create_ethernet(vbi, pic);
> 
>          /* Create mmio transports, so the user can create virtio backends
>           * (which will be automatically plugged in to the transports). If
>     --
>     1.9.0
> 
> 


-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Andreas Färber]

Am 05.03.2014 02:23, schrieb Alex Williamson:
> On Tue, 2014-03-04 at 18:24 -0600, Kim Phillips wrote:
>> On Fri, 28 Feb 2014 11:03:18 -0700
>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>
>>> This all looks reasonable, but I suspect it would be cleaner if
>>> vfio_find_get_group() was in a common file along with basic mmap and
>>> read/write access functions.  Thanks,
>>
>> so rename existing hw/misc/vfio.c to its original name vfio-pci.c, and
>> load all common functions back into a vfio.c?
> 
> I think hw/misc/vfio/{pci.c,common.c,platform.c,etc}  Thanks,

What about hw/vfio/ then?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC 2/2] hw/misc/vfio: add vfio-platform support

[Paolo Bonzini]

Il 05/03/2014 12:28, Andreas Färber ha scritto:
> Am 05.03.2014 02:23, schrieb Alex Williamson:
>> On Tue, 2014-03-04 at 18:24 -0600, Kim Phillips wrote:
>>> On Fri, 28 Feb 2014 11:03:18 -0700
>>> Alex Williamson <alex.williamson@redhat.com> wrote:
>>>
>>>> This all looks reasonable, but I suspect it would be cleaner if
>>>> vfio_find_get_group() was in a common file along with basic mmap and
>>>> read/write access functions.  Thanks,
>>>
>>> so rename existing hw/misc/vfio.c to its original name vfio-pci.c, and
>>> load all common functions back into a vfio.c?
>>
>> I think hw/misc/vfio/{pci.c,common.c,platform.c,etc}  Thanks,
>
> What about hw/vfio/ then?

Agreed.

Paolo

Re: [Qemu-devel] [RFC 1/2] hw/arm/virt: add a Calxeda XGMAC device

[Eric Auger]

Hi Andreas

> If the realize/init function sets up MMIO mappings and IRQs itself
> (rather than expecting the code instantiating the device to do so) then
> you are free to set that field to false.

Thank you for the confirmation

> In this case that would be vbi->memmap[VIRT_ETHERNET].base in
> create_ethernet(), so I have doubts.

Indeed the intent is to move the hardcoded
address currently set in virt.c into something more dynamic in vfio.c' realize.

Best Regards

Eric

On 5 March 2014 12:25, Andreas Färber <afaerber@suse.de> wrote:
> Hi,
>
> Am 05.03.2014 05:27, schrieb Eric Auger:
>> Hi Kim,
>>
>> - Parameter 'driver' expects pluggable device type
>> this can be fixed setting dc->cannot_instantiate_with_device_add_yet to
>> false in vfio_platform_dev_class_init. You do not get the error anymore.
>> However I must aknowledge I have not studies all possible side effects
>> of this setting and maybe there is a cleaner way to achieve the same
>> result.
>
> If the realize/init function sets up MMIO mappings and IRQs itself
> (rather than expecting the code instantiating the device to do so) then
> you are free to set that field to false.
>
> In this case that would be vbi->memmap[VIRT_ETHERNET].base in
> create_ethernet(), so I have doubts.



>
> Regards,
> Andreas
>
>> Then you can add additional properties in dc->props.
>> - location of the device in gpa space and impact on RAM size: my
>> understanding is that device could sit in another location than the
>> hpa's one and you are not compelled to use the same address.
>> - we effectively need to work on fdt tree generation for VFIO devices
>>
>> Best Regards
>>
>> Eric
>>
>>
>> On 26 February 2014 03:37, Kim Phillips <kim.phillips@linaro.org
>> <mailto:kim.phillips@linaro.org>> wrote:
>>
>>     This is a hack and only serves as an example of what needs to be
>>     done to make the next RFC - add vfio-platform support - work
>>     for development purposes on a Calxeda Midway system.  We don't want
>>     mach-virt to always create this ethernet device - DO NOT APPLY, etc.
>>
>>     Initial attempts to convince QEMU to create a memory mapped device
>>     on the command line (e.g., -device vfio-platform,name=fff51000.ethernet)
>>     would fail with "Parameter 'driver' expects pluggable device type".
>>     Any guidance as to how to overcome this apparent design limitation
>>     is welcome.
>>
>>     RAM is reduced from 30 to 1GiB such as to not overlap the xgmac device's
>>     physical address.  Not sure if the 30GiB RAM (or whatever the user sets
>>     it to with -m) could be set up above 0x1_0000_0000, but there is
>>     probably
>>     extra work needed to resolve this type of conflict.
>>
>>     note: vfio-platform interrupt support development may want interrupt
>>     property data filled; here it's omitted for the time being.
>>
>>     Not-signed-off-by: Kim Phillips <kim.phillips@linaro.org
>>     <mailto:kim.phillips@linaro.org>>
>>     ---
>>      hw/arm/virt.c | 24 +++++++++++++++++++++++-
>>      1 file changed, 23 insertions(+), 1 deletion(-)
>>
>>     diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>     index 517f2fe..75edf33 100644
>>     --- a/hw/arm/virt.c
>>     +++ b/hw/arm/virt.c
>>     @@ -65,6 +65,7 @@ enum {
>>          VIRT_GIC_CPU,
>>          VIRT_UART,
>>          VIRT_MMIO,
>>     +    VIRT_ETHERNET,
>>      };
>>
>>      typedef struct MemMapEntry {
>>     @@ -106,7 +107,8 @@ static const MemMapEntry a15memmap[] = {
>>          [VIRT_MMIO] = { 0xa000000, 0x200 },
>>          /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of
>>     that size */
>>          /* 0x10000000 .. 0x40000000 reserved for PCI */
>>     -    [VIRT_MEM] = { 0x40000000, 30ULL * 1024 * 1024 * 1024 },
>>     +    [VIRT_MEM] = { 0x40000000, 1ULL * 1024 * 1024 * 1024 },
>>     +    [VIRT_ETHERNET] = { 0xfff51000, 0x1000 },
>>      };
>>
>>      static const int a15irqmap[] = {
>>     @@ -291,6 +293,25 @@ static void create_uart(const VirtBoardInfo
>>     *vbi, qemu_irq *pic)
>>          g_free(nodename);
>>      }
>>
>>     +static void create_ethernet(const VirtBoardInfo *vbi, qemu_irq *pic)
>>     +{
>>     +    char *nodename;
>>     +    hwaddr base = vbi->memmap[VIRT_ETHERNET].base;
>>     +    hwaddr size = vbi->memmap[VIRT_ETHERNET].size;
>>     +    const char compat[] = "calxeda,hb-xgmac";
>>     +
>>     +    sysbus_create_simple("vfio-platform", base, NULL);
>>     +
>>     +    nodename = g_strdup_printf("/ethernet@%" PRIx64, base);
>>     +    qemu_fdt_add_subnode(vbi->fdt, nodename);
>>     +
>>     +    /* Note that we can't use setprop_string because of the
>>     embedded NUL */
>>     +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat,
>>     sizeof(compat));
>>     +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg", 2,
>>     base, 2, size);
>>     +
>>     +    g_free(nodename);
>>     +}
>>     +
>>      static void create_virtio_devices(const VirtBoardInfo *vbi,
>>     qemu_irq *pic)
>>      {
>>          int i;
>>     @@ -419,6 +440,7 @@ static void machvirt_init(QEMUMachineInitArgs *args)
>>          }
>>
>>          create_uart(vbi, pic);
>>     +    create_ethernet(vbi, pic);
>>
>>          /* Create mmio transports, so the user can create virtio backends
>>           * (which will be automatically plugged in to the transports). If
>>     --
>>     1.9.0
>>
>>
>
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg