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 DCDF6D81113 for ; Wed, 17 Jan 2024 08:10:05 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=9Z0xmTTAaLdOBh5DxQdemYxI5sYJY9I7URl8zqy4yzM=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1705479004; v=1; b=s6iqd+pslwNGdNGwol+8E/Cxw71c83Te22V6Pw5HOgsv3myzLvhewCb8BMCnaPRXgUN4F1fY TM+I4V/hE2wMF+SwQgSXjkp9BlsFwWv++npjs0gI1x2p6MpSZRva/1hdZATRcQIzY0X3z4B1ZlH yGp1NRJuO0ufVuWpwWaxoZBQ= X-Received: by 127.0.0.2 with SMTP id gbI2YY7687511xMq4GLKV1TK; Wed, 17 Jan 2024 00:10:04 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by mx.groups.io with SMTP id smtpd.web10.4651.1705479003939318421 for ; Wed, 17 Jan 2024 00:10:04 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10955"; a="13460539" X-IronPort-AV: E=Sophos;i="6.05,200,1701158400"; d="scan'208";a="13460539" X-Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Jan 2024 00:10:04 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10955"; a="1031270195" X-IronPort-AV: E=Sophos;i="6.05,200,1701158400"; d="scan'208";a="1031270195" X-Received: from shwdesfp01.ccr.corp.intel.com ([10.239.158.151]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 17 Jan 2024 00:10:00 -0800 From: "Zhiguang Liu" To: devel@edk2.groups.io Cc: Zhiguang Liu , Ray Ni , Laszlo Ersek , Rahul Kumar , Gerd Hoffmann , Crystal Lee Subject: [edk2-devel] [PATCH v2] UefiCpuPkg: Fix issue that IsModified is wrongly set in PageTableMap Date: Wed, 17 Jan 2024 16:09:54 +0800 Message-Id: <20240117080954.1414-1-zhiguang.liu@intel.com> MIME-Version: 1.0 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,zhiguang.liu@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: ryNOKQXIfaf4nPn0Q8YJNH5dx7686176AA= Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=s6iqd+ps; 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=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none) REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4614 About the IsModified, current function doesn't consider that hardware also may change the pagetable. The issue is that in the first call of internal function PageTableLibMapInLevel, the function assume page table is not changed, and add ASSERT to check. But hardware may change the page table, which cause the ASSERT happens. Fix the issue by considering the hardware may also change page table, and document the detail in function header. Cc: Ray Ni Cc: Laszlo Ersek Cc: Rahul Kumar Cc: Gerd Hoffmann Cc: Crystal Lee Signed-off-by: Zhiguang Liu --- .../Library/CpuPageTableLib/CpuPageTableMap.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c index 36b2c4e6a3..a3076ff2f6 100644 --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c @@ -274,7 +274,7 @@ IsAttributesAndMaskValidForNonPresentEntry ( Page table entries that map the linear address range are reset to 0 before set to the new attribute when a new physical base address is set. @param[in] Mask The mask used for attribute. The corresponding field in Attribute is ignored if that in Mask is 0. - @param[out] IsModified TRUE means page table is modified. FALSE means page table is not modified. + @param[in, out] IsModified Change IsModified to True if page table is modified and input parameter Modify is TRUE. @retval RETURN_INVALID_PARAMETER For non-present range, Mask->Bits.Present is 0 but some other attributes are provided. @retval RETURN_INVALID_PARAMETER For non-present range, Mask->Bits.Present is 1, Attribute->Bits.Present is 1 but some other attributes are not provided. @@ -567,7 +567,10 @@ PageTableLibMapInLevel ( OriginalCurrentPagingEntry.Uint64 = CurrentPagingEntry->Uint64; PageTableLibSetPle (Level, CurrentPagingEntry, Offset, Attribute, &CurrentMask); - if (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64) { + if (Modify && (OriginalCurrentPagingEntry.Uint64 != CurrentPagingEntry->Uint64)) { + // + // The page table entry can be changed by this function only when Modify is true. + // *IsModified = TRUE; } } @@ -609,7 +612,10 @@ PageTableLibMapInLevel ( // Check if ParentPagingEntry entry is modified here is enough. Except the changes happen in leaf PagingEntry during // the while loop, if there is any other change happens in page table, the ParentPagingEntry must has been modified. // - if (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64) { + if (Modify && (OriginalParentPagingEntry.Uint64 != ParentPagingEntry->Uint64)) { + // + // The page table entry can be changed by this function only when Modify is true. + // *IsModified = TRUE; } @@ -633,7 +639,9 @@ PageTableLibMapInLevel ( Page table entries that map the linear address range are reset to 0 before set to the new attribute when a new physical base address is set. @param[in] Mask The mask used for attribute. The corresponding field in Attribute is ignored if that in Mask is 0. - @param[out] IsModified TRUE means page table is modified. FALSE means page table is not modified. + @param[out] IsModified TRUE means page table is modified by software or hardware. FALSE means page table is not modified by software. + If the output IsModified is FALSE, there is possibility that the page table is changed by hardware. It is ok + because page table can be changed by hardware anytime, and caller don't need to Flush TLB. @retval RETURN_UNSUPPORTED PagingMode is not supported. @retval RETURN_INVALID_PARAMETER PageTable, BufferSize, Attribute or Mask is NULL. -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113936): https://edk2.groups.io/g/devel/message/113936 Mute This Topic: https://groups.io/mt/103781942/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-