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;
Date:   Tue, 1 Aug 2023 12:35:58 +0530
From:   "Gautham R. Shenoy" <gautham.shenoy@amd.com>
To:     Thomas Gleixner <tglx@linutronix.de>
Cc:     LKML <linux-kernel@vger.kernel.org>, 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, Guenter Roeck <linux@roeck-us.net>,
        linux-hwmon@vger.kernel.org, Jean Delvare <jdelvare@suse.com>,
        Huang Rui <ray.huang@amd.com>, Juergen Gross <jgross@suse.com>,
        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: Re: [patch v2 21/38] x86/cpu: Provide cpu_init/parse_topology()
Message-ID: <ZMiu1t8blYAdm2Br@BLR-5CG11610CF.amd.com>
References: <20230728105650.565799744@linutronix.de>
 <20230728120930.839913695@linutronix.de>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230728120930.839913695@linutronix.de>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?95In0x8DZI7tu5t0S0WOfxt+vSkkV2ugDf4kTn09dvwz9EaKFD25GUfK9kx6?=
 =?us-ascii?Q?qujQlsaVnYaLiNmaEXZ9Oipl0ijwT3KmbhLaQfwH3pdj6TTUXcSFJ7ataGzd?=
 =?us-ascii?Q?TpfRUR4KmaIqX1stUZnADeApC6M/ZCdgMrLIrnogi+41apwkT39yMtUTuIw2?=
 =?us-ascii?Q?jC0zvboSSGAxaIUbuu6oHZOdGieV+OsxNk3peH3hkImb1s7BAIPumWoNgNxo?=
 =?us-ascii?Q?PY3cNKuAWtWDsnREX653mOBcquTyO3VPE5E5kJmfaR95bvvF/YrO2Mmrlc+G?=
 =?us-ascii?Q?9zXN5J+bO6siBujGkCF1WTJVEGvAwYa42TLYcpPvUbOMpSCbBCk/l+/u+uIw?=
 =?us-ascii?Q?8Zo1/QkB+wC+viB6UrMgF1nZ/r5Wba3MEEuLCup4o8U2OZ4wTzdC2XQ7+NVl?=
 =?us-ascii?Q?nGqkElRghIINdSTYpR2iItEi9FLXr+eS/FfNEtxR3ApVuNzDLFMqFvOXoccw?=
 =?us-ascii?Q?iIqqday51W+x6UGYlWrXgsoFJP/T/ueK6zBET7pvDmUcsPvOylAEl2kBoits?=
 =?us-ascii?Q?grXananNAkFW9+hbYB9hdiQPAf9nUi0uMbyy1pnzpCLydjqelz/5WIX/5dKM?=
 =?us-ascii?Q?KNEbvtzjm4tKXEOn6zqngF3Fb+W83lTNdYAIcJi+JV7pYyYo9dTwh8WiZghZ?=
 =?us-ascii?Q?xT5qH9yA5wC+3hWX+dqKhx5qt+LklEk9alxCDEtddNMNrNG8S90SxmImpum+?=
 =?us-ascii?Q?RLMEqj0f/WrZlYLD7OjIhRnksq3g3iwNydbGl38j/UDQ41JwKX8EEzusKedJ?=
 =?us-ascii?Q?DvN1D8LdjmZo5ocU18+bNu/+h9qShvpruv4d9BYedOzIJUB366oLDQGegzM8?=
 =?us-ascii?Q?bRELfhjNlUziVDMNFSRUVAdUHv7cH6Y7ewY8/QWV/1D5qxjFpU7ouRz39IUy?=
 =?us-ascii?Q?KWtJgQmDDIiZEoJ20E2wrPit19cEA9oNtmNuhOZeTp45RNrcj8XIFcun9a0b?=
 =?us-ascii?Q?wRekLswhEHxfRMSLoAIl0fLmGLH8eW1T71teWgmDWvQp1bVzkhbJ/Ojwui8v?=
 =?us-ascii?Q?/6UarYi1Ke7AtFoIE9IaqPF3kacL+PfM8V4wlzWlOv3gsR0s2tXEg4RVYo0R?=
 =?us-ascii?Q?I5VN7VAqfdP+UmJ3TcSOE4mCh6V/LrKZKArOFV1g15HVH6nwbWavOv4XB6rr?=
 =?us-ascii?Q?+aYqnV+NfuIqdKNbxg7TBHyQdEVmU09czkacZaD8XyUEBGxkc05ag3Jbcmz8?=
 =?us-ascii?Q?cq3UTJZMLsv87gHMmZ/HkmcfuKYwob5IZCQPWbJtVuhN2WWSbCQO3u+7zNBf?=
 =?us-ascii?Q?ZxiV6IYZ/zgdIDOxJwpznhH1gKadrTJs1Q+2jtlhEQGldSshyDChJ8QDkdz4?=
 =?us-ascii?Q?aoFz99yJiVGHZFasoqB1ghuTXFb3gX/sdSu+qvalNQ4qOEbBScBPSq4qDrxh?=
 =?us-ascii?Q?rxQwbXV++l+AzD1RaZAgipdOz6INotzLBgkNzEu6Xr3GlP8Jwwb5zGsNmfUB?=
 =?us-ascii?Q?Wv/iHulAkTQoRyGVMjstuEVZnkTMQOmhs3VXxXAfjJnKXU22xwY3c4YafHf9?=
 =?us-ascii?Q?T4I6gmjHVGv1ikLhC4Xq086PVGovpFBEHk6LccAWmAp7b19fX8G0b90T49gn?=
 =?us-ascii?Q?nCRFnX9/DTdFpAyzhjUSTtIbimxWkeLpifgzCLDf?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 97c0f596-ed16-46ca-14de-08db925dcd13
