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 1F7C174004D for ; 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 ; Mon, 18 Mar 2024 06:00:26 -0700 X-Received: by mail-ot1-f43.google.com with SMTP id 46e09a7af769-6e6969855c8so399352a34.0 for ; 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" To: Tuan Phan 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: 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: List-Help: Sender: devel@edk2.groups.io List-Id: 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: 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 > 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[] = { 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] -=-=-=-=-=-=-=-=-=-=-=-