[edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
Use newly defined cache management operations for RISC-V where possible It builds up on the support added for RISC-V cache management instructions in BaseLib. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Cc: Laszlo Ersek Signed-off-by: Dhaval Sharma Acked-by: Laszlo Ersek --- Notes: V7: - Added PcdLib - Restructure DEBUG message based on feedback on V6 - Make naming consistent to CMO, remove all CBO references - Add ASSERT for not supported functions instead of plain debug message - Added RB tag V6: - Utilize cache management instructions if HW supports it This patch is part of restructuring on top of v5 MdePkg/MdePkg.dec | 8 + MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 +--- MdePkg/MdePkg.uni | 4 + 4 files changed, 165 insertions(+), 20 deletions(-) diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index ac54338089e8..fa92673ff633 100644 --- a/MdePkg/MdePkg.dec +++ b/MdePkg/MdePkg.dec @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, PcdsPatchableInModule.AARCH64] # @Prompt CPU Rng algorithm's GUID. gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] + # + # Configurability to override RISC-V CPU Features + # BIT 0 = Cache Management Operations. This bit is relevant only if + # previous stage has feature enabled and user wants to disable it. + # + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 + [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] ## This value is used to set the base address of PCI express hierarchy. # @Prompt PCI Express Base Address. diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf index 6fd9cbe5f6c9..601a38d6c109 100644 --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf @@ -56,3 +56,8 @@ [LibraryClasses] BaseLib DebugLib +[LibraryClasses.RISCV64] + PcdLib + +[Pcd.RISCV64] + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c index 4eb18edb9aa7..5b3104afb67e 100644 --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c @@ -2,6 +2,7 @@ RISC-V specific functionality for cache. Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights reserved. + Copyright (c) 2023, Rivos Inc. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent **/ @@ -9,10 +10,115 @@ #include #include #include +#include + +// +// TODO: Grab cache block size and make Cache Management Operation +// enabling decision based on RISC-V CPU HOB in +// future when it is available. +// +#define RISCV_CACHE_BLOCK_SIZE 64 +#define RISCV_CPU_FEATURE_CMO_BITMASK 0x1 + +typedef enum { + Clean, + Flush, + Invld, +} CACHE_OP; + +/** +Verify CBOs are supported by this HW +TODO: Use RISC-V CPU HOB once available. + +**/ +STATIC +BOOLEAN +RiscVIsCMOEnabled ( + VOID + ) +{ + // If CMO is disabled in HW, skip Override check + // Otherwise this PCD can override settings + return ((PcdGet64 (PcdRiscVFeatureOverride) & RISCV_CPU_FEATURE_CMO_BITMASK) != 0); +} + +/** + Performs required opeartion on cache lines in the cache coherency domain + of the calling CPU. If Address is not aligned on a cache line boundary, + then entire cache line containing Address is operated. If Address + Length + is not aligned on a cache line boundary, then the entire cache line + containing Address + Length -1 is operated. + If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT(). + @param Address The base address of the cache lines to + invalidate. + @param Length The number of bytes to invalidate from the instruction + cache. + @param Op Type of CMO operation to be performed + @return Address. + +**/ +STATIC +VOID +CacheOpCacheRange ( + IN VOID *Address, + IN UINTN Length, + IN CACHE_OP Op + ) +{ + UINTN CacheLineSize; + UINTN Start; + UINTN End; + + if (Length == 0) { +return; + } + + if ((Op != Invld) && (Op != Flush) && (Op != Clean)) { +return; + } + + ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Address)); + + CacheLineSize = RISCV_CACHE_BLOCK_SIZE; + + Start = (UINTN)Address; + // + // Calculate the cache line alignment + // + End= (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize - 1); + Start &
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma wrote: > > Use newly defined cache management operations for RISC-V where possible > It builds up on the support added for RISC-V cache management > instructions in BaseLib. > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Zhiguang Liu > Cc: Laszlo Ersek > > Signed-off-by: Dhaval Sharma > Acked-by: Laszlo Ersek > --- > > Notes: > V7: > - Added PcdLib > - Restructure DEBUG message based on feedback on V6 > - Make naming consistent to CMO, remove all CBO references > - Add ASSERT for not supported functions instead of plain debug message > - Added RB tag > V6: > - Utilize cache management instructions if HW supports it > This patch is part of restructuring on top of v5 > > MdePkg/MdePkg.dec | 8 + > MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 > +--- > MdePkg/MdePkg.uni | 4 + > 4 files changed, 165 insertions(+), 20 deletions(-) > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > index ac54338089e8..fa92673ff633 100644 > --- a/MdePkg/MdePkg.dec > +++ b/MdePkg/MdePkg.dec > @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, > PcdsPatchableInModule.AARCH64] ># @Prompt CPU Rng algorithm's GUID. > > gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 > > +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] > + # > + # Configurability to override RISC-V CPU Features > + # BIT 0 = Cache Management Operations. This bit is relevant only if > + # previous stage has feature enabled and user wants to disable it. > + # > + > gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 > + > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] >## This value is used to set the base address of PCI express hierarchy. ># @Prompt PCI Express Base Address. > diff --git > a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > index 6fd9cbe5f6c9..601a38d6c109 100644 > --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > @@ -56,3 +56,8 @@ [LibraryClasses] >BaseLib >DebugLib > > +[LibraryClasses.RISCV64] > + PcdLib > + > +[Pcd.RISCV64] > + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > index 4eb18edb9aa7..5b3104afb67e 100644 > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > @@ -2,6 +2,7 @@ >RISC-V specific functionality for cache. > >Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights > reserved. > + Copyright (c) 2023, Rivos Inc. All rights reserved. > >SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > @@ -9,10 +10,115 @@ > #include > #include > #include > +#include > + > +// > +// TODO: Grab cache block size and make Cache Management Operation > +// enabling decision based on RISC-V CPU HOB in > +// future when it is available. > +// > +#define RISCV_CACHE_BLOCK_SIZE 64 > +#define RISCV_CPU_FEATURE_CMO_BITMASK 0x1 > + > +typedef enum { > + Clean, > + Flush, > + Invld, > +} CACHE_OP; small nit: You may want to do something like CACHE_OP_{CLEAN, FLUSH, INVID}. but since this is a file-local enum I don't consider this merge-blocking. -- Pedro -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110275): https://edk2.groups.io/g/devel/message/110275 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On 10/29/23 20:07, Pedro Falcato wrote: > On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma wrote: >> >> Use newly defined cache management operations for RISC-V where possible >> It builds up on the support added for RISC-V cache management >> instructions in BaseLib. >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> Cc: Laszlo Ersek >> >> Signed-off-by: Dhaval Sharma >> Acked-by: Laszlo Ersek >> --- >> >> Notes: >> V7: >> - Added PcdLib >> - Restructure DEBUG message based on feedback on V6 >> - Make naming consistent to CMO, remove all CBO references >> - Add ASSERT for not supported functions instead of plain debug message >> - Added RB tag >> V6: >> - Utilize cache management instructions if HW supports it >> This patch is part of restructuring on top of v5 >> >> MdePkg/MdePkg.dec | 8 + >> MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + >> MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 >> +--- >> MdePkg/MdePkg.uni | 4 + >> 4 files changed, 165 insertions(+), 20 deletions(-) >> >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec >> index ac54338089e8..fa92673ff633 100644 >> --- a/MdePkg/MdePkg.dec >> +++ b/MdePkg/MdePkg.dec >> @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, >> PcdsPatchableInModule.AARCH64] >># @Prompt CPU Rng algorithm's GUID. >> >> gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 >> >> +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] >> + # >> + # Configurability to override RISC-V CPU Features >> + # BIT 0 = Cache Management Operations. This bit is relevant only if >> + # previous stage has feature enabled and user wants to disable it. >> + # >> + >> gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 >> + >> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] >>## This value is used to set the base address of PCI express hierarchy. >># @Prompt PCI Express Base Address. >> diff --git >> a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> index 6fd9cbe5f6c9..601a38d6c109 100644 >> --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> @@ -56,3 +56,8 @@ [LibraryClasses] >>BaseLib >>DebugLib >> >> +[LibraryClasses.RISCV64] >> + PcdLib >> + >> +[Pcd.RISCV64] >> + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES >> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> index 4eb18edb9aa7..5b3104afb67e 100644 >> --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> @@ -2,6 +2,7 @@ >>RISC-V specific functionality for cache. >> >>Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights >> reserved. >> + Copyright (c) 2023, Rivos Inc. All rights reserved. >> >>SPDX-License-Identifier: BSD-2-Clause-Patent >> **/ >> @@ -9,10 +10,115 @@ >> #include >> #include >> #include >> +#include >> + >> +// >> +// TODO: Grab cache block size and make Cache Management Operation >> +// enabling decision based on RISC-V CPU HOB in >> +// future when it is available. >> +// >> +#define RISCV_CACHE_BLOCK_SIZE 64 >> +#define RISCV_CPU_FEATURE_CMO_BITMASK 0x1 >> + >> +typedef enum { >> + Clean, >> + Flush, >> + Invld, >> +} CACHE_OP; > > small nit: You may want to do something like CACHE_OP_{CLEAN, FLUSH, > INVID}. but since this is a file-local enum I don't consider this > merge-blocking. > Agree with you on the prefix, but not on the uppercase spelling; edk2 (and UEFI) uses CamelCase enumeration constants. Compare: at the very top of "MdePkg/Include/Uefi/UefiSpec.h", we have EFI_ALLOCATE_TYPE with constants like AllocateAnyPages. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110304): https://edk2.groups.io/g/devel/message/110304 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On Sun, Oct 29, 2023 at 08:16:12PM +0530, Dhaval Sharma wrote: > Use newly defined cache management operations for RISC-V where possible > It builds up on the support added for RISC-V cache management > instructions in BaseLib. > Cc: Michael D Kinney > Cc: Liming Gao > Cc: Zhiguang Liu > Cc: Laszlo Ersek > > Signed-off-by: Dhaval Sharma > Acked-by: Laszlo Ersek > --- > > Notes: > V7: > - Added PcdLib > - Restructure DEBUG message based on feedback on V6 > - Make naming consistent to CMO, remove all CBO references > - Add ASSERT for not supported functions instead of plain debug message > - Added RB tag > V6: > - Utilize cache management instructions if HW supports it > This patch is part of restructuring on top of v5 > > MdePkg/MdePkg.dec | 8 + > MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 > +--- > MdePkg/MdePkg.uni | 4 + > 4 files changed, 165 insertions(+), 20 deletions(-) > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > index ac54338089e8..fa92673ff633 100644 > --- a/MdePkg/MdePkg.dec > +++ b/MdePkg/MdePkg.dec > @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, > PcdsPatchableInModule.AARCH64] ># @Prompt CPU Rng algorithm's GUID. > > gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 > > +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] > + # > + # Configurability to override RISC-V CPU Features > + # BIT 0 = Cache Management Operations. This bit is relevant only if > + # previous stage has feature enabled and user wants to disable it. NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that it is explicit. > + # > + > gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 > + Instead of this, can default value match only those features which are enabled by default for qemu virt machine? That way, I think we can avoid having this PCD defined again in RiscVVirt. > [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] >## This value is used to set the base address of PCI express hierarchy. ># @Prompt PCI Express Base Address. > diff --git > a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > index 6fd9cbe5f6c9..601a38d6c109 100644 > --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > @@ -56,3 +56,8 @@ [LibraryClasses] >BaseLib >DebugLib > > +[LibraryClasses.RISCV64] > + PcdLib > + > +[Pcd.RISCV64] > + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > index 4eb18edb9aa7..5b3104afb67e 100644 > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > @@ -2,6 +2,7 @@ >RISC-V specific functionality for cache. > >Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights > reserved. > + Copyright (c) 2023, Rivos Inc. All rights reserved. > >SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > @@ -9,10 +10,115 @@ > #include > #include > #include > +#include > + > +// > +// TODO: Grab cache block size and make Cache Management Operation > +// enabling decision based on RISC-V CPU HOB in > +// future when it is available. > +// > +#define RISCV_CACHE_BLOCK_SIZE 64 > +#define RISCV_CPU_FEATURE_CMO_BITMASK 0x1 > + Can we define these bits in the header file so that the definitions can be used by multiple modules? Thanks, Sunil -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110308): https://edk2.groups.io/g/devel/message/110308 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On Mon, Oct 30, 2023 at 04:48:18PM +0530, Sunil V L wrote: > On Sun, Oct 29, 2023 at 08:16:12PM +0530, Dhaval Sharma wrote: > > Use newly defined cache management operations for RISC-V where possible > > It builds up on the support added for RISC-V cache management > > instructions in BaseLib. > > Cc: Michael D Kinney > > Cc: Liming Gao > > Cc: Zhiguang Liu > > Cc: Laszlo Ersek > > > > Signed-off-by: Dhaval Sharma > > Acked-by: Laszlo Ersek > > --- > > > > Notes: > > V7: > > - Added PcdLib > > - Restructure DEBUG message based on feedback on V6 > > - Make naming consistent to CMO, remove all CBO references > > - Add ASSERT for not supported functions instead of plain debug message > > - Added RB tag > > V6: > > - Utilize cache management instructions if HW supports it > > This patch is part of restructuring on top of v5 > > > > MdePkg/MdePkg.dec | 8 + > > MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + > > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 > > +--- > > MdePkg/MdePkg.uni | 4 + > > 4 files changed, 165 insertions(+), 20 deletions(-) > > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec > > index ac54338089e8..fa92673ff633 100644 > > --- a/MdePkg/MdePkg.dec > > +++ b/MdePkg/MdePkg.dec > > @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, > > PcdsPatchableInModule.AARCH64] > ># @Prompt CPU Rng algorithm's GUID. > > > > gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 > > > > +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] > > + # > > + # Configurability to override RISC-V CPU Features > > + # BIT 0 = Cache Management Operations. This bit is relevant only if > > + # previous stage has feature enabled and user wants to disable it. > NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that > it is explicit. > > > + # > > + > > gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 > > + > Instead of this, can default value match only those features which are > enabled by default for qemu virt machine? That way, I think we can avoid > having this PCD defined again in RiscVVirt. > Sorry, I take back. This is common for all platforms. So, we can't take qemu as reference. Thanks, Sunil -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110309): https://edk2.groups.io/g/devel/message/110309 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that it is explicit. [Dhaval] Well setting it to 1 would mean feature is enabled. Do it would be confusing to see PcdRiscVCpuFeatureDisable == 1 means feature is enabled. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110370): https://edk2.groups.io/g/devel/message/110370 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
Can we define these bits in the header file so that the definitions can be used by multiple modules? [Dhaval] I could put it un Baselib.h (MDE_CPU_RISCV64) but sounds like right now BaseLib.h is free of such #defines. If you think it is still better would do it. I do not have any preference. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110372): https://edk2.groups.io/g/devel/message/110372 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On Mon, Oct 30, 2023 at 11:24:15PM -0700, Dhaval Sharma wrote: > Can we define these bits in the header file so that the definitions can > be used by multiple modules? > [Dhaval] I could put it un Baselib.h (MDE_CPU_RISCV64) but sounds like right > now BaseLib.h is free of such #defines. If you think it is still better would > do it. I do not have any preference. Right, fine then. Thanks, Sunil -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110378): https://edk2.groups.io/g/devel/message/110378 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On 10/30/23 12:18, Sunil V L wrote: > On Sun, Oct 29, 2023 at 08:16:12PM +0530, Dhaval Sharma wrote: >> Use newly defined cache management operations for RISC-V where possible >> It builds up on the support added for RISC-V cache management >> instructions in BaseLib. >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> Cc: Laszlo Ersek >> >> Signed-off-by: Dhaval Sharma >> Acked-by: Laszlo Ersek >> --- >> >> Notes: >> V7: >> - Added PcdLib >> - Restructure DEBUG message based on feedback on V6 >> - Make naming consistent to CMO, remove all CBO references >> - Add ASSERT for not supported functions instead of plain debug message >> - Added RB tag >> V6: >> - Utilize cache management instructions if HW supports it >> This patch is part of restructuring on top of v5 >> >> MdePkg/MdePkg.dec | 8 + >> MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + >> MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 >> +--- >> MdePkg/MdePkg.uni | 4 + >> 4 files changed, 165 insertions(+), 20 deletions(-) >> >> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec >> index ac54338089e8..fa92673ff633 100644 >> --- a/MdePkg/MdePkg.dec >> +++ b/MdePkg/MdePkg.dec >> @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, >> PcdsPatchableInModule.AARCH64] >># @Prompt CPU Rng algorithm's GUID. >> >> gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 >> >> +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] >> + # >> + # Configurability to override RISC-V CPU Features >> + # BIT 0 = Cache Management Operations. This bit is relevant only if >> + # previous stage has feature enabled and user wants to disable it. > NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that > it is explicit. > >> + # >> + >> gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 >> + > Instead of this, can default value match only those features which are > enabled by default for qemu virt machine? That way, I think we can avoid > having this PCD defined again in RiscVVirt. I proposed the all-bits-one value. First, PcdRiscVFeatureOverride is in MdePkg, which is much more general than QEMU. Second, the all-bits-one pattern means that the MdePkg.dec default does not try to disable any features enabled by earlier components in the boot process, *even such features* that it does not know about specifically (i.e., those for which edk2 does not define a specific feature bit). IMO, for any "feature negotiation" bitmask, there are two choices: - clear all bits except those that we know about (and know how to use), - use all-bits-one. The first option is good if unknown (future) features are expected to *break* us. The second option is good if we are unaffected by extra (future) features, and we want to keep everything enabled for the runtime OS that comes after us. I understood that we had case #2 here. If we have case#1 instead, then we should indeed limit the bitmask to those features that *core edk2* knows about. But even in that case, I think MdePkg.dec should be as permissive as possible, and any QEMU-specific limitation should go into the OVMF DSC file. > >> [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] >>## This value is used to set the base address of PCI express hierarchy. >># @Prompt PCI Express Base Address. >> diff --git >> a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> index 6fd9cbe5f6c9..601a38d6c109 100644 >> --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf >> @@ -56,3 +56,8 @@ [LibraryClasses] >>BaseLib >>DebugLib >> >> +[LibraryClasses.RISCV64] >> + PcdLib >> + >> +[Pcd.RISCV64] >> + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES >> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> index 4eb18edb9aa7..5b3104afb67e 100644 >> --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c >> @@ -2,6 +2,7 @@ >>RISC-V specific functionality for cache. >> >>Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights >> reserved. >> + Copyright (c) 2023, Rivos Inc. All rights reserved. >> >>SPDX-License-Identifier: BSD-2-Clause-Patent >> **/ >> @@ -9,10 +10,115 @@ >> #include >> #include >> #include >> +#include >> + >> +// >> +// TODO: Grab cache block size and make Cache Management Operation >> +// enabling decision based on RISC-V CPU HO
Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V
On 10/30/23 12:22, Sunil V L wrote: > On Mon, Oct 30, 2023 at 04:48:18PM +0530, Sunil V L wrote: >> On Sun, Oct 29, 2023 at 08:16:12PM +0530, Dhaval Sharma wrote: >>> Use newly defined cache management operations for RISC-V where possible >>> It builds up on the support added for RISC-V cache management >>> instructions in BaseLib. >>> Cc: Michael D Kinney >>> Cc: Liming Gao >>> Cc: Zhiguang Liu >>> Cc: Laszlo Ersek >>> >>> Signed-off-by: Dhaval Sharma >>> Acked-by: Laszlo Ersek >>> --- >>> >>> Notes: >>> V7: >>> - Added PcdLib >>> - Restructure DEBUG message based on feedback on V6 >>> - Make naming consistent to CMO, remove all CBO references >>> - Add ASSERT for not supported functions instead of plain debug message >>> - Added RB tag >>> V6: >>> - Utilize cache management instructions if HW supports it >>> This patch is part of restructuring on top of v5 >>> >>> MdePkg/MdePkg.dec | 8 + >>> MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 5 + >>> MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c| 168 >>> +--- >>> MdePkg/MdePkg.uni | 4 + >>> 4 files changed, 165 insertions(+), 20 deletions(-) >>> >>> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec >>> index ac54338089e8..fa92673ff633 100644 >>> --- a/MdePkg/MdePkg.dec >>> +++ b/MdePkg/MdePkg.dec >>> @@ -2399,6 +2399,14 @@ [PcdsFixedAtBuild.AARCH64, >>> PcdsPatchableInModule.AARCH64] >>># @Prompt CPU Rng algorithm's GUID. >>> >>> gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x0037 >>> >>> +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] >>> + # >>> + # Configurability to override RISC-V CPU Features >>> + # BIT 0 = Cache Management Operations. This bit is relevant only if >>> + # previous stage has feature enabled and user wants to disable it. >> NIT: I am wondering whether PcdRiscVCpuFeatureDisable is better so that >> it is explicit. >> >>> + # >>> + >>> gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0x|UINT64|0x69 >>> + >> Instead of this, can default value match only those features which are >> enabled by default for qemu virt machine? That way, I think we can avoid >> having this PCD defined again in RiscVVirt. >> > Sorry, I take back. This is common for all platforms. So, we can't take > qemu as reference. yes, and sorry that I'm only seeing your addition now! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#110398): https://edk2.groups.io/g/devel/message/110398 Mute This Topic: https://groups.io/mt/102256468/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-