linux-kernel - Re: [PATCH v6 19/25] KVM: VMX: Emulate read and write to CET MSRs

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: <ZUqEWySrEeJXCoAO@google.com>
Date:   Tue, 7 Nov 2023 10:39:23 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>
Cc:     Yang Weijiang <weijiang.yang@...el.com>, pbonzini@...hat.com,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
        dave.hansen@...el.com, peterz@...radead.org, chao.gao@...el.com,
        rick.p.edgecombe@...el.com, john.allen@....com
Subject: Re: [PATCH v6 19/25] KVM: VMX: Emulate read and write to CET MSRs

On Tue, Nov 07, 2023, Maxim Levitsky wrote:
> On Thu, 2023-11-02 at 16:58 -0700, Sean Christopherson wrote:
> > Ooh, if we're clever, I bet we can extend KVM_{G,S}ET_ONE_REG to also work with
> > existing MSRs, GPRs, and other stuff,
> 
> Not sure if we want to make it work with MSRs. MSRs are a very well defined thing
> on x86, and we already have an API to read/write them.

Yeah, the API is weird though :-)

> Other registers maybe, don't know.
> 
> >  i.e. not force userspace through the funky
> > KVM_SET_MSRS just to set one reg, and not force a RMW of all GPRs just to set
> > RIP or something.
> Setting one GPR like RIP does sound like a valid use case of KVM_SET_ONE_REG.
> 
> >   E.g. use bits 39:32 of the id to encode the register class,
> > bits 31:0 to hold the index within a class, and reserve bits 63:40 for future
> > usage.
> > 
> > Then for KVM-defined registers, we can route them internally as needed, e.g. we
> > can still define MSR_KVM_SSP so that internal it's treated like an MSR, but its
> > index isn't ABI and so can be changed at will.  And future KVM-defined registers
> > wouldn't _need_ to be treated like MSRs, i.e. we could route registers through
> > the MSR APIs if and only if it makes sense to do so.
> 
> I am not sure that even internally I'll treat MSR_KVM_SSP as MSR. 
> An MSR IMHO is a msr, a register is a register, mixing this up will
> just add to the confusion.

I disagree, things like MSR_{FS,GS}_BASE already set the precedent that MSRs and
registers can be separate viewpoints to the same internal CPU state.  AIUI, these
days, whether a register is exposed via an MSR or dedicated ISA largely comes
down to CPL restrictions and performance.

> Honestly if I were to add support for the SSP register, I'll just add a new
> ioctl/capability and vendor callback. All of this code is just harmless
> boilerplate code.

We've had far too many bugs and confusion over error handling for things like
checking "is this value legal" to be considered harmless boilerplate code.

> Even using KVM_GET_ONE_REG/KVM_SET_ONE_REG is probably overkill, although using
> it for new registers is reasonable.

Maybe, but if we're going to bother adding new ioctls() for x86, I don't see any
benefit to reinventing a wheel that's only good for one thing.