Re: [PATCH v4 02/20] RISC-V: Add bitmap reprensenting ISA features common across CPUs
On Thu, Aug 8, 2019 at 12:18 AM Atish Patra wrote: > > On Wed, 2019-08-07 at 12:28 +, Anup Patel wrote: > > This patch adds riscv_isa bitmap which represents Host ISA features > > common across all Host CPUs. The riscv_isa is not same as elf_hwcap > > because elf_hwcap will only have ISA features relevant for user-space > > apps whereas riscv_isa will have ISA features relevant to both kernel > > and user-space apps. > > > > One of the use-case for riscv_isa bitmap is in KVM hypervisor where > > we will use it to do following operations: > > > > 1. Check whether hypervisor extension is available > > 2. Find ISA features that need to be virtualized (e.g. floating > >point support, vector extension, etc.) > > > > Signed-off-by: Anup Patel > > Signed-off-by: Atish Patra > > --- > > arch/riscv/include/asm/hwcap.h | 26 +++ > > arch/riscv/kernel/cpufeature.c | 79 > > -- > > 2 files changed, 102 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/include/asm/hwcap.h > > b/arch/riscv/include/asm/hwcap.h > > index 7ecb7c6a57b1..9b657375aa51 100644 > > --- a/arch/riscv/include/asm/hwcap.h > > +++ b/arch/riscv/include/asm/hwcap.h > > @@ -8,6 +8,7 @@ > > #ifndef __ASM_HWCAP_H > > #define __ASM_HWCAP_H > > > > +#include > > #include > > > > #ifndef __ASSEMBLY__ > > @@ -22,5 +23,30 @@ enum { > > }; > > > > extern unsigned long elf_hwcap; > > + > > +#define RISCV_ISA_EXT_a ('a' - 'a') > > +#define RISCV_ISA_EXT_c ('c' - 'a') > > +#define RISCV_ISA_EXT_d ('d' - 'a') > > +#define RISCV_ISA_EXT_f ('f' - 'a') > > +#define RISCV_ISA_EXT_h ('h' - 'a') > > +#define RISCV_ISA_EXT_i ('i' - 'a') > > +#define RISCV_ISA_EXT_m ('m' - 'a') > > +#define RISCV_ISA_EXT_s ('s' - 'a') > > +#define RISCV_ISA_EXT_u ('u' - 'a') > > As per the discussion in following threads, 'S' & 'U' are not valid ISA > extensions. So we should drop them from here as well. > > http://lists.infradead.org/pipermail/linux-riscv/2019-August/005771.html > > https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01217.html I disagree because we are not checking or enforcing required ISA features here. The asm/hwcap.h should define all possible feature and extension bits defined by the RISC-V spec. The 's' and 'u' bits in ISA mean that S-mode and U-mode are supported. These bits are defined in RISC-V privileged spec as well. Regards, Anup > > > > +#define RISCV_ISA_EXT_zicsr (('z' - 'a') + 1) > > +#define RISCV_ISA_EXT_zifencei (('z' - 'a') + 2) > > +#define RISCV_ISA_EXT_zam(('z' - 'a') + 3) > > +#define RISCV_ISA_EXT_ztso (('z' - 'a') + 4) > > + > > +#define RISCV_ISA_EXT_MAX256 > > + > > +unsigned long riscv_isa_extension_base(const unsigned long > > *isa_bitmap); > > + > > +#define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext) > > + > > +bool __riscv_isa_extension_available(const unsigned long > > *isa_bitmap, int bit); > > +#define riscv_isa_extension_available(isa_bitmap, ext) \ > > + __riscv_isa_extension_available(isa_bitmap, > > RISCV_ISA_EXT_##ext) > > + > > #endif > > #endif > > diff --git a/arch/riscv/kernel/cpufeature.c > > b/arch/riscv/kernel/cpufeature.c > > index b1ade9a49347..4ce71ce5e290 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -6,21 +6,64 @@ > > * Copyright (C) 2017 SiFive > > */ > > > > +#include > > #include > > #include > > #include > > #include > > > > unsigned long elf_hwcap __read_mostly; > > + > > +/* Host ISA bitmap */ > > +static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; > > + > > #ifdef CONFIG_FPU > > bool has_fpu __read_mostly; > > #endif > > > > +/** > > + * riscv_isa_extension_base - Get base extension word > > + * > > + * @isa_bitmap ISA bitmap to use > > + * @returns base extension word as unsigned long value > > + * > > + * NOTE: If isa_bitmap is NULL then Host ISA bitmap will be used. > > + */ > > +unsigned long riscv_isa_extension_base(const unsigned long > > *isa_bitmap) > > +{ > > + if (!isa_bitmap) > > + return riscv_isa[0]; > > + return isa_bitmap[0]; > > +} > > +EXPORT_SYMBOL_GPL(riscv_isa_extension_base); > > + > > +/** > > + * __riscv_isa_extension_available - Check whether given extension > > + * is available or not > > + * > > + * @isa_bitmap ISA bitmap to use > > + * @bit bit position of the desired extension > > + * @returns true or false > > + * > > + * NOTE: If isa_bitmap is NULL then Host ISA bitmap will be used. > > + */ > > +bool __riscv_isa_extension_available(const unsigned long > > *isa_bitmap, int bit) > > +{ > > + const unsigned long *bmap = (isa_bitmap) ? isa_bitmap : > > riscv_isa; > > + > > + if (bit >= RISCV_ISA_EXT_MAX) > > + return false; > > + > > + return test_bit(bit, bmap) ? true : false; > > +} > >
Re: [PATCH v4 02/20] RISC-V: Add bitmap reprensenting ISA features common across CPUs
On Wed, 2019-08-07 at 12:28 +, Anup Patel wrote: > This patch adds riscv_isa bitmap which represents Host ISA features > common across all Host CPUs. The riscv_isa is not same as elf_hwcap > because elf_hwcap will only have ISA features relevant for user-space > apps whereas riscv_isa will have ISA features relevant to both kernel > and user-space apps. > > One of the use-case for riscv_isa bitmap is in KVM hypervisor where > we will use it to do following operations: > > 1. Check whether hypervisor extension is available > 2. Find ISA features that need to be virtualized (e.g. floating >point support, vector extension, etc.) > > Signed-off-by: Anup Patel > Signed-off-by: Atish Patra > --- > arch/riscv/include/asm/hwcap.h | 26 +++ > arch/riscv/kernel/cpufeature.c | 79 > -- > 2 files changed, 102 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/include/asm/hwcap.h > b/arch/riscv/include/asm/hwcap.h > index 7ecb7c6a57b1..9b657375aa51 100644 > --- a/arch/riscv/include/asm/hwcap.h > +++ b/arch/riscv/include/asm/hwcap.h > @@ -8,6 +8,7 @@ > #ifndef __ASM_HWCAP_H > #define __ASM_HWCAP_H > > +#include > #include > > #ifndef __ASSEMBLY__ > @@ -22,5 +23,30 @@ enum { > }; > > extern unsigned long elf_hwcap; > + > +#define RISCV_ISA_EXT_a ('a' - 'a') > +#define RISCV_ISA_EXT_c ('c' - 'a') > +#define RISCV_ISA_EXT_d ('d' - 'a') > +#define RISCV_ISA_EXT_f ('f' - 'a') > +#define RISCV_ISA_EXT_h ('h' - 'a') > +#define RISCV_ISA_EXT_i ('i' - 'a') > +#define RISCV_ISA_EXT_m ('m' - 'a') > +#define RISCV_ISA_EXT_s ('s' - 'a') > +#define RISCV_ISA_EXT_u ('u' - 'a') As per the discussion in following threads, 'S' & 'U' are not valid ISA extensions. So we should drop them from here as well. http://lists.infradead.org/pipermail/linux-riscv/2019-August/005771.html https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01217.html > +#define RISCV_ISA_EXT_zicsr (('z' - 'a') + 1) > +#define RISCV_ISA_EXT_zifencei (('z' - 'a') + 2) > +#define RISCV_ISA_EXT_zam(('z' - 'a') + 3) > +#define RISCV_ISA_EXT_ztso (('z' - 'a') + 4) > + > +#define RISCV_ISA_EXT_MAX256 > + > +unsigned long riscv_isa_extension_base(const unsigned long > *isa_bitmap); > + > +#define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext) > + > +bool __riscv_isa_extension_available(const unsigned long > *isa_bitmap, int bit); > +#define riscv_isa_extension_available(isa_bitmap, ext) \ > + __riscv_isa_extension_available(isa_bitmap, > RISCV_ISA_EXT_##ext) > + > #endif > #endif > diff --git a/arch/riscv/kernel/cpufeature.c > b/arch/riscv/kernel/cpufeature.c > index b1ade9a49347..4ce71ce5e290 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -6,21 +6,64 @@ > * Copyright (C) 2017 SiFive > */ > > +#include > #include > #include > #include > #include > > unsigned long elf_hwcap __read_mostly; > + > +/* Host ISA bitmap */ > +static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; > + > #ifdef CONFIG_FPU > bool has_fpu __read_mostly; > #endif > > +/** > + * riscv_isa_extension_base - Get base extension word > + * > + * @isa_bitmap ISA bitmap to use > + * @returns base extension word as unsigned long value > + * > + * NOTE: If isa_bitmap is NULL then Host ISA bitmap will be used. > + */ > +unsigned long riscv_isa_extension_base(const unsigned long > *isa_bitmap) > +{ > + if (!isa_bitmap) > + return riscv_isa[0]; > + return isa_bitmap[0]; > +} > +EXPORT_SYMBOL_GPL(riscv_isa_extension_base); > + > +/** > + * __riscv_isa_extension_available - Check whether given extension > + * is available or not > + * > + * @isa_bitmap ISA bitmap to use > + * @bit bit position of the desired extension > + * @returns true or false > + * > + * NOTE: If isa_bitmap is NULL then Host ISA bitmap will be used. > + */ > +bool __riscv_isa_extension_available(const unsigned long > *isa_bitmap, int bit) > +{ > + const unsigned long *bmap = (isa_bitmap) ? isa_bitmap : > riscv_isa; > + > + if (bit >= RISCV_ISA_EXT_MAX) > + return false; > + > + return test_bit(bit, bmap) ? true : false; > +} > +EXPORT_SYMBOL_GPL(__riscv_isa_extension_available); > + > void riscv_fill_hwcap(void) > { > struct device_node *node; > const char *isa; > - size_t i; > + char print_str[BITS_PER_LONG+1]; > + size_t i, j, isa_len; > static unsigned long isa2hwcap[256] = {0}; > > isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I; > @@ -32,8 +75,11 @@ void riscv_fill_hwcap(void) > > elf_hwcap = 0; > > + bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX); > + > for_each_of_cpu_node(node) { > unsigned long this_hwcap = 0; > + unsigned long this_isa = 0; > > if