From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 6262DE77188 for ; Fri, 10 Jan 2025 17:30:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=tUTWiLhUl1yqmIBrwgfLsVWcTt3i0J4HoIHAN6RT5/A=; b=xW4srPJUowcQXzpzvjK8DZaDSb HfrfdrYL5QRNOYpioVRZmT3a6N3aFtX2GEP2gEXI5tQbMGn5NOen8R0UGK4/ZGOzVlYV8PXsInnLR XAe96pYgKQPOMGz9oH/OIUOXeat5Y62yOm40vo+KFoquW5EtAgt6ssScZMf2FY2JPfCO0zCod1pRU JElgQbQ34sFaFvaLF4N/QGDbAZ+G1v8mUiQibAphB0xgo4b7qK+r7olvLuUcLsiQhlkfo7F5WbCUm tLBwDye0tuGlAzPyU66xjNS8JnTC5fsQ4bI52GceEpOWKRtvthiiKCwAdcLqwzuogjZYyLJjmAKgt epDcM47w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tWIpn-0000000GQrk-2dOw; Fri, 10 Jan 2025 17:30:11 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1tWIlm-0000000GPxc-48oq for linux-arm-kernel@lists.infradead.org; Fri, 10 Jan 2025 17:26:04 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D26CF1477; Fri, 10 Jan 2025 09:26:28 -0800 (PST) Received: from J2N7QTR9R3 (usa-sjc-imap-foss1.foss.arm.com [10.121.207.14]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 604A33F66E; Fri, 10 Jan 2025 09:25:58 -0800 (PST) Date: Fri, 10 Jan 2025 17:25:50 +0000 From: Mark Rutland To: Alireza Sanaee Cc: devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, robh@kernel.org, linuxarm@huawei.com, shameerali.kolothum.thodi@huawei.com, Jonathan.Cameron@huawei.com, jiangkunkun@huawei.com, yangyicong@hisilicon.com, zhao1.liu@intel.com Subject: Re: [PATCH] arm64: of: handle multiple threads in ARM cpu node Message-ID: References: <20250110161057.445-1-alireza.sanaee@huawei.com> <20250110170211.00004ac2@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250110170211.00004ac2@huawei.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250110_092603_110900_8DDDD118 X-CRM114-Status: GOOD ( 31.33 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jan 10, 2025 at 05:02:11PM +0000, Alireza Sanaee wrote: > On Fri, 10 Jan 2025 16:23:00 +0000 > Mark Rutland wrote: > > Hi Mark, > > Thanks for prompt feedback. > > Please look inline. > > > On Fri, Jan 10, 2025 at 04:10:57PM +0000, Alireza Sanaee wrote: > > > Update `of_parse_and_init_cpus` to parse reg property of CPU node as > > > an array based as per spec for SMT threads. > > > > > > Spec v0.4 Section 3.8.1: > > > > Which spec, and why do we care? > > For the spec, this is what I looked > into https://github.com/devicetree-org/devicetree-specification/releases/download/v0.4/devicetree-specification-v0.4.pdf > Section 3.8.1 > > Sorry I didn't put the link in there. Ok, so that's "The devicetree specification v0.4 from ${URL}", rather than "Spec v0.4". > One limitation with the existing approach is that it is not really > possible to describe shared caches for SMT cores as they will be seen > as separate CPU cores in the device tree. Is there anyway to do so? Can't the existing cache bindings handle that? e.g. give both threads a next-level-cache pointing to the shared L1? > More discussion over sharing caches for threads > here https://lore.kernel.org/kvm/20241219083237.265419-1-zhao1.liu@intel.com/ In that thread Rob refers to earlier discussions, so I don't think that thread alone has enough context. > > > The value of reg is a that defines a unique > > > CPU/thread id for the CPU/threads represented by the CPU node. > > > **If a CPU supports more than one thread (i.e. multiple streams of > > > execution) the reg property is an array with 1 element per > > > thread**. The address-cells on the /cpus node specifies how many > > > cells each element of the array takes. Software can determine the > > > number of threads by dividing the size of reg by the parent node's > > > address-cells. > > > > We already have systems where each thread gets a unique CPU node under > > /cpus, so we can't rely on this to determine the topology. > > I assume we can generate unique values even in reg array, but probably > makes things more complicated. The other bindings use phandles to refer to threads, and phandles point to nodes in the dt, so it's necessary for threads to be given separate nodes. Note that the CPU topology bindings use that to describe threads, see Documentation/devicetree/bindings/cpu/cpu-topology.txt > > Further, there are bindings which rely on being able to address each > > CPU/thread with a unique phandle (e.g. for affinity of PMU > > interrupts), which this would break. > > Regardless, as above I do not think this is a good idea. While it > > allows the DT to be written in a marginally simpler way, it makes > > things more complicated for the kernel and is incompatible with > > bindings that we already support. > > > > If anything "the spec" should be relaxed here. > > Hi Rob, > > If this approach is too disruptive, then shall we fallback to the > approach where go share L1 at next-level-cache entry? Ah, was that previously discussed, and were there any concerns against that approach? To be clear, my main concern here is that threads remain represented as distinct nodes under /cpus; I'm not wedded to the precise solution for representing shared caches. Mark.