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

[Yangyu Chen]

> On Mar 6, 2024, at 01:01, Conor Dooley <conor@xxxxxxxxxx> wrote:
> 
> 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@xxxxxxxxxxxx> wrote:
>>>> On Mar 5, 2024, at 23:54, Palmer Dabbelt <palmer@xxxxxxxxxxx> wrote:
>>>> On Mon, 04 Mar 2024 13:06:00 PST (-0800), cyy@xxxxxxxxxxxx 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.
> 

The patched opensbi is only for work around now as the factory-provided
M-Mode uboot will enable MAEE. I would like option 1 to let the bootloader
choose what to use and patch the DT to tell the linux whether should use
T-Head MAEE as T-Head is likely to use this feature on TEE
(Trusted Execution Environment), so it might never be removed.

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

It in the k230.dtsi file. Only has big core there.