public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Jian J Wang <jian.j.wang@intel.com>
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni <ruiyu.ni@intel.com>, Jiewen Yao <jiewen.yao@intel.com>,
	Eric Dong <eric.dong@intel.com>, Laszlo Ersek <lersek@redhat.com>
Subject: [PATCH 2/2] UefiCpuPkg/CpuDxe: remove all code to flush TLB for APs
Date: Fri, 26 Jan 2018 17:03:07 +0800	[thread overview]
Message-ID: <20180126090307.6872-3-jian.j.wang@intel.com> (raw)
In-Reply-To: <20180126090307.6872-1-jian.j.wang@intel.com>

The reason doing this is that we found that calling StartupAllAps() to
flush TLB for all APs in CpuDxe driver after changing page attributes
will spend a lot of time to complete. If there are many page attributes
update requests, the whole system performance will be slowed down
explicitly, including any shell command and UI operation.

The solution is removing the flush operation for AP in CpuDxe driver
and let AP flush TLB after woken up.

Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 85 +++-------------------------------------
 1 file changed, 5 insertions(+), 80 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index a33ac5519e..a5bf0dfe28 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -89,70 +89,6 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
 
 PAGE_TABLE_POOL   *mPageTablePool = NULL;
 
-/**
-  Enable write protection function for AP.
-
-  @param[in,out] Buffer  The pointer to private data buffer.
-**/
-VOID
-EFIAPI
-SyncCpuEnableWriteProtection (
-  IN OUT VOID *Buffer
-  )
-{
-  AsmWriteCr0 (AsmReadCr0 () | BIT16);
-}
-
-/**
-  CpuFlushTlb function for AP.
-
-  @param[in,out] Buffer  The pointer to private data buffer.
-**/
-VOID
-EFIAPI
-SyncCpuFlushTlb (
-  IN OUT VOID *Buffer
-  )
-{
-  CpuFlushTlb();
-}
-
-/**
-  Sync memory page attributes for AP.
-
-  @param[in] Procedure            A pointer to the function to be run on enabled APs of
-                                  the system.
-**/
-VOID
-SyncMemoryPageAttributesAp (
-  IN EFI_AP_PROCEDURE            Procedure
-  )
-{
-  EFI_STATUS                Status;
-  EFI_MP_SERVICES_PROTOCOL  *MpService;
-
-  Status = gBS->LocateProtocol (
-                  &gEfiMpServiceProtocolGuid,
-                  NULL,
-                  (VOID **)&MpService
-                  );
-  //
-  // Synchronize the update with all APs
-  //
-  if (!EFI_ERROR (Status)) {
-    Status = MpService->StartupAllAPs (
-                          MpService,          // This
-                          Procedure,          // Procedure
-                          FALSE,              // SingleThread
-                          NULL,               // WaitEvent
-                          0,                  // TimeoutInMicrosecsond
-                          NULL,               // ProcedureArgument
-                          NULL                // FailedCpuList
-                          );
-    ASSERT (Status == EFI_SUCCESS || Status == EFI_NOT_STARTED || Status == EFI_NOT_READY);
-  }
-}
-
 /**
   Return current paging context.
 
@@ -574,20 +510,6 @@ IsReadOnlyPageWriteProtected (
   return ((AsmReadCr0 () & BIT16) != 0);
 }
 
-/**
-  Disable write protection function for AP.
-
-  @param[in,out] Buffer  The pointer to private data buffer.
-**/
-VOID
-EFIAPI
-SyncCpuDisableWriteProtection (
-  IN OUT VOID *Buffer
-  )
-{
-  AsmWriteCr0 (AsmReadCr0() & ~BIT16);
-}
-
 /**
  Disable Write Protect on pages marked as read-only.
 **/
@@ -835,10 +757,13 @@ AssignMemoryPageAttributes (
   if (!EFI_ERROR(Status)) {
     if ((PagingContext == NULL) && IsModified) {
       //
-      // Flush TLB as last step
+      // Flush TLB as last step.
+      //
+      // Note: Don't flush TLB for APs here. It will take a lot of time to
+      // complete, and then slow down boot performance of the whole system
+      // if page attributes are requested frequently to update.
       //
       CpuFlushTlb();
-      SyncMemoryPageAttributesAp (SyncCpuFlushTlb);
     }
   }
 
-- 
2.15.1.windows.2



  parent reply	other threads:[~2018-01-26  8:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-26  9:03 [PATCH 0/2] Fix bad performance in changing page attributes Jian J Wang
2018-01-26  9:03 ` [PATCH 1/2] UefiCpuPkg/MpInitLib: force flushing TLB for AP in mwait loop mode Jian J Wang
2018-01-26  9:14   ` Ni, Ruiyu
2018-01-26  9:03 ` Jian J Wang [this message]
2018-01-26  9:13   ` [PATCH 2/2] UefiCpuPkg/CpuDxe: remove all code to flush TLB for APs Ni, Ruiyu
2018-01-26  9:18     ` Wang, Jian J

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180126090307.6872-3-jian.j.wang@intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox