[Qemu-devel] [PATCH v2 0/4] Allow sysbus devices to be attached via commandline

[Qemu-devel] [PATCH v2 0/4] Allow sysbus devices to be attached via commandline

[Alistair Francis]

This patch allows sysbus devices to be attached via
command line arguments.

This can be used to build an entire machine from the command
line or to just add devices that aren't in the machine_init
code.

A peripheral can be added with the following syntax:
-device cadence_uart,addr=0xE0000000,irq=27

A CPU can be added with either of the following:
-device cpu,model=cortex-a9,type=arm-cpu,reset-cbar=0xF8F00000,midr=0x413        FC090
-sysbusdev device=cpu,name=microblaze-cp

RAM or ROM can be attached with this command:
-device memory,name=zynq.ext_ram,addr=0x00000000,size=0x8000000

Multiple IRQ lines can be used as well as multiple properties:
-device pl330,addr=0xF8003000,irq=13,irq=14,irq=15,irq=16,irq=17,\
irq=40,irq=41,irq=42,irq=43,num_chnls=8,num_periph_req=4,num_events=16

This implementation uses a three round multi-pass method. This will
hopefully allow devices attached via the command line to be connected
to other command line devices (I haven't managed to get that working
yet though)

With Li Guang's blob loader implemented it is also possible to boot
images while using "-M none".

V2:
Use -device to attach the devices (Thanks Markus Armbruster)
Make the method much more generic
Allow CPUs and Memory to be attached via command line
Allow properties to be passed in via the command line

Thanks to Markus Armbruster and Peter Crosthwaite for
feedback on the first version


Alistair Francis (4):
  qemu-option.c: Add qemu_opt functions that step over arguments
  qdev-monitor: Implement three functions used to connect devices
  vl.c: Enable adding devices to the system bus
  qemu-options.hx: Update the command line documentation for -device

 include/hw/boards.h       |    2 +
 include/monitor/qdev.h    |    3 +
 include/qemu/option.h     |    2 +
 include/qemu/option_int.h |    1 +
 qdev-monitor.c            |  237 ++++++++++++++++++++++++++++++++++++++++++++-
 qemu-options.hx           |    8 +-
 util/qemu-option.c        |   30 ++++++
 vl.c                      |   74 +++++++++++++-
 8 files changed, 346 insertions(+), 11 deletions(-)

[Qemu-devel] [PATCH v2 1/4] qemu-option.c: Add qemu_opt functions that step over arguments

[Alistair Francis]

This adds two functions, qemu_opt_step() and qemu_opt_name_step()
which iterate over the comma separated stings in the QemuOpts*
argument. This allows accessing multiple arguments with the
same name by iterating over all of the arguments

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 include/qemu/option.h |    2 ++
 util/qemu-option.c    |   30 ++++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 8c0ac34..ad20cd4 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -110,6 +110,8 @@ struct QemuOptsList {
 };
 
 const char *qemu_opt_get(QemuOpts *opts, const char *name);
+const char *qemu_opt_name_step(QemuOpts *opts, int num);
+const char *qemu_opt_step(QemuOpts *opts, int num);
 /**
  * qemu_opt_has_help_opt:
  * @opts: options to search for a help request
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 9d898af..4192c13 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -541,6 +541,36 @@ void print_option_help(QEMUOptionParameter *list)
 
 /* ------------------------------------------------------------------ */
 
+const char *qemu_opt_name_step(QemuOpts *opts, int num)
+{
+    QemuOpt *opt;
+    int i = 0;
+
+    QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
+        if (i < num) {
+            i++;
+            continue;
+        }
+        return opt->name;
+    }
+    return NULL;
+}
+
+const char *qemu_opt_step(QemuOpts *opts, int num)
+{
+    QemuOpt *opt;
+    int i = 0;
+
+    QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) {
+        if (i < num) {
+            i++;
+            continue;
+        }
+        return opt->str;
+    }
+    return NULL;
+}
+
 static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name)
 {
     QemuOpt *opt;
-- 
1.7.1

[Qemu-devel] [PATCH v2 2/4] qdev-monitor: Implement three functions used to connect devices

[Alistair Francis]

These functions are used to attach devices that are passed in
via the command line using the -device argument.

The reason for using three functions is to allow a multi-pass
approach. This can then be extended to allow devices to
connect to other devices that are being connected via the
command line (for example DMA/ethernet AXI-Stream).

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 include/hw/boards.h       |    2 +
 include/monitor/qdev.h    |    3 +
 include/qemu/option_int.h |    1 +
 qdev-monitor.c            |  237 ++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 241 insertions(+), 2 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index dd2c70d..d06bf86 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -15,6 +15,8 @@ typedef struct QEMUMachineInitArgs {
     const char *kernel_cmdline;
     const char *initrd_filename;
     const char *cpu_model;
+    DeviceState *intc;
+    Object *cpu;
 } QEMUMachineInitArgs;
 
 typedef void QEMUMachineInitFunc(QEMUMachineInitArgs *args);
diff --git a/include/monitor/qdev.h b/include/monitor/qdev.h
index 8d16e11..18a5b3d 100644
--- a/include/monitor/qdev.h
+++ b/include/monitor/qdev.h
@@ -11,5 +11,8 @@ void do_info_qdm(Monitor *mon, const QDict *qdict);
 int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 int qdev_device_help(QemuOpts *opts);
 DeviceState *qdev_device_add(QemuOpts *opts);
+DeviceState *qdev_device_create(QemuOpts *opts);
+DeviceState *qdev_device_init(QemuOpts *opts, DeviceState* intc);
+DeviceState *qdev_device_connect(QemuOpts *opts, DeviceState* intc);
 
 #endif
diff --git a/include/qemu/option_int.h b/include/qemu/option_int.h
index 8212fa4..fdab055 100644
--- a/include/qemu/option_int.h
+++ b/include/qemu/option_int.h
@@ -47,6 +47,7 @@ struct QemuOpts {
     char *id;
     QemuOptsList *list;
     Location loc;
+    void *opaque;
     QTAILQ_HEAD(QemuOptHead, QemuOpt) head;
     QTAILQ_ENTRY(QemuOpts) next;
 };
diff --git a/qdev-monitor.c b/qdev-monitor.c
index 9268c87..5920cb6 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -24,6 +24,8 @@
 #include "qmp-commands.h"
 #include "sysemu/arch_init.h"
 #include "qemu/config-file.h"
+#include "exec/address-spaces.h"
+#include "qemu/option_int.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -148,10 +150,27 @@ static int set_property(const char *name, const char *value, void *opaque)
     Object *obj = opaque;
     Error *err = NULL;
 
-    if (strcmp(name, "driver") == 0)
+    if (strcmp(name, "driver") == 0) {
         return 0;
-    if (strcmp(name, "bus") == 0)
+    }
+    if (strcmp(name, "bus") == 0) {
+        return 0;
+    }
+    if (strcmp(name, "addr") == 0) {
+        return 0;
+    }
+    if (strcmp(name, "irq") == 0) {
+        return 0;
+    }
+    if (strcmp(name, "model") == 0) {
+        return 0;
+    }
+    if (strcmp(name, "name") == 0) {
         return 0;
+    }
+    if (strcmp(name, "type") == 0) {
+        return 0;
+    }
 
     object_property_parse(obj, value, name, &err);
     if (err != NULL) {
@@ -567,6 +586,220 @@ DeviceState *qdev_device_add(QemuOpts *opts)
 }
 
 
+DeviceState *qdev_device_create(QemuOpts *opts)
+{
+    ObjectClass *oc;
+    DeviceClass *dc;
+    const char *driver, *path;
+    DeviceState *dev = NULL;
+    BusState *bus = NULL;
+
+    hwaddr addr;
+    MemoryRegion *ram;
+    MemoryRegion *address_space_mem = get_system_memory();;
+    uint64_t ram_size;
+
+    Object *cpu;
+    ObjectClass *cpu_oc;
+    Error *err = NULL;
+
+    driver = qemu_opt_get(opts, "driver");
+    if (!driver) {
+        qerror_report(QERR_MISSING_PARAMETER, "driver");
+        exit(1);
+    }
+
+    if (!strcmp(driver, "cpu")) {
+        /* The device being added is a cpu */
+        if (qemu_opt_get(opts, "name")) {
+            cpu = object_new(qemu_opt_get(opts, "name"));
+        } else {
+            cpu_oc = cpu_class_by_name(qemu_opt_get(opts, "type"),
+                                       qemu_opt_get(opts, "model"));
+            cpu = object_new(object_class_get_name(cpu_oc));
+        }
+
+        /* Set Properties */
+        qemu_opt_foreach(opts, set_property, cpu, 1);
+
+        object_property_set_bool(cpu, true, "realized", &err);
+        if (err) {
+            error_report("%s", error_get_pretty(err));
+            exit(1);
+        }
+
+        return NULL;
+    }
+
+    if (!strcmp(driver, "memory")) {
+        /* The device being added is a memory controller */
+        ram = g_new(MemoryRegion, 1);
+
+        sscanf(qemu_opt_get(opts, "size"), "%X", (uint *) &ram_size);
+        sscanf(qemu_opt_get(opts, "addr"), "%X", (uint *) &addr);
+
+        memory_region_init_ram(ram, NULL, qemu_opt_get(opts, "name"),
+                               (uint) ram_size);
+        vmstate_register_ram_global(ram);
+        memory_region_add_subregion(address_space_mem, (uint) addr, ram);
+
+        return NULL;
+    }
+
+    /* find the driver */
+    oc = object_class_by_name(driver);
+    if (!oc) {
+        const char *typename = find_typename_by_alias(driver);
+
+        if (typename) {
+            driver = typename;
+            oc = object_class_by_name(driver);
+        }
+    }
+
+    if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) {
+        qerror_report(ERROR_CLASS_GENERIC_ERROR,
+                      "'%s' is not a valid device model name", driver);
+        exit(1);
+    }
+
+    if (object_class_is_abstract(oc)) {
+        qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver",
+                      "non-abstract device type");
+        exit(1);
+    }
+
+    dc = DEVICE_CLASS(oc);
+    if (dc->cannot_instantiate_with_device_add_yet) {
+        /* Find the Bus */
+        path = qemu_opt_get(opts, "bus");
+        if (path != NULL) {
+            bus = qbus_find(path);
+            if (!bus) {
+                exit(1);
+            }
+            if (!object_dynamic_cast(OBJECT(bus), dc->bus_type)) {
+                qerror_report(QERR_BAD_BUS_FOR_DEVICE,
+                              driver, object_get_typename(OBJECT(bus)));
+                exit(1);
+            }
+        } else if (dc->bus_type != NULL) {
+            bus = qbus_find_recursive(sysbus_get_default(), NULL, dc->bus_type);
+            if (!bus) {
+                qerror_report(QERR_NO_BUS_FOR_DEVICE,
+                              dc->bus_type, driver);
+                exit(1);
+            }
+        }
+        if (qdev_hotplug && bus && !bus->allow_hotplug) {
+            qerror_report(QERR_BUS_NO_HOTPLUG, bus->name);
+            exit(1);
+        }
+
+        /* create driver */
+        dev = DEVICE(object_new(driver));
+
+        if (bus) {
+            qdev_set_parent_bus(dev, bus);
+        }
+    } else {
+        /* Init the device later */
+        dev = NULL;
+    }
+
+    opts->opaque = (void *) dev;
+    return dev;
+}
+
+DeviceState *qdev_device_init(QemuOpts *opts, DeviceState* intc)
+{
+    DeviceClass *dc;
+    const char *driver;
+    DeviceState *dev = NULL;
+
+    driver = qemu_opt_get(opts, "driver");
+    if (!driver) {
+        qerror_report(QERR_MISSING_PARAMETER, "driver");
+        exit(1);
+    }
+
+    dev = (DeviceState *) opts->opaque;
+
+    if (!dev) {
+        return NULL;
+    }
+
+    dc = DEVICE_CLASS(object_class_by_name(driver));
+    if (!dc->cannot_instantiate_with_device_add_yet) {
+        return NULL;
+    }
+
+    /* Set Properties */
+    if (qemu_opt_foreach(opts, set_property, dev, 1) != 0) {
+        object_unparent(OBJECT(dev));
+        object_unref(OBJECT(dev));
+        exit(1);
+    }
+
+    qdev_init_nofail(dev);
+
+    return dev;
+}
+
+DeviceState *qdev_device_connect(QemuOpts *opts, DeviceState* intc)
+{
+    DeviceClass *dc;
+    const char *driver;
+    DeviceState *dev = NULL;
+    int irq, i, n = 0;
+    hwaddr addr;
+
+    driver = qemu_opt_get(opts, "driver");
+    if (!driver) {
+        qerror_report(QERR_MISSING_PARAMETER, "driver");
+        exit(1);
+    }
+
+    dev = (DeviceState *) opts->opaque;
+
+    if (!dev) {
+        return NULL;
+    }
+
+    dc = DEVICE_CLASS(object_class_by_name(driver));
+    if (!dc->cannot_instantiate_with_device_add_yet) {
+        return NULL;
+    }
+
+    if (qemu_opt_get(opts, "addr")) {
+        sscanf(qemu_opt_get(opts, "addr"), "%X", (uint *) &addr);
+        sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, (uint) addr);
+    }
+
+    if (qemu_opt_get(opts, "irq") && intc) {
+        for (i = 0; ; i++) {
+            if (!qemu_opt_name_step(opts, i)) {
+                break;
+            }
+            if (strcmp(qemu_opt_name_step(opts, i), "irq")) {
+                continue;
+            }
+
+            sscanf(qemu_opt_step(opts, i), "%d", &irq);
+
+            sysbus_connect_irq(SYS_BUS_DEVICE(dev), n,
+                               qdev_get_gpio_in(intc, irq));
+
+            n++;
+        }
+    } else if (first_cpu && dev->num_gpio_in > 0) {
+        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
+                           qdev_get_gpio_in(DEVICE(first_cpu), 0));
+    }
+
+    return dev;
+}
+
 #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## __VA_ARGS__)
 static void qbus_print(Monitor *mon, BusState *bus, int indent);
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 3/4] vl.c: Enable adding devices to the system bus

[Alistair Francis]

This removes the old method to connect devices and replaces it
with three calls to the three qdev-monitor functions added
in the previous patch.

This allows complete machines to be built via the command line as
well as just attaching simple sysbus devices.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 vl.c |   74 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 68 insertions(+), 6 deletions(-)

diff --git a/vl.c b/vl.c
index 9975e5a..809882c 100644
--- a/vl.c
+++ b/vl.c
@@ -97,6 +97,7 @@ int main(int argc, char **argv)
 #include "qemu-options.h"
 #include "qmp-commands.h"
 #include "qemu/main-loop.h"
+#include "qemu/option_int.h"
 #ifdef CONFIG_VIRTFS
 #include "fsdev/qemu-fsdev.h"
 #endif
@@ -2381,14 +2382,63 @@ static int device_help_func(QemuOpts *opts, void *opaque)
     return qdev_device_help(opts);
 }
 
+static int device_add_func(QemuOpts *opts, void *opaque)
+{
+    DeviceState *dev;
+
+    if (strcmp(qemu_opt_get(opts, "driver"), "memory") &&
+        strcmp(qemu_opt_get(opts, "driver"), "cpu") &&
+        opts->opaque == NULL) {
+        dev = qdev_device_add(opts);
+        if (!dev) {
+            return -1;
+        }
+        object_unref(OBJECT(dev));
+    }
+    return 0;
+}
+
+static int device_create_func(QemuOpts *opts, void *opaque)
+{
+    DeviceState *dev;
+
+    dev = qdev_device_create(opts);
+
+    return 0;
+}
+
 static int device_init_func(QemuOpts *opts, void *opaque)
 {
     DeviceState *dev;
+    QEMUMachineInitArgs *current_machine = (QEMUMachineInitArgs *) opaque;
+    DeviceState *intc = current_machine->intc;
 
-    dev = qdev_device_add(opts);
-    if (!dev)
-        return -1;
-    object_unref(OBJECT(dev));
+    dev = qdev_device_init(opts, intc);
+
+    if (dev && (dev->num_gpio_in > 32)) {
+        /* Store the Interupt Controller */
+        current_machine->intc = dev;
+    }
+
+    return 0;
+}
+
+static int device_connect_func(QemuOpts *opts, void *opaque)
+{
+    DeviceState *dev;
+    QEMUMachineInitArgs *current_machine = (QEMUMachineInitArgs *) opaque;
+    DeviceState *intc = current_machine->intc;
+
+    dev = qdev_device_connect(opts, intc);
+
+    if (dev && (dev->num_gpio_in > 0)) {
+        /* Store the Interupt Controller */
+        current_machine->intc = dev;
+    }
+
+    if (dev) {
+        object_unref(OBJECT(dev));
+    }
     return 0;
 }
 
@@ -4377,11 +4427,21 @@ int main(int argc, char **argv, char **envp)
                                  .kernel_filename = kernel_filename,
                                  .kernel_cmdline = kernel_cmdline,
                                  .initrd_filename = initrd_filename,
-                                 .cpu_model = cpu_model };
+                                 .cpu_model = cpu_model,
+                                 .intc = NULL };
 
     current_machine->init_args = args;
     machine->init(&current_machine->init_args);
 
+    /* Create devices */
+    qemu_opts_foreach(qemu_find_opts("device"), device_create_func, NULL, 1);
+    /* Init devices */
+    qemu_opts_foreach(qemu_find_opts("device"), device_init_func,
+                          &current_machine->init_args, 1);
+    /* Init devices */
+    qemu_opts_foreach(qemu_find_opts("device"), device_connect_func,
+                          &current_machine->init_args, 1);
+
     audio_init();
 
     cpu_synchronize_all_post_init();
@@ -4395,8 +4455,10 @@ int main(int argc, char **argv, char **envp)
     }
 
     /* init generic devices */
-    if (qemu_opts_foreach(qemu_find_opts("device"), device_init_func, NULL, 1) != 0)
+    if (qemu_opts_foreach(qemu_find_opts("device"), device_add_func,
+                          NULL, 1) != 0) {
         exit(1);
+    }
 
     net_check_clients();
 
