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: Jiewen Yao <jiewen.yao@intel.com>,
	Star Zeng <star.zeng@intel.com>, Laszlo Ersek <lersek@redhat.com>,
	Michael Kinney <michael.d.kinney@intel.com>
Subject: [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page
Date: Mon, 18 Sep 2017 11:08:55 +0800	[thread overview]
Message-ID: <20170918030855.11876-3-jian.j.wang@intel.com> (raw)
In-Reply-To: <20170918030855.11876-1-jian.j.wang@intel.com>

>From CpuDxe driver perspective, it doesn't update GCD memory attributes 
from current page table setup during its initialization. So the memory
attributes in GCD might not reflect all memory attributes in real world.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Suggested-by: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 +++
 UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 92 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 106 insertions(+)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index b386f3b677..4e8fa100e0 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -863,6 +863,11 @@ RefreshGcdMemoryAttributes (
     FreePool (MemorySpaceMap);
   }
 
+  //
+  // Update page attributes
+  //
+  RefreshGcdMemoryAttributesFromPaging();
+
   mIsFlushingGCD = FALSE;
 }
 
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index 4861abee76..a25b35c6eb 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -52,6 +52,10 @@
                                        EFI_MEMORY_UCE   \
                                        )
 
+#define EFI_MEMORY_PAGETYPE_MASK      (EFI_MEMORY_RP  | \
+                                       EFI_MEMORY_XP  | \
+                                       EFI_MEMORY_RO    \
+                                       )
 
 /**
   Flush CPU data cache. If the instruction cache is fully coherent
@@ -261,5 +265,10 @@ SetDataSelectors (
   UINT16 Selector
   );
 
+VOID
+RefreshGcdMemoryAttributesFromPaging (
+  VOID
+  );
+
 #endif
 
diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 2c61e7503e..bca4a497ac 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -23,6 +23,8 @@
 #include <Library/DebugLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Protocol/MpService.h>
+
+#include "CpuDxe.h"
 #include "CpuPageTable.h"
 
 ///
@@ -767,6 +769,96 @@ AssignMemoryPageAttributes (
   return Status;
 }
 
+/**
+  Update GCD memory space attributes according to current page table setup.
+**/
+VOID
+RefreshGcdMemoryAttributesFromPaging (
+  VOID
+  )
+{
+  EFI_STATUS                          Status;
+  UINTN                               NumberOfDescriptors;
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR     *MemorySpaceMap;
+  PAGE_TABLE_LIB_PAGING_CONTEXT       PagingContext;
+  PAGE_ATTRIBUTE                      PageAttribute;
+  UINT64                              *PageEntry;
+  UINT64                              PageLength;
+  UINT64                              MemorySpaceLength;
+  UINT64                              Length;
+  UINT64                              BaseAddress;
+  UINT64                              PageStartAddress;
+  UINT64                              Attributes;
+  BOOLEAN                             DoUpdate;
+  UINTN                               Index;
+
+  //
+  // Assuming that memory space map returned is sorted already; otherwise sort
+  // them in the order of lowest address to highest address.
+  //
+  Status = gDS->GetMemorySpaceMap (&NumberOfDescriptors, &MemorySpaceMap);
+  ASSERT_EFI_ERROR (Status);
+
+  GetCurrentPagingContext (&PagingContext);
+
+  BaseAddress = 0;
+  PageLength  = 0;
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {
+    if (MemorySpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeNonExistent) {
+      continue;
+    }
+
+    if (MemorySpaceMap[Index].BaseAddress >= (BaseAddress + PageLength)) {
+      //
+      // Current memory space starts at a new page. Resetting PageLength will
+      // trigger a retrieval of page attributes at new address.
+      //
+      PageLength = 0;
+    }
+
+    // Sync real page attributes to GCD
+    BaseAddress       = MemorySpaceMap[Index].BaseAddress;
+    MemorySpaceLength = MemorySpaceMap[Index].Length;
+    while (MemorySpaceLength > 0) {
+      if (PageLength == 0) {
+        PageEntry = GetPageTableEntry (&PagingContext, BaseAddress, &PageAttribute);
+        if (PageEntry == NULL) {
+          break;
+        }
+
+        //
+        // Note current memory space might start in the middle of a page
+        //
+        PageStartAddress  = (*PageEntry) & (UINT64)PageAttributeToMask(PageAttribute);
+        PageLength        = PageAttributeToLength (PageAttribute) - (BaseAddress - PageStartAddress);
+        Attributes        = GetAttributesFromPageEntry (PageEntry);
+
+        if (Attributes != (MemorySpaceMap[Index].Attributes & EFI_MEMORY_PAGETYPE_MASK)) {
+          DoUpdate = TRUE;
+          Attributes |= (MemorySpaceMap[Index].Attributes & ~EFI_MEMORY_PAGETYPE_MASK);
+        } else {
+          DoUpdate = FALSE;
+        }
+      }
+
+      Length = MIN (PageLength, MemorySpaceLength);
+      if (DoUpdate) {
+        gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Attributes);
+        gDS->SetMemorySpaceAttributes (BaseAddress, Length, Attributes);
+        DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx - %016lx (%08lx -> %08lx)\r\n",
+                             Index, BaseAddress, BaseAddress + Length - 1,
+                             MemorySpaceMap[Index].Attributes, Attributes));
+      }
+
+      PageLength        -= Length;
+      MemorySpaceLength -= Length;
+      BaseAddress       += Length;
+    }
+  }
+
+  FreePool (MemorySpaceMap);
+}
+
 /**
   Initialize the Page Table lib.
 **/
-- 
2.14.1.windows.1



  parent reply	other threads:[~2017-09-18  3:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-18  3:08 [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Jian J Wang
2017-09-18  3:08 ` [PATCH 1/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD Jian J Wang
2017-09-18  3:08 ` Jian J Wang [this message]
2017-09-19  6:09 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver 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=20170918030855.11876-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