------ Original Message ------
From: "Atish Patra" <Atish.Patra@...>
To: "xypron.glpk@..." <xypron.glpk@...>; "seanga2@..." <seanga2@...>; "mchitale@..." <mchitale@...>
Cc: "tech-unixplatformspec@..." <tech-unixplatformspec@...>
Sent: 4/29/2021 12:30:40 PM
Subject: Re: [RISC-V] [tech-unixplatformspec] [PATCH v1 2/2] Section 3.1.4 System Peripherals.

On Tue, 2021-04-27 at 23:35 -0400, Sean Anderson wrote:

On 4/27/21 7:52 PM, Atish Patra wrote:
> On Mon, 2021-04-26 at 12:56 +0530, Mayuresh Chitale wrote:
> >
> >
> > On Sun, Apr 25, 2021 at 2:55 AM Heinrich Schuchardt <
> > xypron.glpk@...> wrote:
> > > Am 24. April 2021 20:37:58 MESZ schrieb Sean Anderson
> > > <seanga2@...>:
> > > > On 4/11/21 11:02 AM, Mayuresh Chitale wrote:
> > > > > This patch is an initial draft for the section
> > > > > 3.1.4 - System Peripherals.
> > > > >
> > > > > Signed-off-by: Mayuresh Chitale <mchitale@...>
> > > > > ---
> > > > > riscv-platform-spec.adoc | 31
> > > > > +++++++++++++++++++++++++++---
> > > > > -
> > > > > 1 file changed, 27 insertions(+), 4 deletions(-)
> > > > >
> > > > > diff --git a/riscv-platform-spec.adoc b/riscv-platform-
> > > > > spec.adoc
> > > > > index 003181c..f164545 100644
> > > > > --- a/riscv-platform-spec.adoc
> > > > > +++ b/riscv-platform-spec.adoc
> > > > > @@ -52,10 +52,33 @@ include::profiles.adoc[]
> > > > > * Interrupt Assignments
> > > > >
> > > > > ==== System Peripherals
> > > > > -* UART/Serial Console
> > > > > -* Clocks
> > > > > -* Timers
> > > > > -* Watchdog Timers
> > > > > +* *UART/Serial Console* +
> > > > > +In order to facilitate the bringup and debug of the low
> > > > > level
> > > > initial platform
> > > > > +software(firmware, bootloaders, kernel etc), the platform
> > > > > shall
> > > > implement a UART
> > > > > +port compatible with PC16550D.
> > > > > +* *Clock and Timers* +
> > > > > +** Platforms shall provide a 10ns resolution 64-bit
> > > > > counter
> > > with
> > > > strictly monotonic updates.
> > > > > +** The counter shall have a minimum update frequency of
> > > > > 10MHz.
> > > > > +** Platforms shall implement the time CSR.
>
> In order to allow the existing hardware to comply with the
> Linux2022
> spec, we should add additional clarification.
>
> It is recommended that platforms shall implement the time CSR. In
> absence of time CSR, the M-mode software must emulate the time CSR
> read
> by trapping the access. The software emulation approach is
> deprecated
> and will be removed in the next version of the specification.
>
> > > > > +** Platforms shall advertise the timebase to the operating
> > > systems
> > > > via the
> > > > > +`timebase-frequency` DT property.
> > > >
> > > > This is the first mention of DT AFAICT, so we should clarify
> > > > where
> > > it
> > > > comes from and what the relevant specifications are. Should
> > > > we
> > > wait to
> > > > use whatever [1] standardizes on? Will we need to be forward-
> > > compatible
> > > > in this regard?
> > > >
> > > > --Sean
> > > >
> > > > [1] https://github.com/riscv/configuration-structure
> > >
> > > [1] is about saving configuration data in the SoC. It is not
> > > about
> > > the format in which the data is presented by the firmware.
> > >
> > > But mentioning a DT property without a node and a format is
> > > insufficient. If this is defined somewhere else, we need a
> > > reference.
> > >
> >
> >
> > This property is defined in the cpus node for riscv:
> >
> > https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/riscv/cpus.yaml
> > . We can refer to this in the platform spec.
> > >
>
> The larger objective of the platform spec is to serve all the Rich-
> OS
> (Linux/FreeBSD/Windows) in future. That's why, we shouldn't point
> to
> any Linux specific DT bindings from platform spec.
>
> How about rewording it in a way that it is agnostic to OS ?
>
> Platforms shall advertise the timebase to the operating systems via
> the
> appropriate hardware discovery method.

