Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa

[Conor Dooley]

Been implementing feedback, so going back through this

On Tue, Jun 27, 2023 at 12:30:25PM +0100, Conor Dooley wrote:
> On Mon, Jun 26, 2023 at 11:35:10PM -0700, Atish Patra wrote:
> > On Mon, Jun 26, 2023 at 5:40 PM Stefan O'Rear <sorear@fastmail.com> wrote:
> > > On Mon, Jun 26, 2023, at 6:10 AM, Conor Dooley wrote:
> 
> > > > Off-list, some of the RVI folks have committed to shoring up the wording
> > > > in either the ISA specifications, the riscv-isa-manual or
> > > > so that in the future, modifications to and additions or removals of
> > > > features will require a new extension. Codifying that assertion
> > > > somewhere would make it quite unlikely that compatibility would be
> > > > broken, but we have the tools required to deal with it, if & when it
> > > > crops up.
> > > > It is in our collective interest, as consumers of extension meanings, to
> > > > define a scheme that enforces compatibility.
> > > >
> > > > The use of individual properties, rather than elements in a single
> > >
> > > no longer individual properties
> > >
> > > > string, will also permit validation that the properties have a meaning,
> > > > as well as potentially reject mutually exclusive combinations, or
> > > > enforce dependencies between extensions. That would not have be possible
> > >
> > > Under what circumstances is a device tree which declares support for a
> > > superset extension (e.g. m) required to also declare support for its 
> > > subsets
> > > (e.g. zmmul)?  There are compatibility issues in both directions.
> > >
> > > Proposal: If an extension X is a superset of an extension Y and X is 
> > > present
> > > in riscv,isa-extensions, Y must also be present if Y was ratified or added
> > > to the schema before X, but need not also be present if Y was ratified 
> > > after
> > > or at the same time as X.  If X "depends on" Y, then Y must be present in
> > > riscv,isa-extensions even if X and Y were ratified at the same time.
> 
> Yes, I think that this all makes sense from a compatibility point of
> view. Splitting it up:
> 
> > > If an extension X is a superset of an extension Y and X is present
> > > in riscv,isa-extensions, Y must also be present if Y was ratified or added
> > > to the schema before X
> 
> This makes total sense from a "being nice to" software point of view.
> 
> > > but need not also be present if Y was ratified after
> > > or at the same time as X.
> 
> It may make sense to reduce this to only after, or not permit the
> supersets at all where they are ratified alongside their subsets.
> Cross that bridge when we come to it perhaps.

I ending up doing some proof of concept implementation of this for linux
the other day, I think "at or at the same time" is the way to go. Up to
me to enforce while reviewing binding patches I guess!

> > > If X "depends on" Y, then Y must be present in
> > > riscv,isa-extensions even if X and Y were ratified at the same tim
> 
> For Linux, this is already the case for F & D. I think that's a good
> policy to follow.

> > > > +        - const: i
> > > > +          description: |
> > > > +            The base integer instruction set, as ratified in the
> > > > 20191213
> > > > +            version of the unprivileged ISA specification, with the
> > > > exception of
> > > > +            counter access.
> > > > +            Counter access was removed after the ratification of the
> > > > 20191213
> > > > +            version of the unprivileged specification and shunted into
> > > > the
> > > > +            Zicntr and Zihpm extensions.
> > >
> > > I think this may belong in the description of zicsr?  rdcycle in 20191213
> > > is a special case of csrrs, which is in zicsr not the base.
> 
> Sorry, this is a bit unclear. Do you mean that the sentence you have
> provided here should be in the Zicsr entry?

I went and checked this, rdcycle appears in chapter 10 "Counters", not
chapter 9 "Zicsr". I'll slightly reword it & put it in both sections
since the specs are (IMO) unclear in this regard.

Cheers,
Conor.

signature.asc
Description: PGP signature