-- 
1.7.1

[Qemu-devel] [PATCH v2 4/4] qemu-options.hx: Update the command line documentation for -device

[Alistair Francis]

Update the command line documentation to include the new sysbus
connection ability

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---

 qemu-options.hx |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 2d33815..131248b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -315,8 +315,8 @@ DEF("device", HAS_ARG, QEMU_OPTION_device,
     "-device driver[,prop[=value][,...]]\n"
     "                add device (based on driver)\n"
     "                prop=value,... sets driver properties\n"
-    "                use '-device help' to print all possible drivers\n"
-    "                use '-device driver,help' to print all possible properties\n",
+    "                use '-device help' to print all possible non sysbus drivers\n"
+    "                use '-device driver,help' to print all possible non sysbus properties\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -device @var{driver}[,@var{prop}[=@var{value}][,...]]
@@ -324,7 +324,9 @@ STEXI
 Add device @var{driver}.  @var{prop}=@var{value} sets driver
 properties.  Valid properties depend on the driver.  To get help on
 possible drivers and properties, use @code{-device help} and
-@code{-device @var{driver},help}.
+@code{-device @var{driver},help}. Valid properties for sysbus devices
+include: addr and irq.  Which specifies the sysbus device base address
+and irq number respectivly.
 ETEXI
 
 DEF("name", HAS_ARG, QEMU_OPTION_name,
-- 
1.7.1

Re: [Qemu-devel] [PATCH v2 0/4] Allow sysbus devices to be attached via commandline

[Alistair Francis]

CC Konstanty

On Fri, Apr 11, 2014 at 4:34 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> This patch allows sysbus devices to be attached via
> command line arguments.
>
> This can be used to build an entire machine from the command
> line or to just add devices that aren't in the machine_init
> code.
>
> A peripheral can be added with the following syntax:
> -device cadence_uart,addr=0xE0000000,irq=27
>
> A CPU can be added with either of the following:
> -device cpu,model=cortex-a9,type=arm-cpu,reset-cbar=0xF8F00000,midr=0x413        FC090
> -sysbusdev device=cpu,name=microblaze-cp
>
> RAM or ROM can be attached with this command:
> -device memory,name=zynq.ext_ram,addr=0x00000000,size=0x8000000
>
> Multiple IRQ lines can be used as well as multiple properties:
> -device pl330,addr=0xF8003000,irq=13,irq=14,irq=15,irq=16,irq=17,\
> irq=40,irq=41,irq=42,irq=43,num_chnls=8,num_periph_req=4,num_events=16
>
> This implementation uses a three round multi-pass method. This will
> hopefully allow devices attached via the command line to be connected
> to other command line devices (I haven't managed to get that working
> yet though)
>
> With Li Guang's blob loader implemented it is also possible to boot
> images while using "-M none".
>
> V2:
> Use -device to attach the devices (Thanks Markus Armbruster)
> Make the method much more generic
> Allow CPUs and Memory to be attached via command line
> Allow properties to be passed in via the command line
>
> Thanks to Markus Armbruster and Peter Crosthwaite for
> feedback on the first version
>
>
> Alistair Francis (4):
>   qemu-option.c: Add qemu_opt functions that step over arguments
>   qdev-monitor: Implement three functions used to connect devices
>   vl.c: Enable adding devices to the system bus
>   qemu-options.hx: Update the command line documentation for -device
>
>  include/hw/boards.h       |    2 +
>  include/monitor/qdev.h    |    3 +
>  include/qemu/option.h     |    2 +
>  include/qemu/option_int.h |    1 +
>  qdev-monitor.c            |  237 ++++++++++++++++++++++++++++++++++++++++++++-
>  qemu-options.hx           |    8 +-
>  util/qemu-option.c        |   30 ++++++
>  vl.c                      |   74 +++++++++++++-
>  8 files changed, 346 insertions(+), 11 deletions(-)
>

Re: [Qemu-devel] [PATCH v2 3/4] vl.c: Enable adding devices to the system bus

[Peter Maydell]

On 11 April 2014 07:34, Alistair Francis <alistair.francis@xilinx.com> wrote:
> This removes the old method to connect devices and replaces it
> with three calls to the three qdev-monitor functions added
> in the previous patch.
>
> This allows complete machines to be built via the command line as
> well as just attaching simple sysbus devices.

>  static int device_init_func(QemuOpts *opts, void *opaque)
>  {
>      DeviceState *dev;
> +    QEMUMachineInitArgs *current_machine = (QEMUMachineInitArgs *) opaque;
> +    DeviceState *intc = current_machine->intc;
>
> -    dev = qdev_device_add(opts);
> -    if (!dev)
> -        return -1;
> -    object_unref(OBJECT(dev));
> +    dev = qdev_device_init(opts, intc);
> +
> +    if (dev && (dev->num_gpio_in > 32)) {
> +        /* Store the Interupt Controller */
> +        current_machine->intc = dev;
> +    }

What is this doing here?? Interrupt controllers should
not be special cases, and they're certainly not
guaranteed to be the only things with 32 GPIO
inputs...

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/4] Allow sysbus devices to be attached via commandline

[Peter Maydell]

On 11 April 2014 07:34, Alistair Francis <alistair.francis@xilinx.com> wrote:
> This patch allows sysbus devices to be attached via
> command line arguments.
>
> This can be used to build an entire machine from the command
> line or to just add devices that aren't in the machine_init
> code.
>
> A peripheral can be added with the following syntax:
> -device cadence_uart,addr=0xE0000000,irq=27
>
> A CPU can be added with either of the following:
> -device cpu,model=cortex-a9,type=arm-cpu,reset-cbar=0xF8F00000,midr=0x413        FC090
> -sysbusdev device=cpu,name=microblaze-cp

I don't see how this can possibly be sufficient information
to wire up the CPU properly. How would you specify
where the generic timer outputs go on an A15?
How are you going to handle the private peripheral
mappings? Is the user supposed to provide another
argument for the a9mpcore_priv device?

> RAM or ROM can be attached with this command:
> -device memory,name=zynq.ext_ram,addr=0x00000000,size=0x8000000

How would you use this if you needed to manage
multiple separate address spaces? I'm hoping we can
get rid of address_space_memory at some point
in favour of actually properly modelling when different
CPUs or DMA masters have different views of the world,
so I don't want us to tie ourselves into an incorrect
model by command line back-compat problems.

> Multiple IRQ lines can be used as well as multiple properties:
> -device pl330,addr=0xF8003000,irq=13,irq=14,irq=15,irq=16,irq=17,\
> irq=40,irq=41,irq=42,irq=43,num_chnls=8,num_periph_req=4,num_events=16

This doesn't seem to actually specify anywhere how those
IRQ numbers are supposed to be interpreted. You need
to somehow say "connect this IRQ output line up to
device X's GPIO input line Y" (where X will usually but not
necessarily be an interrupt controller).

Again addr= is assuming a single system wide address
space.

I also think that "allow machine specification by the
command line" is a terrible goal -- certainly we should allow
users the flexibility to put machines together from individual
devices, but we should do that with a reasonably usable
configuration or scripting language (and then we can use
that internally for our own board models). If you try to
specify things using command line argument syntax as
your primary approach then the result is going to end
up with hard-coded shortcuts (like the address space and
which-interrupt-controller problems I mention above) that
you've ended up with to try to make the command line
vaguely comprehensible. But the real comprehensibility
problem is from trying to do it with a single line of text
with highly constrained syntax conventions.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/4] Allow sysbus devices to be attached via commandline

