Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts

From: Conor Dooley <conor@kernel.org>
To: Yangyu Chen <cyy@cyyself.name>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
	linux-riscv@lists.infradead.org, dlemoal@kernel.org,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	guoren@kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v3 6/7] riscv: dts: add initial canmv-k230 and k230-evb dts
Date: Tue, 5 Mar 2024 17:01:34 +0000	[thread overview]
Message-ID: <20240305-shorten-disallow-eae65fdc8fdb@spud> (raw)
In-Reply-To: <tencent_B2C1146D4947315C8018AE90EDC64F3E6207@qq.com>

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

Hey,

Just chiming so that things don't get misinterpreted.

On Wed, Mar 06, 2024 at 12:37:16AM +0800, Yangyu Chen wrote:
> > On Mar 6, 2024, at 00:22, Yangyu Chen <cyy@cyyself.name> wrote:
> >> On Mar 5, 2024, at 23:54, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> >> On Mon, 04 Mar 2024 13:06:00 PST (-0800), cyy@cyyself.name wrote:
> >>> - Svpbmt and T-Head MAEE both supported
> >>> 
> >>> T-Head C908 does support both Svpbmt and T-Head MAEE for page-based memory
> >>> attributes and is controlled by csr.mxstatus. If the kernel wants to use
> >>> svpbmt, the m-mode software should set BIT(21) of csr.mxstatus to zero
> >>> before entering the s-mode kernel. Otherwise, the kernel will not boot as 0
> >>> on T-Head MAEE represent to NonCachable Memory and it will lose dirty cache
> >>> lines modification that haven't been written back to the memory.
> >> 
> >> So I guess we need the bootloader to just be accurate here?  ie: whatever
> >> extension it tells S-mode kernels is enabled is how S-mode behaves, and
> >> then this should just work itself out.

Correct, the bootloader/firmware "just" needs to write this bit to match
what it passes to onwards in the devicetree.

> > Yes. Currently, I have patched OpenSBI to disable MAEE. Conor Dooley said
> > from a public irc group that he wants to control the enable of T-Head

(#riscv on libera, the usual location)

I also suggested that that, given we can use the standard extensions,
we should use them instead of the custom extensions/errata.

> > variation of zicbom and svpbmt from dts, in addition to mimplid or
> > something now.

Correct. I'm find with the impid == archid == 0 condition, given that's
what we need to keep to avoid regressions, but if any future T-Head CPUs
want to enable MAEE (ERRATA_THEAD_PBMT) or the custom CMOs
(ERRATA_THEAD_CMO) these should be enabled from DT. Particularly when
these CPUs can be configured to either use the T-Head versions or the
standard extensions.

> > I think that will be a better way for the bootloader to tell
> > the kernel whether the T-Head MAEE should be enabled.

You've got three options I guess. You could patch the DT in the bootloader,
or use a fixed DT that matches the bootloader, or you could use the DT
passed to the bootloader and parse the extensions to decide whether or not
to enable MAEE or Svpbmt. Seems you're going for option 2.

> > Link: https://github.com/cyyself/opensbi/commit/b113c1c01d700314a4a696297ec09031a9399354
> > 
> > Furthermore, I wonder whether a CPU node like this would be acceptable.
> > I don't have any other details of how another CPU from K230 SoC works on
> > Linux.

A CPU node like what? It is not clear to me.

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