On Fri, Feb 22, 2019 at 09:37:34PM +0100, Paolo Bonzini wrote:
> On 18/02/19 01:57, Krish Sadhukhan wrote:
> > According to section "Checks on VMX Controls" in Intel SDM vol 3C, the
> > following checks are performed on vmentry of L2 guests:
> >
> > - The CR0 field must not set any bit to a value not supported in VMX
> > operation.
> > - The CR4 field must not set any bit to a value not supported in VMX
> > operation.
> > - On processors that support Intel 64 architecture, the CR3 field must
> > be such that bits 63:52 and bits in the range 51:32 beyond the
> > processor’s physical-address width must be 0.
> >
> > Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> > Reviewed-by: Liam Merwick <liam.merwick@xxxxxxxxxx>
> > ---
> > lib/x86/processor.h | 1 +
> > x86/vmx_tests.c | 94 +++++++++++++++++++++++++++++++++++++++++++++
> > 2 files changed, 95 insertions(+)
> >
> > diff --git a/lib/x86/processor.h b/lib/x86/processor.h
> > index 15237a5..916e67d 100644
> > --- a/lib/x86/processor.h
> > +++ b/lib/x86/processor.h
> > @@ -40,6 +40,7 @@
> > #define X86_CR4_UMIP 0x00000800
> > #define X86_CR4_VMXE 0x00002000
> > #define X86_CR4_PCIDE 0x00020000
> > +#define X86_CR4_SMEP 0x00100000
> > #define X86_CR4_SMAP 0x00200000
> > #define X86_CR4_PKE 0x00400000
> >
> > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> > index 66a87f6..f5324ad 100644
> > --- a/x86/vmx_tests.c
> > +++ b/x86/vmx_tests.c
> > @@ -4995,6 +4995,98 @@ static void test_sysenter_field(u32 field, const char *name)
> > vmcs_write(field, addr_saved);
> > }
> >
> > +static void test_ctl_reg(const char *cr_name, u64 cr, u64 fixed0, u64 fixed1)
> > +{
> > + u64 val;
> > + u64 cr_saved = vmcs_read(cr);
> > + int i;
> > +
> > + val = fixed0 & fixed1;
> > + if (cr == HOST_CR4)
> > + vmcs_write(cr, val | X86_CR4_PAE);
> > + else
> > + vmcs_write(cr, val);
> > + report_prefix_pushf("%s %lx", cr_name, val);
> > + if (val == fixed0)
> > + test_vmx_vmlaunch(0, false);
> > + else
> > + test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
> > + false);
> > + report_prefix_pop();
> > +
> > + for (i = 0; i < 64; i++) {
> > +
> > + /* Set a bit when the corresponding bit in fixed1 is 0 */
> > + if ((fixed1 & (1ull << i)) == 0) {
> > + if (cr == HOST_CR4 && ((1ull << i) & X86_CR4_SMEP ||
> > + (1ull << i) & X86_CR4_SMAP))
> > + continue;
> > +
> > + vmcs_write(cr, cr_saved | (1ull << i));
> > + report_prefix_pushf("%s %llx", cr_name,
> > + cr_saved | (1ull << i));
> > + test_vmx_vmlaunch(
> > + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
> > + false);
> > + report_prefix_pop();
> > + }
> > +
> > + /* Unset a bit when the corresponding bit in fixed0 is 1 */
> > + if (fixed0 & (1ull << i)) {
> > + vmcs_write(cr, cr_saved & ~(1ull << i));
> > + report_prefix_pushf("%s %llx", cr_name,
> > + cr_saved & ~(1ull << i));
> > + test_vmx_vmlaunch(
> > + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
> > + false);
> > + report_prefix_pop();
> > + }
> > + }
> > +
> > + vmcs_write(cr, cr_saved);
> > +}
> > +
> > +/*
> > + * 1. The CR0 field must not set any bit to a value not supported in VMX
> > + * operation.
> > + * 2. The CR4 field must not set any bit to a value not supported in VMX
> > + * operation.
> > + * 3. On processors that support Intel 64 architecture, the CR3 field must
> > + * be such that bits 63:52 and bits in the range 51:32 beyond the
> > + * processor’s physical-address width must be 0.
> > + *
> > + * [Intel SDM]
> > + */
> > +static void test_host_ctl_regs(void)
> > +{
> > + u64 fixed0, fixed1, cr3, cr3_saved;
> > + int i;
> > +
> > + /* Test CR0 */
> > + fixed0 = rdmsr(MSR_IA32_VMX_CR0_FIXED0);
> > + fixed1 = rdmsr(MSR_IA32_VMX_CR0_FIXED1);
> > + test_ctl_reg("HOST_CR0", HOST_CR0, fixed0, fixed1);
> > +
> > + /* Test CR4 */
> > + fixed0 = rdmsr(MSR_IA32_VMX_CR4_FIXED0);
> > + fixed1 = rdmsr(MSR_IA32_VMX_CR4_FIXED1) &
> > + ~(X86_CR4_SMEP | X86_CR4_SMAP);
> > + test_ctl_reg("HOST_CR4", HOST_CR4, fixed0, fixed1);
> > +
> > + /* Test CR3 */
> > + cr3_saved = vmcs_read(HOST_CR3);
> > + for (i = cpuid_maxphyaddr(); i < 64; i++) {
> > + cr3 = cr3_saved | (1ul << i);
> > + vmcs_write(HOST_CR3, cr3);
> > + report_prefix_pushf("HOST_CR3 %lx", cr3);
> > + test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
> > + false);
> > + report_prefix_pop();
> > + }
> > +
> > + vmcs_write(HOST_CR3, cr3_saved);
> > +}
> > +
> > /*
> > * Check that the virtual CPU checks the VMX Host State Area as
> > * documented in the Intel SDM.
> > @@ -5008,6 +5100,8 @@ static void vmx_host_state_area_test(void)
> > */
> > vmcs_write(GUEST_RFLAGS, 0);
> >
> > + test_host_ctl_regs();
> > +
> > test_sysenter_field(HOST_SYSENTER_ESP, "HOST_SYSENTER_ESP");
> > test_sysenter_field(HOST_SYSENTER_EIP, "HOST_SYSENTER_EIP");
> > }
> >
>
> Applied, thanks.
>
> Paolo
The entirety of test_ctl_reg() was dropped when this was applied. It's
still the head commit, maybe you can just do a fixup?
x86/vmx_tests.c: In function ‘test_host_ctl_regs’:
x86/vmx_tests.c:5111:2: error: implicit declaration of function ‘test_ctl_reg’ [-Werror=implicit-function-declaration]
test_ctl_reg("HOST_CR0", HOST_CR0, fixed0, fixed1);
^
x86/vmx_tests.c: At top level:
cc1: error: unrecognized command line option ‘-Wno-frame-address’ [-Werror]
