[PATCH v2 0/3] docs: riscv: Some clarifies on hwprobe misaligned performance

[PATCH v2 0/3] docs: riscv: Some clarifies on hwprobe misaligned performance

[Yangyu Chen]

This patchset clarifies some unclear things about hwprobe's misaligned
performance. Including:

- hwprobe misaligned performance is only applied to scalar from patch [1]
- The defined keys of RISCV_HWPROBE_MISALIGNED_* are values not bitmasks

I cherry-picked [1] rather than write dependency because the original patch
was submitted with lines wrapped to 80 characters. We can't directly apply
that patch using `git am` .

Changes in v2:
- Add fixes to the code in arch/riscv/and include/asm/hwprobe.h

v1: https://lore.kernel.org/linux-riscv/tencent_9D721BDDF88C04DBB5151D57711D62524209@qq.com/

[1] https://lore.kernel.org/linux-riscv/CAJgzZorn5anPH8dVPqvjVWmLKqTi5bkLDR=FH-ZAcdXFnNe8Eg@mail.gmail.com/

Yangyu Chen (2):
  docs: riscv: hwprobe: Clarify misaligned keys are values not bitmasks
  RISC-V: hwprobe: not treat KEY_CPUPERF_0 as bitmask

enh (1):
  docs: riscv: Clarify risc-v hwprobe RISCV_HWPROBE_MISALIGNED_* docs.

 Documentation/arch/riscv/hwprobe.rst | 31 ++++++++++++++++------------
 arch/riscv/include/asm/hwprobe.h     |  1 -
 2 files changed, 18 insertions(+), 14 deletions(-)

-- 
2.45.1

[PATCH v2 1/3] docs: riscv: Clarify risc-v hwprobe RISCV_HWPROBE_MISALIGNED_* docs.

[Yangyu Chen]

From: enh <enh@google.com>

These only tell you about scalar accesses, not vector accesses.

Signed-off-by: Elliott Hughes <enh@google.com>
Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 Documentation/arch/riscv/hwprobe.rst | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index 204cd4433af5..d720712e9734 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -196,21 +196,21 @@ The following keys are defined:
   information about the selected set of processors.
 
   * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
