[RFC,0/6] Deprecate riscv,isa DT property?

From: Conor Dooley <conor.dooley@microchip.com>

Yo,

So here's some bits that I have been poking at on top of the recent bits
of ISA string parser work:
https://lore.kernel.org/linux-riscv/20230504-divisive-unsavory-5a2ff0c3c2d1@spud/

TL;DR is that I do not trust the riscv,isa property to carry sufficient
information to not cause us problems in the future.

Note that this is a very very early RFC, and the implementation etc is
intended to be *demonstrative* rather than acceptable.

Problem
=======

I've been kinda triggered by the whole "Zicsr and Zifencei are not part
of i" thing, where the dt-binding was defined prior to that split and
thus `i` means `zicsr_zifencei` without a real way to differentiate
between the two. From a Linux kernel point of view, it's "fine" because
we require require Zicsr and Zifencei, so a system without them will not
get far enough along for this problem to even manifest - but that's just
the example that we already have in front of us & we don't know what
might be done in the future when it comes to backwards-compatilibty
issues.

Yes you might say, expand the dt-binding to allow the version numbers,
as the Linux kernel's parser already supports strings containing the
version number (although it just ignores them). But that may not be the
case for any other consumer of the riscv,isa property - and such an
expansion of the dt-binding may actually cause them problems. A valid
parser for the current dt-binding may very well fall over if it is
expanded to allow free-form numbering.

Secondly, it is not realistic to maintain a list of every possible
version that someone may insert for every extension to do an explicit
comparison, nor can we rely on RVI interpreting "backwards compatible"
in a way that means software intended for the older version will still
run. (Or for that matter, can we rely on vendors *at all*).
If we could rely on that, then we could at least read "x2p2" and realise
that we can fall back to "x2p0", but I don't think we have that luxury.

The other thought I had was that perhaps some software may choose not to
implement version x.y.0 of an extension and only support x.z.0, z > y
for some reason. We'd want to refuse that extension if the extension is
found, but the version is not listed as being something compatible with
x.z.0, and while the ISA spec does say that the default assumption is
2p0 for unversioned extensions in its current form, I struggle to
extrapolate that to extensions not currently part of the unpriv spec,
but rather defined on their own.

Proposal
========

Instead, I propose a per-extension key/value property, for example
riscv,isa-extension-v = "v1.0.0"
in the style of compatible strings - so the value is not what hardware
implements, but rather the minimum-known version for which the
programming model is compatible.
Until something comes along that is not compatible with v1.0.0 that we
want to support in the kernel, no new keys need to be added to the
kernel, just changes to the dt-binding.

The binding for it is currently set up so that either you need to
be compatible version with v1.0.0, or add a special case. Although
v1.0.0 in this case is just a placeholder number, it could be v2.0.0 or
any other number. It could even be "initial" to something like that, to
match against whatever the first released spec version is.

	As an aside, the dt-binding doesn't actually work properly for
	enforcement etc at present, but I wanted to get some feedback
	before going too far down the rabbit hole there.

This method gives us the implemented version -> compatible version "for
free", as it is done by the creator of the DT, rather than software
running on the platform.
We can hopefully be strict about what people are inserting version wise,
using dt-validate, rather than it being pot-luck as to what gets filled in,
if anything.
I'm very reluctant to add more complexity to the property, and therefore
parsers, when a key-value type interface is more easily used with
standard OF functions - of_property_present(), of_property_read_string()
etc to use the Linux kernel's examples.

Another benefit of this approach is that we, by way of the dt-binding,
control the meaning of the versions.
If a vendor decides to release something using Xfoo, but provides no
version information, we can then assign one ourselves in case Xfoo in
their next SoC is not quite the same. Something similar came up this
morning, talking with Heiko about the TH1520, and how to explain the
meaning of "_xfoo" properties in "riscv,isa". The ISA spec documentation
is pointed to by the binding for that, but vendor properties are
obviously not described there. At the expensive of bloating the binding
file, the proposed scheme would provide a way to stably document vendor
properties.

I guess I am trying to design in some flexibility rather than two years
down the line realise that the isa string is a source of problems, and
have to try and retrofit something in.

I would like to encourage people to populate their DT with every
extension under the sun that they support, even if software doesn't use
it right now (look at the starfive folk that didn't add the bitmanip
until told to) so that if/when it is used in the future these boards
will pick up the support automagically.

