Marcelo Tosatti wrote on 2013-01-22:
> On Wed, Jan 16, 2013 at 06:21:11PM +0800, Yang Zhang wrote:
>> From: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>
>> basically to benefit from apicv, we need to enable virtualized x2apic mode.
>> Currently, we only enable it when guest is really using x2apic.
>>
>> Also, clear MSR bitmap for corresponding x2apic MSRs when guest enabled
> x2apic:
>> 0x800 - 0x8ff: no read intercept for apicv register virtualization,
>> except APIC ID and TMCCT which need software's
> assistance to
>> get right value.
>>
>> Signed-off-by: Kevin Tian <kevin.tian@xxxxxxxxx>
>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>> ---
>> arch/x86/include/asm/kvm_host.h | 1 + arch/x86/include/asm/vmx.h
>> | 1 + arch/x86/kvm/lapic.c | 20 ++--
>> arch/x86/kvm/lapic.h | 5 + arch/x86/kvm/svm.c
>> | 6 + arch/x86/kvm/vmx.c | 204
>> +++++++++++++++++++++++++++++++++++---- 6 files changed, 209
>> insertions(+), 28 deletions(-)
>> diff --git a/arch/x86/include/asm/kvm_host.h
>> b/arch/x86/include/asm/kvm_host.h index c431b33..35aa8e6 100644 ---
>> a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h
>> @@ -697,6 +697,7 @@ struct kvm_x86_ops {
>> void (*enable_nmi_window)(struct kvm_vcpu *vcpu); void
>> (*enable_irq_window)(struct kvm_vcpu *vcpu); void
>> (*update_cr8_intercept)(struct kvm_vcpu *vcpu, int tpr, int irr);
>> + void (*set_virtual_x2apic_mode)(struct kvm_vcpu *vcpu, bool set);
>> int (*set_tss_addr)(struct kvm *kvm, unsigned int addr); int
>> (*get_tdp_level)(void); u64 (*get_mt_mask)(struct kvm_vcpu *vcpu,
>> gfn_t gfn, bool is_mmio);
>> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
>> index 44c3f7e..0a54df0 100644
>> --- a/arch/x86/include/asm/vmx.h
>> +++ b/arch/x86/include/asm/vmx.h
>> @@ -139,6 +139,7 @@
>> #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001 #define
>> SECONDARY_EXEC_ENABLE_EPT 0x00000002 #define
>> SECONDARY_EXEC_RDTSCP 0x00000008 +#define
>> SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE 0x00000010 #define
>> SECONDARY_EXEC_ENABLE_VPID 0x00000020 #define
>> SECONDARY_EXEC_WBINVD_EXITING 0x00000040 #define
>> SECONDARY_EXEC_UNRESTRICTED_GUEST 0x00000080
>> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
>> index 0664c13..f39aee3 100644
>> --- a/arch/x86/kvm/lapic.c
>> +++ b/arch/x86/kvm/lapic.c
>> @@ -140,11 +140,6 @@ static inline int apic_enabled(struct kvm_lapic *apic)
>> (LVT_MASK | APIC_MODE_MASK | APIC_INPUT_POLARITY | \
>> APIC_LVT_REMOTE_IRR | APIC_LVT_LEVEL_TRIGGER)
>> -static inline int apic_x2apic_mode(struct kvm_lapic *apic)
>> -{
>> - return apic->vcpu->arch.apic_base & X2APIC_ENABLE;
>> -}
>> -
>> static inline int kvm_apic_id(struct kvm_lapic *apic)
>> {
>> return (kvm_apic_get_reg(apic, APIC_ID) >> 24) & 0xff;
>> @@ -1323,12 +1318,17 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu,
> u64 value)
>> if (!kvm_vcpu_is_bsp(apic->vcpu))
>> value &= ~MSR_IA32_APICBASE_BSP;
>> - vcpu->arch.apic_base = value;
>> - if (apic_x2apic_mode(apic)) {
>> - u32 id = kvm_apic_id(apic);
>> - u32 ldr = ((id >> 4) << 16) | (1 << (id & 0xf));
>> - kvm_apic_set_ldr(apic, ldr);
>> + if ((vcpu->arch.apic_base ^ value) & X2APIC_ENABLE) {
>> + if (value & X2APIC_ENABLE) {
>> + u32 id = kvm_apic_id(apic);
>> + u32 ldr = ((id >> 4) << 16) | (1 << (id & 0xf));
>> + kvm_apic_set_ldr(apic, ldr);
>> + kvm_x86_ops->set_virtual_x2apic_mode(vcpu, true);
>> + } else
>> + kvm_x86_ops->set_virtual_x2apic_mode(vcpu, false);
>> }
>> +
>> + vcpu->arch.apic_base = value;
>
> Simpler to have
>
> if (apic_x2apic_mode(apic)) {
> ...
> kvm_x86_ops->set_virtual_x2apic_mode(vcpu, true);
> } else {
> kvm_x86_ops->set_virtual_x2apic_mode(vcpu, false);
> }
>
> Also it must be done after assignment of vcpu->arch.apic_base (this
> patch has vcpu->arch.apic_base being read from
> ->set_virtual_x2apic_mode() path).
>
>> +static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu)
>> +{
>> + unsigned long *msr_bitmap;
>> +
>> + if (apic_x2apic_mode(vcpu->arch.apic))
>
> vcpu->arch.apic can be NULL.
Actually, call apic_x2apic_mode to check whether use x2apic msr bitmap is wrong.
VCPU uses x2apic but it may not set virtual x2apic mode bit due to TPR shadow not enabled or irqchip not in kernel. Check the virtual x2apic mode bit in vmcs directly should be a better choice. How about the follow code:
static void vmx_set_msr_bitmap(struct kvm_vcpu *vcpu)
{
unsigned long *msr_bitmap;
if (vmcs_read32(SECONDARY_VM_EXEC_CONTROL) & SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE)
if (is_long_mode(vcpu))
msr_bitmap = vmx_msr_bitmap_longmode_x2apic;
else
msr_bitmap = vmx_msr_bitmap_legacy_x2apic;
else
if (is_long_mode(vcpu))
msr_bitmap = vmx_msr_bitmap_longmode;
else
msr_bitmap = vmx_msr_bitmap_legacy;
vmcs_write64(MSR_BITMAP, __pa(msr_bitmap));
}
Best regards,
Yang
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
