[edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

[Jian J Wang]

Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm() to
check if current processor is in SMM mode or not. But this is not correct
because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
running in SMRAM or from SMM driver. It cannot guarantee if the caller is
running in SMM mode. Because SMM mode will load its own page table, adding
an extra check of saved DXE page table base address against current CR3
register value can help to get the correct answer for sure (in SMM mode or
not in SMM mode).

This is an issue caused by check-in at

  d106cf71eabaacff63c14626a4a87346b93074dd

Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c
index 850eed60e7..df021798c0 100644
--- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
+++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
@@ -136,7 +136,14 @@ IsInSmm (
 mSmmBase2->InSmm (mSmmBase2, );
   }
 
-  return InSmm;
+  //
+  // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM
+  // or from SMM driver. It cannot tell if the caller is running in SMM mode.
+  // Check page table base address to guarantee that because SMM mode willl
+  // load its own page table.
+  //
+  return (InSmm &&
+  mPagingContext.ContextData.X64.PageTableBase != 
(UINT64)AsmReadCr3());
 }
 
 /**
-- 
2.16.2.windows.1

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

Re: [edk2] Reg: Intel Rangley Support in EDK

[Dhanasekar Jaganathan]

Hi Nate,

Thanks for the info. I will try this.

My Rangley platform has no onboard or offboard vga support. Com Port /
Serial console is used for display and communication.
I am booting the platform by coreboot with UEFI payload. I am trying to
install Ubuntu server OS.

When I boot into Shell, I am not seeing actually shell (graphics are not
good) and keystroke are working properly.
When I boot into EDK Setup, I am not getting proper setup page (graphics
are not good) and keystroke are not working.

When I try to boot Ubuntu Server, I am getting below error,


*"error: no suitable video mode found.Booting in blind mode"*

It seems I am missing some video/graphics settings in UEFI payload. If you
know the fix, Please provide me.

Thanks,
Dhanasekar



On Fri, Jul 13, 2018 at 7:14 AM, Desimone, Nathaniel L <
nathaniel.l.desim...@intel.com> wrote:

> Hi Dhanasekar,
>
> There is nothing pre-built and off-the-shelf ready today. But we do have a
> generalized infrastructure for open source Intel UEFI platforms called
> MinPlatformPkg. Please see the following:
>
> https://github.com/tianocore/edk2-platforms/tree/devel-
> MinPlatform/Platform/Intel
>
> Notice that MinPlatformPkg requires a *OpenBoardPkg that supports the
> desired platform. To my knowledge no one at Intel has looked at
> implementing a RangleyOpenBoardPkg thus far. The focus has been on recently
> released platforms, Rangley is 4 years old at this point. If you are so
> inclined, you are welcome to try implementing a RangleyOpenBoardPkg. I
> would recommend using KabylakeOpenBoardPkg as a starting point.
>
> Thanks,
> Nate
>
> On 7/11/18, 8:52 PM, "edk2-devel on behalf of Dhanasekar Jaganathan" <
> edk2-devel-boun...@lists.01.org on behalf of jdhanasekar...@gmail.com>
> wrote:
>
> Hi,
>
> I have booted Intel Rangley / MohonPeak platform in coreboot with Intel
> FSP.
> I am unable to install UEFI OS in coreboot (sometime).
>
> EDK bios will install both UEFI and Legacy OS.
> Does Open EDK supports Intel Rangley platform?.
> Is code base available for Intel Rangely platform?
>
>
>
> Thanks,
> Dhanasekar
> ___
> 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-IntelAtomProcessorE3900] Platform VTd DXE Driver.

[zwei4]

Add platform driver to produce gEdkiiPlatformVTdPolicyProtocolGuid for VT-d DXE 
driver to consume.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Wei 
CC: Mang Guo 
---
 .../PlatformVTdDxe/PlatformVTdDxe.c| 410 +
 .../PlatformVTdDxe/PlatformVTdDxe.inf  |  62 
 .../PlatformVTdDxe/PlatformVTdDxe.uni  |  20 +
 .../PlatformVTdDxe/PlatformVTdDxeExtra.uni |  20 +
 4 files changed, 512 insertions(+)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxe.c
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxe.inf
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxe.uni
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxeExtra.uni

diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxe.c
 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxe.c
new file mode 100644
index 00..a2fb391527
--- /dev/null
+++ 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdDxe/PlatformVTdDxe.c
@@ -0,0 +1,410 @@
+/** @file
+  Platform VTd Sample driver.
+
+  Copyright (c) 2017 - 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 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+typedef struct {
+  ACPI_EXTENDED_HID_DEVICE_PATH  I2cController;
+  UINT8  HidStr[8];
+  UINT8  UidStr[1];
+  UINT8  CidStr[8];
+} PLATFORM_I2C_CONTROLLER_DEVICE_PATH;
+
+typedef struct {
+  ACPI_EXTENDED_HID_DEVICE_PATH  I2cDevice;
+  UINT8  HidStr[13];
+  UINT8  UidStr[1];
+  UINT8  CidStr[13];
+} PLATFORM_I2C_DEVICE_DEVICE_PATH;
+
+typedef struct {
+  PLATFORM_I2C_CONTROLLER_DEVICE_PATH  I2cController;
+  PLATFORM_I2C_DEVICE_DEVICE_PATH  I2cDevice;
+  EFI_DEVICE_PATH_PROTOCOL End;
+} PLATFORM_I2C_DEVICE_PATH;
+
+typedef struct {
+  ACPI_HID_DEVICE_PATH  PciRootBridge;
+  PCI_DEVICE_PATH   PciDevice;
+  EFI_DEVICE_PATH_PROTOCOL  EndDevicePath;
+} PLATFORM_PCI_DEVICE_PATH;
+
+typedef struct {
+  ACPI_HID_DEVICE_PATH  PciRootBridge;
+  PCI_DEVICE_PATH   PciBridge;
+  PCI_DEVICE_PATH   PciDevice;
+  EFI_DEVICE_PATH_PROTOCOL  EndDevicePath;
+} PLATFORM_PCI_BRIDGE_DEVICE_PATH;
+
+typedef struct {
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
+  UINT16Segment;
+  VTD_SOURCE_ID SourceId;
+} PLATFORM_ACPI_DEVICE_MAPPING;
+
+#define PLATFORM_PCI_ROOT_BRIDGE \
+  { \
+{ \
+  ACPI_DEVICE_PATH, \
+  ACPI_DP, \
+  { \
+(UINT8) (sizeof (ACPI_HID_DEVICE_PATH)), \
+(UINT8) ((sizeof (ACPI_HID_DEVICE_PATH)) >> 8) \
+  }, \
+}, \
+EISA_PNP_ID (0x0A03), \
+0 \
+  }
+
+#define PLATFORM_END_ENTIRE \
+  { \
+END_DEVICE_PATH_TYPE, END_ENTIRE_DEVICE_PATH_SUBTYPE, { 
END_DEVICE_PATH_LENGTH, 0 } \
+  }
+
+#define PLATFORM_PCI(Device, Function) \
+  { \
+{ \
+  HARDWARE_DEVICE_PATH, \
+  HW_PCI_DP, \
+  { \
+(UINT8) (sizeof (PCI_DEVICE_PATH)), \
+(UINT8) ((sizeof (PCI_DEVICE_PATH)) >> 8) \
+  } \
+}, \
+(Function), \
+(Device) \
+  }
+
+#define PLATFORM_I2C(Hid, Uid, Cid, HidStr, UidStr, CidStr) \
+  { \
+{ \
+  { \
+ACPI_DEVICE_PATH, \
+ACPI_EXTENDED_DP, \
+{sizeof(ACPI_EXTENDED_HID_DEVICE_PATH) + sizeof(HidStr) + 
sizeof(UidStr) + sizeof(CidStr), 0} \
+  }, \
+  Hid, \
+  Uid, \
+  Cid \
+}, \
+HidStr, \
+UidStr, \
+CidStr \
+  }
+
+PLATFORM_I2C_DEVICE_PATH mPlatformI2CDevicePath = {
+  PLATFORM_I2C(0, 2, 0, "INT33C3", "", "INT33C3"),
+  PLATFORM_I2C(0, 1, 0, "I2C01\\TPANEL", "", "I2C01\\TPANEL"),
+  PLATFORM_END_ENTIRE
+};
+
+PLATFORM_ACPI_DEVICE_MAPPING  mAcpiDeviceMapping[] = {
+  {
+(EFI_DEVICE_PATH_PROTOCOL *),
+0x0, // Segment
+{{0x01, 0x15, 0x00}} // Function, Device, Bus
+  }
+};
+
+PLATFORM_PCI_BRIDGE_DEVICE_PATH mPlatformPciBridgeDevicePath = {
+  PLATFORM_PCI_ROOT_BRIDGE,
+  PLATFORM_PCI(0x1C, 1),
+  PLATFORM_PCI(0, 0),
+  PLATFORM_END_ENTIRE
+};
+
+#pragma pack(1)
+
+typedef struct {
+  

[edk2] [Patch][edk2-platforms/devel-IntelAtomProcessorE3900] Platform VT-d Information PEIM.

[zwei4]

This platform VT-d Information PEIM produces gEdkiiVTdInfoPpiGuid to expose 
DMAR (DMA Remapping Table).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Wei 
CC: Mang Guo 
---
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.c| 334 +
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.inf  |  55 
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.uni  |  20 ++
 .../PlatformVTdInfoPei/PlatformVTdInfoPeiExtra.uni |  20 ++
 4 files changed, 429 insertions(+)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.inf
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.uni
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPeiExtra.uni

diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
new file mode 100644
index 00..b342e99f22
--- /dev/null
+++ 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
@@ -0,0 +1,334 @@
+/** @file
+  Platform VTd Info PEI driver.
+
+  Copyright (c) 2017 - 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 
+#include 
+#include 
+#include 
+#include 
+
+
+#define R_SA_MCHBAR   (0x48)
+#define R_SA_GGC  (0x50)
+#define N_SKL_SA_GGC_GGMS_OFFSET  (0x6)
+#define B_SKL_SA_GGC_GGMS_MASK(0xc0)
+#define N_SKL_SA_GGC_GMS_OFFSET   (0x8)
+#define B_SKL_SA_GGC_GMS_MASK (0xff00)
+#define V_SKL_SA_GGC_GGMS_8MB 3
+#define R_SA_TOLUD(0xbc)
+
+//
+// VT-d Engine base address.
+//
+#define R_SA_MCHBAR_VTD1_OFFSET  0x6C88  ///< DMA Remapping HW UNIT1 for IGD
+#define R_SA_MCHBAR_VTD2_OFFSET  0x6C80  ///< DMA Remapping HW UNIT2 for all 
other - PEG, USB, SATA etc
+
+#define SA_VTD_ENGINE_NUMBER 2
+
+EFI_GUID gEdkiiSiliconInitializedPpiGuid = {0x82a72dc8, 0x61ec, 0x403e, {0xb1, 
0x5a, 0x8d, 0x7a, 0x3a, 0x71, 0x84, 0x98}};
+
+typedef struct {
+  EFI_ACPI_DMAR_HEADER DmarHeader;
+  //
+  // VTd engine 1 - integrated graphic
+  //
+  EFI_ACPI_DMAR_DRHD_HEADERDrhd1;
+  EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER  Drhd11;
+  EFI_ACPI_DMAR_PCI_PATH   Drhd111;
+  //
+  // VTd engine 2 - all rest
+  //
+  EFI_ACPI_DMAR_DRHD_HEADERDrhd2;
+  //
+  // RMRR 1 - integrated graphic
+  //
+  EFI_ACPI_DMAR_RMRR_HEADERRmrr1;
+  EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER  Rmrr11;
+  EFI_ACPI_DMAR_PCI_PATH   Rmrr111;
+} MY_VTD_INFO_PPI;
+
+MY_VTD_INFO_PPI  mPlatformVTdSample = {
+  { // DmarHeader
+{ // Header
+  EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE,
+  sizeof(MY_VTD_INFO_PPI),
+  EFI_ACPI_DMAR_REVISION,
+},
+0x26, // HostAddressWidth
+  },
+
+  { // Drhd1
+{ // Header
+  EFI_ACPI_DMAR_TYPE_DRHD,
+  sizeof(EFI_ACPI_DMAR_DRHD_HEADER) +
+sizeof(EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER) +
+sizeof(EFI_ACPI_DMAR_PCI_PATH)
+},
+0, // Flags
+0, // Reserved
+0, // SegmentNumber
+0xFED9 // RegisterBaseAddress -- TO BE PATCHED
+  },
+  { // Drhd11
+EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT,
+sizeof(EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER) +
+  sizeof(EFI_ACPI_DMAR_PCI_PATH),
+0, // Reserved2
+0, // EnumerationId
+0  // StartBusNumber
+  },
+  { // Drhd111
+2,  // Device
+0   // Function
+  },
+
+  { // Drhd2
+{ // Header
+  EFI_ACPI_DMAR_TYPE_DRHD,
+  sizeof(EFI_ACPI_DMAR_DRHD_HEADER)
+},
+EFI_ACPI_DMAR_DRHD_FLAGS_INCLUDE_PCI_ALL, // Flags
+0, // Reserved
+0, // SegmentNumber
+0xFED91000 // RegisterBaseAddress -- TO BE PATCHED
+  },
+
+  { // Rmrr1
+{ // Header
+  EFI_ACPI_DMAR_TYPE_RMRR,
+  sizeof(EFI_ACPI_DMAR_RMRR_HEADER) +
+sizeof(EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER) +
+sizeof(EFI_ACPI_DMAR_PCI_PATH)
+},
+{0}, // Reserved
+0, // SegmentNumber
+0x0, // ReservedMemoryRegionBaseAddress -- TO BE PATCHED
+0x0 // ReservedMemoryRegionLimitAddress -- TO BE PATCHED
+  },
+  { // Rmrr11
+EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT,
+

[edk2] [Patch][edk2-platforms/devel-MinnowBoardMax-UDK2017] Platform VT-d Information PEIM.

[zwei4]

This platform VT-d Information PEIM produces gEdkiiVTdInfoPpiGuid to expose 
DMAR (DMA Remapping Table).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Wei 
CC: Mang Guo 
---
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.c| 334 +
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.inf  |  55 
 .../PlatformVTdInfoPei/PlatformVTdInfoPei.uni  |  20 ++
 .../PlatformVTdInfoPei/PlatformVTdInfoPeiExtra.uni |  20 ++
 4 files changed, 429 insertions(+)
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.inf
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.uni
 create mode 100644 
Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPeiExtra.uni

diff --git 
a/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
new file mode 100644
index 00..b342e99f22
--- /dev/null
+++ 
b/Platform/BroxtonPlatformPkg/Common/PlatformSettings/PlatformVTdInfoPei/PlatformVTdInfoPei.c
@@ -0,0 +1,334 @@
+/** @file
+  Platform VTd Info PEI driver.
+
+  Copyright (c) 2017 - 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 
+#include 
+#include 
+#include 
+#include 
+
+
+#define R_SA_MCHBAR   (0x48)
+#define R_SA_GGC  (0x50)
+#define N_SKL_SA_GGC_GGMS_OFFSET  (0x6)
+#define B_SKL_SA_GGC_GGMS_MASK(0xc0)
+#define N_SKL_SA_GGC_GMS_OFFSET   (0x8)
+#define B_SKL_SA_GGC_GMS_MASK (0xff00)
+#define V_SKL_SA_GGC_GGMS_8MB 3
+#define R_SA_TOLUD(0xbc)
+
+//
+// VT-d Engine base address.
+//
+#define R_SA_MCHBAR_VTD1_OFFSET  0x6C88  ///< DMA Remapping HW UNIT1 for IGD
+#define R_SA_MCHBAR_VTD2_OFFSET  0x6C80  ///< DMA Remapping HW UNIT2 for all 
other - PEG, USB, SATA etc
+
+#define SA_VTD_ENGINE_NUMBER 2
+
+EFI_GUID gEdkiiSiliconInitializedPpiGuid = {0x82a72dc8, 0x61ec, 0x403e, {0xb1, 
0x5a, 0x8d, 0x7a, 0x3a, 0x71, 0x84, 0x98}};
+
+typedef struct {
+  EFI_ACPI_DMAR_HEADER DmarHeader;
+  //
+  // VTd engine 1 - integrated graphic
+  //
+  EFI_ACPI_DMAR_DRHD_HEADERDrhd1;
+  EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER  Drhd11;
+  EFI_ACPI_DMAR_PCI_PATH   Drhd111;
+  //
+  // VTd engine 2 - all rest
+  //
+  EFI_ACPI_DMAR_DRHD_HEADERDrhd2;
+  //
+  // RMRR 1 - integrated graphic
+  //
+  EFI_ACPI_DMAR_RMRR_HEADERRmrr1;
+  EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER  Rmrr11;
+  EFI_ACPI_DMAR_PCI_PATH   Rmrr111;
+} MY_VTD_INFO_PPI;
+
+MY_VTD_INFO_PPI  mPlatformVTdSample = {
+  { // DmarHeader
+{ // Header
+  EFI_ACPI_4_0_DMA_REMAPPING_TABLE_SIGNATURE,
+  sizeof(MY_VTD_INFO_PPI),
+  EFI_ACPI_DMAR_REVISION,
+},
+0x26, // HostAddressWidth
+  },
+
+  { // Drhd1
+{ // Header
+  EFI_ACPI_DMAR_TYPE_DRHD,
+  sizeof(EFI_ACPI_DMAR_DRHD_HEADER) +
+sizeof(EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER) +
+sizeof(EFI_ACPI_DMAR_PCI_PATH)
+},
+0, // Flags
+0, // Reserved
+0, // SegmentNumber
+0xFED9 // RegisterBaseAddress -- TO BE PATCHED
+  },
+  { // Drhd11
+EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT,
+sizeof(EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER) +
+  sizeof(EFI_ACPI_DMAR_PCI_PATH),
+0, // Reserved2
+0, // EnumerationId
+0  // StartBusNumber
+  },
+  { // Drhd111
+2,  // Device
+0   // Function
+  },
+
+  { // Drhd2
+{ // Header
+  EFI_ACPI_DMAR_TYPE_DRHD,
+  sizeof(EFI_ACPI_DMAR_DRHD_HEADER)
+},
+EFI_ACPI_DMAR_DRHD_FLAGS_INCLUDE_PCI_ALL, // Flags
+0, // Reserved
+0, // SegmentNumber
+0xFED91000 // RegisterBaseAddress -- TO BE PATCHED
+  },
+
+  { // Rmrr1
+{ // Header
+  EFI_ACPI_DMAR_TYPE_RMRR,
+  sizeof(EFI_ACPI_DMAR_RMRR_HEADER) +
+sizeof(EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER) +
+sizeof(EFI_ACPI_DMAR_PCI_PATH)
+},
+{0}, // Reserved
+0, // SegmentNumber
+0x0, // ReservedMemoryRegionBaseAddress -- TO BE PATCHED
+0x0 // ReservedMemoryRegionLimitAddress -- TO BE PATCHED
+  },
+  { // Rmrr11
+EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT,
+

Re: [edk2] [patch] MdeModulePkg/PerformanceMeasurement.h: Correct the license

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Bi, Dandan
>Sent: Thursday, July 12, 2018 10:08 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Zeng, Star 
>Subject: [patch] MdeModulePkg/PerformanceMeasurement.h: Correct the
>license
>
>Corrected to use the BSD license.
>
>Cc: Liming Gao 
>Cc: Star Zeng 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Dandan Bi 
>---
> MdeModulePkg/Include/Guid/PerformanceMeasurement.h | 31 +++---
>
> 1 file changed, 9 insertions(+), 22 deletions(-)
>
>diff --git a/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
>b/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
>index f2955c7..0c3eb59 100644
>--- a/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
>+++ b/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
>@@ -1,31 +1,18 @@
> /** @file
>
>-Copyright (c) 2017, Microsoft Corporation
>-Copyright (c) 2018, Intel Corporation. All rights reserved.
>+Performance measurement protocol, allows logging performance data.
>
>-All rights reserved.
>-Redistribution and use in source and binary forms, with or without
>-modification, are permitted provided that the following conditions are met:
>-1. Redistributions of source code must retain the above copyright notice,
>-this list of conditions and the following disclaimer.
>-2. Redistributions in binary form must reproduce the above copyright notice,
>-this list of conditions and the following disclaimer in the documentation
>- and/or other materials provided with the distribution.
>-
>-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
>CONTRIBUTORS "AS IS" AND
>-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>THE IMPLIED
>-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>PURPOSE ARE DISCLAIMED.
>-IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
>FOR ANY DIRECT,
>-INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
>DAMAGES (INCLUDING,
>-BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
>LOSS OF USE,
>-DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
>ON ANY THEORY OF
>-LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
>NEGLIGENCE
>-OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
>EVEN IF
>-ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>+Copyright (c) 2017, Microsoft Corporation
>+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 that accompanies this
>distribution.
>+The full text of the license may be found at
>+http://opensource.org/licenses/bsd-license.php.
>
>-Performance measurement protocol, allows logging performance data.
>+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>BASIS,
>+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>
> **/
>
> #ifndef _PERFORMANCE_MEASUREMENT_H_
> #define _PERFORMANCE_MEASUREMENT_H_
>--
>1.9.5.msysgit.1

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