ACPI
====

This whole proposal is written for a pre-ACPI world, and I have yet to
give any thought to how such a key-value interface would work there.
I'm not really sure how to deal with that, given they have some ECR
process yada yada, but thoughts on that side of things would be very
much appreciated.

Why x.y.z rather than x.y per the ISA specs?
============================================

I said the same, Palmer wanted x.y.z. For example, the T-HEAD vector stuff
is 0.7.1 & he cited an example (that now eludes me) of a breaking change
in an extension between 1.0 and 1.0.1. God knows how vendors will choose
to version things, so having the extra level is likely advantageous.

Other stuff
===========

The code here is very much in an RFC state. I tested it on an Icicle kit
as a PoC - and it does work, but I have not even remotely tested it
sufficiently.

The dt-binding changes need to be worked on as they do not actually
enforce anything!

I've intentionally only send this to the linux lists, despite this
having wider impact, as it is in a very early state & there's no point
involving all & sundry if the idea is hated.
If it is not universally derided, I will send the binding patches to
various other lists also.

What do I hate about this?
==========================

I fear bloat in the dt-binding and devicetrees as properties are added
mostly. Depending on what I have to do to get enforcement with
dt-validate, a complicated binding is also a concern.

Suggestions etc very much welcome :)

Cheers,
Conor.

CC: Rob Herring <robh+dt@kernel.org>
CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
CC: Conor Dooley <conor+dt@kernel.org>
CC: Palmer Dabbelt <palmer@dabbelt.com>
CC: Paul Walmsley <paul.walmsley@sifive.com>
CC: Heiko Stuebner <heiko@sntech.de>
CC: Andrew Jones <ajones@ventanamicro.com>
CC: Sunil V L <sunilvl@ventanamicro.com>
CC: Yangyu Chen <cyy@cyyself.name>
CC: devicetree@vger.kernel.org
CC: linux-riscv@lists.infradead.org

Conor Dooley (6):
  dt-bindings: riscv: clarify what an unversioned extension means
  dt-bindings: riscv: add riscv,isa-extension-* property and
    incompatible example
  RISC-V: deprecate riscv,isa & replace it with per-extension properties
  RISC-V: add support for riscv,isa-base property
  RISC-V: drop a needless check in print_isa_ext()
  riscv: dts: microchip: use new riscv,isa-extension-* properties for
    mpfs

 .../devicetree/bindings/riscv/cpus.yaml       |  64 +++++-
 arch/riscv/boot/dts/microchip/mpfs.dtsi       |  42 +++-
 arch/riscv/include/asm/hwcap.h                |  29 ++-
 arch/riscv/kernel/cpu.c                       | 124 +++---------
 arch/riscv/kernel/cpufeature.c                | 188 +++++++++++++++---
 5 files changed, 316 insertions(+), 131 deletions(-)

On Mon, May 8, 2023 at 11:20 AM Conor Dooley <conor@kernel.org> wrote:
>
> From: Conor Dooley <conor.dooley@microchip.com>
>
> Yo,
>
> So here's some bits that I have been poking at on top of the recent bits
> of ISA string parser work:
> https://lore.kernel.org/linux-riscv/20230504-divisive-unsavory-5a2ff0c3c2d1@spud/
>
> TL;DR is that I do not trust the riscv,isa property to carry sufficient
> information to not cause us problems in the future.
>
> Note that this is a very very early RFC, and the implementation etc is
> intended to be *demonstrative* rather than acceptable.
>
> Problem
> =======
>
> I've been kinda triggered by the whole "Zicsr and Zifencei are not part
> of i" thing, where the dt-binding was defined prior to that split and
> thus `i` means `zicsr_zifencei` without a real way to differentiate
> between the two. From a Linux kernel point of view, it's "fine" because
> we require require Zicsr and Zifencei, so a system without them will not
> get far enough along for this problem to even manifest - but that's just
> the example that we already have in front of us & we don't know what
> might be done in the future when it comes to backwards-compatilibty
> issues.
>
> Yes you might say, expand the dt-binding to allow the version numbers,
> as the Linux kernel's parser already supports strings containing the
> version number (although it just ignores them). But that may not be the
> case for any other consumer of the riscv,isa property - and such an
> expansion of the dt-binding may actually cause them problems. A valid
> parser for the current dt-binding may very well fall over if it is
> expanded to allow free-form numbering.
>
> Secondly, it is not realistic to maintain a list of every possible
> version that someone may insert for every extension to do an explicit
> comparison, nor can we rely on RVI interpreting "backwards compatible"
> in a way that means software intended for the older version will still
> run. (Or for that matter, can we rely on vendors *at all*).
> If we could rely on that, then we could at least read "x2p2" and realise
> that we can fall back to "x2p0", but I don't think we have that luxury.
>
> The other thought I had was that perhaps some software may choose not to
> implement version x.y.0 of an extension and only support x.z.0, z > y
> for some reason. We'd want to refuse that extension if the extension is
> found, but the version is not listed as being something compatible with
> x.z.0, and while the ISA spec does say that the default assumption is
> 2p0 for unversioned extensions in its current form, I struggle to
> extrapolate that to extensions not currently part of the unpriv spec,
> but rather defined on their own.
>

