Re: [Qemu-devel] [RFC PATCH v0 8/9] target-i386: Set apic_id during CPU initfn

[Bharata B Rao]

On Mon, Dec 14, 2015 at 03:44:06PM -0200, Eduardo Habkost wrote:
> On Thu, Dec 10, 2015 at 11:45:43AM +0530, Bharata B Rao wrote:
> > Move back the setting of apic_id to instance_init routine (x86_cpu_initfn)
> > This is needed to initialize X86 CPUs using generic cpu-package device.
> 
> Could you explain where exactly apic_id will be used, to make it
> necessary to initialize it earlier?

There is a check in x86_cpu_realizefn() to see if apic_id has been
initialized properly. Hence I thought x86 target will require apic_id
to have been initialized before CPU realization and that is what
the existing code does via pc_cpus_init() and pc_new_cpu(). i.e.,
apic_id property is set before setting the realize property to true.
However...

> 
> > 
> > TODO: I am not fully aware of the general direction in which apic_id
> > changes in X86 have evolved and hence not sure if this is indeed aligned 
> > with
> > the X86 way of doing things. This is just to help the PoC implementation
> > that I have in this patchset to convert PC CPUs initialization into
> > cpu-package device based initialization.
> 
> You shouldn't initialize apic_id on initfn. APIC ID depends (and
> will depend) on different CPU properties related to topology,
> including (but not limited to) CPU index and CPU topology
> properties we may introduce in the future, so it should be done
> later (at realize time), not on initfn.

... with the current patchset, I just experimented now by moving the setting
of apic_id to x86_cpu_realizefn() and things work just fine. I was in fact
pleasantly surprised to see that I could hot add a cpu core by hot plugging
the cpu-core device on x86 too.

> 
> Also, cpu_index is initialized by cpu_exec_init(), and
> cpu_exec_init() must not be called by initfn. The cpu_exec_init()
> call should (and will) be moved to realize in x86 and all other
> architectures.

Right, I have already moved cpu_exec_init() call to realizefn for PowerPC.

Regards,
Bharata.