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

["Jürgen Groß" <jgross@suse.com>]

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

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@suse.com>
>> ---
>> 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@intel.com/
> 
>>   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 #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 --]