+Eduardo as target-i386 maintainer
Am 02.04.2015 um 14:34 schrieb Paolo Bonzini:
>
>
> 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 <na...@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.
I don't understand why this is necessary: The cpu_index doesn't change,
therefore the BSP designation won't change either. Is this a hot-unplug
preparation? Or is a KVM-specific call missing here? Either way, the
commit message could use some clarification.
The API change itself looks okay.
Regards,
Andreas
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu,
Graham Norton; HRB 21284 (AG Nürnberg)
