diff mbox series

[v2,3/6] RISC-V: hwprobe: Add support for RISCV_HWPROBE_BASE_BEHAVIOR_IMA

Message ID 20230206201455.1790329-4-evan@rivosinc.com (mailing list archive)
State Superseded
Delegated to: Palmer Dabbelt
Headers show
Series RISC-V Hardware Probing User Interface | expand

Checks

Context Check Description
conchuod/tree_selection fail Failed to apply to next/pending-fixes or riscv/for-next

Commit Message

Evan Green Feb. 6, 2023, 8:14 p.m. UTC 
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

Conor Dooley Feb. 15, 2023, 9:25 p.m. UTC | #1 
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 ;)
Conor Dooley Feb. 15, 2023, 10:09 p.m. UTC | #2 
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;
> +
diff mbox series

Patch

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).
+* :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.
diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 08d1c3bdd78a..7e52f1e1fe10 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -8,6 +8,6 @@ 
 
 #include <uapi/asm/hwprobe.h>
 
-#define RISCV_HWPROBE_MAX_KEY 2
+#define RISCV_HWPROBE_MAX_KEY 4
 
 #endif
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 591802047460..ce39d6e74103 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -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
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index 868a12384f5a..74e0d72c877d 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -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