Re: [PATCH 1/2] kvm-unit-tests: VMX: The framework of EPT for nested VMX testing

Paolo Bonzini
2013-09-09T13:45:22Z

Il 09/09/2013 06:57, Arthur Chunqi Li ha scritto:
> The framework of EPT for nested VMX, including functions to build up
> EPT paging structures, read/set EPT PTEs and setup a range of 1:1 map
> EPT.
>
> Signed-off-by: Arthur Chunqi Li <yzt356@gmail.com>
> ---
> x86/vmx.c | 159 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> x86/vmx.h | 76 +++++++++++++++++++++++++++++
> 2 files changed, 231 insertions(+), 4 deletions(-)
>
> @@ -336,10 +489,8 @@ static void init_vmx(void)
> : MSR_IA32_VMX_ENTRY_CTLS);
> ctrl_cpu_rev[0].val = rdmsr(basic.ctrl ? MSR_IA32_VMX_TRUE_PROC
> : MSR_IA32_VMX_PROCBASED_CTLS);
> - if (ctrl_cpu_rev[0].set & CPU_SECONDARY)
> - ctrl_cpu_rev[1].val = rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2);
> - if (ctrl_cpu_rev[1].set & CPU_EPT || ctrl_cpu_rev[1].set & CPU_VPID)
> - ept_vpid.val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
> + ctrl_cpu_rev[1].val = rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2);
> + ept_vpid.val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);

This is because these MSRs are confusing.

Your definitions are:

union vmx_ctrl_cpu {
u64 val;
struct {
u32 set, clr;
};
};

so "set" are the low 32-bit and "clr" are the high 32-bits.

This is how the SDM's description should be read:

set clr
0 0 if bit is 0, ok. if bit is 1, fail
=> reserved, must be 0
0 1 if bit is 0, ok. if bit is 1, ok
=> supported by processor
1 0 if bit is 0, fail. if bit is 1, fail
=> impossible
1 1 if bit is 0, fail. if bit is 1, ok
=> reserved, must be 1
So the right fix is:

if ((ctrl_cpu_rev[0].clr & CPU_SECONDARY) != 0)
ctrl_cpu_rev[1].val = rdmsr(MSR_IA32_VMX_PROCBASED_CTLS2);
if ((ctrl_cpu_rev[1].clr & (CPU_EPT | CPU_VPID)) != 0)
ept_vpid.val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);

While looking at this I found another related bug. This line:

ctrl_cpu[1] |= ctrl_cpu_rev[1].set & ctrl_cpu_rev[1].clr;

should be

ctrl_cpu[1] = (ctrl_cpu[1] | ctrl_cpu_rev[1].set) & ctrl_cpu_rev[1].clr;

which is the same as other lines using the MSRs.

Paolo

> write_cr0((read_cr0() & fix_cr0_clr) | fix_cr0_set);
> write_cr4((read_cr4() & fix_cr4_clr) | fix_cr4_set | X86_CR4_VMXE);
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 28595d8..742c2b2 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -432,6 +432,59 @@ enum Ctrl1 {
> #define HYPERCALL_MASK 0xFFF
> #define HYPERCALL_VMEXIT 0x1
>
> +#define EPTP_PG_WALK_LEN_SHIFT 3ul
> +#define EPTP_AD_FLAG (1ul << 6)
> +
> +#define EPT_MEM_TYPE_UC 0ul
> +#define EPT_MEM_TYPE_WC 1ul
> +#define EPT_MEM_TYPE_WT 4ul
> +#define EPT_MEM_TYPE_WP 5ul
> +#define EPT_MEM_TYPE_WB 6ul
> +
> +#define EPT_RA 1ul
> +#define EPT_WA 2ul
> +#define EPT_EA 4ul
> +#define EPT_PRESENT (EPT_RA | EPT_WA | EPT_EA)
> +#define EPT_ACCESS_FLAG (1ul << 8)
> +#define EPT_DIRTY_FLAG (1ul << 9)
> +#define EPT_LARGE_PAGE (1ul << 7)
> +#define EPT_MEM_TYPE_SHIFT 3ul
> +#define EPT_IGNORE_PAT (1ul << 6)
> +#define EPT_SUPPRESS_VE (1ull << 63)
> +
> +#define EPT_CAP_WT 1ull
> +#define EPT_CAP_PWL4 (1ull << 6)
> +#define EPT_CAP_UC (1ull << 8)
> +#define EPT_CAP_WB (1ull << 14)
> +#define EPT_CAP_2M_PAGE (1ull << 16)
> +#define EPT_CAP_1G_PAGE (1ull << 17)
> +#define EPT_CAP_INVEPT (1ull << 20)
> +#define EPT_CAP_INVEPT_SINGLE (1ull << 25)
> +#define EPT_CAP_INVEPT_ALL (1ull << 26)
> +#define EPT_CAP_AD_FLAG (1ull << 21)
> +
> +#define PAGE_SIZE_2M (512 * PAGE_SIZE)
> +#define PAGE_SIZE_1G (512 * PAGE_SIZE_2M)
> +#define EPT_PAGE_LEVEL 4
> +#define EPT_PGDIR_WIDTH 9
> +#define EPT_PGDIR_MASK 511
> +#define PAGE_MASK (~(PAGE_SIZE-1))
> +
> +#define EPT_VLT_RD 1
> +#define EPT_VLT_WR (1 << 1)
> +#define EPT_VLT_FETCH (1 << 2)
> +#define EPT_VLT_PERM_RD (1 << 3)
> +#define EPT_VLT_PERM_WR (1 << 4)
> +#define EPT_VLT_PERM_EX (1 << 5)
> +#define EPT_VLT_LADDR_VLD (1 << 7)
> +#define EPT_VLT_PADDR (1 << 8)
> +
> +#define MAGIC_VAL_1 0x12345678ul
> +#define MAGIC_VAL_2 0x87654321ul
> +#define MAGIC_VAL_3 0xfffffffful
> +
> +#define INVEPT_SINGLE 1
> +#define INVEPT_GLOBAL 2
>
> extern struct regs regs;
>
> @@ -472,8 +525,31 @@ static inline int vmcs_save(struct vmcs **vmcs)
> return ret;
> }
>
> +static inline void invept(unsigned long type, u64 eptp)
> +{
> + struct {
> + u64 eptp, gpa;
> + } operand = {eptp, 0};
> + asm volatile("invept %0, %1\n" ::"m"(operand),"r"(type));
> +}
> +
> void report(const char *name, int result);
> void print_vmexit_info();
> +void install_ept_entry(unsigned long *pml4, int pte_level,
> + unsigned long guest_addr, unsigned long pte,
> + unsigned long *pt_page);
> +void install_1g_ept(unsigned long *pml4, unsigned long phys,
> + unsigned long guest_addr, u64 perm);
> +void install_2m_ept(unsigned long *pml4, unsigned long phys,
> + unsigned long guest_addr, u64 perm);
> +void install_ept(unsigned long *pml4, unsigned long phys,
> + unsigned long guest_addr, u64 perm);
> +int setup_ept_range(unsigned long *pml4, unsigned long start,
> + unsigned long len, int map_1g, int map_2m, u64 perm);
> +unsigned long get_ept_pte(unsigned long *pml4,
> + unsigned long guest_addr, int level);
> +int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
> + int level, u64 pte_val);
>
> #endif
>
>