Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Palmer Dabbelt]

On Thu, 15 Dec 2022 12:17:38 PST (-0800), geert@linux-m68k.org wrote:
> Hi Conor,
>
> On Thu, Dec 15, 2022 at 8:54 PM Conor Dooley <conor@kernel.org> wrote:
>> On Thu, Dec 15, 2022 at 05:46:42PM +0000, Lad, Prabhakar wrote:
>> > On Thu, Dec 15, 2022 at 11:10 AM Geert Uytterhoeven
>> > <geert@linux-m68k.org> wrote:
>> > > On Thu, Dec 15, 2022 at 12:06 PM Lad, Prabhakar
>> > > <prabhakar.csengg@gmail.com> wrote:
>> > > > On Thu, Dec 15, 2022 at 10:36 AM Geert Uytterhoeven
>> > > > <geert@linux-m68k.org> wrote:
>> > > > > On Mon, Dec 12, 2022 at 12:58 PM Prabhakar <prabhakar.csengg@gmail.com> wrote:
>> > > > > > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> > > > > >
>> > > > > > I/O Coherence Port (IOCP) provides an AXI interface for connecting
>> > > > > > external non-caching masters, such as DMA controllers. The accesses
>> > > > > > from IOCP are coherent with D-Caches and L2 Cache.
>> > > > > >
>> > > > > > IOCP is a specification option and is disabled on the Renesas RZ/Five
>> > > > > > SoC due to this reason IP blocks using DMA will fail.
>> > > > > >
>> > > > > > The Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
>> > > > > > block that allows dynamic adjustment of memory attributes in the runtime.
>> > > > > > It contains a configurable amount of PMA entries implemented as CSR
>> > > > > > registers to control the attributes of memory locations in interest.
>> > > > > > Below are the memory attributes supported:
>> > > > > > * Device, Non-bufferable
>> > > > > > * Device, bufferable
>> > > > > > * Memory, Non-cacheable, Non-bufferable
>> > > > > > * Memory, Non-cacheable, Bufferable
>> > > > > > * Memory, Write-back, No-allocate
>> > > > > > * Memory, Write-back, Read-allocate
>> > > > > > * Memory, Write-back, Write-allocate
>> > > > > > * Memory, Write-back, Read and Write-allocate
>> > > > > >
>> > > > > > More info about PMA (section 10.3):
>> > > > > > Link: http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf
>> > > > > >
>> > > > > > As a workaround for SoCs with IOCP disabled CMO needs to be handled by
>> > > > > > software. Firstly OpenSBI configures the memory region as
>> > > > > > "Memory, Non-cacheable, Bufferable" and passes this region as a global
>> > > > > > shared dma pool as a DT node. With DMA_GLOBAL_POOL enabled all DMA
>> > > > > > allocations happen from this region and synchronization callbacks are
>> > > > > > implemented to synchronize when doing DMA transactions.
>> > > > > >
>> > > > > > Example PMA region passes as a DT node from OpenSBI:
>> > > > > >     reserved-memory {
>> > > > > >         #address-cells = <2>;
>> > > > > >         #size-cells = <2>;
>> > > > > >         ranges;
>> > > > > >
>> > > > > >         pma_resv0@58000000 {
>> > > > > >             compatible = "shared-dma-pool";
>> > > > > >             reg = <0x0 0x58000000 0x0 0x08000000>;
>> > > > > >             no-map;
>> > > > > >             linux,dma-default;
>> > > > > >         };
>> > > > > >     };
>> > > > > >
>> > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>> > > > >
>> > > > > Thanks for your patch!
>> > > > >
>> > > > > >  arch/riscv/include/asm/cacheflush.h       |   8 +
>> > > > > >  arch/riscv/include/asm/errata_list.h      |  28 ++-
>> > > > > >  drivers/soc/renesas/Kconfig               |   6 +
>> > > > > >  drivers/soc/renesas/Makefile              |   2 +
>> > > > > >  drivers/soc/renesas/rzfive/Kconfig        |   6 +
>> > > > > >  drivers/soc/renesas/rzfive/Makefile       |   3 +
>> > > > > >  drivers/soc/renesas/rzfive/ax45mp_cache.c | 256 ++++++++++++++++++++++
>> > > > >
>> > > > > Given this touches arch/riscv/include/asm/, I don't think the
>> > > > > code belongs under drivers/soc/renesas/.
>> > > > >
>> > > > Ok. Do you have any suggestions on where you want me to put this code?
>> > >
>> > > As it plugs into core riscv functionality, I think it should be under
>> > > arch/riscv/.
>> > > if the RISC-V maintainers object to that, another option is
>> > > drivers/soc/andestech/ or (new) drivers/cache/
>> > >
>> > RISC-V maintainers had already made it clear to not to include vendor
>> > specific stuff in the arch/riscv folder, so I'll consider putting this

We have vendor-specific behavior in arch/riscv now, and it's even in the 
policies (the behavior has been there for a while, the policy is new).  
There's probably a bunch of posts saying otherwise because we used to 
not take vendor-specific behavior, but that's not the case any more.

>> > into drivers/cache/ folder to sync with the bindings.
>> >
>> > Conor/Palmer - do you have any objections/suggestions?
>>
>> I'm not its maintainer so sorta moot what I say, but having drivers in
>> arch/riscv makes little sense to me..

Some drivers are pretty tightly coupled to an ISA, I'm thinking of 
things like interrupts/timers where we're writing CSRs.  I could see how 
cache controllers would fall into that category, doubly so as they get 
integrated to the memory model.  That leaves us in sort of a grey area 
and there's certainly ports that have way more drivers in arch so it's 
not an uncommon way to do things.

I generally lean pretty heavily towards keeping things that could at all 
be considered out of arch/riscv and instead have them in some sort of 
driver folder.  That probably results in more total work to do, as we've 
got to have arch/riscv interfaces with one use case and sync up between 
multiple trees sometimes to get stuff merged.  I think it also results 
in better code: being closer to the other drivers makes us more likely 
to get reviewed by folks who understand the space, and it's generally 
easier to share code in the subsystems.

>> Putting stuff in drivers/cache does sound like a good idea since the
>> binding is going there too.
>>
>> The SiFive ccache driver is in drivers/soc and it was suggested to me
>> this week that there's likely going to be a second SiFive cache driver
>> at some point in the near future. Plus Microchip are going to have to
>> add cache management stuff to the existing SiFive ccache driver.
>> Having them be their own thing makes sense in my mind - especially since
>> they're not tied to SoCs sold by Andes or SiFive.
>>
>> I had a quick, and I mean *quick* look through other soc drivers to see
>> if there were any other cache controller drivers but nothing stood out
>> to me. Maybe someone else has more of a clue there. Ditto for misc, had
>> a look but nothing seemed obvious.
>
> Usually they're under arch/:
> $ git ls-files -- "arch/*cache*" | wc -l
> 148
> $ git ls-files -- "drivers/*cache*" | wc -l
> 63
>
> E.g. arch/arm/mm/cache-l2x0.c.

Above all I like to do things as normally as possible, so if most cache 
drivers are in arch then I'm OK with.  Looks like we originally had it 
arch/riscv, though, until 9209fb51896f ("riscv: move sifive_l2_cache.c 
to drivers/soc") moved it out.

I don't really remember this history/rationale here, at the time the 
SiFive cache driver wasn't really doing anything all that exciting: it 
was just way enable and some statistics, we hadn't really planned on 
using it for the non-coherent stuff that folks are now trying to 
retrofit it to handle.  That sort of thing makes it a lot more coupled 
to the ISA.

Given that we already moved the SiFive one out it seems sane to just 
start with the rest in drivers/soc/$VENDOR.  Looks like it was 
Christoph's idea to do the move, so I'm adding him in case he's got an 
opinion (and also the SOC alias, as that seems generally relevant).

> Gr{oetje,eeting}s,
>
>                         Geert

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Christoph Hellwig]

On Thu, Dec 15, 2022 at 01:40:30PM -0800, Palmer Dabbelt wrote:
> Given that we already moved the SiFive one out it seems sane to just start
> with the rest in drivers/soc/$VENDOR.  Looks like it was Christoph's idea to
> do the move, so I'm adding him in case he's got an opinion (and also the SOC
> alias, as that seems generally relevant).

Well, it isn't an integral architecture feature, so it doesn't really
beloing into arch.  Even the irqchip and timer drivers that are more
less architectural are in drivers/ as they aren't really core
architecture code. 

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Palmer Dabbelt]

On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
> On Thu, Dec 15, 2022 at 01:40:30PM -0800, Palmer Dabbelt wrote:
>> Given that we already moved the SiFive one out it seems sane to just start
>> with the rest in drivers/soc/$VENDOR.  Looks like it was Christoph's idea to
>> do the move, so I'm adding him in case he's got an opinion (and also the SOC
>> alias, as that seems generally relevant).
>
> Well, it isn't an integral architecture feature, so it doesn't really
> beloing into arch.  Even the irqchip and timer drivers that are more
> less architectural are in drivers/ as they aren't really core
> architecture code.

That makes sense to me, it just looks like the SiFive ccache is the only 
one that's in drivers/soc/$VENDOR, the rest are in arch.  It looks like 
mostly older ports that have vendor-specific cache files in arch (ie, 
arm has it but arm64 doesn't).  Maybe that's just because the newer 
architectures sorted out standard ISA interfaces for these and thus 
don't need the vendor-specific bits?  I think we're likely to end up 
with quite a few of these vendor-specific cache management schemes on 
RISC-V.

I'm always happy to keep stuff out of arch/riscv, though.  So maybe we 
just buck the trend here and stick to drivers/soc/$VENDOR like we did 
for the first one?

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Arnd Bergmann]

On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
> On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
>> On Thu, Dec 15, 2022 at 01:40:30PM -0800, Palmer Dabbelt wrote:
>>> Given that we already moved the SiFive one out it seems sane to just start
>>> with the rest in drivers/soc/$VENDOR.  Looks like it was Christoph's idea to
>>> do the move, so I'm adding him in case he's got an opinion (and also the SOC
>>> alias, as that seems generally relevant).
>>
>> Well, it isn't an integral architecture feature, so it doesn't really
>> beloing into arch.  Even the irqchip and timer drivers that are more
>> less architectural are in drivers/ as they aren't really core
>> architecture code.
>
> That makes sense to me, it just looks like the SiFive ccache is the only 
> one that's in drivers/soc/$VENDOR, the rest are in arch.  It looks like 
> mostly older ports that have vendor-specific cache files in arch (ie, 
> arm has it but arm64 doesn't).  Maybe that's just because the newer 
> architectures sorted out standard ISA interfaces for these and thus 
> don't need the vendor-specific bits?  I think we're likely to end up 
> with quite a few of these vendor-specific cache management schemes on 
> RISC-V.
>
> I'm always happy to keep stuff out of arch/riscv, though.  So maybe we 
> just buck the trend here and stick to drivers/soc/$VENDOR like we did 
> for the first one?

I don't particularly like drivers/soc/ to become more of a dumping
ground for random drivers. If there are several SoCs that have the
same requirement to do a particular thing, the logical step would
be to put them into a proper subsystem, with a well-defined interface
to dma-mapping and virtualization frameworks.

The other things we have in drivers/soc/ are usually either
soc_device drivers for identifying the system, or they export
interfaces used by soc specific drivers.

     Arnd

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2497 bytes --]

On Fri, Dec 16, 2022 at 09:04:20PM +0100, Arnd Bergmann wrote:
> On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
> > On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
> >> On Thu, Dec 15, 2022 at 01:40:30PM -0800, Palmer Dabbelt wrote:
> >>> Given that we already moved the SiFive one out it seems sane to just start
> >>> with the rest in drivers/soc/$VENDOR.  Looks like it was Christoph's idea to
> >>> do the move, so I'm adding him in case he's got an opinion (and also the SOC
> >>> alias, as that seems generally relevant).
> >>
> >> Well, it isn't an integral architecture feature, so it doesn't really
> >> beloing into arch.  Even the irqchip and timer drivers that are more
> >> less architectural are in drivers/ as they aren't really core
> >> architecture code.
> >
> > That makes sense to me, it just looks like the SiFive ccache is the only 
> > one that's in drivers/soc/$VENDOR, the rest are in arch.  It looks like 
> > mostly older ports that have vendor-specific cache files in arch (ie, 
> > arm has it but arm64 doesn't).  Maybe that's just because the newer 
> > architectures sorted out standard ISA interfaces for these and thus 
> > don't need the vendor-specific bits?  I think we're likely to end up 
> > with quite a few of these vendor-specific cache management schemes on 
> > RISC-V.
> >
> > I'm always happy to keep stuff out of arch/riscv, though.  So maybe we 
> > just buck the trend here and stick to drivers/soc/$VENDOR like we did 
> > for the first one?
> 
> I don't particularly like drivers/soc/ to become more of a dumping
> ground for random drivers. If there are several SoCs that have the
> same requirement to do a particular thing, the logical step would
> be to put them into a proper subsystem, with a well-defined interface
> to dma-mapping and virtualization frameworks.
> 
> The other things we have in drivers/soc/ are usually either
> soc_device drivers for identifying the system, or they export
> interfaces used by soc specific drivers.

Sounds like that's two "not in my back yard" votes from the maintainers
in question..
Doing drivers/cache would allow us to co-locate the RISC-V cache
management bits since it is not just going to be the ax45mp l2 driver
that will need to have them.

Would it be okay to put this driver in soc/andestech for now & then move
it, and the SiFive one, once we've got patches posted for cache
management with that?

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Lad, Prabhakar]

Hi Conor,

On Sat, Dec 17, 2022 at 10:52 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Fri, Dec 16, 2022 at 09:04:20PM +0100, Arnd Bergmann wrote:
> > On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
> > > On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
> > >> On Thu, Dec 15, 2022 at 01:40:30PM -0800, Palmer Dabbelt wrote:
> > >>> Given that we already moved the SiFive one out it seems sane to just start
> > >>> with the rest in drivers/soc/$VENDOR.  Looks like it was Christoph's idea to
> > >>> do the move, so I'm adding him in case he's got an opinion (and also the SOC
> > >>> alias, as that seems generally relevant).
> > >>
> > >> Well, it isn't an integral architecture feature, so it doesn't really
> > >> beloing into arch.  Even the irqchip and timer drivers that are more
> > >> less architectural are in drivers/ as they aren't really core
> > >> architecture code.
> > >
> > > That makes sense to me, it just looks like the SiFive ccache is the only
> > > one that's in drivers/soc/$VENDOR, the rest are in arch.  It looks like
> > > mostly older ports that have vendor-specific cache files in arch (ie,
> > > arm has it but arm64 doesn't).  Maybe that's just because the newer
> > > architectures sorted out standard ISA interfaces for these and thus
> > > don't need the vendor-specific bits?  I think we're likely to end up
> > > with quite a few of these vendor-specific cache management schemes on
> > > RISC-V.
> > >
> > > I'm always happy to keep stuff out of arch/riscv, though.  So maybe we
> > > just buck the trend here and stick to drivers/soc/$VENDOR like we did
> > > for the first one?
> >
> > I don't particularly like drivers/soc/ to become more of a dumping
> > ground for random drivers. If there are several SoCs that have the
> > same requirement to do a particular thing, the logical step would
> > be to put them into a proper subsystem, with a well-defined interface
> > to dma-mapping and virtualization frameworks.
> >
> > The other things we have in drivers/soc/ are usually either
> > soc_device drivers for identifying the system, or they export
> > interfaces used by soc specific drivers.
>
> Sounds like that's two "not in my back yard" votes from the maintainers
> in question..
> Doing drivers/cache would allow us to co-locate the RISC-V cache
> management bits since it is not just going to be the ax45mp l2 driver
> that will need to have them.
>
> Would it be okay to put this driver in soc/andestech for now & then move
> it, and the SiFive one, once we've got patches posted for cache
> management with that?
>
I think to start with it would be better if we place the Andes cache
driver in the proposed "drivers/cache" folder. We would avoid
unnecessary movement of code?

Cheers,
Prabhakar

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3271 bytes --]

