[PATCH 0/6] RISC-V: Enable cbo.zero in usermode

[PATCH 0/6] RISC-V: Enable cbo.zero in usermode

[Andrew Jones]

In order for usermode to issue cbo.zero, it needs privilege granted to
issue the extension instruction (patch 2) and to know that the extension
is available and its block size (patch 3). Patch 1 could be separate from
this series (it just fixes up some error messages), patches 4-5 convert
the hwprobe selftest to a statically-linked, TAP test and patch 6 adds a
new hwprobe test for the new information as well as testing CBO
instructions can or cannot be issued as appropriate.

Thanks,
drew


Andrew Jones (6):
  RISC-V: Make zicbom/zicboz errors consistent
  RISC-V: Enable cbo.zero in usermode
  RISC-V: hwprobe: Expose Zicboz extension and its block size
  RISC-V: selftests: Statically link hwprobe test
  RISC-V: selftests: Convert hwprobe test to kselftest API
  RISC-V: selftests: Add CBO tests

 Documentation/riscv/hwprobe.rst               |   6 +
 arch/riscv/include/asm/cpufeature.h           |   2 +
 arch/riscv/include/asm/csr.h                  |   1 +
 arch/riscv/include/asm/hwcap.h                |  16 ++
 arch/riscv/include/asm/hwprobe.h              |   2 +-
 arch/riscv/include/uapi/asm/hwprobe.h         |   2 +
 arch/riscv/kernel/cpufeature.c                |  10 +-
 arch/riscv/kernel/setup.c                     |   4 +
 arch/riscv/kernel/smpboot.c                   |   4 +
 arch/riscv/kernel/sys_riscv.c                 |  41 +++--
 tools/testing/selftests/riscv/Makefile        |   2 +-
 .../testing/selftests/riscv/hwprobe/Makefile  |   7 +-
 tools/testing/selftests/riscv/hwprobe/cbo.c   | 160 ++++++++++++++++++
 .../testing/selftests/riscv/hwprobe/hwprobe.c |  64 +++----
 .../testing/selftests/riscv/hwprobe/hwprobe.h |  15 ++
 15 files changed, 272 insertions(+), 64 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/hwprobe/cbo.c
 create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.h

-- 
2.41.0


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

[PATCH 1/6] RISC-V: Make zicbom/zicboz errors consistent

[Andrew Jones]

commit c818fea83de4 ("riscv: say disabling zicbom if no or bad
riscv,cbom-block-size found") improved the error messages for
zicbom but zicboz was missed since its patches were in flight
at the same time. Get 'em now.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/kernel/cpufeature.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index a8f66c015229..31843e9cc80c 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -87,10 +87,10 @@ static bool riscv_isa_extension_check(int id)
 		return true;
 	case RISCV_ISA_EXT_ZICBOZ:
 		if (!riscv_cboz_block_size) {
-			pr_err("Zicboz detected in ISA string, but no cboz-block-size found\n");
+			pr_err("Zicboz detected in ISA string, disabling as no cboz-block-size found\n");
 			return false;
 		} else if (!is_power_of_2(riscv_cboz_block_size)) {
-			pr_err("cboz-block-size present, but is not a power-of-2\n");
+			pr_err("Zicboz disabled as cboz-block-size present, but is not a power-of-2\n");
 			return false;
 		}
 		return true;
-- 
2.41.0


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

Re: [PATCH 1/6] RISC-V: Make zicbom/zicboz errors consistent

[Conor Dooley]

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

On Wed, Aug 09, 2023 at 01:55:18PM +0200, Andrew Jones wrote:
> commit c818fea83de4 ("riscv: say disabling zicbom if no or bad
> riscv,cbom-block-size found") improved the error messages for
> zicbom but zicboz was missed since its patches were in flight
> at the same time. Get 'em now.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

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

[PATCH 2/6] RISC-V: Enable cbo.zero in usermode

[Andrew Jones]

When Zicboz is present, enable its instruction (cbo.zero) in
usermode by setting its respective senvcfg bit. We don't bother
trying to set this bit per-task, which would also require an
interface for tasks to request enabling and/or disabling. Instead,
permanently set the bit for each hart which has the extension when
bringing it online.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/asm/cpufeature.h |  2 ++
 arch/riscv/include/asm/csr.h        |  1 +
 arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
 arch/riscv/kernel/cpufeature.c      |  6 ++++++
 arch/riscv/kernel/setup.c           |  4 ++++
 arch/riscv/kernel/smpboot.c         |  4 ++++
 6 files changed, 33 insertions(+)

diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
index 23fed53b8815..788fd575c21a 100644
--- a/arch/riscv/include/asm/cpufeature.h
+++ b/arch/riscv/include/asm/cpufeature.h
@@ -30,4 +30,6 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
 /* Per-cpu ISA extensions. */
 extern struct riscv_isainfo hart_isa[NR_CPUS];
 
+void riscv_user_isa_enable(void);
+
 #endif
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 7bac43a3176e..e187e76e3df4 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -273,6 +273,7 @@
 #define CSR_SIE			0x104
 #define CSR_STVEC		0x105
 #define CSR_SCOUNTEREN		0x106
+#define CSR_SENVCFG		0x10a
 #define CSR_SSCRATCH		0x140
 #define CSR_SEPC		0x141
 #define CSR_SCAUSE		0x142
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index f041bfa7f6a0..4929faecb75f 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -66,6 +66,7 @@
 #ifndef __ASSEMBLY__
 
 #include <linux/jump_label.h>
+#include <asm/cpufeature.h>
 
 unsigned long riscv_get_elf_hwcap(void);
 
@@ -130,6 +131,21 @@ riscv_has_extension_unlikely(const unsigned long ext)
 	return true;
 }
 
+static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
+{
+	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
+		return true;
+
+	return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
+}
+
+static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
+{
+	if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
+		return true;
+
+	return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
+}
 #endif
 
 #endif /* _ASM_RISCV_HWCAP_H */
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 31843e9cc80c..fc0bf300acc7 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -391,6 +391,12 @@ unsigned long riscv_get_elf_hwcap(void)
 	return hwcap;
 }
 
+void riscv_user_isa_enable(void)
+{
+	if (riscv_this_cpu_has_extension_unlikely(RISCV_ISA_EXT_ZICBOZ))
+		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
+}
+
 #ifdef CONFIG_RISCV_ALTERNATIVE
 /*
  * Alternative patch sites consider 48 bits when determining when to patch
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 971fe776e2f8..2f053f0763a1 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -25,6 +25,7 @@
 #include <asm/acpi.h>
 #include <asm/alternative.h>
 #include <asm/cacheflush.h>
+#include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
 #include <asm/early_ioremap.h>
 #include <asm/pgtable.h>
@@ -308,9 +309,12 @@ void __init setup_arch(char **cmdline_p)
 	riscv_fill_hwcap();
 	init_rt_signal_env();
 	apply_boot_alternatives();
+
 	if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
 	    riscv_isa_extension_available(NULL, ZICBOM))
 		riscv_noncoherent_supported();
+
+	riscv_user_isa_enable();
 }
 
 static int __init topology_init(void)
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index f4d6acb38dd0..502b04abda0b 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -25,6 +25,8 @@
 #include <linux/of.h>
 #include <linux/sched/task_stack.h>
 #include <linux/sched/mm.h>
+
+#include <asm/cpufeature.h>
 #include <asm/cpu_ops.h>
 #include <asm/irq.h>
 #include <asm/mmu_context.h>
@@ -252,6 +254,8 @@ asmlinkage __visible void smp_callin(void)
 			elf_hwcap &= ~COMPAT_HWCAP_ISA_V;
 	}
 
+	riscv_user_isa_enable();
+
 	/*
 	 * Remote TLB flushes are ignored while the CPU is offline, so emit
 	 * a local TLB flush right now just in case.
-- 
2.41.0


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

Re: [PATCH 2/6] RISC-V: Enable cbo.zero in usermode

[Evan Green]

On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> When Zicboz is present, enable its instruction (cbo.zero) in
> usermode by setting its respective senvcfg bit. We don't bother
> trying to set this bit per-task, which would also require an
> interface for tasks to request enabling and/or disabling. Instead,
> permanently set the bit for each hart which has the extension when
> bringing it online.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/include/asm/cpufeature.h |  2 ++
>  arch/riscv/include/asm/csr.h        |  1 +
>  arch/riscv/include/asm/hwcap.h      | 16 ++++++++++++++++
>  arch/riscv/kernel/cpufeature.c      |  6 ++++++
>  arch/riscv/kernel/setup.c           |  4 ++++
>  arch/riscv/kernel/smpboot.c         |  4 ++++
>  6 files changed, 33 insertions(+)
>
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 23fed53b8815..788fd575c21a 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -30,4 +30,6 @@ DECLARE_PER_CPU(long, misaligned_access_speed);
>  /* Per-cpu ISA extensions. */
>  extern struct riscv_isainfo hart_isa[NR_CPUS];
>
> +void riscv_user_isa_enable(void);
> +
>  #endif
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 7bac43a3176e..e187e76e3df4 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -273,6 +273,7 @@
>  #define CSR_SIE                        0x104
>  #define CSR_STVEC              0x105
>  #define CSR_SCOUNTEREN         0x106
> +#define CSR_SENVCFG            0x10a
>  #define CSR_SSCRATCH           0x140
>  #define CSR_SEPC               0x141
>  #define CSR_SCAUSE             0x142
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index f041bfa7f6a0..4929faecb75f 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -66,6 +66,7 @@
>  #ifndef __ASSEMBLY__
>
>  #include <linux/jump_label.h>
> +#include <asm/cpufeature.h>
>
>  unsigned long riscv_get_elf_hwcap(void);
>
> @@ -130,6 +131,21 @@ riscv_has_extension_unlikely(const unsigned long ext)
>         return true;
>  }
>
> +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> +{
> +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> +               return true;
> +
> +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> +}
> +
> +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> +{
> +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> +               return true;
> +
> +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> +}

