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


Anup Patel
 

Hi Abner,

 

Yesterday, we move ACLINT spec under the RISC-V International GitHub.

 

New link of the ACLINT spec is:

https://github.com/riscv/riscv-aclint/blob/master/riscv-aclint.adoc

 

You can even clone https://github.com/riscv/riscv-aclint.git and build a PDF version of the ACLINT spec.

 

Regards,

Anup

 

From: tech-unixplatformspec@... <tech-unixplatformspec@...> On Behalf Of Abner Chang
Sent: 25 May 2021 09:47
To: Alistair Francis <Alistair.Francis@...>
Cc: sunilvl@...; tech-unixplatformspec@...; mchitale@...
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH v4 1/2] riscv-platform-spec: PLIC and CLINT for Linux-2022 platform

 

 

 

Alistair Francis <Alistair.Francis@...> 2021524 週一 下午3:23寫道:

On Mon, 2021-05-24 at 14:52 +0800, Abner Chang wrote:
>
>
> Alistair Francis <Alistair.Francis@...> 2021524 週一
> 上午11:42寫道:
> > On Mon, 2021-05-24 at 11:03 +0800, Abner Chang wrote:
> > > From: Abner Chang <renba.chang@...>
> > >
> > > Initial description of PLIC  CLINT section of Linux-2022
> > > platform.
> > >
> > > Is this what we want to see of CLINT/Machine mode timer in the
> > > platform spec?
> > >
> > > 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 | 104 +++++++++++++++++++++++----------
> > > --
> > --
> > > --
> > >  1 file changed, 62 insertions(+), 42 deletions(-)
> > >
> > > diff --git a/riscv-platform-spec.adoc b/riscv-platform-spec.adoc
> > > index 160c74a..f8838ab 100644
> > > --- a/riscv-platform-spec.adoc
> > > +++ b/riscv-platform-spec.adoc
> > > @@ -49,15 +49,37 @@ 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[RISC-V
> > >  PLIC Specification]
> > > +for the implementation reference of PLIC operation parameters)
> > > +
> > > +===== CLINT[[CLINT]]
> > > +On the contrast to PLIC, the Core Local Interrupt (CLINT)
> > > provides
> >
> > I don't think this is a contrast to the PLIC, it's just a different
> > functionality.
> >
>
> Can rephase it. 
> >
> > I'm also not sure the CLINT should be in the interrupt controller
> > section. It is the Core Local INTerruptor (CLINT), not an interrupt
> > controller.
> >
>
> Hmm.. CLINT is not a sort of interrupt controller but seems to me it
> is unnecessary to create another section apart from the Interrupt
> Controller section.

It probably makes more sense to add a timer section and put the CLNIT
there instead of forcing it into the interrupt controller section, when
it isn't really an interrupt controller.

> >
> > > +facilities to trigger local interrupt of
> > <<MachineModeTimer,Machine
> > > mode timer>> to hart.
> > > +
> > > +===== Interrupt Assignments
> > >  
> > >  ==== System Peripherals
> > > -* UART/Serial Console
> > > -* Clocks
> > > -* Timers
> > > -* Watchdog Timers
> > > +===== UART/Serial Console
> > > +===== Clocks
> > > +===== Timers
> > > +
> > > +===== Machine Mode Timer[[MachineModeTimer]]
> > > +Machine mode timer is required for Linux-2022 platform and
> > > incorporate with
> > > +<<CLINT,CLINT>> to trigger the timer event to hart. The format
> > > of
> > > Machine mode timer operation parameters
> > > +(`mtime` and `mtimecmp` registers) must compliant with RISC-V
> >
> > s/must compliant/must be compliant/g
> >
>
> will correct this 
> >
> > > Privilege specification section 3.1.10.
> > > +The base address of the memory map registers of Machine mode
> > > timer
> > > is platform implementation-specific,
> > > +however the offset of `mtime` and `mtimecmp` registers are fixed
> > as
> > > +<<machineModeTimerRegs,Machine mode timer registers>>.
> > > +
> > > +
> > > +===== Watchdog Timers
> > >  
> > >  ==== Boot Process
> > >  * Firmware
> > > @@ -289,9 +311,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[CLIC]
> > > +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[CLIC]
> > >  or both a CLIC and and PLIC.
> > >  
> > >  If using just a PLIC the system must continue to use the
> > > original
> > > basic
> > > @@ -303,38 +324,9 @@ 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
> > > -implemented. The two registers must be memory mapped as required
> > by
> > > the RISC-V
> > > -specification.
> > > -
> > > -The Embedded-2022 specification requires that the registers be
> > > mapped
> > > -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 |
> > > -===========================
> > > ----------------------------
> > > +==== Machine Mode Timer
> > > +The Embedded-2022 specification requires RISC-V
> > > <<MachineModeTimer,Machine mode timer>> to be implemented.
> > > +The Machine mode timer could be incorporated with the Core Local
> > > Interrupt (<<CLINT, CLINT>>) or the specific event trigger
> > mechanism
> >
> > CLINT is INTerruptor not just Interrupt.
> >
>
> yes, will correct it
> >
> > > according to the platform design.
> >
> > What specific trigger mechanism?
> >
>
> platform implementation-specific trigger mechanism?  or just remove