That's a fair point. However, any new RVI ISA extension will only have v1.0
as per my knowledge. Any new feature will have to be part of a
different extension.
At least that was the plan discussed last year.

https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983222655

Are you aware of any discussion that changes this ?

> Proposal
> ========
>
> Instead, I propose a per-extension key/value property, for example
> riscv,isa-extension-v = "v1.0.0"
> in the style of compatible strings - so the value is not what hardware
> implements, but rather the minimum-known version for which the
> programming model is compatible.
> Until something comes along that is not compatible with v1.0.0 that we
> want to support in the kernel, no new keys need to be added to the
> kernel, just changes to the dt-binding.
>
> The binding for it is currently set up so that either you need to
> be compatible version with v1.0.0, or add a special case. Although
> v1.0.0 in this case is just a placeholder number, it could be v2.0.0 or
> any other number. It could even be "initial" to something like that, to
> match against whatever the first released spec version is.
>
>         As an aside, the dt-binding doesn't actually work properly for
>         enforcement etc at present, but I wanted to get some feedback
>         before going too far down the rabbit hole there.
>
> This method gives us the implemented version -> compatible version "for
> free", as it is done by the creator of the DT, rather than software
> running on the platform.
> We can hopefully be strict about what people are inserting version wise,
> using dt-validate, rather than it being pot-luck as to what gets filled in,
> if anything.
> I'm very reluctant to add more complexity to the property, and therefore
> parsers, when a key-value type interface is more easily used with
> standard OF functions - of_property_present(), of_property_read_string()
> etc to use the Linux kernel's examples.
>
> Another benefit of this approach is that we, by way of the dt-binding,
> control the meaning of the versions.
> If a vendor decides to release something using Xfoo, but provides no
> version information, we can then assign one ourselves in case Xfoo in
> their next SoC is not quite the same. Something similar came up this
> morning, talking with Heiko about the TH1520, and how to explain the
> meaning of "_xfoo" properties in "riscv,isa". The ISA spec documentation
> is pointed to by the binding for that, but vendor properties are
> obviously not described there. At the expensive of bloating the binding
> file, the proposed scheme would provide a way to stably document vendor
> properties.
>
> I guess I am trying to design in some flexibility rather than two years
> down the line realise that the isa string is a source of problems, and
> have to try and retrofit something in.
>
> I would like to encourage people to populate their DT with every
> extension under the sun that they support, even if software doesn't use
> it right now (look at the starfive folk that didn't add the bitmanip
> until told to) so that if/when it is used in the future these boards
> will pick up the support automagically.
>
> ACPI
> ====
>
> This whole proposal is written for a pre-ACPI world, and I have yet to
> give any thought to how such a key-value interface would work there.
> I'm not really sure how to deal with that, given they have some ECR
> process yada yada, but thoughts on that side of things would be very
> much appreciated.
>
> Why x.y.z rather than x.y per the ISA specs?
> ============================================
>
> I said the same, Palmer wanted x.y.z. For example, the T-HEAD vector stuff
> is 0.7.1 & he cited an example (that now eludes me) of a breaking change
> in an extension between 1.0 and 1.0.1. God knows how vendors will choose
> to version things, so having the extra level is likely advantageous.
>
> Other stuff
> ===========
>
> The code here is very much in an RFC state. I tested it on an Icicle kit
> as a PoC - and it does work, but I have not even remotely tested it
> sufficiently.
>
> The dt-binding changes need to be worked on as they do not actually
> enforce anything!
>
> I've intentionally only send this to the linux lists, despite this
> having wider impact, as it is in a very early state & there's no point
> involving all & sundry if the idea is hated.
> If it is not universally derided, I will send the binding patches to
> various other lists also.
>
> What do I hate about this?
> ==========================
>
> I fear bloat in the dt-binding and devicetrees as properties are added
> mostly. Depending on what I have to do to get enforcement with
> dt-validate, a complicated binding is also a concern.
>
> Suggestions etc very much welcome :)
>
> Cheers,
> Conor.
>
> CC: Rob Herring <robh+dt@kernel.org>
> CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> CC: Conor Dooley <conor+dt@kernel.org>
> CC: Palmer Dabbelt <palmer@dabbelt.com>
> CC: Paul Walmsley <paul.walmsley@sifive.com>
> CC: Heiko Stuebner <heiko@sntech.de>
> CC: Andrew Jones <ajones@ventanamicro.com>
> CC: Sunil V L <sunilvl@ventanamicro.com>
> CC: Yangyu Chen <cyy@cyyself.name>
> CC: devicetree@vger.kernel.org
> CC: linux-riscv@lists.infradead.org
>
> Conor Dooley (6):
>   dt-bindings: riscv: clarify what an unversioned extension means
>   dt-bindings: riscv: add riscv,isa-extension-* property and
>     incompatible example
>   RISC-V: deprecate riscv,isa & replace it with per-extension properties
>   RISC-V: add support for riscv,isa-base property
>   RISC-V: drop a needless check in print_isa_ext()
>   riscv: dts: microchip: use new riscv,isa-extension-* properties for
>     mpfs
>
>  .../devicetree/bindings/riscv/cpus.yaml       |  64 +++++-
>  arch/riscv/boot/dts/microchip/mpfs.dtsi       |  42 +++-
>  arch/riscv/include/asm/hwcap.h                |  29 ++-
>  arch/riscv/kernel/cpu.c                       | 124 +++---------
>  arch/riscv/kernel/cpufeature.c                | 188 +++++++++++++++---
>  5 files changed, 316 insertions(+), 131 deletions(-)
>
> --
> 2.39.2
>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

