cache: Introduce device cache support

[carlocaione]

The cache API currently shipped in Zephyr is assuming that the cache controller is always on-core thus managed at the arch level. This is not always the case because many SoCs rely on external cache controllers as a peripheral external to the core (for example PL310 cache controller and the L2Cxxx family).

Move the cache code into the drivers directory and extend the API to support device-based cache controllers.

This is the outcome of the discussion carried out in #33122 and this solution should tick all the boxes and requirements. The actual changes are overall minimal, just some ifdef to hook up eventual cache devices and a new Kconfig to switch between ARCH and DEVICE cache controllers.

[npitre]

I'm a bit hesitant wrt the naming. To me "device cache" makes me think about caching for particular devices, not about a device providing caching services. What about s/device_/external_/ ?

[carlocaione]

What about s/device_/external_/ ?

As usual I'm not strongly opinionated on naming, but FWIW external sounds fine to me.

[andyross]

Is this code motion really going to be helpful? I mean, it's true that caching is done at all sort of crazy layers by different platforms, but those distinctions and behavior are always going to be platform-specific. Questions like "Is this particular DMA controller upstream or downstream of the cache?" or "How do I configure this address range to be uncached" are just never going to be presentable in a uniform way. (Also weirder stuff like "convert this cached pointer to an uncached pointer to the same memory", as some Xtensa devices can do). Apps need to know the hardware in use if they want to do this.

Isn't the right place to do this in the arch layer still, providing an abstraction useful to whatever specific SOCs are present in that arch?

What value does having a generic controller API tailored to an ARM cache controller bring to, say, a RISC-V platform with entirely different semantics?

Not really worth a -1, as it's just code motion. I just don't see the value beyond making one particular architecture aesthetically cleaner.

[dcpleung]

(Hm... just thought of this one after looking at the code.)

How about a cache controller API with prefix cache_*? We can still implement arch-specific functions under arch/, and keep external controller under drivers/cache (for example). Then we can have something like CONFIG_ARCH_HAS_CACHE_CONTROL implying CONFIG_CACHE_USE_ARCH (for lack of better naming) where the arch level code is enclosed in this config under #ifdef. This can be disabled downstream in SoC or board configs. Additional cache controller can implement the APIs as-is (without the api->* redirection) with their own kconfig. It's similar to how timer drivers implement all the sys_clock_* functions.

[carlocaione]

Is this code motion really going to be helpful?

This is not only a code motion, the point of this PR is also adding the possibility to have an external cache controller. This is done because external cache controllers exist indeed and as such they should be treated as regular devices. PL310 is indeed just an example, but it's a pretty valid one because it has a DT entry, MMIO registers, PM controls, etc... why should be that a special case of a device without a proper driver?

Apps need to know the hardware in use if they want to do this.

Sure, if they need something out of the ordinary they can implement whatever they need, but the current cache API is really only covering the basics and that's it.

What value does having a generic controller API tailored to an ARM cache controller bring to, say, a RISC-V platform with entirely different semantics?

What do you see in the API that is tailored to the ARM cache controller? The API is pretty generic (basically flushing and invalidating) and that should be standard all across the architectures. Also you are assuming that external cache controllers are always arch-specific but that's not always the case.

[andyross]

I'm just concerned about a future where we have a "generic" cache layer in the "driver" directory that turns out to be ARM specific and all the SOCs continue to support their own internal APIs for the complicated stuff, gaining us nothing but confusion.

The point being: this doesn't look like a generic solution to me at all, really. It looks like you're moving an ARM-specific cache controller into the driver layer, which isn't wrong by itself, I just don't think this is actually solving anything interesting vs. just having an API indirection under arch/arm or soc/arm or wherever.

[carlocaione]

I'm just concerned about a future where we have a "generic" cache layer in the "driver" directory that turns out to be ARM specific

I'm probably not familiar enough with other architectures but what do you see about the current API that turns out to be ARM specific?

and all the SOCs continue to support their own internal APIs for the complicated stuff, gaining us nothing but confusion.

Right now all the architectures in Zephyr with a cache implementation (ARM64, X86, ARC but not Xtensa) are already using the cache API (or at least they have hooks for that) so not sure what you are referring to here. And also Xtensa cache functions are basically the same as the ones in this API.

The point being: this doesn't look like a generic solution to me at all, really. It looks like you're moving an ARM-specific cache controller into the driver layer, which isn't wrong by itself, I just don't think this is actually solving anything interesting vs. just having an API indirection under arch/arm or soc/arm or wherever.

What I'm trying to solve here is basically having a way to use an external cache controller independent from a specific arch.

