From bfc08c281291407473257f5dc6907bd53af22c18 Mon Sep 17 00:00:00 2001 From: Zide Chen Date: Wed, 22 May 2019 22:58:31 -0700 Subject: [PATCH] hv: move msr_bitmap from acrn_vm to acrn_vcpu_arch At the time the guest is switching to X2APIC mode, different VCPUs in the same VM could expect the setting of the per VM msr_bitmap differently, which could cause problems. Considering different approaches to address this issue: 1) only guest BSP can update msr_bitmap when switching to X2APIC. 2) carefully re-write the update_msr_bitmap_x2apic_xxx() functions to make sure any bit in the bitmap won't be toggled by the VCPUs. 3) make msr_bitmap as per VCPU. We chose option 3) because it's simple and clean, though it takes more memory than other options. BTW, need to remove const modifier from update_msr_bitmap_x2apic_xxx() functions to get it compiled. Tracked-On: #3166 Signed-off-by: Zide Chen Signed-off-by: Sainath Grandhi Acked-by: Anthony Xu --- hypervisor/arch/x86/guest/vmsr.c | 86 +++++++++++++++--------- hypervisor/include/arch/x86/guest/vcpu.h | 4 ++ hypervisor/include/arch/x86/guest/vm.h | 2 - hypervisor/include/arch/x86/msr.h | 4 +- 4 files changed, 62 insertions(+), 34 deletions(-) diff --git a/hypervisor/arch/x86/guest/vmsr.c b/hypervisor/arch/x86/guest/vmsr.c index 771754b24..69d0d3afe 100644 --- a/hypervisor/arch/x86/guest/vmsr.c +++ b/hypervisor/arch/x86/guest/vmsr.c @@ -279,6 +279,9 @@ static void intercept_x2apic_msrs(uint8_t *msr_bitmap_arg, uint32_t mode) } } +/** + * @pre vcpu != NULL + */ static void init_msr_area(struct acrn_vcpu *vcpu) { vcpu->arch.msr_area.guest[MSR_AREA_TSC_AUX].msr_index = MSR_IA32_TSC_AUX; @@ -287,40 +290,39 @@ static void init_msr_area(struct acrn_vcpu *vcpu) vcpu->arch.msr_area.host[MSR_AREA_TSC_AUX].value = vcpu->pcpu_id; } +/** + * @pre vcpu != NULL + */ void init_msr_emulation(struct acrn_vcpu *vcpu) { + uint8_t *msr_bitmap = vcpu->arch.msr_bitmap; uint32_t msr, i; - uint8_t *msr_bitmap; uint64_t value64; - if (is_vcpu_bsp(vcpu)) { - msr_bitmap = vcpu->vm->arch_vm.msr_bitmap; - - for (i = 0U; i < NUM_GUEST_MSRS; i++) { - enable_msr_interception(msr_bitmap, emulated_guest_msrs[i], INTERCEPT_READ_WRITE); - } - - for (i = 0U; i < NUM_MTRR_MSRS; i++) { - enable_msr_interception(msr_bitmap, mtrr_msrs[i], INTERCEPT_READ_WRITE); - } - - intercept_x2apic_msrs(msr_bitmap, INTERCEPT_READ_WRITE); - - for (i = 0U; i < NUM_UNSUPPORTED_MSRS; i++) { - enable_msr_interception(msr_bitmap, unsupported_msrs[i], INTERCEPT_READ_WRITE); - } - - /* RDT-A disabled: CPUID.07H.EBX[12], CPUID.10H */ - for (msr = MSR_IA32_L3_MASK_0; msr < MSR_IA32_BNDCFGS; msr++) { - enable_msr_interception(msr_bitmap, msr, INTERCEPT_READ_WRITE); - } - - /* don't need to intercept rdmsr for these MSRs */ - enable_msr_interception(msr_bitmap, MSR_IA32_TIME_STAMP_COUNTER, INTERCEPT_WRITE); + for (i = 0U; i < NUM_GUEST_MSRS; i++) { + enable_msr_interception(msr_bitmap, emulated_guest_msrs[i], INTERCEPT_READ_WRITE); } + for (i = 0U; i < NUM_MTRR_MSRS; i++) { + enable_msr_interception(msr_bitmap, mtrr_msrs[i], INTERCEPT_READ_WRITE); + } + + intercept_x2apic_msrs(msr_bitmap, INTERCEPT_READ_WRITE); + + for (i = 0U; i < NUM_UNSUPPORTED_MSRS; i++) { + enable_msr_interception(msr_bitmap, unsupported_msrs[i], INTERCEPT_READ_WRITE); + } + + /* RDT-A disabled: CPUID.07H.EBX[12], CPUID.10H */ + for (msr = MSR_IA32_L3_MASK_0; msr < MSR_IA32_BNDCFGS; msr++) { + enable_msr_interception(msr_bitmap, msr, INTERCEPT_READ_WRITE); + } + + /* don't need to intercept rdmsr for these MSRs */ + enable_msr_interception(msr_bitmap, MSR_IA32_TIME_STAMP_COUNTER, INTERCEPT_WRITE); + /* Setup MSR bitmap - Intel SDM Vol3 24.6.9 */ - value64 = hva2hpa(vcpu->vm->arch_vm.msr_bitmap); + value64 = hva2hpa(vcpu->arch.msr_bitmap); exec_vmwrite64(VMX_MSR_BITMAP_FULL, value64); pr_dbg("VMX_MSR_BITMAP: 0x%016llx ", value64); @@ -328,6 +330,9 @@ void init_msr_emulation(struct acrn_vcpu *vcpu) init_msr_area(vcpu); } +/** + * @pre vcpu != NULL + */ int32_t rdmsr_vmexit_handler(struct acrn_vcpu *vcpu) { int32_t err = 0; @@ -436,6 +441,10 @@ int32_t rdmsr_vmexit_handler(struct acrn_vcpu *vcpu) * TSC, the logical processor also adds (or subtracts) value X from * the IA32_TSC_ADJUST MSR. */ + +/** + * @pre vcpu != NULL + */ static void set_guest_tsc(struct acrn_vcpu *vcpu, uint64_t guest_tsc) { uint64_t tsc_delta, tsc_offset_delta, tsc_adjust; @@ -458,6 +467,10 @@ static void set_guest_tsc(struct acrn_vcpu *vcpu, uint64_t guest_tsc) * MSR adds (or subtracts) value X from that MSR, the logical * processor also adds (or subtracts) value X from the TSC." */ + +/** + * @pre vcpu != NULL + */ static void set_guest_tsc_adjust(struct acrn_vcpu *vcpu, uint64_t tsc_adjust) { uint64_t tsc_offset, tsc_adjust_delta; @@ -473,6 +486,9 @@ static void set_guest_tsc_adjust(struct acrn_vcpu *vcpu, uint64_t tsc_adjust) vcpu_set_guest_msr(vcpu, MSR_IA32_TSC_ADJUST, tsc_adjust); } +/** + * @pre vcpu != NULL + */ static void set_guest_ia32_misc_enalbe(struct acrn_vcpu *vcpu, uint64_t v) { uint32_t eax, ebx = 0U, ecx = 0U, edx = 0U; @@ -520,6 +536,9 @@ static void set_guest_ia32_misc_enalbe(struct acrn_vcpu *vcpu, uint64_t v) } } +/** + * @pre vcpu != NULL + */ int32_t wrmsr_vmexit_handler(struct acrn_vcpu *vcpu) { int32_t err = 0; @@ -630,11 +649,14 @@ int32_t wrmsr_vmexit_handler(struct acrn_vcpu *vcpu) return err; } -void update_msr_bitmap_x2apic_apicv(const struct acrn_vcpu *vcpu) +/** + * @pre vcpu != NULL + */ +void update_msr_bitmap_x2apic_apicv(struct acrn_vcpu *vcpu) { uint8_t *msr_bitmap; - msr_bitmap = vcpu->vm->arch_vm.msr_bitmap; + msr_bitmap = vcpu->arch.msr_bitmap; /* * For platforms that do not support register virtualization * all x2APIC MSRs need to intercepted. So no need to update @@ -666,9 +688,13 @@ void update_msr_bitmap_x2apic_apicv(const struct acrn_vcpu *vcpu) * - XAPICID/LDR: Read to XAPICID/LDR need to be trapped to guarantee guest always see right vlapic_id. * - ICR: Write to ICR need to be trapped to avoid milicious IPI. */ -void update_msr_bitmap_x2apic_passthru(const struct acrn_vcpu *vcpu) + +/** + * @pre vcpu != NULL + */ +void update_msr_bitmap_x2apic_passthru(struct acrn_vcpu *vcpu) { - uint8_t *msr_bitmap = vcpu->vm->arch_vm.msr_bitmap; + uint8_t *msr_bitmap = vcpu->arch.msr_bitmap; intercept_x2apic_msrs(msr_bitmap, INTERCEPT_DISABLE); enable_msr_interception(msr_bitmap, MSR_IA32_EXT_XAPICID, INTERCEPT_READ); diff --git a/hypervisor/include/arch/x86/guest/vcpu.h b/hypervisor/include/arch/x86/guest/vcpu.h index f2091fa66..a2ddabc96 100644 --- a/hypervisor/include/arch/x86/guest/vcpu.h +++ b/hypervisor/include/arch/x86/guest/vcpu.h @@ -297,6 +297,10 @@ struct msr_store_area { struct acrn_vcpu_arch { /* vmcs region for this vcpu, MUST be 4KB-aligned */ uint8_t vmcs[PAGE_SIZE]; + + /* MSR bitmap region for this vcpu, MUST be 4-Kbyte aligned */ + uint8_t msr_bitmap[PAGE_SIZE]; + /* per vcpu lapic */ struct acrn_vlapic vlapic; diff --git a/hypervisor/include/arch/x86/guest/vm.h b/hypervisor/include/arch/x86/guest/vm.h index 734f4996c..c495fe441 100644 --- a/hypervisor/include/arch/x86/guest/vm.h +++ b/hypervisor/include/arch/x86/guest/vm.h @@ -89,8 +89,6 @@ enum vm_state { struct vm_arch { /* I/O bitmaps A and B for this VM, MUST be 4-Kbyte aligned */ uint8_t io_bitmap[PAGE_SIZE*2]; - /* MSR bitmap region for this VM, MUST be 4-Kbyte aligned */ - uint8_t msr_bitmap[PAGE_SIZE]; uint64_t guest_init_pml4;/* Guest init pml4 */ /* EPT hierarchy for Normal World */ diff --git a/hypervisor/include/arch/x86/msr.h b/hypervisor/include/arch/x86/msr.h index 1924b5e3b..195c97d68 100644 --- a/hypervisor/include/arch/x86/msr.h +++ b/hypervisor/include/arch/x86/msr.h @@ -581,8 +581,8 @@ struct acrn_vcpu; void init_msr_emulation(struct acrn_vcpu *vcpu); uint32_t vmsr_get_guest_msr_index(uint32_t msr); -void update_msr_bitmap_x2apic_apicv(const struct acrn_vcpu *vcpu); -void update_msr_bitmap_x2apic_passthru(const struct acrn_vcpu *vcpu); +void update_msr_bitmap_x2apic_apicv(struct acrn_vcpu *vcpu); +void update_msr_bitmap_x2apic_passthru(struct acrn_vcpu *vcpu); #endif /* ASSEMBLER */