Hi Xin,
On 2/7/25 04:07, Xin Li wrote:
> On 2/6/2025 8:17 AM, Reinette Chatre wrote:
>>>> + wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
>>> This is the existing code, however it would be better to use wrmsrl()
>>> when the higher 32-bit are all 0s:
>>>
>>> wrmsrl(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config);
>>>
>> Could you please elaborate what makes this change better?
>
> In short, it takes one less argument, and doesn't pass an argument of 0.
>
> The longer story is that hpa and I are refactoring the MSR access APIs
> to accommodate the immediate form of MSR access instructions. And we
> are not happy about that there are too many MSR access APIs and their
> uses are *random*. The native wrmsr() and wrmsrl() are essentially the
> same and the only difference is that wrmsr() passes a 64-bit value to be
> written into a MSR in *2* u32 arguments. But we already have struct msr
> defined in asm/shared/msr.h as:
> struct msr {
> union {
> struct {
> u32 l;
> u32 h;
> };
> u64 q;
> };
> };
>
> it's more natural to do the same job with this data structure in most
> cases. And we want to remove wrmsr() and only keep wrmsrl(), thus a
> developer won't have to figure out which one is better to use :-P.
>
> For that to happen, one cleanup is to replace wrmsr(msr, low, 0) with
> wrmsrl(msr, low) (low is automatically converted to u64 from u32).
>
> However, I'm fine if Babu wants to keep it as-is.
Thanks for the explanation. Changed it to use wrmsrl().
--
Thanks
Babu Moger
