linux-kernel - Re: [PATCH 3/4] RISC-V: Support case insensitive ISA string parsing.

Open Source and information security mailing list archives

Date:   Mon, 29 Jul 2019 18:31:47 +0000
From:   Atish Patra <Atish.Patra@....com>
To:     "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
        "anup@...infault.org" <anup@...infault.org>
CC:     "alankao@...estech.com" <alankao@...estech.com>,
        "daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        Anup Patel <Anup.Patel@....com>,
        "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "johan@...nel.org" <johan@...nel.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "palmer@...ive.com" <palmer@...ive.com>,
        "allison@...utok.net" <allison@...utok.net>
Subject: Re: [PATCH 3/4] RISC-V: Support case insensitive ISA string parsing.

On Sat, 2019-07-27 at 01:16 -0700, Paul Walmsley wrote:
> On Sat, 27 Jul 2019, Anup Patel wrote:
> 
> > If your only objection is uppercase letter not agreeing with YMAL
> > schema
> > then why not fix the YMAL schema to have regex for RISC-V ISA
> > string?
> 
> I don't agree with you that the specification compels software to
> accept 
> arbitrary case combinations in the riscv,isa DT string.
> 
> > The YMAL schema should not enforce any artificial restriction which
> > is
> > theoretically allowed in the RISC-V spec.
> 
> Unless someone can come up with a compelling reason for why
> restricting 
> the DT ISA strings to all lowercase letters and numbers is
> insufficient to 
> express the full range of options in the spec,

The yaml document did not specify anything about all isa-strings has to
be lowercase unless I missed something. The two enum values are all
lowercase but the description says all ISA strings are documented in
ISA specification which describes the ISA strings to be case
insensitive. IMHO, this creates confusion resulting the patch.
The existing enum strings are already outdated with kvm patchset.

I am fine with dropping this patch if yaml clearly document the case
sensititve thing.

Following approaches can done to avoid this confusion in future.

1. Update the enum strings with every new extension added.
	- Good chance that somebody miss something and this gets
outdated in future.

2. Just add one line in DT binding description saying that 

"All isa strings has to be lowercase strings". We should mandate this
in Unix Platform specification as well to be in sync.

Thoughts ?

>  the additional complexity 
> to add mixed-case parsing, both in this patch and in the other
> patches in 
> this series, seems pointless.
> 

> 
> - Paul
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

-- 
Regards,
Atish

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