Received-SPF: pass (google.com: domain of linux-kernel+bounces-87706-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Thu, 29 Feb 2024 15:40:37 -0800 (PST)
Subject:     Re: [PATCH -fixes v4 2/3] riscv: Add a custom ISA extension for the [ms]envcfg CSR
In-Reply-To: <20240229-establish-itinerary-5d08f6c3ee43@spud>
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
From: Palmer Dabbelt <palmer@dabbelt.com>
To: Conor Dooley <conor@kernel.org>
Message-ID: <mhng-0128d165-88ba-4952-b875-c35b06ce9224@palmer-ri-x1c9>
Precedence: bulk
Mime-Version: 1.0 (MHng)
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit

On Thu, 29 Feb 2024 10:30:10 PST (-0800), Conor Dooley wrote:
> 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.

Ya, I'm in the same boat.  I think we can get away without putting these 
into DT until we end up with something odd going on, like some other 
flavor of *envcf from some vendor being weird.

>> -- 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.