linux-kernel - Re: [PATCH V2] KVM: x86/pmu: Disable vPMU if EVENTSEL

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZRIYbu4wSVW9a+8i@google.com>
Date:   Mon, 25 Sep 2023 16:31:58 -0700
From:   Sean Christopherson <seanjc@...gle.com>
To:     Like Xu <like.xu.linux@...il.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Ravi Bangoria <ravi.bangoria@....com>,
        Manali Shukla <manali.shukla@....com>
Subject: Re: [PATCH V2] KVM: x86/pmu: Disable vPMU if EVENTSEL_GUESTONLY bit
 doesn't exist

On Thu, Sep 14, 2023, Like Xu wrote:
> On 7/4/2023 11:37 pm, Sean Christopherson wrote:
> > On Fri, Apr 07, 2023, Like Xu wrote:
> /*
>  * The guest vPMU counter emulation depends on the EVENTSEL_GUESTONLY bit.
>  * If this bit is present on the host, the host needs to support at least
> the PERFCTR_CORE.
>  */

...

> > 	/*
> > 	 * KVM requires guest-only event support in order to isolate guest PMCs
> > 	 * from host PMCs.  SVM doesn't provide a way to atomically load MSRs
> > 	 * on VMRUN, and manually adjusting counts before/after VMRUN is not
> > 	 * accurate enough to properly virtualize a PMU.
> > 	 */
> > 
> > But now I'm really confused, because if I'm reading the code correctly, perf
> > invokes amd_core_hw_config() for legacy PMUs, i.e. even if PERFCTR_CORE isn't
> > supported.  And the APM documents the host/guest bits only for "Core Performance
> > Event-Select Registers".
> > 
> > So either (a) GUESTONLY isn't supported on legacy CPUs and perf is relying on AMD
> > CPUs ignoring reserved bits or (b) GUESTONLY _is_ supported on legacy PMUs and
> > pmu_has_guestonly_mode() is checking the wrong MSR when running on older CPUs.
> > 
> > And if (a) is true, then how on earth does KVM support vPMU when running on a
> > legacy PMU?  Is vPMU on AMD just wildly broken?  Am I missing something?
> > 
> 
> (a) It's true and AMD guest vPMU have only been implemented accurately with
> the help of this GUESTONLY bit.
> 
> There are two other scenarios worth discussing here: one is support L2 vPMU
> on the PERFCTR_CORE+ host and this proposal is disabling it; and the other
> case is to support AMD legacy vPMU on the PERFCTR_CORE+ host.

Oooh, so the really problematic case is when PERFCTR_CORE+ is supported but
GUESTONLY is not, in which case KVM+perf *think* they can use GUESTONLY (and
HOSTONLY).

That's a straight up KVM (as L0) bug, no?  I don't see anything in the APM that
suggests those bits are optional, i.e. KVM is blatantly violating AMD's architecture
by ignoring those bits.

I would rather fix KVM (as L0).  It doesn't seem _that_ hard to support, e.g.
modify reprogram_counter() to disable the counter if it's supposed to be silent
for the current mode, and reprogram all counters if EFER.SVME is toggled, and on
all nested transitions.