Re: [PATCH RFC 0/6] x86/msr: Rename MSR access functions
From: Sean Christopherson
Date: Wed Apr 22 2026 - 15:21:32 EST
On Wed, Apr 22, 2026, Juergen Gross wrote:
> On 20.04.26 15:44, Sean Christopherson wrote:
> > On Mon, Apr 20, 2026, Jürgen Groß wrote:
> > > On 20.04.26 13:41, Peter Zijlstra wrote:
> > > > On Mon, Apr 20, 2026 at 01:35:12PM +0200, Peter Zijlstra wrote:
> > > > > On Mon, Apr 20, 2026 at 11:16:28AM +0200, Juergen Gross wrote:
> > > > >
> > > > > > - Use functions instead of macros for accessing MSRs, which will drop
> > > > > > modifying variables passed as a parameter.
> > > > > >
> > > > > > - Eliminate multiple accessors doing exactly the same thing (e.g.
> > > > > > rdmsrl() and rdmsrq()).
> > > > >
> > > > > So far so sane.
> > > > >
> > > > > > - Instead of having function names based on the underlying instruction
> > > > > > mnemonics, have functions of a common name space (msr_*()).
> > > > >
> > > > > Not sure on this one. The whole msr_{read,write}_{safe,noser}() thing is
> > > > > a royal pain. Also 'noser' reads to me as the noun that goes with 'to
> > > > > nose' [he that noses (around), like baker: he that bakes].
> > > > >
> > > > > I would much rather we just stick to the mnemonics here. All of this
> > > > > really is about wrapping single instructions, no need to make it an
> > > > > unreadable mess.
> > > >
> > > > Also, the _safe suffix should just go away. All MSR accessors should be
> > > > 'safe'.
> > >
> > > That would be fine by me, but I'd like to have some confirmation this is
> > > really the route to go.
> >
> > I don't care what the suffix is called, or if there is even a suffix, but there
> > needs to be a way for the caller to communicate that it wants to handle faults,
> > so that the "caller isn't going to handle a fault" case generates a WARN if the
> > access does #GP.
>
> So this calls for keeping the *_safe() variants (keeping the name or not).
>
> Using the label based error handling for those is a no-go IMHO, as we are
> trying to inline all MSR accesses as much as possible in order to avoid
> calls and the associated possible retpoline stuff (request by Thomas Gleixner
> IIRC). Doing the WARN inline will bloat code more than necessary, especially
> as we already have the central WARN today when fixing up the #GP.
I don't see why those two are mutually exclusive. Keep the WARN in the exception
fixup code, and let the caller use a label (or not). Label-based error handling
should allow for the best of both worlds, as the RDMSR/WRMSR can be inlined, with
the (presumably) unlikely error path being shoved into an out-of-line, cold path.
> In order not having to modify the logic for current use cases it would be
> best to keep the main interface of the *_safe() functions (let them return
> 0/-errno, use a pointer for the value returned by rdmsr_safe()).
>
> We can add label variants on top using macros.
But that's likely going to defeat the value of using labels. Or at least make
it more difficult to generate optimal code.