[Alexander Graf]

On 11.04.14 09:55, Peter Maydell wrote:
> On 11 April 2014 07:34, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> This patch allows sysbus devices to be attached via
>> command line arguments.
>>
>> This can be used to build an entire machine from the command
>> line or to just add devices that aren't in the machine_init
>> code.
>>
>> A peripheral can be added with the following syntax:
>> -device cadence_uart,addr=0xE0000000,irq=27
>>
>> A CPU can be added with either of the following:
>> -device cpu,model=cortex-a9,type=arm-cpu,reset-cbar=0xF8F00000,midr=0x413        FC090
>> -sysbusdev device=cpu,name=microblaze-cp
> I don't see how this can possibly be sufficient information
> to wire up the CPU properly. How would you specify
> where the generic timer outputs go on an A15?
> How are you going to handle the private peripheral
> mappings? Is the user supposed to provide another
> argument for the a9mpcore_priv device?
>
>> RAM or ROM can be attached with this command:
>> -device memory,name=zynq.ext_ram,addr=0x00000000,size=0x8000000
> How would you use this if you needed to manage
> multiple separate address spaces? I'm hoping we can
> get rid of address_space_memory at some point
> in favour of actually properly modelling when different
> CPUs or DMA masters have different views of the world,
> so I don't want us to tie ourselves into an incorrect
> model by command line back-compat problems.
>
>> Multiple IRQ lines can be used as well as multiple properties:
>> -device pl330,addr=0xF8003000,irq=13,irq=14,irq=15,irq=16,irq=17,\
>> irq=40,irq=41,irq=42,irq=43,num_chnls=8,num_periph_req=4,num_events=16
> This doesn't seem to actually specify anywhere how those
> IRQ numbers are supposed to be interpreted. You need
> to somehow say "connect this IRQ output line up to
> device X's GPIO input line Y" (where X will usually but not
> necessarily be an interrupt controller).
>
> Again addr= is assuming a single system wide address
> space.
>
> I also think that "allow machine specification by the
> command line" is a terrible goal -- certainly we should allow
> users the flexibility to put machines together from individual
> devices, but we should do that with a reasonably usable
> configuration or scripting language (and then we can use
> that internally for our own board models). If you try to
> specify things using command line argument syntax as
> your primary approach then the result is going to end
> up with hard-coded shortcuts (like the address space and
> which-interrupt-controller problems I mention above) that
> you've ended up with to try to make the command line
> vaguely comprehensible. But the real comprehensibility
> problem is from trying to do it with a single line of text
> with highly constrained syntax conventions.

I agree. And both things should be orthogonal goals. I think it makes a 
lot of sense to work towards converting machine files into say python 
scripts.

But even then users will still want to define additional devices using 
-device on the command line on top of that base machine file. So we need 
a solution to link up devices with "facilities provided by the boad" as 
well - to a certain extent.

I'm not sure if it's worth it to worry about the case where a -device 
provides interfaces that you would need to hook up to from -device 
again. We already have working interfaces for that really.


Alex

Re: [Qemu-devel] [PATCH v2 0/4] Allow sysbus devices to be attached via commandline

[Peter Crosthwaite]

On Fri, Apr 11, 2014 at 5:55 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 11 April 2014 07:34, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> This patch allows sysbus devices to be attached via
>> command line arguments.
>>
>> This can be used to build an entire machine from the command
>> line or to just add devices that aren't in the machine_init
>> code.
>>
>> A peripheral can be added with the following syntax:
>> -device cadence_uart,addr=0xE0000000,irq=27
>>
>> A CPU can be added with either of the following:
>> -device cpu,model=cortex-a9,type=arm-cpu,reset-cbar=0xF8F00000,midr=0x413        FC090
>> -sysbusdev device=cpu,name=microblaze-cp
>
> I don't see how this can possibly be sufficient information
> to wire up the CPU properly. How would you specify
> where the generic timer outputs go on an A15?
> How are you going to handle the private peripheral
> mappings? Is the user supposed to provide another
> argument for the a9mpcore_priv device?
>
>> RAM or ROM can be attached with this command:
>> -device memory,name=zynq.ext_ram,addr=0x00000000,size=0x8000000
>
> How would you use this if you needed to manage
> multiple separate address spaces? I'm hoping we can
> get rid of address_space_memory at some point
> in favour of actually properly modelling when different
> CPUs or DMA masters have different views of the world,
> so I don't want us to tie ourselves into an incorrect
> model by command line back-compat problems.
>