On Thu, May 11, 2023 at 02:27:44PM -0700, Atish Patra wrote:

> > The other thought I had was that perhaps some software may choose not to
> > implement version x.y.0 of an extension and only support x.z.0, z > y
> > for some reason. We'd want to refuse that extension if the extension is
> > found, but the version is not listed as being something compatible with
> > x.z.0, and while the ISA spec does say that the default assumption is
> > 2p0 for unversioned extensions in its current form, I struggle to
> > extrapolate that to extensions not currently part of the unpriv spec,
> > but rather defined on their own.
> >
> 
> That's a fair point. However, any new RVI ISA extension will only have v1.0
> as per my knowledge. Any new feature will have to be part of a
> different extension.
> At least that was the plan discussed last year.

That's more than last year at this point, and nothing has changed in the
documentation! Talk's cheap, ehh?

> https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983222655
> 
> Are you aware of any discussion that changes this ?

It's called "trust issues". I am far less worried about the addition of
new features though than the removal of existing ones.
Part of me fears for fence.i-less systems for example, but there would
be other ways to bodge around the mess if it comes to pass.
If we are *sure* that no extensions will modify features additively or
subtractively, then this may not be needed at all & I can avoid having
to bend dt-validate to my will.
We have no guarantees for vendor extensions on that front either,
they're free to do what they like w.r.t. versioning, no?

On Thu, May 11, 2023 at 2:47 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, May 11, 2023 at 02:27:44PM -0700, Atish Patra wrote:
>
> > > The other thought I had was that perhaps some software may choose not to
> > > implement version x.y.0 of an extension and only support x.z.0, z > y
> > > for some reason. We'd want to refuse that extension if the extension is
> > > found, but the version is not listed as being something compatible with
> > > x.z.0, and while the ISA spec does say that the default assumption is
> > > 2p0 for unversioned extensions in its current form, I struggle to
> > > extrapolate that to extensions not currently part of the unpriv spec,
> > > but rather defined on their own.
> > >
> >
> > That's a fair point. However, any new RVI ISA extension will only have v1.0
> > as per my knowledge. Any new feature will have to be part of a
> > different extension.
> > At least that was the plan discussed last year.
>
> That's more than last year at this point, and nothing has changed in the
> documentation! Talk's cheap, ehh?
>

