public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: Jonathan Cameron <jic23@kernel•org>
To: Srirangan Madhavan <smadhavan@nvidia•com>
Cc: Mark Rutland <mark.rutland@arm•com>,
	Lorenzo Pieralisi <lpieralisi@kernel•org>,
	Sudeep Holla <sudeep.holla@kernel•org>,
	Conor Dooley <conor@kernel•org>,
	Catalin Marinas <catalin.marinas@arm•com>,
	Will Deacon <will@kernel•org>, Dan Williams <djbw@kernel•org>,
	Thierry Reding <treding@nvidia•com>,
	Jonathan Hunter <jonathanh@nvidia•com>,
	linux-arm-kernel@lists•infradead.org,
	linux-kernel@vger•kernel.org, linux-tegra@vger•kernel.org
Subject: Re: [PATCH v1 2/2] cache: add SMCCC-backed cache invalidate provider
Date: Tue, 2 Jun 2026 16:03:20 +0100	[thread overview]
Message-ID: <20260602160320.34f9e3a1@jic23-huawei> (raw)
In-Reply-To: <20260602082145.404939-3-smadhavan@nvidia.com>

On Tue,  2 Jun 2026 08:21:45 +0000
Srirangan Madhavan <smadhavan@nvidia•com> wrote:

> Add a cache maintenance provider for the Arm SMCCC cache clean+invalidate
> interface.
> 
> The provider discovers SMCCC support and attributes at init time,
> serializes firmware calls, handles transient BUSY and RATE_LIMITED
> responses with bounded retries, and registers with the generic cache
> coherency framework used by memregion callers.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia•com>
3 things inline.

With the loop style updated to either drop that optimization of
eliding of the last sleep or use a while (1) loop and the kconfig
modified to not imply there are _no_ registered providers.

Reviewed-by: Jonathan Cameron <jic23@kernel•org>

The comment in Makefile isn't really about this patch, just something
I noticed whilst wondering why it wasn't ordered.

> ---
>  drivers/cache/Kconfig           |  11 +++
>  drivers/cache/Makefile          |   1 +
>  drivers/cache/arm_smccc_cache.c | 160 ++++++++++++++++++++++++++++++++
>  3 files changed, 172 insertions(+)
>  create mode 100644 drivers/cache/arm_smccc_cache.c
> 
> diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
> index 1518449d47b5..5d7ef3d15979 100644
> --- a/drivers/cache/Kconfig
> +++ b/drivers/cache/Kconfig
> @@ -42,6 +42,17 @@ menuconfig CACHEMAINT_FOR_HOTPLUG
>  
>  if CACHEMAINT_FOR_HOTPLUG
>  
> +config ARM_SMCCC_CACHE
> +	bool "Arm SMCCC cache maintenance provider"
> +	depends on ARM64 && HAVE_ARM_SMCCC_DISCOVERY
> +	help
> +	  Enable support for the Arm SMCCC cache clean+invalidate
> +	  interface as a provider for memory hotplug-like cache
> +	  maintenance operations.
> +	  The provider registers only when firmware advertises the
> +	  SMCCC calls and attributes, so systems without firmware support
> +	  continue without a registered provider.

"without this registered provider". or "without registering a provider."

We need to be careful that we don't give the impressions this is the only
option for Arm64 systems that do SMCCC and need a cache maintenance
provider. The HiSilcon systems that use HISI_SOC_HHA meet all those
conditions - except that they don't use this service.

> +
>  config HISI_SOC_HHA
>  	tristate "HiSilicon Hydra Home Agent (HHA) device driver"
>  	depends on (ARM64 && ACPI) || COMPILE_TEST
> diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile
> index b3362b15d6c1..6d91085aafe4 100644
> --- a/drivers/cache/Makefile
> +++ b/drivers/cache/Makefile
> @@ -4,4 +4,5 @@ obj-$(CONFIG_AX45MP_L2_CACHE)		+= ax45mp_cache.o
>  obj-$(CONFIG_SIFIVE_CCACHE)		+= sifive_ccache.o
>  obj-$(CONFIG_STARFIVE_STARLINK_CACHE)	+= starfive_starlink_cache.o
>  
Maybe we should add a comment here
# Depend on CACHEMAINT_FOR_HOTPLUG

