[kvm-unit-tests PATCH 0/3] x86/msr: Add tests for command MSRs

[kvm-unit-tests PATCH 0/3] x86/msr: Add tests for command MSRs

[Sean Christopherson]

Add testcases for the write-only MSRs IA32_PRED_CMD and IA32_FLUSH_CMD.

Note, this depends on the x2APIC MSR series[*].  Unless someone yells,
I'll include both in a pull request later this week.

[*] https://lkml.kernel.org/r/20230107011737.577244-1-seanjc%40google.com

Sean Christopherson (3):
  x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)
  x86/msr: Add testcases for MSR_IA32_PRED_CMD and its IBPB command
  x86/msr: Add testcases for MSR_IA32_FLUSH_CMD and its L1D_FLUSH
    command

 lib/x86/msr.h       |  4 ++++
 lib/x86/processor.h |  1 +
 x86/msr.c           | 38 ++++++++++++++++++++++++++++++++++++++
 x86/vmexit.c        |  2 +-
 4 files changed, 44 insertions(+), 1 deletion(-)


base-commit: fa1bddcf1565fc90b98f760358ff74d741fd9a2f
-- 
2.40.0.348.gf938b09366-goog

[kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Sean Christopherson]

Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
the nVMX library.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/msr.h | 1 +
 x86/vmexit.c  | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index c9869be5..29fff553 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -34,6 +34,7 @@
 /* Intel MSRs. Some also available on other CPUs */
 #define MSR_IA32_SPEC_CTRL              0x00000048
 #define MSR_IA32_PRED_CMD               0x00000049
+#define PRED_CMD_IBPB			BIT(0)
 
 #define MSR_IA32_PMC0                  0x000004c1
 #define MSR_IA32_PERFCTR0		0x000000c1
diff --git a/x86/vmexit.c b/x86/vmexit.c
index b1eed8d1..2e8866e1 100644
--- a/x86/vmexit.c
+++ b/x86/vmexit.c
@@ -463,7 +463,7 @@ static int has_spec_ctrl(void)
 
 static void wr_ibpb_msr(void)
 {
-	wrmsr(MSR_IA32_PRED_CMD, 1);
+	wrmsr(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);
 }
 
 static void toggle_cr0_wp(void)
-- 
2.40.0.348.gf938b09366-goog

[kvm-unit-tests PATCH 2/3] x86/msr: Add testcases for MSR_IA32_PRED_CMD and its IBPB command

[Sean Christopherson]

Add test coverage to verify MSR_IA32_PRED_CMD is write-only, that it can
be written with '0' (nop command) and '1' (IBPB command) when IBPB is
supported by the CPU (SPEC_CTRL on Intel, IBPB on AMD), and that writing
any other bit (1-63) triggers a #GP due to the bits/commands being
reserved.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 x86/msr.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/x86/msr.c b/x86/msr.c
index 97cf5987..13cb6391 100644
--- a/x86/msr.c
+++ b/x86/msr.c
@@ -85,6 +85,15 @@ static void test_msr_rw(u32 msr, const char *name, unsigned long long val)
 	__test_msr_rw(msr, name, val, 0);
 }
 
+static void test_wrmsr(u32 msr, const char *name, unsigned long long val)
+{
+	unsigned char vector = wrmsr_safe(msr, val);
+
+	report(!vector,
+	       "Expected success on WRSMR(%s, 0x%llx), got vector %d",
+	       name, val, vector);
+}
+
 static void test_wrmsr_fault(u32 msr, const char *name, unsigned long long val)
 {
 	unsigned char vector = wrmsr_safe(msr, val);
@@ -271,6 +280,23 @@ static void test_x2apic_msrs(void)
 	__test_x2apic_msrs(true);
 }
 