DT or ACPI are the only hw discovery method that platform spec allows.
All the rich operating systems have very mature support for FDT/ACPI.
There is no reason to support yet another hw discovery method via
config.

I this leaving this unspecified makes this standard too weak. There
should be a standard way of discovering how to use a standard CSR.
The
way I see it, there are a few ways of going about this:

I did not suggest to leave it unspecified. Timebase frequency must be
specified in DT/ACPI for operating systems. I was just not sure if
details of the DT binding (fixed by Linux kernel) should be
documented/referred in the platform specification.

I believe we can refer to these and use them as is in the platform spec.
Any addition or modification, if required should to be done in the original source i.e in this case the kernel documentation for the timebase-frequency property.

However, if everybody feels that we should refer to Linux kernel DT
binding in the platform spec, it is fine by me.

* Create a new SBI function to discover the tick rate. This has the
advantage of being very easy to integrate with existing systems.
It
also does not depend on the OS, as all OSs will implement SBI. The
disadvantage is that this is not very extensible. If we want to
expose
further information (what memory is reserved, what devices are in
use,
etc.), adding SBI calls is probably not the best approach.
* Use the RISC-V configuration structure. As I understand it, this
structure is supposed to be flexible enough to encode information
like
what frequency a timer or clock is. I think using the same
structure
for passing information to the supervisor as well as the OS is OK.
Unfortunately, this structure does not exist yet. Perhaps
something
for a future spec.
* Allow for multiple configuration structures. Fow now, we can just
specify what properties to use when using devicetree for
configuration. But we should also allow other configuration
structures
to be specified in the future. One question is whether the OS
should
specify which configuration it expects, or whether the supervisor
should provide something and the OS must accept multiple
configuration
structures. I think the OS specifying the config probably makes
the
most sense, but it could complicate the bootloader/supervisor.

--Sean

>
> > > Best regards
> > >
> > > Heinrich
> > >
> > > >
> > > > > +** Platforms shall implement the
> > > > > +
> > > > > https://lists.riscv.org/g/tech-privileged/message/404[Sstc]
> > > > extension.
> > > > > +** Platforms shall delegate the supervisor timer interrupt
> > > > > to
> > > 'S'
> > > > mode and if
> > > > > +the 'H' extension is implemented, the virtual supervisor
> > > > > timer
> > > > interrupt to 'VS' mode.
> > > > > +
> > > > > +* Watchdog Timers +
> > > > > +** Platforms shall implement a two stage watchdog timer.
> > > > > +** The software shall periodically update the watchdog
> > > > > stage 1
> > > value
> > > > to a
> > > > > +future moment.
> > > > > +** If the mtime/time increments past the watchdog stage 1
> > > compare
> > > > value then an
> > > > > +interrupt shall be raised and routed to a RISC V hart and
> > > > > the
> > > > watchdog stage 2
> > > > > +shall be activated.
> > > > > +** If the mtime/time increments past the watchdog stage 2
> > > compare
> > > > value an
> > > > > +interrupt shall be raised and routed to an external
> > > > > hardware
> > > unit
> > > > such as a
> > > > > +BMC to affect a system reset. If the watchdog stage1
> > > > > compare
> > > value
> > > > was updated
> > > > > +before such an event occurred then the watchdog stage 1
> > > interrupt
> > > > shall be
> > > > > +deasserted and the watchdog stage 2 shall be deactivated.
> > > > >
> > > > > ==== Boot Process
> > > > > * Firmware
> > > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
> >
>

--
Regards,
Atish