Am 17.04.2012 12:28, schrieb Igor Mammedov:
> ----- Original Message -----
>> From: "Andreas Färber" <afaer...@suse.de>
>> To: "Paolo Bonzini" <pbonz...@redhat.com>, "Igor Mammedov"
>> <imamm...@redhat.com>
>> Cc: qemu-devel@nongnu.org, aligu...@us.ibm.com, "jan kiszka"
>> <jan.kis...@siemens.com>
>> Sent: Tuesday, April 17, 2012 10:46:14 AM
>> Subject: Re: [PATCH RFC 6/6] target-i386: make cpus childs of /machine
>>
>> Am 17.04.2012 09:19, schrieb Paolo Bonzini:
>>> Il 17/04/2012 01:37, Igor Mammedov ha scritto:
>>>> From: Igor Mammedov <niall...@gmail.com>
>>>>
>>>> Signed-off-by: Igor Mammedov <niall...@gmail.com>
>>>> ---
>>>> target-i386/helper.c | 4 ++++
>>>> 1 files changed, 4 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/target-i386/helper.c b/target-i386/helper.c
>>>> index de7637c..1996b97 100644
>>>> --- a/target-i386/helper.c
>>>> +++ b/target-i386/helper.c
>>>> @@ -1134,6 +1134,7 @@ CPUX86State *cpu_x86_init(const char
>>>> *cpu_model)
>>>> X86CPU *cpu;
>>>> CPUX86State *env;
>>>> Error *errp = NULL;
>>>> + char cpuname[8];
>>>>
>>>> cpu = X86_CPU(object_new(TYPE_X86_CPU));
>>>> env = &cpu->env;
>>>> @@ -1146,6 +1147,9 @@ CPUX86State *cpu_x86_init(const char
>>>> *cpu_model)
>>>> }
>>>> }
>>>>
>>>> + snprintf(cpuname, sizeof(cpuname), "cpu%d",
>>>> env->cpuid_apic_id);
>>>> + object_property_add_child(container_get("/machine"), cpuname,
>>>> OBJECT(cpu), NULL);
>>>> +
>>>> object_property_set_bool(OBJECT(cpu), true, "realized",
>>>> &errp);
>>>> if (errp) {
>>>> object_delete(OBJECT(cpu));
>>>
>>> I think the right name would be /machine/cpu[%d]/cpu. The local
>>> APIC
>>> for example should reside under /machine/cpu[%d]/apic.
>>
>> Depends on how we model the CPU, I was kinda waiting for feedback on
>> that.
>>
>> I would prefer /machine/cpu[%d], with the APIC being a child
>> .../apic,
>> if possible. That however depends on how the device'ification of the
>> CPU
> Ok, I'll change /machine/cpu%d to /machine/cpu[%d].
Note that I needed a similar patch recently in my quest to move fields
from CPU_COMMON into CPUState:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu (WIP)
current state:
https://github.com/afaerber/qemu-cpu/commit/1318ad13cda52e694caea5cc72293c5879db3f9f
I chose to do it in pc.c instead because for other architectures the
placement will be a machine-specific decision.
I've used cpu_index, but it seems cpuid_apic_id is assigned only once,
from cpu_index, so it should be identical. What's the difference?
Comparing our code I see that I do an unnecessary NUL termination after
snprintf(), whereas your buffer should probably be 9 chars to account
for a maximum of 255 CPUs (3 letters, 2 square brackets, 3 digits, NUL).
In particular I needed /machine/cpu[n] to pass the X86CPU to the APIC in
a sensible way - next commit on that branch, currently:
https://github.com/afaerber/qemu-cpu/commit/1134efc143aa1629f8961ef058416e1acfa50f8e
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
