diff mbox

[v2,3/4] vl.c: Enable adding devices to the system bus

Message ID d894862d5aee6b5fa3cfefc51498c2dced8df296.1397197001.git.alistair.francis@xilinx.com
State New
Headers show

Commit Message

Alistair Francis April 11, 2014, 6:34 a.m. UTC 
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(-)

Comments

Peter Maydell April 11, 2014, 7:45 a.m. UTC | #1 
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
Alistair Francis April 14, 2014, 1:07 a.m. UTC | #2 
On Fri, Apr 11, 2014 at 5:45 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> 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...
>
They are only special cases to connect the other devices to. This is not ideal
and I couldn't figure out any way to determine what the interrupt controller is.

As discussed in the other thread, this could be fixed with named GPIOs and that
is a much better idea

> thanks
> -- PMM
>
diff mbox

Patch

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();