Another way to do this would be to add a parameter to
riscv_has_extension_*() (as there are very few users), then these new
functions can turn around and call those with the new parameter set to
hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
only advantage to it I can argue is it keeps the code flows more
unified.

-Evan

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

Re: [PATCH 2/6] RISC-V: Enable cbo.zero in usermode

[Andrew Jones]

On Wed, Aug 09, 2023 at 09:00:35AM -0700, Evan Green wrote:
> On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
...
> > +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> > +{
> > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > +               return true;
> > +
> > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > +}
> > +
> > +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> > +{
> > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > +               return true;
> > +
> > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > +}
> 
> Another way to do this would be to add a parameter to
> riscv_has_extension_*() (as there are very few users), then these new
> functions can turn around and call those with the new parameter set to
> hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
> only advantage to it I can argue is it keeps the code flows more
> unified.
>

I like unification, but I think I'd prefer we create wrappers and
try to avoid callers needing to construct hart_isa[].isa parameters
themselves. I'm also not a big fan of the NULL parameter needed when
riscv_isa_extension_available() is invoked for the riscv_isa bitmap.
So we need:

  1. check if an extension is in riscv_isa
  2. check if an extension is in a bitmap provided by the caller
  3. check if an extension is in this cpu's isa bitmap
  4. check if an extension is in the isa bitmap of a cpu provided by the
     caller

The only one we can optimize with alternatives is (1), so it definitely
gets wrappers (riscv_has_extension_likely/unlikely()). (3) and (4) can
also get wrappers which first try the optimized (1), like I have above.
Actually (3)'s wrapper could be based on (4)'s, or only provide wrappers
for (4)

 static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
 {
     if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
         return true;

     return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
 }

 static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
 {
     if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
         return true;

     return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
 }

and then use smp_processor_id() directly in the callers that need
to check this_cpu's extensions.

For case (2), I'd advocate we rename __riscv_isa_extension_available() to
riscv_has_extension() and drop the riscv_isa_extension_available() macro
in order to avoid having some calls with RISCV_ISA_EXT_* spelled out and
others that rely on the pasting. And, ideally, we'd convert most
riscv_has_extension(NULL, ext) calls to riscv_has_extension_[un]likely().

Thanks,
drew

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

Re: [PATCH 2/6] RISC-V: Enable cbo.zero in usermode

[Conor Dooley]

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

On Wed, Aug 09, 2023 at 06:58:15PM +0200, Andrew Jones wrote:
> On Wed, Aug 09, 2023 at 09:00:35AM -0700, Evan Green wrote:
> > On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> ...
> > > +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> > > +{
> > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > > +               return true;
> > > +
> > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > +}
> > > +
> > > +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> > > +{
> > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > > +               return true;
> > > +
> > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > +}
> > 
> > Another way to do this would be to add a parameter to
> > riscv_has_extension_*() (as there are very few users), then these new
> > functions can turn around and call those with the new parameter set to
> > hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
> > only advantage to it I can argue is it keeps the code flows more
> > unified.
> >
> 
> I like unification, but I think I'd prefer we create wrappers and
> try to avoid callers needing to construct hart_isa[].isa parameters
> themselves. I'm also not a big fan of the NULL parameter needed when
> riscv_isa_extension_available() is invoked for the riscv_isa bitmap.
> So we need:
> 
>   1. check if an extension is in riscv_isa
>   2. check if an extension is in a bitmap provided by the caller
>   3. check if an extension is in this cpu's isa bitmap
>   4. check if an extension is in the isa bitmap of a cpu provided by the
>      caller
> 
> The only one we can optimize with alternatives is (1), so it definitely
> gets wrappers (riscv_has_extension_likely/unlikely()). (3) and (4) can
> also get wrappers which first try the optimized (1), like I have above.
> Actually (3)'s wrapper could be based on (4)'s, or only provide wrappers
> for (4)
> 
>  static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
>  {
>      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
>          return true;
> 
>      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
>  }
> 
>  static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
>  {
>      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))

Why are you gating on CONFIG_RISCV_ALTERNATIVE here?

>          return true;
> 
>      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
>  }
> 
> and then use smp_processor_id() directly in the callers that need
> to check this_cpu's extensions.
> 
> For case (2), I'd advocate we rename __riscv_isa_extension_available() to
> riscv_has_extension() and drop the riscv_isa_extension_available() macro
> in order to avoid having some calls with RISCV_ISA_EXT_* spelled out and
> others that rely on the pasting.

> And, ideally, we'd convert most
> riscv_has_extension(NULL, ext) calls to riscv_has_extension_[un]likely().

> I'm also not a big fan of the NULL parameter needed when
> riscv_isa_extension_available() is invoked for the riscv_isa bitmap

Rather than actually act on my concerns about
__riscv_isa_extension_available(), I've been busy devoting my spare
time to playing MMOs with the excuse of not wanting to fiddle further
with cpufeature.c et al until Palmer merged the new DT property stuff,
but splitting out your case 1 above seems like it would really help
there. The NULL argument case is the one I think has the potential to
be a footgun in the face of config options.
Split out we can document that purpose of each function & hopefully
have one set of functions that deals with "this extension was detected
to be present in the hardware" and one that does "this extension was
detected & supported by this particular kernel".

I'll try to take a proper look at this series tomorrow :)

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 2/6] RISC-V: Enable cbo.zero in usermode

[Andrew Jones]

On Wed, Aug 09, 2023 at 07:12:58PM +0100, Conor Dooley wrote:
> On Wed, Aug 09, 2023 at 06:58:15PM +0200, Andrew Jones wrote:
> > On Wed, Aug 09, 2023 at 09:00:35AM -0700, Evan Green wrote:
> > > On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> > ...
> > > > +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> > > > +{
> > > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > > > +               return true;
> > > > +
> > > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > > +}
> > > > +
> > > > +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> > > > +{
> > > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > > > +               return true;
> > > > +
> > > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > > +}
> > > 
> > > Another way to do this would be to add a parameter to
> > > riscv_has_extension_*() (as there are very few users), then these new
> > > functions can turn around and call those with the new parameter set to
> > > hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
> > > only advantage to it I can argue is it keeps the code flows more
> > > unified.
> > >
> > 
> > I like unification, but I think I'd prefer we create wrappers and
> > try to avoid callers needing to construct hart_isa[].isa parameters
> > themselves. I'm also not a big fan of the NULL parameter needed when
> > riscv_isa_extension_available() is invoked for the riscv_isa bitmap.
> > So we need:
> > 
> >   1. check if an extension is in riscv_isa
> >   2. check if an extension is in a bitmap provided by the caller
> >   3. check if an extension is in this cpu's isa bitmap
> >   4. check if an extension is in the isa bitmap of a cpu provided by the
> >      caller
> > 
> > The only one we can optimize with alternatives is (1), so it definitely
> > gets wrappers (riscv_has_extension_likely/unlikely()). (3) and (4) can
> > also get wrappers which first try the optimized (1), like I have above.
> > Actually (3)'s wrapper could be based on (4)'s, or only provide wrappers
> > for (4)
> > 
> >  static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> >  {
> >      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> >          return true;
> > 
> >      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> >  }
> > 
> >  static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
> >  {
> >      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> 
> Why are you gating on CONFIG_RISCV_ALTERNATIVE here?

This ensures we remove the riscv_has_extension_[un]likely() call
when that call would end up using its
__riscv_isa_extension_available(NULL, ext) fallback. If that fallback
where to return false, then we'd still need to make the
__riscv_isa_extension_available(hart_isa[cpu].isa, ext) call, doubling
the cost. Whereas, when we gate on CONFIG_RISCV_ALTERNATIVE, we know that
riscv_has_extension_[un]likely() will use an alternative to check the
global set of extensions. When the extension is there, the compiler
ensures that everything vanishes. When it's not, we'll fallback to a
single search of the cpu's isa bitmap.

> 
> >          return true;
> > 
> >      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> >  }
> > 
> > and then use smp_processor_id() directly in the callers that need
> > to check this_cpu's extensions.
> > 
> > For case (2), I'd advocate we rename __riscv_isa_extension_available() to
> > riscv_has_extension() and drop the riscv_isa_extension_available() macro
> > in order to avoid having some calls with RISCV_ISA_EXT_* spelled out and
> > others that rely on the pasting.
> 
> > And, ideally, we'd convert most
> > riscv_has_extension(NULL, ext) calls to riscv_has_extension_[un]likely().
> 
> > I'm also not a big fan of the NULL parameter needed when
> > riscv_isa_extension_available() is invoked for the riscv_isa bitmap
> 
> Rather than actually act on my concerns about
> __riscv_isa_extension_available(), I've been busy devoting my spare
> time to playing MMOs with the excuse of not wanting to fiddle further
> with cpufeature.c et al until Palmer merged the new DT property stuff,
> but splitting out your case 1 above seems like it would really help
> there. The NULL argument case is the one I think has the potential to
> be a footgun in the face of config options.
> Split out we can document that purpose of each function & hopefully
> have one set of functions that deals with "this extension was detected
> to be present in the hardware" and one that does "this extension was
> detected & supported by this particular kernel".

