On 10/29/23 20:07, Pedro Falcato wrote:
> On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma <dha...@rivosinc.com> 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 <michael.d.kin...@intel.com>
>> Cc: Liming Gao <gaolim...@byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang....@intel.com>
>> Cc: Laszlo Ersek <ler...@redhat.com>
>>
>> Signed-off-by: Dhaval Sharma <dha...@rivosinc.com>
>> Acked-by: Laszlo Ersek <ler...@redhat.com>
>> ---
>>
>> 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*|0x00000037
>>
>> +[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|0xFFFFFFFFFFFFFFFF|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.<BR>
>> + Copyright (c) 2023, Rivos Inc. All rights reserved.<BR>
>>
>> SPDX-License-Identifier: BSD-2-Clause-Patent
>> **/
>> @@ -9,10 +10,115 @@
>> #include <Base.h>
>> #include <Library/BaseLib.h>
>> #include <Library/DebugLib.h>
>> +#include <Library/PcdLib.h>
>> +
>> +//
>> +// 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]
-=-=-=-=-=-=-=-=-=-=-=-
