Received: by 2002:a05:6358:11c7:b0:104:8066:f915 with SMTP id i7csp2209964rwl; Thu, 13 Apr 2023 03:18:19 -0700 (PDT) X-Google-Smtp-Source: AKy350ZQXubAOL9yw/9NlCYb/TmLQc4DGDYimgiquxNu2FMeobJyjMCLrY0evjIr/IR8xVCRMpFI X-Received: by 2002:a05:6402:1342:b0:504:d768:5c0b with SMTP id y2-20020a056402134200b00504d7685c0bmr1972557edw.35.1681381098815; Thu, 13 Apr 2023 03:18:18 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1681381098; cv=none; d=google.com; s=arc-20160816; b=PBxgavY4FPoM1xPycM7qFPzka4FX1zTe3ypYEWks1CSZ4Qp09kyur0vbYSX04rKh4k cWAldBkQVLkfTp/ndNSlb5cs7UZIBu+VeCi0WjMAbkoCNBdVf4U79gcnud9V2E8Mdnxv CvTgdbdTgOF4mcxnKXJT3/OyXOX9hvu0+4XD/0Shr5RR7Mxhz7YXUSQGtNeGnpQhtmaM qzglEO/Woff+rU4xmHazJOTSMLKv6vYKWN+RU1+9DYNM+i0rLOhvdMJ/i5W0pZQ3VJXT 0uXc6TU5HeK4mxF9CIIQpAQlhZMexWpsOBTg6CwCOsjQ0NZBTcjXp6lFu2CQW9h+Ktuv eESQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id; bh=WjHgoZ9slwC2TxUpwoQdp7UCb7x07/1B9J3uwKnyNy4=; b=K20ukrx3qXpEr2hFsKi7JyRXD9OilPGFqRQ4imNPT6dtLF5aYiAr5DmwSIbwUrVf2f VWs0Yx2lySvK62nqklR4xhGBjPy8G9+JYojYhlGt+DbrhwzJXfmvgiHc0obd24W7n22n vEbEb2RyVeJRQ1HkI54juatMwsznv3Mi92KX8CRLFgvK+qxIopY5ru9qX41xRGx1cGEc VSGivLVThmfrTtjsER7punQyXPDijcZAf+d6MkV1WFklkIn1gJZQ4OrUtP8+SYAx0m7J WuvIQTh4iR5oI3YvRpPH77gemv9Rgy85CfUtugQXEFtW98cHaRQjJdvmTek4RzaVVxZ4 1HOg== ARC-Authentication-Results: i=1; mx.google.com; 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=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Return-Path: Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id i23-20020a50fc17000000b004ace3a42dc2si1548276edr.356.2023.04.13.03.17.53; Thu, 13 Apr 2023 03:18:18 -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; 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=fail (p=NONE sp=NONE dis=NONE) header.from=arm.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230211AbjDMKRg (ORCPT + 99 others); Thu, 13 Apr 2023 06:17:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37202 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230213AbjDMKRU (ORCPT ); Thu, 13 Apr 2023 06:17:20 -0400 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 17AF7FF for ; Thu, 13 Apr 2023 03:17:15 -0700 (PDT) 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 8E90B4B3; Thu, 13 Apr 2023 03:17:59 -0700 (PDT) Received: from [10.34.100.129] (pierre123.nice.arm.com [10.34.100.129]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id A8E0A3F6C4; Thu, 13 Apr 2023 03:17:13 -0700 (PDT) Message-ID: Date: Thu, 13 Apr 2023 12:17:09 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.9.0 Subject: Re: [PATCH v3 4/4] cacheinfo: Add use_arch[|_cache]_info field/function To: Sudeep Holla Cc: linux-kernel@vger.kernel.org, Radu Rendec , Alexandre Ghiti , Conor Dooley , Will Deacon , Greg Kroah-Hartman , "Rafael J. Wysocki" , Palmer Dabbelt , Gavin Shan References: <20230413091436.230134-1-pierre.gondois@arm.com> <20230413091436.230134-5-pierre.gondois@arm.com> <20230413094952.ncdldvv2ysjqwbsv@bogus> Content-Language: en-US From: Pierre Gondois In-Reply-To: <20230413094952.ncdldvv2ysjqwbsv@bogus> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-5.3 required=5.0 tests=BAYES_00,NICE_REPLY_A, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE 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: X-Mailing-List: linux-kernel@vger.kernel.org On 4/13/23 11:49, Sudeep Holla wrote: > On Thu, Apr 13, 2023 at 11:14:34AM +0200, Pierre Gondois wrote: >> The cache information can be extracted from either a Device >> Tree (DT), the PPTT ACPI table, or arch registers (clidr_el1 >> for arm64). >> >> The clidr_el1 register is used only if DT/ACPI information is not >> available. It does not states how caches are shared among CPUs. >> >> Add a use_arch_cache_info field/function to identify when the >> DT/ACPI doesn't provide cache information. Use this information >> to assume L1 caches are privates and L2 and higher are shared among >> all CPUs. >> > > I have tentatively merged first 3 patches along with Radu's series(waiting > for build tests still before confirming). I am not yet sure on this. > >> Signed-off-by: Pierre Gondois >> --- >> drivers/base/cacheinfo.c | 13 ++++++++++++- >> include/linux/cacheinfo.h | 10 ++++++++++ >> 2 files changed, 22 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c >> index 06de9a468958..49dbb4357911 100644 >> --- a/drivers/base/cacheinfo.c >> +++ b/drivers/base/cacheinfo.c >> @@ -40,7 +40,8 @@ static inline bool cache_leaves_are_shared(struct cacheinfo *this_leaf, >> * For non DT/ACPI systems, assume unique level 1 caches, >> * system-wide shared caches for all other levels. >> */ >> - if (!(IS_ENABLED(CONFIG_OF) || IS_ENABLED(CONFIG_ACPI))) >> + if (!(IS_ENABLED(CONFIG_OF) || IS_ENABLED(CONFIG_ACPI)) || >> + this_leaf->use_arch_info) > > Can't we just use use_arch_cache_info() here ? I think that if we use use_arch_cache_info() here, then arm64 platforms will always return here and never check fw_token/this_leaf->id values. Indeed, we also need to know that no cache information is available in DT/ACPI, cf. [1] > >> return (this_leaf->level != 1) && (sib_leaf->level != 1); >> >> if ((sib_leaf->attributes & CACHE_ID) && >> @@ -349,6 +350,7 @@ static int cache_shared_cpu_map_setup(unsigned int cpu) >> struct cpu_cacheinfo *this_cpu_ci = get_cpu_cacheinfo(cpu); >> struct cacheinfo *this_leaf, *sib_leaf; >> unsigned int index, sib_index; >> + bool use_arch_info = false; >> int ret = 0; >> >> if (this_cpu_ci->cpu_map_populated) >> @@ -361,6 +363,12 @@ static int cache_shared_cpu_map_setup(unsigned int cpu) >> */ >> if (!last_level_cache_is_valid(cpu)) { >> ret = cache_setup_properties(cpu); >> + if (ret && use_arch_cache_info()) { >> + // Possibility to rely on arch specific information. [1] >> + use_arch_info = true; >> + ret = 0; >> + } >> + >> if (ret) >> return ret; >> } >> @@ -370,6 +378,9 @@ static int cache_shared_cpu_map_setup(unsigned int cpu) >> >> this_leaf = per_cpu_cacheinfo_idx(cpu, index); >> >> + if (use_arch_info) >> + this_leaf->use_arch_info = true; >> + >> cpumask_set_cpu(cpu, &this_leaf->shared_cpu_map); >> for_each_online_cpu(i) { >> struct cpu_cacheinfo *sib_cpu_ci = get_cpu_cacheinfo(i); >> diff --git a/include/linux/cacheinfo.h b/include/linux/cacheinfo.h >> index 908e19d17f49..fed675b251a2 100644 >> --- a/include/linux/cacheinfo.h >> +++ b/include/linux/cacheinfo.h >> @@ -66,6 +66,7 @@ struct cacheinfo { >> #define CACHE_ALLOCATE_POLICY_MASK \ >> (CACHE_READ_ALLOCATE | CACHE_WRITE_ALLOCATE) >> #define CACHE_ID BIT(4) >> + bool use_arch_info; > > Do you see the need to stash this value as it is either globally true or > false based on the arch ? A static variable could be used instead and set to true if we fail to fetch the cache information from DT/ACPI, cf. [1]. The only possible transition for this variable would be from false->true. I'll check if this works like this. > >> void *fw_token; >> bool disable_sysfs; >> void *priv; >> @@ -129,4 +130,13 @@ static inline int get_cpu_cacheinfo_id(int cpu, int level) >> return -1; >> } >> >> +static inline bool use_arch_cache_info(void) >> +{ >> +#if defined(CONFIG_ARM64) >> + return true; >> +#else >> + return false; >> +#endif >> +} >> + > > Can we just have it as: > #ifdef CONFIG_ARM64 > #define use_arch_cache_info() (true) > #else > #define use_arch_cache_info() (false) > #endif Yes sure, I'll post a v4 with this along Conor's requested change. > >> #endif /* _LINUX_CACHEINFO_H */ >> -- >> 2.25.1 >> >