Re: [PATCH v2] RISC-V: Don't trust V from the riscv,isa DT property on T-Head CPUs

[Conor Dooley <conor@kernel.org>]

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

On Fri, Jul 14, 2023 at 01:12:32AM +0800, Jisheng Zhang wrote:
> On Thu, Jul 13, 2023 at 06:04:22PM +0100, Conor Dooley wrote:
> > Jumping on top of Palmer's reply cos I had already started replying...
> > On Thu, Jul 13, 2023 at 09:56:34AM -0700, Palmer Dabbelt wrote:
> > > On Thu, 13 Jul 2023 09:36:49 PDT (-0700), jszhang@kernel.org wrote:
> > > > On Wed, Jul 12, 2023 at 06:48:02PM +0100, Conor Dooley wrote:

> > > > FWICT, new T-HEAD's riscv cores such as C908 support standard RVV-1.0,
> > > > this patch looks like a big hammer for T-HEAD. I do understand why
> > > 
> > > Ya, it's a big hammer.  There's no extant systems with the C908, though, and
> > > given that the C906 and C910 alias marchid/mimplid it's kind of hard to
> > > trust any of those values for T-Head systems.  We could check for the 0s and
> > > hope T-Head starts setting something else, but I'm not sure that's a net win
> > > (we've also got the C920 in the Sophgo chip, which IIUC is V-0.7.1 too).
> > 
> > (In reply to Jisheng mostly)
> > It is most definitely a big hammer. And yes, we did talk about the c908
> > & its standard implementation of vector before submitting this. Unless
> > Guo can confirm that the c908 (and later CPU cores) will start setting
> > mimpid & mvendorid, I don't really see what the alternatives are? *
> 
> In mainline kernel, three SoCs which powered by T-HEAD cpu are
> supported: D1, D1s and TH1520, they don't contain the "v" in riscv,isa
> dt property.

Yup, and they will stay that way ;)

> > Whacking in a list of DT compatibles to blacklist? That doesn't seem
> > like something that would scale.
> > Open to ideas on that front for sure, smaller hammers are always better!
> > 
> > @Palmer, from what I am told, the c920 does put zeros in those CSRs,
> > so we are okay on that front.
> > 
> > * If they do do something other than 0s, the errata handling will need
> >   an update anyway, so the big hammer could be revised in tandem...
> > 
> > > > this patch is provided, but can we mitigate the situation by carefully
> > > > review the DTs? Per my understanding, dts is also part of linux kernel.
> > > 
> > > That would break compatibility with existing firmware.  It's certainly
> > > something that has happened before, but we try to avoid it where possible.
> > 
> > (Mostly in reply to Jisheng again)
> > Sure, some devicetrees are part of the kernel, but not all are - they may
> > be passed up from U-Boot or OpenSBI etc & contain "v" in riscv,isa.
> 
> If so this looks like a bug of u-boot and opensbi.
> 
> PS: does u-boot/opensbi modify "riscv,isa" property dynamically? Or
> there's below usage case:
> mainline linux kernel + dtb which is built from u-boot/opensbi source
> code rather than linux kernel.

Its the latter I am thinking of. If someone wants to go and double check
that there are no vendors shipping T-Head cores with firmware that
behaves that way, then the patch could I suppose be dropped.


[-- 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