[PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

[PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

[Andy Chiu]

The series composes of two parts. The first part provides a quick fix for
the issue on a recent thread[1]. The issue happens when a platform has
ununified vector register length across multiple cores. Specifically,
patch 1 adds a comment at a callsite of riscv_setup_vsize to clarify how
vlenb is observed by the system. Patch 2 fixes the issue by failing the
boot process of a secondary core if vlenb mismatches.

The second part of the series provide a finer grain view of the Vector
extension. Patch 3 give the obsolete ISA parser the ability to expand
ISA extensions for sigle letter extensions. Patch 3, 4 introduces Zve32x,
Zve32f, Zve64x, Zve64f, Zve64d for isa parsing and hwprobe. Patch 5
updates all callsites such that Vector subextensions are maximumly
supported by the kernel.

Two parts of the series are sent together to ease the effort of picking
dependency patches. The first part can be merged independent of the
second one if necessary.

The series is tested on a QEMU and verified that booting, Vector
programs context-switch, signal, ptrace, prctl interfaces works when we
only report partial V from the ISA.

Note that the signal test was performed after applying the commit
c27fa53b858b ("riscv: Fix vector state restore in rt_sigreturn()")

This patch should be able to apply on risc-v for-next branch on top of
the commit ba5ea59f768f ("riscv: Do not save the scratch CSR during suspend")

[1]: https://lore.kernel.org/all/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/T/#u

Changes in v4:
- Add a patch to trigger prctl test on ZVE32X (9)
- Add a patch to fix integer promotion bug in hwprobe (8)
- Fix a build fail on !CONFIG_RISCV_ISA_V (7)
- Add more comment in the assembly code change (2)
- Link to v3: https://lore.kernel.org/r/20240318-zve-detection-v3-0-e12d42107fa8@sifive.com

Changelog v3:
 - Include correct maintainers and mailing list into CC.
 - Cleanup isa string parser code (3)
 - Adjust extensions order and name (4, 5)
 - Refine commit message (6)

Changelog v2:
 - Update comments and commit messages (1, 2, 7)
 - Refine isa_exts[] lists for zve extensions (4)
 - Add a patch for dt-binding (5)
 - Make ZVE* extensions depend on has_vector(ZVE32X) (6, 7)

---
---
Andy Chiu (9):
      riscv: vector: add a comment when calling riscv_setup_vsize()
      riscv: smp: fail booting up smp if inconsistent vlen is detected
      riscv: cpufeature: call match_isa_ext() for single-letter extensions
      riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection
      dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description
      riscv: hwprobe: add zve Vector subextensions into hwprobe interface
      riscv: vector: adjust minimum Vector requirement to ZVE32X
      hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro
      selftest: run vector prctl test for ZVE32X

 Documentation/arch/riscv/hwprobe.rst               | 15 ++++++
 .../devicetree/bindings/riscv/extensions.yaml      | 30 ++++++++++++
 arch/riscv/include/asm/hwcap.h                     |  5 ++
 arch/riscv/include/asm/switch_to.h                 |  2 +-
 arch/riscv/include/asm/vector.h                    | 25 ++++++----
 arch/riscv/include/asm/xor.h                       |  2 +-
 arch/riscv/include/uapi/asm/hwprobe.h              |  7 ++-
 arch/riscv/kernel/cpufeature.c                     | 56 ++++++++++++++++++----
 arch/riscv/kernel/head.S                           | 19 +++++---
 arch/riscv/kernel/kernel_mode_vector.c             |  4 +-
 arch/riscv/kernel/process.c                        |  4 +-
 arch/riscv/kernel/signal.c                         |  6 +--
 arch/riscv/kernel/smpboot.c                        | 14 ++++--
 arch/riscv/kernel/sys_hwprobe.c                    | 13 ++++-
 arch/riscv/kernel/vector.c                         | 15 +++---
 arch/riscv/lib/uaccess.S                           |  2 +-
 .../testing/selftests/riscv/vector/vstate_prctl.c  |  6 +--
 17 files changed, 174 insertions(+), 51 deletions(-)
---
base-commit: ba5ea59f768f67d127b319b26ba209ff67e0d9a5
change-id: 20240318-zve-detection-50106d2da527

Best regards,
-- 
Andy Chiu <andy.chiu@sifive.com>

[PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

[Andy Chiu]

The series composes of two parts. The first part provides a quick fix for
the issue on a recent thread[1]. The issue happens when a platform has
ununified vector register length across multiple cores. Specifically,
patch 1 adds a comment at a callsite of riscv_setup_vsize to clarify how
vlenb is observed by the system. Patch 2 fixes the issue by failing the
boot process of a secondary core if vlenb mismatches.

The second part of the series provide a finer grain view of the Vector
extension. Patch 3 give the obsolete ISA parser the ability to expand
ISA extensions for sigle letter extensions. Patch 3, 4 introduces Zve32x,
Zve32f, Zve64x, Zve64f, Zve64d for isa parsing and hwprobe. Patch 5
updates all callsites such that Vector subextensions are maximumly
supported by the kernel.

Two parts of the series are sent together to ease the effort of picking
dependency patches. The first part can be merged independent of the
second one if necessary.

The series is tested on a QEMU and verified that booting, Vector
programs context-switch, signal, ptrace, prctl interfaces works when we
only report partial V from the ISA.

Note that the signal test was performed after applying the commit
c27fa53b858b ("riscv: Fix vector state restore in rt_sigreturn()")

This patch should be able to apply on risc-v for-next branch on top of
the commit ba5ea59f768f ("riscv: Do not save the scratch CSR during suspend")

[1]: https://lore.kernel.org/all/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/T/#u

Changes in v4:
- Add a patch to trigger prctl test on ZVE32X (9)
- Add a patch to fix integer promotion bug in hwprobe (8)
- Fix a build fail on !CONFIG_RISCV_ISA_V (7)
- Add more comment in the assembly code change (2)
- Link to v3: https://lore.kernel.org/r/20240318-zve-detection-v3-0-e12d42107fa8@sifive.com

Changelog v3:
 - Include correct maintainers and mailing list into CC.
 - Cleanup isa string parser code (3)
 - Adjust extensions order and name (4, 5)
 - Refine commit message (6)

Changelog v2:
 - Update comments and commit messages (1, 2, 7)
 - Refine isa_exts[] lists for zve extensions (4)
 - Add a patch for dt-binding (5)
 - Make ZVE* extensions depend on has_vector(ZVE32X) (6, 7)

---
---
Andy Chiu (9):
      riscv: vector: add a comment when calling riscv_setup_vsize()
      riscv: smp: fail booting up smp if inconsistent vlen is detected
      riscv: cpufeature: call match_isa_ext() for single-letter extensions
      riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection
      dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description
      riscv: hwprobe: add zve Vector subextensions into hwprobe interface
      riscv: vector: adjust minimum Vector requirement to ZVE32X
      hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro
      selftest: run vector prctl test for ZVE32X

 Documentation/arch/riscv/hwprobe.rst               | 15 ++++++
 .../devicetree/bindings/riscv/extensions.yaml      | 30 ++++++++++++
 arch/riscv/include/asm/hwcap.h                     |  5 ++
 arch/riscv/include/asm/switch_to.h                 |  2 +-
 arch/riscv/include/asm/vector.h                    | 25 ++++++----
 arch/riscv/include/asm/xor.h                       |  2 +-
 arch/riscv/include/uapi/asm/hwprobe.h              |  7 ++-
 arch/riscv/kernel/cpufeature.c                     | 56 ++++++++++++++++++----
 arch/riscv/kernel/head.S                           | 19 +++++---
 arch/riscv/kernel/kernel_mode_vector.c             |  4 +-
 arch/riscv/kernel/process.c                        |  4 +-
 arch/riscv/kernel/signal.c                         |  6 +--
 arch/riscv/kernel/smpboot.c                        | 14 ++++--
 arch/riscv/kernel/sys_hwprobe.c                    | 13 ++++-
 arch/riscv/kernel/vector.c                         | 15 +++---
 arch/riscv/lib/uaccess.S                           |  2 +-
 .../testing/selftests/riscv/vector/vstate_prctl.c  |  6 +--
 17 files changed, 174 insertions(+), 51 deletions(-)
---
base-commit: ba5ea59f768f67d127b319b26ba209ff67e0d9a5
change-id: 20240318-zve-detection-50106d2da527

Best regards,
-- 
Andy Chiu <andy.chiu@sifive.com>


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

[PATCH v4 1/9] riscv: vector: add a comment when calling riscv_setup_vsize()

[Andy Chiu]

The function would fail when it detects the calling hart's vlen doesn't
match the first one's. The boot hart is the first hart calling this
function during riscv_fill_hwcap, so it is impossible to fail here. Add
a comment about this behavior.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v2:
 - update the comment (Conor)
---
 arch/riscv/kernel/cpufeature.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3ed2359eae35..d22b12072579 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -683,6 +683,10 @@ void __init riscv_fill_hwcap(void)
 	}
 
 	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
+		/*
+		 * This callsite can't fail here. It cannot fail when called on
+		 * the boot hart.
+		 */
 		riscv_v_setup_vsize();
 		/*
 		 * ISA string in device tree might have 'v' flag, but

-- 
2.44.0.rc2

[PATCH v4 1/9] riscv: vector: add a comment when calling riscv_setup_vsize()

[Andy Chiu]

The function would fail when it detects the calling hart's vlen doesn't
match the first one's. The boot hart is the first hart calling this
function during riscv_fill_hwcap, so it is impossible to fail here. Add
a comment about this behavior.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v2:
 - update the comment (Conor)
---
 arch/riscv/kernel/cpufeature.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3ed2359eae35..d22b12072579 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -683,6 +683,10 @@ void __init riscv_fill_hwcap(void)
 	}
 
 	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
+		/*
+		 * This callsite can't fail here. It cannot fail when called on
+		 * the boot hart.
+		 */
 		riscv_v_setup_vsize();
 		/*
 		 * ISA string in device tree might have 'v' flag, but

-- 
2.44.0.rc2


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

[PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Andy Chiu]

Currently we only support Vector for SMP platforms, that is, all SMP
cores have the same vlenb. If we happen to detect a mismatching vlen, it
is better to just fail bootting it up to prevent further race/scheduling
issues.

Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
regular call in the early assembly. So a core would be parked right
after a return from smp_callin. Note that a successful smp_callin
does not return.

Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
Reported-by: Conor Dooley <conor.dooley@microchip.com>
Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v4:
 - update comment also in the assembly code (Yunhui)
Changelog v2:
 - update commit message to explain asm code change (Conor)
---
 arch/riscv/kernel/head.S    | 19 ++++++++++++-------
 arch/riscv/kernel/smpboot.c | 14 +++++++++-----
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 4236a69c35cb..a00f7523cb91 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -165,9 +165,20 @@ secondary_start_sbi:
 #endif
 	call .Lsetup_trap_vector
 	scs_load_current
-	tail smp_callin
+	call smp_callin
 #endif /* CONFIG_SMP */
 
+.align 2
+.Lsecondary_park:
+	/*
+	 * Park this hart if we:
+	 *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
+	 *  - receive an early trap, before setup_trap_vector finished
+	 *  - fail in smp_callin(), as a successful one wouldn't return
+	 */
+	wfi
+	j .Lsecondary_park
+
 .align 2
 .Lsetup_trap_vector:
 	/* Set trap vector to exception handler */
@@ -181,12 +192,6 @@ secondary_start_sbi:
 	csrw CSR_SCRATCH, zero
 	ret
 
-.align 2
-.Lsecondary_park:
-	/* We lack SMP support or have too many harts, so park this hart */
-	wfi
-	j .Lsecondary_park
-
 SYM_CODE_END(_start)
 
 SYM_CODE_START(_start_kernel)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index d41090fc3203..673437ccc13d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
 	struct mm_struct *mm = &init_mm;
 	unsigned int curr_cpuid = smp_processor_id();
 
+	if (has_vector()) {
+		/*
+		 * Return as early as possible so the hart with a mismatching
+		 * vlen won't boot.
+		 */
+		if (riscv_v_setup_vsize())
+			return;
+	}
+
 	/* All kernel threads share the same mm context.  */
 	mmgrab(mm);
 	current->active_mm = mm;
@@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
 	numa_add_cpu(curr_cpuid);
 	set_cpu_online(curr_cpuid, 1);
 
-	if (has_vector()) {
-		if (riscv_v_setup_vsize())
-			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
-	}
-
 	riscv_user_isa_enable();
 
 	/*

-- 
2.44.0.rc2

[PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Andy Chiu]

Currently we only support Vector for SMP platforms, that is, all SMP
cores have the same vlenb. If we happen to detect a mismatching vlen, it
is better to just fail bootting it up to prevent further race/scheduling
issues.

Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
regular call in the early assembly. So a core would be parked right
after a return from smp_callin. Note that a successful smp_callin
does not return.

Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
Reported-by: Conor Dooley <conor.dooley@microchip.com>
Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v4:
 - update comment also in the assembly code (Yunhui)
Changelog v2:
 - update commit message to explain asm code change (Conor)
---
 arch/riscv/kernel/head.S    | 19 ++++++++++++-------
 arch/riscv/kernel/smpboot.c | 14 +++++++++-----
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 4236a69c35cb..a00f7523cb91 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -165,9 +165,20 @@ secondary_start_sbi:
 #endif
 	call .Lsetup_trap_vector
 	scs_load_current
-	tail smp_callin
+	call smp_callin
 #endif /* CONFIG_SMP */
 
+.align 2
+.Lsecondary_park:
+	/*
+	 * Park this hart if we:
+	 *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
+	 *  - receive an early trap, before setup_trap_vector finished
+	 *  - fail in smp_callin(), as a successful one wouldn't return
+	 */
+	wfi
+	j .Lsecondary_park
+
 .align 2
 .Lsetup_trap_vector:
 	/* Set trap vector to exception handler */
@@ -181,12 +192,6 @@ secondary_start_sbi:
 	csrw CSR_SCRATCH, zero
 	ret
 
-.align 2
-.Lsecondary_park:
-	/* We lack SMP support or have too many harts, so park this hart */
-	wfi
-	j .Lsecondary_park
-
 SYM_CODE_END(_start)
 
 SYM_CODE_START(_start_kernel)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index d41090fc3203..673437ccc13d 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
 	struct mm_struct *mm = &init_mm;
 	unsigned int curr_cpuid = smp_processor_id();
 
+	if (has_vector()) {
+		/*
+		 * Return as early as possible so the hart with a mismatching
+		 * vlen won't boot.
+		 */
+		if (riscv_v_setup_vsize())
+			return;
+	}
+
 	/* All kernel threads share the same mm context.  */
 	mmgrab(mm);
 	current->active_mm = mm;
@@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
 	numa_add_cpu(curr_cpuid);
 	set_cpu_online(curr_cpuid, 1);
 
-	if (has_vector()) {
-		if (riscv_v_setup_vsize())
-			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
-	}
-
 	riscv_user_isa_enable();
 
 	/*

-- 
2.44.0.rc2


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

[PATCH v4 3/9] riscv: cpufeature: call match_isa_ext() for single-letter extensions

[Andy Chiu]

Single-letter extensions may also imply multiple subextensions. For
example, Vector extension implies zve64d, and zve64d implies zve64f.

Extension parsing for "riscv,isa-extensions" has the ability to resolve
the dependency by calling match_isa_ext(). This patch makes deprecated
parser call the same function for single letter extensions.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v3:
- Remove set_bit for single-letter extensions as they are all checked in
match_isa_ext. (Clément)
---
 arch/riscv/kernel/cpufeature.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index d22b12072579..f6f3ece60d69 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -468,16 +468,15 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc
 
 		if (unlikely(ext_err))
 			continue;
+
+		for (int i = 0; i < riscv_isa_ext_count; i++)
+			match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo);
+
 		if (!ext_long) {
 			int nr = tolower(*ext) - 'a';
 
-			if (riscv_isa_extension_check(nr)) {
+			if (riscv_isa_extension_check(nr))
 				*this_hwcap |= isa2hwcap[nr];
-				set_bit(nr, isainfo->isa);
-			}
-		} else {
-			for (int i = 0; i < riscv_isa_ext_count; i++)
-				match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo);
 		}
 	}
 }

-- 
2.44.0.rc2

[PATCH v4 3/9] riscv: cpufeature: call match_isa_ext() for single-letter extensions

[Andy Chiu]

Single-letter extensions may also imply multiple subextensions. For
example, Vector extension implies zve64d, and zve64d implies zve64f.

Extension parsing for "riscv,isa-extensions" has the ability to resolve
the dependency by calling match_isa_ext(). This patch makes deprecated
parser call the same function for single letter extensions.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v3:
- Remove set_bit for single-letter extensions as they are all checked in
match_isa_ext. (Clément)
---
 arch/riscv/kernel/cpufeature.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index d22b12072579..f6f3ece60d69 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -468,16 +468,15 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc
 
 		if (unlikely(ext_err))
 			continue;
+
+		for (int i = 0; i < riscv_isa_ext_count; i++)
+			match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo);
+
 		if (!ext_long) {
 			int nr = tolower(*ext) - 'a';
 
-			if (riscv_isa_extension_check(nr)) {
+			if (riscv_isa_extension_check(nr))
 				*this_hwcap |= isa2hwcap[nr];
-				set_bit(nr, isainfo->isa);
-			}
-		} else {
-			for (int i = 0; i < riscv_isa_ext_count; i++)
-				match_isa_ext(&riscv_isa_ext[i], ext, ext_end, isainfo);
 		}
 	}
 }

-- 
2.44.0.rc2


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

[PATCH v4 4/9] riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection

[Andy Chiu]

Multiple Vector subextensions are added. Also, the patch takes care of
the dependencies of Vector subextensions by macro expansions. So, if
some "embedded" platform only reports "zve64f" on the ISA string, the
parser is able to expand it to zve32x zve32f zve64x and zve64f.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v3:
 - renumber RISCV_ISA_EXT_ZVE* to rebase on top of 6.9
 - alphabetically sort added extensions (Clément)
Changelog v2:
 - remove the extension itself from its isa_exts[] list (Clément)
 - use riscv_zve64d_exts for v's extension list (Samuel)
---
 arch/riscv/include/asm/hwcap.h |  5 +++++
 arch/riscv/kernel/cpufeature.c | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..f64d4e98e67c 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,11 @@
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_ZVE32X		75
+#define RISCV_ISA_EXT_ZVE32F		76
+#define RISCV_ISA_EXT_ZVE64X		77
+#define RISCV_ISA_EXT_ZVE64F		78
+#define RISCV_ISA_EXT_ZVE64D		79
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index f6f3ece60d69..38d09de518b1 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -188,6 +188,35 @@ static const unsigned int riscv_zvbb_exts[] = {
 	RISCV_ISA_EXT_ZVKB
 };
 
+#define RISCV_ISA_EXT_ZVE32F_IMPLY_LIST	\
+	RISCV_ISA_EXT_ZVE32X,
+
+#define RISCV_ISA_EXT_ZVE64F_IMPLY_LIST	\
+	RISCV_ISA_EXT_ZVE64X,		\
+	RISCV_ISA_EXT_ZVE32F,		\
+	RISCV_ISA_EXT_ZVE32F_IMPLY_LIST
+
+#define RISCV_ISA_EXT_ZVE64D_IMPLY_LIST	\
+	RISCV_ISA_EXT_ZVE64F,		\
+	RISCV_ISA_EXT_ZVE64F_IMPLY_LIST
+
+static const unsigned int riscv_zve32f_exts[] = {
+	RISCV_ISA_EXT_ZVE32F_IMPLY_LIST
+};
+
+static const unsigned int riscv_zve64f_exts[] = {
+	RISCV_ISA_EXT_ZVE64F_IMPLY_LIST
+};
+
+static const unsigned int riscv_zve64d_exts[] = {
+	RISCV_ISA_EXT_ZVE64D_IMPLY_LIST
+};
+
+static const unsigned int riscv_zve64x_exts[] = {
+	RISCV_ISA_EXT_ZVE32X,
+	RISCV_ISA_EXT_ZVE64X
+};
+
 /*
  * While the [ms]envcfg CSRs were not defined until version 1.12 of the RISC-V
  * privileged ISA, the existence of the CSRs is implied by any extension which
@@ -245,7 +274,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
 	__RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
 	__RISCV_ISA_EXT_DATA(c, RISCV_ISA_EXT_c),
-	__RISCV_ISA_EXT_DATA(v, RISCV_ISA_EXT_v),
+	__RISCV_ISA_EXT_SUPERSET(v, RISCV_ISA_EXT_v, riscv_zve64d_exts),
 	__RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
 	__RISCV_ISA_EXT_SUPERSET(zicbom, RISCV_ISA_EXT_ZICBOM, riscv_xlinuxenvcfg_exts),
 	__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZICBOZ, riscv_xlinuxenvcfg_exts),
@@ -280,6 +309,11 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(ztso, RISCV_ISA_EXT_ZTSO),
 	__RISCV_ISA_EXT_SUPERSET(zvbb, RISCV_ISA_EXT_ZVBB, riscv_zvbb_exts),
 	__RISCV_ISA_EXT_DATA(zvbc, RISCV_ISA_EXT_ZVBC),
+	__RISCV_ISA_EXT_SUPERSET(zve32f, RISCV_ISA_EXT_ZVE32F, riscv_zve32f_exts),
+	__RISCV_ISA_EXT_DATA(zve32x, RISCV_ISA_EXT_ZVE32X),
+	__RISCV_ISA_EXT_SUPERSET(zve64d, RISCV_ISA_EXT_ZVE64D, riscv_zve64d_exts),
+	__RISCV_ISA_EXT_SUPERSET(zve64f, RISCV_ISA_EXT_ZVE64F, riscv_zve64f_exts),
+	__RISCV_ISA_EXT_SUPERSET(zve64x, RISCV_ISA_EXT_ZVE64X, riscv_zve64x_exts),
 	__RISCV_ISA_EXT_DATA(zvfh, RISCV_ISA_EXT_ZVFH),
 	__RISCV_ISA_EXT_DATA(zvfhmin, RISCV_ISA_EXT_ZVFHMIN),
 	__RISCV_ISA_EXT_DATA(zvkb, RISCV_ISA_EXT_ZVKB),

-- 
2.44.0.rc2

[PATCH v4 4/9] riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection

[Andy Chiu]

Multiple Vector subextensions are added. Also, the patch takes care of
the dependencies of Vector subextensions by macro expansions. So, if
some "embedded" platform only reports "zve64f" on the ISA string, the
parser is able to expand it to zve32x zve32f zve64x and zve64f.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v3:
 - renumber RISCV_ISA_EXT_ZVE* to rebase on top of 6.9
 - alphabetically sort added extensions (Clément)
Changelog v2:
 - remove the extension itself from its isa_exts[] list (Clément)
 - use riscv_zve64d_exts for v's extension list (Samuel)
---
 arch/riscv/include/asm/hwcap.h |  5 +++++
 arch/riscv/kernel/cpufeature.c | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e17d0078a651..f64d4e98e67c 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,11 @@
 #define RISCV_ISA_EXT_ZTSO		72
 #define RISCV_ISA_EXT_ZACAS		73
 #define RISCV_ISA_EXT_XANDESPMU		74
+#define RISCV_ISA_EXT_ZVE32X		75
+#define RISCV_ISA_EXT_ZVE32F		76
+#define RISCV_ISA_EXT_ZVE64X		77
+#define RISCV_ISA_EXT_ZVE64F		78
+#define RISCV_ISA_EXT_ZVE64D		79
 
 #define RISCV_ISA_EXT_XLINUXENVCFG	127
 
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index f6f3ece60d69..38d09de518b1 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -188,6 +188,35 @@ static const unsigned int riscv_zvbb_exts[] = {
 	RISCV_ISA_EXT_ZVKB
 };
 
+#define RISCV_ISA_EXT_ZVE32F_IMPLY_LIST	\
+	RISCV_ISA_EXT_ZVE32X,
+
+#define RISCV_ISA_EXT_ZVE64F_IMPLY_LIST	\
+	RISCV_ISA_EXT_ZVE64X,		\
+	RISCV_ISA_EXT_ZVE32F,		\
+	RISCV_ISA_EXT_ZVE32F_IMPLY_LIST
+
+#define RISCV_ISA_EXT_ZVE64D_IMPLY_LIST	\
+	RISCV_ISA_EXT_ZVE64F,		\
+	RISCV_ISA_EXT_ZVE64F_IMPLY_LIST
+
+static const unsigned int riscv_zve32f_exts[] = {
+	RISCV_ISA_EXT_ZVE32F_IMPLY_LIST
+};
+
+static const unsigned int riscv_zve64f_exts[] = {
+	RISCV_ISA_EXT_ZVE64F_IMPLY_LIST
+};
+
+static const unsigned int riscv_zve64d_exts[] = {
+	RISCV_ISA_EXT_ZVE64D_IMPLY_LIST
+};
+
+static const unsigned int riscv_zve64x_exts[] = {
+	RISCV_ISA_EXT_ZVE32X,
+	RISCV_ISA_EXT_ZVE64X
+};
+
 /*
  * While the [ms]envcfg CSRs were not defined until version 1.12 of the RISC-V
  * privileged ISA, the existence of the CSRs is implied by any extension which
@@ -245,7 +274,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
 	__RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
 	__RISCV_ISA_EXT_DATA(c, RISCV_ISA_EXT_c),
-	__RISCV_ISA_EXT_DATA(v, RISCV_ISA_EXT_v),
+	__RISCV_ISA_EXT_SUPERSET(v, RISCV_ISA_EXT_v, riscv_zve64d_exts),
 	__RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
 	__RISCV_ISA_EXT_SUPERSET(zicbom, RISCV_ISA_EXT_ZICBOM, riscv_xlinuxenvcfg_exts),
 	__RISCV_ISA_EXT_SUPERSET(zicboz, RISCV_ISA_EXT_ZICBOZ, riscv_xlinuxenvcfg_exts),
@@ -280,6 +309,11 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 	__RISCV_ISA_EXT_DATA(ztso, RISCV_ISA_EXT_ZTSO),
 	__RISCV_ISA_EXT_SUPERSET(zvbb, RISCV_ISA_EXT_ZVBB, riscv_zvbb_exts),
 	__RISCV_ISA_EXT_DATA(zvbc, RISCV_ISA_EXT_ZVBC),
+	__RISCV_ISA_EXT_SUPERSET(zve32f, RISCV_ISA_EXT_ZVE32F, riscv_zve32f_exts),
+	__RISCV_ISA_EXT_DATA(zve32x, RISCV_ISA_EXT_ZVE32X),
+	__RISCV_ISA_EXT_SUPERSET(zve64d, RISCV_ISA_EXT_ZVE64D, riscv_zve64d_exts),
+	__RISCV_ISA_EXT_SUPERSET(zve64f, RISCV_ISA_EXT_ZVE64F, riscv_zve64f_exts),
+	__RISCV_ISA_EXT_SUPERSET(zve64x, RISCV_ISA_EXT_ZVE64X, riscv_zve64x_exts),
 	__RISCV_ISA_EXT_DATA(zvfh, RISCV_ISA_EXT_ZVFH),
 	__RISCV_ISA_EXT_DATA(zvfhmin, RISCV_ISA_EXT_ZVFHMIN),
 	__RISCV_ISA_EXT_DATA(zvkb, RISCV_ISA_EXT_ZVKB),

-- 
2.44.0.rc2


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

[PATCH v4 5/9] dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description

[Andy Chiu]

Add description for Zve32x Zve32f Zve64x Zve64f Zve64d ISA extensions.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v3:
 - Correct extension names and their order (Stefan)
Changelog v2:
 - new patch since v2
---
 .../devicetree/bindings/riscv/extensions.yaml      | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 468c646247aa..cfed80ad5540 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -381,6 +381,36 @@ properties:
             instructions, as ratified in commit 56ed795 ("Update
             riscv-crypto-spec-vector.adoc") of riscv-crypto.
 
+        - const: zve32f
+          description:
+            The standard Zve32f extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve32x
+          description:
+            The standard Zve32x extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve64d
+          description:
+            The standard Zve64d extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve64f
+          description:
+            The standard Zve64f extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve64x
+          description:
+            The standard Zve64x extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
         - const: zvfh
           description:
             The standard Zvfh extension for vectored half-precision

-- 
2.44.0.rc2

[PATCH v4 5/9] dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description

[Andy Chiu]

Add description for Zve32x Zve32f Zve64x Zve64f Zve64d ISA extensions.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v3:
 - Correct extension names and their order (Stefan)
Changelog v2:
 - new patch since v2
---
 .../devicetree/bindings/riscv/extensions.yaml      | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
index 468c646247aa..cfed80ad5540 100644
--- a/Documentation/devicetree/bindings/riscv/extensions.yaml
+++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
@@ -381,6 +381,36 @@ properties:
             instructions, as ratified in commit 56ed795 ("Update
             riscv-crypto-spec-vector.adoc") of riscv-crypto.
 
+        - const: zve32f
+          description:
+            The standard Zve32f extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve32x
+          description:
+            The standard Zve32x extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve64d
+          description:
+            The standard Zve64d extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve64f
+          description:
+            The standard Zve64f extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
+        - const: zve64x
+          description:
+            The standard Zve64x extension for embedded processors, as ratified
+            in commit 6f702a2 ("Vector extensions are now ratified") of
+            riscv-v-spec.
+
         - const: zvfh
           description:
             The standard Zvfh extension for vectored half-precision

-- 
2.44.0.rc2


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

[PATCH v4 6/9] riscv: hwprobe: add zve Vector subextensions into hwprobe interface

[Andy Chiu]

The following Vector subextensions for "embedded" platforms are added
into RISCV_HWPROBE_KEY_IMA_EXT_0:
 - ZVE32X
 - ZVE32F
 - ZVE64X
 - ZVE64F
 - ZVE64D

Extensions ending with an X indicates that the platform doesn't have a
vector FPU.
Extensions ending with F/D mean that whether single (F) or double (D)
precision vector operation is supported.
The number 32 or 64 follows from ZVE tells the maximum element length.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Clément Léger <cleger@rivosinc.com>
---
Changelog v2:
 - zve* extensions in hwprobe depends on whether kernel supports v, so
   include them after has_vector(). Fix a typo. (Clément)
---
 Documentation/arch/riscv/hwprobe.rst  | 15 +++++++++++++++
 arch/riscv/include/uapi/asm/hwprobe.h |  5 +++++
 arch/riscv/kernel/sys_hwprobe.c       |  5 +++++
 3 files changed, 25 insertions(+)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index b2bcc9eed9aa..d0b02e012e5d 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -188,6 +188,21 @@ The following keys are defined:
        manual starting from commit 95cf1f9 ("Add changes requested by Ved
        during signoff")
 
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE32X`: The Vector sub-extension Zve32x is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE32F`: The Vector sub-extension Zve32f is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE64X`: The Vector sub-extension Zve64x is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE64F`: The Vector sub-extension Zve64f is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE64D`: The Vector sub-extension Zve64d 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 9f2a8e3ff204..b9a0876e969f 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -59,6 +59,11 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
 #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
 #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
+#define		RISCV_HWPROBE_EXT_ZVE32X	(1ULL << 36)
+#define		RISCV_HWPROBE_EXT_ZVE32F	(1ULL << 37)
+#define		RISCV_HWPROBE_EXT_ZVE64X	(1ULL << 38)
+#define		RISCV_HWPROBE_EXT_ZVE64F	(1ULL << 39)
+#define		RISCV_HWPROBE_EXT_ZVE64D	(1ULL << 40)
 #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_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 8cae41a502dd..c8219b82fbfc 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -113,6 +113,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		EXT_KEY(ZICOND);
 
 		if (has_vector()) {
+			EXT_KEY(ZVE32X);
+			EXT_KEY(ZVE32F);
+			EXT_KEY(ZVE64X);
+			EXT_KEY(ZVE64F);
+			EXT_KEY(ZVE64D);
 			EXT_KEY(ZVBB);
 			EXT_KEY(ZVBC);
 			EXT_KEY(ZVKB);

-- 
2.44.0.rc2

[PATCH v4 6/9] riscv: hwprobe: add zve Vector subextensions into hwprobe interface

[Andy Chiu]

The following Vector subextensions for "embedded" platforms are added
into RISCV_HWPROBE_KEY_IMA_EXT_0:
 - ZVE32X
 - ZVE32F
 - ZVE64X
 - ZVE64F
 - ZVE64D

Extensions ending with an X indicates that the platform doesn't have a
vector FPU.
Extensions ending with F/D mean that whether single (F) or double (D)
precision vector operation is supported.
The number 32 or 64 follows from ZVE tells the maximum element length.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Reviewed-by: Clément Léger <cleger@rivosinc.com>
---
Changelog v2:
 - zve* extensions in hwprobe depends on whether kernel supports v, so
   include them after has_vector(). Fix a typo. (Clément)
---
 Documentation/arch/riscv/hwprobe.rst  | 15 +++++++++++++++
 arch/riscv/include/uapi/asm/hwprobe.h |  5 +++++
 arch/riscv/kernel/sys_hwprobe.c       |  5 +++++
 3 files changed, 25 insertions(+)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index b2bcc9eed9aa..d0b02e012e5d 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -188,6 +188,21 @@ The following keys are defined:
        manual starting from commit 95cf1f9 ("Add changes requested by Ved
        during signoff")
 
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE32X`: The Vector sub-extension Zve32x is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE32F`: The Vector sub-extension Zve32f is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE64X`: The Vector sub-extension Zve64x is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE64F`: The Vector sub-extension Zve64f is
+    supported, as defined by version 1.0 of the RISC-V Vector extension manual.
+
+  * :c:macro:`RISCV_HWPROBE_EXT_ZVE64D`: The Vector sub-extension Zve64d 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 9f2a8e3ff204..b9a0876e969f 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -59,6 +59,11 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
 #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)
 #define		RISCV_HWPROBE_EXT_ZICOND	(1ULL << 35)
+#define		RISCV_HWPROBE_EXT_ZVE32X	(1ULL << 36)
+#define		RISCV_HWPROBE_EXT_ZVE32F	(1ULL << 37)
+#define		RISCV_HWPROBE_EXT_ZVE64X	(1ULL << 38)
+#define		RISCV_HWPROBE_EXT_ZVE64F	(1ULL << 39)
+#define		RISCV_HWPROBE_EXT_ZVE64D	(1ULL << 40)
 #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_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 8cae41a502dd..c8219b82fbfc 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -113,6 +113,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		EXT_KEY(ZICOND);
 
 		if (has_vector()) {
+			EXT_KEY(ZVE32X);
+			EXT_KEY(ZVE32F);
+			EXT_KEY(ZVE64X);
+			EXT_KEY(ZVE64F);
+			EXT_KEY(ZVE64D);
 			EXT_KEY(ZVBB);
 			EXT_KEY(ZVBC);
 			EXT_KEY(ZVKB);

-- 
2.44.0.rc2


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

[PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Andy Chiu]

Make has_vector take one argument. This argument represents the minimum
Vector subextension that the following Vector actions assume.

Also, change riscv_v_first_use_handler(), and boot code that calls
riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
ZVE32X.

Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
compiled and run with ZVE32X should be supported by the kernel on most
aspects. This includes context-switch, signal, ptrace, prctl, and
hwprobe.

One exception is that ELF_HWCAP returns 'V' only if full V is supported
on the platform. This means that the system without a full V must not
rely on ELF_HWCAP to tell whether it is allowable to execute Vector
without first invoking a prctl() check.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Acked-by: Joel Granados <j.granados@samsung.com>
---
Changelog v4:
- check static_assert for !CONFIG_RISCV_ISA_V case in has_vector.
Changelog v2:
 - update the comment in hwprobe.
---
 arch/riscv/include/asm/switch_to.h     |  2 +-
 arch/riscv/include/asm/vector.h        | 25 ++++++++++++++++---------
 arch/riscv/include/asm/xor.h           |  2 +-
 arch/riscv/kernel/cpufeature.c         |  5 ++++-
 arch/riscv/kernel/kernel_mode_vector.c |  4 ++--
 arch/riscv/kernel/process.c            |  4 ++--
 arch/riscv/kernel/signal.c             |  6 +++---
 arch/riscv/kernel/smpboot.c            |  2 +-
 arch/riscv/kernel/sys_hwprobe.c        |  8 ++++++--
 arch/riscv/kernel/vector.c             | 15 +++++++++------
 arch/riscv/lib/uaccess.S               |  2 +-
 11 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 7efdb0584d47..df1adf196c4f 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -78,7 +78,7 @@ do {							\
 	struct task_struct *__next = (next);		\
 	if (has_fpu())					\
 		__switch_to_fpu(__prev, __next);	\
-	if (has_vector())					\
+	if (has_vector(ZVE32X))			\
 		__switch_to_vector(__prev, __next);	\
 	((last) = __switch_to(__prev, __next));		\
 } while (0)
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 731dcd0ed4de..ed5fb6515d54 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -8,6 +8,19 @@
 
 #include <linux/types.h>
 #include <uapi/asm-generic/errno.h>
+#include <asm/cpufeature.h>
+#include <asm/hwcap.h>
+
+#define has_vector(VEXT)									\
+({												\
+	static_assert(RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE32X ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE32F ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64X ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64F ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64D ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_v);					\
+	IS_ENABLED(CONFIG_RISCV_ISA_V) && riscv_has_extension_unlikely(RISCV_ISA_EXT_##VEXT);	\
+})
 
 #ifdef CONFIG_RISCV_ISA_V
 
@@ -15,9 +28,9 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 #include <asm/ptrace.h>
-#include <asm/cpufeature.h>
 #include <asm/csr.h>
 #include <asm/asm.h>
+#include <asm/bug.h>
 
 extern unsigned long riscv_v_vsize;
 int riscv_v_setup_vsize(void);
@@ -35,11 +48,6 @@ static inline u32 riscv_v_flags(void)
 	return READ_ONCE(current->thread.riscv_v_flags);
 }
 
-static __always_inline bool has_vector(void)
-{
-	return riscv_has_extension_unlikely(RISCV_ISA_EXT_v);
-}
-
 static inline void __riscv_v_vstate_clean(struct pt_regs *regs)
 {
 	regs->status = (regs->status & ~SR_VS) | SR_VS_CLEAN;
@@ -131,7 +139,7 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
 	riscv_v_enable();
 	asm volatile (
 		".option push\n\t"
-		".option arch, +v\n\t"
+		".option arch, +zve32x\n\t"
 		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
 		"vle8.v		v0, (%1)\n\t"
 		"add		%1, %1, %0\n\t"
@@ -153,7 +161,7 @@ static inline void __riscv_v_vstate_discard(void)
 	riscv_v_enable();
 	asm volatile (
 		".option push\n\t"
-		".option arch, +v\n\t"
+		".option arch, +zve32x\n\t"
 		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
 		"vmv.v.i	v0, -1\n\t"
 		"vmv.v.i	v8, -1\n\t"
@@ -267,7 +275,6 @@ bool riscv_v_vstate_ctrl_user_allowed(void);
 struct pt_regs;
 
 static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; }
-static __always_inline bool has_vector(void) { return false; }
 static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; }
 static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
 static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
diff --git a/arch/riscv/include/asm/xor.h b/arch/riscv/include/asm/xor.h
index 96011861e46b..46042ef5a2f7 100644
--- a/arch/riscv/include/asm/xor.h
+++ b/arch/riscv/include/asm/xor.h
@@ -61,7 +61,7 @@ static struct xor_block_template xor_block_rvv = {
 	do {        \
 		xor_speed(&xor_block_8regs);    \
 		xor_speed(&xor_block_32regs);    \
-		if (has_vector()) { \
+		if (has_vector(ZVE32X)) { \
 			xor_speed(&xor_block_rvv);\
 		} \
 	} while (0)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 38d09de518b1..8b52060649d2 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -715,12 +715,15 @@ void __init riscv_fill_hwcap(void)
 		elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
 	}
 
-	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
+	if (__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ZVE32X)) {
 		/*
 		 * This callsite can't fail here. It cannot fail when called on
 		 * the boot hart.
 		 */
 		riscv_v_setup_vsize();
+	}
+
+	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
 		/*
 		 * ISA string in device tree might have 'v' flag, but
 		 * CONFIG_RISCV_ISA_V is disabled in kernel.
diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
index 6afe80c7f03a..0d4d1a03d1c7 100644
--- a/arch/riscv/kernel/kernel_mode_vector.c
+++ b/arch/riscv/kernel/kernel_mode_vector.c
@@ -208,7 +208,7 @@ void kernel_vector_begin(void)
 {
 	bool nested = false;
 
-	if (WARN_ON(!has_vector()))
+	if (WARN_ON(!has_vector(ZVE32X)))
 		return;
 
 	BUG_ON(!may_use_simd());
@@ -236,7 +236,7 @@ EXPORT_SYMBOL_GPL(kernel_vector_begin);
  */
 void kernel_vector_end(void)
 {
-	if (WARN_ON(!has_vector()))
+	if (WARN_ON(!has_vector(ZVE32X)))
 		return;
 
 	riscv_v_disable();
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 92922dbd5b5c..919e72f9fff6 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -178,7 +178,7 @@ void flush_thread(void)
 void arch_release_task_struct(struct task_struct *tsk)
 {
 	/* Free the vector context of datap. */
-	if (has_vector())
+	if (has_vector(ZVE32X))
 		riscv_v_thread_free(tsk);
 }
 
@@ -225,7 +225,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		p->thread.s[0] = 0;
 	}
 	p->thread.riscv_v_flags = 0;
-	if (has_vector())
+	if (has_vector(ZVE32X))
 		riscv_v_thread_alloc(p);
 	p->thread.ra = (unsigned long)ret_from_fork;
 	p->thread.sp = (unsigned long)childregs; /* kernel sp */
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 501e66debf69..a96e6e969a3f 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -188,7 +188,7 @@ static long restore_sigcontext(struct pt_regs *regs,
 
 			return 0;
 		case RISCV_V_MAGIC:
-			if (!has_vector() || !riscv_v_vstate_query(regs) ||
+			if (!has_vector(ZVE32X) || !riscv_v_vstate_query(regs) ||
 			    size != riscv_v_sc_size)
 				return -EINVAL;
 
@@ -210,7 +210,7 @@ static size_t get_rt_frame_size(bool cal_all)
 
 	frame_size = sizeof(*frame);
 
-	if (has_vector()) {
+	if (has_vector(ZVE32X)) {
 		if (cal_all || riscv_v_vstate_query(task_pt_regs(current)))
 			total_context_size += riscv_v_sc_size;
 	}
@@ -283,7 +283,7 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
 	if (has_fpu())
 		err |= save_fp_state(regs, &sc->sc_fpregs);
 	/* Save the vector state. */
-	if (has_vector() && riscv_v_vstate_query(regs))
+	if (has_vector(ZVE32X) && riscv_v_vstate_query(regs))
 		err |= save_v_state(regs, (void __user **)&sc_ext_ptr);
 	/* Write zero to fp-reserved space and check it on restore_sigcontext */
 	err |= __put_user(0, &sc->sc_extdesc.reserved);
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 673437ccc13d..7252666ce0da 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -214,7 +214,7 @@ asmlinkage __visible void smp_callin(void)
 	struct mm_struct *mm = &init_mm;
 	unsigned int curr_cpuid = smp_processor_id();
 
-	if (has_vector()) {
+	if (has_vector(ZVE32X)) {
 		/*
 		 * Return as early as possible so the hart with a mismatching
 		 * vlen won't boot.
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index c8219b82fbfc..e7c3fcac62a1 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -69,7 +69,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 	if (riscv_isa_extension_available(NULL, c))
 		pair->value |= RISCV_HWPROBE_IMA_C;
 
-	if (has_vector())
+	if (has_vector(v))
 		pair->value |= RISCV_HWPROBE_IMA_V;
 
 	/*
@@ -112,7 +112,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		EXT_KEY(ZACAS);
 		EXT_KEY(ZICOND);
 
-		if (has_vector()) {
+		/*
+		 *  Vector crypto and ZVE* extensions are supported only if
+		 *  kernel has minimum V support of ZVE32X.
+		 */
+		if (has_vector(ZVE32X)) {
 			EXT_KEY(ZVE32X);
 			EXT_KEY(ZVE32F);
 			EXT_KEY(ZVE64X);
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 6727d1d3b8f2..e8a47fa72351 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -53,7 +53,7 @@ int riscv_v_setup_vsize(void)
 
 void __init riscv_v_setup_ctx_cache(void)
 {
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return;
 
 	riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
@@ -173,8 +173,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
 	u32 __user *epc = (u32 __user *)regs->epc;
 	u32 insn = (u32)regs->badaddr;
 
+	if (!has_vector(ZVE32X))
+		return false;
+
 	/* Do not handle if V is not supported, or disabled */
-	if (!(ELF_HWCAP & COMPAT_HWCAP_ISA_V))
+	if (!riscv_v_vstate_ctrl_user_allowed())
 		return false;
 
 	/* If V has been enabled then it is not the first-use trap */
@@ -213,7 +216,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
 	bool inherit;
 	int cur, next;
 
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return;
 
 	next = riscv_v_ctrl_get_next(tsk);
@@ -235,7 +238,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
 
 long riscv_v_vstate_ctrl_get_current(void)
 {
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return -EINVAL;
 
 	return current->thread.vstate_ctrl & PR_RISCV_V_VSTATE_CTRL_MASK;
@@ -246,7 +249,7 @@ long riscv_v_vstate_ctrl_set_current(unsigned long arg)
 	bool inherit;
 	int cur, next;
 
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return -EINVAL;
 
 	if (arg & ~PR_RISCV_V_VSTATE_CTRL_MASK)
@@ -296,7 +299,7 @@ static struct ctl_table riscv_v_default_vstate_table[] = {
 
 static int __init riscv_v_sysctl_init(void)
 {
-	if (has_vector())
+	if (has_vector(ZVE32X))
 		if (!register_sysctl("abi", riscv_v_default_vstate_table))
 			return -EINVAL;
 	return 0;
diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index bc22c078aba8..bbe143bb32a0 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -14,7 +14,7 @@
 
 SYM_FUNC_START(__asm_copy_to_user)
 #ifdef CONFIG_RISCV_ISA_V
-	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_v, CONFIG_RISCV_ISA_V)
+	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)
 	REG_L	t0, riscv_v_usercopy_threshold
 	bltu	a2, t0, fallback_scalar_usercopy
 	tail enter_vector_usercopy

-- 
2.44.0.rc2

[PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Andy Chiu]

Make has_vector take one argument. This argument represents the minimum
Vector subextension that the following Vector actions assume.

Also, change riscv_v_first_use_handler(), and boot code that calls
riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
ZVE32X.

Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
compiled and run with ZVE32X should be supported by the kernel on most
aspects. This includes context-switch, signal, ptrace, prctl, and
hwprobe.

One exception is that ELF_HWCAP returns 'V' only if full V is supported
on the platform. This means that the system without a full V must not
rely on ELF_HWCAP to tell whether it is allowable to execute Vector
without first invoking a prctl() check.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
Acked-by: Joel Granados <j.granados@samsung.com>
---
Changelog v4:
- check static_assert for !CONFIG_RISCV_ISA_V case in has_vector.
Changelog v2:
 - update the comment in hwprobe.
---
 arch/riscv/include/asm/switch_to.h     |  2 +-
 arch/riscv/include/asm/vector.h        | 25 ++++++++++++++++---------
 arch/riscv/include/asm/xor.h           |  2 +-
 arch/riscv/kernel/cpufeature.c         |  5 ++++-
 arch/riscv/kernel/kernel_mode_vector.c |  4 ++--
 arch/riscv/kernel/process.c            |  4 ++--
 arch/riscv/kernel/signal.c             |  6 +++---
 arch/riscv/kernel/smpboot.c            |  2 +-
 arch/riscv/kernel/sys_hwprobe.c        |  8 ++++++--
 arch/riscv/kernel/vector.c             | 15 +++++++++------
 arch/riscv/lib/uaccess.S               |  2 +-
 11 files changed, 46 insertions(+), 29 deletions(-)

diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 7efdb0584d47..df1adf196c4f 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -78,7 +78,7 @@ do {							\
 	struct task_struct *__next = (next);		\
 	if (has_fpu())					\
 		__switch_to_fpu(__prev, __next);	\
-	if (has_vector())					\
+	if (has_vector(ZVE32X))			\
 		__switch_to_vector(__prev, __next);	\
 	((last) = __switch_to(__prev, __next));		\
 } while (0)
diff --git a/arch/riscv/include/asm/vector.h b/arch/riscv/include/asm/vector.h
index 731dcd0ed4de..ed5fb6515d54 100644
--- a/arch/riscv/include/asm/vector.h
+++ b/arch/riscv/include/asm/vector.h
@@ -8,6 +8,19 @@
 
 #include <linux/types.h>
 #include <uapi/asm-generic/errno.h>
+#include <asm/cpufeature.h>
+#include <asm/hwcap.h>
+
+#define has_vector(VEXT)									\
+({												\
+	static_assert(RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE32X ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE32F ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64X ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64F ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_ZVE64D ||				\
+		      RISCV_ISA_EXT_##VEXT == RISCV_ISA_EXT_v);					\
+	IS_ENABLED(CONFIG_RISCV_ISA_V) && riscv_has_extension_unlikely(RISCV_ISA_EXT_##VEXT);	\
+})
 
 #ifdef CONFIG_RISCV_ISA_V
 
@@ -15,9 +28,9 @@
 #include <linux/sched.h>
 #include <linux/sched/task_stack.h>
 #include <asm/ptrace.h>
-#include <asm/cpufeature.h>
 #include <asm/csr.h>
 #include <asm/asm.h>
+#include <asm/bug.h>
 
 extern unsigned long riscv_v_vsize;
 int riscv_v_setup_vsize(void);
@@ -35,11 +48,6 @@ static inline u32 riscv_v_flags(void)
 	return READ_ONCE(current->thread.riscv_v_flags);
 }
 
-static __always_inline bool has_vector(void)
-{
-	return riscv_has_extension_unlikely(RISCV_ISA_EXT_v);
-}
-
 static inline void __riscv_v_vstate_clean(struct pt_regs *regs)
 {
 	regs->status = (regs->status & ~SR_VS) | SR_VS_CLEAN;
@@ -131,7 +139,7 @@ static inline void __riscv_v_vstate_restore(struct __riscv_v_ext_state *restore_
 	riscv_v_enable();
 	asm volatile (
 		".option push\n\t"
-		".option arch, +v\n\t"
+		".option arch, +zve32x\n\t"
 		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
 		"vle8.v		v0, (%1)\n\t"
 		"add		%1, %1, %0\n\t"
@@ -153,7 +161,7 @@ static inline void __riscv_v_vstate_discard(void)
 	riscv_v_enable();
 	asm volatile (
 		".option push\n\t"
-		".option arch, +v\n\t"
+		".option arch, +zve32x\n\t"
 		"vsetvli	%0, x0, e8, m8, ta, ma\n\t"
 		"vmv.v.i	v0, -1\n\t"
 		"vmv.v.i	v8, -1\n\t"
@@ -267,7 +275,6 @@ bool riscv_v_vstate_ctrl_user_allowed(void);
 struct pt_regs;
 
 static inline int riscv_v_setup_vsize(void) { return -EOPNOTSUPP; }
-static __always_inline bool has_vector(void) { return false; }
 static inline bool riscv_v_first_use_handler(struct pt_regs *regs) { return false; }
 static inline bool riscv_v_vstate_query(struct pt_regs *regs) { return false; }
 static inline bool riscv_v_vstate_ctrl_user_allowed(void) { return false; }
diff --git a/arch/riscv/include/asm/xor.h b/arch/riscv/include/asm/xor.h
index 96011861e46b..46042ef5a2f7 100644
--- a/arch/riscv/include/asm/xor.h
+++ b/arch/riscv/include/asm/xor.h
@@ -61,7 +61,7 @@ static struct xor_block_template xor_block_rvv = {
 	do {        \
 		xor_speed(&xor_block_8regs);    \
 		xor_speed(&xor_block_32regs);    \
-		if (has_vector()) { \
+		if (has_vector(ZVE32X)) { \
 			xor_speed(&xor_block_rvv);\
 		} \
 	} while (0)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 38d09de518b1..8b52060649d2 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -715,12 +715,15 @@ void __init riscv_fill_hwcap(void)
 		elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
 	}
 
-	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
+	if (__riscv_isa_extension_available(NULL, RISCV_ISA_EXT_ZVE32X)) {
 		/*
 		 * This callsite can't fail here. It cannot fail when called on
 		 * the boot hart.
 		 */
 		riscv_v_setup_vsize();
+	}
+
+	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
 		/*
 		 * ISA string in device tree might have 'v' flag, but
 		 * CONFIG_RISCV_ISA_V is disabled in kernel.
diff --git a/arch/riscv/kernel/kernel_mode_vector.c b/arch/riscv/kernel/kernel_mode_vector.c
index 6afe80c7f03a..0d4d1a03d1c7 100644
--- a/arch/riscv/kernel/kernel_mode_vector.c
+++ b/arch/riscv/kernel/kernel_mode_vector.c
@@ -208,7 +208,7 @@ void kernel_vector_begin(void)
 {
 	bool nested = false;
 
-	if (WARN_ON(!has_vector()))
+	if (WARN_ON(!has_vector(ZVE32X)))
 		return;
 
 	BUG_ON(!may_use_simd());
@@ -236,7 +236,7 @@ EXPORT_SYMBOL_GPL(kernel_vector_begin);
  */
 void kernel_vector_end(void)
 {
-	if (WARN_ON(!has_vector()))
+	if (WARN_ON(!has_vector(ZVE32X)))
 		return;
 
 	riscv_v_disable();
diff --git a/arch/riscv/kernel/process.c b/arch/riscv/kernel/process.c
index 92922dbd5b5c..919e72f9fff6 100644
--- a/arch/riscv/kernel/process.c
+++ b/arch/riscv/kernel/process.c
@@ -178,7 +178,7 @@ void flush_thread(void)
 void arch_release_task_struct(struct task_struct *tsk)
 {
 	/* Free the vector context of datap. */
-	if (has_vector())
+	if (has_vector(ZVE32X))
 		riscv_v_thread_free(tsk);
 }
 
@@ -225,7 +225,7 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		p->thread.s[0] = 0;
 	}
 	p->thread.riscv_v_flags = 0;
-	if (has_vector())
+	if (has_vector(ZVE32X))
 		riscv_v_thread_alloc(p);
 	p->thread.ra = (unsigned long)ret_from_fork;
 	p->thread.sp = (unsigned long)childregs; /* kernel sp */
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 501e66debf69..a96e6e969a3f 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -188,7 +188,7 @@ static long restore_sigcontext(struct pt_regs *regs,
 
 			return 0;
 		case RISCV_V_MAGIC:
-			if (!has_vector() || !riscv_v_vstate_query(regs) ||
+			if (!has_vector(ZVE32X) || !riscv_v_vstate_query(regs) ||
 			    size != riscv_v_sc_size)
 				return -EINVAL;
 
@@ -210,7 +210,7 @@ static size_t get_rt_frame_size(bool cal_all)
 
 	frame_size = sizeof(*frame);
 
-	if (has_vector()) {
+	if (has_vector(ZVE32X)) {
 		if (cal_all || riscv_v_vstate_query(task_pt_regs(current)))
 			total_context_size += riscv_v_sc_size;
 	}
@@ -283,7 +283,7 @@ static long setup_sigcontext(struct rt_sigframe __user *frame,
 	if (has_fpu())
 		err |= save_fp_state(regs, &sc->sc_fpregs);
 	/* Save the vector state. */
-	if (has_vector() && riscv_v_vstate_query(regs))
+	if (has_vector(ZVE32X) && riscv_v_vstate_query(regs))
 		err |= save_v_state(regs, (void __user **)&sc_ext_ptr);
 	/* Write zero to fp-reserved space and check it on restore_sigcontext */
 	err |= __put_user(0, &sc->sc_extdesc.reserved);
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 673437ccc13d..7252666ce0da 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -214,7 +214,7 @@ asmlinkage __visible void smp_callin(void)
 	struct mm_struct *mm = &init_mm;
 	unsigned int curr_cpuid = smp_processor_id();
 
-	if (has_vector()) {
+	if (has_vector(ZVE32X)) {
 		/*
 		 * Return as early as possible so the hart with a mismatching
 		 * vlen won't boot.
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index c8219b82fbfc..e7c3fcac62a1 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -69,7 +69,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 	if (riscv_isa_extension_available(NULL, c))
 		pair->value |= RISCV_HWPROBE_IMA_C;
 
-	if (has_vector())
+	if (has_vector(v))
 		pair->value |= RISCV_HWPROBE_IMA_V;
 
 	/*
@@ -112,7 +112,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 		EXT_KEY(ZACAS);
 		EXT_KEY(ZICOND);
 
-		if (has_vector()) {
+		/*
+		 *  Vector crypto and ZVE* extensions are supported only if
+		 *  kernel has minimum V support of ZVE32X.
+		 */
+		if (has_vector(ZVE32X)) {
 			EXT_KEY(ZVE32X);
 			EXT_KEY(ZVE32F);
 			EXT_KEY(ZVE64X);
diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
index 6727d1d3b8f2..e8a47fa72351 100644
--- a/arch/riscv/kernel/vector.c
+++ b/arch/riscv/kernel/vector.c
@@ -53,7 +53,7 @@ int riscv_v_setup_vsize(void)
 
 void __init riscv_v_setup_ctx_cache(void)
 {
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return;
 
 	riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
@@ -173,8 +173,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
 	u32 __user *epc = (u32 __user *)regs->epc;
 	u32 insn = (u32)regs->badaddr;
 
+	if (!has_vector(ZVE32X))
+		return false;
+
 	/* Do not handle if V is not supported, or disabled */
-	if (!(ELF_HWCAP & COMPAT_HWCAP_ISA_V))
+	if (!riscv_v_vstate_ctrl_user_allowed())
 		return false;
 
 	/* If V has been enabled then it is not the first-use trap */
@@ -213,7 +216,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
 	bool inherit;
 	int cur, next;
 
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return;
 
 	next = riscv_v_ctrl_get_next(tsk);
@@ -235,7 +238,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
 
 long riscv_v_vstate_ctrl_get_current(void)
 {
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return -EINVAL;
 
 	return current->thread.vstate_ctrl & PR_RISCV_V_VSTATE_CTRL_MASK;
@@ -246,7 +249,7 @@ long riscv_v_vstate_ctrl_set_current(unsigned long arg)
 	bool inherit;
 	int cur, next;
 
-	if (!has_vector())
+	if (!has_vector(ZVE32X))
 		return -EINVAL;
 
 	if (arg & ~PR_RISCV_V_VSTATE_CTRL_MASK)
@@ -296,7 +299,7 @@ static struct ctl_table riscv_v_default_vstate_table[] = {
 
 static int __init riscv_v_sysctl_init(void)
 {
-	if (has_vector())
+	if (has_vector(ZVE32X))
 		if (!register_sysctl("abi", riscv_v_default_vstate_table))
 			return -EINVAL;
 	return 0;
diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
index bc22c078aba8..bbe143bb32a0 100644
--- a/arch/riscv/lib/uaccess.S
+++ b/arch/riscv/lib/uaccess.S
@@ -14,7 +14,7 @@
 
 SYM_FUNC_START(__asm_copy_to_user)
 #ifdef CONFIG_RISCV_ISA_V
-	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_v, CONFIG_RISCV_ISA_V)
+	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)
 	REG_L	t0, riscv_v_usercopy_threshold
 	bltu	a2, t0, fallback_scalar_usercopy
 	tail enter_vector_usercopy

-- 
2.44.0.rc2


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

[PATCH v4 8/9] hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro

[Andy Chiu]

(1 << 31) is a signed negative integer, and it was sign-extended when
being or'ed into the "missing" variable. This casues hwprobe not
reflecing extensions named after RISCV_HWPROBE_EXT_ZVFHMIN. Fix it by
defining it as a unsigend long long.

Fixes: 5dadda5e6a59 ("riscv: hwprobe: export Zvfh[min] ISA extensions")
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v4:
 - new patch since v4
---
 arch/riscv/include/uapi/asm/hwprobe.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index b9a0876e969f..dfa7bdbcce92 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -54,7 +54,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZFHMIN	(1 << 28)
 #define		RISCV_HWPROBE_EXT_ZIHINTNTL	(1 << 29)
 #define		RISCV_HWPROBE_EXT_ZVFH		(1 << 30)
-#define		RISCV_HWPROBE_EXT_ZVFHMIN	(1 << 31)
+#define		RISCV_HWPROBE_EXT_ZVFHMIN	(1ULL << 31)
 #define		RISCV_HWPROBE_EXT_ZFA		(1ULL << 32)
 #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
 #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)

-- 
2.44.0.rc2

[PATCH v4 8/9] hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro

[Andy Chiu]

(1 << 31) is a signed negative integer, and it was sign-extended when
being or'ed into the "missing" variable. This casues hwprobe not
reflecing extensions named after RISCV_HWPROBE_EXT_ZVFHMIN. Fix it by
defining it as a unsigend long long.

Fixes: 5dadda5e6a59 ("riscv: hwprobe: export Zvfh[min] ISA extensions")
Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v4:
 - new patch since v4
---
 arch/riscv/include/uapi/asm/hwprobe.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index b9a0876e969f..dfa7bdbcce92 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -54,7 +54,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZFHMIN	(1 << 28)
 #define		RISCV_HWPROBE_EXT_ZIHINTNTL	(1 << 29)
 #define		RISCV_HWPROBE_EXT_ZVFH		(1 << 30)
-#define		RISCV_HWPROBE_EXT_ZVFHMIN	(1 << 31)
+#define		RISCV_HWPROBE_EXT_ZVFHMIN	(1ULL << 31)
 #define		RISCV_HWPROBE_EXT_ZFA		(1ULL << 32)
 #define		RISCV_HWPROBE_EXT_ZTSO		(1ULL << 33)
 #define		RISCV_HWPROBE_EXT_ZACAS		(1ULL << 34)

-- 
2.44.0.rc2


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

[PATCH v4 9/9] selftest: run vector prctl test for ZVE32X

[Andy Chiu]

The minimal requirement for running Vector subextension on Linux is
ZVE32X. So change the test accordingly to run prctl as long as it find
it.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v4:
 - new patch since v4
---
 tools/testing/selftests/riscv/vector/vstate_prctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/riscv/vector/vstate_prctl.c b/tools/testing/selftests/riscv/vector/vstate_prctl.c
index 27668fb3b6d0..895177f6bf4c 100644
--- a/tools/testing/selftests/riscv/vector/vstate_prctl.c
+++ b/tools/testing/selftests/riscv/vector/vstate_prctl.c
@@ -88,16 +88,16 @@ int main(void)
 		return -2;
 	}
 
-	if (!(pair.value & RISCV_HWPROBE_IMA_V)) {
+	if (!(pair.value & RISCV_HWPROBE_EXT_ZVE32X)) {
 		rc = prctl(PR_RISCV_V_GET_CONTROL);
 		if (rc != -1 || errno != EINVAL) {
-			ksft_test_result_fail("GET_CONTROL should fail on kernel/hw without V\n");
+			ksft_test_result_fail("GET_CONTROL should fail on kernel/hw without ZVE32X\n");
 			return -3;
 		}
 
 		rc = prctl(PR_RISCV_V_SET_CONTROL, PR_RISCV_V_VSTATE_CTRL_ON);
 		if (rc != -1 || errno != EINVAL) {
-			ksft_test_result_fail("GET_CONTROL should fail on kernel/hw without V\n");
+			ksft_test_result_fail("SET_CONTROL should fail on kernel/hw without ZVE32X\n");
 			return -4;
 		}
 

-- 
2.44.0.rc2

[PATCH v4 9/9] selftest: run vector prctl test for ZVE32X

[Andy Chiu]

The minimal requirement for running Vector subextension on Linux is
ZVE32X. So change the test accordingly to run prctl as long as it find
it.

Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
---
Changelog v4:
 - new patch since v4
---
 tools/testing/selftests/riscv/vector/vstate_prctl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/riscv/vector/vstate_prctl.c b/tools/testing/selftests/riscv/vector/vstate_prctl.c
index 27668fb3b6d0..895177f6bf4c 100644
--- a/tools/testing/selftests/riscv/vector/vstate_prctl.c
+++ b/tools/testing/selftests/riscv/vector/vstate_prctl.c
@@ -88,16 +88,16 @@ int main(void)
 		return -2;
 	}
 
-	if (!(pair.value & RISCV_HWPROBE_IMA_V)) {
+	if (!(pair.value & RISCV_HWPROBE_EXT_ZVE32X)) {
 		rc = prctl(PR_RISCV_V_GET_CONTROL);
 		if (rc != -1 || errno != EINVAL) {
-			ksft_test_result_fail("GET_CONTROL should fail on kernel/hw without V\n");
+			ksft_test_result_fail("GET_CONTROL should fail on kernel/hw without ZVE32X\n");
 			return -3;
 		}
 
 		rc = prctl(PR_RISCV_V_SET_CONTROL, PR_RISCV_V_VSTATE_CTRL_ON);
 		if (rc != -1 || errno != EINVAL) {
-			ksft_test_result_fail("GET_CONTROL should fail on kernel/hw without V\n");
+			ksft_test_result_fail("SET_CONTROL should fail on kernel/hw without ZVE32X\n");
 			return -4;
 		}
 

-- 
2.44.0.rc2


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

Re: [PATCH v4 1/9] riscv: vector: add a comment when calling riscv_setup_vsize()

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1281 bytes --]

On Fri, Apr 12, 2024 at 02:48:57PM +0800, Andy Chiu wrote:
> The function would fail when it detects the calling hart's vlen doesn't
> match the first one's. The boot hart is the first hart calling this
> function during riscv_fill_hwcap, so it is impossible to fail here. Add
> a comment about this behavior.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v2:
>  - update the comment (Conor)
> ---
>  arch/riscv/kernel/cpufeature.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3ed2359eae35..d22b12072579 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -683,6 +683,10 @@ void __init riscv_fill_hwcap(void)
>  	}
>  
>  	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
> +		/*
> +		 * This callsite can't fail here. It cannot fail when called on
> +		 * the boot hart.

I am loathe to comment on this again, so 
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
but you could just write this as "This cannot fail when called on the
boot hart."

Cheers,
Conor.

> +		 */
>  		riscv_v_setup_vsize();
>  		/*
>  		 * ISA string in device tree might have 'v' flag, but
> 
> -- 
> 2.44.0.rc2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 1/9] riscv: vector: add a comment when calling riscv_setup_vsize()

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1281 bytes --]

On Fri, Apr 12, 2024 at 02:48:57PM +0800, Andy Chiu wrote:
> The function would fail when it detects the calling hart's vlen doesn't
> match the first one's. The boot hart is the first hart calling this
> function during riscv_fill_hwcap, so it is impossible to fail here. Add
> a comment about this behavior.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v2:
>  - update the comment (Conor)
> ---
>  arch/riscv/kernel/cpufeature.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 3ed2359eae35..d22b12072579 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -683,6 +683,10 @@ void __init riscv_fill_hwcap(void)
>  	}
>  
>  	if (elf_hwcap & COMPAT_HWCAP_ISA_V) {
> +		/*
> +		 * This callsite can't fail here. It cannot fail when called on
> +		 * the boot hart.

I am loathe to comment on this again, so 
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
but you could just write this as "This cannot fail when called on the
boot hart."

Cheers,
Conor.

> +		 */
>  		riscv_v_setup_vsize();
>  		/*
>  		 * ISA string in device tree might have 'v' flag, but
> 
> -- 
> 2.44.0.rc2
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 908 bytes --]

On Fri, Apr 12, 2024 at 02:48:58PM +0800, Andy Chiu wrote:
> Currently we only support Vector for SMP platforms, that is, all SMP
> cores have the same vlenb. If we happen to detect a mismatching vlen, it
> is better to just fail bootting it up to prevent further race/scheduling
> issues.
> 
> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> regular call in the early assembly. So a core would be parked right
> after a return from smp_callin. Note that a successful smp_callin
> does not return.
> 
> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> Reported-by: Conor Dooley <conor.dooley@microchip.com>
> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 908 bytes --]

On Fri, Apr 12, 2024 at 02:48:58PM +0800, Andy Chiu wrote:
> Currently we only support Vector for SMP platforms, that is, all SMP
> cores have the same vlenb. If we happen to detect a mismatching vlen, it
> is better to just fail bootting it up to prevent further race/scheduling
> issues.
> 
> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> regular call in the early assembly. So a core would be parked right
> after a return from smp_callin. Note that a successful smp_callin
> does not return.
> 
> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> Reported-by: Conor Dooley <conor.dooley@microchip.com>
> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 4/9] riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1990 bytes --]

On Fri, Apr 12, 2024 at 02:49:00PM +0800, Andy Chiu wrote:
> Multiple Vector subextensions are added. Also, the patch takes care of
> the dependencies of Vector subextensions by macro expansions. So, if
> some "embedded" platform only reports "zve64f" on the ISA string, the
> parser is able to expand it to zve32x zve32f zve64x and zve64f.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v3:
>  - renumber RISCV_ISA_EXT_ZVE* to rebase on top of 6.9
>  - alphabetically sort added extensions (Clément)
> Changelog v2:
>  - remove the extension itself from its isa_exts[] list (Clément)
>  - use riscv_zve64d_exts for v's extension list (Samuel)
> ---
>  arch/riscv/include/asm/hwcap.h |  5 +++++
>  arch/riscv/kernel/cpufeature.c | 36 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..f64d4e98e67c 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,11 @@
>  #define RISCV_ISA_EXT_ZTSO		72
>  #define RISCV_ISA_EXT_ZACAS		73
>  #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_ZVE32X		75
> +#define RISCV_ISA_EXT_ZVE32F		76
> +#define RISCV_ISA_EXT_ZVE64X		77
> +#define RISCV_ISA_EXT_ZVE64F		78
> +#define RISCV_ISA_EXT_ZVE64D		79
>  
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index f6f3ece60d69..38d09de518b1 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -188,6 +188,35 @@ static const unsigned int riscv_zvbb_exts[] = {
>  	RISCV_ISA_EXT_ZVKB
>  };
>  
> +#define RISCV_ISA_EXT_ZVE32F_IMPLY_LIST	\
> +	RISCV_ISA_EXT_ZVE32X,

Not really a reason to have a list here, there's only one thing implied.
Otherwise,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 4/9] riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1990 bytes --]

On Fri, Apr 12, 2024 at 02:49:00PM +0800, Andy Chiu wrote:
> Multiple Vector subextensions are added. Also, the patch takes care of
> the dependencies of Vector subextensions by macro expansions. So, if
> some "embedded" platform only reports "zve64f" on the ISA string, the
> parser is able to expand it to zve32x zve32f zve64x and zve64f.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v3:
>  - renumber RISCV_ISA_EXT_ZVE* to rebase on top of 6.9
>  - alphabetically sort added extensions (Clément)
> Changelog v2:
>  - remove the extension itself from its isa_exts[] list (Clément)
>  - use riscv_zve64d_exts for v's extension list (Samuel)
> ---
>  arch/riscv/include/asm/hwcap.h |  5 +++++
>  arch/riscv/kernel/cpufeature.c | 36 +++++++++++++++++++++++++++++++++++-
>  2 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e17d0078a651..f64d4e98e67c 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -81,6 +81,11 @@
>  #define RISCV_ISA_EXT_ZTSO		72
>  #define RISCV_ISA_EXT_ZACAS		73
>  #define RISCV_ISA_EXT_XANDESPMU		74
> +#define RISCV_ISA_EXT_ZVE32X		75
> +#define RISCV_ISA_EXT_ZVE32F		76
> +#define RISCV_ISA_EXT_ZVE64X		77
> +#define RISCV_ISA_EXT_ZVE64F		78
> +#define RISCV_ISA_EXT_ZVE64D		79
>  
>  #define RISCV_ISA_EXT_XLINUXENVCFG	127
>  
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index f6f3ece60d69..38d09de518b1 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -188,6 +188,35 @@ static const unsigned int riscv_zvbb_exts[] = {
>  	RISCV_ISA_EXT_ZVKB
>  };
>  
> +#define RISCV_ISA_EXT_ZVE32F_IMPLY_LIST	\
> +	RISCV_ISA_EXT_ZVE32X,

Not really a reason to have a list here, there's only one thing implied.
Otherwise,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 5/9] dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 340 bytes --]

On Fri, Apr 12, 2024 at 02:49:01PM +0800, Andy Chiu wrote:
> Add description for Zve32x Zve32f Zve64x Zve64f Zve64d ISA extensions.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>

Technically this should be before the patch using them in the kernel,
but 
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 5/9] dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 340 bytes --]

On Fri, Apr 12, 2024 at 02:49:01PM +0800, Andy Chiu wrote:
> Add description for Zve32x Zve32f Zve64x Zve64f Zve64d ISA extensions.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>

Technically this should be before the patch using them in the kernel,
but 
Acked-by: Conor Dooley <conor.dooley@microchip.com>

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 3/9] riscv: cpufeature: call match_isa_ext() for single-letter extensions

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 515 bytes --]

On Fri, Apr 12, 2024 at 02:48:59PM +0800, Andy Chiu wrote:
> Single-letter extensions may also imply multiple subextensions. For
> example, Vector extension implies zve64d, and zve64d implies zve64f.
> 
> Extension parsing for "riscv,isa-extensions" has the ability to resolve
> the dependency by calling match_isa_ext(). This patch makes deprecated
> parser call the same function for single letter extensions.

Sure, why not..

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 3/9] riscv: cpufeature: call match_isa_ext() for single-letter extensions

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 515 bytes --]

On Fri, Apr 12, 2024 at 02:48:59PM +0800, Andy Chiu wrote:
> Single-letter extensions may also imply multiple subextensions. For
> example, Vector extension implies zve64d, and zve64d implies zve64f.
> 
> Extension parsing for "riscv,isa-extensions" has the ability to resolve
> the dependency by calling match_isa_ext(). This patch makes deprecated
> parser call the same function for single letter extensions.

Sure, why not..

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5922 bytes --]

+CC Eric, Jerry

On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
> Make has_vector take one argument. This argument represents the minimum
> Vector subextension that the following Vector actions assume.
> 
> Also, change riscv_v_first_use_handler(), and boot code that calls
> riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
> ZVE32X.
> 
> Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
> compiled and run with ZVE32X should be supported by the kernel on most
> aspects. This includes context-switch, signal, ptrace, prctl, and
> hwprobe.
> 
> One exception is that ELF_HWCAP returns 'V' only if full V is supported
> on the platform. This means that the system without a full V must not
> rely on ELF_HWCAP to tell whether it is allowable to execute Vector
> without first invoking a prctl() check.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> Acked-by: Joel Granados <j.granados@samsung.com>

I'm not sure that I like this patch to be honest. As far as I can tell,
every user here of has_vector(ext) is ZVE32X, so why bother actually
having an argument?

Could we just document that has_vector() is just a tyre kick of "is
there a vector unit and are we allowed to use it", and anything
requiring more than the bare-minimum (so zve32x?)must explicitly check
for that form of vector using riscv_has_extension_[un]likely()?

Finally, the in-kernel crypto stuff or other things that use
can_use_simd() to check for vector support - do they all function correctly
with all of the vector flavours? I don't understand the vector
extensions well enough to evaluate that - I know that they do check for
the individual extensions like Zvkb during probe but don't have anything
for the vector version (at least in the chacha20 and sha256 glue code).
If they don't, then we need to make sure those drivers do not probe with
the cut-down variants.

Eric/Jerry (although read the previous paragraph too):
I noticed that the sha256 glue code calls crypto_simd_usable(), and in
turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
does not call either, which seems to violate the edict in
kernel_vector_begin()'s kerneldoc:
"Must not be called unless may_use_simd() returns true."

What am I missing there?

Cheers,
Conor.


> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index c8219b82fbfc..e7c3fcac62a1 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -69,7 +69,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  	if (riscv_isa_extension_available(NULL, c))
>  		pair->value |= RISCV_HWPROBE_IMA_C;
>  
> -	if (has_vector())
> +	if (has_vector(v))
>  		pair->value |= RISCV_HWPROBE_IMA_V;
>  
>  	/*
> @@ -112,7 +112,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		EXT_KEY(ZACAS);
>  		EXT_KEY(ZICOND);
>  
> -		if (has_vector()) {
> +		/*
> +		 *  Vector crypto and ZVE* extensions are supported only if
> +		 *  kernel has minimum V support of ZVE32X.
> +		 */
> +		if (has_vector(ZVE32X)) {
>  			EXT_KEY(ZVE32X);
>  			EXT_KEY(ZVE32F);
>  			EXT_KEY(ZVE64X);

I find this to be an indicate of the new has_vector() being a poor API,
as it is confusing that a check
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 6727d1d3b8f2..e8a47fa72351 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -53,7 +53,7 @@ int riscv_v_setup_vsize(void)
>  
>  void __init riscv_v_setup_ctx_cache(void)
>  {
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return;
>  
>  	riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
> @@ -173,8 +173,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
>  	u32 __user *epc = (u32 __user *)regs->epc;
>  	u32 insn = (u32)regs->badaddr;
>  
> +	if (!has_vector(ZVE32X))
> +		return false;
> +
>  	/* Do not handle if V is not supported, or disabled */
> -	if (!(ELF_HWCAP & COMPAT_HWCAP_ISA_V))
> +	if (!riscv_v_vstate_ctrl_user_allowed())
>  		return false;
>  
>  	/* If V has been enabled then it is not the first-use trap */
> @@ -213,7 +216,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
>  	bool inherit;
>  	int cur, next;
>  
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return;
>  
>  	next = riscv_v_ctrl_get_next(tsk);
> @@ -235,7 +238,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
>  
>  long riscv_v_vstate_ctrl_get_current(void)
>  {
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return -EINVAL;
>  
>  	return current->thread.vstate_ctrl & PR_RISCV_V_VSTATE_CTRL_MASK;
> @@ -246,7 +249,7 @@ long riscv_v_vstate_ctrl_set_current(unsigned long arg)
>  	bool inherit;
>  	int cur, next;
>  
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return -EINVAL;
>  
>  	if (arg & ~PR_RISCV_V_VSTATE_CTRL_MASK)
> @@ -296,7 +299,7 @@ static struct ctl_table riscv_v_default_vstate_table[] = {
>  
>  static int __init riscv_v_sysctl_init(void)
>  {
> -	if (has_vector())
> +	if (has_vector(ZVE32X))
>  		if (!register_sysctl("abi", riscv_v_default_vstate_table))
>  			return -EINVAL;
>  	return 0;
> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
> index bc22c078aba8..bbe143bb32a0 100644
> --- a/arch/riscv/lib/uaccess.S
> +++ b/arch/riscv/lib/uaccess.S
> @@ -14,7 +14,7 @@
>  
>  SYM_FUNC_START(__asm_copy_to_user)
>  #ifdef CONFIG_RISCV_ISA_V
> -	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_v, CONFIG_RISCV_ISA_V)
> +	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)
>  	REG_L	t0, riscv_v_usercopy_threshold
>  	bltu	a2, t0, fallback_scalar_usercopy
>  	tail enter_vector_usercopy
> 
> -- 
> 2.44.0.rc2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 5922 bytes --]

+CC Eric, Jerry

On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
> Make has_vector take one argument. This argument represents the minimum
> Vector subextension that the following Vector actions assume.
> 
> Also, change riscv_v_first_use_handler(), and boot code that calls
> riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
> ZVE32X.
> 
> Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
> compiled and run with ZVE32X should be supported by the kernel on most
> aspects. This includes context-switch, signal, ptrace, prctl, and
> hwprobe.
> 
> One exception is that ELF_HWCAP returns 'V' only if full V is supported
> on the platform. This means that the system without a full V must not
> rely on ELF_HWCAP to tell whether it is allowable to execute Vector
> without first invoking a prctl() check.
> 
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> Acked-by: Joel Granados <j.granados@samsung.com>

I'm not sure that I like this patch to be honest. As far as I can tell,
every user here of has_vector(ext) is ZVE32X, so why bother actually
having an argument?

Could we just document that has_vector() is just a tyre kick of "is
there a vector unit and are we allowed to use it", and anything
requiring more than the bare-minimum (so zve32x?)must explicitly check
for that form of vector using riscv_has_extension_[un]likely()?

Finally, the in-kernel crypto stuff or other things that use
can_use_simd() to check for vector support - do they all function correctly
with all of the vector flavours? I don't understand the vector
extensions well enough to evaluate that - I know that they do check for
the individual extensions like Zvkb during probe but don't have anything
for the vector version (at least in the chacha20 and sha256 glue code).
If they don't, then we need to make sure those drivers do not probe with
the cut-down variants.

Eric/Jerry (although read the previous paragraph too):
I noticed that the sha256 glue code calls crypto_simd_usable(), and in
turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
does not call either, which seems to violate the edict in
kernel_vector_begin()'s kerneldoc:
"Must not be called unless may_use_simd() returns true."

What am I missing there?

Cheers,
Conor.


> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index c8219b82fbfc..e7c3fcac62a1 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -69,7 +69,7 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  	if (riscv_isa_extension_available(NULL, c))
>  		pair->value |= RISCV_HWPROBE_IMA_C;
>  
> -	if (has_vector())
> +	if (has_vector(v))
>  		pair->value |= RISCV_HWPROBE_IMA_V;
>  
>  	/*
> @@ -112,7 +112,11 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  		EXT_KEY(ZACAS);
>  		EXT_KEY(ZICOND);
>  
> -		if (has_vector()) {
> +		/*
> +		 *  Vector crypto and ZVE* extensions are supported only if
> +		 *  kernel has minimum V support of ZVE32X.
> +		 */
> +		if (has_vector(ZVE32X)) {
>  			EXT_KEY(ZVE32X);
>  			EXT_KEY(ZVE32F);
>  			EXT_KEY(ZVE64X);

I find this to be an indicate of the new has_vector() being a poor API,
as it is confusing that a check
> diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> index 6727d1d3b8f2..e8a47fa72351 100644
> --- a/arch/riscv/kernel/vector.c
> +++ b/arch/riscv/kernel/vector.c
> @@ -53,7 +53,7 @@ int riscv_v_setup_vsize(void)
>  
>  void __init riscv_v_setup_ctx_cache(void)
>  {
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return;
>  
>  	riscv_v_user_cachep = kmem_cache_create_usercopy("riscv_vector_ctx",
> @@ -173,8 +173,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
>  	u32 __user *epc = (u32 __user *)regs->epc;
>  	u32 insn = (u32)regs->badaddr;
>  
> +	if (!has_vector(ZVE32X))
> +		return false;
> +
>  	/* Do not handle if V is not supported, or disabled */
> -	if (!(ELF_HWCAP & COMPAT_HWCAP_ISA_V))
> +	if (!riscv_v_vstate_ctrl_user_allowed())
>  		return false;
>  
>  	/* If V has been enabled then it is not the first-use trap */
> @@ -213,7 +216,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
>  	bool inherit;
>  	int cur, next;
>  
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return;
>  
>  	next = riscv_v_ctrl_get_next(tsk);
> @@ -235,7 +238,7 @@ void riscv_v_vstate_ctrl_init(struct task_struct *tsk)
>  
>  long riscv_v_vstate_ctrl_get_current(void)
>  {
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return -EINVAL;
>  
>  	return current->thread.vstate_ctrl & PR_RISCV_V_VSTATE_CTRL_MASK;
> @@ -246,7 +249,7 @@ long riscv_v_vstate_ctrl_set_current(unsigned long arg)
>  	bool inherit;
>  	int cur, next;
>  
> -	if (!has_vector())
> +	if (!has_vector(ZVE32X))
>  		return -EINVAL;
>  
>  	if (arg & ~PR_RISCV_V_VSTATE_CTRL_MASK)
> @@ -296,7 +299,7 @@ static struct ctl_table riscv_v_default_vstate_table[] = {
>  
>  static int __init riscv_v_sysctl_init(void)
>  {
> -	if (has_vector())
> +	if (has_vector(ZVE32X))
>  		if (!register_sysctl("abi", riscv_v_default_vstate_table))
>  			return -EINVAL;
>  	return 0;
> diff --git a/arch/riscv/lib/uaccess.S b/arch/riscv/lib/uaccess.S
> index bc22c078aba8..bbe143bb32a0 100644
> --- a/arch/riscv/lib/uaccess.S
> +++ b/arch/riscv/lib/uaccess.S
> @@ -14,7 +14,7 @@
>  
>  SYM_FUNC_START(__asm_copy_to_user)
>  #ifdef CONFIG_RISCV_ISA_V
> -	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_v, CONFIG_RISCV_ISA_V)
> +	ALTERNATIVE("j fallback_scalar_usercopy", "nop", 0, RISCV_ISA_EXT_ZVE32X, CONFIG_RISCV_ISA_V)
>  	REG_L	t0, riscv_v_usercopy_threshold
>  	bltu	a2, t0, fallback_scalar_usercopy
>  	tail enter_vector_usercopy
> 
> -- 
> 2.44.0.rc2
> 

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

Hi Conor,

On Thu, Apr 18, 2024 at 12:02:10PM +0100, Conor Dooley wrote:
> +CC Eric, Jerry
> 
> On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
> > Make has_vector take one argument. This argument represents the minimum
> > Vector subextension that the following Vector actions assume.
> > 
> > Also, change riscv_v_first_use_handler(), and boot code that calls
> > riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
> > ZVE32X.
> > 
> > Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
> > compiled and run with ZVE32X should be supported by the kernel on most
> > aspects. This includes context-switch, signal, ptrace, prctl, and
> > hwprobe.
> > 
> > One exception is that ELF_HWCAP returns 'V' only if full V is supported
> > on the platform. This means that the system without a full V must not
> > rely on ELF_HWCAP to tell whether it is allowable to execute Vector
> > without first invoking a prctl() check.
> > 
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > Acked-by: Joel Granados <j.granados@samsung.com>
> 
> I'm not sure that I like this patch to be honest. As far as I can tell,
> every user here of has_vector(ext) is ZVE32X, so why bother actually
> having an argument?
> 
> Could we just document that has_vector() is just a tyre kick of "is
> there a vector unit and are we allowed to use it", and anything
> requiring more than the bare-minimum (so zve32x?)must explicitly check
> for that form of vector using riscv_has_extension_[un]likely()?
> 
> Finally, the in-kernel crypto stuff or other things that use
> can_use_simd() to check for vector support - do they all function correctly
> with all of the vector flavours? I don't understand the vector
> extensions well enough to evaluate that - I know that they do check for
> the individual extensions like Zvkb during probe but don't have anything
> for the vector version (at least in the chacha20 and sha256 glue code).
> If they don't, then we need to make sure those drivers do not probe with
> the cut-down variants.

As far as I know, none of the RISC-V vector crypto code has been tested with
Zve* yet.  Currently it always checks for VLEN >= 128, which should exclude most
Zve* implementations.

Currently it doesn't check for EEW >= 64, even though it sometimes assumes that.
It looks like a check for EEW >= 64 needs to be added in order to exclude Zve32x
and Zve32f implementations that don't support EEW == 64.

If it would be useful to do so, we should be able to enable some of the code
with a smaller VLEN and/or EEW once it has been tested in those configurations.
Some of it should work, but some of it won't be able to work.  (For example, the
SHA512 instructions require EEW==64.)

Also note that currently all the RISC-V vector crypto code only supports riscv64
(XLEN=64).  Similarly, that could be relaxed in the future if people really need
the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
need to be revised and tested in that configuration.

> Eric/Jerry (although read the previous paragraph too):
> I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> does not call either, which seems to violate the edict in
> kernel_vector_begin()'s kerneldoc:
> "Must not be called unless may_use_simd() returns true."

skcipher algorithms can only be invoked in process and softirq context.  This
differs from shash algorithms which can be invoked in any context.

My understanding is that, like arm64, RISC-V always allows non-nested
kernel-mode vector to be used in process and softirq context -- and in fact,
this was intentionally done in order to support use cases like this.  So that's
why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
kernel_vector_begin().

Has that changed?  If so, why?

Some architectures like x86 do provide no-SIMD fallbacks for all skcipher
algorithms, but it's very annoying to do.  We were hoping to avoid that in
RISC-V.

- Eric

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

Hi Conor,

On Thu, Apr 18, 2024 at 12:02:10PM +0100, Conor Dooley wrote:
> +CC Eric, Jerry
> 
> On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
> > Make has_vector take one argument. This argument represents the minimum
> > Vector subextension that the following Vector actions assume.
> > 
> > Also, change riscv_v_first_use_handler(), and boot code that calls
> > riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
> > ZVE32X.
> > 
> > Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
> > compiled and run with ZVE32X should be supported by the kernel on most
> > aspects. This includes context-switch, signal, ptrace, prctl, and
> > hwprobe.
> > 
> > One exception is that ELF_HWCAP returns 'V' only if full V is supported
> > on the platform. This means that the system without a full V must not
> > rely on ELF_HWCAP to tell whether it is allowable to execute Vector
> > without first invoking a prctl() check.
> > 
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > Acked-by: Joel Granados <j.granados@samsung.com>
> 
> I'm not sure that I like this patch to be honest. As far as I can tell,
> every user here of has_vector(ext) is ZVE32X, so why bother actually
> having an argument?
> 
> Could we just document that has_vector() is just a tyre kick of "is
> there a vector unit and are we allowed to use it", and anything
> requiring more than the bare-minimum (so zve32x?)must explicitly check
> for that form of vector using riscv_has_extension_[un]likely()?
> 
> Finally, the in-kernel crypto stuff or other things that use
> can_use_simd() to check for vector support - do they all function correctly
> with all of the vector flavours? I don't understand the vector
> extensions well enough to evaluate that - I know that they do check for
> the individual extensions like Zvkb during probe but don't have anything
> for the vector version (at least in the chacha20 and sha256 glue code).
> If they don't, then we need to make sure those drivers do not probe with
> the cut-down variants.

As far as I know, none of the RISC-V vector crypto code has been tested with
Zve* yet.  Currently it always checks for VLEN >= 128, which should exclude most
Zve* implementations.

Currently it doesn't check for EEW >= 64, even though it sometimes assumes that.
It looks like a check for EEW >= 64 needs to be added in order to exclude Zve32x
and Zve32f implementations that don't support EEW == 64.

If it would be useful to do so, we should be able to enable some of the code
with a smaller VLEN and/or EEW once it has been tested in those configurations.
Some of it should work, but some of it won't be able to work.  (For example, the
SHA512 instructions require EEW==64.)

Also note that currently all the RISC-V vector crypto code only supports riscv64
(XLEN=64).  Similarly, that could be relaxed in the future if people really need
the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
need to be revised and tested in that configuration.

> Eric/Jerry (although read the previous paragraph too):
> I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> does not call either, which seems to violate the edict in
> kernel_vector_begin()'s kerneldoc:
> "Must not be called unless may_use_simd() returns true."

skcipher algorithms can only be invoked in process and softirq context.  This
differs from shash algorithms which can be invoked in any context.

My understanding is that, like arm64, RISC-V always allows non-nested
kernel-mode vector to be used in process and softirq context -- and in fact,
this was intentionally done in order to support use cases like this.  So that's
why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
kernel_vector_begin().

Has that changed?  If so, why?

Some architectures like x86 do provide no-SIMD fallbacks for all skcipher
algorithms, but it's very annoying to do.  We were hoping to avoid that in
RISC-V.

- Eric

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4630 bytes --]

On Thu, Apr 18, 2024 at 08:52:56AM -0700, Eric Biggers wrote:
> Hi Conor,
> 
> On Thu, Apr 18, 2024 at 12:02:10PM +0100, Conor Dooley wrote:
> > +CC Eric, Jerry
> > 
> > On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
> > > Make has_vector take one argument. This argument represents the minimum
> > > Vector subextension that the following Vector actions assume.
> > > 
> > > Also, change riscv_v_first_use_handler(), and boot code that calls
> > > riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
> > > ZVE32X.
> > > 
> > > Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
> > > compiled and run with ZVE32X should be supported by the kernel on most
> > > aspects. This includes context-switch, signal, ptrace, prctl, and
> > > hwprobe.
> > > 
> > > One exception is that ELF_HWCAP returns 'V' only if full V is supported
> > > on the platform. This means that the system without a full V must not
> > > rely on ELF_HWCAP to tell whether it is allowable to execute Vector
> > > without first invoking a prctl() check.
> > > 
> > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > > Acked-by: Joel Granados <j.granados@samsung.com>
> > 
> > I'm not sure that I like this patch to be honest. As far as I can tell,
> > every user here of has_vector(ext) is ZVE32X, so why bother actually
> > having an argument?
> > 
> > Could we just document that has_vector() is just a tyre kick of "is
> > there a vector unit and are we allowed to use it", and anything
> > requiring more than the bare-minimum (so zve32x?)must explicitly check
> > for that form of vector using riscv_has_extension_[un]likely()?
> > 
> > Finally, the in-kernel crypto stuff or other things that use
> > can_use_simd() to check for vector support - do they all function correctly
> > with all of the vector flavours? I don't understand the vector
> > extensions well enough to evaluate that - I know that they do check for
> > the individual extensions like Zvkb during probe but don't have anything
> > for the vector version (at least in the chacha20 and sha256 glue code).
> > If they don't, then we need to make sure those drivers do not probe with
> > the cut-down variants.
> 
> As far as I know, none of the RISC-V vector crypto code has been tested with
> Zve* yet.  Currently it always checks for VLEN >= 128, which should exclude most
> Zve* implementations.
> 
> Currently it doesn't check for EEW >= 64, even though it sometimes assumes that.
> It looks like a check for EEW >= 64 needs to be added in order to exclude Zve32x
> and Zve32f implementations that don't support EEW == 64.

Cool, glad I asked then :)

> If it would be useful to do so, we should be able to enable some of the code
> with a smaller VLEN and/or EEW once it has been tested in those configurations.
> Some of it should work, but some of it won't be able to work.  (For example, the
> SHA512 instructions require EEW==64.)
> 
> Also note that currently all the RISC-V vector crypto code only supports riscv64
> (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> need to be revised and tested in that configuration.
> 
> > Eric/Jerry (although read the previous paragraph too):
> > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > does not call either, which seems to violate the edict in
> > kernel_vector_begin()'s kerneldoc:
> > "Must not be called unless may_use_simd() returns true."
> 
> skcipher algorithms can only be invoked in process and softirq context.  This
> differs from shash algorithms which can be invoked in any context.
> 
> My understanding is that, like arm64, RISC-V always allows non-nested
> kernel-mode vector to be used in process and softirq context -- and in fact,
> this was intentionally done in order to support use cases like this.  So that's
> why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> kernel_vector_begin().

I see, thanks for explaining that. I think you should probably check
somewhere if has_vector() returns true in that driver though before
using vector instructions. Only checking vlen seems to me like relying on
an implementation detail and if we set vlen for the T-Head/0.7.1 vector
it'd be fooled. That said, I don't think that any of the 0.7.1 vector
systems actually support Zvkb, but I hope you get my drift.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 4630 bytes --]

On Thu, Apr 18, 2024 at 08:52:56AM -0700, Eric Biggers wrote:
> Hi Conor,
> 
> On Thu, Apr 18, 2024 at 12:02:10PM +0100, Conor Dooley wrote:
> > +CC Eric, Jerry
> > 
> > On Fri, Apr 12, 2024 at 02:49:03PM +0800, Andy Chiu wrote:
> > > Make has_vector take one argument. This argument represents the minimum
> > > Vector subextension that the following Vector actions assume.
> > > 
> > > Also, change riscv_v_first_use_handler(), and boot code that calls
> > > riscv_v_setup_vsize() to accept the minimum Vector sub-extension,
> > > ZVE32X.
> > > 
> > > Most kernel/user interfaces requires minimum of ZVE32X. Thus, programs
> > > compiled and run with ZVE32X should be supported by the kernel on most
> > > aspects. This includes context-switch, signal, ptrace, prctl, and
> > > hwprobe.
> > > 
> > > One exception is that ELF_HWCAP returns 'V' only if full V is supported
> > > on the platform. This means that the system without a full V must not
> > > rely on ELF_HWCAP to tell whether it is allowable to execute Vector
> > > without first invoking a prctl() check.
> > > 
> > > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > > Acked-by: Joel Granados <j.granados@samsung.com>
> > 
> > I'm not sure that I like this patch to be honest. As far as I can tell,
> > every user here of has_vector(ext) is ZVE32X, so why bother actually
> > having an argument?
> > 
> > Could we just document that has_vector() is just a tyre kick of "is
> > there a vector unit and are we allowed to use it", and anything
> > requiring more than the bare-minimum (so zve32x?)must explicitly check
> > for that form of vector using riscv_has_extension_[un]likely()?
> > 
> > Finally, the in-kernel crypto stuff or other things that use
> > can_use_simd() to check for vector support - do they all function correctly
> > with all of the vector flavours? I don't understand the vector
> > extensions well enough to evaluate that - I know that they do check for
> > the individual extensions like Zvkb during probe but don't have anything
> > for the vector version (at least in the chacha20 and sha256 glue code).
> > If they don't, then we need to make sure those drivers do not probe with
> > the cut-down variants.
> 
> As far as I know, none of the RISC-V vector crypto code has been tested with
> Zve* yet.  Currently it always checks for VLEN >= 128, which should exclude most
> Zve* implementations.
> 
> Currently it doesn't check for EEW >= 64, even though it sometimes assumes that.
> It looks like a check for EEW >= 64 needs to be added in order to exclude Zve32x
> and Zve32f implementations that don't support EEW == 64.

Cool, glad I asked then :)

> If it would be useful to do so, we should be able to enable some of the code
> with a smaller VLEN and/or EEW once it has been tested in those configurations.
> Some of it should work, but some of it won't be able to work.  (For example, the
> SHA512 instructions require EEW==64.)
> 
> Also note that currently all the RISC-V vector crypto code only supports riscv64
> (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> need to be revised and tested in that configuration.
> 
> > Eric/Jerry (although read the previous paragraph too):
> > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > does not call either, which seems to violate the edict in
> > kernel_vector_begin()'s kerneldoc:
> > "Must not be called unless may_use_simd() returns true."
> 
> skcipher algorithms can only be invoked in process and softirq context.  This
> differs from shash algorithms which can be invoked in any context.
> 
> My understanding is that, like arm64, RISC-V always allows non-nested
> kernel-mode vector to be used in process and softirq context -- and in fact,
> this was intentionally done in order to support use cases like this.  So that's
> why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> kernel_vector_begin().

I see, thanks for explaining that. I think you should probably check
somewhere if has_vector() returns true in that driver though before
using vector instructions. Only checking vlen seems to me like relying on
an implementation detail and if we set vlen for the T-Head/0.7.1 vector
it'd be fooled. That said, I don't think that any of the 0.7.1 vector
systems actually support Zvkb, but I hope you get my drift.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > If it would be useful to do so, we should be able to enable some of the code
> > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > Some of it should work, but some of it won't be able to work.  (For example, the
> > SHA512 instructions require EEW==64.)
> > 
> > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > need to be revised and tested in that configuration.
> > 
> > > Eric/Jerry (although read the previous paragraph too):
> > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > does not call either, which seems to violate the edict in
> > > kernel_vector_begin()'s kerneldoc:
> > > "Must not be called unless may_use_simd() returns true."
> > 
> > skcipher algorithms can only be invoked in process and softirq context.  This
> > differs from shash algorithms which can be invoked in any context.
> > 
> > My understanding is that, like arm64, RISC-V always allows non-nested
> > kernel-mode vector to be used in process and softirq context -- and in fact,
> > this was intentionally done in order to support use cases like this.  So that's
> > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > kernel_vector_begin().
> 
> I see, thanks for explaining that. I think you should probably check
> somewhere if has_vector() returns true in that driver though before
> using vector instructions. Only checking vlen seems to me like relying on
> an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> systems actually support Zvkb, but I hope you get my drift.

All the algorithms check for at least one of the vector crypto extensions being
supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
should return whether the ratified version of Zvkb is supported, and likewise
for the other vector crypto extensions.  The ratified version of the vector
crypto extensions depends on the ratified version of the vector extension.  So
there should be no issue.  If there is, the RISC-V core architecture code needs
to be fixed to not declare that extensions are supported when they are actually
incompatible non-standard versions of those extensions.  Incompatible
non-standard extensions should be represented as separate extensions.

- Eric

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > If it would be useful to do so, we should be able to enable some of the code
> > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > Some of it should work, but some of it won't be able to work.  (For example, the
> > SHA512 instructions require EEW==64.)
> > 
> > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > need to be revised and tested in that configuration.
> > 
> > > Eric/Jerry (although read the previous paragraph too):
> > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > does not call either, which seems to violate the edict in
> > > kernel_vector_begin()'s kerneldoc:
> > > "Must not be called unless may_use_simd() returns true."
> > 
> > skcipher algorithms can only be invoked in process and softirq context.  This
> > differs from shash algorithms which can be invoked in any context.
> > 
> > My understanding is that, like arm64, RISC-V always allows non-nested
> > kernel-mode vector to be used in process and softirq context -- and in fact,
> > this was intentionally done in order to support use cases like this.  So that's
> > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > kernel_vector_begin().
> 
> I see, thanks for explaining that. I think you should probably check
> somewhere if has_vector() returns true in that driver though before
> using vector instructions. Only checking vlen seems to me like relying on
> an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> systems actually support Zvkb, but I hope you get my drift.

All the algorithms check for at least one of the vector crypto extensions being
supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
should return whether the ratified version of Zvkb is supported, and likewise
for the other vector crypto extensions.  The ratified version of the vector
crypto extensions depends on the ratified version of the vector extension.  So
there should be no issue.  If there is, the RISC-V core architecture code needs
to be fixed to not declare that extensions are supported when they are actually
incompatible non-standard versions of those extensions.  Incompatible
non-standard extensions should be represented as separate extensions.

- Eric

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > If it would be useful to do so, we should be able to enable some of the code
> > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > SHA512 instructions require EEW==64.)
> > > 
> > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > need to be revised and tested in that configuration.
> > > 
> > > > Eric/Jerry (although read the previous paragraph too):
> > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > does not call either, which seems to violate the edict in
> > > > kernel_vector_begin()'s kerneldoc:
> > > > "Must not be called unless may_use_simd() returns true."
> > > 
> > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > differs from shash algorithms which can be invoked in any context.
> > > 
> > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > this was intentionally done in order to support use cases like this.  So that's
> > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > kernel_vector_begin().
> > 
> > I see, thanks for explaining that. I think you should probably check
> > somewhere if has_vector() returns true in that driver though before
> > using vector instructions. Only checking vlen seems to me like relying on
> > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > systems actually support Zvkb, but I hope you get my drift.
> 
> All the algorithms check for at least one of the vector crypto extensions being
> supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> should return whether the ratified version of Zvkb is supported, and likewise
> for the other vector crypto extensions.  The ratified version of the vector
> crypto extensions depends on the ratified version of the vector extension.  So
> there should be no issue.  If there is, the RISC-V core architecture code needs
> to be fixed to not declare that extensions are supported when they are actually
> incompatible non-standard versions of those extensions.  Incompatible
> non-standard extensions should be represented as separate extensions.
> 

It probably makes sense to check has_vector() to exclude Zve* for now, though.

I am just concerned about how you're suggesting that non-standard extensions
might be pretending to be standard ones and individual users of kernel-mode
vector would need to work around that.  I think that neither has_vector() nor
'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
vector extension is non-standard.

- Eric

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > If it would be useful to do so, we should be able to enable some of the code
> > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > SHA512 instructions require EEW==64.)
> > > 
> > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > need to be revised and tested in that configuration.
> > > 
> > > > Eric/Jerry (although read the previous paragraph too):
> > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > does not call either, which seems to violate the edict in
> > > > kernel_vector_begin()'s kerneldoc:
> > > > "Must not be called unless may_use_simd() returns true."
> > > 
> > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > differs from shash algorithms which can be invoked in any context.
> > > 
> > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > this was intentionally done in order to support use cases like this.  So that's
> > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > kernel_vector_begin().
> > 
> > I see, thanks for explaining that. I think you should probably check
> > somewhere if has_vector() returns true in that driver though before
> > using vector instructions. Only checking vlen seems to me like relying on
> > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > systems actually support Zvkb, but I hope you get my drift.
> 
> All the algorithms check for at least one of the vector crypto extensions being
> supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> should return whether the ratified version of Zvkb is supported, and likewise
> for the other vector crypto extensions.  The ratified version of the vector
> crypto extensions depends on the ratified version of the vector extension.  So
> there should be no issue.  If there is, the RISC-V core architecture code needs
> to be fixed to not declare that extensions are supported when they are actually
> incompatible non-standard versions of those extensions.  Incompatible
> non-standard extensions should be represented as separate extensions.
> 

It probably makes sense to check has_vector() to exclude Zve* for now, though.

I am just concerned about how you're suggesting that non-standard extensions
might be pretending to be standard ones and individual users of kernel-mode
vector would need to work around that.  I think that neither has_vector() nor
'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
vector extension is non-standard.

- Eric

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 6252 bytes --]

On Thu, Apr 18, 2024 at 10:39:46AM -0700, Eric Biggers wrote:
> On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > > If it would be useful to do so, we should be able to enable some of the code
> > > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > > SHA512 instructions require EEW==64.)
> > > > 
> > > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > > need to be revised and tested in that configuration.
> > > > 
> > > > > Eric/Jerry (although read the previous paragraph too):
> > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > > does not call either, which seems to violate the edict in
> > > > > kernel_vector_begin()'s kerneldoc:
> > > > > "Must not be called unless may_use_simd() returns true."
> > > > 
> > > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > > differs from shash algorithms which can be invoked in any context.
> > > > 
> > > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > > this was intentionally done in order to support use cases like this.  So that's
> > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > > kernel_vector_begin().
> > > 
> > > I see, thanks for explaining that. I think you should probably check
> > > somewhere if has_vector() returns true in that driver though before
> > > using vector instructions. Only checking vlen seems to me like relying on
> > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > > systems actually support Zvkb, but I hope you get my drift.
> > 
> > All the algorithms check for at least one of the vector crypto extensions being
> > supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> > should return whether the ratified version of Zvkb is supported, and likewise
> > for the other vector crypto extensions.  The ratified version of the vector
> > crypto extensions depends on the ratified version of the vector extension.

That's great if it does require that the version of the vector extension
must be standard. Higher quality spec than most if it does. But
"supported" in the context of riscv_isa_extension_available() means that
the hardware supports it (or set of harts), not that the currently
running kernel does. The Kconfig deps that must be met for the code to be
built at least mean the kernel is built with vector support, leaving only
"the kernel was built with vector support and the hardware supports vector
but for $reason the kernel refused to enable it".

I'm not sure if that final condition is actually possible with the system
ending up in a broken state, however - I'm not sure that we ever do turn
off access to the VPU at present (after we mark it usable), and if we do
it doesn't get reflected in has_vector() so the kernel and userspace would
both break, with what a crypto driver does probably being the least of
your worries.

> > So
> > there should be no issue.  If there is, the RISC-V core architecture code needs
> > to be fixed to not declare that extensions are supported when they are actually
> > incompatible non-standard versions of those extensions.  Incompatible
> > non-standard extensions should be represented as separate extensions.
> > 
> 
> It probably makes sense to check has_vector() to exclude Zve* for now, though.

I think you might actually be better served at present, given the code can
only be built if the core vector code is, by using
riscv_isa_extension_available(NULL, v). That way you know for sure that
you're getting the ratified extension and nothing else.

Prior to this conversation I thought that has_vector() should return true
if there's a standard compliant vector unit available - given all users
Andy added only need Zve32x.

> I am just concerned about how you're suggesting that non-standard extensions
> might be pretending to be standard ones and individual users of kernel-mode
> vector would need to work around that.

I am absolutely not suggesting that non-standard extensions should
masquerade as standard ones, I don't know where you got that from. What
I said was that a non-standard vector extension could reuse riscv_v_vlen
(and should IMO for simplicity reasons), not that any of the APIs we have
for checking extension availability would lie and say it was standard.
riscv_v_vlen having a value greater than 128 is not one of those APIs ;)

> I think that neither has_vector() nor
> 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> vector extension is non-standard.

riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
was present in DT or ACPI for all harts. It doesn't check whether or not
the required config option for vector has been set or anything related
to dependencies. has_vector() at least checks that the vector core has
been enabled (and uses the alternative-patched version of the check
given it is used in some hotter paths). That's kinda moot for code
that's only built if the vector core stuff is enabled as I said above
though.

We could of course make riscv_isa_extension_available() check
extension dependencies, but I'd rather leave dt validation to the dt
tooling (apparently ACPI tables are never wrong...). Either would allow
you to rely on the crypto extensions present only when the standard vector
extensions unless someone's DT/ACPI stuff is shite, but then they keep the
pieces IMO :)

Hope that makes sense?
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 6252 bytes --]

On Thu, Apr 18, 2024 at 10:39:46AM -0700, Eric Biggers wrote:
> On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > > If it would be useful to do so, we should be able to enable some of the code
> > > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > > SHA512 instructions require EEW==64.)
> > > > 
> > > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > > need to be revised and tested in that configuration.
> > > > 
> > > > > Eric/Jerry (although read the previous paragraph too):
> > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > > does not call either, which seems to violate the edict in
> > > > > kernel_vector_begin()'s kerneldoc:
> > > > > "Must not be called unless may_use_simd() returns true."
> > > > 
> > > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > > differs from shash algorithms which can be invoked in any context.
> > > > 
> > > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > > this was intentionally done in order to support use cases like this.  So that's
> > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > > kernel_vector_begin().
> > > 
> > > I see, thanks for explaining that. I think you should probably check
> > > somewhere if has_vector() returns true in that driver though before
> > > using vector instructions. Only checking vlen seems to me like relying on
> > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > > systems actually support Zvkb, but I hope you get my drift.
> > 
> > All the algorithms check for at least one of the vector crypto extensions being
> > supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> > should return whether the ratified version of Zvkb is supported, and likewise
> > for the other vector crypto extensions.  The ratified version of the vector
> > crypto extensions depends on the ratified version of the vector extension.

That's great if it does require that the version of the vector extension
must be standard. Higher quality spec than most if it does. But
"supported" in the context of riscv_isa_extension_available() means that
the hardware supports it (or set of harts), not that the currently
running kernel does. The Kconfig deps that must be met for the code to be
built at least mean the kernel is built with vector support, leaving only
"the kernel was built with vector support and the hardware supports vector
but for $reason the kernel refused to enable it".

I'm not sure if that final condition is actually possible with the system
ending up in a broken state, however - I'm not sure that we ever do turn
off access to the VPU at present (after we mark it usable), and if we do
it doesn't get reflected in has_vector() so the kernel and userspace would
both break, with what a crypto driver does probably being the least of
your worries.

> > So
> > there should be no issue.  If there is, the RISC-V core architecture code needs
> > to be fixed to not declare that extensions are supported when they are actually
> > incompatible non-standard versions of those extensions.  Incompatible
> > non-standard extensions should be represented as separate extensions.
> > 
> 
> It probably makes sense to check has_vector() to exclude Zve* for now, though.

I think you might actually be better served at present, given the code can
only be built if the core vector code is, by using
riscv_isa_extension_available(NULL, v). That way you know for sure that
you're getting the ratified extension and nothing else.

Prior to this conversation I thought that has_vector() should return true
if there's a standard compliant vector unit available - given all users
Andy added only need Zve32x.

> I am just concerned about how you're suggesting that non-standard extensions
> might be pretending to be standard ones and individual users of kernel-mode
> vector would need to work around that.

I am absolutely not suggesting that non-standard extensions should
masquerade as standard ones, I don't know where you got that from. What
I said was that a non-standard vector extension could reuse riscv_v_vlen
(and should IMO for simplicity reasons), not that any of the APIs we have
for checking extension availability would lie and say it was standard.
riscv_v_vlen having a value greater than 128 is not one of those APIs ;)

> I think that neither has_vector() nor
> 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> vector extension is non-standard.

riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
was present in DT or ACPI for all harts. It doesn't check whether or not
the required config option for vector has been set or anything related
to dependencies. has_vector() at least checks that the vector core has
been enabled (and uses the alternative-patched version of the check
given it is used in some hotter paths). That's kinda moot for code
that's only built if the vector core stuff is enabled as I said above
though.

We could of course make riscv_isa_extension_available() check
extension dependencies, but I'd rather leave dt validation to the dt
tooling (apparently ACPI tables are never wrong...). Either would allow
you to rely on the crypto extensions present only when the standard vector
extensions unless someone's DT/ACPI stuff is shite, but then they keep the
pieces IMO :)

Hope that makes sense?
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 6649 bytes --]

On Thu, Apr 18, 2024 at 07:26:00PM +0100, Conor Dooley wrote:
> On Thu, Apr 18, 2024 at 10:39:46AM -0700, Eric Biggers wrote:
> > On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> > > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > > > If it would be useful to do so, we should be able to enable some of the code
> > > > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > > > SHA512 instructions require EEW==64.)
> > > > > 
> > > > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > > > need to be revised and tested in that configuration.
> > > > > 
> > > > > > Eric/Jerry (although read the previous paragraph too):
> > > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > > > does not call either, which seems to violate the edict in
> > > > > > kernel_vector_begin()'s kerneldoc:
> > > > > > "Must not be called unless may_use_simd() returns true."
> > > > > 
> > > > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > > > differs from shash algorithms which can be invoked in any context.
> > > > > 
> > > > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > > > this was intentionally done in order to support use cases like this.  So that's
> > > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > > > kernel_vector_begin().
> > > > 
> > > > I see, thanks for explaining that. I think you should probably check
> > > > somewhere if has_vector() returns true in that driver though before
> > > > using vector instructions. Only checking vlen seems to me like relying on
> > > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > > > systems actually support Zvkb, but I hope you get my drift.
> > > 
> > > All the algorithms check for at least one of the vector crypto extensions being
> > > supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> > > should return whether the ratified version of Zvkb is supported, and likewise
> > > for the other vector crypto extensions.  The ratified version of the vector
> > > crypto extensions depends on the ratified version of the vector extension.
> 
> That's great if it does require that the version of the vector extension
> must be standard. Higher quality spec than most if it does. But
> "supported" in the context of riscv_isa_extension_available() means that
> the hardware supports it (or set of harts), not that the currently
> running kernel does. The Kconfig deps that must be met for the code to be
> built at least mean the kernel is built with vector support, leaving only
> "the kernel was built with vector support and the hardware supports vector
> but for $reason the kernel refused to enable it".
> 
> I'm not sure if that final condition is actually possible with the system
> ending up in a broken state, however - I'm not sure that we ever do turn
> off access to the VPU at present (after we mark it usable), and if we do
> it doesn't get reflected in has_vector() so the kernel and userspace would
> both break, with what a crypto driver does probably being the least of
> your worries.
> 
> > > So
> > > there should be no issue.  If there is, the RISC-V core architecture code needs
> > > to be fixed to not declare that extensions are supported when they are actually
> > > incompatible non-standard versions of those extensions.  Incompatible
> > > non-standard extensions should be represented as separate extensions.
> > > 
> > 
> > It probably makes sense to check has_vector() to exclude Zve* for now, though.
> 
> I think you might actually be better served at present, given the code can
> only be built if the core vector code is, by using
> riscv_isa_extension_available(NULL, v). That way you know for sure that
> you're getting the ratified extension and nothing else.

Poor choice of wording here - I meant, of course, the "main" vector
extension, rather than the Zve* variants.

> Prior to this conversation I thought that has_vector() should return true
> if there's a standard compliant vector unit available - given all users
> Andy added only need Zve32x.
> 
> > I am just concerned about how you're suggesting that non-standard extensions
> > might be pretending to be standard ones and individual users of kernel-mode
> > vector would need to work around that.
> 
> I am absolutely not suggesting that non-standard extensions should
> masquerade as standard ones, I don't know where you got that from. What
> I said was that a non-standard vector extension could reuse riscv_v_vlen
> (and should IMO for simplicity reasons), not that any of the APIs we have
> for checking extension availability would lie and say it was standard.
> riscv_v_vlen having a value greater than 128 is not one of those APIs ;)
> 
> > I think that neither has_vector() nor
> > 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> > vector extension is non-standard.
> 
> riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
> was present in DT or ACPI for all harts. It doesn't check whether or not
> the required config option for vector has been set or anything related
> to dependencies. has_vector() at least checks that the vector core has
> been enabled (and uses the alternative-patched version of the check
> given it is used in some hotter paths). That's kinda moot for code
> that's only built if the vector core stuff is enabled as I said above
> though.
> 
> We could of course make riscv_isa_extension_available() check
> extension dependencies, but I'd rather leave dt validation to the dt
> tooling (apparently ACPI tables are never wrong...). Either would allow
> you to rely on the crypto extensions present only when the standard vector
> extensions unless someone's DT/ACPI stuff is shite, but then they keep the
> pieces IMO :)
> 
> Hope that makes sense?
> Conor.



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 6649 bytes --]

On Thu, Apr 18, 2024 at 07:26:00PM +0100, Conor Dooley wrote:
> On Thu, Apr 18, 2024 at 10:39:46AM -0700, Eric Biggers wrote:
> > On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> > > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > > > If it would be useful to do so, we should be able to enable some of the code
> > > > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > > > SHA512 instructions require EEW==64.)
> > > > > 
> > > > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > > > need to be revised and tested in that configuration.
> > > > > 
> > > > > > Eric/Jerry (although read the previous paragraph too):
> > > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > > > does not call either, which seems to violate the edict in
> > > > > > kernel_vector_begin()'s kerneldoc:
> > > > > > "Must not be called unless may_use_simd() returns true."
> > > > > 
> > > > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > > > differs from shash algorithms which can be invoked in any context.
> > > > > 
> > > > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > > > this was intentionally done in order to support use cases like this.  So that's
> > > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > > > kernel_vector_begin().
> > > > 
> > > > I see, thanks for explaining that. I think you should probably check
> > > > somewhere if has_vector() returns true in that driver though before
> > > > using vector instructions. Only checking vlen seems to me like relying on
> > > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > > > systems actually support Zvkb, but I hope you get my drift.
> > > 
> > > All the algorithms check for at least one of the vector crypto extensions being
> > > supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> > > should return whether the ratified version of Zvkb is supported, and likewise
> > > for the other vector crypto extensions.  The ratified version of the vector
> > > crypto extensions depends on the ratified version of the vector extension.
> 
> That's great if it does require that the version of the vector extension
> must be standard. Higher quality spec than most if it does. But
> "supported" in the context of riscv_isa_extension_available() means that
> the hardware supports it (or set of harts), not that the currently
> running kernel does. The Kconfig deps that must be met for the code to be
> built at least mean the kernel is built with vector support, leaving only
> "the kernel was built with vector support and the hardware supports vector
> but for $reason the kernel refused to enable it".
> 
> I'm not sure if that final condition is actually possible with the system
> ending up in a broken state, however - I'm not sure that we ever do turn
> off access to the VPU at present (after we mark it usable), and if we do
> it doesn't get reflected in has_vector() so the kernel and userspace would
> both break, with what a crypto driver does probably being the least of
> your worries.
> 
> > > So
> > > there should be no issue.  If there is, the RISC-V core architecture code needs
> > > to be fixed to not declare that extensions are supported when they are actually
> > > incompatible non-standard versions of those extensions.  Incompatible
> > > non-standard extensions should be represented as separate extensions.
> > > 
> > 
> > It probably makes sense to check has_vector() to exclude Zve* for now, though.
> 
> I think you might actually be better served at present, given the code can
> only be built if the core vector code is, by using
> riscv_isa_extension_available(NULL, v). That way you know for sure that
> you're getting the ratified extension and nothing else.

Poor choice of wording here - I meant, of course, the "main" vector
extension, rather than the Zve* variants.

> Prior to this conversation I thought that has_vector() should return true
> if there's a standard compliant vector unit available - given all users
> Andy added only need Zve32x.
> 
> > I am just concerned about how you're suggesting that non-standard extensions
> > might be pretending to be standard ones and individual users of kernel-mode
> > vector would need to work around that.
> 
> I am absolutely not suggesting that non-standard extensions should
> masquerade as standard ones, I don't know where you got that from. What
> I said was that a non-standard vector extension could reuse riscv_v_vlen
> (and should IMO for simplicity reasons), not that any of the APIs we have
> for checking extension availability would lie and say it was standard.
> riscv_v_vlen having a value greater than 128 is not one of those APIs ;)
> 
> > I think that neither has_vector() nor
> > 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> > vector extension is non-standard.
> 
> riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
> was present in DT or ACPI for all harts. It doesn't check whether or not
> the required config option for vector has been set or anything related
> to dependencies. has_vector() at least checks that the vector core has
> been enabled (and uses the alternative-patched version of the check
> given it is used in some hotter paths). That's kinda moot for code
> that's only built if the vector core stuff is enabled as I said above
> though.
> 
> We could of course make riscv_isa_extension_available() check
> extension dependencies, but I'd rather leave dt validation to the dt
> tooling (apparently ACPI tables are never wrong...). Either would allow
> you to rely on the crypto extensions present only when the standard vector
> extensions unless someone's DT/ACPI stuff is shite, but then they keep the
> pieces IMO :)
> 
> Hope that makes sense?
> Conor.



[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

On Thu, Apr 18, 2024 at 07:26:00PM +0100, Conor Dooley wrote:
> That's great if it does require that the version of the vector extension
> must be standard. Higher quality spec than most if it does. But
> "supported" in the context of riscv_isa_extension_available() means that
> the hardware supports it (or set of harts), not that the currently
> running kernel does. The Kconfig deps that must be met for the code to be
> built at least mean the kernel is built with vector support, leaving only
> "the kernel was built with vector support and the hardware supports vector
> but for $reason the kernel refused to enable it".
> 
> I'm not sure if that final condition is actually possible with the system
> ending up in a broken state, however - I'm not sure that we ever do turn
> off access to the VPU at present (after we mark it usable), and if we do
> it doesn't get reflected in has_vector() so the kernel and userspace would
> both break, with what a crypto driver does probably being the least of
> your worries.
>
> > I am just concerned about how you're suggesting that non-standard extensions
> > might be pretending to be standard ones and individual users of kernel-mode
> > vector would need to work around that.
> 
> I am absolutely not suggesting that non-standard extensions should
> masquerade as standard ones, I don't know where you got that from. What
> I said was that a non-standard vector extension could reuse riscv_v_vlen
> (and should IMO for simplicity reasons), not that any of the APIs we have
> for checking extension availability would lie and say it was standard.
> riscv_v_vlen having a value greater than 128 is not one of those APIs ;)

It sounded like you were suggesting that a CPU could plausibly have a
pre-standard version of the vector extension but also have standard Zvkb.  I
don't think this makes sense, due to the dependency.

> > I think that neither has_vector() nor
> > 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> > vector extension is non-standard.
> 
> riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
> was present in DT or ACPI for all harts. It doesn't check whether or not
> the required config option for vector has been set or anything related
> to dependencies. has_vector() at least checks that the vector core has
> been enabled (and uses the alternative-patched version of the check
> given it is used in some hotter paths). That's kinda moot for code
> that's only built if the vector core stuff is enabled as I said above
> though.
> 
> We could of course make riscv_isa_extension_available() check
> extension dependencies, but I'd rather leave dt validation to the dt
> tooling (apparently ACPI tables are never wrong...). Either would allow
> you to rely on the crypto extensions present only when the standard vector
> extensions unless someone's DT/ACPI stuff is shite, but then they keep the
> pieces IMO :)
> 
> Hope that makes sense?

If the RISC-V kernel ever disables V, then it should also disable everything
that depends on V.

This would be similar to how on x86, if the kernel decides to disable AVX to
mitigate the Gather Data Sampling vulnerability, it also disables AVX2, AVX512,
VAES, VPCLMULQDQ, etc.  See cpuid_deps[] in arch/x86/kernel/cpu/cpuid-deps.c.

Sometimes CPU features depend on other ones.  That's just the way things work.
Whenever possible that should be handled centrally, not pushed down to every
user both in-kernel and userspace.

- Eric

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Eric Biggers]

On Thu, Apr 18, 2024 at 07:26:00PM +0100, Conor Dooley wrote:
> That's great if it does require that the version of the vector extension
> must be standard. Higher quality spec than most if it does. But
> "supported" in the context of riscv_isa_extension_available() means that
> the hardware supports it (or set of harts), not that the currently
> running kernel does. The Kconfig deps that must be met for the code to be
> built at least mean the kernel is built with vector support, leaving only
> "the kernel was built with vector support and the hardware supports vector
> but for $reason the kernel refused to enable it".
> 
> I'm not sure if that final condition is actually possible with the system
> ending up in a broken state, however - I'm not sure that we ever do turn
> off access to the VPU at present (after we mark it usable), and if we do
> it doesn't get reflected in has_vector() so the kernel and userspace would
> both break, with what a crypto driver does probably being the least of
> your worries.
>
> > I am just concerned about how you're suggesting that non-standard extensions
> > might be pretending to be standard ones and individual users of kernel-mode
> > vector would need to work around that.
> 
> I am absolutely not suggesting that non-standard extensions should
> masquerade as standard ones, I don't know where you got that from. What
> I said was that a non-standard vector extension could reuse riscv_v_vlen
> (and should IMO for simplicity reasons), not that any of the APIs we have
> for checking extension availability would lie and say it was standard.
> riscv_v_vlen having a value greater than 128 is not one of those APIs ;)

It sounded like you were suggesting that a CPU could plausibly have a
pre-standard version of the vector extension but also have standard Zvkb.  I
don't think this makes sense, due to the dependency.

> > I think that neither has_vector() nor
> > 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> > vector extension is non-standard.
> 
> riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
> was present in DT or ACPI for all harts. It doesn't check whether or not
> the required config option for vector has been set or anything related
> to dependencies. has_vector() at least checks that the vector core has
> been enabled (and uses the alternative-patched version of the check
> given it is used in some hotter paths). That's kinda moot for code
> that's only built if the vector core stuff is enabled as I said above
> though.
> 
> We could of course make riscv_isa_extension_available() check
> extension dependencies, but I'd rather leave dt validation to the dt
> tooling (apparently ACPI tables are never wrong...). Either would allow
> you to rely on the crypto extensions present only when the standard vector
> extensions unless someone's DT/ACPI stuff is shite, but then they keep the
> pieces IMO :)
> 
> Hope that makes sense?

If the RISC-V kernel ever disables V, then it should also disable everything
that depends on V.

This would be similar to how on x86, if the kernel decides to disable AVX to
mitigate the Gather Data Sampling vulnerability, it also disables AVX2, AVX512,
VAES, VPCLMULQDQ, etc.  See cpuid_deps[] in arch/x86/kernel/cpu/cpuid-deps.c.

Sometimes CPU features depend on other ones.  That's just the way things work.
Whenever possible that should be handled centrally, not pushed down to every
user both in-kernel and userspace.

- Eric

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 784 bytes --]

