lib/cache_maint.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Currently, if there are no registered handlers,
cpu_cache_invalidate_memregion() returns 0. Callers are expected to check
cpu_cache_has_invalidate_memregion() and return -ENXIO themselves, instead
of calling this function. If a caller forgets to do this, they may
erroneously think the operation succeeded. This is in contrast to x86,
where cpu_cache_invalidate_memregion() itself checks if the operation is
supported, and returns -ENXIO if it is not.
Signed-off-by: Bence Csokas <bence.csokas@arm.com>
---
lib/cache_maint.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/cache_maint.c b/lib/cache_maint.c
index 9256a9ffc34c..132d2816d6b1 100644
--- a/lib/cache_maint.c
+++ b/lib/cache_maint.c
@@ -59,12 +59,12 @@ static int cache_inval_done_one(struct cache_coherency_ops_inst *cci)
static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
{
- int ret;
struct cache_coherency_ops_inst *cci;
struct cc_inval_params params = {
.addr = addr,
.size = size,
};
+ int ret = -ENXIO;
guard(rwsem_read)(&cache_ops_instance_list_lock);
list_for_each_entry(cci, &cache_ops_instance_list, node) {
@@ -78,7 +78,7 @@ static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
return ret;
}
- return 0;
+ return ret;
}
struct cache_coherency_ops_inst *
--
2.55.0On Tue, 22 Sep 2026 09:29:31 +0200 Bence Csokas <bence.csokas@arm.com> wrote: > Currently, if there are no registered handlers, > cpu_cache_invalidate_memregion() returns 0. Callers are expected to check > cpu_cache_has_invalidate_memregion() and return -ENXIO themselves, instead > of calling this function. If a caller forgets to do this, they may > erroneously think the operation succeeded. This is in contrast to x86, > where cpu_cache_invalidate_memregion() itself checks if the operation is > supported, and returns -ENXIO if it is not. Do you expect any callers to be altered as a result of this change? If so, full details would be helpful.
Hi,
On 2026. 09. 23. 4:18, Andrew Morton wrote:
> On Tue, 22 Sep 2026 09:29:31 +0200 Bence Csokas <bence.csokas@arm.com> wrote:
>
>> Currently, if there are no registered handlers,
>> cpu_cache_invalidate_memregion() returns 0. Callers are expected to check
>> cpu_cache_has_invalidate_memregion() and return -ENXIO themselves, instead
>> of calling this function. If a caller forgets to do this, they may
>> erroneously think the operation succeeded. This is in contrast to x86,
>> where cpu_cache_invalidate_memregion() itself checks if the operation is
>> supported, and returns -ENXIO if it is not.
>
> Do you expect any callers to be altered as a result of this change?
>
> If so, full details would be helpful.
Current callers, not necessarily. Future callers, maybe.
Presently, as far as I see, there are 2 consumers: CXL and NVDIMM, with
3 call sites in total.
2 of these call sites [1] [2] are in the form of:
if (!has_invalidate()) {
if (TEST)
return 0;
return -ENXIO;
}
invalidate();
These types of call sites can stay as-is.
The remaining one [3] is in a void function, and in the form:
if (has_invalidate())
invalidate();
Lastly, while not a consumer, here's what x86 [4] does. It's possible a
future caller will want to replicate this:
if (!has_invalidate())
return -ENXIO;
invalidate();
These types of call sites can do away with calling has_invalidate()
going forward.
[1]
https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/cxl/core/region.c#L242
[2]
https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/nvdimm/region_devs.c#L93
[3]
https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/nvdimm/region.c#L113
[4]
https://elixir.bootlin.com/linux/v7.3-rc3/source/arch/x86/mm/pat/set_memory.c#L372
Bence
On Wed, 23 Sep 2026 10:21:15 +0200
Bence Csókás <bence.csokas@arm.com> wrote:
> Hi,
>
> On 2026. 09. 23. 4:18, Andrew Morton wrote:
> > On Tue, 22 Sep 2026 09:29:31 +0200 Bence Csokas <bence.csokas@arm.com> wrote:
> >
> >> Currently, if there are no registered handlers,
> >> cpu_cache_invalidate_memregion() returns 0. Callers are expected to check
> >> cpu_cache_has_invalidate_memregion() and return -ENXIO themselves, instead
> >> of calling this function. If a caller forgets to do this, they may
> >> erroneously think the operation succeeded. This is in contrast to x86,
> >> where cpu_cache_invalidate_memregion() itself checks if the operation is
> >> supported, and returns -ENXIO if it is not.
> >
> > Do you expect any callers to be altered as a result of this change?
> >
> > If so, full details would be helpful.
>
> Current callers, not necessarily. Future callers, maybe.
There is a race condition if someone unbinds the handlers that might
expose an error after this (correctly) when it didn't before.
In general that race exists in the setup path as well because we don't have
any way to know 'how many' devices to expect with assumption being
they have all turned up if any have. Not great, but short of adding
something to the firmware specification there wasn't a solution and
that process would be slow. We can revisit that if it becomes a problem
in practice.
>
> Presently, as far as I see, there are 2 consumers: CXL and NVDIMM, with
> 3 call sites in total.
>
> 2 of these call sites [1] [2] are in the form of:
>
> if (!has_invalidate()) {
> if (TEST)
> return 0;
> return -ENXIO;
> }
> invalidate();
>
> These types of call sites can stay as-is.
>
> The remaining one [3] is in a void function, and in the form:
>
> if (has_invalidate())
> invalidate();
>
> Lastly, while not a consumer, here's what x86 [4] does. It's possible a
> future caller will want to replicate this:
>
> if (!has_invalidate())
> return -ENXIO;
> invalidate();
>
> These types of call sites can do away with calling has_invalidate()
> going forward.
I'd be against any callers doing that as would rely on future implementations
of the interface. No support vs got an error tend to need different handling
and relying on particular error codes is likely to bite us eventually.
Jonathan
>
> [1]
> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/cxl/core/region.c#L242
> [2]
> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/nvdimm/region_devs.c#L93
> [3]
> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/nvdimm/region.c#L113
> [4]
> https://elixir.bootlin.com/linux/v7.3-rc3/source/arch/x86/mm/pat/set_memory.c#L372
>
> Bence
On Tue, 22 Sep 2026 09:29:31 +0200
Bence Csokas <bence.csokas@arm.com> wrote:
> Currently, if there are no registered handlers,
> cpu_cache_invalidate_memregion() returns 0. Callers are expected to check
> cpu_cache_has_invalidate_memregion() and return -ENXIO themselves, instead
> of calling this function. If a caller forgets to do this, they may
> erroneously think the operation succeeded. This is in contrast to x86,
> where cpu_cache_invalidate_memregion() itself checks if the operation is
> supported, and returns -ENXIO if it is not.
>
> Signed-off-by: Bence Csokas <bence.csokas@arm.com>
As you noted short of a race we should never hit this in practice as
all callers must check for support first. Still good to close
that race.
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Jonathan
> ---
> lib/cache_maint.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/lib/cache_maint.c b/lib/cache_maint.c
> index 9256a9ffc34c..132d2816d6b1 100644
> --- a/lib/cache_maint.c
> +++ b/lib/cache_maint.c
> @@ -59,12 +59,12 @@ static int cache_inval_done_one(struct cache_coherency_ops_inst *cci)
>
> static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
> {
> - int ret;
> struct cache_coherency_ops_inst *cci;
> struct cc_inval_params params = {
> .addr = addr,
> .size = size,
> };
> + int ret = -ENXIO;
>
> guard(rwsem_read)(&cache_ops_instance_list_lock);
> list_for_each_entry(cci, &cache_ops_instance_list, node) {
> @@ -78,7 +78,7 @@ static int cache_invalidate_memregion(phys_addr_t addr, size_t size)
> return ret;
> }
>
> - return 0;
> + return ret;
> }
>
> struct cache_coherency_ops_inst *
>
© 2016 - 2026 Red Hat, Inc.