Re: [PATCH v3 1/5] x86/msr: Switch users of native_wrmsr() to native_wrmsrq()

[Jürgen Groß]

On 26.10.26 19:31, Reinette Chatre wrote:
> Hi Juergen,
>
> On 10/26/26 7:11 AM, Juergen Gross wrote:
> > Switch all users of native_wrmsr() to native_wrmsrq() in order to
> > prepare removal of native_wrmsr().
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> > V2:
> > - switch order of hi and lo parts when oring them (Reinette Chatre)
> > V3:
> > - fix another lo | hi instance (Reinette Chatre)
> > ---
>
> No new comments since v1
> 	https://lore.kernel.org/lkml/d6239cec-02dd-4b4e-bdd6-2bd6f88d8753@xxxxxxxxx/
>
> >   enum es_result __vc_handle_msr(struct ghcb *ghcb, struct es_em_ctxt *ctxt, bool write);
> > diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> > index 0408ac7f66fd..24ded41652e3 100644
> > --- a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> > +++ b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> > @@ -175,7 +175,7 @@ int resctrl_arch_pseudo_lock_fn(void *_plr)
> >   	 * pseudo-locked followed by reading of kernel memory to load it
> >   	 * into the cache.
> >   	 */
> > -	native_wrmsr(MSR_IA32_PQR_ASSOC, rmid_p, plr->closid);
> > +	native_wrmsrq(MSR_IA32_PQR_ASSOC, ((u64)plr->closid << 32) | rmid_p);
>
> nit: The parentheses are unnecessary

Indeed they are.

> >   
> >   	/*
> >   	 * Cache was flushed earlier. Now access kernel memory to read it
> > @@ -212,7 +212,7 @@ int resctrl_arch_pseudo_lock_fn(void *_plr)
> >   	 * Critical section end: restore closid with capacity bitmask that
> >   	 * does not overlap with pseudo-locked region.
> >   	 */
> > -	native_wrmsr(MSR_IA32_PQR_ASSOC, rmid_p, closid_p);
> > +	native_wrmsrq(MSR_IA32_PQR_ASSOC, (u64)closid_p << 32 | rmid_p);
>
> This one looks good.
>
> Thank you.
>
> >   
> >   	/* Re-enable the hardware prefetcher(s) */
> >   	wrmsrq(MSR_MISC_FEATURE_CONTROL, saved_msr);
>
> While this change is fine I am curious why it has a different shape from
> the other msr conversions you did that use the struct msr to compose the value?
>
> Consider, for example:
> 	
> 	commit 4967c466ca38 ("x86/resctrl: Stop using 32-bit MSR interfaces")

In most cases using struct msr was the more efficient way to do it. I agree
that I could have avoided using it here and there.

> I am curious especially since this cover letter note that this patch is based on the
> series that the above merged commit is from.

The base is mentioned in order to avoid context conflicts when applying the two
series.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature