All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.