linux-kernel - Re: [PATCH 1/2] x86, Clean up smp_num

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Sun, 01 Jun 2014 19:19:45 -0400
From:	Prarit Bhargava <prarit@...hat.com>
To:	Oren Twaig <oren@...lemp.com>
CC:	linux-kernel@...r.kernel.org, pbonzini@...hat.com,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
	Borislav Petkov <bp@...e.de>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <ak@...ux.intel.com>, Dave Jones <davej@...hat.com>,
	Torsten Kaiser <just.for.lkml@...glemail.com>,
	Jan Beulich <JBeulich@...e.com>,
	Jan Kiszka <jan.kiszka@...mens.com>,
	Toshi Kani <toshi.kani@...com>,
	Andrew Jones <drjones@...hat.com>,
	"Shai (Shai@...leMP.com)" <Shai@...lemp.com>
Subject: Re: [PATCH 1/2] x86, Clean up smp_num_siblings calculation



On 06/01/2014 05:23 AM, Oren Twaig wrote:
> Hi Prarit,
> 
> See below,
> 
> On 05/30/2014 02:43 PM, Prarit Bhargava wrote:
>> I have a system on which I have disabled threading in the BIOS, and I am booting
>> the kernel with the option "idle=poll".
>>
>> The kernel displays
>>
>> process: WARNING: polling idle and HT enabled, performance may degrade
>>
>> which is incorrect -- I've already disabled HT.
>>
>> This warning is issued here:
>>
>> void select_idle_routine(const struct cpuinfo_x86 *c)
>> {
>>         if (boot_option_idle_override == IDLE_POLL && smp_num_siblings > 1)
>>                 pr_warn_once("WARNING: polling idle and HT enabled, performance may degrade\n");
>>
>> >From my understanding of the other ares of kernel that use
>> smp_num_siblings, the value is supposed to be the the number of threads
>> per core.
>>
>> The value of smp_num_siblings is incorrect.  In theory, it should be 1 but it
>> is reported as 2.  When I looked into how smp_num_siblings is calculated I
>> found the following call sequence in the kernel:
>>
>> start_kernel ->
>>         check_bugs ->
>>                 identify_boot_cpu ->
>>                                 identify_cpu ->
>>                                         c_init = init_intel
>>                                                 init_intel ->
>>                                                         detect_extended_topology
>>                                                         (sets value)
>>
>>                                         OR
>>
>>                                         c_init = init_amd
>>                                                 init_amd -> amd_detect_cmp
>>                                                              -> amd_get_topology
>>                                                                 (sets value)
>>                                                          -> detect_ht()
>>                                         ...            (sets value)
>>                                         detect_ht()
>>                                         (also sets value)
>>
>> ie) it is set three times in some cases and is overwritten by the call
>> to detect_ht() from identify_cpu() in all cases.
>>
>> It should be noted that nothing in the identify_cpu() path or the cpu_up()
>> path requires smp_num_siblings to be set, prior to the final call to
>> detect_ht().
>>
>> For x86 boxes, smp_num_siblings is set to a value read in a CPUID call in
>> detect_ht(). This value is the *factory defined* value in all cases; even
>> if HT is disabled in BIOS the value still returns 2 if the CPU supports
>> HT.  AMD also reports the factory defined value in all cases.
> 
> The above is incorrect in case of X-TOPOLOGY mode. I such a case the information
> about number of siblings comes from the LEVEL_MAX_SIBLINGS() macro and the
> X86_FEATURE_XTOPOLOGY flag is set to skip detect_ht() work :
> void detect_ht(struct cpuinfo_x86 *c)
> ...
>     if (cpu_has(c, X86_FEATURE_XTOPOLOGY))
>             return;
> 
> In addition, the information about the number of sibling no longer comes from
> CPUID(0x1)->ebx but rather from the 0xb leaf of CPUID.
> 
> I've marked below the problematic code change.

I will do a [v2] of the patchset that omits this change

>> -    core_level_siblings = smp_num_siblings = LEVEL_MAX_SIBLINGS(ebx);
>> +    core_level_siblings = LEVEL_MAX_SIBLINGS(ebx);

and then removes the setting of smp_num_siblings in 2/2.

Thanks,

P.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/