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

[Peter Crosthwaite]

Hi,

On Sat, Jul 13, 2013 at 6:37 AM, Peter Maydell <address@hidden> wrote:
> 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 <address@hidden>
> ---
>  hw/arm/vexpress.c |   99 
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
>
> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
> index d1c28aa..dcc86b9 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"
> @@ -70,6 +77,7 @@ enum {
>      VE_ETHERNET,
>      VE_USB,
>      VE_DAPROM,
> +    VE_VIRTIO,
>  };
>
>  static hwaddr motherboard_legacy_map[] = {
> @@ -88,6 +96,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,
> @@ -132,6 +141,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,
> @@ -392,6 +402,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)

This seems pretty general to me and maybe belongs in the device tree API. Some
of the PPC machines could make good use of this. From e500:

snprintf(mpic, sizeof(mpic), "%s/address@hidden", soc, 
MPC8544_MPIC_REGS_OFFSET);
    qemu_devtree_add_subnode(fdt, mpic);
    qemu_devtree_setprop_string(fdt, mpic, "device_type", "open-pic");
    qemu_devtree_setprop_string(fdt, mpic, "compatible", "fsl,mpic");
    qemu_devtree_setprop_cells(fdt, mpic, "reg", MPC8544_MPIC_REGS_OFFSET,
                               0x40000);
    qemu_devtree_setprop_cell(fdt, mpic, "#address-cells", 0);
    qemu_devtree_setprop_cell(fdt, mpic, "#interrupt-cells", 2);

The nasty intc specific stuff is a problem, but perhaps at least the
boiler plate
creation+reg+compatibile can be factored out for all to use?

Regards,
Peter