| Message ID | 1427719163-5429-6-git-send-email-namit@cs.technion.ac.il (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 30/03/2015 14:39, Nadav Amit wrote: > After reset, the CPU can change the BSP, which will be used upon INIT. Reset > should return the BSP which QEMU asked for, and therefore handled accordingly. > > To quote: "If the MP protocol has completed and a BSP is chosen, subsequent > INITs (either to a specific processor or system wide) do not cause the MP > protocol to be repeated." > [Intel SDM 8.4.2: MP Initialization Protocol Requirements and Restrictions] > > Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> Please provide a kvm-unit-tests testcase for this. Paolo > --- > arch/x86/kvm/lapic.c | 2 -- > arch/x86/kvm/svm.c | 2 +- > arch/x86/kvm/vmx.c | 2 +- > include/linux/kvm_host.h | 7 ++++++- > 4 files changed, 8 insertions(+), 5 deletions(-) > > diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c > index 17da6fc..b0dbf68 100644 > --- a/arch/x86/kvm/lapic.c > +++ b/arch/x86/kvm/lapic.c > @@ -1498,8 +1498,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value) > return; > } > > - if (!kvm_vcpu_is_bsp(apic->vcpu)) > - value &= ~MSR_IA32_APICBASE_BSP; > vcpu->arch.apic_base = value; > > /* update jump label if enable bit changes */ > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 1ef4c0d..ef5bf21 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -1261,7 +1261,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) > > svm->vcpu.arch.apic_base = APIC_DEFAULT_PHYS_BASE | > MSR_IA32_APICBASE_ENABLE; > - if (kvm_vcpu_is_bsp(&svm->vcpu)) > + if (kvm_vcpu_is_reset_bsp(&svm->vcpu)) > svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP; > > svm_init_osvw(&svm->vcpu); > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 8aee6db..7e370b2 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -4709,7 +4709,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) > if (!init_event) { > apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | > MSR_IA32_APICBASE_ENABLE; > - if (kvm_vcpu_is_bsp(&vmx->vcpu)) > + if (kvm_vcpu_is_reset_bsp(&vmx->vcpu)) > apic_base_msr.data |= MSR_IA32_APICBASE_BSP; > apic_base_msr.host_initiated = true; > kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 0f574eb..8365cae 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -968,11 +968,16 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > #endif /* CONFIG_HAVE_KVM_EVENTFD */ > > #ifdef CONFIG_KVM_APIC_ARCHITECTURE > -static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) > +static inline bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu) > { > return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id; > } > > +static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) > +{ > + return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0; > +} > + > bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu); > > #else > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Paolo, It appears you are right and I have not tested 4 and 5 well enough. I’ll repost them (the others were tested presumably well enough). Two short questions: Can I use init.c in the kvm-unit-tests ? Why is it disabled? Nadav Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 30/03/2015 14:39, Nadav Amit wrote: >> After reset, the CPU can change the BSP, which will be used upon INIT. Reset >> should return the BSP which QEMU asked for, and therefore handled accordingly. >> >> To quote: "If the MP protocol has completed and a BSP is chosen, subsequent >> INITs (either to a specific processor or system wide) do not cause the MP >> protocol to be repeated." >> [Intel SDM 8.4.2: MP Initialization Protocol Requirements and Restrictions] >> >> Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> > > Please provide a kvm-unit-tests testcase for this. > > Paolo > >> --- >> arch/x86/kvm/lapic.c | 2 -- >> arch/x86/kvm/svm.c | 2 +- >> arch/x86/kvm/vmx.c | 2 +- >> include/linux/kvm_host.h | 7 ++++++- >> 4 files changed, 8 insertions(+), 5 deletions(-) >> >> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c >> index 17da6fc..b0dbf68 100644 >> --- a/arch/x86/kvm/lapic.c >> +++ b/arch/x86/kvm/lapic.c >> @@ -1498,8 +1498,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value) >> return; >> } >> >> - if (!kvm_vcpu_is_bsp(apic->vcpu)) >> - value &= ~MSR_IA32_APICBASE_BSP; >> vcpu->arch.apic_base = value; >> >> /* update jump label if enable bit changes */ >> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c >> index 1ef4c0d..ef5bf21 100644 >> --- a/arch/x86/kvm/svm.c >> +++ b/arch/x86/kvm/svm.c >> @@ -1261,7 +1261,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) >> >> svm->vcpu.arch.apic_base = APIC_DEFAULT_PHYS_BASE | >> MSR_IA32_APICBASE_ENABLE; >> - if (kvm_vcpu_is_bsp(&svm->vcpu)) >> + if (kvm_vcpu_is_reset_bsp(&svm->vcpu)) >> svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP; >> >> svm_init_osvw(&svm->vcpu); >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index 8aee6db..7e370b2 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -4709,7 +4709,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) >> if (!init_event) { >> apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | >> MSR_IA32_APICBASE_ENABLE; >> - if (kvm_vcpu_is_bsp(&vmx->vcpu)) >> + if (kvm_vcpu_is_reset_bsp(&vmx->vcpu)) >> apic_base_msr.data |= MSR_IA32_APICBASE_BSP; >> apic_base_msr.host_initiated = true; >> kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 0f574eb..8365cae 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -968,11 +968,16 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) >> #endif /* CONFIG_HAVE_KVM_EVENTFD */ >> >> #ifdef CONFIG_KVM_APIC_ARCHITECTURE >> -static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) >> +static inline bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu) >> { >> return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id; >> } >> >> +static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) >> +{ >> + return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0; >> +} >> + >> bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu); >> >> #else > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 30/03/2015 16:40, Nadav Amit wrote: > Paolo, > > It appears you are right and I have not tested 4 and 5 well enough. I’ll > repost them (the others were tested presumably well enough). > > Two short questions: > Can I use init.c in the kvm-unit-tests ? > Why is it disabled? Because QEMU support for INIT is incomplete, so the tests would fail ("Uh, hard reset!"). IIRC sending init to BSP with APIC_DEST_SELF is also not supported by actual hardware (or at least not supported "officially") so that test would also have to be changed to not use a shortcut. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 17da6fc..b0dbf68 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -1498,8 +1498,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value) return; } - if (!kvm_vcpu_is_bsp(apic->vcpu)) - value &= ~MSR_IA32_APICBASE_BSP; vcpu->arch.apic_base = value; /* update jump label if enable bit changes */ diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 1ef4c0d..ef5bf21 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -1261,7 +1261,7 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id) svm->vcpu.arch.apic_base = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; - if (kvm_vcpu_is_bsp(&svm->vcpu)) + if (kvm_vcpu_is_reset_bsp(&svm->vcpu)) svm->vcpu.arch.apic_base |= MSR_IA32_APICBASE_BSP; svm_init_osvw(&svm->vcpu); diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 8aee6db..7e370b2 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -4709,7 +4709,7 @@ static void vmx_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event) if (!init_event) { apic_base_msr.data = APIC_DEFAULT_PHYS_BASE | MSR_IA32_APICBASE_ENABLE; - if (kvm_vcpu_is_bsp(&vmx->vcpu)) + if (kvm_vcpu_is_reset_bsp(&vmx->vcpu)) apic_base_msr.data |= MSR_IA32_APICBASE_BSP; apic_base_msr.host_initiated = true; kvm_set_apic_base(&vmx->vcpu, &apic_base_msr); diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 0f574eb..8365cae 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -968,11 +968,16 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) #endif /* CONFIG_HAVE_KVM_EVENTFD */ #ifdef CONFIG_KVM_APIC_ARCHITECTURE -static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) +static inline bool kvm_vcpu_is_reset_bsp(struct kvm_vcpu *vcpu) { return vcpu->kvm->bsp_vcpu_id == vcpu->vcpu_id; } +static inline bool kvm_vcpu_is_bsp(struct kvm_vcpu *vcpu) +{ + return (vcpu->arch.apic_base & MSR_IA32_APICBASE_BSP) != 0; +} + bool kvm_vcpu_compatible(struct kvm_vcpu *vcpu); #else
After reset, the CPU can change the BSP, which will be used upon INIT. Reset should return the BSP which QEMU asked for, and therefore handled accordingly. To quote: "If the MP protocol has completed and a BSP is chosen, subsequent INITs (either to a specific processor or system wide) do not cause the MP protocol to be repeated." [Intel SDM 8.4.2: MP Initialization Protocol Requirements and Restrictions] Signed-off-by: Nadav Amit <namit@cs.technion.ac.il> --- arch/x86/kvm/lapic.c | 2 -- arch/x86/kvm/svm.c | 2 +- arch/x86/kvm/vmx.c | 2 +- include/linux/kvm_host.h | 7 ++++++- 4 files changed, 8 insertions(+), 5 deletions(-)