[PATCH v1 01/13] riscv: fatorize hwprobe ISA extension reporting

Clément Léger posted 13 patches 2 years, 2 months ago
There is a newer version of this series
[PATCH v1 01/13] riscv: fatorize hwprobe ISA extension reporting
Posted by Clément Léger 2 years, 2 months ago
Factorize ISA extension reporting by using a macro rather than
copy/pasting extension names. This will allow adding new extensions more
easily.

Signed-off-by: Clément Léger <cleger@rivosinc.com>
---
 arch/riscv/kernel/sys_riscv.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index 473159b5f303..5ce593ce07a4 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -145,20 +145,18 @@ 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 CHECK_ISA_EXT(__ext)							\
+		do {								\
+			if (riscv_isa_extension_available(isainfo->isa, __ext))	\
+				pair->value |= RISCV_HWPROBE_EXT_##__ext;	\
+			else							\
+				missing |= RISCV_HWPROBE_EXT_##__ext;		\
+		} while (false)							\
+
+		CHECK_ISA_EXT(ZBA);
+		CHECK_ISA_EXT(ZBB);
+		CHECK_ISA_EXT(ZBS);
+#undef CHECK_ISA_EXT
 	}
 
 	/* Now turn off reporting features if any CPU is missing it. */
-- 
2.42.0

Re: [PATCH v1 01/13] riscv: fatorize hwprobe ISA extension reporting
Posted by Conor Dooley 2 years, 2 months ago
Drew,

On Wed, Oct 11, 2023 at 01:14:26PM +0200, Clément Léger wrote:
> Factorize ISA extension reporting by using a macro rather than
> copy/pasting extension names. This will allow adding new extensions more
> easily.
> 
> Signed-off-by: Clément Léger <cleger@rivosinc.com>
> ---
>  arch/riscv/kernel/sys_riscv.c | 26 ++++++++++++--------------
>  1 file changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> index 473159b5f303..5ce593ce07a4 100644
> --- a/arch/riscv/kernel/sys_riscv.c
> +++ b/arch/riscv/kernel/sys_riscv.c
> @@ -145,20 +145,18 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
>  	for_each_cpu(cpu, cpus) {

We were gonna add a comment here about when it is and is not safe to use
riscv_isa_extension_available() IIRC. Did that ever end up in a patch?

>  		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 CHECK_ISA_EXT(__ext)							\
> +		do {								\
> +			if (riscv_isa_extension_available(isainfo->isa, __ext))	\
> +				pair->value |= RISCV_HWPROBE_EXT_##__ext;	\
> +			else							\
> +				missing |= RISCV_HWPROBE_EXT_##__ext;		\
> +		} while (false)							\
> +
> +		CHECK_ISA_EXT(ZBA);
> +		CHECK_ISA_EXT(ZBB);
> +		CHECK_ISA_EXT(ZBS);
> +#undef CHECK_ISA_EXT
>  	}
>  
>  	/* Now turn off reporting features if any CPU is missing it. */
> -- 
> 2.42.0
> 
Re: [PATCH v1 01/13] riscv: fatorize hwprobe ISA extension reporting
Posted by Andrew Jones 2 years, 2 months ago
On Thu, Oct 12, 2023 at 02:53:43PM +0100, Conor Dooley wrote:
> Drew,
> 
> On Wed, Oct 11, 2023 at 01:14:26PM +0200, Clément Léger wrote:
> > Factorize ISA extension reporting by using a macro rather than
> > copy/pasting extension names. This will allow adding new extensions more
> > easily.
> > 
> > Signed-off-by: Clément Léger <cleger@rivosinc.com>
> > ---
> >  arch/riscv/kernel/sys_riscv.c | 26 ++++++++++++--------------
> >  1 file changed, 12 insertions(+), 14 deletions(-)
> > 
> > diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> > index 473159b5f303..5ce593ce07a4 100644
> > --- a/arch/riscv/kernel/sys_riscv.c
> > +++ b/arch/riscv/kernel/sys_riscv.c
> > @@ -145,20 +145,18 @@ static void hwprobe_isa_ext0(struct riscv_hwprobe *pair,
> >  	for_each_cpu(cpu, cpus) {
> 
> We were gonna add a comment here about when it is and is not safe to use
> riscv_isa_extension_available() IIRC. Did that ever end up in a patch?

Yup, it's in [1]. But that series may be hung up on spec stuff, so maybe
it'd be better for Clément to integrate it. And, it appears we definitely
need this macro, because it has now been suggested by three different
people :-) (I later saw Samuel was first[2], but I hadn't seen his before
submitting mine, otherwise I would have given him the credit.)

[1] https://lore.kernel.org/all/20230918131518.56803-11-ajones@ventanamicro.com/
[2] https://lore.kernel.org/all/20230712084134.1648008-4-sameo@rivosinc.com/

Thanks,
drew

> 
> >  		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 CHECK_ISA_EXT(__ext)							\
> > +		do {								\
> > +			if (riscv_isa_extension_available(isainfo->isa, __ext))	\
> > +				pair->value |= RISCV_HWPROBE_EXT_##__ext;	\
> > +			else							\
> > +				missing |= RISCV_HWPROBE_EXT_##__ext;		\
> > +		} while (false)							\
> > +
> > +		CHECK_ISA_EXT(ZBA);
> > +		CHECK_ISA_EXT(ZBB);
> > +		CHECK_ISA_EXT(ZBS);
> > +#undef CHECK_ISA_EXT
> >  	}
> >  
> >  	/* Now turn off reporting features if any CPU is missing it. */
> > -- 
> > 2.42.0
> > 
Re: [PATCH v1 01/13] riscv: fatorize hwprobe ISA extension reporting
Posted by Robert P. J. Day 2 years, 2 months ago
  correct misspelling of "fatorize" in subject line

rday