RE: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI

From: Liang, Kan
Date: Wed May 10 2017 - 11:48:41 EST




>
> > > > +static void flip_smm_bit(void *data) {
> > > > + bool set = *(int *)data;
>
> data points to an unsigned long. So while this works on LE machines this is
> still crap.

I will change the code as below.
+static void flip_smm_bit(void *data)
+{
+ unsigned long set = *(unsigned long *)data;
+
+ if (set > 0) {

>
> > > > + if (set) {
> > > > + msr_set_bit(MSR_IA32_DEBUGCTLMSR,
> > > > + DEBUGCTLMSR_FREEZE_IN_SMM_BIT);
> > > > + } else {
> > > > + msr_clear_bit(MSR_IA32_DEBUGCTLMSR,
> > > > + DEBUGCTLMSR_FREEZE_IN_SMM_BIT);
> > > > + }
>
> I probably forgot, but why do we need an open coded version of __flip_bit()
> instead of reusing that?
>
msr_set_bit/msr_clear_bit is enough for our requirement. There is no extra
work needed.
__flip_bit is static inline. If we want to directly use it, we have to expose
it by an additional patch. If so, we will have two sets of interfaces to flip MSR
bit. I think it's too much.
What do you think?

Thanks,
Kan