Sounds good to me!

> 
> I'll try to take a proper look at this series tomorrow :)
>

Thanks!
drew

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

Re: [PATCH 2/6] RISC-V: Enable cbo.zero in usermode

[Conor Dooley]

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

On Thu, Aug 10, 2023 at 09:31:54AM +0200, Andrew Jones wrote:
> On Wed, Aug 09, 2023 at 07:12:58PM +0100, Conor Dooley wrote:
> > On Wed, Aug 09, 2023 at 06:58:15PM +0200, Andrew Jones wrote:
> > > On Wed, Aug 09, 2023 at 09:00:35AM -0700, Evan Green wrote:
> > > > On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> > > ...
> > > > > +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> > > > > +{
> > > > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > > > > +               return true;
> > > > > +
> > > > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > > > +}
> > > > > +
> > > > > +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> > > > > +{
> > > > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > > > > +               return true;
> > > > > +
> > > > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > > > +}
> > > > 
> > > > Another way to do this would be to add a parameter to
> > > > riscv_has_extension_*() (as there are very few users), then these new
> > > > functions can turn around and call those with the new parameter set to
> > > > hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
> > > > only advantage to it I can argue is it keeps the code flows more
> > > > unified.
> > > >
> > > 
> > > I like unification, but I think I'd prefer we create wrappers and
> > > try to avoid callers needing to construct hart_isa[].isa parameters
> > > themselves. I'm also not a big fan of the NULL parameter needed when
> > > riscv_isa_extension_available() is invoked for the riscv_isa bitmap.
> > > So we need:
> > > 
> > >   1. check if an extension is in riscv_isa
> > >   2. check if an extension is in a bitmap provided by the caller
> > >   3. check if an extension is in this cpu's isa bitmap
> > >   4. check if an extension is in the isa bitmap of a cpu provided by the
> > >      caller
> > > 
> > > The only one we can optimize with alternatives is (1), so it definitely
> > > gets wrappers (riscv_has_extension_likely/unlikely()). (3) and (4) can
> > > also get wrappers which first try the optimized (1), like I have above.
> > > Actually (3)'s wrapper could be based on (4)'s, or only provide wrappers
> > > for (4)
> > > 
> > >  static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> > >  {
> > >      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > >          return true;
> > > 
> > >      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > >  }
> > > 
> > >  static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
> > >  {
> > >      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > 
> > Why are you gating on CONFIG_RISCV_ALTERNATIVE here?
> 
> This ensures we remove the riscv_has_extension_[un]likely() call
> when that call would end up using its
> __riscv_isa_extension_available(NULL, ext) fallback. If that fallback
> where to return false, then we'd still need to make the
> __riscv_isa_extension_available(hart_isa[cpu].isa, ext) call, doubling
> the cost. Whereas, when we gate on CONFIG_RISCV_ALTERNATIVE, we know that
> riscv_has_extension_[un]likely() will use an alternative to check the
> global set of extensions. When the extension is there, the compiler
> ensures that everything vanishes. When it's not, we'll fallback to a
> single search of the cpu's isa bitmap.

Right, that is what I suspected that you were trying to accomplish here.
I was not just not entirely sure whether it was or you'd just missed the
fallback path. In my original mail I was just going to say "Please add a
comment here as to why you want to avoid the fallback", but figured I
should figure out your intent first!

Just to note, alternatives are available on all !XIP kernels now, so
it's only in the case that the fallback path will be activated.

> > >          return true;
> > > 
> > >      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > >  }
> > > 
> > > and then use smp_processor_id() directly in the callers that need
> > > to check this_cpu's extensions.
> > > 
> > > For case (2), I'd advocate we rename __riscv_isa_extension_available() to
> > > riscv_has_extension() and drop the riscv_isa_extension_available() macro
> > > in order to avoid having some calls with RISCV_ISA_EXT_* spelled out and
> > > others that rely on the pasting.
> > 
> > > And, ideally, we'd convert most
> > > riscv_has_extension(NULL, ext) calls to riscv_has_extension_[un]likely().
> > 
> > > I'm also not a big fan of the NULL parameter needed when
> > > riscv_isa_extension_available() is invoked for the riscv_isa bitmap
> > 
> > Rather than actually act on my concerns about
> > __riscv_isa_extension_available(), I've been busy devoting my spare
> > time to playing MMOs with the excuse of not wanting to fiddle further
> > with cpufeature.c et al until Palmer merged the new DT property stuff,
> > but splitting out your case 1 above seems like it would really help
> > there. The NULL argument case is the one I think has the potential to
> > be a footgun in the face of config options.
> > Split out we can document that purpose of each function & hopefully
> > have one set of functions that deals with "this extension was detected
> > to be present in the hardware" and one that does "this extension was
> > detected & supported by this particular kernel".
> 
> Sounds good to me!

I figure said change should be independent of what's going on in this
series?

[-- 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 2/6] RISC-V: Enable cbo.zero in usermode

[Andrew Jones]

