Re: [PATCH v11 10/23] x86/resctrl: Remove MSR reading of event configuration value

From: Xin Li
Date: Fri Feb 07 2025 - 05:08:47 EST


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!
Xin