[edk2] [PATCH] MdePkg UefiPciSegmentLibPciRootBridgeIo: Remove redundant dependency

2018-09-17 Thread shenglei
PiDxe.h is not used PciSegmentLib.h.
So "#include " is deleted.
https://bugzilla.tianocore.org/show_bug.cgi?id=1183

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdePkg/Library/UefiPciSegmentLibPciRootBridgeIo/PciSegmentLib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdePkg/Library/UefiPciSegmentLibPciRootBridgeIo/PciSegmentLib.h 
b/MdePkg/Library/UefiPciSegmentLibPciRootBridgeIo/PciSegmentLib.h
index 699a5b5341..639902f3d3 100644
--- a/MdePkg/Library/UefiPciSegmentLibPciRootBridgeIo/PciSegmentLib.h
+++ b/MdePkg/Library/UefiPciSegmentLibPciRootBridgeIo/PciSegmentLib.h
@@ -16,7 +16,6 @@
 #ifndef __DXE_PCI_SEGMENT_LIB__
 #define __DXE_PCI_SEGMENT_LIB__
 
-#include 
 
 #include 
 
-- 
2.18.0.windows.1

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


[edk2] [PATCH] MdePkg UefiPciLibPciRootBridgeIo: Remove redundant dependency

2018-09-17 Thread shenglei
PiDxe.h is not used PciSegmentLib.h.
So "#include " is deleted.
https://bugzilla.tianocore.org/show_bug.cgi?id=1183

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdePkg/Library/UefiPciLibPciRootBridgeIo/PciLib.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MdePkg/Library/UefiPciLibPciRootBridgeIo/PciLib.c 
b/MdePkg/Library/UefiPciLibPciRootBridgeIo/PciLib.c
index 575d9c1f91..f89e3e2551 100644
--- a/MdePkg/Library/UefiPciLibPciRootBridgeIo/PciLib.c
+++ b/MdePkg/Library/UefiPciLibPciRootBridgeIo/PciLib.c
@@ -13,8 +13,6 @@
 
 **/
 
-#include 
-
 #include 
 
 #include 
-- 
2.18.0.windows.1

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


Re: [edk2] [Patch 00/14] Update MSR definitions.

2018-09-17 Thread Ni, Ruiyu

On 9/18/2018 9:43 AM, Eric Dong wrote:

Current MSR definition are follow the SDM 2016-09 version. The latest
SDM is 2018-05. This patch serial update the MSR related definition to
follow the latest SDM 2018-05 version. MSR related defintion are saved
at UefiCpuPkg\Include\Register\.

The changes for this serial includes:
1. Add new MSR definition and file.
2. Remove old MSR definition which not defined in new SDM.
3. Change MSR name to follow new SDM, keep old one for compatibility.
4. Change MSR data structure definition to follow new SDM.
5. Update comments to follow the new SDM, mainly related to chapter info.

Below changes are incompatible changes:
2. Remove old MSR definition which not defined in new SDM.
For this one, i search edk2 codebase, not found any code uses it. so no
impact for edk2 codebase. Detail changes see patch 9 ~ 11.

4. Change MSR data structure definition to follow new SDM.
For this one, new data structure just change the original reserved bits to
valid bits, should have no impact for the current code. Detail see patch 8
and patch 14

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 


Eric Dong (14):
   UefiCpuPkg/Include/Register/Msr: Update reference spec info.
   UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h: Add new MSR file
 for goldmont plus microarchitecture.
   UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h: Add new MSR.
   UefiCpuPkg/Include/Register/Msr/*.h: Add new MSR.
   UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h: Add new MSR.
   UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h: Add new MSRs.
   UefiCpuPkg/Include/Register/ArchitecturalMsr.h: Add new MSR.
   UefiCpuPkg/Include/Register/ArchitecturalMsr.h: Change structure
 definition.
   UefiCpuPkg/Include/Register/Msr/Core2Msr.h: Remove old MSR.
   UefiCpuPkg/Include/Register/Msr/P6Msr.h: Remove old MSR.
   UefiCpuPkg/Include/Register/Msr/CoreMsr.h: Remove old MSR.
   UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h: Add new MSR name and
 keep old one.
   UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h: Add new MSR name and
 keep old one.
   UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h: Change structure
 definition.

  UefiCpuPkg/Include/Register/ArchitecturalMsr.h|  130 +-
  UefiCpuPkg/Include/Register/Msr.h |7 +-
  UefiCpuPkg/Include/Register/Msr/AtomMsr.h |   28 +-
  UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h|   62 +-
  UefiCpuPkg/Include/Register/Msr/Core2Msr.h|  102 +-
  UefiCpuPkg/Include/Register/Msr/CoreMsr.h |   74 +-
  UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h |   88 +-
  UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h |  272 
  UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h |   62 +-
  UefiCpuPkg/Include/Register/Msr/HaswellMsr.h  |   34 +-
  UefiCpuPkg/Include/Register/Msr/IvyBridgeMsr.h|8 +-
  UefiCpuPkg/Include/Register/Msr/NehalemMsr.h  |   52 +-
  UefiCpuPkg/Include/Register/Msr/P6Msr.h   |   60 +-
  UefiCpuPkg/Include/Register/Msr/Pentium4Msr.h |  202 +--
  UefiCpuPkg/Include/Register/Msr/PentiumMMsr.h |   22 +-
  UefiCpuPkg/Include/Register/Msr/PentiumMsr.h  |   12 +-
  UefiCpuPkg/Include/Register/Msr/SandyBridgeMsr.h  |   49 +-
  UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h   |  100 +-
  UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h  | 1602 -
  UefiCpuPkg/Include/Register/Msr/Xeon5600Msr.h |8 +-
  UefiCpuPkg/Include/Register/Msr/XeonDMsr.h|   84 +-
  UefiCpuPkg/Include/Register/Msr/XeonE7Msr.h   |6 +-
  UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h  |  326 -
  23 files changed, 2813 insertions(+), 577 deletions(-)
  create mode 100644 UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h


Acked-by: Ruiyu Ni 

--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch 01/14] UefiCpuPkg/Include/Register/Msr: Update reference spec info.

2018-09-17 Thread Eric Dong
Latest SDM has moved MSR related content from volume 3 chapter 35 to volume 4
chapter 2. Current MSR's comments need to be updated to reference the new
chapter info.

Changes includes:
  1. Update referenced chapter info from some MSRs.
  2. Update referenced SDM version info.

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 

---
 UefiCpuPkg/Include/Register/ArchitecturalMsr.h   |  44 ++---
 UefiCpuPkg/Include/Register/Msr/AtomMsr.h|  28 ++--
 UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h   |   8 +-
 UefiCpuPkg/Include/Register/Msr/Core2Msr.h   |  42 ++---
 UefiCpuPkg/Include/Register/Msr/CoreMsr.h|  26 +--
 UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h|  54 +++---
 UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h|   6 +-
 UefiCpuPkg/Include/Register/Msr/HaswellMsr.h |  34 ++--
 UefiCpuPkg/Include/Register/Msr/IvyBridgeMsr.h   |   8 +-
 UefiCpuPkg/Include/Register/Msr/NehalemMsr.h |  52 +++---
 UefiCpuPkg/Include/Register/Msr/P6Msr.h  |  12 +-
 UefiCpuPkg/Include/Register/Msr/Pentium4Msr.h| 202 +++
 UefiCpuPkg/Include/Register/Msr/PentiumMMsr.h|  22 +--
 UefiCpuPkg/Include/Register/Msr/PentiumMsr.h |  12 +-
 UefiCpuPkg/Include/Register/Msr/SandyBridgeMsr.h |  49 +++---
 UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h  |  52 +++---
 UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h |  14 +-
 UefiCpuPkg/Include/Register/Msr/Xeon5600Msr.h|   8 +-
 UefiCpuPkg/Include/Register/Msr/XeonDMsr.h   |  28 ++--
 UefiCpuPkg/Include/Register/Msr/XeonE7Msr.h  |   6 +-
 UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h |  24 +--
 21 files changed, 359 insertions(+), 372 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h 
b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
index 34fdf5be3a..5d2242aa80 100644
--- a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
+++ b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
@@ -6,7 +6,7 @@
   returned is a single 32-bit or 64-bit value, then a data structure is not
   provided for that MSR.
 
-  Copyright (c) 2016 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -16,16 +16,8 @@
   WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
   @par Specification Reference:
-  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 3,
-  September 2016, Chapter 35 Model-Specific-Registers (MSR), Section 35.1.
-
-  @par Specification Reference:
-  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 3,
-  September 2016, Appendix A VMX Capability Reporting Facility, Section A.1.
-
-  @par Specification Reference:
-  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 3,
-  September 2016, Appendix A VMX Capability Reporting Facility, Section A.6.
+  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 4,
+  May 2018, Volume 4: Model-Specific-Registers (MSR)
 
 **/
 
@@ -33,7 +25,7 @@
 #define __ARCHITECTURAL_MSR_H__
 
 /**
-  See Section 35.22, "MSRs in Pentium Processors.". Pentium Processor (05_01H).
+  See Section 2.22, "MSRs in Pentium Processors.". Pentium Processor (05_01H).
 
   @param  ECX  MSR_IA32_P5_MC_ADDR (0x)
   @param  EAX  Lower 32-bits of MSR value.
@@ -52,7 +44,7 @@
 
 
 /**
-  See Section 35.22, "MSRs in Pentium Processors.". DF_DM = 05_01H.
+  See Section 2.22, "MSRs in Pentium Processors.". DF_DM = 05_01H.
 
   @param  ECX  MSR_IA32_P5_MC_TYPE (0x0001)
   @param  EAX  Lower 32-bits of MSR value.
@@ -91,7 +83,7 @@
 
 
 /**
-  See Section 17.15, "Time-Stamp Counter.". Introduced at Display Family /
+  See Section 17.17, "Time-Stamp Counter.". Introduced at Display Family /
   Display Model 05_01H.
 
   @param  ECX  MSR_IA32_TIME_STAMP_COUNTER (0x0010)
@@ -493,9 +485,8 @@ typedef union {
 UINT32  Valid:1;
 UINT32  Reserved1:1;
 ///
-/// [Bit 2] Determines whether executions of VMXOFF unblock SMIs under the
-/// default treatment of SMIs and SMM.  Executions of VMXOFF unblock SMIs
-/// unless bit 2 is 1 (the value of bit 0 is irrelevant).
+/// [Bit 2] Controls SMI unblocking by VMXOFF (see Section 34.14.4). If
+/// IA32_VMX_MISC[28].
 ///
 UINT32  BlockSmi:1;
 UINT32  Reserved2:9;
@@ -1953,7 +1944,7 @@ typedef union {
 
 
 /**
-  SMRR Range Mask. (Writeable only in SMM)  Range Mask of SMM memory range. If
+  SMRR Range Mask (Writeable only in SMM) Range Mask of SMM memory range. If
   IA32_MTRRCAP[SMRR] = 1.
 
   @param  ECX  MSR_IA32_SMRR_PHYSMASK (0x01F3)
@@ -4417,13 +4408,13 @@ typedef union {
   ///
   struct {
 ///
-/// [Bit 0] 

[edk2] [Patch 02/14] UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h: Add new MSR file for goldmont plus microarchitecture.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Add new MSR file which used for goldmont plus microarchitecture.

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr.h |   7 +-
 UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h | 272 ++
 2 files changed, 276 insertions(+), 3 deletions(-)
 create mode 100644 UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h

diff --git a/UefiCpuPkg/Include/Register/Msr.h 
b/UefiCpuPkg/Include/Register/Msr.h
index 0ac8d5bdfd..abe0e136de 100644
--- a/UefiCpuPkg/Include/Register/Msr.h
+++ b/UefiCpuPkg/Include/Register/Msr.h
@@ -6,7 +6,7 @@
   returned is a single 32-bit or 64-bit value, then a data structure is not
   provided for that MSR.
 
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 ~ 2018, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -16,8 +16,8 @@
   WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
   @par Specification Reference:
-  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 3,
-  September 2016, Chapter 35 Model-Specific-Registers (MSR), Chapter 35.
+  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 4,
+  May 2018, Volume 4: Model-Specific-Registers (MSR)
 
 **/
 
@@ -29,6 +29,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h 
b/UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h
new file mode 100644
index 00..d050464b7f
--- /dev/null
+++ b/UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h
@@ -0,0 +1,272 @@
+/** @file
+  MSR Defintions for Intel Atom processors based on the Goldmont Plus 
microarchitecture.
+
+  Provides defines for Machine Specific Registers(MSR) indexes. Data structures
+  are provided for MSRs that contain one or more bit fields.  If the MSR value
+  returned is a single 32-bit or 64-bit value, then a data structure is not
+  provided for that MSR.
+
+  Copyright (c) 2018, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  @par Specification Reference:
+  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 4,
+  May 2018, Volume 4: Model-Specific-Registers (MSR)
+
+**/
+
+#ifndef __GOLDMONT_PLUS_MSR_H__
+#define __GOLDMONT_PLUS_MSR_H__
+
+#include 
+
+/**
+  Is Intel Atom processors based on the Goldmont plus microarchitecture?
+
+  @param   DisplayFamily  Display Family ID
+  @param   DisplayModel   Display Model ID
+
+  @retval  TRUE   Yes, it is.
+  @retval  FALSE  No, it isn't.
+**/
+#define IS_GOLDMONT_PLUS_PROCESSOR(DisplayFamily, DisplayModel) \
+  (DisplayFamily == 0x06 && \
+   (\
+DisplayModel == 0x7A\
+)   \
+   )
+
+/**
+  Core. (R/W) See Table 2-2. See Section 18.6.2.4, "Processor Event Based
+  Sampling (PEBS).".
+
+  @param  ECX  MSR_GOLDMONT_PLUS_PEBS_ENABLE (0x03F1)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type MSR_GOLDMONT_PLUS_PEBS_ENABLE_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type MSR_GOLDMONT_PLUS_PEBS_ENABLE_REGISTER.
+
+  Example usage
+  @code
+  MSR_GOLDMONT_PLUS_PEBS_ENABLE_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_GOLDMONT_PLUS_PEBS_ENABLE);
+  AsmWriteMsr64 (MSR_GOLDMONT_PLUS_PEBS_ENABLE, Msr.Uint64);
+  @endcode
+**/
+#define MSR_GOLDMONT_PLUS_PEBS_ENABLE0x03F1
+
+/**
+  MSR information returned for MSR index #MSR_GOLDMONT_PLUS_PEBS_ENABLE
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+///
+/// [Bit 0] Enable PEBS trigger and recording for the programmed event
+/// (precise or otherwise) on IA32_PMC0.
+///
+UINT32  Fix_Me_1:1;
+///
+/// [Bit 1] Enable PEBS trigger and recording for the programmed event
+/// (precise or otherwise) on IA32_PMC1.
+///
+UINT32  Fix_Me_2:1;
+///
+/// [Bit 2] Enable PEBS trigger and recording for the programmed event
+/// (precise or otherwise) on IA32_PMC2.
+///
+UINT32  Fix_Me_3:1;
+///
+/// [Bit 3] Enable PEBS trigger and recording for the programmed event
+/// (precise or otherwise) on IA32_PMC3.
+///
+UINT32  Fix_Me_4:

[edk2] [Patch 09/14] UefiCpuPkg/Include/Register/Msr/Core2Msr.h: Remove old MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Remove old MSR which not existed in 2018-05 version spec:
 1. MSR_CORE2_BBL_CR_CTL3

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/Core2Msr.h | 60 --
 1 file changed, 60 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/Core2Msr.h 
b/UefiCpuPkg/Include/Register/Msr/Core2Msr.h
index 22317fa1de..f01f7c5c97 100644
--- a/UefiCpuPkg/Include/Register/Msr/Core2Msr.h
+++ b/UefiCpuPkg/Include/Register/Msr/Core2Msr.h
@@ -471,66 +471,6 @@ typedef union {
   UINT64  Uint64;
 } MSR_CORE2_FSB_FREQ_REGISTER;
 
-
-/**
-  Shared.
-
-  @param  ECX  MSR_CORE2_BBL_CR_CTL3 (0x011E)
-  @param  EAX  Lower 32-bits of MSR value.
-   Described by the type MSR_CORE2_BBL_CR_CTL3_REGISTER.
-  @param  EDX  Upper 32-bits of MSR value.
-   Described by the type MSR_CORE2_BBL_CR_CTL3_REGISTER.
-
-  Example usage
-  @code
-  MSR_CORE2_BBL_CR_CTL3_REGISTER  Msr;
-
-  Msr.Uint64 = AsmReadMsr64 (MSR_CORE2_BBL_CR_CTL3);
-  AsmWriteMsr64 (MSR_CORE2_BBL_CR_CTL3, Msr.Uint64);
-  @endcode
-  @note MSR_CORE2_BBL_CR_CTL3 is defined as MSR_BBL_CR_CTL3 in SDM.
-**/
-#define MSR_CORE2_BBL_CR_CTL30x011E
-
-/**
-  MSR information returned for MSR index #MSR_CORE2_BBL_CR_CTL3
-**/
-typedef union {
-  ///
-  /// Individual bit fields
-  ///
-  struct {
-///
-/// [Bit 0] L2 Hardware Enabled (RO) 1 = If the L2 is hardware-enabled 0 =
-/// Indicates if the L2 is hardware-disabled.
-///
-UINT32  L2HardwareEnabled:1;
-UINT32  Reserved1:7;
-///
-/// [Bit 8] L2 Enabled (R/W)  1 = L2 cache has been initialized 0 =
-/// Disabled (default) Until this bit is set the processor will not
-/// respond to the WBINVD instruction or the assertion of the FLUSH# input.
-///
-UINT32  L2Enabled:1;
-UINT32  Reserved2:14;
-///
-/// [Bit 23] L2 Not Present (RO)  1. = L2 Present 2. = L2 Not Present.
-///
-UINT32  L2NotPresent:1;
-UINT32  Reserved3:8;
-UINT32  Reserved4:32;
-  } Bits;
-  ///
-  /// All bit fields as a 32-bit value
-  ///
-  UINT32  Uint32;
-  ///
-  /// All bit fields as a 64-bit value
-  ///
-  UINT64  Uint64;
-} MSR_CORE2_BBL_CR_CTL3_REGISTER;
-
-
 /**
   Shared.
 
-- 
2.15.0.windows.1

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


[edk2] [Patch 12/14] UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h: Add new MSR name and keep old one.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Change MSR name:
1. MSR_SKYLAKE_SGXOWNER0 => MSR_SKYLAKE_SGXOWNEREPOCH0
2. MSR_SKYLAKE_SGXOWNER1 => MSR_SKYLAKE_SGXOWNEREPOCH1
  2. Keep old MSR definition(MSR_SKYLAKE_SGXOWNER0/1) for compatibility
1. Use below coding style to define old MSR
 #define MSR_SKYLAKE_SGXOWNER0  MSR_SKYLAKE_SGXOWNEREPOCH0

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h | 40 +++-
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h 
b/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h
index 90cde86ccb..88f2c28eae 100644
--- a/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h
@@ -197,10 +197,12 @@ typedef union {
 
 
 /**
-  Package. Lower 64 Bit OwnerEpoch Component of SGX Key (RO). Low 64 bits of
-  an 128-bit external entropy value for key derivation of an enclave.
+  Package. Lower 64 Bit CR_SGXOWNEREPOCH (W) Writes do not update
+  CR_SGXOWNEREPOCH if CPUID.(EAX=12H, ECX=0):EAX.SGX1 is 1 on any thread in
+  the package. Lower 64 bits of an 128-bit external entropy value for key
+  derivation of an enclave.
 
-  @param  ECX  MSR_SKYLAKE_SGXOWNER0 (0x0300)
+  @param  ECX  MSR_SKYLAKE_SGXOWNEREPOCH0 (0x0300)
   @param  EAX  Lower 32-bits of MSR value.
   @param  EDX  Upper 32-bits of MSR value.
 
@@ -208,18 +210,24 @@ typedef union {
   @code
   UINT64  Msr;
 
-  Msr = AsmReadMsr64 (MSR_SKYLAKE_SGXOWNER0);
+  Msr = 0;
+  AsmWriteMsr64 (MSR_SKYLAKE_SGXOWNEREPOCH0, Msr);
   @endcode
-  @note MSR_SKYLAKE_SGXOWNER0 is defined as MSR_SGXOWNER0 in SDM.
+  @note MSR_SKYLAKE_SGXOWNEREPOCH0 is defined as MSR_SGXOWNER0 in SDM.
 **/
-#define MSR_SKYLAKE_SGXOWNER00x0300
-
+#define MSR_SKYLAKE_SGXOWNEREPOCH00x0300
 
+//
+// Define MSR_SKYLAKE_SGXOWNER0 for compatibility due to name change in the 
SDM.
+//
+#define MSR_SKYLAKE_SGXOWNER0 
MSR_SKYLAKE_SGXOWNEREPOCH0
 /**
-  Package. Upper 64 Bit OwnerEpoch Component of SGX Key (RO). Upper 64 bits of
-  an 128-bit external entropy value for key derivation of an enclave.
+  Package. Upper 64 Bit CR_SGXOWNEREPOCH (W) Writes do not update
+  CR_SGXOWNEREPOCH if CPUID.(EAX=12H, ECX=0):EAX.SGX1 is 1 on any thread in
+  the package. Upper 64 bits of an 128-bit external entropy value for key
+  derivation of an enclave.
 
-  @param  ECX  MSR_SKYLAKE_SGXOWNER1 (0x0301)
+  @param  ECX  MSR_SKYLAKE_SGXOWNEREPOCH1 (0x0301)
   @param  EAX  Lower 32-bits of MSR value.
   @param  EDX  Upper 32-bits of MSR value.
 
@@ -227,11 +235,17 @@ typedef union {
   @code
   UINT64  Msr;
 
-  Msr = AsmReadMsr64 (MSR_SKYLAKE_SGXOWNER1);
+  Msr = 0;
+  AsmWriteMsr64 (MSR_SKYLAKE_SGXOWNEREPOCH1, Msr);
   @endcode
-  @note MSR_SKYLAKE_SGXOWNER1 is defined as MSR_SGXOWNER1 in SDM.
+  @note MSR_SKYLAKE_SGXOWNEREPOCH1 is defined as MSR_SGXOWNER1 in SDM.
 **/
-#define MSR_SKYLAKE_SGXOWNER10x0301
+#define MSR_SKYLAKE_SGXOWNEREPOCH10x0301
+
+//
+// Define MSR_SKYLAKE_SGXOWNER1 for compatibility due to name change in the 
SDM.
+//
+#define MSR_SKYLAKE_SGXOWNER1 MSR_SKYLAKE_SGXOWNEREPOCH1
 
 
 /**
-- 
2.15.0.windows.1

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


[edk2] [Patch 14/14] UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h: Change structure definition.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Update MSR structure definition, change some reserved fields to useful 
fields:
 1. MSR_XEON_PHI_PKG_CST_CONFIG_CONTROL_REGISTER
 2. MSR_XEON_PHI_SMM_MCA_CAP_REGISTER
  2. For MSR_XEON_PHI_PMG_IO_CAPTURE_BASE_REGISTER structure, it expand the 
field range.
 Old definition like below:
   typedef union {
 ///
 /// Individual bit fields
 ///
 struct {
   ///
   /// [Bits 15:0] LVL_2 Base Address (R/W).
   ///
   UINT32  Lvl2Base:16;
   ///
   /// [Bits 18:16] C-state Range (R/W)  Specifies the encoding value 
of the
   /// maximum C-State code name to be included when IO read to MWAIT
   /// redirection is enabled by MSR_PKG_CST_CONFIG_CONTROL[bit10]: 
100b - C4
   /// is the max C-State to include 110b - C6 is the max C-State to 
include.
   ///
   UINT32  CStateRange:3;
   UINT32  Reserved1:13;
   UINT32  Reserved2:32;
 } Bits;
 ///
 /// All bit fields as a 32-bit value
 ///
 UINT32  Uint32;
 ///
 /// All bit fields as a 64-bit value
 ///
 UINT64  Uint64;
   } MSR_XEON_PHI_PMG_IO_CAPTURE_BASE_REGISTER;
This patch make below changes for this data structure, it expand 
"CStateRange" field width.
  old one:
UINT32  CStateRange:3;
UINT32  Reserved1:13;
  new one:
UINT32  CStateRange:7;
UINT32  Reserved1:9;

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h | 53 ++--
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h 
b/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h
index da74c2402c..77bd310d83 100644
--- a/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h
@@ -278,8 +278,26 @@ typedef union {
 /// [Bit 15] CFG Lock (R/WO).
 ///
 UINT32  CFGLock:1;
-UINT32  Reserved3:16;
-UINT32  Reserved4:32;
+UINT32  Reserved3:10;
+///
+/// [Bit 26] C1 State Auto Demotion Enable (R/W) When set, the processor
+/// will conditionally demote C3/C6/C7 requests to C1 based on uncore
+/// auto-demote information.
+///
+UINT32  C1StateAutoDemotionEnable:1;
+UINT32  Reserved4:1;
+///
+/// [Bit 28] C1 State Auto Undemotion Enable (R/W) When set, enables
+/// Undemotion from Demoted C1.
+///
+UINT32  C1StateAutoUndemotionEnable:1;
+///
+/// [Bit 29] PKG C-State Auto Demotion Enable (R/W) When set, enables
+/// Package C state demotion.
+///
+UINT32  PKGC_StateAutoDemotionEnable:1;
+UINT32  Reserved5:2;
+UINT32  Reserved6:32;
   } Bits;
   ///
   /// All bit fields as a 32-bit value
@@ -325,13 +343,12 @@ typedef union {
 ///
 UINT32  Lvl2Base:16;
 ///
-/// [Bits 18:16] C-state Range (R/W)  Specifies the encoding value of the
-/// maximum C-State code name to be included when IO read to MWAIT
-/// redirection is enabled by MSR_PKG_CST_CONFIG_CONTROL[bit10]: 100b - C4
-/// is the max C-State to include 110b - C6 is the max C-State to include.
+/// [Bits 22:16] C-State Range (R/W) The IO-port block size in which
+/// IO-redirection will be executed (0-127). Should be programmed based on
+/// the number of LVLx registers existing in the chipset.
 ///
-UINT32  CStateRange:3;
-UINT32  Reserved1:13;
+UINT32  CStateRange:7;
+UINT32  Reserved1:9;
 UINT32  Reserved2:32;
   } Bits;
   ///
@@ -477,8 +494,22 @@ typedef union {
   /// Individual bit fields
   ///
   struct {
-UINT32  Reserved1:32;
-UINT32  Reserved2:26;
+///
+/// [Bits 31:0] Bank Support (SMM-RO) One bit per MCA bank. If the bit is
+/// set, that bank supports Enhanced MCA (Default all 0; does not support
+/// EMCA).
+///
+UINT32  BankSupport:32;
+UINT32  Reserved1:24;
+///
+/// [Bit 56] Targeted SMI (SMM-RO) Set if targeted SMI is supported.
+///
+UINT32  TargetedSMI:1;
+///
+/// [Bit 57] SMM_CPU_SVRSTR (SMM-RO) Set if SMM SRAM save/restore feature
+/// is supported.
+///
+UINT32  SMM_CPU_SVRSTR:1;
 ///
 /// [Bit 58] SMM_Code_Access_Chk (SMM-RO) If set to 1 indicates that the
 /// SMM code access restriction is supported and a host-space interface
@@ -491,7 +522,7 @@ typedef union {
 /// available to SMM handler.
 ///
 UINT32  Long_Flow_Indication:1;
-UINT32  Reserved3:4;
+UINT32  Reserved2:4;
   } Bits;
   ///
   /// All bit fields as a 64-bit value
-- 
2.15.0.windows.1

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


[edk2] [Patch 13/14] UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h: Add new MSR name and keep old one.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Change MSR name:
1. MSR_GOLDMONT_SGXOWNER0 => MSR_GOLDMONT_SGXOWNEREPOCH0
2. MSR_GOLDMONT_SGXOWNER1 => MSR_GOLDMONT_SGXOWNEREPOCH1
  2. Keep old MSR definition (MSR_GOLDMONT_SGXOWNER0/1) for compatibility.
1. Define old MSR like below style:
   #define MSR_GOLDMONT_SGXOWNER0   MSR_GOLDMONT_SGXOWNEREPOCH0

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h | 34 +++
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h 
b/UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h
index a9061133c9..383f31ee55 100644
--- a/UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h
@@ -843,10 +843,12 @@ typedef union {
 
 
 /**
-  Package. Lower 64 Bit OwnerEpoch Component of SGX Key (RO). Low 64 bits of
-  an 128-bit external entropy value for key derivation of an enclave.
+  Package. Lower 64 Bit CR_SGXOWNEREPOCH (W) Writes do not update
+  CR_SGXOWNEREPOCH if CPUID.(EAX=12H, ECX=0):EAX.SGX1 is 1 on any thread in
+  the package. Lower 64 bits of an 128-bit external entropy value for key
+  derivation of an enclave.
 
-  @param  ECX  MSR_GOLDMONT_SGXOWNER0 (0x0300)
+  @param  ECX  MSR_GOLDMONT_SGXOWNEREPOCH0 (0x0300)
   @param  EAX  Lower 32-bits of MSR value.
   @param  EDX  Upper 32-bits of MSR value.
 
@@ -854,18 +856,24 @@ typedef union {
   @code
   UINT64  Msr;
 
-  Msr = AsmReadMsr64 (MSR_GOLDMONT_SGXOWNER0);
+  Msr = AsmReadMsr64 (MSR_GOLDMONT_SGXOWNEREPOCH0);
   @endcode
-  @note MSR_GOLDMONT_SGXOWNER0 is defined as MSR_SGXOWNER0 in SDM.
+  @note MSR_GOLDMONT_SGXOWNEREPOCH0 is defined as MSR_SGXOWNEREPOCH0 in SDM.
 **/
-#define MSR_GOLDMONT_SGXOWNER0   0x0300
+#define MSR_GOLDMONT_SGXOWNEREPOCH0   0x0300
+
+
+//
+// Define MSR_GOLDMONT_SGXOWNER0 for compatibility due to name change in the 
SDM.
+//
+#define MSR_GOLDMONT_SGXOWNER0
MSR_GOLDMONT_SGXOWNEREPOCH0
 
 
 /**
   Package. Upper 64 Bit OwnerEpoch Component of SGX Key (RO). Upper 64 bits of
   an 128-bit external entropy value for key derivation of an enclave.
 
-  @param  ECX  MSR_GOLDMONT_SGXOWNER1 (0x0301)
+  @param  ECX  MSR_GOLDMONT_SGXOWNEREPOCH1 (0x0301)
   @param  EAX  Lower 32-bits of MSR value.
   @param  EDX  Upper 32-bits of MSR value.
 
@@ -873,11 +881,17 @@ typedef union {
   @code
   UINT64  Msr;
 
-  Msr = AsmReadMsr64 (MSR_GOLDMONT_SGXOWNER1);
+  Msr = AsmReadMsr64 (MSR_GOLDMONT_SGXOWNEREPOCH1);
   @endcode
-  @note MSR_GOLDMONT_SGXOWNER1 is defined as MSR_SGXOWNER1 in SDM.
+  @note MSR_GOLDMONT_SGXOWNEREPOCH1 is defined as MSR_SGXOWNEREPOCH1 in SDM.
 **/
-#define MSR_GOLDMONT_SGXOWNER1   0x0301
+#define MSR_GOLDMONT_SGXOWNEREPOCH1   0x0301
+
+
+//
+// Define MSR_GOLDMONT_SGXOWNER1 for compatibility due to name change in the 
SDM.
+//
+#define MSR_GOLDMONT_SGXOWNER1
MSR_GOLDMONT_SGXOWNEREPOCH1
 
 
 /**
-- 
2.15.0.windows.1

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


[edk2] [Patch 11/14] UefiCpuPkg/Include/Register/Msr/CoreMsr.h: Remove old MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Remove old MSR which not existed in 2018-05 version spec:
 1. MSR_CORE_ROB_CR_BKUPTMPDR6

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/CoreMsr.h | 48 ---
 1 file changed, 48 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/CoreMsr.h 
b/UefiCpuPkg/Include/Register/Msr/CoreMsr.h
index bb2bdd2ca1..a4315d6e56 100644
--- a/UefiCpuPkg/Include/Register/Msr/CoreMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/CoreMsr.h
@@ -555,54 +555,6 @@ typedef union {
 **/
 #define MSR_CORE_LER_TO_LIP  0x01DE
 
-
-/**
-  Unique.
-
-  @param  ECX  MSR_CORE_ROB_CR_BKUPTMPDR6 (0x01E0)
-  @param  EAX  Lower 32-bits of MSR value.
-   Described by the type MSR_CORE_ROB_CR_BKUPTMPDR6_REGISTER.
-  @param  EDX  Upper 32-bits of MSR value.
-   Described by the type MSR_CORE_ROB_CR_BKUPTMPDR6_REGISTER.
-
-  Example usage
-  @code
-  MSR_CORE_ROB_CR_BKUPTMPDR6_REGISTER  Msr;
-
-  Msr.Uint64 = AsmReadMsr64 (MSR_CORE_ROB_CR_BKUPTMPDR6);
-  AsmWriteMsr64 (MSR_CORE_ROB_CR_BKUPTMPDR6, Msr.Uint64);
-  @endcode
-  @note MSR_CORE_ROB_CR_BKUPTMPDR6 is defined as ROB_CR_BKUPTMPDR6 in SDM.
-**/
-#define MSR_CORE_ROB_CR_BKUPTMPDR6   0x01E0
-
-/**
-  MSR information returned for MSR index #MSR_CORE_ROB_CR_BKUPTMPDR6
-**/
-typedef union {
-  ///
-  /// Individual bit fields
-  ///
-  struct {
-UINT32  Reserved1:2;
-///
-/// [Bit 2] Fast Strings Enable bit. (Default, enabled).
-///
-UINT32  FastStrings:1;
-UINT32  Reserved2:29;
-UINT32  Reserved3:32;
-  } Bits;
-  ///
-  /// All bit fields as a 32-bit value
-  ///
-  UINT32  Uint32;
-  ///
-  /// All bit fields as a 64-bit value
-  ///
-  UINT64  Uint64;
-} MSR_CORE_ROB_CR_BKUPTMPDR6_REGISTER;
-
-
 /**
   Unique.
 
-- 
2.15.0.windows.1

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


[edk2] [Patch 08/14] UefiCpuPkg/Include/Register/ArchitecturalMsr.h: Change structure definition.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Change fields which is reserved in old version: MSR_IA32_RTIT_CTL_REGISTER

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h 
b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
index b467ffaf26..312cbaf3da 100644
--- a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
+++ b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
@@ -4647,7 +4647,14 @@ typedef union {
 /// [Bit 3] User.
 ///
 UINT32  User:1;
-UINT32  Reserved1:2;
+///
+/// [Bit 4] PwrEvtEn.
+///
+UINT32  PwrEvtEn:1;
+///
+/// [Bit 5] FUPonPTW.
+///
+UINT32  FUPonPTW:1;
 ///
 /// [Bit 6] FabricEn. If (CPUID.(EAX=07H, ECX=0):ECX[3] = 1).
 ///
@@ -4672,7 +4679,10 @@ typedef union {
 /// [Bit 11] DisRETC.
 ///
 UINT32  DisRETC:1;
-UINT32  Reserved2:1;
+///
+/// [Bit 12] PTWEn.
+///
+UINT32  PTWEn:1;
 ///
 /// [Bit 13] BranchEn.
 ///
@@ -4681,17 +4691,17 @@ typedef union {
 /// [Bits 17:14] MTCFreq. If (CPUID.(EAX=07H, ECX=0):EBX[3] = 1).
 ///
 UINT32  MTCFreq:4;
-UINT32  Reserved3:1;
+UINT32  Reserved1:1;
 ///
 /// [Bits 22:19] CYCThresh. If (CPUID.(EAX=07H, ECX=0):EBX[1] = 1).
 ///
 UINT32  CYCThresh:4;
-UINT32  Reserved4:1;
+UINT32  Reserved2:1;
 ///
 /// [Bits 27:24] PSBFreq. If (CPUID.(EAX=07H, ECX=0):EBX[1] = 1).
 ///
 UINT32  PSBFreq:4;
-UINT32  Reserved5:4;
+UINT32  Reserved3:4;
 ///
 /// [Bits 35:32] ADDR0_CFG. If (CPUID.(EAX=07H, ECX=1):EAX[2:0] > 0).
 ///
@@ -4708,7 +4718,7 @@ typedef union {
 /// [Bits 47:44] ADDR3_CFG. If (CPUID.(EAX=07H, ECX=1):EAX[2:0] > 3).
 ///
 UINT32  ADDR3_CFG:4;
-UINT32  Reserved6:16;
+UINT32  Reserved4:16;
   } Bits;
   ///
   /// All bit fields as a 64-bit value
-- 
2.15.0.windows.1

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


[edk2] [Patch 05/14] UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h: Add new MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Add new MSR definition:
 1. MSR_XEON_PHI_PPIN_CTL
 2. MSR_XEON_PHI_PPIN
 3. MSR_XEON_PHI_MISC_FEATURE_ENABLES
 4. MSR_XEON_PHI_MSRUNCORE_RATIO_LIMIT
  2. Add DisplayModule == 0x85 supports.

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h | 249 ++-
 1 file changed, 246 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h 
b/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h
index d7aa3ae850..da74c2402c 100644
--- a/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h
@@ -38,7 +38,8 @@
 #define IS_XEON_PHI_PROCESSOR(DisplayFamily, DisplayModel) \
   (DisplayFamily == 0x06 && \
(\
-DisplayModel == 0x57\
+DisplayModel == 0x57 || \
+DisplayModel == 0x85\
 )   \
)
 
@@ -85,9 +86,89 @@ typedef union {
   UINT64  Uint64;
 } MSR_XEON_PHI_SMI_COUNT_REGISTER;
 
+/**
+  Package. Protected Processor Inventory Number Enable Control (R/W).
+
+  @param  ECX  MSR_XEON_PHI_PPIN_CTL (0x004E)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type MSR_XEON_PHI_PPIN_CTL_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type MSR_XEON_PHI_PPIN_CTL_REGISTER.
+
+  Example usage
+  @code
+  MSR_XEON_PHI_PPIN_CTL_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_XEON_PHI_PPIN_CTL);
+  AsmWriteMsr64 (MSR_XEON_PHI_PPIN_CTL, Msr.Uint64);
+  @endcode
+**/
+#define MSR_XEON_PHI_PPIN_CTL0x004E
 
 /**
-  Package. See http://biosbits.org.
+  MSR information returned for MSR index #MSR_XEON_PHI_PPIN_CTL
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+///
+/// [Bit 0] LockOut (R/WO) Set 1 to prevent further writes to
+/// MSR_PPIN_CTL. Writing 1 to MSR_PPINCTL[bit 0] is permitted only if
+/// MSR_PPIN_CTL[bit 1] is clear. Default is 0. BIOS should provide an
+/// opt-in menu to enable the user to turn on MSR_PPIN_CTL[bit 1] for a
+/// privileged inventory initialization agent to access MSR_PPIN. After
+/// reading MSR_PPIN, the privileged inventory initialization agent should
+/// write '01b' to MSR_PPIN_CTL to disable further access to MSR_PPIN and
+/// prevent unauthorized modification to MSR_PPIN_CTL.
+///
+UINT32  LockOut:1;
+///
+/// [Bit 1] Enable_PPIN (R/W) If 1, enables MSR_PPIN to be accessible
+/// using RDMSR. Once set, an attempt to write 1 to MSR_PPIN_CTL[bit 0]
+/// will cause #GP. If 0, an attempt to read MSR_PPIN will cause #GP.
+/// Default is 0.
+///
+UINT32  Enable_PPIN:1;
+UINT32  Reserved1:30;
+UINT32  Reserved2:32;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32  Uint32;
+  ///
+  /// All bit fields as a 64-bit value
+  ///
+  UINT64  Uint64;
+} MSR_XEON_PHI_PPIN_CTL_REGISTER;
+
+
+/**
+  Package. Protected Processor Inventory Number (R/O). Protected Processor
+  Inventory Number (R/O) A unique value within a given CPUID
+  family/model/stepping signature that a privileged inventory initialization
+  agent can access to identify each physical processor, when access to
+  MSR_PPIN is enabled. Access to MSR_PPIN is permitted only if
+  MSR_PPIN_CTL[bits 1:0] = '10b'.
+
+  @param  ECX  MSR_XEON_PHI_PPIN (0x004F)
+  @param  EAX  Lower 32-bits of MSR value.
+  @param  EDX  Upper 32-bits of MSR value.
+
+  Example usage
+  @code
+  UINT64  Msr;
+
+  Msr = AsmReadMsr64 (MSR_XEON_PHI_PPIN);
+  @endcode
+**/
+#define MSR_XEON_PHI_PPIN0x004F
+
+/**
+  Package. Platform Information Contains power management and other model
+  specific features enumeration. See http://biosbits.org.
 
   @param  ECX  MSR_XEON_PHI_PLATFORM_INFO (0x00CE)
   @param  EAX  Lower 32-bits of MSR value.
@@ -317,6 +398,56 @@ typedef union {
 } MSR_XEON_PHI_FEATURE_CONFIG_REGISTER;
 
 
+/**
+  Thread. MISC_FEATURE_ENABLES.
+
+  @param  ECX  MSR_XEON_PHI_MISC_FEATURE_ENABLES (0x0140)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type 
MSR_XEON_PHI_MISC_FEATURE_ENABLES_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type 
MSR_XEON_PHI_MISC_FEATURE_ENABLES_REGISTER.
+
+  Example usage
+  @code
+  MSR_XEON_PHI_MISC_FEATURE_ENABLES_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_XEON_PHI_MISC_FEATURE_ENABLES);
+  AsmWriteMsr64 (MSR_XEON_PHI_MISC_FEATURE_ENABLES, Msr.Uint64);
+  @endcode
+**/
+#define MSR_XEON_PHI_MISC_FEATURE_ENABLES0x0140
+
+/**
+  MSR information returned for MSR index #MSR_XEON_PHI_MISC_FEATURE_ENABLES
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+UINT32  Reserved1:1;
+///
+/// [Bit 1] User Mode MONITOR and MWA

[edk2] [Patch 07/14] UefiCpuPkg/Include/Register/ArchitecturalMsr.h: Add new MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Add new MSRs: MSR_IA32_L2_QOS_CFG/MSR_IA32_CSTAR.

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/ArchitecturalMsr.h | 64 ++
 1 file changed, 64 insertions(+)

diff --git a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h 
b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
index 5d2242aa80..b467ffaf26 100644
--- a/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
+++ b/UefiCpuPkg/Include/Register/ArchitecturalMsr.h
@@ -5908,6 +5908,51 @@ typedef union {
   UINT64  Uint64;
 } MSR_IA32_L3_QOS_CFG_REGISTER;
 
+/**
+  L2 QOS Configuration (R/W). If ( CPUID.(EAX=10H, ECX=2):ECX.[2] = 1 ).
+
+  @param  ECX  MSR_IA32_L2_QOS_CFG (0x0C82)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type MSR_IA32_L2_QOS_CFG_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type MSR_IA32_L2_QOS_CFG_REGISTER.
+
+  Example usage
+  @code
+  MSR_IA32_L2_QOS_CFG_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_IA32_L2_QOS_CFG);
+  AsmWriteMsr64 (MSR_IA32_L2_QOS_CFG, Msr.Uint64);
+  @endcode
+  @note MSR_IA32_L2_QOS_CFG is defined as IA32_L2_QOS_CFG in SDM.
+**/
+#define MSR_IA32_L2_QOS_CFG  0x0C82
+
+/**
+  MSR information returned for MSR index #MSR_IA32_L2_QOS_CFG
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+///
+/// [Bit 0] Enable (R/W) Set 1 to enable L2 CAT masks and COS to operate
+/// in Code and Data Prioritization (CDP) mode.
+///
+UINT32  Enable:1;
+UINT32  Reserved1:31;
+UINT32  Reserved2:32;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32  Uint32;
+  ///
+  /// All bit fields as a 64-bit value
+  ///
+  UINT64  Uint64;
+} MSR_IA32_L2_QOS_CFG_REGISTER;
 
 /**
   Monitoring Event Select Register (R/W). If ( CPUID.(EAX=07H, ECX=0):EBX.[12]
@@ -6380,6 +6425,25 @@ typedef union {
 **/
 #define MSR_IA32_LSTAR   0xC082
 
+/**
+  IA-32e Mode System Call Target Address (R/W) Not used, as the SYSCALL
+  instruction is not recognized in compatibility mode. If
+  CPUID.8001:EDX.[29] = 1.
+
+  @param  ECX  MSR_IA32_CSTAR (0xC083)
+  @param  EAX  Lower 32-bits of MSR value.
+  @param  EDX  Upper 32-bits of MSR value.
+
+  Example usage
+  @code
+  UINT64  Msr;
+
+  Msr = AsmReadMsr64 (MSR_IA32_CSTAR);
+  AsmWriteMsr64 (MSR_IA32_CSTAR, Msr);
+  @endcode
+  @note MSR_IA32_CSTAR is defined as IA32_CSTAR in SDM.
+**/
+#define MSR_IA32_CSTAR   0xC083
 
 /**
   System Call Flag Mask (R/W). If CPUID.8001:EDX.[29] = 1.
-- 
2.15.0.windows.1

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


[edk2] [Patch 10/14] UefiCpuPkg/Include/Register/Msr/P6Msr.h: Remove old MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Remove MSR which not existed in 2018-05 version spec: 
MSR_P6_ROB_CR_BKUPTMPDR6.

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/P6Msr.h | 48 -
 1 file changed, 48 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/P6Msr.h 
b/UefiCpuPkg/Include/Register/Msr/P6Msr.h
index d8af2db3da..9cef72b239 100644
--- a/UefiCpuPkg/Include/Register/Msr/P6Msr.h
+++ b/UefiCpuPkg/Include/Register/Msr/P6Msr.h
@@ -1153,54 +1153,6 @@ typedef union {
 **/
 #define MSR_P6_LASTINTTOIP   0x01DE
 
