Date   

Re: [edk2-platforms][PATCH v3 1/9] Platform/ARM/SgiPkg: create individual

Ard Biesheuvel
 

Hello Aditya,

Could you please use a meaningful subject line for this patch?

On Wed, 25 Mar 2020 at 11:53, Aditya Angadi <aditya.angadi@arm.com> wrote:

From: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>

In preparation for adding support for Reference Design (RD) platforms
that have different base addresses for GIC distributor or redistributor,
create individual platform description files for all SGI/RD platforms
and move GIC related base addresses from the common SGI/RD platform
description file to individual platform description files.
The existing platform description is then included by individual
platform description files.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Aditya Angadi <aditya.angadi@arm.com>
---
Platform/ARM/SgiPkg/Include/SgiPlatform.h | 7 +---
Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf | 3 +-
Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c | 8 ++---
Platform/ARM/SgiPkg/RdE1Edge.dsc | 37 ++++++++++++++++++++
Platform/ARM/SgiPkg/RdN1Edge.dsc | 37 ++++++++++++++++++++
Platform/ARM/SgiPkg/Sgi575.dsc | 37 ++++++++++++++++++++
Platform/ARM/SgiPkg/SgiPlatform.dec | 5 ++-
Platform/ARM/SgiPkg/SgiPlatform.dsc | 25 ++-----------
8 files changed, 124 insertions(+), 35 deletions(-)

diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index e36a412155ff..d87fb2b5409f 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2018, ARM Limited. All rights reserved.
+* Copyright (c) 2018-2020, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -45,11 +45,6 @@
#define SGI_SUBSYS_GENERIC_WDOG_BASE 0x2A440000
#define SGI_SUBSYS_GENERIC_WDOG_SZ SIZE_128KB

-// Sub System Peripherals - GIC
-#define SGI_SUBSYS_GENERIC_GIC_BASE 0x30000000
-#define SGI_SUBSYS_GENERIC_GICR_BASE 0x300C0000
-#define SGI_SUBSYS_GENERIC_GIC_SZ SIZE_1MB
-
// Expansion AXI - Platform Peripherals - HDLCD1
#define SGI_EXP_PLAT_PERIPH_HDLCD1_BASE 0x7FF60000
#define SGI_EXP_PLAT_PERIPH_HDLCD1_SZ SIZE_64KB
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
index 3db70e900d61..a918afef5fba 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLib.inf
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2018, ARM Limited. All rights reserved.
+# Copyright (c) 2018-2020, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -42,6 +42,7 @@ [FixedPcd]

gArmSgiTokenSpaceGuid.PcdDramBlock2Base
gArmSgiTokenSpaceGuid.PcdDramBlock2Size
+ gArmSgiTokenSpaceGuid.PcdGicSize

