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 06D8B740032 for ; Tue, 2 Apr 2024 22:42:33 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=PsRWz/cbP7hN6t4AKnx7tvZaFguKLFbj2t1t28g9wvI=; c=relaxed/simple; d=groups.io; h=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:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20240206; t=1712097752; v=1; b=NMQ8Mbm+vuxml7d3czhAYhBOVQe04Vt8Q+skskzVXbx2Yq5xatPIQNPUpWcn449GmgEya1t2 l8FQ2zRJ4YZWL+ndpIks37buZUF9g7xPIwKmVm0CO49Le6TA03HCpPGLjJo0fgBpzsE9a3lSuOL Oj+vq1WxC0zw40QdsiHcLyepZNIE/qVRtaV+HGwslcrn4CFZP0HnXILwj649e1s06BI10xM7Q4k ne43799rK9n+iWaHMqeqoMyelKEneYH8WbThVEUoB2ufAkWch39043iY8Tun/ceVKyMak6Hyrw9 v7lCR+uPwp9AWaEvv6J2LvfDIA5gLs5bBBIFiB90MlOiw== X-Received: by 127.0.0.2 with SMTP id ROBEYY7687511xBcybDaGWHT; Tue, 02 Apr 2024 15:42:32 -0700 X-Received: from mail-ua1-f42.google.com (mail-ua1-f42.google.com [209.85.222.42]) by mx.groups.io with SMTP id smtpd.web10.1743.1712097751776809438 for ; Tue, 02 Apr 2024 15:42:32 -0700 X-Received: by mail-ua1-f42.google.com with SMTP id a1e0cc1a2514c-7e3c64fe983so42943241.0 for ; Tue, 02 Apr 2024 15:42:31 -0700 (PDT) X-Gm-Message-State: uHmVGOEDvdjNilIJntQiaEbux7686176AA= X-Google-Smtp-Source: AGHT+IHBfkW9rpaySrSRv6xRJs87xL9TnU8ufZjhO1Tz71fPtTuGfFh2oboB4PvvmWdVG7YFO89q5PwvVI9bAAz+X8M= X-Received: by 2002:a05:6122:200a:b0:4d4:2176:6b6a with SMTP id l10-20020a056122200a00b004d421766b6amr1138626vkd.8.1712097750192; Tue, 02 Apr 2024 15:42:30 -0700 (PDT) MIME-Version: 1.0 References: <20240314201917.1756-1-tphan@ventanamicro.com> <20240314201917.1756-4-tphan@ventanamicro.com> <17BE38330B281CA5.29196@groups.io> In-Reply-To: <17BE38330B281CA5.29196@groups.io> From: "Tuan Phan" Date: Tue, 2 Apr 2024 15:42:19 -0700 Message-ID: Subject: Re: [edk2-devel] [PATCH v4 3/4] UefiCpuPkg: RISC-V: MMU: Support Svpbmt extension To: devel@edk2.groups.io, tphan@ventanamicro.com Cc: Sunil V L , michael.d.kinney@intel.com, gaoliming@byosoft.com.cn, zhiguang.liu@intel.com, kraxel@redhat.com, lersek@redhat.com, rahul1.kumar@intel.com, ray.ni@intel.com, jiewen.yao@intel.com, andrei.warkentin@intel.com, ardb+tianocore@kernel.org 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 Resent-Date: Tue, 02 Apr 2024 15:42:32 -0700 Resent-From: tphan@ventanamicro.com Reply-To: devel@edk2.groups.io,tphan@ventanamicro.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="00000000000038b368061524d276" X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=NMQ8Mbm+; 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 --00000000000038b368061524d276 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Mar 19, 2024 at 9:45=E2=80=AFAM Tuan Phan via groups.io wrote: > Hi Sunil, > > On Mon, Mar 18, 2024 at 6:00=E2=80=AFAM Sunil V L > wrote: > >> Hi Tuan, >> >> On Thu, Mar 14, 2024 at 01:19:16PM -0700, Tuan Phan wrote: >> > The GCD EFI_MEMORY_UC and EFI_MEMORY_WC memory attributes will be >> > supported when Svpbmt extension available. >> > >> > Cc: Gerd Hoffmann >> > Cc: Laszlo Ersek >> > Cc: Rahul Kumar >> > Cc: Ray Ni >> > Signed-off-by: Tuan Phan >> > --- >> > .../Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c | 106 ++++++++++++++---= - >> > .../BaseRiscVMmuLib/BaseRiscVMmuLib.inf | 1 + >> > 2 files changed, 86 insertions(+), 21 deletions(-) >> > >> > diff --git a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c >> b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c >> > index 46ba4b4709b1..34300dca5c34 100644 >> > --- a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c >> > +++ b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c >> > @@ -36,6 +36,11 @@ >> > #define PTE_PPN_SHIFT 10 >> > #define RISCV_MMU_PAGE_SHIFT 12 >> > >> > +#define RISCV_CPU_FEATURE_PBMT_BITMASK BIT2 >> > +#define PTE_PBMT_NC BIT61 >> > +#define PTE_PBMT_IO BIT62 >> > +#define PTE_PBMT_MASK (PTE_PBMT_NC | PTE_PBMT_IO) >> > + >> > STATIC UINTN mModeSupport[] =3D { SATP_MODE_SV57, SATP_MODE_SV48, >> SATP_MODE_SV39, SATP_MODE_OFF }; >> > STATIC UINTN mMaxRootTableLevel; >> > STATIC UINTN mBitPerLevel; >> > @@ -487,32 +492,82 @@ UpdateRegionMapping ( >> > /** >> > Convert GCD attribute to RISC-V page attribute. >> > >> > - @param GcdAttributes The GCD attribute. >> > + @param GcdAttributes The GCD attribute. >> > + @param RiscVAttributes The pointer of RISC-V page attribute. >> > >> > - @return The RISC-V page attribute. >> > + @retval EFI_INVALID_PARAMETER The RiscVAttributes is NULL or cach= e >> type mask not valid. >> > + @retval EFI_SUCCESS The operation succesfully. >> > >> > **/ >> > STATIC >> > -UINT64 >> > +EFI_STATUS >> > GcdAttributeToPageAttribute ( >> > - IN UINT64 GcdAttributes >> > + IN UINT64 GcdAttributes, >> > + OUT UINT64 *RiscVAttributes >> > ) >> > { >> > - UINT64 RiscVAttributes; >> > + UINT64 CacheTypeMask; >> > + BOOLEAN PmbtExtEnabled; >> > >> Why not read the PCD once and save in a static variable? >> > I can put it into a static variable if you think it is more clean. > Looks like PcdRiscVFeatureOverride can be a patchable PCD so putting it to a static variable may not work. > >> > - RiscVAttributes =3D RISCV_PG_R | RISCV_PG_W | RISCV_PG_X; >> > + if (RiscVAttributes =3D=3D NULL) { >> > + return EFI_INVALID_PARAMETER; >> > + } >> > + >> > + *RiscVAttributes =3D RISCV_PG_R | RISCV_PG_W | RISCV_PG_X; >> > + >> > + PmbtExtEnabled =3D FALSE; >> > + if ((PcdGet64 (PcdRiscVFeatureOverride) & >> RISCV_CPU_FEATURE_PBMT_BITMASK) !=3D 0) { >> > + PmbtExtEnabled =3D TRUE; >> > + } >> > >> > // Determine protection attributes >> > if ((GcdAttributes & EFI_MEMORY_RO) !=3D 0) { >> > - RiscVAttributes &=3D ~(UINT64)(RISCV_PG_W); >> > + *RiscVAttributes &=3D ~(UINT64)(RISCV_PG_W); >> > } >> > >> > // Process eXecute Never attribute >> > if ((GcdAttributes & EFI_MEMORY_XP) !=3D 0) { >> > - RiscVAttributes &=3D ~(UINT64)RISCV_PG_X; >> > + *RiscVAttributes &=3D ~(UINT64)RISCV_PG_X; >> > + } >> > + >> > + CacheTypeMask =3D GcdAttributes & EFI_CACHE_ATTRIBUTE_MASK; >> > + if ((CacheTypeMask !=3D 0) && >> > + (((CacheTypeMask - 1) & CacheTypeMask) !=3D 0)) >> > + { >> > + DEBUG (( >> > + DEBUG_ERROR, >> > + "%a: More than one bit set in cache type mask (0x%LX)\n", >> > + __func__, >> > + CacheTypeMask >> > + )); >> > + return EFI_INVALID_PARAMETER; >> > + } >> > + >> > + switch (CacheTypeMask) { >> > + case EFI_MEMORY_UC: >> > + if (PmbtExtEnabled) { >> > + *RiscVAttributes |=3D PTE_PBMT_IO; >> > + } >> > + >> > + break; >> > + case EFI_MEMORY_WC: >> > + if (PmbtExtEnabled) { >> > + *RiscVAttributes |=3D PTE_PBMT_NC; >> > + } else { >> > + DEBUG (( >> > + DEBUG_VERBOSE, >> > + "%a: EFI_MEMORY_WC set but Pmbt extension not available\n", >> > + __func__ >> > + )); >> > + } >> > + >> > + break; >> > + default: >> > + // Default PMA mode >> > + break; >> > } >> > >> > - return RiscVAttributes; >> > + return EFI_SUCCESS; >> > } >> > >> > /** >> > @@ -535,29 +590,38 @@ RiscVSetMemoryAttributes ( >> > IN UINT64 Attributes >> > ) >> > { >> > - UINT64 PageAttributesSet; >> > + UINT64 PageAttributesSet; >> > + UINT64 PageAttributesClear; >> > + EFI_STATUS Status; >> > >> > - PageAttributesSet =3D GcdAttributeToPageAttribute (Attributes); >> > + Status =3D GcdAttributeToPageAttribute (Attributes, >> &PageAttributesSet); >> > + if (EFI_ERROR (Status)) { >> > + return Status; >> > + } >> > >> Is there a reason to do this prior to checking RiscVMmuEnabled()? >> > Only reason is return error due to invalid parameter before return > EFI_SUCCESS if Mmu not enabled. > >> >> > if (!RiscVMmuEnabled ()) { >> > return EFI_SUCCESS; >> > } >> > >> > - DEBUG ( >> > - ( >> > - DEBUG_VERBOSE, >> > - "%a: Set %llX page attribute 0x%X\n", >> > - __func__, >> > - BaseAddress, >> > - PageAttributesSet >> > - ) >> > - ); >> > + PageAttributesClear =3D PTE_ATTRIBUTES_MASK; >> > + if ((PcdGet64 (PcdRiscVFeatureOverride) & >> RISCV_CPU_FEATURE_PBMT_BITMASK) !=3D 0) { >> > + PageAttributesClear |=3D PTE_PBMT_MASK; >> > + } >> > + >> I think static variable would be better. >> >> > + DEBUG (( >> > + DEBUG_VERBOSE, >> > + "%a: %LX: set attributes 0x%LX, clear attributes 0x%LX\n", >> > + __func__, >> > + BaseAddress, >> > + PageAttributesSet, >> > + PageAttributesClear >> > + )); >> > >> > return UpdateRegionMapping ( >> > BaseAddress, >> > Length, >> > PageAttributesSet, >> > - PTE_ATTRIBUTES_MASK, >> > + PageAttributesClear, >> > (UINT64 *)RiscVGetRootTranslateTable (), >> > TRUE >> > ); >> > diff --git a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf >> b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf >> > index 51ebe1750e97..1dbaa81f3608 100644 >> > --- a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf >> > +++ b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf >> > @@ -28,3 +28,4 @@ >> > >> > [Pcd] >> > gUefiCpuPkgTokenSpaceGuid.PcdCpuRiscVMmuMaxSatpMode ## CONSUMES >> > + gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride ## CONSUMES >> > -- >> > 2.25.1 >> > >> >=20 > > -=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 (#117338): https://edk2.groups.io/g/devel/message/117338 Mute This Topic: https://groups.io/mt/104934719/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- --00000000000038b368061524d276 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Mar 19, 2024= at 9:45=E2=80=AFAM Tuan Phan via groups.io <tphan=3Dventanamicro.co= m@groups.io> wrote:
Hi=C2=A0Sunil,

