[Qemu-devel] [PATCH v3 0/8] Add virtio-mmio and use it in vexpress

[Qemu-devel] [PATCH v3 0/8] Add virtio-mmio and use it in vexpress

[Peter Maydell]

This patch series adds an implementation of the virtio-mmio
transport, and uses it in the vexpress-a9 and vexpress-a15
board models.

The basic idea is that the board instantiates some transports,
the user can create backends which automatically plug into them
(via -device virtio-blk-backend and the like), and we tell the
guest kernel about them using a new arm/boot hook that lets
the board modify the user's device tree blob.

Changes v1->v2:
 * move the 'is guest provided queue size too large?' check
   from virtio-mmio into the core virtio 'set_num' function
 * rebase (including adding owner parameter to memory region
   init call in virtio-mmio.c)
Changes v2->v3:
 * drop the overcomplicated error return encoding from
   qemu_devtree_setprop_sized_cells() and just catch
   "user specified ram size too big for dtb" in boot.c

Peter Maydell (8):
  device_tree: Add qemu_devtree_setprop_sized_cells() utility functions
  arm/boot: Use qemu_devtree_setprop_sized_cells()
  virtio: Add support for guest setting of queue size
  virtio: Support transports which can specify the vring alignment
  virtio: Implement MMIO based virtio transport
  arm/boot: Allow boards to modify the FDT blob
  vexpress: Make VEDBoardInfo extend arm_boot_info
  vexpress: Add virtio-mmio transports

 device_tree.c                  |   33 ++++
 hw/arm/boot.c                  |   34 ++--
 hw/arm/vexpress.c              |  128 ++++++++++--
 hw/virtio/Makefile.objs        |    1 +
 hw/virtio/virtio-mmio.c        |  421 ++++++++++++++++++++++++++++++++++++++++
 hw/virtio/virtio.c             |   40 +++-
 include/hw/arm/arm.h           |    4 +
 include/hw/virtio/virtio-bus.h |    6 +
 include/hw/virtio/virtio.h     |    2 +
 include/sysemu/device_tree.h   |   59 ++++++
 10 files changed, 689 insertions(+), 39 deletions(-)
 create mode 100644 hw/virtio/virtio-mmio.c

-- 
1.7.9.5

[Qemu-devel] [PATCH v3 1/8] device_tree: Add qemu_devtree_setprop_sized_cells() utility functions

[Peter Maydell]

We already have a qemu_devtree_setprop_cells() which sets a dtb
property to an array of cells whose values are specified by varargs.
However for the fairly common case of setting a property to a list
of addresses or of address,size pairs the number of cells used by
each element in the list depends on the parent's #address-cells
and #size-cells properties. To make this easier we provide an analogous
qemu_devtree_setprop_sized_cells() macro which allows the number
of cells used by each element to be specified. This is implemented
using an underlying qemu_devtree_setprop_sized_cells_from_array()
function which takes the values and sizes as an array; this may
also be directly useful for cases where the cell contents are
constructed programmatically.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 device_tree.c                |   33 +++++++++++++++++++++++
 include/sysemu/device_tree.h |   59 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 92 insertions(+)

diff --git a/device_tree.c b/device_tree.c
index 10cf3d0..ffec99a 100644
--- a/device_tree.c
+++ b/device_tree.c
@@ -308,3 +308,36 @@ void qemu_devtree_dumpdtb(void *fdt, int size)
         exit(g_file_set_contents(dumpdtb, fdt, size, NULL) ? 0 : 1);
     }
 }
+
+int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
+                                                const char *node_path,
+                                                const char *property,
+                                                int numvalues,
+                                                uint64_t *values)
+{
+    uint32_t *propcells;
+    uint64_t value;
+    int cellnum, vnum, ncells;
+    uint32_t hival;
+
+    propcells = g_new0(uint32_t, numvalues * 2);
+
+    cellnum = 0;
+    for (vnum = 0; vnum < numvalues; vnum++) {
+        ncells = values[vnum * 2];
+        if (ncells != 1 && ncells != 2) {
+            return -1;
+        }
+        value = values[vnum * 2 + 1];
+        hival = cpu_to_be32(value >> 32);
+        if (ncells > 1) {
+            propcells[cellnum++] = hival;
+        } else if (hival != 0) {
+            return -1;
+        }
+        propcells[cellnum++] = cpu_to_be32(value);
+    }
+
+    return qemu_devtree_setprop(fdt, node_path, property, propcells,
+                                cellnum * sizeof(uint32_t));
+}
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index f0b3f35..2b58baf 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -51,4 +51,63 @@ int qemu_devtree_add_subnode(void *fdt, const char *name);
 
 void qemu_devtree_dumpdtb(void *fdt, int size);
 
+/**
+ * qemu_devtree_setprop_sized_cells_from_array:
+ * @fdt: device tree blob
+ * @node_path: node to set property on
+ * @property: property to set
+ * @numvalues: number of values
+ * @values: array of number-of-cells, value pairs
+ *
+ * Set the specified property on the specified node in the device tree
+ * to be an array of cells. The values of the cells are specified via
+ * the values list, which alternates between "number of cells used by
+ * this value" and "value".
+ * number-of-cells must be either 1 or 2 (other values will result in
+ * an error being returned). If a value is too large to fit in the
+ * number of cells specified for it, an error is returned.
+ *
+ * This function is useful because device tree nodes often have cell arrays
+ * which are either lists of addresses or lists of address,size tuples, but
+ * the number of cells used for each element vary depending on the
+ * #address-cells and #size-cells properties of their parent node.
+ * If you know all your cell elements are one cell wide you can use the
+ * simpler qemu_devtree_setprop_cells(). If you're not setting up the
+ * array programmatically, qemu_devtree_setprop_sized_cells may be more
+ * convenient.
+ *
+ * Return value: 0 on success, <0 on error.
+ */
+int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
+                                                const char *node_path,
+                                                const char *property,
+                                                int numvalues,
+                                                uint64_t *values);
+
+/**
+ * qemu_devtree_setprop_sized_cells:
+ * @fdt: device tree blob
+ * @node_path: node to set property on
+ * @property: property to set
+ * @...: list of number-of-cells, value pairs
+ *
+ * Set the specified property on the specified node in the device tree
+ * to be an array of cells. The values of the cells are specified via
+ * the variable arguments, which alternates between "number of cells
+ * used by this value" and "value".
+ *
+ * This is a convenience wrapper for the function
+ * qemu_devtree_setprop_sized_cells_from_array().
+ *
+ * Return value: 0 on success, <0 on error.
+ */
+#define qemu_devtree_setprop_sized_cells(fdt, node_path, property, ...)       \
+    ({                                                                        \
+        uint64_t qdt_tmp[] = { __VA_ARGS__ };                                 \
+        qemu_devtree_setprop_sized_cells_from_array(fdt, node_path,           \
+                                                    property,                 \
+                                                    ARRAY_SIZE(qdt_tmp) / 2,  \
+                                                    qdt_tmp);                 \
+    })
+
 #endif /* __DEVICE_TREE_H__ */
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v3 1/8] device_tree: Add qemu_devtree_setprop_sized_cells() utility functions