Yup. I will poke RVI folks to check if it still is the plan or changed !!

> > https://github.com/riscv/riscv-isa-manual/issues/781#issuecomment-983222655
> >
> > Are you aware of any discussion that changes this ?
>
> It's called "trust issues". I am far less worried about the addition of
> new features though than the removal of existing ones.
> Part of me fears for fence.i-less systems for example, but there would
> be other ways to bodge around the mess if it comes to pass.
> If we are *sure* that no extensions will modify features additively or
> subtractively, then this may not be needed at all & I can avoid having
> to bend dt-validate to my will.

Fair enough. Let's get some clarification first from RVI. It must be
documented in unpriv
spec. Otherwise, there is no point of promise :)

> We have no guarantees for vendor extensions on that front either,
> they're free to do what they like w.r.t. versioning, no?

Vendor extensions are wild west. Who knows what scheme they will use.
We will likely have a vendor specific string parsing logic. They can do whatever
in that to figure out the version if they require it.

On Thu, May 11, 2023 at 03:34:24PM -0700, Atish Patra wrote:
> On Thu, May 11, 2023 at 2:47 PM Conor Dooley <conor@kernel.org> wrote:
> > On Thu, May 11, 2023 at 02:27:44PM -0700, Atish Patra wrote:

> > That's more than last year at this point, and nothing has changed in the
> > documentation! Talk's cheap, ehh?
> >
> 
> Yup. I will poke RVI folks to check if it still is the plan or changed !!

Sounds good, thanks!

> We will likely have a vendor specific string parsing logic.

Complicating the parsing logic is the exact sort of crap that I want
to avoid.

On Fri, May 12, 2023 at 11:01:09AM -0700, Palmer Dabbelt wrote:
> On Thu, 11 May 2023 15:38:10 PDT (-0700), Conor Dooley wrote:
> > On Thu, May 11, 2023 at 03:34:24PM -0700, Atish Patra wrote:
> > > On Thu, May 11, 2023 at 2:47 PM Conor Dooley <conor@kernel.org> wrote:
> > > > On Thu, May 11, 2023 at 02:27:44PM -0700, Atish Patra wrote:
> > 
> > > > That's more than last year at this point, and nothing has changed in the
> > > > documentation! Talk's cheap, ehh?
> > > >
> > > 
> > > Yup. I will poke RVI folks to check if it still is the plan or changed !!
> > 
> > Sounds good, thanks!

There has been some movement on that front, shall see where it goes
:upsidedown_smile:

> > > We will likely have a vendor specific string parsing logic.
> > 
> > Complicating the parsing logic is the exact sort of crap that I want
> > to avoid.
> 
> Ya, I think we're reallly overcomplicating things with the ISA strings.
> Let's just deprecate them and move to something that doesn't need all the
> bespoke string parsing.

Versioning aside, although that removes a large part of the motivation,
the interface becomes quite nice:
of_property_present(node, "riscv,isa-extension-zicbom")

That also gives us the ability to define what supported vendor
extensions actually mean in a dt-binding, which to me is a big win in
terms of the aforementioned "wild west".

