Re: [PATCH v1 1/2] RISC-V: skip parsing multi-letter extensions starting with caps

From: Conor Dooley <conor@kernel.org>
To: Yangyu Chen <cyy@cyyself.name>
Cc: ajones@ventanamicro.com, conor.dooley@microchip.com,
	devicetree@vger.kernel.org, i@zenithal.me, krzk+dt@kernel.org,
	linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	paul.walmsley@sifive.com, robh+dt@kernel.org, soha@lohu.info,
	twd2.me@gmail.com
Subject: Re: [PATCH v1 1/2] RISC-V: skip parsing multi-letter extensions starting with caps
Date: Wed, 26 Apr 2023 18:47:58 +0100	[thread overview]
Message-ID: <20230426-candy-deceiver-b3ff230bf7f6@spud> (raw)
In-Reply-To: <tencent_0F23181FB02085B690E30BEE4BCC49087506@qq.com>

[-- Attachment #1.1: Type: text/plain, Size: 2484 bytes --]

On Thu, Apr 27, 2023 at 01:11:00AM +0800, Yangyu Chen wrote:
> Hi,
> 
> On Wed, 26 Apr 2023 15:37:58 +0100, Conor Dooley wrote:
> > Perhaps the thing to do is to actually take Yangyu's first patch and my
> > second one, since the problem with backwards compatibility doesn't stop
> > the kernel from being more permissive?
> 
> How about taking my first patch[1] since the ECR[2] mentioned that
> the format of the ISA string is defined in the RISC-V unprivileged
> specification?

That is what I suggested, no? Your 1/2 with a revised subject noting
that ACPI may need it, rather than talking about DT. See also my
comments to Drew about the "perils" of letting undefined spec versions
have control over your ABI.

> However, I think we still need to stop the parser if
> some characters that the parser is not able to handle as Andrew Jones
> mentioned in the previous mail[3]. In this case, we still need to add
> some code to stop parsing if any error happens.

Currently it skips extensions that are not valid, but keeps parsing.
Apart from the case where SZX are capital letters it "should" either
parse into something resembling correct or skip. If we start parsing
in a case-invariant manner, I don't see any immediately problem with
what we currently have.

I just don't really get what we need to "protect" the kernel from.
If someone controls the dtb, I think what they do to the isa string is
probably the least of our worries.

> In my humble opinion, backward compatibility can be solved by backports
> to LTS kernels.

The binding might lie in Linux, but that does not mean we can fix the
problem by backporting parser changes to stable. There are other users
and Linux kernels that would pre-date the change that we would be
inflicting this relaxation on for no benefit at all. U-Boot for example
does a case-sensitive comparison.

> I think the better option is to allow using uppercase
> letters in the device-tree document to make the parser coherent with
> RISC-V ISA specification but recommend using all lowercase letters for
> better backward compatibility.

Any addition of uppercase to that binding will get my NAK.
There is no realistic benefit to doing so, only potential for disruption.
DT generators should emit DT that complies with bindings ¯\_(ツ)_/¯.

I'll go take a proper look at your 1/2 from the other day. I had a
comment about it that I didn't leave, but will do so now.

Thanks,
Conor.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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