+static void test_cmd_msrs(void)
+{
+	int i;
+
+	test_rdmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD");
+	if (this_cpu_has(X86_FEATURE_SPEC_CTRL) ||
+	    this_cpu_has(X86_FEATURE_AMD_IBPB)) {
+		test_wrmsr(MSR_IA32_PRED_CMD, "PRED_CMD", 0);
+		test_wrmsr(MSR_IA32_PRED_CMD, "PRED_CMD", PRED_CMD_IBPB);
+	} else {
+		test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", 0);
+		test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", PRED_CMD_IBPB);
+	}
+	for (i = 1; i < 64; i++)
+		test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", BIT_ULL(i));
+}
+
 int main(int ac, char **av)
 {
 	/*
@@ -283,6 +309,7 @@ int main(int ac, char **av)
 		test_misc_msrs();
 		test_mce_msrs();
 		test_x2apic_msrs();
+		test_cmd_msrs();
 	}
 
 	return report_summary();
-- 
2.40.0.348.gf938b09366-goog

[kvm-unit-tests PATCH 3/3] x86/msr: Add testcases for MSR_IA32_FLUSH_CMD and its L1D_FLUSH command

[Sean Christopherson]

Add test coverage to verify MSR_IA32_FLUSH_CMD is write-only, that it can
be written with '0' (nop command) and '1' (L1D flush command) when the L1D
flush command is suported, and that writing any other bit (1-63) triggers
a #GP due to the bits/commands being reserved.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 lib/x86/msr.h       |  3 +++
 lib/x86/processor.h |  1 +
 x86/msr.c           | 11 +++++++++++
 3 files changed, 15 insertions(+)

diff --git a/lib/x86/msr.h b/lib/x86/msr.h
index 29fff553..0e3fd037 100644
--- a/lib/x86/msr.h
+++ b/lib/x86/msr.h
@@ -36,6 +36,9 @@
 #define MSR_IA32_PRED_CMD               0x00000049
 #define PRED_CMD_IBPB			BIT(0)
 
+#define MSR_IA32_FLUSH_CMD		0x0000010b
+#define L1D_FLUSH			BIT(0)
+
 #define MSR_IA32_PMC0                  0x000004c1
 #define MSR_IA32_PERFCTR0		0x000000c1
 #define MSR_IA32_PERFCTR1		0x000000c2
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index aed6d180..e32c84f7 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -246,6 +246,7 @@ static inline bool is_intel(void)
 #define	X86_FEATURE_SHSTK		(CPUID(0x7, 0, ECX, 7))
 #define	X86_FEATURE_IBT			(CPUID(0x7, 0, EDX, 20))
 #define	X86_FEATURE_SPEC_CTRL		(CPUID(0x7, 0, EDX, 26))
+#define	X86_FEATURE_FLUSH_L1D		(CPUID(0x7, 0, EDX, 28))
 #define	X86_FEATURE_ARCH_CAPABILITIES	(CPUID(0x7, 0, EDX, 29))
 #define	X86_FEATURE_PKS			(CPUID(0x7, 0, ECX, 31))
 
diff --git a/x86/msr.c b/x86/msr.c
index 13cb6391..f6be2be7 100644
--- a/x86/msr.c
+++ b/x86/msr.c
@@ -295,6 +295,17 @@ static void test_cmd_msrs(void)
 	}
 	for (i = 1; i < 64; i++)
 		test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", BIT_ULL(i));
+
+	test_rdmsr_fault(MSR_IA32_FLUSH_CMD, "FLUSH_CMD");
+	if (this_cpu_has(X86_FEATURE_FLUSH_L1D)) {
+		test_wrmsr(MSR_IA32_FLUSH_CMD, "FLUSH_CMD", 0);
+		test_wrmsr(MSR_IA32_FLUSH_CMD, "FLUSH_CMD", L1D_FLUSH);
+	} else {
+		test_wrmsr_fault(MSR_IA32_FLUSH_CMD, "FLUSH_CMD", 0);
+		test_wrmsr_fault(MSR_IA32_FLUSH_CMD, "FLUSH_CMD", L1D_FLUSH);
+	}
+	for (i = 1; i < 64; i++)
+		test_wrmsr_fault(MSR_IA32_FLUSH_CMD, "FLUSH_CMD", BIT_ULL(i));
 }
 
 int main(int ac, char **av)
-- 
2.40.0.348.gf938b09366-goog

Re: [kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Xiaoyao Li]

On 3/28/2023 1:02 PM, Sean Christopherson wrote:
> Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
> the nVMX library.

What does nVMX mean here?

> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   lib/x86/msr.h | 1 +
>   x86/vmexit.c  | 2 +-
>   2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/x86/msr.h b/lib/x86/msr.h
> index c9869be5..29fff553 100644
> --- a/lib/x86/msr.h
> +++ b/lib/x86/msr.h
> @@ -34,6 +34,7 @@
>   /* Intel MSRs. Some also available on other CPUs */
>   #define MSR_IA32_SPEC_CTRL              0x00000048
>   #define MSR_IA32_PRED_CMD               0x00000049
> +#define PRED_CMD_IBPB			BIT(0)
>   
>   #define MSR_IA32_PMC0                  0x000004c1
>   #define MSR_IA32_PERFCTR0		0x000000c1
> diff --git a/x86/vmexit.c b/x86/vmexit.c
> index b1eed8d1..2e8866e1 100644
> --- a/x86/vmexit.c
> +++ b/x86/vmexit.c
> @@ -463,7 +463,7 @@ static int has_spec_ctrl(void)
>   
>   static void wr_ibpb_msr(void)
>   {
> -	wrmsr(MSR_IA32_PRED_CMD, 1);
> +	wrmsr(MSR_IA32_PRED_CMD, PRED_CMD_IBPB);
>   }
>   
>   static void toggle_cr0_wp(void)

Re: [kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Sean Christopherson]

On Tue, Mar 28, 2023, Xiaoyao Li wrote:
> On 3/28/2023 1:02 PM, Sean Christopherson wrote:
> > Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
> > the nVMX library.
> 
> What does nVMX mean here?

Nested VMX.  From KUT's perspective, the testing exists to validate KVM's nested
VMX implementation.  If it's at all confusing, I'll drop the 'n'  And we've already
established that KUT can be used on bare metal, even if that's not the primary use
case.

Re: [kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Xiaoyao Li]

On 3/28/2023 11:07 PM, Sean Christopherson wrote:
> On Tue, Mar 28, 2023, Xiaoyao Li wrote:
>> On 3/28/2023 1:02 PM, Sean Christopherson wrote:
>>> Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
>>> the nVMX library.
>>
>> What does nVMX mean here?
> 
> Nested VMX.  From KUT's perspective, the testing exists to validate KVM's nested
> VMX implementation.  If it's at all confusing, I'll drop the 'n'  And we've already
> established that KUT can be used on bare metal, even if that's not the primary use
> case.

So vmexit.flat is supposed to be ran in L1 VM?

I'm confused and interested in how KUT is used on bare metal.

Re: [kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Sean Christopherson]

On Wed, Mar 29, 2023, Xiaoyao Li wrote:
> On 3/28/2023 11:07 PM, Sean Christopherson wrote:
> > On Tue, Mar 28, 2023, Xiaoyao Li wrote:
> > > On 3/28/2023 1:02 PM, Sean Christopherson wrote:
> > > > Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
> > > > the nVMX library.
> > > 
> > > What does nVMX mean here?
> > 
> > Nested VMX.  From KUT's perspective, the testing exists to validate KVM's nested
> > VMX implementation.  If it's at all confusing, I'll drop the 'n'  And we've already
> > established that KUT can be used on bare metal, even if that's not the primary use
> > case.
> 
> So vmexit.flat is supposed to be ran in L1 VM?

Not all of the tests can be run on bare metal, e.g. I can't imagine the VMware
backdoor test works either.

> I'm confused and interested in how KUT is used on bare metal.

I haven't used KUT on bare metal myself, but the idea is pretty much the same as
running under QEMU/KVM: boot into the KUT "kernel" after getting through firmwrae
instead of transferring control to an actual OS.  I assume the biggest challenges
are getting the image loaded, and getting info out of KUT, e.g. having a serial
port with something on the backend to capture/display output.

Re: [kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Xiaoyao Li]

On 3/30/2023 12:36 AM, Sean Christopherson wrote:
> On Wed, Mar 29, 2023, Xiaoyao Li wrote:
>> On 3/28/2023 11:07 PM, Sean Christopherson wrote:
>>> On Tue, Mar 28, 2023, Xiaoyao Li wrote:
>>>> On 3/28/2023 1:02 PM, Sean Christopherson wrote:
>>>>> Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
>>>>> the nVMX library.
>>>> What does nVMX mean here?
>>> Nested VMX.  From KUT's perspective, the testing exists to validate KVM's nested
>>> VMX implementation.  If it's at all confusing, I'll drop the 'n'  And we've already
>>> established that KUT can be used on bare metal, even if that's not the primary use
>>> case.
>> So vmexit.flat is supposed to be ran in L1 VM?
> Not all of the tests can be run on bare metal, e.g. I can't imagine the VMware
> backdoor test works either.
> 

Sorry, I think neither I ask clearly nor you got my point.

You said "the testing exists to validate KVM's nested VMX 
implementation". So I want to know what's the expected usage to run 
vmexit.flat.

If for nested, we need to first boot a VM and then inside the VM we run 
the vmexit.flat with QEMU, right?

That's what confuses me. Isn't vmexit.flat supposed to be directly used 
on the host with QEMU? In this case, nothing to do with nested.

Re: [kvm-unit-tests PATCH 1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)

[Sean Christopherson]

On Thu, Mar 30, 2023, Xiaoyao Li wrote:
> On 3/30/2023 12:36 AM, Sean Christopherson wrote:
> > On Wed, Mar 29, 2023, Xiaoyao Li wrote:
> > > On 3/28/2023 11:07 PM, Sean Christopherson wrote:
> > > > On Tue, Mar 28, 2023, Xiaoyao Li wrote:
> > > > > On 3/28/2023 1:02 PM, Sean Christopherson wrote:
> > > > > > Add a define for PRED_CMD_IBPB and use it to replace the open coded '1' in
> > > > > > the nVMX library.
> > > > > What does nVMX mean here?
> > > > Nested VMX.  From KUT's perspective, the testing exists to validate KVM's nested
> > > > VMX implementation.  If it's at all confusing, I'll drop the 'n'  And we've already
> > > > established that KUT can be used on bare metal, even if that's not the primary use
> > > > case.
> > > So vmexit.flat is supposed to be ran in L1 VM?
> > Not all of the tests can be run on bare metal, e.g. I can't imagine the VMware
> > backdoor test works either.
> > 
> 
> Sorry, I think neither I ask clearly nor you got my point.
> 
> You said "the testing exists to validate KVM's nested VMX implementation".
> So I want to know what's the expected usage to run vmexit.flat.
> 
> If for nested, we need to first boot a VM and then inside the VM we run the
> vmexit.flat with QEMU, right?
> 
> That's what confuses me. Isn't vmexit.flat supposed to be directly used on
> the host with QEMU? In this case, nothing to do with nested.

Oof, my bad, I had a literacy problem.  I somehow read "vmx.c" instead of "vmexit.c",
and never picked up on what you were saying.  I'll fix the changelog.

Re: [kvm-unit-tests PATCH 0/3] x86/msr: Add tests for command MSRs

[Sean Christopherson]

On Mon, 27 Mar 2023 22:02:28 -0700, Sean Christopherson wrote:
> Add testcases for the write-only MSRs IA32_PRED_CMD and IA32_FLUSH_CMD.
> 
> Note, this depends on the x2APIC MSR series[*].  Unless someone yells,
> I'll include both in a pull request later this week.
> 
> [*] https://lkml.kernel.org/r/20230107011737.577244-1-seanjc%40google.com
> 
> [...]

Applied to kvm-x86 next, thanks!

[1/3] x86: Add define for MSR_IA32_PRED_CMD's PRED_CMD_IBPB (bit 0)
      https://github.com/kvm-x86/kvm-unit-tests/commit/056a56f6e8d0
[2/3] x86/msr: Add testcases for MSR_IA32_PRED_CMD and its IBPB command
      https://github.com/kvm-x86/kvm-unit-tests/commit/835268e75d12
[3/3] x86/msr: Add testcases for MSR_IA32_FLUSH_CMD and its L1D_FLUSH command
      https://github.com/kvm-x86/kvm-unit-tests/commit/408e9eaae1c6

--
https://github.com/kvm-x86/kvm-unit-tests/tree/next

Re: [kvm-unit-tests PATCH 2/3] x86/msr: Add testcases for MSR_IA32_PRED_CMD and its IBPB command

[Paolo Bonzini]

On Tue, Mar 28, 2023 at 7:02 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Add test coverage to verify MSR_IA32_PRED_CMD is write-only, that it can
> be written with '0' (nop command) and '1' (IBPB command) when IBPB is
> supported by the CPU (SPEC_CTRL on Intel, IBPB on AMD), and that writing
> any other bit (1-63) triggers a #GP due to the bits/commands being
> reserved.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

I have a machine here (run-of-the-mill Skylake Xeon Gold) where
MSR_IA32_PRED_CMD does not fail for bits 1-63, so I am dropping that
bit.

Paolo

> ---
>  x86/msr.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/x86/msr.c b/x86/msr.c
> index 97cf5987..13cb6391 100644
> --- a/x86/msr.c
> +++ b/x86/msr.c
> @@ -85,6 +85,15 @@ static void test_msr_rw(u32 msr, const char *name, unsigned long long val)
>         __test_msr_rw(msr, name, val, 0);
>  }
>
> +static void test_wrmsr(u32 msr, const char *name, unsigned long long val)
> +{
> +       unsigned char vector = wrmsr_safe(msr, val);
> +
> +       report(!vector,
> +              "Expected success on WRSMR(%s, 0x%llx), got vector %d",
> +              name, val, vector);
> +}
> +
>  static void test_wrmsr_fault(u32 msr, const char *name, unsigned long long val)
>  {
>         unsigned char vector = wrmsr_safe(msr, val);
> @@ -271,6 +280,23 @@ static void test_x2apic_msrs(void)
>         __test_x2apic_msrs(true);
>  }
>
> +static void test_cmd_msrs(void)
> +{
> +       int i;
> +
> +       test_rdmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD");
> +       if (this_cpu_has(X86_FEATURE_SPEC_CTRL) ||
> +           this_cpu_has(X86_FEATURE_AMD_IBPB)) {
> +               test_wrmsr(MSR_IA32_PRED_CMD, "PRED_CMD", 0);
> +               test_wrmsr(MSR_IA32_PRED_CMD, "PRED_CMD", PRED_CMD_IBPB);
> +       } else {
> +               test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", 0);
> +               test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", PRED_CMD_IBPB);
> +       }
> +       for (i = 1; i < 64; i++)
> +               test_wrmsr_fault(MSR_IA32_PRED_CMD, "PRED_CMD", BIT_ULL(i));
> +}
> +
>  int main(int ac, char **av)
>  {
>         /*
> @@ -283,6 +309,7 @@ int main(int ac, char **av)
>                 test_misc_msrs();
>                 test_mce_msrs();
>                 test_x2apic_msrs();
> +               test_cmd_msrs();
>         }
>
>         return report_summary();
> --
> 2.40.0.348.gf938b09366-goog
>