Hey Evan, Having been talking to Palmer about this series at FOSDEM, it was a very pleasant surprise to see this when I saw this in my inbox when I landed back home. I do very much intend reviewing this, but... On 6 February 2023 20:14:49 GMT, Evan Green <evan@xxxxxxxxxxxx> wrote: > >These are very much up for discussion, as it's a pretty big new user >interface and it's quite a bit different from how we've historically >done things: this isn't just providing an ISA string to userspace, this >has its own format for providing information to userspace. > >There's been a bunch of off-list discussions about this, including at >Plumbers. The original plan was to do something involving providing an >ISA string to userspace, but ISA strings just aren't sufficient for a >stable ABI any more: in order to parse an ISA string users need the >version of the specifications that the string is written to, the version >of each extension (sometimes at a finer granularity than the RISC-V >releases/versions encode), and the expected use case for the ISA string >(ie, is it a U-mode or M-mode string). That's a lot of complexity to >try and keep ABI compatible and it's probably going to continue to grow, >as even if there's no more complexity in the specifications we'll have >to deal with the various ISA string parsing oddities that end up all >over userspace. > >Instead this patch set takes a very different approach and provides a set >of key/value pairs that encode various bits about the system. The big >advantage here is that we can clearly define what these mean so we can >ensure ABI stability, but it also allows us to encode information that's >unlikely to ever appear in an ISA string (see the misaligned access >performance, for example). The resulting interface looks a lot like >what arm64 and x86 do, and will hopefully fit well into something like >ACPI in the future. > >The actual user interface is a syscall. I'm not really sure that's the >right way to go about this, but it makes for flexible prototying. >Various other approaches have been talked about like making HWCAP2 a >pointer, having a VDSO routine, or exposing this via sysfs. Those seem >like generally reasonable approaches, but I've yet to figure out a way This all looks to be the same cover message as Palmer submitted with the v1, so, as I'd mentioned to him the other day, I'd like to do a bit of an investigation into the sysfs approach drew suggested on the v1. So, if it's a little bit before you hear - I've certainly not forgotten about the series! Thanks, Conor. >to get the general case working without a syscall as that's the only way >I've come up with to deal with the heterogenous CPU case. Happy to hear >if someone has a better idea, though, as I don't really want to add a >syscall if we can avoid it. > >An example series in glibc exposing this syscall and using it in an >ifunc selector for memcpy can be found at [1]. > >[1] https://public-inbox.org/libc-alpha/20230206194819.1679472-1-evan@xxxxxxxxxxxx/T/#t > >Changes in v2: > - Changed the interface to look more like poll(). Rather than supplying > key_offset and getting back an array of values with numerically > contiguous keys, have the user pre-fill the key members of the array, > and the kernel will fill in the corresponding values. For any key it > doesn't recognize, it will set the key of that element to -1. This > allows usermode to quickly ask for exactly the elements it cares > about, and not get bogged down in a back and forth about newer keys > that older kernels might not recognize. In other words, the kernel > can communicate that it doesn't recognize some of the keys while > still providing the data for the keys it does know. > - Added a shortcut to the cpuset parameters that if a size of 0 and > NULL is provided for the CPU set, the kernel will use a cpu mask of > all online CPUs. This is convenient because I suspect most callers > will only want to act on a feature if it's supported on all CPUs, and > it's a headache to dynamically allocate an array of all 1s, not to > mention a waste to have the kernel loop over all of the offline bits. > - Fixed logic error in if(of_property_read_string...) that caused crash > - Include cpufeature.h in cpufeature.h to avoid undeclared variable > warning. > - Added a _MASK define > - Fix random checkpatch complaints > - Updated the selftests to the new API and added some more. > - Fixed indentation, comments in .S, and general checkpatch complaints. > >Evan Green (4): > RISC-V: Move struct riscv_cpuinfo to new header > RISC-V: Add a syscall for HW probing > RISC-V: hwprobe: Support probing of misaligned access performance > selftests: Test the new RISC-V hwprobe interface > >Palmer Dabbelt (2): > RISC-V: hwprobe: Add support for RISCV_HWPROBE_BASE_BEHAVIOR_IMA > dt-bindings: Add RISC-V misaligned access performance > > .../devicetree/bindings/riscv/cpus.yaml | 15 ++ > Documentation/riscv/hwprobe.rst | 66 ++++++ > Documentation/riscv/index.rst | 1 + > arch/riscv/include/asm/cpufeature.h | 23 +++ > arch/riscv/include/asm/hwprobe.h | 13 ++ > arch/riscv/include/asm/smp.h | 9 + > arch/riscv/include/asm/syscall.h | 3 + > arch/riscv/include/uapi/asm/hwprobe.h | 35 ++++ > arch/riscv/include/uapi/asm/unistd.h | 8 + > arch/riscv/kernel/cpu.c | 11 +- > arch/riscv/kernel/cpufeature.c | 31 ++- > arch/riscv/kernel/sys_riscv.c | 192 +++++++++++++++++- > tools/testing/selftests/Makefile | 1 + > tools/testing/selftests/riscv/Makefile | 58 ++++++ > .../testing/selftests/riscv/hwprobe/Makefile | 10 + > .../testing/selftests/riscv/hwprobe/hwprobe.c | 89 ++++++++ > .../selftests/riscv/hwprobe/sys_hwprobe.S | 12 ++ > tools/testing/selftests/riscv/libc.S | 46 +++++ > 18 files changed, 613 insertions(+), 10 deletions(-) > create mode 100644 Documentation/riscv/hwprobe.rst > create mode 100644 arch/riscv/include/asm/cpufeature.h > create mode 100644 arch/riscv/include/asm/hwprobe.h > create mode 100644 arch/riscv/include/uapi/asm/hwprobe.h > create mode 100644 tools/testing/selftests/riscv/Makefile > create mode 100644 tools/testing/selftests/riscv/hwprobe/Makefile > create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.c > create mode 100644 tools/testing/selftests/riscv/hwprobe/sys_hwprobe.S > create mode 100644 tools/testing/selftests/riscv/libc.S >
