Re: [Qemu-devel] [PATCH for-1.4 07/12] target-i386/cpu: Introduce apic_id_for_cpu() function

[Eduardo Habkost]

On Mon, Jan 21, 2013 at 04:39:56PM +0100, Andreas Färber wrote:
> Am 21.01.2013 12:31, schrieb Eduardo Habkost:
> > On Mon, Jan 21, 2013 at 12:18:55PM +0100, Andreas Färber wrote:
> >> Am 17.01.2013 21:59, schrieb Eduardo Habkost:
> >>> This function will be used by both the CPU initialization code and the
> >>> fw_cfg table initialization code.
> >>>
> >>> Later this function will be updated to generate APIC IDs according to
> >>> the CPU topology.
> >>>
> >>> Signed-off-by: Eduardo Habkost <address@hidden>
> >>> ---
> >>>  target-i386/cpu.c | 17 ++++++++++++++++-
> >>>  target-i386/cpu.h |  2 ++
> >>>  2 files changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> >>> index d1a14d5..d90789d 100644
> >>> --- a/target-i386/cpu.c
> >>> +++ b/target-i386/cpu.c
> >>> @@ -2194,6 +2194,21 @@ void x86_cpu_realize(Object *obj, Error **errp)
> >>>      cpu_reset(CPU(cpu));
> >>>  }
> >>>  
> >>> +/* Calculates initial APIC ID for a specific CPU index
> >>> + *
> >>> + * Currently we need to be able to calculate the APIC ID from the CPU 
> >>> index
> >>> + * alone (without requiring a CPU object), as the QEMU<->Seabios 
> >>> interfaces have
> >>> + * no concept of "CPU index", and the NUMA tables on fw_cfg need the 
> >>> APIC ID of
> >>> + * all CPUs up to max_cpus.
> >>> + */
> >>> +uint32_t apic_id_for_cpu(unsigned int cpu_index)
> >>
> >> Can we rather make this x86_cpu_apic_id(X86CPU *cpu) to account for
> >> future changes to topology modelling?
> > 
> > We can't make it get a X86CPU as parameter, because the ACPI tables have
> > to be built up to max_cpus, before the CPUs get actually created. But it
> > can be renamed, yes.
> 
> I see. Is this a limitation of the current code?

No, it is a requirement of the BIOS: it needs to know in advance the
list of APIC IDs of all possible CPUs up to max_cpus, so it can build
the ACPI tables properly.

> 
> IIUC the cpu_index is set once during qemu_init_vcpu() by counting the
> CPUArchStates in the first_cpu list. So if we hot-unplug a non-last CPU
> this is gonna result in chaos. Therefore I was hoping we could do
> something more clever than hardcoding cpu_index in our internal API ...
> but we can refactor that as follow-up when the need comes up.

So we have to fix the way the CPU index is calculated when we
implemented hotplug/hotunplug. We need a way to identify the CPU we are
calculating the APIC ID for, and today the only "CPU identifier" we have
is the CPU index.

In the future we can consider making apic_id_for_cpu() get the
package/core/thread IDs directly. But to do that, we first need to
actually model the package/core/thread lists in the PC CPU creation
code.

-- 
Eduardo