Re: [RESEND PATCH v5 1/2] riscv-platform-spec: PLIC and CLINT for Linux-2022 platform


Abner Chang
 



Alistair Francis <Alistair.Francis@...> 於 2021年5月26日 週三 下午6:57寫道:
On Wed, 2021-05-26 at 10:40 +0000, Anup Patel wrote:
> Hi Alistair,
>
> > -----Original Message-----
> > From: tech-unixplatformspec@... <tech-
> > unixplatformspec@...> On Behalf Of Alistair Francis
> > Sent: 26 May 2021 15:48
> > To: renba.chang@...; tech-unixplatformspec@...
> > Cc: sunilvl@...; mchitale@...
> > Subject: Re: [RISC-V] [tech-unixplatformspec] [RESEND PATCH v5 1/2]
> > riscv-
> > platform-spec: PLIC and CLINT for Linux-2022 platform
> >
> > On Wed, 2021-05-26 at 12:57 +0800, renba.chang@... wrote:
> > > From: Abner Chang <renba.chang@...>
> > >
> > > Initial description of PLIC  CLINT section of Linux-2022 platform.
> > >
> > > On v5 commit,
> > > - Remove CLINT from platform spec
> > > - Require ACLINT on Linux2020 platform and have a link to
> > > https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc
> > > .
> > > - Remove Machine mode timer from previous patch because that is in
> > > the
> > > scope of ACLINT
> > > - For Embedded-2022 platform, mention Machine mode timer and refer
> > > to
> > > ACLINT for the definition of registers
> > >
> > > On v4 commit,
> > > - PLIC section with [DEPRECATED] in Linux- 2022 chapter
> > > - CLINT section in Linux- 2022 chapter for M-mode timer. We don't
> > > mention
> > >   IPI because AIA already supported it.
> > > - In Embedded-2022 Machine mode timer section, CLINT is not
> > > mandatory.
> > > - Separate section in appendix for the Machine mode timer registers
> > >
> > > On v3 commit,
> > > - Address review comments.
> > >
> > > On v2 commit,
> > > - CLINT is not deprecated.
> > >
> > > - Add a standalone section for Machine Mode Timer in System
> > > Peripherals.
> > >   Do you think this is a good place for Machine Mode Timer?
> > >   @Mayuresh, please check if you are ok with this change, not sure
> > > if
> > > this
> > >   overlaps with your text or not (The timer setion). I can remove
> > > this
> > >   if you prefer to put this with your patch.
> > >
> > > - In Embedded-2022, refer to Machine Mode Timer in System
> > > Peripherals
> > >   section and CLINT in Linux-2022 Platform.
> > >   @Alistair, is this ok?
> > >
> > > On v1 commit,
> > > - Not sure where to put the [DEPRECATED].
> > > - Change the reference of PLIC in section 2.2.2. Interrupt
> > > Controller
> > > to
> > >   1.1.3.2 PLIC + CLINT section.
> > >
> > > Signed-off-by: Abner Chang <renba.chang@...>
> > > Cc: Alistair Francis <alistair.francis@...>
> > > Cc: Sunil V L <sunilvl@...>
> > > Cc: Mayuresh Chitale <mchitale@...>
> > > ---
> > >  riscv-platform-spec.adoc | 52 ++++++++++++++++++------------------
> > > --
> > > --
> > >  1 file changed, 23 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
> > > index
> > > 160c74a..db6dce3 100644
> > > --- a/riscv-platform-spec.adoc
> > > +++ b/riscv-platform-spec.adoc
> > > @@ -49,9 +49,23 @@ include::profiles.adoc[]
> > >  * Start Address
> > >
> > >  ==== Interrupt Controller
> > > -* AIA
> > > -* PLIC + CLINT
> > > -* Interrupt Assignments
> > > +===== AIA[[AIA]]
> > > +===== PLIC[DEPRECATED][[PLIC]]
> > > +The Platform Level Interrupt Controller (PLIC) provides facilities
> > > to route
> > > +the non-local interrupts to the external interrupt of a hart
> > > context
> > > +with a given privilege mode in a given hart. The number of non-
> > > local
> > > interrupt
> > > +sources supported by PLIC and how does each of them connect to the
> > > hart
> > > +context is PLIC core implementation-specific. +
> > > +(Refer to
> > > https://github.com/riscv/riscv-plic-spec/blob/master/riscv-plic.adoc[R
> > > ISC-V
> > >  PLIC Specification]
> > > +for the implementation reference of PLIC operation parameters)
> > > +
> > > +===== ACLINT
> > > +Linux-2020 platform requires the Advanced Core Local Interruptor
> > > (ACLINT, refer
> > > +to
> > > https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc[RI
> > > SC-V
> > >  ACLINT Specification])
> > > +to provide facilities to route inter-processor interrupt and
> > > Machine
> > > mode timer
> > > +interrupt to each RISC-V processor hart.
> > > +
> > > +===== Interrupt Assignments
> > >
> > >  ==== System Peripherals
> > >  * UART/Serial Console
> > > @@ -289,9 +303,8 @@ Any RISC-V system that uses at least RV32/64G
> > > can
> > > meet the Embedded-2022
> > >  specification.
> > >
> > >  ==== Interrupt Controller
> > > -Embedded systems are recommended to use a spec compliant
> > > -https://github.com/riscv/riscv-plic-spec[PLIC], a spec compliant
> > > -
> > > https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc[CL
> > > IC]
> > > +Embedded systems are recommended to use a spec compliant
> > > <<PLIC,PLIC>>, a spec
> > > +compliant
> > > https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc[CL
> > > IC]
> > >  or both a CLIC and and PLIC.
> > >
> > >  If using just a PLIC the system must continue to use the original
> > > basic @@ -303,8 +316,8 @@ must be supported.
> > >  Embedded systems cannot use a non-compliant interrupt controller
> > > and
> > > still
> > >  call it a PLIC or CLIC.
> > >
> > > -==== Machine Timer
> > > -The RISC-V machine timer (controlled via `mtime` and `mtimecmp`)
> > > must
> > > be
> > > +==== Machine Mode Timer
> > > +The RISC-V Machine mode timer (controlled via `mtime` and
> > > `mtimecmp`) must be
> > >  implemented. The two registers must be memory mapped as required
> > > by
> > > the RISC-V
> > >  specification.
> > >
> > > @@ -314,27 +327,8 @@ adjacent to each other with the `mtime` region
> > > at
> > > the lower address.
> > >  The starting address of this region can be located anywhere in
> > >  memory, including inside other peripherals, as long as the start
> > > address is
> > >  4 byte aligned.
> > > -
> > > -An example of the memory layout for a 32-bit system with a single
> > > hart is below
> > > -
> > > --------------------------
> > > -=========================
> > > -| 0x00 |  mtime low     |
> > > -| 0x04 |  mtime high    |
> > > -| 0x08 |  mtimecmp low  |
> > > -| 0x0C |  mtimecmp high |
> > > -=========================
> > > --------------------------
> > > -
> > > -and for a 64-bit system with 2 harts
> > > -
> > > ----------------------------
> > > -===========================
> > > -| 0x00 |  mtime           |
> > > -| 0x08 |  mtimecmp hart 1 |
> > > -| 0x10 |  mtimecmp hart 2 |
> > > -===========================
> >
> > You have removed the original  example, but not the original
> > description. If we
> > aren't supporting the adjancent addresses the paragraph above this
> > should be
> > removed.
> >
> > I still prefer to allow either adjacent registers (as currently
> > described) OR the new ACLINT, but if everyone else thinks the ACLINT
> > is the
> > only way to go then that's fine.
>
> The ACLINT MTIMER register organization is intentionally kept
> compatible
> with CLINT timer registers so that ACLINT is backward compatible.

Yep that's great for Linux systems. I haven't seen any embedded systems
besides SiFive that use the CLINT though.

For example SweRVolf uses a specific system controller:
https://github.com/chipsalliance/Cores-SweRVolf/#system-controller

and OpenTitan has it's own timer:
https://docs.opentitan.org/hw/ip/rv_timer/doc/

The ESP32 doesn't even include mtime so that's a different problem.

>
> If you want a different register layout then this has to be defined
> somewhere. The hardware cost of organizing registers like CLINT
> is not much so it's difficult to justify why we want to organize
> registers differently.

For devices that define their own timer I agree that arranging them
like the CLINT shouldn't be a problem. For ones that include the
mtime/mtimecmp in an other system controller I'm worried they will
instead just not try and meet the embedded spec.

We have already defined the layout here, so I don't think it requires
any more work. I agree 100% that it's more of a burden for software to
support two possible timer layouts, but if HW vendors don't opt to
follow the spec then we have to support a lot more.

Anyway, I'm not completely against only allowing ACLINT, I just wanted
to put it out there that it might be difficult for some vendors to
meet. We aren't requiring use of the PLIC or CLIC so it seems a little
strange to then mandate the ACLINT.
 
Alistair,
I can understand your concern just like I have to the server extension. The inflexible spec may lead vendor/OEM hard to follow.
Because the Machine mode timer I added in Linux2022 platform and System Peripherals section is removed from the patch, I will just leave the one in Embedded 2022 section untouched for further discussion.
Seems ACLINT is not designed to give the flexible offsets to mtime and mtimecmp to afford the adjacent register layout unless we introduce a new capability to MTIME device in ACLINT. We probably can work on making MTIME device register layout more flexible in ACLINT spec, I don't have the preference here...and I remember we would like to separate MTIME from CLINT spec, but now it goes into another spec :)

Regards,
Abner 

Alistair

>
> Regards,
> Anup
>
> >
> > Alistair
> >
> > > ----------------------------
> > > +
> > > https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc[RI
> > > SC-V
> > >  ACLINT Specification]
> > > +defines the memory map layout of Machine mode timer registers.
> > >
> > >  ==== Memory Map
> > >  It is recommended that main memory and loadable code (not ROM)
> > > start
> > > at
> >
> >
> >
> >
> >
>

Join tech-unixplatformspec@lists.riscv.org to automatically receive all group messages.