or something like that to avoid them being 'sorted' into the group above
at somepoint in future.

> +obj-$(CONFIG_ARM_SMCCC_CACHE)		+= arm_smccc_cache.o
>  obj-$(CONFIG_HISI_SOC_HHA)		+= hisi_soc_hha.o
> diff --git a/drivers/cache/arm_smccc_cache.c b/drivers/cache/arm_smccc_cache.c
> new file mode 100644
> index 000000000000..ff6bca91a1a1
> --- /dev/null
> +++ b/drivers/cache/arm_smccc_cache.c
> @@ -0,0 +1,160 @@

> + static int smccc_cache_wbinv(struct cache_coherency_ops_inst *cci,
> +			     struct cc_inval_params *invp)
> +{
> +	struct smccc_cache *cache = container_of(cci, struct smccc_cache, cci);
> +	struct arm_smccc_res res = {};
> +	unsigned long delay_us = smccc_cache_delay_us(cache);
> +	int ret;
> +
> +	if (!invp->size)
> +		return -EINVAL;
> +
> +	/*
> +	 * Serialize the full retry sequence. With the default bounds, a caller
> +	 * may hold the mutex across up to four 20ms backoff sleeps.
> +	 */
> +	guard(mutex)(&cache->lock);
> +
> +	for (unsigned int i = 0; i < SMCCC_CACHE_MAX_RETRIES; i++) {
> +		/* Long firmware operations can trigger watchdog checks. */
> +		touch_nmi_watchdog();
> +
> +		arm_smccc_1_1_invoke(ARM_SMCCC_ARCH_CLEAN_INV_MEMREGION,
> +				     invp->addr, invp->size, 0UL, &res);
> +
> +		ret = smccc_cache_status_to_errno((s32)res.a0);
> +		if (!ret)
> +			return 0;
> +
> +		if (ret != -EBUSY && ret != -EAGAIN)
> +			return ret;
> +
> +		if (i + 1 == SMCCC_CACHE_MAX_RETRIES)
> +			break;

The for loop bound makes little sense if you are going to do
this.  Use a while loop.  Or don't worry about optimising out the final
sleep as we never expect to hit this.

> +
> +		fsleep(delay_us);
> +	}
> +
> +	return -EBUSY;
> +}



      reply	other threads:[~2026-06-02 15:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-02  8:21 [PATCH v1 0/2] Add SMCCC cache clean/invalidate provider Srirangan Madhavan
2026-06-02  8:21 ` [PATCH v1 1/2] arm64: smccc: add cache clean/invalidate IDs and return codes Srirangan Madhavan
2026-06-02 14:49   ` Jonathan Cameron
2026-06-02  8:21 ` [PATCH v1 2/2] cache: add SMCCC-backed cache invalidate provider Srirangan Madhavan
2026-06-02 15:03   ` Jonathan Cameron [this message]

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=20260602160320.34f9e3a1@jic23-huawei \
    --to=jic23@kernel$(echo .)org \
    --cc=catalin.marinas@arm$(echo .)com \
    --cc=conor@kernel$(echo .)org \
    --cc=djbw@kernel$(echo .)org \
    --cc=jonathanh@nvidia$(echo .)com \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    --cc=linux-kernel@vger$(echo .)kernel.org \
    --cc=linux-tegra@vger$(echo .)kernel.org \
    --cc=lpieralisi@kernel$(echo .)org \
    --cc=mark.rutland@arm$(echo .)com \
    --cc=smadhavan@nvidia$(echo .)com \
    --cc=sudeep.holla@kernel$(echo .)org \
    --cc=treding@nvidia$(echo .)com \
    --cc=will@kernel$(echo .)org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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