[Peter Crosthwaite]

Hi Peter,

On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> We already have a qemu_devtree_setprop_cells() which sets a dtb
> property to an array of cells whose values are specified by varargs.
> However for the fairly common case of setting a property to a list
> of addresses or of address,size pairs the number of cells used by
> each element in the list depends on the parent's #address-cells
> and #size-cells properties. To make this easier we provide an analogous
> qemu_devtree_setprop_sized_cells() macro which allows the number
> of cells used by each element to be specified. This is implemented
> using an underlying qemu_devtree_setprop_sized_cells_from_array()
> function which takes the values and sizes as an array; this may
> also be directly useful for cases where the cell contents are
> constructed programmatically.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> ---
>  device_tree.c                |   33 +++++++++++++++++++++++
>  include/sysemu/device_tree.h |   59 ++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 92 insertions(+)
>
> diff --git a/device_tree.c b/device_tree.c
> index 10cf3d0..ffec99a 100644
> --- a/device_tree.c
> +++ b/device_tree.c
> @@ -308,3 +308,36 @@ void qemu_devtree_dumpdtb(void *fdt, int size)
>          exit(g_file_set_contents(dumpdtb, fdt, size, NULL) ? 0 : 1);
>      }
>  }
> +
> +int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
> +                                                const char *node_path,
> +                                                const char *property,
> +                                                int numvalues,
> +                                                uint64_t *values)
> +{
> +    uint32_t *propcells;
> +    uint64_t value;
> +    int cellnum, vnum, ncells;
> +    uint32_t hival;

ncells, hival and value do not persist across iterations of the for loop ...

> +
> +    propcells = g_new0(uint32_t, numvalues * 2);
> +
> +    cellnum = 0;
> +    for (vnum = 0; vnum < numvalues; vnum++) {

... so you could define them here instead for localisation.

Regards,
Peter

> +        ncells = values[vnum * 2];
> +        if (ncells != 1 && ncells != 2) {
> +            return -1;
> +        }
> +        value = values[vnum * 2 + 1];
> +        hival = cpu_to_be32(value >> 32);
> +        if (ncells > 1) {
> +            propcells[cellnum++] = hival;
> +        } else if (hival != 0) {
> +            return -1;
> +        }
> +        propcells[cellnum++] = cpu_to_be32(value);
> +    }
> +
> +    return qemu_devtree_setprop(fdt, node_path, property, propcells,
> +                                cellnum * sizeof(uint32_t));
> +}
> diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
> index f0b3f35..2b58baf 100644
> --- a/include/sysemu/device_tree.h
> +++ b/include/sysemu/device_tree.h
> @@ -51,4 +51,63 @@ int qemu_devtree_add_subnode(void *fdt, const char *name);
>
>  void qemu_devtree_dumpdtb(void *fdt, int size);
>
> +/**
> + * qemu_devtree_setprop_sized_cells_from_array:
> + * @fdt: device tree blob
> + * @node_path: node to set property on
> + * @property: property to set
> + * @numvalues: number of values
> + * @values: array of number-of-cells, value pairs
> + *
> + * Set the specified property on the specified node in the device tree
> + * to be an array of cells. The values of the cells are specified via
> + * the values list, which alternates between "number of cells used by
> + * this value" and "value".
> + * number-of-cells must be either 1 or 2 (other values will result in
> + * an error being returned). If a value is too large to fit in the
> + * number of cells specified for it, an error is returned.
> + *
> + * This function is useful because device tree nodes often have cell arrays
> + * which are either lists of addresses or lists of address,size tuples, but
> + * the number of cells used for each element vary depending on the
> + * #address-cells and #size-cells properties of their parent node.
> + * If you know all your cell elements are one cell wide you can use the
> + * simpler qemu_devtree_setprop_cells(). If you're not setting up the
> + * array programmatically, qemu_devtree_setprop_sized_cells may be more
> + * convenient.
> + *
> + * Return value: 0 on success, <0 on error.
> + */
> +int qemu_devtree_setprop_sized_cells_from_array(void *fdt,
> +                                                const char *node_path,
> +                                                const char *property,
> +                                                int numvalues,
> +                                                uint64_t *values);
> +
> +/**
> + * qemu_devtree_setprop_sized_cells:
> + * @fdt: device tree blob
> + * @node_path: node to set property on
> + * @property: property to set
> + * @...: list of number-of-cells, value pairs
> + *
> + * Set the specified property on the specified node in the device tree
> + * to be an array of cells. The values of the cells are specified via
> + * the variable arguments, which alternates between "number of cells
> + * used by this value" and "value".
> + *
> + * This is a convenience wrapper for the function
> + * qemu_devtree_setprop_sized_cells_from_array().
> + *
> + * Return value: 0 on success, <0 on error.
> + */
> +#define qemu_devtree_setprop_sized_cells(fdt, node_path, property, ...)       \
> +    ({                                                                        \
> +        uint64_t qdt_tmp[] = { __VA_ARGS__ };                                 \
> +        qemu_devtree_setprop_sized_cells_from_array(fdt, node_path,           \
> +                                                    property,                 \
> +                                                    ARRAY_SIZE(qdt_tmp) / 2,  \
> +                                                    qdt_tmp);                 \
> +    })
> +
>  #endif /* __DEVICE_TREE_H__ */
> --
> 1.7.9.5
>
>

[Qemu-devel] [PATCH v3 2/8] arm/boot: Use qemu_devtree_setprop_sized_cells()

[Peter Maydell]

