Re: [PATCH 03/22] KVM: x86: Support IBPB_BRTYPE and SBPB

From: Sean Christopherson <seanjc@google.com>
To: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Borislav Petkov <bp@alien8.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Babu Moger <babu.moger@amd.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	David.Kaplan@amd.com, Nikolay Borisov <nik.borisov@suse.com>,
	gregkh@linuxfoundation.org, Thomas Gleixner <tglx@linutronix.de>
Subject: Re: [PATCH 03/22] KVM: x86: Support IBPB_BRTYPE and SBPB
Date: Thu, 24 Aug 2023 09:39:03 -0700	[thread overview]
Message-ID: <ZOeHp0skdfrARNFe@google.com> (raw)
In-Reply-To: <20230821170520.dcovzudamnoqp7jc@treble>

On Mon, Aug 21, 2023, Josh Poimboeuf wrote:
> On Mon, Aug 21, 2023 at 04:35:41PM +0000, Sean Christopherson wrote:
> > There are more wrinkles though.  KVM passes through MSR_IA32_PRED_CMD based on
> > IBPB.  If hardware supports both IBPB and SBPB, but userspace does NOT exposes
> > SBPB to the guest, then KVM will create a virtualization hole where the guest can
> > write SBPB against userspace's wishes.  I haven't read up on SBPB enought o know
> > whether or not that's problematic.
> > 
> > And conversely, if userspace expoes SBPB but not IBPB, then KVM will intercept
> > writes to MSR_IA32_PRED_CMD and probably tank guest performance.  Again, I haven't
> > paid attention enough to know if this is a reasonable configuration, i.e. whether
> > or not it's worth caring about in KVM.
> > 
> > If the virtualization holes are deemed safe, then the easiest thing would be to
> > treat MSR_IA32_PRED_CMD as existing if either IBPB or SBPB exists.  E.g.
> 
> I can't think of a reason why the holes wouldn't be safe, i.e. AFAICT
> there's no harm in letting the guest do whatever type of barrier it
> wants even if it's not technically supported by their configuration.
> 
> Question: if we're just always passing PRED_CMD through, what's the
> point of having any PRED_CMD code in kvm_set_msr_common at all?

Emulation :-(  KVM's emulator supports WRMSR, and on Intel without unrestricted
guest, it's unfortunately necessary for KVM to emulate large swaths of guest code.
Emulating WRMSR on other hardware setups is far less likely, but still plausible.

KVM's ABI is also that userspace is allowed to write guest MSRs that KVM says exist,
so KVM needs to at least not reject KVM_SET_MSRS.

Whether or not it makes sense for KVM to forward the WRMSR the hardware is
definitely debatable, especially for writes from host userspace.  But IIUC, at
at worst the WRMSR from KVM could be a noisy neighbor for an SMT sibling, so IMO
it's not worth the brain power needed to determine whether or not KVM can safely
omit the WRMSR.

> Also, since you're clearly more qualified to write this patch than me,
> can I nominate you to do so? :-)

Sorry, didn't mean to ghost you.  I can write the patch, but I won't get to it
before next week some time.