On Fri, May 12, 2023 at 3:05 PM Conor Dooley <conor@kernel.org> wrote:
>
> +CC Greg, Mark, Krste, Philipp, Andrew,
>
> (this is LKML now, no top posting or html replies)
>
> On Fri, May 12, 2023 at 08:40:10PM +0100, Conor Dooley wrote:
> > On Fri, May 12, 2023 at 11:01:09AM -0700, Palmer Dabbelt wrote:
> > > On Thu, 11 May 2023 15:38:10 PDT (-0700), Conor Dooley wrote:
> > > > On Thu, May 11, 2023 at 03:34:24PM -0700, Atish Patra wrote:
> > > > > On Thu, May 11, 2023 at 2:47 PM Conor Dooley <conor@kernel.org> wrote:
> > > > > > On Thu, May 11, 2023 at 02:27:44PM -0700, Atish Patra wrote:
> > > >
> > > > > > That's more than last year at this point, and nothing has changed in the
> > > > > > documentation! Talk's cheap, ehh?
> > > > > >
> > > > >
> > > > > Yup. I will poke RVI folks to check if it still is the plan or changed !!
> > > >
> > > > Sounds good, thanks!
> >
> > There has been some movement on that front, shall see where it goes
> > :upsidedown_smile:
>
> There's been some off-list discussion prompted by Atish with some of the
> RVI spec folk, from which the upshot __appears__ to be an understanding
> that using version numbering to indicate removal of ISA features is a bad
> idea.
> I'm hoping that this results in the enshrinement of this in the ISA
> specs, so that we have something concrete to point to as the basis for
> not needing to handle version numbering.
> Certainly that'd be great for ACPI and remove concerns there.
>
> > > > > We will likely have a vendor specific string parsing logic.
> > > >
> > > > Complicating the parsing logic is the exact sort of crap that I want
> > > > to avoid.
> > >
> > > Ya, I think we're reallly overcomplicating things with the ISA strings.
> > > Let's just deprecate them and move to something that doesn't need all the
> > > bespoke string parsing.
> >
> > Versioning aside, although that removes a large part of the motivation,
> > the interface becomes quite nice:
> > of_property_present(node, "riscv,isa-extension-zicbom")
>
> My current feeling is that, rather than introducing a key-value type of
> property, adding boolean properties for extensions is the way to go
> here. The "riscv,isa" part of the DT ABI pre-dates even the ratification

The only problem with boolean properties is you lose the ability to
add extra information
about an ISA extension in case we require it. One of the examples is
CMO extensions.
The current riscv,isa string parsing scheme that doesn't have
infrastructure to do that either.

We had some related discussions in the past about how to extend the
key-value pair to include
that value.

https://lore.kernel.org/lkml/CAOnJCUKgt1+SVXTBmGChJf74JrsqeqACXbjQAXnhFALkXhPFew@mail.gmail.com/

> of the base extensions (and thus the removal of some features...).
> Starting again with a new property would allow us to define extensions
> as per their ratified state, rather than the intermediate & incompatible
> states that we have currently got with "riscv,isa".
> Such a model does rely on the strengthening of the wording in the
> specification.
>
> This had the advantage of being, as I mention above, even easier to
> parse in software than the key-value pair business - but also is
> trivially implemented in a dt-binding. What I have been trying to do
> with the validation of the key-value stuff does not appear to be readily
> doable!
>
> (Another drawback that has come to mind is that we have no way to
> validate whether mutually exclusive extensions have been added with
> "riscv,isa")
>
> > That also gives us the ability to define what supported vendor
> > extensions actually mean in a dt-binding, which to me is a big win in
> > terms of the aforementioned "wild west".
>
> Vendor extensions etc are oft quoted as one of the strengths of RISC-V,
> and my feeling is that "riscv,isa" is not a mechanism where we can
> easily handle meanings - especially for vendor stuff where there is
> otherwise no centralised location for _xfoo -> feature mappings.
>
> Cheers,
> Conor.

On 08/05/2023 20:16, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> C'est la vie, the spec folks reserve the ability to make incompatible
> changes between major versions of an extension. Their idea of backwards
> compatibility appears driven by the hardware perspective - it's
> backwards compatible if a later version is a subset of the existing
> extension. IOW, if you supported `x` in vN, you still support `x` in
> vN+1.
> However in software terms, code that was built for the vN's `x`
> extension may not work with the new definition.
> 
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