-
-/**
-
-
-  @param  ECX  MSR_P6_ROB_CR_BKUPTMPDR6 (0x01E0)
-  @param  EAX  Lower 32-bits of MSR value.
-   Described by the type MSR_P6_ROB_CR_BKUPTMPDR6_REGISTER.
-  @param  EDX  Upper 32-bits of MSR value.
-   Described by the type MSR_P6_ROB_CR_BKUPTMPDR6_REGISTER.
-
-  Example usage
-  @code
-  MSR_P6_ROB_CR_BKUPTMPDR6_REGISTER  Msr;
-
-  Msr.Uint64 = AsmReadMsr64 (MSR_P6_ROB_CR_BKUPTMPDR6);
-  AsmWriteMsr64 (MSR_P6_ROB_CR_BKUPTMPDR6, Msr.Uint64);
-  @endcode
-  @note MSR_P6_ROB_CR_BKUPTMPDR6 is defined as ROB_CR_BKUPTMPDR6 in SDM.
-**/
-#define MSR_P6_ROB_CR_BKUPTMPDR6 0x01E0
-
-/**
-  MSR information returned for MSR index #MSR_P6_ROB_CR_BKUPTMPDR6
-**/
-typedef union {
-  ///
-  /// Individual bit fields
-  ///
-  struct {
-UINT32  Reserved1:2;
-///
-/// [Bit 2] Fast Strings Enable bit. Default is enabled.
-///
-UINT32  FastStrings:1;
-UINT32  Reserved2:29;
-UINT32  Reserved3:32;
-  } Bits;
-  ///
-  /// All bit fields as a 32-bit value
-  ///
-  UINT32  Uint32;
-  ///
-  /// All bit fields as a 64-bit value
-  ///
-  UINT64  Uint64;
-} MSR_P6_ROB_CR_BKUPTMPDR6_REGISTER;
-
-
 /**
 
 
-- 
2.15.0.windows.1

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


[edk2] [Patch 06/14] UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h: Add new MSRs.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Add new MSR definitions.
  2. Add support platform info.

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h | 1548 +-
 1 file changed, 1547 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h 
b/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h
index 866fe30f05..90cde86ccb 100644
--- a/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h
@@ -39,7 +39,11 @@
   (DisplayFamily == 0x06 && \
(\
 DisplayModel == 0x4E || \
-DisplayModel == 0x5E\
+DisplayModel == 0x5E || \
+DisplayModel == 0x55 || \
+DisplayModel == 0x8E || \
+DisplayModel == 0x9E || \
+DisplayModel == 0x66\
 )   \
)
 
@@ -124,6 +128,74 @@ typedef union {
 #define MSR_SKYLAKE_LASTBRANCH_TOS   0x01C9
 
 
+/**
+  Core. Power Control Register See http://biosbits.org.
+
+  @param  ECX  MSR_SKYLAKE_POWER_CTL (0x01FC)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type MSR_SKYLAKE_POWER_CTL_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type MSR_SKYLAKE_POWER_CTL_REGISTER.
+
+  Example usage
+  @code
+  MSR_SKYLAKE_POWER_CTL_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_SKYLAKE_POWER_CTL);
+  AsmWriteMsr64 (MSR_SKYLAKE_POWER_CTL, Msr.Uint64);
+  @endcode
+**/
+#define MSR_SKYLAKE_POWER_CTL 0x01FC
+
+/**
+  MSR information returned for MSR index #MSR_SKYLAKE_POWER_CTL
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+UINT32  Reserved1:1;
+///
+/// [Bit 1] Package. C1E Enable (R/W) When set to '1', will enable the CPU
+/// to switch to the Minimum Enhanced Intel SpeedStep Technology operating
+/// point when all execution cores enter MWAIT (C1).
+///
+UINT32  C1EEnable:1;
+UINT32  Reserved2:17;
+///
+/// [Bit 19] Disable Race to Halt Optimization (R/W) Setting this bit
+/// disables the Race to Halt optimization and avoids this optimization
+/// limitation to execute below the most efficient frequency ratio.
+/// Default value is 0 for processors that support Race to Halt
+/// optimization. Default value is 1 for processors that do not support
+/// Race to Halt optimization.
+///
+UINT32  Fix_Me_1:1;
+///
+/// [Bit 20] Disable Energy Efficiency Optimization (R/W) Setting this bit
+/// disables the P-States energy efficiency optimization. Default value is
+/// 0. Disable/enable the energy efficiency optimization in P-State legacy
+/// mode (when IA32_PM_ENABLE[HWP_ENABLE] = 0), has an effect only in the
+/// turbo range or into PERF_MIN_CTL value if it is not zero set. In HWP
+/// mode (IA32_PM_ENABLE[HWP_ENABLE] == 1), has an effect between the OS
+/// desired or OS maximize to the OS minimize performance setting.
+///
+UINT32  DisableEnergyEfficiencyOptimization:1;
+UINT32  Reserved3:11;
+UINT32  Reserved4:32;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32  Uint32;
+  ///
+  /// All bit fields as a 64-bit value
+  ///
+  UINT64  Uint64;
+} MSR_SKYLAKE_POWER_CTL_REGISTER;
+
+
 /**
   Package. Lower 64 Bit OwnerEpoch Component of SGX Key (RO). Low 64 bits of
   an 128-bit external entropy value for key derivation of an enclave.
@@ -2254,4 +2326,1478 @@ typedef union {
   UINT64  Uint64;
 } MSR_SKYLAKE_UNC_PERF_GLOBAL_STATUS_REGISTER;
 
+
+/**
+  Package. NPK Address Used by AET Messages (R/W).
+
+  @param  ECX  MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE (0x0080)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type 
MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type 
MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE_REGISTER.
+
+  Example usage
+  @code
+  MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE);
+  AsmWriteMsr64 (MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE, Msr.Uint64);
+  @endcode
+**/
+#define MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE   0x0080
+
+/**
+  MSR information returned for MSR index
+  #MSR_SKYLAKE_TRACE_HUB_STH_ACPIBAR_BASE
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+///
+/// [Bit 0] Lock Bit If set, this MSR cannot be re-written anymore. Lock
+/// bit has to be set in order for the AET packets to be directed to NPK
+/// MMIO.
+///
+UINT32  Fix_Me_1:1;
+UINT32  Reserved:17;
+///
+/// [Bits 31:18] ACPIBAR_BASE_ADDRESS AET target address in NPK MMIO space.
+///
+UINT32  ACPIBAR_BASE_ADDRESS:14;
+///
+/// [Bits 63:32] ACPIBAR_BASE_ADDRESS AET target address i

[edk2] [Patch 04/14] UefiCpuPkg/Include/Register/Msr/*.h: Add new MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Add new MSR: MSR_*_MSRUNCORE_RATIO_LIMIT

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h | 54 +
 UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h  | 56 +-
 UefiCpuPkg/Include/Register/Msr/XeonDMsr.h | 56 +-
 3 files changed, 164 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h 
b/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h
index 4e50f72008..a7a1967420 100644
--- a/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h
@@ -285,6 +285,60 @@ typedef union {
 } MSR_BROADWELL_TURBO_RATIO_LIMIT_REGISTER;
 
 
+/**
+  Package. Uncore Ratio Limit (R/W) Out of reset, the min_ratio and max_ratio
+  fields represent the widest possible range of uncore frequencies. Writing to
+  these fields allows software to control the minimum and the maximum
+  frequency that hardware will select.
+
+  @param  ECX  MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT (0x0620)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type 
MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type 
MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT_REGISTER.
+
+  Example usage
+  @code
+  MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT);
+  AsmWriteMsr64 (MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT, Msr.Uint64);
+  @endcode
+**/
+#define MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT  0x0620
+
+/**
+  MSR information returned for MSR index #MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+///
+/// [Bits 6:0] MAX_RATIO This field is used to limit the max ratio of the
+/// LLC/Ring.
+///
+UINT32  MAX_RATIO:7;
+UINT32  Reserved2:1;
+///
+/// [Bits 14:8] MIN_RATIO Writing to this field controls the minimum
+/// possible ratio of the LLC/Ring.
+///
+UINT32  MIN_RATIO:7;
+UINT32  Reserved3:17;
+UINT32  Reserved4:32;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32  Uint32;
+  ///
+  /// All bit fields as a 64-bit value
+  ///
+  UINT64  Uint64;
+} MSR_BROADWELL_MSRUNCORE_RATIO_LIMIT_REGISTER;
+
 /**
   Package. PP0 Energy Status (R/O)  See Section 14.9.4, "PP0/PP1 RAPL
   Domains.".
diff --git a/UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h 
b/UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h
index a75bdb2e13..985183b320 100644
--- a/UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h
@@ -846,7 +846,61 @@ typedef union {
 
 
 /**
-  Package. Reserved (R/O)  Reads return 0.
+  Package. Uncore Ratio Limit (R/W) Out of reset, the min_ratio and max_ratio
+  fields represent the widest possible range of uncore frequencies. Writing to
+  these fields allows software to control the minimum and the maximum
+  frequency that hardware will select.
+
+  @param  ECX  MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT (0x0620)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type 
MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type 
MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT_REGISTER.
+
+  Example usage
+  @code
+  MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT);
+  AsmWriteMsr64 (MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT, Msr.Uint64);
+  @endcode
+**/
+#define MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT  0x0620
+
+/**
+  MSR information returned for MSR index #MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+///
+/// [Bits 6:0] MAX_RATIO This field is used to limit the max ratio of the
+/// LLC/Ring.
+///
+UINT32  MAX_RATIO:7;
+UINT32  Reserved1:1;
+///
+/// [Bits 14:8] MIN_RATIO Writing to this field controls the minimum
+/// possible ratio of the LLC/Ring.
+///
+UINT32  MIN_RATIO:7;
+UINT32  Reserved2:17;
+UINT32  Reserved3:32;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32  Uint32;
+  ///
+  /// All bit fields as a 64-bit value
+  ///
+  UINT64  Uint64;
+} MSR_HASWELL_E_MSRUNCORE_RATIO_LIMIT_REGISTER;
+
+/**
+  Package. Reserved (R/O) Reads return 0.
 
   @param  ECX  MSR_HASWELL_E_PP0_ENERGY_STATUS (0x0639)
   @param  EAX  Lower 32-bits of MSR value.
diff --git a/UefiCpuPkg/Include/Register/Msr/XeonDMsr.h 
b/UefiCpuPkg/Include/Register/Msr/XeonDMsr.h
index cf013ea887..6dc4ee999e 100644
--- a/UefiCpuPkg/Include/Register/Msr/XeonDMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/XeonDMsr.h
@@ -754,7 +754,61 @@ typedef union {
 
 
 /**
-  P

[edk2] [Patch 03/14] UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h: Add new MSR.

2018-09-17 Thread Eric Dong
Changes includes:
  1. Add new MSR: MSR_SILVERMONT_PLATFORM_INFO

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h | 48 +
 1 file changed, 48 insertions(+)

diff --git a/UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h 
b/UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h
index 03bbd0af7c..c3d0f8c208 100644
--- a/UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h
+++ b/UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h
@@ -375,6 +375,54 @@ typedef union {
 } MSR_SILVERMONT_FSB_FREQ_REGISTER;
 
 
+/**
+  Package. Platform Information: Contains power management and other model
+  specific features enumeration. See http://biosbits.org.
+
+  @param  ECX  MSR_SILVERMONT_PLATFORM_INFO (0x00CE)
+  @param  EAX  Lower 32-bits of MSR value.
+   Described by the type MSR_SILVERMONT_PLATFORM_INFO_REGISTER.
+  @param  EDX  Upper 32-bits of MSR value.
+   Described by the type MSR_SILVERMONT_PLATFORM_INFO_REGISTER.
+
+  Example usage
+  @code
+  MSR_SILVERMONT_PLATFORM_INFO_REGISTER  Msr;
+
+  Msr.Uint64 = AsmReadMsr64 (MSR_SILVERMONT_PLATFORM_INFO);
+  AsmWriteMsr64 (MSR_SILVERMONT_PLATFORM_INFO, Msr.Uint64);
+  @endcode
+**/
+#define MSR_SILVERMONT_PLATFORM_INFO 0x00CE
+
+/**
+  MSR information returned for MSR index #MSR_SILVERMONT_PLATFORM_INFO
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+UINT32  Reserved1:8;
+///
+/// [Bits 15:8] Package. Maximum Non-Turbo Ratio (R/O) This is the ratio
+/// of the maximum frequency that does not require turbo. Frequency =
+/// ratio * Scalable Bus Frequency.
+///
+UINT32  MaximumNon_TurboRatio:8;
+UINT32  Reserved2:16;
+UINT32  Reserved3:32;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32  Uint32;
+  ///
+  /// All bit fields as a 64-bit value
+  ///
+  UINT64  Uint64;
+} MSR_SILVERMONT_PLATFORM_INFO_REGISTER;
+
 /**
   Module. C-State Configuration Control (R/W)  Note: C-state values are
   processor specific C-state code names, unrelated to MWAIT extension C-state
-- 
2.15.0.windows.1

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


[edk2] [Patch 00/14] Update MSR definitions.

2018-09-17 Thread Eric Dong
Current MSR definition are follow the SDM 2016-09 version. The latest
SDM is 2018-05. This patch serial update the MSR related definition to
follow the latest SDM 2018-05 version. MSR related defintion are saved
at UefiCpuPkg\Include\Register\.

The changes for this serial includes:
1. Add new MSR definition and file.
2. Remove old MSR definition which not defined in new SDM.
3. Change MSR name to follow new SDM, keep old one for compatibility.
4. Change MSR data structure definition to follow new SDM.
5. Update comments to follow the new SDM, mainly related to chapter info.

Below changes are incompatible changes:
2. Remove old MSR definition which not defined in new SDM.
For this one, i search edk2 codebase, not found any code uses it. so no
impact for edk2 codebase. Detail changes see patch 9 ~ 11.

4. Change MSR data structure definition to follow new SDM.
For this one, new data structure just change the original reserved bits to
valid bits, should have no impact for the current code. Detail see patch 8
and patch 14

Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 


Eric Dong (14):
  UefiCpuPkg/Include/Register/Msr: Update reference spec info.
  UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h: Add new MSR file
for goldmont plus microarchitecture.
  UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h: Add new MSR.
  UefiCpuPkg/Include/Register/Msr/*.h: Add new MSR.
  UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h: Add new MSR.
  UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h: Add new MSRs.
  UefiCpuPkg/Include/Register/ArchitecturalMsr.h: Add new MSR.
  UefiCpuPkg/Include/Register/ArchitecturalMsr.h: Change structure
definition.
  UefiCpuPkg/Include/Register/Msr/Core2Msr.h: Remove old MSR.
  UefiCpuPkg/Include/Register/Msr/P6Msr.h: Remove old MSR.
  UefiCpuPkg/Include/Register/Msr/CoreMsr.h: Remove old MSR.
  UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h: Add new MSR name and
keep old one.
  UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h: Add new MSR name and
keep old one.
  UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h: Change structure
definition.

 UefiCpuPkg/Include/Register/ArchitecturalMsr.h|  130 +-
 UefiCpuPkg/Include/Register/Msr.h |7 +-
 UefiCpuPkg/Include/Register/Msr/AtomMsr.h |   28 +-
 UefiCpuPkg/Include/Register/Msr/BroadwellMsr.h|   62 +-
 UefiCpuPkg/Include/Register/Msr/Core2Msr.h|  102 +-
 UefiCpuPkg/Include/Register/Msr/CoreMsr.h |   74 +-
 UefiCpuPkg/Include/Register/Msr/GoldmontMsr.h |   88 +-
 UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h |  272 
 UefiCpuPkg/Include/Register/Msr/HaswellEMsr.h |   62 +-
 UefiCpuPkg/Include/Register/Msr/HaswellMsr.h  |   34 +-
 UefiCpuPkg/Include/Register/Msr/IvyBridgeMsr.h|8 +-
 UefiCpuPkg/Include/Register/Msr/NehalemMsr.h  |   52 +-
 UefiCpuPkg/Include/Register/Msr/P6Msr.h   |   60 +-
 UefiCpuPkg/Include/Register/Msr/Pentium4Msr.h |  202 +--
 UefiCpuPkg/Include/Register/Msr/PentiumMMsr.h |   22 +-
 UefiCpuPkg/Include/Register/Msr/PentiumMsr.h  |   12 +-
 UefiCpuPkg/Include/Register/Msr/SandyBridgeMsr.h  |   49 +-
 UefiCpuPkg/Include/Register/Msr/SilvermontMsr.h   |  100 +-
 UefiCpuPkg/Include/Register/Msr/SkylakeMsr.h  | 1602 -
 UefiCpuPkg/Include/Register/Msr/Xeon5600Msr.h |8 +-
 UefiCpuPkg/Include/Register/Msr/XeonDMsr.h|   84 +-
 UefiCpuPkg/Include/Register/Msr/XeonE7Msr.h   |6 +-
 UefiCpuPkg/Include/Register/Msr/XeonPhiMsr.h  |  326 -
 23 files changed, 2813 insertions(+), 577 deletions(-)
 create mode 100644 UefiCpuPkg/Include/Register/Msr/GoldmontPlusMsr.h

-- 
2.15.0.windows.1

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


Re: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits

2018-09-17 Thread Wu, Hao A
> -Original Message-
> From: Marcin Wojtas [mailto:m...@semihalf.com]
> Sent: Monday, September 17, 2018 11:00 PM
> To: Wu, Hao A
> Cc: edk2-devel-01; Tomasz Michalec; nad...@marvell.com; Gao, Liming;
> Kinney, Michael D
> Subject: Re: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix
> SdMmcHcReset to set only necesery bits
> 
> Hi Hao,
> 
> pon., 17 wrz 2018 o 09:17 Wu, Hao A  napisał(a):
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > > Marcin Wojtas
> > > Sent: Sunday, September 16, 2018 6:26 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Tian, Feng; t...@semihalf.com; Wu, Hao A; nad...@marvell.com; Gao,
> > > Liming; Kinney, Michael D
> > > Subject: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix
> > > SdMmcHcReset to set only necesery bits
> > >
> > > From: Tomasz Michalec 
> > >
> > > SdMmcHcReset used to set all bits of Software Reset Register to 1
> > > including reserved ones.
> >
> > Hi,
> >
> > I did a quick search of the SD Host Controller Simplified Specification, I
> > do not find the spec prohibit setting all the bits when performing a reset
> > for the host controller.
> >
> > Do you met with issues during the controller reset with the current logic?
> >
> 
> Yes, with SwResetMask = 0xFF, on all my boards (regardless speed),
> SdMmcHcWaitMmioSet times out on each interface. With
> SwReset = SD_MMC_HC_SW_RST_ALL it's all ok.

Thanks Marcin,

Got it. Could you help to file a EDK2 Bugzilla tracker as well? Thanks in
advance.

Also some minor comments below:

> 
> Best regards,
> Marcin
> 
> > Best Regards,
> > Hao Wu
> >
> > >
> > > Now only first bit is set which means "Software Reset for All".
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Marcin Wojtas 
> > > ---
> > >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 5 +
> > >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 6 +++---
> > >  2 files changed, 8 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > index e389d52..bcc31bd 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > > @@ -63,6 +63,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY
> > > KIND, EITHER EXPRESS OR IMPLIED.
> > >  #define SD_MMC_HC_CTRL_VER0xFE
> > >
> > >  //
> > > +// SD Software Reset Register bits description
> > > +//
> > > +#define SD_MMC_HC_SW_RST_ALL  BIT0

I think you can directly use BIT0 in .c file rather than introducing a new
macro here when performing "Software Reset for All".

> > > +
> > > +//
> > >  // The transfer modes supported by SD Host Controller
> > >  // Simplified Spec 3.0 Table 1-2
> > >  //
> > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > index 9672b5b..9d9bca8 100644
> > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > > @@ -454,8 +454,8 @@ SdMmcHcReset (
> > >}
> > >
> > >PciIo   = Private->PciIo;
> > > -  SwReset = 0xFF;
> > > -  Status  = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE,
> sizeof
> > > (SwReset), &SwReset);
> > > +  SwReset = SD_MMC_HC_SW_RST_ALL;
> > > +  Status  = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_SW_RST, sizeof
> > > (SwReset), &SwReset);
> > >
> > >if (EFI_ERROR (Status)) {
> > >  DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write full 1 fails: %r\n",
> Status));

Could you help to refine the debug message above to reflect the change?

Best Regards,
Hao Wu

> > > @@ -467,7 +467,7 @@ SdMmcHcReset (
> > >   Slot,
> > >   SD_MMC_HC_SW_RST,
> > >   sizeof (SwReset),
> > > - 0xFF,
> > > + SD_MMC_HC_SW_RST_ALL,
> > >   0x00,
> > >   SD_MMC_HC_GENERIC_TIMEOUT
> > >   );
> > > --
> > > 2.7.4
> > >
> > > ___
> > > 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


Re: [edk2] [Patch] MdeModulePkg/Ip4Dxe: Refine the coding style.

2018-09-17 Thread Bi, Dandan
Reviewed-by: Dandan Bi 

Thanks,
Dandan

-Original Message-
From: Wu, Jiaxin 
Sent: Monday, September 17, 2018 11:14 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Bi, Dandan 
; Wu, Jiaxin 
Subject: [Patch] MdeModulePkg/Ip4Dxe: Refine the coding style.

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1180
Remove the trailing white spaces.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Bi Dandan 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Wu Jiaxin 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
index b1af5294fb..4cb3b32688 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Impl.c
@@ -673,13 +673,13 @@ Ip4ConfigProtocol (
 
 //
 // Add a route to this connected network in the instance route table.
 //
 Ip4AddRoute (
-  IpInstance->RouteTable, 
-  Ip & Netmask, 
-  Netmask, 
+  IpInstance->RouteTable,
+  Ip & Netmask,
+  Netmask,
   IP4_ALLZERO_ADDRESS
   );
   } else {
 //
 // Use the default address. Check the state.
-- 
2.17.1.windows.2

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


Re: [edk2] [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs

2018-09-17 Thread Wang, Jian J
I have no strong opinion for this proposal. But if we decide to do it finally,
I'd suggest to add some warning messages for any probably surprising setting
combinations.

Regards,
Jian


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Monday, September 17, 2018 6:14 PM
> To: Zeng, Star ; Wang, Jian J ;
> edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Ni, Ruiyu 
> ;
> Yao, Jiewen 
> Subject: Re: [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs
> 
> On 09/17/18 07:57, Zeng, Star wrote:
> > How about we see the problem in another way?
> >
> > If my understanding is correct, current discussion and patches think FALSE/0
> means disable/clear NX, but that is not the fact.
> > According to the code implementation, FALSE/0 seems mean *AS IS* to do
> thing (no code to disable/clear NX).
> >
> > PcdSetNxForStack
> > TRUE: Set NX for stack.
> > FALSE: No code to clear NX for stack.
> >
> > PcdDxeNxMemoryProtectionPolicy
> > BITX 1: Set NX for that memory type.
> > BITX 0: No code to clear NX for that memory type.
> >
> > PcdImageProtectionPolicy
> > BITX 1: Set NX for the image data section.
> > BITX 0: Not code to clear NX for the image data section.
> >
> > So, how about we think one PCD just works for itself and it does not impact
> other PCDs to protect?
> > That means TRUE/1 is to protect and FALSE/0 is *AS IS* to do nothing.
> > The description of these PCDs could be enhancement if we think it is a good
> way to see the problem.
> 
> Sure, that too could work for me, but then the documentation in the DEC
> / UNI files has to be really clear.
> 
> The initial worry for the current discussion was that some platform might
> - protect e.g. BootServicesData type memory,
> - not set PcdSetNxForStack,
> - expect the stack to remain executable.
> 
> The actual results might surprise the platform owner.
> 
> If the documentation dispelled any possible misconceptions, I think your
> idea could work too (and it would be a lot easier to code).
> 
> Thanks
> Laszlo
> 
> 
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Wang, Jian J
> > Sent: Monday, September 17, 2018 10:11 AM
> > To: Laszlo Ersek ; edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Ard Biesheuvel
> ; Ni, Ruiyu ; Yao, Jiewen
> 
> > Subject: RE: [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs
> >
> > Laszlo,
> >
> > Thanks for the comments.
> >
> > Regards,
> > Jian
> >
> >> -Original Message-
> >> From: Laszlo Ersek [mailto:ler...@redhat.com]
> >> Sent: Friday, September 14, 2018 5:51 PM
> >> To: Wang, Jian J ; edk2-devel@lists.01.org
> >> Cc: Zeng, Star ; Ard Biesheuvel
> >> ; Ni, Ruiyu ; Yao,
> >> Jiewen 
> >> Subject: Re: [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs
> >>
> >> I've got some comments on the code as well:
> >>
> >> On 09/14/18 07:13, Jian J Wang wrote:
> >>>  BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116
> >>>
> >>>  Currently IA32_EFER.NXE is only set against PcdSetNxForStack. This
> >>>  confuses developers because following two other PCDs also need NXE
> >>>  to be set, but actually not.
> >>>
> >>>  PcdDxeNxMemoryProtectionPolicy
> >>>  PcdImageProtectionPolicy
> >>>
> >>>  This patch solves this issue by adding logic to enable IA32_EFER.NXE
> >>>  if any of those PCDs have anything enabled.
> >>>
> >>>  Due to the fact that NX memory type of stack (enabled by
> >>> PcdSetNxForStack)
> >>>  and image data section (enabled by PcdImageProtectionPolicy) are
> >>> also
> >>>  part of PcdDxeNxMemoryProtectionPolicy, this patch also add more
> >>> checks
> >>>  to warn (ASSERT) users any unreasonable setting combinations. For
> >>> example,
> >>>
> >>>  PcdSetNxForStack == FALSE &&
> >>>    (PcdDxeNxMemoryProtectionPolicy & (1 < >>> != 0
> >>>
> >>>  PcdImageProtectionPolicy == 0 &&
> >>>    (PcdDxeNxMemoryProtectionPolicy & (1 <<
> >>> EfiRuntimeServicesData)) != 0
> >>>
> >>>  PcdImageProtectionPolicy == 0 &&
> >>>    (PcdDxeNxMemoryProtectionPolicy & (1 < >>> != 0
> >>>
> >>>  PcdImageProtectionPolicy == 0 &&
> >>>    (PcdDxeNxMemoryProtectionPolicy & (1 < >>>
> >>>  In other words, PcdSetNxForStack and PcdImageProtectionPolicy have
> >>>  priority over PcdDxeNxMemoryProtectionPolicy.
> >>>
> >>>  Cc: Star Zeng 
> >>>  Cc: Laszlo Ersek 
> >>>  Cc: Ard Biesheuvel 
> >>>  Cc: Ruiyu Ni 
> >>>  Cc: Jiewen Yao 
> >>>  Contributed-under: TianoCore Contribution Agreement 1.1
> >>>  Signed-off-by: Jian J Wang 
> >>>  ---
> >>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 +
> >>>   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  4 +-
> >>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 55 +++
> ++
> >> ++-
> >>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 33 +++
> ++
> >> +
> >>>   4 files changed, 91 insertions(+), 3 deletions(-)
> >>>
> >>>  diff --
> >> git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> >> b/MdeModulePkg/Core/DxeIp lPeim/DxeIpl.inf
> 

Re: [edk2] [PATCH v3 1/3] MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence

2018-09-17 Thread Marcin Wojtas
Hi Hao,

pon., 17 wrz 2018 o 09:18 Wu, Hao A  napisał(a):
>
> > -Original Message-
> > From: Marcin Wojtas [mailto:m...@semihalf.com]
> > Sent: Sunday, September 16, 2018 6:26 AM
> > To: edk2-devel@lists.01.org
> > Cc: Tian, Feng; Kinney, Michael D; Gao, Liming; leif.lindh...@linaro.org; 
> > Wu,
> > Hao A; ard.biesheu...@linaro.org; nad...@marvell.com; m...@semihalf.com;
> > j...@semihalf.com; t...@semihalf.com
> > Subject: [PATCH v3 1/3] MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock
> > and bus width sequence
> >
> > According to JESD84-B50-1 chapter A.6 (documentation about eMMC4.5
> > standard) step "Changing the data bus width" (A.6.3) should be
> > executed after step "Switching to high-speed mode" (A.6.2).
>
> Hi,
>
> My understanding to the spec is that the spec seems do not impose a
> sequence for 'Switching to high-speed mode' and 'Changing the data bus
> width'.
>
> My find is that both operation (A.6.2 & A.6.3 of JESD84-B50-1 ) requires:
>
> * Do these steps after the bus is initialized according to A.6.1, Bus
> * initialization.
>
> Also, for HS200 mode selection, the flow chart in Section 6.6.4 shows the
> bus width set before bus mode switch. So I think the current code is
> taking this as a reference when switching the 'High Speed' mode.
>
> I tried the eMMC device on my side, the current code implementation and
> codes after applying your patch both work. So do you met with issues with
> certain device with this sequence? If not, I prefer to keep the current
> logic.
>

I re-checked on my boards with unchanged logic - all 3 can work in
High Speed. Patch is pretty old, I can't recall exact circumstances.
I'll drop it in v4.

Best regards,
Marcin

> Best Regards,
> Hao Wu
>
> >
> > This patch fixes the bus-width/clock-setting sequence
> > in EmmcSwitchToHighSpeed ().
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 +
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > index c5fd214..12935ef 100755
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -745,10 +745,6 @@ EmmcSwitchToHighSpeed (
> >UINT8   HostCtrl1;
> >UINT8   HostCtrl2;
> >
> > -  Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, 
> > BusWidth);
> > -  if (EFI_ERROR (Status)) {
> > -return Status;
> > -  }
> >//
> >// Set to Hight Speed timing
> >//
> > @@ -783,6 +779,11 @@ EmmcSwitchToHighSpeed (
> >
> >HsTiming = 1;
> >Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> > ClockFreq);
> > +  if (EFI_ERROR (Status)) {
> > +return Status;
> > +  }
> > +
> > +  Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, 
> > BusWidth);
> >
> >return Status;
> >  }
> > --
> > 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch 3/5] ShellPkg/TftpDynamicCommand: Add one option for tftp command to specify windowsize.

2018-09-17 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Wu, Jiaxin
> Sent: Sunday, September 16, 2018 10:44 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Carsey,
> Jaben ; Shao, Ming ; Wu,
> Jiaxin 
> Subject: [Patch 3/5] ShellPkg/TftpDynamicCommand: Add one option for tftp
> command to specify windowsize.
> Importance: High
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=886
> 
> This patch is to define one new option for TFTP shell command to specify the
> windowsize option as defined in RFC 7440. Valid range is between 1 and 64,
> default value is 1.
> 
> Note that: RFC 7440 does not mention max window size value, but for the
> stability reason, the value is limited to 64.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Carsey Jaben 
> Cc: Shao Ming 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Wu Jiaxin 
> ---
>  .../DynamicCommand/TftpDynamicCommand/Tftp.c  | 65
> ---
>  .../TftpDynamicCommand/Tftp.uni   |  6 +-
>  2 files changed, 59 insertions(+), 12 deletions(-)
> 
> diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> index 44be6d4e76..c66be6b9d9 100644
> --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> @@ -181,10 +181,11 @@ DownloadFile (
>IN   EFI_MTFTP4_PROTOCOL  *Mtftp4,
>IN   CONST CHAR16 *FilePath,
>IN   CONST CHAR8  *AsciiFilePath,
>IN   UINTNFileSize,
>IN   UINT16   BlockSize,
> +  IN   UINT16   WindowSize,
>OUT  VOID **Data
>);
> 
>  /**
>Update the progress of a file download
> @@ -225,10 +226,11 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>{L"-l", TypeValue},
>{L"-r", TypeValue},
>{L"-c", TypeValue},
>{L"-t", TypeValue},
>{L"-s", TypeValue},
> +  {L"-w", TypeValue},
>{NULL , TypeMax}
>};
> 
>  ///
>  /// The default block size (512) of tftp is defined in the RFC1350.
> @@ -237,11 +239,21 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>  ///
>  /// The valid range of block size option is defined in the RFC2348.
>  ///
>  #define MTFTP_MIN_BLKSIZE  8
>  #define MTFTP_MAX_BLKSIZE  65464
> -
> +///
> +/// The default windowsize (1) of tftp.
> +///
> +#define MTFTP_DEFAULT_WINDOWSIZE   1
> +///
> +/// The valid range of window size option.
> +/// Note that: RFC 7440 does not mention max window size value, but for
> the
> +/// stability reason, the value is limited to 64.
> +///
> +#define MTFTP_MIN_WINDOWSIZE   1
> +#define MTFTP_MAX_WINDOWSIZE   64
> 
>  /**
>Function for 'tftp' command.
> 
>@param[in] ImageHandle  Handle to the Image (NULL if Internal).
> @@ -286,19 +298,21 @@ RunTftp (
>UINTN   FileSize;
>UINTN   DataSize;
>VOID*Data;
>SHELL_FILE_HANDLE   FileHandle;
>UINT16  BlockSize;
> +  UINT16  WindowSize;
> 
>ShellStatus = SHELL_INVALID_PARAMETER;
>ProblemParam= NULL;
>NicFound= FALSE;
>AsciiRemoteFilePath = NULL;
>Handles = NULL;
>FileSize= 0;
>DataSize= 0;
>BlockSize   = MTFTP_DEFAULT_BLKSIZE;
> +  WindowSize  = MTFTP_DEFAULT_WINDOWSIZE;
> 
>//
>// Initialize the Shell library (we must be in non-auto-init...)
>//
>Status = ShellInitialize ();
> @@ -434,10 +448,24 @@ RunTftp (
>);
>goto Error;
>  }
>}
> 
> +  ValueStr = ShellCommandLineGetValue (CheckPackage, L"-w");
> +  if (ValueStr != NULL) {
> +if (!StringToUint16 (ValueStr, &WindowSize)) {
> +  goto Error;
> +}
> +if (WindowSize < MTFTP_MIN_WINDOWSIZE || WindowSize >
> MTFTP_MAX_WINDOWSIZE) {
> +  ShellPrintHiiEx (
> +-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> +mTftpHiiHandle, L"tftp", ValueStr
> +  );
> +  goto Error;
> +}
> +  }
> +
>//
>// Locate all MTFTP4 Service Binding protocols
>//
>ShellStatus = SHELL_NOT_FOUND;
>Status = gBS->LocateHandleBuffer (
> @@ -508,11 +536,11 @@ RunTftp (
>  mTftpHiiHandle, RemoteFilePath, NicName, Status
>);
>goto NextHandle;
>  }
> 
> -Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, &Data);
> +Status = DownloadFile (Mtftp4, RemoteFilePath, AsciiRemoteFilePath,
> FileSize, BlockSize, WindowSize, &Data);
>  if (EFI_ERROR (Status)) {
>ShellPrintHiiEx (
>  -1, -1, NULL, STRING_TOKEN (STR_TFTP_ERR_DOWNLOAD),
>  mTftpHiiHandle, RemoteFilePath, NicName, Status
>);
> @@ -894,20 +922,21 @@ DownloadFile (
>IN   EFI_MTFTP4_PROTOCOL  *Mtftp4,
>IN   CONST CHAR16 *FilePath,
>IN   CONST CHAR8  *AsciiFilePath,
>IN   UINTNFileSize,
>   

Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.

2018-09-17 Thread Laszlo Ersek
On 09/17/18 18:20, Duran, Leo wrote:
> 
>> -Original Message-
>> From: Ni, Ruiyu 
>> Sent: Thursday, September 13, 2018 11:44 PM
>> To: Duran, Leo ; Laszlo Ersek ;
>> edk2-devel@lists.01.org
>> Cc: Dong, Eric 
>> Subject: Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
>> MTRRs prior to MTRR change.
>>
>> On 9/14/2018 3:31 AM, Duran, Leo wrote:
>>>
>>>
 -Original Message-
 From: Ni, Ruiyu 
 Sent: Wednesday, September 12, 2018 9:39 PM
 To: Duran, Leo ; Laszlo Ersek
>> ;
 edk2-devel@lists.01.org
 Cc: Dong, Eric 
 Subject: RE: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
 MTRRs prior to MTRR change.

 Leo,
 Sorry I was in leave yesterday so didn't see the mail.
 Which MSRs are shared? Only the
>> MSR_IA32_MTRR_DEF_TYPE_REGISTER?
 Or all the MSRs that configures the CPU MTRR setting?

>>>
>>> Hi Ray,
>>> The MTTR config MSRs are also shared by threads within a core.
>>>
>>
>> Hi Leo,
>> Do you think that fixing the caller is more proper?
> 
> Hi Ray,
> Actually,
> The proposed PCD is the simplest solution, as that works for us and does not 
> change the existing (default) flow.
> 
> That is,
> I'd prefer making a decision about the PCD in platform-specific code, rather 
> than introducing complex detection and heuristics at the caller level in EDK2 
> (just for AMD).
> 
> So, please approve the PCD.

- From my side, if it works for you, it works for me. (The general trend
has been to avoid adding more PCDs to the "core" package DEC files, but
I'm 100% neutral on that.)

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


Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling MTRRs prior to MTRR change.

2018-09-17 Thread Duran, Leo


> -Original Message-
> From: Ni, Ruiyu 
> Sent: Thursday, September 13, 2018 11:44 PM
> To: Duran, Leo ; Laszlo Ersek ;
> edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: Re: [edk2] [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
> MTRRs prior to MTRR change.
> 
> On 9/14/2018 3:31 AM, Duran, Leo wrote:
> >
> >
> >> -Original Message-
> >> From: Ni, Ruiyu 
> >> Sent: Wednesday, September 12, 2018 9:39 PM
> >> To: Duran, Leo ; Laszlo Ersek
> ;
> >> edk2-devel@lists.01.org
> >> Cc: Dong, Eric 
> >> Subject: RE: [PATCH] UefiCpuPkg/MtrrLib: Add flag to skip disabling
> >> MTRRs prior to MTRR change.
> >>
> >> Leo,
> >> Sorry I was in leave yesterday so didn't see the mail.
> >> Which MSRs are shared? Only the
> MSR_IA32_MTRR_DEF_TYPE_REGISTER?
> >> Or all the MSRs that configures the CPU MTRR setting?
> >>
> >
> > Hi Ray,
> > The MTTR config MSRs are also shared by threads within a core.
> >
> 
> Hi Leo,
> Do you think that fixing the caller is more proper?

Hi Ray,
Actually,
The proposed PCD is the simplest solution, as that works for us and does not 
change the existing (default) flow.

That is,
I'd prefer making a decision about the PCD in platform-specific code, rather 
than introducing complex detection and heuristics at the caller level in EDK2 
(just for AMD).

So, please approve the PCD.

Thanks,
Leo

> 
> >> I also agree with Laszlo's comments to fix the caller if all MSRs
> >> relating to MTRR are shared.
> >> That will be to fix MpInitLib and CpuDxe driver.
> >>
> >> Thanks/Ray
> >>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 3/3] MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot

2018-09-17 Thread Marcin Wojtas
Hi Hao,

pon., 17 wrz 2018 o 09:18 Wu, Hao A  napisał(a):
>
> Hi,
>
> Could you help to file a EDK2 Bugzilla tracker at:
> https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2
>
> to have a general description of the issue you met? And reference the
> tracker within the commit message?
> Really appreciate the help, thanks in advance.
>
> With that,
> Reviewed-by: Hao Wu 
>

I created bug and will refer to it in v4:
https://bugzilla.tianocore.org/show_bug.cgi?id=1182

Best regards,
Marcin
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits

2018-09-17 Thread Marcin Wojtas
Hi Hao,

pon., 17 wrz 2018 o 09:17 Wu, Hao A  napisał(a):
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Marcin Wojtas
> > Sent: Sunday, September 16, 2018 6:26 AM
> > To: edk2-devel@lists.01.org
> > Cc: Tian, Feng; t...@semihalf.com; Wu, Hao A; nad...@marvell.com; Gao,
> > Liming; Kinney, Michael D
> > Subject: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix
> > SdMmcHcReset to set only necesery bits
> >
> > From: Tomasz Michalec 
> >
> > SdMmcHcReset used to set all bits of Software Reset Register to 1
> > including reserved ones.
>
> Hi,
>
> I did a quick search of the SD Host Controller Simplified Specification, I
> do not find the spec prohibit setting all the bits when performing a reset
> for the host controller.
>
> Do you met with issues during the controller reset with the current logic?
>

Yes, with SwResetMask = 0xFF, on all my boards (regardless speed),
SdMmcHcWaitMmioSet times out on each interface. With
SwReset = SD_MMC_HC_SW_RST_ALL it's all ok.

Best regards,
Marcin

> Best Regards,
> Hao Wu
>
> >
> > Now only first bit is set which means "Software Reset for All".
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 5 +
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 6 +++---
> >  2 files changed, 8 insertions(+), 3 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index e389d52..bcc31bd 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -63,6 +63,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> > KIND, EITHER EXPRESS OR IMPLIED.
> >  #define SD_MMC_HC_CTRL_VER0xFE
> >
> >  //
> > +// SD Software Reset Register bits description
> > +//
> > +#define SD_MMC_HC_SW_RST_ALL  BIT0
> > +
> > +//
> >  // The transfer modes supported by SD Host Controller
> >  // Simplified Spec 3.0 Table 1-2
> >  //
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index 9672b5b..9d9bca8 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -454,8 +454,8 @@ SdMmcHcReset (
> >}
> >
> >PciIo   = Private->PciIo;
> > -  SwReset = 0xFF;
> > -  Status  = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE, sizeof
> > (SwReset), &SwReset);
> > +  SwReset = SD_MMC_HC_SW_RST_ALL;
> > +  Status  = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_SW_RST, sizeof
> > (SwReset), &SwReset);
> >
> >if (EFI_ERROR (Status)) {
> >  DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write full 1 fails: %r\n", 
> > Status));
> > @@ -467,7 +467,7 @@ SdMmcHcReset (
> >   Slot,
> >   SD_MMC_HC_SW_RST,
> >   sizeof (SwReset),
> > - 0xFF,
> > + SD_MMC_HC_SW_RST_ALL,
> >   0x00,
> >   SD_MMC_HC_GENERIC_TIMEOUT
> >   );
> > --
> > 2.7.4
> >
> > ___
> > 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


[edk2] [Patch][edk2-platforms/devel-MinnowBoardMax-UDK2017] Produce SMBIOS type 1

2018-09-17 Thread Guo, Mang
Produce SMBIOS type 1 table before ready to boot event so BDS driver can get 
product name, CUP info from SMBIOS table and show the info in front page before 
boot.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Guo Mang 
CC: David Wei 
---
 .../SmBiosMiscDxe/MiscSystemManufacturerFunction.c | 26 +++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c 
b/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
index 1199374..40a84a8 100644
--- a/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
+++ b/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
@@ -206,11 +206,14 @@ AddSmbiosManuCallback (
   CHAR16*Uuid;
   UINT64TempData;
   UINTN Index;
+  static BOOLEANRemoveTable = TRUE;
+  EFI_SMBIOS_TABLE_HEADER   *Record;
 
 
 
-
+if (Event != NULL) {
   gBS->CloseEvent (Event);   // Unload this event.
+}
 
   DEBUG ((EFI_D_INFO, "Executing AddSmbiosManuCallback.\n"));
 
@@ -453,6 +456,21 @@ AddSmbiosManuCallback (
   // Now we have got the full smbios record, call smbios protocol to add this 
record.
   //
   SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
+  if (RemoveTable == TRUE) {
+Status = EFI_SUCCESS;
+while (!EFI_ERROR(Status)) {
+  Status = Smbios->GetNext (Smbios, &SmbiosHandle, NULL, &Record, NULL);
+  if (Record->Type == SMBIOS_TYPE_SYSTEM_INFORMATION) {
+Status = Smbios-> Remove(
+Smbios,
+SmbiosHandle
+);
+RemoveTable = FALSE;
+break;
+  }
+   }
+ }
+
   Status = Smbios-> Add(
   Smbios,
   NULL,
@@ -481,6 +499,9 @@ MISC_SMBIOS_TABLE_FUNCTION(MiscSystemManufacturer)
   static BOOLEANCallbackIsInstalledManu = FALSE;
   EFI_EVENT AddSmbiosManuCallbackEvent;
 
+
+  Status = AddSmbiosManuCallback (NULL, RecordData);
+
   if (CallbackIsInstalledManu == FALSE) {
 CallbackIsInstalledManu = TRUE;   // Prevent more than 1 callback.
 DEBUG ((EFI_D_INFO, "Create Smbios Manu callback.\n"));
@@ -492,10 +513,9 @@ MISC_SMBIOS_TABLE_FUNCTION(MiscSystemManufacturer)
RecordData,
&AddSmbiosManuCallbackEvent
);
-return Status;
   }
 
-  return EFI_SUCCESS;
+  return Status;
 
 }
 
-- 
2.10.1.windows.1

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


Re: [edk2] bootloader only see first disk

2018-09-17 Thread Aurelien Minet
Hi Thomas,

Le 17/09/2018 à 10:34, Thomas Lamprecht a écrit :
> Hi Laszlo,
>
> On 9/16/18 12:04 PM, Laszlo Ersek wrote:
>> Hello Aurélien,
>>
>> adding Thomas to the CC list, and commenting at the bottom:
> thanks for CC'ing and thanks to you Aurélien for writing your report.
>
>> On 09/15/18 18:36, Aurélien wrote:
>>> Hi,
>>>
>>> I don't know if it is the right place to report a bug, sorry if not.
>>>
>>> I'm a Promox user and the last version (5.2) has a package versionned
>>> 20180612-1 actually  (
>>> https://git.proxmox.com/?p=pve-edk2-firmware.git;a=summary ) which is
>>> based on 5a56c0493955cf55e7eef96dbba815cfbb067d7d  commit of edk2. A
>>> build from the current master a few days ago has the same problem. The
>>> previous Promox version (5.1 whith this pve-qemu-kvm package) use a
>>> edk2 version dating from 2017 or before (it could be from before
>>> September 2016 but I'm not sure), which has no problem. I tested on my
>>> workstation running gentoo which has a version based on UDK2017 branch
>>> with source code from February 11, 2018 and it's working too. So I
>>> think the problem is located in  the OVMF_CODE.fd code from 2018 (but
>>> without any certainty).
>>>
>>> Actual behaviour (with all the various Linux distribution: Debian 9,
>>> Ubuntu 18.04, Centos 7.5 ...) with a VM which has EFI enabled:
>>> - grub is loaded and only see the first disk (where it is installed),
>>>   the "ls" command in the grub shell return: "(hd0) (hd0,gpt1)
>>>   (cd0)"   and  there is also hd1 and hd2 and in my case this is
>>>   problematic as /boot is on hd1.
>>> - when pressing "escape" on the boot to go the the setup screen and
>>>   just chose "continue" it's ok (grub sees all the disks), my current
>>>   workaround to boot the installed OS.
>>> - when installing an OS like Ubuntu, CentOS ... on the first reboot
>>>   it's ok (I thinks something in the installation, like seting a new
>>>   boot entry with grub-install, efibootmgr)
>>>
>>> Expected behaviour:
>>> - grub see all the disk (the ls command return hd0, hd1 hd2 ) and
>>>   not the first one (where the EFI boot partition is located).
>>>   (sorry, I haven't tried any other boot loader)
>>>
>>> - Step to reproduce:
>>>   start a VM with 3 disks and EFI enabled (OVMF_CODE), install a Linux
>>>   on the 2nd disk (/ & swap) and the EFI boot partition on the first
>>>   disk (/boot/efi on "sda1"), cold start the VM.
>>>
>>> I can help by testing with specific version (or another boot loader),
>>> tell me.
>>>
>>> Regards
>>>
>>> Aurélien
>>>
>>> ps: typical command line to start the VM:
>>> /usr/bin/kvm \
>>>   -id 20 \
>>>   -name testvm.mydomain \
>>>   -chardev 'socket,id=qmp,path=/var/run/qemu-server/20.qmp,server,nowait' \
>>>   -mon 'chardev=qmp,mode=control' \
>>>   -pidfile /var/run/qemu-server/20.pid \
>>>   -daemonize \
>>>   -smbios 'type=1,uuid=88dc4e6b-34b8-46a7-a573-26596c34855d' \
>>>   -drive 
>>> 'if=pflash,unit=0,format=raw,readonly,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd'
>>>  \
>>>   -drive 
>>> 'if=pflash,unit=1,format=raw,id=drive-efidisk0,file=/mnt/pve/pve-nfs-sas/images/20/vm-20-disk-4.raw'
>>>  \
>>>   -smp '8,sockets=1,cores=8,maxcpus=8' \
>>>   -nodefaults \
>>>   -boot 
>>> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
>>>  \
>>>   -vga qxl \
>>>   -vnc unix:/var/run/qemu-server/20.vnc,x509,password \
>>>   -cpu host,+kvm_pv_unhalt,+kvm_pv_eoi \
>>>   -m 512 \
>>>   -object 'memory-backend-ram,id=ram-node0,size=512M' \
>>>   -numa 'node,nodeid=0,cpus=0-7,memdev=ram-node0' \
>>>   -k fr \
>>>   -object 'iothread,id=iothread-virtioscsi1' \
>>>   -object 'iothread,id=iothread-virtioscsi2' \
>>>   -machine 'type=pc-i440fx-2.9' \
>>>   -device 'pci-bridge,id=pci.3,chassis_nr=3,bus=pci.0,addr=0x5' \
>>>   -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
>>>   -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
>>>   -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
>>>   -chardev 'socket,path=/var/run/qemu-server/20.qga,server,nowait,id=qga0' \
>>>   -device 'virtio-serial,id=qga0,bus=pci.0,addr=0x8' \
>>>   -device 'virtserialport,chardev=qga0,name=org.qemu.guest_agent.0' \
>>>   -spice 
>>> 'tls-port=61004,addr=127.0.0.1,tls-ciphers=HIGH,seamless-migration=on' \
>>>   -device 'virtio-serial,id=spice,bus=pci.0,addr=0x9' \
>>>   -chardev 'spicevmc,id=vdagent,name=vdagent' \
>>>   -device 'virtserialport,chardev=vdagent,name=com.redhat.spice.0' \
>>>   -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
>>>   -iscsi 'initiator-name=iqn.1993-08.org.debian:01:f9e4631950da' \
>>>   -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
>>>   -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=100' \
>>>   -device 'virtio-scsi-pci,id=virtioscsi0,bus=pci.3,addr=0x1' \
>>>   -drive 
>>> 'file=/mnt/pve/pve-nfs-sas/images/20/vm-20-disk-1.raw,if=none,id=drive-scsi0,format=raw,cache=none,aio=native,detect-zeroes=on'
>>>  \
>>

[edk2] bootloader only see first disk

2018-09-17 Thread Aurelien
Hi Laszlo,

On 16/09/2018 12:04, Laszlo Ersek wrote:
> Hello Aurélien,
> 
> adding Thomas to the CC list, and commenting at the bottom:
> 
> On 09/15/18 18:36, Aurélien wrote:
>> Hi,
>>
>> I don't know if it is the right place to report a bug, sorry if not.
>>
>> I'm a Promox user and the last version (5.2) has a package versionned
>> 20180612-1 actually  (
>> https://git.proxmox.com/?p=pve-edk2-firmware.git;a=summary ) which is
>> based on 5a56c0493955cf55e7eef96dbba815cfbb067d7d  commit of edk2. A
>> build from the current master a few days ago has the same problem. The
>> previous Promox version (5.1 whith this pve-qemu-kvm package) use a
>> edk2 version dating from 2017 or before (it could be from before
>> September 2016 but I'm not sure), which has no problem. I tested on my
>> workstation running gentoo which has a version based on UDK2017 branch
>> with source code from February 11, 2018 and it's working too. So I
>> think the problem is located in  the OVMF_CODE.fd code from 2018 (but
>> without any certainty).
>>
>> Actual behaviour (with all the various Linux distribution: Debian 9,
>> Ubuntu 18.04, Centos 7.5 ...) with a VM which has EFI enabled:
>> - grub is loaded and only see the first disk (where it is installed),
>>   the "ls" command in the grub shell return: "(hd0) (hd0,gpt1)
>>   (cd0)"   and  there is also hd1 and hd2 and in my case this is
>>   problematic as /boot is on hd1.
>> - when pressing "escape" on the boot to go the the setup screen and
>>   just chose "continue" it's ok (grub sees all the disks), my current
>>   workaround to boot the installed OS.
>> - when installing an OS like Ubuntu, CentOS ... on the first reboot
>>   it's ok (I thinks something in the installation, like seting a new
>>   boot entry with grub-install, efibootmgr)
>>
>> Expected behaviour:
>> - grub see all the disk (the ls command return hd0, hd1 hd2 ) and
>>   not the first one (where the EFI boot partition is located).
>>   (sorry, I haven't tried any other boot loader)
>>
>> - Step to reproduce:
>>   start a VM with 3 disks and EFI enabled (OVMF_CODE), install a Linux
>>   on the 2nd disk (/ & swap) and the EFI boot partition on the first
>>   disk (/boot/efi on "sda1"), cold start the VM.
>>
>> I can help by testing with specific version (or another boot loader),
>> tell me.
>>
>> Regards
>>
>> Aurélien
>>
>> ps: typical command line to start the VM:
>> /usr/bin/kvm \
>>   -id 20 \
>>   -name testvm.mydomain \
>>   -chardev 'socket,id=qmp,path=/var/run/qemu-server/20.qmp,server,nowait' \
>>   -mon 'chardev=qmp,mode=control' \
>>   -pidfile /var/run/qemu-server/20.pid \
>>   -daemonize \
>>   -smbios 'type=1,uuid=88dc4e6b-34b8-46a7-a573-26596c34855d' \
>>   -drive 
>> 'if=pflash,unit=0,format=raw,readonly,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd'
>>  \
>>   -drive 
>> 'if=pflash,unit=1,format=raw,id=drive-efidisk0,file=/mnt/pve/pve-nfs-sas/images/20/vm-20-disk-4.raw'
>>  \
>>   -smp '8,sockets=1,cores=8,maxcpus=8' \
>>   -nodefaults \
>>   -boot 
>> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
>>  \
>>   -vga qxl \
>>   -vnc unix:/var/run/qemu-server/20.vnc,x509,password \
>>   -cpu host,+kvm_pv_unhalt,+kvm_pv_eoi \
>>   -m 512 \
>>   -object 'memory-backend-ram,id=ram-node0,size=512M' \
>>   -numa 'node,nodeid=0,cpus=0-7,memdev=ram-node0' \
>>   -k fr \
>>   -object 'iothread,id=iothread-virtioscsi1' \
>>   -object 'iothread,id=iothread-virtioscsi2' \
>>   -machine 'type=pc-i440fx-2.9' \
>>   -device 'pci-bridge,id=pci.3,chassis_nr=3,bus=pci.0,addr=0x5' \
>>   -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
>>   -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
>>   -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
>>   -chardev 'socket,path=/var/run/qemu-server/20.qga,server,nowait,id=qga0' \
>>   -device 'virtio-serial,id=qga0,bus=pci.0,addr=0x8' \
>>   -device 'virtserialport,chardev=qga0,name=org.qemu.guest_agent.0' \
>>   -spice 
>> 'tls-port=61004,addr=127.0.0.1,tls-ciphers=HIGH,seamless-migration=on' \
>>   -device 'virtio-serial,id=spice,bus=pci.0,addr=0x9' \
>>   -chardev 'spicevmc,id=vdagent,name=vdagent' \
>>   -device 'virtserialport,chardev=vdagent,name=com.redhat.spice.0' \
>>   -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
>>   -iscsi 'initiator-name=iqn.1993-08.org.debian:01:f9e4631950da' \
>>   -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
>>   -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=100' \
>>   -device 'virtio-scsi-pci,id=virtioscsi0,bus=pci.3,addr=0x1' \
>>   -drive 
>> 'file=/mnt/pve/pve-nfs-sas/images/20/vm-20-disk-1.raw,if=none,id=drive-scsi0,format=raw,cache=none,aio=native,detect-zeroes=on'
>>  \
>>   -device 
>> 'scsi-hd,bus=virtioscsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=200'
>>  \
>>   -device 
>> 'virtio-scsi-pci,id=virtioscsi1,bus=pci.3,addr=0x2,iothread=iothread-virtioscsi1'
>>  \
>>   -drive 
>> '

Re: [edk2] [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs

2018-09-17 Thread Laszlo Ersek
On 09/17/18 07:57, Zeng, Star wrote:
> How about we see the problem in another way?
> 
> If my understanding is correct, current discussion and patches think FALSE/0 
> means disable/clear NX, but that is not the fact.
> According to the code implementation, FALSE/0 seems mean *AS IS* to do thing 
> (no code to disable/clear NX).
> 
> PcdSetNxForStack
> TRUE: Set NX for stack.
> FALSE: No code to clear NX for stack.
> 
> PcdDxeNxMemoryProtectionPolicy
> BITX 1: Set NX for that memory type.
> BITX 0: No code to clear NX for that memory type.
> 
> PcdImageProtectionPolicy
> BITX 1: Set NX for the image data section.
> BITX 0: Not code to clear NX for the image data section.
> 
> So, how about we think one PCD just works for itself and it does not impact 
> other PCDs to protect?
> That means TRUE/1 is to protect and FALSE/0 is *AS IS* to do nothing.
> The description of these PCDs could be enhancement if we think it is a good 
> way to see the problem.

Sure, that too could work for me, but then the documentation in the DEC
/ UNI files has to be really clear.

The initial worry for the current discussion was that some platform might
- protect e.g. BootServicesData type memory,
- not set PcdSetNxForStack,
- expect the stack to remain executable.

The actual results might surprise the platform owner.

If the documentation dispelled any possible misconceptions, I think your
idea could work too (and it would be a lot easier to code).

Thanks
Laszlo


> 
> 
> Thanks,
> Star
> -Original Message-
> From: Wang, Jian J 
> Sent: Monday, September 17, 2018 10:11 AM
> To: Laszlo Ersek ; edk2-devel@lists.01.org
> Cc: Zeng, Star ; Ard Biesheuvel 
> ; Ni, Ruiyu ; Yao, Jiewen 
> 
> Subject: RE: [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs
> 
> Laszlo,
> 
> Thanks for the comments.
> 
> Regards,
> Jian
> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Friday, September 14, 2018 5:51 PM
>> To: Wang, Jian J ; edk2-devel@lists.01.org
>> Cc: Zeng, Star ; Ard Biesheuvel 
>> ; Ni, Ruiyu ; Yao, 
>> Jiewen 
>> Subject: Re: [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs
>>
>> I've got some comments on the code as well:
>>
>> On 09/14/18 07:13, Jian J Wang wrote:
>>>  BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116
>>>
>>>  Currently IA32_EFER.NXE is only set against PcdSetNxForStack. This
>>>  confuses developers because following two other PCDs also need NXE
>>>  to be set, but actually not.
>>>
>>>  PcdDxeNxMemoryProtectionPolicy
>>>  PcdImageProtectionPolicy
>>>
>>>  This patch solves this issue by adding logic to enable IA32_EFER.NXE
>>>  if any of those PCDs have anything enabled.
>>>
>>>  Due to the fact that NX memory type of stack (enabled by 
>>> PcdSetNxForStack)
>>>  and image data section (enabled by PcdImageProtectionPolicy) are 
>>> also
>>>  part of PcdDxeNxMemoryProtectionPolicy, this patch also add more 
>>> checks
>>>  to warn (ASSERT) users any unreasonable setting combinations. For 
>>> example,
>>>
>>>  PcdSetNxForStack == FALSE &&
>>>    (PcdDxeNxMemoryProtectionPolicy & (1 <>> != 0
>>>
>>>  PcdImageProtectionPolicy == 0 &&
>>>    (PcdDxeNxMemoryProtectionPolicy & (1 << 
>>> EfiRuntimeServicesData)) != 0
>>>
>>>  PcdImageProtectionPolicy == 0 &&
>>>    (PcdDxeNxMemoryProtectionPolicy & (1 <>> != 0
>>>
>>>  PcdImageProtectionPolicy == 0 &&
>>>    (PcdDxeNxMemoryProtectionPolicy & (1 <>>
>>>  In other words, PcdSetNxForStack and PcdImageProtectionPolicy have
>>>  priority over PcdDxeNxMemoryProtectionPolicy.
>>>
>>>  Cc: Star Zeng 
>>>  Cc: Laszlo Ersek 
>>>  Cc: Ard Biesheuvel 
>>>  Cc: Ruiyu Ni 
>>>  Cc: Jiewen Yao 
>>>  Contributed-under: TianoCore Contribution Agreement 1.1
>>>  Signed-off-by: Jian J Wang 
>>>  ---
>>>   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 +
>>>   MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  4 +-
>>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 55 +
>> ++-
>>>   MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 33 +
>> +
>>>   4 files changed, 91 insertions(+), 3 deletions(-)
>>>
>>>  diff --
>> git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf 
>> b/MdeModulePkg/Core/DxeIp lPeim/DxeIpl.inf
>>>  index fd82657404..068e700074 100644
>>>  --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>>  +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>>  @@ -117,6 +117,8 @@
>>>
>>>   [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
>>> gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack   ## 
>>> SOMETIM
>> ES_CONSUMES
>>>  +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ##
>> SOMETIMES_CONSUMES
>>>  +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy   ## 
>>> SOME
>> TIMES_CONSUMES
>>>
>>>   [Depex]
>>> gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
>>>  diff --
>> git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/ 
>> Core/DxeIplPeim/Ia32/DxeLoadFunc.c
>>>  index 

Re: [edk2] [PATCH V3 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

2018-09-17 Thread Ni, Ruiyu

On 9/17/2018 5:08 PM, Star Zeng wrote:

https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch adds new EfiLocateXXXAcpiTable() APIs in UefiLib
for the request and also the following patch to remove the
duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
  MdePkg/Include/Library/UefiLib.h   |  68 ++
  MdePkg/Library/UefiLib/Acpi.c  | 428 +
  MdePkg/Library/UefiLib/UefiLib.inf |   3 +
  3 files changed, 499 insertions(+)
  create mode 100644 MdePkg/Library/UefiLib/Acpi.c

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index f80067f11103..468bffc3cb66 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
  #ifndef __UEFI_LIB_H__
  #define __UEFI_LIB_H__
  
+#include 

+
  #include 
  #include 
  #include 
@@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
IN UINT64OpenMode,
IN UINT64Attributes
);
+
+/**
+  This function locates next ACPI table in XSDT/RSDT based on Signature and
+  previous returned Table.
+
+  If PreviousTable is NULL:
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  If PreviousTable is not NULL:
+  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in XSDT in
+ gEfiAcpi20TableGuid system configuration table.
+  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi20TableGuid system configuration table.
+  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi10TableGuid system configuration table.
+
+  It's not supported that PreviousTable is not NULL but 
PreviousTable->Signature
+  is not same with Signature, NULL will be returned.
+
+  @param Signature  ACPI table signature.
+  @param PreviousTable  Pointer to previous returned table to locate next
+table, or NULL to locate first table.
+
+  @return Next ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateNextAcpiTable (
+  IN UINT32 Signature,
+  IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
+  );
+
+/**
+  This function locates first ACPI table in XSDT/RSDT based on Signature.
+
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  @param Signature  ACPI table signature.
+
+  @return First ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateFirstAcpiTable (
+  IN UINT32 Signature
+  );
+
  #endif
diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
new file mode 100644
index ..e16ccc7e7b74
--- /dev/null
+++ b/MdePkg/Library/UefiLib/Acpi.c
@@ -0,0 +1,428 @@
+/** @file
+  This module provides help function for finding ACPI table.
+
+  Copyright (c) 2018, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "UefiLibInternal.h"
+#include 
+#include 
+
+/**
+  This function scans ACPI table in XSDT/RSDT.
+
+  @param SdtACPI XSDT/RSDT.
+  @param TablePointerSize   Size of table pointer: 8(XSDT) or 4(RSDT).

Re: [edk2] [PATCH V3 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()

2018-09-17 Thread Ni, Ruiyu

On 9/17/2018 5:08 PM, Star Zeng wrote:

https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch updates PcatRealTimeClockRuntimeDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
  PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80 +-
  1 file changed, 3 insertions(+), 77 deletions(-)

diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c 
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 2105acf35f7b..7965eb8aa55b 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -1203,49 +1203,6 @@ IsWithinOneDay (
  }
  
  /**

-  This function find ACPI table with the specified signature in RSDT or XSDT.
-
-  @param Sdt  ACPI RSDT or XSDT.
-  @param SignatureACPI table signature.
-  @param TablePointerSize Size of table pointer: 4 or 8.
-
-  @return ACPI table or NULL if not found.
-**/
-VOID *
-ScanTableInSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER*Sdt,
-  IN UINT32 Signature,
-  IN UINTN  TablePointerSize
-  )
-{
-  UINTN  Index;
-  UINTN  EntryCount;
-  UINTN  EntryBase;
-  EFI_ACPI_DESCRIPTION_HEADER*Table;
-
-  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
TablePointerSize;
-
-  EntryBase = (UINTN) (Sdt + 1);
-  for (Index = 0; Index < EntryCount; Index++) {
-//
-// When TablePointerSize is 4 while sizeof (VOID *) is 8, make sure the 
upper 4 bytes are zero.
-//
-Table = 0;
-CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize), 
TablePointerSize);
-
-if (Table == NULL) {
-  continue;
-}
-
-if (Table->Signature == Signature) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
Get the century RTC address from the ACPI FADT table.
  
@return  The century RTC address or 0 if not found.

@@ -1255,42 +1212,11 @@ GetCenturyRtcAddress (
VOID
)
  {
-  EFI_STATUSStatus;
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
  
-  Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **) &Rsdp);

-  if (EFI_ERROR (Status)) {
-Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid, (VOID **) 
&Rsdp);
-  }
-
-  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
-return 0;
-  }
-
-  Fadt = NULL;
-
-  //
-  // Find FADT in XSDT
-  //
-  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && 
Rsdp->XsdtAddress != 0) {
-Fadt = ScanTableInSDT (
- (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress,
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (UINTN)
- );
-  }
-
-  //
-  // Find FADT in RSDT
-  //
-  if (Fadt == NULL && Rsdp->RsdtAddress != 0) {
-Fadt = ScanTableInSDT (
- (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress,
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (UINT32)
- );
-  }
+  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLocateFirstAcpiTable 
(
+ 
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
+ );
  
if ((Fadt != NULL) &&

(Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 0x80)


Reviewed-by: Ruiyu Ni 

--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH V3 4/6] PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()

2018-09-17 Thread Star Zeng
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch updates PcatRealTimeClockRuntimeDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c | 80 +-
 1 file changed, 3 insertions(+), 77 deletions(-)

diff --git a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c 
b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
index 2105acf35f7b..7965eb8aa55b 100644
--- a/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
+++ b/PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c
@@ -1203,49 +1203,6 @@ IsWithinOneDay (
 }
 
 /**
-  This function find ACPI table with the specified signature in RSDT or XSDT.
-
-  @param Sdt  ACPI RSDT or XSDT.
-  @param SignatureACPI table signature.
-  @param TablePointerSize Size of table pointer: 4 or 8.
-
-  @return ACPI table or NULL if not found.
-**/
-VOID *
-ScanTableInSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER*Sdt,
-  IN UINT32 Signature,
-  IN UINTN  TablePointerSize
-  )
-{
-  UINTN  Index;
-  UINTN  EntryCount;
-  UINTN  EntryBase;
-  EFI_ACPI_DESCRIPTION_HEADER*Table;
-
-  EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
TablePointerSize;
-
-  EntryBase = (UINTN) (Sdt + 1);
-  for (Index = 0; Index < EntryCount; Index++) {
-//
-// When TablePointerSize is 4 while sizeof (VOID *) is 8, make sure the 
upper 4 bytes are zero.
-//
-Table = 0;
-CopyMem (&Table, (VOID *) (EntryBase + Index * TablePointerSize), 
TablePointerSize);
-
-if (Table == NULL) {
-  continue;
-}
-
-if (Table->Signature == Signature) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
   Get the century RTC address from the ACPI FADT table.
 
   @return  The century RTC address or 0 if not found.
@@ -1255,42 +1212,11 @@ GetCenturyRtcAddress (
   VOID
   )
 {
-  EFI_STATUSStatus;
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
   EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
 
-  Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **) 
&Rsdp);
-  if (EFI_ERROR (Status)) {
-Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid, (VOID **) 
&Rsdp);
-  }
-
-  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
-return 0;
-  }
-
-  Fadt = NULL;
-
-  //
-  // Find FADT in XSDT
-  //
-  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION 
&& Rsdp->XsdtAddress != 0) {
-Fadt = ScanTableInSDT (
- (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress,
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (UINTN)
- );
-  }
-
-  //
-  // Find FADT in RSDT
-  //
-  if (Fadt == NULL && Rsdp->RsdtAddress != 0) {
-Fadt = ScanTableInSDT (
- (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress,
- EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
- sizeof (UINT32)
- );
-  }
+  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLocateFirstAcpiTable 
(
+ 
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE
+ );
 
   if ((Fadt != NULL) &&
   (Fadt->Century > RTC_ADDRESS_REGISTER_D) && (Fadt->Century < 0x80)
-- 
2.7.0.windows.1

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


[edk2] [PATCH V3 3/6] MdeModulePkg S3SaveStateDxe: Use new EfiLocateFirstAcpiTable()

2018-09-17 Thread Star Zeng
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch updates S3SaveStateDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../Acpi/S3SaveStateDxe/AcpiS3ContextSave.c| 208 +
 .../Acpi/S3SaveStateDxe/S3SaveStateDxe.inf |   3 +-
 2 files changed, 5 insertions(+), 206 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c 
b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
index 3f99023f110f..fadafd2b608b 100644
--- a/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
+++ b/MdeModulePkg/Universal/Acpi/S3SaveStateDxe/AcpiS3ContextSave.c
@@ -22,8 +22,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
-#include 
 #include 
 #include 
 
@@ -76,208 +76,6 @@ AllocateMemoryBelow4G (
 }
 
 /**
-
-  This function scan ACPI table in RSDT.
-
-  @param Rsdt  ACPI RSDT
-  @param Signature ACPI table signature
-
-  @return ACPI table
-
-**/
-VOID *
-ScanTableInRSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER*Rsdt,
-  IN UINT32 Signature
-  )
-{
-  UINTN  Index;
-  UINT32 EntryCount;
-  UINT32 *EntryPtr;
-  EFI_ACPI_DESCRIPTION_HEADER*Table;
-
-  if (Rsdt == NULL) {
-return NULL;
-  }
-
-  EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
sizeof(UINT32);
-
-  EntryPtr = (UINT32 *)(Rsdt + 1);
-  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
-Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(*EntryPtr));
-if (Table->Signature == Signature) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
-
-  This function scan ACPI table in XSDT.
-
-  @param Xsdt  ACPI XSDT
-  @param Signature ACPI table signature
-
-  @return ACPI table
-
-**/
-VOID *
-ScanTableInXSDT (
-  IN EFI_ACPI_DESCRIPTION_HEADER*Xsdt,
-  IN UINT32 Signature
-  )
-{
-  UINTN  Index;
-  UINT32 EntryCount;
-  UINT64 EntryPtr;
-  UINTN  BasePtr;
-  EFI_ACPI_DESCRIPTION_HEADER*Table;
-
-  if (Xsdt == NULL) {
-return NULL;
-  }
-
-  EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
sizeof(UINT64);
-
-  BasePtr = (UINTN)(Xsdt + 1);
-  for (Index = 0; Index < EntryCount; Index ++) {
-CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), 
sizeof(UINT64));
-Table = (EFI_ACPI_DESCRIPTION_HEADER *)((UINTN)(EntryPtr));
-if (Table->Signature == Signature) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
-  To find Facs in FADT.
-
-  @param Fadt   FADT table pointer
-
-  @return  Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *
-FindAcpiFacsFromFadt (
-  IN EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt
-  )
-{
-  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
-  UINT64Data64;
-
-  if (Fadt == NULL) {
-return NULL;
-  }
-
-  if (Fadt->Header.Revision < 
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
-Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE 
*)(UINTN)Fadt->FirmwareCtrl;
-  } else {
-if (Fadt->FirmwareCtrl != 0) {
-  Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE 
*)(UINTN)Fadt->FirmwareCtrl;
-} else {
-  CopyMem (&Data64, &Fadt->XFirmwareCtrl, sizeof(UINT64));
-  Facs = (EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)Data64;
-}
-  }
-  return Facs;
-}
-
-/**
-  To find Facs in Acpi tables.
-
-  To find Firmware ACPI control strutcure in Acpi Tables since the S3 waking 
vector is stored
-  in the table.
-
-  @param AcpiTableGuid   The guid used to find ACPI table in UEFI 
ConfigurationTable.
-
-  @return  Facs table pointer.
-**/
-EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *
-FindAcpiFacsTableByAcpiGuid (
-  IN EFI_GUID  *AcpiTableGuid
-  )
-{
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
-  EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
-  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt;
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-  EFI_ACPI_2_0_FIRMWARE_ACPI_CONTROL_STRUCTURE  *Facs;
-  UINTN Index;
-
-  Rsdp  = NULL;
-  //
-  // found ACPI table RSD_PTR from system table
-  //
-  for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
-if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid),

[edk2] [PATCH V3 2/6] IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable()

2018-09-17 Thread Star Zeng
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch updates IntelVTdDxe to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c| 136 +
 1 file changed, 3 insertions(+), 133 deletions(-)

diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c 
b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 24723fe4972a..92b09f985533 100644
--- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
@@ -868,116 +868,6 @@ ParseDmarAcpiTableRmrr (
 }
 
 /**
-  This function scan ACPI table in RSDT.
-
-  @param[in]  Rsdt  ACPI RSDT
-  @param[in]  Signature ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInRSDT (
-  IN RSDT_TABLE   *Rsdt,
-  IN UINT32   Signature
-  )
-{
-  UINTN Index;
-  UINT32EntryCount;
-  UINT32*EntryPtr;
-  EFI_ACPI_DESCRIPTION_HEADER   *Table;
-
-  EntryCount = (Rsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
sizeof(UINT32);
-
-  EntryPtr = &Rsdt->Entry;
-  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
-Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
-if ((Table != NULL) && (Table->Signature == Signature)) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in XSDT.
-
-  @param[in]  Xsdt  ACPI XSDT
-  @param[in]  Signature ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInXSDT (
-  IN XSDT_TABLE   *Xsdt,
-  IN UINT32   Signature
-  )
-{
-  UINTNIndex;
-  UINT32   EntryCount;
-  UINT64   EntryPtr;
-  UINTNBasePtr;
-  EFI_ACPI_DESCRIPTION_HEADER  *Table;
-
-  EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
sizeof(UINT64);
-
-  BasePtr = (UINTN)(&(Xsdt->Entry));
-  for (Index = 0; Index < EntryCount; Index ++) {
-CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), 
sizeof(UINT64));
-Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
-if ((Table != NULL) && (Table->Signature == Signature)) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in RSDP.
-
-  @param[in]  Rsdp  ACPI RSDP
-  @param[in]  Signature ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-FindAcpiPtr (
-  IN EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
-  IN UINT32   Signature
-  )
-{
-  EFI_ACPI_DESCRIPTION_HEADER*AcpiTable;
-  RSDT_TABLE *Rsdt;
-  XSDT_TABLE *Xsdt;
-
-  AcpiTable = NULL;
-
-  //
-  // Check ACPI2.0 table
-  //
-  Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
-  Xsdt = NULL;
-  if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
-Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
-  }
-  //
-  // Check Xsdt
-  //
-  if (Xsdt != NULL) {
-AcpiTable = ScanTableInXSDT (Xsdt, Signature);
-  }
-  //
-  // Check Rsdt
-  //
-  if ((AcpiTable == NULL) && (Rsdt != NULL)) {
-AcpiTable = ScanTableInRSDT (Rsdt, Signature);
-  }
-
-  return AcpiTable;
-}
-
-/**
   Get the DMAR ACPI table.
 
   @retval EFI_SUCCESS   The DMAR ACPI table is got.
@@ -989,33 +879,13 @@ GetDmarAcpiTable (
   VOID
   )
 {
-  VOID  *AcpiTable;
-  EFI_STATUSStatus;
-
   if (mAcpiDmarTable != NULL) {
 return EFI_ALREADY_STARTED;
   }
 
-  AcpiTable = NULL;
-  Status = EfiGetSystemConfigurationTable (
- &gEfiAcpi20TableGuid,
- &AcpiTable
- );
-  if (EFI_ERROR (Status)) {
-Status = EfiGetSystemConfigurationTable (
-   &gEfiAcpi10TableGuid,
-   &AcpiTable
-   );
-  }
-  if (EFI_ERROR (Status)) {
-return EFI_NOT_FOUND;
-  }
-  ASSERT (AcpiTable != NULL);
-
-  mAcpiDmarTable = FindAcpiPtr (
-  (EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER 
*)AcpiTable,
-  EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
-  );
+  mAcpiDmarTable = (EFI_ACPI_DMAR_HEADER *) EfiLocateFirstAcpiTable (
+  
EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE
+  );
   if (mAcpiDmarTable == NULL) {
 return EFI_NOT_FOUND

[edk2] [PATCH V3 5/6] ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()

2018-09-17 Thread Star Zeng
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch updates DpDynamicCommand to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Dandan Bi 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Ruiyu Ni 
---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c  | 136 +
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h  |   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni|   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf |   2 -
 .../DpDynamicCommand/DpDynamicCommand.inf  |   2 -
 5 files changed, 3 insertions(+), 139 deletions(-)

diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c 
b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index 2c094b63c94a..c14931055cbf 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -118,116 +118,6 @@ DumpStatistics( void )
 }
 
 /**
-  This function scan ACPI table in RSDT.
-
-  @param  RsdtACPI RSDT
-  @param  Signature   ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInRSDT (
-  IN RSDT_TABLE   *Rsdt,
-  IN UINT32   Signature
-  )
-{
-  UINTN Index;
-  UINT32EntryCount;
-  UINT32*EntryPtr;
-  EFI_ACPI_DESCRIPTION_HEADER   *Table;
-
-  EntryCount = (Rsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
sizeof(UINT32);
-
-  EntryPtr = &Rsdt->Entry;
-  for (Index = 0; Index < EntryCount; Index ++, EntryPtr ++) {
-Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(*EntryPtr));
-if (Table->Signature == Signature) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in XSDT.
-
-  @param  Xsdt   ACPI XSDT
-  @param  Signature  ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-ScanTableInXSDT (
-  IN XSDT_TABLE   *Xsdt,
-  IN UINT32   Signature
-  )
-{
-  UINTNIndex;
-  UINT32   EntryCount;
-  UINT64   EntryPtr;
-  UINTNBasePtr;
-  EFI_ACPI_DESCRIPTION_HEADER  *Table;
-
-  EntryCount = (Xsdt->Header.Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / 
sizeof(UINT64);
-
-  BasePtr = (UINTN)(&(Xsdt->Entry));
-  for (Index = 0; Index < EntryCount; Index ++) {
-CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * sizeof(UINT64)), 
sizeof(UINT64));
-Table = (EFI_ACPI_DESCRIPTION_HEADER*)((UINTN)(EntryPtr));
-if (Table->Signature == Signature) {
-  return Table;
-}
-  }
-
-  return NULL;
-}
-
-/**
-  This function scan ACPI table in RSDP.
-
-  @param  Rsdp   ACPI RSDP
-  @param  Signature  ACPI table signature
-
-  @return ACPI table
-**/
-VOID *
-FindAcpiPtr (
-  IN EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp,
-  IN UINT32   Signature
-  )
-{
-  EFI_ACPI_DESCRIPTION_HEADER*AcpiTable;
-  RSDT_TABLE *Rsdt;
-  XSDT_TABLE *Xsdt;
-
-  AcpiTable = NULL;
-
-  //
-  // Check ACPI2.0 table
-  //
-  Rsdt = (RSDT_TABLE *)(UINTN)Rsdp->RsdtAddress;
-  Xsdt = NULL;
-  if ((Rsdp->Revision >= 2) && (Rsdp->XsdtAddress < (UINT64)(UINTN)-1)) {
-Xsdt = (XSDT_TABLE *)(UINTN)Rsdp->XsdtAddress;
-  }
-  //
-  // Check Xsdt
-  //
-  if (Xsdt != NULL) {
-AcpiTable = ScanTableInXSDT (Xsdt, Signature);
-  }
-  //
-  // Check Rsdt
-  //
-  if ((AcpiTable == NULL) && (Rsdt != NULL)) {
-AcpiTable = ScanTableInRSDT (Rsdt, Signature);
-  }
-
-  return AcpiTable;
-}
-
-/**
   Get Boot performance table form Acpi table.
 
 **/
@@ -235,31 +125,11 @@ EFI_STATUS
 GetBootPerformanceTable (
   )
 {
-  EFI_STATUS  Status;
-  VOID*AcpiTable;
   FIRMWARE_PERFORMANCE_TABLE  *FirmwarePerformanceTable;
 
-  AcpiTable = NULL;
-
-  Status = EfiGetSystemConfigurationTable (
- &gEfiAcpi20TableGuid,
- &AcpiTable
- );
-  if (EFI_ERROR (Status)) {
-Status = EfiGetSystemConfigurationTable (
-   &gEfiAcpi10TableGuid,
-   &AcpiTable
- );
-  }
-  if (EFI_ERROR(Status) || AcpiTable == NULL) {
-ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_DP_GET_ACPI_TABLE_FAIL), 
mDpHiiHandle);
-return Status;
-  }
-
-  FirmwarePerformanceTable = FindAcpiPtr (
-  (EFI_ACPI_5_0_ROOT_SYSTEM_DESCRIPTION_POINTER 
*)AcpiTable,
-  EFI_ACPI_5_0_FIRMWARE_PERFORMANCE_DATA_TABLE_SIGNATURE
-  );
+  FirmwarePerformance

[edk2] [PATCH V3 0/6] Add new EfiLocateXXXAcpiTable() APIs

2018-09-17 Thread Star Zeng
It is the V3 patch series of
https://lists.01.org/pipermail/edk2-devel/2018-August/029214.html
It is according to the discussion at
https://lists.01.org/pipermail/edk2-devel/2018-September/029750.html

V3:
1. Pick Ray/Laszlo/Eric's RB.
2. Remove ASSERT about Signature check.
3. Merge ScanTableInRSDT and ScanTableInXSDT.

It is the V2 patch series of
https://lists.01.org/pipermail/edk2-devel/2018-August/029214.html
It is according to the discussion at
https://lists.01.org/pipermail/edk2-devel/2018-September/029348.html

V2:
1. Add EfiLocateFirstAcpiTable() and EfiLocateNextAcpiTable() instead
   of EfiFindAcpiTableBySignature() to support locating both single
   ACPI table instance and multiple ACPI table instances cases.
2. Support locating DSDT.
3. Support locating multiple ACPI table instances case by
   EfiLocateNextAcpiTable().

Test done:
1. Call EfiLocateFirstAcpiTable() before ACPI configuration table is
   installed, NULL is returned.
2. Call EfiLocateFirstAcpiTable() to locate FACS after FACS is installed
   but FADT is not installed, NULL is returned.
3. Call EfiLocateFirstAcpiTable() to locate FADT/DSDT/FACS/FPDT/DMAR
   at late phase, correct ACPI table pointer is returned.
4. Call EfiLocateNextAcpiTable() to locate SSDTs at late phase, all
   SSDTs are returned correctly.
5. Run same test cases above after setting PcdAcpiExposedTableVersions
   to 0x2, same results are with above.
6. Run same test cases above with 32Bits build, same results are with
   above.

The code for this patch series is also at
g...@github.com:lzeng14/edk2.git branch LocateAcpiTable_UefiLibV3

https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch adds new EfiLocateXXXAcpiTable() API in UefiLib
for the request and removing the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Ruiyu Ni 
Cc: Dandan Bi 
Cc: Eric Dong 
Cc: Laszlo Ersek 

Star Zeng (6):
  MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs
  IntelSiliconPkg IntelVTdDxe: Use new EfiLocateFirstAcpiTable()
  MdeModulePkg S3SaveStateDxe: Use new EfiLocateFirstAcpiTable()
  PcAtChipsetPkg PcRtc: Use new EfiLocateFirstAcpiTable()
  ShellPkg DpDynamicCommand: Use new EfiLocateFirstAcpiTable()
  UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()

 .../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c| 136 +--
 .../Acpi/S3SaveStateDxe/AcpiS3ContextSave.c| 208 +-
 .../Acpi/S3SaveStateDxe/S3SaveStateDxe.inf |   3 +-
 MdePkg/Include/Library/UefiLib.h   |  68 
 MdePkg/Library/UefiLib/Acpi.c  | 428 +
 MdePkg/Library/UefiLib/UefiLib.inf |   3 +
 PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcRtc.c |  80 +---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c  | 136 +--
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.h  |   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.uni|   1 -
 ShellPkg/DynamicCommand/DpDynamicCommand/DpApp.inf |   2 -
 .../DpDynamicCommand/DpDynamicCommand.inf  |   2 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |  84 +---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |   3 +-
 15 files changed, 519 insertions(+), 640 deletions(-)
 create mode 100644 MdePkg/Library/UefiLib/Acpi.c

-- 
2.7.0.windows.1

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


[edk2] [PATCH V3 6/6] UefiCpuPkg PiSmmCpuDxeSmm: Use new EfiLocateFirstAcpiTable()

2018-09-17 Thread Star Zeng
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch updates PiSmmCpuDxeSmm to use new
EfiLocateFirstAcpiTable() and remove the duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
Reviewed-by: Eric Dong 
Reviewed-by: Laszlo Ersek 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |  4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 84 ++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h |  3 +-
 3 files changed, 6 insertions(+), 85 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index a7fb7b0b1482..0fdc1b134ea3 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -4,7 +4,7 @@
 # This SMM driver performs SMM initialization, deploy SMM Entry Vector,
 # provides CPU specific services in SMM.
 #
-# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
 # Copyright (c) 2017, AMD Incorporated. All rights reserved.
 #
 # This program and the accompanying materials
@@ -114,8 +114,6 @@ [Protocols]
 [Guids]
   gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it 
is used for S3 boot.
   gEfiGlobalVariableGuid   ## SOMETIMES_PRODUCES ## 
Variable:L"SmmProfileData"
-  gEfiAcpi20TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
-  gEfiAcpi10TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
   gEfiMemoryAttributesTableGuid## CONSUMES ## SystemTable
 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
index a743cf64f907..91b8e7ddb991 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c
@@ -1,7 +1,7 @@
 /** @file
 Enable SMM profile.
 
-Copyright (c) 2012 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
 Copyright (c) 2017, AMD Incorporated. All rights reserved.
 
 This program and the accompanying materials
@@ -726,84 +726,6 @@ InitPaging (
 }
 
 /**
-  To find FADT in ACPI tables.
-
-  @param AcpiTableGuid   The GUID used to find ACPI table in UEFI 
ConfigurationTable.
-
-  @return  FADT table pointer.
-**/
-EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *
-FindAcpiFadtTableByAcpiGuid (
-  IN EFI_GUID  *AcpiTableGuid
-  )
-{
-  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
-  EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-  UINTN Index;
-  UINT32Data32;
-  Rsdp  = NULL;
-  Rsdt  = NULL;
-  Fadt  = NULL;
-  //
-  // found ACPI table RSD_PTR from system table
-  //
-  for (Index = 0; Index < gST->NumberOfTableEntries; Index++) {
-if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
AcpiTableGuid)) {
-  //
-  // A match was found.
-  //
-  Rsdp = gST->ConfigurationTable[Index].VendorTable;
-  break;
-}
-  }
-
-  if (Rsdp == NULL) {
-return NULL;
-  }
-
-  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
-  if (Rsdt == NULL || Rsdt->Signature != 
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
-return NULL;
-  }
-
-  for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt->Length; 
Index = Index + sizeof (UINT32)) {
-
-Data32  = *(UINT32 *) ((UINT8 *) Rsdt + Index);
-Fadt= (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *) (UINTN) 
Data32;
-if (Fadt->Header.Signature == 
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
-  break;
-}
-  }
-
-  if (Fadt == NULL || Fadt->Header.Signature != 
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
-return NULL;
-  }
-
-  return Fadt;
-}
-
-/**
-  To find FADT in ACPI tables.
-
-  @return  FADT table pointer.
-**/
-EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE  *
-FindAcpiFadtTable (
-  VOID
-  )
-{
-  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
-
-  Fadt = FindAcpiFadtTableByAcpiGuid (&gEfiAcpi20TableGuid);
-  if (Fadt != NULL) {
-return Fadt;
-  }
-
-  return FindAcpiFadtTableByAcpiGuid (&gEfiAcpi10TableGuid);
-}
-
-/**
   To get system port address of the SMI Command Port in FADT table.
 
 **/
