mbox series

[RFC,0/2] AX45MP: Add support to non-coherent DMA

Message ID 20220906102154.32526-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
Headers show
Series AX45MP: Add support to non-coherent DMA | expand

Message

Prabhakar Sept. 6, 2022, 10:21 a.m. UTC
Hi All,

On the Andes AX45MP core, cache coherency is a specification option so it
may not be supported. In this case DMA will fail. To get around with this
issue this patch series  does the below:

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. PMA
regions are passed from the cpu core node which are configured as
non-cacheable and non-bufferable with the SBI call.

        ax45mp: cpu@0 {
                compatible = "andestech,ax45mp", "riscv";
                device_type = "cpu";
                ....
                pma-regions = <0x0 0x00000000 0x0 0x14000000>,
                              <0x0 0x20000000 0x0 0x10000000>,
                              <0x0 0x58000000 0x0 0x08000000>;
                ....
        };

We provide callbacks to synchronize specific content between memory and
cache. We allocate a global DMA coherent pool (which is marked as
non-cached using PMA) so that DMA memory allocations happens from this
pool and we implement the below callbacks:

- arch_sync_dma_for_device()
- arch_sync_dma_for_cpu()
- arch_dma_alloc()
- arch_dma_free()

Below are the configs that are enabled:

- DMA_GLOBAL_POOL
- ARCH_HAS_SYNC_DMA_FOR_CPU
- ARCH_HAS_SYNC_DMA_FOR_DEVICE

        l2cache: cache-controller@13400000 {
                compatible = "andestech,ax45mp-cache", "cache";
                cache-size = <0x40000>;
                cache-line-size = <64>;
                cache-sets = <1024>;
                cache-unified;
                reg = <0x0 0x13400000 0x0 0x100000>;
	};

Due to the above approach custom SBI calls have been implemented. The
above implementation is in preparation for adding support for Renesas
RZ/Five SoC which uses the AX45MP core. As with the above approach the
kernel image might not be generic so that it can be used on other
platforms, so sending it as an RFC (without DT binding patches).

OpenSBI implementation isn't upstreamed yet, public repo for access is
available at [0].

[0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA

Cheers,
Prabhakar

Lad Prabhakar (2):
  riscv: vendors: andes: Add support to configure the PMA regions
  riscv: vendors: andes: Add support for non-cohernet dma

 arch/riscv/Kbuild                             |   2 +
 arch/riscv/include/asm/sbi.h                  |   1 +
 arch/riscv/vendors/Makefile                   |   3 +
 arch/riscv/vendors/andes/Makefile             |   4 +
 arch/riscv/vendors/andes/ax45mp.c             |  93 ++++++
 arch/riscv/vendors/andes/ax45mp_cache.c       | 296 ++++++++++++++++++
 arch/riscv/vendors/andes/ax45mp_nocache_dma.c |  65 ++++
 arch/riscv/vendors/andes/include/proc.h       |   9 +
 arch/riscv/vendors/andes/include/sbi.h        |  27 ++
 9 files changed, 500 insertions(+)
 create mode 100644 arch/riscv/vendors/Makefile
 create mode 100644 arch/riscv/vendors/andes/Makefile
 create mode 100644 arch/riscv/vendors/andes/ax45mp.c
 create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
 create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
 create mode 100644 arch/riscv/vendors/andes/include/proc.h
 create mode 100644 arch/riscv/vendors/andes/include/sbi.h

Comments

Conor Dooley Sept. 8, 2022, 9:44 p.m. UTC | #1
Hey,

I had a quick run through this today so if there's discussion
about this next week I at least will have some idea of what I
am talking about...

(I ended up not doing a quick run...)

On 06/09/2022 11:21, Lad Prabhakar wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hi All,
> 
> On the Andes AX45MP core, cache coherency is a specification option so it
> may not be supported. In this case DMA will fail. To get around with this
> issue this patch series  does the below:

You say "may not be supported" - is it or is it not supported by the
core on your SoC? Do some of the cheaper SKUs not support it?

From what Biju has said, you need non-coherent DMA for your eMMC, USB
and ethernet controllers to work? To me, that seems like something that
would be quite important to point out here..


> 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. PMA
> regions are passed from the cpu core node which are configured as
> non-cacheable and non-bufferable with the SBI call.
> 
>         ax45mp: cpu@0 {
>                 compatible = "andestech,ax45mp", "riscv";
>                 device_type = "cpu";
>                 ....
>                 pma-regions = <0x0 0x00000000 0x0 0x14000000>,
>                               <0x0 0x20000000 0x0 0x10000000>,
>                               <0x0 0x58000000 0x0 0x08000000>;
>                 ....
>         };
> 
> We provide callbacks to synchronize specific content between memory and
> cache. We allocate a global DMA coherent pool (which is marked as
> non-cached using PMA) so that DMA memory allocations happens from this
> pool and we implement the below callbacks:
> 
> - arch_sync_dma_for_device()
> - arch_sync_dma_for_cpu()

These two already exist in arch/riscv/mm/dma-noncoherent.c using the
alternatives mechanism.

> - arch_dma_alloc()
> - arch_dma_free()
> 
> Below are the configs that are enabled:
> 
> - DMA_GLOBAL_POOL
> - ARCH_HAS_SYNC_DMA_FOR_CPU
> - ARCH_HAS_SYNC_DMA_FOR_DEVICE

For these two see:
arch/riscv/Kconfig & RISCV_DMA_NONCOHERENT

> 
>         l2cache: cache-controller@13400000 {
>                 compatible = "andestech,ax45mp-cache", "cache";
>                 cache-size = <0x40000>;
>                 cache-line-size = <64>;
>                 cache-sets = <1024>;
>                 cache-unified;
>                 reg = <0x0 0x13400000 0x0 0x100000>;
>         };
> 
> Due to the above approach custom SBI calls have been implemented. The
> above implementation is in preparation for adding support for Renesas
> RZ/Five SoC which uses the AX45MP core. As with the above approach the
> kernel image might not be generic so that it can be used on other
> platforms, so sending it as an RFC (without DT binding patches).
> 
> OpenSBI implementation isn't upstreamed yet, public repo for access is
> available at [0].

When you say "isn't upstreamed yet", what is the actual status? Where in
the process are you or have you not started that yet? Does openSBI even
allow custom extensions to be upstreamed?

> 
> [0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA
> 
> Cheers,
> Prabhakar
> 
> Lad Prabhakar (2):
>   riscv: vendors: andes: Add support to configure the PMA regions
>   riscv: vendors: andes: Add support for non-cohernet dma
> 

Anyway, a couple of drive-by comments, having made the wild assumption
that this can be accepted upstream.

>  arch/riscv/Kbuild                             |   2 +
>  arch/riscv/include/asm/sbi.h                  |   1 +
>  arch/riscv/vendors/Makefile                   |   3 +
>  arch/riscv/vendors/andes/Makefile             |   4 +
>  arch/riscv/vendors/andes/ax45mp_cache.c       | 296 ++++++++++++++++++

Surely this should be in drivers/soc/andestech, just like the SiFive L2
controller is in drivers/soc/sifive rather in a subdirectory of the
arch?

>  arch/riscv/vendors/andes/ax45mp_nocache_dma.c |  65 ++++

This looks like it should be implemented as errata/alternatives just
like the non-coherent stuff on the D1 is done.

>  arch/riscv/vendors/andes/include/proc.h       |   9 +

And I think this would fall away if implemented as errata/alternatives.

>  arch/riscv/vendors/andes/include/sbi.h        |  27 ++
>  arch/riscv/vendors/andes/ax45mp.c             |  93 ++++++

idk where this would go though, if it is even something that is
acceptable, given the policy I linked the other day from:
https://www.kernel.org/doc/html/latest/riscv/patch-acceptance.html#submit-checklist-addendum

There is SiFive specific errata but it is implemented using mimpid etc
rather than compatible/dt. As I said in my initial mails, I am quite
interested in vendor SBI extensions in the kernel. If you did check out
the link I sent, our stuff is a world away from yours - it's isolated to
a driver where we are using SBI ECALLs to communicate with other harts
which are running something other than Linux in AMP configurations.
Pretty much we can do everything we want to do without touching a
single line of code in arch/riscv, so although the statement in that doc
applies to both of us here it does not apply evenly :s

It's all a bit unclear to me what the story is here, because obviously
you are doing things that Zicbo* is meant to do (just like the D1), but
your hardware's design and initial tapeout predates the existance of
Zicbom. Makes me start to wonder, what happens for <insert idea> that
eventually becomes an extension? Where does the line get draw for "you
did something that is not a ratified extension, therefore you are not
permitted upstream"? A line obviously does have to be drawn *somewhere*
and the easiest place to draw that line is "non ratified extensions are
a no-go". But then again, why allow the D1 but not you?

Obviously this is not a runner for someone not using an FPGA or similar,
but the InterHart Communication IP that we are using the SBI ECALLs for
is a fabric core, so we (in theory) could re-write it so that instead of
using an ECALL which routes communication via the e51 "monitor" core we
_could_ write directly to the registers of the IHC block. There's clear
security/isolation benefits for doing things via an ECALL which is why
that method was chosen but if we opened for the direct writes/reads the
driver would be upstream acceptable...

Don't get me wrong, I completely understand why a policy of not allowing
extensions that have not been ratified makes sense - *but* at the same
time if touching arch code is not required it does not feel very much
different to me than adding a driver for a fabric core in the first
place. I mentioned this sort of thing a while back on IRC and Jess made
the point that similar sorts of things are done by some of the Qualcomm
for their remoteproc as we would be doing for ours with the IHC. In your
case, if all of your ECALLs are in drivers/soc - the maintainership
burden for any churn would be on you/Geert etc rather than on the RISC-V
maintainer.

TL;DR of that is maybe a more nuanced policy of "no non-ratified
extensions that touch arch/riscv" could be a possibility but I would
completely understand if a "what's sauce for the goose is sauce for the
gander" approach was taken here and a blanket ban remains in place.

As I have said a bunch of times, this is all just my 2 cents etc and I
am as much of a punter here as you are... but maybe since I am in the
same sort of boat I at least have a fleshed out opinion. ¯\_()_/¯

Hopefully either Palmer can weigh in here or we do get a BoF & the
chance to have a chat about this sort of thing & maybe have a more
nuanced policy - or at the very least something that makes it clearer
that vendor extensions are a complete no-go upstream.

Conor.

>  9 files changed, 500 insertions(+)
>  create mode 100644 arch/riscv/vendors/Makefile
>  create mode 100644 arch/riscv/vendors/andes/Makefile
>  create mode 100644 arch/riscv/vendors/andes/ax45mp.c
>  create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
>  create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
>  create mode 100644 arch/riscv/vendors/andes/include/proc.h
>  create mode 100644 arch/riscv/vendors/andes/include/sbi.h
> 
> --
> 2.25.1
>
Lad, Prabhakar Sept. 9, 2022, 10:21 a.m. UTC | #2
Hi Conor,

On Thu, Sep 8, 2022 at 10:44 PM <Conor.Dooley@microchip.com> wrote:
>
> Hey,
>
> I had a quick run through this today so if there's discussion
> about this next week I at least will have some idea of what I
> am talking about...
>
Thanks for going through this!

> (I ended up not doing a quick run...)
>
:)

> On 06/09/2022 11:21, Lad Prabhakar wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi All,
> >
> > On the Andes AX45MP core, cache coherency is a specification option so it
> > may not be supported. In this case DMA will fail. To get around with this
> > issue this patch series  does the below:
>
> You say "may not be supported" - is it or is it not supported by the
> core on your SoC? Do some of the cheaper SKUs not support it?
>
Yep, it's not supported by the core on the RZ/Five SoC.

> From what Biju has said, you need non-coherent DMA for your eMMC, USB
> and ethernet controllers to work? To me, that seems like something that
> would be quite important to point out here..
>
Agreed!

>
> > 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. PMA
> > regions are passed from the cpu core node which are configured as
> > non-cacheable and non-bufferable with the SBI call.
> >
> >         ax45mp: cpu@0 {
> >                 compatible = "andestech,ax45mp", "riscv";
> >                 device_type = "cpu";
> >                 ....
> >                 pma-regions = <0x0 0x00000000 0x0 0x14000000>,
> >                               <0x0 0x20000000 0x0 0x10000000>,
> >                               <0x0 0x58000000 0x0 0x08000000>;
> >                 ....
> >         };
> >
> > We provide callbacks to synchronize specific content between memory and
> > cache. We allocate a global DMA coherent pool (which is marked as
> > non-cached using PMA) so that DMA memory allocations happens from this
> > pool and we implement the below callbacks:
> >
> > - arch_sync_dma_for_device()
> > - arch_sync_dma_for_cpu()
>
> These two already exist in arch/riscv/mm/dma-noncoherent.c using the
> alternatives mechanism.
>
I need to investigate if this can be re-used.

> > - arch_dma_alloc()
> > - arch_dma_free()
> >
> > Below are the configs that are enabled:
> >
> > - DMA_GLOBAL_POOL
> > - ARCH_HAS_SYNC_DMA_FOR_CPU
> > - ARCH_HAS_SYNC_DMA_FOR_DEVICE
>
> For these two see:
> arch/riscv/Kconfig & RISCV_DMA_NONCOHERENT
>
Thanks for the pointers, SoCs requiring this would just select
RISCV_DMA_NONCOHERENT

> >
> >         l2cache: cache-controller@13400000 {
> >                 compatible = "andestech,ax45mp-cache", "cache";
> >                 cache-size = <0x40000>;
> >                 cache-line-size = <64>;
> >                 cache-sets = <1024>;
> >                 cache-unified;
> >                 reg = <0x0 0x13400000 0x0 0x100000>;
> >         };
> >
> > Due to the above approach custom SBI calls have been implemented. The
> > above implementation is in preparation for adding support for Renesas
> > RZ/Five SoC which uses the AX45MP core. As with the above approach the
> > kernel image might not be generic so that it can be used on other
> > platforms, so sending it as an RFC (without DT binding patches).
> >
> > OpenSBI implementation isn't upstreamed yet, public repo for access is
> > available at [0].
>
> When you say "isn't upstreamed yet", what is the actual status? Where in
> the process are you or have you not started that yet? Does openSBI even
> allow custom extensions to be upstreamed?
>
TBH we haven't started yet. I'm really new to OpenSBI stuff, not sure
if custom extensions are allowed. I didn't find anything mentioned in
[0]

[0] https://github.com/riscv-software-src/opensbi/blob/master/docs/contributing.md

> >
> > [0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA
> >
> > Cheers,
> > Prabhakar
> >
> > Lad Prabhakar (2):
> >   riscv: vendors: andes: Add support to configure the PMA regions
> >   riscv: vendors: andes: Add support for non-cohernet dma
> >
>
> Anyway, a couple of drive-by comments, having made the wild assumption
> that this can be accepted upstream.
>
:)

> >  arch/riscv/Kbuild                             |   2 +
> >  arch/riscv/include/asm/sbi.h                  |   1 +
> >  arch/riscv/vendors/Makefile                   |   3 +
> >  arch/riscv/vendors/andes/Makefile             |   4 +
> >  arch/riscv/vendors/andes/ax45mp_cache.c       | 296 ++++++++++++++++++
>
> Surely this should be in drivers/soc/andestech, just like the SiFive L2
> controller is in drivers/soc/sifive rather in a subdirectory of the
> arch?
>
Agreed, I will move this to soc dir. maybe instead of andestech,
drivers/soc/renesas/rzfive maybe?

> >  arch/riscv/vendors/andes/ax45mp_nocache_dma.c |  65 ++++
>
> This looks like it should be implemented as errata/alternatives just
> like the non-coherent stuff on the D1 is done.
>
Agreed (I need to do more reading).

> >  arch/riscv/vendors/andes/include/proc.h       |   9 +
>
> And I think this would fall away if implemented as errata/alternatives.
>
I need to investigate it.

> >  arch/riscv/vendors/andes/include/sbi.h        |  27 ++
> >  arch/riscv/vendors/andes/ax45mp.c             |  93 ++++++
>
> idk where this would go though, if it is even something that is
> acceptable, given the policy I linked the other day from:
> https://www.kernel.org/doc/html/latest/riscv/patch-acceptance.html#submit-checklist-addendum
>
In the worst case we could move this to the lower layer to set up the
PMA regions and can drop this from the kernel.

> There is SiFive specific errata but it is implemented using mimpid etc
> rather than compatible/dt. As I said in my initial mails, I am quite
> interested in vendor SBI extensions in the kernel. If you did check out
> the link I sent, our stuff is a world away from yours - it's isolated to
> a driver where we are using SBI ECALLs to communicate with other harts
> which are running something other than Linux in AMP configurations.
> Pretty much we can do everything we want to do without touching a
> single line of code in arch/riscv, so although the statement in that doc
> applies to both of us here it does not apply evenly :s
>
I agree, as your code doesn't touch the core and just can be
implemented as a standalone driver with ecalls.

> It's all a bit unclear to me what the story is here, because obviously
> you are doing things that Zicbo* is meant to do (just like the D1), but
> your hardware's design and initial tapeout predates the existance of
> Zicbom. Makes me start to wonder, what happens for <insert idea> that
> eventually becomes an extension? Where does the line get draw for "you
> did something that is not a ratified extension, therefore you are not
> permitted upstream"? A line obviously does have to be drawn *somewhere*
> and the easiest place to draw that line is "non ratified extensions are
> a no-go". But then again, why allow the D1 but not you?
>
Yes, with a brief look we are doing something similar to things that
have been done for Zicbo*.

> Obviously this is not a runner for someone not using an FPGA or similar,
> but the InterHart Communication IP that we are using the SBI ECALLs for
> is a fabric core, so we (in theory) could re-write it so that instead of
> using an ECALL which routes communication via the e51 "monitor" core we
> _could_ write directly to the registers of the IHC block. There's clear
> security/isolation benefits for doing things via an ECALL which is why
> that method was chosen but if we opened for the direct writes/reads the
> driver would be upstream acceptable...
>
> Don't get me wrong, I completely understand why a policy of not allowing
> extensions that have not been ratified makes sense - *but* at the same
> time if touching arch code is not required it does not feel very much
> different to me than adding a driver for a fabric core in the first
> place. I mentioned this sort of thing a while back on IRC and Jess made
> the point that similar sorts of things are done by some of the Qualcomm
> for their remoteproc as we would be doing for ours with the IHC. In your
> case, if all of your ECALLs are in drivers/soc - the maintainership
> burden for any churn would be on you/Geert etc rather than on the RISC-V
> maintainer.
>
Agreed a policy as such would keep the core code clean for
non-ratified extensions. I was wondering if we could make use of
staging directory for non-ratified extensions as lots of vendors might
want to have their SoCs upstreamed but are waiting for the extensions
to be ratified. And whenever the extensions are ratified we can always
move it to the core?

> TL;DR of that is maybe a more nuanced policy of "no non-ratified
> extensions that touch arch/riscv" could be a possibility but I would
> completely understand if a "what's sauce for the goose is sauce for the
> gander" approach was taken here and a blanket ban remains in place.
>
> As I have said a bunch of times, this is all just my 2 cents etc and I
> am as much of a punter here as you are... but maybe since I am in the
> same sort of boat I at least have a fleshed out opinion. ¯\_()_/¯
>
Thanks for weighing in.

> Hopefully either Palmer can weigh in here or we do get a BoF & the
> chance to have a chat about this sort of thing & maybe have a more
> nuanced policy - or at the very least something that makes it clearer
> that vendor extensions are a complete no-go upstream.
>
Fingers crossed.

Cheers,
Prabhakar
Atish Patra Sept. 9, 2022, 11:06 p.m. UTC | #3
On Thu, Sep 8, 2022 at 2:45 PM <Conor.Dooley@microchip.com> wrote:
>
> Hey,
>
> I had a quick run through this today so if there's discussion
> about this next week I at least will have some idea of what I
> am talking about...
>
> (I ended up not doing a quick run...)
>
> On 06/09/2022 11:21, Lad Prabhakar wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >
> > Hi All,
> >
> > On the Andes AX45MP core, cache coherency is a specification option so it
> > may not be supported. In this case DMA will fail. To get around with this
> > issue this patch series  does the below:
>
> You say "may not be supported" - is it or is it not supported by the
> core on your SoC? Do some of the cheaper SKUs not support it?
>
> From what Biju has said, you need non-coherent DMA for your eMMC, USB
> and ethernet controllers to work? To me, that seems like something that
> would be quite important to point out here..
>
>
> > 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. PMA
> > regions are passed from the cpu core node which are configured as
> > non-cacheable and non-bufferable with the SBI call.
> >
> >         ax45mp: cpu@0 {
> >                 compatible = "andestech,ax45mp", "riscv";
> >                 device_type = "cpu";
> >                 ....
> >                 pma-regions = <0x0 0x00000000 0x0 0x14000000>,
> >                               <0x0 0x20000000 0x0 0x10000000>,
> >                               <0x0 0x58000000 0x0 0x08000000>;
> >                 ....
> >         };
> >
> > We provide callbacks to synchronize specific content between memory and
> > cache. We allocate a global DMA coherent pool (which is marked as
> > non-cached using PMA) so that DMA memory allocations happens from this
> > pool and we implement the below callbacks:
> >
> > - arch_sync_dma_for_device()
> > - arch_sync_dma_for_cpu()
>
> These two already exist in arch/riscv/mm/dma-noncoherent.c using the
> alternatives mechanism.
>
> > - arch_dma_alloc()
> > - arch_dma_free()
> >
> > Below are the configs that are enabled:
> >
> > - DMA_GLOBAL_POOL
> > - ARCH_HAS_SYNC_DMA_FOR_CPU
> > - ARCH_HAS_SYNC_DMA_FOR_DEVICE
>
> For these two see:
> arch/riscv/Kconfig & RISCV_DMA_NONCOHERENT
>
> >
> >         l2cache: cache-controller@13400000 {
> >                 compatible = "andestech,ax45mp-cache", "cache";
> >                 cache-size = <0x40000>;
> >                 cache-line-size = <64>;
> >                 cache-sets = <1024>;
> >                 cache-unified;
> >                 reg = <0x0 0x13400000 0x0 0x100000>;
> >         };
> >
> > Due to the above approach custom SBI calls have been implemented. The
> > above implementation is in preparation for adding support for Renesas
> > RZ/Five SoC which uses the AX45MP core. As with the above approach the
> > kernel image might not be generic so that it can be used on other
> > platforms, so sending it as an RFC (without DT binding patches).
> >
> > OpenSBI implementation isn't upstreamed yet, public repo for access is
> > available at [0].
>
> When you say "isn't upstreamed yet", what is the actual status? Where in
> the process are you or have you not started that yet? Does openSBI even
> allow custom extensions to be upstreamed?
>
> >
> > [0] https://github.com/renesas-rz/rz_opensbi/tree/work/OpenSBI-PMA
> >
> > Cheers,
> > Prabhakar
> >
> > Lad Prabhakar (2):
> >   riscv: vendors: andes: Add support to configure the PMA regions
> >   riscv: vendors: andes: Add support for non-cohernet dma
> >
>
> Anyway, a couple of drive-by comments, having made the wild assumption
> that this can be accepted upstream.
>
> >  arch/riscv/Kbuild                             |   2 +
> >  arch/riscv/include/asm/sbi.h                  |   1 +
> >  arch/riscv/vendors/Makefile                   |   3 +
> >  arch/riscv/vendors/andes/Makefile             |   4 +
> >  arch/riscv/vendors/andes/ax45mp_cache.c       | 296 ++++++++++++++++++
>
> Surely this should be in drivers/soc/andestech, just like the SiFive L2
> controller is in drivers/soc/sifive rather in a subdirectory of the
> arch?
>
> >  arch/riscv/vendors/andes/ax45mp_nocache_dma.c |  65 ++++
>
> This looks like it should be implemented as errata/alternatives just
> like the non-coherent stuff on the D1 is done.
>
> >  arch/riscv/vendors/andes/include/proc.h       |   9 +
>
> And I think this would fall away if implemented as errata/alternatives.
>
> >  arch/riscv/vendors/andes/include/sbi.h        |  27 ++
> >  arch/riscv/vendors/andes/ax45mp.c             |  93 ++++++
>
> idk where this would go though, if it is even something that is
> acceptable, given the policy I linked the other day from:
> https://www.kernel.org/doc/html/latest/riscv/patch-acceptance.html#submit-checklist-addendum
>
> There is SiFive specific errata but it is implemented using mimpid etc
> rather than compatible/dt. As I said in my initial mails, I am quite
> interested in vendor SBI extensions in the kernel. If you did check out
> the link I sent, our stuff is a world away from yours - it's isolated to
> a driver where we are using SBI ECALLs to communicate with other harts
> which are running something other than Linux in AMP configurations.
> Pretty much we can do everything we want to do without touching a
> single line of code in arch/riscv, so although the statement in that doc
> applies to both of us here it does not apply evenly :s
>
> It's all a bit unclear to me what the story is here, because obviously
> you are doing things that Zicbo* is meant to do (just like the D1), but
> your hardware's design and initial tapeout predates the existance of
> Zicbom. Makes me start to wonder, what happens for <insert idea> that
> eventually becomes an extension? Where does the line get draw for "you
> did something that is not a ratified extension, therefore you are not
> permitted upstream"? A line obviously does have to be drawn *somewhere*
> and the easiest place to draw that line is "non ratified extensions are
> a no-go". But then again, why allow the D1 but not you?
>
> Obviously this is not a runner for someone not using an FPGA or similar,
> but the InterHart Communication IP that we are using the SBI ECALLs for
> is a fabric core, so we (in theory) could re-write it so that instead of
> using an ECALL which routes communication via the e51 "monitor" core we
> _could_ write directly to the registers of the IHC block. There's clear
> security/isolation benefits for doing things via an ECALL which is why
> that method was chosen but if we opened for the direct writes/reads the
> driver would be upstream acceptable...
>
> Don't get me wrong, I completely understand why a policy of not allowing
> extensions that have not been ratified makes sense - *but* at the same
> time if touching arch code is not required it does not feel very much
> different to me than adding a driver for a fabric core in the first
> place. I mentioned this sort of thing a while back on IRC and Jess made
> the point that similar sorts of things are done by some of the Qualcomm
> for their remoteproc as we would be doing for ours with the IHC. In your
> case, if all of your ECALLs are in drivers/soc - the maintainership
> burden for any churn would be on you/Geert etc rather than on the RISC-V
> maintainer.
>
> TL;DR of that is maybe a more nuanced policy of "no non-ratified
> extensions that touch arch/riscv" could be a possibility but I would
> completely understand if a "what's sauce for the goose is sauce for the
> gander" approach was taken here and a blanket ban remains in place.
>
> As I have said a bunch of times, this is all just my 2 cents etc and I
> am as much of a punter here as you are... but maybe since I am in the
> same sort of boat I at least have a fleshed out opinion. ¯\_()_/¯
>
> Hopefully either Palmer can weigh in here or we do get a BoF & the
> chance to have a chat about this sort of thing & maybe have a more
> nuanced policy - or at the very least something that makes it clearer
> that vendor extensions are a complete no-go upstream.
>

RISC-V BoF is scheduled on Wednesday.

https://lpc.events/event/16/contributions/1389/

> Conor.
>
> >  9 files changed, 500 insertions(+)
> >  create mode 100644 arch/riscv/vendors/Makefile
> >  create mode 100644 arch/riscv/vendors/andes/Makefile
> >  create mode 100644 arch/riscv/vendors/andes/ax45mp.c
> >  create mode 100644 arch/riscv/vendors/andes/ax45mp_cache.c
> >  create mode 100644 arch/riscv/vendors/andes/ax45mp_nocache_dma.c
> >  create mode 100644 arch/riscv/vendors/andes/include/proc.h
> >  create mode 100644 arch/riscv/vendors/andes/include/sbi.h
> >
> > --
> > 2.25.1
> >
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv