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.w...@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Laszlo Ersek 
<ler...@redhat.com>; Yao, Jiewen <jiewen....@intel.com>; Dong, Eric 
<eric.d...@intel.com>; Zeng, Star <star.z...@intel.com>
Subject: RE: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD

Reviewed-by: Star Zeng <star.z...@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-boun...@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.kin...@intel.com>; Laszlo Ersek 
<ler...@redhat.com>; Yao, Jiewen <jiewen....@intel.com>; Dong, Eric 
<eric.d...@intel.com>; Zeng, Star <star.z...@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....@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
Cc: Star Zeng <star.z...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Michael Kinney <michael.d.kin...@intel.com>
Suggested-by: Jiewen Yao <jiewen....@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.w...@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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to