Message-ID: <CAH1PCMYX8RSy6pdUwANycxzjX_sfrnvjOFVysAFMFsv2Q8P3jw@mail.gmail.com>
Date: Tue, 30 Dec 2025 08:56:54 +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

On Mon, Dec 29, 2025 at 7:50 AM Alex Elder <elder@...cstar.com> wrote:
>
> On 12/27/25 10:10 PM, Guodong Xu wrote:
> > 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.
>
> Isn't a Za64rs reservation set related to atomic operations,
> though?  They are related to load-reserved/store conditional
> instructions, which are introduced in the atomic instructions
> section.

Yes, you are right. Let me update the relationship:
Za64rs defines a hard's reservation set implementation detail.

It is consumed by two extensions: Zalrsc and Zawrs.

Za64rs -> Zalrsc -> A
Za64rs -> Zawrs -> Zalrsc -> A

I would say maybe all these relationships deserve to be validated in
cpufeature.c (so far, they are not.)

BR,
Guodong Xu

>
> I was mistaken about Zicclsm, that's related to unaligned
> accesses, but does not apply to atomic operations.
>
> > 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.
>
> Yes I think this is a better way to handle it.  Conor?
>
> > Btw, cpufeature.c validate() deserves another patch/patchset.
> > I'll be happy to add that if we reach a consensus here.
>
> Yes I think you should do this once there is concensus.
>
>
>
> >
> >>
> >>
> >>> 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.
>
> That's fine.  I don't think I felt strongly about any of the things
> I said about the exact wording used here.
>
> Thanks.
>
>                                         -Alex
>
> > 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