| Message ID | 20240322134728.151255-2-ajones@ventanamicro.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f6b56df883d426d7a49c2b039a24c34306c72824 |
| Headers | show |
| Series | RISC-V: selftests: cbo: Ensure asm operands match constraints, take 2 | expand |
On Fri, Mar 22, 2024 at 02:47:28PM +0100, Andrew Jones wrote: > Commit 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands > match constraints") attempted to ensure MK_CBO() would always > provide to a compile-time constant when given a constant, but > cpu_to_le32() isn't necessarily going to do that. Switch to manually > shifting the bytes, when needed, to finally get this right. > > Reported-by: Woodrow Shen <woodrow.shen@sifive.com> > Closes: https://lore.kernel.org/all/CABquHATcBTUwfLpd9sPObBgNobqQKEAZ2yxk+TWSpyO5xvpXpg@mail.gmail.com/ > Fixes: a29e2a48afe3 ("RISC-V: selftests: Add CBO tests") > Fixes: 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands match constraints") > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> This has been sitting in patchwork for a while and I don't see a message from either the reporter saying this was a good fix or from the selftests maintainer that it has been picked up. I was gonna mark it "handled elsewhere" but I noticed that this was only sent to the riscv maintainers so we should probably harass Palmer today to pick this up?
On Wed, Apr 24, 2024 at 10:41:10AM +0100, Conor Dooley wrote: > On Fri, Mar 22, 2024 at 02:47:28PM +0100, Andrew Jones wrote: > > Commit 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands > > match constraints") attempted to ensure MK_CBO() would always > > provide to a compile-time constant when given a constant, but > > cpu_to_le32() isn't necessarily going to do that. Switch to manually > > shifting the bytes, when needed, to finally get this right. > > > > Reported-by: Woodrow Shen <woodrow.shen@sifive.com> > > Closes: https://lore.kernel.org/all/CABquHATcBTUwfLpd9sPObBgNobqQKEAZ2yxk+TWSpyO5xvpXpg@mail.gmail.com/ > > Fixes: a29e2a48afe3 ("RISC-V: selftests: Add CBO tests") > > Fixes: 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands match constraints") > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > This has been sitting in patchwork for a while and I don't see a message > from either the reporter saying this was a good fix or from the selftests > maintainer that it has been picked up. I was gonna mark it "handled > elsewhere" but I noticed that this was only sent to the riscv > maintainers so we should probably harass Palmer today to pick this up? > Yup. Thanks for the reminder to follow up! drew
Hi Andrew and Conor, I was missing this thread and just now this patch was verified by my local, thank you for reminding me. Cheers, On Wed, Apr 24, 2024 at 6:33 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Wed, Apr 24, 2024 at 10:41:10AM +0100, Conor Dooley wrote: > > On Fri, Mar 22, 2024 at 02:47:28PM +0100, Andrew Jones wrote: > > > Commit 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands > > > match constraints") attempted to ensure MK_CBO() would always > > > provide to a compile-time constant when given a constant, but > > > cpu_to_le32() isn't necessarily going to do that. Switch to manually > > > shifting the bytes, when needed, to finally get this right. > > > > > > Reported-by: Woodrow Shen <woodrow.shen@sifive.com> > > > Closes: https://lore.kernel.org/all/CABquHATcBTUwfLpd9sPObBgNobqQKEAZ2yxk+TWSpyO5xvpXpg@mail.gmail.com/ > > > Fixes: a29e2a48afe3 ("RISC-V: selftests: Add CBO tests") > > > Fixes: 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands match constraints") > > > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > > > This has been sitting in patchwork for a while and I don't see a message > > from either the reporter saying this was a good fix or from the selftests > > maintainer that it has been picked up. I was gonna mark it "handled > > elsewhere" but I noticed that this was only sent to the riscv > > maintainers so we should probably harass Palmer today to pick this up? > > > > Yup. Thanks for the reminder to follow up! > > drew
Hello: This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt <palmer@rivosinc.com>: On Fri, 22 Mar 2024 14:47:28 +0100 you wrote: > Commit 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands > match constraints") attempted to ensure MK_CBO() would always > provide to a compile-time constant when given a constant, but > cpu_to_le32() isn't necessarily going to do that. Switch to manually > shifting the bytes, when needed, to finally get this right. > > Reported-by: Woodrow Shen <woodrow.shen@sifive.com> > Closes: https://lore.kernel.org/all/CABquHATcBTUwfLpd9sPObBgNobqQKEAZ2yxk+TWSpyO5xvpXpg@mail.gmail.com/ > Fixes: a29e2a48afe3 ("RISC-V: selftests: Add CBO tests") > Fixes: 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands match constraints") > Signed-off-by: Andrew Jones <ajones@ventanamicro.com> > > [...] Here is the summary with links: - RISC-V: selftests: cbo: Ensure asm operands match constraints, take 2 https://git.kernel.org/riscv/c/f6b56df883d4 You are awesome, thank you!
diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c index c537d52fafc5..a40541bb7c7d 100644 --- a/tools/testing/selftests/riscv/hwprobe/cbo.c +++ b/tools/testing/selftests/riscv/hwprobe/cbo.c @@ -19,7 +19,7 @@ #include "hwprobe.h" #include "../../kselftest.h" -#define MK_CBO(fn) cpu_to_le32((fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15) +#define MK_CBO(fn) le32_bswap((uint32_t)(fn) << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15) static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 }; diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.h b/tools/testing/selftests/riscv/hwprobe/hwprobe.h index e3fccb390c4d..f3de970c3222 100644 --- a/tools/testing/selftests/riscv/hwprobe/hwprobe.h +++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.h @@ -4,6 +4,16 @@ #include <stddef.h> #include <asm/hwprobe.h> +#if __BYTE_ORDER == __BIG_ENDIAN +# define le32_bswap(_x) \ + ((((_x) & 0x000000ffU) << 24) | \ + (((_x) & 0x0000ff00U) << 8) | \ + (((_x) & 0x00ff0000U) >> 8) | \ + (((_x) & 0xff000000U) >> 24)) +#else +# define le32_bswap(_x) (_x) +#endif + /* * Rather than relying on having a new enough libc to define this, just do it * ourselves. This way we don't need to be coupled to a new-enough libc to
Commit 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands match constraints") attempted to ensure MK_CBO() would always provide to a compile-time constant when given a constant, but cpu_to_le32() isn't necessarily going to do that. Switch to manually shifting the bytes, when needed, to finally get this right. Reported-by: Woodrow Shen <woodrow.shen@sifive.com> Closes: https://lore.kernel.org/all/CABquHATcBTUwfLpd9sPObBgNobqQKEAZ2yxk+TWSpyO5xvpXpg@mail.gmail.com/ Fixes: a29e2a48afe3 ("RISC-V: selftests: Add CBO tests") Fixes: 0de65288d75f ("RISC-V: selftests: cbo: Ensure asm operands match constraints") Signed-off-by: Andrew Jones <ajones@ventanamicro.com> --- tools/testing/selftests/riscv/hwprobe/cbo.c | 2 +- tools/testing/selftests/riscv/hwprobe/hwprobe.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-)