From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <bounce+27952+116839+7686176+12367111@groups.io>
Received: from mail02.groups.io (mail02.groups.io [66.175.222.108])
by spool.mail.gandi.net (Postfix) with ESMTPS id 1F7C174004D
for <rebecca@openfw.io>; Mon, 18 Mar 2024 13:00:27 +0000 (UTC)
DKIM-Signature: a=rsa-sha256; bh=fOqmiyXNCV9dsLj8jKoMnCKJjMmjOGXuM5QfsHi4Fa4=;
c=relaxed/simple; d=groups.io;
h=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:Resent-Date:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Disposition;
s=20240206; t=1710766826; v=1;
b=uUOzOS/N14mNdvaIlLVbmGTz53j+so6OWpHWXQ3/7gD5W/bZwhzXb+Gjw0s/21wD9mgYwdxx
/g7rfwrnlC1BRxlwdOi/mlHxKUFOyqYprf703G/UkBeAEYEUFbVYdSjfSXHH9AYzIgGNpRWnJpn
S21WHm7Z02XQ4YO8lrALW/BMtc6WD36i0MezchhP0n6LytYRXeYEcZLn+NspF5JBj/1/4A0tYoS
wgx7r709IbOJt84//GoXpwFVUmIyXuV7XRmsIalnUNb+zt8MWwGWgfpEGM9O944mwIAkZ1Ng3mJ
zSfz3xMvFfGiOA41CxmVxsPtPqKnLIvrkJqqfcNL5XrLQ==
X-Received: by 127.0.0.2 with SMTP id urrLYY7687511xWFWv4AU4je; Mon, 18 Mar 2024 06:00:26 -0700
X-Received: from mail-ot1-f43.google.com (mail-ot1-f43.google.com [209.85.210.43])
by mx.groups.io with SMTP id smtpd.web10.43477.1710766825911702796
for <devel@edk2.groups.io>;
Mon, 18 Mar 2024 06:00:26 -0700
X-Received: by mail-ot1-f43.google.com with SMTP id 46e09a7af769-6e6969855c8so399352a34.0
for <devel@edk2.groups.io>; Mon, 18 Mar 2024 06:00:25 -0700 (PDT)
X-Gm-Message-State: 72kcTUC4KJVj0X36a9TPx7l1x7686176AA=
X-Google-Smtp-Source: AGHT+IFEk/0I/1gtEyqHTkeR6EdHXN/zc6IKMjFzOLs05zp0n9OzZJRGQojgzkdlBlsTABWQXvixow==
X-Received: by 2002:a9d:6d02:0:b0:6e4:f67f:9d46 with SMTP id o2-20020a9d6d02000000b006e4f67f9d46mr12472882otp.31.1710766824933;
Mon, 18 Mar 2024 06:00:24 -0700 (PDT)
X-Received: from sunil-laptop ([106.51.185.90])
by smtp.gmail.com with ESMTPSA id w14-20020a056830144e00b006e693faef04sm307617otp.79.2024.03.18.06.00.19
(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);
Mon, 18 Mar 2024 06:00:24 -0700 (PDT)
Date: Mon, 18 Mar 2024 18:30:14 +0530
From: "Sunil V L" <sunilvl@ventanamicro.com>
To: Tuan Phan <tphan@ventanamicro.com>
Cc: devel@edk2.groups.io, 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
Subject: Re: [edk2-devel] [PATCH v4 3/4] UefiCpuPkg: RISC-V: MMU: Support Svpbmt extension
Message-ID: <Zfg63v8lfMKa0rW5@sunil-laptop>
References: <20240314201917.1756-1-tphan@ventanamicro.com>
<20240314201917.1756-4-tphan@ventanamicro.com>
MIME-Version: 1.0
In-Reply-To: <20240314201917.1756-4-tphan@ventanamicro.com>
Precedence: Bulk
List-Subscribe: <mailto:devel+subscribe@edk2.groups.io>
List-Help: <mailto:devel+help@edk2.groups.io>
Sender: devel@edk2.groups.io
List-Id: <devel.edk2.groups.io>
Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io
Resent-Date: Mon, 18 Mar 2024 06:00:26 -0700
Reply-To: devel@edk2.groups.io,sunilvl@ventanamicro.com
List-Unsubscribe-Post: List-Unsubscribe=One-Click
List-Unsubscribe: <https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/plugh>
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=20240206 header.b="uUOzOS/N";
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
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 <ray.ni@intel.com>
> Signed-off-by: Tuan Phan <tphan@ventanamicro.com>
> ---
> .../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[] = { 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 cache 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?
> - RiscVAttributes = RISCV_PG_R | RISCV_PG_W | RISCV_PG_X;
> + if (RiscVAttributes == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *RiscVAttributes = RISCV_PG_R | RISCV_PG_W | RISCV_PG_X;
> +
> + PmbtExtEnabled = FALSE;
> + if ((PcdGet64 (PcdRiscVFeatureOverride) & RISCV_CPU_FEATURE_PBMT_BITMASK) != 0) {
> + PmbtExtEnabled = TRUE;
> + }
>
> // Determine protection attributes
> if ((GcdAttributes & EFI_MEMORY_RO) != 0) {
> - RiscVAttributes &= ~(UINT64)(RISCV_PG_W);
> + *RiscVAttributes &= ~(UINT64)(RISCV_PG_W);
> }
>
> // Process eXecute Never attribute
> if ((GcdAttributes & EFI_MEMORY_XP) != 0) {
> - RiscVAttributes &= ~(UINT64)RISCV_PG_X;
> + *RiscVAttributes &= ~(UINT64)RISCV_PG_X;
> + }
> +
> + CacheTypeMask = GcdAttributes & EFI_CACHE_ATTRIBUTE_MASK;
> + if ((CacheTypeMask != 0) &&
> + (((CacheTypeMask - 1) & CacheTypeMask) != 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 |= PTE_PBMT_IO;
> + }
> +
> + break;
> + case EFI_MEMORY_WC:
> + if (PmbtExtEnabled) {
> + *RiscVAttributes |= 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 = GcdAttributeToPageAttribute (Attributes);
> + Status = GcdAttributeToPageAttribute (Attributes, &PageAttributesSet);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
>
Is there a reason to do this prior to checking RiscVMmuEnabled()?
> if (!RiscVMmuEnabled ()) {
> return EFI_SUCCESS;
> }
>
> - DEBUG (
> - (
> - DEBUG_VERBOSE,
> - "%a: Set %llX page attribute 0x%X\n",
> - __func__,
> - BaseAddress,
> - PageAttributesSet
> - )
> - );
> + PageAttributesClear = PTE_ATTRIBUTES_MASK;
> + if ((PcdGet64 (PcdRiscVFeatureOverride) & RISCV_CPU_FEATURE_PBMT_BITMASK) != 0) {
> + PageAttributesClear |= 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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116839): https://edk2.groups.io/g/devel/message/116839
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]
-=-=-=-=-=-=-=-=-=-=-=-