Message-ID: <50559ab9-c537-4796-9538-495bf3d14069@riscstar.com>
Date: Sun, 28 Dec 2025 17:50:22 -0600
From: Alex Elder <elder@...cstar.com>
To: Guodong Xu <guodong@...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 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.

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