* [PATCH 1/2] MdeModulePkg/Core: Fix out-of-sync issue in GCD
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 ` Jian J Wang
2017-09-18 3:08 ` [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page Jian J Wang
2017-09-19 6:09 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Wang, Jian J
2 siblings, 0 replies; 4+ 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
>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: 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] 4+ messages in thread* [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page
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
2017-09-19 6:09 ` [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver Wang, Jian J
2 siblings, 0 replies; 4+ 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
>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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 0/2] Fixe out-of-sync issue between GCD and CPU driver
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 ` [PATCH 2/2] UefiCpuPkg/CpuDxe: Fix out-of-sync issue in page Jian J Wang
@ 2017-09-19 6:09 ` Wang, Jian J
2 siblings, 0 replies; 4+ 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] 4+ messages in thread