| Message ID | 1358456378-29248-8-git-send-email-ehabkost@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 <ehabkost@redhat.com> > --- > 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? Andreas > +{ > + /* right now APIC ID == CPU index. this will eventually change to use > + * the CPU topology configuration properly > + */ > + return cpu_index; > +} > + > static void x86_cpu_initfn(Object *obj) > { > CPUState *cs = CPU(obj); > @@ -2228,7 +2243,7 @@ static void x86_cpu_initfn(Object *obj) > x86_cpuid_get_tsc_freq, > x86_cpuid_set_tsc_freq, NULL, NULL, NULL); > > - env->cpuid_apic_id = cs->cpu_index; > + env->cpuid_apic_id = apic_id_for_cpu(cs->cpu_index); > > /* init various static tables used in TCG mode */ > if (tcg_enabled() && !inited) { > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 9d4fcf9..d86c0af 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -1255,4 +1255,6 @@ void disable_kvm_pv_eoi(void); > /* Return name of 32-bit register, from a R_* constant */ > const char *get_register_name_32(unsigned int reg); > > +uint32_t apic_id_for_cpu(unsigned int cpu_index); > + > #endif /* CPU_I386_H */
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 <ehabkost@redhat.com> > > --- > > 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.
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 <ehabkost@redhat.com> >>> --- >>> 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? 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. Andreas
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 <ehabkost@redhat.com> > >>> --- > >>> 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.
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) +{ + /* right now APIC ID == CPU index. this will eventually change to use + * the CPU topology configuration properly + */ + return cpu_index; +} + static void x86_cpu_initfn(Object *obj) { CPUState *cs = CPU(obj); @@ -2228,7 +2243,7 @@ static void x86_cpu_initfn(Object *obj) x86_cpuid_get_tsc_freq, x86_cpuid_set_tsc_freq, NULL, NULL, NULL); - env->cpuid_apic_id = cs->cpu_index; + env->cpuid_apic_id = apic_id_for_cpu(cs->cpu_index); /* init various static tables used in TCG mode */ if (tcg_enabled() && !inited) { diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 9d4fcf9..d86c0af 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -1255,4 +1255,6 @@ void disable_kvm_pv_eoi(void); /* Return name of 32-bit register, from a R_* constant */ const char *get_register_name_32(unsigned int reg); +uint32_t apic_id_for_cpu(unsigned int cpu_index); + #endif /* CPU_I386_H */
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 <ehabkost@redhat.com> --- target-i386/cpu.c | 17 ++++++++++++++++- target-i386/cpu.h | 2 ++ 2 files changed, 18 insertions(+), 1 deletion(-)