Re: [PATCH v5 0/6] Provide a fraemework for RISC-V ISA extensions

[Nick Kossifidis <mick@ics.forth.gr>]

Στις 2022-03-11 02:21, Atish Kumar Patra έγραψε:
> On Thu, Mar 10, 2022 at 3:50 PM Palmer Dabbelt <palmer@dabbelt.com> 
> wrote:
>> 
>> On Tue, 22 Feb 2022 12:48:05 PST (-0800), Atish Patra wrote:
>> > This series implements a generic framework to parse multi-letter ISA
>> > extensions. This series is based on Tsukasa's v3 isa extension improvement
>> > series[1]. I have fixed few bugs and improved comments from that series
>> > (PATCH1-3). I have not used PATCH 4 from that series as we are not using
>> > ISA extension versioning as of now. We can add that later if required.
>> >
>> > PATCH 4 allows the probing of multi-letter extensions via a macro.
>> > It continues to use the common isa extensions between all the harts.
>> > Thus hetergenous hart systems will only see the common ISA extensions.
>> >
>> > PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions
>> > via /proc/cpuinfo.
>> >
>> > Here is the example output of /proc/cpuinfo:
>> > (with debug patches in Qemu and Linux kernel)
>> >
>> > # cat /proc/cpuinfo
>> > processor     : 0
>> > hart          : 0
>> > isa           : rv64imafdch
>> > isa-ext               : svpbmt svnapot svinval
>> 
>> I know it might seem a bit pedantic, but I really don't want to
>> introduce a new format for encoding ISA extensions -- doubly so if 
>> this
>> is the only way we're giving this info to userspace, as then we're 
>> just
>> asking folks to turn this into a defacto ABI.  Every time we try to do
>> something that's sort of like an ISA string but not exactly what's in
>> the spec we end up getting burned, and while I don't see a specific 
>> way
> 
> I agree that this is an ABI change/improvement which is impossible to
> modify later.
> However, this is a Linux specific ABI. Do you think the RISC-V spec
> will ever say anything about how /proc/cpuinfo is shown to the user ?
> 

Actually there was a discussion on chairs at some point on how isa 
extensions will be represented as a single string. If I recall correctly 
they wanted a way to compare features between implementations so this 
was something the user should be able to read as well. I'm ccing Philipp 
from the Software HC in case he has more details on this.

I also believe we need to discuss this a bit further, also I thought we 
agreed that having everything as a single string (riscv-isa) on the 
device tree doesn't scale, there were some other suggestions regarding 
for example mmu extensions being declared inside an mmu sub-node etc. 
This patch series will not only make it hard to change /proc/cpuinfo 
output in the future, but also establishes a device-tree binding for all 
isa extensions through the riscv-isa string that we also won't be able 
to modify later on.

Regards,
Nick