Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 6 Jul 2023 13:05:04 -0700
From:   Charlie Jenkins <charlie@rivosinc.com>
To:     =?iso-8859-1?Q?R=E9mi?= Denis-Courmont <remi@remlab.net>
Cc:     linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/3] RISC-V: Add T-Head 0.7.1 vector extension to hwprobe
Message-ID: <ZKcecLVQ61P886R/@ghost>
References: <20230705-thead_vendor_extensions-v1-0-ad6915349c4d@rivosinc.com>
 <20230705-thead_vendor_extensions-v1-2-ad6915349c4d@rivosinc.com>
 <2286282.ElGaqSPkdT@basile.remlab.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <2286282.ElGaqSPkdT@basile.remlab.net>
Precedence: bulk

On Thu, Jul 06, 2023 at 07:32:36PM +0300, Rémi Denis-Courmont wrote:
> Le torstaina 6. heinäkuuta 2023, 6.30.18 EEST Charlie Jenkins a écrit :
> > Using vendor extensions in hwprobe, add the ability to query if the
> > 0.7.1 vector extension is available. It is determined to be available
> > only if the kernel is compiled with vector support, and the user is
> > using the c906.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> >  arch/riscv/Kconfig.vendor                       | 11 +++++++++++
> >  arch/riscv/include/asm/extensions.h             | 16 ++++++++++++++++
> >  arch/riscv/kernel/sys_riscv.c                   | 20 ++++++++++++++++++++
> >  arch/riscv/vendor_extensions/Makefile           |  2 ++
> >  arch/riscv/vendor_extensions/thead/Makefile     |  8 ++++++++
> >  arch/riscv/vendor_extensions/thead/extensions.c | 24
> > ++++++++++++++++++++++++ 6 files changed, 81 insertions(+)
> > 
> > diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor
> > index 213ac3e6fed5..b8b9d15153eb 100644
> > --- a/arch/riscv/Kconfig.vendor
> > +++ b/arch/riscv/Kconfig.vendor
> > @@ -1,3 +1,14 @@
> >  menu "Vendor extensions selection"
> > 
> > +config VENDOR_EXTENSIONS_THEAD
> > +	bool "T-HEAD vendor extensions"
> > +	depends on RISCV_ALTERNATIVE
> > +	default n
> > +	help
> > +	  All T-HEAD vendor extensions Kconfig depend on this Kconfig. 
> Disabling
> > +	  this Kconfig will disable all T-HEAD vendor extensions. Please say 
> "Y"
> > +	  here if your platform uses T-HEAD vendor extensions.
> > +
> > +	  Otherwise, please say "N" here to avoid unnecessary overhead.
> > +
> >  endmenu # "Vendor extensions selection"
> > diff --git a/arch/riscv/include/asm/extensions.h
> > b/arch/riscv/include/asm/extensions.h new file mode 100644
> > index 000000000000..27ce294a3d65
> > --- /dev/null
> > +++ b/arch/riscv/include/asm/extensions.h
> > @@ -0,0 +1,16 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + * Copyright (C) 2023 by Rivos Inc.
> > + */
> > +#ifndef __ASM_EXTENSIONS_H
> > +#define __ASM_EXTENSIONS_H
> > +
> > +#include <asm/hwprobe.h>
> > +#include <linux/cpumask.h>
> > +
> > +#define THEAD_ISA_EXT0 (RISCV_HWPROBE_VENDOR_EXTENSION_SPACE)
> > +#define THEAD_ISA_EXT0_V0_7_1 (1 << 0)
> > +
> > +int hwprobe_thead(__u64 marchid, __u64 mimpid, struct riscv_hwprobe *pair,
> > +		  const struct cpumask *cpus);
> > +#endif
> > diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
> > index 2351a5f7b8b1..58b12eaeaf46 100644
> > --- a/arch/riscv/kernel/sys_riscv.c
> > +++ b/arch/riscv/kernel/sys_riscv.c
> > @@ -13,6 +13,7 @@
> >  #include <asm/vector.h>
> >  #include <asm/switch_to.h>
> >  #include <asm/uaccess.h>
> > +#include <asm/extensions.h>
> >  #include <asm/unistd.h>
> >  #include <asm-generic/mman-common.h>
> >  #include <vdso/vsyscall.h>
> > @@ -192,6 +193,25 @@ static int hwprobe_vendor(__u64 mvendorid, struct
> > riscv_hwprobe *pair, const struct cpumask *cpus)
> >  {
> >  	switch (mvendorid) {
> > +#ifdef CONFIG_VENDOR_EXTENSIONS_THEAD
> > +	case THEAD_VENDOR_ID:
> > +		struct riscv_hwprobe marchid = {
> > +			.key = RISCV_HWPROBE_KEY_MARCHID,
> > +			.value = 0
> > +		};
> > +		struct riscv_hwprobe mimpid = {
> > +			.key = RISCV_HWPROBE_KEY_MIMPID,
> > +			.value = 0
> > +		};
> > +
> > +		hwprobe_arch_id(&marchid, cpus);
> > +		hwprobe_arch_id(&mimpid, cpus);
> > +		if (marchid.value != -1ULL && mimpid.value != -1ULL)
> > +			hwprobe_thead(marchid.value, mimpid.value, 
> pair, cpus);
> > +		else
> > +			return -1;
> > +		break;
> > +#endif
> >  	default:
> >  		return -1;
> >  	}
> > diff --git a/arch/riscv/vendor_extensions/Makefile
> > b/arch/riscv/vendor_extensions/Makefile index e815895e9372..38c3e80469fd
> > 100644
> > --- a/arch/riscv/vendor_extensions/Makefile
> > +++ b/arch/riscv/vendor_extensions/Makefile
> > @@ -1,3 +1,5 @@
> >  ifdef CONFIG_RELOCATABLE
> >  KBUILD_CFLAGS += -fno-pie
> >  endif
> > +
> > +obj-$(CONFIG_VENDOR_EXTENSIONS_THEAD) += thead/
> > diff --git a/arch/riscv/vendor_extensions/thead/Makefile
> > b/arch/riscv/vendor_extensions/thead/Makefile new file mode 100644
> > index 000000000000..7cf43c629b66
> > --- /dev/null
> > +++ b/arch/riscv/vendor_extensions/thead/Makefile
> > @@ -0,0 +1,8 @@
> > +ifdef CONFIG_FTRACE
> > +CFLAGS_REMOVE_extensions.o = $(CC_FLAGS_FTRACE)
> > +endif
> > +ifdef CONFIG_KASAN
> > +KASAN_SANITIZE_extensions.o := n
> > +endif
> > +
> > +obj-y += extensions.o
> > diff --git a/arch/riscv/vendor_extensions/thead/extensions.c
> > b/arch/riscv/vendor_extensions/thead/extensions.c new file mode 100644
> > index 000000000000..a177501bc99c
> > --- /dev/null
> > +++ b/arch/riscv/vendor_extensions/thead/extensions.c
> > @@ -0,0 +1,24 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2023 by Rivos Inc.
> > + */
> > +
> > +#include <asm/extensions.h>
> > +
> > +int hwprobe_thead(__u64 marchid, __u64 mimpid, struct riscv_hwprobe *pair,
> > +		  const struct cpumask *cpus)
> > +{
> > +	pair->value = 0;
> > +	switch (pair->key) {
> > +	case THEAD_ISA_EXT0:
> > +#ifdef CONFIG_RISCV_ISA_V
> > +		if (marchid == 0 && mimpid == 0)
> > +			pair->value |= THEAD_ISA_EXT0_V0_7_1;
> 
> I'm not sure I follow the code, but exposing an extension to userspace that 
> the kernel does not handle is a bad idea. AFAIK, the initialisation and 
> context switching code must be in place first.
> 
> And I don't suppose that this can *simply* piggy-back on the existing RVV 
> 1.0.0 support, because user-space  assumes 1.0.0-compliant behaviour if the 
> existing vector flag is set in hwprobe().
> 
> Indeed, I recall somebody else posted a recent patchset ostensibly with the 
> same goal that was a lot more involved than this.
> 
The main goal of this patch was to get vendor extensions tied into
hwprobe. I selected this vector extension to start with, but it looks
like it is more complicated than I had anticipated.
> > +#endif
> > +		break;
> > +	default:
> > +		return -1;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> 
> -- 
> 雷米‧德尼-库尔蒙
> http://www.remlab.net/
> 
> 
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv