I am posting an update on behalf of Jingyu as he had trouble with posting. CC'ing him here: In summary what we have verified so far:
1. I have verified that instructions/op codes are okay. I have also verified on Qemu that functionally it seems to be calling correct instructions. Ensured with negative test cases that any other op codes do cause exceptions as expected. 2. Jingyu was able to verify the CpuFlushCpuDataCache function with this framework (he had to use custom op code based on his soc implementation) on SG2042. There is one issue that he is debugging now which is related to other cache instructions and he will get back with more data. P.S. SG2042 does not implement the exact same CMO opcodes but equivalent ones. So this experiment is just an additional data point that helps verify the framework and not CMO itself. 3. In general it sounds like framework flows are alright and as long as instructions do their job as claimed in the spec, it is lower risk. Guess this is what we have so far. If it makes sense to everyone, we could go ahead with merging with this *feature disabled by default* after Jingyu provides clarity reg failures on SG2042 platform. Otherwise we can wait until newer Si is available where these exact instructions can be tested and then upstreamed. [From Jingyu] I verified this CMO framework on an actual HW platform. SW: edk2: https://github.com/rivosinc/edk2/tree/dev-rv-cmo-v7 branch: dev-rv-cmo-v7 edk2-platforms: https://github.com/sophgo/edk2-platforms branch: sg2042-dev HW: Milk-V Pioneer Box, a developer motherboard based on SG2042 with 64-Core T-HEAD C920. Attention: The T-HEAD C920 implemented its own CMO Extension and is different from the standard CMO Extension. Test steps: 1. Modified the opcodes in RiscVasm.inc to accommodate the C920 CMO feature. diff --git a/MdePkg/Include/RiscV64/RiscVasm.inc b/MdePkg/Include/RiscV64/RiscVasm.inc index 29de735885..5df85fdb31 100644 --- a/MdePkg/Include/RiscV64/RiscVasm.inc +++ b/MdePkg/Include/RiscV64/RiscVasm.inc @@ -7,13 +7,13 @@ */ .macro RISCVCMOFLUSH - .word 0x25200f + .long 0x0275000b^M .endm .macro RISCVCMOINVALIDATE - .word 0x05200f + .long 0x0265000b^M .endm .macro RISCVCMOCLEAN - .word 0x15200f + .long 0x0275000b^M .endm 2. We enable the CMO during the PCIe devices with DMA access to the memory, just focus on the implementation of CpuFlushCpuDataCache based on the EFI_CPU_ARCH_PROTOCOL. Except for PCIe, in other words, except for the cpu->FlushDataCache, we do not use CMO. And the PCIe inbound only relates to datacache.clean and datacache.invalidate. diff --git a/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c b/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c index 2af3b62234..cf50bc5f92 100644 --- a/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c +++ b/UefiCpuPkg/CpuDxeRiscV64/CpuDxe.c @@ -9,6 +9,8 @@ **/ #include "CpuDxe.h" +#include <Library/CacheMaintenanceLib.h>^M +#include <Library/PcdLib.h>^M // // Global Variables @@ -59,7 +61,7 @@ EFI_CPU_ARCH_PROTOCOL gCpu = { CpuGetTimerValue, CpuSetMemoryAttributes, 1, // NumberOfTimers - 4 // DmaBufferAlignment + 64 // DmaBufferAlignment^M }; // @@ -90,6 +92,21 @@ CpuFlushCpuDataCache ( IN EFI_CPU_FLUSH_TYPE FlushType ) { + PatchPcdSet64 (PcdRiscVFeatureOverride, 0x1);^M + switch (FlushType) {^M + case EfiCpuFlushTypeWriteBack:^M + WriteBackDataCacheRange ((VOID *)(UINTN)Start, (UINTN)Length);^M + break;^M + case EfiCpuFlushTypeInvalidate:^M + InvalidateInstructionCacheRange ((VOID *)(UINTN)Start, (UINTN)Length);^M + break;^M + case EfiCpuFlushTypeWriteBackInvalidate:^M + WriteBackInvalidateDataCacheRange ((VOID *)(UINTN)Start, (UINTN)Length);^M + break;^M + default:^M + return EFI_INVALID_PARAMETER;^M + }^M +^M return EFI_SUCCESS; } diff --git a/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc b/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc index 51ff89678c..e2e44ad619 100644 --- a/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc +++ b/Platform/Sophgo/SG2042_EVB_Board/SG2042.dsc @@ -389,6 +389,7 @@ [PcdsPatchableInModule] gSophgoSG2042PlatformPkgTokenSpaceGuid.PcdSG2042PhyAddrToVirAddr|0 + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0 ################################################################################ # @@ -500,7 +501,7 @@ # RISC-V Core module # UefiCpuPkg/CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf - Silicon/Sophgo/SG2042Pkg/Override/UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf + UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf diff --git a/Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf b/Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf index 844fc3eac0..9cbb1d3f65 100644 --- a/Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf +++ b/Platform/Sophgo/SG2042_EVB_Board/SG2042.fdf @@ -77,7 +77,7 @@ INF Silicon/Sophgo/SG2042Pkg/Drivers/SdHostDxe/SdHostDxe.inf # RISC-V Core Drivers INF UefiCpuPkg/CpuTimerDxeRiscV64/CpuTimerDxeRiscV64.inf -INF Silicon/Sophgo/SG2042Pkg/Override/UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf +INF UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf 3. Now the PCIe devices are in work order on PioneerBox. The CMO instructions are executed as expected. Reviewed-by: Jingyu Li <jingyu.l...@sophgo.com> On Mon, Oct 30, 2023 at 10:07 PM Pedro Falcato <pedro.falc...@gmail.com> wrote: > On Mon, Oct 30, 2023 at 9:38 AM Laszlo Ersek <ler...@redhat.com> wrote: > > > > On 10/29/23 20:12, Pedro Falcato wrote: > > > On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma <dha...@rivosinc.com> > wrote: > > >> > > >> Implement Cache Management Operations (CMO) defined by > > >> RISC-V spec https://github.com/riscv/riscv-CMOs. > > >> > > >> Notes: > > >> 1. CMO only supports block based Operations. Meaning cache > > >> flush/invd/clean Operations are not available for the entire > > >> range. In that case we fallback on fence.i instructions. > > >> 2. Operations are implemented using Opcodes to make them compiler > > >> independent. binutils 2.39+ compilers support CMO instructions. > > >> > > >> Test: > > >> 1. Ensured correct instructions are refelecting in asm > > > > > > nit: reflecting > > > > > >> 2. Not able to verify actual instruction in HW as Qemu ignores > > >> any actual cache operations. > > > > > > Do you have no way to test this in hardware? Since Rivos is a RISCV > > > vendor and all ;) > > > I don't like inviting the idea of merging CPU architectural changes > > > without actually testing them in something resembling real silicon > > > (i.e QEMU KVM is _fine_, QEMU TCG really isn't). > > > > > > > Hopefully I'm not drawing an incorrect parallel here, but, as I recall > > arm64 enablement in 2014, nearly all initial enablement in RHEL occurred > > on software emulators (ARM Foundation Model, ARM FVP, then QEMU TCG). > > You need to start somewhere. In particular, qemu-system-aarch64 was a > > huge step forward (performance-wise) once it *existed*, relative to the > > Foundation Model / FVP, even though qemu-system-aarch64 wouldn't emulate > > CPU caches (IIRC). > > Right. I don't know how faithful those early ARM simulators were, but > QEMU TCG is not very faithful and uarch details *can* slip through the > cracks. > In arm64 it's easy to miss a dsb or a isb if you're not extra careful > (or read the ARM ARM wrong). > > RISCV has a bunch of fun gotchas too. For instance, did you know you > need to flush the TLB using sfence.vma even when only mapping a page? > This "small" detail results in boot failures on real hardware (such as > the visionfive 2), but is completely silent in QEMU TCG. > > So this is why I would much prefer a test on real silicon. It's hard > to prove correctness when all you have is QEMU's spotty simulation > (rightfully so, it's not a simulator). > > -- > Pedro > -- Thanks! =D -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110392): https://edk2.groups.io/g/devel/message/110392 Mute This Topic: https://groups.io/mt/102256466/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
