On Thu, Mar 10, 2022 at 3:50 PM Palmer Dabbelt <palmer@xxxxxxxxxxx> 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 ? and we do have similar precedence in other arch /proc/cpuinfo output in x86: flags : fpu vme .... arch_capabilities vmx flags : vnmi preemption_timer ..tsc_scaling /proc/cpuinfo output in arm64: Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 > that could happen here that's what's happened so many times before I > just don't want to risk it. > > I've gone ahead and removed some of this information (isa-ext, and all > the single-letter extensions we can't properly turn on yet) from > /proc/cpuinfo, modifying the last patch to do so. It's at > palmer/riscv-isa, LMK if that's OK. > Few changes required on your tree: riscv_isa_ext_data definition is no longer required and this should be to show hypervisor extension: +static const char *base_riscv_exts = "imafdch"; > I'm not opposed to doing something here, I just really don't want to > rush into it as we've already got enough complexity around the various > flavors of ISA strings. I don't see a big pressing need to provide this > information to userspace, but having the rest of this sorted out is > great (and there's some dependencies on this) so I'd prefer to just > stick to what we know is good. > "isa-ext" row is kind of helpful to inform the developer to verify that a specific extension is available without looking at extension specific dmesg logs. It proved to be useful while developing sstc/sscofpmf. Is it better if we just dump the entire ISA string as before so that we know which extensions are available at least ? > > mmu : sv48 > > > > processor : 1 > > hart : 1 > > isa : rv64imafdch > > isa-ext : svpbmt svnapot svinval > > mmu : sv48 > > > > processor : 2 > > hart : 2 > > isa : rv64imafdch > > isa-ext : svpbmt svnapot svinval > > mmu : sv48 > > > > processor : 3 > > hart : 3 > > isa : rv64imafdch > > isa-ext : svpbmt svnapot svinval > > mmu : sv48 > > > > Anybody adding support for any new multi-letter extensions should add an > > entry to the riscv_isa_ext_id and the isa extension array. > > E.g. The patch[2] adds the support for various ISA extensions. > > > > [1] https://lore.kernel.org/all/0f568515-a05e-8204-aae3-035975af3ee8@xxxxxxxxxxxx/T/ > > [2] https://github.com/atishp04/linux/commit/e9e240c9a854dceb434ceb53bdbe82a657bee5f2 > > > > Changes from v4->v5: > > 1. Improved the /proc/cpuinfo to include only valid & enabled extensions > > 2. Improved the multi-letter parsing by skipping the 'su' modes generated in > > Qemu as suggested by Tsukasa. > > > > Changes from v3->v4: > > 1. Changed temporary variable for current hart isa to a bitmap > > 2. Added reviewed-by tags. > > 3. Improved comments > > > > Changes from v2->v3: > > 1. Updated comments to mark clearly a fix required for Qemu only. > > 2. Fixed a bug where the 1st multi-letter extension can be present without _ > > 3. Added Tested by tags. > > > > Changes from v1->v2: > > 1. Instead of adding a separate DT property use the riscv,isa property. > > 2. Based on Tsukasa's v3 isa extension improvement series. > > > > Atish Patra (3): > > RISC-V: Implement multi-letter ISA extension probing framework > > RISC-V: Do no continue isa string parsing without correct XLEN > > RISC-V: Improve /proc/cpuinfo output for ISA extensions > > > > Tsukasa OI (3): > > RISC-V: Correctly print supported extensions > > RISC-V: Minimal parser for "riscv, isa" strings > > RISC-V: Extract multi-letter extension names from "riscv, isa" > > > > arch/riscv/include/asm/hwcap.h | 25 +++++++ > > arch/riscv/kernel/cpu.c | 51 ++++++++++++- > > arch/riscv/kernel/cpufeature.c | 130 +++++++++++++++++++++++++++------ > > 3 files changed, 183 insertions(+), 23 deletions(-)
