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

From	"Liang, Kan" <kan.liang@intel.com>
Newsgroups	linux.kernel
Subject	RE: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI
Date	2017-05-10 17:50 +0200
Message-ID	<tFC2d-2vc-1@gated-at.bofh.it> (permalink)
References	<tqmLM-87i-17@gated-at.bofh.it> <txCiK-7dQ-19@gated-at.bofh.it> <tER3k-4uz-7@gated-at.bofh.it> <tFAjM-1pF-13@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw


> 
> > > > +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

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

RE: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI "Liang, Kan" <kan.liang@intel.com> - 2017-05-08 15:40 +0200
  RE: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI Thomas Gleixner <tglx@linutronix.de> - 2017-05-10 16:00 +0200
    RE: [PATCH V5] perf/x86: add sysfs entry to freeze counter on SMI "Liang, Kan" <kan.liang@intel.com> - 2017-05-10 17:50 +0200

csiph-web