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 CE9FECA101F for ; Wed, 10 Sep 2025 19:29:34 +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:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=xtDtdwfWBZTvma6YjsFDJZKjZ+f/8JFESEhqdI3H56Y=; b=3UXeVKW9KlNEU9LSDljqMBq52l EG5WTdDE9FSVE98Sl+GSEOQVNxIhE4aGJAYpP+Smn36qFQCn7fbWUgR6cpkr2Cw08FqlTU6rv3Eo8 jsizFMoTD66JNEyJoqmtsvyVjRVMD1r810BbvwqLPsWrW/nB3gYVZNPyQ+S+SKg0Laie4z045u9FI JuSCcvBCT6huCj43Rx7wCEJTYf2wB2U0tafYQBs9eEXlK9qzfU2otC96RM5FirZSCQLg83IYxBfbN wRtODOTclpGVDZJLWfcwP7pMbrLCQ6l5ZJIin818mr/drn4JjvEAJkhUXLwrLd5YYlzRiG866M/qq svx4p3+g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uwQVT-0000000GHPG-3Xv0; Wed, 10 Sep 2025 19:29:27 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uwQVR-0000000GHOI-0gxG for linux-arm-kernel@lists.infradead.org; Wed, 10 Sep 2025 19:29:26 +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 8771D16F2; Wed, 10 Sep 2025 12:29:15 -0700 (PDT) Received: from [10.1.197.69] (eglon.cambridge.arm.com [10.1.197.69]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id D5EC03F694; Wed, 10 Sep 2025 12:29:17 -0700 (PDT) Message-ID: <7353f490-2cac-4b97-9f48-c612b9034561@arm.com> Date: Wed, 10 Sep 2025 20:29:17 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 06/33] ACPI / PPTT: Add a helper to fill a cpumask from a cache_id To: Dave Martin Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-acpi@vger.kernel.org, devicetree@vger.kernel.org, D Scott Phillips OS , carl@os.amperecomputing.com, lcherian@marvell.com, bobo.shaobowang@huawei.com, tan.shaopeng@fujitsu.com, baolin.wang@linux.alibaba.com, Jamie Iles , Xin Hao , peternewman@google.com, dfustini@baylibre.com, amitsinght@marvell.com, David Hildenbrand , Rex Nie , Koba Ko , Shanker Donthineni , fenghuay@nvidia.com, baisheng.gao@unisoc.com, Jonathan Cameron , Rob Herring , Rohit Mathew , Rafael Wysocki , Len Brown , Lorenzo Pieralisi , Hanjun Guo , Sudeep Holla , Krzysztof Kozlowski , Conor Dooley , Catalin Marinas , Will Deacon , Greg Kroah-Hartman , Danilo Krummrich References: <20250822153048.2287-1-james.morse@arm.com> <20250822153048.2287-7-james.morse@arm.com> <2e4c3c00-b248-421e-8ff1-d24b7b03be1a@arm.com> Content-Language: en-GB From: James Morse In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250910_122925_298771_8339CBE3 X-CRM114-Status: GOOD ( 33.31 ) 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 Hi Dave, On 09/09/2025 11:14, Dave Martin wrote: > On Thu, Aug 28, 2025 at 04:58:16PM +0100, James Morse wrote: >> On 27/08/2025 11:53, Dave Martin wrote: >>> On Fri, Aug 22, 2025 at 03:29:47PM +0000, James Morse wrote: >>>> MPAM identifies CPUs by the cache_id in the PPTT cache structure. >>>> >>>> The driver needs to know which CPUs are associated with the cache, >>>> the CPUs may not all be online, so cacheinfo does not have the >>>> information. >>>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c >>>> index 660457644a5b..cb93a9a7f9b6 100644 >>>> --- a/drivers/acpi/pptt.c >>>> +++ b/drivers/acpi/pptt.c >>>> @@ -971,3 +971,65 @@ int find_acpi_cache_level_from_id(u32 cache_id) > > [...] > >>>> + * acpi_pptt_get_cpumask_from_cache_id() - Get the cpus associated with the >>>> + * specified cache >>>> + * @cache_id: The id field of the unified cache >>>> + * @cpus: Where to build the cpumask >>>> + * >>>> + * Determine which CPUs are below this cache in the PPTT. This allows the property >>>> + * to be found even if the CPUs are offline. >>>> + * >>>> + * The PPTT table must be rev 3 or later, >>>> + * >>>> + * Return: -ENOENT if the PPTT doesn't exist, or the cache cannot be found. >>>> + * Otherwise returns 0 and sets the cpus in the provided cpumask. >>>> + */ >>>> +int acpi_pptt_get_cpumask_from_cache_id(u32 cache_id, cpumask_t *cpus) >>>> +{ > > [...] > >>>> + /* >>>> + * If we found the cache first, we'd still need to walk from each cpu. >>>> + */ >>>> + for_each_possible_cpu(cpu) { > > [...] > >>> Again, it feels like we are repeating the same walk multiple times to >>> determine how deep the table is (on which point the table is self- >>> describing anyway), and then again to derive some static property, and >>> then we are then doing all of that work multiple times to derive >>> different static properties, etc. >>> >>> Can we not just walk over the tables once and stash the derived >>> properties somewhere? >> >> That is possible - but its a more invasive change to the PPTT parsing code. >> Before the introduction of the leaf flag, the search for a processor also included a >> search to check if the discovered node was a leaf. >> >> I think this is trading time - walking over the table multiple times, against the memory >> you'd need to de-serialise the tree to find the necessary properties quickly. I think the >> reason Jeremy L went this way was because there may never be another request into this >> code, so being ready with a quick answer was a waste of memory. >> >> MPAM doesn't change this - all these things are done up front during driver probing, and >> the values are cached by the driver. > > I guess that's true. > >>> I'm still getting my head around this parsing code, so I'm not saying >>> that the approach is incorrect here -- just wondering whether there is >>> a way to make it simpler. >> >> It's walked at boot, and on cpu-hotplug. Neither are particularly performance critical. > Do we do this only for unknown late secondaries (e.g., that haven't > previously come online?) No, each time a CPU comes online. > I haven't gone to track this down but, if not, > this cuts across the assertion that "there may never be another request > into this code". CPU hotplug is optional - you don't have to bounce CPUs. It's very common on mobile parts for power saving. I think its fairly unusual on server parts, once CPUs are online they stay online. The cacheinfo code doesn't cache this, it re-reads it every time. That turns out to be because of PowerPC where some of these properties can be changed while a CPU is offline. Sure, we could have a Kconfig thing to say ARCH_STATIC_TABLES_ARE_STATIC, but that would be a different piece of work. (I've had a couple of stabs at this, but cacheinfo is the shape it needs to be) > cpu hotlug is slow in practice, but gratuitous cost on this path should > still be avoided where feasible. > >> I agree that as platforms get bigger, there will be a tipping point ... I don't think >> anyone has complained yet! > > Ack -- when in ACPI, do as the ACPI folks do, I guess. Thanks, James