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 B60D8D801EA for ; Tue, 6 Feb 2024 01:58:18 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=6wwYqoMack3nsp7WuXOjhm7JL6vJYSjzoApDetKVy90=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: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=1707184697; v=1; b=SJ6j4xbCTLtrFV7yIazQDFaarLzDxggqJ3Xz7z7eLgcjKmOF1mEzjw33iQLtpr0gFTzj/Z8K 87PfZLJJ/9WWGbef1jtwck1I5gMzsthRvGvJUCnj6XubJQ1trF20uLPLL9kGFdktgEd5b6BQo3h GbVb4g0ZKQSOV7uj8++G3eCo= X-Received: by 127.0.0.2 with SMTP id i8I3YY7687511xIhbdrQuqWa; Mon, 05 Feb 2024 17:58:17 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.10]) by mx.groups.io with SMTP id smtpd.web11.12552.1707184694246358540 for ; Mon, 05 Feb 2024 17:58:16 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10975"; a="18074366" X-IronPort-AV: E=Sophos;i="6.05,245,1701158400"; d="scan'208";a="18074366" X-Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orvoesa102.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Feb 2024 17:58:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10975"; a="909469804" X-IronPort-AV: E=Sophos;i="6.05,245,1701158400"; d="scan'208";a="909469804" X-Received: from shwdeopenlab702.ccr.corp.intel.com ([10.239.55.43]) by fmsmga002-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 05 Feb 2024 17:58:14 -0800 From: "duntan" To: devel@edk2.groups.io Cc: Zhou Jianfeng , Ray Ni , Laszlo Ersek , Rahul Kumar , Gerd Hoffmann Subject: [edk2-devel] [Patch V2 1/3] UefiCpuPkg: Reduce and optimize access to attribute Date: Tue, 6 Feb 2024 09:57:55 +0800 Message-Id: <20240206015757.1816-2-dun.tan@intel.com> In-Reply-To: <20240206015757.1816-1-dun.tan@intel.com> References: <20240206015757.1816-1-dun.tan@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,dun.tan@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: DGwtMQUqGVTRU9xZm7nnkUTyx7686176AA= 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=SJ6j4xbC; 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) From: Zhou Jianfeng This commit is to reduce and optimize access to attribute in CpuPageTableLib. Unreasonable writing to attribute of page table may leads to expection. The assembly code for C code Pnle->Bits.Present = Attribute->Bits.Present looks like: and dword [rcx], 0xfffffffe and eax, 0x1 or [rcx], eax In case Pnle->Bits.Present and Attribute->Bits.Present is 1, Pnle->Bits.Present will be set to 0 for short time(2 instructions) which is unexpected. If some other core is accessing the page, it may leads to expection. This change reduce and optimize access to attribute of page table, attribute of page table is set only when it need to be changed. Signed-off-by: Zhou Jianfeng Reviewed-by: Ray Ni Cc: Laszlo Ersek Cc: Rahul Kumar Cc: Gerd Hoffmann --- UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------- 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c index 36b2c4e6a3..ae4caf8dfe 100644 --- a/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c +++ b/UefiCpuPkg/Library/CpuPageTableLib/CpuPageTableMap.c @@ -26,52 +26,59 @@ PageTableLibSetPte4K ( IN IA32_MAP_ATTRIBUTE *Mask ) { + IA32_PTE_4K LocalPte4K; + + LocalPte4K.Uint64 = Pte4K->Uint64; if (Mask->Bits.PageTableBaseAddressLow || Mask->Bits.PageTableBaseAddressHigh) { - Pte4K->Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (Pte4K->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40); + LocalPte4K.Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (Pte4K->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_40); } if (Mask->Bits.Present) { - Pte4K->Bits.Present = Attribute->Bits.Present; + LocalPte4K.Bits.Present = Attribute->Bits.Present; } if (Mask->Bits.ReadWrite) { - Pte4K->Bits.ReadWrite = Attribute->Bits.ReadWrite; + LocalPte4K.Bits.ReadWrite = Attribute->Bits.ReadWrite; } if (Mask->Bits.UserSupervisor) { - Pte4K->Bits.UserSupervisor = Attribute->Bits.UserSupervisor; + LocalPte4K.Bits.UserSupervisor = Attribute->Bits.UserSupervisor; } if (Mask->Bits.WriteThrough) { - Pte4K->Bits.WriteThrough = Attribute->Bits.WriteThrough; + LocalPte4K.Bits.WriteThrough = Attribute->Bits.WriteThrough; } if (Mask->Bits.CacheDisabled) { - Pte4K->Bits.CacheDisabled = Attribute->Bits.CacheDisabled; + LocalPte4K.Bits.CacheDisabled = Attribute->Bits.CacheDisabled; } if (Mask->Bits.Accessed) { - Pte4K->Bits.Accessed = Attribute->Bits.Accessed; + LocalPte4K.Bits.Accessed = Attribute->Bits.Accessed; } if (Mask->Bits.Dirty) { - Pte4K->Bits.Dirty = Attribute->Bits.Dirty; + LocalPte4K.Bits.Dirty = Attribute->Bits.Dirty; } if (Mask->Bits.Pat) { - Pte4K->Bits.Pat = Attribute->Bits.Pat; + LocalPte4K.Bits.Pat = Attribute->Bits.Pat; } if (Mask->Bits.Global) { - Pte4K->Bits.Global = Attribute->Bits.Global; + LocalPte4K.Bits.Global = Attribute->Bits.Global; } if (Mask->Bits.ProtectionKey) { - Pte4K->Bits.ProtectionKey = Attribute->Bits.ProtectionKey; + LocalPte4K.Bits.ProtectionKey = Attribute->Bits.ProtectionKey; } if (Mask->Bits.Nx) { - Pte4K->Bits.Nx = Attribute->Bits.Nx; + LocalPte4K.Bits.Nx = Attribute->Bits.Nx; + } + + if (Pte4K->Uint64 != LocalPte4K.Uint64) { + Pte4K->Uint64 = LocalPte4K.Uint64; } } @@ -93,54 +100,61 @@ PageTableLibSetPleB ( IN IA32_MAP_ATTRIBUTE *Mask ) { + IA32_PAGE_LEAF_ENTRY_BIG_PAGESIZE LocalPleB; + + LocalPleB.Uint64 = PleB->Uint64; if (Mask->Bits.PageTableBaseAddressLow || Mask->Bits.PageTableBaseAddressHigh) { - PleB->Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (PleB->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_39); + LocalPleB.Uint64 = (IA32_MAP_ATTRIBUTE_PAGE_TABLE_BASE_ADDRESS (Attribute) + Offset) | (PleB->Uint64 & ~IA32_PE_BASE_ADDRESS_MASK_39); } - PleB->Bits.MustBeOne = 1; + LocalPleB.Bits.MustBeOne = 1; if (Mask->Bits.Present) { - PleB->Bits.Present = Attribute->Bits.Present; + LocalPleB.Bits.Present = Attribute->Bits.Present; } if (Mask->Bits.ReadWrite) { - PleB->Bits.ReadWrite = Attribute->Bits.ReadWrite; + LocalPleB.Bits.ReadWrite = Attribute->Bits.ReadWrite; } if (Mask->Bits.UserSupervisor) { - PleB->Bits.UserSupervisor = Attribute->Bits.UserSupervisor; + LocalPleB.Bits.UserSupervisor = Attribute->Bits.UserSupervisor; } if (Mask->Bits.WriteThrough) { - PleB->Bits.WriteThrough = Attribute->Bits.WriteThrough; + LocalPleB.Bits.WriteThrough = Attribute->Bits.WriteThrough; } if (Mask->Bits.CacheDisabled) { - PleB->Bits.CacheDisabled = Attribute->Bits.CacheDisabled; + LocalPleB.Bits.CacheDisabled = Attribute->Bits.CacheDisabled; } if (Mask->Bits.Accessed) { - PleB->Bits.Accessed = Attribute->Bits.Accessed; + LocalPleB.Bits.Accessed = Attribute->Bits.Accessed; } if (Mask->Bits.Dirty) { - PleB->Bits.Dirty = Attribute->Bits.Dirty; + LocalPleB.Bits.Dirty = Attribute->Bits.Dirty; } if (Mask->Bits.Pat) { - PleB->Bits.Pat = Attribute->Bits.Pat; + LocalPleB.Bits.Pat = Attribute->Bits.Pat; } if (Mask->Bits.Global) { - PleB->Bits.Global = Attribute->Bits.Global; + LocalPleB.Bits.Global = Attribute->Bits.Global; } if (Mask->Bits.ProtectionKey) { - PleB->Bits.ProtectionKey = Attribute->Bits.ProtectionKey; + LocalPleB.Bits.ProtectionKey = Attribute->Bits.ProtectionKey; } if (Mask->Bits.Nx) { - PleB->Bits.Nx = Attribute->Bits.Nx; + LocalPleB.Bits.Nx = Attribute->Bits.Nx; + } + + if (PleB->Uint64 != LocalPleB.Uint64) { + PleB->Uint64 = LocalPleB.Uint64; } } @@ -186,24 +200,27 @@ PageTableLibSetPnle ( IN IA32_MAP_ATTRIBUTE *Mask ) { + IA32_PAGE_NON_LEAF_ENTRY LocalPnle; + + LocalPnle.Uint64 = Pnle->Uint64; if (Mask->Bits.Present) { - Pnle->Bits.Present = Attribute->Bits.Present; + LocalPnle.Bits.Present = Attribute->Bits.Present; } if (Mask->Bits.ReadWrite) { - Pnle->Bits.ReadWrite = Attribute->Bits.ReadWrite; + LocalPnle.Bits.ReadWrite = Attribute->Bits.ReadWrite; } if (Mask->Bits.UserSupervisor) { - Pnle->Bits.UserSupervisor = Attribute->Bits.UserSupervisor; + LocalPnle.Bits.UserSupervisor = Attribute->Bits.UserSupervisor; } if (Mask->Bits.Nx) { - Pnle->Bits.Nx = Attribute->Bits.Nx; + LocalPnle.Bits.Nx = Attribute->Bits.Nx; } - Pnle->Bits.Accessed = 0; - Pnle->Bits.MustBeZero = 0; + LocalPnle.Bits.Accessed = 0; + LocalPnle.Bits.MustBeZero = 0; // // Set the attributes (WT, CD, A) to 0. @@ -211,8 +228,11 @@ PageTableLibSetPnle ( // So, it implictly requires PAT[0] is Write Back. // Create a new parameter if caller requires to use a different memory type for accessing page directories. // - Pnle->Bits.WriteThrough = 0; - Pnle->Bits.CacheDisabled = 0; + LocalPnle.Bits.WriteThrough = 0; + LocalPnle.Bits.CacheDisabled = 0; + if (Pnle->Uint64 != LocalPnle.Uint64) { + Pnle->Uint64 = LocalPnle.Uint64; + } } /** -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#115136): https://edk2.groups.io/g/devel/message/115136 Mute This Topic: https://groups.io/mt/104191039/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-