From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933261AbcECOnV (ORCPT ); Tue, 3 May 2016 10:43:21 -0400 Received: from relay3.sgi.com ([192.48.152.1]:57284 "EHLO relay.sgi.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S932269AbcECOnT (ORCPT ); Tue, 3 May 2016 10:43:19 -0400 Subject: Re: [PATCH 07/21] X86_64, UV: Disable Obsolete APIC ID fixup code used only on UV1 To: Ingo Molnar References: <20160429215402.458042580@asylum.americas.sgi.com> <20160429215403.767900970@asylum.americas.sgi.com> <20160503073512.GA13474@gmail.com> Cc: Ingo Molnar , "H. Peter Anvin" , Thomas Gleixner , Andrew Morton , Len Brown , Dimitri Sivanich , Russ Anderson , John Estabrook , Andrew Banman , Nathan Zimmer , x86@kernel.org, linux-kernel@vger.kernel.org From: Mike Travis Message-ID: <5728B900.8050102@sgi.com> Date: Tue, 3 May 2016 07:43:12 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <20160503073512.GA13474@gmail.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/3/2016 12:35 AM, Ingo Molnar wrote: > > * Mike Travis 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