linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Catalin Marinas <catalin.marinas@arm.com>
To: Conor Dooley <conor@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org,
	Conor Dooley <conor.dooley@microchip.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Jonathan Cameron <jonathan.cameron@huawei.com>,
	Will Deacon <will@kernel.org>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v1] arm64, lib: make ARM64 select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION not GENERIC_CPU_CACHE_MAINTENANCE
Date: Thu, 20 Nov 2025 17:30:46 +0000	[thread overview]
Message-ID: <aR9QRuUigBsxxelm@arm.com> (raw)
In-Reply-To: <20251119-zippy-distinct-1e2a7da7b69b@spud>

On Wed, Nov 19, 2025 at 07:08:27PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> Randy pointed out that the newly added GENERIC_CPU_CACHE_MAINTENANCE
> was unusual, in that it selected an ARCH_HAS_... option, unlikely
> anything else in lib/Kconfig. Switch things around, so that arm64
> selects ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION and then
> GENERIC_CPU_CACHE_MAINENANCE will in turn be automatically enabled.
> 
> Suggested-by: Randy Dunlap <rdunlap@infradead.org>
> Acked-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Catalin, you voiced a take on this already apparently that lead to the
> current implementation so I'd like an ack from you or Will here.
> My comment on the original thread, in response to Randy saying it was
> backwards, that accompanied the diff was:
>  Maybe it is backwards, but I feel like this way is more logical. ARM64
>  has memregion invalidation only because this generic approach is
>  enabled, so the arch selects what it needs to get the support.
>  Alternatively, something like (diff was here) implies (to me at least)
>  that arm64 has memregion invalidation as an architectural feature and
>  that the GENERIC_CPU_CACHE_MAINTENANCE option is a just common
>  cross-arch code, like generic entry etc, rather than being the option
>  gating the drivers that provide the feature in the first place.
> Ultimately, the .config produced is the same either way, just depends on
> what impression you want to give in the arch Kconfig, which might not
> really be a big deal, just semantics. Either way, I'd like an ack :)

I really don't remember what I said before ;). As you describe above,
there are two somewhat complementary options -
ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION where an arch port can provide
this API and GENERIC_CPU_CACHE_MAINTENANCE as a generic way of providing
the same API. arm64 does the latter.

> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 5f7f63d24931..75b2507f7eb2 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -21,6 +21,7 @@ config ARM64
>  	select ARCH_ENABLE_THP_MIGRATION if TRANSPARENT_HUGEPAGE
>  	select ARCH_HAS_CACHE_LINE_SIZE
>  	select ARCH_HAS_CC_PLATFORM
> +	select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
>  	select ARCH_HAS_CURRENT_STACK_POINTER
>  	select ARCH_HAS_DEBUG_VIRTUAL
>  	select ARCH_HAS_DEBUG_VM_PGTABLE
> @@ -146,7 +147,6 @@ config ARM64
>  	select GENERIC_ARCH_TOPOLOGY
>  	select GENERIC_CLOCKEVENTS_BROADCAST
>  	select GENERIC_CPU_AUTOPROBE
> -	select GENERIC_CPU_CACHE_MAINTENANCE
>  	select GENERIC_CPU_DEVICES
>  	select GENERIC_CPU_VULNERABILITIES
>  	select GENERIC_EARLY_IOREMAP
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 09aec4a1e13f..ac223e627bc5 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -544,8 +544,9 @@ config ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
>  	bool
>  
>  config GENERIC_CPU_CACHE_MAINTENANCE
> -	bool
> -	select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
> +	def_bool y
> +	depends on ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION
> +	depends on ARM64

That's what we do if GENERIC_CPU_CACHE_MAINTENANCE depends on some arch
code but that's not the case here. GENERIC_CPU_CACHE_MAINTENANCE is an
alternative implementation that an arch can select if it does not
provide its own. I find the current code without the above patch better.

Maybe what gets confusing here is that the core code uses
ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION directly. A more involved fix
would be something like:

config CPU_CACHE_INVALIDATE_MEMREGION
	def_bool y
	depends on ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION ||
		GENERIC_CPU_CACHE_MAINTENANCE

and then go and change all the uses of
ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION.

Up to you, the current code also works for me.

-- 
Catalin


  parent reply	other threads:[~2025-11-20 17:30 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-19 19:08 [PATCH v1] arm64, lib: make ARM64 select ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION not GENERIC_CPU_CACHE_MAINTENANCE Conor Dooley
2025-11-19 21:22 ` Randy Dunlap
2025-11-20 17:30 ` Catalin Marinas [this message]
2025-11-20 19:29   ` Conor Dooley
2025-11-20 20:13     ` Randy Dunlap
2025-11-20 21:40       ` Catalin Marinas
2025-11-20 23:25         ` Randy Dunlap
2025-11-21 10:48           ` Jonathan Cameron
2025-11-21 18:44             ` Conor Dooley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aR9QRuUigBsxxelm@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=conor.dooley@microchip.com \
    --cc=conor@kernel.org \
    --cc=jonathan.cameron@huawei.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rdunlap@infradead.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).