From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 059DB9411D8 for ; Thu, 27 Jul 2023 05:47:43 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=XhNfbR1fIMqGUBBtlIuPfm5+Sw2FOqegYSLGeFClSmo=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1690436862; v=1; b=QvwhQZL9Oo+Nj151RaB/v3jz+DHNud6OaOG2M6ErOSswZZLQzH7DN68mch3excL1d73le3mv Dw+ptn8o3ZZMTm3RBlImG4PDvtx5p6LY8zSrrfho0koIt1S3CPnArU7imAEcJI+0jADHmH0rEu0 hU44iDW1JgX+VVF8sP5V2nro= X-Received: by 127.0.0.2 with SMTP id f0cPYY7687511xEjfyjTOhzy; Wed, 26 Jul 2023 22:47:42 -0700 X-Received: from mail-qv1-f49.google.com (mail-qv1-f49.google.com [209.85.219.49]) by mx.groups.io with SMTP id smtpd.web11.1383.1690436861194158435 for ; Wed, 26 Jul 2023 22:47:41 -0700 X-Received: by mail-qv1-f49.google.com with SMTP id 6a1803df08f44-6378cec43ddso3928986d6.2 for ; Wed, 26 Jul 2023 22:47:40 -0700 (PDT) X-Gm-Message-State: ffPcDbhtPra7vUsutxjHziYhx7686176AA= X-Google-Smtp-Source: APBJJlHHG/XYy3juDYDQoldw5d0s/zBoMLCRlnplCwPWAF05+VKyRskKY34DzW/u9HlHCfYvhAhXzOddNwqQOy2LQRk= X-Received: by 2002:a05:6214:3c8b:b0:63d:203:eb3b with SMTP id ok11-20020a0562143c8b00b0063d0203eb3bmr4299737qvb.36.1690436859963; Wed, 26 Jul 2023 22:47:39 -0700 (PDT) MIME-Version: 1.0 References: <20230713093331.267164-1-dhaval@rivosinc.com> <20230713093331.267164-2-dhaval@rivosinc.com> In-Reply-To: From: "Dhaval Sharma" Date: Thu, 27 Jul 2023 11:17:28 +0530 Message-ID: Subject: Re: [edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO To: Sunil V L Cc: devel@edk2.groups.io, Ard Biesheuvel , Jiewen Yao , Jordan Justen , Gerd Hoffmann , Andrei Warkentin Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,dhaval@rivosinc.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="0000000000008dc1560601718038" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=QvwhQZL9; dmarc=none; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --0000000000008dc1560601718038 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Thanks for your feedback. 1. Reg coding style, I will remove _ and resubmit but somehow PR CI seemed to pass for me (https://github.com/tianocore/edk2/pull/4636). 2. For size and ext discovery should I wait until your ext discovery patch is merged? 3. Thanks for catching the issue with SENVCFG. Will fix it in the next revision after #2 is addressed. On Mon, Jul 24, 2023 at 10:03=E2=80=AFAM Sunil V L wrote: > Hi Dhaval, > > On Thu, Jul 13, 2023 at 03:03:31PM +0530, Dhaval wrote: > > From: Dhaval Sharma > > > > Implementing code to support Cache Management Operations > > (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs > > > > Notes: > > 1. CMO only supports block based Operations. Meaning complete > > cache flush/invd/clean Operations are not available. In that case > > we fallback on fence.i instructions. > > 2. Rely on the fact that platform init has initialized CMO and this > > implementation just checks if it is enabled. > > 3. In order to avoid compiler dependency injecting byte code. > > > > Test: > > 1. Ensured correct instructions are refelecting in asm > > 2. Able to boot platform with RiscVVirtQemu config > > 3. Not able to verify actual instruction in HW as Qemu ignores > > any actual cache operations. > > > > Cc: Ard Biesheuvel > > Cc: Jiewen Yao > > Cc: Jordan Justen > > Cc: Gerd Hoffmann > > Cc: Sunil V L > > Cc: Andrei Warkentin > > Signed-off-by: Dhaval Sharma > > --- > > > > Notes: > > v4: > > - Removed CMO specific directory in Base Lib > > - Implemented compiler independent code for CMO > > - Merged CMO implementation with fence.i > > - Added logic to confirm CMO is enabled > > > > MdePkg/Library/BaseLib/BaseLib.inf | 2 +- > > MdePkg/Include/Register/RiscV64/RiscVEncoding.h | 4 + > > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 > ++++++++++++++++++-- > > MdePkg/Library/BaseLib/RiscV64/FlushCache.S | 21 -- > > MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S | 64 +++++++ > > 5 files changed, 254 insertions(+), 37 deletions(-) > > > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf > b/MdePkg/Library/BaseLib/BaseLib.inf > > index 03c7b02e828b..53389389448c 100644 > > --- a/MdePkg/Library/BaseLib/BaseLib.inf > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf > > @@ -400,7 +400,7 @@ [Sources.RISCV64] > > RiscV64/RiscVCpuBreakpoint.S | GCC > > RiscV64/RiscVCpuPause.S | GCC > > RiscV64/RiscVInterrupt.S | GCC > > - RiscV64/FlushCache.S | GCC > > + RiscV64/RiscVCacheMgmt.S | GCC > > RiscV64/CpuScratch.S | GCC > > RiscV64/ReadTimer.S | GCC > > RiscV64/RiscVMmu.S | GCC > > diff --git a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > > index 5c2989b797bf..ea1493578bd5 100644 > > --- a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > > +++ b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h > > @@ -85,6 +85,10 @@ > > /* Supervisor Configuration */ > > #define CSR_SENVCFG 0x10a > > > > +/* Defined CBO bits*/ > > +#define SENVCFG_CBCFE 0x40UL > > +#define SENVCFG_CBIE 0x30UL > > + > > /* Supervisor Trap Handling */ > > #define CSR_SSCRATCH 0x140 > > #define CSR_SEPC 0x141 > > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > index d08fb9f193ca..8b853e5b69fa 100644 > > --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c > > @@ -1,7 +1,8 @@ > > /** @file > > - RISC-V specific functionality for cache. > > + Implement Risc-V Cache Management Operations > > > > 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 > > **/ > > @@ -10,13 +11,21 @@ > > #include > > #include > > > > +#define RV64_CACHE_BLOCK_SIZE 64 > > + > Can we avoid hard coding this? We can get it from DT. > > > +typedef enum { > > + Clean, > > + Flush, > > + Invld, > > +} CACHE_OP; > > + > > /** > > RISC-V invalidate instruction cache. > > > > **/ > > VOID > > EFIAPI > > -RiscVInvalidateInstCacheAsm ( > > +RiscVInvalidateInstCacheAsm_Fence ( > This is not EDK2 coding style... and other similar functions below. > > > VOID > > ); > > > > @@ -26,13 +35,144 @@ RiscVInvalidateInstCacheAsm ( > > **/ > > VOID > > EFIAPI > > -RiscVInvalidateDataCacheAsm ( > > +RiscVInvalidateDataCacheAsm_Fence ( > > VOID > > ); > > > > +/** > > + RISC-V flush cache block. Atomically perform a clean operation > > + followed by an invalidate operation > > + > > +**/ > > +VOID > > +EFIAPI > > +RiscVCpuCacheFlushAsm_Cbo ( > > + UINTN > > + ); > > + > > +/** > > +Perform a write transfer to another cache or to memory if the > > +data in the copy of the cache block have been modified by a store > > +operation > > + > > +**/ > > +VOID > > +EFIAPI > > +RiscVCpuCacheCleanAsm_Cbo ( > > + UINTN > > + ); > > + > > +/** > > +Deallocate the copy of the cache block > > + > > +**/ > > +VOID > > +EFIAPI > > +RiscVCpuCacheInvalAsm_Cbo ( > > + UINTN > > + ); > > + > > +/** > > +Verify CBOs are supported by this HW > > +CBCFE =3D=3D Cache Block Clean and Flush instruction Enable > > +CBIE =3D=3D Cache Block Invalidate instruction Enable > > + > > +**/ > > +UINTN > > +RiscvIsCbcfeEnabledAsm ( > > + VOID > > + ); > > + > > +UINTN > > +RiscvIsCbiEnabledAsm ( > > + VOID > > + ); > > + > > +/** > > + 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. If the CPU is in a physical addressing mode, > > + then Address is a physical address. If the CPU is in a virtu= al > > + addressing mode, then Address is a virtual address. > > + > > + @param Length The number of bytes to invalidate from the instructi= on > > + cache. > > + > > + @param Op Type of CMO operation to be performed > > + > > + @return Address. > > + > > +**/ > > +VOID * > > +EFIAPI > > +CacheOpCacheRange ( > > + IN VOID *Address, > > + IN UINTN Length, > > + IN CACHE_OP Op > > + ) > > +{ > > + UINTN CacheLineSize; > > + UINTN Start; > > + UINTN End; > > + > > + if (Length =3D=3D 0) { > > + return Address; > > + } > > + > > + ASSERT ((Length - 1) <=3D (MAX_ADDRESS - (UINTN)Address)); > > + > > + // > > + // Cache line size is 8 * Bits 15-08 of EBX returned from CPUID 01H > > + // > This comments is invalid for RISC-V. > > > + CacheLineSize =3D RV64_CACHE_BLOCK_SIZE; > > + > > + Start =3D (UINTN)Address; > > + // > > + // Calculate the cache line alignment > > + // > > + End =3D (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize = - > 1); > > + Start &=3D ~((UINTN)CacheLineSize - 1); > > + > > + DEBUG ( > > + (DEBUG_INFO, > > + "%a Performing Cache Management Operation %d \n", __func__, Op) > > + ); > > + > > + do { > > + switch (Op) { > > + case Invld: > > + RiscVCpuCacheInvalAsm_Cbo (Start); > > + break; > > + case Flush: > > + RiscVCpuCacheFlushAsm_Cbo (Start); > > + break; > > + case Clean: > > + RiscVCpuCacheCleanAsm_Cbo (Start); > > + break; > > + default: > > + DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported operation\n")); > > + break; > > + } > > + > > + Start =3D Start + CacheLineSize; > > + } while (Start !=3D End); > > + > > + return Address; > > +} > > + > > /** > > Invalidates the entire instruction cache in cache coherency domain o= f > the > > - calling CPU. > > + calling CPU. Risc-V does not have currently an CBO implementation > which can > > + invalidate entire I-cache. Hence using Fence instruction for now. > P.S. Fence > > + instruction may or may not implement full I-cache invd functionality > on all > > + implementations. > > > > **/ > > VOID > > @@ -41,7 +181,7 @@ InvalidateInstructionCache ( > > VOID > > ) > > { > > - RiscVInvalidateInstCacheAsm (); > > + RiscVInvalidateInstCacheAsm_Fence (); > > } > > > > /** > > @@ -76,12 +216,17 @@ InvalidateInstructionCacheRange ( > > IN UINTN Length > > ) > > { > > - DEBUG ( > > - (DEBUG_WARN, > > - "%a:RISC-V unsupported function.\n" > > - "Invalidating the whole instruction cache instead.\n", __func__) > > - ); > > - InvalidateInstructionCache (); > > + if (RiscvIsCbiEnabledAsm () =3D=3D RETURN_SUCCESS) { > > + CacheOpCacheRange (Address, Length, Invld); > > + } else { > > + DEBUG ( > > + (DEBUG_WARN, > > + "%a:RISC-V unsupported function.\n" > > + "Invalidating the whole instruction cache instead.\n", __func__= ) > > + ); > > + InvalidateInstructionCache (); > > + } > > + > > return Address; > > } > > > > @@ -137,7 +282,12 @@ WriteBackInvalidateDataCacheRange ( > > IN UINTN Length > > ) > > { > > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__))= ; > > + if (RiscvIsCbcfeEnabledAsm () =3D=3D RETURN_SUCCESS) { > > + CacheOpCacheRange (Address, Length, Flush); > > + } else { > > + DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", > __func__)); > > + } > > + > > return Address; > > } > > > > @@ -192,7 +342,12 @@ WriteBackDataCacheRange ( > > IN UINTN Length > > ) > > { > > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__))= ; > > + if (RiscvIsCbcfeEnabledAsm () =3D=3D RETURN_SUCCESS) { > > + CacheOpCacheRange (Address, Length, Clean); > > + } else { > > + DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", > __func__)); > > + } > > + > > return Address; > > } > > > > @@ -213,7 +368,12 @@ InvalidateDataCache ( > > VOID > > ) > > { > > - RiscVInvalidateDataCacheAsm (); > > + DEBUG ( > > + (DEBUG_WARN, > > + "%a:RISC-V unsupported function.\n" > > + "Invalidating the whole instruction cache instead.\n", __func__) > Data cache? > > + ); > > + RiscVInvalidateDataCacheAsm_Fence (); > > } > > > > /** > > @@ -250,6 +410,16 @@ InvalidateDataCacheRange ( > > IN UINTN Length > > ) > > { > > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__))= ; > > + if (RiscvIsCbiEnabledAsm () =3D=3D RETURN_SUCCESS) { > > + CacheOpCacheRange (Address, Length, Invld); > > + } else { > > + DEBUG ( > > + (DEBUG_WARN, > > + "%a:RISC-V unsupported function.\n" > > + "Invalidating the whole instruction cache instead.\n", __func__= ) > Data cache? > > > + ); > > + InvalidateDataCache (); > > + } > > + > > return Address; > > } > > diff --git a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S > b/MdePkg/Library/BaseLib/RiscV64/FlushCache.S > > deleted file mode 100644 > > index 7c10fdd268af..000000000000 > > --- a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S > > +++ /dev/null > > @@ -1,21 +0,0 @@ > > > -//----------------------------------------------------------------------= -------- > > -// > > -// RISC-V cache operation. > > -// > > -// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All > rights reserved.
> > -// > > -// SPDX-License-Identifier: BSD-2-Clause-Patent > > -// > > > -//----------------------------------------------------------------------= -------- > > - > > -.align 3 > > -ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm) > > -ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm) > > - > > -ASM_PFX(RiscVInvalidateInstCacheAsm): > > - fence.i > > - ret > > - > > -ASM_PFX(RiscVInvalidateDataCacheAsm): > > - fence > > - ret > Have you done git mv first to rename the file so that git history is > maintained? > > > diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S > b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S > > new file mode 100644 > > index 000000000000..ecf391632221 > > --- /dev/null > > +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S > > @@ -0,0 +1,64 @@ > > > +//----------------------------------------------------------------------= -------- > > +// > > +// RISC-V cache operation. > > +// > > +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All > rights reserved.
> > +// Copyright (c) 2022, Rivos Inc. All rights reserved.
> > +// > > +// SPDX-License-Identifier: BSD-2-Clause-Patent > > +// > > > +//----------------------------------------------------------------------= -------- > > +#include > > + > > +.align 3 > > +ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm_Fence) > > +ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm_Fence) > > + > > +ASM_PFX(RiscVInvalidateInstCacheAsm_Fence): > > + fence.i > > + ret > > + > > +ASM_PFX(RiscVInvalidateDataCacheAsm_Fence): > > + fence > > + ret > > + > > +ASM_GLOBAL ASM_PFX (RiscVCpuCacheFlushAsm_Cbo) > > +ASM_PFX (RiscVCpuCacheFlushAsm_Cbo): > > + > > + .long 0x0025200f > Can we have macros instead of magic number? > > > + ret > > + > > +ASM_GLOBAL ASM_PFX (RiscVCpuCacheCleanAsm_Cbo) > > +ASM_PFX (RiscVCpuCacheCleanAsm_Cbo): > > + .long 0x0015200f > > + ret > > + > > +ASM_GLOBAL ASM_PFX (RiscVCpuCacheInvalAsm_Cbo) > > +ASM_PFX (RiscVCpuCacheInvalAsm_Cbo): > > + .long 0x0005200f > > + ret > > + > > +ASM_GLOBAL ASM_PFX(RiscvIsCbiEnabledAsm) > > +ASM_PFX(RiscvIsCbiEnabledAsm): > > + li a0, 3 > > + csrr a1, CSR_SENVCFG > > + and a1, a1, SENVCFG_CBIE > > This is not correct. SENVCFG is only for U-mode. It can not be relied > upon in S-mode. We have to use extension discovery itself. Same comment > for CBCFE also. > > Thanks, > Sunil > > > + beqz a1, skip > > + > > + and a1, a1, 0x10 > > + beqz a1, skip > > + > > + mv a0, x0 > > +skip: > > + ret > > + > > +ASM_GLOBAL ASM_PFX(RiscvIsCbcfeEnabledAsm) > > +ASM_PFX(RiscvIsCbcfeEnabledAsm): > > + li a0, 3 > > + csrr a1, CSR_SENVCFG > > + and a1, a1, SENVCFG_CBCFE > > + beqz a1, next > > + > > + mv a0, x0 > > +next: > > + ret > > -- > > 2.34.1 > > > --=20 Thanks! =3DD -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107292): https://edk2.groups.io/g/devel/message/107292 Mute This Topic: https://groups.io/mt/100117169/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --0000000000008dc1560601718038 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Thanks for your feedback.
  1. Re= g coding style, I will remove _ and resubmit but somehow PR CI seemed to pa= ss for me (https://github.com/tianocore/edk2/pull/4636).
  2. F= or size and ext discovery should I wait until your ext discovery patch is m= erged?
  3. Thanks for catching the issue with SENVCFG. Will fix it in t= he next revision after #2 is addressed.
On Mon, Jul 24, 2023 at 10:03=E2=80= =AFAM Sunil V L <sunilvl@ventanamicro.com> wrote:
Hi Dhaval,

On Thu, Jul 13, 2023 at 03:03:31PM +0530, Dhaval wrote:
> From: Dhaval Sharma <dhaval@rivosinc.com>
>
> Implementing code to support Cache Management Operations
> (CMO) defined by RV spec https://github.com/riscv/riscv-CMOs=
>
> Notes:
> 1. CMO only supports block based Operations. Meaning complete
>=C2=A0 =C2=A0 cache flush/invd/clean Operations are not available. In t= hat case
>=C2=A0 =C2=A0 we fallback on fence.i instructions.
> 2. Rely on the fact that platform init has initialized CMO and this >=C2=A0 =C2=A0 implementation just checks if it is enabled.
> 3. In order to avoid compiler dependency injecting byte code.
>
> Test:
> 1. Ensured correct instructions are refelecting in asm
> 2. Able to boot platform with RiscVVirtQemu config
> 3. Not able to verify actual instruction in HW as Qemu ignores
> any actual cache operations.
>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Sunil V L <sunilvl@ventanamicro.com>
> Cc: Andrei Warkentin <andrei.warkentin@intel.com>
> Signed-off-by: Dhaval Sharma <dhaval@rivosinc.com>
> ---
>
> Notes:
>=C2=A0 =C2=A0 =C2=A0v4:
>=C2=A0 =C2=A0 =C2=A0- Removed CMO specific directory in Base Lib
>=C2=A0 =C2=A0 =C2=A0- Implemented compiler independent code for CMO
>=C2=A0 =C2=A0 =C2=A0- Merged CMO implementation with fence.i
>=C2=A0 =C2=A0 =C2=A0- Added logic to confirm CMO is enabled
>
>=C2=A0 MdePkg/Library/BaseLib/BaseLib.inf=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 =C2=A02 +-
>=C2=A0 MdePkg/Include/Register/RiscV64/RiscVEncoding.h=C2=A0 =C2=A0 =C2= =A0|=C2=A0 =C2=A04 +
>=C2=A0 MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 200 ++++++= ++++++++++++--
>=C2=A0 MdePkg/Library/BaseLib/RiscV64/FlushCache.S=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0|=C2=A0 21 --
>=C2=A0 MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S=C2=A0 =C2=A0 =C2= =A0|=C2=A0 64 +++++++
>=C2=A0 5 files changed, 254 insertions(+), 37 deletions(-)
>
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseL= ib/BaseLib.inf
> index 03c7b02e828b..53389389448c 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -400,7 +400,7 @@ [Sources.RISCV64]
>=C2=A0 =C2=A0 RiscV64/RiscVCpuBreakpoint.S=C2=A0 =C2=A0 =C2=A0 | GCC >=C2=A0 =C2=A0 RiscV64/RiscVCpuPause.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0| GCC
>=C2=A0 =C2=A0 RiscV64/RiscVInterrupt.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 | GCC
> -=C2=A0 RiscV64/FlushCache.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 | GCC
> +=C2=A0 RiscV64/RiscVCacheMgmt.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | G= CC
>=C2=A0 =C2=A0 RiscV64/CpuScratch.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 | GCC
>=C2=A0 =C2=A0 RiscV64/ReadTimer.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0| GCC
>=C2=A0 =C2=A0 RiscV64/RiscVMmu.S=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 | GCC
> diff --git a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h b/MdePkg/= Include/Register/RiscV64/RiscVEncoding.h
> index 5c2989b797bf..ea1493578bd5 100644
> --- a/MdePkg/Include/Register/RiscV64/RiscVEncoding.h
> +++ b/MdePkg/Include/Register/RiscV64/RiscVEncoding.h
> @@ -85,6 +85,10 @@
>=C2=A0 /* Supervisor Configuration */
>=C2=A0 #define CSR_SENVCFG=C2=A0 0x10a
>=C2=A0
> +/* Defined CBO bits*/
> +#define SENVCFG_CBCFE=C2=A0 0x40UL
> +#define SENVCFG_CBIE=C2=A0 =C2=A00x30UL
> +
>=C2=A0 /* Supervisor Trap Handling */
>=C2=A0 #define CSR_SSCRATCH=C2=A0 0x140
>=C2=A0 #define CSR_SEPC=C2=A0 =C2=A0 =C2=A0 0x141
> diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c b/Mde= Pkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> index d08fb9f193ca..8b853e5b69fa 100644
> --- a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> +++ b/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c
> @@ -1,7 +1,8 @@
>=C2=A0 /** @file
> -=C2=A0 RISC-V specific functionality for cache.
> +=C2=A0 Implement Risc-V Cache Management Operations
>=C2=A0
>=C2=A0 =C2=A0 Copyright (c) 2020, Hewlett Packard Enterprise Developmen= t LP. All rights reserved.<BR>
> +=C2=A0 Copyright (c) 2023, Rivos Inc. All rights reserved.<BR><= br> >=C2=A0
>=C2=A0 =C2=A0 SPDX-License-Identifier: BSD-2-Clause-Patent
>=C2=A0 **/
> @@ -10,13 +11,21 @@
>=C2=A0 #include <Library/BaseLib.h>
>=C2=A0 #include <Library/DebugLib.h>
>=C2=A0
> +#define RV64_CACHE_BLOCK_SIZE=C2=A0 64
> +
Can we avoid hard coding this? We can get it from DT.

> +typedef enum {
> +=C2=A0 Clean,
> +=C2=A0 Flush,
> +=C2=A0 Invld,
> +} CACHE_OP;
> +
>=C2=A0 /**
>=C2=A0 =C2=A0 RISC-V invalidate instruction cache.
>=C2=A0
>=C2=A0 **/
>=C2=A0 VOID
>=C2=A0 EFIAPI
> -RiscVInvalidateInstCacheAsm (
> +RiscVInvalidateInstCacheAsm_Fence (
This is not EDK2 coding style... and other similar functions below.

>=C2=A0 =C2=A0 VOID
>=C2=A0 =C2=A0 );
>=C2=A0
> @@ -26,13 +35,144 @@ RiscVInvalidateInstCacheAsm (
>=C2=A0 **/
>=C2=A0 VOID
>=C2=A0 EFIAPI
> -RiscVInvalidateDataCacheAsm (
> +RiscVInvalidateDataCacheAsm_Fence (
>=C2=A0 =C2=A0 VOID
>=C2=A0 =C2=A0 );
>=C2=A0
> +/**
> +=C2=A0 RISC-V flush cache block. Atomically perform a clean operation=
> +=C2=A0 followed by an invalidate operation
> +
> +**/
> +VOID
> +EFIAPI
> +RiscVCpuCacheFlushAsm_Cbo (
> +=C2=A0 UINTN
> +=C2=A0 );
> +
> +/**
> +Perform a write transfer to another cache or to memory if the
> +data in the copy of the cache block have been modified by a store
> +operation
> +
> +**/
> +VOID
> +EFIAPI
> +RiscVCpuCacheCleanAsm_Cbo (
> +=C2=A0 UINTN
> +=C2=A0 );
> +
> +/**
> +Deallocate the copy of the cache block
> +
> +**/
> +VOID
> +EFIAPI
> +RiscVCpuCacheInvalAsm_Cbo (
> +=C2=A0 UINTN
> +=C2=A0 );
> +
> +/**
> +Verify CBOs are supported by this HW
> +CBCFE =3D=3D Cache Block Clean and Flush instruction Enable
> +CBIE =3D=3D Cache Block Invalidate instruction Enable
> +
> +**/
> +UINTN
> +RiscvIsCbcfeEnabledAsm (
> +=C2=A0 VOID
> +=C2=A0 );
> +
> +UINTN
> +RiscvIsCbiEnabledAsm (
> +=C2=A0 VOID
> +=C2=A0 );
> +
> +/**
> +=C2=A0 Performs required opeartion on cache lines in the cache cohere= ncy domain
> +=C2=A0 of the calling CPU. If Address is not aligned on a cache line = boundary,
> +=C2=A0 then entire cache line containing Address is operated. If Addr= ess + Length
> +=C2=A0 is not aligned on a cache line boundary, then the entire cache= line
> +=C2=A0 containing Address + Length -1 is operated.
> +
> +=C2=A0 If Length is greater than (MAX_ADDRESS - Address + 1), then AS= SERT().
> +
> +=C2=A0 @param=C2=A0 Address The base address of the cache lines to > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 invalidate. If the CPU is in a phy= sical addressing mode,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 then Address is a physical address= . If the CPU is in a virtual
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 addressing mode, then Address is a= virtual address.
> +
> +=C2=A0 @param=C2=A0 Length=C2=A0 The number of bytes to invalidate fr= om the instruction
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 cache.
> +
> +=C2=A0 @param=C2=A0 Op=C2=A0 Type of CMO operation to be performed > +
> +=C2=A0 @return Address.
> +
> +**/
> +VOID *
> +EFIAPI
> +CacheOpCacheRange (
> +=C2=A0 IN VOID=C2=A0 =C2=A0 =C2=A0 *Address,
> +=C2=A0 IN UINTN=C2=A0 =C2=A0 =C2=A0Length,
> +=C2=A0 IN CACHE_OP=C2=A0 Op
> +=C2=A0 )
> +{
> +=C2=A0 UINTN=C2=A0 CacheLineSize;
> +=C2=A0 UINTN=C2=A0 Start;
> +=C2=A0 UINTN=C2=A0 End;
> +
> +=C2=A0 if (Length =3D=3D 0) {
> +=C2=A0 =C2=A0 return Address;
> +=C2=A0 }
> +
> +=C2=A0 ASSERT ((Length - 1) <=3D (MAX_ADDRESS - (UINTN)Address));<= br> > +
> +=C2=A0 //
> +=C2=A0 // Cache line size is 8 * Bits 15-08 of EBX returned from CPUI= D 01H
> +=C2=A0 //
This comments is invalid for RISC-V.

> +=C2=A0 CacheLineSize =3D RV64_CACHE_BLOCK_SIZE;
> +
> +=C2=A0 Start =3D (UINTN)Address;
> +=C2=A0 //
> +=C2=A0 // Calculate the cache line alignment
> +=C2=A0 //
> +=C2=A0 End=C2=A0 =C2=A0 =3D (Start + Length + (CacheLineSize - 1)) &a= mp; ~(CacheLineSize - 1);
> +=C2=A0 Start &=3D ~((UINTN)CacheLineSize - 1);
> +
> +=C2=A0 DEBUG (
> +=C2=A0 =C2=A0 (DEBUG_INFO,
> +=C2=A0 =C2=A0 =C2=A0"%a Performing Cache Management Operation %d= \n", __func__, Op)
> +=C2=A0 =C2=A0 );
> +
> +=C2=A0 do {
> +=C2=A0 =C2=A0 switch (Op) {
> +=C2=A0 =C2=A0 =C2=A0 case Invld:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 RiscVCpuCacheInvalAsm_Cbo (Start);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 =C2=A0 case Flush:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 RiscVCpuCacheFlushAsm_Cbo (Start);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 =C2=A0 case Clean:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 RiscVCpuCacheCleanAsm_Cbo (Start);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 DEBUG ((DEBUG_ERROR, "%a:RISC-V unsu= pported operation\n"));
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 Start =3D Start + CacheLineSize;
> +=C2=A0 } while (Start !=3D End);
> +
> +=C2=A0 return Address;
> +}
> +
>=C2=A0 /**
>=C2=A0 =C2=A0 Invalidates the entire instruction cache in cache coheren= cy domain of the
> -=C2=A0 calling CPU.
> +=C2=A0 calling CPU. Risc-V does not have currently an CBO implementat= ion which can
> +=C2=A0 invalidate entire I-cache. Hence using Fence instruction for n= ow. P.S. Fence
> +=C2=A0 instruction may or may not implement full I-cache invd functio= nality on all
> +=C2=A0 implementations.
>=C2=A0
>=C2=A0 **/
>=C2=A0 VOID
> @@ -41,7 +181,7 @@ InvalidateInstructionCache (
>=C2=A0 =C2=A0 VOID
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 RiscVInvalidateInstCacheAsm ();
> +=C2=A0 RiscVInvalidateInstCacheAsm_Fence ();
>=C2=A0 }
>=C2=A0
>=C2=A0 /**
> @@ -76,12 +216,17 @@ InvalidateInstructionCacheRange (
>=C2=A0 =C2=A0 IN UINTN=C2=A0 Length
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 DEBUG (
> -=C2=A0 =C2=A0 (DEBUG_WARN,
> -=C2=A0 =C2=A0 =C2=A0"%a:RISC-V unsupported function.\n"
> -=C2=A0 =C2=A0 =C2=A0"Invalidating the whole instruction cache in= stead.\n", __func__)
> -=C2=A0 =C2=A0 );
> -=C2=A0 InvalidateInstructionCache ();
> +=C2=A0 if (RiscvIsCbiEnabledAsm () =3D=3D RETURN_SUCCESS) {
> +=C2=A0 =C2=A0 CacheOpCacheRange (Address, Length, Invld);
> +=C2=A0 } else {
> +=C2=A0 =C2=A0 DEBUG (
> +=C2=A0 =C2=A0 =C2=A0 (DEBUG_WARN,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0"%a:RISC-V unsupported function.\n&qu= ot;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0"Invalidating the whole instruction c= ache instead.\n", __func__)
> +=C2=A0 =C2=A0 =C2=A0 );
> +=C2=A0 =C2=A0 InvalidateInstructionCache ();
> +=C2=A0 }
> +
>=C2=A0 =C2=A0 return Address;
>=C2=A0 }
>=C2=A0
> @@ -137,7 +282,12 @@ WriteBackInvalidateDataCacheRange (
>=C2=A0 =C2=A0 IN=C2=A0 =C2=A0 =C2=A0 UINTN=C2=A0 Length
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n&q= uot;, __func__));
> +=C2=A0 if (RiscvIsCbcfeEnabledAsm () =3D=3D RETURN_SUCCESS) {
> +=C2=A0 =C2=A0 CacheOpCacheRange (Address, Length, Flush);
> +=C2=A0 } else {
> +=C2=A0 =C2=A0 DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported functi= on.\n", __func__));
> +=C2=A0 }
> +
>=C2=A0 =C2=A0 return Address;
>=C2=A0 }
>=C2=A0
> @@ -192,7 +342,12 @@ WriteBackDataCacheRange (
>=C2=A0 =C2=A0 IN=C2=A0 =C2=A0 =C2=A0 UINTN=C2=A0 Length
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n&q= uot;, __func__));
> +=C2=A0 if (RiscvIsCbcfeEnabledAsm () =3D=3D RETURN_SUCCESS) {
> +=C2=A0 =C2=A0 CacheOpCacheRange (Address, Length, Clean);
> +=C2=A0 } else {
> +=C2=A0 =C2=A0 DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported functi= on.\n", __func__));
> +=C2=A0 }
> +
>=C2=A0 =C2=A0 return Address;
>=C2=A0 }
>=C2=A0
> @@ -213,7 +368,12 @@ InvalidateDataCache (
>=C2=A0 =C2=A0 VOID
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 RiscVInvalidateDataCacheAsm ();
> +=C2=A0 DEBUG (
> +=C2=A0 =C2=A0 (DEBUG_WARN,
> +=C2=A0 =C2=A0 =C2=A0"%a:RISC-V unsupported function.\n"
> +=C2=A0 =C2=A0 =C2=A0"Invalidating the whole instruction cache in= stead.\n", __func__)
Data cache?
> +=C2=A0 =C2=A0 );
> +=C2=A0 RiscVInvalidateDataCacheAsm_Fence ();
>=C2=A0 }
>=C2=A0
>=C2=A0 /**
> @@ -250,6 +410,16 @@ InvalidateDataCacheRange (
>=C2=A0 =C2=A0 IN=C2=A0 =C2=A0 =C2=A0 UINTN=C2=A0 Length
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n&q= uot;, __func__));
> +=C2=A0 if (RiscvIsCbiEnabledAsm () =3D=3D RETURN_SUCCESS) {
> +=C2=A0 =C2=A0 CacheOpCacheRange (Address, Length, Invld);
> +=C2=A0 } else {
> +=C2=A0 =C2=A0 DEBUG (
> +=C2=A0 =C2=A0 =C2=A0 (DEBUG_WARN,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0"%a:RISC-V unsupported function.\n&qu= ot;
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0"Invalidating the whole instruction c= ache instead.\n", __func__)
Data cache?

> +=C2=A0 =C2=A0 =C2=A0 );
> +=C2=A0 =C2=A0 InvalidateDataCache ();
> +=C2=A0 }
> +
>=C2=A0 =C2=A0 return Address;
>=C2=A0 }
> diff --git a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S b/MdePkg/Libr= ary/BaseLib/RiscV64/FlushCache.S
> deleted file mode 100644
> index 7c10fdd268af..000000000000
> --- a/MdePkg/Library/BaseLib/RiscV64/FlushCache.S
> +++ /dev/null
> @@ -1,21 +0,0 @@
> -//-------------------------------------------------------------------= -----------
> -//
> -// RISC-V cache operation.
> -//
> -// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All= rights reserved.<BR>
> -//
> -// SPDX-License-Identifier: BSD-2-Clause-Patent
> -//
> -//-------------------------------------------------------------------= -----------
> -
> -.align 3
> -ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm)
> -ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm)
> -
> -ASM_PFX(RiscVInvalidateInstCacheAsm):
> -=C2=A0 =C2=A0 fence.i
> -=C2=A0 =C2=A0 ret
> -
> -ASM_PFX(RiscVInvalidateDataCacheAsm):
> -=C2=A0 =C2=A0 fence
> -=C2=A0 =C2=A0 ret
Have you done git mv first to rename the file so that git history is
maintained?

> diff --git a/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S b/MdePkg/= Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> new file mode 100644
> index 000000000000..ecf391632221
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/RiscV64/RiscVCacheMgmt.S
> @@ -0,0 +1,64 @@
> +//-------------------------------------------------------------------= -----------
> +//
> +// RISC-V cache operation.
> +//
> +// Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All= rights reserved.<BR>
> +// Copyright (c) 2022, Rivos Inc. All rights reserved.<BR>
> +//
> +// SPDX-License-Identifier: BSD-2-Clause-Patent
> +//
> +//-------------------------------------------------------------------= -----------
> +#include <Register/RiscV64/RiscVImpl.h>
> +
> +.align 3
> +ASM_GLOBAL ASM_PFX(RiscVInvalidateInstCacheAsm_Fence)
> +ASM_GLOBAL ASM_PFX(RiscVInvalidateDataCacheAsm_Fence)
> +
> +ASM_PFX(RiscVInvalidateInstCacheAsm_Fence):
> +=C2=A0 =C2=A0 fence.i
> +=C2=A0 =C2=A0 ret
> +
> +ASM_PFX(RiscVInvalidateDataCacheAsm_Fence):
> +=C2=A0 =C2=A0 fence
> +=C2=A0 =C2=A0 ret
> +
> +ASM_GLOBAL ASM_PFX (RiscVCpuCacheFlushAsm_Cbo)
> +ASM_PFX (RiscVCpuCacheFlushAsm_Cbo):
> +
> +=C2=A0 .long 0x0025200f
Can we have macros instead of magic number?

> +=C2=A0 ret
> +
> +ASM_GLOBAL ASM_PFX (RiscVCpuCacheCleanAsm_Cbo)
> +ASM_PFX (RiscVCpuCacheCleanAsm_Cbo):
> +=C2=A0 .long 0x0015200f
> +=C2=A0 ret
> +
> +ASM_GLOBAL ASM_PFX (RiscVCpuCacheInvalAsm_Cbo)
> +ASM_PFX (RiscVCpuCacheInvalAsm_Cbo):
> +=C2=A0 .long 0x0005200f
> +=C2=A0 ret
> +
> +ASM_GLOBAL ASM_PFX(RiscvIsCbiEnabledAsm)
> +ASM_PFX(RiscvIsCbiEnabledAsm):
> +=C2=A0 =C2=A0 li=C2=A0 =C2=A0 a0, 3
> +=C2=A0 =C2=A0 csrr=C2=A0 a1, CSR_SENVCFG
> +=C2=A0 =C2=A0 and=C2=A0 =C2=A0a1, a1, SENVCFG_CBIE

This is not correct. SENVCFG is only for U-mode. It can not be relied
upon in S-mode. We have to use extension discovery itself. Same comment
for CBCFE also.

Thanks,
Sunil

> +=C2=A0 =C2=A0 beqz=C2=A0 a1, skip
> +
> +=C2=A0 =C2=A0 and=C2=A0 =C2=A0a1, a1, 0x10
> +=C2=A0 =C2=A0 beqz=C2=A0 a1, skip
> +
> +=C2=A0 =C2=A0 mv=C2=A0 =C2=A0 a0, x0
> +skip:
> +=C2=A0 =C2=A0 ret
> +
> +ASM_GLOBAL ASM_PFX(RiscvIsCbcfeEnabledAsm)
> +ASM_PFX(RiscvIsCbcfeEnabledAsm):
> +=C2=A0 =C2=A0 li=C2=A0 =C2=A0 a0, 3
> +=C2=A0 =C2=A0 csrr=C2=A0 a1, CSR_SENVCFG
> +=C2=A0 =C2=A0 and=C2=A0 =C2=A0a1, a1, SENVCFG_CBCFE
> +=C2=A0 =C2=A0 beqz=C2=A0 a1, next
> +
> +=C2=A0 =C2=A0 mv=C2=A0 =C2=A0 a0, x0
> +next:
> +=C2=A0 =C2=A0 ret
> --
> 2.34.1
>


--
Thanks!
=3DD
_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#107292) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--0000000000008dc1560601718038--