Re: [Qemu-devel] target-i386: clear bsp bit when designating bsp

[Paolo Bonzini <pbonzini@redhat.com>]

On 02/04/2015 01:58, Nadav Amit wrote:
> Since the BSP bit is writable on real hardware, during reset all the CPUs which
> were not chosen to be the BSP should have their BSP bit cleared. This fix is
> required for KVM to work correctly when it changes the BSP bit.
> 
> An additional fix is required for QEMU tcg to allow software to change the BSP
> bit.
> 
> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il>
> ---
>  hw/intc/apic_common.c  | 8 ++++++--
>  include/hw/i386/apic.h | 2 +-
>  target-i386/cpu.c      | 4 +---
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 0858b45..042e960 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -215,14 +215,18 @@ void apic_init_reset(DeviceState *dev)
>      }
>  }
>  
> -void apic_designate_bsp(DeviceState *dev)
> +void apic_designate_bsp(DeviceState *dev, bool bsp)
>  {
>      if (dev == NULL) {
>          return;
>      }
>  
>      APICCommonState *s = APIC_COMMON(dev);
> -    s->apicbase |= MSR_IA32_APICBASE_BSP;
> +    if (bsp) {
> +        s->apicbase |= MSR_IA32_APICBASE_BSP;
> +    } else {
> +        s->apicbase &= ~MSR_IA32_APICBASE_BSP;
> +    }
>  }
>  
>  static void apic_reset_common(DeviceState *dev)
> diff --git a/include/hw/i386/apic.h b/include/hw/i386/apic.h
> index 1d48e02..51eb6d3 100644
> --- a/include/hw/i386/apic.h
> +++ b/include/hw/i386/apic.h
> @@ -21,7 +21,7 @@ void apic_sipi(DeviceState *s);
>  void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
>                                     TPRAccess access);
>  void apic_poll_irq(DeviceState *d);
> -void apic_designate_bsp(DeviceState *d);
> +void apic_designate_bsp(DeviceState *d, bool bsp);
>  
>  /* pc.c */
>  DeviceState *cpu_get_current_apic(void);
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index b2d1c95..03b33cf 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -2714,9 +2714,7 @@ static void x86_cpu_reset(CPUState *s)
>  
>  #if !defined(CONFIG_USER_ONLY)
>      /* We hard-wire the BSP to the first CPU. */
> -    if (s->cpu_index == 0) {
> -        apic_designate_bsp(cpu->apic_state);
> -    }
> +    apic_designate_bsp(cpu->apic_state, s->cpu_index == 0);
>  
>      s->halted = !cpu_is_bsp(cpu);
>  
> 

Thanks, applied locally.

Paolo