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

[Reinette Chatre]

Hi Juergen,

On 6/28/26 11:39 PM, 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>
> ---

(just looking at resctrl changes ...)

> diff --git a/arch/x86/kernel/cpu/resctrl/pseudo_lock.c b/arch/x86/kernel/cpu/resctrl/pseudo_lock.c
> index 0408ac7f66fd..ad4c0d79ce1d 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, rmid_p | ((u64)plr->closid << 32));
>  

This is ok but I find that it could be easier to read if changed to:
	native_wrmsrq(MSR_IA32_PQR_ASSOC, (u64)plr->closid << 32 | rmid_p)

Above matches existing and familiar pattern of "high << 32 | low" when preparing
register values as found in, for example arch/x86/include/asm/msr.h, that matches
the mental model of ordering registers from MSB to LSB.
The extra parentheses seem unnecessary.

>  	/*
>  	 * 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, rmid_p | ((u64)closid_p << 32));
>  
>  	/* Re-enable the hardware prefetcher(s) */
>  	wrmsrq(MSR_MISC_FEATURE_CONTROL, saved_msr);

Reinette