public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
@ 2017-09-18  3:08 Jian J Wang
  2017-09-19  6:09 ` Wang, Jian J
  0 siblings, 1 reply; 13+ messages in thread
From: Jian J Wang @ 2017-09-18  3:08 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jiewen Yao, Star Zeng, Laszlo Ersek, Michael Kinney

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't accept
page related attributes. That means users cannot use it to change page
attributes, and have to turn to CPU arch protocol to do it, which is not
be allowed by PI spec.

>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>

Jian J Wang (2):
  MdeModulePkg/Core: Fix out-of-sync issue in GCD
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page attributes

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++++--------
 UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 +++
 UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 92 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+), 18 deletions(-)

-- 
2.14.1.windows.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-18  3:08 Jian J Wang
@ 2017-09-19  6:09 ` Wang, Jian J
  0 siblings, 0 replies; 13+ messages in thread
From: Wang, Jian J @ 2017-09-19  6:09 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Yao, Jiewen, Zeng, Star

I found there's a logic hole in code. A new patch will be sent out.

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J Wang
Sent: Monday, September 18, 2017 11:09 AM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't accept
page related attributes. That means users cannot use it to change page
attributes, and have to turn to CPU arch protocol to do it, which is not
be allowed by PI spec.

>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>

Jian J Wang (2):
  MdeModulePkg/Core: Fix out-of-sync issue in GCD
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page attributes

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++++--------
 UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 +++
 UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 92 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 133 insertions(+), 18 deletions(-)

-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
       [not found] <Fixe out-of-sync issue between GCD and CPU driver>
