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

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2326 bytes --]

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.

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.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]