[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 2/2] cpu: for cpu-user and cpu-softmmu and make

From: Andreas Färber
Subject: Re: [Qemu-devel] [PATCH 2/2] cpu: for cpu-user and cpu-softmmu and make cpu-softmmu a child of DeviceState
Date: Sat, 23 Jun 2012 16:00:28 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:12.0) Gecko/20120421 Thunderbird/12.0

Am 22.06.2012 21:36, schrieb Anthony Liguori:
> The line between linux-user and softmmu is not very well defined right now.
> linux-user really don't want to include devices and making CpuState a child of
> DeviceState would require pulling lots of stuff into linux-user.
> To solve this, we simply fork cpu-user and cpu-softmmu letting them evolve
> independently.

I still don't understand why.

>From your announcement on IRC I thought that you would leave TYPE_CPU as
is and introdruce TYPE_SOFTMMU_CPU / TYPE_USER_CPU as derived
intermediate types. But here you are just replacing my suggested #ifdefs
with code duplication...

The QOM CPUState part 4 series on the list I reminded you of starts
moving more stuff into struct CPUState, which is supposed to be used by
common code, i.e., both softmmu and *-user.

With this patch of yours I'd need to move the fields from central
CPU_COMMON into *two* structs kept in sync, which doesn't feel like an
advantage to me. Apart from fields for include/qemu/cpu.h I was also
planning on placing helper functions into qom/cpu.c, which now would
need to be duplicated, like cpu_common_reset() for the fields added. So
if there's some error it is likely to get fixed in one version but
forgotten in the other one. That would be really bad. If functions do
mostly the same thing they should be compiled from the same source -
basics of Product Line Engineering. If there's functions that only apply
to one of them then I'm fine placing them in a cpu-softmmu or cpu-user
file respectively. But this patch 2/2 just seems to make more work
without real gains to avoid #ifdefs - we'll get some anyway due to w32
and unrolling those into even more file copies sounds even worse.

Apart from tcg/, which you keep iterating, linux-user also reuses most
of target-*/ (except for supervisor- and hypervisor-level instructions)
as well as core CPU execution infrastructure, which I'm trying to
streamline through CPUState compiled only twice rather than per each
*-softmmu and *-*-user.

So IMO the key point is that we don't want them to evolve independently.
We want to code efficiently and to build QEMU efficiently.


For reference my previous suggestion for CPUState-as-a-DeviceState was:

diff --git a/include/qemu/cpu.h b/include/qemu/cpu.h
index 78b65b3..a4d84a5 100644
--- a/include/qemu/cpu.h
+++ b/include/qemu/cpu.h
@@ -21,6 +21,9 @@
 #define QEMU_CPU_H

 #include "qemu/object.h"
+#include "hw/qdev.h"

  * SECTION:cpu
@@ -45,7 +48,11 @@ typedef struct CPUState CPUState;
 typedef struct CPUClass {
     /*< private >*/
     ObjectClass parent_class;
+    DeviceClass parent_class;
     /*< public >*/

     void (*reset)(CPUState *cpu);
diff --git a/qom/cpu.c b/qom/cpu.c
index 5b36046..17b796f 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -36,14 +36,26 @@ static void cpu_common_reset(CPUState *cpu)

 static void cpu_class_init(ObjectClass *klass, void *data)
+    DeviceClass *dc = DEVICE_CLASS(klass);
     CPUClass *k = CPU_CLASS(klass);

+    /* Overwrite this in subclasses for which hotplug is supported. */
+    dc->no_user = 1;
     k->reset = cpu_common_reset;

 static TypeInfo cpu_type_info = {
     .name = TYPE_CPU,
     .parent = TYPE_OBJECT,
+    .parent = TYPE_DEVICE,
     .instance_size = sizeof(CPUState),
     .abstract = true,
     .class_size = sizeof(CPUClass),

SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

reply via email to

[Prev in Thread] Current Thread [Next in Thread]