On Mon, Dec 19, 2022 at 12:43:44PM +0000, Lad, Prabhakar wrote:
> Hi Conor,
> 
> On Sat, Dec 17, 2022 at 10:52 PM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Fri, Dec 16, 2022 at 09:04:20PM +0100, Arnd Bergmann wrote:
> > > On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
> > > > On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
> > > >> On Thu, Dec 15, 2022 at 01:40:30PM -0800, Palmer Dabbelt wrote:
> > > >>> Given that we already moved the SiFive one out it seems sane to just start
> > > >>> with the rest in drivers/soc/$VENDOR.  Looks like it was Christoph's idea to
> > > >>> do the move, so I'm adding him in case he's got an opinion (and also the SOC
> > > >>> alias, as that seems generally relevant).
> > > >>
> > > >> Well, it isn't an integral architecture feature, so it doesn't really
> > > >> beloing into arch.  Even the irqchip and timer drivers that are more
> > > >> less architectural are in drivers/ as they aren't really core
> > > >> architecture code.
> > > >
> > > > That makes sense to me, it just looks like the SiFive ccache is the only
> > > > one that's in drivers/soc/$VENDOR, the rest are in arch.  It looks like
> > > > mostly older ports that have vendor-specific cache files in arch (ie,
> > > > arm has it but arm64 doesn't).  Maybe that's just because the newer
> > > > architectures sorted out standard ISA interfaces for these and thus
> > > > don't need the vendor-specific bits?  I think we're likely to end up
> > > > with quite a few of these vendor-specific cache management schemes on
> > > > RISC-V.
> > > >
> > > > I'm always happy to keep stuff out of arch/riscv, though.  So maybe we
> > > > just buck the trend here and stick to drivers/soc/$VENDOR like we did
> > > > for the first one?
> > >
> > > I don't particularly like drivers/soc/ to become more of a dumping
> > > ground for random drivers. If there are several SoCs that have the
> > > same requirement to do a particular thing, the logical step would
> > > be to put them into a proper subsystem, with a well-defined interface
> > > to dma-mapping and virtualization frameworks.
> > >
> > > The other things we have in drivers/soc/ are usually either
> > > soc_device drivers for identifying the system, or they export
> > > interfaces used by soc specific drivers.
> >
> > Sounds like that's two "not in my back yard" votes from the maintainers
> > in question..
> > Doing drivers/cache would allow us to co-locate the RISC-V cache
> > management bits since it is not just going to be the ax45mp l2 driver
> > that will need to have them.
> >
> > Would it be okay to put this driver in soc/andestech for now & then move
> > it, and the SiFive one, once we've got patches posted for cache
> > management with that?
> >
> I think to start with it would be better if we place the Andes cache
> driver in the proposed "drivers/cache" folder. We would avoid
> unnecessary movement of code?

Uh, go for it I guess. I'll try to put out a few patches for moving the
sifive ccache driver & adding management stuff to it whenever I
inevitably get bored over Christmas.
The offer to make me responsible for applying patches for drivers/cache
still stands.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Arnd Bergmann]

On Sat, Dec 17, 2022, at 23:52, Conor Dooley wrote:
> On Fri, Dec 16, 2022 at 09:04:20PM +0100, Arnd Bergmann wrote:
>> On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
>> > On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
>>
>> I don't particularly like drivers/soc/ to become more of a dumping
>> ground for random drivers. If there are several SoCs that have the
>> same requirement to do a particular thing, the logical step would
>> be to put them into a proper subsystem, with a well-defined interface
>> to dma-mapping and virtualization frameworks.
>> 
>> The other things we have in drivers/soc/ are usually either
>> soc_device drivers for identifying the system, or they export
>> interfaces used by soc specific drivers.
>
> Sounds like that's two "not in my back yard" votes from the maintainers
> in question..
> Doing drivers/cache would allow us to co-locate the RISC-V cache
> management bits since it is not just going to be the ax45mp l2 driver
> that will need to have them.
>
> Would it be okay to put this driver in soc/andestech for now & then move
> it, and the SiFive one, once we've got patches posted for cache
> management with that?

I actually had a look at both of these drivers now and
found that they do entirely different things, so I would
revise what I had said earlier. Sorry for not having paid
enough attention at first.

The Sifive L2 cache driver handles an interrupt from the
cache controller that is trigger by data corruption
(corectable or uncorrectable). This is used as an
implementation detail of drivers/edac/sifive_edac.c
and could probably just be merged into that file.

The Andes cache driver in this series on the other hand
does not do EDAC at all but instead handles cache maintenance
for the dma-mapping interface by hooking into the
inline-asm implementation details of arch/riscv/mm/dma-noncoherent.c
as an errata fix. If we expect more nonstandard ways
to manage cache controllers for this, I think this
needs a proper interface in arch/riscv or drivers/cache.

This could be done the same way as arch/arm/include/asm/cacheflush.h
with CPU specific cache management callback pointers, but
can't really be a separate device driver without interacting
with low-level architecture code.

     Arnd

Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3287 bytes --]

On Thu, Dec 29, 2022 at 03:05:37PM +0100, Arnd Bergmann wrote:
> On Sat, Dec 17, 2022, at 23:52, Conor Dooley wrote:
> > On Fri, Dec 16, 2022 at 09:04:20PM +0100, Arnd Bergmann wrote:
> >> On Fri, Dec 16, 2022, at 17:32, Palmer Dabbelt wrote:
> >> > On Thu, 15 Dec 2022 23:02:58 PST (-0800), Christoph Hellwig wrote:
> >>
> >> I don't particularly like drivers/soc/ to become more of a dumping
> >> ground for random drivers. If there are several SoCs that have the
> >> same requirement to do a particular thing, the logical step would
> >> be to put them into a proper subsystem, with a well-defined interface
> >> to dma-mapping and virtualization frameworks.
> >> 
> >> The other things we have in drivers/soc/ are usually either
> >> soc_device drivers for identifying the system, or they export
> >> interfaces used by soc specific drivers.
> >
> > Sounds like that's two "not in my back yard" votes from the maintainers
> > in question..
> > Doing drivers/cache would allow us to co-locate the RISC-V cache
> > management bits since it is not just going to be the ax45mp l2 driver
> > that will need to have them.
> >
> > Would it be okay to put this driver in soc/andestech for now & then move
> > it, and the SiFive one, once we've got patches posted for cache
> > management with that?
> 
> I actually had a look at both of these drivers now and
> found that they do entirely different things, so I would
> revise what I had said earlier. Sorry for not having paid
> enough attention at first.

Eh, I wouldn't consider it to be your fault as, I at least, have been
ignoring this difference as...

> The Sifive L2 cache driver handles an interrupt from the
> cache controller that is trigger by data corruption
> (corectable or uncorrectable). This is used as an
> implementation detail of drivers/edac/sifive_edac.c
> and could probably just be merged into that file.
> 
> The Andes cache driver in this series on the other hand
> does not do EDAC at all but instead handles cache maintenance
> for the dma-mapping interface by hooking into the
> inline-asm implementation details of arch/riscv/mm/dma-noncoherent.c
> as an errata fix.

...we (Microchip) need to add similar cache maintenance to the SiFive
driver. Should have posted patches by now, but conferences + Christmas
have delayed that a bit.

> If we expect more nonstandard ways
> to manage cache controllers for this, I think this
> needs a proper interface in arch/riscv or drivers/cache.

The Zicbo* extensions for cache management arrived after people had
already been shipping SoCs that need stuff that isn't cache coherent.
On top of the two already mentioned, I am told there are two other
non-Zicbo* cache management solutions from SiFive alone - so I think it
is likely that we'll have variants here, unfortunately.

> This could be done the same way as arch/arm/include/asm/cacheflush.h
> with CPU specific cache management callback pointers, but
> can't really be a separate device driver without interacting
> with low-level architecture code.

I'll take a look at our patches again this week (they're 5.15 based, so
well out of date, vendor tree stuff). I'll try to whip up something
based on top of this series.

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[RFC v5.1 0/9] Generic function based cache management operations (was Re: [PATCH v5 6/6] soc: renesas: Add L2 cache management for RZ/Five SoC)

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

Hey all, hopefully the $subject arrived intact!

See here for prior discussion:
https://lore.kernel.org/linux-riscv/Y62nOqzyuUKqYDpq@spud/#R

Rather than continue that back and forth about where these drivers
belonged, I've gone and created a mock-up for what it would look like
if we picked drivers/cache.

IMO, doing drivers/cache for the detail of vendor behaviour makes sense
as it'll keep 'em out of arch/riscv while keeping the drivers
together.
I've taken v5 of Prabhakar's patchset & hacked at it a bit, so here
is a tyre-kicked (more) generic implementation of cache management stuff
via functions.
I initially went and moved both the ax45mp and ccache drivers to
drivers/cache & created a "subsystem" like interface, but quickly
realised it was all far too trivial to exist & was intrinsically tied to
the ALT_CMO_OP alternative setup on RISC-V.
I trashed that so, and instead copied the interface currently used by
riscv_set_cacheinfo_ops(), and created a corollary which registers cache
management operations. Or maintenance, IDC which.
Instead of having the vendor specific cache controller function in the
errata, the generic one is there instead, which then calls the one
registered by the vendor driver.
I figure that even if no other non-coherent implementation ends up making
it upstream, ax45mp is not going to be the only Andestech product that
has a "no-iocp" variant...

I sent this over the Prabhakar the other day & it works for him, so I am
now sending this here as RFC. I've not yet tested it on PolarFire SoC,
as we have to sort out some "fun" where archid etc are all zero.
We'll require some massaging before we can use the alternatives framework
as is, so that'll have to come at a later date. I included a patch, but
it's marked "DON'T APPLY" for a reason ;)

Notably, I have not fixed up any comments which were left against v5,
bar a rebase on top of riscv/for-next. I blindly "fixed" the pmem issue,
so that needs to be fixed properly for a v6, alongside the other couple
bits.

The other thing that is missing, and I could not think immediately of a
way to do it, is a way to add additional users of this type of CMO to
the alternative without having to add a specific entry for each vendor.
I had first thought about using archid of 0, but that runs into problems
pretty quickly as that "archid" is what we use for enabling CPU_FEATURE
stuff, like Zicbom itself.
Since archid uses the JEDEC IDs, perhaps we could squat on either a
continuation code (or some vendor that won't make a RISC-V CPU, like
Actel *cough*).
I didn't go through with mocking that up, as until we sort out PolarFire
SoC, there's only one user for this that I, at least, would like to have
supported properly.
So if anyone has ideas that are in any way less hacky than that, I am all
ears.

Thanks,
Conor.

Conor Dooley (2):
  cache,soc: Move SiFive CCache driver & create drivers/cache
  RISC-V: create a function based cache management interface

Daire McNamara (1):
  [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

Lad Prabhakar (6):
  riscv: asm: alternative-macros: Introduce ALTERNATIVE_3() macro
  riscv: asm: vendorid_list: Add Andes Technology to the vendors list
  riscv: errata: Add Andes alternative ports
  riscv: mm: dma-noncoherent: Pass direction and operation to
    ALT_CMO_OP()
  dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation
    for L2 cache controller
  soc: renesas: Add L2 cache management for RZ/Five SoC

 .../cache/andestech,ax45mp-cache.yaml         |  81 ++++++
 MAINTAINERS                                   |  15 +-
 arch/riscv/Kconfig.erratas                    |  26 ++
 arch/riscv/errata/Makefile                    |   1 +
 arch/riscv/errata/andes/Makefile              |   1 +
 arch/riscv/errata/andes/errata.c              |  93 +++++++
 arch/riscv/include/asm/alternative-macros.h   |  46 +++-
 arch/riscv/include/asm/alternative.h          |   3 +
 arch/riscv/include/asm/cacheflush.h           |  23 ++
 arch/riscv/include/asm/errata_list.h          |  41 ++-
 arch/riscv/include/asm/vendorid_list.h        |   1 +
 arch/riscv/kernel/alternative.c               |   5 +
 arch/riscv/mm/dma-noncoherent.c               |  36 ++-
 arch/riscv/mm/pmem.c                          |   4 +-
 drivers/Kconfig                               |   2 +
 drivers/Makefile                              |   2 +
 drivers/cache/Kconfig                         |  25 ++
 drivers/{soc/sifive => cache}/Makefile        |   2 +
 drivers/cache/ax45mp_cache.c                  | 253 ++++++++++++++++++
 drivers/{soc/sifive => cache}/sifive_ccache.c |  47 +++-
 drivers/edac/sifive_edac.c                    |   2 +-
 drivers/soc/Kconfig                           |   1 -
 drivers/soc/Makefile                          |   1 -
 drivers/soc/renesas/Kconfig                   |   4 +
 drivers/soc/sifive/Kconfig                    |  10 -
 include/{soc/sifive => cache}/sifive_ccache.h |   0
 26 files changed, 684 insertions(+), 41 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
 create mode 100644 arch/riscv/errata/andes/Makefile
 create mode 100644 arch/riscv/errata/andes/errata.c
 create mode 100644 drivers/cache/Kconfig
 rename drivers/{soc/sifive => cache}/Makefile (62%)
 create mode 100644 drivers/cache/ax45mp_cache.c
 rename drivers/{soc/sifive => cache}/sifive_ccache.c (86%)
 delete mode 100644 drivers/soc/sifive/Kconfig
 rename include/{soc/sifive => cache}/sifive_ccache.h (100%)


base-commit: b07de94d4501c61a3015cb0035227e449c51d87e
-- 
2.39.0

[RFC v5.1 1/9] riscv: asm: alternative-macros: Introduce ALTERNATIVE_3() macro

[Conor Dooley]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Introduce ALTERNATIVE_3() macro.

A vendor wants to replace an old_content, but another vendor has used
ALTERNATIVE_2() to patch its customized content at the same location.
In this case, this vendor can use macro ALTERNATIVE_3() and then replace
ALTERNATIVE_2() with ALTERNATIVE_3() to append its customized content.

While at it update comment above ALTERNATIVE_2() macro and make it generic
so that the comment holds good for any new addition of ALTERNATIVE_X()
macros.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/alternative-macros.h | 46 ++++++++++++++++++---
 1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
index 7226e2462584..a5b4691520da 100644
--- a/arch/riscv/include/asm/alternative-macros.h
+++ b/arch/riscv/include/asm/alternative-macros.h
@@ -50,8 +50,17 @@
 	ALT_NEW_CONTENT \vendor_id_2, \errata_id_2, \enable_2, \new_c_2
 .endm
 
+.macro ALTERNATIVE_CFG_3 old_c, new_c_1, vendor_id_1, errata_id_1, enable_1,	\
+				new_c_2, vendor_id_2, errata_id_2, enable_2,	\
+				new_c_3, vendor_id_3, errata_id_3, enable_3
+       ALTERNATIVE_CFG_2 \old_c, \new_c_1, \vendor_id_1, \errata_id_1, \enable_1,	\
+                                 \new_c_2, \vendor_id_2, \errata_id_2, \enable_2
+       ALT_NEW_CONTENT \vendor_id_3, \errata_id_3, \enable_3, \new_c_3
+.endm
+
 #define __ALTERNATIVE_CFG(...)		ALTERNATIVE_CFG __VA_ARGS__
 #define __ALTERNATIVE_CFG_2(...)	ALTERNATIVE_CFG_2 __VA_ARGS__
+#define __ALTERNATIVE_CFG_3(...)	ALTERNATIVE_CFG_3 __VA_ARGS__
 
 #else /* !__ASSEMBLY__ */
 
@@ -98,6 +107,13 @@
 	__ALTERNATIVE_CFG(old_c, new_c_1, vendor_id_1, errata_id_1, enable_1)	\
 	ALT_NEW_CONTENT(vendor_id_2, errata_id_2, enable_2, new_c_2)
 
+#define __ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, errata_id_1, enable_1,	\
+				   new_c_2, vendor_id_2, errata_id_2, enable_2,	\
+				   new_c_3, vendor_id_3, errata_id_3, enable_3)	\
+	__ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, errata_id_1, enable_1,	\
+                                   new_c_2, vendor_id_2, errata_id_2, enable_2)	\
+	ALT_NEW_CONTENT(vendor_id_3, errata_id_3, enable_3, new_c_3)
+
 #endif /* __ASSEMBLY__ */
 
 #define _ALTERNATIVE_CFG(old_c, new_c, vendor_id, errata_id, CONFIG_k)	\
