On Mon, Oct 24, 2022, Like Xu wrote:
+static inline u32 gp_counter_base(void)
> +{
> + return pmu.msr_gp_counter_base;
> +}
> +
> +static inline void set_gp_counter_base(u32 new_base)
> +{
> + pmu.msr_gp_counter_base = new_base;
> +}
> +
> +static inline u32 gp_event_select_base(void)
> +{
> + return pmu.msr_gp_event_select_base;
> +}
> +
> +static inline void set_gp_event_select_base(u32 new_base)
> +{
> + pmu.msr_gp_event_select_base = new_base;
> +}
> +
> +static inline u32 gp_counter_msr(unsigned int i)
> +{
> + return gp_counter_base() + i;
> +}
> +
> +static inline u32 gp_event_select_msr(unsigned int i)
As propsed in the previous version, I think it makes sense to make these look
like macros so that it's more obvious that the callers are computing an MSR index
and not getting the MSR, e.g.
MSR_GP_EVENT_SELECTx(i)
> +{
> + return gp_event_select_base() + i;
> +}
> +
> +static inline void write_gp_counter_value(unsigned int i, u64 value)
> +{
> + wrmsr(gp_counter_msr(i), value);
> +}
> +
> +static inline void write_gp_event_select(unsigned int i, u64 value)
> +{
> + wrmsr(gp_event_select_msr(i), value);
> +}
Almost all of these one-line wrappers are unnecessary. "struct pmu_caps pmu" is
already exposed, just reference "pmu" directly. And for the rdmsr/wrmsr wrappers,
the code I wanted to dedup was the calculation of the MSR index, hiding the actual
WRMSR and RDMSR operations are a net-negative IMO.
