Received-SPF: pass (google.com: domain of linux-kernel+bounces-185331-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <tencent_9D721BDDF88C04DBB5151D57711D62524209@qq.com> <tencent_338DF690631BAE788C4CC858233E9FBAE006@qq.com>
In-Reply-To: <tencent_338DF690631BAE788C4CC858233E9FBAE006@qq.com>
From: Evan Green <evan@rivosinc.com>
Date: Tue, 21 May 2024 11:36:06 -0700
Message-ID: <CALs-HssGcNso6vTfbcsiWX1h_46jgDDRcEWcfZCTpxXYnubcng@mail.gmail.com>
Subject: Re: [PATCH 2/2] docs: riscv: hwprobe: Clarify misaligned keys are
 values not bitmasks
To: Yangyu Chen <cyy@cyyself.name>
Cc: linux-riscv@lists.infradead.org, Elliott Hughes <enh@google.com>, 
	Charlie Jenkins <charlie@rivosinc.com>, Jonathan Corbet <corbet@lwn.net>, 
	Paul Walmsley <paul.walmsley@sifive.com>, Palmer Dabbelt <palmer@dabbelt.com>, 
	=?UTF-8?B?Q2zDqW1lbnQgTMOpZ2Vy?= <cleger@rivosinc.com>, 
	Conor Dooley <conor.dooley@microchip.com>, Andrew Jones <ajones@ventanamicro.com>, 
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Sat, May 18, 2024 at 9:00=E2=80=AFAM Yangyu Chen <cyy@cyyself.name> wrot=
e:
>
> 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>

Hm, we also have this problem in the code, since
hwprobe_key_is_bitmask() returns true for KEY_CPUPERF_0. This results
in wrong information being returned for queries using the WHICH_CPU
flag. If usermode asked for the set of CPUs that was specifically SLOW
or EMULATED, the returned cpuset would also include cpus that were
FAST. I believe all other queries are okay.

The one-liner fix is to just not return true for that key in
hwprobe_key_is_bitmask(). But that's technically user-visible: if some
software relied on the buggy behavior of FAST cpus being swept up in
the query for SLOW or EMULATED cpus, this change would expose that.
The grownups-eat-their-vegetables thing to do would be to define a new
key that returns this same value, but doesn't return true in
hwprobe_key_is_bitmask(). What do people think?

-Evan

> ---
>  Documentation/arch/riscv/hwprobe.rst | 31 ++++++++++++++++------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/arch/riscv/hwprobe.rst b/Documentation/arch/ri=
scv/hwprobe.rst
> index 239be63f5089..4abfa3f9fe44 100644
> --- a/Documentation/arch/riscv/hwprobe.rst
> +++ b/Documentation/arch/riscv/hwprobe.rst
> @@ -188,25 +188,30 @@ The following keys are defined:
>         manual starting from commit 95cf1f9 ("Add changes requested by Ve=
d
>         during signoff")
>
> -* :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains perfor=
mance
> +* :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A value that contains performa=
nce
>    information about the selected set of processors.
>
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of misa=
ligned
> -    scalar accesses is unknown.
> +  * :c:macro:`RISCV_HWPROBE_MISALIGNED_MASK`: The bitmask of the misalig=
ned
> +    access performance field in the value of key `RISCV_HWPROBE_KEY_CPUP=
ERF_0`.
>
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_EMULATED`: Misaligned scalar acce=
sses are
> -    emulated via software, either in or below the kernel.  These accesse=
s 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 su=
pported
> -    directly in hardware, or trapped and emulated by software.
> +    * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNKNOWN`: The performance of mi=
saligned
> +      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 ac=
cesses are
> +      emulated via software, either in or below the kernel.  These acces=
ses are
> +      always extremely slow.
>
> -  * :c:macro:`RISCV_HWPROBE_MISALIGNED_UNSUPPORTED`: Misaligned scalar a=
ccesses
> -    are not supported at all and will generate a misaligned address faul=
t.
> +    * :c:macro:`RISCV_HWPROBE_MISALIGNED_SLOW`: Misaligned scalar access=
es 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 access=
es 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 fa=
ult.
>
>  * :c:macro:`RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE`: An unsigned int which
>    represents the size of the Zicboz block in bytes.
> --
> 2.43.0
>