Re: [PATCH v4 2/2] cache: add SMCCC-backed cache invalidate provider
From: Andre Przywara
Date: Thu Oct 15 2026 - 06:25:18 EST
Hi,
On 10/15/26 09:43, Bence Csókás wrote:
Hi,
On 2026. 10. 14. 17:38, Andre Przywara wrote:
+static int smccc_cache_probe(struct arm_smccc_device *sdev)
+{
+ struct arm_smccc_res res = {};
+ struct smccc_cache *cache;
+ int ret;
+
+ arm_smccc_1_1_invoke(sdev->func_id, &res);
I think you should provide the explicit function ID name here, and not rely on which function exactly is used to probe for the interface's existence.
I thought that was the point of the matcher, that you could in theory
But that probe() function here is not a "matcher" or a check function, that is done in the SMCCC bus generic code, using a function ID that has no side effects and does not return -1.
The reason for this SMC call here is to learn the attributes, they are used below to fill the latency and rate_limit fields in the struct.
So we should then make this explicit, I think, by using the ARM_SMCCC_ARCH_CLEAN_INV_MEMREGION_ATTRIBUTES name.
assign the same probe handler to any function ID? For example if there's some FW that implements this (or a compatible) function, but at a different ID, you would only update the match table? At least that's how most subsystems (I2C, PCI, USB) do it.
Not sure I understand this, this is a driver specifically for the CLEANINV_MEM_REGION SMCCC interface, I don't see how you would reuse that, or plug any other interfaces to it.
Where does it say that? My copy of DEN 0028 1.7 H, released 2026/Jul/24, only says the following:+ if ((s32)res.a0)
+ return -ENODEV;
+
+ cache = cache_coherency_ops_instance_alloc(&smccc_cache_ops,
+ struct smccc_cache, cci);
+ if (!cache)
+ return -ENOMEM;
+
+ mutex_init(&cache->lock);
+ cache->latency_us = lower_32_bits(res.a2);
+ cache->rate_limit = res.a3;
According to the spec you would need lower_32_bits() for a3 as well - if we are going to use this value. But I don't think we need to, see above.
X3 (uint64_t attr_2)
Rate Limit: The maximum number of SMCCCC_ARCH_CLEAN_INV_MEMREGION calls allowed per second. If set to 0, it indicates that this field is not reported by the implementation.
See my second-to-last comment on v2 of this patch:
https://lore.kernel.org/lkml/591e7ac9-310e-40f4-ab22-161d6ea8f487@xxxxxxx/
Maybe I have an outdated copy already? ;_;
No, I had! Mine said v1.7H, but is was an early version, which had the upper 32 bits of rate_limit as reserved.
Indeed in the later public version it's a 64-bit field, so ignore my comment. Sorry for the noise!
Cheers,
Andre