Message-ID: <CAH1PCMatKR4rHuBdw0fih5P8naE=KU4Vp5-KNjeDeO-LsEe81g@mail.gmail.com>
Date: Sun, 28 Dec 2025 12:10:08 +0800
From: Guodong Xu <guodong@...cstar.com>
To: Alex Elder <elder@...cstar.com>
Cc: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Paul Walmsley <pjw@...nel.org>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Alexandre Ghiti <alex@...ti.fr>, Yixun Lan <dlan@...too.org>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Thomas Gleixner <tglx@...utronix.de>, 
	Samuel Holland <samuel.holland@...ive.com>, Anup Patel <anup@...infault.org>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Jiri Slaby <jirislaby@...nel.org>, 
	Lubomir Rintel <lkundrak@...sk>, Yangyu Chen <cyy@...self.name>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Conor Dooley <conor@...nel.org>, 
	Heinrich Schuchardt <xypron.glpk@....de>, Kevin Meng Zhang <zhangmeng.kevin@...ux.spacemit.com>, 
	Andrew Jones <ajones@...tanamicro.com>, devicetree@...r.kernel.org, 
	linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	spacemit@...ts.linux.dev, linux-serial@...r.kernel.org
Subject: Re: [PATCH v2 08/13] dt-bindings: riscv: Add descriptions for Za64rs,
 Ziccamoa, Ziccif, and Zicclsm

Hi, Alex, Conor

On Sat, Dec 27, 2025 at 5:28 AM Alex Elder <elder@...cstar.com> wrote:
>
> On 12/22/25 7:04 AM, Guodong Xu wrote:
> > Add descriptions for four extensions: Za64rs, Ziccamoa, Ziccif, and
> > Zicclsm. These extensions are ratified in RISC-V Profiles Version 1.0
> > (commit b1d806605f87 "Updated to ratified state.").
>
> I think stating the RISC-V profiles commit ID here (in the commit
> header) is good.
>
> I do *not* think it's necessary to include it in the descriptions
> for the extensions, below, but I seem to be late to the party in
> expressing this opinion...
>
> That commit ID is related to this repository:
>    https://github.com/riscv/riscv-profiles.git
>
> I have a few other comments below but generally I think what you
> did looks good.  I have one overall question though.
>
> > They are introduced as new extension names for existing features and
> > regulate implementation details for RISC-V Profile compliance. According
> > to RISC-V Profiles Version 1.0 and RVA23 Profiles Version 1.0, they are

Thank you for the review.

Together with the filenames, I also listed the Version numbers.

These are officially released versions of profile documents. I mean they
won't be changed without modifying the version number.

> > mandatory for the following profiles:
> >
> >   - za64rs: Mandatory in RVA22U64, RVA23U64
> >   - ziccamoa: Mandatory in RVA20U64, RVA22U64, RVA23U64
> >   - ziccif: Mandatory in RVA20U64, RVA22U64, RVA23U64
> >   - zicclsm: Mandatory in RVA20U64, RVA22U64, RVA23U64
>
> I did not verify your statements about where these are
> optional and mandatory, but I assume they're correct.

Yes they are correct. As far as what stated in the two profile documents.

>
> > Since Ziccamoa depends on the 'A' extension, add a schema check to
> > enforce this dependency.
>
> All of these extensions are related to atomic operations, right?
> Don't *all* of them (not just Ziccamoa) depend on the A extension?


Za64rs and Zicclsm: no, they are not 'A'. They are cache related.

Ziccrse and Ziccamoa: yes, they are 'A' related.

Ziccrse specifies the main memory must support "RsrvEventual", which is one
(totally there are four) of the support level for Load-Reserved/
Store-Conditional (LR/SC) atomic instructions.

And in RVA profiles, two named features (exts) are added:
Ziccrse: which further define the level of LR/SC operations being supported.
Ziccamoa: which further define the level of AMOs instructions being supported.


We already know that "A" = Zaamo + Zalrsc;

