Message ID | 20230724155329.474037902@linutronix.de |
---|---|
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:9010:0:b0:3e4:2afc:c1 with SMTP id l16csp1989499vqg; Mon, 24 Jul 2023 11:54:46 -0700 (PDT) X-Google-Smtp-Source: APBJJlFBxIdtWJD6DPCCTsncmqgFpocrl+8fRPZqQqzc0LNwMVwabaEMIZZf2XVjN4h6EJrShsCr X-Received: by 2002:a17:902:e88d:b0:1b3:ea47:796c with SMTP id w13-20020a170902e88d00b001b3ea47796cmr10293168plg.29.1690224886204; Mon, 24 Jul 2023 11:54:46 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1690224886; cv=none; d=google.com; s=arc-20160816; b=Z67Z9n/4cR4x0O0IYVS6KCERCawhx1ILbMswyjjygqHFKPaIbOPSCAGys3GVNuX/3y lFKlJ1JU+1Hzp2a2VrUi++X9hZXM7KYkcJeqdlrWRXlp8qBgySgL4u5OOqYSne//HTbW kzGFzPeYdp0s0Vdnp2kQpezUk01Y9is/A1WxY9h50QaGuy3eMCHzy10Fa1zJDyBXgm/G MYnrjrcmT2MBEHmpxr6+z7MLEI87sTifKU+3/LFPBT/BKtOp+Oc71eoeryNgLIJiCtrn TPmSpfbCtWUM3tpzufh/OdiWo+IaJtbe8fe73kuCpnd+vsmb1dwUs/Gkrewm3yY+cjmh Jtow== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:date:subject:cc:to:from:dkim-signature :dkim-signature:message-id; bh=x0/YxfQ7KycD3cE3OrPXPIszM9wkw+ZpilEoSpWAuFg=; fh=cDlgSYzM/IiVZ/EC06yL1+GD3+pSK8bXSWGje/Xzf2k=; b=bQZd+cr3SnbEaZBtSC0FPuxTWebVjceTDkMiotDXiuiqAYbP31CmiEuX3UyBXm5ald K9tJ5k7MFvnRaPBpJPLhjmfz+lxfKWmJvleZvv77UFjp4v3hy+Jv5Hm/fqCvUgPhMvM0 uYCF1FsMoKLRYqAeTdJ74mHSA+TIWQGCV2ZkyKUlCO3z5feF5DVBt0izU3rERgocXOOc Q83RevU2eEK9uQkNCafBlP0Bd4b5l5vACUQK6alLiwQIVNDZ4bPKbw3kcXeUAPuavfR/ hm0dHr6eitloPUC6JMDNnOzvK/d6+PEszfZAB9S8NXJ28XpIiJiI3eazO/WSFqX1pXyh 25Aw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=Jx8F+1qI; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id j8-20020a170903024800b001b9ea0f0e8esi10222568plh.650.2023.07.24.11.54.33; Mon, 24 Jul 2023 11:54:46 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=Jx8F+1qI; dkim=neutral (no key) header.i=@linutronix.de; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231614AbjGXRoD (ORCPT <rfc822;kautuk.consul.80@gmail.com> + 99 others); Mon, 24 Jul 2023 13:44:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45074 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231157AbjGXRny (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 24 Jul 2023 13:43:54 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [IPv6:2a0a:51c0:0:12e:550::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 88ABC1702; Mon, 24 Jul 2023 10:43:52 -0700 (PDT) Message-ID: <20230724155329.474037902@linutronix.de> DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1690220630; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc; bh=x0/YxfQ7KycD3cE3OrPXPIszM9wkw+ZpilEoSpWAuFg=; b=Jx8F+1qIV/RGTyVkOzfnDcLC62XmPX7bXKtGtISTgk1sG3J2QfAYkjFkCmx3xtdwVQHUM1 V4RL1ttMTCHXD1oVAPt1xA7mQbIJIx1EZxvQHkTsqY+Pb4Hlv38vbMfxJhiSHN+sqM2wED VlyS8y2t7xB4rsifZc7d5WBs0kBzzMk/wbYGEbkwISIIRy1b8S/u4TFervAtBds/1VMtxk 9N43EGGKJgJP8t2T5entdUybG/5rjk+l2K/uRbm07/vo2paaoFyV2KH2IYJ2i23htM6tN+ afTyRHRvTaAEh/dxOCVIDauANzXZ+Q9dJSn8T3Ik2wT8PQov/wDCCjjLEywrcw== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1690220630; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc; bh=x0/YxfQ7KycD3cE3OrPXPIszM9wkw+ZpilEoSpWAuFg=; b=NY6Sj68bJiuzUoSEOZuvhu53pQhB1UEyUse7mVx0GXYydmSIElcW0UW5H9lGjY3V0jXUCU MZjKNPMnf7+8SUAg== From: Thomas Gleixner <tglx@linutronix.de> To: LKML <linux-kernel@vger.kernel.org> Cc: x86@kernel.org, Tom Lendacky <thomas.lendacky@amd.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Arjan van de Ven <arjan@linux.intel.com>, "James E.J. Bottomley" <jejb@linux.ibm.com>, Dick Kennedy <dick.kennedy@broadcom.com>, James Smart <james.smart@broadcom.com>, "Martin K. Petersen" <martin.petersen@oracle.com>, linux-scsi@vger.kernel.org, linux-hwmon@vger.kernel.org, Jean Delvare <jdelvare@suse.com>, Huang Rui <ray.huang@amd.com>, Guenter Roeck <linux@roeck-us.net>, Steve Wahl <steve.wahl@hpe.com>, Mike Travis <mike.travis@hpe.com>, Dimitri Sivanich <dimitri.sivanich@hpe.com>, Russ Anderson <russ.anderson@hpe.com> Subject: [patch 00/29] x86/cpu: Rework the topology evaluation Date: Mon, 24 Jul 2023 19:43:50 +0200 (CEST) X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1772329249944529746 X-GMAIL-MSGID: 1772329249944529746 |
Series |
x86/cpu: Rework the topology evaluation
|
|
Message
Thomas Gleixner
July 24, 2023, 5:43 p.m. UTC
Hi! A recent commit to the CPUID leaf 0xb/0x1f parser made me look deeper at the way how topology is evaluated. That "fix" is just yet another cure the sypmtom hack which completely ignores the underlying disaster. The way how topology evaluation works is to overwrite the relevant variables as often as possible. E.g. smp_num_siblings gets overwritten a gazillion times, which is wrong to begin with. The boot CPU writes it 3 times, each AP two times. What's worse is that this just works by chance on hybrid systems due to the fact that the existing ones all seem to boot on a P-Core which has SMT. Would it boot on a E-Core which has no SMT, then parts of the early topology evaluation including the primary thread mask which is required for parallel CPU bringup would be completely wrong. Overwriting it later on with the correct value does not help at all. What's wrong today with hybrid already is the number of cores per package. On an ADL with 8 P-Cores and 8 E-cores the resulting number of cores per package is evaluated to be 12. Which is not further surprising because the CPUID 0xb/0x1f parser looks at the number of logical processors at core level and divides them by the number of SMP siblings. 24 / 2 = 12 Just that this CPU has obviously 16 cores not 12. It's is even clearly documented in the SDM that this is wrong. "Bits 15-00: The number of logical processors across all instances of this domain within the next higher- scoped domain relative to this current logical processor. (For example, in a processor socket/package comprising "M" dies of "N" cores each, where each core has "L" processors, the "die" domain subleaf value of this field would be M*N*L. In an asymmetric topology this would be the summation of the value across the lower domain level instances to create each upper domain level instance.) This number reflects configuration as shipped by Intel. Note, software must not use this field to enumerate processor topology. Software must not use the value of EBX[15:0] to enumerate processor topology of the system. The value is only intended for display and diagnostic purposes. The actual number of logical processors available to BIOS/OS/Applications may be different from the value of EBX[15:0], depending on software and platform hardware configurations." This "_NOT_ to use for topology evaluation" sentence existed even before hybrid came along and got ignored. The code worked by chance, but with hybrid all bets are off. The code completely falls apart once CPUID leaf 0x1f enumerates any topology level between CORE and DIE, but that's not a suprise. The proper thing to do is to actually evaluate the full topology including the non-present (hotpluggable) CPUs based on the APICIDs which are provided by the firmware and a proper topology domain level parser. This can exactly tell the number of physical packages, logical packages etc. _before_ even booting a single AP. All of that can be evaluated upfront. Aside of that there are too many places which do their own topology evaluation, but there is absolutely no central point which can actually provide all of that information in a consistent way. This needs to change. This series implements another piece towards this: sane CPUID evaluation, which is done at _one_ place in a proper well defined order instead of having it sprinkled all over the CPUID evaluation code. At the end of this series this is pretty much bug compatible with the current CPUID evaluation code in respect to the cores per package evaluation, but it gets rid of overwriting things like smp_num_siblings, which is now written once, but is still not capable to work correctly on a hybrid machine which boots from a non SMT core. These things can only be fixed up in the next step(s). When I tried to go further with this I ran into yet another pile of historical layers of duct tape and haywire with a gazillion of random variables sprinkled all over the place. That's still work in progress. It actually works, but the last step which switches over is not yet in a shape that can be easily reviewed. Stay tuned. The series is based on the APIC cleanup series: https://lore.kernel.org/lkml/20230724131206.500814398@linutronix.de and also available on top of that from git: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git topo-cpuid-v1 Thanks, tglx --- arch/x86/kernel/cpu/topology.c | 168 -------------------- b/Documentation/arch/x86/topology.rst | 12 - b/arch/x86/events/amd/core.c | 2 b/arch/x86/events/amd/uncore.c | 2 b/arch/x86/events/intel/uncore.c | 2 b/arch/x86/include/asm/apic.h | 1 b/arch/x86/include/asm/cacheinfo.h | 3 b/arch/x86/include/asm/cpuid.h | 32 +++ b/arch/x86/include/asm/processor.h | 58 ++++-- b/arch/x86/include/asm/smp.h | 2 b/arch/x86/include/asm/topology.h | 51 +++++- b/arch/x86/include/asm/x86_init.h | 2 b/arch/x86/kernel/amd_nb.c | 8 b/arch/x86/kernel/apic/apic_flat_64.c | 7 b/arch/x86/kernel/apic/apic_noop.c | 3 b/arch/x86/kernel/apic/apic_numachip.c | 11 - b/arch/x86/kernel/apic/bigsmp_32.c | 6 b/arch/x86/kernel/apic/local.h | 1 b/arch/x86/kernel/apic/probe_32.c | 6 b/arch/x86/kernel/apic/x2apic_cluster.c | 1 b/arch/x86/kernel/apic/x2apic_phys.c | 6 b/arch/x86/kernel/apic/x2apic_uv_x.c | 63 +------ b/arch/x86/kernel/cpu/Makefile | 5 b/arch/x86/kernel/cpu/amd.c | 156 ------------------ b/arch/x86/kernel/cpu/cacheinfo.c | 51 ++---- b/arch/x86/kernel/cpu/centaur.c | 4 b/arch/x86/kernel/cpu/common.c | 108 +----------- b/arch/x86/kernel/cpu/cpu.h | 14 + b/arch/x86/kernel/cpu/debugfs.c | 98 +++++++++++ b/arch/x86/kernel/cpu/hygon.c | 133 --------------- b/arch/x86/kernel/cpu/intel.c | 38 ---- b/arch/x86/kernel/cpu/mce/amd.c | 4 b/arch/x86/kernel/cpu/mce/apei.c | 4 b/arch/x86/kernel/cpu/mce/core.c | 4 b/arch/x86/kernel/cpu/mce/inject.c | 7 b/arch/x86/kernel/cpu/proc.c | 8 b/arch/x86/kernel/cpu/topology.h | 51 ++++++ b/arch/x86/kernel/cpu/topology_amd.c | 179 +++++++++++++++++++++ b/arch/x86/kernel/cpu/topology_common.c | 233 ++++++++++++++++++++++++++++ b/arch/x86/kernel/cpu/topology_ext.c | 136 ++++++++++++++++ b/arch/x86/kernel/cpu/zhaoxin.c | 18 -- b/arch/x86/kernel/smpboot.c | 64 ++++--- b/arch/x86/kernel/vsmp_64.c | 13 - b/arch/x86/mm/amdtopology.c | 35 +--- b/arch/x86/xen/apic.c | 8 b/drivers/edac/amd64_edac.c | 4 b/drivers/edac/mce_amd.c | 4 b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 b/drivers/hwmon/fam15h_power.c | 7 b/drivers/scsi/lpfc/lpfc_init.c | 8 b/drivers/virt/acrn/hsm.c | 2 51 files changed, 964 insertions(+), 881 deletions(-)
Comments
On Mon, Jul 24, 2023 at 07:43:50PM +0200, Thomas Gleixner wrote:
+- a few small nits
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
We have successfully booted a 32-socket, 3840 cpu HPE Sapphire Rapids system with your patchset applied, as found in: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git topo-cpuid-v1 # cat /sys/kernel/debug/x86/topo/cpus/3839 initial_apicid: ff7 apicid: ff7 pkg_id: 31 die_id: 31 cu_id: 255 core_id: 59 logical_pkg_id: 31 logical_die_id: 31 llc_id: 3968 l2c_id: 4086 amd_node_id: 0 amd_nodes_per_pkg: 0 max_cores: 60 max_die_per_pkg: 1 smp_num_siblings: 2 We were also able to boot a 60-socket, 2160 cpu HPE SuperDome Flex (SkyLake) with the patchset. # cat /sys/kernel/debug/x86/topo/cpus/2159 initial_apicid: ef7 apicid: ef7 pkg_id: 59 die_id: 59 cu_id: 255 core_id: 27 logical_pkg_id: 59 logical_die_id: 59 llc_id: 3776 l2c_id: 3830 amd_node_id: 0 amd_nodes_per_pkg: 0 max_cores: 18 max_die_per_pkg: 1 smp_num_siblings: 2 On Mon, Jul 24, 2023 at 07:43:50PM +0200, Thomas Gleixner wrote: > Hi! > > A recent commit to the CPUID leaf 0xb/0x1f parser made me look deeper at > the way how topology is evaluated. That "fix" is just yet another cure the > sypmtom hack which completely ignores the underlying disaster. > > The way how topology evaluation works is to overwrite the relevant > variables as often as possible. E.g. smp_num_siblings gets overwritten a > gazillion times, which is wrong to begin with. The boot CPU writes it 3 > times, each AP two times. > > What's worse is that this just works by chance on hybrid systems due to the > fact that the existing ones all seem to boot on a P-Core which has > SMT. Would it boot on a E-Core which has no SMT, then parts of the early > topology evaluation including the primary thread mask which is required for > parallel CPU bringup would be completely wrong. Overwriting it later on > with the correct value does not help at all. > > What's wrong today with hybrid already is the number of cores per package. > On an ADL with 8 P-Cores and 8 E-cores the resulting number of cores per > package is evaluated to be 12. Which is not further surprising because the > CPUID 0xb/0x1f parser looks at the number of logical processors at core > level and divides them by the number of SMP siblings. > > 24 / 2 = 12 > > Just that this CPU has obviously 16 cores not 12. > > It's is even clearly documented in the SDM that this is wrong. > > "Bits 15-00: The number of logical processors across all instances of this > domain within the next higher- scoped domain relative to this current > logical processor. (For example, in a processor socket/package comprising > "M" dies of "N" cores each, where each core has "L" processors, the > "die" domain subleaf value of this field would be M*N*L. In an asymmetric > topology this would be the summation of the value across the lower domain > level instances to create each upper domain level instance.) This number > reflects configuration as shipped by Intel. Note, software must not use > this field to enumerate processor topology. > > Software must not use the value of EBX[15:0] to enumerate processor topology > of the system. The value is only intended for display and diagnostic purposes. > The actual number of logical processors available to BIOS/OS/Applications > may be different from the value of EBX[15:0], depending on software and > platform hardware configurations." > > This "_NOT_ to use for topology evaluation" sentence existed even before > hybrid came along and got ignored. The code worked by chance, but with > hybrid all bets are off. The code completely falls apart once CPUID leaf > 0x1f enumerates any topology level between CORE and DIE, but that's not a > suprise. > > The proper thing to do is to actually evaluate the full topology including > the non-present (hotpluggable) CPUs based on the APICIDs which are provided > by the firmware and a proper topology domain level parser. This can exactly > tell the number of physical packages, logical packages etc. _before_ even > booting a single AP. All of that can be evaluated upfront. > > Aside of that there are too many places which do their own topology > evaluation, but there is absolutely no central point which can actually > provide all of that information in a consistent way. This needs to change. > > This series implements another piece towards this: sane CPUID evaluation, > which is done at _one_ place in a proper well defined order instead of > having it sprinkled all over the CPUID evaluation code. > > At the end of this series this is pretty much bug compatible with the > current CPUID evaluation code in respect to the cores per package > evaluation, but it gets rid of overwriting things like smp_num_siblings, > which is now written once, but is still not capable to work correctly on a > hybrid machine which boots from a non SMT core. These things can only be > fixed up in the next step(s). > > When I tried to go further with this I ran into yet another pile of > historical layers of duct tape and haywire with a gazillion of random > variables sprinkled all over the place. That's still work in progress. It > actually works, but the last step which switches over is not yet in a shape > that can be easily reviewed. Stay tuned. > > The series is based on the APIC cleanup series: > > https://lore.kernel.org/lkml/20230724131206.500814398@linutronix.de > > and also available on top of that from git: > > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git topo-cpuid-v1 > > Thanks, > > tglx > --- > arch/x86/kernel/cpu/topology.c | 168 -------------------- > b/Documentation/arch/x86/topology.rst | 12 - > b/arch/x86/events/amd/core.c | 2 > b/arch/x86/events/amd/uncore.c | 2 > b/arch/x86/events/intel/uncore.c | 2 > b/arch/x86/include/asm/apic.h | 1 > b/arch/x86/include/asm/cacheinfo.h | 3 > b/arch/x86/include/asm/cpuid.h | 32 +++ > b/arch/x86/include/asm/processor.h | 58 ++++-- > b/arch/x86/include/asm/smp.h | 2 > b/arch/x86/include/asm/topology.h | 51 +++++- > b/arch/x86/include/asm/x86_init.h | 2 > b/arch/x86/kernel/amd_nb.c | 8 > b/arch/x86/kernel/apic/apic_flat_64.c | 7 > b/arch/x86/kernel/apic/apic_noop.c | 3 > b/arch/x86/kernel/apic/apic_numachip.c | 11 - > b/arch/x86/kernel/apic/bigsmp_32.c | 6 > b/arch/x86/kernel/apic/local.h | 1 > b/arch/x86/kernel/apic/probe_32.c | 6 > b/arch/x86/kernel/apic/x2apic_cluster.c | 1 > b/arch/x86/kernel/apic/x2apic_phys.c | 6 > b/arch/x86/kernel/apic/x2apic_uv_x.c | 63 +------ > b/arch/x86/kernel/cpu/Makefile | 5 > b/arch/x86/kernel/cpu/amd.c | 156 ------------------ > b/arch/x86/kernel/cpu/cacheinfo.c | 51 ++---- > b/arch/x86/kernel/cpu/centaur.c | 4 > b/arch/x86/kernel/cpu/common.c | 108 +----------- > b/arch/x86/kernel/cpu/cpu.h | 14 + > b/arch/x86/kernel/cpu/debugfs.c | 98 +++++++++++ > b/arch/x86/kernel/cpu/hygon.c | 133 --------------- > b/arch/x86/kernel/cpu/intel.c | 38 ---- > b/arch/x86/kernel/cpu/mce/amd.c | 4 > b/arch/x86/kernel/cpu/mce/apei.c | 4 > b/arch/x86/kernel/cpu/mce/core.c | 4 > b/arch/x86/kernel/cpu/mce/inject.c | 7 > b/arch/x86/kernel/cpu/proc.c | 8 > b/arch/x86/kernel/cpu/topology.h | 51 ++++++ > b/arch/x86/kernel/cpu/topology_amd.c | 179 +++++++++++++++++++++ > b/arch/x86/kernel/cpu/topology_common.c | 233 ++++++++++++++++++++++++++++ > b/arch/x86/kernel/cpu/topology_ext.c | 136 ++++++++++++++++ > b/arch/x86/kernel/cpu/zhaoxin.c | 18 -- > b/arch/x86/kernel/smpboot.c | 64 ++++--- > b/arch/x86/kernel/vsmp_64.c | 13 - > b/arch/x86/mm/amdtopology.c | 35 +--- > b/arch/x86/xen/apic.c | 8 > b/drivers/edac/amd64_edac.c | 4 > b/drivers/edac/mce_amd.c | 4 > b/drivers/gpu/drm/amd/amdkfd/kfd_topology.c | 2 > b/drivers/hwmon/fam15h_power.c | 7 > b/drivers/scsi/lpfc/lpfc_init.c | 8 > b/drivers/virt/acrn/hsm.c | 2 > 51 files changed, 964 insertions(+), 881 deletions(-) > > >
On 7/24/2023 10:43 AM, Thomas Gleixner wrote: > The series is based on the APIC cleanup series: > > https://lore.kernel.org/lkml/20230724131206.500814398@linutronix.de > > and also available on top of that from git: > > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git topo-cpuid-v1 > The series boots fine on an old Sandy Bridge 2S system. There is a print from topology_update_die_map() which is missing from dmesg but it seems mostly harmless. > [ 0.085686] smpboot: x86: Booting SMP configuration:> [ 0.085690] .... node #0, CPUs: #1 #2 #3 #4 #5 #6 #7 #8 #9 > [ 0.089253] .... node #1, CPUs: #10 #11 #12 #13 #14 #15 #16 #17 #18 #19 > [ 0.000000] smpboot: CPU 10 Converting physical 0 to logical die 1 ^^ The "Converting physical..." line doesn't show up with the patches applied. > [ 0.134035] .... node #0, CPUs: #20 #21 #22 #23 #24 #25 #26 #27 #28 #29 > [ 0.140239] .... node #1, CPUs: #30 #31 #32 #33 #34 #35 #36 #37 #38 #39 Please let me know if you need more information. Tested-by: Sohil Mehta <sohil.mehta@intel.com>
On Wed, Jul 26 2023 at 15:15, Sohil Mehta wrote: > On 7/24/2023 10:43 AM, Thomas Gleixner wrote: >> The series is based on the APIC cleanup series: >> >> https://lore.kernel.org/lkml/20230724131206.500814398@linutronix.de >> >> and also available on top of that from git: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git topo-cpuid-v1 >> > > The series boots fine on an old Sandy Bridge 2S system. There is a print > from topology_update_die_map() which is missing from dmesg but it seems > mostly harmless. > >> [ 0.085686] smpboot: x86: Booting SMP configuration:> [ 0.085690] .... node #0, CPUs: #1 #2 #3 #4 #5 > #6 #7 #8 #9 >> [ 0.089253] .... node #1, CPUs: #10 #11 #12 #13 #14 #15 #16 #17 #18 #19 >> [ 0.000000] smpboot: CPU 10 Converting physical 0 to logical die 1 > > ^^ The "Converting physical..." line doesn't show up with the patches > applied. That message comes from the complete nonsense in the current upstream code that cpuinfo::die_id is made relative to the package. That's just wrong. My rework uses the physical die id which is unique by definition and therefore does not need this conversion. The logical ID is the same as the physical id in that case. >> [ 0.134035] .... node #0, CPUs: #20 #21 #22 #23 #24 #25 #26 #27 #28 #29 >> [ 0.140239] .... node #1, CPUs: #30 #31 #32 #33 #34 #35 #36 #37 #38 #39 > > Please let me know if you need more information. > > Tested-by: Sohil Mehta <sohil.mehta@intel.com> There is a real and unrelated snafu vs. that logical package and logical die management which I discovered today. I missed the fact that this cruft abuses cpuinfo as permanent storage, which breaks CPU offline/online as the online operation reinitializes the topology information. I pushed out a fixed version to git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/topology earlier. It doesn't have a tag yet. Thanks, tglx
On 7/26/2023 3:38 PM, Thomas Gleixner wrote: > There is a real and unrelated snafu vs. that logical package and logical > die management which I discovered today. I missed the fact that this > cruft abuses cpuinfo as permanent storage, which breaks CPU > offline/online as the online operation reinitializes the topology > information. > > I pushed out a fixed version to > > git://git.kernel.org/pub/scm/linux/kernel/git/tglx/devel.git x86/topology > > earlier. It doesn't have a tag yet. > Thanks, I'll try this out and experiment with CPU hotplug as well. -Sohil