On Thu, Apr 18, 2024 at 11:41:29AM -0700, Eric Biggers wrote:

> If the RISC-V kernel ever disables V, then it should also disable everything
> that depends on V.
> 
> This would be similar to how on x86, if the kernel decides to disable AVX to
> mitigate the Gather Data Sampling vulnerability, it also disables AVX2, AVX512,
> VAES, VPCLMULQDQ, etc.  See cpuid_deps[] in arch/x86/kernel/cpu/cpuid-deps.c.
> 
> Sometimes CPU features depend on other ones.  That's just the way things work.

🤨

> Whenever possible that should be handled centrally, not pushed down to every
> user both in-kernel and userspace.

FWIW, anything detected but rejected while probing the DT or ACPI tables
will cause riscv_isa_extension_available() (or other APIs) to return false.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 784 bytes --]

On Thu, Apr 18, 2024 at 11:41:29AM -0700, Eric Biggers wrote:

> If the RISC-V kernel ever disables V, then it should also disable everything
> that depends on V.
> 
> This would be similar to how on x86, if the kernel decides to disable AVX to
> mitigate the Gather Data Sampling vulnerability, it also disables AVX2, AVX512,
> VAES, VPCLMULQDQ, etc.  See cpuid_deps[] in arch/x86/kernel/cpu/cpuid-deps.c.
> 
> Sometimes CPU features depend on other ones.  That's just the way things work.