@ 2017-09-19  6:10 ` Jian J Wang
  2017-09-19  6:10   ` [PATCH 1/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe Jian J Wang
                     ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Jian J Wang @ 2017-09-19  6:10 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jiewen Yao, Eric Dong, Star Zeng, Laszlo Ersek, Michael Kinney

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't
accept page related attributes. That means users cannot use it to
change page attributes, and have to turn to CPU arch protocol to do it,
which is not be allowed by PI spec.

>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: Eric Dong <eric.dong@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>

Jian J Wang (2):
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
  MdeModulePkg/Core: Fix out-of-sync issue in GCD

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++--------
 UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 ++
 UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 99 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 140 insertions(+), 18 deletions(-)

-- 
2.14.1.windows.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
  2017-09-19  6:10 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Jian J Wang
@ 2017-09-19  6:10   ` Jian J Wang
  2017-09-19  6:10   ` [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD Jian J Wang
  2017-09-20  5:11   ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Wang, Jian J
  2 siblings, 0 replies; 13+ messages in thread
From: Jian J Wang @ 2017-09-19  6:10 UTC (permalink / raw)
  To: edk2-devel; +Cc: Eric Dong, Jiewen Yao, Star Zeng, Laszlo Ersek, Michael Kinney

>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: Eric Dong <eric.dong@intel.com>
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 | 99 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 113 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..ae93f3f553 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,103 @@ 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;
+  UINT64                              Capabilities;
+  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;
+    } else {
+      //
+      // In case current memory space is not adjacent to last one
+      //
+      PageLength -= (MemorySpaceMap[Index].BaseAddress - BaseAddress);
+    }
+
+    // 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);
+          Capabilities = Attributes | MemorySpaceMap[Index].Capabilities;
+        } else {
+          DoUpdate = FALSE;
+        }
+      }
+
+      Length = MIN (PageLength, MemorySpaceLength);
+      if (DoUpdate) {
+        gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
+        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



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD
  2017-09-19  6:10 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Jian J Wang
  2017-09-19  6:10   ` [PATCH 1/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe Jian J Wang
@ 2017-09-19  6:10   ` Jian J Wang
  2017-09-20  9:30     ` Zeng, Star
  2017-09-20  5:11   ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Wang, Jian J
  2 siblings, 1 reply; 13+ messages in thread
From: Jian J Wang @ 2017-09-19  6:10 UTC (permalink / raw)
  To: edk2-devel; +Cc: Jiewen Yao, Eric Dong, Star Zeng, Laszlo Ersek, Michael Kinney

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't
accept page related attributes. That means users cannot use it to
change page attributes, and have to turn to CPU arch protocol to do
it, which is not be allowed by PI spec.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@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>
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 45 ++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index a06f8bb77c..e9d1d5b612 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -40,6 +40,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #define PRESENT_MEMORY_ATTRIBUTES     (EFI_RESOURCE_ATTRIBUTE_PRESENT)
 
+#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
+                                       EFI_MEMORY_WT | EFI_MEMORY_WB | \
+                                       EFI_MEMORY_WP | EFI_MEMORY_UCE)
+
+#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
+                                        EFI_MEMORY_RO)
+
 #define INVALID_CPU_ARCH_ATTRIBUTES   0xffffffff
 
 //
@@ -654,28 +661,30 @@ ConverToCpuArchAttributes (
   UINT64 Attributes
   )
 {
-  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
-    return EFI_MEMORY_UC;
-  }
+  UINT64      CpuArchAttributes;
 
-  if ( (Attributes & EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
-    return EFI_MEMORY_WC;
+  if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES |
+                      NONEXCLUSIVE_MEMORY_ATTRIBUTES)) != 0) {
+    return INVALID_CPU_ARCH_ATTRIBUTES;
   }
 
-  if ( (Attributes & EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
-    return EFI_MEMORY_WT;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WB) == EFI_MEMORY_WB) {
-    return EFI_MEMORY_WB;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WP) == EFI_MEMORY_WP) {
-    return EFI_MEMORY_WP;
-  }
-
-  return INVALID_CPU_ARCH_ATTRIBUTES;
+  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
 
+  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
+    CpuArchAttributes |= EFI_MEMORY_UC;
+  } else if ( (Attributes & EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
+    CpuArchAttributes |= EFI_MEMORY_WC;
+  } else if ( (Attributes & EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
+    CpuArchAttributes |= EFI_MEMORY_WT;
+  } else if ( (Attributes & EFI_MEMORY_WB) == EFI_MEMORY_WB) {
+    CpuArchAttributes |= EFI_MEMORY_WB;
+  } else if ( (Attributes & EFI_MEMORY_UCE) == EFI_MEMORY_UCE) {
+    CpuArchAttributes |= EFI_MEMORY_UCE;
+  } else if ( (Attributes & EFI_MEMORY_WP) == EFI_MEMORY_WP) {
+    CpuArchAttributes |= EFI_MEMORY_WP;
+  }
+
+  return CpuArchAttributes;
 }
 
 
-- 
2.14.1.windows.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-19  6:10 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Jian J Wang
  2017-09-19  6:10   ` [PATCH 1/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe Jian J Wang
  2017-09-19  6:10   ` [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD Jian J Wang
@ 2017-09-20  5:11   ` Wang, Jian J
  2017-09-20  6:07     ` Yao, Jiewen
  2 siblings, 1 reply; 13+ messages in thread
From: Wang, Jian J @ 2017-09-20  5:11 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Yao, Jiewen, Dong, Eric,
	Zeng, Star

Jiewen,

Any comments on this patch?


-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J Wang
Sent: Tuesday, September 19, 2017 2:10 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

There're two issues here actually.

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't
accept page related attributes. That means users cannot use it to
change page attributes, and have to turn to CPU arch protocol to do it,
which is not be allowed by PI spec.

>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: Eric Dong <eric.dong@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>

Jian J Wang (2):
  UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
  MdeModulePkg/Core: Fix out-of-sync issue in GCD

 MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++--------
 UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 ++
 UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 99 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 140 insertions(+), 18 deletions(-)

-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-20  5:11   ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Wang, Jian J
@ 2017-09-20  6:07     ` Yao, Jiewen
  2017-09-20  7:53       ` Wang, Jian J
  0 siblings, 1 reply; 13+ messages in thread
From: Yao, Jiewen @ 2017-09-20  6:07 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Dong, Eric, Zeng, Star

Hi
I take a look at the driver. I think the update is good.

Would you please share what test you have done with the updated UEFI memory map?
Such as UEFI Windows boot, UEFI Linux boot?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, September 20, 2017 1:12 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU
> driver
> 
> Jiewen,
> 
> Any comments on this patch?
> 
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Tuesday, September 19, 2017 2:10 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
> 
> There're two issues here actually.
> 
> From GCD perspective, its SetMemorySpaceAttributes() method doesn't
> accept page related attributes. That means users cannot use it to
> change page attributes, and have to turn to CPU arch protocol to do it,
> which is not be allowed by PI spec.
> 
> 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: Eric Dong <eric.dong@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>
> 
> Jian J Wang (2):
>   UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
>   MdeModulePkg/Core: Fix out-of-sync issue in GCD
> 
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++--------
>  UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 ++
>  UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 99
> ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 140 insertions(+), 18 deletions(-)
> 
> --
> 2.14.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-20  6:07     ` Yao, Jiewen
@ 2017-09-20  7:53       ` Wang, Jian J
  2017-09-20  8:03         ` Yao, Jiewen
  2017-09-21 15:05         ` Anthony PERARD
  0 siblings, 2 replies; 13+ messages in thread
From: Wang, Jian J @ 2017-09-20  7:53 UTC (permalink / raw)
  To: Yao, Jiewen, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Dong, Eric, Zeng, Star

Jiewen,

Yes. The patch has passed the validation of the UEFI booting Windows 10, Windows 7 and Fedora 25.

If no comments from you and others, I'll push the patch to upstream soon.

Thanks,
Jian Wang

-----Original Message-----
From: Yao, Jiewen 
Sent: Wednesday, September 20, 2017 2:08 PM
To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

Hi
I take a look at the driver. I think the update is good.

Would you please share what test you have done with the updated UEFI memory map?
Such as UEFI Windows boot, UEFI Linux boot?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, September 20, 2017 1:12 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU
> driver
> 
> Jiewen,
> 
> Any comments on this patch?
> 
> 
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Tuesday, September 19, 2017 2:10 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
> 
> There're two issues here actually.
> 
> From GCD perspective, its SetMemorySpaceAttributes() method doesn't
> accept page related attributes. That means users cannot use it to
> change page attributes, and have to turn to CPU arch protocol to do it,
> which is not be allowed by PI spec.
> 
> 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: Eric Dong <eric.dong@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>
> 
> Jian J Wang (2):
>   UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
>   MdeModulePkg/Core: Fix out-of-sync issue in GCD
> 
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++--------
>  UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 ++
>  UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 99
> ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 140 insertions(+), 18 deletions(-)
> 
> --
> 2.14.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-20  7:53       ` Wang, Jian J
@ 2017-09-20  8:03         ` Yao, Jiewen
  2017-09-21 15:05         ` Anthony PERARD
  1 sibling, 0 replies; 13+ messages in thread
From: Yao, Jiewen @ 2017-09-20  8:03 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Dong, Eric, Zeng, Star

Thanks. I have no other comments.

From: Wang, Jian J
Sent: Wednesday, September 20, 2017 3:54 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

Jiewen,

Yes. The patch has passed the validation of the UEFI booting Windows 10, Windows 7 and Fedora 25.

If no comments from you and others, I'll push the patch to upstream soon.

Thanks,
Jian Wang

-----Original Message-----
From: Yao, Jiewen
Sent: Wednesday, September 20, 2017 2:08 PM
To: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: RE: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

Hi
I take a look at the driver. I think the update is good.

Would you please share what test you have done with the updated UEFI memory map?
Such as UEFI Windows boot, UEFI Linux boot?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, September 20, 2017 1:12 PM
> To: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Laszlo Ersek
> <lersek@redhat.com<mailto:lersek@redhat.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Dong, Eric
> <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: RE: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU
> driver
>
> Jiewen,
>
> Any comments on this patch?
>
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Tuesday, September 19, 2017 2:10 PM
> To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Laszlo Ersek
> <lersek@redhat.com<mailto:lersek@redhat.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Dong, Eric
> <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
>
> There're two issues here actually.
>
> From GCD perspective, its SetMemorySpaceAttributes() method doesn't
> accept page related attributes. That means users cannot use it to
> change page attributes, and have to turn to CPU arch protocol to do it,
> which is not be allowed by PI spec.
>
> 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<mailto:jiewen.yao@intel.com>>
> Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Cc: Michael Kinney <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> Suggested-by: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
>
> Jian J Wang (2):
>   UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe
>   MdeModulePkg/Core: Fix out-of-sync issue in GCD
>
>  MdeModulePkg/Core/Dxe/Gcd/Gcd.c  | 45 ++++++++++--------
>  UefiCpuPkg/CpuDxe/CpuDxe.c       |  5 ++
>  UefiCpuPkg/CpuDxe/CpuDxe.h       |  9 ++++
>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 99
> ++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 140 insertions(+), 18 deletions(-)
>
> --
> 2.14.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD
  2017-09-19  6:10   ` [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD Jian J Wang
@ 2017-09-20  9:30     ` Zeng, Star
  2017-09-21  0:02       ` Wang, Jian J
  0 siblings, 1 reply; 13+ messages in thread
From: Zeng, Star @ 2017-09-20  9:30 UTC (permalink / raw)
  To: Wang, Jian J, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Yao, Jiewen, Dong, Eric,
	Zeng, Star

Reviewed-by: Star Zeng <star.zeng@intel.com> for this patch.

Please notice that the MemoryProtection.c is using gCpu->SetMemoryAttributes but not GCD SetMemorySpaceAttributes.
You should need update it to use GCD SetMemorySpaceAttributes, you can have separated patch to cover it.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J Wang
Sent: Tuesday, September 19, 2017 2:10 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't accept page related attributes. That means users cannot use it to change page attributes, and have to turn to CPU arch protocol to do it, which is not be allowed by PI spec.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@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>
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 45 ++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index a06f8bb77c..e9d1d5b612 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -40,6 +40,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #define PRESENT_MEMORY_ATTRIBUTES     (EFI_RESOURCE_ATTRIBUTE_PRESENT)
 
+#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
+                                       EFI_MEMORY_WT | EFI_MEMORY_WB | \
+                                       EFI_MEMORY_WP | EFI_MEMORY_UCE)
+
+#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
+                                        EFI_MEMORY_RO)
+
 #define INVALID_CPU_ARCH_ATTRIBUTES   0xffffffff
 
 //
@@ -654,28 +661,30 @@ ConverToCpuArchAttributes (
   UINT64 Attributes
   )
 {
-  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
-    return EFI_MEMORY_UC;
-  }
+  UINT64      CpuArchAttributes;
 
-  if ( (Attributes & EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
-    return EFI_MEMORY_WC;
+  if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES |
+                      NONEXCLUSIVE_MEMORY_ATTRIBUTES)) != 0) {
+    return INVALID_CPU_ARCH_ATTRIBUTES;
   }
 
-  if ( (Attributes & EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
-    return EFI_MEMORY_WT;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WB) == EFI_MEMORY_WB) {
-    return EFI_MEMORY_WB;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WP) == EFI_MEMORY_WP) {
-    return EFI_MEMORY_WP;
-  }
-
-  return INVALID_CPU_ARCH_ATTRIBUTES;
+  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
 
+  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
+    CpuArchAttributes |= EFI_MEMORY_UC;  } else if ( (Attributes & 
+ EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
+    CpuArchAttributes |= EFI_MEMORY_WC;  } else if ( (Attributes & 
+ EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
+    CpuArchAttributes |= EFI_MEMORY_WT;  } else if ( (Attributes & 
+ EFI_MEMORY_WB) == EFI_MEMORY_WB) {
+    CpuArchAttributes |= EFI_MEMORY_WB;  } else if ( (Attributes & 
+ EFI_MEMORY_UCE) == EFI_MEMORY_UCE) {
+    CpuArchAttributes |= EFI_MEMORY_UCE;  } else if ( (Attributes & 
+ EFI_MEMORY_WP) == EFI_MEMORY_WP) {
+    CpuArchAttributes |= EFI_MEMORY_WP;  }
+
+  return CpuArchAttributes;
 }
 
 
--
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD
  2017-09-20  9:30     ` Zeng, Star
@ 2017-09-21  0:02       ` Wang, Jian J
  0 siblings, 0 replies; 13+ messages in thread
From: Wang, Jian J @ 2017-09-21  0:02 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org
  Cc: Kinney, Michael D, Laszlo Ersek, Yao, Jiewen, Dong, Eric

Sure. I'll submit a new patch after enough validation. Thanks for the review.

-----Original Message-----
From: Zeng, Star 
Sent: Wednesday, September 20, 2017 5:30 PM
To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: RE: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD

Reviewed-by: Star Zeng <star.zeng@intel.com> for this patch.

Please notice that the MemoryProtection.c is using gCpu->SetMemoryAttributes but not GCD SetMemorySpaceAttributes.
You should need update it to use GCD SetMemorySpaceAttributes, you can have separated patch to cover it.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J Wang
Sent: Tuesday, September 19, 2017 2:10 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD

>From GCD perspective, its SetMemorySpaceAttributes() method doesn't accept page related attributes. That means users cannot use it to change page attributes, and have to turn to CPU arch protocol to do it, which is not be allowed by PI spec.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Eric Dong <eric.dong@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>
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 45 ++++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index a06f8bb77c..e9d1d5b612 100644
--- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
+++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
@@ -40,6 +40,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 #define PRESENT_MEMORY_ATTRIBUTES     (EFI_RESOURCE_ATTRIBUTE_PRESENT)
 
+#define EXCLUSIVE_MEMORY_ATTRIBUTES   (EFI_MEMORY_UC | EFI_MEMORY_WC | \
+                                       EFI_MEMORY_WT | EFI_MEMORY_WB | \
+                                       EFI_MEMORY_WP | EFI_MEMORY_UCE)
+
+#define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | EFI_MEMORY_RP | \
+                                        EFI_MEMORY_RO)
+
 #define INVALID_CPU_ARCH_ATTRIBUTES   0xffffffff
 
 //
@@ -654,28 +661,30 @@ ConverToCpuArchAttributes (
   UINT64 Attributes
   )
 {
-  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
-    return EFI_MEMORY_UC;
-  }
+  UINT64      CpuArchAttributes;
 
-  if ( (Attributes & EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
-    return EFI_MEMORY_WC;
+  if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES |
+                      NONEXCLUSIVE_MEMORY_ATTRIBUTES)) != 0) {
+    return INVALID_CPU_ARCH_ATTRIBUTES;
   }
 
-  if ( (Attributes & EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
-    return EFI_MEMORY_WT;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WB) == EFI_MEMORY_WB) {
-    return EFI_MEMORY_WB;
-  }
-
-  if ( (Attributes & EFI_MEMORY_WP) == EFI_MEMORY_WP) {
-    return EFI_MEMORY_WP;
-  }
-
-  return INVALID_CPU_ARCH_ATTRIBUTES;
+  CpuArchAttributes = Attributes & NONEXCLUSIVE_MEMORY_ATTRIBUTES;
 
+  if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) {
+    CpuArchAttributes |= EFI_MEMORY_UC;  } else if ( (Attributes & 
+ EFI_MEMORY_WC ) == EFI_MEMORY_WC) {
+    CpuArchAttributes |= EFI_MEMORY_WC;  } else if ( (Attributes & 
+ EFI_MEMORY_WT ) == EFI_MEMORY_WT) {
+    CpuArchAttributes |= EFI_MEMORY_WT;  } else if ( (Attributes & 
+ EFI_MEMORY_WB) == EFI_MEMORY_WB) {
+    CpuArchAttributes |= EFI_MEMORY_WB;  } else if ( (Attributes & 
+ EFI_MEMORY_UCE) == EFI_MEMORY_UCE) {
+    CpuArchAttributes |= EFI_MEMORY_UCE;  } else if ( (Attributes & 
+ EFI_MEMORY_WP) == EFI_MEMORY_WP) {
+    CpuArchAttributes |= EFI_MEMORY_WP;  }
+
+  return CpuArchAttributes;
 }
 
 
--
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-20  7:53       ` Wang, Jian J
  2017-09-20  8:03         ` Yao, Jiewen
@ 2017-09-21 15:05         ` Anthony PERARD
  2017-09-22  1:16           ` Wang, Jian J
  1 sibling, 1 reply; 13+ messages in thread
From: Anthony PERARD @ 2017-09-21 15:05 UTC (permalink / raw)
  To: Wang, Jian J
  Cc: Yao, Jiewen, edk2-devel@lists.01.org, Kinney, Michael D,
	Laszlo Ersek, Dong, Eric, Zeng, Star

On Wed, Sep 20, 2017 at 07:53:36AM +0000, Wang, Jian J wrote:
> Jiewen,
> 
> Yes. The patch has passed the validation of the UEFI booting Windows 10, Windows 7 and Fedora 25.
> 
> If no comments from you and others, I'll push the patch to upstream soon.
> 

Hi,

I think this patch series does not build, while building OVMF we have
those errors:

GNUmakefile:527: recipe for target '/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe/OUTPUT/Cp
uPageTable.obj' failed
make[8]: Leaving directory '/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe'
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c: In function 'RefreshGcdMemoryAttributesFromPaging':
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c:852:10: error: 'DoUpdate' may be used uninitialized in this functio
n [-Werror=maybe-uninitialized]
       if (DoUpdate) {
          ^
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c:853:9: error: 'Capabilities' may be used uninitialized in this func
tion [-Werror=maybe-uninitialized]
         gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
         ^
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c:855:90: error: 'Attributes' may be used uninitialized in this funct
ion [-Werror=maybe-uninitialized]
         DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx - %016lx (%08lx -> %08lx)\r\n",
                                                                                          ^
cc1: all warnings being treated as errors



Full log can be found at:
http://logs.test-lab.xenproject.org/osstest/logs/113661/build-amd64/6.ts-xen-build.log

Regards,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
  2017-09-21 15:05         ` Anthony PERARD
@ 2017-09-22  1:16           ` Wang, Jian J
  0 siblings, 0 replies; 13+ messages in thread
From: Wang, Jian J @ 2017-09-22  1:16 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: Yao, Jiewen, edk2-devel@lists.01.org, Kinney, Michael D,
	Laszlo Ersek, Dong, Eric, Zeng, Star

Thanks for catching it. Patch is on the way.

-----Original Message-----
From: Anthony PERARD [mailto:anthony.perard@citrix.com] 
Sent: Thursday, September 21, 2017 11:05 PM
To: Wang, Jian J <jian.j.wang@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek <lersek@redhat.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver

On Wed, Sep 20, 2017 at 07:53:36AM +0000, Wang, Jian J wrote:
> Jiewen,
> 
> Yes. The patch has passed the validation of the UEFI booting Windows 10, Windows 7 and Fedora 25.
> 
> If no comments from you and others, I'll push the patch to upstream soon.
> 

Hi,

I think this patch series does not build, while building OVMF we have
those errors:

GNUmakefile:527: recipe for target '/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe/OUTPUT/Cp
uPageTable.obj' failed
make[8]: Leaving directory '/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/CpuDxe/CpuDxe'
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c: In function 'RefreshGcdMemoryAttributesFromPaging':
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c:852:10: error: 'DoUpdate' may be used uninitialized in this functio
n [-Werror=maybe-uninitialized]
       if (DoUpdate) {
          ^
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c:853:9: error: 'Capabilities' may be used uninitialized in this func
tion [-Werror=maybe-uninitialized]
         gDS->SetMemorySpaceCapabilities (BaseAddress, Length, Capabilities);
         ^
/home/osstest/build.113661.build-amd64/xen/tools/firmware/ovmf-dir-remote/UefiCpuPkg/CpuDxe/CpuPageTable.c:855:90: error: 'Attributes' may be used uninitialized in this funct
ion [-Werror=maybe-uninitialized]
         DEBUG ((DEBUG_INFO, "Update memory space attribute: [%02d] %016lx - %016lx (%08lx -> %08lx)\r\n",
                                                                                          ^
cc1: all warnings being treated as errors



Full log can be found at:
http://logs.test-lab.xenproject.org/osstest/logs/113661/build-amd64/6.ts-xen-build.log

Regards,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2017-09-22  1:13 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <Fixe out-of-sync issue between GCD and CPU driver>
2017-09-19  6:10 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Jian J Wang
2017-09-19  6:10   ` [PATCH 1/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in CpuDxe Jian J Wang
2017-09-19  6:10   ` [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD Jian J Wang
2017-09-20  9:30     ` Zeng, Star
2017-09-21  0:02       ` Wang, Jian J
2017-09-20  5:11   ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Wang, Jian J
2017-09-20  6:07     ` Yao, Jiewen
2017-09-20  7:53       ` Wang, Jian J
2017-09-20  8:03         ` Yao, Jiewen
2017-09-21 15:05         ` Anthony PERARD
2017-09-22  1:16           ` Wang, Jian J
2017-09-18  3:08 Jian J Wang
2017-09-19  6:09 ` Wang, Jian J

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox