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 36529941D18 for ; Tue, 24 Oct 2023 12:45:41 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=/BDLqfzbqHHOG962czXpX7hk6WzdnMSzBGumO/AHmV8=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20140610; t=1698151540; v=1; b=cl+vuLDl0Z0lrCLr3xRMm4SGWD3wWS4GMtCd5yYfH8cWKGEJSManB02CaDv1Lw3tHRQ+Pktx Gc6peg4vIIRe6L+5CRsYrb7uSebxC9Mg+39yWCz+N/uud3mF4bo4wY5EddP6c1YUzPVFlanhN+E qayOiO3csryuVWRLH8aYNuE8= X-Received: by 127.0.0.2 with SMTP id wStNYY7687511xDilh6xw095; Tue, 24 Oct 2023 05:45:40 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.146702.1698151540058951696 for ; Tue, 24 Oct 2023 05:45:40 -0700 X-Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-10-F54jgJ0dNvOAib8x_7t00A-1; Tue, 24 Oct 2023 08:45:37 -0400 X-MC-Unique: F54jgJ0dNvOAib8x_7t00A-1 X-Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 70C08811E8D; Tue, 24 Oct 2023 12:45:37 +0000 (UTC) X-Received: from [10.39.195.39] (unknown [10.39.195.39]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 39827492BFB; Tue, 24 Oct 2023 12:45:36 +0000 (UTC) Message-ID: <03852129-ac51-86da-ffdb-b5106febb909@redhat.com> Date: Tue, 24 Oct 2023 14:45:35 +0200 MIME-Version: 1.0 Subject: Re: [edk2-devel] [PATCH v6 4/5] MdePkg: Utilize Cache Management Operations Implementation For RISC-V To: devel@edk2.groups.io, dhaval@rivosinc.com Cc: Michael D Kinney , Liming Gao , Zhiguang Liu References: <20231021173314.19363-1-dhaval@rivosinc.com> <20231021173314.19363-5-dhaval@rivosinc.com> From: "Laszlo Ersek" In-Reply-To: <20231021173314.19363-5-dhaval@rivosinc.com> X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.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,lersek@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: YKS0CbERZvPT3aAt298nBwLmx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=cl+vuLDl; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=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 On 10/21/23 19:33, 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 >=20 > Signed-off-by: Dhaval Sharma > --- >=20 > Notes: > V1: > - Utilize cache management instructions if HW supports it > This patch is part of restructuring on top of v5 >=20 > MdePkg/MdePkg.dec | 8= + > MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf | 2= + > MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c | 159= +++++++++++++++++--- > MdePkg/MdePkg.uni | 4= + > 4 files changed, 154 insertions(+), 19 deletions(-) >=20 > 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.A= ARCH64] > # @Prompt CPU Rng algorithm's GUID. > gEfiMdePkgTokenSpaceGuid.PcdCpuRngSupportedAlgorithm|{0x00,0x00,0x00,0= x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}|VOID*|0x00= 000037 > =20 > +[PcdsFixedAtBuild.RISCV64, PcdsPatchableInModule.RISCV64] > + # > + # Configurability to override RISC-V CPU Features > + # BIT 0 =3D Cache Management Operations. This bit is relevant only if > + # previous stage has feature enabled and user wants to disable it. > + # > + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride|0xFFFFFFFFFFFFFFFF|UI= NT64|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/BaseCacheMaintenanceL= ib.inf b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > index 6fd9cbe5f6c9..39a7fb963b49 100644 > --- a/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > +++ b/MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf > @@ -56,3 +56,5 @@ [LibraryClasses] > BaseLib > DebugLib > =20 > +[Pcd.RISCV64] > + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES > diff --git a/MdePkg/Library/BaseCacheMaintenanceLib/RiscVCache.c b/MdePkg= /Library/BaseCacheMaintenanceLib/RiscVCache.c > index 4eb18edb9aa7..6851970c9e16 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 > =20 > Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rig= hts reserved.
> + Copyright (c) 2023, Rivos Inc. All rights reserved.
> =20 > SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > @@ -9,10 +10,111 @@ > #include > #include > #include > +#include One nit: given that we're introducing a PcdLib dependency here, we should also add the following to the library instance INF file: [LibraryClasses.RISCV64] PcdLib I've not deeply verified the implementation in this patch, but structurally it looks OK to me. So you can add: Acked-by: Laszlo Ersek to the next version (with the LibraryClasses addition). Thanks Laszlo > + > +// TODO: This will be removed once RISC-V CPU HOB 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 > + ) > +{ > + // TODO: Add check for CMO from CPU HOB. > + // If CMO is disabled in HW, skip Override check > + // Otherwise this PCD can override settings > + return ((PcdGet64 (PcdRiscVFeatureOverride) & RISCV_CPU_FEATURE_CMO_BI= TMASK) !=3D 0); > +} > + > +/** > + Performs required opeartion on cache lines in the cache coherency doma= in > + of the calling CPU. If Address is not aligned on a cache line boundary= , > + then entire cache line containing Address is operated. If Address + Le= ngth > + 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 =3D=3D 0) { > + return; > + } > + > + if ((Op !=3D Invld) && (Op !=3D Flush) && (Op !=3D Clean)) { > + return; > + } > + > + ASSERT ((Length - 1) <=3D (MAX_ADDRESS - (UINTN)Address)); > + > + CacheLineSize =3D RISCV_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: > + RiscVCpuCacheInvalAsmCbo (Start); > + break; > + case Flush: > + RiscVCpuCacheFlushAsmCbo (Start); > + break; > + case Clean: > + RiscVCpuCacheCleanAsmCbo (Start); > + break; > + default: > + break; > + } > + > + Start =3D Start + CacheLineSize; > + } while (Start !=3D End); > +} > =20 > /** > Invalidates the entire instruction cache in cache coherency domain of = the > - calling CPU. > + calling CPU. Risc-V does not have currently an CBO implementation whic= h 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 o= n all > + implementations. > =20 > **/ > VOID > @@ -56,12 +158,17 @@ InvalidateInstructionCacheRange ( > IN UINTN Length > ) > { > - DEBUG ( > - (DEBUG_WARN, > - "%a:RISC-V unsupported function.\n" > - "Invalidating the whole instruction cache instead.\n", __func__) > - ); > - InvalidateInstructionCache (); > + if (RiscVIsCMOEnabled ()) { > + 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; > } > =20 > @@ -117,7 +224,12 @@ WriteBackInvalidateDataCacheRange ( > IN UINTN Length > ) > { > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__)); > + if (RiscVIsCMOEnabled ()) { > + CacheOpCacheRange (Address, Length, Flush); > + } else { > + DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__))= ; > + } > + > return Address; > } > =20 > @@ -156,10 +268,7 @@ WriteBackDataCache ( > =20 > If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT(). > =20 > - @param Address The base address of the data cache lines to write back= . If > - the CPU is in a physical addressing mode, then Address= is a > - physical address. If the CPU is in a virtual addressin= g > - mode, then Address is a virtual address. > + @param Address The base address of the data cache lines to write back= . > @param Length The number of bytes to write back from the data cache. > =20 > @return Address of cache written in main memory. > @@ -172,7 +281,12 @@ WriteBackDataCacheRange ( > IN UINTN Length > ) > { > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__)); > + if (RiscVIsCMOEnabled ()) { > + CacheOpCacheRange (Address, Length, Clean); > + } else { > + DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__))= ; > + } > + > return Address; > } > =20 > @@ -214,10 +328,7 @@ InvalidateDataCache ( > =20 > If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT(). > =20 > - @param Address The base address of the data 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 addressin= g mode, > - then Address is a virtual address. > + @param Address The base address of the data cache lines to invalidate= . > @param Length The number of bytes to invalidate from the data cache. > =20 > @return Address. > @@ -230,6 +341,16 @@ InvalidateDataCacheRange ( > IN UINTN Length > ) > { > - DEBUG ((DEBUG_ERROR, "%a:RISC-V unsupported function.\n", __func__)); > + if (RiscVIsCMOEnabled ()) { > + CacheOpCacheRange (Address, Length, Invld); > + } else { > + DEBUG ( > + (DEBUG_WARN, > + "%a:RISC-V unsupported function.\n" > + "Invalidating the whole Data cache instead.\n", __func__) > + ); > + InvalidateDataCache (); > + } > + > return Address; > } > diff --git a/MdePkg/MdePkg.uni b/MdePkg/MdePkg.uni > index 5c1fa24065c7..f49c33191054 100644 > --- a/MdePkg/MdePkg.uni > +++ b/MdePkg/MdePkg.uni > @@ -287,6 +287,10 @@ > =20 > #string STR_gEfiMdePkgTokenSpaceGuid_PcdGuidedExtractHandlerTableAddress= _HELP #language en-US "This value is used to set the available memory addr= ess to store Guided Extract Handlers. The required memory space is decided = by the value of PcdMaximumGuidedExtractHandler." > =20 > +#string STR_gEfiMdePkgTokenSpaceGuid_PcdRiscVFeatureOverride_PROMPT #la= nguage en-US "RISC-V Feature Override" > + > +#string STR_gEfiMdePkgTokenSpaceGuid_PcdRiscVFeatureOverride_HELP #lang= uage en-US "This value is used to Override Any RISC-V specific features sup= ported by this PCD" > + > #string STR_gEfiMdePkgTokenSpaceGuid_PcdPciExpressBaseAddress_PROMPT #l= anguage en-US "PCI Express Base Address" > =20 > #string STR_gEfiMdePkgTokenSpaceGuid_PcdPciExpressBaseAddress_HELP #lan= guage en-US "This value is used to set the base address of PCI express hier= archy." -=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 (#110005): https://edk2.groups.io/g/devel/message/110005 Mute This Topic: https://groups.io/mt/102103783/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/19134562= 12/xyzzy [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-