Re: [PATCH -next v21 03/27] riscv: hwprobe: Add support for probing V in RISCV_HWPROBE_KEY_IMA_EXT_0

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 27 Jun 2023 17:30:33 PDT (-0700), sorear@xxxxxxxxxxxx wrote:
On Mon, Jun 5, 2023, at 7:07 AM, Andy Chiu wrote:
Probing kernel support for Vector extension is available now. This only
add detection for V only. Extenions like Zvfh, Zk are not in this scope.

Signed-off-by: Andy Chiu <andy.chiu@xxxxxxxxxx>
Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
Reviewed-by: Evan Green <evan@xxxxxxxxxxxx>
Reviewed-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
---
Changelog v20:
 - Fix a typo in document, and remove duplicated probes (Heiko)
 - probe V extension in RISCV_HWPROBE_KEY_IMA_EXT_0 key only (Palmer,
   Evan)
---
 Documentation/riscv/hwprobe.rst       | 3 +++
 arch/riscv/include/uapi/asm/hwprobe.h | 1 +
 arch/riscv/kernel/sys_riscv.c         | 4 ++++
 3 files changed, 8 insertions(+)

diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
index 9f0dd62dcb5d..7431d9d01c73 100644
--- a/Documentation/riscv/hwprobe.rst
+++ b/Documentation/riscv/hwprobe.rst
@@ -64,6 +64,9 @@ The following keys are defined:
   * :c:macro:`RISCV_HWPROBE_IMA_C`: The C extension is supported, as defined
     by version 2.2 of the RISC-V ISA manual.

+  * :c:macro:`RISCV_HWPROBE_IMA_V`: The V extension is supported, as defined by
+    version 1.0 of the RISC-V Vector extension manual.
+
 * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
   information about the selected set of processors.

diff --git a/arch/riscv/include/uapi/asm/hwprobe.h
b/arch/riscv/include/uapi/asm/hwprobe.h
index 8d745a4ad8a2..7c6fdcf7ced5 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -25,6 +25,7 @@ struct riscv_hwprobe {
 #define RISCV_HWPROBE_KEY_IMA_EXT_0	4
 #define		RISCV_HWPROBE_IMA_FD		(1 << 0)
 #define		RISCV_HWPROBE_IMA_C		(1 << 1)
+#define		RISCV_HWPROBE_IMA_V		(1 << 2)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
 #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
diff --git a/arch/riscv/kernel/sys_riscv.c
b/arch/riscv/kernel/sys_riscv.c
index 5db29683ebee..88357a848797 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -10,6 +10,7 @@
 #include <asm/cpufeature.h>
 #include <asm/hwprobe.h>
 #include <asm/sbi.h>
+#include <asm/vector.h>
 #include <asm/switch_to.h>
 #include <asm/uaccess.h>
 #include <asm/unistd.h>
@@ -171,6 +172,9 @@ static void hwprobe_one_pair(struct riscv_hwprobe
*pair,
 		if (riscv_isa_extension_available(NULL, c))
 			pair->value |= RISCV_HWPROBE_IMA_C;

+		if (has_vector())
+			pair->value |= RISCV_HWPROBE_IMA_V;
+
 		break;

I am concerned by the exception this is making.  I believe the intention of
riscv_hwprobe is to replace AT_HWCAP as the single point of truth for userspace
to make instruction use decisions.  Since this does not check riscv_v_vstate_ctrl_user_allowed,
application code which wants to know if V instructions are usable must use
AT_HWCAP instead, unlike all other extensions for which the relevant data is
available within the hwprobe return.

I guess we were vague in the docs about what "supported" means, but IIRC the goal was for riscv_hwprobe() to indicate what's supported by both the HW and the kernel. In other words, hwprobe should indicate what's possible to enable -- even if there's some additional steps necessary to enable it.

We can at least make this a little more explicit with something like

   diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
   index 19165ebd82ba..7f82a5385bc3 100644
   --- a/Documentation/riscv/hwprobe.rst
   +++ b/Documentation/riscv/hwprobe.rst
   @@ -27,6 +27,13 @@ AND of the values for the specified CPUs. Usermode can supply NULL for cpus and
    0 for cpu_count as a shortcut for all online CPUs. There are currently no flags,
    this value must be zero for future compatibility.
+Calls to `sys_riscv_hwprobe()` indicate the features supported by both the
   +kernel and the hardware that the system is running on.  For example, if the
   +hardware supports the V extension and the kernel has V support enabled then
   +`RISCV_HWPROBE_KEY_IMA_EXT_0`/`RISCV_HWPROBE_IMA_V` will be set even if the V
   +extension is disabled via a userspace-controlled tunable such as
   +`PR_RISCV_V_SET_CONTROL`.
   +
    On success 0 is returned, on failure a negative error code is returned.
The following keys are defined:
   @@ -65,7 +72,10 @@ The following keys are defined:
        by version 2.2 of the RISC-V ISA manual.
* :c:macro:`RISCV_HWPROBE_IMA_V`: The V extension is supported, as defined by
   -    version 1.0 of the RISC-V Vector extension manual.
   +    version 1.0 of the RISC-V Vector extension manual.  For strict uABI
   +    compatibility some systems may disable V by default even when the hardware
   +    supports in, in which case users must call `prctl(PR_RISCV_V_SET_CONTROL,
   +    ...` to explicitly allow V to be used.
* :c:macro:`RISCV_HWPROBE_EXT_ZBA`: The Zba address generation extension is
           supported, as defined in version 1.0 of the Bit-Manipulation ISA

IMO that's the better way to go that to require that userspace tries to enable
V via the prctl() first, but we haven't released this yet so in theory we could
still change it.

We'd have a similar discussion for some of the counters that need to feed
through the perf interface, though those are still in flight...

Assuming this is intentional, what is the path forward for future extensions
that cannot be used from userspace without additional conditions being met?
For instance, if we add support in the future for the Zve* extensions, the V
bit would not be set in HWCAP for them, which would require library code to
use the prctl interface unless we define the hwcap bits to imply userspace
usability.

In this case a system that supports some of the Zve extensions but not the full V extension would not be probably from userspace, as V would not be set anywhere. The way to support that would be to add new bits into hwprobe to indicate those extensions, it just wasn't clear that anyone was interested in building Linux-flavored systems that supported only some a strict subset of V.

Happy to see patches if you know of some hardware in the pipeline, though ;)


-s

 	case RISCV_HWPROBE_KEY_CPUPERF_0:
--
2.17.1


_______________________________________________
linux-riscv mailing list
linux-riscv@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/linux-riscv



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux