linux-kernel - Re: [PATCH -fixes v4 2/3] riscv: Add a custom ISA extension for the [ms]envcfg CSR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 29 Feb 2024 18:30:10 +0000
From: Conor Dooley <conor@...nel.org>
To: Palmer Dabbelt <palmer@...belt.com>
Cc: samuel.holland@...ive.com, ajones@...tanamicro.com,
	linux-kernel@...r.kernel.org, alex@...ti.fr,
	linux-riscv@...ts.infradead.org, sorear@...tmail.com,
	stable@...r.kernel.org
Subject: Re: [PATCH -fixes v4 2/3] riscv: Add a custom ISA extension for the
 [ms]envcfg CSR

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.

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)