KVM: x86: Don't treat ENTER and LEAVE as branches, because they aren't

[PATCH] KVM: x86: Don't treat ENTER and LEAVE as branches, because they aren't

Posted by Sean Christopherson 1 week, 6 days ago

Remove the IsBranch flag from ENTER and LEAVE in KVM's emulator, as ENTER
and LEAVE are stack operations, not branches.  Add forced emulation of
said instructions to the PMU counters test to prove that KVM diverges from
hardware, and to guard against regressions.

Fixes: 018d70ffcfec ("KVM: x86: Update vPMCs when retiring branch instructions")
Cc: Jim Mattson <jmattson@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/emulate.c                              | 4 ++--
 tools/testing/selftests/kvm/x86/pmu_counters_test.c | 8 +++++---
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 542d3664afa3..23929151a5b8 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -4330,8 +4330,8 @@ static const struct opcode opcode_table[256] = {
 	I(DstReg | SrcMemFAddr | ModRM | No64 | Src2DS, em_lseg),
 	G(ByteOp, group11), G(0, group11),
 	/* 0xC8 - 0xCF */
-	I(Stack | SrcImmU16 | Src2ImmByte | IsBranch, em_enter),
-	I(Stack | IsBranch, em_leave),
+	I(Stack | SrcImmU16 | Src2ImmByte, em_enter),
+	I(Stack, em_leave),
 	I(ImplicitOps | SrcImmU16 | IsBranch, em_ret_far_imm),
 	I(ImplicitOps | IsBranch, em_ret_far),
 	D(ImplicitOps | IsBranch), DI(SrcImmByte | IsBranch, intn),
diff --git a/tools/testing/selftests/kvm/x86/pmu_counters_test.c b/tools/testing/selftests/kvm/x86/pmu_counters_test.c
index 8aaaf25b6111..89c1e462cd1c 100644
--- a/tools/testing/selftests/kvm/x86/pmu_counters_test.c
+++ b/tools/testing/selftests/kvm/x86/pmu_counters_test.c
@@ -14,10 +14,10 @@
 #define NUM_BRANCH_INSNS_RETIRED	(NUM_LOOPS)
 
 /*
- * Number of instructions in each loop. 1 CLFLUSH/CLFLUSHOPT/NOP, 1 MFENCE,
- * 1 LOOP.
+ * Number of instructions in each loop. 1 ENTER, 1 CLFLUSH/CLFLUSHOPT/NOP,
+ * 1 MFENCE, 1 LEAVE, 1 LOOP.
  */
-#define NUM_INSNS_PER_LOOP		4
+#define NUM_INSNS_PER_LOOP		6
 
 /*
  * Number of "extra" instructions that will be counted, i.e. the number of
@@ -210,9 +210,11 @@ do {										\
 	__asm__ __volatile__("wrmsr\n\t"					\
 			     " mov $" __stringify(NUM_LOOPS) ", %%ecx\n\t"	\
 			     "1:\n\t"						\
+			     FEP "enter $0, $0\n\t"				\
 			     clflush "\n\t"					\
 			     "mfence\n\t"					\
 			     "mov %[m], %%eax\n\t"				\
+			     FEP "leave\n\t"					\
 			     FEP "loop 1b\n\t"					\
 			     FEP "mov %%edi, %%ecx\n\t"				\
 			     FEP "xor %%eax, %%eax\n\t"				\

base-commit: c8fbf7ceb2ae3f64b0c377c8c21f6df577a13eb4
-- 
2.51.0.470.ga7dc726c21-goog

Re: [PATCH] KVM: x86: Don't treat ENTER and LEAVE as branches, because they aren't

Posted by Chao Gao 1 week, 2 days ago

On Thu, Sep 18, 2025 at 05:46:39PM -0700, Sean Christopherson wrote:
>Remove the IsBranch flag from ENTER and LEAVE in KVM's emulator, as ENTER
>and LEAVE are stack operations, not branches.  Add forced emulation of
>said instructions to the PMU counters test to prove that KVM diverges from
>hardware, and to guard against regressions.
>
>Fixes: 018d70ffcfec ("KVM: x86: Update vPMCs when retiring branch instructions")
>Cc: Jim Mattson <jmattson@google.com>
>Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Chao Gao <chao.gao@intel.com>

one nit below:

>---
> arch/x86/kvm/emulate.c                              | 4 ++--
> tools/testing/selftests/kvm/x86/pmu_counters_test.c | 8 +++++---
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
>diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
>index 542d3664afa3..23929151a5b8 100644
>--- a/arch/x86/kvm/emulate.c
>+++ b/arch/x86/kvm/emulate.c
>@@ -4330,8 +4330,8 @@ static const struct opcode opcode_table[256] = {
> 	I(DstReg | SrcMemFAddr | ModRM | No64 | Src2DS, em_lseg),
> 	G(ByteOp, group11), G(0, group11),
> 	/* 0xC8 - 0xCF */
>-	I(Stack | SrcImmU16 | Src2ImmByte | IsBranch, em_enter),
>-	I(Stack | IsBranch, em_leave),
>+	I(Stack | SrcImmU16 | Src2ImmByte, em_enter),
>+	I(Stack, em_leave),
> 	I(ImplicitOps | SrcImmU16 | IsBranch, em_ret_far_imm),
> 	I(ImplicitOps | IsBranch, em_ret_far),
> 	D(ImplicitOps | IsBranch), DI(SrcImmByte | IsBranch, intn),
>diff --git a/tools/testing/selftests/kvm/x86/pmu_counters_test.c b/tools/testing/selftests/kvm/x86/pmu_counters_test.c
>index 8aaaf25b6111..89c1e462cd1c 100644
>--- a/tools/testing/selftests/kvm/x86/pmu_counters_test.c
>+++ b/tools/testing/selftests/kvm/x86/pmu_counters_test.c
>@@ -14,10 +14,10 @@
> #define NUM_BRANCH_INSNS_RETIRED	(NUM_LOOPS)
> 
> /*
>- * Number of instructions in each loop. 1 CLFLUSH/CLFLUSHOPT/NOP, 1 MFENCE,
>- * 1 LOOP.
>+ * Number of instructions in each loop. 1 ENTER, 1 CLFLUSH/CLFLUSHOPT/NOP,
>+ * 1 MFENCE, 1 LEAVE, 1 LOOP.

	      ^ 1 MOV,

7803339fa929 ("Use data load to trigger LLC references/misses in Intel PMU")
forgot to update this comment. Otherwise it is a bit confusing that the comment
lists only 5 instructions while the macro is 6.


>  */
>-#define NUM_INSNS_PER_LOOP		4
>+#define NUM_INSNS_PER_LOOP		6
> 
> /*
>  * Number of "extra" instructions that will be counted, i.e. the number of
>@@ -210,9 +210,11 @@ do {										\
> 	__asm__ __volatile__("wrmsr\n\t"					\
> 			     " mov $" __stringify(NUM_LOOPS) ", %%ecx\n\t"	\
> 			     "1:\n\t"						\
>+			     FEP "enter $0, $0\n\t"				\
> 			     clflush "\n\t"					\
> 			     "mfence\n\t"					\
> 			     "mov %[m], %%eax\n\t"				\
>+			     FEP "leave\n\t"					\
> 			     FEP "loop 1b\n\t"					\
> 			     FEP "mov %%edi, %%ecx\n\t"				\
> 			     FEP "xor %%eax, %%eax\n\t"				\
>
>base-commit: c8fbf7ceb2ae3f64b0c377c8c21f6df577a13eb4
>-- 
>2.51.0.470.ga7dc726c21-goog
>
>

Re: [PATCH] KVM: x86: Don't treat ENTER and LEAVE as branches, because they aren't

Posted by Jim Mattson 1 week, 5 days ago

On Thu, Sep 18, 2025 at 5:46 PM Sean Christopherson <seanjc@google.com> wrote:
>
> Remove the IsBranch flag from ENTER and LEAVE in KVM's emulator, as ENTER
> and LEAVE are stack operations, not branches.  Add forced emulation of
> said instructions to the PMU counters test to prove that KVM diverges from
> hardware, and to guard against regressions.
>
> Fixes: 018d70ffcfec ("KVM: x86: Update vPMCs when retiring branch instructions")
> Cc: Jim Mattson <jmattson@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Jim Mattson <jmattson@google.com>