On 13 May 2023, at 08:47, Anup Patel <apatel@ventanamicro.com> wrote:
> 
> On Sat, May 13, 2023 at 3:35 AM Conor Dooley <conor@kernel.org> wrote:
>> 
>> +CC Greg, Mark, Krste, Philipp, Andrew,
>> 
>> (this is LKML now, no top posting or html replies)
>> 
>> On Fri, May 12, 2023 at 08:40:10PM +0100, Conor Dooley wrote:
>>> On Fri, May 12, 2023 at 11:01:09AM -0700, Palmer Dabbelt wrote:
>>>> On Thu, 11 May 2023 15:38:10 PDT (-0700), Conor Dooley wrote:
>>>>> On Thu, May 11, 2023 at 03:34:24PM -0700, Atish Patra wrote:
>>>>>> On Thu, May 11, 2023 at 2:47 PM Conor Dooley <conor@kernel.org> wrote:
>>>>>>> On Thu, May 11, 2023 at 02:27:44PM -0700, Atish Patra wrote:
>>>>> 
>>>>>>> That's more than last year at this point, and nothing has changed in the
>>>>>>> documentation! Talk's cheap, ehh?
>>>>>>> 
>>>>>> 
>>>>>> Yup. I will poke RVI folks to check if it still is the plan or changed !!
>>>>> 
>>>>> Sounds good, thanks!
>>> 
>>> There has been some movement on that front, shall see where it goes
>>> :upsidedown_smile:
>> 
>> There's been some off-list discussion prompted by Atish with some of the
>> RVI spec folk, from which the upshot __appears__ to be an understanding
>> that using version numbering to indicate removal of ISA features is a bad
>> idea.
>> I'm hoping that this results in the enshrinement of this in the ISA
>> specs, so that we have something concrete to point to as the basis for
>> not needing to handle version numbering.
>> Certainly that'd be great for ACPI and remove concerns there.
>> 
>>>>>> We will likely have a vendor specific string parsing logic.
>>>>> 
>>>>> Complicating the parsing logic is the exact sort of crap that I want
>>>>> to avoid.
>>>> 
>>>> Ya, I think we're reallly overcomplicating things with the ISA strings.
>>>> Let's just deprecate them and move to something that doesn't need all the
>>>> bespoke string parsing.
>>> 
>>> Versioning aside, although that removes a large part of the motivation,
>>> the interface becomes quite nice:
>>> of_property_present(node, "riscv,isa-extension-zicbom")
>> 
>> My current feeling is that, rather than introducing a key-value type of
>> property, adding boolean properties for extensions is the way to go
>> here. The "riscv,isa" part of the DT ABI pre-dates even the ratification
>> of the base extensions (and thus the removal of some features...).
>> Starting again with a new property would allow us to define extensions
>> as per their ratified state, rather than the intermediate & incompatible
>> states that we have currently got with "riscv,isa".
>> Such a model does rely on the strengthening of the wording in the
>> specification.
> 
> ISA string parsed for both DT and ACPI.
> 
> For ACPI, moving to a per-extension bit in a bitmap and defining
> a new bit with every ISA extension will be very very inconvenient
> for updating the ACPI specs. We should continue the ISA string
> parsing at least for ACPI.
> 
> For DT, users can either use "riscv,isa" DT property or use boolean
> DT properties.

Can we please not gratuitously have two ways of doing the same thing.

I say this as a non-Linux OS that has to deal with whatever Linux
decides to do with device trees. It is a total nuisance when you flip
flop on things and we have to follow suit. Please consider the breakage
very carefully.

Jess

>> This had the advantage of being, as I mention above, even easier to
>> parse in software than the key-value pair business - but also is
>> trivially implemented in a dt-binding. What I have been trying to do
>> with the validation of the key-value stuff does not appear to be readily
>> doable!
>> 
>> (Another drawback that has come to mind is that we have no way to
>> validate whether mutually exclusive extensions have been added with
>> "riscv,isa")
>> 
>>> That also gives us the ability to define what supported vendor
>>> extensions actually mean in a dt-binding, which to me is a big win in
>>> terms of the aforementioned "wild west".
>> 
>> Vendor extensions etc are oft quoted as one of the strengths of RISC-V,
>> and my feeling is that "riscv,isa" is not a mechanism where we can
>> easily handle meanings - especially for vendor stuff where there is
>> otherwise no centralised location for _xfoo -> feature mappings.
>> 
>> Cheers,
>> Conor.
> 
> Regards,
> Anup
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

On Mon, May 15, 2023 at 10:08:35AM +0530, Sunil V L wrote:
> On Sat, May 13, 2023 at 01:17:03PM +0530, Anup Patel wrote:

> > ISA string parsed for both DT and ACPI.
> > 
> > For ACPI, moving to a per-extension bit in a bitmap and defining
> > a new bit with every ISA extension will be very very inconvenient
> > for updating the ACPI specs. We should continue the ISA string
> > parsing at least for ACPI.
> > 
> > For DT, users can either use "riscv,isa" DT property or use boolean
> > DT properties.
> > 
> From ACPI perspective, the format better be backed by unpriv (or any
> other) spec from RVI considering it is a standard across OSs and to
> avoid any maintenance issues.

DT is also used across multiple OSes, I am not sure what your point is
here.
The problem, for DT in particular, is defining __which__ version of the
unpriv spec meaning is derived from, not whether to use definitions from
the specifications.