gArmTokenSpaceGuid.PcdSystemMemoryBase
gArmTokenSpaceGuid.PcdSystemMemorySize
diff --git a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
index 845aeaf4dd49..8d0ad4ec9c84 100644
--- a/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
+++ b/Platform/ARM/SgiPkg/Library/PlatformLib/PlatformLibMem.c
@@ -1,6 +1,6 @@
/** @file
*
-* Copyright (c) 2018, ARM Limited. All rights reserved.
+* Copyright (c) 2018-2020, ARM Limited. All rights reserved.
*
* SPDX-License-Identifier: BSD-2-Clause-Patent
*
@@ -93,9 +93,9 @@ ArmPlatformGetVirtualMemoryMap (
VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

// Sub System Peripherals - GIC-600
- VirtualMemoryTable[++Index].PhysicalBase = SGI_SUBSYS_GENERIC_GIC_BASE;
- VirtualMemoryTable[Index].VirtualBase = SGI_SUBSYS_GENERIC_GIC_BASE;
- VirtualMemoryTable[Index].Length = SGI_SUBSYS_GENERIC_GIC_SZ;
+ VirtualMemoryTable[++Index].PhysicalBase = FixedPcdGet64(PcdGicDistributorBase);
+ VirtualMemoryTable[Index].VirtualBase = FixedPcdGet64(PcdGicDistributorBase);
+ VirtualMemoryTable[Index].Length = FixedPcdGet64(PcdGicSize);
Please use a space before '('

VirtualMemoryTable[Index].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;

// Expansion AXI - Platform Peripherals - HDLCD1
diff --git a/Platform/ARM/SgiPkg/RdE1Edge.dsc b/Platform/ARM/SgiPkg/RdE1Edge.dsc
new file mode 100644
index 000000000000..082cbb0157f7
--- /dev/null
+++ b/Platform/ARM/SgiPkg/RdE1Edge.dsc
@@ -0,0 +1,37 @@
+#
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+ PLATFORM_NAME = ArmSgi
+ PLATFORM_GUID = c834de39-c5b0-458b-8ea3-882427179b8a
+ PLATFORM_VERSION = 0.1
+ DSC_SPECIFICATION = 0x0001001B
+ OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME)
+ SUPPORTED_ARCHITECTURES = AARCH64|ARM
+ BUILD_TARGETS = NOOPT|DEBUG|RELEASE
+ SKUID_IDENTIFIER = DEFAULT
+ FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf
If you are going to split these into separate DSCs, please do this
first in a separate patch, and update the PLATFORM_NAME accordingly.

If there is no need to use separate FDFs than you can keep using a
shared one. If there is, please do the split in the same patch.

+ BUILD_NUMBER = 1
+
+# include common definitions from SgiPlatform.dsc
+!include Platform/ARM/SgiPkg/SgiPlatform.dsc
+
Please rename that file to use a .dsc.inc extension to make it clear
this is an include file

+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFixedAtBuild.common]
+ # GIC Base Addresses
+ gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
+ gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
+ gArmSgiTokenSpaceGuid.PcdGicSize|0x100000
diff --git a/Platform/ARM/SgiPkg/RdN1Edge.dsc b/Platform/ARM/SgiPkg/RdN1Edge.dsc
new file mode 100644
index 000000000000..6774990ad6f6
--- /dev/null
+++ b/Platform/ARM/SgiPkg/RdN1Edge.dsc
@@ -0,0 +1,37 @@
+#
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+ PLATFORM_NAME = ArmSgi
+ PLATFORM_GUID = dbc75915-03df-4640-8f3d-3d3abf7c119b
+ PLATFORM_VERSION = 0.1
+ DSC_SPECIFICATION = 0x0001001B
+ OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME)
+ SUPPORTED_ARCHITECTURES = AARCH64|ARM
+ BUILD_TARGETS = NOOPT|DEBUG|RELEASE
+ SKUID_IDENTIFIER = DEFAULT
+ FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf
+ BUILD_NUMBER = 1
+
+# include common definitions from SgiPlatform.dsc
+!include Platform/ARM/SgiPkg/SgiPlatform.dsc
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFixedAtBuild.common]
+ # GIC Base Addresses
+ gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
+ gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
+ gArmSgiTokenSpaceGuid.PcdGicSize|0x100000
diff --git a/Platform/ARM/SgiPkg/Sgi575.dsc b/Platform/ARM/SgiPkg/Sgi575.dsc
new file mode 100644
index 000000000000..3c1904c2da91
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Sgi575.dsc
@@ -0,0 +1,37 @@
+#
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+ PLATFORM_NAME = ArmSgi
+ PLATFORM_GUID = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae
+ PLATFORM_VERSION = 0.1
+ DSC_SPECIFICATION = 0x0001001B
+ OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME)
+ SUPPORTED_ARCHITECTURES = AARCH64|ARM
+ BUILD_TARGETS = NOOPT|DEBUG|RELEASE
+ SKUID_IDENTIFIER = DEFAULT
+ FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf
+ BUILD_NUMBER = 1
+
+# include common definitions from SgiPlatform.dsc
+!include Platform/ARM/SgiPkg/SgiPlatform.dsc
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFixedAtBuild.common]
+ # GIC Base Addresses
+ gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
+ gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
+ gArmSgiTokenSpaceGuid.PcdGicSize|0x100000
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dec b/Platform/ARM/SgiPkg/SgiPlatform.dec
index 9d70ec677776..4ac3dec91e3d 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dec
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dec
@@ -1,5 +1,5 @@
#
-# Copyright (c) 2018, ARM Limited. All rights reserved.
+# Copyright (c) 2018-2020, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -49,5 +49,8 @@ [PcdsFixedAtBuild]
gArmSgiTokenSpaceGuid.PcdVirtioNetSize|0x00000000|UINT32|0x00000008
gArmSgiTokenSpaceGuid.PcdVirtioNetInterrupt|0x00000000|UINT32|0x00000009

+ # GIC
+ gArmSgiTokenSpaceGuid.PcdGicSize|0|UINT64|0x0000000A
Wouldn't a UINT32 be sufficient to describe the size of a MMIO block?

+
[Ppis]
gNtFwConfigDtInfoPpiGuid = { 0x6f606eb3, 0x9123, 0x4e15, { 0xa8, 0x9b, 0x0f, 0xac, 0x66, 0xef, 0xd0, 0x17 } }
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
index 5226c5751e98..4e1fcefb1442 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
@@ -1,26 +1,9 @@
#
-# Copyright (c) 2018, ARM Limited. All rights reserved.
+# Copyright (c) 2018-2020, ARM Limited. All rights reserved.
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#

-################################################################################
-#
-# Defines Section - statements that will be processed to create a Makefile.
-#
-################################################################################
-[Defines]
- PLATFORM_NAME = ArmSgi
- PLATFORM_GUID = 3a6b2eae-0275-4b6e-a5d1-bd2ba1ce1fae
- PLATFORM_VERSION = 0.1
- DSC_SPECIFICATION = 0x0001001B
- OUTPUT_DIRECTORY = Build/$(PLATFORM_NAME)
- SUPPORTED_ARCHITECTURES = AARCH64|ARM
- BUILD_TARGETS = NOOPT|DEBUG|RELEASE
- SKUID_IDENTIFIER = DEFAULT
- FLASH_DEFINITION = Platform/ARM/SgiPkg/SgiPlatform.fdf
- BUILD_NUMBER = 1
-
!include Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

[BuildOptions]
@@ -93,7 +76,7 @@ [LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION, Libr

################################################################################
#
-# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+# Pcd Section - list of all EDK II PCD Entries common to all SGI/RD platforms
#
################################################################################

@@ -126,10 +109,6 @@ [PcdsFixedAtBuild.common]
gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000
gArmTokenSpaceGuid.PcdSystemMemorySize|0x7F000000

- # GIC Base Addresses
- gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
- gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x300C0000
-
#
# PCIe
#
--
2.17.1


Re: [PATCH v2 28/28] Platform/NXP/LS1043aRdbPkg: Add PEI Phase

Pankaj Bansal
 

-----Original Message-----
From: Leif Lindholm <leif@nuviainc.com>
Sent: Monday, March 30, 2020 5:49 PM
To: Pankaj Bansal (OSS) <pankaj.bansal@oss.nxp.com>
Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>; Michael D Kinney
<michael.d.kinney@intel.com>; devel@edk2.groups.io; Varun Sethi
<V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Jon Nettleton <jon@solid-run.com>
Subject: Re: [PATCH v2 28/28] Platform/NXP/LS1043aRdbPkg: Add PEI Phase

On Fri, Mar 20, 2020 at 20:05:43 +0530, Pankaj Bansal wrote:
From: Pankaj Bansal <pankaj.bansal@nxp.com>

Add PEI phase to LS1043aRdb. This is needed becuase we need to have
dynamic PCDs support to be able to reserve memory before reporting
memory to UEFI fimrware.

Signed-off-by: Pankaj Bansal <pankaj.bansal@nxp.com>
---
Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc | 9 ---
Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf | 18 +++--
.../MemoryInitPeiLib/MemoryInitPeiLib.c | 77 ++++++++++---------
.../MemoryInitPeiLib/MemoryInitPeiLib.inf | 3 +-
Silicon/NXP/NxpQoriqLs.dsc.inc | 59 ++++++++++----
5 files changed, 99 insertions(+), 67 deletions(-)

diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
index d486c9b36fab..d45fd67c03b5 100644
--- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
+++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.dsc
@@ -30,15 +30,6 @@ [LibraryClasses.common]
RealTimeClockLib|Silicon/Maxim/Library/Ds1307RtcLib/Ds1307RtcLib.inf

[PcdsFixedAtBuild.common]
-
- #
- # LS1043a board Specific PCDs
- # XX (DRAM - Region 1 2GB)
- # (NOR - IFC Region 1 512MB)
- gArmTokenSpaceGuid.PcdSystemMemoryBase|0x80000000
- gArmTokenSpaceGuid.PcdSystemMemorySize|0x7BE00000
-
gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x02000000
-
#
# RTC Pcds
#
diff --git a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
index 99fbc87e1200..931d0bb14f9b 100644
--- a/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
+++ b/Platform/NXP/LS1043aRdbPkg/LS1043aRdbPkg.fdf
@@ -24,10 +24,10 @@

[FD.LS1043ARDB_EFI]
BaseAddress = 0x82000000|gArmTokenSpaceGuid.PcdFdBaseAddress #The
base address of the FLASH Device.
-Size = 0x000ED000|gArmTokenSpaceGuid.PcdFdSize #The size in
bytes of the FLASH Device
+Size = 0x00140000|gArmTokenSpaceGuid.PcdFdSize #The size in
bytes of the FLASH Device
ErasePolarity = 1
-BlockSize = 0x1
-NumBlocks = 0xED000
+BlockSize = 0x40000
+NumBlocks = 0x5

#################################################################
###############
#
@@ -44,7 +44,7 @@ [FD.LS1043ARDB_EFI]
# RegionType <FV, DATA, or FILE>
#
#################################################################
###############
-0x00000000|0x000ED000
+0x00000000|0x00140000
gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
FV = FVMAIN_COMPACT

@@ -159,7 +159,15 @@ [FV.FVMAIN_COMPACT]
READ_LOCK_CAP = TRUE
READ_LOCK_STATUS = TRUE

- INF ArmPlatformPkg/PrePi/PeiUniCore.inf
+ INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+ INF MdeModulePkg/Core/Pei/PeiMain.inf
+ INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+ INF
MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+ INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+ INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+ INF ArmPkg/Drivers/CpuPei/CpuPei.inf
+ INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+ INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf

FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF
PROCESSING_REQUIRED = TRUE {
diff --git a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
index 54d026ef1270..7fdf9cb77a6e 100644
--- a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
+++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.c
@@ -46,30 +46,12 @@ InitMmu (
}
}

-/*++
-
-Routine Description:
-
-
-
-Arguments:
-
- FileHandle - Handle of the file being invoked.
- PeiServices - Describes the list of possible PEI Services.
-
-Returns:
-
- Status - EFI_SUCCESS if the boot mode could be set
-
---*/
The above line caused me an unexpected level of excitement this
morning, as my "put back the CRs SMTP strips out" script treated the
--- as a diff separator.

Now, I *have* seen the use of /*++ --*/ elsewhere in the tree, but
this syntax is *not* described in the coding style and should not be
used. While this is a delete statement, there is an addition below
using the same format. The doxygen tags to use are /** and **/.

Fortunately, I can't spot any of these in the rest of the set.

Please send an updated version of this patch - alone if it's the only
patch that needs changes, or with a v4 if such is required.
I have not received any comments on other patches so far.
Does that mean all patches are OK (except above)?
If that is the case, then I can send only this patch after update.
If some rework is needed for other patches as well, I will send this patch along with other reworked patches in v3.


EFI_STATUS
EFIAPI
-MemoryPeim (
- IN EFI_PHYSICAL_ADDRESS UefiMemoryBase,
- IN UINT64 UefiMemorySize
+MemoryInitPeiLibConstructor (
+ VOID
)
{
- ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
ARM_SMC_ARGS ArmSmcArgs;
INT32 Index;
UINTN DramSize;
@@ -82,18 +64,6 @@ MemoryPeim (
UINTN FdTop;
BOOLEAN FoundSystemMem;

- // Get Virtual Memory Map from the Platform Library
- ArmPlatformGetVirtualMemoryMap (&MemoryTable);
-
- //
- // Ensure MemoryTable[0].Length which is size of DRAM has been set
- // by ArmPlatformGetVirtualMemoryMap ()
- //
- ASSERT (MemoryTable[0].Length != 0);
-
- //
- // Now, the permanent memory has been installed, we can call
AllocatePages()
- //
ResourceAttributes = (
EFI_RESOURCE_ATTRIBUTE_PRESENT |
EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
@@ -133,8 +103,8 @@ MemoryPeim (

ASSERT (!DramSize);

- FdBase = (UINTN)FixedPcdGet64 (PcdFdBaseAddress);
- FdTop = FdBase + (UINTN)FixedPcdGet32 (PcdFdSize);
+ FdBase = (UINTN)PcdGet64 (PcdFdBaseAddress);
+ FdTop = FdBase + (UINTN)PcdGet32 (PcdFdSize);

// Declare memory regios to system
for (Index = MAX_DRAM_REGIONS - 1; Index >= 0; Index--) {
@@ -178,8 +148,8 @@ MemoryPeim (
);
};
// Mark the memory covering the Firmware Device as boot services data
- BuildMemoryAllocationHob (FixedPcdGet64 (PcdFdBaseAddress),
- FixedPcdGet32 (PcdFdSize),
+ BuildMemoryAllocationHob (PcdGet64 (PcdFdBaseAddress),
+ PcdGet32 (PcdFdSize),
EfiBootServicesData);
} else {
BuildResourceDescriptorHob (
@@ -199,16 +169,49 @@ MemoryPeim (
Top = DramRegions[Index].BaseAddress + DramRegions[Index].Size;

if (FdBase >= BaseAddress && FdTop <= Top) {
- Size -= (UINTN)FixedPcdGet32 (PcdFdSize);
+ Size -= (UINTN)PcdGet32 (PcdFdSize);
}

if (Size >= FixedPcdGet32 (PcdSystemMemoryUefiRegionSize)) {
FoundSystemMem = TRUE;
+ PcdSet64S (PcdSystemMemoryBase, BaseAddress);
+ PcdSet64S (PcdSystemMemorySize, Size);
}
}

ASSERT (FoundSystemMem);

+ return EFI_SUCCESS;
+}
+
+/*++
Here is the incorrect addition.

(I'm not reviewing the set backwards, this was just the only patch
that wouldn't apply cleanly after conversion.)

/
Leif

+
+Routine Description:
+
+
+
+Arguments:
+
+ FileHandle - Handle of the file being invoked.
+ PeiServices - Describes the list of possible PEI Services.
+
+Returns:
+
+ Status - EFI_SUCCESS if the boot mode could be set
+
+--*/
+EFI_STATUS
+EFIAPI
+MemoryPeim (
+ IN EFI_PHYSICAL_ADDRESS UefiMemoryBase,
+ IN UINT64 UefiMemorySize
+ )
+{
+ ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
+
+ // Get Virtual Memory Map from the Platform Library
+ ArmPlatformGetVirtualMemoryMap (&MemoryTable);
+
// Build Memory Allocation Hob
InitMmu (MemoryTable);

diff --git a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
index ad2371115b17..a33f8cd3f743 100644
--- a/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
+++ b/Silicon/NXP/Library/MemoryInitPeiLib/MemoryInitPeiLib.inf
@@ -13,7 +13,8 @@ [Defines]
FILE_GUID = 55ddb6e0-70b5-11e0-b33e-0002a5d5c51b
MODULE_TYPE = BASE
VERSION_STRING = 1.0
- LIBRARY_CLASS = MemoryInitPeiLib|SEC PEIM DXE_DRIVER
+ LIBRARY_CLASS = MemoryInitPeiLib|PEIM
+ CONSTRUCTOR = MemoryInitPeiLibConstructor

[Sources]
MemoryInitPeiLib.c
diff --git a/Silicon/NXP/NxpQoriqLs.dsc.inc b/Silicon/NXP/NxpQoriqLs.dsc.inc
index b2b10ce28a93..a3f18abb37b1 100644
--- a/Silicon/NXP/NxpQoriqLs.dsc.inc
+++ b/Silicon/NXP/NxpQoriqLs.dsc.inc
@@ -93,6 +93,7 @@ [LibraryClasses.common]
CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
NonDiscoverableDeviceRegistrationLib|MdeModulePkg/Library/NonDiscoverabl
eDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeRep
ortStatusCodeLib.inf
+
UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecomp
ressLib.inf

I2cLib|Silicon/NXP/Library/I2cLib/I2cLib.inf
ResetSystemLib|ArmPkg/Library/ArmSmcPsciResetSystemLib/ArmSmcPsciResetS
ystemLib.inf
@@ -106,20 +107,24 @@ [LibraryClasses.common]

[LibraryClasses.common.SEC]
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-
UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecomp
ressLib.inf
-
ExtractGuidedSectionLib|EmbeddedPkg/Library/PrePiExtractGuidedSectionLib/P
rePiExtractGuidedSectionLib.inf
-
LzmaDecompressLib|MdeModulePkg/Library/LzmaCustomDecompressLib/Lzma
CustomDecompressLib.inf
- PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
- HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
-
PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiH
obListPointerLib.inf
-
MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiM
emoryAllocationLib.inf
+
DebugAgentLib|ArmPkg/Library/DebugAgentSymbolsBaseLib/DebugAgentSymb
olsBaseLib.inf
+ HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
+ PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
+
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServi
cesTablePointerLib.inf
+
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllo
cationLib.inf
+
+[LibraryClasses.common.PEI_CORE]
+ PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
+ HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
+ PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
+
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllo
cationLib.inf
+
PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib
.inf
+
ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtrac
tGuidedSectionLib.inf
+
ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiRepo
rtStatusCodeLib.inf
+
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/
OemHookStatusCodeLibNull.inf

- # 1/123 faster than Stm or Vstm version
- BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
-
- # Uncomment to turn on GDB stub in SEC.
-
#DebugAgentLib|EmbeddedPkg/Library/GdbDebugAgent/GdbDebugAgent.inf
+
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServi
cesTablePointerLib.inf

[LibraryClasses.common.PEIM]
PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
@@ -128,14 +133,16 @@ [LibraryClasses.common.PEIM]
PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLib/PeiServi
cesTablePointerLib.inf
HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllo
cationLib.inf
+
PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib
.inf
+
ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtrac
tGuidedSectionLib.inf
ReportStatusCodeLib|MdeModulePkg/Library/PeiReportStatusCodeLib/PeiRepo
rtStatusCodeLib.inf
+
OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/
OemHookStatusCodeLibNull.inf

[LibraryClasses.common.DXE_CORE]
HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/D
xeCoreMemoryAllocationLib.inf
DxeCoreEntryPoint|MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.inf
ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtra
ctGuidedSectionLib.inf
-
UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecomp
ressLib.inf
DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerf
ormanceLib.inf

@@ -207,6 +214,9 @@ [PcdsDynamicDefault.common]
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoHorizontalResolution|640
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupVideoVerticalResolution|480

+ gArmTokenSpaceGuid.PcdSystemMemoryBase|0
+ gArmTokenSpaceGuid.PcdSystemMemorySize|0
+
[PcdsDynamicHii.common.DEFAULT]
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalV
ariableGuid|0x0|10

@@ -227,6 +237,12 @@ [PcdsFixedAtBuild.common]
gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320

+ ## Base of DRAM
+ ## since TFA puts Fd at 0x2000000 offset from DRAM base, we can use this
space
+ ## for temporary ram
+ gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x80000000
+
gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x02000000
+
!if $(TARGET) == RELEASE
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x27
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x81000001
@@ -284,13 +300,26 @@ [PcdsFixedAtBuild.common]
#################################################################
###############
[Components.common]
#
- # SEC
+ # PEI Phase modules
#
- ArmPlatformPkg/PrePi/PeiUniCore.inf
+ ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+
+ MdeModulePkg/Core/Pei/PeiMain.inf
MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
<LibraryClasses>
PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
}
+ MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+ MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+
+ ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+ ArmPkg/Drivers/CpuPei/CpuPei.inf
+ ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+
+ MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
+ <LibraryClasses>
+
NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecom
pressLib.inf
+ }

#
# DXE
--
2.17.1


Re: [PATCH] Maintainers: switch to my Arm email address

Ard Biesheuvel
 

On Thu, 5 Mar 2020 at 15:33, Laszlo Ersek <lersek@redhat.com> wrote:

On 03/05/20 13:30, Leif Lindholm wrote:
On Thu, Mar 05, 2020 at 12:23:46 +0100, Ard Biesheuvel wrote:
I no longer work for Linaro (and haven't for a while) so in anticipation
of losing access to my @linaro.org mailbox, let's switch to the ARM one
for my Tiancore contributions and maintainerships. Update the .mailmap
at the same time so the tooling can rewrite history where needed.
I have no issue either way, but I also don't think a proper decision
was made last time around the .mailmap topic came up as to whether it
was intended to span organisation changes.
We said that stewards wouldn't have the power to approve such, but the
subject contributors would.

At present it does not in this project, and I would prefer for such a
change to happen after an explicit policy decision rather than by
accident.

In the meantime, for the Maintainers.txt changes:
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
I'm fine with both keeping and dropping the .mailmap hunk, as it is
being proposed by Ard.

So either way:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks all

Given the fact that the mailmap is only used when explicitly asked for
it, adding the entry is not going to make it more likely that
unsuspecting contributors will end up using my @arm.com email address
after looking at my contributions in the Git history. So I dropped the
mailmap hunk and pushed this to edk2 master.


How to change RAID Card's configuration without going into its setup ui

Tiger Liu(BJ-RD)
 

Hi, Experts:
I am studying how to remote control some hardware's configuration.

Such as changing RAID Card's configuration without going into its setup ui.

So, one question confused me.
If RAID Card's firmware is written with UEFI driver model, so I can use some standard UEFI protocol to pass some configurations to RAID card's firmware?
Or must use RAID Card vendor's private protocol to transfer new configurations?

Such as:
EFI Adapter Information Protocol, SetInformation function.

Thanks


保密声明:
本邮件含有保密或专有信息,仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for the sole use of its intended recipient. Any unauthorized review, use, copying or forwarding of this email or the content of this email is strictly prohibited.


Re: API breakages and their implications. Was: [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

Leif Lindholm
 

On Tue, Mar 31, 2020 at 01:53:21 +0000, Ni, Ray wrote:
Leif,
Please understand that the concern of this change is all the platforms that uses
this serial port lib must be changed otherwise build breaks.
Yes. This is the nature of collaborative development.
This is something we on the ARM side have lived with since we got
involved with EDK2, being the less-deployed user of the codebase, and
that is fine.

TianoCore specifically produced the UDK to make life easier for those
who are unable to track upstream, and we have followed that up with
stable tags. I would highly recommend that any team that feels that
this change would be too much effort to move to edk2-stable202002. Of
course, they would then need to manually cherry-pick any improvements
and security fixes from master. That is their choice.

Nevertheless, breaking existing platforms is a side effect, not a
goal. So if an alternative solution can be found (which is not a hack
that is going to affect the codebase negatively over time and simply
need to be fixed properly at a later date), then clearly that is
preferable.

"This breaks many platforms" is a good argument for seeing if a
solution can be found that does not break (as) many platforms. It is
not an argument for duplicating drivers when the change needed for
those platforms is trivial.

These days, Linux tends to deal with API breakages (and other things)
using a semantic patch tool called Coccinelle[1]. It would not be
unreasonable, and indeed it would be helpful to us on the non-x86 side
if something similar was adopted (and semantic patches mandated) for
API breakages in EDK2.

[1] http://coccinelle.lip6.fr/sp.php

Regards,

Leif

Ard,
Using Guided HOB sounds a good idea to me: )
The benefits of using HOB is:
Length field in the HOB header can be used for extension if more parameters are needed.
DXE can have the HOB access as well.

EFI_SEC_HOB_DATA_PPI can be used to return the new Guided HOB from SEC phase if needed.

Thanks,
Ray


-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Sent: Monday, March 30, 2020 3:45 PM
To: Leif Lindholm <leif@nuviainc.com>
Cc: Jiang, Guomin <guomin.jiang@intel.com>; devel@edk2.groups.io; pankaj.bansal@nxp.com; Ni, Ray <ray.ni@intel.com>;
Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ma, Maurice <maurice.ma@intel.com>; Dong,
Guo <guo.dong@intel.com>; You, Benjamin <benjamin.you@intel.com>; Meenakshi Aggarwal
<meenakshi.aggarwal@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>
Subject: Re: [edk2-devel] [PATCH 1/1] MdeModulePkg: UART Dynamic clock freq Support

On Mon, 30 Mar 2020 at 09:35, Leif Lindholm <leif@nuviainc.com> wrote:

Hi Jiang,

It is not a question of effort of copying a driver, it is a question
that copying drivers is something that should be avoided wherever
practically possible. I did not think this topic was still under
debate.

If the existing 16550 SerialPortLib is overspecialised to the point
where it only works on a subset of 16550 implementations, then it
should change. There are going to be more non-PC systems turning up
with 16550 UARTs - should they each copy/modify their drivers?

If there are better ways of solving that problem, please suggest.
But more duplicated drivers is not the answer.
Could we use a GUIDed HOB? If it exists, we use its contents, and if
it doesn't, we use the default set by the FixedPCD.


Re: [PATCH v1] ShellPkg: Fix 'ping' command Ip4 receive flow.

Maciej Rabeda
 

Siyuan/Jiaxin,

Can we get this patch reviewed? I cannot give myself green light on this patch :)

On 26-Mar-20 04:16, Gao, Zhichao wrote:
The ping command implementation would go into the ip4/ip6 protocol. But I am not familiar with the network part.

Jiaxin/Siyuan,

Can you help to review this patch?

Thanks,
Zhichao

-----Original Message-----
From: Laszlo Ersek [mailto:lersek@redhat.com]
Sent: Wednesday, March 25, 2020 7:34 PM
To: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>
Cc: devel@edk2.groups.io; maciej.rabeda@linux.intel.com
Subject: Re: [edk2-devel] [PATCH v1] ShellPkg: Fix 'ping' command Ip4
receive flow.

Ray, Zhichao,

On 02/27/20 12:02, Maciej Rabeda wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2032

'ping' command's receive flow utilizes a single Rx token which it
attempts to reuse before recycling the previously received packet.
This causes a situation where under ICMP traffic,
Ping6OnEchoReplyReceived() function will receive an already recycled
packet with EFI_SUCCESS token status and finally dereference invalid
pointers from RxData structure.

Cc: Ray Ni <ray.ni@intel.com>
Cc: Zhichao Gao <zhichao.gao@intel.com>
Signed-off-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>
---
ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
can you please review this ShellPkg patch? It's been on the list for almost a
month now.

Thanks
Laszlo

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index 23567fa2c1bb..a3fa32515192 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -614,6 +614,11 @@ Ping6OnEchoReplyReceived (

ON_EXIT:

+ //
+ // Recycle the packet before reusing RxToken // gBS->SignalEvent
+ (Private->IpChoice ==
+ PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
RxToken.Packet.R
+ xData)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private-
RxToken.Packe
+ t.RxData)->RecycleSignal);
+
if (Private->RxCount < Private->SendNum) {
//
// Continue to receive icmp echo reply packets.
@@ -632,10 +637,6 @@ ON_EXIT:
//
Private->Status = EFI_SUCCESS;
}
- //
- // Singal to recycle the each rxdata here, not at the end of process.
- //
- gBS->SignalEvent (Private->IpChoice ==
PING_IP_CHOICE_IP6?((EFI_IP6_RECEIVE_DATA*)Private-
RxToken.Packet.RxD
ata)->RecycleSignal:((EFI_IP4_RECEIVE_DATA*)Private-
RxToken.Packet.Rx
Data)->RecycleSignal);
}

/**


Re: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

Guomin Jiang
 

There is a spell error in the comments of VariableServiceGetVariable() in Variable.c.
- @return EFI_BUFFER_TO_SMALL DataSize is too small for the result.
+ @return EFI_BUFFER_TOO_SMALL DataSize is too small for the result.

Need create new bugs for it or fix in this comment directly?

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Wang, Jian J
Sent: Monday, March 30, 2020 12:16 PM
To: michael.kubacki@outlook.com; devel@edk2.groups.io
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Wu, Hao A <hao.a.wu@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 1/3] MdeModulePkg Variable: Return
GetVariable() attr if EFI_BUFFER_TOO_SMALL


Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

-----Original Message-----
From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
Sent: Saturday, March 28, 2020 5:56 AM
To: devel@edk2.groups.io
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH v3 1/3] MdeModulePkg Variable: Return GetVariable()
attr if EFI_BUFFER_TOO_SMALL

From: Michael Kubacki <michael.kubacki@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062

The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
"Attributes" parameter description states:

"If not NULL, a pointer to the memory location to return the
attributes bitmask for the variable. See 'Related Definitions.'
If not NULL, then Attributes is set on output both when EFI_SUCCESS
and when EFI_BUFFER_TOO_SMALL is returned."

The attributes were previously only returned from the implementation
in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
EFI_BUFFER_TOO_SMALL according to spec.

Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c | 10
+++++++---
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.
c |
10 ++++++----
2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index d23aea4bc712..1e71fc642c76 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -18,6 +18,8 @@

Copyright (c) 2006 - 2020, Intel Corporation. All rights
reserved.<BR>
(C) Copyright 2015-2018 Hewlett Packard Enterprise Development LP<BR>
+Copyright (c) Microsoft Corporation.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -2379,9 +2381,6 @@ VariableServiceGetVariable (
}

CopyMem (Data, GetVariableDataPtr (Variable.CurrPtr,
mVariableModuleGlobal->VariableGlobal.AuthFormat), VarDataSize);
- if (Attributes != NULL) {
- *Attributes = Variable.CurrPtr->Attributes;
- }

*DataSize = VarDataSize;
UpdateVariableInfo (VariableName, VendorGuid, Variable.Volatile,
TRUE, FALSE, FALSE, FALSE, &gVariableInfo); @@ -2395,6 +2394,11 @@
VariableServiceGetVariable (
}

Done:
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ if (Attributes != NULL && Variable.CurrPtr != NULL) {
+ *Attributes = Variable.CurrPtr->Attributes;
+ }
+ }
ReleaseLockOnlyAtBootTime (&mVariableModuleGlobal-
VariableGlobal.VariableServicesLock);
return Status;
}
diff --git
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
e.c
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
e.c
index 2cf0ed32ae55..ca833fb0244d 100644
---
a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
e.c
+++
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDx
e.c
@@ -14,6 +14,7 @@
InitCommunicateBuffer() is really function to check the variable data size.

Copyright (c) 2010 - 2019, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -642,10 +643,6 @@ FindVariableInRuntimeCache (
}

CopyMem (Data, GetVariableDataPtr (RtPtrTrack.CurrPtr,
mVariableAuthFormat), TempDataSize);
- if (Attributes != NULL) {
- *Attributes = RtPtrTrack.CurrPtr->Attributes;
- }
-
*DataSize = TempDataSize;

UpdateVariableInfo (VariableName, VendorGuid,
RtPtrTrack.Volatile, TRUE, FALSE, FALSE, TRUE, &mVariableInfo); @@
-661,6 +658,11 @@ FindVariableInRuntimeCache (
}

Done:
+ if (Status == EFI_SUCCESS || Status == EFI_BUFFER_TOO_SMALL) {
+ if (Attributes != NULL && RtPtrTrack.CurrPtr != NULL) {
+ *Attributes = RtPtrTrack.CurrPtr->Attributes;
+ }
+ }
mVariableRuntimeCacheReadLock = FALSE;

return Status;
--
2.16.3.windows.1


Re: [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

Guomin Jiang
 

Reviewed-by: Guomin Jiang <guomin.jiang@intel.com>

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Wang, Jian J
Sent: Monday, March 30, 2020 12:19 PM
To: michael.kubacki@outlook.com; devel@edk2.groups.io
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Wu, Hao A <hao.a.wu@intel.com>
Subject: Re: [edk2-devel] [PATCH v3 2/3] MdeModulePkg VariablePei:
Return GetVariable() attr if EFI_BUFFER_TOO_SMALL

Reviewed-by: Jian J Wang <jian.j.wang@intel.com>

Regards,
Jian

-----Original Message-----
From: michael.kubacki@outlook.com <michael.kubacki@outlook.com>
Sent: Saturday, March 28, 2020 5:56 AM
To: devel@edk2.groups.io
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Gao, Liming
<liming.gao@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Wu, Hao A <hao.a.wu@intel.com>
Subject: [PATCH v3 2/3] MdeModulePkg VariablePei: Return GetVariable()
attr if EFI_BUFFER_TOO_SMALL

From: Michael Kubacki <michael.kubacki@microsoft.com>

REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2062

This commit makes the behavior for PeiGetVariable() match the
following specification-defined behavior. It is now consistent with
the DXE/SMM variable driver implementation.

The UEFI specification v2.8 Errata A Section 8.2 "GetVariable()"
"Attributes" parameter description states:

"If not NULL, a pointer to the memory location to return the
attributes bitmask for the variable. See 'Related Definitions.'
If not NULL, then Attributes is set on output both when EFI_SUCCESS
and when EFI_BUFFER_TOO_SMALL is returned."

The attributes were previously only returned from the implementation
in Variable.c on EFI_SUCCESS. They are now returned on EFI_SUCCESS or
EFI_BUFFER_TOO_SMALL according to spec.

Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
MdeModulePkg/Universal/Variable/Pei/Variable.c | 19
++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/Pei/Variable.c
b/MdeModulePkg/Universal/Variable/Pei/Variable.c
index f61465fc3045..f420b58165b7 100644
--- a/MdeModulePkg/Universal/Variable/Pei/Variable.c
+++ b/MdeModulePkg/Universal/Variable/Pei/Variable.c
@@ -3,6 +3,7 @@
PEI ReadOnly Varaiable2 PPI. These services operates the non
volatile storage space.

Copyright (c) 2006 - 2019, Intel Corporation. All rights
reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -1047,17 +1048,17 @@ PeiGetVariable (
}

GetVariableNameOrData (&StoreInfo, GetVariableDataPtr
(Variable.CurrPtr, VariableHeader, StoreInfo.AuthFlag), VarDataSize,
Data);
-
- if (Attributes != NULL) {
- *Attributes = VariableHeader->Attributes;
- }
-
- *DataSize = VarDataSize;
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
} else {
- *DataSize = VarDataSize;
- return EFI_BUFFER_TOO_SMALL;
+ Status = EFI_BUFFER_TOO_SMALL;
}
+
+ if (Attributes != NULL) {
+ *Attributes = VariableHeader->Attributes; } *DataSize =
+ VarDataSize;
+
+ return Status;
}

/**
--
2.16.3.windows.1


Re: [PATCH v2 1/2] DynamicTablesPkg: SRAT: Fix entry points

Ard Biesheuvel
 

On Tue, 31 Mar 2020 at 09:26, Sami Mujawar <sami.mujawar@arm.com> wrote:

VS2017 reports 'warning C4028: formal parameter 2 different
from declaration' for the library constructor and destructor
interfaces for the SRAT Generator modules.

Remove the CONST qualifier for the ImageHandle and the
SystemTable pointer in the library constructor and destructor
to make it compatible with the formal declaration.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/702_srat_vs2017_compile_warning_v2

Notes:
V2:
- Update commit message to reflect the update to the CONST [SAMI]
qualifier at 2 places in the constructor & destructor.

DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
index 5d56af66608d862e6eca81da812d719f110867d2..74cb7d92a5d8cddd3df8334f3ab55e6fa3e7267a 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
@@ -800,8 +800,8 @@ ACPI_TABLE_GENERATOR SratGenerator = {
EFI_STATUS
EFIAPI
AcpiSratLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -823,8 +823,8 @@ AcpiSratLibConstructor (
EFI_STATUS
EFIAPI
AcpiSratLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


[PATCH v2 1/2] DynamicTablesPkg: SRAT: Fix entry points

Sami Mujawar
 

VS2017 reports 'warning C4028: formal parameter 2 different
from declaration' for the library constructor and destructor
interfaces for the SRAT Generator modules.

Remove the CONST qualifier for the ImageHandle and the
SystemTable pointer in the library constructor and destructor
to make it compatible with the formal declaration.

Signed-off-by: Sami Mujawar <sami.mujawar@arm.com>
---

The changes can be seen at:
https://github.com/samimujawar/edk2/tree/702_srat_vs2017_compile_warning_v2

Notes:
V2:
- Update commit message to reflect the update to the CONST [SAMI]
qualifier at 2 places in the constructor & destructor.

DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
index 5d56af66608d862e6eca81da812d719f110867d2..74cb7d92a5d8cddd3df8334f3ab55e6fa3e7267a 100644
--- a/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
+++ b/DynamicTablesPkg/Library/Acpi/Arm/AcpiSratLibArm/SratGenerator.c
@@ -800,8 +800,8 @@ ACPI_TABLE_GENERATOR SratGenerator = {
EFI_STATUS
EFIAPI
AcpiSratLibConstructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
@@ -823,8 +823,8 @@ AcpiSratLibConstructor (
EFI_STATUS
EFIAPI
AcpiSratLibDestructor (
- IN CONST EFI_HANDLE ImageHandle,
- IN EFI_SYSTEM_TABLE * CONST SystemTable
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE * SystemTable
)
{
EFI_STATUS Status;
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


Re: [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

Ard Biesheuvel
 

On Tue, 31 Mar 2020 at 03:40, Jiang, Guomin <guomin.jiang@intel.com> wrote:

Hi Laszlo,

Thanks for you spending time review the changes.

And I just want to present how to reproduce the build error.

When build OvmfPkgX64, you can encounter this issue with your local change. The error as below:
TcgPei.lib(AutoGen.obj) : error LNK2001: unresolved external symbol _fltused
c:\sourcecodes\tiano\Build\OvmfX64\DEBUG_VS2019\X64\SecurityPkg\Tcg\TcgPei\TcgPei\DEBUG\TcgPei.dll : fatal error LNK1120: 1 unresolved externals

The build command: build -p OvmfPkg\OvmfPkgX64.dsc -b DEBUG -t VS2019 -a X64 -D TPM_ENABLE
The code changes for reproducing this symptom:
- int GLOBAL_USED _fltused = 1;
+ //int GLOBAL_USED _fltused = 1;
The machine: WIN10
The branch: edk2 master
Doesn't the build error go away as well if you simply add FloatUsed.c
to all BaseCryptLib flavours if Visual Studio is being used?

Something like

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 1bbe4f435aec..f40bf18e7f5d 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -27,6 +27,7 @@ [Defines]
#

[Sources]
+ FloatUsed.c | MSFT
InternalCryptLib.h
Hash/CryptMd4.c
Hash/CryptMd5.c


[PATCH] UnitTestFrameworkPkg/UnitTestLib: Correct dereferred pointer.

Guomin Jiang
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2609

The copied pointer (SavedState) will be updated by LoadUnitTestCache
call. But the change of SavedState will not update source pointer, which
is NewFramework->SavedState in this case.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c b/UnitT=
estFrameworkPkg/Library/UnitTestLib/UnitTestLib.c
index b136992d99..71050b5618 100644
--- a/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c
+++ b/UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.c
@@ -209,7 +209,7 @@ InitUnitTestFramework (
EFI_STATUS Status;=0D
UNIT_TEST_FRAMEWORK_HANDLE NewFrameworkHandle;=0D
UNIT_TEST_FRAMEWORK *NewFramework;=0D
- UNIT_TEST_SAVE_HEADER *SavedState;=0D
+ UNIT_TEST_SAVE_HEADER **SavedState;=0D
=0D
Status =3D EFI_SUCCESS;=0D
NewFramework =3D NULL;=0D
@@ -264,8 +264,8 @@ InitUnitTestFramework (
// If there is a persisted context, load it now.=0D
//=0D
if (DoesCacheExist (NewFrameworkHandle)) {=0D
- SavedState =3D (UNIT_TEST_SAVE_HEADER *)NewFramework->SavedState;=0D
- Status =3D LoadUnitTestCache (NewFrameworkHandle, &SavedState);=0D
+ SavedState =3D (UNIT_TEST_SAVE_HEADER **)(&NewFramework->SavedState);=
=0D
+ Status =3D LoadUnitTestCache (NewFrameworkHandle, SavedState);=0D
if (EFI_ERROR (Status)) {=0D
//=0D
// Don't actually report it as an error, but emit a warning.=0D
--=20
2.25.1.windows.1


Re: [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

Ard Biesheuvel
 

On Tue, 31 Mar 2020 at 08:31, Sean via Groups.Io
<sean.brogan=microsoft.com@groups.io> wrote:

@Ard -
pflash change: https://github.com/spbrogan/edk2/pull/12

Logging change - I actually switched OVMF to use stdio since the log is captured either way and now it shows up in the web log output. https://github.com/spbrogan/edk2/pull/13
Even better.

Do you have instructions for the cmdline for Qemu for Arm? Is it something you would like to run?
Not sure I follow. Which command line are we talking about?

All the builds can be seen here or clicking the icon in github:
https://dev.azure.com/tianocore/edk2-ci-play/_build?view=folders&treeState=XEFybVZpcnRQa2ckXEVtdWxhdG9yUGtnJFxPVk1G
Thanks again for the effort. This is going to help tremendously.


Re: [PATCH] .azurepipelines: Enable CI for OvmfPkg and EmulatorPkg

Sean
 

@Ard - 
pflash change: https://github.com/spbrogan/edk2/pull/12

Logging change - I actually switched OVMF to use stdio since the log is captured either way and now it shows up in the web log output.  https://github.com/spbrogan/edk2/pull/13 

Do you have instructions for the cmdline for Qemu for Arm?  Is it something you would like to run?  

All the builds can be seen here or clicking the icon in github: 
https://dev.azure.com/tianocore/edk2-ci-play/_build?view=folders&treeState=XEFybVZpcnRQa2ckXEVtdWxhdG9yUGtnJFxPVk1G


Re: [PATCH 1/3] Platform/Intel: Add all pathes of feature domains to package path

Liming Gao
 

EDKII DEC spec https://github.com/tianocore-docs/edk2-DecSpecification/tree/release/1.27/2_dec_file_overview

DEC File Overview

An EDK II Package (directory) is a directory that contains an EDK II package declaration (DEC) file. Only one DEC file is permitted per directory. EDK II Packages cannot be nested within other EDK II Packages.

Thanks
Liming

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: 2020年3月31日 13:03
To: Gao, Liming <liming.gao@intel.com>; Luo, Heng <heng.luo@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature domains to package path

Liming,
Where can I find the rule?

Thanks,
Ray

-----Original Message-----
From: Gao, Liming <liming.gao@intel.com>
Sent: Tuesday, March 31, 2020 10:52 AM
To: Luo, Heng <heng.luo@intel.com>; Ni, Ray <ray.ni@intel.com>;
devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature
domains to package path

Ray:
Package has dec file in its root directory. Package DSC file is optional.

Thanks
Liming
-----Original Message-----
From: Luo, Heng <heng.luo@intel.com>
Sent: 2020年3月31日 9:25
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
<liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature
domains to package path

Hi Liming,
I will apply the change below if you agree to we treat a folder that contains ".dec" and "dsc" files as a package directory:

diff --git a/Platform/Intel/build_bios.py
b/Platform/Intel/build_bios.py index b9ad980510..bb25699ed8 100644
--- a/Platform/Intel/build_bios.py
+++ b/Platform/Intel/build_bios.py
@@ -16,6 +16,7 @@ imported functions from board directory import os
import re import sys
+import glob
import signal
import shutil
import argparse
@@ -123,7 +124,10 @@ def pre_build(build_config, build_type="DEBUG", silent=False, toolchain=None):
# add all feature domains in WORKSPACE_FEATURES to package path
for filename in os.listdir(config["WORKSPACE_FEATURES"]):
filepath = os.path.join(config["WORKSPACE_FEATURES"], filename)
- if os.path.isdir(filepath):
+ # feature domains folder does not contain dec or dsc file
+ if os.path.isdir(filepath) and \
+ not glob.glob(os.path.join(filepath, "*.dec")) and \
+ not glob.glob(os.path.join(filepath, "*.dsc")):
config["PACKAGES_PATH"] += os.pathsep + filepath
config["PACKAGES_PATH"] += os.pathsep + config["WORKSPACE_DRIVERS"]
config["PACKAGES_PATH"] += os.pathsep + \

Best Regards
Heng

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Monday, March 30, 2020 5:01 PM
To: Luo, Heng <heng.luo@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
<liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature
domains to package path

+ # add all feature domains in WORKSPACE_FEATURES to package path
+ for filename in os.listdir(config["WORKSPACE_FEATURES"]):
+ filepath = os.path.join(config["WORKSPACE_FEATURES"], filename)
+ if os.path.isdir(filepath):
+ config["PACKAGES_PATH"] += os.pathsep + filepath
Will this change include "AdvancedFeaturePkg" and "TemplateFeaturePkg"
folder as well?

Can you please revise the patch to skip adding folders that contains
package contents to the PACKAGES_PATH?

Liming,
What's the criteria of a package? Can we treat a folder that contains ".dec"
and "dsc" files as a package directory?

Thanks,
Ray


Re: [Patch V2 0/3] Fix build error of OpenBoard

Liming Gao
 

Reviewed-by: Liming Gao <liming.gao@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Heng Luo
Sent: 2020年3月31日 11:49
To: devel@edk2.groups.io
Subject: [edk2-devel] [Patch V2 0/3] Fix build error of OpenBoard

*** BLURB HERE ***

Heng Luo (3):
Platform/Intel: Add all pathes of feature domains to package path
Features/Intel: Add LogoFeaturePkg to TemporaryBuildWorkaround
Features/Intel: Correct wrong codes and remove unnecessary codes

Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.dsc | 4 +++- Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaround.inf | 5 ++++-
Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc | 9 ---------
Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf | 2 +-
Platform/Intel/build_bios.py | 10 +++++++++-
5 files changed, 17 insertions(+), 13 deletions(-)

--
2.24.0.windows.2


Re: [PATCH 1/3] Platform/Intel: Add all pathes of feature domains to package path

Ni, Ray
 

Liming,
Where can I find the rule?

Thanks,
Ray

-----Original Message-----
From: Gao, Liming <liming.gao@intel.com>
Sent: Tuesday, March 31, 2020 10:52 AM
To: Luo, Heng <heng.luo@intel.com>; Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature domains to package path

Ray:
Package has dec file in its root directory. Package DSC file is optional.

Thanks
Liming
-----Original Message-----
From: Luo, Heng <heng.luo@intel.com>
Sent: 2020年3月31日 9:25
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature domains to package path

Hi Liming,
I will apply the change below if you agree to we treat a folder that contains ".dec" and "dsc" files as a package directory:

diff --git a/Platform/Intel/build_bios.py b/Platform/Intel/build_bios.py index b9ad980510..bb25699ed8 100644
--- a/Platform/Intel/build_bios.py
+++ b/Platform/Intel/build_bios.py
@@ -16,6 +16,7 @@ imported functions from board directory import os import re import sys
+import glob
import signal
import shutil
import argparse
@@ -123,7 +124,10 @@ def pre_build(build_config, build_type="DEBUG", silent=False, toolchain=None):
# add all feature domains in WORKSPACE_FEATURES to package path
for filename in os.listdir(config["WORKSPACE_FEATURES"]):
filepath = os.path.join(config["WORKSPACE_FEATURES"], filename)
- if os.path.isdir(filepath):
+ # feature domains folder does not contain dec or dsc file
+ if os.path.isdir(filepath) and \
+ not glob.glob(os.path.join(filepath, "*.dec")) and \
+ not glob.glob(os.path.join(filepath, "*.dsc")):
config["PACKAGES_PATH"] += os.pathsep + filepath
config["PACKAGES_PATH"] += os.pathsep + config["WORKSPACE_DRIVERS"]
config["PACKAGES_PATH"] += os.pathsep + \

Best Regards
Heng

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Monday, March 30, 2020 5:01 PM
To: Luo, Heng <heng.luo@intel.com>; devel@edk2.groups.io
Cc: Bi, Dandan <dandan.bi@intel.com>; Gao, Liming
<liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH 1/3] Platform/Intel: Add all pathes of feature
domains to package path

+ # add all feature domains in WORKSPACE_FEATURES to package path
+ for filename in os.listdir(config["WORKSPACE_FEATURES"]):
+ filepath = os.path.join(config["WORKSPACE_FEATURES"], filename)
+ if os.path.isdir(filepath):
+ config["PACKAGES_PATH"] += os.pathsep + filepath
Will this change include "AdvancedFeaturePkg" and "TemplateFeaturePkg"
folder as well?

Can you please revise the patch to skip adding folders that contains
package contents to the PACKAGES_PATH?

Liming,
What's the criteria of a package? Can we treat a folder that contains ".dec"
and "dsc" files as a package directory?

Thanks,
Ray


[Patch V2 3/3] Features/Intel: Correct wrong codes and remove unnecessary codes

Heng Luo
 

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2644

Correct wrong codes and remove unnecessary codes in LogoFeaturePkg.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
---
Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc | 9 --=
-------
Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf | 2 +-
2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeatur=
e.dsc b/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc
index fca0bfd540..d2dcdeb36a 100644
--- a/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc
+++ b/Features/Intel/UserInterface/LogoFeaturePkg/Include/LogoFeature.dsc
@@ -25,15 +25,6 @@
!error "DXE_ARCH must be specified to build this feature!"=0D
!endif=0D
=0D
-##########################################################################=
######=0D
-#=0D
-# Packages Section - Make sure PCD can be directly used in a conditional s=
tatement=0D
-# in a DSC which includes this DSC file.=0D
-#=0D
-##########################################################################=
######=0D
-[Packages]=0D
- LogoFeaturePkg/LogoFeaturePkg.dec=0D
-=0D
##########################################################################=
######=0D
#=0D
# Library Class section - list of all Library Classes needed by this featu=
re.=0D
diff --git a/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory=
.fdf b/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
index 080c87223c..fead9f3b02 100644
--- a/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
+++ b/Features/Intel/UserInterface/LogoFeaturePkg/Include/PostMemory.fdf
@@ -6,7 +6,7 @@
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
##=0D
-!if gSmbiosFeaturePkgTokenSpaceGuid.PcdJpgEnable =3D=3D TRUE=0D
+!if gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable =3D=3D TRUE=0D
INF LogoFeaturePkg/LogoDxe/JpegLogoDxe.inf=0D
!else=0D
INF LogoFeaturePkg/LogoDxe/LogoDxe.inf=0D
--=20
2.24.0.windows.2


[Patch V2 2/3] Features/Intel: Add LogoFeaturePkg to TemporaryBuildWorkaround

Heng Luo
 

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2644

Need to add LogoFeaturePkg to TemporaryBuildWorkaround because
OpenBoard still includes TemporaryBuildWorkaround for building BIOS.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
---
Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildW=
orkaround.dsc | 4 +++-
Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildW=
orkaround.inf | 5 ++++-
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tem=
poraryBuildWorkaround.dsc b/Features/Intel/AdvancedFeaturePkg/TemporaryBuil=
dWorkaround/TemporaryBuildWorkaround.dsc
index 227ae00908..c62f9ecc6e 100644
--- a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryB=
uildWorkaround.dsc
+++ b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryB=
uildWorkaround.dsc
@@ -13,7 +13,7 @@
# When the BaseTools update is complete, this file can entirely be removed=
=0D
# from this package.=0D
#=0D
-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -49,6 +49,8 @@
gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable =
|FALSE=0D
gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable =
|FALSE=0D
gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable =
|FALSE=0D
+ gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable =
|FALSE=0D
+ gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable =
|FALSE=0D
!endif=0D
=0D
#=0D
diff --git a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tem=
poraryBuildWorkaround.inf b/Features/Intel/AdvancedFeaturePkg/TemporaryBuil=
dWorkaround/TemporaryBuildWorkaround.inf
index 74176d1989..00818fbe0a 100644
--- a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryB=
uildWorkaround.inf
+++ b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryB=
uildWorkaround.inf
@@ -13,7 +13,7 @@
# When the BaseTools update is complete, this file can entirely be removed=
=0D
# from this package.=0D
#=0D
-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>=0D
+# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>=0D
#=0D
# SPDX-License-Identifier: BSD-2-Clause-Patent=0D
#=0D
@@ -40,6 +40,7 @@
PowerManagement/S3FeaturePkg/S3FeaturePkg.dec=0D
SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dec=0D
UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec=0D
+ UserInterface/LogoFeaturePkg/LogoFeaturePkg.dec=0D
=0D
[FeaturePcd]=0D
gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable=0D
@@ -49,6 +50,8 @@
gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable=0D
gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable=0D
gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable=0D
+ gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable=0D
+ gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable=0D
=0D
[Sources]=0D
TemporaryBuildWorkaround.c=0D
--=20
2.24.0.windows.2


[Patch V2 1/3] Platform/Intel: Add all pathes of feature domains to package path

Heng Luo
 

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2644

Add all pathes of feature domains to package path in build_bios.py.

Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Signed-off-by: Heng Luo <heng.luo@intel.com>
---

Notes:
v2:
- Skip adding folders that contains package contents to the PACKAGES_PATH. [Ray Ni]

Platform/Intel/build_bios.py | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Platform/Intel/build_bios.py b/Platform/Intel/build_bios.py
index 1ef35aca0a..8f855f63eb 100644
--- a/Platform/Intel/build_bios.py
+++ b/Platform/Intel/build_bios.py
@@ -3,7 +3,7 @@
# Builds BIOS using configuration files and dynamically
# imported functions from board directory
#
-# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#

@@ -16,6 +16,7 @@ imported functions from board directory
import os
import re
import sys
+import glob
import signal
import shutil
import argparse
@@ -120,6 +121,13 @@ def pre_build(build_config, build_type="DEBUG", silent=False, toolchain=None):
config["PACKAGES_PATH"] += os.pathsep + config["WORKSPACE_SILICON"]
config["PACKAGES_PATH"] += os.pathsep + config["WORKSPACE_SILICON_BIN"]
config["PACKAGES_PATH"] += os.pathsep + config["WORKSPACE_FEATURES"]
+ # add all feature domains in WORKSPACE_FEATURES to package path
+ for filename in os.listdir(config["WORKSPACE_FEATURES"]):
+ filepath = os.path.join(config["WORKSPACE_FEATURES"], filename)
+ # feature domains folder does not contain dec file
+ if os.path.isdir(filepath) and \
+ not glob.glob(os.path.join(filepath, "*.dec")):
+ config["PACKAGES_PATH"] += os.pathsep + filepath
config["PACKAGES_PATH"] += os.pathsep + config["WORKSPACE_DRIVERS"]
config["PACKAGES_PATH"] += os.pathsep + \
os.path.join(config["WORKSPACE"], "FSP")
--
2.24.0.windows.2