Re: [PATCH RFC 0/6] x86/msr: Rename MSR access functions

[Peter Zijlstra <peterz@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 2447 bytes --]

On Mon, Apr 20, 2026 at 01:49:02PM +0200, Jürgen Groß wrote:
> On 20.04.26 13:35, 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].
> 
> Naming is hard. :-)
> 
> What about s/ser/sync/ then?
> 
> > 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.
> 
> I'm pretty sure most of the wrmsr*() use cases could switch to the non
> serializing variants. The problem not making the serializing aspect visible
> in the function name will probably result in most new instances still using
> the serializing variant instead of the probably possible non serializing one.
> 
> Many of those use cases will even suffer more, as they won't use the
> immediate form of WRMSRNS then, which would waste the additional benefits of
> that instruction.

I'm confused, if we have a wrmsrns() function, that could see if the msr
argument was a constant and use the immediate form, no?

That is, we have the following instructions: RDMSR, WRMSR, WRMSRNS
And we should have the exact same functions:

	val = rdmsr(msr);
	wrmsr(msr, val);
	wrmsrns(msr, val);

The only interesting question is what to do with the 'safe' aspect. The
instruction takes a fault, we do the extable, but rdmsr() above already
has a return value, so that can't be used.

One option is to, like uaccess and the proposed overflow, is to use
labels like:

	val = rdmsr(msr, label);

And then, even though the wrmsr*() functions have the return available,
do we want to be consistent and do:

	wrmsr(msr, val, label);
	wrmsrns(msr, val, label);

rather than be inconsistent and have them have a boolean return for
success.

What am I missing?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]