And sys-busses should be referencable and instantiable objects in
their own right. Can we create a sysbus with:

-device sysbus,name=blah

This should all work somehow with Edgars recent per-master-address
series. Masters then also need to specify their attachments in a data
driven way.

-device arm-cpu,master-bus=blah

>> Multiple IRQ lines can be used as well as multiple properties:
>> -device pl330,addr=0xF8003000,irq=13,irq=14,irq=15,irq=16,irq=17,\
>> irq=40,irq=41,irq=42,irq=43,num_chnls=8,num_periph_req=4,num_events=16
>
> This doesn't seem to actually specify anywhere how those
> IRQ numbers are supposed to be interpreted. You need
> to somehow say "connect this IRQ output line up to
> device X's GPIO input line Y" (where X will usually but not
> necessarily be an interrupt controller).
>

This is probably blocked by consolidation of GPIOs and IRQs.
Unforuntately, Sysbus keeps it's own data structure and APIs for IRQs
quite separate from TYPE_DEVICEs GPIO API. I also think purely
numbered and unnamed GPIOs need to be phased-out as its a blocker on
quite a few features.

So anyways heres my proposal around IRQs/GPIOs. I have a series on
list that adds the capability to name GPIOs. We  can use it to
consolidate sysbus::IRQ and device::GPIO by naming all sysbus IRQ
lists (lets just call them "irqs") and moving them into the regular
device->gpio structure. Existing sysbus_foo_irq APIs just back onto
this change so there is no tree-wide change needed. Then instead of
command line IRQ support we add command line GPIO support and the one
soln. just works for all forms of wires.

> Again addr= is assuming a single system wide address
> space.
>
> I also think that "allow machine specification by the
> command line" is a terrible goal -- certainly we should allow
> users the flexibility to put machines together from individual
> devices, but we should do that with a reasonably usable
> configuration or scripting language (and then we can use
> that internally for our own board models). If you try to
> specify things using command line argument syntax as
> your primary approach then the result is going to end
> up with hard-coded shortcuts (like the address space and
> which-interrupt-controller problems I mention above) that
> you've ended up with to try to make the command line
> vaguely comprehensible.

I feel its something of the opposite here. The ultimate goal of an
entire machine on the command line brings to light all these issues
around generalization that are ultimately applicable to machine
extension. Whether not you actually use -M none { -device } is beside
the point. If you assume you have nothing, and aim to be able to do
anything you have less worries about future incapability.

But the real comprehensibility
> problem is from trying to do it with a single line of text
> with highly constrained syntax conventions.
>

And this email thread has quite nicely listed out those constraints. I
think we should get rid of them and continue evolving this idea until
it works.

Regards,
Peter

> thanks
> -- PMM
>

Re: [Qemu-devel] [PATCH v2 4/4] qemu-options.hx: Update the command line documentation for -device

[Peter Crosthwaite]

On Fri, Apr 11, 2014 at 4:35 PM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> Update the command line documentation to include the new sysbus
> connection ability
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
>  qemu-options.hx |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 2d33815..131248b 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -315,8 +315,8 @@ DEF("device", HAS_ARG, QEMU_OPTION_device,
>      "-device driver[,prop[=value][,...]]\n"
>      "                add device (based on driver)\n"
>      "                prop=value,... sets driver properties\n"
> -    "                use '-device help' to print all possible drivers\n"
> -    "                use '-device driver,help' to print all possible properties\n",
> +    "                use '-device help' to print all possible non sysbus drivers\n"

I'm not sure why sysbus devs should be excluded here - is this just a bug?

> +    "                use '-device driver,help' to print all possible non sysbus properties\n",

Any reason this doesn't work?

Regards,
Peter

>      QEMU_ARCH_ALL)
>  STEXI
>  @item -device @var{driver}[,@var{prop}[=@var{value}][,...]]
> @@ -324,7 +324,9 @@ STEXI
>  Add device @var{driver}.  @var{prop}=@var{value} sets driver
>  properties.  Valid properties depend on the driver.  To get help on
>  possible drivers and properties, use @code{-device help} and
> -@code{-device @var{driver},help}.
> +@code{-device @var{driver},help}. Valid properties for sysbus devices
> +include: addr and irq.  Which specifies the sysbus device base address
> +and irq number respectivly.
>  ETEXI
>
>  DEF("name", HAS_ARG, QEMU_OPTION_name,
> --
> 1.7.1
>
>