date:20190912

[edk2-devel] [edk2-platforms][PATCH V2 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

2019-09-12 Thread Kubacki, Michael A

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

These build failures can be reproduced simply by building the
AdvancedFeaturePkg.dsc file in GCC5. To build the whole package
DSC (not pull individual features into other packages), set
the WORKSPACE variable to the edk2 directory in the workspace
as is done by executing edksetup.sh then create the PACKAGES_PATH
variable and add the Platform/Intel and Silicon/Intel directories
to the variable value. Then start the build of AdvancedFeaturePkg.dsc:
  'build -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc'

This change corrects the following issues reported by GCC:
  * BmcAcpi.c - Cast the pointer actual parameter types passed to
functions to the formal parameter type.
  * OsWdt.c - Return the Status variable as the function return value
  in DriverInit ().
  * PeiIpmiInit.c - Return the Status variable as the function return
value in PeimIpmiInterfaceInit ()
  * SolStatus.c - Remove the unused variable SOLEnabled. The variable
  was not removed after a code refactoring.

All future contributions to AdvancedFeaturePkg must successfully build
in GCC after this change.

Cc: Dandan Bi 
Cc: Sai Chaganty 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
---
 Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c  | 16 
+
 Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c | 15 ++--
 Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c  |  9 
 Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c  | 24 

 4 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c 
b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
index 3b330da160..990b4b9e83 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
@@ -1,7 +1,7 @@
 /** @file
   BMC ACPI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -154,7 +154,7 @@ UpdateDeviceSsdtTable (
   IoRsc->BaseAddressMax = PcdGet16(PcdIpmiIoBaseAddress);
 }
   }
-  
+
   return EFI_SUCCESS;
 }
 
@@ -202,7 +202,7 @@ BmcAcpiEntryPoint (
   //
   // Locate the firmware volume protocol
   //
-  Status = LocateSupportProtocol (, , 1);
+  Status = LocateSupportProtocol (, (VOID **) 
, 1);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -216,7 +216,15 @@ BmcAcpiEntryPoint (
   while (!EFI_ERROR (Status)) {
 CurrentTable = NULL;
 
-Status = FwVol->ReadSection (FwVol, , EFI_SECTION_RAW, 
Instance, , (UINTN *) , );
+Status = FwVol->ReadSection (
+  FwVol,
+  ,
+  EFI_SECTION_RAW,
+  Instance,
+  (VOID **) ,
+  (UINTN *) ,
+  
+  );
 if (!EFI_ERROR (Status)) {
   //
   // Perform any table specific updates.
diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c 
b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
index 8245aac8e9..062d20c44e 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
@@ -1,7 +1,7 @@
 /** @file
-IPMI stack initialization in PEI.
+  IPMI stack initialization in PEI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -27,7 +27,7 @@ Routine Description:
 
 Arguments:
 
-Returns: 
+Returns:
   Status
 
 --*/
@@ -35,7 +35,7 @@ Returns:
   EFI_STATUS   Status;
   IPMI_GET_DEVICE_ID_RESPONSE  BmcInfo;
   UINT32   Retries;
-  
+
   //
   // Set up a loop to retry for up to 30 seconds. Calculate retries not timeout
   // so that in case KCS is not enabled and EfiIpmiSendCommand() returns
@@ -71,9 +71,10 @@ Returns:
   The entry point of the Ipmi PEIM.
 
   @param  FileHandle  Handle of the file being invoked.
-  @param  PeiServices Describes the list of possible PEI Services. 
+  @param  PeiServices Describes the list of possible PEI Services.
 
   @retval EFI_SUCCESS   Indicates that Ipmi initialization completed 
successfully.
+  @retval OthersIndicates that Ipmi initialization could not complete 
successfully.
 **/
 EFI_STATUS
 EFIAPI
@@ -85,11 +86,11 @@ PeimIpmiInterfaceInit (
   BOOLEAN  UpdateMode;
   EFI_STATUS   Status;
 
-  DEBUG((EFI_D_ERROR,"IPMI Peim:Get BMC Device Id\n"));
+  DEBUG ((DEBUG_INFO, "IPMI Peim:Get BMC Device Id\n"));
 
   //
   // Get the Device ID and check if the system is in Force Update mode.
   //
   Status = GetDeviceId ();
-  return EFI_SUCCESS;
+  return Status;
 }
diff

Re: [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables

2019-09-12 Thread Nate DeSimone

The patch series has been pushed as 5dd12a154d.. 9ea86f187d

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Zhang, Shenglei
Sent: Wednesday, September 11, 2019 8:27 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A ; Chiu, Chasel 
; Desimone, Nathaniel L 
; Gao, Liming 
Subject: [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables

1.Add error handling to enhance status checking.
2.Initialize the variables before used and add check before
  FreePool().

v2: Update copyright in 01/02.

Cc: Michael Kubacki 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 

Shenglei Zhang (2):
  MinPlatformPkg/AcpiTables: Initialize variables before used
  MinPlatformPkg/AcpiTables: Add error handling to
SortCpuLocalApicInTable

 .../Acpi/AcpiTables/AcpiPlatform.c| 29 ---
 1 file changed, 18 insertions(+), 11 deletions(-)

-- 
2.18.0.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47200): https://edk2.groups.io/g/devel/message/47200
Mute This Topic: https://groups.io/mt/34111550/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: UninstallMultipleProtocols, checkpoint 7-9

2019-09-12 Thread Heinrich Schuchardt

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

BS.UninstallMultipleProtocolInterfaces - InterfaceTestCheckpoint7 - 9
expect UninstallMultipleProtocols() to return EFI_ACCESS_DENIED but the
UEFI spec has:

"If any errors are generated while the protocol interfaces are being
uninstalled, then the protocols uninstalled prior to the error will be
reinstalled with the boot service
EFI_BOOT_SERVICES.InstallProtocolInterface() and the status code
EFI_INVALID_PARAMETER is returned."

So the SCT should check for EFI_INVALID_PARAMETER and not for
EFI_ACCESS_DENIED.

Correct the assertions.

InterfaceCheckpoint6 has already been corrected with commit 30c4031acbdb
("uefi-sct/SctPkg: assertion for UninstallMultipleProtocols")

Signed-off-by: Heinrich Schuchardt 
---
 .../BlackBoxTest/ProtocolHandlerBBTestFunction_2.c  | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ProtocolHandlerServices/BlackBoxTest/ProtocolHandlerBBTestFunction_2.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ProtocolHandlerServices/BlackBoxTest/ProtocolHandlerBBTestFunction_2.c
index fe6146b2..104e93b5 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ProtocolHandlerServices/BlackBoxTest/ProtocolHandlerBBTestFunction_2.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/BootServices/ProtocolHandlerServices/BlackBoxTest/ProtocolHandlerBBTestFunction_2.c
@@ -13108,7 +13108,7 @@ 
BBTestUninstallMultipleProtocolInterfacesInterfaceTestCheckPoint7 (
 //
 // Step 1: check return status
 //
-if (EFI_ACCESS_DENIED == Status) {
+if (EFI_INVALID_PARAMETER == Status) {
   AssertionType = EFI_TEST_ASSERTION_PASSED;
 } else {
   AssertionType = EFI_TEST_ASSERTION_FAILED;
@@ -13453,7 +13453,7 @@ 
BBTestUninstallMultipleProtocolInterfacesInterfaceTestCheckPoint8 (
 //
 // Step 1: check return status
 //
-if (EFI_ACCESS_DENIED == Status) {
+if (EFI_INVALID_PARAMETER == Status) {
   AssertionType = EFI_TEST_ASSERTION_PASSED;
 } else {
   AssertionType = EFI_TEST_ASSERTION_FAILED;
@@ -13807,7 +13807,7 @@ 
BBTestUninstallMultipleProtocolInterfacesInterfaceTestCheckPoint9 (
 //
 // Step 1: check return status
 //
-if (EFI_ACCESS_DENIED == Status) {
+if (EFI_INVALID_PARAMETER == Status) {
   AssertionType = EFI_TEST_ASSERTION_PASSED;
 } else {
   AssertionType = EFI_TEST_ASSERTION_FAILED;
-- 
2.23.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47199): https://edk2.groups.io/g/devel/message/47199
Mute This Topic: https://groups.io/mt/34118784/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers

2019-09-12 Thread Nate DeSimone

Your whitespace doesn't quite match the edk2 coding style guidelines, but we 
can fix that during commit.

Reviewed-by: Nate DeSimone 

-Original Message-
From: Zhang, Shenglei 
Sent: Wednesday, September 11, 2019 7:55 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A ; Chiu, Chasel 
; Desimone, Nathaniel L 
; Gao, Liming 
Subject: [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers

In DxeCheckBootVariable.c, add check for BootOrder and Variable that return 
EFI_NOT_FOUND when they are NULL.
In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL when allocating 
memory to what it points to.

Cc: Michael Kubacki 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---

v2: Update copyright

 .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++-
 .../Test/Library/TestPointCheckLib/DxeCheckGcd.c  | 6 --
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git 
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
 
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
index 85bd5b3d..98130683 100644
--- 
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckBootVariable.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -130,6 +130,9 @@ TestPointCheckLoadOptionVariable (
   for (ListIndex = 0; ListIndex < 
sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]); 
ListIndex++) {
 UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder", 
mLoadOptionVariableList[ListIndex]);
 Status = GetVariable2 (BootOrderName, , (VOID 
**), );
+if(BootOrder == NULL) {
+  return EFI_NOT_FOUND;;
+}
 if (EFI_ERROR(Status)) {
   continue;
 }
@@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable (
 for (Index = 0; ; Index++) {
   UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", 
mKeyOptionVariableList[ListIndex], Index);
   Status = GetVariable2 (KeyOptionName, , 
, );
+  if(Variable == NULL) {
+return EFI_NOT_FOUND;;
+  }
   if (!EFI_ERROR(Status)) {
 DumpKeyOption (KeyOptionName, Variable, Size);
   } else {
diff --git 
a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c 
b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
index 82709d44..c90b37f2 100644
--- a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c
+++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh
+++ eckGcd.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017-2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -241,7 +241,9 @@ TestPointDumpGcd (
   }
 }
 if (GcdMemoryMap != NULL) {
-  *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * 
sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+  if (GcdIoMap != NULL){
+*GcdIoMap = AllocateCopyPool (NumberOfDescriptors * 
sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap);
+  }
   *GcdIoMapNumberOfDescriptors = NumberOfDescriptors;
 }
   }
--
2.18.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47198): https://edk2.groups.io/g/devel/message/47198
Mute This Topic: https://groups.io/mt/33110621/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

2019-09-12 Thread Kubacki, Michael A

While I'm all for improvements throughout the file, the scope of this change is 
just to fix the GCC failures
and make general style improvements to impacted lines. I'll send a patch V2 
with the DEBUG
macro updated to DEBUG_ERROR. Anything further peripheral cleanup is a slippery 
slope for this patch.

Thanks,
Michael

> -Original Message-
> From: Chaganty, Rangasai V
> Sent: Thursday, September 12, 2019 1:31 AM
> To: Kubacki, Michael A ;
> devel@edk2.groups.io
> Cc: Bi, Dandan ; Gao, Liming 
> Subject: RE: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix
> GCC Build Failures
> 
> Two comments on SolStatus.c:
> 1. "SolEnabled" seems more appropriate variable name than just "Enabled.
> Should we consider renaming the variable name?
> 2. Please replace EFI_D_ERROR with DEBUG_ERROR
> 
> -Original Message-
> From: Kubacki, Michael A
> Sent: Wednesday, September 11, 2019 11:09 PM
> To: devel@edk2.groups.io
> Cc: Bi, Dandan ; Chaganty, Rangasai V
> ; Gao, Liming 
> Subject: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC
> Build Failures
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=2059
> 
> These build failures can be reproduced simply by building the
> AdvancedFeaturePkg.dsc file in GCC5. To build the whole package DSC (not
> pull individual features into other packages), set the WORKSPACE variable to
> the edk2 directory in the workspace as is done by executing edksetup.sh
> then create the PACKAGES_PATH variable and add the Platform/Intel and
> Silicon/Intel directories to the variable value. Then start the build of
> AdvancedFeaturePkg.dsc:
>   'build -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc'
> 
> This change corrects the following issues reported by GCC:
>   * BmcAcpi.c - Cast the pointer actual parameter types passed to
> functions to the formal parameter type.
>   * OsWdt.c - Return the Status variable as the function return value
>   in DriverInit ().
>   * PeiIpmiInit.c - Return the Status variable as the function return
> value in PeimIpmiInterfaceInit ()
>   * SolStatus.c - Remove the unused variable SOLEnabled. The variable
>   was not removed after a code refactoring.
> 
> All future contributions to AdvancedFeaturePkg must successfully build in
> GCC after this change.
> 
> Cc: Dandan Bi 
> Cc: Sai Chaganty 
> Cc: Liming Gao 
> Signed-off-by: Michael Kubacki 
> ---
>  Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c  | 16
> 
>  Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c | 15
> ---
>  Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c  |  9 +--
> --
>  Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c  | 14
> +-
>  4 files changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
> b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
> index 3b330da160..990b4b9e83 100644
> --- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
> +++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
> @@ -1,7 +1,7 @@
>  /** @file
>BMC ACPI.
> 
> -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -154,7 +154,7 @@ UpdateDeviceSsdtTable (
>IoRsc->BaseAddressMax = PcdGet16(PcdIpmiIoBaseAddress);
>  }
>}
> -
> +
>return EFI_SUCCESS;
>  }
> 
> @@ -202,7 +202,7 @@ BmcAcpiEntryPoint (
>//
>// Locate the firmware volume protocol
>//
> -  Status = LocateSupportProtocol (,
> , 1);
> +  Status = LocateSupportProtocol (,
> + (VOID **) , 1);
>if (EFI_ERROR (Status)) {
>  return Status;
>}
> @@ -216,7 +216,15 @@ BmcAcpiEntryPoint (
>while (!EFI_ERROR (Status)) {
>  CurrentTable = NULL;
> 
> -Status = FwVol->ReadSection (FwVol, ,
> EFI_SECTION_RAW, Instance, , (UINTN *) , );
> +Status = FwVol->ReadSection (
> +  FwVol,
> +  ,
> +  EFI_SECTION_RAW,
> +  Instance,
> +  (VOID **) ,
> +  (UINTN *) ,
> +  
> +  );
>  if (!EFI_ERROR (Status)) {
>//
>// Perform any table specific updates.
> diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
> b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
> index 8245aac8e9..062d20c44e 100644
> --- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
> +++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
> @@ -1,7 +1,7 @@
>  /** @file
> -IPMI stack initialization in PEI.
> +  IPMI stack initialization in PEI.
> 
> -Copyright (c) 2018, Intel Corporation. All rights reserved.
> +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
>

Re: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable

2019-09-12 Thread Nate DeSimone

Reviewed-by: Nate DeSimone 

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Zhang, Shenglei
Sent: Wednesday, September 11, 2019 8:27 PM
To: devel@edk2.groups.io
Cc: Kubacki, Michael A ; Chiu, Chasel 
; Desimone, Nathaniel L 
; Gao, Liming 
Subject: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error 
handling to SortCpuLocalApicInTable

Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
As a result, when SortCpuLocalApicInTable is called, error handling is needed. 
So add "if" statement to judge the returned Status from SortCpuLocalApicInTable 
() in function InstallMadtFromScratch().

Cc: Michael Kubacki 
Cc: Chasel Chiu 
Cc: Nate DeSimone 
Cc: Liming Gao 
Signed-off-by: Shenglei Zhang 
---
 .../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c   | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c 
b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 2cc55ee8..ae25d753 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
 
   if(MAX_CPU_NUM <= Index) {
 DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index 
Bufferflow\n"));
-ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
+return EFI_INVALID_PARAMETER;
   }
 
   TempVal = mCpuApicIdOrderTable[Index].ApicId;
@@ -874,7 +874,11 @@ InstallMadtFromScratch (
   DetectApicIdMap();
 
   // Call for Local APIC ID Reorder
-  SortCpuLocalApicInTable ();
+  Status = SortCpuLocalApicInTable ();
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
+goto Done;
+  }
 
   MaxMadtStructCount = (UINT32) (
 MAX_CPU_NUM +// processor local APIC structures
--
2.18.0.windows.1





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47196): https://edk2.groups.io/g/devel/message/47196
Mute This Topic: https://groups.io/mt/34111553/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v3] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Philippe Mathieu-Daudé

On 9/12/19 6:58 PM, Ard Biesheuvel wrote:
> After upgrading the CI system we use for building the ArmVirtPkg
> targets, we started seeing failures due to the NOOPT build running
> out of space when using the CLANG38 toolchain definition combined
> with clang 7.
> 
> We really don't want to increase the FD/FV sizes in general to
> accommodate this, so parameterize the relevant quantities and
> increase them by 50% for NOOPT builds.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
> v3: - don't rely on fragile ordering of DEFINEs for the target dependent
>   default value, but instead, use a single FD_SIZE_IN_MB macro whose
>   default is DEFINEd either to 2 or 3 depend on the build target. That
>   permits switching back to 2 MB for NOOPT builds if desired while
>   changing the default to 3 MB
> - fix a few image header definitions that I missed for ARM32 + Xen
> 
> v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> and b) to avoid adding another .inc file
> update kernel header field, as pointed out by Philippe
> 
> 
>  ArmVirtPkg/ArmVirt.dsc.inc   | 15 
>  ArmVirtPkg/ArmVirtQemu.fdf   | 14 +---
>  ArmVirtPkg/ArmVirtQemuKernel.fdf | 24 +---
>  ArmVirtPkg/ArmVirtXen.fdf| 24 +---
>  4 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index a4ae25d982a2..10037c938eb8 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -10,6 +10,21 @@
>  [Defines]
>DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
>  
> +!if $(TARGET) != NOOPT
> +  DEFINE FD_SIZE_IN_MB= 2
> +!else
> +  DEFINE FD_SIZE_IN_MB= 3
> +!endif
> +
> +!if $(FD_SIZE_IN_MB) == 2
> +  DEFINE FD_SIZE  = 0x20
> +  DEFINE FD_NUM_BLOCKS= 0x200
> +!endif
> +!if $(FD_SIZE_IN_MB) == 3
> +  DEFINE FD_SIZE  = 0x30
> +  DEFINE FD_NUM_BLOCKS= 0x300
> +!endif
> +
>  
> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>  
> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> index c2169cb7964b..2c8936a1ae15 100644
> --- a/ArmVirtPkg/ArmVirtQemu.fdf
> +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>  
> +[Defines]
> +!if $(FD_SIZE_IN_MB) == 2
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
> +!endif
> +!if $(FD_SIZE_IN_MB) == 3
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
> +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
>  ErasePolarity = 1
>  
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>  
>  
> 
>  #
> @@ -59,7 +67,7 @@ DATA = {
>  !endif
>  }
>  
> -0x1000|0x001ff000
> +0x1000|$(FVMAIN_COMPACT_SIZE)
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>  
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf 
> b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> index f675b6d65ee1..72fc8dd698f8 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>  
> +[Defines]
> +!if $(FD_SIZE_IN_MB) == 2
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1f8000
> +!endif
> +!if $(FD_SIZE_IN_MB) == 3
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2f8000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
> +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
>  ErasePolarity = 1
>  
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>  
>  
> 
>  #
> @@ -56,7 +64,12 @@ DATA = {
>0x01, 0x00, 0x00, 0x10, # code0: adr x1, .
>0xff, 0x1f, 0x00, 0x14, # code1: b 0x8000
>0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
> +!if $(FD_SIZE_IN_MB) == 2
>0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00,

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Philippe Mathieu-Daudé

On 9/12/19 7:13 PM, Ard Biesheuvel wrote:
> On Thu, 12 Sep 2019 at 18:05, Philippe Mathieu-Daudé  
> wrote:
>>
>> On 9/12/19 6:50 PM, Ard Biesheuvel wrote:
>>> On Thu, 12 Sep 2019 at 16:20, Philippe Mathieu-Daudé  
>>> wrote:

 Hi Ard,

 On 9/11/19 6:23 PM, Ard Biesheuvel wrote:
> After upgrading the CI system we use for building the ArmVirtPkg
> targets, we started seeing failures due to the NOOPT build running
> out of space when using the CLANG38 toolchain definition combined
> with clang 7.
>
> We really don't want to increase the FD/FV sizes in general to
> accommodate this, so parameterize the relevant quantities and
> increase them by 50% for NOOPT builds.
>
> Signed-off-by: Ard Biesheuvel 
> ---
> v2: implement suggestions by Laszlo on 1) how to parameterize this 
> further,
> and b) to avoid adding another .inc file
> update kernel header field, as pointed out by Philippe
>
>  ArmVirtPkg/ArmVirt.dsc.inc   | 28 
>  ArmVirtPkg/ArmVirtQemu.fdf   | 14 +++---
>  ArmVirtPkg/ArmVirtQemuKernel.fdf | 19 ++---
>  ArmVirtPkg/ArmVirtXen.fdf| 14 +++---
>  4 files changed, 66 insertions(+), 9 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index a4ae25d982a2..d6b58e5c018b 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -10,6 +10,34 @@
>  [Defines]
>DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
>
> +  #
> +  # Flash size selection. Setting FD_SIZE_IN_KB on the command line 
> directly to
> +  # one of the supported values, in place of any of the convenience 
> macros, is
> +  # permitted.
> +  #
> +!if $(TARGET) == NOOPT
> +  DEFINE FD_SIZE_3MB  = TRUE
> +!endif
> +
> +!ifdef $(FD_SIZE_2MB)
> +  DEFINE FD_SIZE_IN_KB= 2048
> +!else
> +!ifdef $(FD_SIZE_3MB)
> +  DEFINE FD_SIZE_IN_KB= 3072
> +!else
> +  DEFINE FD_SIZE_IN_KB= 2048
> +!endif
> +!endif
> +
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FD_SIZE  = 0x20
> +  DEFINE FD_NUM_BLOCKS= 0x200
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FD_SIZE  = 0x30
> +  DEFINE FD_NUM_BLOCKS= 0x300
> +!endif
> +
>  
> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> index c2169cb7964b..d3950c8be05e 100644
> --- a/ArmVirtPkg/ArmVirtQemu.fdf
> +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>
> +[Defines]
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The 
> size in bytes of the FLASH Device
> +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The 
> size in bytes of the FLASH Device
>  ErasePolarity = 1
>
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>
>  
> 
>  #
> @@ -59,7 +67,7 @@ DATA = {
>  !endif
>  }
>
> -0x1000|0x001ff000
> +0x1000|$(FVMAIN_COMPACT_SIZE)
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf 
> b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> index f675b6d65ee1..46ec967e1cc0 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>
> +[Defines]
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1f8000
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2f8000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The 
> size in bytes of the FLASH Device
>

[edk2-devel] [PATCH v3] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Ard Biesheuvel

After upgrading the CI system we use for building the ArmVirtPkg
targets, we started seeing failures due to the NOOPT build running
out of space when using the CLANG38 toolchain definition combined
with clang 7.

We really don't want to increase the FD/FV sizes in general to
accommodate this, so parameterize the relevant quantities and
increase them by 50% for NOOPT builds.

Signed-off-by: Ard Biesheuvel 
---
v3: - don't rely on fragile ordering of DEFINEs for the target dependent
  default value, but instead, use a single FD_SIZE_IN_MB macro whose
  default is DEFINEd either to 2 or 3 depend on the build target. That
  permits switching back to 2 MB for NOOPT builds if desired while
  changing the default to 3 MB
- fix a few image header definitions that I missed for ARM32 + Xen

v2: implement suggestions by Laszlo on 1) how to parameterize this further,
and b) to avoid adding another .inc file
update kernel header field, as pointed out by Philippe


 ArmVirtPkg/ArmVirt.dsc.inc   | 15 
 ArmVirtPkg/ArmVirtQemu.fdf   | 14 +---
 ArmVirtPkg/ArmVirtQemuKernel.fdf | 24 +---
 ArmVirtPkg/ArmVirtXen.fdf| 24 +---
 4 files changed, 68 insertions(+), 9 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index a4ae25d982a2..10037c938eb8 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -10,6 +10,21 @@
 [Defines]
   DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
 
+!if $(TARGET) != NOOPT
+  DEFINE FD_SIZE_IN_MB= 2
+!else
+  DEFINE FD_SIZE_IN_MB= 3
+!endif
+
+!if $(FD_SIZE_IN_MB) == 2
+  DEFINE FD_SIZE  = 0x20
+  DEFINE FD_NUM_BLOCKS= 0x200
+!endif
+!if $(FD_SIZE_IN_MB) == 3
+  DEFINE FD_SIZE  = 0x30
+  DEFINE FD_NUM_BLOCKS= 0x300
+!endif
+
 
[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
 
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index c2169cb7964b..2c8936a1ae15 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -20,14 +20,22 @@
 #
 

 
+[Defines]
+!if $(FD_SIZE_IN_MB) == 2
+  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
+!endif
+!if $(FD_SIZE_IN_MB) == 3
+  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
+!endif
+
 [FD.QEMU_EFI]
 BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU assigns 
0 - 0x800 for a BootROM
-Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size in 
bytes of the FLASH Device
+Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size in 
bytes of the FLASH Device
 ErasePolarity = 1
 
 # This one is tricky, it must be: BlockSize * NumBlocks = Size
 BlockSize = 0x1000
-NumBlocks = 0x200
+NumBlocks = $(FD_NUM_BLOCKS)
 
 

 #
@@ -59,7 +67,7 @@ DATA = {
 !endif
 }
 
-0x1000|0x001ff000
+0x1000|$(FVMAIN_COMPACT_SIZE)
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf b/ArmVirtPkg/ArmVirtQemuKernel.fdf
index f675b6d65ee1..72fc8dd698f8 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
+++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
@@ -20,14 +20,22 @@
 #
 

 
+[Defines]
+!if $(FD_SIZE_IN_MB) == 2
+  DEFINE FVMAIN_COMPACT_SIZE  = 0x1f8000
+!endif
+!if $(FD_SIZE_IN_MB) == 3
+  DEFINE FVMAIN_COMPACT_SIZE  = 0x2f8000
+!endif
+
 [FD.QEMU_EFI]
 BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU assigns 
0 - 0x800 for a BootROM
-Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size in 
bytes of the FLASH Device
+Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size in 
bytes of the FLASH Device
 ErasePolarity = 1
 
 # This one is tricky, it must be: BlockSize * NumBlocks = Size
 BlockSize = 0x1000
-NumBlocks = 0x200
+NumBlocks = $(FD_NUM_BLOCKS)
 
 

 #
@@ -56,7 +64,12 @@ DATA = {
   0x01, 0x00, 0x00, 0x10, # code0: adr x1, .
   0xff, 0x1f, 0x00, 0x14, # code1: b 0x8000
   0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
+!if $(FD_SIZE_IN_MB) == 2
   0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 2 MB
+!endif
+!if $(FD_SIZE_IN_MB) == 3
+  0x00, 0x00, 0x30, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 3 MB
+!endif
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # flags
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res2
   0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, # res3
@@ -76,12 +89,17

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Ard Biesheuvel

On Thu, 12 Sep 2019 at 16:20, Philippe Mathieu-Daudé  wrote:
>
> Hi Ard,
>
> On 9/11/19 6:23 PM, Ard Biesheuvel wrote:
> > After upgrading the CI system we use for building the ArmVirtPkg
> > targets, we started seeing failures due to the NOOPT build running
> > out of space when using the CLANG38 toolchain definition combined
> > with clang 7.
> >
> > We really don't want to increase the FD/FV sizes in general to
> > accommodate this, so parameterize the relevant quantities and
> > increase them by 50% for NOOPT builds.
> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> > v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> > and b) to avoid adding another .inc file
> > update kernel header field, as pointed out by Philippe
> >
> >  ArmVirtPkg/ArmVirt.dsc.inc   | 28 
> >  ArmVirtPkg/ArmVirtQemu.fdf   | 14 +++---
> >  ArmVirtPkg/ArmVirtQemuKernel.fdf | 19 ++---
> >  ArmVirtPkg/ArmVirtXen.fdf| 14 +++---
> >  4 files changed, 66 insertions(+), 9 deletions(-)
> >
> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> > index a4ae25d982a2..d6b58e5c018b 100644
> > --- a/ArmVirtPkg/ArmVirt.dsc.inc
> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> > @@ -10,6 +10,34 @@
> >  [Defines]
> >DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
> >
> > +  #
> > +  # Flash size selection. Setting FD_SIZE_IN_KB on the command line 
> > directly to
> > +  # one of the supported values, in place of any of the convenience 
> > macros, is
> > +  # permitted.
> > +  #
> > +!if $(TARGET) == NOOPT
> > +  DEFINE FD_SIZE_3MB  = TRUE
> > +!endif
> > +
> > +!ifdef $(FD_SIZE_2MB)
> > +  DEFINE FD_SIZE_IN_KB= 2048
> > +!else
> > +!ifdef $(FD_SIZE_3MB)
> > +  DEFINE FD_SIZE_IN_KB= 3072
> > +!else
> > +  DEFINE FD_SIZE_IN_KB= 2048
> > +!endif
> > +!endif
> > +
> > +!if $(FD_SIZE_IN_KB) == 2048
> > +  DEFINE FD_SIZE  = 0x20
> > +  DEFINE FD_NUM_BLOCKS= 0x200
> > +!endif
> > +!if $(FD_SIZE_IN_KB) == 3072
> > +  DEFINE FD_SIZE  = 0x30
> > +  DEFINE FD_NUM_BLOCKS= 0x300
> > +!endif
> > +
> >  
> > [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> >GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> > index c2169cb7964b..d3950c8be05e 100644
> > --- a/ArmVirtPkg/ArmVirtQemu.fdf
> > +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> > @@ -20,14 +20,22 @@
> >  #
> >  
> > 
> >
> > +[Defines]
> > +!if $(FD_SIZE_IN_KB) == 2048
> > +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
> > +!endif
> > +!if $(FD_SIZE_IN_KB) == 3072
> > +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
> > +!endif
> > +
> >  [FD.QEMU_EFI]
> >  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> > assigns 0 - 0x800 for a BootROM
> > -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> > in bytes of the FLASH Device
> > +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> > in bytes of the FLASH Device
> >  ErasePolarity = 1
> >
> >  # This one is tricky, it must be: BlockSize * NumBlocks = Size
> >  BlockSize = 0x1000
> > -NumBlocks = 0x200
> > +NumBlocks = $(FD_NUM_BLOCKS)
> >
> >  
> > 
> >  #
> > @@ -59,7 +67,7 @@ DATA = {
> >  !endif
> >  }
> >
> > -0x1000|0x001ff000
> > +0x1000|$(FVMAIN_COMPACT_SIZE)
> >  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> >  FV = FVMAIN_COMPACT
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf 
> > b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > index f675b6d65ee1..46ec967e1cc0 100644
> > --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> > @@ -20,14 +20,22 @@
> >  #
> >  
> > 
> >
> > +[Defines]
> > +!if $(FD_SIZE_IN_KB) == 2048
> > +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1f8000
> > +!endif
> > +!if $(FD_SIZE_IN_KB) == 3072
> > +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2f8000
> > +!endif
> > +
> >  [FD.QEMU_EFI]
> >  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> > assigns 0 - 0x800 for a BootROM
> > -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> > in bytes of the FLASH Device
> > +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> > in bytes of the FLASH Device
> >  ErasePolarity = 1
> >
> >  # This one is tricky, it must be: BlockSize * NumBlocks = Size
> >  BlockSize = 0x1000
> > -NumBlocks = 0x200
> > +NumBlocks = $(FD_NUM_BLOCKS)
> >
> >  
> > 
> >  #
> > @@ -56,7 +64,12 @@ DATA = {
> >

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Philippe Mathieu-Daudé

Hi Ard,

On 9/11/19 6:23 PM, Ard Biesheuvel wrote:
> After upgrading the CI system we use for building the ArmVirtPkg
> targets, we started seeing failures due to the NOOPT build running
> out of space when using the CLANG38 toolchain definition combined
> with clang 7.
> 
> We really don't want to increase the FD/FV sizes in general to
> accommodate this, so parameterize the relevant quantities and
> increase them by 50% for NOOPT builds.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
> v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> and b) to avoid adding another .inc file
> update kernel header field, as pointed out by Philippe
> 
>  ArmVirtPkg/ArmVirt.dsc.inc   | 28 
>  ArmVirtPkg/ArmVirtQemu.fdf   | 14 +++---
>  ArmVirtPkg/ArmVirtQemuKernel.fdf | 19 ++---
>  ArmVirtPkg/ArmVirtXen.fdf| 14 +++---
>  4 files changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index a4ae25d982a2..d6b58e5c018b 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -10,6 +10,34 @@
>  [Defines]
>DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
>  
> +  #
> +  # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly 
> to
> +  # one of the supported values, in place of any of the convenience macros, 
> is
> +  # permitted.
> +  #
> +!if $(TARGET) == NOOPT
> +  DEFINE FD_SIZE_3MB  = TRUE
> +!endif
> +
> +!ifdef $(FD_SIZE_2MB)
> +  DEFINE FD_SIZE_IN_KB= 2048
> +!else
> +!ifdef $(FD_SIZE_3MB)
> +  DEFINE FD_SIZE_IN_KB= 3072
> +!else
> +  DEFINE FD_SIZE_IN_KB= 2048
> +!endif
> +!endif
> +
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FD_SIZE  = 0x20
> +  DEFINE FD_NUM_BLOCKS= 0x200
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FD_SIZE  = 0x30
> +  DEFINE FD_NUM_BLOCKS= 0x300
> +!endif
> +
>  
> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>  
> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> index c2169cb7964b..d3950c8be05e 100644
> --- a/ArmVirtPkg/ArmVirtQemu.fdf
> +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>  
> +[Defines]
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
> +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
>  ErasePolarity = 1
>  
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>  
>  
> 
>  #
> @@ -59,7 +67,7 @@ DATA = {
>  !endif
>  }
>  
> -0x1000|0x001ff000
> +0x1000|$(FVMAIN_COMPACT_SIZE)
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>  
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf 
> b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> index f675b6d65ee1..46ec967e1cc0 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>  
> +[Defines]
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1f8000
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2f8000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
> +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
>  ErasePolarity = 1
>  
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>  
>  
> 
>  #
> @@ -56,7 +64,12 @@ DATA = {
>0x01, 0x00, 0x00, 0x10, # code0: adr x1, .
>0xff, 0x1f, 0x00, 0x14, # code1: b 0x8000
>0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, # text_offset: 512 KB
> +!if $(FD_SIZE_IN_KB) == 2048
>0x00, 0x00, 0x20, 0x00, 0x00, 0x00, 0x00, 0x00, # image_size: 2 MB
> +!endif
>

Re: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

2019-09-12 Thread Nate DeSimone

Hi Jiewen,

Looking at the code yes you are right, sorry I forgot what exactly VarCheckLib 
hooked in to versus didn’t hook into. The other one that same to mind was the 
EDKII_VARIABLE_LOCK_PROTOCOL, but again, that only affects writes not reads.

Since the current variable driver does not have any confidentiality feature I 
think this should be fine from a security standpoint. However, if we ever add a 
feature to make certain variables confidential then this will need to be 
revisited.

Thanks,
Nate

From: Yao, Jiewen
Sent: Tuesday, September 10, 2019 8:32 PM
To: Desimone, Nathaniel L ; 
devel@edk2.groups.io; Johnson, Michael ; Kubacki, 
Michael A 
Subject: RE: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

Nate
I believe this SMI reduction work only handle GetVariable.

VarCheckLib only handles SetVariable.
VarCheckLib does not handle GetVaraible.

Thank you
Yao Jiewen

From: Desimone, Nathaniel L
Sent: Wednesday, September 11, 2019 10:43 AM
To: devel@edk2.groups.io; Yao, Jiewen 
mailto:jiewen@intel.com>>; Johnson, Michael 
mailto:michael.john...@intel.com>>; Kubacki, Michael 
A mailto:michael.a.kuba...@intel.com>>
Subject: RE: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

Hi All,

There is a security issue with regard to the way VarCheckLib works. There are 
plenty of usages of VarCheckLib that are intended to prevent ring0 from reading 
a variable after ReadyToBoot() is called. If we assume a malicious operating 
system, then having a ring0 buffered version of the variable that VarCheckLib 
is attempting to prevent the OS from reading would provide a backdoor for the 
OS to read that protected variable’s contents.

Thanks,
Nate

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Yao, Jiewen
Sent: Sunday, September 8, 2019 3:36 PM
To: devel@edk2.groups.io; Johnson, Michael 
mailto:michael.john...@intel.com>>; Kubacki, Michael 
A mailto:michael.a.kuba...@intel.com>>
Subject: Re: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

Hey, from security perspective, I am not clear what is difference on below 2 
scenario – TPM or read-modify-write.

Whenever we return some data from SMM, we are in ring0, any program in ring0 
may modify the variable content in the communication buffer.
If we assume ring0 is malicious, then the malicious code may let one AP keep 
looping to monitor the communication data, when BSP call get (authenticated) 
variable. Once communication buffer is updated and status is filled to 
EFI_SUCCESS, the AP may modify the communication buffer, then the BSP will 
return *modified* data to caller. Or an interrupt handler in BSP may also 
modify the communication buffer before the data is returned to caller. This 
race condition exists today.

I think putting cache buffer to SMM just raise the BAR, but *NOT* a security 
solution, because SMM communication buffer in reserved memory is same as cache 
buffer.

Thank you
Yao Jiewen

From: devel@edk2.groups.io 
mailto:devel@edk2.groups.io>> On Behalf Of Johnson, 
Michael
Sent: Saturday, September 7, 2019 5:52 AM
To: Kubacki, Michael A 
mailto:michael.a.kuba...@intel.com>>; 
devel@edk2.groups.io
Subject: Re: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

Yes - both things I bring up are just *different* ring 0 accesses than are 
(easily) allowed today, so are not fundamentally new.  Generally we either 
trust all of ring 0 or none of it, so neither is a showstopper.

Reading back from real varstore/SMM if the variable has the auth attribute 
removes any interesting vectors so all that changes is a bad ring 0 agent can 
go through memory instead of the RT API, which is not threatening.

I have no problems if write and auth-read come from SMM and all else comes from 
cache.

From: Kubacki, Michael A
Sent: Friday, September 6, 2019 2:48 PM
To: Johnson, Michael 
mailto:michael.john...@intel.com>>; 
devel@edk2.groups.io
Subject: RE: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

My understanding is both of your points return to the issue of a ring 0 entity 
potentially modifying the runtime cache. As the SetVariable ( ) API is already 
accessible to ring 0, the variables could similarly be updated today so that 
should not be an issue. You have a good point for authenticated variables where 
the update is authenticated in SMM so the variable data should continue to be 
returned from SMM.

How about if the variable has the authenticated attribute set, those are sent 
to GetVariable ( ) in SMM? This should be relatively rare with the most common 
case likely being secure boot related keys.

From: Johnson, Michael 
mailto:michael.john...@intel.com>>
Sent: Thursday, September 5, 2019 1:59 PM
To: Kubacki; Kubacki, Michael A 
mailto:michael.a.kuba...@intel.com>>;

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Ard Biesheuvel

On Thu, 12 Sep 2019 at 15:46, Laszlo Ersek  wrote:
>
> On 09/11/19 18:23, Ard Biesheuvel wrote:
> > After upgrading the CI system we use for building the ArmVirtPkg
> > targets, we started seeing failures due to the NOOPT build running
> > out of space when using the CLANG38 toolchain definition combined
> > with clang 7.
> >
> > We really don't want to increase the FD/FV sizes in general to
> > accommodate this, so parameterize the relevant quantities and
> > increase them by 50% for NOOPT builds.
> >
> > Signed-off-by: Ard Biesheuvel 
> > ---
> > v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> > and b) to avoid adding another .inc file
> > update kernel header field, as pointed out by Philippe
> >
> >  ArmVirtPkg/ArmVirt.dsc.inc   | 28 
> >  ArmVirtPkg/ArmVirtQemu.fdf   | 14 +++---
> >  ArmVirtPkg/ArmVirtQemuKernel.fdf | 19 ++---
> >  ArmVirtPkg/ArmVirtXen.fdf| 14 +++---
> >  4 files changed, 66 insertions(+), 9 deletions(-)
> >
> > diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> > index a4ae25d982a2..d6b58e5c018b 100644
> > --- a/ArmVirtPkg/ArmVirt.dsc.inc
> > +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> > @@ -10,6 +10,34 @@
> >  [Defines]
> >DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
> >
> > +  #
> > +  # Flash size selection. Setting FD_SIZE_IN_KB on the command line 
> > directly to
> > +  # one of the supported values, in place of any of the convenience 
> > macros, is
> > +  # permitted.
> > +  #
> > +!if $(TARGET) == NOOPT
> > +  DEFINE FD_SIZE_3MB  = TRUE
> > +!endif
>
> Sorry, I must have been unclear -- I meant the macros FD_SIZE_2MB /
> FD_SIZE_3MB / FD_SIZE_IN_KB as a replacement for the $(TARGET) based
> logic, not as an addition to it.
>
> Is it important to select the 3MB build by default, in case "-b NOOPT"
> is given on the build command line?
>

Yes.

> My thinking was that, in your CI env, you could pass -D FD_SIZE_3MB
> specifically, whenever you build for -b NOOPT. Other users building for
> -b NOOPT would see no change.
>
> My suggestion would be:
>
> (1) s/for NOOPT builds/with -D FD_SIZE_3MB/ in the commit message
>
> (2) drop the above three lines (the TARGET-based conditional)
>
> With those:
>
> Reviewed-by: Laszlo Ersek 
>
> and you could go ahead and push the patch (no repost needed).
>
> OTOH, if you'd really like to set 3MB for NOOPT automatically, then I
> think we'll need a way for letting users override that new default, back
> to 2MB.
>
> ... Or is such an override possible with this patch already, perhaps?
>

Yes. If you set -b NOOPT -D FD_SIZE_2MB (in any order), you will get a
2 MB image.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47186): https://edk2.groups.io/g/devel/message/47186
Mute This Topic: https://groups.io/mt/34105412/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Laszlo Ersek

On 09/12/19 16:46, Laszlo Ersek wrote:
> On 09/11/19 18:23, Ard Biesheuvel wrote:
>> After upgrading the CI system we use for building the ArmVirtPkg
>> targets, we started seeing failures due to the NOOPT build running
>> out of space when using the CLANG38 toolchain definition combined
>> with clang 7.
>>
>> We really don't want to increase the FD/FV sizes in general to
>> accommodate this, so parameterize the relevant quantities and
>> increase them by 50% for NOOPT builds.
>>
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> v2: implement suggestions by Laszlo on 1) how to parameterize this further,
>> and b) to avoid adding another .inc file
>> update kernel header field, as pointed out by Philippe

> (1) s/for NOOPT builds/with -D FD_SIZE_3MB/ in the commit message

(there are two instances: subject line, and commit message body)

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47185): https://edk2.groups.io/g/devel/message/47185
Mute This Topic: https://groups.io/mt/34105412/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

2019-09-12 Thread Laszlo Ersek

On 09/11/19 18:23, Ard Biesheuvel wrote:
> After upgrading the CI system we use for building the ArmVirtPkg
> targets, we started seeing failures due to the NOOPT build running
> out of space when using the CLANG38 toolchain definition combined
> with clang 7.
> 
> We really don't want to increase the FD/FV sizes in general to
> accommodate this, so parameterize the relevant quantities and
> increase them by 50% for NOOPT builds.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
> v2: implement suggestions by Laszlo on 1) how to parameterize this further,
> and b) to avoid adding another .inc file
> update kernel header field, as pointed out by Philippe
> 
>  ArmVirtPkg/ArmVirt.dsc.inc   | 28 
>  ArmVirtPkg/ArmVirtQemu.fdf   | 14 +++---
>  ArmVirtPkg/ArmVirtQemuKernel.fdf | 19 ++---
>  ArmVirtPkg/ArmVirtXen.fdf| 14 +++---
>  4 files changed, 66 insertions(+), 9 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index a4ae25d982a2..d6b58e5c018b 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -10,6 +10,34 @@
>  [Defines]
>DEFINE DEBUG_PRINT_ERROR_LEVEL = 0x804F
>  
> +  #
> +  # Flash size selection. Setting FD_SIZE_IN_KB on the command line directly 
> to
> +  # one of the supported values, in place of any of the convenience macros, 
> is
> +  # permitted.
> +  #
> +!if $(TARGET) == NOOPT
> +  DEFINE FD_SIZE_3MB  = TRUE
> +!endif

Sorry, I must have been unclear -- I meant the macros FD_SIZE_2MB /
FD_SIZE_3MB / FD_SIZE_IN_KB as a replacement for the $(TARGET) based
logic, not as an addition to it.

Is it important to select the 3MB build by default, in case "-b NOOPT"
is given on the build command line?

My thinking was that, in your CI env, you could pass -D FD_SIZE_3MB
specifically, whenever you build for -b NOOPT. Other users building for
-b NOOPT would see no change.

My suggestion would be:

(1) s/for NOOPT builds/with -D FD_SIZE_3MB/ in the commit message

(2) drop the above three lines (the TARGET-based conditional)

With those:

Reviewed-by: Laszlo Ersek 

and you could go ahead and push the patch (no repost needed).

OTOH, if you'd really like to set 3MB for NOOPT automatically, then I
think we'll need a way for letting users override that new default, back
to 2MB.

... Or is such an override possible with this patch already, perhaps?

Thanks!
Laszlo


> +
> +!ifdef $(FD_SIZE_2MB)
> +  DEFINE FD_SIZE_IN_KB= 2048
> +!else
> +!ifdef $(FD_SIZE_3MB)
> +  DEFINE FD_SIZE_IN_KB= 3072
> +!else
> +  DEFINE FD_SIZE_IN_KB= 2048
> +!endif
> +!endif
> +
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FD_SIZE  = 0x20
> +  DEFINE FD_NUM_BLOCKS= 0x200
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FD_SIZE  = 0x30
> +  DEFINE FD_NUM_BLOCKS= 0x300
> +!endif
> +
>  
> [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>  
> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> index c2169cb7964b..d3950c8be05e 100644
> --- a/ArmVirtPkg/ArmVirtQemu.fdf
> +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>  
> +[Defines]
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1ff000
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2ff000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   = 0x|gArmTokenSpaceGuid.PcdFdBaseAddress  # QEMU 
> assigns 0 - 0x800 for a BootROM
> -Size  = 0x0020|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
> +Size  = $(FD_SIZE)|gArmTokenSpaceGuid.PcdFdSize # The size 
> in bytes of the FLASH Device
>  ErasePolarity = 1
>  
>  # This one is tricky, it must be: BlockSize * NumBlocks = Size
>  BlockSize = 0x1000
> -NumBlocks = 0x200
> +NumBlocks = $(FD_NUM_BLOCKS)
>  
>  
> 
>  #
> @@ -59,7 +67,7 @@ DATA = {
>  !endif
>  }
>  
> -0x1000|0x001ff000
> +0x1000|$(FVMAIN_COMPACT_SIZE)
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>  
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.fdf 
> b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> index f675b6d65ee1..46ec967e1cc0 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.fdf
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.fdf
> @@ -20,14 +20,22 @@
>  #
>  
> 
>  
> +[Defines]
> +!if $(FD_SIZE_IN_KB) == 2048
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x1f8000
> +!endif
> +!if $(FD_SIZE_IN_KB) == 3072
> +  DEFINE FVMAIN_COMPACT_SIZE  = 0x2f8000
> +!endif
> +
>  [FD.QEMU_EFI]
>  BaseAddress   =

Re: [edk2-devel] Copyright EDK II C Coding Standards Specification

2019-09-12 Thread Stephano Cetola


On 9/11/2019 11:16 PM, Heinrich Schuchardt wrote:

Hello Stephano,

the EDK II C Coding Standards Specification is available at:

https://raw.githubusercontent.com/tianocore-docs/Docs/master/Specifications/CCS_2_1_Draft.pdf

That's a link to an older doc. Check out the latest docs here:

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications




It is copyright restricted, allowing only copying for internal use.



The new docs should all use BSD license language. Let me know if you run 
into any that aren't formatted in that way.


Cheers,
Stephano

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47183): https://edk2.groups.io/g/devel/message/47183
Mute This Topic: https://groups.io/mt/34112329/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [Patch V2] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock

2019-09-12 Thread Dong, Eric

Cc other reviewers.

Reviewed-by: Eric Dong 

Thanks,
Eric
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of John E
> Lofgren
> Sent: Tuesday, September 10, 2019 2:41 AM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [Patch V2] UefiCpuPkg/CpuExceptionHandlerLib: Fix
> #AC split lock
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2150
> 
> V2 changes:
> Add xchg 16 bit instructions to handle sgdt and sidt base
> 63:48 bits and 47:32 bits.
> Add comment to explain why xchg 64bit isnt being used
> 
> Fix #AC split lock's caused by seperating base and limit from sgdt and sidt by
> changing xchg operands to 32-bit to stop from crossing cacheline.
> 
> Signed-off-by: John E Lofgren 
> ---
> 
> UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nas
> m | 20 ++--
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> index 4db1a09f28..7b7642b290 100644
> ---
> a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.n
> asm
> +++
> b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.
> +++ nasm
> @@ -180,21 +180,29 @@ HasErrorCode:
>  pushqword [rbp + 24]
> 
>  ;; UINT64  Gdtr[2], Idtr[2];
> +; sidt and sgdt saves 10 bytes to memory, 8 bytes = base and 2 bytes =
> limit.
> +; To avoid #AC split lock when separating base and limit into their
> +; own separate 64 bit memory, we can’t use 64 bit xchg since base
> [63:48] bits
> +; may cross the cache line.
>  xor rax, rax
>  pushrax
>  pushrax
>  sidt[rsp]
> -xchgrax, [rsp + 2]
> -xchgrax, [rsp]
> -xchgrax, [rsp + 8]
> +xchgeax, [rsp + 2]
> +xchgeax, [rsp]
> +xchgeax, [rsp + 8]
> +xchg ax, [rsp + 6]
> +xchg ax, [rsp + 4]
> 
>  xor rax, rax
>  pushrax
>  pushrax
>  sgdt[rsp]
> -xchgrax, [rsp + 2]
> -xchgrax, [rsp]
> -xchgrax, [rsp + 8]
> +xchgeax, [rsp + 2]
> +xchgeax, [rsp]
> +xchgeax, [rsp + 8]
> +xchg ax, [rsp + 6]
> +xchg ax, [rsp + 4]
> 
>  ;; UINT64  Ldtr, Tr;
>  xor rax, rax
> --
> 2.16.2.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47182): https://edk2.groups.io/g/devel/message/47182
Mute This Topic: https://groups.io/mt/34083544/21656
Mute #ac: https://groups.io/mk?hashtag=ac=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 1/1] uefi-sct/SctPkg AdapterInfo SetInformation pass on EFI_UNSUPPORTED

2019-09-12 Thread Tom Zhao

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

AdapterInfoProtocolTest SetInformation Func and Conf change to
pass on EFI_UNSUPPORTED, better fitting the UEFI Spec.

Cc: Supreeth Venkatesh 
Cc: Eric Jin Eric Jin 
Signed-off-by: Tom Zhao 
---
 
uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestConformance.c
 | 4 ++--
 
uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestFunction.c
| 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestConformance.c
b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestConformance.c
index fb42398e4a97..3dc048db5333 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestConformance.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestConformance.c
@@ -556,8 +556,8 @@ BBTestSetInformationConformanceTestCheckpoint2 (
NULL,
InformationBlockSize
);
-  -if ( Status != EFI_INVALID_PARAMETER && Status !=
EFI_WRITE_PROTECTED) {
+
+if ( Status != EFI_INVALID_PARAMETER && Status !=
EFI_WRITE_PROTECTED && Status != EFI_UNSUPPORTED) {
 AssertionType = EFI_TEST_ASSERTION_FAILED;
 } else {
 AssertionType = EFI_TEST_ASSERTION_PASSED;
diff --git
a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestFunction.c
b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestFunction.c
index 334bb9edc493..4b054b0ef7d8 100644
---
a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestFunction.c
+++
b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/AdapterInfo/BlackBoxTest/AdapterInfoBBTestFunction.c
@@ -372,11 +372,11 @@ BBTestSetInformationFunctionTestCheckpoint1 (
  InformationBlockSize1
  );
 -if (Status == EFI_SUCCESS || Status ==  EFI_WRITE_PROTECTED)
-  AssertionType = EFI_TEST_ASSERTION_PASSED;  +if (Status ==
EFI_SUCCESS || Status ==  EFI_WRITE_PROTECTED || Status == EFI_UNSUPPORTED)
+  AssertionType = EFI_TEST_ASSERTION_PASSED;
 else
-  AssertionType = EFI_TEST_ASSERTION_FAILED; -+
AssertionType = EFI_TEST_ASSERTION_FAILED;
+
 StandardLib->RecordAssertion (
  StandardLib,
  AssertionType,
-- 
2.21.0


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47181): https://edk2.groups.io/g/devel/message/47181
Mute This Topic: https://groups.io/mt/34113242/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

2019-09-12 Thread Chaganty, Rangasai V

Two comments on SolStatus.c:
1. "SolEnabled" seems more appropriate variable name than just "Enabled. Should 
we consider renaming the variable name?
2. Please replace EFI_D_ERROR with DEBUG_ERROR

-Original Message-
From: Kubacki, Michael A 
Sent: Wednesday, September 11, 2019 11:09 PM
To: devel@edk2.groups.io
Cc: Bi, Dandan ; Chaganty, Rangasai V 
; Gao, Liming 
Subject: [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build 
Failures

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

These build failures can be reproduced simply by building the 
AdvancedFeaturePkg.dsc file in GCC5. To build the whole package DSC (not pull 
individual features into other packages), set the WORKSPACE variable to the 
edk2 directory in the workspace as is done by executing edksetup.sh then create 
the PACKAGES_PATH variable and add the Platform/Intel and Silicon/Intel 
directories to the variable value. Then start the build of 
AdvancedFeaturePkg.dsc:
  'build -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc'

This change corrects the following issues reported by GCC:
  * BmcAcpi.c - Cast the pointer actual parameter types passed to
functions to the formal parameter type.
  * OsWdt.c - Return the Status variable as the function return value
  in DriverInit ().
  * PeiIpmiInit.c - Return the Status variable as the function return
value in PeimIpmiInterfaceInit ()
  * SolStatus.c - Remove the unused variable SOLEnabled. The variable
  was not removed after a code refactoring.

All future contributions to AdvancedFeaturePkg must successfully build in GCC 
after this change.

Cc: Dandan Bi 
Cc: Sai Chaganty 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
---
 Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c  | 16 

 Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c | 15 
---
 Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c  |  9 +
 Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c  | 14 
+-
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c 
b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
index 3b330da160..990b4b9e83 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
@@ -1,7 +1,7 @@
 /** @file
   BMC ACPI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -154,7 +154,7 @@ UpdateDeviceSsdtTable (
   IoRsc->BaseAddressMax = PcdGet16(PcdIpmiIoBaseAddress);
 }
   }
-
+
   return EFI_SUCCESS;
 }
 
@@ -202,7 +202,7 @@ BmcAcpiEntryPoint (
   //
   // Locate the firmware volume protocol
   //
-  Status = LocateSupportProtocol (, , 1);
+  Status = LocateSupportProtocol (, 
+ (VOID **) , 1);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -216,7 +216,15 @@ BmcAcpiEntryPoint (
   while (!EFI_ERROR (Status)) {
 CurrentTable = NULL;
 
-Status = FwVol->ReadSection (FwVol, , EFI_SECTION_RAW, 
Instance, , (UINTN *) , );
+Status = FwVol->ReadSection (
+  FwVol,
+  ,
+  EFI_SECTION_RAW,
+  Instance,
+  (VOID **) ,
+  (UINTN *) ,
+  
+  );
 if (!EFI_ERROR (Status)) {
   //
   // Perform any table specific updates.
diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c 
b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
index 8245aac8e9..062d20c44e 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
@@ -1,7 +1,7 @@
 /** @file
-IPMI stack initialization in PEI.
+  IPMI stack initialization in PEI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -27,7 +27,7 @@ Routine Description:
 
 Arguments:
 
-Returns: 
+Returns:
   Status
 
 --*/
@@ -35,7 +35,7 @@ Returns:
   EFI_STATUS   Status;
   IPMI_GET_DEVICE_ID_RESPONSE  BmcInfo;
   UINT32   Retries;
-
+
   //
   // Set up a loop to retry for up to 30 seconds. Calculate retries not timeout
   // so that in case KCS is not enabled and EfiIpmiSendCommand() returns @@ 
-71,9 +71,10 @@ Returns:
   The entry point of the Ipmi PEIM.
 
   @param  FileHandle  Handle of the file being invoked.
-  @param  PeiServices Describes the list of possible PEI Services. 
+  @param  PeiServices Describes the list of possible PEI Services.
 
   @retval EFI_SUCCESS   Indicates that Ipmi initialization completed 
successfully.
+  @retval Others

[edk2-devel] [PATCH] BaseTools:Replace PlatformInfo with PlatformAutoGen for Moudle

2019-09-12 Thread Fan, ZhijuX

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=2188

build -p MdeModulePkg\MdeModulePkg.dsc -a IA32 -m
MdeModulePkg\Universal\PCD\Pei\Pcd.inf

Error:
AttributeError: 'PlatformInfo' object has no attribute
'DynamicPcdList'

The DSC data object used to build a separate module today 
is PlatformInfo rather than PlatformAutoGen
'PlatformAutoGen' object has attribute 'DynamicPcdList'

This patch is going to fixed this issue

Cc: Liming Gao 
Cc: Bob Feng 
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/build/build.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/BaseTools/Source/Python/build/build.py 
b/BaseTools/Source/Python/build/build.py
index e81d3d6486..e845c139c9 100755
--- a/BaseTools/Source/Python/build/build.py
+++ b/BaseTools/Source/Python/build/build.py
@@ -1860,6 +1860,9 @@ class Build():
 Ma = ModuleAutoGen(Wa, Module, BuildTarget, 
ToolChain, Arch, self.PlatformFile,Pa.DataPipe)
 if Ma is None:
 continue
+if Ma.PcdIsDriver:
+Ma.PlatformInfo = Pa
+Ma.Workspace = Wa
 MaList.append(Ma)
 
 if GlobalData.gBinCacheSource and self.Target in 
[None, "", "all"]:
-- 
2.14.1.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47179): https://edk2.groups.io/g/devel/message/47179
Mute This Topic: https://groups.io/mt/34112948/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

<>

[edk2-devel] [PATCH] BaseTools:change some incorrect parameter defaults

2019-09-12 Thread Fan, ZhijuX

BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1858

for Dict={},There are pitfalls in the way this default parameter is set
and Dict is not used in functions, other functions have these two cases,
I will change some incorrect parameter defaults

This patch is going to fix this issue

Cc: Liming Gao 
Cc: Bob Feng 
Signed-off-by: Zhiju.Fan 
---
 BaseTools/Source/Python/AutoGen/BuildEngine.py |  4 +++-
 BaseTools/Source/Python/AutoGen/GenMake.py |  6 --
 BaseTools/Source/Python/Common/RangeExpression.py  |  4 +++-
 BaseTools/Source/Python/Common/StringUtils.py  | 10 +++---
 BaseTools/Source/Python/GenFds/AprioriSection.py   |  4 +++-
 BaseTools/Source/Python/GenFds/CompressSection.py  |  4 +++-
 BaseTools/Source/Python/GenFds/DataSection.py  |  4 +++-
 BaseTools/Source/Python/GenFds/EfiSection.py   |  4 +++-
 BaseTools/Source/Python/GenFds/FfsFileStatement.py |  5 -
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |  5 +++--
 BaseTools/Source/Python/GenFds/Fv.py   |  4 +++-
 BaseTools/Source/Python/GenFds/FvImageSection.py   |  4 +++-
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py |  2 +-
 BaseTools/Source/Python/GenFds/GuidSection.py  |  4 +++-
 BaseTools/Source/Python/GenFds/OptRomFileStatement.py  |  5 -
 BaseTools/Source/Python/GenFds/Region.py   |  4 +++-
 BaseTools/Source/Python/GenFds/Section.py  |  2 +-
 BaseTools/Source/Python/GenFds/UiSection.py|  4 +++-
 BaseTools/Source/Python/GenFds/VerSection.py   |  4 +++-
 19 files changed, 60 insertions(+), 23 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/BuildEngine.py 
b/BaseTools/Source/Python/AutoGen/BuildEngine.py
index bb91534477..069a3a1c9d 100644
--- a/BaseTools/Source/Python/AutoGen/BuildEngine.py
+++ b/BaseTools/Source/Python/AutoGen/BuildEngine.py
@@ -176,7 +176,9 @@ class FileBuildRule:
 CommandString = "\n\t".join(self.CommandList)
 return "%s : %s\n\t%s" % (DestString, SourceString, CommandString)
 
-def Instantiate(self, Macros={}):
+def Instantiate(self, Macros = None):
+if Macros is None:
+Macros = {}
 NewRuleObject = copy.copy(self)
 NewRuleObject.BuildTargets = {}
 NewRuleObject.DestFileList = []
diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 4f85a93055..3185ebe368 100755
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -205,10 +205,12 @@ class BuildFile(object):
 def GetRemoveDirectoryCommand(self, DirList):
 return [self._RD_TEMPLATE_[self._FileType] % {'dir':Dir} for Dir in 
DirList]
 
-def PlaceMacro(self, Path, MacroDefinitions={}):
+def PlaceMacro(self, Path, MacroDefinitions=None):
 if Path.startswith("$("):
 return Path
 else:
+if MacroDefinitions is None:
+MacroDefinitions = {}
 PathLength = len(Path)
 for MacroName in MacroDefinitions:
 MacroValue = MacroDefinitions[MacroName]
@@ -1762,4 +1764,4 @@ def GetDependencyList(AutoGenObject, FileCache, File, 
ForceList, SearchPathList)
 
 # This acts like the main() function for the script, unless it is 'import'ed 
into another script.
 if __name__ == '__main__':
-pass
\ No newline at end of file
+pass
diff --git a/BaseTools/Source/Python/Common/RangeExpression.py 
b/BaseTools/Source/Python/Common/RangeExpression.py
index e9296e03f6..039d281467 100644
--- a/BaseTools/Source/Python/Common/RangeExpression.py
+++ b/BaseTools/Source/Python/Common/RangeExpression.py
@@ -342,7 +342,9 @@ class RangeExpression(BaseExpression):
 raise BadExpression(ERR_STRING_EXPR % Operator)
 
 
-def __init__(self, Expression, PcdDataType, SymbolTable = {}):
+def __init__(self, Expression, PcdDataType, SymbolTable = None):
+if SymbolTable is None:
+SymbolTable = {}
 super(RangeExpression, self).__init__(self, Expression, PcdDataType, 
SymbolTable)
 self._NoProcess = False
 if not isinstance(Expression, type('')):
diff --git a/BaseTools/Source/Python/Common/StringUtils.py 
b/BaseTools/Source/Python/Common/StringUtils.py
index 0febbc0034..73dafa797a 100644
--- a/BaseTools/Source/Python/Common/StringUtils.py
+++ b/BaseTools/Source/Python/Common/StringUtils.py
@@ -243,8 +243,10 @@ def SplitModuleType(Key):
 #
 # @retval NewList   A new string list whose macros are replaced
 #
-def ReplaceMacros(StringList, MacroDefinitions={}, SelfReplacement=False):
+def ReplaceMacros(StringList, MacroDefinitions=None, SelfReplacement=False):
 NewList = []
+if MacroDefinitions is None:
+MacroDefinitions = {}
 for String in StringList:
 if isinstance(String, type('')):
 NewList.append(ReplaceMacro(String, MacroDefinitions, 
SelfReplacement))

[edk2-devel] [edk2-core] [PATCH v3 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec

2019-09-12 Thread Heinrich Schuchardt

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

The UEFI spec requires that if any error occurs in
UninstallMultipleProtocolInterfaces(), EFI_INVALID_PARAMETER is returned
and not the return code of UninstallProtocolInterface().

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Dandan Bi 
---
v3
Use @retval instead of @return.
The protocols are reinstalled in the same order in which they are
uninstalled.
v2
Adjust the subject line.
Adjust the function comments to clarify the behavior.
This replaces https://edk2.groups.io/g/devel/message/46974
---
 MdeModulePkg/Core/Dxe/Hand/Handle.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c 
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index b2721b3ab2..81a13c6ae5 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -802,20 +802,27 @@ Done:
 
 
 
-
 /**
   Uninstalls a list of protocol interface in the boot services environment.
-  This function calls UnisatllProtocolInterface() in a loop. This is
+  This function calls UninstallProtocolInterface() in a loop. This is
   basically a lib function to save space.
 
-  @param  Handle The handle to uninstall the protocol
+  If any errors are generated while the protocol interfaces are being
+  uninstalled, then the protocol interfaces uninstalled prior to the error will
+  be reinstalled and EFI_INVALID_PARAMETER will be returned.
+
+  @param  Handle The handle to uninstall the protocol 
interfaces
+ from.
   @param  ...EFI_GUID followed by protocol instance. A NULL
- terminates the  list. The pairs are the
+ terminates the list. The pairs are the
  arguments to UninstallProtocolInterface(). All
  the protocols are added to Handle.
 
-  @return Status code
-
+  @retval EFI_SUCCESSif all protocol interfaces where uninstalled.
+  @retval EFI_INVALID_PARAMETER  if any protocol interface could not be
+ uninstalled and an attempt was made to
+ reinstall previously uninstalled protocol
+ interfaces.
 **/
 EFI_STATUS
 EFIAPI
@@ -864,6 +871,7 @@ CoreUninstallMultipleProtocolInterfaces (
   CoreInstallProtocolInterface (, Protocol, EFI_NATIVE_INTERFACE, 
Interface);
 }
 VA_END (Args);
+Status = EFI_INVALID_PARAMETER;
   }
 
   return Status;
-- 
2.20.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47177): https://edk2.groups.io/g/devel/message/47177
Mute This Topic: https://groups.io/mt/34112406/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec

2019-09-12 Thread Heinrich Schuchardt


On 9/12/19 3:59 AM, Bi, Dandan wrote:

Hi ,

Thanks for the update.
I don't know how you send this patch, the format and subject seems a little 
different from V1 patch. Anyway it doesn't block my review of this patch.

I have two minor comments for the function comments:
1. Per EDKII C Coding Spec, @retval for each unique return value, @return for a 
function's return values when those values aren't easily described by @retval 
commands.


Thanks for pointing me to the coding spec (available at
https://github.com/tianocore-docs/Docs/raw/master/Specifications/CCS_2_1_Draft.pdf).

I will correct that. Looking at the rest of the file you will other
examples of incorrect use of @return: CoreUnregisterProtocolNotifyEvent.


So here I think we should use @retval instead of @return.
2. Per my understanding, the protocol interfaces uninstalled prior to the error 
seems not to be reinstalled in reverse order of uninstalling, the 
reinstallation seems in the same order of previous uninstalling (start from the 
first pair after the handle to next...). Please help double check this issue 
and update the comments if needed .


You are right. VA_ARG() moves form first to last.

Thanks for reviewing. Best regards

Heinrich



Since these are the comments update and with these addressed,  Reviewed-by: Dandan Bi 


Thanks,
Dandan

-Original Message-
From: Heinrich Schuchardt [mailto:xypron.g...@gmx.de]
Sent: Tuesday, September 10, 2019 4:12 PM
To: EDK II Development ; Bi, Dandan

Cc: Wu, Hao A ; Wang, Jian J ;
Gao, Liming ; Zeng, Star ; Yao,
Jiewen ; Laszlo Ersek ; Jin, Eric
; Supreeth Venkatesh ;
Stephano Cetola ; Heinrich Schuchardt

Subject: [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in
UninstallMultipleProtocol follow Spec

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

The UEFI spec requires that if any error occurs in
UninstallMultipleProtocolInterfaces(), EFI_INVALID_PARAMETER is returned
and not the return code of UninstallProtocolInterface().

Signed-off-by: Heinrich Schuchardt 
---
v2
Adjust the subject line.
Adjust the function comments to clarify the behavior.
This replaces https://edk2.groups.io/g/devel/message/46974
---
  MdeModulePkg/Core/Dxe/Hand/Handle.c | 21 +++--
  1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Hand/Handle.c
b/MdeModulePkg/Core/Dxe/Hand/Handle.c
index b2721b3ab2..719ba98261 100644
--- a/MdeModulePkg/Core/Dxe/Hand/Handle.c
+++ b/MdeModulePkg/Core/Dxe/Hand/Handle.c
@@ -802,20 +802,28 @@ Done:
- /**   Uninstalls a list of protocol interface in the boot services
environment.-  This function calls UnisatllProtocolInterface() in a loop. This
is+  This function calls UninstallProtocolInterface() in a loop. This is   
basically a
lib function to save space. -  @param  Handle The handle to 
uninstall
the protocol+  If any errors are generated while the protocol interfaces are
being+  uninstalled, then the protocol interfaces uninstalled prior to the error
will+  be reinstalled in reverse order of uninstalling and
EFI_INVALID_PARAMETER is+  returned.++  @param  Handle The
handle to uninstall the protocol interfaces+ 
from.   @param  ...
EFI_GUID followed by protocol instance. A NULL- 
terminates
the  list. The pairs are the+ terminates the 
list. The pairs are
the  arguments to UninstallProtocolInterface(). 
All
the protocols are added to Handle. -  @return Status code-+  @return
EFI_SUCCESSif all protocol interfaces where uninstalled.+  @return
EFI_INVALID_PARAMETER  if any protocol interface could not be+
uninstalled and an attempt was made to+ 
reinstall previously
uninstalled protocol+ interfaces. **/ 
EFI_STATUS EFIAPI@@ -
864,6 +872,7 @@ CoreUninstallMultipleProtocolInterfaces (
CoreInstallProtocolInterface (, Protocol, EFI_NATIVE_INTERFACE,
Interface); } VA_END (Args);+Status = EFI_INVALID_PARAMETER;   }
return Status;--
2.20.1






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47176): https://edk2.groups.io/g/devel/message/47176
Mute This Topic: https://groups.io/mt/34089787/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Copyright EDK II C Coding Standards Specification

2019-09-12 Thread Heinrich Schuchardt


Hello Stephano,

the EDK II C Coding Standards Specification is available at:

https://raw.githubusercontent.com/tianocore-docs/Docs/master/Specifications/CCS_2_1_Draft.pdf

It is copyright restricted, allowing only copying for internal use.

As EDK II has moved to be open source wouldn't it make sense to apply
some more liberal copyright license for the accompanying material too, e.g.

https://creativecommons.org/licenses/by/4.0/

Best regards

Heinrich

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47174): https://edk2.groups.io/g/devel/message/47174
Mute This Topic: https://groups.io/mt/34112329/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable

2019-09-12 Thread Kubacki, Michael A

Reviewed-by: Michael Kubacki 

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Zhang,
> Shenglei
> Sent: Wednesday, September 11, 2019 8:27 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Gao, Liming 
> Subject: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error
> handling to SortCpuLocalApicInTable
> 
> Change ASSERT_EFI_ERROR to return value when the "if" statement is true.
> As a result, when SortCpuLocalApicInTable is called, error handling is needed.
> So add "if" statement to judge the returned Status from
> SortCpuLocalApicInTable () in function InstallMadtFromScratch().
> 
> Cc: Michael Kubacki 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Signed-off-by: Shenglei Zhang 
> ---
>  .../Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c   | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 2cc55ee8..ae25d753 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -354,7 +354,7 @@ SortCpuLocalApicInTable (
> 
>if(MAX_CPU_NUM <= Index) {
>  DEBUG ((EFI_D_ERROR, "Asserting the SortCpuLocalApicInTable Index
> Bufferflow\n"));
> -ASSERT_EFI_ERROR(EFI_INVALID_PARAMETER);
> +return EFI_INVALID_PARAMETER;
>}
> 
>TempVal = mCpuApicIdOrderTable[Index].ApicId;
> @@ -874,7 +874,11 @@ InstallMadtFromScratch (
>DetectApicIdMap();
> 
>// Call for Local APIC ID Reorder
> -  SortCpuLocalApicInTable ();
> +  Status = SortCpuLocalApicInTable ();
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((EFI_D_ERROR, "SortCpuLocalApicInTable failed: %r\n", Status));
> +goto Done;
> +  }
> 
>MaxMadtStructCount = (UINT32) (
>  MAX_CPU_NUM +// processor local APIC structures
> --
> 2.18.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47175): https://edk2.groups.io/g/devel/message/47175
Mute This Topic: https://groups.io/mt/34111553/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used

2019-09-12 Thread Kubacki, Michael A

Reviewed-by: Michael Kubacki 

> -Original Message-
> From: Zhang, Shenglei
> Sent: Wednesday, September 11, 2019 8:27 PM
> To: devel@edk2.groups.io
> Cc: Kubacki, Michael A ; Chiu, Chasel
> ; Desimone, Nathaniel L
> ; Gao, Liming 
> Subject: [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables
> before used
> 
> MadtStructs, NewMadtTable and MaxMadtStructCount are not initialized
> before used or at the proper place. So assign values to them at the beginning
> and change the logic when freeing MadtStructs and NewMadtTable.
> 
> Cc: Michael Kubacki 
> Cc: Chasel Chiu 
> Cc: Nate DeSimone 
> Cc: Liming Gao 
> Signed-off-by: Shenglei Zhang 
> ---
>  .../Acpi/AcpiTables/AcpiPlatform.c| 21 +++
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 5eb72792..2cc55ee8 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1,7 +1,7 @@
>  /** @file
>ACPI Platform Driver
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
>  **/
> @@ -867,13 +867,15 @@ InstallMadtFromScratch (
>UINT32  PcIoApicMask;
>UINTN   PcIoApicIndex;
> 
> +  MadtStructs = NULL;
> +  NewMadtTable = NULL;
> +  MaxMadtStructCount = 0;
> +
>DetectApicIdMap();
> 
>// Call for Local APIC ID Reorder
>SortCpuLocalApicInTable ();
> 
> -  NewMadtTable = NULL;
> -
>MaxMadtStructCount = (UINT32) (
>  MAX_CPU_NUM +// processor local APIC structures
>  MAX_CPU_NUM +// processor local x2APIC structures
> @@ -1115,14 +1117,15 @@ Done:
>//
>// Free memory
>//
> -  for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> -if (MadtStructs[MadtStructsIndex] != NULL) {
> -  FreePool (MadtStructs[MadtStructsIndex]);
> +  if (MadtStructs != NULL){
> +for (MadtStructsIndex = 0; MadtStructsIndex < MaxMadtStructCount;
> MadtStructsIndex++) {
> +  if (MadtStructs[MadtStructsIndex] != NULL) {
> +FreePool (MadtStructs[MadtStructsIndex]);
> +  }
>  }
> +FreePool (MadtStructs);
>}
> -
> -  FreePool (MadtStructs);
> -
> +
>if (NewMadtTable != NULL) {
>  FreePool (NewMadtTable);
>}
> --
> 2.18.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#47173): https://edk2.groups.io/g/devel/message/47173
Mute This Topic: https://groups.io/mt/34111552/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

2019-09-12 Thread Kubacki, Michael A

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

These build failures can be reproduced simply by building the
AdvancedFeaturePkg.dsc file in GCC5. To build the whole package
DSC (not pull individual features into other packages), set
the WORKSPACE variable to the edk2 directory in the workspace
as is done by executing edksetup.sh then create the PACKAGES_PATH
variable and add the Platform/Intel and Silicon/Intel directories
to the variable value. Then start the build of AdvancedFeaturePkg.dsc:
  'build -p AdvancedFeaturePkg/AdvancedFeaturePkg.dsc'

This change corrects the following issues reported by GCC:
  * BmcAcpi.c - Cast the pointer actual parameter types passed to
functions to the formal parameter type.
  * OsWdt.c - Return the Status variable as the function return value
  in DriverInit ().
  * PeiIpmiInit.c - Return the Status variable as the function return
value in PeimIpmiInterfaceInit ()
  * SolStatus.c - Remove the unused variable SOLEnabled. The variable
  was not removed after a code refactoring.

All future contributions to AdvancedFeaturePkg must successfully build
in GCC after this change.

Cc: Dandan Bi 
Cc: Sai Chaganty 
Cc: Liming Gao 
Signed-off-by: Michael Kubacki 
---
 Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c  | 16 

 Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c | 15 
---
 Platform/Intel/AdvancedFeaturePkg/Ipmi/OsWdt/OsWdt.c  |  9 +
 Platform/Intel/AdvancedFeaturePkg/Ipmi/SolStatus/SolStatus.c  | 14 
+-
 4 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c 
b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
index 3b330da160..990b4b9e83 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/BmcAcpi/BmcAcpi.c
@@ -1,7 +1,7 @@
 /** @file
   BMC ACPI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -154,7 +154,7 @@ UpdateDeviceSsdtTable (
   IoRsc->BaseAddressMax = PcdGet16(PcdIpmiIoBaseAddress);
 }
   }
-  
+
   return EFI_SUCCESS;
 }
 
@@ -202,7 +202,7 @@ BmcAcpiEntryPoint (
   //
   // Locate the firmware volume protocol
   //
-  Status = LocateSupportProtocol (, , 1);
+  Status = LocateSupportProtocol (, (VOID **) 
, 1);
   if (EFI_ERROR (Status)) {
 return Status;
   }
@@ -216,7 +216,15 @@ BmcAcpiEntryPoint (
   while (!EFI_ERROR (Status)) {
 CurrentTable = NULL;
 
-Status = FwVol->ReadSection (FwVol, , EFI_SECTION_RAW, 
Instance, , (UINTN *) , );
+Status = FwVol->ReadSection (
+  FwVol,
+  ,
+  EFI_SECTION_RAW,
+  Instance,
+  (VOID **) ,
+  (UINTN *) ,
+  
+  );
 if (!EFI_ERROR (Status)) {
   //
   // Perform any table specific updates.
diff --git a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c 
b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
index 8245aac8e9..062d20c44e 100644
--- a/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
+++ b/Platform/Intel/AdvancedFeaturePkg/Ipmi/IpmiInit/PeiIpmiInit.c
@@ -1,7 +1,7 @@
 /** @file
-IPMI stack initialization in PEI.
+  IPMI stack initialization in PEI.
 
-Copyright (c) 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -27,7 +27,7 @@ Routine Description:
 
 Arguments:
 
-Returns: 
+Returns:
   Status
 
 --*/
@@ -35,7 +35,7 @@ Returns:
   EFI_STATUS   Status;
   IPMI_GET_DEVICE_ID_RESPONSE  BmcInfo;
   UINT32   Retries;
-  
+
   //
   // Set up a loop to retry for up to 30 seconds. Calculate retries not timeout
   // so that in case KCS is not enabled and EfiIpmiSendCommand() returns
@@ -71,9 +71,10 @@ Returns:
   The entry point of the Ipmi PEIM.
 
   @param  FileHandle  Handle of the file being invoked.
-  @param  PeiServices Describes the list of possible PEI Services. 
+  @param  PeiServices Describes the list of possible PEI Services.
 
   @retval EFI_SUCCESS   Indicates that Ipmi initialization completed 
successfully.
+  @retval OthersIndicates that Ipmi initialization could not complete 
successfully.
 **/
 EFI_STATUS
 EFIAPI
@@ -85,11 +86,11 @@ PeimIpmiInterfaceInit (
   BOOLEAN  UpdateMode;
   EFI_STATUS   Status;
 
-  DEBUG((EFI_D_ERROR,"IPMI Peim:Get BMC Device Id\n"));
+  DEBUG ((DEBUG_INFO, "IPMI Peim:Get BMC Device Id\n"));
 
   //
   // Get the Device ID and check if the system is in Force Update mode.
   //
   Status = GetDeviceId ();
-  return EFI_SUCCESS;
+  return Status;
 }
diff

[edk2-devel] [Patch v2] SecurityPkg Tcg2Config: Move common definitions to new Tcg2Internal.h

2019-09-12 Thread Liming Gao

Common definitions are not consumed by VFR. They are not required to be
defined in Tcg2ConfigNvData.h with WA way. New shared internal header
file is added to include those common definitions.

Cc: Jian Wang 
Cc: Chao Zhang 
Signed-off-by: Liming Gao 
---
In V2:
 Keep struct TCG2_DEVICE_DETECTION in Tcg2ConfigNvData.h.
 Only remove BUGBUG definition.

 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c   |  1 +
 SecurityPkg/Tcg/Tcg2Config/TpmDetection.c |  1 +
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf  |  1 +
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.h   |  3 +++
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h | 35 ---
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf  |  1 +
 SecurityPkg/Tcg/Tcg2Config/Tcg2Internal.h | 26 
 7 files changed, 33 insertions(+), 35 deletions(-)
 create mode 100644 SecurityPkg/Tcg/Tcg2Config/Tcg2Internal.h

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
index a50a128766..a15919685e 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
@@ -23,6 +23,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 
 #include "Tcg2ConfigNvData.h"
+#include "Tcg2Internal.h"
 
 TPM_INSTANCE_ID  mTpmInstanceId[] = TPM_INSTANCE_ID_LIST;
 
diff --git a/SecurityPkg/Tcg/Tcg2Config/TpmDetection.c 
b/SecurityPkg/Tcg/Tcg2Config/TpmDetection.c
index 2f220c6c90..eeaadc5e2f 100644
--- a/SecurityPkg/Tcg/Tcg2Config/TpmDetection.c
+++ b/SecurityPkg/Tcg/Tcg2Config/TpmDetection.c
@@ -20,6 +20,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 
 #include "Tcg2ConfigNvData.h"
+#include "Tcg2Internal.h"
 
 /**
   This routine check both SetupVariable and real TPM device, and return final 
TpmDevice configuration.
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
index 866e47d23e..6417ab82a1 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
@@ -30,6 +30,7 @@
   Tcg2Config.vfr
   Tcg2ConfigStrings.uni
   Tcg2ConfigNvData.h
+  Tcg2Internal.h
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.h 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.h
index 4b0e18dfbb..af542d52ef 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.h
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.h
@@ -36,6 +36,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 
 #include "Tcg2ConfigNvData.h"
+#include "Tcg2Internal.h"
+
+#define TCG2_PROTOCOL_VERSION_DEFAULT0x0001
 
 //
 // Tool generated IFR binary data and String package data
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
index d0a124684f..a91c052850 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigNvData.h
@@ -13,15 +13,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 
-//
-// BUGBUG: In order to pass VfrCompiler, we have to redefine below MACRO, 
which already in .
-//
-#ifndef __TCG2_H__
-#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2   0x0001
-#define EFI_TCG2_EVENT_LOG_FORMAT_TCG_2 0x0002
-#endif
-#define EFI_TCG2_EVENT_LOG_FORMAT_ALL   
(EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2 | EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)
-
 #define TCG2_CONFIGURATION_VARSTORE_ID  0x0001
 #define TCG2_CONFIGURATION_INFO_VARSTORE_ID 0x0002
 #define TCG2_VERSION_VARSTORE_ID0x0003
@@ -55,9 +46,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define TPM_DEVICE_INTERFACE_MAX   TPM_DEVICE_INTERFACE_PTP_FIFO
 #define TPM_DEVICE_INTERFACE_DEFAULT   TPM_DEVICE_INTERFACE_PTP_CRB
 
-#define TCG2_PROTOCOL_VERSION_DEFAULT0x0001
-#define EFI_TCG2_EVENT_LOG_FORMAT_DEFAULTEFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2
-
 #define TCG2_PPI_VERSION_1_20x322E31  // "1.2"
 #define TCG2_PPI_VERSION_1_30x332E31  // "1.3"
 
@@ -97,27 +85,4 @@ typedef struct {
 #define TCG2_DEVICE_DETECTION_NAME  L"TCG2_DEVICE_DETECTION"
 #define TCG2_VERSION_NAME   L"TCG2_VERSION"
 
-#define TPM_INSTANCE_ID_LIST  { \
-  {TPM_DEVICE_INTERFACE_NONE,   TPM_DEVICE_NULL},  \
-  {TPM_DEVICE_INTERFACE_TPM12,  TPM_DEVICE_1_2},   \
-  {TPM_DEVICE_INTERFACE_TPM20_DTPM, TPM_DEVICE_2_0_DTPM},  \
-}
-
-//
-// BUGBUG: In order to pass VfrCompiler, we have to redefine GUID here.
-//
-#ifndef __BASE_H__
-typedef struct {
-  UINT32  Data1;
-  UINT16  Data2;
-  UINT16  Data3;
-  UINT8   Data4[8];
-} GUID;
-#endif
-
-typedef struct {
-  GUID   TpmInstanceGuid;
-  UINT8  TpmDevice;
-} TPM_INSTANCE_ID;
-
 #endif
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
index 837cbd12f0..f2aa3234ad 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -28,6 +28,7 @@

[edk2-devel] [edk2-platforms][PATCH V2 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

Re: [edk2-devel] [PATCH v2 0/2] Add error handling and initialize variables

[edk2-devel] [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: UninstallMultipleProtocols, checkpoint 7-9

Re: [edk2-devel] [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

Re: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable

Re: [edk2-devel] [PATCH v3] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

[edk2-devel] [PATCH v3] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] [edk2-rfc] [edk2-devel] UEFI Variable SMI Reduction

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] [PATCH v2] ArmVirtPkg: increase FD/FV size for NOOPT builds

Re: [edk2-devel] Copyright EDK II C Coding Standards Specification

Re: [edk2-devel] [Patch V2] UefiCpuPkg/CpuExceptionHandlerLib: Fix #AC split lock

[edk2-devel] [PATCH v1 1/1] uefi-sct/SctPkg AdapterInfo SetInformation pass on EFI_UNSUPPORTED

Re: [edk2-devel] [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

[edk2-devel] [PATCH] BaseTools:Replace PlatformInfo with PlatformAutoGen for Moudle

[edk2-devel] [PATCH] BaseTools:change some incorrect parameter defaults

[edk2-devel] [edk2-core] [PATCH v3 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec

Re: [edk2-devel] [edk2-core] [PATCH v2 1/1] MdeModulePkg: Make retval in UninstallMultipleProtocol follow Spec

[edk2-devel] Copyright EDK II C Coding Standards Specification

Re: [edk2-devel] [PATCH v2 2/2] MinPlatformPkg/AcpiTables: Add error handling to SortCpuLocalApicInTable

Re: [edk2-devel] [PATCH v2 1/2] MinPlatformPkg/AcpiTables: Initialize variables before used

[edk2-devel] [edk2-platforms][PATCH V1 1/1] AdvancedFeaturePkg/Ipmi: Fix GCC Build Failures

[edk2-devel] [Patch v2] SecurityPkg Tcg2Config: Move common definitions to new Tcg2Internal.h

28 matches

Site Navigation

Mail list logo

Footer information