🤨

> Whenever possible that should be handled centrally, not pushed down to every
> user both in-kernel and userspace.

FWIW, anything detected but rejected while probing the DT or ACPI tables
will cause riscv_isa_extension_available() (or other APIs) to return false.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [External] [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[yunhui cui]

Hi Andy,

On Fri, Apr 12, 2024 at 2:50 PM Andy Chiu <andy.chiu@sifive.com> wrote:
>
> Currently we only support Vector for SMP platforms, that is, all SMP
> cores have the same vlenb. If we happen to detect a mismatching vlen, it
> is better to just fail bootting it up to prevent further race/scheduling
> issues.
>
> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> regular call in the early assembly. So a core would be parked right
> after a return from smp_callin. Note that a successful smp_callin
> does not return.
>
> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> Reported-by: Conor Dooley <conor.dooley@microchip.com>
> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v4:
>  - update comment also in the assembly code (Yunhui)
> Changelog v2:
>  - update commit message to explain asm code change (Conor)
> ---
>  arch/riscv/kernel/head.S    | 19 ++++++++++++-------
>  arch/riscv/kernel/smpboot.c | 14 +++++++++-----
>  2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 4236a69c35cb..a00f7523cb91 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -165,9 +165,20 @@ secondary_start_sbi:
>  #endif
>         call .Lsetup_trap_vector
>         scs_load_current
> -       tail smp_callin
> +       call smp_callin
>  #endif /* CONFIG_SMP */
>
> +.align 2
> +.Lsecondary_park:
> +       /*
> +        * Park this hart if we:
> +        *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
> +        *  - receive an early trap, before setup_trap_vector finished
> +        *  - fail in smp_callin(), as a successful one wouldn't return
> +        */
> +       wfi
> +       j .Lsecondary_park
> +
>  .align 2
>  .Lsetup_trap_vector:
>         /* Set trap vector to exception handler */
> @@ -181,12 +192,6 @@ secondary_start_sbi:
>         csrw CSR_SCRATCH, zero
>         ret
>
> -.align 2
> -.Lsecondary_park:
> -       /* We lack SMP support or have too many harts, so park this hart */
> -       wfi
> -       j .Lsecondary_park
> -
>  SYM_CODE_END(_start)
>
>  SYM_CODE_START(_start_kernel)
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index d41090fc3203..673437ccc13d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
>         struct mm_struct *mm = &init_mm;
>         unsigned int curr_cpuid = smp_processor_id();
>
> +       if (has_vector()) {
> +               /*
> +                * Return as early as possible so the hart with a mismatching
> +                * vlen won't boot.
> +                */
> +               if (riscv_v_setup_vsize())
> +                       return;
> +       }
> +
>         /* All kernel threads share the same mm context.  */
>         mmgrab(mm);
>         current->active_mm = mm;
> @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
>         numa_add_cpu(curr_cpuid);
>         set_cpu_online(curr_cpuid, 1);
>
> -       if (has_vector()) {
> -               if (riscv_v_setup_vsize())
> -                       elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> -       }
> -
>         riscv_user_isa_enable();
>
>         /*
>
> --
> 2.44.0.rc2
>
>

Reviewed-by: Yunhui Cui <cuiyunhui@bytedance.com>


Thanks,
Yunhui

Re: [External] [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[yunhui cui]

Hi Andy,

On Fri, Apr 12, 2024 at 2:50 PM Andy Chiu <andy.chiu@sifive.com> wrote:
>
> Currently we only support Vector for SMP platforms, that is, all SMP
> cores have the same vlenb. If we happen to detect a mismatching vlen, it
> is better to just fail bootting it up to prevent further race/scheduling
> issues.
>
> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> regular call in the early assembly. So a core would be parked right
> after a return from smp_callin. Note that a successful smp_callin
> does not return.
>
> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> Reported-by: Conor Dooley <conor.dooley@microchip.com>
> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v4:
>  - update comment also in the assembly code (Yunhui)
> Changelog v2:
>  - update commit message to explain asm code change (Conor)
> ---
>  arch/riscv/kernel/head.S    | 19 ++++++++++++-------
>  arch/riscv/kernel/smpboot.c | 14 +++++++++-----
>  2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 4236a69c35cb..a00f7523cb91 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -165,9 +165,20 @@ secondary_start_sbi:
>  #endif
>         call .Lsetup_trap_vector
>         scs_load_current
> -       tail smp_callin
> +       call smp_callin
>  #endif /* CONFIG_SMP */
>
> +.align 2
> +.Lsecondary_park:
> +       /*
> +        * Park this hart if we:
> +        *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
> +        *  - receive an early trap, before setup_trap_vector finished
> +        *  - fail in smp_callin(), as a successful one wouldn't return
> +        */
> +       wfi
> +       j .Lsecondary_park
> +
>  .align 2
>  .Lsetup_trap_vector:
>         /* Set trap vector to exception handler */
> @@ -181,12 +192,6 @@ secondary_start_sbi:
>         csrw CSR_SCRATCH, zero
>         ret
>
> -.align 2
> -.Lsecondary_park:
> -       /* We lack SMP support or have too many harts, so park this hart */
> -       wfi
> -       j .Lsecondary_park
> -
>  SYM_CODE_END(_start)
>
>  SYM_CODE_START(_start_kernel)
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index d41090fc3203..673437ccc13d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
>         struct mm_struct *mm = &init_mm;
>         unsigned int curr_cpuid = smp_processor_id();
>
> +       if (has_vector()) {
> +               /*
> +                * Return as early as possible so the hart with a mismatching
> +                * vlen won't boot.
> +                */
> +               if (riscv_v_setup_vsize())
> +                       return;
> +       }
> +
>         /* All kernel threads share the same mm context.  */
>         mmgrab(mm);
>         current->active_mm = mm;
> @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
>         numa_add_cpu(curr_cpuid);
>         set_cpu_online(curr_cpuid, 1);
>
> -       if (has_vector()) {
> -               if (riscv_v_setup_vsize())
> -                       elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> -       }
> -
>         riscv_user_isa_enable();
>
>         /*
>
> --
> 2.44.0.rc2
>
>

Reviewed-by: Yunhui Cui <cuiyunhui@bytedance.com>


Thanks,
Yunhui

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

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Alexandre Ghiti]

Hi Andy,

On 12/04/2024 08:48, Andy Chiu wrote:
> Currently we only support Vector for SMP platforms, that is, all SMP
> cores have the same vlenb. If we happen to detect a mismatching vlen, it
> is better to just fail bootting it up to prevent further race/scheduling
> issues.
>
> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> regular call in the early assembly. So a core would be parked right
> after a return from smp_callin. Note that a successful smp_callin
> does not return.
>
> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> Reported-by: Conor Dooley <conor.dooley@microchip.com>
> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v4:
>   - update comment also in the assembly code (Yunhui)
> Changelog v2:
>   - update commit message to explain asm code change (Conor)
> ---
>   arch/riscv/kernel/head.S    | 19 ++++++++++++-------
>   arch/riscv/kernel/smpboot.c | 14 +++++++++-----
>   2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 4236a69c35cb..a00f7523cb91 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -165,9 +165,20 @@ secondary_start_sbi:
>   #endif
>   	call .Lsetup_trap_vector
>   	scs_load_current
> -	tail smp_callin
> +	call smp_callin
>   #endif /* CONFIG_SMP */
>   
> +.align 2
> +.Lsecondary_park:
> +	/*
> +	 * Park this hart if we:
> +	 *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
> +	 *  - receive an early trap, before setup_trap_vector finished
> +	 *  - fail in smp_callin(), as a successful one wouldn't return
> +	 */
> +	wfi
> +	j .Lsecondary_park
> +
>   .align 2
>   .Lsetup_trap_vector:
>   	/* Set trap vector to exception handler */
> @@ -181,12 +192,6 @@ secondary_start_sbi:
>   	csrw CSR_SCRATCH, zero
>   	ret
>   
> -.align 2
> -.Lsecondary_park:
> -	/* We lack SMP support or have too many harts, so park this hart */
> -	wfi
> -	j .Lsecondary_park
> -
>   SYM_CODE_END(_start)
>   
>   SYM_CODE_START(_start_kernel)
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index d41090fc3203..673437ccc13d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
>   	struct mm_struct *mm = &init_mm;
>   	unsigned int curr_cpuid = smp_processor_id();
>   
> +	if (has_vector()) {
> +		/*
> +		 * Return as early as possible so the hart with a mismatching
> +		 * vlen won't boot.
> +		 */
> +		if (riscv_v_setup_vsize())
> +			return;
> +	}
> +
>   	/* All kernel threads share the same mm context.  */
>   	mmgrab(mm);
>   	current->active_mm = mm;
> @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
>   	numa_add_cpu(curr_cpuid);
>   	set_cpu_online(curr_cpuid, 1);
>   
> -	if (has_vector()) {
> -		if (riscv_v_setup_vsize())
> -			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> -	}
> -
>   	riscv_user_isa_enable();
>   
>   	/*
>

So this should go into -fixes, would you mind sending a single patch for 
this fix?

Your patch 8 is actually already fixed by Clement's patch 
https://lore.kernel.org/linux-riscv/20240409143839.558784-1-cleger@rivosinc.com/ 
and I already mentioned this one to Palmer.

Thanks,

Alex

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Alexandre Ghiti]

Hi Andy,

On 12/04/2024 08:48, Andy Chiu wrote:
> Currently we only support Vector for SMP platforms, that is, all SMP
> cores have the same vlenb. If we happen to detect a mismatching vlen, it
> is better to just fail bootting it up to prevent further race/scheduling
> issues.
>
> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> regular call in the early assembly. So a core would be parked right
> after a return from smp_callin. Note that a successful smp_callin
> does not return.
>
> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> Reported-by: Conor Dooley <conor.dooley@microchip.com>
> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> ---
> Changelog v4:
>   - update comment also in the assembly code (Yunhui)
> Changelog v2:
>   - update commit message to explain asm code change (Conor)
> ---
>   arch/riscv/kernel/head.S    | 19 ++++++++++++-------
>   arch/riscv/kernel/smpboot.c | 14 +++++++++-----
>   2 files changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 4236a69c35cb..a00f7523cb91 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -165,9 +165,20 @@ secondary_start_sbi:
>   #endif
>   	call .Lsetup_trap_vector
>   	scs_load_current
> -	tail smp_callin
> +	call smp_callin
>   #endif /* CONFIG_SMP */
>   
> +.align 2
> +.Lsecondary_park:
> +	/*
> +	 * Park this hart if we:
> +	 *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
> +	 *  - receive an early trap, before setup_trap_vector finished
> +	 *  - fail in smp_callin(), as a successful one wouldn't return
> +	 */
> +	wfi
> +	j .Lsecondary_park
> +
>   .align 2
>   .Lsetup_trap_vector:
>   	/* Set trap vector to exception handler */
> @@ -181,12 +192,6 @@ secondary_start_sbi:
>   	csrw CSR_SCRATCH, zero
>   	ret
>   
> -.align 2
> -.Lsecondary_park:
> -	/* We lack SMP support or have too many harts, so park this hart */
> -	wfi
> -	j .Lsecondary_park
> -
>   SYM_CODE_END(_start)
>   
>   SYM_CODE_START(_start_kernel)
> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> index d41090fc3203..673437ccc13d 100644
> --- a/arch/riscv/kernel/smpboot.c
> +++ b/arch/riscv/kernel/smpboot.c
> @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
>   	struct mm_struct *mm = &init_mm;
>   	unsigned int curr_cpuid = smp_processor_id();
>   
> +	if (has_vector()) {
> +		/*
> +		 * Return as early as possible so the hart with a mismatching
> +		 * vlen won't boot.
> +		 */
> +		if (riscv_v_setup_vsize())
> +			return;
> +	}
> +
>   	/* All kernel threads share the same mm context.  */
>   	mmgrab(mm);
>   	current->active_mm = mm;
> @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
>   	numa_add_cpu(curr_cpuid);
>   	set_cpu_online(curr_cpuid, 1);
>   
> -	if (has_vector()) {
> -		if (riscv_v_setup_vsize())
> -			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> -	}
> -
>   	riscv_user_isa_enable();
>   
>   	/*
>

So this should go into -fixes, would you mind sending a single patch for 
this fix?

Your patch 8 is actually already fixed by Clement's patch 
https://lore.kernel.org/linux-riscv/20240409143839.558784-1-cleger@rivosinc.com/ 
and I already mentioned this one to Palmer.

Thanks,

Alex


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

Re: [PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Fri, 12 Apr 2024 14:48:56 +0800 you wrote:
> The series composes of two parts. The first part provides a quick fix for
> the issue on a recent thread[1]. The issue happens when a platform has
> ununified vector register length across multiple cores. Specifically,
> patch 1 adds a comment at a callsite of riscv_setup_vsize to clarify how
> vlenb is observed by the system. Patch 2 fixes the issue by failing the
> boot process of a secondary core if vlenb mismatches.
> 
> [...]

Here is the summary with links:
  - [v4,1/9] riscv: vector: add a comment when calling riscv_setup_vsize()
    (no matching commit)
  - [v4,2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected
    (no matching commit)
  - [v4,3/9] riscv: cpufeature: call match_isa_ext() for single-letter extensions
    (no matching commit)
  - [v4,4/9] riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection
    (no matching commit)
  - [v4,5/9] dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description
    (no matching commit)
  - [v4,6/9] riscv: hwprobe: add zve Vector subextensions into hwprobe interface
    (no matching commit)
  - [v4,7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X
    (no matching commit)
  - [v4,8/9] hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro
    https://git.kernel.org/riscv/c/5ea6764d9095
  - [v4,9/9] selftest: run vector prctl test for ZVE32X
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v4 0/9] Support Zve32[xf] and Zve64[xfd] Vector subextensions

[patchwork-bot+linux-riscv]

Hello:

This series was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Fri, 12 Apr 2024 14:48:56 +0800 you wrote:
> The series composes of two parts. The first part provides a quick fix for
> the issue on a recent thread[1]. The issue happens when a platform has
> ununified vector register length across multiple cores. Specifically,
> patch 1 adds a comment at a callsite of riscv_setup_vsize to clarify how
> vlenb is observed by the system. Patch 2 fixes the issue by failing the
> boot process of a secondary core if vlenb mismatches.
> 
> [...]

Here is the summary with links:
  - [v4,1/9] riscv: vector: add a comment when calling riscv_setup_vsize()
    (no matching commit)
  - [v4,2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected
    (no matching commit)
  - [v4,3/9] riscv: cpufeature: call match_isa_ext() for single-letter extensions
    (no matching commit)
  - [v4,4/9] riscv: cpufeature: add zve32[xf] and zve64[xfd] isa detection
    (no matching commit)
  - [v4,5/9] dt-bindings: riscv: add Zve32[xf] Zve64[xfd] ISA extension description
    (no matching commit)
  - [v4,6/9] riscv: hwprobe: add zve Vector subextensions into hwprobe interface
    (no matching commit)
  - [v4,7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X
    (no matching commit)
  - [v4,8/9] hwprobe: fix integer promotion in RISCV_HWPROBE_EXT macro
    https://git.kernel.org/riscv/c/5ea6764d9095
  - [v4,9/9] selftest: run vector prctl test for ZVE32X
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Andy Chiu]

On Thu, Apr 25, 2024 at 4:01 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Andy,
>
> On 12/04/2024 08:48, Andy Chiu wrote:
> > Currently we only support Vector for SMP platforms, that is, all SMP
> > cores have the same vlenb. If we happen to detect a mismatching vlen, it
> > is better to just fail bootting it up to prevent further race/scheduling
> > issues.
> >
> > Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> > regular call in the early assembly. So a core would be parked right
> > after a return from smp_callin. Note that a successful smp_callin
> > does not return.
> >
> > Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> > Reported-by: Conor Dooley <conor.dooley@microchip.com>
> > Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
> > Changelog v4:
> >   - update comment also in the assembly code (Yunhui)
> > Changelog v2:
> >   - update commit message to explain asm code change (Conor)
> > ---
> >   arch/riscv/kernel/head.S    | 19 ++++++++++++-------
> >   arch/riscv/kernel/smpboot.c | 14 +++++++++-----
> >   2 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index 4236a69c35cb..a00f7523cb91 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -165,9 +165,20 @@ secondary_start_sbi:
> >   #endif
> >       call .Lsetup_trap_vector
> >       scs_load_current
> > -     tail smp_callin
> > +     call smp_callin
> >   #endif /* CONFIG_SMP */
> >
> > +.align 2
> > +.Lsecondary_park:
> > +     /*
> > +      * Park this hart if we:
> > +      *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
> > +      *  - receive an early trap, before setup_trap_vector finished
> > +      *  - fail in smp_callin(), as a successful one wouldn't return
> > +      */
> > +     wfi
> > +     j .Lsecondary_park
> > +
> >   .align 2
> >   .Lsetup_trap_vector:
> >       /* Set trap vector to exception handler */
> > @@ -181,12 +192,6 @@ secondary_start_sbi:
> >       csrw CSR_SCRATCH, zero
> >       ret
> >
> > -.align 2
> > -.Lsecondary_park:
> > -     /* We lack SMP support or have too many harts, so park this hart */
> > -     wfi
> > -     j .Lsecondary_park
> > -
> >   SYM_CODE_END(_start)
> >
> >   SYM_CODE_START(_start_kernel)
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index d41090fc3203..673437ccc13d 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
> >       struct mm_struct *mm = &init_mm;
> >       unsigned int curr_cpuid = smp_processor_id();
> >
> > +     if (has_vector()) {
> > +             /*
> > +              * Return as early as possible so the hart with a mismatching
> > +              * vlen won't boot.
> > +              */
> > +             if (riscv_v_setup_vsize())
> > +                     return;
> > +     }
> > +
> >       /* All kernel threads share the same mm context.  */
> >       mmgrab(mm);
> >       current->active_mm = mm;
> > @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
> >       numa_add_cpu(curr_cpuid);
> >       set_cpu_online(curr_cpuid, 1);
> >
> > -     if (has_vector()) {
> > -             if (riscv_v_setup_vsize())
> > -                     elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> > -     }
> > -
> >       riscv_user_isa_enable();
> >
> >       /*
> >
>
> So this should go into -fixes, would you mind sending a single patch for
> this fix?

I thought it would be magically picked up by a bot as long as we have
a fix tag. Am I assuming something wrong?

>
> Your patch 8 is actually already fixed by Clement's patch
> https://lore.kernel.org/linux-riscv/20240409143839.558784-1-cleger@rivosinc.com/

Okay, I will drop it at the next revision.

> and I already mentioned this one to Palmer.
>
> Thanks,
>
> Alex
>

Thanks,
Andy

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Andy Chiu]

On Thu, Apr 25, 2024 at 4:01 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>
> Hi Andy,
>
> On 12/04/2024 08:48, Andy Chiu wrote:
> > Currently we only support Vector for SMP platforms, that is, all SMP
> > cores have the same vlenb. If we happen to detect a mismatching vlen, it
> > is better to just fail bootting it up to prevent further race/scheduling
> > issues.
> >
> > Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
> > regular call in the early assembly. So a core would be parked right
> > after a return from smp_callin. Note that a successful smp_callin
> > does not return.
> >
> > Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
> > Reported-by: Conor Dooley <conor.dooley@microchip.com>
> > Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
> > Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
> > ---
> > Changelog v4:
> >   - update comment also in the assembly code (Yunhui)
> > Changelog v2:
> >   - update commit message to explain asm code change (Conor)
> > ---
> >   arch/riscv/kernel/head.S    | 19 ++++++++++++-------
> >   arch/riscv/kernel/smpboot.c | 14 +++++++++-----
> >   2 files changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> > index 4236a69c35cb..a00f7523cb91 100644
> > --- a/arch/riscv/kernel/head.S
> > +++ b/arch/riscv/kernel/head.S
> > @@ -165,9 +165,20 @@ secondary_start_sbi:
> >   #endif
> >       call .Lsetup_trap_vector
> >       scs_load_current
> > -     tail smp_callin
> > +     call smp_callin
> >   #endif /* CONFIG_SMP */
> >
> > +.align 2
> > +.Lsecondary_park:
> > +     /*
> > +      * Park this hart if we:
> > +      *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
> > +      *  - receive an early trap, before setup_trap_vector finished
> > +      *  - fail in smp_callin(), as a successful one wouldn't return
> > +      */
> > +     wfi
> > +     j .Lsecondary_park
> > +
> >   .align 2
> >   .Lsetup_trap_vector:
> >       /* Set trap vector to exception handler */
> > @@ -181,12 +192,6 @@ secondary_start_sbi:
> >       csrw CSR_SCRATCH, zero
> >       ret
> >
> > -.align 2
> > -.Lsecondary_park:
> > -     /* We lack SMP support or have too many harts, so park this hart */
> > -     wfi
> > -     j .Lsecondary_park
> > -
> >   SYM_CODE_END(_start)
> >
> >   SYM_CODE_START(_start_kernel)
> > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
> > index d41090fc3203..673437ccc13d 100644
> > --- a/arch/riscv/kernel/smpboot.c
> > +++ b/arch/riscv/kernel/smpboot.c
> > @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
> >       struct mm_struct *mm = &init_mm;
> >       unsigned int curr_cpuid = smp_processor_id();
> >
> > +     if (has_vector()) {
> > +             /*
> > +              * Return as early as possible so the hart with a mismatching
> > +              * vlen won't boot.
> > +              */
> > +             if (riscv_v_setup_vsize())
> > +                     return;
> > +     }
> > +
> >       /* All kernel threads share the same mm context.  */
> >       mmgrab(mm);
> >       current->active_mm = mm;
> > @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
> >       numa_add_cpu(curr_cpuid);
> >       set_cpu_online(curr_cpuid, 1);
> >
> > -     if (has_vector()) {
> > -             if (riscv_v_setup_vsize())
> > -                     elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
> > -     }
> > -
> >       riscv_user_isa_enable();
> >
> >       /*
> >
>
> So this should go into -fixes, would you mind sending a single patch for
> this fix?

I thought it would be magically picked up by a bot as long as we have
a fix tag. Am I assuming something wrong?

>
> Your patch 8 is actually already fixed by Clement's patch
> https://lore.kernel.org/linux-riscv/20240409143839.558784-1-cleger@rivosinc.com/

Okay, I will drop it at the next revision.

> and I already mentioned this one to Palmer.
>
> Thanks,
>
> Alex
>

Thanks,
Andy

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

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Alexandre Ghiti]

Hi Andy,

On 08/05/2024 10:21, Andy Chiu wrote:
> On Thu, Apr 25, 2024 at 4:01 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>> Hi Andy,
>>
>> On 12/04/2024 08:48, Andy Chiu wrote:
>>> Currently we only support Vector for SMP platforms, that is, all SMP
>>> cores have the same vlenb. If we happen to detect a mismatching vlen, it
>>> is better to just fail bootting it up to prevent further race/scheduling
>>> issues.
>>>
>>> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
>>> regular call in the early assembly. So a core would be parked right
>>> after a return from smp_callin. Note that a successful smp_callin
>>> does not return.
>>>
>>> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
>>> Reported-by: Conor Dooley <conor.dooley@microchip.com>
>>> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
>>> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
>>> ---
>>> Changelog v4:
>>>    - update comment also in the assembly code (Yunhui)
>>> Changelog v2:
>>>    - update commit message to explain asm code change (Conor)
>>> ---
>>>    arch/riscv/kernel/head.S    | 19 ++++++++++++-------
>>>    arch/riscv/kernel/smpboot.c | 14 +++++++++-----
>>>    2 files changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>>> index 4236a69c35cb..a00f7523cb91 100644
>>> --- a/arch/riscv/kernel/head.S
>>> +++ b/arch/riscv/kernel/head.S
>>> @@ -165,9 +165,20 @@ secondary_start_sbi:
>>>    #endif
>>>        call .Lsetup_trap_vector
>>>        scs_load_current
>>> -     tail smp_callin
>>> +     call smp_callin
>>>    #endif /* CONFIG_SMP */
>>>
>>> +.align 2
>>> +.Lsecondary_park:
>>> +     /*
>>> +      * Park this hart if we:
>>> +      *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
>>> +      *  - receive an early trap, before setup_trap_vector finished
>>> +      *  - fail in smp_callin(), as a successful one wouldn't return
>>> +      */
>>> +     wfi
>>> +     j .Lsecondary_park
>>> +
>>>    .align 2
>>>    .Lsetup_trap_vector:
>>>        /* Set trap vector to exception handler */
>>> @@ -181,12 +192,6 @@ secondary_start_sbi:
>>>        csrw CSR_SCRATCH, zero
>>>        ret
>>>
>>> -.align 2
>>> -.Lsecondary_park:
>>> -     /* We lack SMP support or have too many harts, so park this hart */
>>> -     wfi
>>> -     j .Lsecondary_park
>>> -
>>>    SYM_CODE_END(_start)
>>>
>>>    SYM_CODE_START(_start_kernel)
>>> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
>>> index d41090fc3203..673437ccc13d 100644
>>> --- a/arch/riscv/kernel/smpboot.c
>>> +++ b/arch/riscv/kernel/smpboot.c
>>> @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
>>>        struct mm_struct *mm = &init_mm;
>>>        unsigned int curr_cpuid = smp_processor_id();
>>>
>>> +     if (has_vector()) {
>>> +             /*
>>> +              * Return as early as possible so the hart with a mismatching
>>> +              * vlen won't boot.
>>> +              */
>>> +             if (riscv_v_setup_vsize())
>>> +                     return;
>>> +     }
>>> +
>>>        /* All kernel threads share the same mm context.  */
>>>        mmgrab(mm);
>>>        current->active_mm = mm;
>>> @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
>>>        numa_add_cpu(curr_cpuid);
>>>        set_cpu_online(curr_cpuid, 1);
>>>
>>> -     if (has_vector()) {
>>> -             if (riscv_v_setup_vsize())
>>> -                     elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
>>> -     }
>>> -
>>>        riscv_user_isa_enable();
>>>
>>>        /*
>>>
>> So this should go into -fixes, would you mind sending a single patch for
>> this fix?
> I thought it would be magically picked up by a bot as long as we have
> a fix tag. Am I assuming something wrong?


It gets backported to stable when it is merged, but then it is missing 
from the first stable releases (as long as it is not merged).

But anyway, 6.9 fixes are all out, so let's hope this series makes it to 
6.10.

Thanks,

Alex


>
>> Your patch 8 is actually already fixed by Clement's patch
>> https://lore.kernel.org/linux-riscv/20240409143839.558784-1-cleger@rivosinc.com/
> Okay, I will drop it at the next revision.
>
>> and I already mentioned this one to Palmer.
>>
>> Thanks,
>>
>> Alex
>>
> Thanks,
> Andy
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v4 2/9] riscv: smp: fail booting up smp if inconsistent vlen is detected

[Alexandre Ghiti]

Hi Andy,

On 08/05/2024 10:21, Andy Chiu wrote:
> On Thu, Apr 25, 2024 at 4:01 AM Alexandre Ghiti <alex@ghiti.fr> wrote:
>> Hi Andy,
>>
>> On 12/04/2024 08:48, Andy Chiu wrote:
>>> Currently we only support Vector for SMP platforms, that is, all SMP
>>> cores have the same vlenb. If we happen to detect a mismatching vlen, it
>>> is better to just fail bootting it up to prevent further race/scheduling
>>> issues.
>>>
>>> Also, move .Lsecondary_park forward and chage `tail smp_callin` into a
>>> regular call in the early assembly. So a core would be parked right
>>> after a return from smp_callin. Note that a successful smp_callin
>>> does not return.
>>>
>>> Fixes: 7017858eb2d7 ("riscv: Introduce riscv_v_vsize to record size of Vector context")
>>> Reported-by: Conor Dooley <conor.dooley@microchip.com>
>>> Closes: https://lore.kernel.org/linux-riscv/20240228-vicinity-cornstalk-4b8eb5fe5730@spud/
>>> Signed-off-by: Andy Chiu <andy.chiu@sifive.com>
>>> ---
>>> Changelog v4:
>>>    - update comment also in the assembly code (Yunhui)
>>> Changelog v2:
>>>    - update commit message to explain asm code change (Conor)
>>> ---
>>>    arch/riscv/kernel/head.S    | 19 ++++++++++++-------
>>>    arch/riscv/kernel/smpboot.c | 14 +++++++++-----
>>>    2 files changed, 21 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>>> index 4236a69c35cb..a00f7523cb91 100644
>>> --- a/arch/riscv/kernel/head.S
>>> +++ b/arch/riscv/kernel/head.S
>>> @@ -165,9 +165,20 @@ secondary_start_sbi:
>>>    #endif
>>>        call .Lsetup_trap_vector
>>>        scs_load_current
>>> -     tail smp_callin
>>> +     call smp_callin
>>>    #endif /* CONFIG_SMP */
>>>
>>> +.align 2
>>> +.Lsecondary_park:
>>> +     /*
>>> +      * Park this hart if we:
>>> +      *  - have too many harts on CONFIG_RISCV_BOOT_SPINWAIT
>>> +      *  - receive an early trap, before setup_trap_vector finished
>>> +      *  - fail in smp_callin(), as a successful one wouldn't return
>>> +      */
>>> +     wfi
>>> +     j .Lsecondary_park
>>> +
>>>    .align 2
>>>    .Lsetup_trap_vector:
>>>        /* Set trap vector to exception handler */
>>> @@ -181,12 +192,6 @@ secondary_start_sbi:
>>>        csrw CSR_SCRATCH, zero
>>>        ret
>>>
>>> -.align 2
>>> -.Lsecondary_park:
>>> -     /* We lack SMP support or have too many harts, so park this hart */
>>> -     wfi
>>> -     j .Lsecondary_park
>>> -
>>>    SYM_CODE_END(_start)
>>>
>>>    SYM_CODE_START(_start_kernel)
>>> diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
>>> index d41090fc3203..673437ccc13d 100644
>>> --- a/arch/riscv/kernel/smpboot.c
>>> +++ b/arch/riscv/kernel/smpboot.c
>>> @@ -214,6 +214,15 @@ asmlinkage __visible void smp_callin(void)
>>>        struct mm_struct *mm = &init_mm;
>>>        unsigned int curr_cpuid = smp_processor_id();
>>>
>>> +     if (has_vector()) {
>>> +             /*
>>> +              * Return as early as possible so the hart with a mismatching
>>> +              * vlen won't boot.
>>> +              */
>>> +             if (riscv_v_setup_vsize())
>>> +                     return;
>>> +     }
>>> +
>>>        /* All kernel threads share the same mm context.  */
>>>        mmgrab(mm);
>>>        current->active_mm = mm;
>>> @@ -226,11 +235,6 @@ asmlinkage __visible void smp_callin(void)
>>>        numa_add_cpu(curr_cpuid);
>>>        set_cpu_online(curr_cpuid, 1);
>>>
>>> -     if (has_vector()) {
>>> -             if (riscv_v_setup_vsize())
>>> -                     elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
>>> -     }
>>> -
>>>        riscv_user_isa_enable();
>>>
>>>        /*
>>>
>> So this should go into -fixes, would you mind sending a single patch for
>> this fix?
> I thought it would be magically picked up by a bot as long as we have
> a fix tag. Am I assuming something wrong?


It gets backported to stable when it is merged, but then it is missing 
from the first stable releases (as long as it is not merged).

But anyway, 6.9 fixes are all out, so let's hope this series makes it to 
6.10.

Thanks,

Alex


>
>> Your patch 8 is actually already fixed by Clement's patch
>> https://lore.kernel.org/linux-riscv/20240409143839.558784-1-cleger@rivosinc.com/
> Okay, I will drop it at the next revision.
>
>> and I already mentioned this one to Palmer.
>>
>> Thanks,
>>
>> Alex
>>
> Thanks,
> Andy
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Andy Chiu]

Hi Conor,

On Fri, Apr 19, 2024 at 2:26 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Apr 18, 2024 at 10:39:46AM -0700, Eric Biggers wrote:
> > On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> > > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > > > If it would be useful to do so, we should be able to enable some of the code
> > > > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > > > SHA512 instructions require EEW==64.)
> > > > >
> > > > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > > > need to be revised and tested in that configuration.
> > > > >
> > > > > > Eric/Jerry (although read the previous paragraph too):
> > > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > > > does not call either, which seems to violate the edict in
> > > > > > kernel_vector_begin()'s kerneldoc:
> > > > > > "Must not be called unless may_use_simd() returns true."
> > > > >
> > > > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > > > differs from shash algorithms which can be invoked in any context.
> > > > >
> > > > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > > > this was intentionally done in order to support use cases like this.  So that's
> > > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > > > kernel_vector_begin().
> > > >
> > > > I see, thanks for explaining that. I think you should probably check
> > > > somewhere if has_vector() returns true in that driver though before
> > > > using vector instructions. Only checking vlen seems to me like relying on
> > > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > > > systems actually support Zvkb, but I hope you get my drift.
> > >
> > > All the algorithms check for at least one of the vector crypto extensions being
> > > supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> > > should return whether the ratified version of Zvkb is supported, and likewise
> > > for the other vector crypto extensions.  The ratified version of the vector
> > > crypto extensions depends on the ratified version of the vector extension.
>
> That's great if it does require that the version of the vector extension
> must be standard. Higher quality spec than most if it does. But
> "supported" in the context of riscv_isa_extension_available() means that
> the hardware supports it (or set of harts), not that the currently
> running kernel does. The Kconfig deps that must be met for the code to be
> built at least mean the kernel is built with vector support, leaving only
> "the kernel was built with vector support and the hardware supports vector
> but for $reason the kernel refused to enable it".
>
> I'm not sure if that final condition is actually possible with the system
> ending up in a broken state, however - I'm not sure that we ever do turn
> off access to the VPU at present (after we mark it usable), and if we do
> it doesn't get reflected in has_vector() so the kernel and userspace would
> both break, with what a crypto driver does probably being the least of
> your worries.
>
> > > So
> > > there should be no issue.  If there is, the RISC-V core architecture code needs
> > > to be fixed to not declare that extensions are supported when they are actually
> > > incompatible non-standard versions of those extensions.  Incompatible
> > > non-standard extensions should be represented as separate extensions.
> > >
> >
> > It probably makes sense to check has_vector() to exclude Zve* for now, though.
>
> I think you might actually be better served at present, given the code can
> only be built if the core vector code is, by using
> riscv_isa_extension_available(NULL, v). That way you know for sure that
> you're getting the ratified extension and nothing else.
>
> Prior to this conversation I thought that has_vector() should return true
> if there's a standard compliant vector unit available - given all users
> Andy added only need Zve32x.
>
> > I am just concerned about how you're suggesting that non-standard extensions
> > might be pretending to be standard ones and individual users of kernel-mode
> > vector would need to work around that.
>
> I am absolutely not suggesting that non-standard extensions should
> masquerade as standard ones, I don't know where you got that from. What
> I said was that a non-standard vector extension could reuse riscv_v_vlen
> (and should IMO for simplicity reasons), not that any of the APIs we have
> for checking extension availability would lie and say it was standard.
> riscv_v_vlen having a value greater than 128 is not one of those APIs ;)
>
> > I think that neither has_vector() nor
> > 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> > vector extension is non-standard.
>
> riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
> was present in DT or ACPI for all harts. It doesn't check whether or not
> the required config option for vector has been set or anything related
> to dependencies. has_vector() at least checks that the vector core has
> been enabled (and uses the alternative-patched version of the check
> given it is used in some hotter paths). That's kinda moot for code
> that's only built if the vector core stuff is enabled as I said above
> though.
>
> We could of course make riscv_isa_extension_available() check
> extension dependencies, but I'd rather leave dt validation to the dt
> tooling (apparently ACPI tables are never wrong...). Either would allow
> you to rely on the crypto extensions present only when the standard vector
> extensions unless someone's DT/ACPI stuff is shite, but then they keep the
> pieces IMO :)

Should we check if "v" presents for vector crypto extensions in
riscv_isa_extension_check()? We are not checking this for now. So a
kernel compiled with RISCV_ISA_V still has a problem if its isa-string
includes any of vector crypto ("zvbb, zvkg, etc") but not "v".

>
> Hope that makes sense?
> Conor.

Cheers,
Andy

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Andy Chiu]

Hi Conor,

On Fri, Apr 19, 2024 at 2:26 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Apr 18, 2024 at 10:39:46AM -0700, Eric Biggers wrote:
> > On Thu, Apr 18, 2024 at 10:32:03AM -0700, Eric Biggers wrote:
> > > On Thu, Apr 18, 2024 at 05:53:55PM +0100, Conor Dooley wrote:
> > > > > If it would be useful to do so, we should be able to enable some of the code
> > > > > with a smaller VLEN and/or EEW once it has been tested in those configurations.
> > > > > Some of it should work, but some of it won't be able to work.  (For example, the
> > > > > SHA512 instructions require EEW==64.)
> > > > >
> > > > > Also note that currently all the RISC-V vector crypto code only supports riscv64
> > > > > (XLEN=64).  Similarly, that could be relaxed in the future if people really need
> > > > > the vector crypto acceleration on 32-bit CPUs...  But similarly, the code would
> > > > > need to be revised and tested in that configuration.
> > > > >
> > > > > > Eric/Jerry (although read the previous paragraph too):
> > > > > > I noticed that the sha256 glue code calls crypto_simd_usable(), and in
> > > > > > turn may_use_simd() before kernel_vector_begin(). The chacha20 glue code
> > > > > > does not call either, which seems to violate the edict in
> > > > > > kernel_vector_begin()'s kerneldoc:
> > > > > > "Must not be called unless may_use_simd() returns true."
> > > > >
> > > > > skcipher algorithms can only be invoked in process and softirq context.  This
> > > > > differs from shash algorithms which can be invoked in any context.
> > > > >
> > > > > My understanding is that, like arm64, RISC-V always allows non-nested
> > > > > kernel-mode vector to be used in process and softirq context -- and in fact,
> > > > > this was intentionally done in order to support use cases like this.  So that's
> > > > > why the RISC-V skcipher algorithms don't check for may_use_simd() before calling
> > > > > kernel_vector_begin().
> > > >
> > > > I see, thanks for explaining that. I think you should probably check
> > > > somewhere if has_vector() returns true in that driver though before
> > > > using vector instructions. Only checking vlen seems to me like relying on
> > > > an implementation detail and if we set vlen for the T-Head/0.7.1 vector
> > > > it'd be fooled. That said, I don't think that any of the 0.7.1 vector
> > > > systems actually support Zvkb, but I hope you get my drift.
> > >
> > > All the algorithms check for at least one of the vector crypto extensions being
> > > supported, for example Zvkb.  'if (riscv_isa_extension_available(NULL, ZVKB))'
> > > should return whether the ratified version of Zvkb is supported, and likewise
> > > for the other vector crypto extensions.  The ratified version of the vector
> > > crypto extensions depends on the ratified version of the vector extension.
>
> That's great if it does require that the version of the vector extension
> must be standard. Higher quality spec than most if it does. But
> "supported" in the context of riscv_isa_extension_available() means that
> the hardware supports it (or set of harts), not that the currently
> running kernel does. The Kconfig deps that must be met for the code to be
> built at least mean the kernel is built with vector support, leaving only
> "the kernel was built with vector support and the hardware supports vector
> but for $reason the kernel refused to enable it".
>
> I'm not sure if that final condition is actually possible with the system
> ending up in a broken state, however - I'm not sure that we ever do turn
> off access to the VPU at present (after we mark it usable), and if we do
> it doesn't get reflected in has_vector() so the kernel and userspace would
> both break, with what a crypto driver does probably being the least of
> your worries.
>
> > > So
> > > there should be no issue.  If there is, the RISC-V core architecture code needs
> > > to be fixed to not declare that extensions are supported when they are actually
> > > incompatible non-standard versions of those extensions.  Incompatible
> > > non-standard extensions should be represented as separate extensions.
> > >
> >
> > It probably makes sense to check has_vector() to exclude Zve* for now, though.
>
> I think you might actually be better served at present, given the code can
> only be built if the core vector code is, by using
> riscv_isa_extension_available(NULL, v). That way you know for sure that
> you're getting the ratified extension and nothing else.
>
> Prior to this conversation I thought that has_vector() should return true
> if there's a standard compliant vector unit available - given all users
> Andy added only need Zve32x.
>
> > I am just concerned about how you're suggesting that non-standard extensions
> > might be pretending to be standard ones and individual users of kernel-mode
> > vector would need to work around that.
>
> I am absolutely not suggesting that non-standard extensions should
> masquerade as standard ones, I don't know where you got that from. What
> I said was that a non-standard vector extension could reuse riscv_v_vlen
> (and should IMO for simplicity reasons), not that any of the APIs we have
> for checking extension availability would lie and say it was standard.
> riscv_v_vlen having a value greater than 128 is not one of those APIs ;)
>
> > I think that neither has_vector() nor
> > 'if (riscv_isa_extension_available(NULL, ZVKB))' should return true if the CPU's
> > vector extension is non-standard.
>
> riscv_isa_extension_available(NULL, ZVKB) only checks whether the extension
> was present in DT or ACPI for all harts. It doesn't check whether or not
> the required config option for vector has been set or anything related
> to dependencies. has_vector() at least checks that the vector core has
> been enabled (and uses the alternative-patched version of the check
> given it is used in some hotter paths). That's kinda moot for code
> that's only built if the vector core stuff is enabled as I said above
> though.
>
> We could of course make riscv_isa_extension_available() check
> extension dependencies, but I'd rather leave dt validation to the dt
> tooling (apparently ACPI tables are never wrong...). Either would allow
> you to rely on the crypto extensions present only when the standard vector
> extensions unless someone's DT/ACPI stuff is shite, but then they keep the
> pieces IMO :)

Should we check if "v" presents for vector crypto extensions in
riscv_isa_extension_check()? We are not checking this for now. So a
kernel compiled with RISCV_ISA_V still has a problem if its isa-string
includes any of vector crypto ("zvbb, zvkg, etc") but not "v".

>
> Hope that makes sense?
> Conor.

Cheers,
Andy

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 1758 bytes --]

On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> Hi Conor,
> 
> Should we check if "v" presents for vector crypto extensions in
> riscv_isa_extension_check()? We are not checking this for now. So a
> kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> includes any of vector crypto ("zvbb, zvkg, etc") but not "v".


Yeah, one of the things I took away from this discussion is that we need
to improve the implementation of both the methods we have at the moment
for drivers etc to check if extensions are present and usable.
In general, I don't think checks like that are "safe" to do in
riscv_isa_extension_check(), because the dependencies may not all have
been resolved when we probe an extension (Clement's current Zca etc
series improves the situation though by only calling the checks after
we probe all extensions).

The simple V cases are all fine though - the DT binding and ACPI rules
for compatible strings all mandate that single-letter extensions must
come before multi-letter ones. For riscv,isa-extensions we control the
probe ordering and probe V before any multi-letter stuff. Additionally,
we should make it a requirement for V to be present if things that
depend on it are.

That said, is it permitted by the specs to have any of the extensions
you mention without the full V extension, but with one of the cut-down
variants you mention here? If not, I'd be more interested in figuring
out the non-extension dependencies: whether or not the kernel itself
supports vector and if the kernel has opted to disable vector due to
detecting that harts have mismatching vector lengths.

TL;DR: I think we should add some checks in riscv_isa_extension_check().

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 1758 bytes --]

On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> Hi Conor,
> 
> Should we check if "v" presents for vector crypto extensions in
> riscv_isa_extension_check()? We are not checking this for now. So a
> kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> includes any of vector crypto ("zvbb, zvkg, etc") but not "v".


Yeah, one of the things I took away from this discussion is that we need
to improve the implementation of both the methods we have at the moment
for drivers etc to check if extensions are present and usable.
In general, I don't think checks like that are "safe" to do in
riscv_isa_extension_check(), because the dependencies may not all have
been resolved when we probe an extension (Clement's current Zca etc
series improves the situation though by only calling the checks after
we probe all extensions).

The simple V cases are all fine though - the DT binding and ACPI rules
for compatible strings all mandate that single-letter extensions must
come before multi-letter ones. For riscv,isa-extensions we control the
probe ordering and probe V before any multi-letter stuff. Additionally,
we should make it a requirement for V to be present if things that
depend on it are.

That said, is it permitted by the specs to have any of the extensions
you mention without the full V extension, but with one of the cut-down
variants you mention here? If not, I'd be more interested in figuring
out the non-extension dependencies: whether or not the kernel itself
supports vector and if the kernel has opted to disable vector due to
detecting that harts have mismatching vector lengths.

TL;DR: I think we should add some checks in riscv_isa_extension_check().

Thanks,
Conor.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 2381 bytes --]

On Thu, May 09, 2024 at 08:48:09AM +0100, Conor Dooley wrote:
> On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> > Hi Conor,
> > 
> > Should we check if "v" presents for vector crypto extensions in
> > riscv_isa_extension_check()? We are not checking this for now. So a
> > kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> > includes any of vector crypto ("zvbb, zvkg, etc") but not "v".
> 
> 
> Yeah, one of the things I took away from this discussion is that we need
> to improve the implementation of both the methods we have at the moment
> for drivers etc to check if extensions are present and usable.
> In general, I don't think checks like that are "safe" to do in
> riscv_isa_extension_check(), because the dependencies may not all have
> been resolved when we probe an extension (Clement's current Zca etc
> series improves the situation though by only calling the checks after
> we probe all extensions).
> 
> The simple V cases are all fine though - the DT binding and ACPI rules
> for compatible strings all mandate that single-letter extensions must
> come before multi-letter ones. For riscv,isa-extensions we control the
> probe ordering and probe V before any multi-letter stuff. Additionally,
> we should make it a requirement for V to be present if things that
> depend on it are.
> 
> That said, is it permitted by the specs to have any of the extensions
> you mention without the full V extension, but with one of the cut-down
> variants you mention here? If not, I'd be more interested in figuring
> out the non-extension dependencies: whether or not the kernel itself
> supports vector and if the kernel has opted to disable vector due to
> detecting that harts have mismatching vector lengths.
> 
> TL;DR: I think we should add some checks in riscv_isa_extension_check().

Also, unless this only becomes a problem with this series that adds the
cut-down forms of vector, I think this is a separate problem to solve
and I can send some patches for it (along with some other cleanup I'd like
to do as a result of Eric's comments) and you can just submit the v2 you
were planning to without it. I can't, off the top of my head, think of
why this particular series would break the vector crypto stuff though,
the problems with enabling extensions seem underlying.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 2381 bytes --]

On Thu, May 09, 2024 at 08:48:09AM +0100, Conor Dooley wrote:
> On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> > Hi Conor,
> > 
> > Should we check if "v" presents for vector crypto extensions in
> > riscv_isa_extension_check()? We are not checking this for now. So a
> > kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> > includes any of vector crypto ("zvbb, zvkg, etc") but not "v".
> 
> 
> Yeah, one of the things I took away from this discussion is that we need
> to improve the implementation of both the methods we have at the moment
> for drivers etc to check if extensions are present and usable.
> In general, I don't think checks like that are "safe" to do in
> riscv_isa_extension_check(), because the dependencies may not all have
> been resolved when we probe an extension (Clement's current Zca etc
> series improves the situation though by only calling the checks after
> we probe all extensions).
> 
> The simple V cases are all fine though - the DT binding and ACPI rules
> for compatible strings all mandate that single-letter extensions must
> come before multi-letter ones. For riscv,isa-extensions we control the
> probe ordering and probe V before any multi-letter stuff. Additionally,
> we should make it a requirement for V to be present if things that
> depend on it are.
> 
> That said, is it permitted by the specs to have any of the extensions
> you mention without the full V extension, but with one of the cut-down
> variants you mention here? If not, I'd be more interested in figuring
> out the non-extension dependencies: whether or not the kernel itself
> supports vector and if the kernel has opted to disable vector due to
> detecting that harts have mismatching vector lengths.
> 
> TL;DR: I think we should add some checks in riscv_isa_extension_check().

Also, unless this only becomes a problem with this series that adds the
cut-down forms of vector, I think this is a separate problem to solve
and I can send some patches for it (along with some other cleanup I'd like
to do as a result of Eric's comments) and you can just submit the v2 you
were planning to without it. I can't, off the top of my head, think of
why this particular series would break the vector crypto stuff though,
the problems with enabling extensions seem underlying.

Thanks,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 3127 bytes --]

On Thu, May 09, 2024 at 09:25:25AM +0100, Conor Dooley wrote:
> On Thu, May 09, 2024 at 08:48:09AM +0100, Conor Dooley wrote:
> > On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> > > Hi Conor,
> > > 
> > > Should we check if "v" presents for vector crypto extensions in
> > > riscv_isa_extension_check()? We are not checking this for now. So a
> > > kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> > > includes any of vector crypto ("zvbb, zvkg, etc") but not "v".
> > 
> > 
> > Yeah, one of the things I took away from this discussion is that we need
> > to improve the implementation of both the methods we have at the moment
> > for drivers etc to check if extensions are present and usable.
> > In general, I don't think checks like that are "safe" to do in
> > riscv_isa_extension_check(), because the dependencies may not all have
> > been resolved when we probe an extension (Clement's current Zca etc
> > series improves the situation though by only calling the checks after
> > we probe all extensions).
> > 
> > The simple V cases are all fine though - the DT binding and ACPI rules
> > for compatible strings all mandate that single-letter extensions must
> > come before multi-letter ones. For riscv,isa-extensions we control the
> > probe ordering and probe V before any multi-letter stuff. Additionally,
> > we should make it a requirement for V to be present if things that
> > depend on it are.
> > 
> > That said, is it permitted by the specs to have any of the extensions
> > you mention without the full V extension, but with one of the cut-down
> > variants you mention here? If not, I'd be more interested in figuring
> > out the non-extension dependencies: whether or not the kernel itself
> > supports vector and if the kernel has opted to disable vector due to
> > detecting that harts have mismatching vector lengths.
> > 
> > TL;DR: I think we should add some checks in riscv_isa_extension_check().
> 
> Also, unless this only becomes a problem with this series that adds the
> cut-down forms of vector, I think this is a separate problem to solve
> and I can send some patches for it (along with some other cleanup I'd like
> to do as a result of Eric's comments) and you can just submit the v2 you
> were planning to without it. I can't, off the top of my head, think of
> why this particular series would break the vector crypto stuff though,
> the problems with enabling extensions seem underlying.

Here's something buggy that I chucked together as an idea of what I
meant:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-check_vector
Beware, it is entirely untested :)
It's based on both this series and patches 2 & 3 of Charlie's series doing
the T-Head vector stuff. It really needs Clement's extension_check()
rework that I mentioned 2 mails ago to function correctly for any of these
vector subsets. Without Clement's stuff, it'll have "random" behaviour
depending on probe order for riscv,isa and a determinate, but incorrect,
behaviour otherwise.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v4 7/9] riscv: vector: adjust minimum Vector requirement to ZVE32X

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3127 bytes --]

On Thu, May 09, 2024 at 09:25:25AM +0100, Conor Dooley wrote:
> On Thu, May 09, 2024 at 08:48:09AM +0100, Conor Dooley wrote:
> > On Thu, May 09, 2024 at 02:56:30PM +0800, Andy Chiu wrote:
> > > Hi Conor,
> > > 
> > > Should we check if "v" presents for vector crypto extensions in
> > > riscv_isa_extension_check()? We are not checking this for now. So a
> > > kernel compiled with RISCV_ISA_V still has a problem if its isa-string
> > > includes any of vector crypto ("zvbb, zvkg, etc") but not "v".
> > 
> > 
> > Yeah, one of the things I took away from this discussion is that we need
> > to improve the implementation of both the methods we have at the moment
> > for drivers etc to check if extensions are present and usable.
> > In general, I don't think checks like that are "safe" to do in
> > riscv_isa_extension_check(), because the dependencies may not all have
> > been resolved when we probe an extension (Clement's current Zca etc
> > series improves the situation though by only calling the checks after
> > we probe all extensions).
> > 
> > The simple V cases are all fine though - the DT binding and ACPI rules
> > for compatible strings all mandate that single-letter extensions must
> > come before multi-letter ones. For riscv,isa-extensions we control the
> > probe ordering and probe V before any multi-letter stuff. Additionally,
> > we should make it a requirement for V to be present if things that
> > depend on it are.
> > 
> > That said, is it permitted by the specs to have any of the extensions
> > you mention without the full V extension, but with one of the cut-down
> > variants you mention here? If not, I'd be more interested in figuring
> > out the non-extension dependencies: whether or not the kernel itself
> > supports vector and if the kernel has opted to disable vector due to
> > detecting that harts have mismatching vector lengths.
> > 
> > TL;DR: I think we should add some checks in riscv_isa_extension_check().
> 
> Also, unless this only becomes a problem with this series that adds the
> cut-down forms of vector, I think this is a separate problem to solve
> and I can send some patches for it (along with some other cleanup I'd like
> to do as a result of Eric's comments) and you can just submit the v2 you
> were planning to without it. I can't, off the top of my head, think of
> why this particular series would break the vector crypto stuff though,
> the problems with enabling extensions seem underlying.

Here's something buggy that I chucked together as an idea of what I
meant:
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-check_vector
Beware, it is entirely untested :)
It's based on both this series and patches 2 & 3 of Charlie's series doing
the T-Head vector stuff. It really needs Clement's extension_check()
rework that I mentioned 2 mails ago to function correctly for any of these
vector subsets. Without Clement's stuff, it'll have "random" behaviour
depending on probe order for riscv,isa and a determinate, but incorrect,
behaviour otherwise.

Cheers,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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