[abrodkin]

What I'm trying to solve here is basically having a way to use an external cache controller independent from a specific arch.

Please pardon my lack of a wider knowledge (I'm surely more knowledgeable about just one particular architecture) still, does that thing (I mean "cache controller independent from a specific arch") really exist?

I guess aforementioned PL310 controller is rarely if at all being used with any other processors except ARM. Or is it?

In ARC (and I guess all other modern CPU arches) we also have more interesting cache controllers in addition to the L1 instruction/data caches. Like shared L2 or L3 caches with their own controllers which might have tons of features, configuration registers and what not.

That said if that "generic-ness" of the cache controller is only applicable to a particular architecture, maybe keep it in the corresponding arch folder?

[carlocaione]

Please pardon my lack of a wider knowledge (I'm surely more knowledgeable about just one particular architecture) still, does that thing (I mean "cache controller independent from a specific arch") really exist?

Yes, there is an ongoing effort to have SoC with a shared external cache controller shared by different cores with different architectures. It will be upstreamed in the near future.

I guess aforementioned PL310 controller is rarely if at all being used with any other processors except ARM. Or is it?

Correct. But the PL310 example was just to prove the need to support external cache controllers.

That said if that "generic-ness" of the cache controller is only applicable to a particular architecture, maybe keep it in the corresponding arch folder?

That's not possible because the cache controller I'll be working on later will be used by different architectures at the same time.

So this work is going to address two needs:

1. Support arch-specific external cache controllers (like PL310 and probably others)
2. Support external cache controllers shared by multiple architectures, sometimes at the same time

[carlocaione]

How about a cache controller API with prefix cache_*?
...
Additional cache controller can implement the APIs as-is (without the api->* redirection) with their own kconfig.

You mean having a __subsystem struct to define a cache controller API? I think that's not going to work because you do not really want to have a device object to hook up the struct to. In general I think we should aim to have a "system-wide" cache API so that we can do something like dcache_all(K_CACHE_WB) without having to specify the actual cache device we want to act on.

We can still implement arch-specific functions under arch/, and keep external controller under drivers/cache (for example). Then we can have something like CONFIG_ARCH_HAS_CACHE_CONTROL implying CONFIG_CACHE_USE_ARCH (for lack of better naming) where the arch level code is enclosed in this config under #ifdef. This can be disabled downstream in SoC or board configs.

Uhm, I fail to see how that is different from the case proposed in this PR. Basically CONFIG_ARCH_HAS_CACHE_CONTROL == CONFIG_CACHE_MANAGEMENT and CONFIG_CACHE_USE_ARCH == CONFIG_HAS_ARCH_CACHE, but for the rest I think we are on the same page.

It's similar to how timer drivers implement all the sys_clock_* functions.

sys_clock_* is using weak functions that we decided to not use, besides that I think I got your point.

@dcpleung I'm going to push a new revision with a better separation between arch and device layer, maybe you will like that more 😉

[dcpleung]

Thanks. This looks a lot cleaner.

Though I still think we should prefix everything with cache_ so it would be easier for code completion in IDEs.

As for sys_clock_*, that is just an example. What I am thinking is that there would be no weak functions. The cache control functions will be provided by files under arch/ if kconfig selects arch cache code. Same with cache controller drivers. Though, with this scheme, you can only have one active cache controller, and won't be able to do multi-layer. (not sure if it is ever needed)

[carlocaione]

Though I still think we should prefix everything with cache_ so it would be easier for code completion in IDEs.

Good point. I still think that cache_{d,i}cache_* is a bit of an odd naming. I'll revise this after getting more comments on the current PR.

As for sys_clock_*, that is just an example. What I am thinking is that there would be no weak functions. The cache control functions will be provided by files under arch/ if kconfig selects arch cache code. Same with cache controller drivers. Though, with this scheme, you can only have one active cache controller, and won't be able to do multi-layer. (not sure if it is ever needed)

We are not interested at the moment in multi-layer. The current assumption is that you can have one single external cache controller at time. I think that the current state of this PR should allow already to do what you have in mind.

[npitre]

Good point. I still think that `cache_{d,i}cache_*` is a bit of an odd naming.

Better have `dcache_*` and `icache_*`.

[dcpleung]

Maybe cache_data_* and cache_instr_*?

[ioannisg]

Is this intentional to keep the EXTERNAL prefix here, while the directory path is just "cache"?

[carlocaione]

I found it to be more descriptive but I'll change it to match the directory name.

[ioannisg]

Looks ok to me. I had a minor comment about an un-needed file, and a question about the Kconfig naming, using EXTERNAL