[edk2] [Patch] BaseTools: ElfConvert Tool update VerboseMsg to same with the comment

[Yonghong Zhu]

Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=994
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/C/GenFw/Elf32Convert.c | 2 +-
 BaseTools/Source/C/GenFw/Elf64Convert.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/GenFw/Elf32Convert.c 
b/BaseTools/Source/C/GenFw/Elf32Convert.c
index af5ff93..3d7de6d 100644
--- a/BaseTools/Source/C/GenFw/Elf32Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf32Convert.c
@@ -688,11 +688,11 @@ WriteSections32 (
 
   default:
 //
 //  Ignore for unkown section type.
 //
-VerboseMsg ("%s unknown section type %x. We directly copy this section 
into Coff file", mInImageName, (unsigned)Shdr->sh_type);
+VerboseMsg ("%s unknown section type %x. We ignore this unknown 
section type.", mInImageName, (unsigned)Shdr->sh_type);
 break;
   }
 }
   }
 
diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c 
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 15da89c..469979c 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -822,11 +822,11 @@ WriteSections64 (
 
   default:
 //
 //  Ignore for unkown section type.
 //
-VerboseMsg ("%s unknown section type %x. We directly copy this section 
into Coff file", mInImageName, (unsigned)Shdr->sh_type);
+VerboseMsg ("%s unknown section type %x. We ignore this unknown 
section type.", mInImageName, (unsigned)Shdr->sh_type);
 break;
   }
 }
   }
 
-- 
2.6.1.windows.1

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

Re: [edk2] Reg: Intel Rangley Support in EDK

[Desimone, Nathaniel L]

Hi Dhanasekar,

There is nothing pre-built and off-the-shelf ready today. But we do have a 
generalized infrastructure for open source Intel UEFI platforms called 
MinPlatformPkg. Please see the following:

https://github.com/tianocore/edk2-platforms/tree/devel-MinPlatform/Platform/Intel

Notice that MinPlatformPkg requires a *OpenBoardPkg that supports the desired 
platform. To my knowledge no one at Intel has looked at implementing a 
RangleyOpenBoardPkg thus far. The focus has been on recently released 
platforms, Rangley is 4 years old at this point. If you are so inclined, you 
are welcome to try implementing a RangleyOpenBoardPkg. I would recommend using 
KabylakeOpenBoardPkg as a starting point.

Thanks,
Nate

On 7/11/18, 8:52 PM, "edk2-devel on behalf of Dhanasekar Jaganathan" 
 wrote:

Hi,

I have booted Intel Rangley / MohonPeak platform in coreboot with Intel
FSP.
I am unable to install UEFI OS in coreboot (sometime).

EDK bios will install both UEFI and Legacy OS.
Does Open EDK supports Intel Rangley platform?.
Is code base available for Intel Rangely platform?



Thanks,
Dhanasekar
___
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/3] UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Friday, July 13, 2018 6:14 AM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: Re: [edk2] [Patch v2 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to
> memory to save time.
> 
> On 07/12/18 23:59, Laszlo Ersek wrote:
> > On 07/12/18 12:49, Eric Dong wrote:
> >> Read uCode from memory has better performance than from flash.
> >> But it needs extra effort to let BSP copy uCode from flash to memory.
> >> Also BSP already enable cache in SEC phase, so it use less time to
> >> relocate uCode from flash to memory. After verification, if system
> >> has more than one processor, it will reduce some time if load uCode
> >> from memory.
> >>
> >> This change enable this optimization.
> >>
> >> Cc: Laszlo Ersek 
> >> Cc: Ruiyu Ni 
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Eric Dong 
> >> ---
> >>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 34
> >> +-
> >>  1 file changed, 33 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> >> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> >> index 108eea0a6f..c3cd6d7d51 100644
> >> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> >> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> >> @@ -1520,6 +1520,7 @@ MpInitLibInitialize (
> >>UINTNApResetVectorSize;
> >>UINTNBackupBufferAddr;
> >>UINTNApIdtBase;
> >> +  VOID *MicrocodePatchInRam;
> >>
> >>OldCpuMpData = GetCpuMpDataFromGuidedHob ();
> >>if (OldCpuMpData == NULL) {
> >> @@ -1587,8 +1588,39 @@ MpInitLibInitialize (
> >>CpuMpData->SwitchBspFlag= FALSE;
> >>CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
> >>CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData-
> >CpuData + MaxLogicalProcessorNumber);
> >> -  CpuMpData->MicrocodePatchAddress= PcdGet64
> (PcdCpuMicrocodePatchAddress);
> >>CpuMpData->MicrocodePatchRegionSize = PcdGet64
> >> (PcdCpuMicrocodePatchRegionSize);
> >> +  //
> >> +  // If platform has more than one CPU, relocate microcode to memory
> >> + to reduce  // loading microcode time.
> >> +  //
> >> +  MicrocodePatchInRam = NULL;
> >> +  if (MaxLogicalProcessorNumber > 1) {
> >> +MicrocodePatchInRam = AllocatePages (
> >> +EFI_SIZE_TO_PAGES (
> >> +  (UINTN)CpuMpData->MicrocodePatchRegionSize
> >> +  )
> >> +);
> >> +ASSERT (MicrocodePatchInRam != NULL);  }  if
> >> + (MicrocodePatchInRam == NULL) {
> >> +//
> >> +// there is only one processor, or no microcode patch is available, or
> >> +// memory allocation failed
> >> +//
> >> +CpuMpData->MicrocodePatchAddress = PcdGet64
> >> + (PcdCpuMicrocodePatchAddress);  } else {
> >> +//
> >> +// there are multiple processors, and a microcode patch is available,
> and
> >> +// memory allocation succeeded
> >> +//
> >> +CopyMem (
> >> +  MicrocodePatchInRam,
> >> +  (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
> >> +  (UINTN)CpuMpData->MicrocodePatchRegionSize
> >> +  );
> >> +CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
> >> + }
> >> +
> >>InitializeSpinLock(>MpLock);
> >>
> >>//
> >>
> >
> > Reviewed-by: Laszlo Ersek 
> >
> 
> Sorry, I have to take that back -- please do not commit this patch.
> 
> For this version of the patch:
> 
> Nacked-by: Laszlo Ersek 
> 
> The ASSERT() is wrong. Again, in the code above, AllocatePages() can return
> NULL not only because of memory allocation failure, but also because the
> number of pages to allocate can be zero! If the platform has no microcode
> patch to apply.
> 
> I knew this full well when I suggested the code, but then I forgot about it 
> when
> you mentioned the ASSERT(). I think you also knew about it, and forgot about
> it too. :)
> 
> In particular, this patch would make it *impossible* to boot OVMF with
> multiple processors, because OVMF *never* provides a microcode update.
> 
> So, please remove the ASSERT.
> 

Agree, I removed ASSERT code and send V3 patches.

> Alternatively, you could modify the ASSERT() like this:
> 
>   //
>   // if we attempt actual memory allocation, we expect it to succeed
>   //
>   ASSERT (
> (CpuMpData->MicrocodePatchRegionSize == 0) ||
> (MicrocodePatchInRam != NULL)
> );
> 
> Thanks,
> Laszlo
> ___
> 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 2/2] ArmVirtPkg/ArmVirtQemu: enable the IPv6 stack

[Laszlo Ersek]

Add the IPv6 stack to ArmVirtQemu with a cumulative port of the following
OvmfPkg commits:

* 36c6413f76e5 "OvmfPkg: enable the IPv6 support", 2014-12-19

* 96302b80d90e "OvmfPkg: Enable Network2 Shell Commands for IPv6",
   2016-03-08

* 6d0f8941bdc2 "OvmfPkg: always resolve OpenSslLib, IntrinsicLib and
   BaseCryptLib", 2017-01-17

* 32e22f20c985 "OvmfPkg: correct the IScsiDxe module included for the IPv6
   stack", 2017-01-17

The IPv6-enabled IScsiDxe driver depends on BaseCryptLib, and the
"CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf" instance depends on
IntrinsicLib and OpensslLib. This is why commit 6d0f8941bdc2 is relevant.

However, unlike in OvmfPkg, in ArmVirtPkg we'll precisely track the
firmware features that require these library classes. (The OvmfPkg
discussion was quite complex, and the OvmfPkg solution was a compromise:
.)

The ArmVirtXen platform is not extended with the relevant drivers because
currently it doesn't include any networking support.

Cc: Ard Biesheuvel 
Cc: Julien Grall 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1007
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 ArmVirtPkg/ArmVirt.dsc.inc   | 18 +++---
 ArmVirtPkg/ArmVirtQemu.dsc   | 13 -
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 13 -
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 12 +++-
 4 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 76c400a5de21..8bb54c5e6579 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -78,10 +78,13 @@ [LibraryClasses.common]
   # Networking Requirements
   NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
   DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
   IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
+!if $(NETWORK_IP6_ENABLE) == TRUE
+  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
+!endif
 !if $(HTTP_BOOT_ENABLE) == TRUE
   HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
 !endif
 
   #
@@ -142,18 +145,24 @@ [LibraryClasses.common]
   UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
 
   XenIoMmioLib|OvmfPkg/Library/XenIoMmioLib/XenIoMmioLib.inf
 
   #
-  # Secure Boot dependencies
+  # CryptoPkg libraries needed by multiple firmware features
   #
-!if $(SECURE_BOOT_ENABLE) == TRUE
+!if ($(SECURE_BOOT_ENABLE) == TRUE) || ($(NETWORK_IP6_ENABLE) == TRUE)
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+!endif
+
+  #
+  # Secure Boot dependencies
+  #
+!if $(SECURE_BOOT_ENABLE) == TRUE
   
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
-  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
 
   # re-use the UserPhysicalPresent() dummy implementation from the ovmf tree
   PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
 !else
   
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
@@ -404,10 +413,13 @@ [Components.common]
   
NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
   
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
   
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
   
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
   
NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
+!if $(NETWORK_IP6_ENABLE) == TRUE
+  
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
+!endif
   
HandleParsingLib|ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
   
BcfgCommandLib|ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf
 
 
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 742df3628577..885c6b14b844 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -32,10 +32,11 @@ [Defines]
   #
   # Defines for default states.  These can be changed on the command line.
   # -D FLAG=VALUE
   #
   DEFINE SECURE_BOOT_ENABLE  = FALSE
+  DEFINE NETWORK_IP6_ENABLE  = FALSE
   DEFINE HTTP_BOOT_ENABLE= FALSE
 
 !include ArmVirtPkg/ArmVirt.dsc.inc
 
 [LibraryClasses.common]
@@ -342,14 +343,24 @@ [Components.common]
   MdeModulePkg/Universal/Network/Dhcp4Dxe/Dhcp4Dxe.inf
   MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Dxe.inf
   MdeModulePkg/Universal/Network/MnpDxe/MnpDxe.inf
   

[edk2] [PATCH 1/2] ArmVirtPkg: unify HttpLib resolutions in "ArmVirt.dsc.inc"

[Laszlo Ersek]

We already resolve a number of networking-related library classes in
ArmVirt.dsc.inc; follow suit with HttpLib.

Cc: Ard Biesheuvel 
Cc: Julien Grall 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1007
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 ArmVirtPkg/ArmVirt.dsc.inc   | 3 +++
 ArmVirtPkg/ArmVirtQemu.dsc   | 4 
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 4 
 3 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 7464ac70ed1b..76c400a5de21 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -78,10 +78,13 @@ [LibraryClasses.common]
   # Networking Requirements
   NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
   DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
   UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
   IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
+!if $(HTTP_BOOT_ENABLE) == TRUE
+  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
+!endif
 
   #
   # It is not possible to prevent the ARM compiler from inserting calls to 
intrinsic functions.
   # This library provides the instrinsic functions such a compiler may 
generate calls to.
   #
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 744d127a1033..742df3628577 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -62,14 +62,10 @@ [LibraryClasses.common]
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
   
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
   
PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-!endif
-
 [LibraryClasses.common.PEIM]
   
ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index e59f53b72884..4ae61ec635eb 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -62,14 +62,10 @@ [LibraryClasses.common]
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
   
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
   
PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
 
-!if $(HTTP_BOOT_ENABLE) == TRUE
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-!endif
-
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
 [BuildOptions.ARM.EDKII.SEC, BuildOptions.ARM.EDKII.BASE]
   # Avoid MOVT/MOVW instruction pairs in code that may end up in the PIE
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 0/2] ArmVirtPkg/ArmVirtQemu: enable the IPv6 stack

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: armvirt_ipv6_bz1007

Tested the "-D NETWORK_IP6_ENABLE" ArmVirtQemu build with PXEv4, PXEv6,
HTTPv4, HTTPv6 netboot.

Build-tested the ArmVirtQemuKernel build (32- and 64-bit), and the
ArmVirtXen build (64-bit).

Ard, if the series is OK, can you please push it for me as well? Thank
you.

Cc: Ard Biesheuvel 
Cc: Julien Grall 

Thanks
Laszlo

Laszlo Ersek (2):
  ArmVirtPkg: unify HttpLib resolutions in "ArmVirt.dsc.inc"
  ArmVirtPkg/ArmVirtQemu: enable the IPv6 stack

 ArmVirtPkg/ArmVirt.dsc.inc   | 21 +---
 ArmVirtPkg/ArmVirtQemu.dsc   | 17 +++-
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 17 +++-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 12 ++-
 4 files changed, 53 insertions(+), 14 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] Inquiry regarding early DxeIplPeim loading.

[Marvin H?user]

Good day developers,

While checking out which edk2 modules request being shadowed, I came across 
DxeIplPeim being one of them, however I am not sure why it was designed this 
way.

If the Boot Mode is != S3, the module will register for shadowing and 
immediately return during the pre-memory phase
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c#L92
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c#L111

If the Boot Mode is S3, the module will register a Memory Discovered event to 
install crucial PPIs...
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c#L125
... and install the DxeIpl PPI before returning
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c#L132

However, by design, the DxeIpl PPI is not located until the very end of 
PeiCore, meaning the dispatcher ran out of modules to dispatch
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c#L467
Hence installing the DxeIpl PPI early in the S3 boot path does not seem to have 
any effect to me, as both paths are left awaiting memory availability (Shadow / 
event). The only functional change would be PeiCore failing to locate the 
DxeIpl PPI in case memory initialization silently fails and code execution 
continues, which is an insane state in the first place.

Am I missing any scenario where this design is helpful? Is there any 
disadvantage for adding a Depex on MemoryDiscovered PPI? Running only after 
memory initialization would shrink the initialization function by removing the 
shadowing request in non-S3 path and the event registration in the S3 path, as 
well as merging the PPI installation code as both registrations end up 
executing the exact same code
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c#L118
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c#L57

The initialization function would collapse to PPI installations, a shadow or 
event registration call would be saved and platforms could safely embed 
DxeIplPeim into a Post Memory FV, such as MinPlatformPkg is using, to have the 
PEIM loaded directly into memory to gain yet more performance. The only 
restriction would be to prohibit compression.

Thanks for your time.

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