In summary, the dependencies among these extensions are:
Ziccrse -> Zalrsc -> A;
Ziccamoa -> Zaamo -> A;

> Furthermore, the A extension is already mandated by RVA23U64, so
> is it really necessary to add this logic?

Hi, Conor

What do you think? I am kind of agree with Alex to remove the schema
checking logic.

Leaving the dependency check to riscv/cpufeature.c, let the .validate call
do the job. If you agree, I can remove the schema checking logic on Ziccamoa
and A in my next version.

Btw, cpufeature.c validate() deserves another patch/patchset.
I'll be happy to add that if we reach a consensus here.

>
>
> > Signed-off-by: Guodong Xu <guodong@...cstar.com>
> > ---
> > v2: New patch.
> > ---
> >   .../devicetree/bindings/riscv/extensions.yaml      | 34 ++++++++++++++++++++++
> >   1 file changed, 34 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > index 385e1deb23996d294e7662693f1257f910a6e129..a6b9d7e3edf86ecfb117ba72e295ef097bdc9831 100644
> > --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
> > @@ -237,6 +237,12 @@ properties:
> >               as ratified at commit 4a69197e5617 ("Update to ratified state") of
> >               riscv-svvptc.
> >
> > +        - const: za64rs
> > +          description:
> > +            The standard Za64rs extension for reservation set size of at most
> > +            64 bytes, as ratified in RISC-V Profiles Version 1.0, with commit
> > +            b1d806605f87 ("Updated to ratified state.")
>
> The more complete description says:
>
>      Reservation sets are contiguous, naturally aligned, and a maximum
>      of 64 bytes.
>
> But as I read on (below) I suppose using the more succinct description
> from the glossary might be best, forcing people who care to go look

That is exactly what I am doing.

> at the reference documents.
>
> > +
> >           - const: zaamo
> >             description: |
> >               The standard Zaamo extension for atomic memory operations as
> > @@ -378,6 +384,27 @@ properties:
> >               in commit 64074bc ("Update version numbers for Zfh/Zfinx") of
> >               riscv-isa-manual.
> >
> > +        - const: ziccamoa
> > +          description:
> > +            The standard Ziccamoa extension for main memory (cacheability and
> > +            coherence) must support all atomics in A, as ratified in RISC-V
> > +            Profiles Version 1.0, with commit b1d806605f87 ("Updated to
> > +            ratified state.")
>
> Similar comment here (but also with a similar caveat):
>

I am using what the RVA23 Profile defines:
"Ziccamoa: Main memory supports all atomics in A"

I prefer to keep it as is.

BR,
Guodong

>    Main memory regions with both the cacheability and coherence PMAs
>    must support all atomics in A.
>
> And I might say "the A extension", but maybe that's a bad idea.
>
> > +
> > +        - const: ziccif
> > +          description:
> > +            The standard Ziccif extension for main memory (cacheability and
> > +            coherence) instruction fetch atomicity, as ratified in RISC-V
> > +            Profiles Version 1.0, with commit b1d806605f87 ("Updated to
> > +            ratified state.")
> > +
> > +        - const: zicclsm
> > +          description:
> > +            The standard Zicclsm extension for main memory (cacheability and
> > +            coherence) must support misaligned loads and stores, as ratified
> > +            in RISC-V Profiles Version 1.0, with commit b1d806605f87 ("Updated
> > +            to ratified state.")
> > +
> >           - const: ziccrse
> >             description:
> >               The standard Ziccrse extension which provides forward progress
> > @@ -795,6 +822,13 @@ properties:
> >           then:
> >             contains:
> >               const: f
> > +      # Ziccamoa depends on A
>
> Maybe more than just depends on the A extension.
>
>                                         -Alex
>
> > +      - if:
> > +          contains:
> > +            const: ziccamoa
> > +        then:
> > +          contains:
> > +            const: a
> >         # Zvfbfmin depends on V or Zve32f
> >         - if:
> >             contains:
> >
>

Powered by blists - more mailing lists