@@ -108,6 +124,13 @@
 	__ALTERNATIVE_CFG_2(old_c, new_c_1, vendor_id_1, errata_id_1, IS_ENABLED(CONFIG_k_1),	\
 				   new_c_2, vendor_id_2, errata_id_2, IS_ENABLED(CONFIG_k_2))
 
+#define _ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, errata_id_1, CONFIG_k_1,		\
+				  new_c_2, vendor_id_2, errata_id_2, CONFIG_k_2,		\
+				  new_c_3, vendor_id_3, errata_id_3, CONFIG_k_3)		\
+	__ALTERNATIVE_CFG_3(old_c, new_c_1, vendor_id_1, errata_id_1, IS_ENABLED(CONFIG_k_1),	\
+				   new_c_2, vendor_id_2, errata_id_2, IS_ENABLED(CONFIG_k_2),	\
+				   new_c_3, vendor_id_3, errata_id_3, IS_ENABLED(CONFIG_k_3))
+
 #else /* CONFIG_RISCV_ALTERNATIVE */
 #ifdef __ASSEMBLY__
 
@@ -152,15 +175,28 @@
 	_ALTERNATIVE_CFG(old_content, new_content, vendor_id, errata_id, CONFIG_k)
 
 /*
- * A vendor wants to replace an old_content, but another vendor has used
- * ALTERNATIVE() to patch its customized content at the same location. In
- * this case, this vendor can create a new macro ALTERNATIVE_2() based
- * on the following sample code and then replace ALTERNATIVE() with
- * ALTERNATIVE_2() to append its customized content.
+ * ALTERNATIVE_x macros allow providing multiple replacement options
+ * for an ALTERNATIVE code section. This is helpful if multiple
+ * implementation variants for the same functionality exist for
+ * different cpu cores.
+ *
+ * Usage:
+ *   ALTERNATIVE_x(old_content,
+ *      new_content1, vendor_id1, errata_id1, CONFIG_k1,
+ *      new_content2, vendor_id2, errata_id2, CONFIG_k2,
+ *      ...
+ *      new_contentx, vendor_idx, errata_idx, CONFIG_kx)
  */
 #define ALTERNATIVE_2(old_content, new_content_1, vendor_id_1, errata_id_1, CONFIG_k_1,		\
 				   new_content_2, vendor_id_2, errata_id_2, CONFIG_k_2)		\
 	_ALTERNATIVE_CFG_2(old_content, new_content_1, vendor_id_1, errata_id_1, CONFIG_k_1,	\
 					new_content_2, vendor_id_2, errata_id_2, CONFIG_k_2)
 
+#define ALTERNATIVE_3(old_content, new_content_1, vendor_id_1, errata_id_1, CONFIG_k_1,		\
+				   new_content_2, vendor_id_2, errata_id_2, CONFIG_k_2,		\
+				   new_content_3, vendor_id_3, errata_id_3, CONFIG_k_3)		\
+       _ALTERNATIVE_CFG_3(old_content, new_content_1, vendor_id_1, errata_id_1, CONFIG_k_1,	\
+                                       new_content_2, vendor_id_2, errata_id_2, CONFIG_k_2,	\
+                                       new_content_3, vendor_id_3, errata_id_3, CONFIG_k_3)
+
 #endif
-- 
2.39.0

[RFC v5.1 2/9] riscv: asm: vendorid_list: Add Andes Technology to the vendors list

[Conor Dooley]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add Andes Technology to the vendors list.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/vendorid_list.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
index cb89af3f0704..e55407ace0c3 100644
--- a/arch/riscv/include/asm/vendorid_list.h
+++ b/arch/riscv/include/asm/vendorid_list.h
@@ -5,6 +5,7 @@
 #ifndef ASM_VENDOR_LIST_H
 #define ASM_VENDOR_LIST_H
 
+#define ANDESTECH_VENDOR_ID	0x31e
 #define SIFIVE_VENDOR_ID	0x489
 #define THEAD_VENDOR_ID		0x5b7
 
-- 
2.39.0

[RFC v5.1 3/9] riscv: errata: Add Andes alternative ports

[Conor Dooley]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add required ports of the Alternative scheme for Andes CPU cores.

I/O Coherence Port (IOCP) provides an AXI interface for connecting external
non-caching masters, such as DMA controllers. IOCP is a specification
option and is disabled on the Renesas RZ/Five SoC due to this reason cache
management needs a software workaround.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/Kconfig.erratas           | 22 +++++++
 arch/riscv/errata/Makefile           |  1 +
 arch/riscv/errata/andes/Makefile     |  1 +
 arch/riscv/errata/andes/errata.c     | 93 ++++++++++++++++++++++++++++
 arch/riscv/include/asm/alternative.h |  3 +
 arch/riscv/include/asm/errata_list.h |  5 ++
 arch/riscv/kernel/alternative.c      |  5 ++
 7 files changed, 130 insertions(+)
 create mode 100644 arch/riscv/errata/andes/Makefile
 create mode 100644 arch/riscv/errata/andes/errata.c

diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
index 69621ae6d647..f0f0c1abd52b 100644
--- a/arch/riscv/Kconfig.erratas
+++ b/arch/riscv/Kconfig.erratas
@@ -1,5 +1,27 @@
 menu "CPU errata selection"
 
+config ERRATA_ANDES
+	bool "Andes AX45MP errata"
+	depends on !XIP_KERNEL
+	select RISCV_ALTERNATIVE
+	help
+	  All Andes errata Kconfig depend on this Kconfig. Disabling
+	  this Kconfig will disable all Andes errata. Please say "Y"
+	  here if your platform uses Andes CPU cores.
+
+	  Otherwise, please say "N" here to avoid unnecessary overhead.
+
+config ERRATA_ANDES_CMO
+	bool "Apply Andes cache management errata"
+	depends on ERRATA_ANDES && MMU && ARCH_R9A07G043
+	select RISCV_DMA_NONCOHERENT
+	default y
+	help
+	  This will apply the cache management errata to handle the
+	  non-standard handling on non-coherent operations on Andes cores.
+
+	  If you don't know what to do here, say "Y".
+
 config ERRATA_SIFIVE
 	bool "SiFive errata"
 	depends on !XIP_KERNEL
diff --git a/arch/riscv/errata/Makefile b/arch/riscv/errata/Makefile
index a1055965fbee..6f1c693af92d 100644
--- a/arch/riscv/errata/Makefile
+++ b/arch/riscv/errata/Makefile
@@ -1,2 +1,3 @@
+obj-$(CONFIG_ERRATA_ANDES) += andes/
 obj-$(CONFIG_ERRATA_SIFIVE) += sifive/
 obj-$(CONFIG_ERRATA_THEAD) += thead/
diff --git a/arch/riscv/errata/andes/Makefile b/arch/riscv/errata/andes/Makefile
new file mode 100644
index 000000000000..2d644e19caef
--- /dev/null
+++ b/arch/riscv/errata/andes/Makefile
@@ -0,0 +1 @@
+obj-y += errata.o
diff --git a/arch/riscv/errata/andes/errata.c b/arch/riscv/errata/andes/errata.c
new file mode 100644
index 000000000000..3d04f15df8d5
--- /dev/null
+++ b/arch/riscv/errata/andes/errata.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Erratas to be applied for Andes CPU cores
+ *
+ *  Copyright (C) 2022 Renesas Electronics Corporation.
+ *
+ * Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include <asm/alternative.h>
+#include <asm/cacheflush.h>
+#include <asm/errata_list.h>
+#include <asm/patch.h>
+#include <asm/sbi.h>
+#include <asm/vendorid_list.h>
+
+#define ANDESTECH_AX45MP_MARCHID	0x8000000000008a45UL
+#define ANDESTECH_AX45MP_MIMPID		0x500UL
+#define ANDESTECH_SBI_EXT_ANDES		0x0900031E
+
+#define RZFIVE_SBI_EXT_IOCP_SW_WORKAROUND	0
+
+static long ax45mp_iocp_sw_workaround(void)
+{
+	struct sbiret ret;
+
+	ret = sbi_ecall(ANDESTECH_SBI_EXT_ANDES, RZFIVE_SBI_EXT_IOCP_SW_WORKAROUND,
+			0, 0, 0, 0, 0, 0);
+
+	return ret.error ? 0 : ret.value;
+}
+
+static bool errata_probe_iocp(unsigned int stage, unsigned long arch_id, unsigned long impid)
+{
+	if (!IS_ENABLED(CONFIG_ERRATA_ANDES_CMO))
+		return false;
+
+	if (arch_id != ANDESTECH_AX45MP_MARCHID || impid != ANDESTECH_AX45MP_MIMPID)
+		return false;
+
+	if (!ax45mp_iocp_sw_workaround())
+		return false;
+
+	/* Set this just to make core cbo code happy */
+	riscv_cbom_block_size = 1;
+	riscv_noncoherent_supported();
+
+	return true;
+}
+
+static u32 andes_errata_probe(unsigned int stage, unsigned long archid, unsigned long impid)
+{
+	u32 cpu_req_errata = 0;
+
+	/*
+	 * In the absence of the I/O Coherency Port, access to certain peripherals
+	 * requires vendor specific DMA handling.
+	 */
+	if (errata_probe_iocp(stage, archid, impid))
+		cpu_req_errata |= BIT(ERRATA_ANDESTECH_NO_IOCP);
+
+	return cpu_req_errata;
+}
+
+void __init_or_module andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
+					      unsigned long archid, unsigned long impid,
+					      unsigned int stage)
+{
+	u32 cpu_req_errata = andes_errata_probe(stage, archid, impid);
+	struct alt_entry *alt;
+	u32 tmp;
+
+	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+		return;
+
+	for (alt = begin; alt < end; alt++) {
+		if (alt->vendor_id != ANDESTECH_VENDOR_ID)
+			continue;
+		if (alt->errata_id >= ERRATA_ANDESTECH_NUMBER)
+			continue;
+
+		tmp = BIT(alt->errata_id);
+		if (cpu_req_errata & tmp) {
+			patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+
+			riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
+						      alt->old_ptr - alt->alt_ptr);
+		}
+	}
+}
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index 1bd4027d34ca..e3a8e603eb5a 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -43,6 +43,9 @@ struct errata_checkfunc_id {
 	bool (*func)(struct alt_entry *alt);
 };
 
+void andes_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
+			     unsigned long archid, unsigned long impid,
+			     unsigned int stage);
 void sifive_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
 			      unsigned long archid, unsigned long impid,
 			      unsigned int stage);
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 4180312d2a70..2ba7e6e74540 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -9,6 +9,11 @@
 #include <asm/csr.h>
 #include <asm/vendorid_list.h>
 
+#ifdef CONFIG_ERRATA_ANDES
+#define ERRATA_ANDESTECH_NO_IOCP	0
+#define ERRATA_ANDESTECH_NUMBER		1
+#endif
+
 #ifdef CONFIG_ERRATA_SIFIVE
 #define	ERRATA_SIFIVE_CIP_453 0
 #define	ERRATA_SIFIVE_CIP_1200 1
diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index 6212ea0eed72..d7027a977ee6 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -40,6 +40,11 @@ static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_inf
 #endif
 
 	switch (cpu_mfr_info->vendor_id) {
+#ifdef CONFIG_ERRATA_ANDES
+	case ANDESTECH_VENDOR_ID:
+		cpu_mfr_info->patch_func = andes_errata_patch_func;
+		break;
+#endif
 #ifdef CONFIG_ERRATA_SIFIVE
 	case SIFIVE_VENDOR_ID:
 		cpu_mfr_info->patch_func = sifive_errata_patch_func;
-- 
2.39.0

[RFC v5.1 4/9] riscv: mm: dma-noncoherent: Pass direction and operation to ALT_CMO_OP()

[Conor Dooley]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Pass direction and operation to ALT_CMO_OP() macro.

Vendors might want to perform different operations based on the direction
and callbacks (arch_sync_dma_for_device/arch_sync_dma_for_cpu/
arch_dma_prep_coherent) so to handle such cases pass the direction and
operation to ALT_CMO_OP() macro. This is in preparation for adding errata
for the Andes CPU core.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[Conor: blindy & incorrectly "fixed" pmem compilation. I've not even
tried to use the defines.]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/cacheflush.h  |  4 ++++
 arch/riscv/include/asm/errata_list.h |  8 ++++++--
 arch/riscv/mm/dma-noncoherent.c      | 15 ++++++++++-----
 arch/riscv/mm/pmem.c                 |  4 ++--
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
index 03e3b95ae6da..e22019668b9e 100644
--- a/arch/riscv/include/asm/cacheflush.h
+++ b/arch/riscv/include/asm/cacheflush.h
@@ -8,6 +8,10 @@
 
 #include <linux/mm.h>
 
+#define NON_COHERENT_SYNC_DMA_FOR_DEVICE	0
+#define NON_COHERENT_SYNC_DMA_FOR_CPU		1
+#define NON_COHERENT_DMA_PREP			2
+
 static inline void local_flush_icache_all(void)
 {
 	asm volatile ("fence.i" ::: "memory");
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 2ba7e6e74540..48e899a8e7a9 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -124,7 +124,7 @@ asm volatile(ALTERNATIVE(						\
 #define THEAD_flush_A0	".long 0x0275000b"
 #define THEAD_SYNC_S	".long 0x0190000b"
 
-#define ALT_CMO_OP(_op, _start, _size, _cachesize)			\
+#define ALT_CMO_OP(_op, _start, _size, _cachesize, _dir, _ops)		\
 asm volatile(ALTERNATIVE_2(						\
 	__nops(6),							\
 	"mv a0, %1\n\t"							\
@@ -146,7 +146,11 @@ asm volatile(ALTERNATIVE_2(						\
 			ERRATA_THEAD_CMO, CONFIG_ERRATA_THEAD_CMO)	\
 	: : "r"(_cachesize),						\
 	    "r"((unsigned long)(_start) & ~((_cachesize) - 1UL)),	\
-	    "r"((unsigned long)(_start) + (_size))			\
+	    "r"((unsigned long)(_start) + (_size)),			\
+	    "r"((unsigned long)(_start)),				\
+	    "r"((unsigned long)(_size)),				\
+	    "r"((unsigned long)(_dir)),					\
+	    "r"((unsigned long)(_ops))					\
 	: "a0")
 
 #define THEAD_C9XX_RV_IRQ_PMU			17
diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
index d919efab6eba..e2b82034f504 100644
--- a/arch/riscv/mm/dma-noncoherent.c
+++ b/arch/riscv/mm/dma-noncoherent.c
@@ -19,13 +19,16 @@ void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
 
 	switch (dir) {
 	case DMA_TO_DEVICE:
-		ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
+		ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size,
+			   dir, NON_COHERENT_SYNC_DMA_FOR_DEVICE);
 		break;
 	case DMA_FROM_DEVICE:
-		ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size);
+		ALT_CMO_OP(clean, vaddr, size, riscv_cbom_block_size,
+			   dir, NON_COHERENT_SYNC_DMA_FOR_DEVICE);
 		break;
 	case DMA_BIDIRECTIONAL:
-		ALT_CMO_OP(flush, vaddr, size, riscv_cbom_block_size);
+		ALT_CMO_OP(flush, vaddr, size, riscv_cbom_block_size,
+			   dir, NON_COHERENT_SYNC_DMA_FOR_DEVICE);
 		break;
 	default:
 		break;
@@ -42,7 +45,8 @@ void arch_sync_dma_for_cpu(phys_addr_t paddr, size_t size,
 		break;
 	case DMA_FROM_DEVICE:
 	case DMA_BIDIRECTIONAL:
-		ALT_CMO_OP(flush, vaddr, size, riscv_cbom_block_size);
+		ALT_CMO_OP(flush, vaddr, size, riscv_cbom_block_size,
+			   dir, NON_COHERENT_SYNC_DMA_FOR_CPU);
 		break;
 	default:
 		break;
@@ -53,7 +57,8 @@ void arch_dma_prep_coherent(struct page *page, size_t size)
 {
 	void *flush_addr = page_address(page);
 
-	ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size);
+	ALT_CMO_OP(flush, flush_addr, size, riscv_cbom_block_size,
+		   0, NON_COHERENT_DMA_PREP);
 }
 
 void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
diff --git a/arch/riscv/mm/pmem.c b/arch/riscv/mm/pmem.c
index 089df92ae876..4233e28d8611 100644
--- a/arch/riscv/mm/pmem.c
+++ b/arch/riscv/mm/pmem.c
@@ -10,12 +10,12 @@
 
 void arch_wb_cache_pmem(void *addr, size_t size)
 {
-	ALT_CMO_OP(clean, addr, size, riscv_cbom_block_size);
+	ALT_CMO_OP(clean, addr, size, riscv_cbom_block_size, 0, 0);
 }
 EXPORT_SYMBOL_GPL(arch_wb_cache_pmem);
 
 void arch_invalidate_pmem(void *addr, size_t size)
 {
-	ALT_CMO_OP(inval, addr, size, riscv_cbom_block_size);
+	ALT_CMO_OP(inval, addr, size, riscv_cbom_block_size, 0, 0);
 }
 EXPORT_SYMBOL_GPL(arch_invalidate_pmem);
-- 
2.39.0

[RFC v5.1 5/9] dt-bindings: cache: r9a07g043f-l2-cache: Add DT binding documentation for L2 cache controller

[Conor Dooley]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Add DT binding documentation for L2 cache controller found on RZ/Five SoC.

The Renesas RZ/Five microprocessor includes a RISC-V CPU Core (AX45MP
Single) from Andes. The AX45MP core has an L2 cache controller, this patch
describes the L2 cache block.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../cache/andestech,ax45mp-cache.yaml         | 81 +++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml

diff --git a/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
new file mode 100644
index 000000000000..9f0be4835ad7
--- /dev/null
+++ b/Documentation/devicetree/bindings/cache/andestech,ax45mp-cache.yaml
@@ -0,0 +1,81 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright (C) 2022 Renesas Electronics Corp.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/cache/andestech,ax45mp-cache.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Andestech AX45MP L2 Cache Controller
+
+maintainers:
+  - Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
+
+description:
+  A level-2 cache (L2C) is used to improve the system performance by providing
+  a large amount of cache line entries and reasonable access delays. The L2C
+  is shared between cores, and a non-inclusive non-exclusive policy is used.
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - andestech,ax45mp-cache
+
+  required:
+    - compatible
+
+properties:
+  compatible:
+    items:
+      - const: andestech,ax45mp-cache
+      - const: cache
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  cache-line-size:
+    const: 64
+
+  cache-level:
+    const: 2
+
+  cache-sets:
+    const: 1024
+
+  cache-size:
+    enum: [131072, 262144, 524288, 1048576, 2097152]
+
+  cache-unified: true
+
+  next-level-cache: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - cache-line-size
+  - cache-level
+  - cache-sets
+  - cache-size
+  - cache-unified
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    cache-controller@2010000 {
+        compatible = "andestech,ax45mp-cache", "cache";
+        reg = <0x13400000 0x100000>;
+        interrupts = <508 IRQ_TYPE_LEVEL_HIGH>;
+        cache-line-size = <64>;
+        cache-level = <2>;
+        cache-sets = <1024>;
+        cache-size = <262144>;
+        cache-unified;
+    };
-- 
2.39.0

[RFC v5.1 6/9] cache,soc: Move SiFive CCache driver & create drivers/cache

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The Zicbo* set of extensions for cache maintenance arrived too late &
several SoCs exist without them that require non-coherent DMA.
As things stand, the StarFive JH7100, Microchip PolarFire SoC & Renesas
RZ/Five all require cache maintenance and lack instructions for this
purpose.
Create a subsystem for cache drivers so that vendor specific behaviour
can be isolated from arch code, but keep the interfaces etc consistent.
Move the existing SiFive CCache driver to create drivers/cache.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 MAINTAINERS                                   | 15 ++++++++-------
 drivers/Kconfig                               |  2 ++
 drivers/Makefile                              |  2 ++
 drivers/{soc/sifive => cache}/Kconfig         |  8 +++++++-
 drivers/{soc/sifive => cache}/Makefile        |  0
 drivers/{soc/sifive => cache}/sifive_ccache.c |  2 +-
 drivers/edac/sifive_edac.c                    |  2 +-
 drivers/soc/Kconfig                           |  1 -
 drivers/soc/Makefile                          |  1 -
 include/{soc/sifive => cache}/sifive_ccache.h |  0
 10 files changed, 21 insertions(+), 12 deletions(-)
 rename drivers/{soc/sifive => cache}/Kconfig (56%)
 rename drivers/{soc/sifive => cache}/Makefile (100%)
 rename drivers/{soc/sifive => cache}/sifive_ccache.c (99%)
 rename include/{soc/sifive => cache}/sifive_ccache.h (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index f61eb221415b..4437e96a657b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19054,13 +19054,6 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
 F:	drivers/dma/sf-pdma/
 
-SIFIVE SOC DRIVERS
-M:	Conor Dooley <conor@kernel.org>
-L:	linux-riscv@lists.infradead.org
-S:	Maintained
-T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
-F:	drivers/soc/sifive/
-
 SILEAD TOUCHSCREEN DRIVER
 M:	Hans de Goede <hdegoede@redhat.com>
 L:	linux-input@vger.kernel.org
@@ -19873,6 +19866,14 @@ S:	Supported
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
 F:	drivers/staging/
 
+STANDALONE CACHE CONTROLLER DRIVERS
+M:	Conor Dooley <conor@kernel.org>
+L:	linux-riscv@lists.infradead.org
+S:	Maintained
+T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
+F:	drivers/cache
+F:	include/cache
+
 STARFIRE/DURALAN NETWORK DRIVER
 M:	Ion Badulescu <ionut@badula.org>
 S:	Odd Fixes
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 968bd0a6fd78..e592ba5276ae 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -241,4 +241,6 @@ source "drivers/peci/Kconfig"
 
 source "drivers/hte/Kconfig"
 
+source "drivers/cache/Kconfig"
+
 endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index bdf1c66141c9..6ff60cf21823 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -38,6 +38,8 @@ obj-y				+= clk/
 # really early.
 obj-$(CONFIG_DMADEVICES)	+= dma/
 
+obj-y				+= cache/
+
 # SOC specific infrastructure drivers.
 obj-y				+= soc/
 
diff --git a/drivers/soc/sifive/Kconfig b/drivers/cache/Kconfig
similarity index 56%
rename from drivers/soc/sifive/Kconfig
rename to drivers/cache/Kconfig
index ed4c571f8771..bc852f005c10 100644
--- a/drivers/soc/sifive/Kconfig
+++ b/drivers/cache/Kconfig
@@ -1,9 +1,15 @@
 # SPDX-License-Identifier: GPL-2.0
 
-if SOC_SIFIVE
+menuconfig CACHE_CONTROLLER
+	bool "Cache controller driver support"
+	default y if RISCV
+
+if CACHE_CONTROLLER
 
 config SIFIVE_CCACHE
 	bool "Sifive Composable Cache controller"
+	depends on RISCV
+	default y
 	help
 	  Support for the composable cache controller on SiFive platforms.
 
diff --git a/drivers/soc/sifive/Makefile b/drivers/cache/Makefile
similarity index 100%
rename from drivers/soc/sifive/Makefile
rename to drivers/cache/Makefile
diff --git a/drivers/soc/sifive/sifive_ccache.c b/drivers/cache/sifive_ccache.c
similarity index 99%
rename from drivers/soc/sifive/sifive_ccache.c
rename to drivers/cache/sifive_ccache.c
index 3684f5b40a80..47e7d6557f85 100644
--- a/drivers/soc/sifive/sifive_ccache.c
+++ b/drivers/cache/sifive_ccache.c
@@ -15,7 +15,7 @@
 #include <linux/device.h>
 #include <linux/bitfield.h>
 #include <asm/cacheinfo.h>
-#include <soc/sifive/sifive_ccache.h>
+#include <cache/sifive_ccache.h>
 
 #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
 #define SIFIVE_CCACHE_DIRECCFIX_HIGH 0x104
diff --git a/drivers/edac/sifive_edac.c b/drivers/edac/sifive_edac.c
index b844e2626fd5..9276839d58c2 100644
--- a/drivers/edac/sifive_edac.c
+++ b/drivers/edac/sifive_edac.c
@@ -10,7 +10,7 @@
 #include <linux/edac.h>
 #include <linux/platform_device.h>
 #include "edac_module.h"
-#include <soc/sifive/sifive_ccache.h>
+#include <cache/sifive_ccache.h>
 
 #define DRVNAME "sifive_edac"
 
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index 5dbb09f843f7..240455963565 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -21,7 +21,6 @@ source "drivers/soc/qcom/Kconfig"
 source "drivers/soc/renesas/Kconfig"
 source "drivers/soc/rockchip/Kconfig"
 source "drivers/soc/samsung/Kconfig"
-source "drivers/soc/sifive/Kconfig"
 source "drivers/soc/sunxi/Kconfig"
 source "drivers/soc/tegra/Kconfig"
 source "drivers/soc/ti/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index fff513bd522d..9ed2e27bbb64 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -27,7 +27,6 @@ obj-y				+= qcom/
 obj-y				+= renesas/
 obj-y				+= rockchip/
 obj-$(CONFIG_SOC_SAMSUNG)	+= samsung/
-obj-$(CONFIG_SOC_SIFIVE)	+= sifive/
 obj-y				+= sunxi/
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra/
 obj-y				+= ti/
diff --git a/include/soc/sifive/sifive_ccache.h b/include/cache/sifive_ccache.h
similarity index 100%
rename from include/soc/sifive/sifive_ccache.h
rename to include/cache/sifive_ccache.h
-- 
2.39.0

[RFC v5.1 7/9] RISC-V: create a function based cache management interface

[Conor Dooley]

From: Conor Dooley <conor.dooley@microchip.com>

The Zicbo* set of extensions for cache maintenance arrived too late &
several SoCs exist without them that require non-coherent DMA.
As things stand, the StarFive JH7100, Microchip PolarFire SoC & Renesas
RZ/Five all require cache maintenance and lack instructions for this
purpose.
Similar to the interface already used by the SiFive CCache driver to add
cacheinfo_ops, create an interface for registering cache management
functions for a given cache controller.

Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
Yes, I made the cmo_patchfunc() __maybe_unused to escape LKP complaints.
The other option that Prabhakar mentioned was having explicit functions
for each of the operations, in which case I cmo_patchfunc() would go
away. I don't particularly like the name of that function, so any
suggestions there would be great!
---
 arch/riscv/Kconfig.erratas          |  4 ++++
 arch/riscv/include/asm/cacheflush.h | 19 +++++++++++++++++++
 arch/riscv/mm/dma-noncoherent.c     | 21 +++++++++++++++++++++
 3 files changed, 44 insertions(+)

diff --git a/arch/riscv/Kconfig.erratas b/arch/riscv/Kconfig.erratas
index f0f0c1abd52b..b8542e6e8f18 100644
--- a/arch/riscv/Kconfig.erratas
+++ b/arch/riscv/Kconfig.erratas
@@ -1,5 +1,8 @@
 menu "CPU errata selection"
 
+config ERRATA_CMO_FUNC
+	bool
+
 config ERRATA_ANDES
 	bool "Andes AX45MP errata"
 	depends on !XIP_KERNEL
@@ -14,6 +17,7 @@ config ERRATA_ANDES
 config ERRATA_ANDES_CMO
 	bool "Apply Andes cache management errata"
 	depends on ERRATA_ANDES && MMU && ARCH_R9A07G043
+	select ERRATA_CMO_FUNC
 	select RISCV_DMA_NONCOHERENT
 	default y
 	help
diff --git a/arch/riscv/include/asm/cacheflush.h b/arch/riscv/include/asm/cacheflush.h
index e22019668b9e..795205ec2028 100644
--- a/arch/riscv/include/asm/cacheflush.h
+++ b/arch/riscv/include/asm/cacheflush.h
@@ -62,6 +62,25 @@ void riscv_noncoherent_supported(void);
 static inline void riscv_noncoherent_supported(void) {}
 #endif
 
+struct riscv_cache_maint_ops {
+	void (*cmo_patchfunc) (unsigned int cache_size, void *vaddr,
+			       size_t size, int dir, int ops);
+};
+
+#ifdef CONFIG_RISCV_DMA_NONCOHERENT
+void riscv_set_cache_maint_ops(struct riscv_cache_maint_ops *ops);
+#else
+static void riscv_set_cache_maint_ops(struct riscv_cache_maint_ops *ops) {}
+#endif
+
+#ifdef CONFIG_ERRATA_CMO_FUNC
+asmlinkage void cmo_patchfunc(unsigned int cache_size, void *vaddr, size_t size,
+			      int dir, int ops);
+#else
+__maybe_unused static void cmo_patchfunc(unsigned int cache_size, void *vaddr,
+					 size_t size, int dir, int ops) {}
+#endif
+
 /*
  * Bits in sys_riscv_flush_icache()'s flags argument.
  */
diff --git a/arch/riscv/mm/dma-noncoherent.c b/arch/riscv/mm/dma-noncoherent.c
index e2b82034f504..2f4f147ea0b9 100644
--- a/arch/riscv/mm/dma-noncoherent.c
+++ b/arch/riscv/mm/dma-noncoherent.c
@@ -83,3 +83,24 @@ void riscv_noncoherent_supported(void)
 	     "Non-coherent DMA support enabled without a block size\n");
 	noncoherent_supported = true;
 }
+
+static struct riscv_cache_maint_ops *rv_cache_maint_ops;
+static DEFINE_STATIC_KEY_FALSE(cmo_patchfunc_present);
+
+void riscv_set_cache_maint_ops(struct riscv_cache_maint_ops *ops)
+{
+	rv_cache_maint_ops = ops;
+	static_branch_enable(&cmo_patchfunc_present);
+}
+EXPORT_SYMBOL_GPL(riscv_set_cache_maint_ops);
+
+#ifdef CONFIG_ERRATA_CMO_FUNC
+asmlinkage void cmo_patchfunc(unsigned int cache_size, void *vaddr, size_t size,
+			      int dir, int ops)
+{
+	if (!static_branch_unlikely(&cmo_patchfunc_present))
+		return;
+
+	rv_cache_maint_ops->cmo_patchfunc(cache_size, vaddr, size, dir, ops);
+}
+#endif
-- 
2.39.0

[RFC v5.1 8/9] soc: renesas: Add L2 cache management for RZ/Five SoC

[Conor Dooley]

From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I/O Coherence Port (IOCP) provides an AXI interface for connecting
external non-caching masters, such as DMA controllers. The accesses
from IOCP are coherent with D-Caches and L2 Cache.

IOCP is a specification option and is disabled on the Renesas RZ/Five
SoC due to this reason IP blocks using DMA will fail.

The Andes AX45MP core has a Programmable Physical Memory Attributes (PMA)
block that allows dynamic adjustment of memory attributes in the runtime.
It contains a configurable amount of PMA entries implemented as CSR
registers to control the attributes of memory locations in interest.
Below are the memory attributes supported:
* Device, Non-bufferable
* Device, bufferable
* Memory, Non-cacheable, Non-bufferable
* Memory, Non-cacheable, Bufferable
* Memory, Write-back, No-allocate
* Memory, Write-back, Read-allocate
* Memory, Write-back, Write-allocate
* Memory, Write-back, Read and Write-allocate

More info about PMA (section 10.3):
Link: http://www.andestech.com/wp-content/uploads/AX45MP-1C-Rev.-5.0.0-Datasheet.pdf

As a workaround for SoCs with IOCP disabled CMO needs to be handled by
software. Firstly OpenSBI configures the memory region as
"Memory, Non-cacheable, Bufferable" and passes this region as a global
shared dma pool as a DT node. With DMA_GLOBAL_POOL enabled all DMA
allocations happen from this region and synchronization callbacks are
implemented to synchronize when doing DMA transactions.

Example PMA region passes as a DT node from OpenSBI:
    reserved-memory {
        #address-cells = <2>;
        #size-cells = <2>;
        ranges;

        pma_resv0@58000000 {
            compatible = "shared-dma-pool";
            reg = <0x0 0x58000000 0x0 0x08000000>;
            no-map;
            linux,dma-default;
        };
    };

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
[Conor: moved the driver to drivers/cache]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 arch/riscv/include/asm/errata_list.h |  28 ++-
 drivers/cache/Kconfig                |   9 +
 drivers/cache/Makefile               |   2 +
 drivers/cache/ax45mp_cache.c         | 253 +++++++++++++++++++++++++++
 drivers/soc/renesas/Kconfig          |   4 +
 5 files changed, 290 insertions(+), 6 deletions(-)
 create mode 100644 drivers/cache/ax45mp_cache.c

diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 48e899a8e7a9..7636b83c1ecb 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -125,8 +125,8 @@ asm volatile(ALTERNATIVE(						\
 #define THEAD_SYNC_S	".long 0x0190000b"
 
 #define ALT_CMO_OP(_op, _start, _size, _cachesize, _dir, _ops)		\
-asm volatile(ALTERNATIVE_2(						\
-	__nops(6),							\
+asm volatile(ALTERNATIVE_3(						\
+	__nops(14),							\
 	"mv a0, %1\n\t"							\
 	"j 2f\n\t"							\
 	"3:\n\t"							\
@@ -134,7 +134,7 @@ asm volatile(ALTERNATIVE_2(						\
 	"add a0, a0, %0\n\t"						\
 	"2:\n\t"							\
 	"bltu a0, %2, 3b\n\t"						\
-	"nop", 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM,		\
+	__nops(9), 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM,	\
 	"mv a0, %1\n\t"							\
 	"j 2f\n\t"							\
 	"3:\n\t"							\
@@ -142,8 +142,24 @@ asm volatile(ALTERNATIVE_2(						\
 	"add a0, a0, %0\n\t"						\
 	"2:\n\t"							\
 	"bltu a0, %2, 3b\n\t"						\
-	THEAD_SYNC_S, THEAD_VENDOR_ID,					\
-			ERRATA_THEAD_CMO, CONFIG_ERRATA_THEAD_CMO)	\
+	THEAD_SYNC_S "\n\t"						\
+	__nops(8), THEAD_VENDOR_ID,					\
+			ERRATA_THEAD_CMO, CONFIG_ERRATA_THEAD_CMO,	\
+	"addi sp,sp,-16\n\t"						\
+	"sd s0,0(sp)\n\t"						\
+	"sd ra,8(sp)\n\t"						\
+	"addi s0,sp,16\n\t"						\
+	"mv a4,%6\n\t"							\
+	"mv a3,%5\n\t"							\
+	"mv a2,%4\n\t"							\
+	"mv a1,%3\n\t"							\
+	"mv a0,%0\n\t"							\
+	"call cmo_patchfunc\n\t"					\
+	"ld ra,8(sp)\n\t"						\
+	"ld s0,0(sp)\n\t"						\
+	"addi sp,sp,16\n\t",						\
+	ANDESTECH_VENDOR_ID, ERRATA_ANDESTECH_NO_IOCP,			\
+	CONFIG_ERRATA_ANDES_CMO)					\
 	: : "r"(_cachesize),						\
 	    "r"((unsigned long)(_start) & ~((_cachesize) - 1UL)),	\
 	    "r"((unsigned long)(_start) + (_size)),			\
@@ -151,7 +167,7 @@ asm volatile(ALTERNATIVE_2(						\
 	    "r"((unsigned long)(_size)),				\
 	    "r"((unsigned long)(_dir)),					\
 	    "r"((unsigned long)(_ops))					\
-	: "a0")
+	: "a0", "a1", "a2", "a3", "a4", "memory")
 
 #define THEAD_C9XX_RV_IRQ_PMU			17
 #define THEAD_C9XX_CSR_SCOUNTEROF		0x5c5
diff --git a/drivers/cache/Kconfig b/drivers/cache/Kconfig
index bc852f005c10..e3309a668d1b 100644
--- a/drivers/cache/Kconfig
+++ b/drivers/cache/Kconfig
@@ -6,6 +6,15 @@ menuconfig CACHE_CONTROLLER
 
 if CACHE_CONTROLLER
 
+config AX45MP_L2_CACHE
+	bool "Andes Technology AX45MP L2 Cache controller"
+	depends on RISCV
+	depends on RISCV_SBI
+	default y
+	help
+	  Support for the L2 cache controller on Andes Technology AX45MP
+	  platforms.
+
 config SIFIVE_CCACHE
 	bool "Sifive Composable Cache controller"
 	depends on RISCV
diff --git a/drivers/cache/Makefile b/drivers/cache/Makefile
index 1f5dc339bf82..f175d16d8611 100644
--- a/drivers/cache/Makefile
+++ b/drivers/cache/Makefile
@@ -1,3 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 
+
+obj-$(CONFIG_AX45MP_L2_CACHE)	+= ax45mp_cache.o
 obj-$(CONFIG_SIFIVE_CCACHE)	+= sifive_ccache.o
diff --git a/drivers/cache/ax45mp_cache.c b/drivers/cache/ax45mp_cache.c
new file mode 100644
index 000000000000..a488b85b9eea
--- /dev/null
+++ b/drivers/cache/ax45mp_cache.c
@@ -0,0 +1,253 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * non-coherent cache functions for Andes AX45MP
+ *
+ * Copyright (C) 2022 Renesas Electronics Corp.
+ */
+
+#include <linux/cacheflush.h>
+#include <linux/cacheinfo.h>
+#include <linux/dma-direction.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+
+#include <asm/cacheflush.h>
+#include <asm/sbi.h>
+
+/* L2 cache registers */
+#define AX45MP_L2C_REG_CTL_OFFSET		0x8
+
+#define AX45MP_L2C_REG_C0_CMD_OFFSET		0x40
+#define AX45MP_L2C_REG_C0_ACC_OFFSET		0x48
+#define AX45MP_L2C_REG_STATUS_OFFSET		0x80
+
+/* D-cache operation */
+#define AX45MP_CCTL_L1D_VA_INVAL		0
+#define AX45MP_CCTL_L1D_VA_WB			1
+
+/* L2 CCTL status */
+#define AX45MP_CCTL_L2_STATUS_IDLE		0
+
+/* L2 CCTL status cores mask */
+#define AX45MP_CCTL_L2_STATUS_C0_MASK		0xf
+
+/* L2 cache operation */
+#define AX45MP_CCTL_L2_PA_INVAL			0x8
+#define AX45MP_CCTL_L2_PA_WB			0x9
+
+#define AX45MP_L2C_REG_PER_CORE_OFFSET		0x10
+#define AX45MP_CCTL_L2_STATUS_PER_CORE_OFFSET	4
+
+#define AX45MP_L2C_REG_CN_CMD_OFFSET(n)	\
+	(AX45MP_L2C_REG_C0_CMD_OFFSET + ((n) * AX45MP_L2C_REG_PER_CORE_OFFSET))
+#define AX45MP_L2C_REG_CN_ACC_OFFSET(n)	\
+	(AX45MP_L2C_REG_C0_ACC_OFFSET + ((n) * AX45MP_L2C_REG_PER_CORE_OFFSET))
+#define AX45MP_CCTL_L2_STATUS_CN_MASK(n)	\
+	(AX45MP_CCTL_L2_STATUS_C0_MASK << ((n) * AX45MP_CCTL_L2_STATUS_PER_CORE_OFFSET))
+
+#define AX45MP_CCTL_REG_UCCTLBEGINADDR_NUM	0x80b
+#define AX45MP_CCTL_REG_UCCTLCOMMAND_NUM	0x80c
+
+#define AX45MP_CACHE_LINE_SIZE			64
+
+struct ax45mp_priv {
+	void __iomem *l2c_base;
+	u32 ax45mp_cache_line_size;
+};
+
+static struct ax45mp_priv *ax45mp_priv;
+static struct riscv_cache_maint_ops ax45mp_cmos;
+
+/* L2 Cache operations */
+static inline uint32_t ax45mp_cpu_l2c_get_cctl_status(void)
+{
+	return readl(ax45mp_priv->l2c_base + AX45MP_L2C_REG_STATUS_OFFSET);
+}
+
+/*
+ * Software trigger CCTL operation (cache maintenance operations) by writing
+ * to ucctlcommand and ucctlbeginaddr registers and write-back an L2 cache
+ * entry.
+ */
+static void ax45mp_cpu_dcache_wb_range(void *start, void *end, int line_size)
+{
+	void __iomem *base = ax45mp_priv->l2c_base;
+	int mhartid = smp_processor_id();
+	unsigned long pa;
+
+	while (end > start) {
+		csr_write(AX45MP_CCTL_REG_UCCTLBEGINADDR_NUM, start);
+		csr_write(AX45MP_CCTL_REG_UCCTLCOMMAND_NUM, AX45MP_CCTL_L1D_VA_WB);
+
+		pa = virt_to_phys(start);
+		writel(pa, base + AX45MP_L2C_REG_CN_ACC_OFFSET(mhartid));
+		writel(AX45MP_CCTL_L2_PA_WB,
+		       base + AX45MP_L2C_REG_CN_CMD_OFFSET(mhartid));
+		while ((ax45mp_cpu_l2c_get_cctl_status() &
+			AX45MP_CCTL_L2_STATUS_CN_MASK(mhartid)) !=
+			AX45MP_CCTL_L2_STATUS_IDLE)
+			;
+
+		start += line_size;
+	}
+}
+
+/*
+ * Software trigger CCTL operation by writing to ucctlcommand and ucctlbeginaddr
+ * registers and invalidate the L2 cache entry.
+ */
+static void ax45mp_cpu_dcache_inval_range(void *start, void *end, int line_size)
+{
+	void __iomem *base = ax45mp_priv->l2c_base;
+	int mhartid = smp_processor_id();
+	unsigned long pa;
+
+	while (end > start) {
+		csr_write(AX45MP_CCTL_REG_UCCTLBEGINADDR_NUM, start);
+		csr_write(AX45MP_CCTL_REG_UCCTLCOMMAND_NUM, AX45MP_CCTL_L1D_VA_INVAL);
+
+		pa = virt_to_phys(start);
+		writel(pa, base + AX45MP_L2C_REG_CN_ACC_OFFSET(mhartid));
+		writel(AX45MP_CCTL_L2_PA_INVAL,
+		       base + AX45MP_L2C_REG_CN_CMD_OFFSET(mhartid));
+		while ((ax45mp_cpu_l2c_get_cctl_status() &
+			AX45MP_CCTL_L2_STATUS_CN_MASK(mhartid)) !=
+			AX45MP_CCTL_L2_STATUS_IDLE)
+			;
+
+		start += line_size;
+	}
+}
+
+static void ax45mp_cpu_dma_inval_range(void *vaddr, size_t size)
+{
+	char cache_buf[2][AX45MP_CACHE_LINE_SIZE];
+	unsigned long start = (unsigned long)vaddr;
+	unsigned long end = start + size;
+	unsigned long old_start = start;
+	unsigned long old_end = end;
+	unsigned long line_size;
+	unsigned long flags;
+
+	if (unlikely(start == end))
+		return;
+
+	line_size = ax45mp_priv->ax45mp_cache_line_size;
+
+	memset(&cache_buf, 0x0, sizeof(cache_buf));
+	start = start & (~(line_size - 1));
+	end = ((end + line_size - 1) & (~(line_size - 1)));
+
+	local_irq_save(flags);
+	if (unlikely(start != old_start))
+		memcpy(&cache_buf[0][0], (void *)start, line_size);
+
+	if (unlikely(end != old_end))
+		memcpy(&cache_buf[1][0], (void *)(old_end & (~(line_size - 1))), line_size);
+
+	ax45mp_cpu_dcache_inval_range(vaddr, (void *)end, line_size);
+
+	if (unlikely(start != old_start))
+		memcpy((void *)start, &cache_buf[0][0], (old_start & (line_size - 1)));
+
+	local_irq_restore(flags);
+}
+
+static void ax45mp_cpu_dma_wb_range(void *vaddr, size_t size)
+{
+	unsigned long start = (unsigned long)vaddr;
+	unsigned long end = start + size;
+	unsigned long line_size;
+	unsigned long flags;
+
+	line_size = ax45mp_priv->ax45mp_cache_line_size;
+	local_irq_save(flags);
+	start = start & (~(line_size - 1));
+	ax45mp_cpu_dcache_wb_range(vaddr, (void *)end, line_size);
+	local_irq_restore(flags);
+}
+
+static void ax45mp_no_iocp_cmo(unsigned int cache_size, void *vaddr, size_t size, int dir, int ops)
+{
+	if (ops == NON_COHERENT_DMA_PREP)
+		return;
+
+	if (ops == NON_COHERENT_SYNC_DMA_FOR_DEVICE) {
+		switch (dir) {
+		case DMA_FROM_DEVICE:
+			ax45mp_cpu_dma_inval_range(vaddr, size);
+			break;
+		case DMA_TO_DEVICE:
+		case DMA_BIDIRECTIONAL:
+			ax45mp_cpu_dma_wb_range(vaddr, size);
+			break;
+		default:
+			break;
+		}
+		return;
+	}
+
+	/* op == NON_COHERENT_SYNC_DMA_FOR_CPU */
+	if (dir == DMA_BIDIRECTIONAL || dir == DMA_FROM_DEVICE)
+		ax45mp_cpu_dma_inval_range(vaddr, size);
+}
+
+static void ax45mp_configure_l2_cache(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = of_property_read_u32(np, "cache-line-size", &ax45mp_priv->ax45mp_cache_line_size);
+	if (ret) {
+		dev_err(dev, "Failed to get cache-line-size, defaulting to 64 bytes\n");
+		ax45mp_priv->ax45mp_cache_line_size = AX45MP_CACHE_LINE_SIZE;
+	}
+
+	if (ax45mp_priv->ax45mp_cache_line_size != AX45MP_CACHE_LINE_SIZE) {
+		dev_err(dev, "Expected cache-line-size to be 64 bytes (found:%u). Defaulting to 64 bytes\n",
+			ax45mp_priv->ax45mp_cache_line_size);
+		ax45mp_priv->ax45mp_cache_line_size = AX45MP_CACHE_LINE_SIZE;
+	}
+}
+
+static int ax45mp_l2c_probe(struct platform_device *pdev)
+{
+	ax45mp_priv = devm_kzalloc(&pdev->dev, sizeof(*ax45mp_priv), GFP_KERNEL);
+	if (!ax45mp_priv)
+		return -ENOMEM;
+
+	ax45mp_priv->l2c_base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(ax45mp_priv->l2c_base))
+		return PTR_ERR(ax45mp_priv->l2c_base);
+
+	ax45mp_configure_l2_cache(pdev);
+
+	ax45mp_cmos.cmo_patchfunc = ax45mp_no_iocp_cmo;
+	riscv_set_cache_maint_ops(&ax45mp_cmos);
+
+	return 0;
+}
+
+static const struct of_device_id ax45mp_cache_ids[] = {
+	{ .compatible = "andestech,ax45mp-cache" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver ax45mp_l2c_driver = {
+	.driver = {
+		.name = "ax45mp-l2c",
+		.of_match_table = ax45mp_cache_ids,
+	},
+	.probe = ax45mp_l2c_probe,
+};
+
+static int __init ax45mp_cache_init(void)
+{
+	return platform_driver_register(&ax45mp_l2c_driver);
+}
+arch_initcall(ax45mp_cache_init);
+
+MODULE_AUTHOR("Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>");
+MODULE_DESCRIPTION("Andes AX45MP L2 cache driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
index 660498252ec5..c3475c084685 100644
--- a/drivers/soc/renesas/Kconfig
+++ b/drivers/soc/renesas/Kconfig
@@ -340,6 +340,10 @@ if RISCV
 config ARCH_R9A07G043
 	bool "RISC-V Platform support for RZ/Five"
 	select ARCH_RZG2L
+	select AX45MP_L2_CACHE
+	select DMA_GLOBAL_POOL
+	select ERRATA_ANDES
+	select ERRATA_ANDES_CMO
 	help
 	  This enables support for the Renesas RZ/Five SoC.
 
-- 
2.39.0

[RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Conor Dooley]

From: Daire McNamara <daire.mcnamara@microchip.com>

SiFive L2 cache controller can flush L2 cache. Expose this capability via
driver.

Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
[Conor: rebase on top of move to cache subsystem]
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
This commit needs more work, and a way to enable it from errata. I've
not gone and done this as PolarFire SoC has archid etc all set to zero.
So we need to go figure out a workaround for this, before adding in
errata enabling code for this. I've included it here as a second user of
the cache management stuff, since what's currently upstream for the
ccache driver does not do any cache management.
---
 drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
index 47e7d6557f85..3c00f205bace 100644
--- a/drivers/cache/sifive_ccache.c
+++ b/drivers/cache/sifive_ccache.c
@@ -9,12 +9,14 @@
 #define pr_fmt(fmt) "CCACHE: " fmt
 
 #include <linux/debugfs.h>
+#include <linux/dma-direction.h>
 #include <linux/interrupt.h>
 #include <linux/of_irq.h>
 #include <linux/of_address.h>
 #include <linux/device.h>
 #include <linux/bitfield.h>
 #include <asm/cacheinfo.h>
+#include <asm/cacheflush.h>
 #include <cache/sifive_ccache.h>
 
 #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
@@ -42,11 +44,15 @@
 #define SIFIVE_CCACHE_WAYENABLE 0x08
 #define SIFIVE_CCACHE_ECCINJECTERR 0x40
 
+#define SIFIVE_CCACHE_FLUSH64 0x200
+#define SIFIVE_CCACHE_FLUSH32 0x240
+
 #define SIFIVE_CCACHE_MAX_ECCINTR 4
 
 static void __iomem *ccache_base;
 static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
 static struct riscv_cacheinfo_ops ccache_cache_ops;
+static struct riscv_cache_maint_ops ccache_cmos;
 static int level;
 
 enum {
@@ -205,6 +211,42 @@ static irqreturn_t ccache_int_handler(int irq, void *device)
 	return IRQ_HANDLED;
 }
 
+static void sifive_ccache_dma_wback_inv(void* vaddr, unsigned long size)
+{
+	void * __iomem flush = ccache_base + SIFIVE_CCACHE_FLUSH64;
+	phys_addr_t start = virt_to_phys(vaddr);
+	phys_addr_t aligned_start = start & ~0x3f;
+	u64 addr;
+	u64 end;
+	u64 aligned_end;
+
+	size += start - aligned_start;
+	end = start + size;
+	aligned_end = end += 0x3f;
+	aligned_end &= ~0x3f;
+
+	for (addr = aligned_start; addr < aligned_end; addr += 64)
+		writeq(addr, flush);
+}
+
+static void sifive_ccache_cmo(unsigned int cache_size, void *vaddr, size_t size,
+			      int dir, int ops)
+{
+	switch (dir) {
+	case DMA_TO_DEVICE:
+		sifive_ccache_dma_wback_inv(vaddr, size);
+		break;
+	case DMA_FROM_DEVICE:
+		sifive_ccache_dma_wback_inv(vaddr, size);
+		break;
+	case DMA_BIDIRECTIONAL:
+		sifive_ccache_dma_wback_inv(vaddr, size);
+		break;
+	default:
+		break;
+	}
+}
+
 static int __init sifive_ccache_init(void)
 {
 	struct device_node *np;
@@ -254,6 +296,9 @@ static int __init sifive_ccache_init(void)
 	ccache_cache_ops.get_priv_group = ccache_get_priv_group;
 	riscv_set_cacheinfo_ops(&ccache_cache_ops);
 
+	ccache_cmos.cmo_patchfunc = sifive_ccache_cmo;
+	riscv_set_cache_maint_ops(&ccache_cmos);
+
 #ifdef CONFIG_DEBUG_FS
 	setup_sifive_debug();
 #endif
-- 
2.39.0

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Palmer Dabbelt]

On Tue, 03 Jan 2023 13:04:01 PST (-0800), Conor Dooley wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> SiFive L2 cache controller can flush L2 cache. Expose this capability via

Do you have a pointer to the datasheet for whatever L2 controller is in 
the PFSOC?  IIRC whether it's possible to correctly flush the cache is 
kind of subtle, as it depends on what else is floating around the SOC.

> driver.
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> [Conor: rebase on top of move to cache subsystem]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> This commit needs more work, and a way to enable it from errata. I've
> not gone and done this as PolarFire SoC has archid etc all set to zero.
> So we need to go figure out a workaround for this, before adding in
> errata enabling code for this. I've included it here as a second user of
> the cache management stuff, since what's currently upstream for the
> ccache driver does not do any cache management.
> ---
>  drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
> index 47e7d6557f85..3c00f205bace 100644
> --- a/drivers/cache/sifive_ccache.c
> +++ b/drivers/cache/sifive_ccache.c
> @@ -9,12 +9,14 @@
>  #define pr_fmt(fmt) "CCACHE: " fmt
>
>  #include <linux/debugfs.h>
> +#include <linux/dma-direction.h>
>  #include <linux/interrupt.h>
>  #include <linux/of_irq.h>
>  #include <linux/of_address.h>
>  #include <linux/device.h>
>  #include <linux/bitfield.h>
>  #include <asm/cacheinfo.h>
> +#include <asm/cacheflush.h>
>  #include <cache/sifive_ccache.h>
>
>  #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
> @@ -42,11 +44,15 @@
>  #define SIFIVE_CCACHE_WAYENABLE 0x08
>  #define SIFIVE_CCACHE_ECCINJECTERR 0x40
>
> +#define SIFIVE_CCACHE_FLUSH64 0x200
> +#define SIFIVE_CCACHE_FLUSH32 0x240
> +
>  #define SIFIVE_CCACHE_MAX_ECCINTR 4
>
>  static void __iomem *ccache_base;
>  static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
>  static struct riscv_cacheinfo_ops ccache_cache_ops;
> +static struct riscv_cache_maint_ops ccache_cmos;
>  static int level;
>
>  enum {
> @@ -205,6 +211,42 @@ static irqreturn_t ccache_int_handler(int irq, void *device)
>  	return IRQ_HANDLED;
>  }
>
> +static void sifive_ccache_dma_wback_inv(void* vaddr, unsigned long size)
> +{
> +	void * __iomem flush = ccache_base + SIFIVE_CCACHE_FLUSH64;
> +	phys_addr_t start = virt_to_phys(vaddr);
> +	phys_addr_t aligned_start = start & ~0x3f;
> +	u64 addr;
> +	u64 end;
> +	u64 aligned_end;
> +
> +	size += start - aligned_start;
> +	end = start + size;
> +	aligned_end = end += 0x3f;
> +	aligned_end &= ~0x3f;
> +
> +	for (addr = aligned_start; addr < aligned_end; addr += 64)
> +		writeq(addr, flush);
> +}
> +
> +static void sifive_ccache_cmo(unsigned int cache_size, void *vaddr, size_t size,
> +			      int dir, int ops)
> +{
> +	switch (dir) {
> +	case DMA_TO_DEVICE:
> +		sifive_ccache_dma_wback_inv(vaddr, size);
> +		break;
> +	case DMA_FROM_DEVICE:
> +		sifive_ccache_dma_wback_inv(vaddr, size);
> +		break;
> +	case DMA_BIDIRECTIONAL:
> +		sifive_ccache_dma_wback_inv(vaddr, size);
> +		break;
> +	default:
> +		break;
> +	}
> +}
> +
>  static int __init sifive_ccache_init(void)
>  {
>  	struct device_node *np;
> @@ -254,6 +296,9 @@ static int __init sifive_ccache_init(void)
>  	ccache_cache_ops.get_priv_group = ccache_get_priv_group;
>  	riscv_set_cacheinfo_ops(&ccache_cache_ops);
>
> +	ccache_cmos.cmo_patchfunc = sifive_ccache_cmo;
> +	riscv_set_cache_maint_ops(&ccache_cmos);
> +
>  #ifdef CONFIG_DEBUG_FS
>  	setup_sifive_debug();
>  #endif

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Arnd Bergmann]

On Tue, Jan 3, 2023, at 22:04, Conor Dooley wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
>
> SiFive L2 cache controller can flush L2 cache. Expose this capability via
> driver.
>
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> [Conor: rebase on top of move to cache subsystem]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> This commit needs more work, and a way to enable it from errata. I've
> not gone and done this as PolarFire SoC has archid etc all set to zero.
> So we need to go figure out a workaround for this, before adding in
> errata enabling code for this. I've included it here as a second user of
> the cache management stuff, since what's currently upstream for the
> ccache driver does not do any cache management.
> ---
>  drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)

My feeling here is that the cacheflush code is unrelated to the
EDAC code and it should just be a separate file. From what I can
tell, all of the existing contents of this file can simply
get merged into drivers/edac/sifive_edac.c, with the newly
added code becoming a standalone driver.

     Arnd

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1526 bytes --]

On Tue, Jan 03, 2023 at 10:28:19PM +0100, Arnd Bergmann wrote:
> On Tue, Jan 3, 2023, at 22:04, Conor Dooley wrote:
> > From: Daire McNamara <daire.mcnamara@microchip.com>
> >
> > SiFive L2 cache controller can flush L2 cache. Expose this capability via
> > driver.
> >
> > Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> > [Conor: rebase on top of move to cache subsystem]
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > This commit needs more work, and a way to enable it from errata. I've
> > not gone and done this as PolarFire SoC has archid etc all set to zero.
> > So we need to go figure out a workaround for this, before adding in
> > errata enabling code for this. I've included it here as a second user of
> > the cache management stuff, since what's currently upstream for the
> > ccache driver does not do any cache management.
> > ---
> >  drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> 
> My feeling here is that the cacheflush code is unrelated to the
> EDAC code and it should just be a separate file. From what I can
> tell, all of the existing contents of this file can simply
> get merged into drivers/edac/sifive_edac.c, with the newly
> added code becoming a standalone driver.

Sure? I'd like to do that independently of whatever is done for the
ax45mp CMOs though, don't think it's worth holding up that platform's
support on me splitting this out.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Arnd Bergmann]

On Wed, Jan 4, 2023, at 01:00, Conor Dooley wrote:
> On Tue, Jan 03, 2023 at 10:28:19PM +0100, Arnd Bergmann wrote:
>> On Tue, Jan 3, 2023, at 22:04, Conor Dooley wrote:
>> > From: Daire McNamara <daire.mcnamara@microchip.com>
>> >
>> > SiFive L2 cache controller can flush L2 cache. Expose this capability via
>> > driver.
>> >
>> > Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
>> > [Conor: rebase on top of move to cache subsystem]
>> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> > ---
>> > This commit needs more work, and a way to enable it from errata. I've
>> > not gone and done this as PolarFire SoC has archid etc all set to zero.
>> > So we need to go figure out a workaround for this, before adding in
>> > errata enabling code for this. I've included it here as a second user of
>> > the cache management stuff, since what's currently upstream for the
>> > ccache driver does not do any cache management.
>> > ---
>> >  drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
>> >  1 file changed, 45 insertions(+)
>> 
>> My feeling here is that the cacheflush code is unrelated to the
>> EDAC code and it should just be a separate file. From what I can
>> tell, all of the existing contents of this file can simply
>> get merged into drivers/edac/sifive_edac.c, with the newly
>> added code becoming a standalone driver.
>
> Sure? I'd like to do that independently of whatever is done for the
> ax45mp CMOs though, don't think it's worth holding up that platform's
> support on me splitting this out.

Right, no need to touch the existing file as part of this series,
it probably just gets in the way of defining a good interface here.

    Arnd

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Conor Dooley]

On 4 January 2023 08:17:41 GMT, Arnd Bergmann <arnd@arndb.de> wrote:
>On Wed, Jan 4, 2023, at 01:00, Conor Dooley wrote:
>> On Tue, Jan 03, 2023 at 10:28:19PM +0100, Arnd Bergmann wrote:
>>> On Tue, Jan 3, 2023, at 22:04, Conor Dooley wrote:
>>> > From: Daire McNamara <daire.mcnamara@microchip.com>
>>> >
>>> > SiFive L2 cache controller can flush L2 cache. Expose this capability via
>>> > driver.
>>> >
>>> > Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
>>> > [Conor: rebase on top of move to cache subsystem]
>>> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>>> > ---
>>> > This commit needs more work, and a way to enable it from errata. I've
>>> > not gone and done this as PolarFire SoC has archid etc all set to zero.
>>> > So we need to go figure out a workaround for this, before adding in
>>> > errata enabling code for this. I've included it here as a second user of
>>> > the cache management stuff, since what's currently upstream for the
>>> > ccache driver does not do any cache management.
>>> > ---
>>> >  drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
>>> >  1 file changed, 45 insertions(+)
>>> 
>>> My feeling here is that the cacheflush code is unrelated to the
>>> EDAC code and it should just be a separate file. From what I can
>>> tell, all of the existing contents of this file can simply
>>> get merged into drivers/edac/sifive_edac.c, with the newly
>>> added code becoming a standalone driver.
>>
>> Sure? I'd like to do that independently of whatever is done for the
>> ax45mp CMOs though, don't think it's worth holding up that platform's
>> support on me splitting this out.
>
>Right, no need to touch the existing file as part of this series,
>it probably just gets in the way of defining a good interface here.

Sure. Can leave it where it was & I'll sort it out later when it's errata etc get added.

Btw, would you mind pointing out where you wanted to have that if/else you mentioned on IRC?

Thanks,
Conor.

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Ben Dooks]

On 03/01/2023 21:04, Conor Dooley wrote:
> From: Daire McNamara <daire.mcnamara@microchip.com>
> 
> SiFive L2 cache controller can flush L2 cache. Expose this capability via
> driver.
> 
> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
> [Conor: rebase on top of move to cache subsystem]
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> This commit needs more work, and a way to enable it from errata. I've
> not gone and done this as PolarFire SoC has archid etc all set to zero.
> So we need to go figure out a workaround for this, before adding in
> errata enabling code for this. I've included it here as a second user of
> the cache management stuff, since what's currently upstream for the
> ccache driver does not do any cache management.

I think errata isn't the right word here, it's more of a system 
requirement for anything that isn't coherent. All the SiFive systems
I have are coherent so won't need this.

> ---
>   drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
>   1 file changed, 45 insertions(+)
> 
> diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
> index 47e7d6557f85..3c00f205bace 100644
> --- a/drivers/cache/sifive_ccache.c
> +++ b/drivers/cache/sifive_ccache.c
> @@ -9,12 +9,14 @@
>   #define pr_fmt(fmt) "CCACHE: " fmt
>   
>   #include <linux/debugfs.h>
> +#include <linux/dma-direction.h>
>   #include <linux/interrupt.h>
>   #include <linux/of_irq.h>
>   #include <linux/of_address.h>
>   #include <linux/device.h>
>   #include <linux/bitfield.h>
>   #include <asm/cacheinfo.h>
> +#include <asm/cacheflush.h>
>   #include <cache/sifive_ccache.h>
>   
>   #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
> @@ -42,11 +44,15 @@
>   #define SIFIVE_CCACHE_WAYENABLE 0x08
>   #define SIFIVE_CCACHE_ECCINJECTERR 0x40
>   
> +#define SIFIVE_CCACHE_FLUSH64 0x200
> +#define SIFIVE_CCACHE_FLUSH32 0x240
> +
>   #define SIFIVE_CCACHE_MAX_ECCINTR 4
>   
>   static void __iomem *ccache_base;
>   static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
>   static struct riscv_cacheinfo_ops ccache_cache_ops;
> +static struct riscv_cache_maint_ops ccache_cmos;
>   static int level;
>   
>   enum {
> @@ -205,6 +211,42 @@ static irqreturn_t ccache_int_handler(int irq, void *device)
>   	return IRQ_HANDLED;
>   }
>   
> +static void sifive_ccache_dma_wback_inv(void* vaddr, unsigned long size)
> +{
> +	void * __iomem flush = ccache_base + SIFIVE_CCACHE_FLUSH64;
> +	phys_addr_t start = virt_to_phys(vaddr);
> +	phys_addr_t aligned_start = start & ~0x3f;
> +	u64 addr;
> +	u64 end;
> +	u64 aligned_end;
> +
> +	size += start - aligned_start;
> +	end = start + size;
> +	aligned_end = end += 0x3f;

I think you meant + 0x3f here. There is an align macro in the kernel
headers, and I'm not sure by inspection if you'd miss the last line
with this code.

> +	aligned_end &= ~0x3f;
> +
> +	for (addr = aligned_start; addr < aligned_end; addr += 64)
> +		writeq(addr, flush);
> +}

The p550 manual states that the zero device flush method is quicker for
any large area flush. However not sure what that level is and whether it
is worth dealing with here? If so we need to have the L3 zero are mapped.

> +
> +static void sifive_ccache_cmo(unsigned int cache_size, void *vaddr, size_t size,
> +			      int dir, int ops)
> +{

technically dir should have been of type "enum dma_data_direction"

> +	switch (dir) {
> +	case DMA_TO_DEVICE:
> +		sifive_ccache_dma_wback_inv(vaddr, size);
> +		break;
> +	case DMA_FROM_DEVICE:
> +		sifive_ccache_dma_wback_inv(vaddr, size);
> +		break;
> +	case DMA_BIDIRECTIONAL:
> +		sifive_ccache_dma_wback_inv(vaddr, size);
> +		break;
> +	default:
> +		break;
> +	}
> +}

I'm not sure why you'd bother checking the dir here, the cache can
only be flushed (I hope DMA_FROM_DEVICE is done /before/ the DMA op).

You could have saved yourself an include if just ignoring dir.

> +
>   static int __init sifive_ccache_init(void)
>   {
>   	struct device_node *np;
> @@ -254,6 +296,9 @@ static int __init sifive_ccache_init(void)
>   	ccache_cache_ops.get_priv_group = ccache_get_priv_group;
>   	riscv_set_cacheinfo_ops(&ccache_cache_ops);
>   
> +	ccache_cmos.cmo_patchfunc = sifive_ccache_cmo;
> +	riscv_set_cache_maint_ops(&ccache_cmos);
> +
>   #ifdef CONFIG_DEBUG_FS
>   	setup_sifive_debug();
>   #endif

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [RFC v5.1 6/9] cache,soc: Move SiFive CCache driver & create drivers/cache

[Ben Dooks]

On 03/01/2023 21:03, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The Zicbo* set of extensions for cache maintenance arrived too late &
> several SoCs exist without them that require non-coherent DMA.
> As things stand, the StarFive JH7100, Microchip PolarFire SoC & Renesas
> RZ/Five all require cache maintenance and lack instructions for this
> purpose.
> Create a subsystem for cache drivers so that vendor specific behaviour
> can be isolated from arch code, but keep the interfaces etc consistent.
> Move the existing SiFive CCache driver to create drivers/cache.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>   MAINTAINERS                                   | 15 ++++++++-------
>   drivers/Kconfig                               |  2 ++
>   drivers/Makefile                              |  2 ++
>   drivers/{soc/sifive => cache}/Kconfig         |  8 +++++++-
>   drivers/{soc/sifive => cache}/Makefile        |  0
>   drivers/{soc/sifive => cache}/sifive_ccache.c |  2 +-
>   drivers/edac/sifive_edac.c                    |  2 +-
>   drivers/soc/Kconfig                           |  1 -
>   drivers/soc/Makefile                          |  1 -
>   include/{soc/sifive => cache}/sifive_ccache.h |  0
>   10 files changed, 21 insertions(+), 12 deletions(-)
>   rename drivers/{soc/sifive => cache}/Kconfig (56%)
>   rename drivers/{soc/sifive => cache}/Makefile (100%)
>   rename drivers/{soc/sifive => cache}/sifive_ccache.c (99%)
>   rename include/{soc/sifive => cache}/sifive_ccache.h (100%)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f61eb221415b..4437e96a657b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19054,13 +19054,6 @@ S:	Maintained
>   F:	Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
>   F:	drivers/dma/sf-pdma/
>   
> -SIFIVE SOC DRIVERS
> -M:	Conor Dooley <conor@kernel.org>
> -L:	linux-riscv@lists.infradead.org
> -S:	Maintained
> -T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
> -F:	drivers/soc/sifive/
> -
>   SILEAD TOUCHSCREEN DRIVER
>   M:	Hans de Goede <hdegoede@redhat.com>
>   L:	linux-input@vger.kernel.org
> @@ -19873,6 +19866,14 @@ S:	Supported
>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
>   F:	drivers/staging/
>   
> +STANDALONE CACHE CONTROLLER DRIVERS
> +M:	Conor Dooley <conor@kernel.org>
> +L:	linux-riscv@lists.infradead.org
> +S:	Maintained
> +T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
> +F:	drivers/cache
> +F:	include/cache
I thought the riscv list was subscribers only?

Maybe if we do the suggestion of other cache drivers here we should
either use the main kernel one or find some arch non-specific list.

> +
>   STARFIRE/DURALAN NETWORK DRIVER
>   M:	Ion Badulescu <ionut@badula.org>
>   S:	Odd Fixes
> diff --git a/drivers/Kconfig b/drivers/Kconfig
> index 968bd0a6fd78..e592ba5276ae 100644
> --- a/drivers/Kconfig
> +++ b/drivers/Kconfig
> @@ -241,4 +241,6 @@ source "drivers/peci/Kconfig"
>   
>   source "drivers/hte/Kconfig"
>   
> +source "drivers/cache/Kconfig"
> +
>   endmenu
> diff --git a/drivers/Makefile b/drivers/Makefile
> index bdf1c66141c9..6ff60cf21823 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -38,6 +38,8 @@ obj-y				+= clk/
>   # really early.
>   obj-$(CONFIG_DMADEVICES)	+= dma/
>   
> +obj-y				+= cache/
> +
>   # SOC specific infrastructure drivers.
>   obj-y				+= soc/
>   
> diff --git a/drivers/soc/sifive/Kconfig b/drivers/cache/Kconfig
> similarity index 56%
> rename from drivers/soc/sifive/Kconfig
> rename to drivers/cache/Kconfig
> index ed4c571f8771..bc852f005c10 100644
> --- a/drivers/soc/sifive/Kconfig
> +++ b/drivers/cache/Kconfig
> @@ -1,9 +1,15 @@
>   # SPDX-License-Identifier: GPL-2.0
>   
> -if SOC_SIFIVE
> +menuconfig CACHE_CONTROLLER
> +	bool "Cache controller driver support"
> +	default y if RISCV
> +
> +if CACHE_CONTROLLER
>   
>   config SIFIVE_CCACHE
>   	bool "Sifive Composable Cache controller"
> +	depends on RISCV
> +	default y
>   	help
>   	  Support for the composable cache controller on SiFive platforms.
>   

Maybe we should find and move the ARM PL cache controllers and
have them here too?

-- 
Ben Dooks				http://www.codethink.co.uk/
Senior Engineer				Codethink - Providing Genius

https://www.codethink.co.uk/privacy.html

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Conor Dooley]

On 4 January 2023 09:45:30 GMT, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>On 03/01/2023 21:04, Conor Dooley wrote:
>> From: Daire McNamara <daire.mcnamara@microchip.com>
>> 
>> SiFive L2 cache controller can flush L2 cache. Expose this capability via
>> driver.
>> 
>> Signed-off-by: Daire McNamara <daire.mcnamara@microchip.com>
>> [Conor: rebase on top of move to cache subsystem]
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>> This commit needs more work, and a way to enable it from errata. I've
>> not gone and done this as PolarFire SoC has archid etc all set to zero.
>> So we need to go figure out a workaround for this, before adding in
>> errata enabling code for this. I've included it here as a second user of
>> the cache management stuff, since what's currently upstream for the
>> ccache driver does not do any cache management.
>
>I think errata isn't the right word here, it's more of a system requirement for anything that isn't coherent. All the SiFive systems
>I have are coherent so won't need this.

That's the mechanism we currently have for turning this stuff on.
This patch is going away anyway for an actual submission, so we can debate this sort of thing when it shows up for real.
This patch does certainly seem to be distracting from the main point, which was supposed to be the interface to arch/riscv.


>> ---
>>   drivers/cache/sifive_ccache.c | 45 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 45 insertions(+)
>> 
>> diff --git a/drivers/cache/sifive_ccache.c b/drivers/cache/sifive_ccache.c
>> index 47e7d6557f85..3c00f205bace 100644
>> --- a/drivers/cache/sifive_ccache.c
>> +++ b/drivers/cache/sifive_ccache.c
>> @@ -9,12 +9,14 @@
>>   #define pr_fmt(fmt) "CCACHE: " fmt
>>     #include <linux/debugfs.h>
>> +#include <linux/dma-direction.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/of_irq.h>
>>   #include <linux/of_address.h>
>>   #include <linux/device.h>
>>   #include <linux/bitfield.h>
>>   #include <asm/cacheinfo.h>
>> +#include <asm/cacheflush.h>
>>   #include <cache/sifive_ccache.h>
>>     #define SIFIVE_CCACHE_DIRECCFIX_LOW 0x100
>> @@ -42,11 +44,15 @@
>>   #define SIFIVE_CCACHE_WAYENABLE 0x08
>>   #define SIFIVE_CCACHE_ECCINJECTERR 0x40
>>   +#define SIFIVE_CCACHE_FLUSH64 0x200
>> +#define SIFIVE_CCACHE_FLUSH32 0x240
>> +
>>   #define SIFIVE_CCACHE_MAX_ECCINTR 4
>>     static void __iomem *ccache_base;
>>   static int g_irq[SIFIVE_CCACHE_MAX_ECCINTR];
>>   static struct riscv_cacheinfo_ops ccache_cache_ops;
>> +static struct riscv_cache_maint_ops ccache_cmos;
>>   static int level;
>>     enum {
>> @@ -205,6 +211,42 @@ static irqreturn_t ccache_int_handler(int irq, void *device)
>>   	return IRQ_HANDLED;
>>   }
>>   +static void sifive_ccache_dma_wback_inv(void* vaddr, unsigned long size)
>> +{
>> +	void * __iomem flush = ccache_base + SIFIVE_CCACHE_FLUSH64;
>> +	phys_addr_t start = virt_to_phys(vaddr);
>> +	phys_addr_t aligned_start = start & ~0x3f;
>> +	u64 addr;
>> +	u64 end;
>> +	u64 aligned_end;
>> +
>> +	size += start - aligned_start;
>> +	end = start + size;
>> +	aligned_end = end += 0x3f;
>
>I think you meant + 0x3f here. There is an align macro in the kernel
>headers, and I'm not sure by inspection if you'd miss the last line
>with this code.
>
>> +	aligned_end &= ~0x3f;
>> +
>> +	for (addr = aligned_start; addr < aligned_end; addr += 64)
>> +		writeq(addr, flush);
>> +}
>
>The p550 manual states that the zero device flush method is quicker for
>any large area flush. However not sure what that level is and whether it
>is worth dealing with here? If so we need to have the L3 zero are mapped.
>
>> +
>> +static void sifive_ccache_cmo(unsigned int cache_size, void *vaddr, size_t size,
>> +			      int dir, int ops)
>> +{
>
>technically dir should have been of type "enum dma_data_direction"
>
>> +	switch (dir) {
>> +	case DMA_TO_DEVICE:
>> +		sifive_ccache_dma_wback_inv(vaddr, size);
>> +		break;
>> +	case DMA_FROM_DEVICE:
>> +		sifive_ccache_dma_wback_inv(vaddr, size);
>> +		break;
>> +	case DMA_BIDIRECTIONAL:
>> +		sifive_ccache_dma_wback_inv(vaddr, size);
>> +		break;
>> +	default:
>> +		break;
>> +	}
>> +}
>
>I'm not sure why you'd bother checking the dir here, the cache can
>only be flushed (I hope DMA_FROM_DEVICE is done /before/ the DMA op).
>
>You could have saved yourself an include if just ignoring dir.
>
>> +
>>   static int __init sifive_ccache_init(void)
>>   {
>>   	struct device_node *np;
>> @@ -254,6 +296,9 @@ static int __init sifive_ccache_init(void)
>>   	ccache_cache_ops.get_priv_group = ccache_get_priv_group;
>>   	riscv_set_cacheinfo_ops(&ccache_cache_ops);
>>   +	ccache_cmos.cmo_patchfunc = sifive_ccache_cmo;
>> +	riscv_set_cache_maint_ops(&ccache_cmos);
>> +
>>   #ifdef CONFIG_DEBUG_FS
>>   	setup_sifive_debug();
>>   #endif
>

Re: [RFC v5.1 6/9] cache,soc: Move SiFive CCache driver & create drivers/cache

[Conor Dooley]

On 4 January 2023 09:50:58 GMT, Ben Dooks <ben.dooks@codethink.co.uk> wrote:
>On 03/01/2023 21:03, Conor Dooley wrote:
>> From: Conor Dooley <conor.dooley@microchip.com>
>> 
>> The Zicbo* set of extensions for cache maintenance arrived too late &
>> several SoCs exist without them that require non-coherent DMA.
>> As things stand, the StarFive JH7100, Microchip PolarFire SoC & Renesas
>> RZ/Five all require cache maintenance and lack instructions for this
>> purpose.
>> Create a subsystem for cache drivers so that vendor specific behaviour
>> can be isolated from arch code, but keep the interfaces etc consistent.
>> Move the existing SiFive CCache driver to create drivers/cache.
>> 
>> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
>> ---
>>   MAINTAINERS                                   | 15 ++++++++-------
>>   drivers/Kconfig                               |  2 ++
>>   drivers/Makefile                              |  2 ++
>>   drivers/{soc/sifive => cache}/Kconfig         |  8 +++++++-
>>   drivers/{soc/sifive => cache}/Makefile        |  0
>>   drivers/{soc/sifive => cache}/sifive_ccache.c |  2 +-
>>   drivers/edac/sifive_edac.c                    |  2 +-
>>   drivers/soc/Kconfig                           |  1 -
>>   drivers/soc/Makefile                          |  1 -
>>   include/{soc/sifive => cache}/sifive_ccache.h |  0
>>   10 files changed, 21 insertions(+), 12 deletions(-)
>>   rename drivers/{soc/sifive => cache}/Kconfig (56%)
>>   rename drivers/{soc/sifive => cache}/Makefile (100%)
>>   rename drivers/{soc/sifive => cache}/sifive_ccache.c (99%)
>>   rename include/{soc/sifive => cache}/sifive_ccache.h (100%)
>> 
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index f61eb221415b..4437e96a657b 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19054,13 +19054,6 @@ S:	Maintained
>>   F:	Documentation/devicetree/bindings/dma/sifive,fu540-c000-pdma.yaml
>>   F:	drivers/dma/sf-pdma/
>>   -SIFIVE SOC DRIVERS
>> -M:	Conor Dooley <conor@kernel.org>
>> -L:	linux-riscv@lists.infradead.org
>> -S:	Maintained
>> -T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
>> -F:	drivers/soc/sifive/
>> -
>>   SILEAD TOUCHSCREEN DRIVER
>>   M:	Hans de Goede <hdegoede@redhat.com>
>>   L:	linux-input@vger.kernel.org
>> @@ -19873,6 +19866,14 @@ S:	Supported
>>   T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
>>   F:	drivers/staging/
>>   +STANDALONE CACHE CONTROLLER DRIVERS
>> +M:	Conor Dooley <conor@kernel.org>
>> +L:	linux-riscv@lists.infradead.org
>> +S:	Maintained
>> +T:	git https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/
>> +F:	drivers/cache
>> +F:	include/cache
>I thought the riscv list was subscribers only?

Shouldn't be. At least, I'm not one on this account and I don't get complaints!

>Maybe if we do the suggestion of other cache drivers here we should
>either use the main kernel one or find some arch non-specific list.

Yah, good point. I just moved the existing one, otherwise my script would yell at me about the maintainers pattern ;)

>
>> +
>>   STARFIRE/DURALAN NETWORK DRIVER
>>   M:	Ion Badulescu <ionut@badula.org>
>>   S:	Odd Fixes
>> diff --git a/drivers/Kconfig b/drivers/Kconfig
>> index 968bd0a6fd78..e592ba5276ae 100644
>> --- a/drivers/Kconfig
>> +++ b/drivers/Kconfig
>> @@ -241,4 +241,6 @@ source "drivers/peci/Kconfig"
>>     source "drivers/hte/Kconfig"
>>   +source "drivers/cache/Kconfig"
>> +
>>   endmenu
>> diff --git a/drivers/Makefile b/drivers/Makefile
>> index bdf1c66141c9..6ff60cf21823 100644
>> --- a/drivers/Makefile
>> +++ b/drivers/Makefile
>> @@ -38,6 +38,8 @@ obj-y				+= clk/
>>   # really early.
>>   obj-$(CONFIG_DMADEVICES)	+= dma/
>>   +obj-y				+= cache/
>> +
>>   # SOC specific infrastructure drivers.
>>   obj-y				+= soc/
>>   diff --git a/drivers/soc/sifive/Kconfig b/drivers/cache/Kconfig
>> similarity index 56%
>> rename from drivers/soc/sifive/Kconfig
>> rename to drivers/cache/Kconfig
>> index ed4c571f8771..bc852f005c10 100644
>> --- a/drivers/soc/sifive/Kconfig
>> +++ b/drivers/cache/Kconfig
>> @@ -1,9 +1,15 @@
>>   # SPDX-License-Identifier: GPL-2.0
>>   -if SOC_SIFIVE
>> +menuconfig CACHE_CONTROLLER
>> +	bool "Cache controller driver support"
>> +	default y if RISCV
>> +
>> +if CACHE_CONTROLLER
>>     config SIFIVE_CCACHE
>>   	bool "Sifive Composable Cache controller"
>> +	depends on RISCV
>> +	default y
>>   	help
>>   	  Support for the composable cache controller on SiFive platforms.
>>   
>
>Maybe we should find and move the ARM PL cache controllers and
>have them here too?

What to do there is up to Arnd et al :)
The maintainers entry would certainly have to look significantly different then.

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Arnd Bergmann]

On Wed, Jan 4, 2023, at 10:23, Conor Dooley wrote:
>>Right, no need to touch the existing file as part of this series,
>>it probably just gets in the way of defining a good interface here.
>
> Sure. Can leave it where it was & I'll sort it out later when it's 
> errata etc get added.
>
> Btw, would you mind pointing out where you wanted to have that if/else 
> you mentioned on IRC?

I meant replacing both of the runtime patching indirections in
arch_sync_dma_for_device(). At the moment, this function calls
ALT_CMO_OP(), which is patched to either call the ZICBOM or the
THEAD variant, and if I read this right you add a third case
there with another level of indirection using static_branch.

I would try to replace both of these indirections and instead
handle it all from C code in arch_sync_dma_for_device() directly,
for the purpose of readability and maintainability.

static inline void dma_cache_clean(void *vaddr, size_t size)
{
        if (!cache_maint_ops.clean)
               zicbom_cache_clean(vaddr, size, riscv_cbom_block_size);
        else
               cache_maint_ops.clean(vaddr, size, riscv_cbom_block_size);
}

void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
                              enum dma_data_direction dir)
{
        void *vaddr = phys_to_virt(paddr);

        switch (dir) {
        case DMA_TO_DEVICE:
        case DMA_FROM_DEVICE:
                dma_cache_clean(vaddr, size);
                break;
        case DMA_BIDIRECTIONAL:
                dma_cache_flush(vaddr, size);
                break;
        default:
                break;
        }
}

which then makes it very clear what the actual code path
is, while leaving the zicbom case free of indirect function
calls. You can still use a static_branch() to optimize the
conditional, but I would try to avoid any extra indirection
levels or errata checks.

     Arnd

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]

Hey Arnd,

On Wed, Jan 04, 2023 at 11:19:44AM +0100, Arnd Bergmann wrote:
> On Wed, Jan 4, 2023, at 10:23, Conor Dooley wrote:
> >>Right, no need to touch the existing file as part of this series,
> >>it probably just gets in the way of defining a good interface here.
> >
> > Sure. Can leave it where it was & I'll sort it out later when it's 
> > errata etc get added.
> >
> > Btw, would you mind pointing out where you wanted to have that if/else 
> > you mentioned on IRC?
> 
> I meant replacing both of the runtime patching indirections in
> arch_sync_dma_for_device(). At the moment, this function calls
> ALT_CMO_OP(), which is patched to either call the ZICBOM or the
> THEAD variant, and if I read this right you add a third case
> there with another level of indirection using static_branch.

Yah, pretty much.

> I would try to replace both of these indirections and instead
> handle it all from C code in arch_sync_dma_for_device() directly,
> for the purpose of readability and maintainability.
> static inline void dma_cache_clean(void *vaddr, size_t size)
> {
>         if (!cache_maint_ops.clean)
>                zicbom_cache_clean(vaddr, size, riscv_cbom_block_size);

And I figure that this function is effectively a wrapper around ALT_CMO_OP()?

>         else
>                cache_maint_ops.clean(vaddr, size, riscv_cbom_block_size);

And this one gets registered by the driver using an interface like the
one I already proposed, just with the cache_maint_ops struct expanded?

Extrapolating, with these changes having an errata would not even be
needed in order to do cache maintenance.
Since the ALT_CMO_OP() version would only be used inside
zicbom_cache_clean(), assuming I understood correctly, a driver could
just register cache_maint_ops for a given platform without having to
muck around with errata.
If so, that seems like a distinct improvement over my suggestion & gets
around the thing I mentioned in 0/9 about a shared case in the
alternative application code.

Again, assuming I understood correctly, I like this a lot.

> }
> 
> void arch_sync_dma_for_device(phys_addr_t paddr, size_t size,
>                               enum dma_data_direction dir)
> {
>         void *vaddr = phys_to_virt(paddr);
> 
>         switch (dir) {
>         case DMA_TO_DEVICE:
>         case DMA_FROM_DEVICE:
>                 dma_cache_clean(vaddr, size);
>                 break;
>         case DMA_BIDIRECTIONAL:
>                 dma_cache_flush(vaddr, size);
>                 break;
>         default:
>                 break;
>         }
> }
> 
> which then makes it very clear what the actual code path
> is, while leaving the zicbom case free of indirect function
> calls. You can still use a static_branch() to optimize the
> conditional, but I would try to avoid any extra indirection
> levels or errata checks.

The other thing that I like about this is we can then remove the various
calls to ALT_CMO_OP() that are scattered around arch/riscv now & replace
them with functions that have more understandable names.

Thanks Arnd!
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Arnd Bergmann]

On Wed, Jan 4, 2023, at 12:56, Conor Dooley wrote:
> On Wed, Jan 04, 2023 at 11:19:44AM +0100, Arnd Bergmann wrote:
>> On Wed, Jan 4, 2023, at 10:23, Conor Dooley wrote:
>> I would try to replace both of these indirections and instead
>> handle it all from C code in arch_sync_dma_for_device() directly,
>> for the purpose of readability and maintainability.
>> static inline void dma_cache_clean(void *vaddr, size_t size)
>> {
>>         if (!cache_maint_ops.clean)
>>                zicbom_cache_clean(vaddr, size, riscv_cbom_block_size);
>
> And I figure that this function is effectively a wrapper around ALT_CMO_OP()?
>
>>         else
>>                cache_maint_ops.clean(vaddr, size, riscv_cbom_block_size);
>
> And this one gets registered by the driver using an interface like the
> one I already proposed, just with the cache_maint_ops struct expanded?

Yes, exactly.

> Extrapolating, with these changes having an errata would not even be
> needed in order to do cache maintenance.
> Since the ALT_CMO_OP() version would only be used inside
> zicbom_cache_clean(), assuming I understood correctly, a driver could
> just register cache_maint_ops for a given platform without having to
> muck around with errata.

That is the idea, and ALT_CMO_OP() itself can just go away
as by just putting the inline asm without the alternative into
the zicbom_cache_clean() version, making the THEAD branch yet
another cache_maint_ops instance.

>> which then makes it very clear what the actual code path
>> is, while leaving the zicbom case free of indirect function
>> calls. You can still use a static_branch() to optimize the
>> conditional, but I would try to avoid any extra indirection
>> levels or errata checks.
>
> The other thing that I like about this is we can then remove the various
> calls to ALT_CMO_OP() that are scattered around arch/riscv now & replace
> them with functions that have more understandable names.

I only see them in arch/riscv/mm/dma-noncoherent.c and arch/riscv/mm/pmem.c,
but yes, both of these should just call the new functions, whatever the
calling conventions end up being.

    Arnd

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3018 bytes --]

On Wed, Jan 04, 2023 at 01:18:45PM +0100, Arnd Bergmann wrote:
> On Wed, Jan 4, 2023, at 12:56, Conor Dooley wrote:
> > On Wed, Jan 04, 2023 at 11:19:44AM +0100, Arnd Bergmann wrote:
> >> On Wed, Jan 4, 2023, at 10:23, Conor Dooley wrote:
> >> I would try to replace both of these indirections and instead
> >> handle it all from C code in arch_sync_dma_for_device() directly,
> >> for the purpose of readability and maintainability.
> >> static inline void dma_cache_clean(void *vaddr, size_t size)
> >> {
> >>         if (!cache_maint_ops.clean)
> >>                zicbom_cache_clean(vaddr, size, riscv_cbom_block_size);
> >
> > And I figure that this function is effectively a wrapper around ALT_CMO_OP()?
> >
> >>         else
> >>                cache_maint_ops.clean(vaddr, size, riscv_cbom_block_size);
> >
> > And this one gets registered by the driver using an interface like the
> > one I already proposed, just with the cache_maint_ops struct expanded?
> 
> Yes, exactly.
> 
> > Extrapolating, with these changes having an errata would not even be
> > needed in order to do cache maintenance.
> > Since the ALT_CMO_OP() version would only be used inside
> > zicbom_cache_clean(), assuming I understood correctly, a driver could
> > just register cache_maint_ops for a given platform without having to
> > muck around with errata.
> 
> That is the idea, and ALT_CMO_OP() itself can just go away
> as by just putting the inline asm without the alternative into
> the zicbom_cache_clean() version, making the THEAD branch yet
> another cache_maint_ops instance.

Perhaps more of a question for Palmer than you, but how about leaving
ALT_CMO_OP as-is in riscv/for-next at the moment, wrapping it in
zicbom_cache_foo() & leaving that extraction for a follow-on work?
There's another conversation going on about expanding the THEAD stuff,
so that could be done on top of Prabhakar's v6.

That series is here:
https://lore.kernel.org/linux-riscv/CAJF2gTQp1bOp9kfoOkbvNnSXQhzrCpG3rn8C+LPPoJtMCCDOdA@mail.gmail.com/T/#t
Although unfortunately Icenowy is having issues getting their patches to
the lists so I assume it'll get let through at some point today.

> >> which then makes it very clear what the actual code path
> >> is, while leaving the zicbom case free of indirect function
> >> calls. You can still use a static_branch() to optimize the
> >> conditional, but I would try to avoid any extra indirection
> >> levels or errata checks.
> >
> > The other thing that I like about this is we can then remove the various
> > calls to ALT_CMO_OP() that are scattered around arch/riscv now & replace
> > them with functions that have more understandable names.
> 
> I only see them in arch/riscv/mm/dma-noncoherent.c and arch/riscv/mm/pmem.c,
> but yes, both of these should just call the new functions, whatever the
> calling conventions end up being.

Dunno why I had it in my head there was a third place. Seeing ghosts
maybe!

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC v5.1 9/9] [DON'T APPLY] cache: sifive-ccache: add cache flushing capability

[Arnd Bergmann]

On Wed, Jan 4, 2023, at 14:20, Conor Dooley wrote:
> On Wed, Jan 04, 2023 at 01:18:45PM +0100, Arnd Bergmann wrote:
>> On Wed, Jan 4, 2023, at 12:56, Conor Dooley wrote:
>> > On Wed, Jan 04, 2023 at 11:19:44AM +0100, Arnd Bergmann wrote:
> Perhaps more of a question for Palmer than you, but how about leaving
> ALT_CMO_OP as-is in riscv/for-next at the moment, wrapping it in
> zicbom_cache_foo() & leaving that extraction for a follow-on work?
> There's another conversation going on about expanding the THEAD stuff,
> so that could be done on top of Prabhakar's v6.

Right, makes sense to me.

      Arnd