@@ -814,7 +736,9 @@ GetSmiCommandPort (
 {
   EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
 
-  Fadt = FindAcpiFadtTable ();
+  Fadt = (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) EfiLoca

[edk2] [PATCH V3 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

2018-09-17 Thread Star Zeng
https://bugzilla.tianocore.org/show_bug.cgi?id=967
Request to add a library function for GetAcpiTable() in order
to get ACPI table using signature as input.

After evaluation, we found there are many duplicated code to
find ACPI table by signature in different modules.

This patch adds new EfiLocateXXXAcpiTable() APIs in UefiLib
for the request and also the following patch to remove the
duplicated code.

Cc: Younas khan 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdePkg/Include/Library/UefiLib.h   |  68 ++
 MdePkg/Library/UefiLib/Acpi.c  | 428 +
 MdePkg/Library/UefiLib/UefiLib.inf |   3 +
 3 files changed, 499 insertions(+)
 create mode 100644 MdePkg/Library/UefiLib/Acpi.c

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index f80067f11103..468bffc3cb66 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef __UEFI_LIB_H__
 #define __UEFI_LIB_H__
 
+#include 
+
 #include 
 #include 
 #include 
@@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
   IN UINT64OpenMode,
   IN UINT64Attributes
   );
+
+/**
+  This function locates next ACPI table in XSDT/RSDT based on Signature and
+  previous returned Table.
+
+  If PreviousTable is NULL:
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  If PreviousTable is not NULL:
+  1. If it could be located in XSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in XSDT in
+ gEfiAcpi20TableGuid system configuration table.
+  2. If it could be located in RSDT in gEfiAcpi20TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi20TableGuid system configuration table.
+  3. If it could be located in RSDT in gEfiAcpi10TableGuid system configuration
+ table, then this function will just locate next table in RSDT in
+ gEfiAcpi10TableGuid system configuration table.
+
+  It's not supported that PreviousTable is not NULL but 
PreviousTable->Signature
+  is not same with Signature, NULL will be returned.
+
+  @param Signature  ACPI table signature.
+  @param PreviousTable  Pointer to previous returned table to locate next
+table, or NULL to locate first table.
+
+  @return Next ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateNextAcpiTable (
+  IN UINT32 Signature,
+  IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
+  );
+
+/**
+  This function locates first ACPI table in XSDT/RSDT based on Signature.
+
+  This function will locate the first ACPI table in XSDT/RSDT based on
+  Signature in gEfiAcpi20TableGuid system configuration table first, and then
+  gEfiAcpi10TableGuid system configuration table.
+  This function will locate in XSDT first, and then RSDT.
+  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
+  FADT.
+  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
+  FirmwareCtrl in FADT.
+
+  @param Signature  ACPI table signature.
+
+  @return First ACPI table or NULL if not found.
+
+**/
+EFI_ACPI_COMMON_HEADER *
+EFIAPI
+EfiLocateFirstAcpiTable (
+  IN UINT32 Signature
+  );
+
 #endif
diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
new file mode 100644
index ..e16ccc7e7b74
--- /dev/null
+++ b/MdePkg/Library/UefiLib/Acpi.c
@@ -0,0 +1,428 @@
+/** @file
+  This module provides help function for finding ACPI table.
+
+  Copyright (c) 2018, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "UefiLibInternal.h"
+#include 
+#include 
+
+/**
+  This function scans ACPI table in XSDT/RSDT.
+
+  @param SdtACPI XSDT/RSDT.
+  @param TablePointerSize   Size of table pointer: 8(XSDT) or 4(RSDT).
+  @param Signature  ACPI table signature.
+

Re: [edk2] bootloader only see first disk

2018-09-17 Thread Thomas Lamprecht
Hi Laszlo,

On 9/16/18 12:04 PM, Laszlo Ersek wrote:
> Hello Aurélien,
> 
> adding Thomas to the CC list, and commenting at the bottom:

thanks for CC'ing and thanks to you Aurélien for writing your report.

> 
> On 09/15/18 18:36, Aurélien wrote:
>> Hi,
>>
>> I don't know if it is the right place to report a bug, sorry if not.
>>
>> I'm a Promox user and the last version (5.2) has a package versionned
>> 20180612-1 actually  (
>> https://git.proxmox.com/?p=pve-edk2-firmware.git;a=summary ) which is
>> based on 5a56c0493955cf55e7eef96dbba815cfbb067d7d  commit of edk2. A
>> build from the current master a few days ago has the same problem. The
>> previous Promox version (5.1 whith this pve-qemu-kvm package) use a
>> edk2 version dating from 2017 or before (it could be from before
>> September 2016 but I'm not sure), which has no problem. I tested on my
>> workstation running gentoo which has a version based on UDK2017 branch
>> with source code from February 11, 2018 and it's working too. So I
>> think the problem is located in  the OVMF_CODE.fd code from 2018 (but
>> without any certainty).
>>
>> Actual behaviour (with all the various Linux distribution: Debian 9,
>> Ubuntu 18.04, Centos 7.5 ...) with a VM which has EFI enabled:
>> - grub is loaded and only see the first disk (where it is installed),
>>   the "ls" command in the grub shell return: "(hd0) (hd0,gpt1)
>>   (cd0)"   and  there is also hd1 and hd2 and in my case this is
>>   problematic as /boot is on hd1.
>> - when pressing "escape" on the boot to go the the setup screen and
>>   just chose "continue" it's ok (grub sees all the disks), my current
>>   workaround to boot the installed OS.
>> - when installing an OS like Ubuntu, CentOS ... on the first reboot
>>   it's ok (I thinks something in the installation, like seting a new
>>   boot entry with grub-install, efibootmgr)
>>
>> Expected behaviour:
>> - grub see all the disk (the ls command return hd0, hd1 hd2 ) and
>>   not the first one (where the EFI boot partition is located).
>>   (sorry, I haven't tried any other boot loader)
>>
>> - Step to reproduce:
>>   start a VM with 3 disks and EFI enabled (OVMF_CODE), install a Linux
>>   on the 2nd disk (/ & swap) and the EFI boot partition on the first
>>   disk (/boot/efi on "sda1"), cold start the VM.
>>
>> I can help by testing with specific version (or another boot loader),
>> tell me.
>>
>> Regards
>>
>> Aurélien
>>
>> ps: typical command line to start the VM:
>> /usr/bin/kvm \
>>   -id 20 \
>>   -name testvm.mydomain \
>>   -chardev 'socket,id=qmp,path=/var/run/qemu-server/20.qmp,server,nowait' \
>>   -mon 'chardev=qmp,mode=control' \
>>   -pidfile /var/run/qemu-server/20.pid \
>>   -daemonize \
>>   -smbios 'type=1,uuid=88dc4e6b-34b8-46a7-a573-26596c34855d' \
>>   -drive 
>> 'if=pflash,unit=0,format=raw,readonly,file=/usr/share/pve-edk2-firmware//OVMF_CODE.fd'
>>  \
>>   -drive 
>> 'if=pflash,unit=1,format=raw,id=drive-efidisk0,file=/mnt/pve/pve-nfs-sas/images/20/vm-20-disk-4.raw'
>>  \
>>   -smp '8,sockets=1,cores=8,maxcpus=8' \
>>   -nodefaults \
>>   -boot 
>> 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg'
>>  \
>>   -vga qxl \
>>   -vnc unix:/var/run/qemu-server/20.vnc,x509,password \
>>   -cpu host,+kvm_pv_unhalt,+kvm_pv_eoi \
>>   -m 512 \
>>   -object 'memory-backend-ram,id=ram-node0,size=512M' \
>>   -numa 'node,nodeid=0,cpus=0-7,memdev=ram-node0' \
>>   -k fr \
>>   -object 'iothread,id=iothread-virtioscsi1' \
>>   -object 'iothread,id=iothread-virtioscsi2' \
>>   -machine 'type=pc-i440fx-2.9' \
>>   -device 'pci-bridge,id=pci.3,chassis_nr=3,bus=pci.0,addr=0x5' \
>>   -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
>>   -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
>>   -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
>>   -chardev 'socket,path=/var/run/qemu-server/20.qga,server,nowait,id=qga0' \
>>   -device 'virtio-serial,id=qga0,bus=pci.0,addr=0x8' \
>>   -device 'virtserialport,chardev=qga0,name=org.qemu.guest_agent.0' \
>>   -spice 
>> 'tls-port=61004,addr=127.0.0.1,tls-ciphers=HIGH,seamless-migration=on' \
>>   -device 'virtio-serial,id=spice,bus=pci.0,addr=0x9' \
>>   -chardev 'spicevmc,id=vdagent,name=vdagent' \
>>   -device 'virtserialport,chardev=vdagent,name=com.redhat.spice.0' \
>>   -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3' \
>>   -iscsi 'initiator-name=iqn.1993-08.org.debian:01:f9e4631950da' \
>>   -drive 'if=none,id=drive-ide2,media=cdrom,aio=threads' \
>>   -device 'ide-cd,bus=ide.1,unit=0,drive=drive-ide2,id=ide2,bootindex=100' \
>>   -device 'virtio-scsi-pci,id=virtioscsi0,bus=pci.3,addr=0x1' \
>>   -drive 
>> 'file=/mnt/pve/pve-nfs-sas/images/20/vm-20-disk-1.raw,if=none,id=drive-scsi0,format=raw,cache=none,aio=native,detect-zeroes=on'
>>  \
>>   -device 
>> 'scsi-hd,bus=virtioscsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0,id=scsi0,bootindex=200'
>>  \
>>   -device 
>> 'virtio-scsi-pci,id=virtioscsi1,b

[edk2] [Patch] BaseTools: remove the not used PyUtility file

2018-09-17 Thread Yonghong Zhu
the PyUtility is not used, so we remove it.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/C/PyUtility/Makefile|  25 ---
 BaseTools/Source/C/PyUtility/PyUtility.c | 106 ---
 BaseTools/Source/C/PyUtility/setup.py|  42 ---
 BaseTools/Source/Python/Common/Misc.py   |  16 +---
 BaseTools/Source/Python/Common/PyUtility.pyd | Bin 6144 -> 0 bytes
 5 files changed, 3 insertions(+), 186 deletions(-)
 delete mode 100644 BaseTools/Source/C/PyUtility/Makefile
 delete mode 100644 BaseTools/Source/C/PyUtility/PyUtility.c
 delete mode 100644 BaseTools/Source/C/PyUtility/setup.py
 delete mode 100644 BaseTools/Source/Python/Common/PyUtility.pyd

diff --git a/BaseTools/Source/C/PyUtility/Makefile 
b/BaseTools/Source/C/PyUtility/Makefile
deleted file mode 100644
index 5829070..000
--- a/BaseTools/Source/C/PyUtility/Makefile
+++ /dev/null
@@ -1,25 +0,0 @@
-## @file
-# Makefile
-#
-# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
-# This program and the accompanying materials
-# are licensed and made available under the terms and conditions of the BSD 
License
-# which accompanies this distribution.The full text of the license may be 
found at
-# http://opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-
-!INCLUDE ..\Makefiles\ms.common
-
-APPNAME = GenSec
-
-LIBS = $(LIB_PATH)\Common.lib
-
-OBJECTS = PyUtility.obj
-
-#CFLAGS = $(CFLAGS) /nodefaultlib:libc.lib
-
-!INCLUDE ..\Makefiles\ms.app
-
diff --git a/BaseTools/Source/C/PyUtility/PyUtility.c 
b/BaseTools/Source/C/PyUtility/PyUtility.c
deleted file mode 100644
index d14b872..000
--- a/BaseTools/Source/C/PyUtility/PyUtility.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/** @file
-Python Utility
-
-Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
-This program and the accompanying materials are licensed and made available
-under the terms and conditions of the BSD License which accompanies this
-distribution.  The full text of the license may be found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include 
-#include 
-#include 
-
-/*
- SaveFileToDisk(FilePath, Content)
-*/
-STATIC
-PyObject*
-SaveFileToDisk (
-  PyObject*Self,
-  PyObject*Args
-  )
-{
-  CHAR8 *File;
-  UINT8 *Data;
-  UINTN DataLength;
-  UINTN WriteBytes;
-  UINTN Status;
-  HANDLEFileHandle;
-  PyObject  *ReturnValue = Py_False;
-
-  Status = PyArg_ParseTuple(
-Args,
-"ss#",
-&File,
-&Data,
-&DataLength
-);
-  if (Status == 0) {
-return NULL;
-  }
-
-  FileHandle = CreateFile(
-File,
-GENERIC_WRITE,
-FILE_SHARE_WRITE|FILE_SHARE_READ|FILE_SHARE_DELETE,
-NULL,
-CREATE_ALWAYS,
-FILE_ATTRIBUTE_NORMAL,
-NULL
-);
-  if (FileHandle == INVALID_HANDLE_VALUE) {
-PyErr_SetString(PyExc_Exception, "File creation failure");
-return NULL;
-  }
-
-  while (WriteFile(FileHandle, Data, DataLength, &WriteBytes, NULL)) {
-if (DataLength <= WriteBytes) {
-  DataLength = 0;
-  break;
-}
-
-Data += WriteBytes;
-DataLength -= WriteBytes;
-  }
-
-  if (DataLength != 0) {
-// file saved unsuccessfully
-PyErr_SetString(PyExc_Exception, "File write failure");
-goto Done;
-  }
-
-  //
-  // Flush buffer may slow down the whole build performance (average 10s 
slower)
-  //
-  //if (!FlushFileBuffers(FileHandle)) {
-  //  PyErr_SetString(PyExc_Exception, "File flush failure");
-  //  goto Done;
-  //}
-
-  // success!
-  ReturnValue = Py_True;
-
-Done:
-  CloseHandle(FileHandle);
-  return ReturnValue;
-}
-
-STATIC INT8 SaveFileToDiskDocs[] = "SaveFileToDisk(): Make sure the file is 
saved to disk\n";
-
-STATIC PyMethodDef PyUtility_Funcs[] = {
-  {"SaveFileToDisk", (PyCFunction)SaveFileToDisk, METH_VARARGS, 
SaveFileToDiskDocs},
-  {NULL, NULL, 0, NULL}
-};
-
-PyMODINIT_FUNC
-initPyUtility(VOID) {
-  Py_InitModule3("PyUtility", PyUtility_Funcs, "Utilties Module Implemented C 
Language");
-}
-
-
diff --git a/BaseTools/Source/C/PyUtility/setup.py 
b/BaseTools/Source/C/PyUtility/setup.py
deleted file mode 100644
index e4d407d..000
--- a/BaseTools/Source/C/PyUtility/setup.py
+++ /dev/null
@@ -1,42 +0,0 @@
-## @file
-# package and install PyEfiCompressor extension
-#
-#  Copyright (c) 2008, Intel Corporation. All rights reserved.
-#
-#  This program and the accompanying materials
-#  are licensed and made available under the terms and conditions of t

Re: [edk2] [Patch] BaseTools: remove PyUtility and open the file with unbuffered

2018-09-17 Thread Zhu, Yonghong
Please ignore this one, I will separate it into two patches.

Best Regards,
Zhu Yonghong


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Monday, September 17, 2018 2:43 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [edk2] [Patch] BaseTools: remove PyUtility and open the file with 
unbuffered

the PyUtility is not used, so we remove it.
And update the open file with unbuffered to avoid the case that the file is not 
wrote but directly used in later when the host's multiple thread is very strong.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/C/PyUtility/Makefile|  25 ---
 BaseTools/Source/C/PyUtility/PyUtility.c | 106 ---
 BaseTools/Source/C/PyUtility/setup.py|  42 ---
 BaseTools/Source/Python/Common/Misc.py   |  16 +---
 BaseTools/Source/Python/Common/PyUtility.pyd | Bin 6144 -> 0 bytes
 5 files changed, 3 insertions(+), 186 deletions(-)  delete mode 100644 
BaseTools/Source/C/PyUtility/Makefile
 delete mode 100644 BaseTools/Source/C/PyUtility/PyUtility.c
 delete mode 100644 BaseTools/Source/C/PyUtility/setup.py
 delete mode 100644 BaseTools/Source/Python/Common/PyUtility.pyd

diff --git a/BaseTools/Source/C/PyUtility/Makefile 
b/BaseTools/Source/C/PyUtility/Makefile
deleted file mode 100644
index 5829070..000
--- a/BaseTools/Source/C/PyUtility/Makefile
+++ /dev/null
@@ -1,25 +0,0 @@
-## @file
-# Makefile
-#
-# Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved. -# 
This program and the accompanying materials -# are licensed and made available 
under the terms and conditions of the BSD License
-# which accompanies this distribution.The full text of the license may be 
found at
-# http://opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -# 
WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-
-!INCLUDE ..\Makefiles\ms.common
-
-APPNAME = GenSec
-
-LIBS = $(LIB_PATH)\Common.lib
-
-OBJECTS = PyUtility.obj
-
-#CFLAGS = $(CFLAGS) /nodefaultlib:libc.lib
-
-!INCLUDE ..\Makefiles\ms.app
-
diff --git a/BaseTools/Source/C/PyUtility/PyUtility.c 
b/BaseTools/Source/C/PyUtility/PyUtility.c
deleted file mode 100644
index d14b872..000
--- a/BaseTools/Source/C/PyUtility/PyUtility.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/** @file
-Python Utility
-
-Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved. -This 
program and the accompanying materials are licensed and made available -under 
the terms and conditions of the BSD License which accompanies this 
-distribution.  The full text of the license may be found at 
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, -WITHOUT 
WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include 
-#include 
-#include 
-
-/*
- SaveFileToDisk(FilePath, Content)
-*/
-STATIC
-PyObject*
-SaveFileToDisk (
-  PyObject*Self,
-  PyObject*Args
-  )
-{
-  CHAR8 *File;
-  UINT8 *Data;
-  UINTN DataLength;
-  UINTN WriteBytes;
-  UINTN Status;
-  HANDLEFileHandle;
-  PyObject  *ReturnValue = Py_False;
-
-  Status = PyArg_ParseTuple(
-Args,
-"ss#",
-&File,
-&Data,
-&DataLength
-);
-  if (Status == 0) {
-return NULL;
-  }
-
-  FileHandle = CreateFile(
-File,
-GENERIC_WRITE,
-FILE_SHARE_WRITE|FILE_SHARE_READ|FILE_SHARE_DELETE,
-NULL,
-CREATE_ALWAYS,
-FILE_ATTRIBUTE_NORMAL,
-NULL
-);
-  if (FileHandle == INVALID_HANDLE_VALUE) {
-PyErr_SetString(PyExc_Exception, "File creation failure");
-return NULL;
-  }
-
-  while (WriteFile(FileHandle, Data, DataLength, &WriteBytes, NULL)) {
-if (DataLength <= WriteBytes) {
-  DataLength = 0;
-  break;
-}
-
-Data += WriteBytes;
-DataLength -= WriteBytes;
-  }
-
-  if (DataLength != 0) {
-// file saved unsuccessfully
-PyErr_SetString(PyExc_Exception, "File write failure");
-goto Done;
-  }
-
-  //
-  // Flush buffer may slow down the whole build performance (average 10s 
slower)
-  //
-  //if (!FlushFileBuffers(FileHandle)) {
-  //  PyErr_SetString(PyExc_Exception, "File flush failure");
-  //  goto Done;
-  //}
-
-  // success!
-  ReturnValue = Py_True;
-
-Done:
-  CloseHandle(FileHandle);
-  return ReturnValue;
-}
-
-STATIC INT8 SaveFileToDiskDocs[] = "SaveFileToDisk(): Make sure the file is 
saved to disk\n";
-
-STATIC PyMethodDef PyUtility_Funcs[] = {
-  {"SaveFileToDisk", (PyCFunction)SaveFileToDisk, METH_VARARGS, 
SaveFileToDiskDocs},
-  {NULL, NULL, 0, NULL}
-};
-
-PyMODINIT_FUNC
-initPyUtility(VOID) {
-  Py_InitModul

Re: [edk2] [PATCH v3 3/3] MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot

2018-09-17 Thread Wu, Hao A
Hi,

Could you help to file a EDK2 Bugzilla tracker at:
https://bugzilla.tianocore.org/enter_bug.cgi?product=EDK2

to have a general description of the issue you met? And reference the
tracker within the commit message?
Really appreciate the help, thanks in advance.

With that,
Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: Marcin Wojtas [mailto:m...@semihalf.com]
> Sent: Sunday, September 16, 2018 6:26 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng; Kinney, Michael D; Gao, Liming; leif.lindh...@linaro.org; Wu,
> Hao A; ard.biesheu...@linaro.org; nad...@marvell.com; m...@semihalf.com;
> j...@semihalf.com; t...@semihalf.com
> Subject: [PATCH v3 3/3] MdeModulePkg/SdMmcPciHcDxe: Execute card detect
> only for RemovableSlot
> 
> Some devices can be non removable (such as eMMC) and checking
> Present State Register on host controller may falsely return
> an information that device is not present. Execute this
> check conditionally on the SloType field value.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 18
> --
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index f923930..bf9869d 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -661,12 +661,18 @@ SdMmcPciHcDriverBindingStart (
>  //
>  // Check whether there is a SD/MMC card attached
>  //
> -Status = SdMmcHcCardDetect (PciIo, Slot, &MediaPresent);
> -if (EFI_ERROR (Status) && (Status != EFI_MEDIA_CHANGED)) {
> -  continue;
> -} else if (!MediaPresent) {
> -  DEBUG ((DEBUG_INFO, "SdMmcHcCardDetect: No device attached in
> Slot[%d]!!!\n", Slot));
> -  continue;
> +if (Private->Slot[Slot].SlotType == RemovableSlot) {
> +  Status = SdMmcHcCardDetect (PciIo, Slot, &MediaPresent);
> +  if (EFI_ERROR (Status) && (Status != EFI_MEDIA_CHANGED)) {
> +continue;
> +  } else if (!MediaPresent) {
> +DEBUG ((
> +  DEBUG_INFO,
> +  "SdMmcHcCardDetect: No device attached in Slot[%d]!!!\n",
> +  Slot
> +  ));
> +continue;
> +  }
>  }
> 
>  Status = SdMmcHcInitHost (Private, Slot);
> --
> 2.7.4

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


Re: [edk2] [PATCH v3 1/3] MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence

2018-09-17 Thread Wu, Hao A
> -Original Message-
> From: Marcin Wojtas [mailto:m...@semihalf.com]
> Sent: Sunday, September 16, 2018 6:26 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng; Kinney, Michael D; Gao, Liming; leif.lindh...@linaro.org; Wu,
> Hao A; ard.biesheu...@linaro.org; nad...@marvell.com; m...@semihalf.com;
> j...@semihalf.com; t...@semihalf.com
> Subject: [PATCH v3 1/3] MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock
> and bus width sequence
> 
> According to JESD84-B50-1 chapter A.6 (documentation about eMMC4.5
> standard) step "Changing the data bus width" (A.6.3) should be
> executed after step "Switching to high-speed mode" (A.6.2).

Hi,

My understanding to the spec is that the spec seems do not impose a
sequence for 'Switching to high-speed mode' and 'Changing the data bus
width'.

My find is that both operation (A.6.2 & A.6.3 of JESD84-B50-1 ) requires:

* Do these steps after the bus is initialized according to A.6.1, Bus
* initialization.

Also, for HS200 mode selection, the flow chart in Section 6.6.4 shows the
bus width set before bus mode switch. So I think the current code is
taking this as a reference when switching the 'High Speed' mode.

I tried the eMMC device on my side, the current code implementation and
codes after applying your patch both work. So do you met with issues with
certain device with this sequence? If not, I prefer to keep the current
logic.

Best Regards,
Hao Wu

> 
> This patch fixes the bus-width/clock-setting sequence
> in EmmcSwitchToHighSpeed ().
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index c5fd214..12935ef 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -745,10 +745,6 @@ EmmcSwitchToHighSpeed (
>UINT8   HostCtrl1;
>UINT8   HostCtrl2;
> 
> -  Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
>//
>// Set to Hight Speed timing
>//
> @@ -783,6 +779,11 @@ EmmcSwitchToHighSpeed (
> 
>HsTiming = 1;
>Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming,
> ClockFreq);
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +
> +  Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
> 
>return Status;
>  }
> --
> 2.7.4

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


Re: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits

2018-09-17 Thread Wu, Hao A
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marcin Wojtas
> Sent: Sunday, September 16, 2018 6:26 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng; t...@semihalf.com; Wu, Hao A; nad...@marvell.com; Gao,
> Liming; Kinney, Michael D
> Subject: [edk2] [PATCH v3 2/3] MdeModulePkg/SdMmcPciHcDxe: Fix
> SdMmcHcReset to set only necesery bits
> 
> From: Tomasz Michalec 
> 
> SdMmcHcReset used to set all bits of Software Reset Register to 1
> including reserved ones.

Hi,

I did a quick search of the SD Host Controller Simplified Specification, I
do not find the spec prohibit setting all the bits when performing a reset
for the host controller.

Do you met with issues during the controller reset with the current logic?

Best Regards,
Hao Wu

> 
> Now only first bit is set which means "Software Reset for All".
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 5 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 6 +++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index e389d52..bcc31bd 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -63,6 +63,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #define SD_MMC_HC_CTRL_VER0xFE
> 
>  //
> +// SD Software Reset Register bits description
> +//
> +#define SD_MMC_HC_SW_RST_ALL  BIT0
> +
> +//
>  // The transfer modes supported by SD Host Controller
>  // Simplified Spec 3.0 Table 1-2
>  //
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 9672b5b..9d9bca8 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -454,8 +454,8 @@ SdMmcHcReset (
>}
> 
>PciIo   = Private->PciIo;
> -  SwReset = 0xFF;
> -  Status  = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE, sizeof
> (SwReset), &SwReset);
> +  SwReset = SD_MMC_HC_SW_RST_ALL;
> +  Status  = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_SW_RST, sizeof
> (SwReset), &SwReset);
> 
>if (EFI_ERROR (Status)) {
>  DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write full 1 fails: %r\n", Status));
> @@ -467,7 +467,7 @@ SdMmcHcReset (
>   Slot,
>   SD_MMC_HC_SW_RST,
>   sizeof (SwReset),
> - 0xFF,
> + SD_MMC_HC_SW_RST_ALL,
>   0x00,
>   SD_MMC_HC_GENERIC_TIMEOUT
>   );
> --
> 2.7.4
> 
> ___
> 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


Re: [edk2] [PATCH V2 1/6] MdePkg UefiLib: Add new EfiLocateXXXAcpiTable() APIs

2018-09-17 Thread Zeng, Star
Ray,

Thanks for the good feedbacks.
I can remove the ASSERT and update the description in the function header.
I can also merge ScanTableInRSDT and ScanTableInXSDT.

Thanks,
Star
-Original Message-
From: Ni, Ruiyu 
Sent: Friday, September 14, 2018 12:41 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Younas khan 
; Yao, Jiewen ; Gao, Liming 

Subject: Re: [edk2] [PATCH V2 1/6] MdePkg UefiLib: Add new 
EfiLocateXXXAcpiTable() APIs

Star,
I have two comments. see below.
On 9/13/2018 6:26 PM, Star Zeng wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=967
> Request to add a library function for GetAcpiTable() in order to get 
> ACPI table using signature as input.
> 
> After evaluation, we found there are many duplicated code to find ACPI 
> table by signature in different modules.
> 
> This patch adds new EfiLocateXXXAcpiTable() APIs in UefiLib for the 
> request and also the following patch to remove the duplicated code.
> 
> Cc: Younas khan 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>   MdePkg/Include/Library/UefiLib.h   |  68 ++
>   MdePkg/Library/UefiLib/Acpi.c  | 488 
> +
>   MdePkg/Library/UefiLib/UefiLib.inf |   3 +
>   3 files changed, 559 insertions(+)
>   create mode 100644 MdePkg/Library/UefiLib/Acpi.c
> 
> diff --git a/MdePkg/Include/Library/UefiLib.h 
> b/MdePkg/Include/Library/UefiLib.h
> index f80067f11103..cf82ff4a920a 100644
> --- a/MdePkg/Include/Library/UefiLib.h
> +++ b/MdePkg/Include/Library/UefiLib.h
> @@ -26,6 +26,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>   #ifndef __UEFI_LIB_H__
>   #define __UEFI_LIB_H__
>   
> +#include 
> +
>   #include 
>   #include 
>   #include 
> @@ -1595,4 +1597,70 @@ EfiOpenFileByDevicePath (
> IN UINT64OpenMode,
> IN UINT64Attributes
> );
> +
> +/**
> +  This function locates next ACPI table in XSDT/RSDT based on 
> +Signature and
> +  previous returned Table.
> +
> +  If PreviousTable is NULL:
> +  This function will locate the first ACPI table in XSDT/RSDT based 
> + on  Signature in gEfiAcpi20TableGuid system configuration table 
> + first, and then  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then 
> + Dsdt in  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, 
> + and then  FirmwareCtrl in FADT.
> +
> +  If PreviousTable is not NULL:
> +  1. If it could be located in XSDT in gEfiAcpi20TableGuid system 
> configuration
> + table, then this function will just locate next table in XSDT in
> + gEfiAcpi20TableGuid system configuration table.
> +  2. If it could be located in RSDT in gEfiAcpi20TableGuid system 
> configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi20TableGuid system configuration table.
> +  3. If it could be located in RSDT in gEfiAcpi10TableGuid system 
> configuration
> + table, then this function will just locate next table in RSDT in
> + gEfiAcpi10TableGuid system configuration table.
> +

1. I don't see difference between the case when PreviousTable is NULL or not. 
We don't need to distinguish between the two cases in above comments.


> +  If PreviousTable is not NULL and PreviousTable->Signature is not same with
> +  Signature, then ASSERT.
I'd suggest to return Invalid Parameter instead of assertion.

> +
> +  @param Signature  ACPI table signature.
> +  @param PreviousTable  Pointer to previous returned table to locate next
> +table, or NULL to locate first table.
> +
> +  @return Next ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateNextAcpiTable (
> +  IN UINT32 Signature,
> +  IN EFI_ACPI_COMMON_HEADER *PreviousTable OPTIONAL
> +  );
> +
> +/**
> +  This function locates first ACPI table in XSDT/RSDT based on Signature.
> +
> +  This function will locate the first ACPI table in XSDT/RSDT based on
> +  Signature in gEfiAcpi20TableGuid system configuration table first, and then
> +  gEfiAcpi10TableGuid system configuration table.
> +  This function will locate in XSDT first, and then RSDT.
> +  For DSDT, this function will locate XDsdt in FADT first, and then Dsdt in
> +  FADT.
> +  For FACS, this function will locate XFirmwareCtrl in FADT first, and then
> +  FirmwareCtrl in FADT.
> +
> +  @param Signature  ACPI table signature.
> +
> +  @return First ACPI table or NULL if not found.
> +
> +**/
> +EFI_ACPI_COMMON_HEADER *
> +EFIAPI
> +EfiLocateFirstAcpiTable (
> +  IN UINT32 Signature
> +  );
> +
>   #endif
> diff --git a/MdePkg/Library/UefiLib/Acpi.c b/MdePkg/Library/UefiLib/Acpi.c
> n