[Qemu-devel] [PATCH v2 0/2] hw/arm/virt: Add flash and RTC devices

[Qemu-devel] [PATCH v2 0/2] hw/arm/virt: Add flash and RTC devices

[Peter Maydell]

These patches add a few extra devices to the 'virt' platform,
mostly for the benefit of UEFI.

 * flash devices, so bootroms like UEFI have somewhere to
   live and somewhere to store their persistent variables
 * a PL031 RTC, since UEFI mandates an RTC

I don't actually have a working UEFI image for the 'virt' platform
currently; tested with a Linux kernel guest.

Changes v1->v2:
 * fail with a helpful error message if the user tries to
   specify the contents of the first flash device in two
   different ways at once

Peter Maydell (2):
  hw/arm/virt: Provide flash devices for boot ROMs
  hw/arm/virt: Provide PL031 RTC

 hw/arm/virt.c | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 100 insertions(+), 1 deletion(-)

-- 
1.9.2

[Qemu-devel] [PATCH v2 1/2] hw/arm/virt: Provide flash devices for boot ROMs

[Peter Maydell]

Add two flash devices to the virt board, so that it can be used for
running guests which want a bootrom image such as UEFI. We provide
two flash devices to make it more convenient to provide both a
read-only UEFI image and a read-write place to store guest-set
UEFI config variables. The '-bios' command line option is set up
to provide an image for the first of the two flash devices.

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 3b55a4b..e658eb0 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -37,6 +37,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/kvm.h"
 #include "hw/boards.h"
+#include "hw/loader.h"
 #include "exec/address-spaces.h"
 #include "qemu/bitops.h"
 #include "qemu/error-report.h"
@@ -94,7 +95,6 @@ typedef struct VirtBoardInfo {
  * a 32 bit VM and leaving space for expansion and in particular for PCI.
  */
 static const MemMapEntry a15memmap[] = {
-    /* Space up to 0x8000000 is reserved for a boot ROM */
     [VIRT_FLASH] = { 0, 0x8000000 },
     [VIRT_CPUPERIPHS] = { 0x8000000, 0x20000 },
     /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
@@ -375,6 +375,73 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
     }
 }
 
