linux-doc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] RISC-V: hwprobe: Misaligned scalar perf fix and rename
@ 2024-06-25 16:51 Evan Green
  2024-06-25 16:51 ` [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key Evan Green
  2024-06-25 16:51 ` [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines Evan Green
  0 siblings, 2 replies; 10+ messages in thread
From: Evan Green @ 2024-06-25 16:51 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Yangyu Chen, Evan Green, Albert Ou, Alexandre Ghiti, Andrew Jones,
	Andy Chiu, Ben Dooks, Björn Töpel, Charlie Jenkins,
	Clément Léger, Conor Dooley, Costa Shulyupin,
	Erick Archer, Gustavo A. R. Silva, Jonathan Corbet, Paul Walmsley,
	linux-doc, linux-kernel, linux-riscv


The CPUPERF0 hwprobe key was documented and identified in code as
a bitmask value, but its contents were an enum. This produced
incorrect behavior in conjunction with the WHICH_CPUS hwprobe flag.
The first patch in this series fixes the bitmask/enum problem by
creating a new hwprobe key that returns the same data, but is
properly described as a value instead of a bitmask. The second patch
renames the value definitions in preparation for adding vector misaligned
access info. The second patch has the side effect of causing users
using the old key values to fail to compile, which may allow them
to see and update to the new key without breaking binary compatibility
with old programs.


Changes in v2:
 - Clarified the distinction of slow and fast refers to misaligned word
   accesses. Previously it just said misaligned accesses, leaving it
   ambiguous as to which type of access was measured.
 - Removed shifts in values (Andrew)
 - Renamed key to RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF (Palmer)
 - Added patch to rename misaligned perf key values (Palmer)

Evan Green (2):
  RISC-V: hwprobe: Add MISALIGNED_PERF key
  RISC-V: hwprobe: Add SCALAR to misaligned perf defines

 Documentation/arch/riscv/hwprobe.rst       | 29 +++++++++++++---------
 arch/riscv/include/asm/hwprobe.h           |  2 +-
 arch/riscv/include/uapi/asm/hwprobe.h      | 13 +++++-----
 arch/riscv/kernel/sys_hwprobe.c            | 11 ++++----
 arch/riscv/kernel/traps_misaligned.c       |  6 ++---
 arch/riscv/kernel/unaligned_access_speed.c | 12 ++++-----
 6 files changed, 40 insertions(+), 33 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key
  2024-06-25 16:51 [PATCH v2 0/2] RISC-V: hwprobe: Misaligned scalar perf fix and rename Evan Green
@ 2024-06-25 16:51 ` Evan Green
  2024-06-26 14:36   ` Conor Dooley
  2024-06-25 16:51 ` [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines Evan Green
  1 sibling, 1 reply; 10+ messages in thread
From: Evan Green @ 2024-06-25 16:51 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Yangyu Chen, Evan Green, Charlie Jenkins, Andrew Jones, Albert Ou,
	Andy Chiu, Clément Léger, Conor Dooley, Costa Shulyupin,
	Jonathan Corbet, Paul Walmsley, linux-doc, linux-kernel,
	linux-riscv

RISCV_HWPROBE_KEY_CPUPERF_0 was mistakenly flagged as a bitmask in
hwprobe_key_is_bitmask(), when in reality it was an enum value. This
causes problems when used in conjunction with RISCV_HWPROBE_WHICH_CPUS,
since SLOW, FAST, and EMULATED have values whose bits overlap with
each other. If the caller asked for the set of CPUs that was SLOW or
EMULATED, the returned set would also include CPUs that were FAST.

Introduce a new hwprobe key, RISCV_HWPROBE_KEY_MISALIGNED_PERF, which
returns the same values in response to a direct query (with no flags),
but is properly handled as an enumerated value. As a result, SLOW,
FAST, and EMULATED are all correctly treated as distinct values under
the new key when queried with the WHICH_CPUS flag.

Leave the old key in place to avoid disturbing applications which may
have already come to rely on the key, with or without its broken
behavior with respect to the WHICH_CPUS flag.

Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag")
Signed-off-by: Evan Green <evan@rivosinc.com>
Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

---

Changes in v2:
 - Clarified the distinction of slow and fast refers to misaligned word
   accesses. Previously it just said misaligned accesses, leaving it
   ambiguous as to which type of access was measured.
 - Removed shifts in values (Andrew)
 - Renamed key to RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF (Palmer)

 Documentation/arch/riscv/hwprobe.rst  | 17 +++++++++++------
 arch/riscv/include/asm/hwprobe.h      |  2 +-
 arch/riscv/include/uapi/asm/hwprobe.h | 13 +++++++------
 arch/riscv/kernel/sys_hwprobe.c       |  1 +
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index fc015b452ebf..c9f570b1ab60 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -207,8 +207,13 @@ The following keys are defined:
   * :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.
+* :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: Deprecated.  Returns similar values to
+     :c:macro:`RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF`, but the key was
+     mistakenly classified as a bitmask rather than a value.
+
+* :c:macro:`RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF`: An enum value describing
+  the performance of misaligned scalar word accesses on the selected set of
+  processors.
 
   * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
     accesses is unknown.
@@ -217,12 +222,12 @@ The following keys are defined:
     emulated via software, either in or below the kernel.  These accesses are
     always extremely slow.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned accesses are slower
-    than equivalent byte accesses.  Misaligned accesses may be supported
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
+    slower than equivalent byte accesses.  Misaligned accesses may be supported
     directly in hardware, or trapped and emulated by software.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned accesses are faster
-    than equivalent byte accesses.
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
+    faster than equivalent byte accesses.
 
   * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
     not supported at all and will generate a misaligned address fault.
diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 630507dff5ea..150a9877b0af 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -8,7 +8,7 @@
 
 #include <uapi/asm/hwprobe.h>
 
-#define RISCV_HWPROBE_MAX_KEY 6
+#define RISCV_HWPROBE_MAX_KEY 7
 
 static inline bool riscv_hwprobe_key_is_valid(__s64 key)
 {
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 7b95fadbea2a..22073533cea8 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -66,13 +66,14 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZVE64F	(1ULL << 40)
 #define		RISCV_HWPROBE_EXT_ZVE64D	(1ULL << 41)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
-#define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
-#define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
-#define		RISCV_HWPROBE_MISALIGNED_SLOW		(2 << 0)
-#define		RISCV_HWPROBE_MISALIGNED_FAST		(3 << 0)
-#define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
-#define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
+#define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	0
+#define		RISCV_HWPROBE_MISALIGNED_EMULATED	1
+#define		RISCV_HWPROBE_MISALIGNED_SLOW		2
+#define		RISCV_HWPROBE_MISALIGNED_FAST		3
+#define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	4
+#define		RISCV_HWPROBE_MISALIGNED_MASK		7
 #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
+#define RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF	7
 /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
 
 /* Flags */
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 83fcc939df67..991ceba67717 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -217,6 +217,7 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 		break;
 
 	case RISCV_HWPROBE_KEY_CPUPERF_0:
+	case RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF:
 		pair->value = hwprobe_misaligned(cpus);
 		break;
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines
  2024-06-25 16:51 [PATCH v2 0/2] RISC-V: hwprobe: Misaligned scalar perf fix and rename Evan Green
  2024-06-25 16:51 ` [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key Evan Green
@ 2024-06-25 16:51 ` Evan Green
  2024-06-25 18:33   ` Charlie Jenkins
  2024-06-25 18:35   ` Conor Dooley
  1 sibling, 2 replies; 10+ messages in thread
From: Evan Green @ 2024-06-25 16:51 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Yangyu Chen, Evan Green, Albert Ou, Alexandre Ghiti, Andrew Jones,
	Andy Chiu, Ben Dooks, Björn Töpel, Charlie Jenkins,
	Clément Léger, Conor Dooley, Costa Shulyupin,
	Erick Archer, Gustavo A. R. Silva, Jonathan Corbet, Paul Walmsley,
	linux-doc, linux-kernel, linux-riscv

In preparation for misaligned vector performance hwprobe keys, rename
the hwprobe key values associated with misaligned scalar accesses to
include the term SCALAR.

Signed-off-by: Evan Green <evan@rivosinc.com>

---

Changes in v2:
 - Added patch to rename misaligned perf key values (Palmer)

 Documentation/arch/riscv/hwprobe.rst       | 20 ++++++++++----------
 arch/riscv/include/uapi/asm/hwprobe.h      | 10 +++++-----
 arch/riscv/kernel/sys_hwprobe.c            | 10 +++++-----
 arch/riscv/kernel/traps_misaligned.c       |  6 +++---
 arch/riscv/kernel/unaligned_access_speed.c | 12 ++++++------
 5 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index c9f570b1ab60..83f7f3c1347f 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -215,22 +215,22 @@ The following keys are defined:
   the performance of misaligned scalar word accesses on the selected set of
   processors.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
-    accesses is unknown.
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN`: The performance of
+    misaligned accesses is unknown.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED`: Misaligned accesses are
     emulated via software, either in or below the kernel.  These accesses are
     always extremely slow.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
-    slower than equivalent byte accesses.  Misaligned accesses may be supported
-    directly in hardware, or trapped and emulated by software.
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned word accesses
+    are slower than equivalent byte accesses.  Misaligned accesses may be
+    supported directly in hardware, or trapped and emulated by software.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
-    faster than equivalent byte accesses.
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned word accesses
+    are faster than equivalent byte accesses.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
-    not supported at all and will generate a misaligned address fault.
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED`: Misaligned accesses
+    are not supported at all and will generate a misaligned address fault.
 
 * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
   represents the size of the Zicboz block in bytes.
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 22073533cea8..e11684d8ae1c 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -66,11 +66,11 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZVE64F	(1ULL << 40)
 #define		RISCV_HWPROBE_EXT_ZVE64D	(1ULL << 41)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
-#define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	0
-#define		RISCV_HWPROBE_MISALIGNED_EMULATED	1
-#define		RISCV_HWPROBE_MISALIGNED_SLOW		2
-#define		RISCV_HWPROBE_MISALIGNED_FAST		3
-#define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	4
+#define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN		0
+#define		RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED	1
+#define		RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW		2
+#define		RISCV_HWPROBE_MISALIGNED_SCALAR_FAST		3
+#define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED	4
 #define		RISCV_HWPROBE_MISALIGNED_MASK		7
 #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
 #define RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF	7
diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
index 991ceba67717..fbf952e7383e 100644
--- a/arch/riscv/kernel/sys_hwprobe.c
+++ b/arch/riscv/kernel/sys_hwprobe.c
@@ -170,13 +170,13 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
 			perf = this_perf;
 
 		if (perf != this_perf) {
-			perf = RISCV_HWPROBE_MISALIGNED_UNKNOWN;
+			perf = RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN;
 			break;
 		}
 	}
 
 	if (perf == -1ULL)
-		return RISCV_HWPROBE_MISALIGNED_UNKNOWN;
+		return RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN;
 
 	return perf;
 }
