From 4dc39fdb8eafe132763c363b50f6893b403a8af2 Mon Sep 17 00:00:00 2001 From: Xiangyang Wu Date: Wed, 11 Jul 2018 11:09:10 +0800 Subject: [PATCH] HV:treewide:Add 16-bit atomic operations and update vpid type There are some integer type conversions reported by static analysis tool for vcpu id, number of created vcpus, and vpid, to reduce these type conversions, redesign vcpu id, number of created vcpus, and vpid type as uint16_t as per their usage, related 16-bit atomic operations shall be added in HV. MISRA C requires that all unsigned constants should have the suffix 'U' (e.g. 0xffU), but the assembler may not accept such C-style constants. Add 16-bit atomic add/dec/store operations; Update temporary variables type and parameters type of related caller; Update vpid type as uint16_t; Replace Macro with constant value for CPU_PAGE_SIZE. Note: According to SDM A.10, there are some bits defined in the IA32_VMX_EPT_VPID_CAP MSR to support the INVVPID instruction, these bits don't mean actual VPID, so the vpid field in the data struct vmx_capability doesn't be updated. V1--V2: update comments for assembly code as per coding style; Signed-off-by: Xiangyang Wu --- hypervisor/arch/x86/cpu_primary.S | 15 ++++++++++----- hypervisor/arch/x86/guest/vcpu.c | 4 ++-- hypervisor/arch/x86/guest/vm.c | 2 +- hypervisor/arch/x86/mmu.c | 18 +++++++++--------- hypervisor/arch/x86/trampoline.S | 12 ++++++++---- hypervisor/arch/x86/vmx.c | 2 +- hypervisor/include/arch/x86/guest/vcpu.h | 2 +- hypervisor/include/arch/x86/guest/vm.h | 2 +- hypervisor/include/arch/x86/mmu.h | 4 ++-- hypervisor/include/arch/x86/vmx.h | 4 ++-- hypervisor/include/lib/atomic.h | 3 +++ 11 files changed, 40 insertions(+), 28 deletions(-) diff --git a/hypervisor/arch/x86/cpu_primary.S b/hypervisor/arch/x86/cpu_primary.S index be389b286..76066966e 100644 --- a/hypervisor/arch/x86/cpu_primary.S +++ b/hypervisor/arch/x86/cpu_primary.S @@ -127,7 +127,8 @@ primary_start_long_mode: /* Initialize temporary stack pointer */ lea _ld_bss_end(%rip), %rsp - add $CPU_PAGE_SIZE,%rsp + /*0x1000 = CPU_PAGE_SIZE*/ + add $0x1000,%rsp /* 16 = CPU_STACK_ALIGN */ and $(~(16 - 1)),%rsp @@ -213,20 +214,24 @@ cpu_primary32_gdt_ptr: .quad cpu_primary32_gdt /* PML4, PDPT, and PD tables initialized to map first 4 GBytes of memory */ - .align CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + .align 0x1000 .global cpu_boot32_page_tables_start cpu_boot32_page_tables_start: /* 0x3 = (IA32E_COMM_P_BIT | IA32E_COMM_RW_BIT) */ .quad cpu_primary32_pdpt_addr + 0x3 - .align CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + .align 0x1000 cpu_primary32_pdpt_addr: address = 0 .rept 4 /* 0x3 = (IA32E_COMM_P_BIT | IA32E_COMM_RW_BIT) */ .quad cpu_primary32_pdt_addr + address + 0x3 - address = address + CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + address = address + 0x1000 .endr - .align CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + .align 0x1000 cpu_primary32_pdt_addr: address = 0 .rept 2048 diff --git a/hypervisor/arch/x86/guest/vcpu.c b/hypervisor/arch/x86/guest/vcpu.c index e21df0f92..a1652986e 100644 --- a/hypervisor/arch/x86/guest/vcpu.c +++ b/hypervisor/arch/x86/guest/vcpu.c @@ -63,7 +63,7 @@ int create_vcpu(uint16_t pcpu_id, struct vm *vm, struct vcpu **rtn_vcpu_handle) * vcpu->vcpu_id = vm->hw.created_vcpus; * vm->hw.created_vcpus++; */ - vcpu->vcpu_id = atomic_xadd(&vm->hw.created_vcpus, 1); + vcpu->vcpu_id = atomic_xadd16(&vm->hw.created_vcpus, 1U); /* vm->hw.vcpu_array[vcpu->vcpu_id] = vcpu; */ atomic_store64( (long *)&vm->hw.vcpu_array[vcpu->vcpu_id], @@ -246,7 +246,7 @@ void destroy_vcpu(struct vcpu *vcpu) (long *)&vcpu->vm->hw.vcpu_array[vcpu->vcpu_id], (long)NULL); - atomic_dec(&vcpu->vm->hw.created_vcpus); + atomic_dec16(&vcpu->vm->hw.created_vcpus); vlapic_free(vcpu); free(vcpu->arch_vcpu.vmcs); diff --git a/hypervisor/arch/x86/guest/vm.c b/hypervisor/arch/x86/guest/vm.c index e96a90b32..4becbeb40 100644 --- a/hypervisor/arch/x86/guest/vm.c +++ b/hypervisor/arch/x86/guest/vm.c @@ -104,7 +104,7 @@ int create_vm(struct vm_description *vm_desc, struct vm **rtn_vm) vm->attr.id = id; vm->attr.boot_idx = id; - atomic_store(&vm->hw.created_vcpus, 0); + atomic_store16(&vm->hw.created_vcpus, 0U); /* gpa_lowtop are used for system start up */ vm->hw.gpa_lowtop = 0UL; diff --git a/hypervisor/arch/x86/mmu.c b/hypervisor/arch/x86/mmu.c index 5c6112728..fb0d8809a 100644 --- a/hypervisor/arch/x86/mmu.c +++ b/hypervisor/arch/x86/mmu.c @@ -52,7 +52,7 @@ static struct vmx_capability { * is the value of the VPID VM-execution control field in the VMCS. * (VM entry ensures that this value is never 0000H). */ -static int vmx_vpid_nr = VMX_MIN_NR_VPID; +static uint16_t vmx_vpid_nr = VMX_MIN_NR_VPID; #define INVEPT_TYPE_SINGLE_CONTEXT 1UL #define INVEPT_TYPE_ALL_CONTEXTS 2UL @@ -71,7 +71,7 @@ struct invept_desc { uint64_t _res; }; -static inline void _invvpid(uint64_t type, int vpid, uint64_t gva) +static inline void _invvpid(uint64_t type, uint16_t vpid, uint64_t gva) { int error = 0; @@ -142,28 +142,28 @@ int check_vmx_mmu_cap(void) return 0; } -int allocate_vpid(void) +uint16_t allocate_vpid(void) { - int vpid = atomic_xadd(&vmx_vpid_nr, 1); + uint16_t vpid = atomic_xadd16(&vmx_vpid_nr, 1U); /* TODO: vpid overflow */ if (vpid >= VMX_MAX_NR_VPID) { pr_err("%s, vpid overflow\n", __func__); /* * set vmx_vpid_nr to VMX_MAX_NR_VPID to disable vpid - * since next atomic_xadd will always large than + * since next atomic_xadd16 will always large than * VMX_MAX_NR_VPID. */ vmx_vpid_nr = VMX_MAX_NR_VPID; - vpid = 0; + vpid = 0U; } return vpid; } -void flush_vpid_single(int vpid) +void flush_vpid_single(uint16_t vpid) { - if (vpid == 0) + if (vpid == 0U) return; _invvpid(VMX_VPID_TYPE_SINGLE_CONTEXT, vpid, 0UL); @@ -171,7 +171,7 @@ void flush_vpid_single(int vpid) void flush_vpid_global(void) { - _invvpid(VMX_VPID_TYPE_ALL_CONTEXT, 0, 0UL); + _invvpid(VMX_VPID_TYPE_ALL_CONTEXT, 0U, 0UL); } void invept(struct vcpu *vcpu) diff --git a/hypervisor/arch/x86/trampoline.S b/hypervisor/arch/x86/trampoline.S index d7af62842..73036be89 100644 --- a/hypervisor/arch/x86/trampoline.S +++ b/hypervisor/arch/x86/trampoline.S @@ -198,21 +198,25 @@ trampoline_gdt_ptr: CPU_Boot_Page_Tables_ptr: .long CPU_Boot_Page_Tables_Start - .align CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + .align 0x1000 .global CPU_Boot_Page_Tables_Start CPU_Boot_Page_Tables_Start: /* 0x3 = (IA32E_COMM_P_BIT | IA32E_COMM_RW_BIT) */ .quad trampoline_pdpt_addr + 0x3 - .align CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + .align 0x1000 .global trampoline_pdpt_addr trampoline_pdpt_addr: address = 0 .rept 4 /* 0x3 = (IA32E_COMM_P_BIT | IA32E_COMM_RW_BIT) */ .quad trampoline_pdt_addr + address + 0x3 - address = address + CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + address = address + 0x1000 .endr - .align CPU_PAGE_SIZE + /*0x1000 = CPU_PAGE_SIZE*/ + .align 0x1000 trampoline_pdt_addr: address = 0 .rept 2048 diff --git a/hypervisor/arch/x86/vmx.c b/hypervisor/arch/x86/vmx.c index 91aa2604b..c088ad80c 100644 --- a/hypervisor/arch/x86/vmx.c +++ b/hypervisor/arch/x86/vmx.c @@ -1239,7 +1239,7 @@ static void init_exec_ctrl(struct vcpu *vcpu) VMX_PROCBASED_CTLS2_RDTSCP | VMX_PROCBASED_CTLS2_UNRESTRICT); - if (vcpu->arch_vcpu.vpid != 0) + if (vcpu->arch_vcpu.vpid != 0U) value32 |= VMX_PROCBASED_CTLS2_VPID; else value32 &= ~VMX_PROCBASED_CTLS2_VPID; diff --git a/hypervisor/include/arch/x86/guest/vcpu.h b/hypervisor/include/arch/x86/guest/vcpu.h index c511d2efc..b574e035a 100644 --- a/hypervisor/include/arch/x86/guest/vcpu.h +++ b/hypervisor/include/arch/x86/guest/vcpu.h @@ -192,7 +192,7 @@ struct vcpu_arch { /* A pointer to the VMCS for this CPU. */ void *vmcs; - int vpid; + uint16_t vpid; /* Holds the information needed for IRQ/exception handling. */ struct { diff --git a/hypervisor/include/arch/x86/guest/vm.h b/hypervisor/include/arch/x86/guest/vm.h index 9479019c5..80d044e40 100644 --- a/hypervisor/include/arch/x86/guest/vm.h +++ b/hypervisor/include/arch/x86/guest/vm.h @@ -23,7 +23,7 @@ struct vm_attr { struct vm_hw_info { uint16_t num_vcpus; /* Number of total virtual cores */ uint16_t exp_num_vcpus; /* Number of real expected virtual cores */ - int created_vcpus; /* Number of created vcpus */ + uint16_t created_vcpus; /* Number of created vcpus */ struct vcpu **vcpu_array; /* vcpu array of this VM */ uint64_t gpa_lowtop; /* top lowmem gpa of this VM */ }; diff --git a/hypervisor/include/arch/x86/mmu.h b/hypervisor/include/arch/x86/mmu.h index 75599b92b..4a5121527 100644 --- a/hypervisor/include/arch/x86/mmu.h +++ b/hypervisor/include/arch/x86/mmu.h @@ -326,8 +326,8 @@ int modify_mem(struct map_params *map_params, void *paddr, void *vaddr, int modify_mem_mt(struct map_params *map_params, void *paddr, void *vaddr, uint64_t size, uint32_t flags); int check_vmx_mmu_cap(void); -int allocate_vpid(void); -void flush_vpid_single(int vpid); +uint16_t allocate_vpid(void); +void flush_vpid_single(uint16_t vpid); void flush_vpid_global(void); void invept(struct vcpu *vcpu); bool check_continuous_hpa(struct vm *vm, uint64_t gpa, uint64_t size); diff --git a/hypervisor/include/arch/x86/vmx.h b/hypervisor/include/arch/x86/vmx.h index 93ef4d2e7..84e12fb50 100644 --- a/hypervisor/include/arch/x86/vmx.h +++ b/hypervisor/include/arch/x86/vmx.h @@ -314,8 +314,8 @@ #define VMX_EPT_INVEPT_SINGLE_CONTEXT (1U << 25) #define VMX_EPT_INVEPT_GLOBAL_CONTEXT (1U << 26) -#define VMX_MIN_NR_VPID 1 -#define VMX_MAX_NR_VPID (1 << 5) +#define VMX_MIN_NR_VPID 1U +#define VMX_MAX_NR_VPID (1U << 5) #define VMX_VPID_TYPE_INDIVIDUAL_ADDR 0UL #define VMX_VPID_TYPE_SINGLE_CONTEXT 1UL diff --git a/hypervisor/include/lib/atomic.h b/hypervisor/include/lib/atomic.h index 92835883d..8d557ef39 100644 --- a/hypervisor/include/lib/atomic.h +++ b/hypervisor/include/lib/atomic.h @@ -53,6 +53,7 @@ static inline void name(volatile type *ptr, type v) \ : "r" (v) \ : "cc", "memory"); \ } +build_atomic_store(atomic_store16, "w", uint16_t, p, v) build_atomic_store(atomic_store, "l", int, p, v) build_atomic_store(atomic_store64, "q", long, p, v) @@ -73,6 +74,7 @@ static inline void name(type *ptr) \ : "=m" (*ptr) \ : "m" (*ptr)); \ } +build_atomic_dec(atomic_dec16, "w", uint16_t, p) build_atomic_dec(atomic_dec, "l", int, p) build_atomic_dec(atomic_dec64, "q", long, p) @@ -167,6 +169,7 @@ static inline type name(type *ptr, type v) \ : "cc", "memory"); \ return v; \ } +build_atomic_xadd(atomic_xadd16, "w", uint16_t, p, v) build_atomic_xadd(atomic_xadd, "l", int, p, v) build_atomic_xadd(atomic_xadd64, "q", long, p, v)