linux-kernel - Re: [v5 PATCH] RISC-V: Fix unsupported isa string info.

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]

Date:   Fri, 6 Sep 2019 23:27:57 +0000
From:   Atish Patra <Atish.Patra@....com>
To:     "hch@...radead.org" <hch@...radead.org>
CC:     "anup@...infault.org" <anup@...infault.org>,
        "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "palmer@...ive.com" <palmer@...ive.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
        "johan@...nel.org" <johan@...nel.org>
Subject: Re: [v5 PATCH] RISC-V: Fix unsupported isa string info.

On Tue, 2019-08-20 at 00:59 -0700, Atish Patra wrote:
> On Sun, 2019-08-18 at 11:16 -0700, hch@...radead.org wrote:
> > On Fri, Aug 16, 2019 at 07:21:52PM +0000, Atish Patra wrote:
> > > > > +	if (isa[0] != '\0') {
> > > > > +		/* Add remainging isa strings */
> > > > > +		for (e = isa; *e != '\0'; ++e) {
> > > > > +#if !defined(CONFIG_VIRTUALIZATION)
> > > > > +			if (e[0] != 'h')
> > > > > +#endif
> > > > > +				seq_write(f, e, 1);
> > > > > +		}
> > > > > +	}
> > > > 
> > > > This one I don't get.  Why do we want to check
> > > > CONFIG_VIRTUALIZATION?
> > > > 
> > > 
> > > If CONFIG_VIRTUALIZATION is not enabled, it shouldn't print that
> > > hypervisor extension "h" in isa extensions.
> > 
> > CONFIG_VIRTUALIZATION doesn't change anything in the kernels
> > capabilities, it just enables other config options. 
> 
> Yes. But it must be enabled if virtualization is supported in kernel.
> The idea was to let userspace know that if virtualization can be used
> or not. 
> 
> 
> >  But more
> > importantly the 'h' extension is only relevant for S-mode software
> > anyway.
> > 
> 
> Do you think we should just print all the extensions available in isa
> string as it is ?
> 
> > > This is just an information to the userspace that some of the
> > > mandatory
> > > ISA extensions ("mafdcsu") are not supported in kernel which may
> > > lead
> > > to undesirable results.
> > 
> > I think we need to sit down decide what the purpose of
> > /proc/cpuinfo
> > is.  IIRC on other architectures is just prints what the hardware
> > supports, not what you can actually make use of.  How else would
> > you
> > find out that you'd need to enable more kernel options to fully
> > utilize the hardware?
> > 
> > Also printing this warning to the kernel log when someone reads the
> > procfs file is very strange.
> 
> Agreed. May be something like this ?
> 
> Let's say f/d is enabled in kernel but cpu doesn't support it.
> "unsupported isa" will only appear if there are any unsupported isa.
> 
> processor       : 3
> hart            : 4
> isa             : rv64imac
> unsupported isa	: fd
> mmu             : sv39
> uarch           : sifive,u54-mc
> 
> May be I am just trying over optimize one corner case :) :).
> /proc/cpuinfo should just print all the isa string. That's it.
> 

Ping ?

> Regards,
> Atish

-- 
Regards,
Atish