search

kendryte / k230_sdk

Kendryte K230 SDK
BSD 2-Clause "Simplified" License
165 stars 35 forks source link

fix issue #48 #53

Open yf13 opened 7 months ago

yf13 commented 7 months ago

PR描述:

Per discussions in issue#48, we should leave PMP to firmware or OS thus allowing mainline OpenSBI or different OS be loaded.

MAEE extension should also be turned off for standard compliance.

These are all needed to support other RTOS like NuttX.

关联issue:

fix #48

xiangbingj commented 7 months ago

Hi,It is not good to give up pmp protection. I think it can be set using pmpcfg2.we will try it.

Our Linux/rtthread used MAEE extension,If it is not set by default in uboot, where is the appropriate place to set it?

yf13 commented 7 months ago

@xiangbingj thanks for the explanations. Discussions in issue#48 seemed accepting those changes, but actually MAEE is essential for SDK Linux.

Are there any concerns for moving these PMP and MAEE settings into SBI? it seems that big core settings is done in SBI.

Or can we lock PMP and enable MAEE only for the "k230_boot" flow and leave unlocked PMP and standard PTE for other commands like "boot_baremetal", "bootm", "go" etc? This way, the same U-Boot can support both SDK and non-SDK scenarios.

ConchuOD commented 6 months ago

@xiangbingj thanks for the explanations. Discussions in issue#48 seemed accepting those changes, but actually MAEE is essential for SDK Linux.

Are there any concerns for moving these PMP and MAEE settings into SBI? it seems that big core settings is done in SBI.

Or can we lock PMP and enable MAEE only for the "k230_boot" flow and leave unlocked PMP and standard PTE for other commands like "boot_baremetal", "bootm", "go" etc? This way, the same U-Boot can support both SDK and non-SDK scenarios.

I think that setting these kinds of things in the SBI implemenation (in your case OpenSBI) makes a lot of sense. OpenSBI already does quite a lot with CSRs etc already and from an "idealogical" perspective it is the place to do this in my opinion. To your linux SDK it should not matter which M-Mode firmware (U-Boot spl or OpenSBI) set the CSRs up, as long as they are configured by the time the kernel has booted.

OpenSBI has to be replaced to boot a mainline kernel on systems that use T-Head CPUs due to the vendor OpenSBI being very hold, so anyone that wants to run mainline is going to be replacing OpenSBI anyway.

CC @cyyself :)

cyyself commented 6 months ago

@xiangbingj thanks for the explanations. Discussions in issue#48 seemed accepting those changes, but actually MAEE is essential for SDK Linux. Are there any concerns for moving these PMP and MAEE settings into SBI? it seems that big core settings is done in SBI. Or can we lock PMP and enable MAEE only for the "k230_boot" flow and leave unlocked PMP and standard PTE for other commands like "boot_baremetal", "bootm", "go" etc? This way, the same U-Boot can support both SDK and non-SDK scenarios.

I think that setting these kinds of things in the SBI implemenation (in your case OpenSBI) makes a lot of sense. OpenSBI already does quite a lot with CSRs etc already and from an "idealogical" perspective it is the place to do this in my opinion. To your linux SDK it should not matter which M-Mode firmware (U-Boot spl or OpenSBI) set the CSRs up, as long as they are configured by the time the kernel has booted.

OpenSBI has to be replaced to boot a mainline kernel on systems that use T-Head CPUs due to the vendor OpenSBI being very hold, so anyone that wants to run mainline is going to be replacing OpenSBI anyway.

CC @cyyself :)

Thanks for CCing me. The original issue #48 was proposed by me and you CCed me might be because of some recent emails from linux-riscv mailing list about builtin dtb. The key on k230_sdk is that once PMP is locked by m-mode u-boot, it can not unlock until CPU reset, which prevents these CSRs from being set by mainline OpenSBI again which is loaded by u-boot m-mode. That's why I submitted a series of patches to OpenSBI to skip the locked PMP when doing the PMP probe.

As for MAEE, I think the most ideal way is to provide a new custom sbi call for the kernel to switch it on and turned off by default for better compatibility to allow the mainline kernel to boot. However, it may need some extra effort to do so.

cyyself commented 6 months ago

@xiangbingj thanks for the explanations. Discussions in issue#48 seemed accepting those changes, but actually MAEE is essential for SDK Linux.

Are there any concerns for moving these PMP and MAEE settings into SBI? it seems that big core settings is done in SBI.

Or can we lock PMP and enable MAEE only for the "k230_boot" flow and leave unlocked PMP and standard PTE for other commands like "boot_baremetal", "bootm", "go" etc? This way, the same U-Boot can support both SDK and non-SDK scenarios.

To be honest, I don't know why we should use set locked PMP in M-Mode U-Boot rather than add a new memory region to OpenSBI to prevent it from being accessed by S-Mode software.

ConchuOD commented 6 months ago

@xiangbingj thanks for the explanations. Discussions in issue#48 seemed accepting those changes, but actually MAEE is essential for SDK Linux. Are there any concerns for moving these PMP and MAEE settings into SBI? it seems that big core settings is done in SBI. Or can we lock PMP and enable MAEE only for the "k230_boot" flow and leave unlocked PMP and standard PTE for other commands like "boot_baremetal", "bootm", "go" etc? This way, the same U-Boot can support both SDK and non-SDK scenarios.

To be honest, I don't know why we should use set locked PMP in M-Mode U-Boot rather than add a new memory region to OpenSBI to prevent it from being accessed by S-Mode software.

I thought the purpose was twofold - you add a reserved memory region to tell an operating system not to use it & you use a PMP to enforce the security boundary etc, so that S-Mode cannot interfer.

ConchuOD commented 6 months ago

Thanks for CCing me. The original issue #48 was proposed by me and you CCed me might be because of some recent emails from linux-riscv mailing list about builtin dtb.

No, it was because I hadn't seen you comment here and the PR seemed stalled waiting for feedback :)

The key on k230_sdk is that once PMP is locked by m-mode u-boot, it can not unlock until CPU reset, which prevents these CSRs from being set by mainline OpenSBI again which is loaded by u-boot m-mode. That's why I submitted a series of patches to OpenSBI to skip the locked PMP when doing the PMP probe.

Yeah, I understood that - I was trying to agree with you that setting up the PMPs OpenSBI and not in M-Mode is a good idea since it will allow supporting mainline linux without having to replace the first stage bootloader.

As for MAEE, I think the most ideal way is to provide a new custom sbi call for the kernel to switch it on and turned off by default for better compatibility to allow the mainline kernel to boot. However, it may need some extra effort to do so.

I dunno. The main user of MAEE is T-Head themselves, and given the state of the 5.10 kernel they use I doubt they'll be implementing an ecall there to enable it. Given mainline supports Svpbmt, I'd rather not do anything other than use Svpbmt :)

The least effort thing to do, to me, would be to move the enablement of MAEE into OpenSBI for the SDK. The SDK version of linux, using the vendor kernel would then continue to function and those of use who want to run mainline linux (and therefore mainline OpenSBI too) can have MAEE disabled.