Re: [PATCH v4 05/16] riscv: Extend cpufeature.c to detect vendor extensions

Conor Dooley
2024-05-01T10:46:14Z

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

On Fri, Apr 26, 2024 at 02:29:19PM -0700, Charlie Jenkins wrote:
> Separate vendor extensions out into one struct per vendor
> instead of adding vendor extensions onto riscv_isa_ext.
>
> Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this
> code.
>
> The xtheadvector vendor extension is added using these changes.

This mostly looks good to me, thanks for the updates. There's one thing
that I think is wrong, but I need to test and will get back to you on...

> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
> arch/riscv/Kconfig | 2 +
> arch/riscv/Kconfig.vendor | 19 ++++++
> arch/riscv/include/asm/cpufeature.h | 18 ++++++
> arch/riscv/include/asm/vendor_extensions.h | 26 ++++++++
> arch/riscv/include/asm/vendor_extensions/thead.h | 19 ++++++
> arch/riscv/kernel/Makefile | 2 +
> arch/riscv/kernel/cpufeature.c | 77 ++++++++++++++++++------
> arch/riscv/kernel/vendor_extensions.c | 18 ++++++
> arch/riscv/kernel/vendor_extensions/Makefile | 3 +
> arch/riscv/kernel/vendor_extensions/thead.c | 36 +++++++++++
> 10 files changed, 200 insertions(+), 20 deletions(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index be09c8836d56..fec86fba3acd 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS
>
> endchoice
>
> +source "arch/riscv/Kconfig.vendor"
> +
> endmenu # "Platform type"
>
> menu "Kernel features"
> diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor
> new file mode 100644
> index 000000000000..4fc86810af1d
> --- /dev/null
> +++ b/arch/riscv/Kconfig.vendor
> @@ -0,0 +1,19 @@
> +menu "Vendor extensions"
> +
> +config RISCV_ISA_VENDOR_EXT
> + bool
> +
> +menu "T-Head"
> +config RISCV_ISA_VENDOR_EXT_THEAD
> + bool "T-Head vendor extension support"
> + select RISCV_ISA_VENDOR_EXT
> + default y
> + help
> + Say N here if you want to disable all T-Head vendor extension
> + support. This will cause any T-Head vendor extensions that are
> + requested to be ignored.

What does "requested to be ignored" mean to a punter configuring a
kernel? I'd expect this to be something like:

"Say N here to disable detection of and support for all T-Head vendor
extensions. Without this option enabled, T-Head vendor extensions will
not be detected at boot and their presence not reported to userspace."

In general, I'd expect something that needs some support in the kernel
(like vector) to function to have a dedicated option, but the likes of
their Zba variant could be detected and reported via hwprobe et al
once RISCV_ISA_VENDOR_EXT_THEAD is enabled.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]