All of lore.kernel.org
 help / color / mirror / Atom feed
From: Conor Dooley <conor@kernel.org>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: samuel.holland@sifive.com, ajones@ventanamicro.com,
	linux-kernel@vger.kernel.org, alex@ghiti.fr,
	linux-riscv@lists.infradead.org, sorear@fastmail.com,
	stable@vger.kernel.org
Subject: Re: [PATCH -fixes v4 2/3] riscv: Add a custom ISA extension for the [ms]envcfg CSR
Date: Thu, 29 Feb 2024 18:30:10 +0000	[thread overview]
Message-ID: <20240229-establish-itinerary-5d08f6c3ee43@spud> (raw)
In-Reply-To: <mhng-94e8034f-eda3-45df-bcf0-1bd5bd9cb869@palmer-ri-x1c9a>


[-- Attachment #1.1: Type: text/plain, Size: 2512 bytes --]

On Thu, Feb 29, 2024 at 10:23:39AM -0800, Palmer Dabbelt wrote:
> On Wed, 28 Feb 2024 02:12:14 PST (-0800), Conor Dooley wrote:
> > On Tue, Feb 27, 2024 at 10:55:34PM -0800, Samuel Holland wrote:
> > > The [ms]envcfg CSR was added in version 1.12 of the RISC-V privileged
> > > ISA (aka S[ms]1p12). However, bits in this CSR are defined by several
> > > other extensions which may be implemented separately from any particular
> > > version of the privileged ISA (for example, some unrelated errata may
> > > prevent an implementation from claiming conformance with Ss1p12). As a
> > > result, Linux cannot simply use the privileged ISA version to determine
> > > if the CSR is present. It must also check if any of these other
> > > extensions are implemented. It also cannot probe the existence of the
> > > CSR at runtime, because Linux does not require Sstrict, so (in the
> > > absence of additional information) it cannot know if a CSR at that
> > > address is [ms]envcfg or part of some non-conforming vendor extension.
> > > 
> > > Since there are several standard extensions that imply the existence of
> > > the [ms]envcfg CSR, it becomes unwieldy to check for all of them
> > > wherever the CSR is accessed. Instead, define a custom Xlinuxenvcfg ISA
> > > extension bit that is implied by the other extensions and denotes that
> > > the CSR exists as defined in the privileged ISA, containing at least one
> > > of the fields common between menvcfg and senvcfg.
> > 
> > > This extension does not need to be parsed from the devicetree or ISA
> > > string because it can only be implemented as a subset of some other
> > > standard extension.
> > 
> > NGL, every time I look at the superset stuff I question whether or not
> > it is a good implementation, but it is nice to see that it at least
> > makes the creation of quasi-extension flags like this straightforward.
> 
> We can always add it to the DT list as a proper extension, but I think for
> this sort of stuff it's good enough for now

Perhaps good enough forever. I was not advocating for adding it as a
permitted DT property - I was just saying that I didn't the complexity
that you mention below, but I was pleasantly surprised that the stuff
?Evan? and I came up with allows for this kind of inferred "extension"
without any changes.

> -- we've already got a bunch of
> complexity for the proper ISA-defined extension dependencies, so it's not
> like we could really get away from it entirely.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

WARNING: multiple messages have this Message-ID (diff)
From: Conor Dooley <conor@kernel.org>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: samuel.holland@sifive.com, ajones@ventanamicro.com,
	linux-kernel@vger.kernel.org, alex@ghiti.fr,
	linux-riscv@lists.infradead.org, sorear@fastmail.com,
	stable@vger.kernel.org
Subject: Re: [PATCH -fixes v4 2/3] riscv: Add a custom ISA extension for the [ms]envcfg CSR
Date: Thu, 29 Feb 2024 18:30:10 +0000	[thread overview]
Message-ID: <20240229-establish-itinerary-5d08f6c3ee43@spud> (raw)
In-Reply-To: <mhng-94e8034f-eda3-45df-bcf0-1bd5bd9cb869@palmer-ri-x1c9a>

[-- Attachment #1: Type: text/plain, Size: 2512 bytes --]

On Thu, Feb 29, 2024 at 10:23:39AM -0800, Palmer Dabbelt wrote:
> On Wed, 28 Feb 2024 02:12:14 PST (-0800), Conor Dooley wrote:
> > On Tue, Feb 27, 2024 at 10:55:34PM -0800, Samuel Holland wrote:
> > > The [ms]envcfg CSR was added in version 1.12 of the RISC-V privileged
> > > ISA (aka S[ms]1p12). However, bits in this CSR are defined by several
> > > other extensions which may be implemented separately from any particular
> > > version of the privileged ISA (for example, some unrelated errata may
> > > prevent an implementation from claiming conformance with Ss1p12). As a
> > > result, Linux cannot simply use the privileged ISA version to determine
> > > if the CSR is present. It must also check if any of these other
> > > extensions are implemented. It also cannot probe the existence of the
> > > CSR at runtime, because Linux does not require Sstrict, so (in the
> > > absence of additional information) it cannot know if a CSR at that
> > > address is [ms]envcfg or part of some non-conforming vendor extension.
> > > 
> > > Since there are several standard extensions that imply the existence of
> > > the [ms]envcfg CSR, it becomes unwieldy to check for all of them
> > > wherever the CSR is accessed. Instead, define a custom Xlinuxenvcfg ISA
> > > extension bit that is implied by the other extensions and denotes that
> > > the CSR exists as defined in the privileged ISA, containing at least one
> > > of the fields common between menvcfg and senvcfg.
> > 
> > > This extension does not need to be parsed from the devicetree or ISA
> > > string because it can only be implemented as a subset of some other
> > > standard extension.
> > 
> > NGL, every time I look at the superset stuff I question whether or not
> > it is a good implementation, but it is nice to see that it at least
> > makes the creation of quasi-extension flags like this straightforward.
> 
> We can always add it to the DT list as a proper extension, but I think for
> this sort of stuff it's good enough for now

Perhaps good enough forever. I was not advocating for adding it as a
permitted DT property - I was just saying that I didn't the complexity
that you mention below, but I was pleasantly surprised that the stuff
?Evan? and I came up with allows for this kind of inferred "extension"
without any changes.

> -- we've already got a bunch of
> complexity for the proper ISA-defined extension dependencies, so it's not
> like we could really get away from it entirely.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

  reply	other threads:[~2024-02-29 18:30 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-28  6:55 [PATCH -fixes v4 0/3] riscv: cbo.zero fixes Samuel Holland
2024-02-28  6:55 ` Samuel Holland
2024-02-28  6:55 ` [PATCH -fixes v4 1/3] riscv: Fix enabling cbo.zero when running in M-mode Samuel Holland
2024-02-28  6:55   ` Samuel Holland
2024-02-28 10:13   ` Conor Dooley
2024-02-28 10:13     ` Conor Dooley
2024-02-28  6:55 ` [PATCH -fixes v4 2/3] riscv: Add a custom ISA extension for the [ms]envcfg CSR Samuel Holland
2024-02-28  6:55   ` Samuel Holland
2024-02-28 10:12   ` Conor Dooley
2024-02-28 10:12     ` Conor Dooley
2024-02-29 18:23     ` Palmer Dabbelt
2024-02-29 18:23       ` Palmer Dabbelt
2024-02-29 18:30       ` Conor Dooley [this message]
2024-02-29 18:30         ` Conor Dooley
2024-02-29 23:40         ` Palmer Dabbelt
2024-02-29 23:40           ` Palmer Dabbelt
2024-02-28 13:23   ` Andrew Jones
2024-02-28 13:23     ` Andrew Jones
2024-02-28  6:55 ` [PATCH -fixes v4 3/3] riscv: Save/restore envcfg CSR during CPU suspend Samuel Holland
2024-02-28  6:55   ` Samuel Holland
2024-02-28 10:14   ` Conor Dooley
2024-02-28 10:14     ` Conor Dooley
2024-02-28 13:27   ` Andrew Jones
2024-02-28 13:27     ` Andrew Jones
2024-02-29 22:10 ` [PATCH -fixes v4 0/3] riscv: cbo.zero fixes patchwork-bot+linux-riscv
2024-02-29 22:10   ` patchwork-bot+linux-riscv

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240229-establish-itinerary-5d08f6c3ee43@spud \
    --to=conor@kernel.org \
    --cc=ajones@ventanamicro.com \
    --cc=alex@ghiti.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=palmer@dabbelt.com \
    --cc=samuel.holland@sifive.com \
    --cc=sorear@fastmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.