diff mbox series

[RFC,v4,10/49] multi-process: setup a machine object for remote device process

Message ID 6df05bbf3cba4611b462879a7b937f40486cea0a.1571905346.git.jag.raman@oracle.com (mailing list archive)
State New, archived
Headers show
Series Initial support of multi-process qemu | expand

Commit Message

Jag Raman Oct. 24, 2019, 9:08 a.m. UTC 
remote-machine object sets up various subsystems of the remote device
process. Instantiate PCI host bridge object and initialize RAM, IO &
PCI memory regions.

Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
---
 exec.c                        |   3 +-
 include/exec/address-spaces.h |   2 +
 include/remote/machine.h      |  46 ++++++++++++++++
 remote/Makefile.objs          |   1 +
 remote/machine.c              | 118 ++++++++++++++++++++++++++++++++++++++++++
 remote/remote-main.c          |   7 +++
 6 files changed, 175 insertions(+), 2 deletions(-)
 create mode 100644 include/remote/machine.h
 create mode 100644 remote/machine.c

Comments

Stefan Hajnoczi Nov. 13, 2019, 4:22 p.m. UTC | #1 
On Thu, Oct 24, 2019 at 05:08:51AM -0400, Jagannathan Raman wrote:
> +static NotifierList machine_init_done_notifiers =
> +    NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers);
> +
> +bool machine_init_done;
> +
> +void qemu_add_machine_init_done_notifier(Notifier *notify)
> +{
> +    notifier_list_add(&machine_init_done_notifiers, notify);
> +    if (machine_init_done) {
> +        notify->notify(notify, NULL);
> +    }
> +}
> +
> +void qemu_remove_machine_init_done_notifier(Notifier *notify)
> +{
> +    notifier_remove(notify);
> +}
> +
> +void qemu_run_machine_init_done_notifiers(void)
> +{
> +    machine_init_done = true;
> +    notifier_list_notify(&machine_init_done_notifiers, NULL);
> +}

qemu_add_machine_init_done_notifier() is already defined in vl.c.
Please share the implementation instead of duplicating it into the
remote program.

> +
> +static void remote_machine_init(Object *obj)
> +{
> +    RemMachineState *s = REMOTE_MACHINE(obj);
> +    RemPCIHost *rem_host;
> +    MemoryRegion *system_memory, *system_io, *pci_memory;
> +
> +    Error *error_abort = NULL;
> +
> +    qemu_mutex_init(&ram_list.mutex);

Please keep global initialization separate from RemMachineState (e.g. do
it in main() or a function called by main()).  This function should only
initialize RemMachineState.

> +
> +    object_property_add_child(object_get_root(), "machine", obj, &error_abort);
> +    if (error_abort) {
> +        error_report_err(error_abort);
> +    }
> +
> +    memory_map_init();

This is global init, please move it elsewhere.

> +
> +    system_memory = get_system_memory();
> +    system_io = get_system_io();
> +
> +    pci_memory = g_new(MemoryRegion, 1);
> +    memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
> +
> +    rem_host = REMOTE_HOST_DEVICE(qdev_create(NULL, TYPE_REMOTE_HOST_DEVICE));
> +
> +    rem_host->mr_pci_mem = pci_memory;
> +    rem_host->mr_sys_mem = system_memory;
> +    rem_host->mr_sys_io = system_io;
> +
> +    s->host = rem_host;

Both s and rem_host are QOM objects.  There should be a child property
relationship between them here.  It will ensure that rem_host is cleaned
up when s is cleaned up.  Please use that instead of a regular C
pointer.
Jag Raman Nov. 18, 2019, 3:29 p.m. UTC | #2 
On 11/13/2019 11:22 AM, Stefan Hajnoczi wrote:
> On Thu, Oct 24, 2019 at 05:08:51AM -0400, Jagannathan Raman wrote:
>> +static NotifierList machine_init_done_notifiers =
>> +    NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers);
>> +
>> +bool machine_init_done;
>> +
>> +void qemu_add_machine_init_done_notifier(Notifier *notify)
>> +{
>> +    notifier_list_add(&machine_init_done_notifiers, notify);
>> +    if (machine_init_done) {
>> +        notify->notify(notify, NULL);
>> +    }
>> +}
>> +
>> +void qemu_remove_machine_init_done_notifier(Notifier *notify)
>> +{
>> +    notifier_remove(notify);
>> +}
>> +
>> +void qemu_run_machine_init_done_notifiers(void)
>> +{
>> +    machine_init_done = true;
>> +    notifier_list_notify(&machine_init_done_notifiers, NULL);
>> +}
> 
> qemu_add_machine_init_done_notifier() is already defined in vl.c.
> Please share the implementation instead of duplicating it into the
> remote program.
> 
>> +
>> +static void remote_machine_init(Object *obj)
>> +{
>> +    RemMachineState *s = REMOTE_MACHINE(obj);
>> +    RemPCIHost *rem_host;
>> +    MemoryRegion *system_memory, *system_io, *pci_memory;
>> +
>> +    Error *error_abort = NULL;
>> +
>> +    qemu_mutex_init(&ram_list.mutex);
> 
> Please keep global initialization separate from RemMachineState (e.g. do
> it in main() or a function called by main()).  This function should only
> initialize RemMachineState.

OK, will do!

> 
>> +
>> +    object_property_add_child(object_get_root(), "machine", obj, &error_abort);
>> +    if (error_abort) {
>> +        error_report_err(error_abort);
>> +    }
>> +
>> +    memory_map_init();
> 
> This is global init, please move it elsewhere.

Got it, thank you!

> 
>> +
>> +    system_memory = get_system_memory();
>> +    system_io = get_system_io();
>> +
>> +    pci_memory = g_new(MemoryRegion, 1);
>> +    memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
>> +
>> +    rem_host = REMOTE_HOST_DEVICE(qdev_create(NULL, TYPE_REMOTE_HOST_DEVICE));
>> +
>> +    rem_host->mr_pci_mem = pci_memory;
>> +    rem_host->mr_sys_mem = system_memory;
>> +    rem_host->mr_sys_io = system_io;
>> +
>> +    s->host = rem_host;
> 
> Both s and rem_host are QOM objects.  There should be a child property
> relationship between them here.  It will ensure that rem_host is cleaned
> up when s is cleaned up.  Please use that instead of a regular C
> pointer.

OK, will add a property linking these two as parent-child.

Thank you!
--
Jag

>
diff mbox series

Patch

diff --git a/exec.c b/exec.c
index 08c4181..129a8a6 100644
--- a/exec.c
+++ b/exec.c
@@ -192,7 +192,6 @@  typedef struct subpage_t {
 #define PHYS_SECTION_UNASSIGNED 0
 
 static void io_mem_init(void);
-static void memory_map_init(void);
 static void tcg_log_global_after_sync(MemoryListener *listener);
 static void tcg_commit(MemoryListener *listener);
 
@@ -2989,7 +2988,7 @@  static void tcg_commit(MemoryListener *listener)
     tlb_flush(cpuas->cpu);
 }
 
-static void memory_map_init(void)
+void memory_map_init(void)
 {
     system_memory = g_malloc(sizeof(*system_memory));
 
diff --git a/include/exec/address-spaces.h b/include/exec/address-spaces.h
index db8bfa9..56a877b 100644
--- a/include/exec/address-spaces.h
+++ b/include/exec/address-spaces.h
@@ -33,6 +33,8 @@  MemoryRegion *get_system_memory(void);
  */
 MemoryRegion *get_system_io(void);
 
+void memory_map_init(void);
+
 extern AddressSpace address_space_memory;
 extern AddressSpace address_space_io;
 
diff --git a/include/remote/machine.h b/include/remote/machine.h
new file mode 100644
index 0000000..a00732d
--- /dev/null
+++ b/include/remote/machine.h
@@ -0,0 +1,46 @@ 
+/*
+ * Remote machine configuration
+ *
+ * Copyright 2019, Oracle and/or its affiliates.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef REMOTE_MACHINE_H
+#define REMOTE_MACHINE_H
+
+#include "qemu/osdep.h"
+#include "qom/object.h"
+#include "hw/boards.h"
+#include "remote/pcihost.h"
+#include "qemu/notify.h"
+
+typedef struct RemMachineState {
+    MachineState parent_obj;
+
+    RemPCIHost *host;
+} RemMachineState;
+
+#define TYPE_REMOTE_MACHINE "remote-machine"
+#define REMOTE_MACHINE(obj) \
+    OBJECT_CHECK(RemMachineState, (obj), TYPE_REMOTE_MACHINE)
+
+void qemu_run_machine_init_done_notifiers(void);
+
+#endif
diff --git a/remote/Makefile.objs b/remote/Makefile.objs
index 2757f5a..13d4c48 100644
--- a/remote/Makefile.objs
+++ b/remote/Makefile.objs
@@ -1,2 +1,3 @@ 
 remote-pci-obj-$(CONFIG_MPQEMU) += remote-main.o
 remote-pci-obj-$(CONFIG_MPQEMU) += pcihost.o
+remote-pci-obj-$(CONFIG_MPQEMU) += machine.o
diff --git a/remote/machine.c b/remote/machine.c
new file mode 100644
index 0000000..4ce197d
--- /dev/null
+++ b/remote/machine.c
@@ -0,0 +1,118 @@ 
+/*
+ * Machine for remote device
+ *
+ * Copyright 2019, Oracle and/or its affiliates.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include <stdint.h>
+#include <sys/types.h>
+
+#include "qemu/osdep.h"
+#include "remote/pcihost.h"
+#include "remote/machine.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "exec/ioport.h"
+#include "exec/ramlist.h"
+#include "qemu/thread.h"
+#include "qom/object.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "qemu/main-loop.h"
+#include "qemu-common.h"
+#include "sysemu/sysemu.h"
+#include "qemu/notify.h"
+
+static NotifierList machine_init_done_notifiers =
+    NOTIFIER_LIST_INITIALIZER(machine_init_done_notifiers);
+
+bool machine_init_done;
+
+void qemu_add_machine_init_done_notifier(Notifier *notify)
+{
+    notifier_list_add(&machine_init_done_notifiers, notify);
+    if (machine_init_done) {
+        notify->notify(notify, NULL);
+    }
+}
+
+void qemu_remove_machine_init_done_notifier(Notifier *notify)
+{
+    notifier_remove(notify);
+}
+
+void qemu_run_machine_init_done_notifiers(void)
+{
+    machine_init_done = true;
+    notifier_list_notify(&machine_init_done_notifiers, NULL);
+}
+
+static void remote_machine_init(Object *obj)
+{
+    RemMachineState *s = REMOTE_MACHINE(obj);
+    RemPCIHost *rem_host;
+    MemoryRegion *system_memory, *system_io, *pci_memory;
+
+    Error *error_abort = NULL;
+
+    qemu_mutex_init(&ram_list.mutex);
+
+    object_property_add_child(object_get_root(), "machine", obj, &error_abort);
+    if (error_abort) {
+        error_report_err(error_abort);
+    }
+
+    memory_map_init();
+
+    system_memory = get_system_memory();
+    system_io = get_system_io();
+
+    pci_memory = g_new(MemoryRegion, 1);
+    memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
+
+    rem_host = REMOTE_HOST_DEVICE(qdev_create(NULL, TYPE_REMOTE_HOST_DEVICE));
+
+    rem_host->mr_pci_mem = pci_memory;
+    rem_host->mr_sys_mem = system_memory;
+    rem_host->mr_sys_io = system_io;
+
+    s->host = rem_host;
+
+    qemu_mutex_lock_iothread();
+    memory_region_add_subregion_overlap(system_memory, 0x0, pci_memory, -1);
+    qemu_mutex_unlock_iothread();
+
+    qdev_init_nofail(DEVICE(rem_host));
+}
+
+static const TypeInfo remote_machine = {
+    .name = TYPE_REMOTE_MACHINE,
+    .parent = TYPE_MACHINE,
+    .instance_size = sizeof(RemMachineState),
+    .instance_init = remote_machine_init,
+};
+
+static void remote_machine_register_types(void)
+{
+    type_register_static(&remote_machine);
+}
+
+type_init(remote_machine_register_types);
diff --git a/remote/remote-main.c b/remote/remote-main.c
index cccad12..bf44310 100644
--- a/remote/remote-main.c
+++ b/remote/remote-main.c
@@ -28,10 +28,17 @@ 
 #include <stdio.h>
 
 #include "qemu/module.h"
+#include "remote/pcihost.h"
+#include "remote/machine.h"
+#include "hw/boards.h"
+#include "hw/qdev-core.h"
+#include "qemu/main-loop.h"
 
 int main(int argc, char *argv[])
 {
     module_call_init(MODULE_INIT_QOM);
 
+    current_machine = MACHINE(REMOTE_MACHINE(object_new(TYPE_REMOTE_MACHINE)));
+
     return 0;
 }