lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aR9QRuUigBsxxelm@arm.com>
Date: Thu, 20 Nov 2025 17:30:46 +0000
From: Catalin Marinas <catalin.marinas@....com>
To: Conor Dooley <conor@...nel.org>
Cc: linux-arm-kernel@...ts.infradead.org,
	Conor Dooley <conor.dooley@...rochip.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Jonathan Cameron <jonathan.cameron@...wei.com>,
	Will Deacon <will@...nel.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] arm64, lib: make ARM64 select
 ARCH_HAS_CPU_CACHE_INVALIDATE_MEMREGION not GENERIC_CPU_CACHE_MAINTENANCE

On Wed, Nov 19, 2025 at 07:08:27PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@...rochip.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@...radead.org>
> Acked-by: Jonathan Cameron <jonathan.cameron@...wei.com>
> Signed-off-by: Conor Dooley <conor.dooley@...rochip.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