On 9/7/19 2:18 PM, Andy Shevchenko wrote: > On Fri, Sep 6, 2019 at 10:47 PM Srinivas Pandruvada > <srinivas.pandruvada@xxxxxxxxxxxxxxx> wrote: >> >> On Fri, 2019-09-06 at 07:50 -0700, Srinivas Pandruvada wrote: >>> On Fri, 2019-09-06 at 16:46 +0300, Andy Shevchenko wrote: >>>> On Fri, Sep 06, 2019 at 05:39:54AM -0400, Prarit Bhargava wrote: >>>>> On 9/5/19 7:37 PM, Srinivas Pandruvada wrote: >>>>>> Read the bucket and core count relationship via MSR and display >>>>>> when displaying turbo ratio limits. >>>>>> + ret = isst_send_msr_command(cpu, 0x1ae, 0, >>>>>> buckets_info); >>>>> >>>>> ^^^ you can get rid of the magic number 0x1ae by doing (sorry for >>>>> the cut-and-paste) >>>>> >>>>> diff --git a/tools/power/x86/intel-speed-select/Makefile >>>>> b/tools/power/x86/intel >>>>> index 12c6939dca2a..087d802ad844 100644 >>>>> --- a/tools/power/x86/intel-speed-select/Makefile >>>>> +++ b/tools/power/x86/intel-speed-select/Makefile >>>>> @@ -15,6 +15,8 @@ endif >>>>> MAKEFLAGS += -r >>>>> >>>>> override CFLAGS += -O2 -Wall -g -D_GNU_SOURCE -I$(OUTPUT)include >>>>> +override CFLAGS += -I../../../include >>>>> +override CFLAGS += >>>>> -DMSRHEADER='"../../../../arch/x86/include/asm/msr-index.h"' >>> >>> No, we can't use msr_index. >> This comment was meant for use of /dev/cpu/X/msr not msr_index. >> I didn't want to bring in dependency on msr-index.h for couple of 2 >> MSRs and the names in msr-index.h doesn't really reflect the actual >> processing, they are doing. For example MSR_TURBO_RATIO_LIMIT1 for >> 0x1ae. The definition of 0x1AE is different on cpu model 0x55 and >> beyond. >> >>> > > It seems not applicable on top of tools patch series I had applied before. > >>>> >>>> I guess it can be done in more neat way. >>>> >>>>> As I've been looking at this code I have been wondering why >>>>> didn't >>>>> you just use >>>>> the standard /dev/cpu/X/msr interface that other x86 power >>>>> utilities (turbostat, >>>>> x86_energy_perf_policy) use? Implementing msr_read() is trivial >>>>> (warning >>>>> untested and uncompiled code) >>> >>> No. We can't. The MSR interface is disabled on several distribution >>> and >>> platforms with secured boot. So some special MSRs are only allowed >>> via >>> this IOCTL interface. >>> Which distros don't have /dev/cpu/X/msr ? None of other Intel tools have this restriction (or requirement depending on your point of view). Why is intel-speed-select special that we have to jump through hoops? P. >>> Thanks, >>> Srinivas >>> >>> >>>> >>>> Actually good point! >>>> >> > >
