Re: [edk2-devel] [PATCH v7 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Sun, Oct 29, 2023 at 2:46 PM Dhaval Sharma <dhaval@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.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
>
> Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> Acked-by: Laszlo Ersek <lersek@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.

-- 
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/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-