On 10/4/19 1:15 PM, Srinivas Pandruvada wrote:
> On Thu, 2019-10-03 at 08:11 -0400, Prarit Bhargava wrote:
>> Three CascadeLake-N models (6252N, 6230N, and 5218N) have SST-PBF
>> support.
>>
>> Return an error if the CascadeLake processor is not one of these
>> specific
>> models.
>>
> This patch sigfaults immediately on CLX.>
>> v2: Add is_clx_n_platform()
>>
>> Signed-off-by: Prarit Bhargava <pra...@redhat.com>
>> ---
>>  .../x86/intel-speed-select/isst-config.c      | 44
>> ++++++++++++++++++-
>>  1 file changed, 42 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/power/x86/intel-speed-select/isst-config.c
>> b/tools/power/x86/intel-speed-select/isst-config.c
>> index f4a23678416e..734a7960458c 100644
>> --- a/tools/power/x86/intel-speed-select/isst-config.c
>> +++ b/tools/power/x86/intel-speed-select/isst-config.c
>> @@ -23,6 +23,7 @@ static int debug_flag;
>>  static FILE *outf;
>>  
>>  static int cpu_model;
>> +static int cpu_stepping;
>>  
>>  #define MAX_CPUS_IN_ONE_REQ 64
>>  static short max_target_cpus;
>> @@ -72,7 +73,16 @@ void debug_printf(const char *format, ...)
>>      va_end(args);
>>  }
>>  
>> -static void update_cpu_model(void)
>> +
>> +int is_clx_n_platform(void)
>> +{
>> +    if (cpu_model == 0x55)
>> +            if (cpu_stepping == 0x6 || cpu_stepping == 0x7)
>> +                    return 1;
>> +    return 0;
>> +}
>> +
>> +static int update_cpu_model(void)
>>  {
>>      unsigned int ebx, ecx, edx;
>>      unsigned int fms, family;
>> @@ -82,6 +92,34 @@ static void update_cpu_model(void)
>>      cpu_model = (fms >> 4) & 0xf;
>>      if (family == 6 || family == 0xf)
>>              cpu_model += ((fms >> 16) & 0xf) << 4;
>> +
>> +    cpu_stepping = fms & 0xf;
>> +
>> +    /* only three CascadeLake-N models are supported */
>> +    if (is_clx_n_platform()) {
>> +            FILE *fp;
>> +            size_t n;
>> +            char *line;
> Need n = 0 and *line = NULL here as getline() will require if it has to
> allocate.

Odd, I ran multiple tests and never hit the segfault :(.  Thanks for fixing.

P.

> 
> Anyway I will update the patchset and post after test.
> 

> Thanks,
> Srinivas
>> +            int ret = 1;
>> +
>> +            fp = fopen("/proc/cpuinfo", "r");
>> +            if (!fp)
>> +                    err(-1, "cannot open /proc/cpuinfo\n");
>> +
>> +            while (getline(&line, &n, fp) > 0) {
>> +                    if (strstr(line, "model name")) {
>> +                            if (strstr(line, "6252N") ||
>> +                                strstr(line, "6230N") ||
>> +                                strstr(line, "5218N"))
>> +                                    ret = 0;
>> +                            break;
>> +                    }
>> +            }
>> +            free(line);
>> +            fclose(fp);
>> +            return ret;
>> +    }
>> +    return 0;
>>  }
>>  
>>  /* Open a file, and exit on failure */
>> @@ -1889,7 +1927,9 @@ static void cmdline(int argc, char **argv)
>>              fprintf(stderr, "Feature name and|or command not
>> specified\n");
>>              exit(0);
>>      }
>> -    update_cpu_model();
>> +    ret = update_cpu_model();
>> +    if (ret)
>> +            err(-1, "Invalid CPU model (%d)\n", cpu_model);
>>      printf("Intel(R) Speed Select Technology\n");
>>      printf("Executing on CPU model:%d[0x%x]\n", cpu_model,
>> cpu_model);
>>      set_max_cpu_num();
> 

Reply via email to