X-MS-Exchange-CrossTenant-AuthSource: DS7PR12MB8252.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Aug 2023 07:06:17.7462
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: PuPryMXNDnBetBmlfKm8NYSjjsvbzJtD17SsgEKIz3rnfygFaODw+DMGR5Rt5wEcTwLSEjQHCvSdi8ao4FHSiA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR12MB8964
X-Spam-Status: No, score=0.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no 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

Hello Thomas,

On Fri, Jul 28, 2023 at 02:13:08PM +0200, Thomas Gleixner wrote:
> Topology evaluation is a complete disaster and impenetrable mess. It's
> scattered all over the place with some vendor implementatins doing early
> evaluation and some not. The most horrific part is the permanent
> overwriting of smt_max_siblings and __max_die_per_package, instead of
> establishing them once on the boot CPU and validating the result on the
> APs.
> 
> The goals are:
> 
>   - One topology evaluation entry point
> 
>   - Proper sharing of pointlessly duplicated code
> 
>   - Proper structuring of the evaluation logic and preferences.
> 
>   - Evaluating important system wide information only once on the boot CPU
> 
>   - Making the 0xb/0x1f leaf parsing less convoluted and actually fixing
>     the short comings of leaf 0x1f evaluation.
> 
> Start to consolidate the topology evaluation code by providing the entry
> points for the early boot CPU evaluation and for the final parsing on the
> boot CPU and the APs.
> 
> Move the trivial pieces into that new code:
> 
>    - The initialization of cpuinfo_x86::topo
> 
>    - The evaluation of CPUID leaf 1, which presets topo::initial_apicid
> 
>    - topo_apicid is set to topo::initial_apicid when invoked from early
>      boot. When invoked for the final evaluation on the boot CPU it reads
>      the actual APIC ID, which makes apic_get_initial_apicid() obsolete
>      once everything is converted over.
> 
> Provide a temporary helper function topo_converted() which shields off the
> not yet converted CPU vendors from invoking code which would break them.
> This shielding covers all vendor CPUs which support SMP, but not the
> historical pure UP ones as they only need the topology info init and
> eventually the initial APIC initialization.
> 
> Provide two new members in cpuinfo_x86::topo to store the maximum number of
> SMT siblings and the number of dies per package and add them to the debugfs
> readout. These two members will be used to populate this information on the
> boot CPU and to validate the APs against it.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  arch/x86/include/asm/topology.h       |   19 +++
>  arch/x86/kernel/cpu/Makefile          |    3 
>  arch/x86/kernel/cpu/common.c          |   23 +---
>  arch/x86/kernel/cpu/cpu.h             |    6 +
>  arch/x86/kernel/cpu/debugfs.c         |   37 ++++++
>  arch/x86/kernel/cpu/topology.h        |   32 +++++
>  arch/x86/kernel/cpu/topology_common.c |  187 ++++++++++++++++++++++++++++++++++
>  7 files changed, 290 insertions(+), 17 deletions(-)
> 
> --- a/arch/x86/include/asm/topology.h
> +++ b/arch/x86/include/asm/topology.h
> @@ -102,6 +102,25 @@ static inline void setup_node_to_cpumask
>  
>  #include <asm-generic/topology.h>
>  
> +/* Topology information */
> +enum x86_topology_domains {
> +	TOPO_SMT_DOMAIN,
> +	TOPO_CORE_DOMAIN,
> +	TOPO_MODULE_DOMAIN,
> +	TOPO_TILE_DOMAIN,
> +	TOPO_DIE_DOMAIN,
> +	TOPO_PKG_DOMAIN,
> +	TOPO_ROOT_DOMAIN,
> +	TOPO_MAX_DOMAIN,
> +};
> +

[..snip..]

> +static void topo_set_ids(struct topo_scan *tscan)
> +{
> +	struct cpuinfo_x86 *c = tscan->c;
> +	u32 apicid = c->topo.apicid;
> +
> +	c->topo.pkg_id = topo_shift_apicid(apicid, TOPO_ROOT_DOMAIN);

Shouldn't this use TOPO_PKG_DOMAIN instead of TOPO_ROOT_DOMAIN ?


> +	c->topo.die_id = topo_shift_apicid(apicid, TOPO_DIE_DOMAIN);
> +
> +	/* Relative core ID */
> +	c->topo.core_id = topo_relative_domain_id(apicid, TOPO_CORE_DOMAIN);
> +}
> +

--
Thanks and Regards
gautham.