Re: vmx_pmu_caps_test fails on Skylake based CPUS due to read only LBRs

[Sean Christopherson <seanjc@google.com>]

On Sun, Nov 03, 2024, Maxim Levitsky wrote:
> On Mon, 2024-10-28 at 08:55 -0700, Sean Christopherson wrote:
> > On Fri, Oct 18, 2024, Maxim Levitsky wrote:
> > > Our CI found another issue, this time with vmx_pmu_caps_test.
> > > 
> > > On 'Intel(R) Xeon(R) Gold 6328HL CPU' I see that all LBR msrs (from/to and
> > > TOS), are always read only - even when LBR is disabled - once I disable the
> > > feature in DEBUG_CTL, all LBR msrs reset to 0, and you can't change their
> > > value manually.  Freeze LBRS on PMI seems not to affect this behavior.

...

> When DEBUG_CTL.LBR=1, the LBRs do work, I see all the registers update,
> although TOS does seem to be stuck at one value, but it does change
> sometimes, and it's non zero.
> 
> The FROM/TO do show healthy amount of updates 
> 
> Note that I read all msrs using 'rdmsr' userspace tool.

I'm pretty sure debugging via 'rdmsr', i.e. /dev/msr, isn't going to work.  I
assume perf is clobbering LBR MSRs on context switch, but I haven't tracked that
down to confirm (the code I see on inspecition is gated on at least one perf
event using LBRs).  My guess is that there's a software bug somewhere in the
perf/KVM exchange.

I confirmed that using 'rdmsr' and 'wrmsr' "loses" values, but that hacking KVM
to read/write all LBRs during initialization works with LBRs disabled.

---
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index f72835e85b6d..c68a5a79c668 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7907,6 +7907,8 @@ static __init u64 vmx_get_perf_capabilities(void)
 {
        u64 perf_cap = PMU_CAP_FW_WRITES;
        u64 host_perf_cap = 0;
+       u64 debugctl, val;
+       int i;
 
        if (!enable_pmu)
                return 0;
@@ -7954,6 +7956,39 @@ static __init u64 vmx_get_perf_capabilities(void)
                perf_cap &= ~PERF_CAP_PEBS_BASELINE;
        }
 
+       if (!vmx_lbr_caps.nr) {
+               pr_warn("Uh, what?  No LBRs...\n");
+               goto out;
+       }
+
+       rdmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
+       if (debugctl & DEBUGCTLMSR_LBR) {
+               pr_warn("Huh, LBRs enabled at KVM load?  debugctl = %llx\n", debugctl);
+               wrmsrl(MSR_IA32_DEBUGCTLMSR, debugctl & ~DEBUGCTLMSR_LBR);
+       }
+
+       for (i = 0; i < vmx_lbr_caps.nr; i++) {
+               wrmsrl(vmx_lbr_caps.from + i, 0xbeef0000 + i);
+               wrmsrl(vmx_lbr_caps.to + i, 0xcafe0000 + i);
+       }
+
+       for (i = 0; i < vmx_lbr_caps.nr; i++) {
+               rdmsrl(vmx_lbr_caps.from + i, val);
+               if (val != 0xbeef0000 + i)
+                       pr_warn("MSR 0x%x Expected %x, got %llx\n",
+                               vmx_lbr_caps.from + i, 0xbeef0000 + i, val);
+               rdmsrl(vmx_lbr_caps.to + i, val);
+               if (val != 0xcafe0000 + i)
+                       pr_warn("MSR 0x%x Expected %x, got %llx\n",
+                               vmx_lbr_caps.from + i, 0xcafe0000 + i, val);
+       }
+
+       pr_warn("Done validating %u from/to LBRs\n", vmx_lbr_caps.nr);
+
+       if (debugctl & DEBUGCTLMSR_LBR)
+               wrmsrl(MSR_IA32_DEBUGCTLMSR, debugctl);
+
+out:
        return perf_cap;
 }
--

And given that perf explicitly disables LBRs (see __intel_pmu_lbr_disable())
before reading LBR MSRs (see intel_pmu_lbr_read()) when taking a snaphot, and
AFAIK no one has complained, I would be very surprised if this is hardware doing
something odd.

---
static noinline int
__intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries,
				  unsigned int cnt, unsigned long flags)
{
	struct cpu_hw_events *cpuc = this_cpu_ptr(&cpu_hw_events);

	intel_pmu_lbr_read();
	cnt = min_t(unsigned int, cnt, x86_pmu.lbr_nr);

	memcpy(entries, cpuc->lbr_entries, sizeof(struct perf_branch_entry) * cnt);
	intel_pmu_enable_all(0);
	local_irq_restore(flags);
	return cnt;
}

static int
intel_pmu_snapshot_branch_stack(struct perf_branch_entry *entries, unsigned int cnt)
{
	unsigned long flags;

	/* must not have branches... */
	local_irq_save(flags);
	__intel_pmu_disable_all(false); /* we don't care about BTS */
	__intel_pmu_lbr_disable();
	/*            ... until here */
	return __intel_pmu_snapshot_branch_stack(entries, cnt, flags);
}
---