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

From: Andrew Jones <ajones@ventanamicro.com>
To: Conor Dooley <conor.dooley@microchip.com>
Cc: linux-riscv@lists.infradead.org, paul.walmsley@sifive.com,
	 palmer@dabbelt.com, aou@eecs.berkeley.edu, evan@rivosinc.com,
	 apatel@ventanamicro.com
Subject: Re: [PATCH 3/6] RISC-V: hwprobe: Expose Zicboz extension and its block size
Date: Thu, 10 Aug 2023 12:57:30 +0200	[thread overview]
Message-ID: <20230810-0c9993116a87372bf4fcfe9e@orel> (raw)
In-Reply-To: <20230810-revolver-krypton-ab1883b361a1@wendy>

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