@@ -184,12 +184,12 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
 static u64 hwprobe_misaligned(const struct cpumask *cpus)
 {
 	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS))
-		return RISCV_HWPROBE_MISALIGNED_FAST;
+		return RISCV_HWPROBE_MISALIGNED_SCALAR_FAST;
 
 	if (IS_ENABLED(CONFIG_RISCV_EMULATED_UNALIGNED_ACCESS) && unaligned_ctl_available())
-		return RISCV_HWPROBE_MISALIGNED_EMULATED;
+		return RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED;
 
-	return RISCV_HWPROBE_MISALIGNED_SLOW;
+	return RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW;
 }
 #endif
 
diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
index b62d5a2f4541..192cd5603e95 100644
--- a/arch/riscv/kernel/traps_misaligned.c
+++ b/arch/riscv/kernel/traps_misaligned.c
@@ -338,7 +338,7 @@ int handle_misaligned_load(struct pt_regs *regs)
 	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
 
 #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
-	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
+	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED;
 #endif
 
 	if (!unaligned_enabled)
@@ -532,13 +532,13 @@ static bool check_unaligned_access_emulated(int cpu)
 	unsigned long tmp_var, tmp_val;
 	bool misaligned_emu_detected;
 
-	*mas_ptr = RISCV_HWPROBE_MISALIGNED_UNKNOWN;
+	*mas_ptr = RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN;
 
 	__asm__ __volatile__ (
 		"       "REG_L" %[tmp], 1(%[ptr])\n"
 		: [tmp] "=r" (tmp_val) : [ptr] "r" (&tmp_var) : "memory");
 
