linux-kernel - Re: [PATCH v2 2/5] RISC-V: Add riscv_isa reprensenting ISA features common across CPUs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <99D22117-E146-44BE-93B5-5D25367931C7@wdc.com>
Date:   Wed, 31 Jul 2019 06:27:53 +0000
From:   Atish Patra <Atish.Patra@....com>
To:     Paul Walmsley <paul.walmsley@...ive.com>,
        Anup Patel <Anup.Patel@....com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Albert Ou <aou@...s.berkeley.edu>,
        Alexios Zavras <alexios.zavras@...el.com>,
        Allison Randal <allison@...utok.net>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Enrico Weigelt <info@...ux.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Johan Hovold <johan@...nel.org>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        Mark Rutland <mark.rutland@....com>,
        Palmer Dabbelt <palmer@...ive.com>,
        Rob Herring <robh+dt@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH v2 2/5] RISC-V: Add riscv_isa reprensenting ISA features
 common across CPUs



On 7/30/19, 9:23 PM, "Paul Walmsley" <paul.walmsley@...ive.com> wrote:

    On Tue, 30 Jul 2019, Atish Patra wrote:
    
    > From: Anup Patel <anup.patel@....com>
    > 
    > This patch adds riscv_isa integer to represent ISA features common
    > across all 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 is KVM hypervisor where riscv_isa will be used
    > 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 <anup.patel@....com>
    > Signed-off-by: Atish Patra <atish.patra@....com>
    > ---
    >  arch/riscv/include/asm/hwcap.h | 25 +++++++++++++++++++++
    >  arch/riscv/kernel/cpufeature.c | 41 +++++++++++++++++++++++++++++++---
    >  2 files changed, 63 insertions(+), 3 deletions(-)
    > 
    > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
    > index 7ecb7c6a57b1..e069f60ad5d2 100644
    > --- a/arch/riscv/include/asm/hwcap.h
    > +++ b/arch/riscv/include/asm/hwcap.h
    > @@ -22,5 +22,30 @@ enum {
    >  };
    >  
    >  extern unsigned long elf_hwcap;
    > +
    > +#define RISCV_ISA_EXT_A		(1UL << ('A' - 'A'))
    
    Are these uppercase variants still needed if we define the ISA string to 
    be all lowercase, per our recent discussion?
    
Argh. Sorry. We have been carrying this patch so long that I completely forgot about the
case sensitive usage here.

    > +#define RISCV_ISA_EXT_a		RISCV_ISA_EXT_A
    > +#define RISCV_ISA_EXT_C		(1UL << ('C' - 'A'))
    > +#define RISCV_ISA_EXT_c		RISCV_ISA_EXT_C
    > +#define RISCV_ISA_EXT_D		(1UL << ('D' - 'A'))
    > +#define RISCV_ISA_EXT_d		RISCV_ISA_EXT_D
    > +#define RISCV_ISA_EXT_F		(1UL << ('F' - 'A'))
    > +#define RISCV_ISA_EXT_f		RISCV_ISA_EXT_F
    > +#define RISCV_ISA_EXT_H		(1UL << ('H' - 'A'))
    > +#define RISCV_ISA_EXT_h		RISCV_ISA_EXT_H
    > +#define RISCV_ISA_EXT_I		(1UL << ('I' - 'A'))
    > +#define RISCV_ISA_EXT_i		RISCV_ISA_EXT_I
    > +#define RISCV_ISA_EXT_M		(1UL << ('M' - 'A'))
    > +#define RISCV_ISA_EXT_m		RISCV_ISA_EXT_M
    > +#define RISCV_ISA_EXT_S		(1UL << ('S' - 'A'))
    > +#define RISCV_ISA_EXT_s		RISCV_ISA_EXT_S
    > +#define RISCV_ISA_EXT_U		(1UL << ('U' - 'A'))
    > +#define RISCV_ISA_EXT_u		RISCV_ISA_EXT_U
    > +
    > +extern unsigned long riscv_isa;
    > +
    > +#define riscv_isa_extension_available(ext_char)	\
    > +		(riscv_isa & RISCV_ISA_EXT_##ext_char)
    > +
    >  #endif
    >  #endif
    > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
    > index b1ade9a49347..177529d48d87 100644
    > --- a/arch/riscv/kernel/cpufeature.c
    > +++ b/arch/riscv/kernel/cpufeature.c
    
    [ ... ]
    
    > @@ -43,8 +49,22 @@ void riscv_fill_hwcap(void)
    >  			continue;
    >  		}
    >  
    > -		for (i = 0; i < strlen(isa); ++i)
    > +		i = 0;
    > +		isa_len = strlen(isa);
    > +#if defined(CONFIG_32BIT)
    > +		if (strncasecmp(isa, "rv32", 4) != 0)
    
    strcmp()?
    
    > +			i += 4;
    > +#elif defined(CONFIG_64BIT)
    > +		if (strncasecmp(isa, "rv64", 4) != 0)
    
    And again here?
    
    > +			i += 4;
    > +#endif
    > +		for (; i < isa_len; ++i) {
    >  			this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
    > +			if ('a' <= isa[i] && isa[i] <= 'z')
    > +				this_isa |= (1UL << (isa[i] - 'a'));
    > +			if ('A' <= isa[i] && isa[i] <= 'Z')
    > +				this_isa |= (1UL << (isa[i] - 'A'));
    
    Are these uppercase variants still needed?
    
Nope. Same as above comment. Apologies for forgetting about these usages.
I will send a v3 removing them.
 
Regards,
Atish   
    - Paul