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

[Atish Patra]

On Mon, 2019-08-12 at 08:02 -0700, Christoph Hellwig wrote:
> > +	for (e = mandatory_ext; *e != '\0'; ++e) {
> > +		if (isa[0] != e[0]) {
> > +#if defined(CONFIG_FP)
> > +			if ((isa[0] == 'f') || (isa[0] == 'd'))
> > +				continue;
> > +#endif
> > +			unsupported_isa[index] = e[0];
> > +			index++;
> > +		}
> 
> I'd just use if (IS_ENABLED()) here to get full compiler coverage.
> Also no need for the inner braces.
> 

Sure. I will do that.

> > +	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.

This can be extended to any other future extensions and related config.

> >  	seq_puts(f, "\n");ther you want to know if a specific extension
> > is enabled
> >  
> >  	/*
> >  	 * If we were given an unsupported ISA in the device tree then
> > print
> >  	 * a bit of info describing what went wrong.
> >  	 */
> > -	if (isa[0] != '\0')
> > -		pr_info("unsupported ISA \"%s\" in device tree\n",
> > orig_isa);
> > +	if (unsupported_isa[0])
> > +		pr_info("unsupported ISA extensions \"%s\" in device
> > tree for cpu [%ld]\n",
> > +			unsupported_isa, cpuid);
> 
> And I'm not even sure why we care about unsupported
> extensions.  Sooner
> or late a few will op up and they should be harmless.

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.


Regards,
Atish