I meant you didn't say what the mechanism is. It should be explicit, so
that there are no ambiguities.

> this? I put this just because the previous feedback says a small
> system may not have CLINT for the m-mode timer.

I would be in favour of saying either do what we already have written
here OR a CLINT.

If we go with something else that's ok, we just need to be clear about
what it is.

> >
> > >  
> > >  ==== Memory Map
> > >  It is recommended that main memory and loadable code (not ROM)
> > start
> > > at
> > > @@ -349,5 +341,33 @@ When PMP is supported it is recommended to
> > > include at least 4 regions, although
> > >  if possible more should be supported to allow more flexibility.
> > > Hardware
> > >  implementations should aim for supporting at least 16 PMP
> > > regions.
> > >  
> > > +// Appendix sections
> > > +== Appendix
> > > +=== Machine Mode Timer Registers [[MachineModeTimerRegs]]
> > > +The base address of Machine mode timer operation parameters is
> > > platform implementation-specific. The Machine mode timer compare
> > > registers (`mtimecmp`) for each hart is located at offset 0 and
> > > the
> > > layout is organized as below table.
> >
> > Can we maintain what we have for the embedded spec?
> >
>
> Do you mean to maintain the register layout(offset) originally
> defined in the 2.2.3. for embedded systems? 

Yes. I worry that mandating a CLINT layout is too restrictive and that
no one will end up following the embedded-2022 spec then. I would be in
favour of saying CLINT or the memory layout we already have.

> I think we have the register layout as below to be compliant with the
> m-mode timer registers defined in SiFive CLINT.

I don't think that's flexible enough and that vendors won't want to
follow it as they already have other IP they are using for timers.

Alistair

 

I will remove CLINT from this patch. 

@Anup, how do you define the MTIME registers in ACLINT? Can we just define the base address for both mtimecmp and mtime thus we can have 0 based offset?

 

Abner

 


> Or do you mean something else?
>
> >  It allows the
> > memory maps to be inside of a MMIO device. For example they can be
> > registers inside of a general system controller.
> >
> >
> > Also it looks like the lines are a little long here, can be bring
> > them
> > back to 80 charecters?
> >
>
> sure. 
> >
> > > +
> > > +.Registers layout of mtimecmp
> > > +[width="60%",cols="1,^3,^3"]
> > > +|=======
> > > +|*Offset*|*Register (8-byte) for RV64*|*Register (4-byte) for
> > RV32*
> > > +|`0x0000` |mtimecmp for hart 0 |mtimecmp low for hart 0
> > > +|`0x0004` ||mtimecmp high for hart 0
> > > +|`0x0008` |mtimecmp for hart 1 |mtimecmp low for hart 1
> > > +|`0x000c` ||mtimecmp high for hart 1
> > > +|... ||
> > > +|`0x7ff0` |mtimecmp for hart 4094|mtimecmp low for hart 4094
> > > +|`0x7ff4` ||mtimecmp high for hart 4094
> > > +|=======
> > > +
> > > +The Machine mode timer counter (`mtime`) is located at offset
> > 0x7ff8
> > > from the base address of operation parameters.
> > > +
> > > +.Registers layout of mtime
> > > +[width="60%",cols="1,^3,^3"]
> > > +|=======
> > > +|*Offset*|*Register (8-byte) for RV64*|*Register (4-byte) for
> > RV32*
> > > +|`0x7ff8` |mtime|mtime low
> > > +|`0x7ffc` ||mtime high
> > > +|=======
> >
> > Are you saying for a small embedded single core CPU we need to put
> > mtimecmp at offset 0x00 and mtime at 0x7ff8? What about all the
> > addresses between them?
> >
> > Alistair
> >
> > > +
> > >  // acknowledge all of the contributors
> > >  include::contributors.adoc[]
> >

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