On Mon, Mar 18, 2024 at 6:00= =E2=80=AFAM Sunil V L <sunilvl@ventanamicro.com> wrote:
Hi Tuan,

On Thu, Mar 14, 2024 at 01:19:16PM -0700, Tuan Phan wrote:
> The GCD EFI_MEMORY_UC and EFI_MEMORY_WC memory attributes will be
> supported when Svpbmt extension available.
>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Ray Ni <r= ay.ni@intel.com>
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> ---
>=C2=A0 .../Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c | 106 ++++++++++++= ++----
>=C2=A0 .../BaseRiscVMmuLib/BaseRiscVMmuLib.inf=C2=A0 =C2=A0 =C2=A0 =C2= =A0|=C2=A0 =C2=A01 +
>=C2=A0 2 files changed, 86 insertions(+), 21 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c b/Ue= fiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> index 46ba4b4709b1..34300dca5c34 100644
> --- a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> +++ b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.c
> @@ -36,6 +36,11 @@
>=C2=A0 #define PTE_PPN_SHIFT=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A010
>=C2=A0 #define RISCV_MMU_PAGE_SHIFT=C2=A0 12
>=C2=A0
> +#define RISCV_CPU_FEATURE_PBMT_BITMASK=C2=A0 BIT2
> +#define PTE_PBMT_NC=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0BIT61
> +#define PTE_PBMT_IO=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0BIT62
> +#define PTE_PBMT_MASK=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0(PTE_PBMT_NC | PTE_PBMT_IO)
> +
>=C2=A0 STATIC UINTN=C2=A0 mModeSupport[] =3D { SATP_MODE_SV57, SATP_MOD= E_SV48, SATP_MODE_SV39, SATP_MODE_OFF };
>=C2=A0 STATIC UINTN=C2=A0 mMaxRootTableLevel;
>=C2=A0 STATIC UINTN=C2=A0 mBitPerLevel;
> @@ -487,32 +492,82 @@ UpdateRegionMapping (
>=C2=A0 /**
>=C2=A0 =C2=A0 Convert GCD attribute to RISC-V page attribute.
>=C2=A0
> -=C2=A0 @param=C2=A0 GcdAttributes The GCD attribute.
> +=C2=A0 @param=C2=A0 GcdAttributes=C2=A0 =C2=A0The GCD attribute.
> +=C2=A0 @param=C2=A0 RiscVAttributes The pointer of RISC-V page attrib= ute.
>=C2=A0
> -=C2=A0 @return=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= The RISC-V page attribute.
> +=C2=A0 @retval EFI_INVALID_PARAMETER=C2=A0 =C2=A0The RiscVAttributes = is NULL or cache type mask not valid.
> +=C2=A0 @retval EFI_SUCCESS=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0The operation succesfully.
>=C2=A0
>=C2=A0 **/
>=C2=A0 STATIC
> -UINT64
> +EFI_STATUS
>=C2=A0 GcdAttributeToPageAttribute (
> -=C2=A0 IN UINT64=C2=A0 GcdAttributes
> +=C2=A0 IN UINT64=C2=A0 =C2=A0GcdAttributes,
> +=C2=A0 OUT UINT64=C2=A0 *RiscVAttributes
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 UINT64=C2=A0 RiscVAttributes;
> +=C2=A0 UINT64=C2=A0 =C2=A0CacheTypeMask;
> +=C2=A0 BOOLEAN=C2=A0 PmbtExtEnabled;
>=C2=A0
Why not read the PCD once and save in a static variable?
I can put it into a static variable if you think it is more clean.=C2=A0=
Looks like=C2=A0PcdRiscVFeatureOverride= can be a patchable PCD so putting it to a static variable may not work.=C2= =A0

> -=C2=A0 RiscVAttributes =3D RISCV_PG_R | RISCV_PG_W | RISCV_PG_X;
> +=C2=A0 if (RiscVAttributes =3D=3D NULL) {
> +=C2=A0 =C2=A0 return EFI_INVALID_PARAMETER;
> +=C2=A0 }
> +
> +=C2=A0 *RiscVAttributes =3D RISCV_PG_R | RISCV_PG_W | RISCV_PG_X;
> +
> +=C2=A0 PmbtExtEnabled =3D FALSE;
> +=C2=A0 if ((PcdGet64 (PcdRiscVFeatureOverride) & RISCV_CPU_FEATUR= E_PBMT_BITMASK) !=3D 0) {
> +=C2=A0 =C2=A0 PmbtExtEnabled =3D TRUE;
> +=C2=A0 }
>=C2=A0
>=C2=A0 =C2=A0 // Determine protection attributes
>=C2=A0 =C2=A0 if ((GcdAttributes & EFI_MEMORY_RO) !=3D 0) {
> -=C2=A0 =C2=A0 RiscVAttributes &=3D ~(UINT64)(RISCV_PG_W);
> +=C2=A0 =C2=A0 *RiscVAttributes &=3D ~(UINT64)(RISCV_PG_W);
>=C2=A0 =C2=A0 }
>=C2=A0
>=C2=A0 =C2=A0 // Process eXecute Never attribute
>=C2=A0 =C2=A0 if ((GcdAttributes & EFI_MEMORY_XP) !=3D 0) {
> -=C2=A0 =C2=A0 RiscVAttributes &=3D ~(UINT64)RISCV_PG_X;
> +=C2=A0 =C2=A0 *RiscVAttributes &=3D ~(UINT64)RISCV_PG_X;
> +=C2=A0 }
> +
> +=C2=A0 CacheTypeMask =3D GcdAttributes & EFI_CACHE_ATTRIBUTE_MASK= ;
> +=C2=A0 if ((CacheTypeMask !=3D 0) &&
> +=C2=A0 =C2=A0 =C2=A0 (((CacheTypeMask - 1) & CacheTypeMask) !=3D = 0))
> +=C2=A0 {
> +=C2=A0 =C2=A0 DEBUG ((
> +=C2=A0 =C2=A0 =C2=A0 DEBUG_ERROR,
> +=C2=A0 =C2=A0 =C2=A0 "%a: More than one bit set in cache type ma= sk (0x%LX)\n",
> +=C2=A0 =C2=A0 =C2=A0 __func__,
> +=C2=A0 =C2=A0 =C2=A0 CacheTypeMask
> +=C2=A0 =C2=A0 =C2=A0 ));
> +=C2=A0 =C2=A0 return EFI_INVALID_PARAMETER;
> +=C2=A0 }
> +
> +=C2=A0 switch (CacheTypeMask) {
> +=C2=A0 =C2=A0 case EFI_MEMORY_UC:
> +=C2=A0 =C2=A0 =C2=A0 if (PmbtExtEnabled) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 *RiscVAttributes |=3D PTE_PBMT_IO;
> +=C2=A0 =C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 case EFI_MEMORY_WC:
> +=C2=A0 =C2=A0 =C2=A0 if (PmbtExtEnabled) {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 *RiscVAttributes |=3D PTE_PBMT_NC;
> +=C2=A0 =C2=A0 =C2=A0 } else {
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 DEBUG ((
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 DEBUG_VERBOSE,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 "%a: EFI_MEMORY_WC set but Pm= bt extension not available\n",
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 __func__
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 ));
> +=C2=A0 =C2=A0 =C2=A0 }
> +
> +=C2=A0 =C2=A0 =C2=A0 break;
> +=C2=A0 =C2=A0 default:
> +=C2=A0 =C2=A0 =C2=A0 // Default PMA mode
> +=C2=A0 =C2=A0 =C2=A0 break;
>=C2=A0 =C2=A0 }
>=C2=A0
> -=C2=A0 return RiscVAttributes;
> +=C2=A0 return EFI_SUCCESS;
>=C2=A0 }
>=C2=A0
>=C2=A0 /**
> @@ -535,29 +590,38 @@ RiscVSetMemoryAttributes (
>=C2=A0 =C2=A0 IN UINT64=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 Attributes
>=C2=A0 =C2=A0 )
>=C2=A0 {
> -=C2=A0 UINT64=C2=A0 PageAttributesSet;
> +=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0 PageAttributesSet;
> +=C2=A0 UINT64=C2=A0 =C2=A0 =C2=A0 PageAttributesClear;
> +=C2=A0 EFI_STATUS=C2=A0 Status;
>=C2=A0
> -=C2=A0 PageAttributesSet =3D GcdAttributeToPageAttribute (Attributes)= ;
> +=C2=A0 Status =3D GcdAttributeToPageAttribute (Attributes, &PageA= ttributesSet);
> +=C2=A0 if (EFI_ERROR (Status)) {
> +=C2=A0 =C2=A0 return Status;
> +=C2=A0 }
>
Is there a reason to do this prior to checking RiscVMmuEnabled()?
Only reason is return error due to invalid parameter before ret= urn=C2=A0 EFI_SUCCESS if Mmu not enabled.=C2=A0

>=C2=A0 =C2=A0 if (!RiscVMmuEnabled ()) {
>=C2=A0 =C2=A0 =C2=A0 return EFI_SUCCESS;
>=C2=A0 =C2=A0 }
>=C2=A0
> -=C2=A0 DEBUG (
> -=C2=A0 =C2=A0 (
> -=C2=A0 =C2=A0 =C2=A0DEBUG_VERBOSE,
> -=C2=A0 =C2=A0 =C2=A0"%a: Set %llX page attribute 0x%X\n", > -=C2=A0 =C2=A0 =C2=A0__func__,
> -=C2=A0 =C2=A0 =C2=A0BaseAddress,
> -=C2=A0 =C2=A0 =C2=A0PageAttributesSet
> -=C2=A0 =C2=A0 )
> -=C2=A0 =C2=A0 );
> +=C2=A0 PageAttributesClear =3D PTE_ATTRIBUTES_MASK;
> +=C2=A0 if ((PcdGet64 (PcdRiscVFeatureOverride) & RISCV_CPU_FEATUR= E_PBMT_BITMASK) !=3D 0) {
> +=C2=A0 =C2=A0 PageAttributesClear |=3D PTE_PBMT_MASK;
> +=C2=A0 }
> +
I think static variable would be better.

> +=C2=A0 DEBUG ((
> +=C2=A0 =C2=A0 DEBUG_VERBOSE,
> +=C2=A0 =C2=A0 "%a: %LX: set attributes 0x%LX, clear attributes 0= x%LX\n",
> +=C2=A0 =C2=A0 __func__,
> +=C2=A0 =C2=A0 BaseAddress,
> +=C2=A0 =C2=A0 PageAttributesSet,
> +=C2=A0 =C2=A0 PageAttributesClear
> +=C2=A0 =C2=A0 ));
>=C2=A0
>=C2=A0 =C2=A0 return UpdateRegionMapping (
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0BaseAddress,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Length,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0PageAttributesSet,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0PTE_ATTRIBUTES_MASK,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0PageAttributesClear,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(UINT64 *)RiscVGetRootT= ranslateTable (),
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0TRUE
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0);
> diff --git a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf b/= UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> index 51ebe1750e97..1dbaa81f3608 100644
> --- a/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> +++ b/UefiCpuPkg/Library/BaseRiscVMmuLib/BaseRiscVMmuLib.inf
> @@ -28,3 +28,4 @@
>=C2=A0
>=C2=A0 [Pcd]
>=C2=A0 =C2=A0 gUefiCpuPkgTokenSpaceGuid.PcdCpuRiscVMmuMaxSatpMode=C2=A0= ## CONSUMES
> +=C2=A0 gEfiMdePkgTokenSpaceGuid.PcdRiscVFeatureOverride=C2=A0 =C2=A0 = =C2=A0## CONSUMES
> --
> 2.25.1
>

_._,_._,_
Groups.io Links:

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

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

_._,_._,_
--00000000000038b368061524d276--