Re: [Qemu-devel] [PATCH 2/7] Convert pc cpu to qdev

[Igor Mammedov <imammedo@redhat.com>]

On 02/16/2012 05:09 PM, Andreas Färber wrote:
> Am 16.02.2012 00:16, schrieb Igor Mammedov:
>> Convert pc cpu to qdev device that is attached to icc bus, later
>> hot-plug ability of icc bus will allow to implement cpu hot-plug.
>>
>> Signed-off-by: Igor Mammedov<imammedo@redhat.com>
>
> This conflicts with CPU QOM'ification across targets (and no longer
> applies due to type_init() introduction).
>
>> ---
>>   hw/pc.c              |   62 +++++++++++++++++++++++++++++++++++++++++++------
>>   target-i386/cpu.h    |    1 +
>>   target-i386/helper.c |   26 ++++++++++++++------
>>   3 files changed, 73 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/pc.c b/hw/pc.c
>> index 33d8090..b8db5dc 100644
>> --- a/hw/pc.c
>> +++ b/hw/pc.c
>> @@ -922,6 +922,12 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
>>       }
>>   }
>>
>> +typedef struct CPUPC {
>> +    ICCBusDevice busdev;
>> +    char *model;
>> +    CPUState state;
>> +} CPUPC;
>
> I don't like this approach. For starters, using CPUState as field rather
> than pointer seems a bad idea to me (CPUX86State would only be slightly
> better). We should have a single code path through which we instantiate
> CPUs, currently: cpu_$arch_init(const char *cpu_model)
> With my series completed, this would return an X86CPU object.
>
> Depending on our liking, we could either place some ICC / APIC /
> whatever fields directly into that, or embed the X86CPU in an object
> such as yours above as a link<X86CPU>. I do feel however that the model
> string is misplaced there. Question is whether this ICC stuff is
> actually part of the CPU or part of the CPU wiring on the mainboard - I
> vaguely remember someone saying that this changed over time...? Having
Yep, since P4 times sysbus used instead of icc so we can just ignore icc.

> both depending on CPU subclass might also be an option, but I'd rather
> leave such decisions as a follow-up to the core QOM'ification.
>

With QOM and your work this patch is obsolete. I see you've already QOM-ified
X86CPU in your qom-cpu tree. With your permission I'll play with it and check
what could be done for cpu hot-plug feature.

-- 
Thanks,
  Igor