Replace the opencoded assembly of the reg property array for the
/memory node with a call to qemu_devtree_setprop_sized_cells().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c |   29 ++++++++---------------------
 1 file changed, 8 insertions(+), 21 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index a2e4032..1780316 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -227,12 +227,10 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
 
 static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
 {
-    uint32_t *mem_reg_property;
-    uint32_t mem_reg_propsize;
     void *fdt = NULL;
     char *filename;
     int size, rc;
-    uint32_t acells, scells, hival;
+    uint32_t acells, scells;
 
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
     if (!filename) {
@@ -255,29 +253,18 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
         goto fail;
     }
 
-    mem_reg_propsize = acells + scells;
-    mem_reg_property = g_new0(uint32_t, mem_reg_propsize);
-    mem_reg_property[acells - 1] = cpu_to_be32(binfo->loader_start);
-    hival = cpu_to_be32(binfo->loader_start >> 32);
-    if (acells > 1) {
-        mem_reg_property[acells - 2] = hival;
-    } else if (hival != 0) {
-        fprintf(stderr, "qemu: dtb file not compatible with "
-                "RAM start address > 4GB\n");
-        goto fail;
-    }
-    mem_reg_property[acells + scells - 1] = cpu_to_be32(binfo->ram_size);
-    hival = cpu_to_be32(binfo->ram_size >> 32);
-    if (scells > 1) {
-        mem_reg_property[acells + scells - 2] = hival;
-    } else if (hival != 0) {
+    if (scells < 2 && binfo->ram_size >= (1ULL << 32)) {
+        /* This is user error so deserves a friendlier error message
+         * than the failure of setprop_sized_cells would provide
+         */
         fprintf(stderr, "qemu: dtb file not compatible with "
                 "RAM size > 4GB\n");
         goto fail;
     }
 
-    rc = qemu_devtree_setprop(fdt, "/memory", "reg", mem_reg_property,
-                              mem_reg_propsize * sizeof(uint32_t));
+    rc = qemu_devtree_setprop_sized_cells(fdt, "/memory", "reg",
+                                          acells, binfo->loader_start,
+                                          scells, binfo->ram_size);
     if (rc < 0) {
         fprintf(stderr, "couldn't set /memory/reg\n");
         goto fail;
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH v3 2/8] arm/boot: Use qemu_devtree_setprop_sized_cells()

[Peter Crosthwaite]

Hi Peter,

On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Replace the opencoded assembly of the reg property array for the
> /memory node with a call to qemu_devtree_setprop_sized_cells().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/boot.c |   29 ++++++++---------------------
>  1 file changed, 8 insertions(+), 21 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index a2e4032..1780316 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -227,12 +227,10 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
>
>  static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>  {
> -    uint32_t *mem_reg_property;
> -    uint32_t mem_reg_propsize;
>      void *fdt = NULL;
>      char *filename;
>      int size, rc;
> -    uint32_t acells, scells, hival;
> +    uint32_t acells, scells;
>
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
>      if (!filename) {
> @@ -255,29 +253,18 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>          goto fail;
>      }
>
> -    mem_reg_propsize = acells + scells;
> -    mem_reg_property = g_new0(uint32_t, mem_reg_propsize);
> -    mem_reg_property[acells - 1] = cpu_to_be32(binfo->loader_start);
> -    hival = cpu_to_be32(binfo->loader_start >> 32);
> -    if (acells > 1) {
> -        mem_reg_property[acells - 2] = hival;
> -    } else if (hival != 0) {
> -        fprintf(stderr, "qemu: dtb file not compatible with "
> -                "RAM start address > 4GB\n");
> -        goto fail;
> -    }

So it confused me for a while as to why this check is deleted (and not
converted), but I'm guessing it is because binfo->loader_start is a
hwaddr which is probably 32 bit? Which I guess would cause a check
equivalent to the one below to werror. Is it possible in an arm build
for hwaddr to be 64 bit and if so should this check be converted?

Regards,
Peter

Re: [Qemu-devel] [PATCH v3 2/8] arm/boot: Use qemu_devtree_setprop_sized_cells()

[Peter Maydell]

On 16 July 2013 15:31, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
> Hi Peter,
>
> On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> Replace the opencoded assembly of the reg property array for the
>> /memory node with a call to qemu_devtree_setprop_sized_cells().
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/arm/boot.c |   29 ++++++++---------------------
>>  1 file changed, 8 insertions(+), 21 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index a2e4032..1780316 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -227,12 +227,10 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
>>
>>  static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>>  {
>> -    uint32_t *mem_reg_property;
>> -    uint32_t mem_reg_propsize;
>>      void *fdt = NULL;
>>      char *filename;
>>      int size, rc;
>> -    uint32_t acells, scells, hival;
>> +    uint32_t acells, scells;
>>
>>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
>>      if (!filename) {
>> @@ -255,29 +253,18 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>>          goto fail;
>>      }
>>
>> -    mem_reg_propsize = acells + scells;
>> -    mem_reg_property = g_new0(uint32_t, mem_reg_propsize);
>> -    mem_reg_property[acells - 1] = cpu_to_be32(binfo->loader_start);
>> -    hival = cpu_to_be32(binfo->loader_start >> 32);
>> -    if (acells > 1) {
>> -        mem_reg_property[acells - 2] = hival;
>> -    } else if (hival != 0) {
>> -        fprintf(stderr, "qemu: dtb file not compatible with "
>> -                "RAM start address > 4GB\n");
>> -        goto fail;
>> -    }
>
> So it confused me for a while as to why this check is deleted (and not
> converted), but I'm guessing it is because binfo->loader_start is a
> hwaddr which is probably 32 bit? Which I guess would cause a check
> equivalent to the one below to werror. Is it possible in an arm build
> for hwaddr to be 64 bit and if so should this check be converted?

It's because the "ram start address won't fit" is basically a
bug in the implementation of a QEMU machine -- it will basically
only fire for a board which put its RAM all beyond the 4GB
boundary (vanishingly unlikely as it would be a really stupid
bit of h/w design) *and* where the user had a DTB with a one-cell
address size field. So I'm happy to have that fail via the
call to set_sized_cells() failing, without calling it out as
a specific error message.

(hwaddr are always 64 bits for everybody now.)

-- PMM

Re: [Qemu-devel] [PATCH v3 2/8] arm/boot: Use qemu_devtree_setprop_sized_cells()

[Peter Crosthwaite]

Hi Peter,

On Wed, Jul 17, 2013 at 12:43 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 16 July 2013 15:31, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote:
>> Hi Peter,
>>
>> On Tue, Jul 16, 2013 at 10:25 PM, Peter Maydell
>> <peter.maydell@linaro.org> wrote:
>>> Replace the opencoded assembly of the reg property array for the
>>> /memory node with a call to qemu_devtree_setprop_sized_cells().
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>  hw/arm/boot.c |   29 ++++++++---------------------
>>>  1 file changed, 8 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>>> index a2e4032..1780316 100644
>>> --- a/hw/arm/boot.c
>>> +++ b/hw/arm/boot.c
>>> @@ -227,12 +227,10 @@ static void set_kernel_args_old(const struct arm_boot_info *info)
>>>
>>>  static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>>>  {
>>> -    uint32_t *mem_reg_property;
>>> -    uint32_t mem_reg_propsize;
>>>      void *fdt = NULL;
>>>      char *filename;
>>>      int size, rc;
>>> -    uint32_t acells, scells, hival;
>>> +    uint32_t acells, scells;
>>>
>>>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, binfo->dtb_filename);
>>>      if (!filename) {
>>> @@ -255,29 +253,18 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
>>>          goto fail;
>>>      }
>>>
>>> -    mem_reg_propsize = acells + scells;
>>> -    mem_reg_property = g_new0(uint32_t, mem_reg_propsize);
>>> -    mem_reg_property[acells - 1] = cpu_to_be32(binfo->loader_start);
>>> -    hival = cpu_to_be32(binfo->loader_start >> 32);
>>> -    if (acells > 1) {
>>> -        mem_reg_property[acells - 2] = hival;
>>> -    } else if (hival != 0) {
>>> -        fprintf(stderr, "qemu: dtb file not compatible with "
>>> -                "RAM start address > 4GB\n");
>>> -        goto fail;
>>> -    }
>>
>> So it confused me for a while as to why this check is deleted (and not
>> converted), but I'm guessing it is because binfo->loader_start is a
>> hwaddr which is probably 32 bit? Which I guess would cause a check
>> equivalent to the one below to werror. Is it possible in an arm build
>> for hwaddr to be 64 bit and if so should this check be converted?
>
> It's because the "ram start address won't fit" is basically a
> bug in the implementation of a QEMU machine -- it will basically
> only fire for a board which put its RAM all beyond the 4GB
> boundary (vanishingly unlikely as it would be a really stupid
> bit of h/w design) *and* where the user had a DTB with a one-cell
> address size field. So I'm happy to have that fail via the
> call to set_sized_cells() failing, without calling it out as
> a specific error message.
>

Makes sense. If you respin maybe its worth a mention in commit message
as its more that just a conversion of existing behaviour? But

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

Regards,
Peter

> (hwaddr are always 64 bits for everybody now.)
>
> -- PMM
>

[Qemu-devel] [PATCH v3 3/8] virtio: Add support for guest setting of queue size

[Peter Maydell]

The MMIO virtio transport spec allows the guest to tell the host how
large the queue size is. Add virtio_queue_set_num() function which
implements this in the QEMU common virtio support code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio.c         |    8 ++++++++
 include/hw/virtio/virtio.h |    1 +
 2 files changed, 9 insertions(+)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 8176c14..01b05f3 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -667,6 +667,14 @@ hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n)
     return vdev->vq[n].pa;
 }
 
+void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
+{
+    if (num <= VIRTQUEUE_MAX_SIZE) {
+        vdev->vq[n].vring.num = num;
+        virtqueue_init(&vdev->vq[n]);
+    }
+}
+
 int virtio_queue_get_num(VirtIODevice *vdev, int n)
 {
     return vdev->vq[n].vring.num;
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index a6c5c53..95c4772 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -198,6 +198,7 @@ void virtio_config_writew(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_config_writel(VirtIODevice *vdev, uint32_t addr, uint32_t data);
 void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
+void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
 int virtio_queue_get_num(VirtIODevice *vdev, int n);
 void virtio_queue_notify(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 4/8] virtio: Support transports which can specify the vring alignment

[Peter Maydell]

Support virtio transports which can specify the vring alignment
(ie where the guest communicates this to the host) by providing
a new virtio_queue_set_align() function. (The default alignment
remains as before.)

Transports which wish to make use of this must set the
has_variable_vring_alignment field in their VirtioBusClass
struct to true; they can then change the alignment via
virtio_queue_set_align().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/virtio/virtio.c             |   32 +++++++++++++++++++++++++++++---
 include/hw/virtio/virtio-bus.h |    6 ++++++
 include/hw/virtio/virtio.h     |    1 +
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 01b05f3..09f62c6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -19,8 +19,11 @@
 #include "qemu/atomic.h"
 #include "hw/virtio/virtio-bus.h"
 
-/* The alignment to use between consumer and producer parts of vring.
- * x86 pagesize again. */
+/*
+ * The alignment to use between consumer and producer parts of vring.
+ * x86 pagesize again. This is the default, used by transports like PCI
+ * which don't provide a means for the guest to tell the host the alignment.
+ */
 #define VIRTIO_PCI_VRING_ALIGN         4096
 
 typedef struct VRingDesc
@@ -54,6 +57,7 @@ typedef struct VRingUsed
 typedef struct VRing
 {
     unsigned int num;
+    unsigned int align;
     hwaddr desc;
     hwaddr avail;
     hwaddr used;
@@ -93,7 +97,7 @@ static void virtqueue_init(VirtQueue *vq)
     vq->vring.avail = pa + vq->vring.num * sizeof(VRingDesc);
     vq->vring.used = vring_align(vq->vring.avail +
                                  offsetof(VRingAvail, ring[vq->vring.num]),
-                                 VIRTIO_PCI_VRING_ALIGN);
+                                 vq->vring.align);
 }
 
 static inline uint64_t vring_desc_addr(hwaddr desc_pa, int i)
@@ -687,6 +691,21 @@ int virtio_queue_get_id(VirtQueue *vq)
     return vq - &vdev->vq[0];
 }
 
+void virtio_queue_set_align(VirtIODevice *vdev, int n, int align)
+{
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    /* Check that the transport told us it was going to do this
+     * (so a buggy transport will immediately assert rather than
+     * silently failing to migrate this state)
+     */
+    assert(k->has_variable_vring_alignment);
+
+    vdev->vq[n].vring.align = align;
+    virtqueue_init(&vdev->vq[n]);
+}
+
 void virtio_queue_notify_vq(VirtQueue *vq)
 {
     if (vq->vring.desc) {
@@ -727,6 +746,7 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
         abort();
 
     vdev->vq[i].vring.num = queue_size;
+    vdev->vq[i].vring.align = VIRTIO_PCI_VRING_ALIGN;
     vdev->vq[i].handle_output = handle_output;
 
     return &vdev->vq[i];
@@ -833,6 +853,9 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
             break;
 
         qemu_put_be32(f, vdev->vq[i].vring.num);
+        if (k->has_variable_vring_alignment) {
+            qemu_put_be32(f, vdev->vq[i].vring.align);
+        }
         qemu_put_be64(f, vdev->vq[i].pa);
         qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
         if (k->save_queue) {
@@ -889,6 +912,9 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
 
     for (i = 0; i < num; i++) {
         vdev->vq[i].vring.num = qemu_get_be32(f);
+        if (k->has_variable_vring_alignment) {
+            vdev->vq[i].vring.align = qemu_get_be32(f);
+        }
         vdev->vq[i].pa = qemu_get_be64(f);
         qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
         vdev->vq[i].signalled_used_valid = false;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 9ed60f9..9217f85 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -62,6 +62,12 @@ typedef struct VirtioBusClass {
      * This is called by virtio-bus just before the device is unplugged.
      */
     void (*device_unplug)(DeviceState *d);
+    /*
+     * Does the transport have variable vring alignment?
+     * (ie can it ever call virtio_queue_set_align()?)
+     * Note that changing this will break migration for this transport.
+     */
+    bool has_variable_vring_alignment;
 } VirtioBusClass;
 
 struct VirtioBusState {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 95c4772..6c2cee6 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -200,6 +200,7 @@ void virtio_queue_set_addr(VirtIODevice *vdev, int n, hwaddr addr);
 hwaddr virtio_queue_get_addr(VirtIODevice *vdev, int n);
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num);
 int virtio_queue_get_num(VirtIODevice *vdev, int n);
+void virtio_queue_set_align(VirtIODevice *vdev, int n, int align);
 void virtio_queue_notify(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
 void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 5/8] virtio: Implement MMIO based virtio transport

[Peter Maydell]

Add support for the generic MMIO based virtio transport.

This patch includes some fixes for bugs spotted by
Ying-Shiuan Pan <yspan@itri.org.tw>.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
[Fred changes: updated to new virtio-bus mechanisms]
Signed-off-by: KONRAD Frederic <fred.konrad@greensocs.com>
[PMM changes:
 * fixed trivial makefile conflict
 * removed unused int_enable
 * host_features doesn't need migrating
 * reset guest accessible state in the reset function
 * minor style fixes like extra blank lines
 * RAZ/WI if there's no backend
 * made transport size 0x200, in line with kvmtool
 * set has_variable_vring_alignment
]
---
 hw/virtio/Makefile.objs |    1 +
 hw/virtio/virtio-mmio.c |  421 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 422 insertions(+)
 create mode 100644 hw/virtio/virtio-mmio.c

diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs
index cbe6d51..1ba53d9 100644
--- a/hw/virtio/Makefile.objs
+++ b/hw/virtio/Makefile.objs
@@ -1,6 +1,7 @@
 common-obj-y += virtio-rng.o
 common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o
 common-obj-y += virtio-bus.o
+common-obj-y += virtio-mmio.o
 common-obj-$(CONFIG_VIRTIO_BLK_DATA_PLANE) += dataplane/
 
 obj-y += virtio.o virtio-balloon.o 
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
new file mode 100644
index 0000000..54d6679
--- /dev/null
+++ b/hw/virtio/virtio-mmio.c
@@ -0,0 +1,421 @@
+/*
+ * Virtio MMIO bindings
+ *
+ * Copyright (c) 2011 Linaro Limited
+ *
+ * Author:
+ *  Peter Maydell <peter.maydell@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "hw/sysbus.h"
+#include "hw/virtio/virtio.h"
+#include "qemu/host-utils.h"
+#include "hw/virtio/virtio-bus.h"
+
+/* #define DEBUG_VIRTIO_MMIO */
+
+#ifdef DEBUG_VIRTIO_MMIO
+
+#define DPRINTF(fmt, ...) \
+do { printf("virtio_mmio: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+/* QOM macros */
+/* virtio-mmio-bus */
+#define TYPE_VIRTIO_MMIO_BUS "virtio-mmio-bus"
+#define VIRTIO_MMIO_BUS(obj) \
+        OBJECT_CHECK(VirtioBusState, (obj), TYPE_VIRTIO_MMIO_BUS)
+#define VIRTIO_MMIO_BUS_GET_CLASS(obj) \
+        OBJECT_GET_CLASS(VirtioBusClass, (obj), TYPE_VIRTIO_MMIO_BUS)
+#define VIRTIO_MMIO_BUS_CLASS(klass) \
+        OBJECT_CLASS_CHECK(VirtioBusClass, (klass), TYPE_VIRTIO_MMIO_BUS)
+
+/* virtio-mmio */
+#define TYPE_VIRTIO_MMIO "virtio-mmio"
+#define VIRTIO_MMIO(obj) \
+        OBJECT_CHECK(VirtIOMMIOProxy, (obj), TYPE_VIRTIO_MMIO)
+
+/* Memory mapped register offsets */
+#define VIRTIO_MMIO_MAGIC 0x0
+#define VIRTIO_MMIO_VERSION 0x4
+#define VIRTIO_MMIO_DEVICEID 0x8
+#define VIRTIO_MMIO_VENDORID 0xc
+#define VIRTIO_MMIO_HOSTFEATURES 0x10
+#define VIRTIO_MMIO_HOSTFEATURESSEL 0x14
+#define VIRTIO_MMIO_GUESTFEATURES 0x20
+#define VIRTIO_MMIO_GUESTFEATURESSEL 0x24
+#define VIRTIO_MMIO_GUESTPAGESIZE 0x28
+#define VIRTIO_MMIO_QUEUESEL 0x30
+#define VIRTIO_MMIO_QUEUENUMMAX 0x34
+#define VIRTIO_MMIO_QUEUENUM 0x38
+#define VIRTIO_MMIO_QUEUEALIGN 0x3c
+#define VIRTIO_MMIO_QUEUEPFN 0x40
+#define VIRTIO_MMIO_QUEUENOTIFY 0x50
+#define VIRTIO_MMIO_INTERRUPTSTATUS 0x60
+#define VIRTIO_MMIO_INTERRUPTACK 0x64
+#define VIRTIO_MMIO_STATUS 0x70
+/* Device specific config space starts here */
+#define VIRTIO_MMIO_CONFIG 0x100
+
+#define VIRT_MAGIC 0x74726976 /* 'virt' */
+#define VIRT_VERSION 1
+#define VIRT_VENDOR 0x554D4551 /* 'QEMU' */
+
+typedef struct {
+    /* Generic */
+    SysBusDevice parent_obj;
+    MemoryRegion iomem;
+    qemu_irq irq;
+    uint32_t host_features;
+    /* Guest accessible state needing migration and reset */
+    uint32_t host_features_sel;
+    uint32_t guest_features_sel;
+    uint32_t guest_page_shift;
+    /* virtio-bus */
+    VirtioBusState bus;
+} VirtIOMMIOProxy;
+
+static void virtio_mmio_bus_new(VirtioBusState *bus, VirtIOMMIOProxy *dev);
+
+static uint64_t virtio_mmio_read(void *opaque, hwaddr offset, unsigned size)
+{
+    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
+    VirtIODevice *vdev = proxy->bus.vdev;
+
+    DPRINTF("virtio_mmio_read offset 0x%x\n", (int)offset);
+
+    if (!vdev) {
+        /* If no backend is present, we treat most registers as
+         * read-as-zero, except for the magic number, version and
+         * vendor ID. This is not strictly sanctioned by the virtio
+         * spec, but it allows us to provide transports with no backend
+         * plugged in which don't confuse Linux's virtio code: the
+         * probe won't complain about the bad magic number, but the
+         * device ID of zero means no backend will claim it.
+         */
+        switch (offset) {
+        case VIRTIO_MMIO_MAGIC:
+            return VIRT_MAGIC;
+        case VIRTIO_MMIO_VERSION:
+            return VIRT_VERSION;
+        case VIRTIO_MMIO_VENDORID:
+            return VIRT_VENDOR;
+        default:
+            return 0;
+        }
+    }
+
+    if (offset >= VIRTIO_MMIO_CONFIG) {
+        offset -= VIRTIO_MMIO_CONFIG;
+        switch (size) {
+        case 1:
+            return virtio_config_readb(vdev, offset);
+        case 2:
+            return virtio_config_readw(vdev, offset);
+        case 4:
+            return virtio_config_readl(vdev, offset);
+        default:
+            abort();
+        }
+    }
+    if (size != 4) {
+        DPRINTF("wrong size access to register!\n");
+        return 0;
+    }
+    switch (offset) {
+    case VIRTIO_MMIO_MAGIC:
+        return VIRT_MAGIC;
+    case VIRTIO_MMIO_VERSION:
+        return VIRT_VERSION;
+    case VIRTIO_MMIO_DEVICEID:
+        return vdev->device_id;
+    case VIRTIO_MMIO_VENDORID:
+        return VIRT_VENDOR;
+    case VIRTIO_MMIO_HOSTFEATURES:
+        if (proxy->host_features_sel) {
+            return 0;
+        }
+        return proxy->host_features;
+    case VIRTIO_MMIO_QUEUENUMMAX:
+        return VIRTQUEUE_MAX_SIZE;
+    case VIRTIO_MMIO_QUEUEPFN:
+        return virtio_queue_get_addr(vdev, vdev->queue_sel)
+            >> proxy->guest_page_shift;
+    case VIRTIO_MMIO_INTERRUPTSTATUS:
+        return vdev->isr;
+    case VIRTIO_MMIO_STATUS:
+        return vdev->status;
+    case VIRTIO_MMIO_HOSTFEATURESSEL:
+    case VIRTIO_MMIO_GUESTFEATURES:
+    case VIRTIO_MMIO_GUESTFEATURESSEL:
+    case VIRTIO_MMIO_GUESTPAGESIZE:
+    case VIRTIO_MMIO_QUEUESEL:
+    case VIRTIO_MMIO_QUEUENUM:
+    case VIRTIO_MMIO_QUEUEALIGN:
+    case VIRTIO_MMIO_QUEUENOTIFY:
+    case VIRTIO_MMIO_INTERRUPTACK:
+        DPRINTF("read of write-only register\n");
+        return 0;
+    default:
+        DPRINTF("bad register offset\n");
+        return 0;
+    }
+    return 0;
+}
+
+static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
+                              unsigned size)
+{
+    VirtIOMMIOProxy *proxy = (VirtIOMMIOProxy *)opaque;
+    VirtIODevice *vdev = proxy->bus.vdev;
+
+    DPRINTF("virtio_mmio_write offset 0x%x value 0x%" PRIx64 "\n",
+            (int)offset, value);
+
+    if (!vdev) {
+        /* If no backend is present, we just make all registers
+         * write-ignored. This allows us to provide transports with
+         * no backend plugged in.
+         */
+        return;
+    }
+
+    if (offset >= VIRTIO_MMIO_CONFIG) {
+        offset -= VIRTIO_MMIO_CONFIG;
+        switch (size) {
+        case 1:
+            virtio_config_writeb(vdev, offset, value);
+            break;
+        case 2:
+            virtio_config_writew(vdev, offset, value);
+            break;
+        case 4:
+            virtio_config_writel(vdev, offset, value);
+            break;
+        default:
+            abort();
+        }
+        return;
+    }
+    if (size != 4) {
+        DPRINTF("wrong size access to register!\n");
+        return;
+    }
+    switch (offset) {
+    case VIRTIO_MMIO_HOSTFEATURESSEL:
+        proxy->host_features_sel = value;
+        break;
+    case VIRTIO_MMIO_GUESTFEATURES:
+        if (!proxy->guest_features_sel) {
+            virtio_set_features(vdev, value);
+        }
+        break;
+    case VIRTIO_MMIO_GUESTFEATURESSEL:
+        proxy->guest_features_sel = value;
+        break;
+    case VIRTIO_MMIO_GUESTPAGESIZE:
+        proxy->guest_page_shift = ctz32(value);
+        if (proxy->guest_page_shift > 31) {
+            proxy->guest_page_shift = 0;
+        }
+        DPRINTF("guest page size %" PRIx64 " shift %d\n", value,
+                proxy->guest_page_shift);
+        break;
+    case VIRTIO_MMIO_QUEUESEL:
+        if (value < VIRTIO_PCI_QUEUE_MAX) {
+            vdev->queue_sel = value;
+        }
+        break;
+    case VIRTIO_MMIO_QUEUENUM:
+        DPRINTF("mmio_queue write %d max %d\n", (int)value, VIRTQUEUE_MAX_SIZE);
+        virtio_queue_set_num(vdev, vdev->queue_sel, value);
+        break;
+    case VIRTIO_MMIO_QUEUEALIGN:
+        virtio_queue_set_align(vdev, vdev->queue_sel, value);
+        break;
+    case VIRTIO_MMIO_QUEUEPFN:
+        if (value == 0) {
+            virtio_reset(vdev);
+        } else {
+            virtio_queue_set_addr(vdev, vdev->queue_sel,
+                                  value << proxy->guest_page_shift);
+        }
+        break;
+    case VIRTIO_MMIO_QUEUENOTIFY:
+        if (value < VIRTIO_PCI_QUEUE_MAX) {
+            virtio_queue_notify(vdev, value);
+        }
+        break;
+    case VIRTIO_MMIO_INTERRUPTACK:
+        vdev->isr &= ~value;
+        virtio_update_irq(vdev);
+        break;
+    case VIRTIO_MMIO_STATUS:
+        virtio_set_status(vdev, value & 0xff);
+        if (vdev->status == 0) {
+            virtio_reset(vdev);
+        }
+        break;
+    case VIRTIO_MMIO_MAGIC:
+    case VIRTIO_MMIO_VERSION:
+    case VIRTIO_MMIO_DEVICEID:
+    case VIRTIO_MMIO_VENDORID:
+    case VIRTIO_MMIO_HOSTFEATURES:
+    case VIRTIO_MMIO_QUEUENUMMAX:
+    case VIRTIO_MMIO_INTERRUPTSTATUS:
+        DPRINTF("write to readonly register\n");
+        break;
+
+    default:
+        DPRINTF("bad register offset\n");
+    }
+}
+
+static const MemoryRegionOps virtio_mem_ops = {
+    .read = virtio_mmio_read,
+    .write = virtio_mmio_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void virtio_mmio_update_irq(DeviceState *opaque, uint16_t vector)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+    int level;
+
+    if (!proxy->bus.vdev) {
+        return;
+    }
+    level = (proxy->bus.vdev->isr != 0);
+    DPRINTF("virtio_mmio setting IRQ %d\n", level);
+    qemu_set_irq(proxy->irq, level);
+}
+
+static unsigned int virtio_mmio_get_features(DeviceState *opaque)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    return proxy->host_features;
+}
+
+static int virtio_mmio_load_config(DeviceState *opaque, QEMUFile *f)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    proxy->host_features_sel = qemu_get_be32(f);
+    proxy->guest_features_sel = qemu_get_be32(f);
+    proxy->guest_page_shift = qemu_get_be32(f);
+    return 0;
+}
+
+static void virtio_mmio_save_config(DeviceState *opaque, QEMUFile *f)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    qemu_put_be32(f, proxy->host_features_sel);
+    qemu_put_be32(f, proxy->guest_features_sel);
+    qemu_put_be32(f, proxy->guest_page_shift);
+}
+
+static void virtio_mmio_reset(DeviceState *d)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+
+    virtio_bus_reset(&proxy->bus);
+    proxy->host_features_sel = 0;
+    proxy->guest_features_sel = 0;
+    proxy->guest_page_shift = 0;
+}
+
+/* virtio-mmio device */
+
+/* This is called by virtio-bus just after the device is plugged. */
+static void virtio_mmio_device_plugged(DeviceState *opaque)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(opaque);
+
+    proxy->host_features |= (0x1 << VIRTIO_F_NOTIFY_ON_EMPTY);
+    proxy->host_features = virtio_bus_get_vdev_features(&proxy->bus,
+                                                        proxy->host_features);
+}
+
+static void virtio_mmio_realizefn(DeviceState *d, Error **errp)
+{
+    VirtIOMMIOProxy *proxy = VIRTIO_MMIO(d);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(d);
+
+    virtio_mmio_bus_new(&proxy->bus, proxy);
+    sysbus_init_irq(sbd, &proxy->irq);
+    memory_region_init_io(&proxy->iomem, OBJECT(d), &virtio_mem_ops, proxy,
+                          TYPE_VIRTIO_MMIO, 0x200);
+    sysbus_init_mmio(sbd, &proxy->iomem);
+}
+
+static void virtio_mmio_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = virtio_mmio_realizefn;
+    dc->reset = virtio_mmio_reset;
+}
+
+static const TypeInfo virtio_mmio_info = {
+    .name          = TYPE_VIRTIO_MMIO,
+    .parent        = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(VirtIOMMIOProxy),
+    .class_init    = virtio_mmio_class_init,
+};
+
+/* virtio-mmio-bus. */
+
+static void virtio_mmio_bus_new(VirtioBusState *bus, VirtIOMMIOProxy *dev)
+{
+    DeviceState *qdev = DEVICE(dev);
+    BusState *qbus;
+
+    qbus_create_inplace((BusState *)bus, TYPE_VIRTIO_MMIO_BUS, qdev, NULL);
+    qbus = BUS(bus);
+    qbus->allow_hotplug = 0;
+}
+
+static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *bus_class = BUS_CLASS(klass);
+    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
+
+    k->notify = virtio_mmio_update_irq;
+    k->save_config = virtio_mmio_save_config;
+    k->load_config = virtio_mmio_load_config;
+    k->get_features = virtio_mmio_get_features;
+    k->device_plugged = virtio_mmio_device_plugged;
+    k->has_variable_vring_alignment = true;
+    bus_class->max_dev = 1;
+}
+
+static const TypeInfo virtio_mmio_bus_info = {
+    .name          = TYPE_VIRTIO_MMIO_BUS,
+    .parent        = TYPE_VIRTIO_BUS,
+    .instance_size = sizeof(VirtioBusState),
+    .class_init    = virtio_mmio_bus_class_init,
+};
+
+static void virtio_mmio_register_types(void)
+{
+    type_register_static(&virtio_mmio_bus_info);
+    type_register_static(&virtio_mmio_info);
+}
+
+type_init(virtio_mmio_register_types)
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 6/8] arm/boot: Allow boards to modify the FDT blob