Re: [edk2] [Patch v2 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

[Laszlo Ersek]

On 07/12/18 23:59, Laszlo Ersek wrote:
> On 07/12/18 12:49, Eric Dong wrote:
>> Read uCode from memory has better performance than from flash.
>> But it needs extra effort to let BSP copy uCode from flash to
>> memory. Also BSP already enable cache in SEC phase, so it use
>> less time to relocate uCode from flash to memory. After
>> verification, if system has more than one processor, it will
>> reduce some time if load uCode from memory.
>>
>> This change enable this optimization.
>>
>> Cc: Laszlo Ersek 
>> Cc: Ruiyu Ni 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Eric Dong 
>> ---
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 +-
>>  1 file changed, 33 insertions(+), 1 deletion(-)
>>
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> index 108eea0a6f..c3cd6d7d51 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> @@ -1520,6 +1520,7 @@ MpInitLibInitialize (
>>UINTNApResetVectorSize;
>>UINTNBackupBufferAddr;
>>UINTNApIdtBase;
>> +  VOID *MicrocodePatchInRam;
>>  
>>OldCpuMpData = GetCpuMpDataFromGuidedHob ();
>>if (OldCpuMpData == NULL) {
>> @@ -1587,8 +1588,39 @@ MpInitLibInitialize (
>>CpuMpData->SwitchBspFlag= FALSE;
>>CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
>>CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + 
>> MaxLogicalProcessorNumber);
>> -  CpuMpData->MicrocodePatchAddress= PcdGet64 
>> (PcdCpuMicrocodePatchAddress);
>>CpuMpData->MicrocodePatchRegionSize = PcdGet64 
>> (PcdCpuMicrocodePatchRegionSize);
>> +  //
>> +  // If platform has more than one CPU, relocate microcode to memory to 
>> reduce
>> +  // loading microcode time.
>> +  //
>> +  MicrocodePatchInRam = NULL;
>> +  if (MaxLogicalProcessorNumber > 1) {
>> +MicrocodePatchInRam = AllocatePages (
>> +EFI_SIZE_TO_PAGES (
>> +  (UINTN)CpuMpData->MicrocodePatchRegionSize
>> +  )
>> +);
>> +ASSERT (MicrocodePatchInRam != NULL);
>> +  }
>> +  if (MicrocodePatchInRam == NULL) {
>> +//
>> +// there is only one processor, or no microcode patch is available, or
>> +// memory allocation failed
>> +//
>> +CpuMpData->MicrocodePatchAddress = PcdGet64 
>> (PcdCpuMicrocodePatchAddress);
>> +  } else {
>> +//
>> +// there are multiple processors, and a microcode patch is available, 
>> and
>> +// memory allocation succeeded
>> +//
>> +CopyMem (
>> +  MicrocodePatchInRam,
>> +  (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
>> +  (UINTN)CpuMpData->MicrocodePatchRegionSize
>> +  );
>> +CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
>> +  }
>> +
>>InitializeSpinLock(>MpLock);
>>  
>>//
>>
> 
> Reviewed-by: Laszlo Ersek 
> 

Sorry, I have to take that back -- please do not commit this patch.

For this version of the patch:

Nacked-by: Laszlo Ersek 

The ASSERT() is wrong. Again, in the code above, AllocatePages() can
return NULL not only because of memory allocation failure, but also
because the number of pages to allocate can be zero! If the platform has
no microcode patch to apply.

I knew this full well when I suggested the code, but then I forgot about
it when you mentioned the ASSERT(). I think you also knew about it, and
forgot about it too. :)

In particular, this patch would make it *impossible* to boot OVMF with
multiple processors, because OVMF *never* provides a microcode update.

So, please remove the ASSERT.

Alternatively, you could modify the ASSERT() like this:

  //
  // if we attempt actual memory allocation, we expect it to succeed
  //
  ASSERT (
(CpuMpData->MicrocodePatchRegionSize == 0) ||
(MicrocodePatchInRam != NULL)
);

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

Re: [edk2] [Patch v2 3/3] UefiCpuPkg/MpInitLib: Load uCode once for each core.

[Laszlo Ersek]

On 07/12/18 12:49, Eric Dong wrote:
> The SDM requires only one thread per core to load the
> microcode.
> 
> This change enables this solution.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 351975e2a2..122c23469d 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -61,6 +61,7 @@ MicrocodeDetect (
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
>UINT32  ProcessorFlags;
> +  UINT32  ThreadId;
>  
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
> @@ -77,6 +78,14 @@ MicrocodeDetect (
>  return;
>}
>  
> +  GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL, );
> +  if (ThreadId != 0) {
> +//
> +// Skip loading microcode if it is not the first thread in one core.
> +//
> +return;
> +  }
> +
>ExtendedTableLength = 0;
>//
>// Here data of CPUID leafs have not been collected into context buffer, so
> 

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

Re: [edk2] [Patch v2 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.

[Laszlo Ersek]

On 07/12/18 12:49, Eric Dong wrote:
> Search uCode costs much time, if AP has same processor type
> with BSP, AP can use BSP saved uCode info to get better performance.
> 
> This change enables this solution.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 34 
> +---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c |  4 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 +--
>  3 files changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index e47f9f4f8f..351975e2a2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -35,11 +35,13 @@ GetCurrentMicrocodeSignature (
>  /**
>Detect whether specified processor can find matching microcode patch and 
> load it.
>  
> -  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
> +  @param[in]  CpuMpDataThe pointer to CPU MP Data structure.
> +  @param[in]  IsBspCallIn  Indicate whether the caller is BSP or not.
>  **/
>  VOID
>  MicrocodeDetect (
> -  IN CPU_MP_DATA *CpuMpData
> +  IN CPU_MP_DATA *CpuMpData,
> +  IN BOOLEAN IsBspCallIn
>)
>  {
>UINT32  ExtendedTableLength;
> @@ -58,6 +60,7 @@ MicrocodeDetect (
>BOOLEAN CorrectMicrocode;
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> +  UINT32  ProcessorFlags;
>  
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
> @@ -67,7 +70,7 @@ MicrocodeDetect (
>}
>  
>CurrentRevision = GetCurrentMicrocodeSignature ();
> -  if (CurrentRevision != 0) {
> +  if (CurrentRevision != 0 && !IsBspCallIn) {
>  //
>  // Skip loading microcode if it has been loaded successfully
>  //
> @@ -87,6 +90,19 @@ MicrocodeDetect (
>PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
>PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
>  
> +  //
> +  // Check whether AP has same processor with BSP.
> +  // If yes, direct use microcode info saved by BSP.
> +  //
> +  if (!IsBspCallIn) {
> +if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
> +(CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
> +MicrocodeData = (VOID *)(UINTN) CpuMpData->MicrocodeDataAddress;
> +LatestRevision = CpuMpData->MicrocodeRevision;
> +goto Done;
> +}
> +  }
> +
>LatestRevision = 0;
>MicrocodeData  = NULL;
>MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + 
> CpuMpData->MicrocodePatchRegionSize);
> @@ -117,6 +133,7 @@ MicrocodeDetect (
>  }
>  if (CheckSum32 == 0) {
>CorrectMicrocode = TRUE;
> +  ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
>  }
>} else if ((MicrocodeEntryPoint->DataSize != 0) &&
>   (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
> @@ -151,6 +168,7 @@ MicrocodeDetect (
>  // Find one
>  //
>  CorrectMicrocode = TRUE;
> +ProcessorFlags = ExtendedTable->ProcessorFlag;
>  break;
>}
>  }
> @@ -188,6 +206,7 @@ MicrocodeDetect (
>  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) 
> MicrocodeEntryPoint) + TotalSize);
>} while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
>  
> +Done:
>if (LatestRevision > CurrentRevision) {
>  //
>  // BIOS only authenticate updates that contain a numerically larger 
> revision
> @@ -211,4 +230,13 @@ MicrocodeDetect (
>ReleaseSpinLock(>MpLock);
>  }
>}
> +
> +  if (IsBspCallIn && (LatestRevision != 0)) {
> +CpuMpData->ProcessorSignature = Eax.Uint32;
> +CpuMpData->ProcessorFlags = ProcessorFlags;
> +CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
> +CpuMpData->MicrocodeRevision = LatestRevision;
> +DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags 
> [0x%08x], \
> +   MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, 
> (UINTN) MicrocodeData, LatestRevision));
> +  }
>  }
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index c3cd6d7d51..3a7bdfa00b 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -410,7 +410,7 @@ ApInitializeSync (
>//
>// Load microcode on AP
>//
> -  MicrocodeDetect (CpuMpData);
> +  MicrocodeDetect (CpuMpData, FALSE);
>//
>// Sync BSP's MTRR table to AP
>//
> @@ -1659,7 +1659,7 @@ MpInitLibInitialize (
>//
>// Load Microcode on BSP
>//
> -  MicrocodeDetect (CpuMpData);
> +  

Re: [edk2] [Patch v2 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

[Laszlo Ersek]

On 07/12/18 12:49, Eric Dong wrote:
> Read uCode from memory has better performance than from flash.
> But it needs extra effort to let BSP copy uCode from flash to
> memory. Also BSP already enable cache in SEC phase, so it use
> less time to relocate uCode from flash to memory. After
> verification, if system has more than one processor, it will
> reduce some time if load uCode from memory.
> 
> This change enable this optimization.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 +-
>  1 file changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 108eea0a6f..c3cd6d7d51 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1520,6 +1520,7 @@ MpInitLibInitialize (
>UINTNApResetVectorSize;
>UINTNBackupBufferAddr;
>UINTNApIdtBase;
> +  VOID *MicrocodePatchInRam;
>  
>OldCpuMpData = GetCpuMpDataFromGuidedHob ();
>if (OldCpuMpData == NULL) {
> @@ -1587,8 +1588,39 @@ MpInitLibInitialize (
>CpuMpData->SwitchBspFlag= FALSE;
>CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
>CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + 
> MaxLogicalProcessorNumber);
> -  CpuMpData->MicrocodePatchAddress= PcdGet64 
> (PcdCpuMicrocodePatchAddress);
>CpuMpData->MicrocodePatchRegionSize = PcdGet64 
> (PcdCpuMicrocodePatchRegionSize);
> +  //
> +  // If platform has more than one CPU, relocate microcode to memory to 
> reduce
> +  // loading microcode time.
> +  //
> +  MicrocodePatchInRam = NULL;
> +  if (MaxLogicalProcessorNumber > 1) {
> +MicrocodePatchInRam = AllocatePages (
> +EFI_SIZE_TO_PAGES (
> +  (UINTN)CpuMpData->MicrocodePatchRegionSize
> +  )
> +);
> +ASSERT (MicrocodePatchInRam != NULL);
> +  }
> +  if (MicrocodePatchInRam == NULL) {
> +//
> +// there is only one processor, or no microcode patch is available, or
> +// memory allocation failed
> +//
> +CpuMpData->MicrocodePatchAddress = PcdGet64 
> (PcdCpuMicrocodePatchAddress);
> +  } else {
> +//
> +// there are multiple processors, and a microcode patch is available, and
> +// memory allocation succeeded
> +//
> +CopyMem (
> +  MicrocodePatchInRam,
> +  (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
> +  (UINTN)CpuMpData->MicrocodePatchRegionSize
> +  );
> +CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
> +  }
> +
>InitializeSpinLock(>MpLock);
>  
>//
> 

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

Re: [edk2] [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/USB

[Laszlo Ersek]

On 07/12/18 14:07, Yao, Jiewen wrote:
> thanks laszlo 
> I like your idea to eliminate the code duplication and fix all the problem by 
> adding a new api in uefi lib. 
> 
> If there is urgency to fix this specific issue, I have no problem on the 
> enhancement. 
> 
> Reviewed by : jiewen@intel.com
> 
> thank you!
> Yao, Jiewen

Commit 79b10d4ce4f0.

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

Re: [edk2] [Patch] ShellPkg/TftpDynamicCommand: Fix the potential assertion and memory leak issue.

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Fu, Siyuan
> Sent: Wednesday, July 11, 2018 5:46 PM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Carsey, Jaben 
> Subject: RE: [Patch] ShellPkg/TftpDynamicCommand: Fix the potential
> assertion and memory leak issue.
> Importance: High
> 
> 
> 
> Reviewed-by: Fu Siyuan 
> 
> > -Original Message-
> > From: Wu, Jiaxin
> > Sent: Thursday, July 12, 2018 8:44 AM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Jiaxin ; Ye, Ting ; Fu,
> > Siyuan ; Carsey, Jaben 
> > Subject: [Patch] ShellPkg/TftpDynamicCommand: Fix the potential
> assertion
> > and memory leak issue.
> >
> > From: Jiaxin Wu 
> >
> > This patch is to fix the issue reported from
> > https://bugzilla.tianocore.org/show_bug.cgi?id=925.
> >
> > DataSize variable was not assigned the value if ShellOpenFileByName
> > returns error.
> > In the such a case, it should not be used to FreePages. Instead, DataSize
> > can be
> > used to record the file size once DownloadFile successfully.
> >
> > Cc: Ye Ting 
> > Cc: Fu Siyuan 
> > Cc: Jaben Carsey 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Wu Jiaxin 
> > ---
> >  ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > index e2491cd54c..44be6d4e76 100644
> > --- a/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > +++ b/ShellPkg/DynamicCommand/TftpDynamicCommand/Tftp.c
> > @@ -517,10 +517,12 @@ RunTftp (
> >  mTftpHiiHandle, RemoteFilePath, NicName, Status
> >);
> >goto NextHandle;
> >  }
> >
> > +DataSize = FileSize;
> > +
> >  if (!EFI_ERROR (ShellFileExists (LocalFilePath))) {
> >ShellDeleteFileByName (LocalFilePath);
> >  }
> >
> >  Status = ShellOpenFileByName (
> > @@ -537,11 +539,10 @@ RunTftp (
> >  mTftpHiiHandle, L"tftp", LocalFilePath
> >);
> >goto NextHandle;
> >  }
> >
> > -DataSize = FileSize;
> >  Status = ShellWriteFile (FileHandle, , Data);
> >  if (!EFI_ERROR (Status)) {
> >ShellStatus = SHELL_SUCCESS;
> >  } else {
> >ShellPrintHiiEx (
> > --
> > 2.17.1.windows.2

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

Re: [edk2] [platforms: PATCH 3/6] Marvell/Library: Armada7k8kSoCDescLib: Enable getting CP base address

[Ard Biesheuvel]

On 12 July 2018 at 09:39, Marcin Wojtas  wrote:
> For upcoming patches there is a need to get the CP110 base address,
> introduce according getter function for it.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h   
>   |  6 ++
>  
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>  | 11 +++
>  2 files changed, 17 insertions(+)
>
> diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
> b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> index d2bcf2a..56efdbe 100644
> --- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> +++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
> @@ -36,6 +36,12 @@ ArmadaSoCDescComPhyGet (
>IN OUT UINTN*DescCount
>);
>
> +UINTN

If this is a memory address, you should use EFI_PHYSICAL_ADDRESS here.

> +EFIAPI
> +ArmadaSoCDescCpBaseGet (
> +  IN UINTN  CpIndex
> +  );
> +
>  //
>  // I2C
>  //
> diff --git 
> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
>  
> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> index 6ce6bad..c7c9c13 100644
> --- 
> a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> +++ 
> b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
> @@ -61,6 +61,17 @@ ArmadaSoCDescComPhyGet (
>return EFI_SUCCESS;
>  }
>
> +UINTN
> +EFIAPI
> +ArmadaSoCDescCpBaseGet (
> +  IN UINTN  CpIndex
> +  )
> +{
> +  ASSERT (CpIndex < FixedPcdGet8 (PcdMaxCpCount));
> +
> +  return MV_SOC_CP_BASE (CpIndex);
> +}
> +
>  EFI_STATUS
>  EFIAPI
>  ArmadaSoCDescI2cGet (
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH 2/6] Marvell/Library: Introduce ArmadaIcuLib class

[Ard Biesheuvel]

On 12 July 2018 at 09:39, Marcin Wojtas  wrote:
> ICU (Interrupt Consolidation Unit) is a mechanism,
> that allows to send a message-based interrupts from the
> CP110 unit (South Bridge) to the Application Processor
> hardware block. After dispatching the interrupts in the
> GIC are generated.
>
> This patch adds a basic version of the library, that
> allows to configure a static mapping between CP110
> interfaces and GIC. It is required for the cases, where
> the OS does not support the ICU controller on its own
> (e.g. ACPI boot).
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Marvell.dec|  1 +
>  Silicon/Marvell/Include/Library/ArmadaIcuLib.h | 45 
>  2 files changed, 46 insertions(+)
>  create mode 100644 Silicon/Marvell/Include/Library/ArmadaIcuLib.h
>
> diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
> index 4def897..616624e 100644
> --- a/Silicon/Marvell/Marvell.dec
> +++ b/Silicon/Marvell/Marvell.dec
> @@ -61,6 +61,7 @@
>
>  [LibraryClasses]
>ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h
> +  ArmadaIcuLib|Include/Library/ArmadaIcuLib.h
>ArmadaSoCDescLib|Include/Library/ArmadaSoCDescLib.h
>SampleAtResetLib|Include/Library/SampleAtResetLib.h
>
> diff --git a/Silicon/Marvell/Include/Library/ArmadaIcuLib.h 
> b/Silicon/Marvell/Include/Library/ArmadaIcuLib.h
> new file mode 100644
> index 000..9b68934
> --- /dev/null
> +++ b/Silicon/Marvell/Include/Library/ArmadaIcuLib.h
> @@ -0,0 +1,45 @@
> +/**
> +*
> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates
> +*
> +*  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.
> +*
> +**/
> +#ifndef __ARMADA_ICU_LIB_H__
> +#define __ARMADA_ICU_LIB_H__
> +
> +typedef enum {
> +  Level = 0,
> +  Edge = 1

Could we have more discriminating identifiers here please?
IcuIrqTypeLevel and IcuIrqTypeEdge perhaps?

> +} ICU_IRQ_TYPE;
> +
> +typedef struct {
> +  UINTN IcuId;
> +  UINTN SpiId;
> +  ICU_IRQ_TYPE IrqType;
> +} ICU_IRQ;
> +
> +typedef struct {
> +  const ICU_IRQ  *Map;
> +  UINTN   Size;
> +} ICU_CONFIG_ENTRY;
> +
> +typedef struct {
> +  ICU_CONFIG_ENTRY NonSecure;
> +  ICU_CONFIG_ENTRY Sei;
> +  ICU_CONFIG_ENTRY Rei;
> +} ICU_CONFIG;
> +
> +EFI_STATUS
> +EFIAPI
> +ArmadaIcuInitialize (
> +  VOID
> +  );
> +
> +#endif /* __ARMADA_ICU_LIB_H__ */
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] MdeModulePkg/PerformanceMeasurement.h: Correct the license

[Ard Biesheuvel]

On 12 July 2018 at 16:08, Dandan Bi  wrote:
> Corrected to use the BSD license.
>
> Cc: Liming Gao 
> Cc: Star Zeng 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 

This needs a Reviewed-by from someone at Microsoft.

> ---
>  MdeModulePkg/Include/Guid/PerformanceMeasurement.h | 31 
> +++---
>  1 file changed, 9 insertions(+), 22 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Guid/PerformanceMeasurement.h 
> b/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
> index f2955c7..0c3eb59 100644
> --- a/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
> +++ b/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
> @@ -1,31 +1,18 @@
>  /** @file
>
> -Copyright (c) 2017, Microsoft Corporation
> -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Performance measurement protocol, allows logging performance data.
>
> -All rights reserved.
> -Redistribution and use in source and binary forms, with or without
> -modification, are permitted provided that the following conditions are met:
> -1. Redistributions of source code must retain the above copyright notice,
> -this list of conditions and the following disclaimer.
> -2. Redistributions in binary form must reproduce the above copyright notice,
> -this list of conditions and the following disclaimer in the documentation
> - and/or other materials provided with the distribution.
> -
> -THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" 
> AND
> -ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> -WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE 
> DISCLAIMED.
> -IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY 
> DIRECT,
> -INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES 
> (INCLUDING,
> -BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> -DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY 
> OF
> -LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING 
> NEGLIGENCE
> -OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> -ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> +Copyright (c) 2017, Microsoft Corporation
> +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 that accompanies this 
> distribution.
> +The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php.
>
> -Performance measurement protocol, allows logging performance data.
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
>  **/
>
>  #ifndef _PERFORMANCE_MEASUREMENT_H_
>  #define _PERFORMANCE_MEASUREMENT_H_
> --
> 1.9.5.msysgit.1
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [patch] MdeModulePkg/PerformanceMeasurement.h: Correct the license

[Dandan Bi]

Corrected to use the BSD license.

Cc: Liming Gao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Include/Guid/PerformanceMeasurement.h | 31 +++---
 1 file changed, 9 insertions(+), 22 deletions(-)

diff --git a/MdeModulePkg/Include/Guid/PerformanceMeasurement.h 
b/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
index f2955c7..0c3eb59 100644
--- a/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
+++ b/MdeModulePkg/Include/Guid/PerformanceMeasurement.h
@@ -1,31 +1,18 @@
 /** @file
 
-Copyright (c) 2017, Microsoft Corporation
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Performance measurement protocol, allows logging performance data.
 
-All rights reserved.
-Redistribution and use in source and binary forms, with or without
-modification, are permitted provided that the following conditions are met:
-1. Redistributions of source code must retain the above copyright notice,
-this list of conditions and the following disclaimer.
-2. Redistributions in binary form must reproduce the above copyright notice,
-this list of conditions and the following disclaimer in the documentation
- and/or other materials provided with the distribution.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE 
DISCLAIMED.
-IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY 
DIRECT,
-INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
-BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
-LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE
-OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
-ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+Copyright (c) 2017, Microsoft Corporation
+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 that accompanies this distribution.
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.
 
-Performance measurement protocol, allows logging performance data.
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
 **/
 
 #ifndef _PERFORMANCE_MEASUREMENT_H_
 #define _PERFORMANCE_MEASUREMENT_H_
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH v2] SecurityPkg: Fix assert when setting key from eMMC/SD/USB

[Yao, Jiewen]

thanks laszlo 
I like your idea to eliminate the code duplication and fix all the problem by 
adding a new api in uefi lib. 

If there is urgency to fix this specific issue, I have no problem on the 
enhancement. 

Reviewed by : jiewen@intel.com

thank you!
Yao, Jiewen


> 在 2018年7月12日，上午5:06，Laszlo Ersek  写道：
> 
>> On 07/11/18 18:06, Laszlo Ersek wrote:
>>> On 07/11/18 17:44, Roman Bacik wrote:
>>> Hi Laszlo,
>>> 
>>> Thank you very much for your review and help. I would prefer the option 2b.
>> 
>> Great, thanks! Let's wait for the SecurityPkg maintainers then, to give
>> their R-b's for your patch. Chao Zhang, Jiewen, can you please comment?
>> 
>> From my side, dependent on the pending commit message and patch
>> whitespace corrections (which I'm willing to implement myself, at push):
>> 
>> Reviewed-by: Laszlo Ersek 
> 
> Here's the TianoCore BZ that I plan to reference in the updated commit
> message, as "further known problems":
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1008
> 
> Thanks
> Laszlo
> 
> 
>>> Thanks,
>>> 
>>> Roman
>>> 
 On Wed, Jul 11, 2018 at 5:05 AM, Laszlo Ersek  wrote:
 
 Hi Roman,
 
> On 07/11/18 00:51, rba...@gmail.com wrote:
> From: Roman Bacik 
> 
> When secure boot is enabled, if one loads keys from a FAT formatted
> eMMC/SD/USB when trying to provision PK/KEK/DB keys via the menu,
> an assert in StrLen() occurs.
> This is because the filename starts on odd address, which is not a uint16
> aligned boundary: https://bugzilla.tianocore.org/show_bug.cgi?id=1003
> 
> Cc: Chao Zhang 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Cc: Vladimir Olovyannikov 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Roman Bacik 
> ---
> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
 | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
 
 Thank you for sending a well-formed patch.
 
 I notice that you sent this email from , which is not
 the same as the Signed-off-by line. I realize you posted from
  for technical reasons, and it should be no problem.
 
 However, I *think* in such cases we usually request the following:
 
 - Using your broadcom.com email address, please respond to this patch
 (not my present email, but your original git posting), keeping full
 context, and just repeat your Signed-off-by line (referencing the
 broadcom address).
 
 I'm CC'ing Jordan and Ard for confirmation -- I believe this is what
 we've done in the past, in cases when submitters had to post their work
 from private addresses due to company email issues.
 
 Technical comments below:
 
> diff --git 
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigFileExplorer.c
 b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
 SecureBootConfigFileExplorer.c
> index 1b6f88804275..19b13a5569a6 100644
> --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
 SecureBootConfigFileExplorer.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/
 SecureBootConfigFileExplorer.c
> @@ -123,6 +123,8 @@ OpenFileByDevicePath(
>   EFI_FILE_PROTOCOL   *Handle1;
>   EFI_FILE_PROTOCOL   *Handle2;
>   EFI_HANDLE  DeviceHandle;
> +  CHAR16  *PathName;
> +  UINTN   PathLength;
> 
>   if ((FilePath == NULL || FileHandle == NULL)) {
> return EFI_INVALID_PARAMETER;
> @@ -173,6 +175,11 @@ OpenFileByDevicePath(
> //
> Handle2  = Handle1;
> Handle1 = NULL;
> +PathLength = DevicePathNodeLength(*FilePath) -
 sizeof(EFI_DEVICE_PATH_PROTOCOL);
> +PathName = AllocateCopyPool(PathLength, ((FILEPATH_DEVICE_PATH*)*
 FilePath)->PathName);
 
 (1) On both lines above, space characters are missing after:
 DevicePathNodeLength, sizeof, and AllocateCopyPool. (Edk2 coding style.)
 I think we can fix this up for you when we push the patch. (I'm willing
 to help with that, but we need SecurityPkg maintainer review first.)
 
 
> +if (PathName == NULL) {
> +  return EFI_OUT_OF_RESOURCES;
> +}
 
 (2) I have now reviewed the original state of the function more
 carefully, and, while the above "return" branch introduces a leak
 *path*, it does not introduce a leak that doesn't already exist!
 
 In fact, the original function has multiple issues:
 
 - If the OpenVolume() call fails, "FileHandle" is set to NULL. That's
 useless; the intent is obviously to set (*FileHandle) to NULL.
 
 - At the top of the "while" loop body, "Handle1" stands for an open
 EFI_FILE_PROTOCOL. If the device path type check at the top of the loop
 body returns 

Re: [edk2] [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib

[Leif Lindholm]

On Thu, Jul 12, 2018 at 12:59:16PM +0200, Marcin Wojtas wrote:
> Hi Leif,
> 
> 
> 
> 2018-07-12 12:35 GMT+02:00 Leif Lindholm :
> > On Thu, Jul 12, 2018 at 09:40:00AM +0200, Marcin Wojtas wrote:
> >> ICU (Interrupt Consolidation Unit) is a mechanism,
> >> that allows to send-message based interrupts from the
> >> CP110 unit (South Bridge) to the Application Processor
> >> hardware block. After dispatching the interrupts in the
> >> GIC are generated.
> >>
> >> This patch adds a basic version of the library, that
> >> allows to configure a static mapping between CP110
> >> interfaces and GICv2 of the Armada7k8k SoC family.
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Marcin Wojtas 
> >> ---
> >>  Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
> >>  Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
> >>  Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 
> >>  3 files changed, 399 insertions(+)
> >>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
> >>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
> >>
> >> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf 
> >> b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >> new file mode 100644
> >> index 000..0010141
> >> --- /dev/null
> >> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >> @@ -0,0 +1,38 @@
> >> +## @file
> >> +#
> >> +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
> >> +#
> >> +#  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.
> >> +#
> >> +##
> >> +
> >> +[Defines]
> >> +  INF_VERSION= 0x0001001A
> >> +  BASE_NAME  = IcuLib
> >> +  FILE_GUID  = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
> >> +  MODULE_TYPE= BASE
> >> +  VERSION_STRING = 1.0
> >> +  LIBRARY_CLASS  = ArmadaIcuLib
> >> +
> >> +[Sources]
> >> +  IcuLib.c
> >> +
> >> +[Packages]
> >> +  MdeModulePkg/MdeModulePkg.dec
> >> +  MdePkg/MdePkg.dec
> >> +  Silicon/Marvell/Marvell.dec
> >> +
> >> +[LibraryClasses]
> >> +  ArmadaSoCDescLib
> >> +  DebugLib
> >> +  IoLib
> >> +  PcdLib
> >> +
> >> +[FixedPcd]
> >> +  gMarvellTokenSpaceGuid.PcdMaxCpCount
> >> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h 
> >> b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> >> new file mode 100644
> >> index 000..4bf2298
> >> --- /dev/null
> >> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> >> @@ -0,0 +1,46 @@
> >> +/**
> >> +*
> >> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
> >> +*
> >> +*  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.
> >> +*
> >> +*  Glossary - abbreviations used in Marvell SampleAtReset library 
> >> implementation:
> >> +*  ICU - Interrupt Consolidation Unit
> >> +*  AP - Application Processor hardware block (Armada 7k8k incorporates 
> >> AP806)
> >> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
> >> +*
> >> +**/
> >> +
> >> +#include 
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#define ICU_REG_BASE(Cp)ArmadaSoCDescCpBaseGet (CpIndex) + 
> >> 0x1E
> >
> > Macro missing parentheses.
> >
> >> +
> >> +#define ICU_SET_SPI_AL(x)   (0x10 + (0x10 * x))
> >> +#define ICU_SET_SPI_AH(x)   (0x14 + (0x10 * x))
> >> +#define ICU_CLR_SPI_AL(x)   (0x18 + (0x10 * x))
> >> +#define ICU_CLR_SPI_AH(x)   (0x1c + (0x10 * x))
> >
> > Can that 0x10 be given a descriptive #define?
> >
> 
> 0x10 is just a part of register offset calculation formula. If you
> wish to replace it with macro, how about:
> #define ICU_GROUP_REGISTER_BASE_SHIFT0x10
> ?

Not SHIFT (which suggests << or >> to me), but OFFSET would work.

> >> +#define ICU_INT_CFG(x)  (0x100 + 4 * x)
> >
> > Does this 4 signify sizeof (UINT32) - i.e. does it get the offset into
> > a series of 32-bit registers? If so, please use that instead.
> 
> Ok.
> 
> > Also, please use parentheses around multiplication consistently
> > (either 

Re: [edk2] [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib

[Marcin Wojtas]

Hi Leif,



2018-07-12 12:35 GMT+02:00 Leif Lindholm :
> On Thu, Jul 12, 2018 at 09:40:00AM +0200, Marcin Wojtas wrote:
>> ICU (Interrupt Consolidation Unit) is a mechanism,
>> that allows to send-message based interrupts from the
>> CP110 unit (South Bridge) to the Application Processor
>> hardware block. After dispatching the interrupts in the
>> GIC are generated.
>>
>> This patch adds a basic version of the library, that
>> allows to configure a static mapping between CP110
>> interfaces and GICv2 of the Armada7k8k SoC family.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas 
>> ---
>>  Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
>>  Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
>>  Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 
>>  3 files changed, 399 insertions(+)
>>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
>>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
>>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
>>
>> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf 
>> b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
>> new file mode 100644
>> index 000..0010141
>> --- /dev/null
>> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
>> @@ -0,0 +1,38 @@
>> +## @file
>> +#
>> +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
>> +#
>> +#  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.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION= 0x0001001A
>> +  BASE_NAME  = IcuLib
>> +  FILE_GUID  = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
>> +  MODULE_TYPE= BASE
>> +  VERSION_STRING = 1.0
>> +  LIBRARY_CLASS  = ArmadaIcuLib
>> +
>> +[Sources]
>> +  IcuLib.c
>> +
>> +[Packages]
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  Silicon/Marvell/Marvell.dec
>> +
>> +[LibraryClasses]
>> +  ArmadaSoCDescLib
>> +  DebugLib
>> +  IoLib
>> +  PcdLib
>> +
>> +[FixedPcd]
>> +  gMarvellTokenSpaceGuid.PcdMaxCpCount
>> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h 
>> b/Silicon/Marvell/Library/IcuLib/IcuLib.h
>> new file mode 100644
>> index 000..4bf2298
>> --- /dev/null
>> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
>> @@ -0,0 +1,46 @@
>> +/**
>> +*
>> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
>> +*
>> +*  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.
>> +*
>> +*  Glossary - abbreviations used in Marvell SampleAtReset library 
>> implementation:
>> +*  ICU - Interrupt Consolidation Unit
>> +*  AP - Application Processor hardware block (Armada 7k8k incorporates 
>> AP806)
>> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
>> +*
>> +**/
>> +
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define ICU_REG_BASE(Cp)ArmadaSoCDescCpBaseGet (CpIndex) + 0x1E
>
> Macro missing parentheses.
>
>> +
>> +#define ICU_SET_SPI_AL(x)   (0x10 + (0x10 * x))
>> +#define ICU_SET_SPI_AH(x)   (0x14 + (0x10 * x))
>> +#define ICU_CLR_SPI_AL(x)   (0x18 + (0x10 * x))
>> +#define ICU_CLR_SPI_AH(x)   (0x1c + (0x10 * x))
>
> Can that 0x10 be given a descriptive #define?
>

0x10 is just a part of register offset calculation formula. If you
wish to replace it with macro, how about:
#define ICU_GROUP_REGISTER_BASE_SHIFT0x10
?


>> +#define ICU_INT_CFG(x)  (0x100 + 4 * x)
>
> Does this 4 signify sizeof (UINT32) - i.e. does it get the offset into
> a series of 32-bit registers? If so, please use that instead.

Ok.

> Also, please use parentheses around multiplication consistently
> (either not at all or everywhere).
>

In previous series (board description) you recommended using the
parentheses, so I'll follow this guideline consistently.

>> +
>> +#define ICU_INT_ENABLE_OFFSET24
>> +#define ICU_IS_EDGE_OFFSET   28
>> +#define ICU_GROUP_OFFSET 29
>> +
>> +#define ICU_MAX_SUPPORTED_UNITS  2
>> +#define ICU_MAX_IRQS_PER_CP  64
>> +
>> +#define MAX_ICU_IRQS 207
>> diff --git 

Re: [edk2] [Patch 0/3] Optimize load uCode performance

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, July 12, 2018 5:58 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch 0/3] Optimize load uCode performance
> 
> On 07/11/18 13:07, Eric Dong wrote:
> > Use below three rules to optimize load uCode performance:
> > 1. Let BSP relocate uCode from flash to memory for better performance.
> > 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look
> >for the uCode again if the CPU ID is same as BSP’s.
> > 3. Only apply uCode in one thread of a core when hyper threading is
> enabled.
> >
> > Test:
> > Use an sample platform which has 1 socket, 4 core, 8 threads, the
> > CpuMpPei driver cost time reduce from 108.4ms to 27.2ms
> >
> > Eric Dong (3):
> >   UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.
> >   UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
> >   UefiCpuPkg/MpInitLib: Load uCode once for one core.
> >
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 43
> +---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c | 17 ++---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 ++--
> >  3 files changed, 63 insertions(+), 8 deletions(-)
> >
> >
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> 
> I also tried to regression test this with OVMF. (I know I requested some
> changes for patch #1, however those would only affect behavior in practice
> when memory allocation fails, so it makes sense for me to test this version of
> the series as well.)
> 
> Unfortunately, the patch set doesn't apply; it fails with the first patch 
> already.
> I tried on top of current master, commit 0a563f3fecfd
> ("BaseTool: Fixed the incorrect cache key.", 2018-07-12).
> 
> Can you please post a v2 that is also rebased on current master?
> 

I just send v2 patches base on the latest codebase, please verify with the 
newer one.

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

Re: [edk2] [Patch 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, July 12, 2018 5:26 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: Re: [edk2] [Patch 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to
> memory to save time.
> 
> Hi Eric,
> 
> On 07/11/18 13:07, Eric Dong wrote:
> > Read uCode from memory has better performance than from flash.
> > But it needs extra effort to let BSP copy uCode from flash to memory.
> > Also BSP already enable cache in SEC phase, so it use less time to
> > relocate uCode from flash to memory. After verification, if system has
> > more than one processor, it will reduce some time if load uCode from
> > memory.
> >
> > This change enable this optimization.
> >
> > Cc: Laszlo Ersek 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 -
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index eec178b419..8b458a4a3a 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -1560,8 +1560,19 @@ MpInitLibInitialize (
> >CpuMpData->SwitchBspFlag= FALSE;
> >CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
> >CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData
> + MaxLogicalProcessorNumber);
> > -  CpuMpData->MicrocodePatchAddress= PcdGet64
> (PcdCpuMicrocodePatchAddress);
> >CpuMpData->MicrocodePatchRegionSize = PcdGet64
> > (PcdCpuMicrocodePatchRegionSize);
> > +  //
> > +  // if platform has more than one CPU, relocate microcode to memory to
> reduce loading microcode time.
> > +  //
> > +  if (MaxLogicalProcessorNumber > 1) {
> > +CpuMpData->MicrocodePatchAddress = (UINT64) (UINTN) AllocatePages
> (EFI_SIZE_TO_PAGES ((UINTN)CpuMpData->MicrocodePatchRegionSize));
> > +if (CpuMpData->MicrocodePatchAddress != 0) {
> > +  CopyMem ((VOID *) (UINTN)CpuMpData->MicrocodePatchAddress,
> (VOID *)(UINTN)(PcdGet64 (PcdCpuMicrocodePatchAddress)),
> (UINTN)CpuMpData->MicrocodePatchRegionSize);
> > +}
> > +  } else {
> > +CpuMpData->MicrocodePatchAddress = PcdGet64
> > + (PcdCpuMicrocodePatchAddress);  }
> > +
> >InitializeSpinLock(>MpLock);
> >//
> >// Save BSP's Control registers to APs
> >
> 
> (1) Can you please break up the added lines to multiple lines? They are
> extremely long, and difficult for me to handle. It should be possible to break
> up both AllocatePages() and CopyMem(), for example.
> 
> (2) The code appears to handle the case well when
> PcdCpuMicrocodePatchRegionSize is zero. In that case,
> EFI_SIZE_TO_PAGES(...) evaluates to zero, and -- I checked --
> AllocatePages() returns NULL. In this case, allocation and copying will not 
> take
> place, and that's fine -- there is nothing to copy and no microcode to 
> install.
> So, OK.
> 

Yes, my original patch has PcdCpuMicrocodePatchRegionSize check, but after 
check I found if it is zero, all the allocate action will do nothing, so I 
removed it.

> (3) However, if there is a microcode update available, but we can't allocate
> memory, things will go wrong. The region size is nonzero, thus
> MicrocodeDetect() will not exit early. But MicrocodePatchAddress will be set
> to 0.
> 

Agree.

> So, I suggest the following instead:
> 
> -
>   VOID *MicrocodePatchInRam;
> 
>   //
>   // If platform has more than one CPU, relocate microcode to memory to
> reduce
>   // loading microcode time.
>   //
>   MicrocodePatchInRam = NULL;
>   if (MaxLogicalProcessorNumber > 1) {
> MicrocodePatchInRam = AllocatePages (
> EFI_SIZE_TO_PAGES (
>   (UINTN)CpuMpData->MicrocodePatchRegionSize
>   )
> );
>   }
>   if (MicrocodePatchInRam == NULL) {
> //
> // there is only one processor, or no microcode patch is available, or
> // memory allocation failed
> //
> CpuMpData->MicrocodePatchAddress = PcdGet64
> (PcdCpuMicrocodePatchAddress);
>   } else {
> //
> // there are multiple processors, and a microcode patch is available, and
> // memory allocation succeeded
> //
> CopyMem (
>   MicrocodePatchInRam,
>   (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
>   (UINTN)CpuMpData->MicrocodePatchRegionSize
>   );
> CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
>   }
> -
> 

Thanks for your sample code, I directly use it.  Also I think allocate memory 
failed is an abnormal case, so I add an ASSERT in the new patch.

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

[edk2] [Patch v2 0/3] Optimize load uCode performance

[Eric Dong]

Use below three rules to optimize load uCode performance:
1. Let BSP relocate uCode from flash to memory for better performance.
2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look 
   for the uCode again if the CPU ID is same as BSP’s.
3. Only apply uCode in one thread of a core when hyper threading is enabled.

v2 changes:
  Fix potential issue if allocate memory failed.

Test:
Use an sample platform which has 1 socket, 4 core, 8 threads, the 
CpuMpPei driver cost time reduce from 108.4ms to 27.2ms


Eric Dong (3):
  UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
  UefiCpuPkg/MpInitLib: Load uCode once for each core.
  UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

 UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 +---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 38 +---
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 ++--
 3 files changed, 84 insertions(+), 8 deletions(-)

-- 
2.15.0.windows.1

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

[edk2] [Patch v2 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.

[Eric Dong]

Search uCode costs much time, if AP has same processor type
with BSP, AP can use BSP saved uCode info to get better performance.

This change enables this solution.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 34 +---
 UefiCpuPkg/Library/MpInitLib/MpLib.c |  4 ++--
 UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 +--
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index e47f9f4f8f..351975e2a2 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -35,11 +35,13 @@ GetCurrentMicrocodeSignature (
 /**
   Detect whether specified processor can find matching microcode patch and 
load it.
 
-  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
+  @param[in]  CpuMpDataThe pointer to CPU MP Data structure.
+  @param[in]  IsBspCallIn  Indicate whether the caller is BSP or not.
 **/
 VOID
 MicrocodeDetect (
-  IN CPU_MP_DATA *CpuMpData
+  IN CPU_MP_DATA *CpuMpData,
+  IN BOOLEAN IsBspCallIn
   )
 {
   UINT32  ExtendedTableLength;
@@ -58,6 +60,7 @@ MicrocodeDetect (
   BOOLEAN CorrectMicrocode;
   VOID*MicrocodeData;
   MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
+  UINT32  ProcessorFlags;
 
   if (CpuMpData->MicrocodePatchRegionSize == 0) {
 //
@@ -67,7 +70,7 @@ MicrocodeDetect (
   }
 
   CurrentRevision = GetCurrentMicrocodeSignature ();
-  if (CurrentRevision != 0) {
+  if (CurrentRevision != 0 && !IsBspCallIn) {
 //
 // Skip loading microcode if it has been loaded successfully
 //
@@ -87,6 +90,19 @@ MicrocodeDetect (
   PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
   PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
 
+  //
+  // Check whether AP has same processor with BSP.
+  // If yes, direct use microcode info saved by BSP.
+  //
+  if (!IsBspCallIn) {
+if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
+(CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
+MicrocodeData = (VOID *)(UINTN) CpuMpData->MicrocodeDataAddress;
+LatestRevision = CpuMpData->MicrocodeRevision;
+goto Done;
+}
+  }
+
   LatestRevision = 0;
   MicrocodeData  = NULL;
   MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + 
CpuMpData->MicrocodePatchRegionSize);
@@ -117,6 +133,7 @@ MicrocodeDetect (
 }
 if (CheckSum32 == 0) {
   CorrectMicrocode = TRUE;
+  ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
 }
   } else if ((MicrocodeEntryPoint->DataSize != 0) &&
  (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
@@ -151,6 +168,7 @@ MicrocodeDetect (
 // Find one
 //
 CorrectMicrocode = TRUE;
+ProcessorFlags = ExtendedTable->ProcessorFlag;
 break;
   }
 }
@@ -188,6 +206,7 @@ MicrocodeDetect (
 MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) 
MicrocodeEntryPoint) + TotalSize);
   } while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
 
+Done:
   if (LatestRevision > CurrentRevision) {
 //
 // BIOS only authenticate updates that contain a numerically larger 
revision
@@ -211,4 +230,13 @@ MicrocodeDetect (
   ReleaseSpinLock(>MpLock);
 }
   }
+
+  if (IsBspCallIn && (LatestRevision != 0)) {
+CpuMpData->ProcessorSignature = Eax.Uint32;
+CpuMpData->ProcessorFlags = ProcessorFlags;
+CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
+CpuMpData->MicrocodeRevision = LatestRevision;
+DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags 
[0x%08x], \
+   MicroData [0x%08x], Revision [0x%08x]\n", Eax.Uint32, ProcessorFlags, 
(UINTN) MicrocodeData, LatestRevision));
+  }
 }
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index c3cd6d7d51..3a7bdfa00b 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -410,7 +410,7 @@ ApInitializeSync (
   //
   // Load microcode on AP
   //
-  MicrocodeDetect (CpuMpData);
+  MicrocodeDetect (CpuMpData, FALSE);
   //
   // Sync BSP's MTRR table to AP
   //
@@ -1659,7 +1659,7 @@ MpInitLibInitialize (
   //
   // Load Microcode on BSP
   //
-  MicrocodeDetect (CpuMpData);
+  MicrocodeDetect (CpuMpData, TRUE);
   //
   // Store BSP's MTRR setting
   //
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 9aedb52636..6958080ac1 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -245,6 +245,11 @@ 

[edk2] [Patch v2 3/3] UefiCpuPkg/MpInitLib: Load uCode once for each core.

[Eric Dong]

The SDM requires only one thread per core to load the
microcode.

This change enables this solution.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index 351975e2a2..122c23469d 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -61,6 +61,7 @@ MicrocodeDetect (
   VOID*MicrocodeData;
   MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
   UINT32  ProcessorFlags;
+  UINT32  ThreadId;
 
   if (CpuMpData->MicrocodePatchRegionSize == 0) {
 //
@@ -77,6 +78,14 @@ MicrocodeDetect (
 return;
   }
 
+  GetProcessorLocationByApicId (GetInitialApicId (), NULL, NULL, );
+  if (ThreadId != 0) {
+//
+// Skip loading microcode if it is not the first thread in one core.
+//
+return;
+  }
+
   ExtendedTableLength = 0;
   //
   // Here data of CPUID leafs have not been collected into context buffer, so
-- 
2.15.0.windows.1

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

[edk2] [Patch v2 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

[Eric Dong]

Read uCode from memory has better performance than from flash.
But it needs extra effort to let BSP copy uCode from flash to
memory. Also BSP already enable cache in SEC phase, so it use
less time to relocate uCode from flash to memory. After
verification, if system has more than one processor, it will
reduce some time if load uCode from memory.

This change enable this optimization.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 34 +-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 108eea0a6f..c3cd6d7d51 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1520,6 +1520,7 @@ MpInitLibInitialize (
   UINTNApResetVectorSize;
   UINTNBackupBufferAddr;
   UINTNApIdtBase;
+  VOID *MicrocodePatchInRam;
 
   OldCpuMpData = GetCpuMpDataFromGuidedHob ();
   if (OldCpuMpData == NULL) {
@@ -1587,8 +1588,39 @@ MpInitLibInitialize (
   CpuMpData->SwitchBspFlag= FALSE;
   CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
   CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + 
MaxLogicalProcessorNumber);
-  CpuMpData->MicrocodePatchAddress= PcdGet64 (PcdCpuMicrocodePatchAddress);
   CpuMpData->MicrocodePatchRegionSize = PcdGet64 
(PcdCpuMicrocodePatchRegionSize);
+  //
+  // If platform has more than one CPU, relocate microcode to memory to reduce
+  // loading microcode time.
+  //
+  MicrocodePatchInRam = NULL;
+  if (MaxLogicalProcessorNumber > 1) {
+MicrocodePatchInRam = AllocatePages (
+EFI_SIZE_TO_PAGES (
+  (UINTN)CpuMpData->MicrocodePatchRegionSize
+  )
+);
+ASSERT (MicrocodePatchInRam != NULL);
+  }
+  if (MicrocodePatchInRam == NULL) {
+//
+// there is only one processor, or no microcode patch is available, or
+// memory allocation failed
+//
+CpuMpData->MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress);
+  } else {
+//
+// there are multiple processors, and a microcode patch is available, and
+// memory allocation succeeded
+//
+CopyMem (
+  MicrocodePatchInRam,
+  (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
+  (UINTN)CpuMpData->MicrocodePatchRegionSize
+  );
+CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
+  }
+
   InitializeSpinLock(>MpLock);
 
   //
-- 
2.15.0.windows.1

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

Re: [edk2] [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib

[Leif Lindholm]

On Thu, Jul 12, 2018 at 09:40:00AM +0200, Marcin Wojtas wrote:
> ICU (Interrupt Consolidation Unit) is a mechanism,
> that allows to send-message based interrupts from the
> CP110 unit (South Bridge) to the Application Processor
> hardware block. After dispatching the interrupts in the
> GIC are generated.
> 
> This patch adds a basic version of the library, that
> allows to configure a static mapping between CP110
> interfaces and GICv2 of the Armada7k8k SoC family.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
>  Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
>  Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 
>  3 files changed, 399 insertions(+)
>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
>
> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf 
> b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> new file mode 100644
> index 000..0010141
> --- /dev/null
> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> @@ -0,0 +1,38 @@
> +## @file
> +#
> +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
> +#
> +#  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.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION= 0x0001001A
> +  BASE_NAME  = IcuLib
> +  FILE_GUID  = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = ArmadaIcuLib
> +
> +[Sources]
> +  IcuLib.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  ArmadaSoCDescLib
> +  DebugLib
> +  IoLib
> +  PcdLib
> +
> +[FixedPcd]
> +  gMarvellTokenSpaceGuid.PcdMaxCpCount
> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h 
> b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> new file mode 100644
> index 000..4bf2298
> --- /dev/null
> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> @@ -0,0 +1,46 @@
> +/**
> +*
> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
> +*
> +*  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.
> +*
> +*  Glossary - abbreviations used in Marvell SampleAtReset library 
> implementation:
> +*  ICU - Interrupt Consolidation Unit
> +*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
> +*
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define ICU_REG_BASE(Cp)ArmadaSoCDescCpBaseGet (CpIndex) + 0x1E

Macro missing parentheses.

> +
> +#define ICU_SET_SPI_AL(x)   (0x10 + (0x10 * x))
> +#define ICU_SET_SPI_AH(x)   (0x14 + (0x10 * x))
> +#define ICU_CLR_SPI_AL(x)   (0x18 + (0x10 * x))
> +#define ICU_CLR_SPI_AH(x)   (0x1c + (0x10 * x))

Can that 0x10 be given a descriptive #define?

> +#define ICU_INT_CFG(x)  (0x100 + 4 * x)

Does this 4 signify sizeof (UINT32) - i.e. does it get the offset into
a series of 32-bit registers? If so, please use that instead.
Also, please use parentheses around multiplication consistently
(either not at all or everywhere).

> +
> +#define ICU_INT_ENABLE_OFFSET24
> +#define ICU_IS_EDGE_OFFSET   28
> +#define ICU_GROUP_OFFSET 29
> +
> +#define ICU_MAX_SUPPORTED_UNITS  2
> +#define ICU_MAX_IRQS_PER_CP  64
> +
> +#define MAX_ICU_IRQS 207
> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.c 
> b/Silicon/Marvell/Library/IcuLib/IcuLib.c
> new file mode 100644
> index 000..4ac98aa
> --- /dev/null
> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.c
> @@ -0,0 +1,315 @@
> +/**
> +*
> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
> +*
> +*  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 

Re: [edk2] [Patch 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, July 12, 2018 5:42 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: Re: [edk2] [Patch 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs
> if possible.
> 
> On 07/11/18 13:07, Eric Dong wrote:
> > Search uCode costs much time, if AP has same processor type with BSP,
> > AP can use BSP saved uCode info to get better performance.
> >
> > This change enables this solution.
> >
> > Cc: Laszlo Ersek 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 34
> +---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c |  4 ++--
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 +--
> >  3 files changed, 42 insertions(+), 7 deletions(-)
> >
> > diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > index e47f9f4f8f..351975e2a2 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> > @@ -35,11 +35,13 @@ GetCurrentMicrocodeSignature (
> >  /**
> >Detect whether specified processor can find matching microcode patch
> and load it.
> >
> > -  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
> > +  @param[in]  CpuMpDataThe pointer to CPU MP Data structure.
> > +  @param[in]  IsBspCallIn  Indicate whether the caller is BSP or not.
> >  **/
> >  VOID
> >  MicrocodeDetect (
> > -  IN CPU_MP_DATA *CpuMpData
> > +  IN CPU_MP_DATA *CpuMpData,
> > +  IN BOOLEAN IsBspCallIn
> >)
> >  {
> >UINT32  ExtendedTableLength;
> > @@ -58,6 +60,7 @@ MicrocodeDetect (
> >BOOLEAN CorrectMicrocode;
> >VOID*MicrocodeData;
> >MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> > +  UINT32  ProcessorFlags;
> >
> >if (CpuMpData->MicrocodePatchRegionSize == 0) {
> >  //
> > @@ -67,7 +70,7 @@ MicrocodeDetect (
> >}
> >
> >CurrentRevision = GetCurrentMicrocodeSignature ();
> > -  if (CurrentRevision != 0) {
> > +  if (CurrentRevision != 0 && !IsBspCallIn) {
> >  //
> >  // Skip loading microcode if it has been loaded successfully
> >  //
> > @@ -87,6 +90,19 @@ MicrocodeDetect (
> >PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
> >PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
> >
> > +  //
> > +  // Check whether AP has same processor with BSP.
> > +  // If yes, direct use microcode info saved by BSP.
> > +  //
> > +  if (!IsBspCallIn) {
> > +if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
> > +(CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {
> 
> Here I have only one comment. (The reason for that is that, on OVMF,
> MicrocodePatchRegionSize is zero, so MicrocodeDetect() will exit
> immediately, on both the APs and the BSP.)
> 
> My comment is that the expression
> 
>   (1 << PlatformId)
> 
> may invoke undefined behavior (and rightfully trigger build breakage with e.g.
> clang) if PlatformId is larger than 31.
> 
> Now, I do see the comment
> 
>   //
>   // The index of platform information resides in bits 50:52 of MSR
> IA32_PLATFORM_ID
>   //
> 
> so I wanted to suggest adding:
> 
>   ASSERT (PlatformId < 8)?
> 
> but then I saw that the same left-shift was already used in two other places.
> 
> So, with or without the ASSERT:
> 
> Acked-by: Laszlo Ersek 

PlatformId get from below code: 
  PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;

PlatformIdMsr.Bits.PlatformId is a three bits value, so it has limit the 
PlatformId value < 8. So I will not add the ASSERT code.

> 
> Thanks
> Laszlo
> 
> 
> 
> > +MicrocodeData = (VOID *)(UINTN) CpuMpData-
> >MicrocodeDataAddress;
> > +LatestRevision = CpuMpData->MicrocodeRevision;
> > +goto Done;
> > +}
> > +  }
> > +
> >LatestRevision = 0;
> >MicrocodeData  = NULL;
> >MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress +
> > CpuMpData->MicrocodePatchRegionSize);
> > @@ -117,6 +133,7 @@ MicrocodeDetect (
> >  }
> >  if (CheckSum32 == 0) {
> >CorrectMicrocode = TRUE;
> > +  ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
> >  }
> >} else if ((MicrocodeEntryPoint->DataSize != 0) &&
> >   (MicrocodeEntryPoint->UpdateRevision >
> > LatestRevision)) { @@ -151,6 +168,7 @@ MicrocodeDetect (
> >  // Find one
> >  //
> >  CorrectMicrocode = TRUE;
> > +ProcessorFlags = ExtendedTable->ProcessorFlag;
> >  break;
> >}
> >  }
> > @@ -188,6 +206,7 @@ MicrocodeDetect (
> 

Re: [edk2] [Patch 0/3] Optimize load uCode performance

[Laszlo Ersek]

On 07/11/18 13:07, Eric Dong wrote:
> Use below three rules to optimize load uCode performance:
> 1. Let BSP relocate uCode from flash to memory for better performance.
> 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look 
>for the uCode again if the CPU ID is same as BSP’s.
> 3. Only apply uCode in one thread of a core when hyper threading is enabled.
> 
> Test:
> Use an sample platform which has 1 socket, 4 core, 8 threads, the CpuMpPei
> driver cost time reduce from 108.4ms to 27.2ms
> 
> Eric Dong (3):
>   UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.
>   UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
>   UefiCpuPkg/MpInitLib: Load uCode once for one core.
> 
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 
> +---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 17 ++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 ++--
>  3 files changed, 63 insertions(+), 8 deletions(-)
> 
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

I also tried to regression test this with OVMF. (I know I requested some
changes for patch #1, however those would only affect behavior in
practice when memory allocation fails, so it makes sense for me to test
this version of the series as well.)

Unfortunately, the patch set doesn't apply; it fails with the first
patch already. I tried on top of current master, commit 0a563f3fecfd
("BaseTool: Fixed the incorrect cache key.", 2018-07-12).

Can you please post a v2 that is also rebased on current master?

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

Re: [edk2] [Patch 3/3] UefiCpuPkg/MpInitLib: Load uCode once for one core.

[Laszlo Ersek]

I've got two comments:

On 07/11/18 13:07, Eric Dong wrote:
> SDM requires one core only needs to load uCode once.

(1) This is a very confusing typo ("one core only").

I totally missed the point until I re-read the cover letter of the
patch. In the cover letter, you say:

> 3. Only apply uCode in one thread of a core when hyper threading is
> enabled.

So please fix the commit message to say, "The SDM requires only one
*thread per core* to load the microcode", or something similar.

(You don't need to add the emphasis, I'm only adding it here to make
myself clear.)

> Also load uCode once can save some time.
> 
> This change enables this solution.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index 351975e2a2..4586e037d2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -61,6 +61,7 @@ MicrocodeDetect (
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
>UINT32  ProcessorFlags;
> +  UINT32  ThreadId;
>  
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
> @@ -77,6 +78,14 @@ MicrocodeDetect (
>  return;
>}
>  
> +  GetProcessorLocationByApicId (GetInitialApicId(), NULL, NULL, );

(2) A space character should be inserted after "GetInitialApicId".

With those fixed:

Acked-by: Laszlo Ersek 

Thanks
Laszlo

> +  if (ThreadId != 0) {
> +//
> +// Skip loading microcode if it is not the first thread in one core.
> +//
> +return;
> +  }
> +
>ExtendedTableLength = 0;
>//
>// Here data of CPUID leafs have not been collected into context buffer, so
> 

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

Re: [edk2] [Patch 2/3] UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.

[Laszlo Ersek]

On 07/11/18 13:07, Eric Dong wrote:
> Search uCode costs much time, if AP has same processor type
> with BSP, AP can use BSP saved uCode info to get better performance.
> 
> This change enables this solution.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 34 
> +---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c |  4 ++--
>  UefiCpuPkg/Library/MpInitLib/MpLib.h | 11 +--
>  3 files changed, 42 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index e47f9f4f8f..351975e2a2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -35,11 +35,13 @@ GetCurrentMicrocodeSignature (
>  /**
>Detect whether specified processor can find matching microcode patch and 
> load it.
>  
> -  @param[in]  CpuMpData  The pointer to CPU MP Data structure.
> +  @param[in]  CpuMpDataThe pointer to CPU MP Data structure.
> +  @param[in]  IsBspCallIn  Indicate whether the caller is BSP or not.
>  **/
>  VOID
>  MicrocodeDetect (
> -  IN CPU_MP_DATA *CpuMpData
> +  IN CPU_MP_DATA *CpuMpData,
> +  IN BOOLEAN IsBspCallIn
>)
>  {
>UINT32  ExtendedTableLength;
> @@ -58,6 +60,7 @@ MicrocodeDetect (
>BOOLEAN CorrectMicrocode;
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> +  UINT32  ProcessorFlags;
>  
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
> @@ -67,7 +70,7 @@ MicrocodeDetect (
>}
>  
>CurrentRevision = GetCurrentMicrocodeSignature ();
> -  if (CurrentRevision != 0) {
> +  if (CurrentRevision != 0 && !IsBspCallIn) {
>  //
>  // Skip loading microcode if it has been loaded successfully
>  //
> @@ -87,6 +90,19 @@ MicrocodeDetect (
>PlatformIdMsr.Uint64 = AsmReadMsr64 (MSR_IA32_PLATFORM_ID);
>PlatformId = (UINT8) PlatformIdMsr.Bits.PlatformId;
>  
> +  //
> +  // Check whether AP has same processor with BSP.
> +  // If yes, direct use microcode info saved by BSP.
> +  //
> +  if (!IsBspCallIn) {
> +if ((CpuMpData->ProcessorSignature == Eax.Uint32) &&
> +(CpuMpData->ProcessorFlags & (1 << PlatformId)) != 0) {

Here I have only one comment. (The reason for that is that, on OVMF,
MicrocodePatchRegionSize is zero, so MicrocodeDetect() will exit
immediately, on both the APs and the BSP.)

My comment is that the expression

  (1 << PlatformId)

may invoke undefined behavior (and rightfully trigger build breakage
with e.g. clang) if PlatformId is larger than 31.

Now, I do see the comment

  //
  // The index of platform information resides in bits 50:52 of MSR 
IA32_PLATFORM_ID
  //

so I wanted to suggest adding:

  ASSERT (PlatformId < 8)?

but then I saw that the same left-shift was already used in two other
places.

So, with or without the ASSERT:

Acked-by: Laszlo Ersek 

Thanks
Laszlo



> +MicrocodeData = (VOID *)(UINTN) CpuMpData->MicrocodeDataAddress;
> +LatestRevision = CpuMpData->MicrocodeRevision;
> +goto Done;
> +}
> +  }
> +
>LatestRevision = 0;
>MicrocodeData  = NULL;
>MicrocodeEnd = (UINTN) (CpuMpData->MicrocodePatchAddress + 
> CpuMpData->MicrocodePatchRegionSize);
> @@ -117,6 +133,7 @@ MicrocodeDetect (
>  }
>  if (CheckSum32 == 0) {
>CorrectMicrocode = TRUE;
> +  ProcessorFlags = MicrocodeEntryPoint->ProcessorFlags;
>  }
>} else if ((MicrocodeEntryPoint->DataSize != 0) &&
>   (MicrocodeEntryPoint->UpdateRevision > LatestRevision)) {
> @@ -151,6 +168,7 @@ MicrocodeDetect (
>  // Find one
>  //
>  CorrectMicrocode = TRUE;
> +ProcessorFlags = ExtendedTable->ProcessorFlag;
>  break;
>}
>  }
> @@ -188,6 +206,7 @@ MicrocodeDetect (
>  MicrocodeEntryPoint = (CPU_MICROCODE_HEADER *) (((UINTN) 
> MicrocodeEntryPoint) + TotalSize);
>} while (((UINTN) MicrocodeEntryPoint < MicrocodeEnd));
>  
> +Done:
>if (LatestRevision > CurrentRevision) {
>  //
>  // BIOS only authenticate updates that contain a numerically larger 
> revision
> @@ -211,4 +230,13 @@ MicrocodeDetect (
>ReleaseSpinLock(>MpLock);
>  }
>}
> +
> +  if (IsBspCallIn && (LatestRevision != 0)) {
> +CpuMpData->ProcessorSignature = Eax.Uint32;
> +CpuMpData->ProcessorFlags = ProcessorFlags;
> +CpuMpData->MicrocodeDataAddress = (UINTN) MicrocodeData;
> +CpuMpData->MicrocodeRevision = LatestRevision;
> +DEBUG ((DEBUG_INFO, "BSP Microcode:: signature [0x%08x], ProcessorFlags 
> [0x%08x], \
> +   

Re: [edk2] [Patch 1/3] UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

[Laszlo Ersek]

Hi Eric,

On 07/11/18 13:07, Eric Dong wrote:
> Read uCode from memory has better performance than from flash.
> But it needs extra effort to let BSP copy uCode from flash to
> memory. Also BSP already enable cache in SEC phase, so it use
> less time to relocate uCode from flash to memory. After
> verification, if system has more than one processor, it will
> reduce some time if load uCode from memory.
>
> This change enable this optimization.
>
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 -
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index eec178b419..8b458a4a3a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1560,8 +1560,19 @@ MpInitLibInitialize (
>CpuMpData->SwitchBspFlag= FALSE;
>CpuMpData->CpuData  = (CPU_AP_DATA *) (CpuMpData + 1);
>CpuMpData->CpuInfoInHob = (UINT64) (UINTN) (CpuMpData->CpuData + 
> MaxLogicalProcessorNumber);
> -  CpuMpData->MicrocodePatchAddress= PcdGet64 
> (PcdCpuMicrocodePatchAddress);
>CpuMpData->MicrocodePatchRegionSize = PcdGet64 
> (PcdCpuMicrocodePatchRegionSize);
> +  //
> +  // if platform has more than one CPU, relocate microcode to memory to 
> reduce loading microcode time.
> +  //
> +  if (MaxLogicalProcessorNumber > 1) {
> +CpuMpData->MicrocodePatchAddress = (UINT64) (UINTN) AllocatePages 
> (EFI_SIZE_TO_PAGES ((UINTN)CpuMpData->MicrocodePatchRegionSize));
> +if (CpuMpData->MicrocodePatchAddress != 0) {
> +  CopyMem ((VOID *) (UINTN)CpuMpData->MicrocodePatchAddress, (VOID 
> *)(UINTN)(PcdGet64 (PcdCpuMicrocodePatchAddress)), 
> (UINTN)CpuMpData->MicrocodePatchRegionSize);
> +}
> +  } else {
> +CpuMpData->MicrocodePatchAddress = PcdGet64 
> (PcdCpuMicrocodePatchAddress);
> +  }
> +
>InitializeSpinLock(>MpLock);
>//
>// Save BSP's Control registers to APs
>

(1) Can you please break up the added lines to multiple lines? They are
extremely long, and difficult for me to handle. It should be possible to
break up both AllocatePages() and CopyMem(), for example.

(2) The code appears to handle the case well when
PcdCpuMicrocodePatchRegionSize is zero. In that case,
EFI_SIZE_TO_PAGES(...) evaluates to zero, and -- I checked --
AllocatePages() returns NULL. In this case, allocation and copying will
not take place, and that's fine -- there is nothing to copy and no
microcode to install. So, OK.

(3) However, if there is a microcode update available, but we can't
allocate memory, things will go wrong. The region size is nonzero, thus
MicrocodeDetect() will not exit early. But MicrocodePatchAddress will be
set to 0.

So, I suggest the following instead:

-
  VOID *MicrocodePatchInRam;

  //
  // If platform has more than one CPU, relocate microcode to memory to reduce
  // loading microcode time.
  //
  MicrocodePatchInRam = NULL;
  if (MaxLogicalProcessorNumber > 1) {
MicrocodePatchInRam = AllocatePages (
EFI_SIZE_TO_PAGES (
  (UINTN)CpuMpData->MicrocodePatchRegionSize
  )
);
  }
  if (MicrocodePatchInRam == NULL) {
//
// there is only one processor, or no microcode patch is available, or
// memory allocation failed
//
CpuMpData->MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress);
  } else {
//
// there are multiple processors, and a microcode patch is available, and
// memory allocation succeeded
//
CopyMem (
  MicrocodePatchInRam,
  (VOID *)(UINTN)PcdGet64 (PcdCpuMicrocodePatchAddress),
  (UINTN)CpuMpData->MicrocodePatchRegionSize
  );
CpuMpData->MicrocodePatchAddress = (UINTN)MicrocodePatchInRam;
  }
-

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

Re: [edk2] [PATCH edk2-platforms v2 1/6] Hisilicon/D0x: Fix invoke SetMemorySpaceAttributes error bug

[Ming]

在 11/07/2018 22:19, Ard Biesheuvel 写道:
> On 4 July 2018 at 09:51, Ming Huang  wrote:
>> The edk2 commit bacfd6e let CpuDxe running latter.
>> CpuDxe is needed by gDS->SetMemorySpaceAttributes, and
>> gDS->SetMemorySpaceAttributes is invoked by some drivers.
>>
>> This issue can solve by adding Depex on gEfiCpuArchProtocolGuid
>> to RealTimeClockLib.
>>
> 
> If this is the case, why do we still need the APRIORI DXE section?
> 

This APRIORI DXE will be removed in V3.

The new PciHostBridge which is developed by Heyi.Guo will add in V3.

Thanks.

>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ming Huang 
>> Signed-off-by: Heyi Guo 
>> ---
>>  Platform/Hisilicon/D03/D03.fdf  
>>  | 4 
>>  
>> Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
>>  | 2 ++
>>  2 files changed, 6 insertions(+)
>>
>> diff --git a/Platform/Hisilicon/D03/D03.fdf b/Platform/Hisilicon/D03/D03.fdf
>> index 1383aa1091..73e2b7e958 100644
>> --- a/Platform/Hisilicon/D03/D03.fdf
>> +++ b/Platform/Hisilicon/D03/D03.fdf
>> @@ -146,6 +146,10 @@ READ_STATUS= TRUE
>>  READ_LOCK_CAP  = TRUE
>>  READ_LOCK_STATUS   = TRUE
>>
>> +  APRIORI DXE {
>> +INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>> +  }
>> +
>>INF MdeModulePkg/Core/Dxe/DxeMain.inf
>>INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>>
>> diff --git 
>> a/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
>>  
>> b/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
>> index 319c35c724..ae7116dc31 100644
>> --- 
>> a/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
>> +++ 
>> b/Platform/Hisilicon/D03/Library/DS3231RealTimeClockLib/DS3231RealTimeClockLib.inf
>> @@ -46,3 +46,5 @@
>>
>>  [Pcd]
>>
>> +[Depex]
>> +  gEfiCpuArchProtocolGuid
>> --
>> 2.17.0
>>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib

[Marcin Wojtas]

Hi Ard,

2018-07-12 9:57 GMT+02:00 Ard Biesheuvel :
>
> On 12 July 2018 at 09:40, Marcin Wojtas  wrote:
> > ICU (Interrupt Consolidation Unit) is a mechanism,
> > that allows to send-message based interrupts from the
> > CP110 unit (South Bridge) to the Application Processor
> > hardware block. After dispatching the interrupts in the
> > GIC are generated.
> >
> > This patch adds a basic version of the library, that
> > allows to configure a static mapping between CP110
> > interfaces and GICv2 of the Armada7k8k SoC family.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
> >  Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
> >  Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 
>
> Does it make sense for a library at this level in the hierarchy (i.e.,
> generic Marvell) to carry all the default mappings for all the
> interrupts you have below? Doesn't those make this library specific to
> 7k8k ?
>
> Perhaps you could add a separate library class to obtain those
> defaults, and have special implementations for different SoC types.


In the local tree I have another SoC family, which is using same CP110
and different Application Processor unit. Initially I did exactly what
you described, but it turned out, the only difference needed for
providing the default mapping was the ICU_MSI structure and CP110 base
addresses.

So finally there was big, unnecessary duplication, which I removed by
putting the SoC-specific description part to the appropriate library.
Would it be ok, if we leave it as-is in such circumstances?

Best regards,
Marcin

>
>
>
> >  3 files changed, 399 insertions(+)
> >  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
> >  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
> >  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
> >
> > diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf 
> > b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> > new file mode 100644
> > index 000..0010141
> > --- /dev/null
> > +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> > @@ -0,0 +1,38 @@
> > +## @file
> > +#
> > +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
> > +#
> > +#  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.
> > +#
> > +##
> > +
> > +[Defines]
> > +  INF_VERSION= 0x0001001A
> > +  BASE_NAME  = IcuLib
> > +  FILE_GUID  = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
> > +  MODULE_TYPE= BASE
> > +  VERSION_STRING = 1.0
> > +  LIBRARY_CLASS  = ArmadaIcuLib
> > +
> > +[Sources]
> > +  IcuLib.c
> > +
> > +[Packages]
> > +  MdeModulePkg/MdeModulePkg.dec
> > +  MdePkg/MdePkg.dec
> > +  Silicon/Marvell/Marvell.dec
> > +
> > +[LibraryClasses]
> > +  ArmadaSoCDescLib
> > +  DebugLib
> > +  IoLib
> > +  PcdLib
> > +
> > +[FixedPcd]
> > +  gMarvellTokenSpaceGuid.PcdMaxCpCount
> > diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h 
> > b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> > new file mode 100644
> > index 000..4bf2298
> > --- /dev/null
> > +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> > @@ -0,0 +1,46 @@
> > +/**
> > +*
> > +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
> > +*
> > +*  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.
> > +*
> > +*  Glossary - abbreviations used in Marvell SampleAtReset library 
> > implementation:
> > +*  ICU - Interrupt Consolidation Unit
> > +*  AP - Application Processor hardware block (Armada 7k8k incorporates 
> > AP806)
> > +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
> > +*
> > +**/
> > +
> > +#include 
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#define ICU_REG_BASE(Cp)ArmadaSoCDescCpBaseGet (CpIndex) + 0x1E
> > +
> > +#define ICU_SET_SPI_AL(x)   (0x10 + (0x10 * x))
> > +#define ICU_SET_SPI_AH(x)   (0x14 + (0x10 * x))
> > +#define ICU_CLR_SPI_AL(x)   (0x18 + (0x10 * x))
> > +#define 

Re: [edk2] [platforms: PATCH 1/6] Marvell/Armada70x0Db: Set correct CP110 count

[Ard Biesheuvel]

On 12 July 2018 at 09:39, Marcin Wojtas  wrote:
> As a preparation for adding the ICU (Interrupt Consolidation
> Unit) library implementation a correct CP110 count is required.
> Do it for Armada70x0Db and fix depending XHCI/AHCI PCD's accordingly.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 

Reviewed-by: Ard Biesheuvel 

> ---
>  Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
> b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> index 5ccee1b..2240a57 100644
> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
> @@ -53,6 +53,9 @@
>  #
>  
> 
>  [PcdsFixedAtBuild.common]
> +  #CP110 count
> +  gMarvellTokenSpaceGuid.PcdMaxCpCount|1
> +
>#MPP
>gMarvellTokenSpaceGuid.PcdMppChipCount|2
>
> @@ -129,8 +132,8 @@
>gMarvellTokenSpaceGuid.PcdPp2Controllers|{ 0x1 }
>
>#PciEmulation
> -  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x1, 0x1, 0x0, 0x0 }
> -  gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1, 0x0 }
> +  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x1, 0x1 }
> +  gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1 }
>gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1, 0x1 }
>
>#RTC
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib

[Ard Biesheuvel]

On 12 July 2018 at 09:40, Marcin Wojtas  wrote:
> ICU (Interrupt Consolidation Unit) is a mechanism,
> that allows to send-message based interrupts from the
> CP110 unit (South Bridge) to the Application Processor
> hardware block. After dispatching the interrupts in the
> GIC are generated.
>
> This patch adds a basic version of the library, that
> allows to configure a static mapping between CP110
> interfaces and GICv2 of the Armada7k8k SoC family.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
>  Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
>  Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 

Does it make sense for a library at this level in the hierarchy (i.e.,
generic Marvell) to carry all the default mappings for all the
interrupts you have below? Doesn't those make this library specific to
7k8k ?

Perhaps you could add a separate library class to obtain those
defaults, and have special implementations for different SoC types.


>  3 files changed, 399 insertions(+)
>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
>  create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c
>
> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf 
> b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> new file mode 100644
> index 000..0010141
> --- /dev/null
> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
> @@ -0,0 +1,38 @@
> +## @file
> +#
> +#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
> +#
> +#  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.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION= 0x0001001A
> +  BASE_NAME  = IcuLib
> +  FILE_GUID  = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = ArmadaIcuLib
> +
> +[Sources]
> +  IcuLib.c
> +
> +[Packages]
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  ArmadaSoCDescLib
> +  DebugLib
> +  IoLib
> +  PcdLib
> +
> +[FixedPcd]
> +  gMarvellTokenSpaceGuid.PcdMaxCpCount
> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h 
> b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> new file mode 100644
> index 000..4bf2298
> --- /dev/null
> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
> @@ -0,0 +1,46 @@
> +/**
> +*
> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
> +*
> +*  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.
> +*
> +*  Glossary - abbreviations used in Marvell SampleAtReset library 
> implementation:
> +*  ICU - Interrupt Consolidation Unit
> +*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
> +*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
> +*
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define ICU_REG_BASE(Cp)ArmadaSoCDescCpBaseGet (CpIndex) + 0x1E
> +
> +#define ICU_SET_SPI_AL(x)   (0x10 + (0x10 * x))
> +#define ICU_SET_SPI_AH(x)   (0x14 + (0x10 * x))
> +#define ICU_CLR_SPI_AL(x)   (0x18 + (0x10 * x))
> +#define ICU_CLR_SPI_AH(x)   (0x1c + (0x10 * x))
> +#define ICU_INT_CFG(x)  (0x100 + 4 * x)
> +
> +#define ICU_INT_ENABLE_OFFSET24
> +#define ICU_IS_EDGE_OFFSET   28
> +#define ICU_GROUP_OFFSET 29
> +
> +#define ICU_MAX_SUPPORTED_UNITS  2
> +#define ICU_MAX_IRQS_PER_CP  64
> +
> +#define MAX_ICU_IRQS 207
> diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.c 
> b/Silicon/Marvell/Library/IcuLib/IcuLib.c
> new file mode 100644
> index 000..4ac98aa
> --- /dev/null
> +++ b/Silicon/Marvell/Library/IcuLib/IcuLib.c
> @@ -0,0 +1,315 @@
> +/**
> +*
> +*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
> +*
> +*  This program and the accompanying materials are licensed and made 
> available
> +*  under the terms and conditions of the BSD License which accompanies this
> +*  

[edk2] [platforms: PATCH 5/6] Marvell/Library: Implement common ArmadaIcuLib

[Marcin Wojtas]

ICU (Interrupt Consolidation Unit) is a mechanism,
that allows to send-message based interrupts from the
CP110 unit (South Bridge) to the Application Processor
hardware block. After dispatching the interrupts in the
GIC are generated.

This patch adds a basic version of the library, that
allows to configure a static mapping between CP110
interfaces and GICv2 of the Armada7k8k SoC family.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Library/IcuLib/IcuLib.inf |  38 +++
 Silicon/Marvell/Library/IcuLib/IcuLib.h   |  46 +++
 Silicon/Marvell/Library/IcuLib/IcuLib.c   | 315 
 3 files changed, 399 insertions(+)
 create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
 create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
 create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c

diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.inf 
b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
new file mode 100644
index 000..0010141
--- /dev/null
+++ b/Silicon/Marvell/Library/IcuLib/IcuLib.inf
@@ -0,0 +1,38 @@
+## @file
+#
+#  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+#
+#  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.
+#
+##
+
+[Defines]
+  INF_VERSION= 0x0001001A
+  BASE_NAME  = IcuLib
+  FILE_GUID  = 0301c9cb-43e6-40a8-96bf-41bd0501e86d
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = ArmadaIcuLib
+
+[Sources]
+  IcuLib.c
+
+[Packages]
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  ArmadaSoCDescLib
+  DebugLib
+  IoLib
+  PcdLib
+
+[FixedPcd]
+  gMarvellTokenSpaceGuid.PcdMaxCpCount
diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.h 
b/Silicon/Marvell/Library/IcuLib/IcuLib.h
new file mode 100644
index 000..4bf2298
--- /dev/null
+++ b/Silicon/Marvell/Library/IcuLib/IcuLib.h
@@ -0,0 +1,46 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  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.
+*
+*  Glossary - abbreviations used in Marvell SampleAtReset library 
implementation:
+*  ICU - Interrupt Consolidation Unit
+*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
+*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
+*
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define ICU_REG_BASE(Cp)ArmadaSoCDescCpBaseGet (CpIndex) + 0x1E
+
+#define ICU_SET_SPI_AL(x)   (0x10 + (0x10 * x))
+#define ICU_SET_SPI_AH(x)   (0x14 + (0x10 * x))
+#define ICU_CLR_SPI_AL(x)   (0x18 + (0x10 * x))
+#define ICU_CLR_SPI_AH(x)   (0x1c + (0x10 * x))
+#define ICU_INT_CFG(x)  (0x100 + 4 * x)
+
+#define ICU_INT_ENABLE_OFFSET24
+#define ICU_IS_EDGE_OFFSET   28
+#define ICU_GROUP_OFFSET 29
+
+#define ICU_MAX_SUPPORTED_UNITS  2
+#define ICU_MAX_IRQS_PER_CP  64
+
+#define MAX_ICU_IRQS 207
diff --git a/Silicon/Marvell/Library/IcuLib/IcuLib.c 
b/Silicon/Marvell/Library/IcuLib/IcuLib.c
new file mode 100644
index 000..4ac98aa
--- /dev/null
+++ b/Silicon/Marvell/Library/IcuLib/IcuLib.c
@@ -0,0 +1,315 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates.
+*
+*  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.
+*
+*  Glossary - abbreviations used in Marvell SampleAtReset library 
implementation:
+*  ICU - Interrupt Consolidation Unit
+*  AP - Application Processor hardware block (Armada 7k8k incorporates AP806)
+*  CP - South Bridge hardware blocks (Armada 7k8k incorporates CP110)
+*
+**/
+
+#include "IcuLib.h"
+
+EFI_EVENT EfiExitBootServicesEvent = (EFI_EVENT)NULL;
+
+STATIC CONST ICU_IRQ 

[edk2] [platforms: PATCH 2/6] Marvell/Library: Introduce ArmadaIcuLib class

[Marcin Wojtas]

ICU (Interrupt Consolidation Unit) is a mechanism,
that allows to send a message-based interrupts from the
CP110 unit (South Bridge) to the Application Processor
hardware block. After dispatching the interrupts in the
GIC are generated.

This patch adds a basic version of the library, that
allows to configure a static mapping between CP110
interfaces and GIC. It is required for the cases, where
the OS does not support the ICU controller on its own
(e.g. ACPI boot).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Marvell.dec|  1 +
 Silicon/Marvell/Include/Library/ArmadaIcuLib.h | 45 
 2 files changed, 46 insertions(+)
 create mode 100644 Silicon/Marvell/Include/Library/ArmadaIcuLib.h

diff --git a/Silicon/Marvell/Marvell.dec b/Silicon/Marvell/Marvell.dec
index 4def897..616624e 100644
--- a/Silicon/Marvell/Marvell.dec
+++ b/Silicon/Marvell/Marvell.dec
@@ -61,6 +61,7 @@
 
 [LibraryClasses]
   ArmadaBoardDescLib|Include/Library/ArmadaBoardDescLib.h
+  ArmadaIcuLib|Include/Library/ArmadaIcuLib.h
   ArmadaSoCDescLib|Include/Library/ArmadaSoCDescLib.h
   SampleAtResetLib|Include/Library/SampleAtResetLib.h
 
diff --git a/Silicon/Marvell/Include/Library/ArmadaIcuLib.h 
b/Silicon/Marvell/Include/Library/ArmadaIcuLib.h
new file mode 100644
index 000..9b68934
--- /dev/null
+++ b/Silicon/Marvell/Include/Library/ArmadaIcuLib.h
@@ -0,0 +1,45 @@
+/**
+*
+*  Copyright (C) 2018, Marvell International Ltd. and its affiliates
+*
+*  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.
+*
+**/
+#ifndef __ARMADA_ICU_LIB_H__
+#define __ARMADA_ICU_LIB_H__
+
+typedef enum {
+  Level = 0,
+  Edge = 1
+} ICU_IRQ_TYPE;
+
+typedef struct {
+  UINTN IcuId;
+  UINTN SpiId;
+  ICU_IRQ_TYPE IrqType;
+} ICU_IRQ;
+
+typedef struct {
+  const ICU_IRQ  *Map;
+  UINTN   Size;
+} ICU_CONFIG_ENTRY;
+
+typedef struct {
+  ICU_CONFIG_ENTRY NonSecure;
+  ICU_CONFIG_ENTRY Sei;
+  ICU_CONFIG_ENTRY Rei;
+} ICU_CONFIG;
+
+EFI_STATUS
+EFIAPI
+ArmadaIcuInitialize (
+  VOID
+  );
+
+#endif /* __ARMADA_ICU_LIB_H__ */
-- 
2.7.4

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

[edk2] [platforms: PATCH 4/6] Marvell/Library: Armada7k8kSoCDescLib: Introduce ICU information

[Marcin Wojtas]

This patch introduces new library callback (ArmadaSoCDescIcuGet ()),
which dynamically allocates and fills MV_SOC_ICU_DESC structure with
the SoC description of ICU (Interrupt Consolidation Unit).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h 
| 12 ++
 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
| 30 +++
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c 
| 39 
 3 files changed, 81 insertions(+)

diff --git 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
index 3072883..c14b985 100644
--- 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
+++ 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h
@@ -44,6 +44,18 @@
 #define MV_SOC_I2C_BASE(I2c) (0x701000 + ((I2c) * 0x100))
 
 //
+// Platform description of ICU (Interrupt Consolidation Unit) controllers
+//
+#define ICU_GIC_MAPPING_OFFSET   0
+#define ICU_NSR_SET_SPI_BASE 0xf03f0040
+#define ICU_NSR_CLEAR_SPI_BASE   0xf03f0048
+#define ICU_SEI_SET_SPI_BASE 0xf03f0230
+#define ICU_SEI_CLEAR_SPI_BASE   0xf03f0230
+#define ICU_REI_SET_SPI_BASE 0xf03f0270
+#define ICU_REI_CLEAR_SPI_BASE   0xf03f0270
+#define ICU_GROUP_UNSUPPORTED0x0
+
+//
 // Platform description of MDIO controllers
 //
 #define MV_SOC_MDIO_BASE(Cp) (MV_SOC_CP_BASE (Cp) + 0x12A200)
diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
index 56efdbe..4d2a85f 100644
--- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
+++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
@@ -58,6 +58,36 @@ ArmadaSoCDescI2cGet (
   );
 
 //
+// ICU (Interrupt Consolidation Unit)
+//
+typedef enum {
+  ICU_GROUP_NSR  = 0,
+  ICU_GROUP_SR   = 1,
+  ICU_GROUP_LPI  = 2,
+  ICU_GROUP_VLPI = 3,
+  ICU_GROUP_SEI  = 4,
+  ICU_GROUP_REI  = 5,
+  ICU_GROUP_MAX,
+} ICU_GROUP;
+
+typedef struct {
+  ICU_GROUP Group;
+  UINTN SetSpiAddr;
+  UINTN ClrSpiAddr;
+} ICU_MSI;
+
+typedef struct {
+  UINTNIcuSpiBase;
+  ICU_MSI  IcuMsi[ICU_GROUP_MAX];
+} MV_SOC_ICU_DESC;
+
+EFI_STATUS
+EFIAPI
+ArmadaSoCDescIcuGet (
+  IN OUT MV_SOC_ICU_DESC  **IcuDesc
+  );
+
+//
 // MDIO
 //
 typedef struct {
diff --git 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
index c7c9c13..8383206 100644
--- 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
+++ 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
@@ -103,6 +103,45 @@ ArmadaSoCDescI2cGet (
   return EFI_SUCCESS;
 }
 
+//
+// Allocate the MSI address per interrupt Group,
+// unsupported Groups get NULL address.
+//
+STATIC
+MV_SOC_ICU_DESC mA7k8kIcuDescTemplate = {
+  ICU_GIC_MAPPING_OFFSET,
+  {
+/* Non secure interrupts */
+{ICU_GROUP_NSR,  ICU_NSR_SET_SPI_BASE,  ICU_NSR_CLEAR_SPI_BASE},
+/* Secure interrupts */
+{ICU_GROUP_SR,   ICU_GROUP_UNSUPPORTED, ICU_GROUP_UNSUPPORTED},
+/* LPI interrupts */
+{ICU_GROUP_LPI,  ICU_GROUP_UNSUPPORTED, ICU_GROUP_UNSUPPORTED},
+/* Virtual LPI interrupts */
+{ICU_GROUP_VLPI, ICU_GROUP_UNSUPPORTED, ICU_GROUP_UNSUPPORTED},
+/* System error interrupts */
+{ICU_GROUP_SEI,  ICU_SEI_SET_SPI_BASE,  ICU_SEI_CLEAR_SPI_BASE},
+/* RAM error interrupts */
+{ICU_GROUP_REI,  ICU_REI_SET_SPI_BASE,  ICU_REI_CLEAR_SPI_BASE},
+  }
+};
+
+EFI_STATUS
+EFIAPI
+ArmadaSoCDescIcuGet (
+  IN OUT MV_SOC_ICU_DESC  **IcuDesc
+  )
+{
+  *IcuDesc = AllocateCopyPool (sizeof (mA7k8kIcuDescTemplate),
+   );
+  if (*IcuDesc == NULL) {
+DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
+return EFI_OUT_OF_RESOURCES;
+  }
+
+  return EFI_SUCCESS;
+}
+
 EFI_STATUS
 EFIAPI
 ArmadaSoCDescMdioGet (
-- 
2.7.4

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

[edk2] [platforms: PATCH 6/6] Marvell/Armada7k8k: Enable ICU configuration

[Marcin Wojtas]

This patch enables the ICU (Interrupt Consolidation Unit)
configuration in the common platform initialization driver.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  | 1 +
 Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf | 1 +
 Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c   | 2 ++
 3 files changed, 4 insertions(+)

diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc 
b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index a9d67a2..27b14ed 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -32,6 +32,7 @@
 #SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #
 [LibraryClasses.common]
+  ArmadaIcuLib|Silicon/Marvell/Library/IcuLib/IcuLib.inf
   
ArmadaSoCDescLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.inf
   
ArmPlatformLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kLib/Armada7k8kLib.inf
   ComPhyLib|Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf
diff --git a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf 
b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
index 803dc6e..5503463 100644
--- a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
+++ b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf
@@ -30,6 +30,7 @@
   Silicon/Marvell/Marvell.dec
 
 [LibraryClasses]
+  ArmadaIcuLib
   ComPhyLib
   DebugLib
   MppLib
diff --git a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c 
b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
index 1efad77..18b6783 100644
--- a/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
+++ b/Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c
@@ -12,6 +12,7 @@
 
 **/
 
+#include 
 #include 
 #include 
 #include 
@@ -40,6 +41,7 @@ ArmadaPlatInitDxeEntryPoint (
   MvComPhyInit ();
   UtmiPhyInit ();
   MppInitialize ();
+  ArmadaIcuInitialize ();
 
   return EFI_SUCCESS;
 }
-- 
2.7.4

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

[edk2] [platforms: PATCH 3/6] Marvell/Library: Armada7k8kSoCDescLib: Enable getting CP base address

[Marcin Wojtas]

For upcoming patches there is a need to get the CP110 base address,
introduce according getter function for it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
|  6 ++
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c 
| 11 +++
 2 files changed, 17 insertions(+)

diff --git a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
index d2bcf2a..56efdbe 100644
--- a/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
+++ b/Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h
@@ -36,6 +36,12 @@ ArmadaSoCDescComPhyGet (
   IN OUT UINTN*DescCount
   );
 
+UINTN
+EFIAPI
+ArmadaSoCDescCpBaseGet (
+  IN UINTN  CpIndex
+  );
+
 //
 // I2C
 //
diff --git 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
index 6ce6bad..c7c9c13 100644
--- 
a/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
+++ 
b/Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c
@@ -61,6 +61,17 @@ ArmadaSoCDescComPhyGet (
   return EFI_SUCCESS;
 }
 
+UINTN
+EFIAPI
+ArmadaSoCDescCpBaseGet (
+  IN UINTN  CpIndex
+  )
+{
+  ASSERT (CpIndex < FixedPcdGet8 (PcdMaxCpCount));
+
+  return MV_SOC_CP_BASE (CpIndex);
+}
+
 EFI_STATUS
 EFIAPI
 ArmadaSoCDescI2cGet (
-- 
2.7.4

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

[edk2] [platforms: PATCH 0/6] Armada7k8k ICU support

[Marcin Wojtas]

Hi,

This patchset introduces support for ICU (Interrupt Consolidation
Unit) of Armada7k8k SoC family. This unit allows to send a
message-based interrupts from the CP110 unit (South Bridge)
to the Application Processor hardware block. After dispatching
the interrupts in the GIC are generated. 

A basic version of the library is introduced, that
allows to configure a static mapping of the interrupt lanes
between CP110 interfaces and GIC. It is required for
the cases, where the OS does not support the ICU
controller on its own (e.g. ACPI boot).

The patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/icu-upstream-r20180712

I'm looking forward to review and any comments/remarks.

Best regards,
Marcin


Marcin Wojtas (6):
  Marvell/Armada70x0Db: Set correct CP110 count
  Marvell/Library: Introduce ArmadaIcuLib class
  Marvell/Library: Armada7k8kSoCDescLib: Enable getting CP base address
  Marvell/Library: Armada7k8kSoCDescLib: Introduce ICU information
  Marvell/Library: Implement common ArmadaIcuLib
  Marvell/Armada7k8k: Enable ICU configuration

 Silicon/Marvell/Marvell.dec
|   1 +
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  
|   1 +
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
|   7 +-
 Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.inf 
|   1 +
 Silicon/Marvell/Library/IcuLib/IcuLib.inf  
|  38 +++
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.h 
|  12 +
 Silicon/Marvell/Include/Library/ArmadaIcuLib.h 
|  45 +++
 Silicon/Marvell/Include/Library/ArmadaSoCDescLib.h 
|  36 +++
 Silicon/Marvell/Library/IcuLib/IcuLib.h
|  46 +++
 Silicon/Marvell/Armada7k8k/Drivers/PlatInitDxe/PlatInitDxe.c   
|   2 +
 Silicon/Marvell/Armada7k8k/Library/Armada7k8kSoCDescLib/Armada7k8kSoCDescLib.c 
|  50 
 Silicon/Marvell/Library/IcuLib/IcuLib.c
| 315 
 12 files changed, 552 insertions(+), 2 deletions(-)
 create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.inf
 create mode 100644 Silicon/Marvell/Include/Library/ArmadaIcuLib.h
 create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.h
 create mode 100644 Silicon/Marvell/Library/IcuLib/IcuLib.c

-- 
2.7.4

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

[edk2] [platforms: PATCH 1/6] Marvell/Armada70x0Db: Set correct CP110 count

[Marcin Wojtas]

As a preparation for adding the ICU (Interrupt Consolidation
Unit) library implementation a correct CP110 count is required.
Do it for Armada70x0Db and fix depending XHCI/AHCI PCD's accordingly.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
---
 Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc 
b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
index 5ccee1b..2240a57 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.dsc
@@ -53,6 +53,9 @@
 #
 

 [PcdsFixedAtBuild.common]
+  #CP110 count
+  gMarvellTokenSpaceGuid.PcdMaxCpCount|1
+
   #MPP
   gMarvellTokenSpaceGuid.PcdMppChipCount|2
 
@@ -129,8 +132,8 @@
   gMarvellTokenSpaceGuid.PcdPp2Controllers|{ 0x1 }
 
   #PciEmulation
-  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x1, 0x1, 0x0, 0x0 }
-  gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1, 0x0 }
+  gMarvellTokenSpaceGuid.PcdPciEXhci|{ 0x1, 0x1 }
+  gMarvellTokenSpaceGuid.PcdPciEAhci|{ 0x1 }
   gMarvellTokenSpaceGuid.PcdPciESdhci|{ 0x1, 0x1 }
 
   #RTC
-- 
2.7.4

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

Re: [edk2] [Patch edk2-platforms\devel-MinPlatform] PurleyOpenBoardPkg: Update batch file not to check the return value of timeout

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Gao, Liming
> Sent: Tuesday, July 10, 2018 9:29 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [Patch edk2-platforms\devel-MinPlatform] PurleyOpenBoardPkg:
> Update batch file not to check the return value of timeout
> 
> timeout command may not work well in some environment.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao 
> Cc: Jiewen Yao 
> ---
>  Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
> b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
> index a6e0605..4c4aec3 100644
> --- a/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
> +++ b/Platform/Intel/PurleyOpenBoardPkg/BoardMtOlympus/bld.bat
> @@ -111,13 +111,13 @@ echo.
>  echo 
>  echo.
>  call postbuild.bat
> +if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
>  timeout 1
>  echo 
>  echo.
>  echo  PostBuild End
>  echo.
>  echo 
> -if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
> 
>  echo %date%  %time%
>  echo.
> @@ -128,7 +128,6 @@ echoPurley Build
> End
>  echo.
>  echo 
> 
> 
> -if %ERRORLEVEL% NEQ 0 EXIT /b %ERRORLEVEL%
>  :done
>  endlocal & EXIT /b %SCRIPT_ERROR%
> 
> --
> 2.10.0.windows.1

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