linux-kernel - Re: [PATCH 07/21] X86_64, UV: Disable Obsolete APIC ID fixup code used only on UV1

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] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 3 May 2016 07:43:12 -0700
From:	Mike Travis <travis@....com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	Ingo Molnar <mingo@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Len Brown <len.brown@...el.com>,
	Dimitri Sivanich <sivanich@....com>,
	Russ Anderson <rja@....com>,
	John Estabrook <estabrook@....com>,
	Andrew Banman <abanman@....com>,
	Nathan Zimmer <nzimmer@....com>, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 07/21] X86_64, UV: Disable Obsolete APIC ID fixup code
 used only on UV1



On 5/3/2016 12:35 AM, Ingo Molnar wrote:
> 
> * Mike Travis <travis@....com> wrote:
> 
>> +config X86_UV1_SUPPORTED
>> +	bool "SGI Ultraviolet Series 1 Supported"
>> +	depends on X86_UV
> 
> So I still think it's much simpler if we simply eliminate this Kconfig 
> complication and have it all compatible. AFAICS the runtime impact on newer 
> systems comes down mostly to a single unlikely branch:
> 
>>  static unsigned int x2apic_get_apic_id(unsigned long x)
>>  {
>> -	unsigned int id;
>> +	if (likely(!uv1_apic_driver))
>> +		return x;
> 
> Thanks,
> 
> 	Ingo
> 


The total removal of the Kconfig option affects approximately 84 total
references not counting further references to definitions that include
the is_uv1_hub() function call.  Here are some of the ones not used
within MMR address or field selection defines:

31> hunt is_uv1_hub | grep -v arch/x86/include/asm/uv/uv_mmrs.h
arch/x86/include/asm/uv/uv_bau.h:#define UV_NET_ENDPOINT_INTD           (is_uv1_hub() ?   \
arch/x86/include/asm/uv/uv_bau.h:#define UV_INTD_SOFT_ACK_TIMEOUT_PERIOD (is_uv1_hub() ?  \

arch/x86/kernel/apic/x2apic_uv_x.c:     if (is_uv1_hub()) {
arch/x86/kernel/apic/x2apic_uv_x.c:     } else if (is_uv1_hub()) {
arch/x86/kernel/apic/x2apic_uv_x.c:             if (is_uv1_hub() || is_uv2_hub() || is_uv3_hub()) {
arch/x86/kernel/apic/x2apic_uv_x.c:                 is_uv1_hub() ? "UV100/1000" : NULL;
arch/x86/platform/uv/tlb_uv.c:  if (is_uv1_hub())
arch/x86/platform/uv/tlb_uv.c:  if (is_uv1_hub()) {
arch/x86/platform/uv/tlb_uv.c:          if (is_uv1_hub())
arch/x86/platform/uv/tlb_uv.c:                  if (!is_uv1_hub())
arch/x86/platform/uv/uv_time.c: if (is_uv1_hub())
arch/x86/platform/uv/uv_time.c: if (is_uv1_hub())
sgikmods/gru/kernel/gru_rev_2_wars.h:#define UV_GR0_HRB_MMR_CONFIG     (is_uv1_hub() ? 0x400128 : 0xc00128)
sgikmods/gru/kernel/gru_rev_2_wars.h:#define UV_GR1_HRB_MMR_CONFIG     (is_uv1_hub() ? 0x800128 : 0x1000128)
sgikmods/gru/kernel/gru_ugly_kabi_hack.h:#define is_uv1_hub()                           1
sgikmods/gru/kernel/gru_ugly_kabi_hack.h:#define is_uv3_hub() (!(is_uv1_hub() || is_uv2_hub()))
sgikmods/gru/kernel/gruhandles.c:               if (is_uv1_hub()) {
sgikmods/gru/kernel/grukservices.c:     if (is_uv1_hub() && head == 0) {
sgikmods/gru/kernel/grukservices.c:             if (is_uv1_hub() && (mq->head.head == 0)) {
sgikmods/gru/kernel/grufile.c:       vdata->vd_tlb_preload_count = !is_uv1_hub() ? req.tlb_preload_count : 0;
sgikmods/gru/kernel/grufile.c:  tlb_preload_count = is_uv1_hub() ? 0 : GRUTLBPRELOADCNT;
sgikmods/gru/kernel/grumain.c:  if (is_uv1_hub() || is_uv2_hub()) {
sgikmods/hwperf/kernel/dashboard_mmrs.h: *      #define UVHxxx  (is_uv1_hub() ? UV1Hxxx :
sgikmods/hwperf/kernel/uv2_mmrs_hwperf.h: *     #define UVH_xxx         (is_uv1_hub() ? UV1H_xxx : UV2H_xxx)
sgikmods/hwperf/kernel/hwperf_main.c:        return !(is_uv1_hub() || is_uv2_hub()); // uv_hub_info->hub_revision >= UV3_HUB_REVISION_BASE;
sgikmods/hwperf/kernel/hwperf_main.c:   if (is_uv1_hub()) {

There are 55 references within uv_mmrs.h that affect any references to
UV1 via MMR address or field selection, such as this one:

#define UV1H_GR0_TLB_MMR_CONTROL 0x401080UL
#define UV2H_GR0_TLB_MMR_CONTROL 0xc01080UL
#define UV3H_GR0_TLB_MMR_CONTROL 0xc01080UL
#define UV4H_GR0_TLB_MMR_CONTROL 0x601080UL
#define UVH_GR0_TLB_MMR_CONTROL (                     \
        is_uv1_hub() ? UV1H_GR0_TLB_MMR_CONTROL :     \
        is_uv2_hub() ? UV2H_GR0_TLB_MMR_CONTROL :     \
        is_uv3_hub() ? UV3H_GR0_TLB_MMR_CONTROL :     \
        /*is_uv4_hub*/ UV4H_GR0_TLB_MMR_CONTROL)


Therefore would it be all right to at least leave in the Kconfig
option and default it to "UV1" enabled?  Or do you feel strongly that
the simplification by removing the Kconfig option is worth whatever
performance hit will be encountered by distros, that will never be
supporting UV1 systems in any future kernel releases?  And to
further that point, it will also impose an artificial performance
hit on SGI UV systems in performance benchmark comparisons?

Please let me know what you decide and I will post the updated
patches immediately.

Thanks!
Mike