[v2,3/6] RISC-V: hwprobe: Add support for RISCV_HWPROBE_BASE_BEHAVIOR_IMA
Commit Message
From: Palmer Dabbelt <palmer@rivosinc.com>
We have an implicit set of base behaviors that userspace depends on,
which are mostly defined in various ISA specifications.
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
Signed-off-by: Evan Green <evan@rivosinc.com>
---
(no changes since v1)
Documentation/riscv/hwprobe.rst | 16 ++++++++++++++++
arch/riscv/include/asm/hwprobe.h | 2 +-
arch/riscv/include/uapi/asm/hwprobe.h | 6 +++++-
arch/riscv/kernel/sys_riscv.c | 23 +++++++++++++++++++++++
4 files changed, 45 insertions(+), 2 deletions(-)
Comments
On Mon, Feb 06, 2023 at 12:14:52PM -0800, Evan Green wrote:
> From: Palmer Dabbelt <palmer@rivosinc.com>
>
> We have an implicit set of base behaviors that userspace depends on,
> which are mostly defined in various ISA specifications.
>
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> Signed-off-by: Evan Green <evan@rivosinc.com>
> ---
>
> (no changes since v1)
>
> Documentation/riscv/hwprobe.rst | 16 ++++++++++++++++
> arch/riscv/include/asm/hwprobe.h | 2 +-
> arch/riscv/include/uapi/asm/hwprobe.h | 6 +++++-
> arch/riscv/kernel/sys_riscv.c | 23 +++++++++++++++++++++++
> 4 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
> index 97771090e972..ce186967861f 100644
> --- a/Documentation/riscv/hwprobe.rst
> +++ b/Documentation/riscv/hwprobe.rst
> @@ -35,3 +35,19 @@ The following keys are defined:
> specifications.
> * :RISCV_HWPROBE_KEY_MIMPLID:: Contains the value of :mimplid:, as per the ISA
> specifications.
> +* :RISCV_HWPROBE_KEY_BASE_BEHAVIOR:: A bitmask containing the base user-visible
> + behavior that this kernel supports. The following base user ABIs are defined:
> + * :RISCV_HWPROBE_BASE_BEHAVIOR_IMA:: Support for rv32ima or rv64ima, as
> + defined by version 2.2 of the user ISA and version 1.10 of the privileged
> + ISA, with the following known exceptions (more exceptions may be added,
> + but only if it can be demonstrated that the user ABI is not broken):
> + * The :fence.i: instruction cannot be directly executed by userspace
> + programs (it may still be executed in userspace via a
> + kernel-controlled mechanism such as the vDSO).
I don't really do the whole rst thing at all, are we able to have
newlines between list items? If we can, I think one would go nicely here.
> +* :RISCV_HWPROBE_KEY_IMA_EXT_0:: A bitmask containing the extensions that are
> + compatible with the :RISCV_HWPROBE_BASE_BEHAVIOR_IMA: base system behavior.
Why do we specifically care if they're compatible with IMA?
What's the "fear" here?
> + * :RISCV_HWPROBE_IMA_FD:: The F and D extensions are supported, as defined
Also, is this IMA and FD thing a kinda commitment to only supporting
hardware that has IMA* or IMAFD*
I know that's what we do now, but only under the hood?
As per usual, I'm probably missing something. What is it?
> + by commit cd20cee ("FMIN/FMAX now implement minimumNumber/maximumNumber,
> + not minNum/maxNum") of the RISC-V ISA manual.
> + * :RISCV_HWPROBE_IMA_C:: The C extension is supported, as defined by
> + version 2.2 of the RISC-V ISA manual.
See, this seems to be how we have to treat specs, list the exact
versions! I don't even have to look to know that this was in the v1 ;)
On Mon, Feb 06, 2023 at 12:14:52PM -0800, Evan Green wrote:
> + case RISCV_HWPROBE_KEY_IMA_EXT_0:
> + {
> + u64 val = 0;
> +
> + if (has_fpu())
> + val |= RISCV_HWPROBE_IMA_FD;
The indent caught my eye here for a sec so my attention was drawn back
here. Would you mind adding a line of whitespace between these checks?
Cheers,
Conor.
> + if (elf_hwcap & RISCV_ISA_EXT_c)
> + val |= RISCV_HWPROBE_IMA_C;
> + ret = set_hwprobe(pairs, val);
> + }
> + break;
> +
@@ -35,3 +35,19 @@ The following keys are defined:
specifications.
* :RISCV_HWPROBE_KEY_MIMPLID:: Contains the value of :mimplid:, as per the ISA
specifications.
+* :RISCV_HWPROBE_KEY_BASE_BEHAVIOR:: A bitmask containing the base user-visible
+ behavior that this kernel supports. The following base user ABIs are defined:
+ * :RISCV_HWPROBE_BASE_BEHAVIOR_IMA:: Support for rv32ima or rv64ima, as
+ defined by version 2.2 of the user ISA and version 1.10 of the privileged
+ ISA, with the following known exceptions (more exceptions may be added,
+ but only if it can be demonstrated that the user ABI is not broken):
+ * The :fence.i: instruction cannot be directly executed by userspace
+ programs (it may still be executed in userspace via a
+ kernel-controlled mechanism such as the vDSO).
+* :RISCV_HWPROBE_KEY_IMA_EXT_0:: A bitmask containing the extensions that are
+ compatible with the :RISCV_HWPROBE_BASE_BEHAVIOR_IMA: base system behavior.
+ * :RISCV_HWPROBE_IMA_FD:: The F and D extensions are supported, as defined
+ by commit cd20cee ("FMIN/FMAX now implement minimumNumber/maximumNumber,
+ not minNum/maxNum") of the RISC-V ISA manual.
+ * :RISCV_HWPROBE_IMA_C:: The C extension is supported, as defined by
+ version 2.2 of the RISC-V ISA manual.
@@ -8,6 +8,6 @@
#include <uapi/asm/hwprobe.h>
-#define RISCV_HWPROBE_MAX_KEY 2
+#define RISCV_HWPROBE_MAX_KEY 4
#endif
@@ -20,6 +20,10 @@ struct riscv_hwprobe {
#define RISCV_HWPROBE_KEY_MVENDORID 0
#define RISCV_HWPROBE_KEY_MARCHID 1
#define RISCV_HWPROBE_KEY_MIMPID 2
+#define RISCV_HWPROBE_KEY_BASE_BEHAVIOR 3
+#define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0)
+#define RISCV_HWPROBE_KEY_IMA_EXT_0 4
+#define RISCV_HWPROBE_IMA_FD (1 << 0)
+#define RISCV_HWPROBE_IMA_C (1 << 1)
/* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
-
#endif
@@ -9,6 +9,7 @@
#include <asm/cacheflush.h>
#include <asm/cpufeature.h>
#include <asm/hwprobe.h>
+#include <asm/switch_to.h>
#include <asm/uaccess.h>
#include <asm/unistd.h>
#include <asm-generic/mman-common.h>
@@ -182,6 +183,28 @@ long do_riscv_hwprobe(struct riscv_hwprobe __user *pairs, long pair_count,
ret = hwprobe_mid(pairs, key, &cpus);
break;
+ /*
+ * The kernel already assumes that the base single-letter ISA
+ * extensions are supported on all harts, and only supports the
+ * IMA base, so just cheat a bit here and tell that to
+ * userspace.
+ */
+ case RISCV_HWPROBE_KEY_BASE_BEHAVIOR:
+ ret = set_hwprobe(pairs, RISCV_HWPROBE_BASE_BEHAVIOR_IMA);
+ break;
+
+ case RISCV_HWPROBE_KEY_IMA_EXT_0:
+ {
+ u64 val = 0;
+
+ if (has_fpu())
+ val |= RISCV_HWPROBE_IMA_FD;
+ if (elf_hwcap & RISCV_ISA_EXT_c)
+ val |= RISCV_HWPROBE_IMA_C;
+ ret = set_hwprobe(pairs, val);
+ }
+ break;
+
/*
* For forward compatibility, unknown keys don't fail the whole
* call, but get their element key set to -1 and value set to 0