Re: [PATCH 0/8] x86/msr: Drop 32-bit variants of *_on_cpu() MSR functions

[Jürgen Groß]

On 05.06.26 11:18, Ingo Molnar wrote:
> * Jürgen Groß <jgross@xxxxxxxx> wrote:
>
> > On 05.06.26 11:05, Ingo Molnar wrote:
> > > * Juergen Gross <jgross@xxxxxxxx> wrote:
> > >
> > > > Drop the variants using 2 32-bit values instead of a single 64-bit one
> > > > of the *_on_cpu() MSR access functions.
> > > >
> > > > Juergen Gross (8):
> > > >     x86/msr: Switch rdmsr_on_cpu() to return a 64-bit quantity
> > > >     x86/msr: Switch all callers of rdmsrq_on_cpu() to use rdmsr_on_cpu()
> > > >     x86/msr: Switch wrmsr_on_cpu() to use a 64-bit quantity
> > > >     x86/msr: Switch all callers of wrmsrq_on_cpu() to use wrmsr_on_cpu()
> > > >     x86/msr: Switch rdmsr_safe_on_cpu() to return a 64-bit quantity
> > > >     x86/msr: Switch all callers of rdmsrq_safe_on_cpu() to use rdmsr_safe_on_cpu()
> > > >     x86/msr: Switch wrmsr_safe_on_cpu() to use a 64-bit quantity
> > > >     x86/msr: Switch all callers of wrmsrq_safe_on_cpu() to use  wrmsr_safe_on_cpu()
> > >
> > > To sum up my review feedback for the invididual patches, we want
> > > to do this instead:
> > >
> > >     x86/msr: Convert rdmsrl_on_cpu() users to rdmsrq_on_cpu()
> > >     x86/msr: Drop the rdmsrl_on_cpu() alias to rdmsrq_on_cpu()
> > >
> > >     x86/msr: Switch all callers of rdmsr_on_cpu() to use rdmsrq_on_cpu()
> > >     x86/msr: Remove the unused rdmsr_on_cpu() API
> > >
> > >     x86/msr: Switch all callers of wrmsr_on_cpu() to use wrmsrq_on_cpu()
> > >     x86/msr: Remove unused wrmsr_on_cpu() API
> > >
> > >     x86/msr: Switch all callers of rdmsr_safe_on_cpu() to use rdmsrq_safe_on_cpu()
> > >     x86/msr: Remove unused rdmsr_safe_on_cpu() API
> > >
> > >     x86/msr: Switch all callers of wrmsr_safe_on_cpu() to use wrmsrq_safe_on_cpu()
> > >     x86/mrs: Remove unused wrmsrq_safe_on_cpu() API
> > >
> > > Note how there's no "conversion" of the 32-bit API itself in this
> > > approach, we just do a straightforward migration of the users to
> > > the already existing 64-bit APIs, then remove any unused APIs.
> >
> > Fine with me, but I just wanted to get rid of the "q" and "l" suffices
> > completely, as they serve no special purpose after dropping all other
> > variants.
> >
> > OTOH if wanted such a switch could be done later easily.
>
> Well, we had a similar discussion back when we standardized on
> rdmsrq() and wrmsrq(), and we use them as our primary 64-bit
> MSR handling APIs. Why have a different pattern in any of the
> derived APIs? It should really use the same conceptual namespace,
> not some confusing mixture of two naming schemes.

In the long run I'd like to do the same conversion for the rdmsr*() and
wrmsr*() interfaces, too (so only offering and using the 64-bit variants).

I understand that this is not guaranteed to be accepted immediately after
this series, so I agree that it is better to keep the "q" suffix for now
in order to avoid confusion.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature