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