-	misaligned_emu_detected = (*mas_ptr == RISCV_HWPROBE_MISALIGNED_EMULATED);
+	misaligned_emu_detected = (*mas_ptr == RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED);
 	/*
 	 * If unaligned_ctl is already set, this means that we detected that all
 	 * CPUS uses emulated misaligned access at boot time. If that changed
diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
index a9a6bcb02acf..160628a2116d 100644
--- a/arch/riscv/kernel/unaligned_access_speed.c
+++ b/arch/riscv/kernel/unaligned_access_speed.c
@@ -34,9 +34,9 @@ static int check_unaligned_access(void *param)
 	struct page *page = param;
 	void *dst;
 	void *src;
-	long speed = RISCV_HWPROBE_MISALIGNED_SLOW;
+	long speed = RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW;
 
-	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_UNKNOWN)
+	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN)
 		return 0;
 
 	/* Make an unaligned destination buffer. */
@@ -95,14 +95,14 @@ static int check_unaligned_access(void *param)
 	}
 
 	if (word_cycles < byte_cycles)
-		speed = RISCV_HWPROBE_MISALIGNED_FAST;
+		speed = RISCV_HWPROBE_MISALIGNED_SCALAR_FAST;
 
 	ratio = div_u64((byte_cycles * 100), word_cycles);
 	pr_info("cpu%d: Ratio of byte access time to unaligned word access is %d.%02d, unaligned accesses are %s\n",
 		cpu,
 		ratio / 100,
 		ratio % 100,
-		(speed == RISCV_HWPROBE_MISALIGNED_FAST) ? "fast" : "slow");
+		(speed == RISCV_HWPROBE_MISALIGNED_SCALAR_FAST) ? "fast" : "slow");
 
 	per_cpu(misaligned_access_speed, cpu) = speed;
 
@@ -110,7 +110,7 @@ static int check_unaligned_access(void *param)
 	 * Set the value of fast_misaligned_access of a CPU. These operations
 	 * are atomic to avoid race conditions.
 	 */
-	if (speed == RISCV_HWPROBE_MISALIGNED_FAST)
+	if (speed == RISCV_HWPROBE_MISALIGNED_SCALAR_FAST)
 		cpumask_set_cpu(cpu, &fast_misaligned_access);
 	else
 		cpumask_clear_cpu(cpu, &fast_misaligned_access);
@@ -188,7 +188,7 @@ static int riscv_online_cpu(unsigned int cpu)
 	static struct page *buf;
 
 	/* We are already set since the last check */
-	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_UNKNOWN)
+	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN)
 		goto exit;
 
 	buf = alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines
  2024-06-25 16:51 ` [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines Evan Green
@ 2024-06-25 18:33   ` Charlie Jenkins
  2024-06-25 18:35   ` Conor Dooley
  1 sibling, 0 replies; 10+ messages in thread
From: Charlie Jenkins @ 2024-06-25 18:33 UTC (permalink / raw)
  To: Evan Green
  Cc: Palmer Dabbelt, Yangyu Chen, Albert Ou, Alexandre Ghiti,
	Andrew Jones, Andy Chiu, Ben Dooks, Björn Töpel,
	Clément Léger, Conor Dooley, Costa Shulyupin,
	Erick Archer, Gustavo A. R. Silva, Jonathan Corbet, Paul Walmsley,
	linux-doc, linux-kernel, linux-riscv

On Tue, Jun 25, 2024 at 09:51:21AM -0700, Evan Green wrote:
> In preparation for misaligned vector performance hwprobe keys, rename
> the hwprobe key values associated with misaligned scalar accesses to
> include the term SCALAR.
> 
> Signed-off-by: Evan Green <evan@rivosinc.com>

Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>

> 
> ---
> 
> Changes in v2:
>  - Added patch to rename misaligned perf key values (Palmer)
> 
>  Documentation/arch/riscv/hwprobe.rst       | 20 ++++++++++----------
>  arch/riscv/include/uapi/asm/hwprobe.h      | 10 +++++-----
>  arch/riscv/kernel/sys_hwprobe.c            | 10 +++++-----
>  arch/riscv/kernel/traps_misaligned.c       |  6 +++---
>  arch/riscv/kernel/unaligned_access_speed.c | 12 ++++++------
>  5 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> index c9f570b1ab60..83f7f3c1347f 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -215,22 +215,22 @@ The following keys are defined:
>    the performance of misaligned scalar word accesses on the selected set of
>    processors.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
> -    accesses is unknown.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN`: The performance of
> +    misaligned accesses is unknown.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED`: Misaligned accesses are
>      emulated via software, either in or below the kernel.  These accesses are
>      always extremely slow.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
> -    slower than equivalent byte accesses.  Misaligned accesses may be supported
> -    directly in hardware, or trapped and emulated by software.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned word accesses
> +    are slower than equivalent byte accesses.  Misaligned accesses may be
> +    supported directly in hardware, or trapped and emulated by software.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
> -    faster than equivalent byte accesses.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned word accesses
> +    are faster than equivalent byte accesses.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
> -    not supported at all and will generate a misaligned address fault.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED`: Misaligned accesses
> +    are not supported at all and will generate a misaligned address fault.
>  
>  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
>    represents the size of the Zicboz block in bytes.
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 22073533cea8..e11684d8ae1c 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -66,11 +66,11 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZVE64F	(1ULL << 40)
>  #define		RISCV_HWPROBE_EXT_ZVE64D	(1ULL << 41)
>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> -#define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	0
> -#define		RISCV_HWPROBE_MISALIGNED_EMULATED	1
> -#define		RISCV_HWPROBE_MISALIGNED_SLOW		2
> -#define		RISCV_HWPROBE_MISALIGNED_FAST		3
> -#define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	4
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN		0
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED	1
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW		2
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_FAST		3
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED	4
>  #define		RISCV_HWPROBE_MISALIGNED_MASK		7
>  #define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
>  #define RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF	7
> diff --git a/arch/riscv/kernel/sys_hwprobe.c b/arch/riscv/kernel/sys_hwprobe.c
> index 991ceba67717..fbf952e7383e 100644
> --- a/arch/riscv/kernel/sys_hwprobe.c
> +++ b/arch/riscv/kernel/sys_hwprobe.c
> @@ -170,13 +170,13 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>  			perf = this_perf;
>  
>  		if (perf != this_perf) {
> -			perf = RISCV_HWPROBE_MISALIGNED_UNKNOWN;
> +			perf = RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN;
>  			break;
>  		}
>  	}
>  
>  	if (perf == -1ULL)
> -		return RISCV_HWPROBE_MISALIGNED_UNKNOWN;
> +		return RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN;
>  
>  	return perf;
>  }
> @@ -184,12 +184,12 @@ static u64 hwprobe_misaligned(const struct cpumask *cpus)
>  static u64 hwprobe_misaligned(const struct cpumask *cpus)
>  {
>  	if (IS_ENABLED(CONFIG_RISCV_EFFICIENT_UNALIGNED_ACCESS))
> -		return RISCV_HWPROBE_MISALIGNED_FAST;
> +		return RISCV_HWPROBE_MISALIGNED_SCALAR_FAST;
>  
>  	if (IS_ENABLED(CONFIG_RISCV_EMULATED_UNALIGNED_ACCESS) && unaligned_ctl_available())
> -		return RISCV_HWPROBE_MISALIGNED_EMULATED;
> +		return RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED;
>  
> -	return RISCV_HWPROBE_MISALIGNED_SLOW;
> +	return RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW;
>  }
>  #endif
>  
> diff --git a/arch/riscv/kernel/traps_misaligned.c b/arch/riscv/kernel/traps_misaligned.c
> index b62d5a2f4541..192cd5603e95 100644
> --- a/arch/riscv/kernel/traps_misaligned.c
> +++ b/arch/riscv/kernel/traps_misaligned.c
> @@ -338,7 +338,7 @@ int handle_misaligned_load(struct pt_regs *regs)
>  	perf_sw_event(PERF_COUNT_SW_ALIGNMENT_FAULTS, 1, regs, addr);
>  
>  #ifdef CONFIG_RISCV_PROBE_UNALIGNED_ACCESS
> -	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_EMULATED;
> +	*this_cpu_ptr(&misaligned_access_speed) = RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED;
>  #endif
>  
>  	if (!unaligned_enabled)
> @@ -532,13 +532,13 @@ static bool check_unaligned_access_emulated(int cpu)
>  	unsigned long tmp_var, tmp_val;
>  	bool misaligned_emu_detected;
>  
> -	*mas_ptr = RISCV_HWPROBE_MISALIGNED_UNKNOWN;
> +	*mas_ptr = RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN;
>  
>  	__asm__ __volatile__ (
>  		"       "REG_L" %[tmp], 1(%[ptr])\n"
>  		: [tmp] "=r" (tmp_val) : [ptr] "r" (&tmp_var) : "memory");
>  
> -	misaligned_emu_detected = (*mas_ptr == RISCV_HWPROBE_MISALIGNED_EMULATED);
> +	misaligned_emu_detected = (*mas_ptr == RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED);
>  	/*
>  	 * If unaligned_ctl is already set, this means that we detected that all
>  	 * CPUS uses emulated misaligned access at boot time. If that changed
> diff --git a/arch/riscv/kernel/unaligned_access_speed.c b/arch/riscv/kernel/unaligned_access_speed.c
> index a9a6bcb02acf..160628a2116d 100644
> --- a/arch/riscv/kernel/unaligned_access_speed.c
> +++ b/arch/riscv/kernel/unaligned_access_speed.c
> @@ -34,9 +34,9 @@ static int check_unaligned_access(void *param)
>  	struct page *page = param;
>  	void *dst;
>  	void *src;
> -	long speed = RISCV_HWPROBE_MISALIGNED_SLOW;
> +	long speed = RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW;
>  
> -	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_UNKNOWN)
> +	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN)
>  		return 0;
>  
>  	/* Make an unaligned destination buffer. */
> @@ -95,14 +95,14 @@ static int check_unaligned_access(void *param)
>  	}
>  
>  	if (word_cycles < byte_cycles)
> -		speed = RISCV_HWPROBE_MISALIGNED_FAST;
> +		speed = RISCV_HWPROBE_MISALIGNED_SCALAR_FAST;
>  
>  	ratio = div_u64((byte_cycles * 100), word_cycles);
>  	pr_info("cpu%d: Ratio of byte access time to unaligned word access is %d.%02d, unaligned accesses are %s\n",
>  		cpu,
>  		ratio / 100,
>  		ratio % 100,
> -		(speed == RISCV_HWPROBE_MISALIGNED_FAST) ? "fast" : "slow");
> +		(speed == RISCV_HWPROBE_MISALIGNED_SCALAR_FAST) ? "fast" : "slow");
>  
>  	per_cpu(misaligned_access_speed, cpu) = speed;
>  
> @@ -110,7 +110,7 @@ static int check_unaligned_access(void *param)
>  	 * Set the value of fast_misaligned_access of a CPU. These operations
>  	 * are atomic to avoid race conditions.
>  	 */
> -	if (speed == RISCV_HWPROBE_MISALIGNED_FAST)
> +	if (speed == RISCV_HWPROBE_MISALIGNED_SCALAR_FAST)
>  		cpumask_set_cpu(cpu, &fast_misaligned_access);
>  	else
>  		cpumask_clear_cpu(cpu, &fast_misaligned_access);
> @@ -188,7 +188,7 @@ static int riscv_online_cpu(unsigned int cpu)
>  	static struct page *buf;
>  
>  	/* We are already set since the last check */
> -	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_UNKNOWN)
> +	if (per_cpu(misaligned_access_speed, cpu) != RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN)
>  		goto exit;
>  
>  	buf = alloc_pages(GFP_KERNEL, MISALIGNED_BUFFER_ORDER);
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines
  2024-06-25 16:51 ` [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines Evan Green
  2024-06-25 18:33   ` Charlie Jenkins
@ 2024-06-25 18:35   ` Conor Dooley
  2024-06-25 19:08     ` Evan Green
  1 sibling, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2024-06-25 18:35 UTC (permalink / raw)
  To: Evan Green
  Cc: Palmer Dabbelt, Yangyu Chen, Albert Ou, Alexandre Ghiti,
	Andrew Jones, Andy Chiu, Ben Dooks, Björn Töpel,
	Charlie Jenkins, Clément Léger, Conor Dooley,
	Costa Shulyupin, Erick Archer, Gustavo A. R. Silva,
	Jonathan Corbet, Paul Walmsley, linux-doc, linux-kernel,
	linux-riscv

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

On Tue, Jun 25, 2024 at 09:51:21AM -0700, Evan Green wrote:
> In preparation for misaligned vector performance hwprobe keys, rename
> the hwprobe key values associated with misaligned scalar accesses to
> include the term SCALAR.
> 
> Signed-off-by: Evan Green <evan@rivosinc.com>
> 
> ---
> 
> Changes in v2:
>  - Added patch to rename misaligned perf key values (Palmer)
> 
>  Documentation/arch/riscv/hwprobe.rst       | 20 ++++++++++----------
>  arch/riscv/include/uapi/asm/hwprobe.h      | 10 +++++-----
>  arch/riscv/kernel/sys_hwprobe.c            | 10 +++++-----
>  arch/riscv/kernel/traps_misaligned.c       |  6 +++---
>  arch/riscv/kernel/unaligned_access_speed.c | 12 ++++++------
>  5 files changed, 29 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> index c9f570b1ab60..83f7f3c1347f 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -215,22 +215,22 @@ The following keys are defined:
>    the performance of misaligned scalar word accesses on the selected set of
>    processors.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
> -    accesses is unknown.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN`: The performance of
> +    misaligned accesses is unknown.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED`: Misaligned accesses are
>      emulated via software, either in or below the kernel.  These accesses are
>      always extremely slow.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
> -    slower than equivalent byte accesses.  Misaligned accesses may be supported
> -    directly in hardware, or trapped and emulated by software.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned word accesses
> +    are slower than equivalent byte accesses.  Misaligned accesses may be
> +    supported directly in hardware, or trapped and emulated by software.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
> -    faster than equivalent byte accesses.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned word accesses
> +    are faster than equivalent byte accesses.
>  
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
> -    not supported at all and will generate a misaligned address fault.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED`: Misaligned accesses
> +    are not supported at all and will generate a misaligned address fault.
>  
>  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
>    represents the size of the Zicboz block in bytes.
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 22073533cea8..e11684d8ae1c 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -66,11 +66,11 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZVE64F	(1ULL << 40)
>  #define		RISCV_HWPROBE_EXT_ZVE64D	(1ULL << 41)
>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> -#define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	0
> -#define		RISCV_HWPROBE_MISALIGNED_EMULATED	1
> -#define		RISCV_HWPROBE_MISALIGNED_SLOW		2
> -#define		RISCV_HWPROBE_MISALIGNED_FAST		3
> -#define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	4
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN		0
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED	1
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW		2
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_FAST		3
> +#define		RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED	4
>  #define		RISCV_HWPROBE_MISALIGNED_MASK		7

How come the "old" names do not need to be preserved for userspace?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines
  2024-06-25 18:35   ` Conor Dooley
@ 2024-06-25 19:08     ` Evan Green
  2024-06-26 14:34       ` Conor Dooley
  0 siblings, 1 reply; 10+ messages in thread
From: Evan Green @ 2024-06-25 19:08 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Palmer Dabbelt, Yangyu Chen, Albert Ou, Alexandre Ghiti,
	Andrew Jones, Andy Chiu, Ben Dooks, Björn Töpel,
	Charlie Jenkins, Clément Léger, Conor Dooley,
	Costa Shulyupin, Erick Archer, Gustavo A. R. Silva,
	Jonathan Corbet, Paul Walmsley, linux-doc, linux-kernel,
	linux-riscv

On Tue, Jun 25, 2024 at 11:35 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Jun 25, 2024 at 09:51:21AM -0700, Evan Green wrote:
> > In preparation for misaligned vector performance hwprobe keys, rename
> > the hwprobe key values associated with misaligned scalar accesses to
> > include the term SCALAR.
> >
> > Signed-off-by: Evan Green <evan@rivosinc.com>
> >
> > ---
> >
> > Changes in v2:
> >  - Added patch to rename misaligned perf key values (Palmer)
> >
> >  Documentation/arch/riscv/hwprobe.rst       | 20 ++++++++++----------
> >  arch/riscv/include/uapi/asm/hwprobe.h      | 10 +++++-----
> >  arch/riscv/kernel/sys_hwprobe.c            | 10 +++++-----
> >  arch/riscv/kernel/traps_misaligned.c       |  6 +++---
> >  arch/riscv/kernel/unaligned_access_speed.c | 12 ++++++------
> >  5 files changed, 29 insertions(+), 29 deletions(-)
> >
> > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > index c9f570b1ab60..83f7f3c1347f 100644
> > --- a/Documentation/arch/riscv/hwprobe.rst
> > +++ b/Documentation/arch/riscv/hwprobe.rst
> > @@ -215,22 +215,22 @@ The following keys are defined:
> >    the performance of misaligned scalar word accesses on the selected set of
> >    processors.
> >
> > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
> > -    accesses is unknown.
> > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN`: The performance of
> > +    misaligned accesses is unknown.
> >
> > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
> > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED`: Misaligned accesses are
> >      emulated via software, either in or below the kernel.  These accesses are
> >      always extremely slow.
> >
> > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
> > -    slower than equivalent byte accesses.  Misaligned accesses may be supported
> > -    directly in hardware, or trapped and emulated by software.
> > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned word accesses
> > +    are slower than equivalent byte accesses.  Misaligned accesses may be
> > +    supported directly in hardware, or trapped and emulated by software.
> >
> > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
> > -    faster than equivalent byte accesses.
> > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned word accesses
> > +    are faster than equivalent byte accesses.
> >
> > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
> > -    not supported at all and will generate a misaligned address fault.
> > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED`: Misaligned accesses
> > +    are not supported at all and will generate a misaligned address fault.
> >
> >  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
> >    represents the size of the Zicboz block in bytes.
> > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > index 22073533cea8..e11684d8ae1c 100644
> > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > @@ -66,11 +66,11 @@ struct riscv_hwprobe {
> >  #define              RISCV_HWPROBE_EXT_ZVE64F        (1ULL << 40)
> >  #define              RISCV_HWPROBE_EXT_ZVE64D        (1ULL << 41)
> >  #define RISCV_HWPROBE_KEY_CPUPERF_0  5
> > -#define              RISCV_HWPROBE_MISALIGNED_UNKNOWN        0
> > -#define              RISCV_HWPROBE_MISALIGNED_EMULATED       1
> > -#define              RISCV_HWPROBE_MISALIGNED_SLOW           2
> > -#define              RISCV_HWPROBE_MISALIGNED_FAST           3
> > -#define              RISCV_HWPROBE_MISALIGNED_UNSUPPORTED    4
> > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN         0
> > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED        1
> > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW            2
> > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_FAST            3
> > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED     4
> >  #define              RISCV_HWPROBE_MISALIGNED_MASK           7
>
> How come the "old" names do not need to be preserved for userspace?

It depends on what exactly the big userspace compatibility rule is.
This preserves binary compatibility, which I think is the big one, but
breaks source compatibility, though with an easy translation to fix.
We could keep the old names around, but then it seems sort of silly to
introduce the new names. I introduced this patch upon request, so I
don't personally have a horse in the race on this one.

-Evan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines
  2024-06-25 19:08     ` Evan Green
@ 2024-06-26 14:34       ` Conor Dooley
  2024-06-26 15:44         ` Evan Green
  0 siblings, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2024-06-26 14:34 UTC (permalink / raw)
  To: Evan Green
  Cc: Palmer Dabbelt, Yangyu Chen, Albert Ou, Alexandre Ghiti,
	Andrew Jones, Andy Chiu, Ben Dooks, Björn Töpel,
	Charlie Jenkins, Clément Léger, Conor Dooley,
	Costa Shulyupin, Erick Archer, Gustavo A. R. Silva,
	Jonathan Corbet, Paul Walmsley, linux-doc, linux-kernel,
	linux-riscv

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

On Tue, Jun 25, 2024 at 12:08:09PM -0700, Evan Green wrote:
> On Tue, Jun 25, 2024 at 11:35 AM Conor Dooley <conor@kernel.org> wrote:
> >
> > On Tue, Jun 25, 2024 at 09:51:21AM -0700, Evan Green wrote:
> > > In preparation for misaligned vector performance hwprobe keys, rename
> > > the hwprobe key values associated with misaligned scalar accesses to
> > > include the term SCALAR.
> > >
> > > Signed-off-by: Evan Green <evan@rivosinc.com>
> > >
> > > ---
> > >
> > > Changes in v2:
> > >  - Added patch to rename misaligned perf key values (Palmer)
> > >
> > >  Documentation/arch/riscv/hwprobe.rst       | 20 ++++++++++----------
> > >  arch/riscv/include/uapi/asm/hwprobe.h      | 10 +++++-----
> > >  arch/riscv/kernel/sys_hwprobe.c            | 10 +++++-----
> > >  arch/riscv/kernel/traps_misaligned.c       |  6 +++---
> > >  arch/riscv/kernel/unaligned_access_speed.c | 12 ++++++------
> > >  5 files changed, 29 insertions(+), 29 deletions(-)
> > >
> > > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > > index c9f570b1ab60..83f7f3c1347f 100644
> > > --- a/Documentation/arch/riscv/hwprobe.rst
> > > +++ b/Documentation/arch/riscv/hwprobe.rst
> > > @@ -215,22 +215,22 @@ The following keys are defined:
> > >    the performance of misaligned scalar word accesses on the selected set of
> > >    processors.
> > >
> > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
> > > -    accesses is unknown.
> > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN`: The performance of
> > > +    misaligned accesses is unknown.
> > >
> > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
> > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED`: Misaligned accesses are
> > >      emulated via software, either in or below the kernel.  These accesses are
> > >      always extremely slow.
> > >
> > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
> > > -    slower than equivalent byte accesses.  Misaligned accesses may be supported
> > > -    directly in hardware, or trapped and emulated by software.
> > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned word accesses
> > > +    are slower than equivalent byte accesses.  Misaligned accesses may be
> > > +    supported directly in hardware, or trapped and emulated by software.
> > >
> > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
> > > -    faster than equivalent byte accesses.
> > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned word accesses
> > > +    are faster than equivalent byte accesses.
> > >
> > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
> > > -    not supported at all and will generate a misaligned address fault.
> > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED`: Misaligned accesses
> > > +    are not supported at all and will generate a misaligned address fault.
> > >
> > >  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
> > >    represents the size of the Zicboz block in bytes.
> > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > > index 22073533cea8..e11684d8ae1c 100644
> > > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > > @@ -66,11 +66,11 @@ struct riscv_hwprobe {
> > >  #define              RISCV_HWPROBE_EXT_ZVE64F        (1ULL << 40)
> > >  #define              RISCV_HWPROBE_EXT_ZVE64D        (1ULL << 41)
> > >  #define RISCV_HWPROBE_KEY_CPUPERF_0  5
> > > -#define              RISCV_HWPROBE_MISALIGNED_UNKNOWN        0
> > > -#define              RISCV_HWPROBE_MISALIGNED_EMULATED       1
> > > -#define              RISCV_HWPROBE_MISALIGNED_SLOW           2
> > > -#define              RISCV_HWPROBE_MISALIGNED_FAST           3
> > > -#define              RISCV_HWPROBE_MISALIGNED_UNSUPPORTED    4
> > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN         0
> > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED        1
> > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW            2
> > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_FAST            3
> > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED     4
> > >  #define              RISCV_HWPROBE_MISALIGNED_MASK           7
> >
> > How come the "old" names do not need to be preserved for userspace?
> 
> It depends on what exactly the big userspace compatibility rule is.
> This preserves binary compatibility, which I think is the big one, but
> breaks source compatibility, though with an easy translation to fix.
> We could keep the old names around, but then it seems sort of silly to
> introduce the new names. I introduced this patch upon request, so I
> don't personally have a horse in the race on this one.

So apparently this isn't quite Palmer actually wanted. In today's call
he suggested that he'd send a new version himself, but also that what we
should do define a new key for scalar /and/ new add new defines values that
contain the word scalar, retaining the old defines. The values can of
course be the same.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key
  2024-06-25 16:51 ` [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key Evan Green
@ 2024-06-26 14:36   ` Conor Dooley
  2024-06-26 15:55     ` Evan Green
  0 siblings, 1 reply; 10+ messages in thread
From: Conor Dooley @ 2024-06-26 14:36 UTC (permalink / raw)
  To: Evan Green
  Cc: Palmer Dabbelt, Yangyu Chen, Charlie Jenkins, Andrew Jones,
	Albert Ou, Andy Chiu, Clément Léger, Conor Dooley,
	Costa Shulyupin, Jonathan Corbet, Paul Walmsley, linux-doc,
	linux-kernel, linux-riscv

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

On Tue, Jun 25, 2024 at 09:51:20AM -0700, Evan Green wrote:
> RISCV_HWPROBE_KEY_CPUPERF_0 was mistakenly flagged as a bitmask in
> hwprobe_key_is_bitmask(), when in reality it was an enum value. This
> causes problems when used in conjunction with RISCV_HWPROBE_WHICH_CPUS,
> since SLOW, FAST, and EMULATED have values whose bits overlap with
> each other. If the caller asked for the set of CPUs that was SLOW or
> EMULATED, the returned set would also include CPUs that were FAST.
> 
> Introduce a new hwprobe key, RISCV_HWPROBE_KEY_MISALIGNED_PERF, which
> returns the same values in response to a direct query (with no flags),
> but is properly handled as an enumerated value. As a result, SLOW,
> FAST, and EMULATED are all correctly treated as distinct values under
> the new key when queried with the WHICH_CPUS flag.
> 
> Leave the old key in place to avoid disturbing applications which may
> have already come to rely on the key, with or without its broken
> behavior with respect to the WHICH_CPUS flag.
> 
> Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag")
> Signed-off-by: Evan Green <evan@rivosinc.com>
> Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> 
> ---
> 
> Changes in v2:
>  - Clarified the distinction of slow and fast refers to misaligned word
>    accesses. Previously it just said misaligned accesses, leaving it
>    ambiguous as to which type of access was measured.

I think if we are gonna be specific, we should be exactly specific as to
what we have tested and say 32-bit if that's what we're probing/testing
with. That'd be consistent with jesse's proposed wording for vector.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines
  2024-06-26 14:34       ` Conor Dooley
@ 2024-06-26 15:44         ` Evan Green
  0 siblings, 0 replies; 10+ messages in thread
From: Evan Green @ 2024-06-26 15:44 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Palmer Dabbelt, Yangyu Chen, Albert Ou, Alexandre Ghiti,
	Andrew Jones, Andy Chiu, Ben Dooks, Björn Töpel,
	Charlie Jenkins, Clément Léger, Conor Dooley,
	Costa Shulyupin, Erick Archer, Gustavo A. R. Silva,
	Jonathan Corbet, Paul Walmsley, linux-doc, linux-kernel,
	linux-riscv

On Wed, Jun 26, 2024 at 7:34 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Jun 25, 2024 at 12:08:09PM -0700, Evan Green wrote:
> > On Tue, Jun 25, 2024 at 11:35 AM Conor Dooley <conor@kernel.org> wrote:
> > >
> > > On Tue, Jun 25, 2024 at 09:51:21AM -0700, Evan Green wrote:
> > > > In preparation for misaligned vector performance hwprobe keys, rename
> > > > the hwprobe key values associated with misaligned scalar accesses to
> > > > include the term SCALAR.
> > > >
> > > > Signed-off-by: Evan Green <evan@rivosinc.com>
> > > >
> > > > ---
> > > >
> > > > Changes in v2:
> > > >  - Added patch to rename misaligned perf key values (Palmer)
> > > >
> > > >  Documentation/arch/riscv/hwprobe.rst       | 20 ++++++++++----------
> > > >  arch/riscv/include/uapi/asm/hwprobe.h      | 10 +++++-----
> > > >  arch/riscv/kernel/sys_hwprobe.c            | 10 +++++-----
> > > >  arch/riscv/kernel/traps_misaligned.c       |  6 +++---
> > > >  arch/riscv/kernel/unaligned_access_speed.c | 12 ++++++------
> > > >  5 files changed, 29 insertions(+), 29 deletions(-)
> > > >
> > > > diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
> > > > index c9f570b1ab60..83f7f3c1347f 100644
> > > > --- a/Documentation/arch/riscv/hwprobe.rst
> > > > +++ b/Documentation/arch/riscv/hwprobe.rst
> > > > @@ -215,22 +215,22 @@ The following keys are defined:
> > > >    the performance of misaligned scalar word accesses on the selected set of
> > > >    processors.
> > > >
> > > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
> > > > -    accesses is unknown.
> > > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN`: The performance of
> > > > +    misaligned accesses is unknown.
> > > >
> > > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
> > > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED`: Misaligned accesses are
> > > >      emulated via software, either in or below the kernel.  These accesses are
> > > >      always extremely slow.
> > > >
> > > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned word accesses are
> > > > -    slower than equivalent byte accesses.  Misaligned accesses may be supported
> > > > -    directly in hardware, or trapped and emulated by software.
> > > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned word accesses
> > > > +    are slower than equivalent byte accesses.  Misaligned accesses may be
> > > > +    supported directly in hardware, or trapped and emulated by software.
> > > >
> > > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned word accesses are
> > > > -    faster than equivalent byte accesses.
> > > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned word accesses
> > > > +    are faster than equivalent byte accesses.
> > > >
> > > > -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned accesses are
> > > > -    not supported at all and will generate a misaligned address fault.
> > > > +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED`: Misaligned accesses
> > > > +    are not supported at all and will generate a misaligned address fault.
> > > >
> > > >  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
> > > >    represents the size of the Zicboz block in bytes.
> > > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > > > index 22073533cea8..e11684d8ae1c 100644
> > > > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > > > @@ -66,11 +66,11 @@ struct riscv_hwprobe {
> > > >  #define              RISCV_HWPROBE_EXT_ZVE64F        (1ULL << 40)
> > > >  #define              RISCV_HWPROBE_EXT_ZVE64D        (1ULL << 41)
> > > >  #define RISCV_HWPROBE_KEY_CPUPERF_0  5
> > > > -#define              RISCV_HWPROBE_MISALIGNED_UNKNOWN        0
> > > > -#define              RISCV_HWPROBE_MISALIGNED_EMULATED       1
> > > > -#define              RISCV_HWPROBE_MISALIGNED_SLOW           2
> > > > -#define              RISCV_HWPROBE_MISALIGNED_FAST           3
> > > > -#define              RISCV_HWPROBE_MISALIGNED_UNSUPPORTED    4
> > > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNKNOWN         0
> > > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_EMULATED        1
> > > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW            2
> > > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_FAST            3
> > > > +#define              RISCV_HWPROBE_MISALIGNED_SCALAR_UNSUPPORTED     4
> > > >  #define              RISCV_HWPROBE_MISALIGNED_MASK           7
> > >
> > > How come the "old" names do not need to be preserved for userspace?
> >
> > It depends on what exactly the big userspace compatibility rule is.
> > This preserves binary compatibility, which I think is the big one, but
> > breaks source compatibility, though with an easy translation to fix.
> > We could keep the old names around, but then it seems sort of silly to
> > introduce the new names. I introduced this patch upon request, so I
> > don't personally have a horse in the race on this one.
>
> So apparently this isn't quite Palmer actually wanted. In today's call
> he suggested that he'd send a new version himself, but also that what we
> should do define a new key for scalar /and/ new add new defines values that
> contain the word scalar, retaining the old defines. The values can of
> course be the same.

Ahh, got it. I'm happy to spin this.
-Evan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key
  2024-06-26 14:36   ` Conor Dooley
@ 2024-06-26 15:55     ` Evan Green
  0 siblings, 0 replies; 10+ messages in thread
From: Evan Green @ 2024-06-26 15:55 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Palmer Dabbelt, Yangyu Chen, Charlie Jenkins, Andrew Jones,
	Albert Ou, Andy Chiu, Clément Léger, Conor Dooley,
	Costa Shulyupin, Jonathan Corbet, Paul Walmsley, linux-doc,
	linux-kernel, linux-riscv

On Wed, Jun 26, 2024 at 7:36 AM Conor Dooley <conor@kernel.org> wrote:
>
> On Tue, Jun 25, 2024 at 09:51:20AM -0700, Evan Green wrote:
> > RISCV_HWPROBE_KEY_CPUPERF_0 was mistakenly flagged as a bitmask in
> > hwprobe_key_is_bitmask(), when in reality it was an enum value. This
> > causes problems when used in conjunction with RISCV_HWPROBE_WHICH_CPUS,
> > since SLOW, FAST, and EMULATED have values whose bits overlap with
> > each other. If the caller asked for the set of CPUs that was SLOW or
> > EMULATED, the returned set would also include CPUs that were FAST.
> >
> > Introduce a new hwprobe key, RISCV_HWPROBE_KEY_MISALIGNED_PERF, which
> > returns the same values in response to a direct query (with no flags),
> > but is properly handled as an enumerated value. As a result, SLOW,
> > FAST, and EMULATED are all correctly treated as distinct values under
> > the new key when queried with the WHICH_CPUS flag.
> >
> > Leave the old key in place to avoid disturbing applications which may
> > have already come to rely on the key, with or without its broken
> > behavior with respect to the WHICH_CPUS flag.
> >
> > Fixes: e178bf146e4b ("RISC-V: hwprobe: Introduce which-cpus flag")
> > Signed-off-by: Evan Green <evan@rivosinc.com>
> > Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> >
> > ---
> >
> > Changes in v2:
> >  - Clarified the distinction of slow and fast refers to misaligned word
> >    accesses. Previously it just said misaligned accesses, leaving it
> >    ambiguous as to which type of access was measured.
>
> I think if we are gonna be specific, we should be exactly specific as to
> what we have tested and say 32-bit if that's what we're probing/testing
> with. That'd be consistent with jesse's proposed wording for vector.

Sure. In this case it's really native word sized accesses. So something like:

* :c:macro:`RISCV_HWPROBE_KEY_MISALIGNED_SCALAR_PERF`: An enum value describing
the performance of misaligned scalar native word accesses on the selected set
of processors.

...

* :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_SLOW`: Misaligned native word
sized accesses are slower than the equivalent quantity of byte accesses.
Misaligned accesses may be supported directly in hardware, or trapped and
emulated by software.

* :c:macro:`RISCV_HWPROBE_MISALIGNED_SCALAR_FAST`: Misaligned native word
sized accesses are faster than the equivalent quantity of byte accesses.

I'm planning to leave the qualifiers off of UNKNOWN, EMULATED, and
UNSUPPORTED, as those likely apply to misaligned accesses of any size.
Let me know if you think we should tweak it further.
-Evan

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-06-26 15:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-25 16:51 [PATCH v2 0/2] RISC-V: hwprobe: Misaligned scalar perf fix and rename Evan Green
2024-06-25 16:51 ` [PATCH v2 1/2] RISC-V: hwprobe: Add MISALIGNED_PERF key Evan Green
2024-06-26 14:36   ` Conor Dooley
2024-06-26 15:55     ` Evan Green
2024-06-25 16:51 ` [PATCH v2 2/2] RISC-V: hwprobe: Add SCALAR to misaligned perf defines Evan Green
2024-06-25 18:33   ` Charlie Jenkins
2024-06-25 18:35   ` Conor Dooley
2024-06-25 19:08     ` Evan Green
2024-06-26 14:34       ` Conor Dooley
2024-06-26 15:44         ` Evan Green

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).