On Wed, Dec 05, 2012 at 03:48:10PM +0100, Andreas Färber wrote:
> Am 04.12.2012 14:19, schrieb Eduardo Habkost:
[...]
> > @@ -43,7 +44,7 @@ static void cpu_class_init(ObjectClass *klass, void *data)
> >
> > static TypeInfo cpu_type_info = {
> > .name = TYPE_CPU,
> > - .parent = TYPE_OBJECT,
> > + .parent = TYPE_DEVICE,
> > .instance_size = sizeof(CPUState),
> > .abstract = true,
> > .class_size = sizeof(CPUClass),
>
> This makes the CPU a device, allowing the user to specify it with
> -device. My preference would be to disable that at first[1] by setting
> DeviceClass::no_user = 1.
I didn't know no_user existed. It makes sense to set it by now, yes.
>
> Have you tested what happens if someone tries to hotplug a CPU device?
> It may be the first device without bus...
I don't know, but I won't be surprised if stuff breaks horribly.
>
> [1] Anthony's and my idea was to handle hotplug at a higher level than
> CPUState - X86Socket containing X86Core containing X86Thread or so.
Yes, and I agree with this approach.
> This
> would require me (or someone) to refactor CPU_COMMON's numa_node (also
> used in sPAPR), nr_cores, nr_threads (also used in mips/Malta) - in a
> non-trivial way. We may need to go from CPU*State to CPUState (possible
> so far) to Core to Socket, for which object_get_parent() would be
> helpful. So far Object::parent is declared private.
Why, exactly, the CPUX86State, CPUThread, or CPUCore objects would need
to access their parents directly? We could just make sure that the
parent provide the necessary information to its children, when creating
them.
The other side of that question is: why exactly we don't allow an object
to know its parent, by design? What's the right mechanism to be used
when a device really needs to send some signal to its parent?
> Are we targetting to do this is two steps, using CPUState at first? Or
> has one of you been investigating how involved this redesign would be?
What do you mean by "using CPUState at first"? I mean, I think we'll
always use CPUState in the code that represents a single VCPU/thread,
and it is not going away soon. The name "CPUState" may be a bit
confusing, though, as it contains the state of a single VCPU/thread (not
a CPU socket).
If you are asking about "qdevifying/subclassing CPUState first", I think
the answer is: yes, making it in two steps sounds better. If we use
no_user, we can more easily change the hierarchy later because there
would be nobody using "-device" to create CPUs yet. And if we have to
design and redo a whole socket/core/thread CPU class hierarchy first, I
don't expect us to finish doing this before QEMU 1.5[1].
When implementing an socket-based interface, I think we may end up with
something like:
- CPUSocket (or CPUPackage)
- creates multiple CPUCore children
- CPUCore
- creates multiple CPUThread children
- maybe have CPUState children directly, to make things simpler
- CPUThread
- creates one CPUState child
- (maybe CPUState can be used directly)
- CPUState
- the class we already have today
- could be renamed to VCPUState or ThreadState, to make it clearer
- CPUID data is handled here
- CPU feature configuration is handled here
- Will have one subclass for each CPU model
Some variations/alternatives I see:
* Just having two levels. e.g.:
- CPUSocket (or CPUPackage)
- creates multiple CPUState children, depending on nr_cores/nr_threads
configuration
- CPUState
- the class we already have today
- could be renamed to VCPUState or ThreadState, to make it clearer
- CPUID data is handled here
- CPU feature configuration is handled here
- Will have one subclass for each CPU model
* Having CPU model subclasses at the CPUSocket level instead of
CPUState level
- It would make the CPUID code really messy: some CPUID bits would
come from the CPUSocket subclass, other from the CPUState itself.
- On the other hand, the external interface may make more sense
if we do it that way. I mean: "create a 4-core SandyBridge CPU
[package]" sounds more logical than "create a CPU package with 4
SandyBridge threads inside it".
[1] Just to explain my expectations: what I _really_ want to have on
QEMU 1.4 is:
- A good machine-type compatibility mechanism that allows us to
update CPU model definitions while keeping compatibility
- Fortunately, the current global-variable-based approach kind-of
works
- But CPU subclasses/properties would give us a cleaner solution
for free (as we could simply use machine-type global properties)
- Any interface that libvirt can use to query for CPU model
information, including:
- Listing available CPU models
- This exists, but it doesn't provide much detail
- Checking which features are going to be enabled by each CPU model
- Checking which features can really be enabled on a host
(considering QEMU + kernel + hardware capabilities)
- CPU subclasses/properties give us (most of) the above for free
I wouldn't want to delay the CPU subclass/property work because of a
CPU socket/core/thread modelling rework, as it would means delaying
(again) the interfaces that libvirt really needs.
--
Eduardo