On Thu, Aug 10, 2023 at 10:34:33AM +0100, Conor Dooley wrote:
> On Thu, Aug 10, 2023 at 09:31:54AM +0200, Andrew Jones wrote:
> > On Wed, Aug 09, 2023 at 07:12:58PM +0100, Conor Dooley wrote:
> > > On Wed, Aug 09, 2023 at 06:58:15PM +0200, Andrew Jones wrote:
> > > > On Wed, Aug 09, 2023 at 09:00:35AM -0700, Evan Green wrote:
> > > > > On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> > > > ...
> > > > > > +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> > > > > > +{
> > > > > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > > > > > +               return true;
> > > > > > +
> > > > > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > > > > +}
> > > > > > +
> > > > > > +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> > > > > > +{
> > > > > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > > > > > +               return true;
> > > > > > +
> > > > > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > > > > +}
> > > > > 
> > > > > Another way to do this would be to add a parameter to
> > > > > riscv_has_extension_*() (as there are very few users), then these new
> > > > > functions can turn around and call those with the new parameter set to
> > > > > hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
> > > > > only advantage to it I can argue is it keeps the code flows more
> > > > > unified.
> > > > >
> > > > 
> > > > I like unification, but I think I'd prefer we create wrappers and
> > > > try to avoid callers needing to construct hart_isa[].isa parameters
> > > > themselves. I'm also not a big fan of the NULL parameter needed when
> > > > riscv_isa_extension_available() is invoked for the riscv_isa bitmap.
> > > > So we need:
> > > > 
> > > >   1. check if an extension is in riscv_isa
> > > >   2. check if an extension is in a bitmap provided by the caller
> > > >   3. check if an extension is in this cpu's isa bitmap
> > > >   4. check if an extension is in the isa bitmap of a cpu provided by the
> > > >      caller
> > > > 
> > > > The only one we can optimize with alternatives is (1), so it definitely
> > > > gets wrappers (riscv_has_extension_likely/unlikely()). (3) and (4) can
> > > > also get wrappers which first try the optimized (1), like I have above.
> > > > Actually (3)'s wrapper could be based on (4)'s, or only provide wrappers
> > > > for (4)
> > > > 
> > > >  static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
> > > >  {
> > > >      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > > >          return true;
> > > > 
> > > >      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > > >  }
> > > > 
> > > >  static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
> > > >  {
> > > >      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > > 
> > > Why are you gating on CONFIG_RISCV_ALTERNATIVE here?
> > 
> > This ensures we remove the riscv_has_extension_[un]likely() call
> > when that call would end up using its
> > __riscv_isa_extension_available(NULL, ext) fallback. If that fallback
> > where to return false, then we'd still need to make the
> > __riscv_isa_extension_available(hart_isa[cpu].isa, ext) call, doubling
> > the cost. Whereas, when we gate on CONFIG_RISCV_ALTERNATIVE, we know that
> > riscv_has_extension_[un]likely() will use an alternative to check the
> > global set of extensions. When the extension is there, the compiler
> > ensures that everything vanishes. When it's not, we'll fallback to a
> > single search of the cpu's isa bitmap.
> 
> Right, that is what I suspected that you were trying to accomplish here.
> I was not just not entirely sure whether it was or you'd just missed the
> fallback path. In my original mail I was just going to say "Please add a
> comment here as to why you want to avoid the fallback", but figured I
> should figure out your intent first!

Thanks, I'll add a comment for v2.

> 
> Just to note, alternatives are available on all !XIP kernels now, so
> it's only in the case that the fallback path will be activated.
> 
> > > >          return true;
> > > > 
> > > >      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
> > > >  }
> > > > 
> > > > and then use smp_processor_id() directly in the callers that need
> > > > to check this_cpu's extensions.
> > > > 
> > > > For case (2), I'd advocate we rename __riscv_isa_extension_available() to
> > > > riscv_has_extension() and drop the riscv_isa_extension_available() macro
> > > > in order to avoid having some calls with RISCV_ISA_EXT_* spelled out and
> > > > others that rely on the pasting.
> > > 
> > > > And, ideally, we'd convert most
> > > > riscv_has_extension(NULL, ext) calls to riscv_has_extension_[un]likely().
> > > 
> > > > I'm also not a big fan of the NULL parameter needed when
> > > > riscv_isa_extension_available() is invoked for the riscv_isa bitmap
> > > 
> > > Rather than actually act on my concerns about
> > > __riscv_isa_extension_available(), I've been busy devoting my spare
> > > time to playing MMOs with the excuse of not wanting to fiddle further
> > > with cpufeature.c et al until Palmer merged the new DT property stuff,
> > > but splitting out your case 1 above seems like it would really help
> > > there. The NULL argument case is the one I think has the potential to
> > > be a footgun in the face of config options.
> > > Split out we can document that purpose of each function & hopefully
> > > have one set of functions that deals with "this extension was detected
> > > to be present in the hardware" and one that does "this extension was
> > > detected & supported by this particular kernel".
> > 
> > Sounds good to me!
> 
> I figure said change should be independent of what's going on in this
> series?

Yeah, I considered doing it as part of this series, but then decided to
only start down the path with two new wrappers, which, for v2, I'll
change to the cpu parameter taking variant. The rest of the rework
would probably be best to do as a separate series, which I can start
soon.

Thanks,
drew

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

Re: [PATCH 2/6] RISC-V: Enable cbo.zero in usermode

[Conor Dooley]

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

On Thu, Aug 10, 2023 at 12:54:56PM +0200, Andrew Jones wrote:
> On Thu, Aug 10, 2023 at 10:34:33AM +0100, Conor Dooley wrote:


> > I figure said change should be independent of what's going on in this
> > series?
> 
> Yeah, I considered doing it as part of this series, but then decided to
> only start down the path with two new wrappers, which, for v2, I'll
> change to the cpu parameter taking variant. The rest of the rework
> would probably be best to do as a separate series, which I can start
> soon.

If you're offering, who am I to refuse!

[-- 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 2/6] RISC-V: Enable cbo.zero in usermode

[Evan Green]

On Wed, Aug 9, 2023 at 9:58 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Wed, Aug 09, 2023 at 09:00:35AM -0700, Evan Green wrote:
> > On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> ...
> > > +static __always_inline bool riscv_this_cpu_has_extension_likely(const unsigned long ext)
> > > +{
> > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
> > > +               return true;
> > > +
> > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > +}
> > > +
> > > +static __always_inline bool riscv_this_cpu_has_extension_unlikely(const unsigned long ext)
> > > +{
> > > +       if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
> > > +               return true;
> > > +
> > > +       return __riscv_isa_extension_available(hart_isa[smp_processor_id()].isa, ext);
> > > +}
> >
> > Another way to do this would be to add a parameter to
> > riscv_has_extension_*() (as there are very few users), then these new
> > functions can turn around and call those with the new parameter set to
> > hart_isa[smp_processor_id()].isa. It's a tossup, so up to you. The
> > only advantage to it I can argue is it keeps the code flows more
> > unified.
> >
>
> I like unification, but I think I'd prefer we create wrappers and
> try to avoid callers needing to construct hart_isa[].isa parameters
> themselves. I'm also not a big fan of the NULL parameter needed when
> riscv_isa_extension_available() is invoked for the riscv_isa bitmap.
> So we need:
>
>   1. check if an extension is in riscv_isa
>   2. check if an extension is in a bitmap provided by the caller
>   3. check if an extension is in this cpu's isa bitmap
>   4. check if an extension is in the isa bitmap of a cpu provided by the
>      caller
>
> The only one we can optimize with alternatives is (1), so it definitely
> gets wrappers (riscv_has_extension_likely/unlikely()). (3) and (4) can
> also get wrappers which first try the optimized (1), like I have above.
> Actually (3)'s wrapper could be based on (4)'s, or only provide wrappers
> for (4)
>
>  static __always_inline bool riscv_cpu_has_extension_likely(int cpu, const unsigned long ext)
>  {
>      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_likely(ext))
>          return true;
>
>      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
>  }
>
>  static __always_inline bool riscv_cpu_has_extension_unlikely(int cpu, const unsigned long ext)
>  {
>      if (IS_ENABLED(CONFIG_RISCV_ALTERNATIVE) && riscv_has_extension_unlikely(ext))
>          return true;
>
>      return __riscv_isa_extension_available(hart_isa[cpu].isa, ext);
>  }
>
> and then use smp_processor_id() directly in the callers that need
> to check this_cpu's extensions.
>
> For case (2), I'd advocate we rename __riscv_isa_extension_available() to
> riscv_has_extension() and drop the riscv_isa_extension_available() macro
> in order to avoid having some calls with RISCV_ISA_EXT_* spelled out and
> others that rely on the pasting. And, ideally, we'd convert most
> riscv_has_extension(NULL, ext) calls to riscv_has_extension_[un]likely().

Sounds ok to me!
-Evan

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

[PATCH 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size

[Andrew Jones]

Expose Zicboz through hwprobe and also provide a key to extract its
respective block size. Opportunistically add a macro and apply it to
current extensions in order to avoid duplicating code.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 Documentation/riscv/hwprobe.rst       |  6 ++++
 arch/riscv/include/asm/hwprobe.h      |  2 +-
 arch/riscv/include/uapi/asm/hwprobe.h |  2 ++
 arch/riscv/kernel/sys_riscv.c         | 41 ++++++++++++++++++---------
 4 files changed, 36 insertions(+), 15 deletions(-)

diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
index 933c715065d6..6a17c2872660 100644
--- a/Documentation/riscv/hwprobe.rst
+++ b/Documentation/riscv/hwprobe.rst
@@ -77,6 +77,9 @@ The following keys are defined:
   * :c:macro:`RISCV_HWPROBE_EXT_ZBS`: The Zbs extension is supported, as defined
        in version 1.0 of the Bit-Manipulation ISA extensions.
 
+  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
+       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
+
 * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
   information about the selected set of processors.
 
@@ -97,3 +100,6 @@ The following keys are defined:
 
   * :c:macro:`RISCV_HWPROBE_MISALIGNED_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/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
index 78936f4ff513..39df8604fea1 100644
--- a/arch/riscv/include/asm/hwprobe.h
+++ b/arch/riscv/include/asm/hwprobe.h
@@ -8,6 +8,6 @@
 
 #include <uapi/asm/hwprobe.h>
 
-#define RISCV_HWPROBE_MAX_KEY 5
+#define RISCV_HWPROBE_MAX_KEY 6
 
 #endif
diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
index 006bfb48343d..86d08a0e617b 100644
--- a/arch/riscv/include/uapi/asm/hwprobe.h
+++ b/arch/riscv/include/uapi/asm/hwprobe.h
@@ -29,6 +29,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_EXT_ZBA		(1 << 3)
 #define		RISCV_HWPROBE_EXT_ZBB		(1 << 4)
 #define		RISCV_HWPROBE_EXT_ZBS		(1 << 5)
+#define		RISCV_HWPROBE_EXT_ZICBOZ	(1 << 6)
 #define RISCV_HWPROBE_KEY_CPUPERF_0	5
 #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
@@ -36,6 +37,7 @@ struct riscv_hwprobe {
 #define		RISCV_HWPROBE_MISALIGNED_FAST		(3 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
 #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
+#define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
 /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
 
 #endif
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index 26ef5526bfb4..7d970358597b 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -145,26 +145,33 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
 	for_each_cpu(cpu, cpus) {
 		struct riscv_isainfo *isainfo = &hart_isa[cpu];
 
-		if (riscv_isa_extension_available(isainfo->isa, ZBA))
-			pair->value |= RISCV_HWPROBE_EXT_ZBA;
-		else
-			missing |= RISCV_HWPROBE_EXT_ZBA;
-
-		if (riscv_isa_extension_available(isainfo->isa, ZBB))
-			pair->value |= RISCV_HWPROBE_EXT_ZBB;
-		else
-			missing |= RISCV_HWPROBE_EXT_ZBB;
-
-		if (riscv_isa_extension_available(isainfo->isa, ZBS))
-			pair->value |= RISCV_HWPROBE_EXT_ZBS;
-		else
-			missing |= RISCV_HWPROBE_EXT_ZBS;
+#define EXT_KEY(ext)									\
+	do {										\
+		if (__riscv_isa_extension_available(isainfo->isa, RISCV_ISA_EXT_##ext))	\
+			pair->value |= RISCV_HWPROBE_EXT_##ext;				\
+		else									\
+			missing |= RISCV_HWPROBE_EXT_##ext;				\
+	} while (false)
+
+		EXT_KEY(ZBA);
+		EXT_KEY(ZBB);
+		EXT_KEY(ZBS);
+		EXT_KEY(ZICBOZ);
+#undef EXT_KEY
 	}
 
 	/* Now turn off reporting features if any CPU is missing it. */
 	pair->value &= ~missing;
 }
 
+static bool hwprobe_ext0_has(const struct cpumask *cpus, unsigned long ext)
+{
+	struct riscv_hwprobe pair;
+
+	hwprobe_isa_ext0(&pair, cpus);
+	return (pair.value & ext);
+}
+
 static u64 hwprobe_misaligned(const struct cpumask *cpus)
 {
 	int cpu;
@@ -215,6 +222,12 @@ static void hwprobe_one_pair(struct riscv_hwprobe *pair,
 		pair->value = hwprobe_misaligned(cpus);
 		break;
 
+	case RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE:
+		pair->value = 0;
+		if (hwprobe_ext0_has(cpus, RISCV_HWPROBE_EXT_ZICBOZ))
+			pair->value = riscv_cboz_block_size;
+		break;
+
 	/*
 	 * For forward compatibility, unknown keys don't fail the whole
 	 * call, but get their element key set to -1 and value set to 0
-- 
2.41.0


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

Re: [PATCH 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size

[Evan Green]

On Wed, Aug 9, 2023 at 4:55 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Expose Zicboz through hwprobe and also provide a key to extract its
> respective block size. Opportunistically add a macro and apply it to
> current extensions in order to avoid duplicating code.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

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

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

Re: [PATCH 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size

[Conor Dooley]

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

On Wed, Aug 09, 2023 at 01:55:20PM +0200, Andrew Jones wrote:
> Expose Zicboz through hwprobe and also provide a key to extract its
> respective block size. Opportunistically add a macro and apply it to
> current extensions in order to avoid duplicating code.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  Documentation/riscv/hwprobe.rst       |  6 ++++
>  arch/riscv/include/asm/hwprobe.h      |  2 +-
>  arch/riscv/include/uapi/asm/hwprobe.h |  2 ++
>  arch/riscv/kernel/sys_riscv.c         | 41 ++++++++++++++++++---------
>  4 files changed, 36 insertions(+), 15 deletions(-)
> 
> diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
> index 933c715065d6..6a17c2872660 100644
> --- a/Documentation/riscv/hwprobe.rst
> +++ b/Documentation/riscv/hwprobe.rst
> @@ -77,6 +77,9 @@ The following keys are defined:
>    * :c:macro:`RISCV_HWPROBE_EXT_ZBS`: The Zbs extension is supported, as defined
>         in version 1.0 of the Bit-Manipulation ISA extensions.
>  
> +  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
> +       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> +
>  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
>    information about the selected set of processors.
>  
> @@ -97,3 +100,6 @@ The following keys are defined:
>  
>    * :c:macro:`RISCV_HWPROBE_MISALIGNED_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/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> index 78936f4ff513..39df8604fea1 100644
> --- a/arch/riscv/include/asm/hwprobe.h
> +++ b/arch/riscv/include/asm/hwprobe.h
> @@ -8,6 +8,6 @@
>  
>  #include <uapi/asm/hwprobe.h>
>  
> -#define RISCV_HWPROBE_MAX_KEY 5
> +#define RISCV_HWPROBE_MAX_KEY 6
>  
>  #endif
> diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> index 006bfb48343d..86d08a0e617b 100644
> --- a/arch/riscv/include/uapi/asm/hwprobe.h
> +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> @@ -29,6 +29,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_EXT_ZBA		(1 << 3)
>  #define		RISCV_HWPROBE_EXT_ZBB		(1 << 4)
>  #define		RISCV_HWPROBE_EXT_ZBS		(1 << 5)
> +#define		RISCV_HWPROBE_EXT_ZICBOZ	(1 << 6)
>  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
>  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> @@ -36,6 +37,7 @@ struct riscv_hwprobe {
>  #define		RISCV_HWPROBE_MISALIGNED_FAST		(3 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
>  #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
> +#define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
>  /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
>  
>  #endif
> diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> index 26ef5526bfb4..7d970358597b 100644
> --- a/arch/riscv/kernel/sys_riscv.c
> +++ b/arch/riscv/kernel/sys_riscv.c
> @@ -145,26 +145,33 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  	for_each_cpu(cpu, cpus) {
>  		struct riscv_isainfo *isainfo = &hart_isa[cpu];
>  
> -		if (riscv_isa_extension_available(isainfo->isa, ZBA))
> -			pair->value |= RISCV_HWPROBE_EXT_ZBA;
> -		else
> -			missing |= RISCV_HWPROBE_EXT_ZBA;
> -
> -		if (riscv_isa_extension_available(isainfo->isa, ZBB))
> -			pair->value |= RISCV_HWPROBE_EXT_ZBB;
> -		else
> -			missing |= RISCV_HWPROBE_EXT_ZBB;
> -
> -		if (riscv_isa_extension_available(isainfo->isa, ZBS))
> -			pair->value |= RISCV_HWPROBE_EXT_ZBS;
> -		else
> -			missing |= RISCV_HWPROBE_EXT_ZBS;
> +#define EXT_KEY(ext)									\
> +	do {										\
> +		if (__riscv_isa_extension_available(isainfo->isa, RISCV_ISA_EXT_##ext))	\
> +			pair->value |= RISCV_HWPROBE_EXT_##ext;				\
> +		else									\
> +			missing |= RISCV_HWPROBE_EXT_##ext;				\
> +	} while (false)

Would you mind adding a comment here that using this macro is only
permitted for extensions that can be used by userspace regardless of kernel
config options?

> +		EXT_KEY(ZBA);
> +		EXT_KEY(ZBB);
> +		EXT_KEY(ZBS);
> +		EXT_KEY(ZICBOZ);
> +#undef EXT_KEY
>  	}
>  
>  	/* Now turn off reporting features if any CPU is missing it. */
>  	pair->value &= ~missing;
>  }

[-- 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 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size

[Andrew Jones]

On Thu, Aug 10, 2023 at 10:49:59AM +0100, Conor Dooley wrote:
> On Wed, Aug 09, 2023 at 01:55:20PM +0200, Andrew Jones wrote:
> > Expose Zicboz through hwprobe and also provide a key to extract its
> > respective block size. Opportunistically add a macro and apply it to
> > current extensions in order to avoid duplicating code.
> > 
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >  Documentation/riscv/hwprobe.rst       |  6 ++++
> >  arch/riscv/include/asm/hwprobe.h      |  2 +-
> >  arch/riscv/include/uapi/asm/hwprobe.h |  2 ++
> >  arch/riscv/kernel/sys_riscv.c         | 41 ++++++++++++++++++---------
> >  4 files changed, 36 insertions(+), 15 deletions(-)
> > 
> > diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst
> > index 933c715065d6..6a17c2872660 100644
> > --- a/Documentation/riscv/hwprobe.rst
> > +++ b/Documentation/riscv/hwprobe.rst
> > @@ -77,6 +77,9 @@ The following keys are defined:
> >    * :c:macro:`RISCV_HWPROBE_EXT_ZBS`: The Zbs extension is supported, as defined
> >         in version 1.0 of the Bit-Manipulation ISA extensions.
> >  
> > +  * :c:macro:`RISCV_HWPROBE_EXT_ZICBOZ`: The Zicboz extension is supported, as
> > +       ratified in commit 3dd606f ("Create cmobase-v1.0.pdf") of riscv-CMOs.
> > +
> >  * :c:macro:`RISCV_HWPROBE_KEY_CPUPERF_0`: A bitmask that contains performance
> >    information about the selected set of processors.
> >  
> > @@ -97,3 +100,6 @@ The following keys are defined:
> >  
> >    * :c:macro:`RISCV_HWPROBE_MISALIGNED_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/asm/hwprobe.h b/arch/riscv/include/asm/hwprobe.h
> > index 78936f4ff513..39df8604fea1 100644
> > --- a/arch/riscv/include/asm/hwprobe.h
> > +++ b/arch/riscv/include/asm/hwprobe.h
> > @@ -8,6 +8,6 @@
> >  
> >  #include <uapi/asm/hwprobe.h>
> >  
> > -#define RISCV_HWPROBE_MAX_KEY 5
> > +#define RISCV_HWPROBE_MAX_KEY 6
> >  
> >  #endif
> > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h
> > index 006bfb48343d..86d08a0e617b 100644
> > --- a/arch/riscv/include/uapi/asm/hwprobe.h
> > +++ b/arch/riscv/include/uapi/asm/hwprobe.h
> > @@ -29,6 +29,7 @@ struct riscv_hwprobe {
> >  #define		RISCV_HWPROBE_EXT_ZBA		(1 << 3)
> >  #define		RISCV_HWPROBE_EXT_ZBB		(1 << 4)
> >  #define		RISCV_HWPROBE_EXT_ZBS		(1 << 5)
> > +#define		RISCV_HWPROBE_EXT_ZICBOZ	(1 << 6)
> >  #define RISCV_HWPROBE_KEY_CPUPERF_0	5
> >  #define		RISCV_HWPROBE_MISALIGNED_UNKNOWN	(0 << 0)
> >  #define		RISCV_HWPROBE_MISALIGNED_EMULATED	(1 << 0)
> > @@ -36,6 +37,7 @@ struct riscv_hwprobe {
> >  #define		RISCV_HWPROBE_MISALIGNED_FAST		(3 << 0)
> >  #define		RISCV_HWPROBE_MISALIGNED_UNSUPPORTED	(4 << 0)
> >  #define		RISCV_HWPROBE_MISALIGNED_MASK		(7 << 0)
> > +#define RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE	6
> >  /* Increase RISCV_HWPROBE_MAX_KEY when adding items. */
> >  
> >  #endif
> > diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> > index 26ef5526bfb4..7d970358597b 100644
> > --- a/arch/riscv/kernel/sys_riscv.c
> > +++ b/arch/riscv/kernel/sys_riscv.c
> > @@ -145,26 +145,33 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> >  	for_each_cpu(cpu, cpus) {
> >  		struct riscv_isainfo *isainfo = &hart_isa[cpu];
> >  
> > -		if (riscv_isa_extension_available(isainfo->isa, ZBA))
> > -			pair->value |= RISCV_HWPROBE_EXT_ZBA;
> > -		else
> > -			missing |= RISCV_HWPROBE_EXT_ZBA;
> > -
> > -		if (riscv_isa_extension_available(isainfo->isa, ZBB))
> > -			pair->value |= RISCV_HWPROBE_EXT_ZBB;
> > -		else
> > -			missing |= RISCV_HWPROBE_EXT_ZBB;
> > -
> > -		if (riscv_isa_extension_available(isainfo->isa, ZBS))
> > -			pair->value |= RISCV_HWPROBE_EXT_ZBS;
> > -		else
> > -			missing |= RISCV_HWPROBE_EXT_ZBS;
> > +#define EXT_KEY(ext)									\
> > +	do {										\
> > +		if (__riscv_isa_extension_available(isainfo->isa, RISCV_ISA_EXT_##ext))	\
> > +			pair->value |= RISCV_HWPROBE_EXT_##ext;				\
> > +		else									\
> > +			missing |= RISCV_HWPROBE_EXT_##ext;				\
> > +	} while (false)
> 
> Would you mind adding a comment here that using this macro is only
> permitted for extensions that can be used by userspace regardless of kernel
> config options?

I can do that. Then, as part of the "extension available" API rework, we
can maybe drop the comment when we use the new call, which will be
documented to say that as well.

Thanks,
drew

> 
> > +		EXT_KEY(ZBA);
> > +		EXT_KEY(ZBB);
> > +		EXT_KEY(ZBS);
> > +		EXT_KEY(ZICBOZ);
> > +#undef EXT_KEY
> >  	}
> >  
> >  	/* Now turn off reporting features if any CPU is missing it. */
> >  	pair->value &= ~missing;
> >  }



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

Re: [PATCH 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size

[Conor Dooley]

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

On Thu, Aug 10, 2023 at 12:57:30PM +0200, Andrew Jones wrote:
> On Thu, Aug 10, 2023 at 10:49:59AM +0100, Conor Dooley wrote:
> > On Wed, Aug 09, 2023 at 01:55:20PM +0200, Andrew Jones wrote:

> > > +#define EXT_KEY(ext)									\
> > > +	do {										\
> > > +		if (__riscv_isa_extension_available(isainfo->isa, RISCV_ISA_EXT_##ext))	\
> > > +			pair->value |= RISCV_HWPROBE_EXT_##ext;				\
> > > +		else									\
> > > +			missing |= RISCV_HWPROBE_EXT_##ext;				\
> > > +	} while (false)
> > 
> > Would you mind adding a comment here that using this macro is only
> > permitted for extensions that can be used by userspace regardless of kernel
> > config options?
> 
> I can do that. Then, as part of the "extension available" API rework, we
> can maybe drop the comment when we use the new call, which will be
> documented to say that as well.

Yeah, exactly.

[-- 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

[PATCH 4/6] RISC-V: selftests: Statically link hwprobe test

[Andrew Jones]

Statically linking makes it more convenient to copy the test to a
minimal busybox environment.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 tools/testing/selftests/riscv/hwprobe/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/riscv/hwprobe/Makefile b/tools/testing/selftests/riscv/hwprobe/Makefile
index ebdbb3c22e54..5f614c3ba598 100644
--- a/tools/testing/selftests/riscv/hwprobe/Makefile
+++ b/tools/testing/selftests/riscv/hwprobe/Makefile
@@ -7,4 +7,4 @@ TEST_GEN_PROGS := hwprobe
 include ../../lib.mk
 
 $(OUTPUT)/hwprobe: hwprobe.c sys_hwprobe.S
-	$(CC) -o$@ $(CFLAGS) $(LDFLAGS) $^
+	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
-- 
2.41.0


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

Re: [PATCH 4/6] RISC-V: selftests: Statically link hwprobe test

[Conor Dooley]

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

On Wed, Aug 09, 2023 at 01:55:21PM +0200, Andrew Jones wrote:
> Statically linking makes it more convenient to copy the test to a
> minimal busybox environment.

As that's mainly what I used for quick testing of stuff, this seems
helpful to me!
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

[PATCH 5/6] RISC-V: selftests: Convert hwprobe test to kselftest API

[Andrew Jones]

Returning (exiting with) negative exit codes isn't user friendly,
because the user must output the exit code with the shell, convert it
from its unsigned 8-bit value back to the negative value, and then
look up where that comes from in the code (which may be multiple
places). Use the kselftests TAP interface, instead.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 .../testing/selftests/riscv/hwprobe/hwprobe.c | 54 +++++++------------
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.c b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
index 09f290a67420..4f15f1f3b4c3 100644
--- a/tools/testing/selftests/riscv/hwprobe/hwprobe.c
+++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
@@ -2,6 +2,8 @@
 #include <stddef.h>
 #include <asm/hwprobe.h>
 
+#include "../../kselftest.h"
+
 /*
  * 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
@@ -16,6 +18,9 @@ int main(int argc, char **argv)
 	unsigned long cpus;
 	long out;
 
+	ksft_print_header();
+	ksft_set_plan(5);
+
 	/* Fake the CPU_SET ops. */
 	cpus = -1;
 
@@ -25,13 +30,16 @@ int main(int argc, char **argv)
 	 */
 	for (long i = 0; i < 8; i++)
 		pairs[i].key = i;
+
 	out = riscv_hwprobe(pairs, 8, 1, &cpus, 0);
 	if (out != 0)
-		return -1;
+		ksft_exit_fail_msg("hwprobe() failed with %ld\n", out);
+
 	for (long i = 0; i < 4; ++i) {
 		/* Fail if the kernel claims not to recognize a base key. */
 		if ((i < 4) && (pairs[i].key != i))
-			return -2;
+			ksft_exit_fail_msg("Failed to recognize base key: key != i, "
+					   "key=%ld, i=%ld\n", pairs[i].key, i);
 
 		if (pairs[i].key != RISCV_HWPROBE_KEY_BASE_BEHAVIOR)
 			continue;
@@ -39,52 +47,30 @@ int main(int argc, char **argv)
 		if (pairs[i].value & RISCV_HWPROBE_BASE_BEHAVIOR_IMA)
 			continue;
 
-		return -3;
+		ksft_exit_fail_msg("Unexpected pair: (%ld, %ld)\n", pairs[i].key, pairs[i].value);
 	}
 
-	/*
-	 * This should also work with a NULL CPU set, but should not work
-	 * with an improperly supplied CPU set.
-	 */
 	out = riscv_hwprobe(pairs, 8, 0, 0, 0);
-	if (out != 0)
-		return -4;
+	ksft_test_result(out == 0, "NULL CPU set\n");
 
 	out = riscv_hwprobe(pairs, 8, 0, &cpus, 0);
-	if (out == 0)
-		return -5;
+	ksft_test_result(out != 0, "Bad CPU set\n");
 
 	out = riscv_hwprobe(pairs, 8, 1, 0, 0);
-	if (out == 0)
-		return -6;
+	ksft_test_result(out != 0, "NULL CPU set with non-zero count\n");
 
-	/*
-	 * Check that keys work by providing one that we know exists, and
-	 * checking to make sure the resultig pair is what we asked for.
-	 */
 	pairs[0].key = RISCV_HWPROBE_KEY_BASE_BEHAVIOR;
 	out = riscv_hwprobe(pairs, 1, 1, &cpus, 0);
-	if (out != 0)
-		return -7;
-	if (pairs[0].key != RISCV_HWPROBE_KEY_BASE_BEHAVIOR)
-		return -8;
+	ksft_test_result(out == 0 && pairs[0].key == RISCV_HWPROBE_KEY_BASE_BEHAVIOR,
+			 "Existing key is maintained\n");
 
-	/*
-	 * Check that an unknown key gets overwritten with -1,
-	 * but doesn't block elements after it.
-	 */
 	pairs[0].key = 0x5555;
 	pairs[1].key = 1;
 	pairs[1].value = 0xAAAA;
 	out = riscv_hwprobe(pairs, 2, 0, 0, 0);
-	if (out != 0)
-		return -9;
-
-	if (pairs[0].key != -1)
-		return -10;
-
-	if ((pairs[1].key != 1) || (pairs[1].value == 0xAAAA))
-		return -11;
+	ksft_test_result(out == 0 && pairs[0].key == -1 &&
+			 pairs[1].key == 1 && pairs[1].value != 0xAAAA,
+			 "Unknown key overwritten with -1 and doesn't block other elements\n");
 
-	return 0;
+	ksft_finished();
 }
-- 
2.41.0


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

[PATCH 6/6] RISC-V: selftests: Add CBO tests

[Andrew Jones]

Add hwprobe test for Zicboz and its block size. Also, when Zicboz is
present, test that cbo.zero may be issued and works. Additionally
test that the Zicbom instructions cause SIGILL and also that cbo.zero
causes SIGILL when Zicboz is not present. Pinning the test to a subset
of cpus with taskset will also restrict the hwprobe calls to that set.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 tools/testing/selftests/riscv/Makefile        |   2 +-
 .../testing/selftests/riscv/hwprobe/Makefile  |   5 +-
 tools/testing/selftests/riscv/hwprobe/cbo.c   | 160 ++++++++++++++++++
 .../testing/selftests/riscv/hwprobe/hwprobe.c |  12 +-
 .../testing/selftests/riscv/hwprobe/hwprobe.h |  15 ++
 5 files changed, 181 insertions(+), 13 deletions(-)
 create mode 100644 tools/testing/selftests/riscv/hwprobe/cbo.c
 create mode 100644 tools/testing/selftests/riscv/hwprobe/hwprobe.h

diff --git a/tools/testing/selftests/riscv/Makefile b/tools/testing/selftests/riscv/Makefile
index f4b3d5c9af5b..7bbeec48292d 100644
--- a/tools/testing/selftests/riscv/Makefile
+++ b/tools/testing/selftests/riscv/Makefile
@@ -16,7 +16,7 @@ CFLAGS := -Wall -O2 -g
 top_srcdir = $(realpath ../../../../)
 
 # Additional include paths needed by kselftest.h and local headers
-CFLAGS += -I$(top_srcdir)/tools/testing/selftests/
+CFLAGS += -I$(top_srcdir)/tools/testing/selftests/ -I$(top_srcdir)/tools/include
 
 CFLAGS += $(KHDR_INCLUDES)
 
diff --git a/tools/testing/selftests/riscv/hwprobe/Makefile b/tools/testing/selftests/riscv/hwprobe/Makefile
index 5f614c3ba598..56e54d66df7e 100644
--- a/tools/testing/selftests/riscv/hwprobe/Makefile
+++ b/tools/testing/selftests/riscv/hwprobe/Makefile
@@ -2,9 +2,12 @@
 # Copyright (C) 2021 ARM Limited
 # Originally tools/testing/arm64/abi/Makefile
 
-TEST_GEN_PROGS := hwprobe
+TEST_GEN_PROGS := hwprobe cbo
 
 include ../../lib.mk
 
 $(OUTPUT)/hwprobe: hwprobe.c sys_hwprobe.S
 	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
+
+$(OUTPUT)/cbo: cbo.c sys_hwprobe.S
+	$(CC) -static -o$@ $(CFLAGS) $(LDFLAGS) $^
diff --git a/tools/testing/selftests/riscv/hwprobe/cbo.c b/tools/testing/selftests/riscv/hwprobe/cbo.c
new file mode 100644
index 000000000000..6fa01e4cec99
--- /dev/null
+++ b/tools/testing/selftests/riscv/hwprobe/cbo.c
@@ -0,0 +1,160 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Run with 'taskset -c <cpu-list> cbo' to only execute hwprobe on a
+ * subset of cpus, as well as only executing the tests on those cpus.
+ */
+#define _GNU_SOURCE
+#include <stdbool.h>
+#include <stdint.h>
+#include <sched.h>
+#include <signal.h>
+#include <assert.h>
+#include <linux/compiler.h>
+#include <asm/ucontext.h>
+
+#include "hwprobe.h"
+#include "../../kselftest.h"
+
+static char mem[4096] __aligned(4096) = { [0 ... 4095] = 0xa5 };
+
+static bool illegal_insn;
+
+static void sigill_handler(int sig, siginfo_t *info, void *context)
+{
+	unsigned long *regs = (unsigned long *)&((ucontext_t *)context)->uc_mcontext;
+	uint32_t insn = *(uint32_t *)regs[0];
+
+	assert(insn >> 20 == regs[11] &&
+	       (insn & ((1 << 20) - 1)) == (10 << 15 | 2 << 12 | 0 << 7 | 15));
+
+	illegal_insn = true;
+	regs[0] += 4;
+}
+
+static void cbo_insn(int fn, char *base)
+{
+	asm volatile(
+	"mv	a0, %0\n"
+	"li	a1, %1\n"
+	".4byte	%1 << 20 | 10 << 15 | 2 << 12 | 0 << 7 | 15\n"
+	: : "r" (base), "i" (fn) : "a0", "a1", "memory");
+}
+
+static void cbo_inval(char *base) { cbo_insn(0, base); }
+static void cbo_clean(char *base) { cbo_insn(1, base); }
+static void cbo_flush(char *base) { cbo_insn(2, base); }
+static void cbo_zero(char *base)  { cbo_insn(4, base); }
+
+static void test_no_zicbom(void)
+{
+	illegal_insn = false;
+	cbo_clean(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.clean\n");
+
+	illegal_insn = false;
+	cbo_flush(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.flush\n");
+
+	illegal_insn = false;
+	cbo_inval(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.inval\n");
+}
+
+static void test_no_zicboz(void)
+{
+	illegal_insn = false;
+	cbo_clean(&mem[0]);
+	ksft_test_result(illegal_insn, "No cbo.zero\n");
+}
+
+static bool is_power_of_2(__u64 n)
+{
+	return n != 0 && (n & (n - 1)) == 0;
+}
+
+static void test_zicboz(__u64 block_size)
+{
+	int i, j;
+
+	illegal_insn = false;
+	cbo_zero(&mem[block_size]);
+	ksft_test_result(!illegal_insn, "cbo.zero\n");
+
+	if (!is_power_of_2(block_size)) {
+		ksft_test_result_skip("cbo.zero check\n");
+		return;
+	}
+
+	assert(block_size <= 1024);
+
+	for (i = 0; i < 4096 / block_size; ++i) {
+		if (i % 2)
+			cbo_zero(&mem[i * block_size]);
+	}
+
+	for (i = 0; i < 4096 / block_size; ++i) {
+		char expected = i % 2 ? 0x0 : 0xa5;
+
+		for (j = 0; j < block_size; ++j) {
+			if (mem[i * block_size + j] != expected) {
+				ksft_test_result_fail("cbo.zero check\n");
+				ksft_print_msg("cbo.zero check: mem[%d] != 0x%x\n",
+					       i * block_size + j, expected);
+				return;
+			}
+		}
+	}
+
+	ksft_test_result_pass("cbo.zero check\n");
+}
+
+int main(int argc, char **argv)
+{
+	struct sigaction act = {
+		.sa_sigaction = &sigill_handler,
+		.sa_flags = SA_SIGINFO,
+	};
+	bool has_zicboz = false;
+	struct riscv_hwprobe pair;
+	cpu_set_t cpus;
+	size_t nr_cpus;
+	long rc;
+
+	rc = sigaction(SIGILL, &act, NULL);
+	assert(rc == 0);
+
+	rc = sched_getaffinity(0, sizeof(cpu_set_t), &cpus);
+	assert(rc == 0);
+	nr_cpus = CPU_COUNT(&cpus);
+
+	ksft_print_header();
+
+	pair.key = RISCV_HWPROBE_KEY_IMA_EXT_0;
+	rc = riscv_hwprobe(&pair, 1, nr_cpus, (unsigned long *)&cpus, 0);
+	if (rc < 0)
+		ksft_exit_fail_msg("hwprobe() failed with %d\n", rc);
+
+	if (pair.key != -1 && (pair.value & RISCV_HWPROBE_EXT_ZICBOZ)) {
+		has_zicboz = true;
+		ksft_set_plan(6);
+	} else {
+		ksft_print_msg("No Zicboz, testing cbo.zero remains privileged\n");
+		ksft_set_plan(4);
+	}
+
+	/* Ensure zicbom instructions remain privileged */
+	test_no_zicbom();
+
+	if (has_zicboz) {
+		pair.key = RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE;
+		rc = riscv_hwprobe(&pair, 1, nr_cpus, (unsigned long *)&cpus, 0);
+		ksft_test_result(rc == 0 && pair.key == RISCV_HWPROBE_KEY_ZICBOZ_BLOCK_SIZE &&
+				 is_power_of_2(pair.value), "Zicboz block size\n");
+		ksft_print_msg("Zicboz block size: %ld\n", pair.value);
+		test_zicboz(pair.value);
+	} else {
+		test_no_zicboz();
+	}
+
+	ksft_finished();
+}
diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.c b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
index 4f15f1f3b4c3..c474891df307 100644
--- a/tools/testing/selftests/riscv/hwprobe/hwprobe.c
+++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.c
@@ -1,17 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-only
-#include <stddef.h>
-#include <asm/hwprobe.h>
-
+#include "hwprobe.h"
 #include "../../kselftest.h"
 
-/*
- * 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
- * contain the call.
- */
-long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
-		   size_t cpu_count, unsigned long *cpus, unsigned int flags);
-
 int main(int argc, char **argv)
 {
 	struct riscv_hwprobe pairs[8];
diff --git a/tools/testing/selftests/riscv/hwprobe/hwprobe.h b/tools/testing/selftests/riscv/hwprobe/hwprobe.h
new file mode 100644
index 000000000000..721b0ce73a56
--- /dev/null
+++ b/tools/testing/selftests/riscv/hwprobe/hwprobe.h
@@ -0,0 +1,15 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef SELFTEST_RISCV_HWPROBE_H
+#define SELFTEST_RISCV_HWPROBE_H
+#include <stddef.h>
+#include <asm/hwprobe.h>
+
+/*
+ * 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
+ * contain the call.
+ */
+long riscv_hwprobe(struct riscv_hwprobe *pairs, size_t pair_count,
+		   size_t cpu_count, unsigned long *cpus, unsigned int flags);
+
+#endif
-- 
2.41.0


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

Re: [PATCH 0/6] RISC-V: Enable cbo.zero in usermode

[patchwork-bot+linux-riscv]

Hello:

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

On Wed,  9 Aug 2023 13:55:17 +0200 you wrote:
> In order for usermode to issue cbo.zero, it needs privilege granted to
> issue the extension instruction (patch 2) and to know that the extension
> is available and its block size (patch 3). Patch 1 could be separate from
> this series (it just fixes up some error messages), patches 4-5 convert
> the hwprobe selftest to a statically-linked, TAP test and patch 6 adds a
> new hwprobe test for the new information as well as testing CBO
> instructions can or cannot be issued as appropriate.
> 
> [...]

Here is the summary with links:
  - [1/6] RISC-V: Make zicbom/zicboz errors consistent
    https://git.kernel.org/riscv/c/66e02b1d6430
  - [2/6] RISC-V: Enable cbo.zero in usermode
    https://git.kernel.org/riscv/c/090925374690
  - [3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size
    https://git.kernel.org/riscv/c/d45243f8c239
  - [4/6] RISC-V: selftests: Statically link hwprobe test
    https://git.kernel.org/riscv/c/0ca410a80172
  - [5/6] RISC-V: selftests: Convert hwprobe test to kselftest API
    https://git.kernel.org/riscv/c/d79a646354a9
  - [6/6] RISC-V: selftests: Add CBO tests
    https://git.kernel.org/riscv/c/026645838c86

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 0/6] RISC-V: Enable cbo.zero in usermode

[Andrew Jones]

On Wed, Aug 30, 2023 at 01:20:34PM +0000, patchwork-bot+linux-riscv@kernel.org wrote:
> Hello:
> 
> This series was applied to riscv/linux.git (for-next)
> by Palmer Dabbelt <palmer@rivosinc.com>:
> 
> On Wed,  9 Aug 2023 13:55:17 +0200 you wrote:
> > In order for usermode to issue cbo.zero, it needs privilege granted to
> > issue the extension instruction (patch 2) and to know that the extension
> > is available and its block size (patch 3). Patch 1 could be separate from
> > this series (it just fixes up some error messages), patches 4-5 convert
> > the hwprobe selftest to a statically-linked, TAP test and patch 6 adds a
> > new hwprobe test for the new information as well as testing CBO
> > instructions can or cannot be issued as appropriate.
> > 
> > [...]
> 
> Here is the summary with links:
>   - [1/6] RISC-V: Make zicbom/zicboz errors consistent
>     https://git.kernel.org/riscv/c/66e02b1d6430
>   - [2/6] RISC-V: Enable cbo.zero in usermode
>     https://git.kernel.org/riscv/c/090925374690
>   - [3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size
>     https://git.kernel.org/riscv/c/d45243f8c239
>   - [4/6] RISC-V: selftests: Statically link hwprobe test
>     https://git.kernel.org/riscv/c/0ca410a80172
>   - [5/6] RISC-V: selftests: Convert hwprobe test to kselftest API
>     https://git.kernel.org/riscv/c/d79a646354a9
>   - [6/6] RISC-V: selftests: Add CBO tests
>     https://git.kernel.org/riscv/c/026645838c86
> 
> You are awesome, thank you!
> -- 
> Deet-doot-dot, I am a bot.
> https://korg.docs.kernel.org/patchwork/pwbot.html
> 
>

Hi Palmer,

This series needs a v2. There were couple minor comments on v1, and
I just found out that I broke the compiling of the nolibc vector
selftest.

I'll post v2 in the next few minutes. Sorry for the hassle!

Thanks,
drew

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