-    accesses is unknown.
+    scalar accesses is unknown.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned accesses are
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned scalar accesses are
     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 scalar 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 scalar 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_UNSUPPORTED`: Misaligned scalar 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.
-- 
2.45.1

[PATCH v2 2/3] docs: riscv: hwprobe: Clarify misaligned keys are values not bitmasks

[Yangyu Chen]

The original documentation says hwprobe keys are bitmasks, but actually,
they are values. This patch clarifies this to avoid confusion.

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 Documentation/arch/riscv/hwprobe.rst | 31 ++++++++++++++++------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/riscv/hwprobe.rst
index d720712e9734..2e212956185d 100644
--- a/Documentation/arch/riscv/hwprobe.rst
+++ b/Documentation/arch/riscv/hwprobe.rst
@@ -192,25 +192,30 @@ The following keys are defined:
        supported as defined in the RISC-V ISA manual starting from commit
        d8ab5c78c207 ("Zihintpause is ratified").
 
-* :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
+* :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A value that contains performance
   information about the selected set of processors.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misaligned
-    scalar accesses is unknown.
+  * :c:macro:`RISCV_HWPROBE_MISALIGNED_MASK`: The bitmask of the misaligned
+    access performance field in the value of key `RISCV_HWPROBE_KEY_CPUPERF_0`.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned scalar accesses are
-    emulated via software, either in or below the kernel.  These accesses are
-    always extremely slow.
+    The following values (not bitmasks) in this field are defined:
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned scalar 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_UNKNOWN`: The performance of misaligned
+      scalar accesses is unknown.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_FAST`: Misaligned scalar accesses are
-    faster than equivalent byte accesses.
+    * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned scalar accesses are
+      emulated via software, either in or below the kernel.  These accesses are
+      always extremely slow.
 
-  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned scalar accesses
-    are not supported at all and will generate a misaligned address fault.
+    * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned scalar 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 scalar accesses are
+      faster than equivalent byte accesses.
+
+    * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned scalar 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.
-- 
2.45.1

[PATCH v2 3/3] RISC-V: hwprobe: not treat KEY_CPUPERF_0 as bitmask

[Yangyu Chen]

Since the value in KEY_CPUPERF_0 is not bitmask, remove the wrong code
in hwprobe.h.

Signed-off-by: Yangyu Chen <cyy@cyyself.name>
---
 arch/riscv/include/asm/hwprobe.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 630507dff5ea..f24cad22bbe1 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -20,7 +20,6 @@ static inline bool hwprobe_key_is_bitmask(__s64 key)
 	switch (key) {
 	case RISCV_HWPROBE_KEY_BASE_BEHAVIOR:
 	case RISCV_HWPROBE_KEY_IMA_EXT_0:
-	case RISCV_HWPROBE_KEY_CPUPERF_0:
 		return true;
 	}
 
-- 
2.45.1

Re: [PATCH v2 3/3] RISC-V: hwprobe: not treat KEY_CPUPERF_0 as bitmask

[Evan Green]

On Thu, May 23, 2024 at 8:36 PM Yangyu Chen <cyy@cyyself.name> wrote:
>
> Since the value in KEY_CPUPERF_0 is not bitmask, remove the wrong code
> in hwprobe.h.
>
> Signed-off-by: Yangyu Chen <cyy@cyyself.name>

I'd expect a Fixes tag, and ideally some discussion on the reasoning
and ramifications of this change.

I posted the other possible fix, declaring a new key, at [1], mostly
so we could see the two options and discuss. I'm okay with either
patch.
-Evan

[1] https://lore.kernel.org/lkml/20240529182649.2635123-1-evan@rivosinc.com/T/#u

> ---
>  arch/riscv/include/asm/hwprobe.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> index 630507dff5ea..f24cad22bbe1 100644
> --- a/arch/riscv/include/asm/hwprobe.h
> +++ b/arch/riscv/include/asm/hwprobe.h
> @@ -20,7 +20,6 @@ static inline bool hwprobe_key_is_bitmask(__s64 key)
>         switch (key) {
>         case RISCV_HWPROBE_KEY_BASE_BEHAVIOR:
>         case RISCV_HWPROBE_KEY_IMA_EXT_0:
> -       case RISCV_HWPROBE_KEY_CPUPERF_0:
>                 return true;
>         }
>
> --
> 2.45.1
>

Re: [PATCH v2 3/3] RISC-V: hwprobe: not treat KEY_CPUPERF_0 as bitmask

[Palmer Dabbelt]

On Wed, 29 May 2024 11:33:42 PDT (-0700), Evan Green wrote:
> On Thu, May 23, 2024 at 8:36 PM Yangyu Chen <cyy@cyyself.name> wrote:
>>
>> Since the value in KEY_CPUPERF_0 is not bitmask, remove the wrong code
>> in hwprobe.h.
>>
>> Signed-off-by: Yangyu Chen <cyy@cyyself.name>
>
> I'd expect a Fixes tag, and ideally some discussion on the reasoning
> and ramifications of this change.
>
> I posted the other possible fix, declaring a new key, at [1], mostly
> so we could see the two options and discuss. I'm okay with either
> patch.

Just to close the loop here as the discussions are on other threads: 
after a bunch of discussions we're going with the new key version.  
Maybe it's a bit pedantic, but since hwprobe is such a fundamental 
compatibility interface we're just going to be super careful.

> -Evan
>
> [1] https://lore.kernel.org/lkml/20240529182649.2635123-1-evan@rivosinc.com/T/#u
>
>> ---
>>  arch/riscv/include/asm/hwprobe.h | 1 -
>>  1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/riscv/include/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
>> index 630507dff5ea..f24cad22bbe1 100644
>> --- a/arch/riscv/include/asm/hwprobe.h
>> +++ b/arch/riscv/include/asm/hwprobe.h
>> @@ -20,7 +20,6 @@ static inline bool hwprobe_key_is_bitmask(__s64 key)
>>         switch (key) {
>>         case RISCV_HWPROBE_KEY_BASE_BEHAVIOR:
>>         case RISCV_HWPROBE_KEY_IMA_EXT_0:
>> -       case RISCV_HWPROBE_KEY_CPUPERF_0:
>>                 return true;
>>         }
>>
>> --
>> 2.45.1
>>