[kvm-unit-tests] x86: Add control register pinning tests
diff mbox series

Message ID 20200617224606.27954-1-john.s.andersen@intel.com
State New
Headers show
Series
  • [kvm-unit-tests] x86: Add control register pinning tests
Related show

Commit Message

Andersen, John June 17, 2020, 10:46 p.m. UTC
Paravirutalized control register pinning adds MSRs guests can use to
discover which bits in CR0/4 they may pin, and MSRs for activating
pinning for any of those bits.

We check that the bits allowed to be pinned for CR4 are UMIP, SMEP, and
SMAP. Only WP should be allowed to be pinned in CR0.

We turn on all of the allowed bits, pin them, then attempt to disable
them. We verify that the attempt to disable was unsuccessful, and that
it generated a general protection fault.

For nested, we check that for when pinning enabled in L1, changing
HOST_CR0/4 will not result in the un-setting of pinned bits. The VMX CR
pinning tests is it's own test so that the pinning doesn't potentially
affect other tests within the same .flat testing VM.

Signed-off-by: John Andersen <john.s.andersen@intel.com>
---
 x86/Makefile.common |   3 +-
 lib/x86/desc.h      |   1 +
 lib/x86/msr.h       |   8 ++
 lib/x86/processor.h |   1 +
 lib/x86/desc.c      |   8 ++
 x86/cr_pin_high.c   | 185 ++++++++++++++++++++++++++++++++++++++++++++
 x86/cr_pin_low.c    |  55 +++++++++++++
 x86/pcid.c          |   8 --
 x86/vmx_tests.c     | 138 +++++++++++++++++++++++++++++++++
 x86/unittests.cfg   |  16 +++-
 10 files changed, 413 insertions(+), 10 deletions(-)
 create mode 100644 x86/cr_pin_high.c
 create mode 100644 x86/cr_pin_low.c

Comments

Nadav Amit June 17, 2020, 10:52 p.m. UTC | #1
> On Jun 17, 2020, at 3:46 PM, John Andersen <john.s.andersen@intel.com> wrote:
> 
> Paravirutalized control register pinning adds MSRs guests can use to
> discover which bits in CR0/4 they may pin, and MSRs for activating
> pinning for any of those bits.
> 

[ sni[

> +static void vmx_cr_pin_test_guest(void)
> +{
> +	unsigned long i, cr0, cr4;
> +
> +	/* Step 1. Skip feature detection to skip handling VMX_CPUID */
> +	/* nop */

I do not quite get this comment. Why do you skip checking whether the
feature is enabled? What happens if KVM/bare-metal/other-hypervisor that
runs this test does not support this feature?
Nadav Amit June 18, 2020, 3:18 a.m. UTC | #2
> On Jun 17, 2020, at 3:52 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>> On Jun 17, 2020, at 3:46 PM, John Andersen <john.s.andersen@intel.com> wrote:
>> 
>> Paravirutalized control register pinning adds MSRs guests can use to
>> discover which bits in CR0/4 they may pin, and MSRs for activating
>> pinning for any of those bits.
> 
> [ sni[
> 
>> +static void vmx_cr_pin_test_guest(void)
>> +{
>> +	unsigned long i, cr0, cr4;
>> +
>> +	/* Step 1. Skip feature detection to skip handling VMX_CPUID */
>> +	/* nop */
> 
> I do not quite get this comment. Why do you skip checking whether the
> feature is enabled? What happens if KVM/bare-metal/other-hypervisor that
> runs this test does not support this feature?

My bad, I was confused between the nested checks and the non-nested ones.

Nevertheless, can we avoid situations in which
rdmsr(MSR_KVM_CR0_PIN_ALLOWED) causes #GP when the feature is not
implemented? Is there some protocol for detection that this feature is
supported by the hypervisor, or do we need something like rdmsr_safe()?
Andersen, John June 18, 2020, 5:08 a.m. UTC | #3
On Wed, Jun 17, 2020 at 08:18:39PM -0700, Nadav Amit wrote:
> > On Jun 17, 2020, at 3:52 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
> > 
> >> On Jun 17, 2020, at 3:46 PM, John Andersen <john.s.andersen@intel.com> wrote:
> >> 
> >> Paravirutalized control register pinning adds MSRs guests can use to
> >> discover which bits in CR0/4 they may pin, and MSRs for activating
> >> pinning for any of those bits.
> > 
> > [ sni[
> > 
> >> +static void vmx_cr_pin_test_guest(void)
> >> +{
> >> +	unsigned long i, cr0, cr4;
> >> +
> >> +	/* Step 1. Skip feature detection to skip handling VMX_CPUID */
> >> +	/* nop */
> > 
> > I do not quite get this comment. Why do you skip checking whether the
> > feature is enabled? What happens if KVM/bare-metal/other-hypervisor that
> > runs this test does not support this feature?
> 
> My bad, I was confused between the nested checks and the non-nested ones.
> 
> Nevertheless, can we avoid situations in which
> rdmsr(MSR_KVM_CR0_PIN_ALLOWED) causes #GP when the feature is not
> implemented? Is there some protocol for detection that this feature is
> supported by the hypervisor, or do we need something like rdmsr_safe()?
> 

Ah, yes we can. By checking the CPUID for the feature bit. Thanks for pointing
this out, I was confused about this. I was operating under the assumption that
the unit tests assume the features in the latest kvm/next are present and
available when the unit tests are being run.

I'm happy to add the check, but I haven't see anywhere else where a
KVM_FEATURE_ was checked for. Which is why it doesn't check in this patch. As
soon as I get an answer from you or anyone else as to if the unit tests assume
that the features in the latest kvm/next are present and available or not when
the unit tests are being run I'll modify if necessary.

Thanks,
John
Nadav Amit June 18, 2020, 6:59 a.m. UTC | #4
> On Jun 17, 2020, at 10:08 PM, Andersen, John <john.s.andersen@intel.com> wrote:
> 
> On Wed, Jun 17, 2020 at 08:18:39PM -0700, Nadav Amit wrote:
>>> On Jun 17, 2020, at 3:52 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
>>> 
>>>> On Jun 17, 2020, at 3:46 PM, John Andersen <john.s.andersen@intel.com> wrote:
>>>> 
>>>> Paravirutalized control register pinning adds MSRs guests can use to
>>>> discover which bits in CR0/4 they may pin, and MSRs for activating
>>>> pinning for any of those bits.
>>> 
>>> [ sni[
>>> 
>>>> +static void vmx_cr_pin_test_guest(void)
>>>> +{
>>>> +	unsigned long i, cr0, cr4;
>>>> +
>>>> +	/* Step 1. Skip feature detection to skip handling VMX_CPUID */
>>>> +	/* nop */
>>> 
>>> I do not quite get this comment. Why do you skip checking whether the
>>> feature is enabled? What happens if KVM/bare-metal/other-hypervisor that
>>> runs this test does not support this feature?
>> 
>> My bad, I was confused between the nested checks and the non-nested ones.
>> 
>> Nevertheless, can we avoid situations in which
>> rdmsr(MSR_KVM_CR0_PIN_ALLOWED) causes #GP when the feature is not
>> implemented? Is there some protocol for detection that this feature is
>> supported by the hypervisor, or do we need something like rdmsr_safe()?
> 
> Ah, yes we can. By checking the CPUID for the feature bit. Thanks for pointing
> this out, I was confused about this. I was operating under the assumption that
> the unit tests assume the features in the latest kvm/next are present and
> available when the unit tests are being run.
> 
> I'm happy to add the check, but I haven't see anywhere else where a
> KVM_FEATURE_ was checked for. Which is why it doesn't check in this patch. As
> soon as I get an answer from you or anyone else as to if the unit tests assume
> that the features in the latest kvm/next are present and available or not when
> the unit tests are being run I'll modify if necessary.

I would appreciate if you add a check of CPUID and not run the test if the 
feature is not supported.

I run the tests on bare-metal (and other non-KVM environment) from time to
time. Doing so allows to find bugs in tests due to wrong assumptions of KVM
test developers. Liran runs the tests using QEMU/WHPX (non-KVM). So allowing
the tests to run on non-KVM environments is important, at least for some of
us, and benefits KVM as well.

While I can disable this specific test using the test parameters, I prefer
that the test will first check the environment they run on. Debugging test
failures on bare-metal is hard enough without the paravirt stuff noise.
Andersen, John June 18, 2020, 1:31 p.m. UTC | #5
On Wed, Jun 17, 2020 at 11:59:10PM -0700, Nadav Amit wrote:
> > On Jun 17, 2020, at 10:08 PM, Andersen, John <john.s.andersen@intel.com> wrote:
> > 
> > On Wed, Jun 17, 2020 at 08:18:39PM -0700, Nadav Amit wrote:
> >>> On Jun 17, 2020, at 3:52 PM, Nadav Amit <nadav.amit@gmail.com> wrote:
> >>> 
> >>>> On Jun 17, 2020, at 3:46 PM, John Andersen <john.s.andersen@intel.com> wrote:
> >>>> 
> >>>> Paravirutalized control register pinning adds MSRs guests can use to
> >>>> discover which bits in CR0/4 they may pin, and MSRs for activating
> >>>> pinning for any of those bits.
> >>> 
> >>> [ sni[
> >>> 
> >>>> +static void vmx_cr_pin_test_guest(void)
> >>>> +{
> >>>> +	unsigned long i, cr0, cr4;
> >>>> +
> >>>> +	/* Step 1. Skip feature detection to skip handling VMX_CPUID */
> >>>> +	/* nop */
> >>> 
> >>> I do not quite get this comment. Why do you skip checking whether the
> >>> feature is enabled? What happens if KVM/bare-metal/other-hypervisor that
> >>> runs this test does not support this feature?
> >> 
> >> My bad, I was confused between the nested checks and the non-nested ones.
> >> 
> >> Nevertheless, can we avoid situations in which
> >> rdmsr(MSR_KVM_CR0_PIN_ALLOWED) causes #GP when the feature is not
> >> implemented? Is there some protocol for detection that this feature is
> >> supported by the hypervisor, or do we need something like rdmsr_safe()?
> > 
> > Ah, yes we can. By checking the CPUID for the feature bit. Thanks for pointing
> > this out, I was confused about this. I was operating under the assumption that
> > the unit tests assume the features in the latest kvm/next are present and
> > available when the unit tests are being run.
> > 
> > I'm happy to add the check, but I haven't see anywhere else where a
> > KVM_FEATURE_ was checked for. Which is why it doesn't check in this patch. As
> > soon as I get an answer from you or anyone else as to if the unit tests assume
> > that the features in the latest kvm/next are present and available or not when
> > the unit tests are being run I'll modify if necessary.
> 
> I would appreciate if you add a check of CPUID and not run the test if the 
> feature is not supported.
> 
> I run the tests on bare-metal (and other non-KVM environment) from time to
> time. Doing so allows to find bugs in tests due to wrong assumptions of KVM
> test developers. Liran runs the tests using QEMU/WHPX (non-KVM). So allowing
> the tests to run on non-KVM environments is important, at least for some of
> us, and benefits KVM as well.
> 
> While I can disable this specific test using the test parameters, I prefer
> that the test will first check the environment they run on. Debugging test
> failures on bare-metal is hard enough without the paravirt stuff noise.
> 

Great point! I'll add the check

Patch
diff mbox series

diff --git a/x86/Makefile.common b/x86/Makefile.common
index ab67ca0..bab7fe2 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -58,7 +58,8 @@  tests-common = $(TEST_DIR)/vmexit.flat $(TEST_DIR)/tsc.flat \
                $(TEST_DIR)/init.flat $(TEST_DIR)/smap.flat \
                $(TEST_DIR)/hyperv_synic.flat $(TEST_DIR)/hyperv_stimer.flat \
                $(TEST_DIR)/hyperv_connections.flat \
-               $(TEST_DIR)/umip.flat $(TEST_DIR)/tsx-ctrl.flat
+               $(TEST_DIR)/umip.flat $(TEST_DIR)/tsx-ctrl.flat \
+               $(TEST_DIR)/cr_pin_low.flat $(TEST_DIR)/cr_pin_high.flat
 
 test_cases: $(tests-common) $(tests)
 
diff --git a/lib/x86/desc.h b/lib/x86/desc.h
index 0fe5cbf..9fb921c 100644
--- a/lib/x86/desc.h
+++ b/lib/x86/desc.h
@@ -211,6 +211,7 @@  extern tss64_t tss;
 #endif
 
 unsigned exception_vector(void);
+int write_cr0_checking(unsigned long val);
 int write_cr4_checking(unsigned long val);
 unsigned exception_error_code(void);
 bool exception_rflags_rf(void);
diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 6ef5502..13152a3 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -431,4 +431,12 @@ 
 #define MSR_VM_IGNNE                    0xc0010115
 #define MSR_VM_HSAVE_PA                 0xc0010117
 
+/* KVM MSRs */
+#define MSR_KVM_CR0_PIN_ALLOWED		0x4b564d08
+#define MSR_KVM_CR4_PIN_ALLOWED		0x4b564d09
+#define MSR_KVM_CR0_PINNED_LOW		0x4b564d0a
+#define MSR_KVM_CR0_PINNED_HIGH		0x4b564d0b
+#define MSR_KVM_CR4_PINNED_LOW		0x4b564d0c
+#define MSR_KVM_CR4_PINNED_HIGH		0x4b564d0d
+
 #endif /* _ASM_X86_MSR_INDEX_H */
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 6e0811e..6769ca6 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -146,6 +146,7 @@  static inline u8 cpuid_maxphyaddr(void)
 #define	X86_FEATURE_SMEP	        (CPUID(0x7, 0, EBX, 7))
 #define	X86_FEATURE_INVPCID		(CPUID(0x7, 0, EBX, 10))
 #define	X86_FEATURE_RTM			(CPUID(0x7, 0, EBX, 11))
+#define	X86_FEATURE_SMEP		(CPUID(0x7, 0, EBX, 7))
 #define	X86_FEATURE_SMAP		(CPUID(0x7, 0, EBX, 20))
 #define	X86_FEATURE_PCOMMIT		(CPUID(0x7, 0, EBX, 22))
 #define	X86_FEATURE_CLFLUSHOPT		(CPUID(0x7, 0, EBX, 23))
diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 451f504..c9363d0 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -251,6 +251,14 @@  unsigned exception_vector(void)
     return vector;
 }
 
+int write_cr0_checking(unsigned long val)
+{
+    asm volatile(ASM_TRY("1f")
+                 "mov %0, %%cr0\n\t"
+                 "1:": : "r" (val));
+    return exception_vector();
+}
+
 int write_cr4_checking(unsigned long val)
 {
     asm volatile(ASM_TRY("1f")
diff --git a/x86/cr_pin_high.c b/x86/cr_pin_high.c
new file mode 100644
index 0000000..c92c16f
--- /dev/null
+++ b/x86/cr_pin_high.c
@@ -0,0 +1,185 @@ 
+/* CR pinning tests. Not including pinning CR0 WP low, that lives in
+ * cr_pin_low.c. This file tests that CR pinning prevents the guest VM from
+ * flipping bit pinned via MSRs in control registers. For CR4 we pin UMIP and
+ * SMEP bits high and SMAP low, and verify we can't toggle them after pinning */
+
+#include "libcflat.h"
+#include "x86/desc.h"
+#include "x86/processor.h"
+#include "x86/vm.h"
+#include "x86/msr.h"
+
+#define USER_BASE	(1 << 24)
+
+#define CR0_PINNED X86_CR0_WP
+#define CR4_SOME_PINNED (X86_CR4_UMIP | X86_CR4_SMEP)
+#define CR4_ALL_PINNED (CR4_SOME_PINNED | X86_CR4_SMAP)
+
+static void test_cr0_pinning(void)
+{
+	unsigned long long r = 0;
+	ulong cr0 = read_cr0();
+	int vector = 0;
+
+	r = rdmsr(MSR_KVM_CR0_PIN_ALLOWED);
+	report(r == CR0_PINNED, "[CR0] MSR_KVM_CR0_PIN_ALLOWED: %llx", r);
+
+	cr0 |= CR0_PINNED;
+
+	vector = write_cr0_checking(cr0);
+	report(vector == 0, "[CR0] enable pinned bits. vector: %d", vector);
+
+	cr0 = read_cr0();
+	report((cr0 & CR0_PINNED) == CR0_PINNED, "[CR0] after enabling pinned bits: %lx", cr0);
+
+	wrmsr(MSR_KVM_CR0_PINNED_HIGH, CR0_PINNED);
+	r = rdmsr(MSR_KVM_CR0_PINNED_HIGH);
+	report(r == CR0_PINNED, "[CR0] enable pinning. MSR_KVM_CR0_PINNED_HIGH: %llx", r);
+
+	vector = write_cr0_checking(cr0);
+	report(vector == 0, "[CR0] write same value");
+
+	vector = write_cr0_checking(cr0 & ~CR0_PINNED);
+	report(vector == GP_VECTOR, "[CR0] disable pinned bits. vector: %d", vector);
+
+	cr0 = read_cr0();
+	report((cr0 & CR0_PINNED) == CR0_PINNED, "[CR0] pinned bits: %lx", cr0 & CR0_PINNED);
+}
+
+static void test_cr4_pin_allowed(void)
+{
+	unsigned long long r = 0;
+
+	r = rdmsr(MSR_KVM_CR4_PIN_ALLOWED);
+	report(r == CR4_ALL_PINNED, "[CR4] MSR_KVM_CR4_PIN_ALLOWED: %llx", r);
+}
+
+static void test_cr4_pinning_some_umip_smep_pinned(void)
+{
+	unsigned long long r = 0;
+	ulong cr4 = read_cr4();
+	int vector = 0;
+
+	cr4 |= CR4_SOME_PINNED;
+
+	vector = write_cr4_checking(cr4);
+	report(vector == 0, "[CR4 SOME] enable pinned bits. vector: %d", vector);
+
+	wrmsr(MSR_KVM_CR4_PINNED_HIGH, CR4_SOME_PINNED);
+	r = rdmsr(MSR_KVM_CR4_PINNED_HIGH);
+	report(r == CR4_SOME_PINNED, "[CR4 SOME] enable pinning. MSR_KVM_CR4_PINNED_HIGH: %llx", r);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_SOME_PINNED) == CR4_SOME_PINNED, "[CR4 SOME] after enabling pinned bits: %lx", cr4);
+	report(1, "[CR4 SOME] cr4: 0x%08lx", cr4);
+
+	vector = write_cr4_checking(cr4);
+	report(vector == 0, "[CR4 SOME] write same value");
+
+	vector = write_cr4_checking(cr4 & ~CR4_SOME_PINNED);
+	report(vector == GP_VECTOR, "[CR4 SOME] disable pinned bits. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_SOME_PINNED) == CR4_SOME_PINNED, "[CR4 SOME] pinned bits: %lx", cr4 & CR4_SOME_PINNED);
+
+	vector = write_cr4_checking(cr4 & ~X86_CR4_SMEP);
+	report(vector == GP_VECTOR, "[CR4 SOME] disable single pinned bit. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_SOME_PINNED) == CR4_SOME_PINNED, "[CR4 SOME] pinned bits: %lx", cr4 & CR4_SOME_PINNED);
+}
+
+static void test_cr4_pinning_all_umip_smep_high_smap_low_pinned(void)
+{
+	unsigned long long r = 0;
+	ulong cr4 = read_cr4();
+	int vector = 0;
+
+	cr4 |= CR4_SOME_PINNED;
+	cr4 &= ~X86_CR4_SMAP;
+
+	report((cr4 & CR4_ALL_PINNED) == CR4_SOME_PINNED, "[CR4 ALL] CHECK: %lx", cr4);
+
+	vector = write_cr4_checking(cr4);
+	report(vector == 0, "[CR4 ALL] write bits to cr4. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_ALL_PINNED) == CR4_SOME_PINNED, "[CR4 ALL] after enabling pinned bits: %lx", cr4);
+
+	wrmsr(MSR_KVM_CR4_PINNED_LOW, X86_CR4_SMAP);
+	r = rdmsr(MSR_KVM_CR4_PINNED_LOW);
+	report(r == X86_CR4_SMAP, "[CR4 ALL] enable pinning. MSR_KVM_CR4_PINNED_LOW: %llx", r);
+
+	vector = write_cr4_checking(cr4);
+	report(vector == 0, "[CR4 ALL] write same value");
+
+	cr4 &= ~CR4_SOME_PINNED;
+	cr4 |= X86_CR4_SMAP;
+
+	vector = write_cr4_checking(cr4);
+	report(vector == GP_VECTOR, "[CR4 ALL] disable pinned bits. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_ALL_PINNED) == CR4_SOME_PINNED, "[CR4 ALL] pinned bits: %lx", cr4 & CR4_ALL_PINNED);
+
+	vector = write_cr4_checking(cr4 | X86_CR4_SMAP);
+	report(vector == GP_VECTOR, "[CR4 ALL] enable pinned low bit. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_ALL_PINNED) == CR4_SOME_PINNED, "[CR4 ALL] pinned bits: %lx", cr4 & CR4_ALL_PINNED);
+
+	vector = write_cr4_checking(cr4 & ~X86_CR4_SMEP);
+	report(vector == GP_VECTOR, "[CR4 ALL] disable pinned high bit. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_ALL_PINNED) == CR4_SOME_PINNED, "[CR4 ALL] pinned bits: %lx", cr4 & CR4_ALL_PINNED);
+
+	vector = write_cr4_checking((cr4 & ~X86_CR4_SMEP) | X86_CR4_SMAP);
+	report(vector == GP_VECTOR, "[CR4 ALL] disable pinned high bit enable pinned low. vector: %d", vector);
+
+	cr4 = read_cr4();
+	report((cr4 & CR4_ALL_PINNED) == CR4_SOME_PINNED, "[CR4 ALL] pinned bits: %lx", cr4 & CR4_ALL_PINNED);
+}
+
+static void test_cr4_pinning(void)
+{
+	test_cr4_pin_allowed();
+
+	if (!this_cpu_has(X86_FEATURE_UMIP)) {
+		printf("UMIP not available\n");
+		return;
+	}
+
+	if (!this_cpu_has(X86_FEATURE_SMEP)) {
+		printf("SMEP not available\n");
+		return;
+	}
+
+	test_cr4_pinning_some_umip_smep_pinned();
+
+	if (!this_cpu_has(X86_FEATURE_SMAP)) {
+		printf("SMAP not enabled\n");
+		return;
+	}
+
+	test_cr4_pinning_all_umip_smep_high_smap_low_pinned();
+}
+
+int main(int ac, char **av)
+{
+	unsigned long i;
+
+	setup_idt();
+	setup_vm();
+
+	// Map first 16MB as supervisor pages
+	for (i = 0; i < USER_BASE; i += PAGE_SIZE) {
+		*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PT_USER_MASK;
+	}
+
+	test_cr0_pinning();
+	test_cr4_pinning();
+
+	return report_summary();
+}
+
diff --git a/x86/cr_pin_low.c b/x86/cr_pin_low.c
new file mode 100644
index 0000000..d5109bf
--- /dev/null
+++ b/x86/cr_pin_low.c
@@ -0,0 +1,55 @@ 
+/* CR pinning tests to check that WP bit in CR0 be pinned low correctly. This
+ * has to be in a separate file than the CR0 high pinning because once pinned we
+ * can't unpin from the guest. So we can't switch from high pinning to low
+ * pinning to test within the same file / VM */
+
+#include "libcflat.h"
+#include "x86/desc.h"
+#include "x86/processor.h"
+#include "x86/vm.h"
+#include "x86/msr.h"
+
+#define USER_BASE	(1 << 24)
+
+#define CR0_PINNED X86_CR0_WP
+
+static void test_cr0_pinning_low(void)
+{
+	unsigned long long r = 0;
+	ulong cr0 = read_cr0();
+	int vector = 0;
+
+	r = rdmsr(MSR_KVM_CR0_PIN_ALLOWED);
+	report(r == CR0_PINNED, "[CR0] MSR_KVM_CR0_PIN_ALLOWED: %llx", r);
+
+	cr0 &= ~CR0_PINNED;
+
+	vector = write_cr0_checking(cr0);
+	report(vector == 0, "[CR0] enable pinned bits. vector: %d", vector);
+
+	cr0 = read_cr0();
+	report((cr0 & CR0_PINNED) != CR0_PINNED, "[CR0] after enabling pinned bits: %lx", cr0);
+
+	wrmsr(MSR_KVM_CR0_PINNED_LOW, CR0_PINNED);
+	r = rdmsr(MSR_KVM_CR0_PINNED_LOW);
+	report(r == CR0_PINNED, "[CR0] enable pinning. MSR_KVM_CR0_PINNED_LOW: %llx", r);
+
+	vector = write_cr0_checking(cr0);
+	report(vector == 0, "[CR0] write same value");
+
+	vector = write_cr0_checking(cr0 | CR0_PINNED);
+	report(vector == GP_VECTOR, "[CR0] disable pinned bits. vector: %d", vector);
+
+	cr0 = read_cr0();
+	report((cr0 & CR0_PINNED) != CR0_PINNED, "[CR0] pinned bits: %lx", cr0 & CR0_PINNED);
+}
+
+int main(int ac, char **av)
+{
+	setup_idt();
+
+	test_cr0_pinning_low();
+
+	return report_summary();
+}
+
diff --git a/x86/pcid.c b/x86/pcid.c
index a8dc8cb..5688699 100644
--- a/x86/pcid.c
+++ b/x86/pcid.c
@@ -10,14 +10,6 @@  struct invpcid_desc {
     unsigned long addr : 64;
 };
 
-static int write_cr0_checking(unsigned long val)
-{
-    asm volatile(ASM_TRY("1f")
-                 "mov %0, %%cr0\n\t"
-                 "1:": : "r" (val));
-    return exception_vector();
-}
-
 static int invpcid_checking(unsigned long type, void *desc)
 {
     asm volatile (ASM_TRY("1f")
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 68f93d3..51fcea9 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7929,6 +7929,142 @@  static void vmentry_movss_shadow_test(void)
 	vmcs_write(GUEST_RFLAGS, X86_EFLAGS_FIXED);
 }
 
+#define USER_BASE	(1 << 24)
+
+#define CR0_PINNED X86_CR0_WP
+#define CR4_SOME_PINNED (X86_CR4_UMIP | X86_CR4_SMEP)
+#define CR4_ALL_PINNED (CR4_SOME_PINNED | X86_CR4_SMAP)
+
+static void vmx_cr_pin_test_guest(void)
+{
+	unsigned long i, cr0, cr4;
+
+	/* Step 1. Skip feature detection to skip handling VMX_CPUID */
+	/* nop */
+
+	/* Step 2. Setup supervisor pages so that we can enable SMAP */
+	setup_vm();
+	for (i = 0; i < USER_BASE; i += PAGE_SIZE) {
+		*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PT_USER_MASK;
+	}
+
+	/* Step 3. Enable CR0/4 bits */
+	cr0 = read_cr0() | X86_CR0_WP;
+	TEST_ASSERT(!write_cr0_checking(cr0));
+	cr0 = read_cr0();
+	report(cr0 & X86_CR0_WP, "L2 cr0 has WP bit: cr0(%lx)", cr0);
+
+	cr4 = read_cr4() | (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP);
+	TEST_ASSERT(!write_cr4_checking(cr4));
+	cr4 = read_cr4();
+	report((cr4 & (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP)) == (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP),
+		"L2 cr4 has correct bits: cr4(%lx)", cr4);
+
+	/* Step 3. Call to host */
+	vmx_set_test_stage(1);
+	vmcall();
+
+	/* Step 4. Disable bits that the host has pinned to make sure host
+	 * pinning doesn't effect us. */
+	cr0 = read_cr0() & ~X86_CR0_WP;
+	TEST_ASSERT(!write_cr0_checking(cr0));
+	cr0 = read_cr0();
+	report(!(cr0 & X86_CR0_WP),
+		"L2 cr0 should not have host pinned WP bit: cr0(%lx)", cr0);
+
+	cr4 = read_cr4() & ~(X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP);
+	TEST_ASSERT(!write_cr4_checking(cr4));
+	cr4 = read_cr4();
+	report(!(cr4 & (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP)),
+		"L2 cr4 should not have host pinned bits: cr4(%lx)", cr4);
+
+	/* Step 5. Call to host */
+	vmx_set_test_stage(2);
+	vmcall();
+
+	/* Step 6. Ensure CR registers still do not contain L1 pinned values */
+	report(!(cr0 & X86_CR0_WP),
+		"L2 cr0 should still not have host pinned WP bit: cr0(%lx)", cr0);
+	report(!(cr4 & (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP)),
+		"L2 cr4 should still not have host pinned bits: cr4(%lx)", cr4);
+
+	vmx_set_test_stage(3);
+}
+
+static void vmx_cr_pin_test(void)
+{
+	unsigned long long r = 0;
+	unsigned long i, cr0, cr4;
+
+	/* Step 1. Ensure we have required CR4 bits */
+	if (!this_cpu_has(X86_FEATURE_UMIP)) {
+		report_skip("UMIP not detected");
+		return;
+	}
+	if (!this_cpu_has(X86_FEATURE_SMEP)) {
+		report_skip("SMEP not detected");
+		return;
+	}
+	if (!this_cpu_has(X86_FEATURE_SMAP)) {
+		report_skip("SMAP not detected");
+		return;
+	}
+	/* Step 2. Setup supervisor pages so that we can enable SMAP */
+	setup_vm();
+	for (i = 0; i < USER_BASE; i += PAGE_SIZE) {
+		*get_pte(phys_to_virt(read_cr3()), phys_to_virt(i)) &= ~PT_USER_MASK;
+	}
+
+	/* Step 3. Enable CR0/4 bits */
+	cr0 = read_cr0() | X86_CR0_WP;
+	TEST_ASSERT(!write_cr0_checking(cr0));
+	cr0 = read_cr0();
+	report(cr0 & X86_CR0_WP, "L1 cr0 has WP bit: cr0(%lx)", cr0);
+
+	cr4 = read_cr4() | (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP);
+	TEST_ASSERT(!write_cr4_checking(cr4));
+	cr4 = read_cr4();
+	report((cr4 & (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP)) == (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP),
+		"L1 cr4 has correct bits: cr4(%lx)", cr4);
+
+	/* Step 3. Pin CR0/4 bits */
+	wrmsr(MSR_KVM_CR0_PINNED_HIGH, CR0_PINNED);
+	r = rdmsr(MSR_KVM_CR0_PINNED_HIGH);
+	report(r == X86_CR0_WP, "MSR_KVM_CR0_PINNED_HIGH: %llx", r);
+
+	wrmsr(MSR_KVM_CR4_PINNED_HIGH, (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP));
+	r = rdmsr(MSR_KVM_CR4_PINNED_HIGH);
+	report(r == (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP), "MSR_KVM_CR4_PINNED_HIGH: %llx", r);
+
+	/* Step 4. Enter guest for first time */
+	test_set_guest(vmx_cr_pin_test_guest);
+	enter_guest();
+	skip_exit_vmcall();
+	TEST_ASSERT_EQ(vmx_get_test_stage(), 1);
+
+	/* Step 5. Modify VMCS so that Host-state contains unpinned cr values */
+	vmcs_write(HOST_CR0, read_cr0() & ~X86_CR0_WP);
+	vmcs_write(HOST_CR4, read_cr4() & ~(X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP));
+
+	/* Step 6. Enter guest second time */
+	enter_guest();
+	skip_exit_vmcall();
+	TEST_ASSERT_EQ(vmx_get_test_stage(), 2);
+
+	/* Step 7. Ensure CR registers still contain pinned values */
+	cr0 = read_cr0();
+	cr4 = read_cr4();
+	report(cr0 & X86_CR0_WP,
+		"L1 cr0 WP bit still pinned: cr0(%lx)", cr0);
+	report((cr4 & (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP)) == (X86_CR4_UMIP | X86_CR4_SMAP | X86_CR4_SMEP),
+		"L1 cr4 bits still pinned: cr4(%lx)", cr4);
+
+	/* Step 8. Run L2 to completion */
+	enter_guest();
+	skip_exit_vmcall();
+	TEST_ASSERT_EQ(vmx_get_test_stage(), 3);
+}
+
 static void vmx_cr_load_test(void)
 {
 	unsigned long cr3, cr4, orig_cr3, orig_cr4;
@@ -10050,6 +10186,8 @@  struct vmx_test vmx_tests[] = {
 	TEST(vmx_init_signal_test),
 	/* VMCS Shadowing tests */
 	TEST(vmx_vmcs_shadow_test),
+	/* CR pinning tests */
+	TEST(vmx_cr_pin_test),
 	/* Regression tests */
 	TEST(vmx_cr_load_test),
 	TEST(vmx_nm_test),
diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 504e04e..708bca4 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -245,9 +245,17 @@  arch = x86_64
 file = umip.flat
 extra_params = -cpu qemu64,+umip
 
+[cr_pin_low]
+file = cr_pin_low.flat
+extra_params = -cpu qemu64,+umip,+smap,+smep
+
+[cr_pin_high]
+file = cr_pin_high.flat
+extra_params = -cpu qemu64,+umip,+smap,+smep
+
 [vmx]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test"
+extra_params = -cpu host,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -atomic_switch_overflow_msrs_test -vmx_init_signal_test -vmx_apic_passthrough_tpr_threshold_test -vmx_cr_pin_test"
 arch = x86_64
 groups = vmx
 
@@ -306,6 +314,12 @@  extra_params = -cpu host,+vmx -append vmx_vmcs_shadow_test
 arch = x86_64
 groups = vmx
 
+[vmx_cr_pin_test]
+file = vmx.flat
+extra_params = -cpu host,+vmx -append vmx_cr_pin_test
+arch = x86_64
+groups = vmx
+
 [debug]
 file = debug.flat
 arch = x86_64