+static void create_one_flash(const char *name, hwaddr flashbase,
+                             hwaddr flashsize)
+{
+    /* Create and map a single flash device. We use the same
+     * parameters as the flash devices on the Versatile Express board.
+     */
+    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
+    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
+    const uint64_t sectorlength = 256 * 1024;
+
+    if (dinfo && qdev_prop_set_drive(dev, "drive", dinfo->bdrv)) {
+        abort();
+    }
+
+    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
+    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
+    qdev_prop_set_uint8(dev, "width", 4);
+    qdev_prop_set_uint8(dev, "device-width", 2);
+    qdev_prop_set_uint8(dev, "big-endian", 0);
+    qdev_prop_set_uint16(dev, "id0", 0x89);
+    qdev_prop_set_uint16(dev, "id1", 0x18);
+    qdev_prop_set_uint16(dev, "id2", 0x00);
+    qdev_prop_set_uint16(dev, "id3", 0x00);
+    qdev_prop_set_string(dev, "name", name);
+    qdev_init_nofail(dev);
+
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, flashbase);
+}
+
+static void create_flash(const VirtBoardInfo *vbi)
+{
+    /* Create two flash devices to fill the VIRT_FLASH space in the memmap.
+     * Any file passed via -bios goes in the first of these.
+     */
+    hwaddr flashsize = vbi->memmap[VIRT_FLASH].size / 2;
+    hwaddr flashbase = vbi->memmap[VIRT_FLASH].base;
+    char *nodename;
+
+    if (bios_name) {
+        const char *fn;
+
+        if (drive_get(IF_PFLASH, 0, 0)) {
+            error_report("The contents of the first flash device may be "
+                         "specified with -bios or with -drive if=pflash... "
+                         "but you cannot use both options at once");
+            exit(1);
+        }
+        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
+        if (!fn || load_image_targphys(fn, flashbase, flashsize) < 0) {
+            error_report("Could not load ROM image '%s'", bios_name);
+            exit(1);
+        }
+    }
+
+    create_one_flash("virt.flash0", flashbase, flashsize);
+    create_one_flash("virt.flash1", flashbase + flashsize, flashsize);
+
+    nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+    qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                 2, flashbase, 2, flashsize,
+                                 2, flashbase + flashsize, 2, flashsize);
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "bank-width", 4);
+    g_free(nodename);
+}
+
 static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 {
     const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
@@ -451,6 +518,8 @@ static void machvirt_init(MachineState *machine)
     vmstate_register_ram_global(ram);
     memory_region_add_subregion(sysmem, vbi->memmap[VIRT_MEM].base, ram);
 
+    create_flash(vbi);
+
     create_gic(vbi, pic);
 
     create_uart(vbi, pic);
-- 
1.9.2

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm/virt: Provide flash devices for boot ROMs

[Paolo Bonzini]

Il 10/06/2014 19:06, Peter Maydell ha scritto:
> Add two flash devices to the virt board, so that it can be used for
> running guests which want a bootrom image such as UEFI. We provide
> two flash devices to make it more convenient to provide both a
> read-only UEFI image and a read-write place to store guest-set
> UEFI config variables. The '-bios' command line option is set up
> to provide an image for the first of the two flash devices.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3b55a4b..e658eb0 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -37,6 +37,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
>  #include "hw/boards.h"
> +#include "hw/loader.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
> @@ -94,7 +95,6 @@ typedef struct VirtBoardInfo {
>   * a 32 bit VM and leaving space for expansion and in particular for PCI.
>   */
>  static const MemMapEntry a15memmap[] = {
> -    /* Space up to 0x8000000 is reserved for a boot ROM */
>      [VIRT_FLASH] = { 0, 0x8000000 },
>      [VIRT_CPUPERIPHS] = { 0x8000000, 0x20000 },
>      /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
> @@ -375,6 +375,73 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>      }
>  }
>
> +static void create_one_flash(const char *name, hwaddr flashbase,
> +                             hwaddr flashsize)
> +{
> +    /* Create and map a single flash device. We use the same
> +     * parameters as the flash devices on the Versatile Express board.
> +     */
> +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
> +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
> +    const uint64_t sectorlength = 256 * 1024;
> +
> +    if (dinfo && qdev_prop_set_drive(dev, "drive", dinfo->bdrv)) {
> +        abort();
> +    }
> +
> +    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
> +    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
> +    qdev_prop_set_uint8(dev, "width", 4);
> +    qdev_prop_set_uint8(dev, "device-width", 2);
> +    qdev_prop_set_uint8(dev, "big-endian", 0);
> +    qdev_prop_set_uint16(dev, "id0", 0x89);
> +    qdev_prop_set_uint16(dev, "id1", 0x18);
> +    qdev_prop_set_uint16(dev, "id2", 0x00);
> +    qdev_prop_set_uint16(dev, "id3", 0x00);
> +    qdev_prop_set_string(dev, "name", name);
> +    qdev_init_nofail(dev);
> +
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, flashbase);
> +}
> +
> +static void create_flash(const VirtBoardInfo *vbi)
> +{
> +    /* Create two flash devices to fill the VIRT_FLASH space in the memmap.
> +     * Any file passed via -bios goes in the first of these.
> +     */
> +    hwaddr flashsize = vbi->memmap[VIRT_FLASH].size / 2;
> +    hwaddr flashbase = vbi->memmap[VIRT_FLASH].base;
> +    char *nodename;
> +
> +    if (bios_name) {
> +        const char *fn;
> +
> +        if (drive_get(IF_PFLASH, 0, 0)) {
> +            error_report("The contents of the first flash device may be "
> +                         "specified with -bios or with -drive if=pflash... "
> +                         "but you cannot use both options at once");
> +            exit(1);
> +        }
> +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        if (!fn || load_image_targphys(fn, flashbase, flashsize) < 0) {
> +            error_report("Could not load ROM image '%s'", bios_name);
> +            exit(1);
> +        }
> +    }
> +
> +    create_one_flash("virt.flash0", flashbase, flashsize);
> +    create_one_flash("virt.flash1", flashbase + flashsize, flashsize);
> +
> +    nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> +                                 2, flashbase, 2, flashsize,
> +                                 2, flashbase + flashsize, 2, flashsize);
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "bank-width", 4);
> +    g_free(nodename);
> +}
> +
>  static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>  {
>      const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
> @@ -451,6 +518,8 @@ static void machvirt_init(MachineState *machine)
>      vmstate_register_ram_global(ram);
>      memory_region_add_subregion(sysmem, vbi->memmap[VIRT_MEM].base, ram);
>
> +    create_flash(vbi);
> +
>      create_gic(vbi, pic);
>
>      create_uart(vbi, pic);
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Re: [Qemu-devel] [PATCH v2 1/2] hw/arm/virt: Provide flash devices for boot ROMs

[Peter Crosthwaite]

On Wed, Jun 11, 2014 at 3:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> Add two flash devices to the virt board, so that it can be used for
> running guests which want a bootrom image such as UEFI. We provide
> two flash devices to make it more convenient to provide both a
> read-only UEFI image and a read-write place to store guest-set
> UEFI config variables. The '-bios' command line option is set up
> to provide an image for the first of the two flash devices.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 3b55a4b..e658eb0 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -37,6 +37,7 @@
>  #include "sysemu/sysemu.h"
>  #include "sysemu/kvm.h"
>  #include "hw/boards.h"
> +#include "hw/loader.h"
>  #include "exec/address-spaces.h"
>  #include "qemu/bitops.h"
>  #include "qemu/error-report.h"
> @@ -94,7 +95,6 @@ typedef struct VirtBoardInfo {
>   * a 32 bit VM and leaving space for expansion and in particular for PCI.
>   */
>  static const MemMapEntry a15memmap[] = {
> -    /* Space up to 0x8000000 is reserved for a boot ROM */
>      [VIRT_FLASH] = { 0, 0x8000000 },
>      [VIRT_CPUPERIPHS] = { 0x8000000, 0x20000 },
>      /* GIC distributor and CPU interfaces sit inside the CPU peripheral space */
> @@ -375,6 +375,73 @@ static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>      }
>  }
>
> +static void create_one_flash(const char *name, hwaddr flashbase,
> +                             hwaddr flashsize)
> +{
> +    /* Create and map a single flash device. We use the same
> +     * parameters as the flash devices on the Versatile Express board.
> +     */
> +    DriveInfo *dinfo = drive_get_next(IF_PFLASH);
> +    DeviceState *dev = qdev_create(NULL, "cfi.pflash01");
> +    const uint64_t sectorlength = 256 * 1024;
> +
> +    if (dinfo && qdev_prop_set_drive(dev, "drive", dinfo->bdrv)) {
> +        abort();
> +    }
> +
> +    qdev_prop_set_uint32(dev, "num-blocks", flashsize / sectorlength);
> +    qdev_prop_set_uint64(dev, "sector-length", sectorlength);
> +    qdev_prop_set_uint8(dev, "width", 4);
> +    qdev_prop_set_uint8(dev, "device-width", 2);
> +    qdev_prop_set_uint8(dev, "big-endian", 0);
> +    qdev_prop_set_uint16(dev, "id0", 0x89);
> +    qdev_prop_set_uint16(dev, "id1", 0x18);
> +    qdev_prop_set_uint16(dev, "id2", 0x00);
> +    qdev_prop_set_uint16(dev, "id3", 0x00);
> +    qdev_prop_set_string(dev, "name", name);
> +    qdev_init_nofail(dev);
> +
> +    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, flashbase);
> +}
> +
> +static void create_flash(const VirtBoardInfo *vbi)
> +{
> +    /* Create two flash devices to fill the VIRT_FLASH space in the memmap.
> +     * Any file passed via -bios goes in the first of these.
> +     */
> +    hwaddr flashsize = vbi->memmap[VIRT_FLASH].size / 2;
> +    hwaddr flashbase = vbi->memmap[VIRT_FLASH].base;
> +    char *nodename;
> +
> +    if (bios_name) {
> +        const char *fn;
> +
> +        if (drive_get(IF_PFLASH, 0, 0)) {

Does the bios actually have to be in pflash? Does the guest expect
pflash controllability or will any memory-mapped read-only device do?
I'm thinking this can be simplified by having the -bios arg back onto
a raw read-only MemoryRegion (inited here) and then the one-and-only
-pflash arg is usable consistently for your read-and-write area
(second half).

Regards,
Peter

> +            error_report("The contents of the first flash device may be "
> +                         "specified with -bios or with -drive if=pflash... "
> +                         "but you cannot use both options at once");
> +            exit(1);
> +        }
> +        fn = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
> +        if (!fn || load_image_targphys(fn, flashbase, flashsize) < 0) {
> +            error_report("Could not load ROM image '%s'", bios_name);
> +            exit(1);
> +        }
> +    }
> +
> +    create_one_flash("virt.flash0", flashbase, flashsize);
> +    create_one_flash("virt.flash1", flashbase + flashsize, flashsize);
> +
> +    nodename = g_strdup_printf("/flash@%" PRIx64, flashbase);
> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "compatible", "cfi-flash");
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> +                                 2, flashbase, 2, flashsize,
> +                                 2, flashbase + flashsize, 2, flashsize);
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "bank-width", 4);
> +    g_free(nodename);
> +}
> +
>  static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
>  {
>      const VirtBoardInfo *board = (const VirtBoardInfo *)binfo;
> @@ -451,6 +518,8 @@ static void machvirt_init(MachineState *machine)
>      vmstate_register_ram_global(ram);
>      memory_region_add_subregion(sysmem, vbi->memmap[VIRT_MEM].base, ram);
>
> +    create_flash(vbi);
> +
>      create_gic(vbi, pic);
>
>      create_uart(vbi, pic);
> --
> 1.9.2
>
>

[Qemu-devel] [PATCH v2 2/2] hw/arm/virt: Provide PL031 RTC

[Peter Maydell]

UEFI mandates that the platform must include an RTC, so provide
one in 'virt', using the PL031.

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

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index e658eb0..b60928e 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -66,6 +66,7 @@ enum {
     VIRT_GIC_CPU,
     VIRT_UART,
     VIRT_MMIO,
+    VIRT_RTC,
 };
 
 typedef struct MemMapEntry {
@@ -93,6 +94,8 @@ typedef struct VirtBoardInfo {
  * high memory region beyond 4GB).
  * This represents a compromise between how much RAM can be given to
  * a 32 bit VM and leaving space for expansion and in particular for PCI.
+ * Note that devices should generally be placed at multiples of 0x10000,
+ * to accommodate guests using 64K pages.
  */
 static const MemMapEntry a15memmap[] = {
     [VIRT_FLASH] = { 0, 0x8000000 },
@@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = {
     [VIRT_GIC_DIST] = { 0x8000000, 0x10000 },
     [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
     [VIRT_UART] = { 0x9000000, 0x1000 },
+    [VIRT_RTC] = { 0x90010000, 0x1000 },
     [VIRT_MMIO] = { 0xa000000, 0x200 },
     /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
     /* 0x10000000 .. 0x40000000 reserved for PCI */
@@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
 
 static const int a15irqmap[] = {
     [VIRT_UART] = 1,
+    [VIRT_RTC] = 2,
     [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
 };
 
@@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
     g_free(nodename);
 }
 
+static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
+{
+    char *nodename;
+    hwaddr base = vbi->memmap[VIRT_RTC].base;
+    hwaddr size = vbi->memmap[VIRT_RTC].size;
+    int irq = vbi->irqmap[VIRT_RTC];
+    const char compat[] = "arm,pl031\0arm,primecell";
+
+    sysbus_create_simple("pl031", base, pic[irq]);
+
+    nodename = g_strdup_printf("/pl031@%" PRIx64, base);
+    qemu_fdt_add_subnode(vbi->fdt, nodename);
+    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
+    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
+                                 2, base, 2, size);
+    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
+                           GIC_FDT_IRQ_TYPE_SPI, irq,
+                           GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
+    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
+    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
+    g_free(nodename);
+}
+
 static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
 {
     int i;
@@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine)
 
     create_uart(vbi, pic);
 
+    create_rtc(vbi, pic);
+
     /* 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.
-- 
1.9.2

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt: Provide PL031 RTC

[Claudio Fontana]

On 10.06.2014 19:06, Peter Maydell wrote:
> UEFI mandates that the platform must include an RTC, so provide
> one in 'virt', using the PL031.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index e658eb0..b60928e 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -66,6 +66,7 @@ enum {
>      VIRT_GIC_CPU,
>      VIRT_UART,
>      VIRT_MMIO,
> +    VIRT_RTC,
>  };
>  
>  typedef struct MemMapEntry {
> @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo {
>   * high memory region beyond 4GB).
>   * This represents a compromise between how much RAM can be given to
>   * a 32 bit VM and leaving space for expansion and in particular for PCI.
> + * Note that devices should generally be placed at multiples of 0x10000,
> + * to accommodate guests using 64K pages.
>   */
>  static const MemMapEntry a15memmap[] = {
>      [VIRT_FLASH] = { 0, 0x8000000 },
> @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = {
>      [VIRT_GIC_DIST] = { 0x8000000, 0x10000 },
>      [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
>      [VIRT_UART] = { 0x9000000, 0x1000 },
> +    [VIRT_RTC] = { 0x90010000, 0x1000 },
>      [VIRT_MMIO] = { 0xa000000, 0x200 },
>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>      /* 0x10000000 .. 0x40000000 reserved for PCI */
> @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
>  
>  static const int a15irqmap[] = {
>      [VIRT_UART] = 1,
> +    [VIRT_RTC] = 2,
>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>  };
>  
> @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
>      g_free(nodename);
>  }
>  
> +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
> +{
> +    char *nodename;
> +    hwaddr base = vbi->memmap[VIRT_RTC].base;
> +    hwaddr size = vbi->memmap[VIRT_RTC].size;
> +    int irq = vbi->irqmap[VIRT_RTC];
> +    const char compat[] = "arm,pl031\0arm,primecell";
> +
> +    sysbus_create_simple("pl031", base, pic[irq]);
> +
> +    nodename = g_strdup_printf("/pl031@%" PRIx64, base);
> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
> +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
> +                                 2, base, 2, size);
> +    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
> +                           GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
> +    g_free(nodename);
> +}
> +
>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>  {
>      int i;
> @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine)
>  
>      create_uart(vbi, pic);
>  
> +    create_rtc(vbi, pic);
> +
>      /* 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.
> 

I am quite happy with the RTC device being added to the virt platform, as this will move me from 1970 in the guest, where I am at the moment. :)

One question I would have is, what would be the best/recommended way as a user of the virt platform
to add buses and devices to the platform? Is using virt as the base platform, and extending it with additional buses and devices a sensible thing to do?

In my case I am particularly interested in the possibility to add a PCI-E bus to the platform in some (any) way, so that QEMU provides support for that, one that does not mean maintaining a separate patchset. Is extending via --device a viable option?
New machine model?

Thank you for any advice,

Claudio

Re: [Qemu-devel] [PATCH v2 2/2] hw/arm/virt: Provide PL031 RTC

[Peter Crosthwaite]

On Thu, Jun 12, 2014 at 2:01 AM, Claudio Fontana
<claudio.fontana@huawei.com> wrote:
> On 10.06.2014 19:06, Peter Maydell wrote:
>> UEFI mandates that the platform must include an RTC, so provide
>> one in 'virt', using the PL031.
>>
>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>> ---
>>  hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++
>>  1 file changed, 30 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index e658eb0..b60928e 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -66,6 +66,7 @@ enum {
>>      VIRT_GIC_CPU,
>>      VIRT_UART,
>>      VIRT_MMIO,
>> +    VIRT_RTC,
>>  };
>>
>>  typedef struct MemMapEntry {
>> @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo {
>>   * high memory region beyond 4GB).
>>   * This represents a compromise between how much RAM can be given to
>>   * a 32 bit VM and leaving space for expansion and in particular for PCI.
>> + * Note that devices should generally be placed at multiples of 0x10000,
>> + * to accommodate guests using 64K pages.
>>   */
>>  static const MemMapEntry a15memmap[] = {
>>      [VIRT_FLASH] = { 0, 0x8000000 },
>> @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = {
>>      [VIRT_GIC_DIST] = { 0x8000000, 0x10000 },
>>      [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
>>      [VIRT_UART] = { 0x9000000, 0x1000 },
>> +    [VIRT_RTC] = { 0x90010000, 0x1000 },
>>      [VIRT_MMIO] = { 0xa000000, 0x200 },
>>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>>      /* 0x10000000 .. 0x40000000 reserved for PCI */
>> @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
>>
>>  static const int a15irqmap[] = {
>>      [VIRT_UART] = 1,
>> +    [VIRT_RTC] = 2,
>>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>>  };
>>
>> @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
>>      g_free(nodename);
>>  }
>>
>> +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>> +{
>> +    char *nodename;
>> +    hwaddr base = vbi->memmap[VIRT_RTC].base;
>> +    hwaddr size = vbi->memmap[VIRT_RTC].size;
>> +    int irq = vbi->irqmap[VIRT_RTC];
>> +    const char compat[] = "arm,pl031\0arm,primecell";
>> +
>> +    sysbus_create_simple("pl031", base, pic[irq]);
>> +
>> +    nodename = g_strdup_printf("/pl031@%" PRIx64, base);
>> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
>> +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
>> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
>> +                                 2, base, 2, size);
>> +    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
>> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
>> +                           GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
>> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
>> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
>> +    g_free(nodename);
>> +}
>> +
>>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>>  {
>>      int i;
>> @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine)
>>
>>      create_uart(vbi, pic);
>>
>> +    create_rtc(vbi, pic);
>> +
>>      /* 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.
>>
>
> I am quite happy with the RTC device being added to the virt platform, as this will move me from 1970 in the guest, where I am at the moment. :)
>
> One question I would have is, what would be the best/recommended way as a user of the virt platform
> to add buses and devices to the platform? Is using virt as the base platform, and extending it with additional buses and devices a sensible thing to do?
>

Yes.

> In my case I am particularly interested in the possibility to add a PCI-E bus to the platform in some (any) way, so that QEMU provides support for that, one that does not mean maintaining a separate patchset. Is extending via --device a viable option?
> New machine model?

For PCI support it should just be a case of adding a PCI controller to
the virt board, then -device should just work for pci devs.

Regards,
Peter

>
> Thank you for any advice,
>
> Claudio
>
>

[Qemu-devel] hw/arm/virt: Provide PCI?

[Claudio Fontana]

On 14.06.2014 01:10, Peter Crosthwaite wrote:
> On Thu, Jun 12, 2014 at 2:01 AM, Claudio Fontana
> <claudio.fontana@huawei.com> wrote:
>> On 10.06.2014 19:06, Peter Maydell wrote:
>>> UEFI mandates that the platform must include an RTC, so provide
>>> one in 'virt', using the PL031.
>>>
>>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
>>> ---
>>>  hw/arm/virt.c | 30 ++++++++++++++++++++++++++++++
>>>  1 file changed, 30 insertions(+)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index e658eb0..b60928e 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -66,6 +66,7 @@ enum {
>>>      VIRT_GIC_CPU,
>>>      VIRT_UART,
>>>      VIRT_MMIO,
>>> +    VIRT_RTC,
>>>  };
>>>
>>>  typedef struct MemMapEntry {
>>> @@ -93,6 +94,8 @@ typedef struct VirtBoardInfo {
>>>   * high memory region beyond 4GB).
>>>   * This represents a compromise between how much RAM can be given to
>>>   * a 32 bit VM and leaving space for expansion and in particular for PCI.
>>> + * Note that devices should generally be placed at multiples of 0x10000,
>>> + * to accommodate guests using 64K pages.
>>>   */
>>>  static const MemMapEntry a15memmap[] = {
>>>      [VIRT_FLASH] = { 0, 0x8000000 },
>>> @@ -101,6 +104,7 @@ static const MemMapEntry a15memmap[] = {
>>>      [VIRT_GIC_DIST] = { 0x8000000, 0x10000 },
>>>      [VIRT_GIC_CPU] = { 0x8010000, 0x10000 },
>>>      [VIRT_UART] = { 0x9000000, 0x1000 },
>>> +    [VIRT_RTC] = { 0x90010000, 0x1000 },
>>>      [VIRT_MMIO] = { 0xa000000, 0x200 },
>>>      /* ...repeating for a total of NUM_VIRTIO_TRANSPORTS, each of that size */
>>>      /* 0x10000000 .. 0x40000000 reserved for PCI */
>>> @@ -109,6 +113,7 @@ static const MemMapEntry a15memmap[] = {
>>>
>>>  static const int a15irqmap[] = {
>>>      [VIRT_UART] = 1,
>>> +    [VIRT_RTC] = 2,
>>>      [VIRT_MMIO] = 16, /* ...to 16 + NUM_VIRTIO_TRANSPORTS - 1 */
>>>  };
>>>
>>> @@ -340,6 +345,29 @@ static void create_uart(const VirtBoardInfo *vbi, qemu_irq *pic)
>>>      g_free(nodename);
>>>  }
>>>
>>> +static void create_rtc(const VirtBoardInfo *vbi, qemu_irq *pic)
>>> +{
>>> +    char *nodename;
>>> +    hwaddr base = vbi->memmap[VIRT_RTC].base;
>>> +    hwaddr size = vbi->memmap[VIRT_RTC].size;
>>> +    int irq = vbi->irqmap[VIRT_RTC];
>>> +    const char compat[] = "arm,pl031\0arm,primecell";
>>> +
>>> +    sysbus_create_simple("pl031", base, pic[irq]);
>>> +
>>> +    nodename = g_strdup_printf("/pl031@%" PRIx64, base);
>>> +    qemu_fdt_add_subnode(vbi->fdt, nodename);
>>> +    qemu_fdt_setprop(vbi->fdt, nodename, "compatible", compat, sizeof(compat));
>>> +    qemu_fdt_setprop_sized_cells(vbi->fdt, nodename, "reg",
>>> +                                 2, base, 2, size);
>>> +    qemu_fdt_setprop_cells(vbi->fdt, nodename, "interrupts",
>>> +                           GIC_FDT_IRQ_TYPE_SPI, irq,
>>> +                           GIC_FDT_IRQ_FLAGS_EDGE_LO_HI);
>>> +    qemu_fdt_setprop_cell(vbi->fdt, nodename, "clocks", vbi->clock_phandle);
>>> +    qemu_fdt_setprop_string(vbi->fdt, nodename, "clock-names", "apb_pclk");
>>> +    g_free(nodename);
>>> +}
>>> +
>>>  static void create_virtio_devices(const VirtBoardInfo *vbi, qemu_irq *pic)
>>>  {
>>>      int i;
>>> @@ -524,6 +552,8 @@ static void machvirt_init(MachineState *machine)
>>>
>>>      create_uart(vbi, pic);
>>>
>>> +    create_rtc(vbi, pic);
>>> +
>>>      /* 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.
>>>
>>
>> I am quite happy with the RTC device being added to the virt platform, as this will move me from 1970 in the guest, where I am at the moment. :)
>>
>> One question I would have is, what would be the best/recommended way as a user of the virt platform
>> to add buses and devices to the platform? Is using virt as the base platform, and extending it with additional buses and devices a sensible thing to do?
>>
> 
> Yes.
> 
>> In my case I am particularly interested in the possibility to add a PCI-E bus to the platform in some (any) way, so that QEMU provides support for that, one that does not mean maintaining a separate patchset. Is extending via --device a viable option?
>> New machine model?
> 
> For PCI support it should just be a case of adding a PCI controller to
> the virt board, then -device should just work for pci devs.

Thanks for your answer; that seems straightforward enough, so would a patch to add pci to the virt board be acceptable?

virt.c shows that there is already a reserved space in the mem map for that.

Thank you,

Claudio

Re: [Qemu-devel] hw/arm/virt: Provide PCI?

[Peter Maydell]

On 16 June 2014 10:18, Claudio Fontana <claudio.fontana@huawei.com> wrote:
> On 14.06.2014 01:10, Peter Crosthwaite wrote:
>> For PCI support it should just be a case of adding a PCI controller to
>> the virt board, then -device should just work for pci devs.
>
> Thanks for your answer; that seems straightforward enough,
> so would a patch to add pci to the virt board be acceptable?

You would need to implement a sensible PCI controller first.

Actually Rob Herring has some prototype patches which
do this (using the kernel's new support for a PCI controller
which can be defined entirely using a device tree). They
have some bugs still, though (the lsi SCSI controller
device doesn't seem to work, for instance).

Rob, would you mind posting your PCI controller patch
as an RFC? That way Claudio could have a look at the
issues with DMA if he wants to move forward here.

thanks
-- PMM