[Peter Maydell]

Add a callback hook in arm_boot_info to allow board models to
modify the device tree blob if they need to. (The major expected
use case is to add virtio-mmio nodes for virtio-mmio transports
that exist in QEMU but not in the hardware.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/boot.c        |    5 +++++
 include/hw/arm/arm.h |    4 ++++
 2 files changed, 9 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 1780316..9fbe5d2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -294,6 +294,11 @@ static int load_dtb(hwaddr addr, const struct arm_boot_info *binfo)
             goto fail;
         }
     }
+
+    if (binfo->modify_dtb) {
+        binfo->modify_dtb(binfo, fdt);
+    }
+
     qemu_devtree_dumpdtb(fdt, size);
 
     cpu_physical_memory_write(addr, fdt, size);
diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h
index 7b2b02d..bae87c6 100644
--- a/include/hw/arm/arm.h
+++ b/include/hw/arm/arm.h
@@ -55,6 +55,10 @@ struct arm_boot_info {
                                  const struct arm_boot_info *info);
     void (*secondary_cpu_reset_hook)(ARMCPU *cpu,
                                      const struct arm_boot_info *info);
+    /* if a board needs to be able to modify a device tree provided by
+     * the user it should implement this hook.
+     */
+    void (*modify_dtb)(const struct arm_boot_info *info, void *fdt);
     /* Used internally by arm_boot.c */
     int is_linux;
     hwaddr initrd_start;
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 7/8] vexpress: Make VEDBoardInfo extend arm_boot_info

[Peter Maydell]

Make the VEDBoardInfo struct extend arm_boot_info; this will
allow us to get at the VEDBoardInfo information inside callbacks
from arm/boot code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/vexpress.c |   31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 7d1b823..0f15337 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -36,8 +36,6 @@
 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
 #define VEXPRESS_FLASH_SECT_SIZE (256 * 1024)
 
-static struct arm_boot_info vexpress_binfo;
-
 /* Address maps for peripherals:
  * the Versatile Express motherboard has two possible maps,
  * the "legacy" one (used for A9) and the "Cortex-A Series"
@@ -153,6 +151,7 @@ typedef void DBoardInitFn(const VEDBoardInfo *daughterboard,
                           qemu_irq *pic);
 
 struct VEDBoardInfo {
+    struct arm_boot_info bootinfo;
     const hwaddr *motherboard_map;
     hwaddr loader_start;
     const hwaddr gic_cpu_if_addr;
@@ -272,7 +271,7 @@ static const uint32_t a9_clocks[] = {
     66670000, /* Test chip reference clock: 66.67MHz */
 };
 
-static const VEDBoardInfo a9_daughterboard = {
+static VEDBoardInfo a9_daughterboard = {
     .motherboard_map = motherboard_legacy_map,
     .loader_start = 0x60000000,
     .gic_cpu_if_addr = 0x1e000100,
@@ -384,7 +383,7 @@ static const uint32_t a15_clocks[] = {
     40000000, /* OSCCLK8: 40MHz : DDR2 PLL reference */
 };
 
-static const VEDBoardInfo a15_daughterboard = {
+static VEDBoardInfo a15_daughterboard = {
     .motherboard_map = motherboard_aseries_map,
     .loader_start = 0x80000000,
     .gic_cpu_if_addr = 0x2c002000,
@@ -396,7 +395,7 @@ static const VEDBoardInfo a15_daughterboard = {
     .init = a15_daughterboard_init,
 };
 
-static void vexpress_common_init(const VEDBoardInfo *daughterboard,
+static void vexpress_common_init(VEDBoardInfo *daughterboard,
                                  QEMUMachineInitArgs *args)
 {
     DeviceState *dev, *sysctl, *pl041;
@@ -524,17 +523,17 @@ static void vexpress_common_init(const VEDBoardInfo *daughterboard,
 
     /* VE_DAPROM: not modelled */
 
-    vexpress_binfo.ram_size = args->ram_size;
-    vexpress_binfo.kernel_filename = args->kernel_filename;
-    vexpress_binfo.kernel_cmdline = args->kernel_cmdline;
-    vexpress_binfo.initrd_filename = args->initrd_filename;
-    vexpress_binfo.nb_cpus = smp_cpus;
-    vexpress_binfo.board_id = VEXPRESS_BOARD_ID;
-    vexpress_binfo.loader_start = daughterboard->loader_start;
-    vexpress_binfo.smp_loader_start = map[VE_SRAM];
-    vexpress_binfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
-    vexpress_binfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
-    arm_load_kernel(ARM_CPU(first_cpu), &vexpress_binfo);
+    daughterboard->bootinfo.ram_size = args->ram_size;
+    daughterboard->bootinfo.kernel_filename = args->kernel_filename;
+    daughterboard->bootinfo.kernel_cmdline = args->kernel_cmdline;
+    daughterboard->bootinfo.initrd_filename = args->initrd_filename;
+    daughterboard->bootinfo.nb_cpus = smp_cpus;
+    daughterboard->bootinfo.board_id = VEXPRESS_BOARD_ID;
+    daughterboard->bootinfo.loader_start = daughterboard->loader_start;
+    daughterboard->bootinfo.smp_loader_start = map[VE_SRAM];
+    daughterboard->bootinfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
+    daughterboard->bootinfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
+    arm_load_kernel(ARM_CPU(first_cpu), &daughterboard->bootinfo);
 }
 
 static void vexpress_a9_init(QEMUMachineInitArgs *args)
-- 
1.7.9.5

[Qemu-devel] [PATCH v3 8/8] vexpress: Add virtio-mmio transports

[Peter Maydell]

Add some virtio-mmio transports to the vexpress board model,
together with a modify_dtb hook which adds them to the device
tree so that the kernel will probe for them. We put them
in a reserved area of the address map.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/vexpress.c |   99 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)

diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
index 0f15337..9586e38 100644
--- a/hw/arm/vexpress.c
+++ b/hw/arm/vexpress.c
@@ -31,11 +31,18 @@
 #include "exec/address-spaces.h"
 #include "sysemu/blockdev.h"
 #include "hw/block/flash.h"
+#include "sysemu/device_tree.h"
+#include <libfdt.h>
 
 #define VEXPRESS_BOARD_ID 0x8e0
 #define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
 #define VEXPRESS_FLASH_SECT_SIZE (256 * 1024)
 
+/* Number of virtio transports to create (0..8; limited by
+ * number of available IRQ lines).
+ */
+#define NUM_VIRTIO_TRANSPORTS 4
+
 /* Address maps for peripherals:
  * the Versatile Express motherboard has two possible maps,
  * the "legacy" one (used for A9) and the "Cortex-A Series"
@@ -71,6 +78,7 @@ enum {
     VE_ETHERNET,
     VE_USB,
     VE_DAPROM,
+    VE_VIRTIO,
 };
 
 static hwaddr motherboard_legacy_map[] = {
@@ -89,6 +97,7 @@ static hwaddr motherboard_legacy_map[] = {
     [VE_WDT] = 0x1000f000,
     [VE_TIMER01] = 0x10011000,
     [VE_TIMER23] = 0x10012000,
+    [VE_VIRTIO] = 0x10013000,
     [VE_SERIALDVI] = 0x10016000,
     [VE_RTC] = 0x10017000,
     [VE_COMPACTFLASH] = 0x1001a000,
@@ -135,6 +144,7 @@ static hwaddr motherboard_aseries_map[] = {
     [VE_WDT] = 0x1c0f0000,
     [VE_TIMER01] = 0x1c110000,
     [VE_TIMER23] = 0x1c120000,
+    [VE_VIRTIO] = 0x1c130000,
     [VE_SERIALDVI] = 0x1c160000,
     [VE_RTC] = 0x1c170000,
     [VE_COMPACTFLASH] = 0x1c1a0000,
@@ -395,6 +405,85 @@ static VEDBoardInfo a15_daughterboard = {
     .init = a15_daughterboard_init,
 };
 
+static int add_virtio_mmio_node(void *fdt, uint32_t acells, uint32_t scells,
+                                hwaddr addr, hwaddr size, uint32_t intc,
+                                int irq)
+{
+    /* Add a virtio_mmio node to the device tree blob:
+     *   virtio_mmio@ADDRESS {
+     *       compatible = "virtio,mmio";
+     *       reg = <ADDRESS, SIZE>;
+     *       interrupt-parent = <&intc>;
+     *       interrupts = <0, irq, 1>;
+     *   }
+     * (Note that the format of the interrupts property is dependent on the
+     * interrupt controller that interrupt-parent points to; these are for
+     * the ARM GIC and indicate an SPI interrupt, rising-edge-triggered.)
+     */
+    int rc;
+    char *nodename = g_strdup_printf("/virtio_mmio@%" PRIx64, addr);
+
+    rc = qemu_devtree_add_subnode(fdt, nodename);
+    rc |= qemu_devtree_setprop_string(fdt, nodename,
+                                      "compatible", "virtio,mmio");
+    rc |= qemu_devtree_setprop_sized_cells(fdt, nodename, "reg",
+                                           acells, addr, scells, size);
+    qemu_devtree_setprop_cells(fdt, nodename, "interrupt-parent", intc);
+    qemu_devtree_setprop_cells(fdt, nodename, "interrupts", 0, irq, 1);
+    g_free(nodename);
+    if (rc) {
+        return -1;
+    }
+    return 0;
+}
+
+static uint32_t find_int_controller(void *fdt)
+{
+    /* Find the FDT node corresponding to the interrupt controller
+     * for virtio-mmio devices. We do this by scanning the fdt for
+     * a node with the right compatibility, since we know there is
+     * only one GIC on a vexpress board.
+     * We return the phandle of the node, or 0 if none was found.
+     */
+    const char *compat = "arm,cortex-a9-gic";
+    int offset;
+
+    offset = fdt_node_offset_by_compatible(fdt, -1, compat);
+    if (offset >= 0) {
+        return fdt_get_phandle(fdt, offset);
+    }
+    return 0;
+}
+
+static void vexpress_modify_dtb(const struct arm_boot_info *info, void *fdt)
+{
+    uint32_t acells, scells, intc;
+    const VEDBoardInfo *daughterboard = (const VEDBoardInfo *)info;
+
+    acells = qemu_devtree_getprop_cell(fdt, "/", "#address-cells");
+    scells = qemu_devtree_getprop_cell(fdt, "/", "#size-cells");
+    intc = find_int_controller(fdt);
+    if (!intc) {
+        /* Not fatal, we just won't provide virtio. This will
+         * happen with older device tree blobs.
+         */
+        fprintf(stderr, "QEMU: warning: couldn't find interrupt controller in "
+                "dtb; will not include virtio-mmio devices in the dtb.\n");
+    } else {
+        int i;
+        const hwaddr *map = daughterboard->motherboard_map;
+
+        /* We iterate backwards here because adding nodes
+         * to the dtb puts them in last-first.
+         */
+        for (i = NUM_VIRTIO_TRANSPORTS - 1; i >= 0; i--) {
+            add_virtio_mmio_node(fdt, acells, scells,
+                                 map[VE_VIRTIO] + 0x200 * i,
+                                 0x200, intc, 40 + i);
+        }
+    }
+}
+
 static void vexpress_common_init(VEDBoardInfo *daughterboard,
                                  QEMUMachineInitArgs *args)
 {
@@ -523,6 +612,15 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
 
     /* VE_DAPROM: not modelled */
 
+    /* Create mmio transports, so the user can create virtio backends
+     * (which will be automatically plugged in to the transports). If
+     * no backend is created the transport will just sit harmlessly idle.
+     */
+    for (i = 0; i < NUM_VIRTIO_TRANSPORTS; i++) {
+        sysbus_create_simple("virtio-mmio", map[VE_VIRTIO] + 0x200 * i,
+                             pic[40 + i]);
+    }
+
     daughterboard->bootinfo.ram_size = args->ram_size;
     daughterboard->bootinfo.kernel_filename = args->kernel_filename;
     daughterboard->bootinfo.kernel_cmdline = args->kernel_cmdline;
@@ -533,6 +631,7 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard,
     daughterboard->bootinfo.smp_loader_start = map[VE_SRAM];
     daughterboard->bootinfo.smp_bootreg_addr = map[VE_SYSREGS] + 0x30;
     daughterboard->bootinfo.gic_cpu_if_addr = daughterboard->gic_cpu_if_addr;
+    daughterboard->bootinfo.modify_dtb = vexpress_modify_dtb;
     arm_load_kernel(ARM_CPU(first_cpu), &daughterboard->bootinfo);
 }
 
-- 
1.7.9.5