Am 12.07.2012 15:22, schrieb Igor Mammedov:
> This patch:
> - moves decision to designate BSP from board into cpu, making cpu
> self-sufficient in this regard. Later it will allow to cleanup hw/pc.c
> and remove cpu_reset and wrappers from there.
> - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior
> described in Inted SDM vol 3a part 1 chapter 8.4.1
> - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu is BSP
>
> patch is based on Jan Kiszka's proposal:
> http://thread.gmane.org/gmane.comp.emulators.qemu/100806
>
> v2:
> - fix build for i386-linux-user
> spotted-by: Peter Maydell <peter.mayd...@linaro.org>
> v3:
> - style change requested by Andreas Färber <afaer...@suse.de>
>
> v4:
> - reuse cpu_is_bsp() rather than open code check if apicbase has BSP bit set
> requested by Gleb Natapov <g...@redhat.com>
> - hijacked Andreas' patch [1] to use X86CPU instead of CPUX86State in
> cpu_is_bsp()
>
> 1) http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg03185.html
>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/apic.h | 5 ++++-
> hw/apic_common.c | 16 +++++++++++++---
> hw/pc.c | 9 ---------
> target-i386/cpu.c | 18 ++++++++++++++++++
> target-i386/helper.c | 1 -
> target-i386/kvm.c | 4 +++-
> 6 files changed, 38 insertions(+), 15 deletions(-)
>
> diff --git a/hw/apic.h b/hw/apic.h
> index 62179ce..4da10b6 100644
> --- a/hw/apic.h
> +++ b/hw/apic.h
> @@ -20,9 +20,12 @@ void apic_init_reset(DeviceState *s);
> void apic_sipi(DeviceState *s);
> void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
> TPRAccess access);
> +void apic_designate_bsp(DeviceState *d);
>
> /* pc.c */
> -int cpu_is_bsp(CPUX86State *env);
> DeviceState *cpu_get_current_apic(void);
>
> +/* cpu.c */
> +bool cpu_is_bsp(X86CPU *cpu);
> +
> #endif
> diff --git a/hw/apic_common.c b/hw/apic_common.c
> index 60b8259..58e63b0 100644
> --- a/hw/apic_common.c
> +++ b/hw/apic_common.c
> @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d)
> trace_cpu_get_apic_base((uint64_t)s->apicbase);
> return s->apicbase;
> } else {
> - trace_cpu_get_apic_base(0);
> - return 0;
> + trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP);
> + return MSR_IA32_APICBASE_BSP;
> }
> }
>
> @@ -201,13 +201,23 @@ void apic_init_reset(DeviceState *d)
> s->timer_expiry = -1;
> }
>
> +void apic_designate_bsp(DeviceState *d)
> +{
> + if (d == NULL) {
> + return;
> + }
> +
> + APICCommonState *s = APIC_COMMON(d);
> + s->apicbase |= MSR_IA32_APICBASE_BSP;
> +}
> +
> static void apic_reset_common(DeviceState *d)
> {
> APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
> bool bsp;
>
> - bsp = cpu_is_bsp(s->cpu_env);
> + bsp = cpu_is_bsp(x86_env_get_cpu(s->cpu_env));
> s->apicbase = 0xfee00000 |
> (bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
>
> diff --git a/hw/pc.c b/hw/pc.c
> index c7e9ab3..50c1715 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -871,12 +871,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
> nb_ne2k++;
> }
>
> -int cpu_is_bsp(CPUX86State *env)
> -{
> - /* We hard-wire the BSP to the first CPU. */
> - return env->cpu_index == 0;
> -}
> -
> DeviceState *cpu_get_current_apic(void)
> {
> if (cpu_single_env) {
> @@ -927,10 +921,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int
> level)
> static void pc_cpu_reset(void *opaque)
> {
> X86CPU *cpu = opaque;
> - CPUX86State *env = &cpu->env;
> -
> cpu_reset(CPU(cpu));
> - env->halted = !cpu_is_bsp(env);
> }
>
> static X86CPU *pc_new_cpu(const char *cpu_model)
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 5521709..0c38b7f 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1686,6 +1686,24 @@ static void x86_cpu_reset(CPUState *s)
> env->dr[7] = DR7_FIXED_1;
> cpu_breakpoint_remove_all(env, BP_CPU);
> cpu_watchpoint_remove_all(env, BP_CPU);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + /* We hard-wire the BSP to the first CPU. */
> + if (env->cpu_index == 0) {
> + apic_designate_bsp(env->apic_state);
> + }
> +
> + env->halted = !cpu_is_bsp(cpu);
> +#endif
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +bool cpu_is_bsp(X86CPU *cpu)
> +{
> + return cpu_get_apic_base(cpu->env.apic_state) & MSR_IA32_APICBASE_BSP;
> +}
> +#endif
> +
> }
I'm okay with this approach too, but I think the above brace is a merge
conflict?
Did you git-grep for "cpu_is_bsp" to be sure you caught all usages?
Andreas
>
> static void mce_init(X86CPU *cpu)
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index d3af6ea..b748d90 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1191,7 +1191,6 @@ void do_cpu_init(X86CPU *cpu)
> env->interrupt_request = sipi;
> env->pat = pat;
> apic_init_reset(env->apic_state);
> - env->halted = !cpu_is_bsp(env);
> }
>
> void do_cpu_sipi(X86CPU *cpu)
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 0d0d8f6..97a2cb1 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -579,11 +579,13 @@ int kvm_arch_init_vcpu(CPUX86State *env)
>
> void kvm_arch_reset_vcpu(CPUX86State *env)
> {
> + X86CPU *cpu = x86_env_get_cpu(env);
> +
> env->exception_injected = -1;
> env->interrupt_injected = -1;
> env->xcr0 = 1;
> if (kvm_irqchip_in_kernel()) {
> - env->mp_state = cpu_is_bsp(env) ? KVM_MP_STATE_RUNNABLE :
> + env->mp_state = cpu_is_bsp(cpu) ? KVM_MP_STATE_RUNNABLE :
> KVM_MP_STATE_UNINITIALIZED;
> } else {
> env->mp_state = KVM_MP_STATE_RUNNABLE;
>
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
