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 1A610D8042C for ; Mon, 24 Jul 2023 04:33:11 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=1sc3l5nSt43iCA/NC5Et/ufZheUX0mu4OyHjNAbdTNY=; c=relaxed/simple; d=groups.io; h=X-Received:X-Received:X-Received:X-Gm-Message-State:X-Google-Smtp-Source:X-Received:X-Received:Date:From:To:Cc:Subject:Message-ID:References:MIME-Version:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition; s=20140610; t=1690173190; v=1; b=PTaAzMpFA0xfzbJGAd/S1mWQ3T1Sgx7GoaE0sbiVWOjQ0reqFcQGUFXB7XzaIAoylcfdi33k x28StflGSfUEAY1inrRL5JHB7FnCgu66UJ5WpK0S+kc/FyWQL5SPjGArxw0FaGD5I52r0Jem+ik mt6zuQ2Pl+BTwYIWSaR9Gaqk= X-Received: by 127.0.0.2 with SMTP id HjTBYY7687511xuPpOJAiVj8; Sun, 23 Jul 2023 21:33:10 -0700 X-Received: from mail-yb1-f177.google.com (mail-yb1-f177.google.com [209.85.219.177]) by mx.groups.io with SMTP id smtpd.web10.40764.1690173189950827544 for ; Sun, 23 Jul 2023 21:33:10 -0700 X-Received: by mail-yb1-f177.google.com with SMTP id 3f1490d57ef6-ca4a6e11f55so3147698276.1 for ; Sun, 23 Jul 2023 21:33:09 -0700 (PDT) X-Gm-Message-State: yhakCplawdKr2ReI3qlwLQBRx7686176AA= X-Google-Smtp-Source: APBJJlEEOeon7pzteh8uLZ89gKnr/c9awaJYHwxxotKaeF6D8qJfip99BTHVv4iPdQnlnxm+FbK+Vg== X-Received: by 2002:a25:2314:0:b0:d09:2cba:bcac with SMTP id j20-20020a252314000000b00d092cbabcacmr3366048ybj.65.1690173189078; Sun, 23 Jul 2023 21:33:09 -0700 (PDT) X-Received: from sunil-laptop ([106.51.190.25]) by smtp.gmail.com with ESMTPSA id p18-20020a170902a41200b001bb9f104333sm1467349plq.12.2023.07.23.21.33.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Jul 2023 21:33:08 -0700 (PDT) Date: Mon, 24 Jul 2023 10:03:01 +0530 From: "Sunil V L" To: Dhaval Cc: devel@edk2.groups.io, Ard Biesheuvel , Jiewen Yao , Jordan Justen , Gerd Hoffmann , Andrei Warkentin Subject: Re: [edk2-devel] [PATCH v4 1/1] MdePkg:Implement RISCV CMO Message-ID: References: <20230713093331.267164-1-dhaval@rivosinc.com> <20230713093331.267164-2-dhaval@rivosinc.com> MIME-Version: 1.0 In-Reply-To: <20230713093331.267164-2-dhaval@rivosinc.com> 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,sunilvl@ventanamicro.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=PTaAzMpF; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io; dmarc=none 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 == Cache Block Clean and Flush instruction Enable > +CBIE == 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 virtual > + addressing mode, then Address is a virtual address. > + > + @param Length The number of bytes to invalidate from the instruction > + 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 == 0) { > + return Address; > + } > + > + ASSERT ((Length - 1) <= (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 = RV64_CACHE_BLOCK_SIZE; > + > + Start = (UINTN)Address; > + // > + // Calculate the cache line alignment > + // > + End = (Start + Length + (CacheLineSize - 1)) & ~(CacheLineSize - 1); > + Start &= ~((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 = Start + CacheLineSize; > + } while (Start != End); > + > + return Address; > +} > + > /** > Invalidates the entire instruction cache in cache coherency domain of 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 () == 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 () == 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 () == 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 () == 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 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107156): https://edk2.groups.io/g/devel/message/107156 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] -=-=-=-=-=-=-=-=-=-=-=-