date:20171004

Re: [edk2] [PATCH 2/5] MdePkg HobLib: Add BuildFv3Hob API

2017-10-04 Thread Zeng, Star

Thanks for the reminder.
I just sent patch series 
https://lists.01.org/pipermail/edk2-devel/2017-October/015614.html as 
supplement.

The FfsProcessFvFile() in EmbeddedPkg/Library/PrePiLib/FwVol.c may need to 
build FV3 HOB with authentication status to be propagated to DXE.
(FfsProcessFvFile() -> FfsFindSectionData() -> FfsProcessSection() -> 
ExtractGuidedSectionDecode()), FfsFindSectionData() and FfsProcessSection() may 
need to be updated to return authentication status to FfsProcessFvFile().

But it is not really needed at all after checking the platforms as they are all 
using MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf 
that has no concept of authentication status at all and always returns 0 
authentication status 
(https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/LzmaCustomDecompressLib/GuidedSectionExtraction.c
 L154 and L172), so the EmbeddedPkg/Library/PrePiLib/FwVol.c can be kept with 
no change. :)


Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, October 4, 2017 11:39 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Gao, Liming ; Ard Biesheuvel 

Subject: Re: [edk2] [PATCH 2/5] MdePkg HobLib: Add BuildFv3Hob API

Hi Star,

On 10/04/17 16:21, Star Zeng wrote:
> Add BuildFv3Hob API in HobLib.h and implement the API in HobLib 
> instances PeiHobLib, DxeHobLib and DxeCoreHobLib.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Include/Library/HobLib.h   | 34 --
>  MdePkg/Library/DxeCoreHobLib/HobLib.c | 35 ++-
>  MdePkg/Library/DxeHobLib/HobLib.c | 32 +
>  MdePkg/Library/PeiHobLib/HobLib.c | 54 
> ++-
>  4 files changed, 151 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/HobLib.h 
> b/MdePkg/Include/Library/HobLib.h index fc48703826c5..6f1f7b3f5f20 
> 100644
> --- a/MdePkg/Include/Library/HobLib.h
> +++ b/MdePkg/Include/Library/HobLib.h
> @@ -6,9 +6,9 @@
>defined in the PI Specification.
>A HOB is a Hand-Off Block, defined in the Framework architecture, that
>allows the PEI phase to pass information to the DXE phase. HOBs are 
> position
> -  independent and can be relocated easily to different memory memory 
> locations.
> +  independent and can be relocated easily to different memory locations.
>  
> -Copyright (c) 2006 - 2016, Intel Corporation. All rights 
> reserved.
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights 
> +reserved.
>  This program and the accompanying materials  are licensed and made 
> available under the terms and conditions of the BSD License  which 
> accompanies this distribution.  The full text of the license may be 
> found at @@ -343,6 +343,36 @@ BuildFv2Hob (
>);
>  
>  /**
> +  Builds a EFI_HOB_TYPE_FV3 HOB.
> +
> +  This function builds a EFI_HOB_TYPE_FV3 HOB.
> +  It can only be invoked during PEI phase;  for DXE phase, it will 
> + ASSERT() since PEI HOB is read-only for DXE phase.
> +
> +  If there is no additional space for HOB creation, then ASSERT().
> +  If the FvImage buffer is not at its required alignment, then ASSERT().
> +
> +  @param BaseAddressThe base address of the Firmware Volume.
> +  @param Length The size of the Firmware Volume in bytes.
> +  @param AuthenticationStatus   The authentication status.
> +  @param ExtractedFvTRUE if the FV was extracted as a file 
> within another firmware volume.
> +FALSE otherwise.
> +  @param FvName The name of the Firmware Volume. Valid only 
> if IsExtractedFv is TRUE
> +  @param FileName   The name of the file. Valid only if 
> IsExtractedFv is TRUE
> +
> +**/
> +VOID
> +EFIAPI
> +BuildFv3Hob (
> +  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
> +  IN  UINT64  Length,
> +  IN  UINT32  AuthenticationStatus,
> +  IN  BOOLEAN ExtractedFv,
> +  IN CONSTEFI_GUID*FvName, OPTIONAL
> +  IN CONSTEFI_GUID*FileName OPTIONAL
> +  );
> +
> +/**
>Builds a Capsule Volume HOB.
>  
>This function builds a Capsule Volume HOB.

do we need to copy the implementation(s) of this function to other instances of 
HobLib?

edk2 has the following HobLib instances:

1 ArmVirtPkg/Library/ArmVirtDxeHobLib/ArmVirtDxeHobLib.inf
2 EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
3 IntelFrameworkPkg/Library/PeiHobLibFramework/PeiHobLibFramework.inf
4 MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
5 MdePkg/Library/DxeHobLib/DxeHobLib.inf
6 MdePkg/Library/PeiHobLib/PeiHobLib.inf

Instances #4 through #6 are extended in this patch; instance #3 is extended in 
the next patch.

Instances #1 and #2 are not changed. Is that OK? (For example, some of the 
ArmVirt platforms use instance

[edk2] [PATCH] Ifconfig : Fixed False information about Media State.

2017-10-04 Thread Meenakshi Aggarwal

Issue : We were setting MediaPresent as TRUE (default), so
even if NetLibDetectMedia() did not set MediaPresent Flag as TRUE,
ifconfig always display Media State as 'Media Present'

Fix : Set MediaPresent as FALSE before calling NetLibDetectMedia()

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Meenakshi Aggarwal 
---
 ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
index 4db07b2..7c05b68 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ifconfig.c
@@ -554,8 +554,6 @@ IfConfigShowInterfaceInfo (
   EFI_IPv4_ADDRESS  Gateway;
   UINT32Index;
   
-  MediaPresent = TRUE;
-
   if (IsListEmpty (IfList)) {
 ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG_INVALID_INTERFACE), gShellNetwork1HiiHandle);
   }
@@ -576,6 +574,8 @@ IfConfigShowInterfaceInfo (
 //
 // Get Media State.
 //
+MediaPresent = FALSE;
+
 NetLibDetectMedia (IfCb->NicHandle, &MediaPresent);
 if (!MediaPresent) {
   ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN 
(STR_IFCONFIG_INFO_MEDIA_STATE), gShellNetwork1HiiHandle, L"Media 
disconnected");
-- 
2.7.4

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

[edk2] [PATCH 1/2] EmbeddedPkg PrePiHobLib: Implement BuildFv3Hob

2017-10-04 Thread Star Zeng

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 EmbeddedPkg/Library/PrePiHobLib/Hob.c | 43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c 
b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
index aff532259ef4..a681587d3ee0 100644
--- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
+++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2010, Apple Inc. All rights reserved.
+  Copyright (c) 2010 - 2017, Apple Inc. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -601,7 +601,48 @@ BuildFv2Hob (
   CopyGuid (&Hob->FileName, FileName);
 }
 
+/**
+  Builds a EFI_HOB_TYPE_FV3 HOB.
+
+  This function builds a EFI_HOB_TYPE_FV3 HOB.
+  It can only be invoked during PEI phase;
+  for DXE phase, it will ASSERT() since PEI HOB is read-only for DXE phase.
+
+  If there is no additional space for HOB creation, then ASSERT().
+
+  @param BaseAddressThe base address of the Firmware Volume.
+  @param Length The size of the Firmware Volume in bytes.
+  @param AuthenticationStatus   The authentication status.
+  @param ExtractedFvTRUE if the FV was extracted as a file within 
another firmware volume.
+FALSE otherwise.
+  @param FvName The name of the Firmware Volume. Valid only if 
IsExtractedFv is TRUE
+  @param FileName   The name of the file. Valid only if 
IsExtractedFv is TRUE
+
+**/
+VOID
+EFIAPI
+BuildFv3Hob (
+  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
+  IN  UINT64  Length,
+  IN  UINT32  AuthenticationStatus,
+  IN  BOOLEAN ExtractedFv,
+  IN CONSTEFI_GUID*FvName, OPTIONAL
+  IN CONSTEFI_GUID*FileName OPTIONAL
+  )
+{
+  EFI_HOB_FIRMWARE_VOLUME3  *Hob;
+
+  Hob = CreateHob (EFI_HOB_TYPE_FV3, (UINT16) sizeof 
(EFI_HOB_FIRMWARE_VOLUME3));
 
+  Hob->BaseAddress  = BaseAddress;
+  Hob->Length   = Length;
+  Hob->AuthenticationStatus = AuthenticationStatus;
+  Hob->ExtractedFv  = ExtractedFv;
+  if (ExtractedFv) {
+CopyGuid (&Hob->FvName, FvName);
+CopyGuid (&Hob->FileName, FileName);
+  }
+}
 
 /**
   Builds a Capsule Volume HOB.
-- 
2.13.3.windows.1

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

[edk2] [PATCH 0/2] Implement BuildFv3Hob for PrePiHobLib and ArmVirtDxeHobLib

2017-10-04 Thread Star Zeng

This patch series is supplement of
https://lists.01.org/pipermail/edk2-devel/2017-October/015589.html.

Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Liming Gao 

Star Zeng (2):
  EmbeddedPkg PrePiHobLib: Implement BuildFv3Hob
  ArmVirtPkg ArmVirtDxeHobLib: Implement BuildFv3Hob

 ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c | 34 +-
 EmbeddedPkg/Library/PrePiHobLib/Hob.c| 43 +++-
 2 files changed, 75 insertions(+), 2 deletions(-)

--
2.13.3.windows.1

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

[edk2] [PATCH 2/2] ArmVirtPkg ArmVirtDxeHobLib: Implement BuildFv3Hob

2017-10-04 Thread Star Zeng

Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c | 34 +++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c 
b/ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c
index 9e617b8e6991..dc84dd32d24a 100644
--- a/ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c
+++ b/ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c
@@ -1,7 +1,7 @@
 /** @file
   HOB Library implemenation for Dxe Phase with DebugLib dependency removed
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 Copyright (c) 2014, Linaro Ltd. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -442,6 +442,38 @@ BuildFv2Hob (
   ASSERT (FALSE);
 }
 
+/**
+  Builds a EFI_HOB_TYPE_FV3 HOB.
+
+  This function builds a EFI_HOB_TYPE_FV3 HOB.
+  It can only be invoked during PEI phase;
+  for DXE phase, it will ASSERT() since PEI HOB is read-only for DXE phase.
+
+  If there is no additional space for HOB creation, then ASSERT().
+  If the FvImage buffer is not at its required alignment, then ASSERT().
+
+  @param BaseAddressThe base address of the Firmware Volume.
+  @param Length The size of the Firmware Volume in bytes.
+  @param AuthenticationStatus   The authentication status.
+  @param ExtractedFvTRUE if the FV was extracted as a file within 
another firmware volume.
+FALSE otherwise.
+  @param FvName The name of the Firmware Volume. Valid only if 
IsExtractedFv is TRUE
+  @param FileName   The name of the file. Valid only if 
IsExtractedFv is TRUE
+
+**/
+VOID
+EFIAPI
+BuildFv3Hob (
+  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
+  IN  UINT64  Length,
+  IN  UINT32  AuthenticationStatus,
+  IN  BOOLEAN ExtractedFv,
+  IN CONSTEFI_GUID*FvName, OPTIONAL
+  IN CONSTEFI_GUID*FileName OPTIONAL
+  )
+{
+  ASSERT (FALSE);
+}
 
 /**
   Builds a Capsule Volume HOB.
-- 
2.13.3.windows.1

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

Re: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems.

2017-10-04 Thread Yao, Jiewen

interesting idea！
I think it is an option.
I would prefer an all-or-none approach.

If we use vender-pcd, we remove all check.
Vise versa, if we use check, we use check in all places.

Mixing runtime check and pcd may bring unnecessary confusing.

thank you!
Yao, Jiewen


> 在 2017年10月5日，上午9:19，Marvin H?user  写道：
> 
> Hey Jiewen,
> Hey Leo,
> 
> May I suggest replacing "StandardSignatureIsAuthenticAMD()" and the PCDs 
> introduced by the series with a Fixed "PcdCpuVendor" enum or alike?
> The contra of "StandardSignatureIsAuthenticAMD()" is that it's a runtime 
> action. From my point of view, this has no notable advantage as boards use 
> either an Intel or an AMD chipset and hence the EDK2 Firmware Package has 
> compilation-time knowledge of the target platform.
> On the other hand, the PCDs introduced by this patch cause the contra that 
> the platform DSC must set the correct vendor-specific (intel, AMD) value.
> If the code checked for the CPU Vendor PCD and used the correct values based 
> on that, the values for the other vendors would get optimized away (no 
> unnecessary runtime actions) and the platform package owner does not need to 
> worry about setting PCDs to AMD-specific values except for the CPU Vendor PCD.
> Backwards-compatibility could be ensured by defaulting to some reserved value 
> for the PCD and handling detection via StandardSignatureIsAuthenticAMD() if 
> the platform DSC does not change it.
> 
> Thanks,
> Marvin.
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Yao, Jiewen
>> Sent: Thursday, October 5, 2017 2:49 AM
>> To: Leo Duran ; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based
>> x86 systems.
>> 
>> Hi Leo
>> I do not suggest we introduce PcdCpuSmmSmramSaveStateMapOffset.
>> 
>> This is unnecessary, because it is CPU attribute but not some end user
>> configurable data.
>> 
>> I think we can use CPUID to distinguish AMD from INTEL. Is that technically
>> possible?
>> 
>> I found we already have code at
>> 
>> C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicX2ApicLib\BaseXApic
>> X2ApicLib.c(1206):if (StandardSignatureIsAuthenticAMD()) {
>> 
>> C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicLib\BaseXApicLib.c(11
>> 11):if (StandardSignatureIsAuthenticAMD()) {
>> 
>> 
>> 
>> Thank you
>> Yao Jiewen
>> 
>> 
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>> Leo Duran
>>> Sent: Thursday, October 5, 2017 3:02 AM
>>> To: edk2-devel@lists.01.org
>>> Subject: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86
>>> systems.
>>> 
>>> This patch-set introduces a couple of FixedPCDs to replace
>>> Intel-specific macros, and better support AMD-based x86 systems.
>>> 
>>> 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map
>> Offset.
>>> 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.
>>> 
>>> Changes since v3:
>>> Correction on cover letter.
>>> 
>>> Changes since v2:
>>> The intent of this revision is to maintain compatibility with existing
>>> packages. To that end, changes to OvmgfPkg and QuarkSocPkg are
>> reverted.
>>> Moreover, pertinent macros are replaced in the C code, rather than on
>>> header files that are shared globally.
>>> 
>>> Changes since v1:
>>> Revision to Cc list for UefiCpuPkg.
>>> 
>>> Leo Duran (5):
>>>  UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support
>>>  UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM
>> support
>>>  UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM
>> support
>>>  UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM
>>> support
>>>  UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library
>>> 
>>> UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  4
>>> +++-
>>> UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  5
>>> +
>>> UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  5
>>> +
>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c|  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S |  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm   |  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm  |  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c|
>>> 10 +-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h|
>>> 2 --
>>> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |
>>> 4 
>>> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
>>> |  4 +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c|  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c |
>>> 4 +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S  |  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm|  4
>>> +++-
>>> UefiCpuPkg/PiSmmCpuDx

Re: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems.

2017-10-04 Thread Marvin H?user

Hey Jiewen,
Hey Leo,

May I suggest replacing "StandardSignatureIsAuthenticAMD()" and the PCDs 
introduced by the series with a Fixed "PcdCpuVendor" enum or alike?
The contra of "StandardSignatureIsAuthenticAMD()" is that it's a runtime 
action. From my point of view, this has no notable advantage as boards use 
either an Intel or an AMD chipset and hence the EDK2 Firmware Package has 
compilation-time knowledge of the target platform.
On the other hand, the PCDs introduced by this patch cause the contra that the 
platform DSC must set the correct vendor-specific (intel, AMD) value.
If the code checked for the CPU Vendor PCD and used the correct values based on 
that, the values for the other vendors would get optimized away (no unnecessary 
runtime actions) and the platform package owner does not need to worry about 
setting PCDs to AMD-specific values except for the CPU Vendor PCD.
Backwards-compatibility could be ensured by defaulting to some reserved value 
for the PCD and handling detection via StandardSignatureIsAuthenticAMD() if the 
platform DSC does not change it.

Thanks,
Marvin.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yao, Jiewen
> Sent: Thursday, October 5, 2017 2:49 AM
> To: Leo Duran ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based
> x86 systems.
> 
> Hi Leo
> I do not suggest we introduce PcdCpuSmmSmramSaveStateMapOffset.
> 
> This is unnecessary, because it is CPU attribute but not some end user
> configurable data.
> 
> I think we can use CPUID to distinguish AMD from INTEL. Is that technically
> possible?
> 
> I found we already have code at
> 
> C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicX2ApicLib\BaseXApic
> X2ApicLib.c(1206):if (StandardSignatureIsAuthenticAMD()) {
> 
> C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicLib\BaseXApicLib.c(11
> 11):if (StandardSignatureIsAuthenticAMD()) {
> 
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Leo Duran
> > Sent: Thursday, October 5, 2017 3:02 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86
> > systems.
> >
> > This patch-set introduces a couple of FixedPCDs to replace
> > Intel-specific macros, and better support AMD-based x86 systems.
> >
> > 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map
> Offset.
> > 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.
> >
> > Changes since v3:
> > Correction on cover letter.
> >
> > Changes since v2:
> > The intent of this revision is to maintain compatibility with existing
> > packages. To that end, changes to OvmgfPkg and QuarkSocPkg are
> reverted.
> > Moreover, pertinent macros are replaced in the C code, rather than on
> > header files that are shared globally.
> >
> > Changes since v1:
> > Revision to Cc list for UefiCpuPkg.
> >
> > Leo Duran (5):
> >   UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support
> >   UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM
> support
> >   UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM
> support
> >   UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM
> > support
> >   UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library
> >
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  4
> > +++-
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  5
> > +
> >  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  5
> > +
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c|  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S |  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm   |  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm  |  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c|
> > 10 +-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h|
> > 2 --
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |
> > 4 
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> > |  4 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c|  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c |
> > 4 +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S  |  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm|  4
> > +++-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm   |  4
> > +++-
> >  UefiCpuPkg/UefiCpuPkg.dec |  9
> > +
> >  17 files changed, 61 insertions(+), 18 deletions(-)
> >
> > --
> > 2.7.4
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-deve

Re: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems.

2017-10-04 Thread Yao, Jiewen

Hi Leo
I do not suggest we introduce PcdCpuSmmSmramSaveStateMapOffset.

This is unnecessary, because it is CPU attribute but not some end user 
configurable data.

I think we can use CPUID to distinguish AMD from INTEL. Is that technically 
possible?

I found we already have code at 
  
C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicX2ApicLib\BaseXApicX2ApicLib.c(1206):
if (StandardSignatureIsAuthenticAMD()) {
  C:\home\EdkIIGit\edk2\UefiCpuPkg\Library\BaseXApicLib\BaseXApicLib.c():   
 if (StandardSignatureIsAuthenticAMD()) {



Thank you
Yao Jiewen


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Leo
> Duran
> Sent: Thursday, October 5, 2017 3:02 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86
> systems.
> 
> This patch-set introduces a couple of FixedPCDs to replace
> Intel-specific macros, and better support AMD-based x86 systems.
> 
> 1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
> 2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.
> 
> Changes since v3:
> Correction on cover letter.
> 
> Changes since v2:
> The intent of this revision is to maintain compatibility with existing
> packages. To that end, changes to OvmgfPkg and QuarkSocPkg are reverted.
> Moreover, pertinent macros are replaced in the C code, rather than on
> header files that are shared globally.
> 
> Changes since v1:
> Revision to Cc list for UefiCpuPkg.
> 
> Leo Duran (5):
>   UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support
>   UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM
> support
>   UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM support
>   UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM
> support
>   UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library
> 
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  4
> +++-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  5
> +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  5
> +
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c|  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S |  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm   |  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm  |  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c|
> 10 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h|
> 2 --
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |
> 4 
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> |  4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c|  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c |
> 4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S  |  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm|  4
> +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm   |  4
> +++-
>  UefiCpuPkg/UefiCpuPkg.dec |  9
> +
>  17 files changed, 61 insertions(+), 18 deletions(-)
> 
> --
> 2.7.4
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v4 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM support

2017-10-04 Thread Leo Duran

Consume a FixedPCD to replace Intel-specific macro.
The new PCD will allow SMM support on AMD-based x86 systems.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf| 5 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 5 +
 2 files changed, 10 insertions(+)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 77908b0..1be2671 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -2,6 +2,8 @@
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -36,5 +38,8 @@
   MemoryAllocationLib
   DebugLib
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset   ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index db8dcdc..a91e9ed 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -3,6 +3,8 @@
 #  is included.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -78,6 +80,9 @@
   gEfiAcpi20TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpi10TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset   ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize ## 
SOMETIMES_CONSUMES
-- 
2.7.4

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

[edk2] [PATCH v4 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library

2017-10-04 Thread Leo Duran

Consume a FixedPCD to replace Intel-specific macro.
The new PCD will allow SMM support on AMD-based x86 systems.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 2d2bc6d..88f43b4 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -2,6 +2,8 @@
 The CPU specific programming for PiSmmCpuDxeSmm module.
 
 Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2017, AMD Incorporated. All rights reserved.
+
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -224,7 +226,7 @@ SmmCpuFeaturesInitializeProcessor (
   //
   // Configure SMBASE.
   //
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
FixedPcdGet16 (PcdCpuSmmSmramSaveStateMapOffset));
   CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
 
   //
-- 
2.7.4

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

[edk2] [PATCH v4 3/5] UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM support

2017-10-04 Thread Leo Duran

Consume a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm|  4 +++-
 12 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
index 02a866b..cc2624e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
@@ -3,6 +3,8 @@ Semaphore mechanism to indicate to the BSP that an AP has 
exited SMM
 after SMBASE relocation.
 
 Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2017, AMD Incorporated. All rights reserved.
+
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -38,7 +40,7 @@ SemaphoreHook (
 
   mRebasedFlag = RebasedFlag;
 
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
FixedPcdGet16 (PcdCpuSmmSmramSaveStateMapOffset));
   mSmmRelocationOriginalAddress = (UINTN)HookReturnFromSmm (
CpuIndex,
CpuState,
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
index 3243a91..25af6e7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
@@ -1,6 +1,8 @@
 #--
 #
 # Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
@@ -35,7 +37,7 @@ ASM_GLOBAL  ASM_PFX(gSmiHandlerIdtr)
 #
 # Constants relating to PROCESSOR_SMM_DESCRIPTOR
 #
-.equDSC_OFFSET, 0xfb00
+.equDSC_OFFSET, (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 .equDSC_GDTPTR, 0x30
 .equDSC_GDTSIZ, 0x38
 .equDSC_CS, 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
index 8296f36..f526778 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
@@ -1,5 +1,7 @@
 
;-- 
;
 ; Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. All rights reserved.
+;
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -29,7 +31,7 @@ MSR_EFER_XD   EQU 0800h
 ;
 ; Constants relating to PROCESSOR_SMM_DESCRIPTOR
 ;
-DSC_OFFSETEQU 0fb00h
+DSC_OFFSETEQU (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 DSC_GDTPTREQU 30h
 DSC_GDTSIZEQU 38h
 DSC_CSEQU 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 4d2383f..9092dcc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -1,5 +1,7 @@
 
;-- 
;
 ; Copyright (c) 2016, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. All rights reserved.
+;
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the licen

[edk2] [PATCH v4 2/5] UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM support

2017-10-04 Thread Leo Duran

Consume a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 
 1 file changed, 4 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 099792e..bf237f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -147,6 +147,10 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CONSUMES
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset   ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileSize   ## 
SOMETIMES_CONSUMES
-- 
2.7.4

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

[edk2] [PATCH v4 0/5] Enhanced SMM support for AMD-based x86 systems.

2017-10-04 Thread Leo Duran

This patch-set introduces a couple of FixedPCDs to replace
Intel-specific macros, and better support AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Changes since v3:
Correction on cover letter.

Changes since v2:
The intent of this revision is to maintain compatibility with existing
packages. To that end, changes to OvmgfPkg and QuarkSocPkg are reverted.
Moreover, pertinent macros are replaced in the C code, rather than on
header files that are shared globally.

Changes since v1:
Revision to Cc list for UefiCpuPkg.

Leo Duran (5):
  UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support
  UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM support
  UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM support
  UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM support
  UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library

 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  5 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  5 +
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm   |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c| 10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h|  2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |  4 
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm   |  4 +++-
 UefiCpuPkg/UefiCpuPkg.dec |  9 +
 17 files changed, 61 insertions(+), 18 deletions(-)

-- 
2.7.4

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

[edk2] [PATCH v4 1/5] UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support

2017-10-04 Thread Leo Duran

Introduce a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/UefiCpuPkg.dec | 9 +
 1 file changed, 9 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 3bd8740..c92c56e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -2,6 +2,7 @@
 # This Package provides UEFI compatible CPU modules and libraries.
 #
 # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017, AMD Incorporated. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License which accompanies this 
distribution.
@@ -204,6 +205,14 @@
   # @Prompt If CPU features will be initialized during S3 resume.
   
gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume|FALSE|BOOLEAN|0x001D
 
+  ## Specifies the Offset of SMRAM Save State Map from SMBASE.
+  # @Prompt SMRAM Save State Map Offset.
+  
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset|0xFC00|UINT16|0x32132113
+
+  ## Specifies the PROCESSOR SMM DESCRIPTOR Offset in SMRAM.
+  # @Prompt SMRAM PSD Offset.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset|0xFB00|UINT16|0x32132114
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Specifies max supported number of Logical Processors.
   # @Prompt Configure max supported number of Logical Processors
-- 
2.7.4

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

[edk2] [PATCH v3 0/5] Enhanced SMM support for AMD-based x86 systems.

2017-10-04 Thread Leo Duran

This patch-set introduces a couple of FixedPCDs to replace
Intel-specific macros, and better support AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

OvmfPkg and QuarkSocPkg:

Changes since v2:
The intent of this revision is to maintain compatibility with existing
packages. To that end, changes to OvmgfPkg and QuarkSocPkg are reverted.
Moreover, pertinent macros are replaced in the C code, rather than on
header files that are shared globally.

Leo Duran (5):
  UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support
  UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM support
  UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM support
  UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM support
  UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library

 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c  |  4 +++-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf|  5 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  5 +
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm   |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c| 10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h|  2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |  4 
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm   |  4 +++-
 UefiCpuPkg/UefiCpuPkg.dec |  9 +
 17 files changed, 61 insertions(+), 18 deletions(-)

-- 
2.7.4

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

[edk2] [PATCH v3 1/5] UefiCpuPkg/UefiCpuPkg.dec: Create FixedPCDs for SMM support

2017-10-04 Thread Leo Duran

Introduce a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/UefiCpuPkg.dec | 9 +
 1 file changed, 9 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 3bd8740..c92c56e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -2,6 +2,7 @@
 # This Package provides UEFI compatible CPU modules and libraries.
 #
 # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017, AMD Incorporated. All rights reserved.
 #
 # This program and the accompanying materials are licensed and made available 
under
 # the terms and conditions of the BSD License which accompanies this 
distribution.
@@ -204,6 +205,14 @@
   # @Prompt If CPU features will be initialized during S3 resume.
   
gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesInitOnS3Resume|FALSE|BOOLEAN|0x001D
 
+  ## Specifies the Offset of SMRAM Save State Map from SMBASE.
+  # @Prompt SMRAM Save State Map Offset.
+  
gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset|0xFC00|UINT16|0x32132113
+
+  ## Specifies the PROCESSOR SMM DESCRIPTOR Offset in SMRAM.
+  # @Prompt SMRAM PSD Offset.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset|0xFB00|UINT16|0x32132114
+
 [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## Specifies max supported number of Logical Processors.
   # @Prompt Configure max supported number of Logical Processors
-- 
2.7.4

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

[edk2] [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Consume FixedPCD to enhance SMM support

2017-10-04 Thread Leo Duran

Consume a FixedPCD to replace Intel-specific macro.
The new PCD will allow SMM support on AMD-based x86 systems.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf| 5 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf | 5 +
 2 files changed, 10 insertions(+)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 77908b0..1be2671 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -2,6 +2,8 @@
 #  The CPU specific programming for PiSmmCpuDxeSmm module.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -36,5 +38,8 @@
   MemoryAllocationLib
   DebugLib
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset   ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index db8dcdc..a91e9ed 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -3,6 +3,8 @@
 #  is included.
 #
 #  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -78,6 +80,9 @@
   gEfiAcpi20TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpi10TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset   ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMsegSize ## 
SOMETIMES_CONSUMES
-- 
2.7.4

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

[edk2] [PATCH v3 2/5] UefiCpuPkg/PiSmmCpuDxeSmm: Consume FixedPCDs to enhance SMM support

2017-10-04 Thread Leo Duran

Consume a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 4 
 1 file changed, 4 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 099792e..bf237f7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -147,6 +147,10 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileRingBuffer ## CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmFeatureControlMsrLock ## CONSUMES
 
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSmramSaveStateMapOffset   ## CONSUMES
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmPSDOffset ## CONSUMES
+
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
SOMETIMES_CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileSize   ## 
SOMETIMES_CONSUMES
-- 
2.7.4

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

[edk2] [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Use FixedPCD on non-STM library

2017-10-04 Thread Leo Duran

Consume a FixedPCD to replace Intel-specific macro.
The new PCD will allow SMM support on AMD-based x86 systems.

PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c 
b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 2d2bc6d..88f43b4 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -2,6 +2,8 @@
 The CPU specific programming for PiSmmCpuDxeSmm module.
 
 Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2017, AMD Incorporated. All rights reserved.
+
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -224,7 +226,7 @@ SmmCpuFeaturesInitializeProcessor (
   //
   // Configure SMBASE.
   //
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
FixedPcdGet16 (PcdCpuSmmSmramSaveStateMapOffset));
   CpuState->x86.SMBASE = (UINT32)CpuHotPlugData->SmBase[CpuIndex];
 
   //
-- 
2.7.4

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

[edk2] [PATCH v3 3/5] UefiCpuPkg/PiSmmCpuDxeSmm: Use FixedPCDs to enhance SMM support

2017-10-04 Thread Leo Duran

Consume a couple of FixedPCDs to replace Intel-specific macros.
The new PCDs will allow SMM support on AMD-based x86 systems.

1) PcdCpuSmmSmramSaveStateMapOffset - SMRAM Save State Map Offset.
2) PcdCpuSmmPSDOffset - Processor SMM Descriptor Offset in SMRAM.

Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Jordan Justen 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leo Duran 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 10 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  2 --
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/Semaphore.c  |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm |  4 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm|  4 +++-
 12 files changed, 35 insertions(+), 17 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
index 02a866b..cc2624e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/Semaphore.c
@@ -3,6 +3,8 @@ Semaphore mechanism to indicate to the BSP that an AP has 
exited SMM
 after SMBASE relocation.
 
 Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2017, AMD Incorporated. All rights reserved.
+
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -38,7 +40,7 @@ SemaphoreHook (
 
   mRebasedFlag = RebasedFlag;
 
-  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
SMRAM_SAVE_STATE_MAP_OFFSET);
+  CpuState = (SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + 
FixedPcdGet16 (PcdCpuSmmSmramSaveStateMapOffset));
   mSmmRelocationOriginalAddress = (UINTN)HookReturnFromSmm (
CpuIndex,
CpuState,
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
index 3243a91..25af6e7 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S
@@ -1,6 +1,8 @@
 #--
 #
 # Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
@@ -35,7 +37,7 @@ ASM_GLOBAL  ASM_PFX(gSmiHandlerIdtr)
 #
 # Constants relating to PROCESSOR_SMM_DESCRIPTOR
 #
-.equDSC_OFFSET, 0xfb00
+.equDSC_OFFSET, (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 .equDSC_GDTPTR, 0x30
 .equDSC_GDTSIZ, 0x38
 .equDSC_CS, 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
index 8296f36..f526778 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm
@@ -1,5 +1,7 @@
 
;-- 
;
 ; Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. All rights reserved.
+;
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the license may be 
found at
@@ -29,7 +31,7 @@ MSR_EFER_XD   EQU 0800h
 ;
 ; Constants relating to PROCESSOR_SMM_DESCRIPTOR
 ;
-DSC_OFFSETEQU 0fb00h
+DSC_OFFSETEQU (FixedPcdGet16 (PcdCpuSmmPSDOffset))
 DSC_GDTPTREQU 30h
 DSC_GDTSIZEQU 38h
 DSC_CSEQU 14
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
index 4d2383f..9092dcc 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm
@@ -1,5 +1,7 @@
 
;-- 
;
 ; Copyright (c) 2016, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. All rights reserved.
+;
 ; This program and the accompanying materials
 ; are licensed and made available under the terms and conditions of the BSD 
License
 ; which accompanies this distribution.  The full text of the licen

Re: [edk2] [PATCH 2/5] MdePkg HobLib: Add BuildFv3Hob API

2017-10-04 Thread Laszlo Ersek

Hi Star,

On 10/04/17 16:21, Star Zeng wrote:
> Add BuildFv3Hob API in HobLib.h and implement the API
> in HobLib instances PeiHobLib, DxeHobLib and DxeCoreHobLib.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdePkg/Include/Library/HobLib.h   | 34 --
>  MdePkg/Library/DxeCoreHobLib/HobLib.c | 35 ++-
>  MdePkg/Library/DxeHobLib/HobLib.c | 32 +
>  MdePkg/Library/PeiHobLib/HobLib.c | 54 
> ++-
>  4 files changed, 151 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Include/Library/HobLib.h b/MdePkg/Include/Library/HobLib.h
> index fc48703826c5..6f1f7b3f5f20 100644
> --- a/MdePkg/Include/Library/HobLib.h
> +++ b/MdePkg/Include/Library/HobLib.h
> @@ -6,9 +6,9 @@
>defined in the PI Specification.
>A HOB is a Hand-Off Block, defined in the Framework architecture, that
>allows the PEI phase to pass information to the DXE phase. HOBs are 
> position
> -  independent and can be relocated easily to different memory memory 
> locations.
> +  independent and can be relocated easily to different memory locations.
>  
> -Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD 
> License
>  which accompanies this distribution.  The full text of the license may be 
> found at
> @@ -343,6 +343,36 @@ BuildFv2Hob (
>);
>  
>  /**
> +  Builds a EFI_HOB_TYPE_FV3 HOB.
> +
> +  This function builds a EFI_HOB_TYPE_FV3 HOB.
> +  It can only be invoked during PEI phase;
> +  for DXE phase, it will ASSERT() since PEI HOB is read-only for DXE phase.
> +
> +  If there is no additional space for HOB creation, then ASSERT().
> +  If the FvImage buffer is not at its required alignment, then ASSERT().
> +
> +  @param BaseAddressThe base address of the Firmware Volume.
> +  @param Length The size of the Firmware Volume in bytes.
> +  @param AuthenticationStatus   The authentication status.
> +  @param ExtractedFvTRUE if the FV was extracted as a file 
> within another firmware volume.
> +FALSE otherwise.
> +  @param FvName The name of the Firmware Volume. Valid only 
> if IsExtractedFv is TRUE
> +  @param FileName   The name of the file. Valid only if 
> IsExtractedFv is TRUE
> +
> +**/
> +VOID
> +EFIAPI
> +BuildFv3Hob (
> +  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
> +  IN  UINT64  Length,
> +  IN  UINT32  AuthenticationStatus,
> +  IN  BOOLEAN ExtractedFv,
> +  IN CONSTEFI_GUID*FvName, OPTIONAL
> +  IN CONSTEFI_GUID*FileName OPTIONAL
> +  );
> +
> +/**
>Builds a Capsule Volume HOB.
>  
>This function builds a Capsule Volume HOB.

do we need to copy the implementation(s) of this function to other
instances of HobLib?

edk2 has the following HobLib instances:

1 ArmVirtPkg/Library/ArmVirtDxeHobLib/ArmVirtDxeHobLib.inf
2 EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
3 IntelFrameworkPkg/Library/PeiHobLibFramework/PeiHobLibFramework.inf
4 MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
5 MdePkg/Library/DxeHobLib/DxeHobLib.inf
6 MdePkg/Library/PeiHobLib/PeiHobLib.inf

Instances #4 through #6 are extended in this patch; instance #3 is
extended in the next patch.

Instances #1 and #2 are not changed. Is that OK? (For example, some of
the ArmVirt platforms use instance #1 for all DXE and "later" modules.
Some other ArmVirt platforms use instance #2.)

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

2017-10-04 Thread Laszlo Ersek

Star,

On 10/04/17 16:09, Zeng, Star wrote:
> Thanks for confirming the urgency.
> 
> I have no strong motivation to keep/remove the ASSERT, I would like Ruiyu to 
> argue and make the decision.
> I mainly want the issue (the code ends up calling this 
> function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext") could 
> be root caused.

it might be interesting to find out about the exact call stack. However,
I'd like to point out that the exact purpose of the
EfiBootManagerIsValidLoadOptionVariableName() function is to check
*whether* the variable name is a valid boot option name or not. If not
-- for whatever reason -- then it shouldn't ASSERT(); it should just
return FALSE.

Perhaps it's relevant: the function was made public in commit 3dc5c1ae5c757.

Thanks
Laszlo

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
> Sent: Wednesday, October 4, 2017 9:54 PM
> To: Zeng, Star 
> Cc: Yao, Jiewen ; Ni, Ruiyu ; 
> edk2-devel@lists.01.org; Dong, Eric ; 
> leif.lindh...@linaro.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 
> 'BootNext' varname
> 
> On 4 October 2017 at 14:49, Zeng, Star  wrote:
>> Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it 
>> will be rejected by 
>> MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that will check the 
>> VariableName against UEFI spec “Table 13. Global Variables”
>> if the VendorGuid is gEfiGlobalVariableGuid.
>>
>>
>>
>> I would suspect there is a bug at other place if the code ends up 
>> calling this function(EfiBootManagerIsValidLoadOptionVariableName) on 
>> L"BootNext".
>>
> 
> That still does not mean you should ASSERT() here. The state of the variable 
> store != the internals of the code, and so it should be considered external 
> input to some extent. ASSERTs are meant to catch programming errors, not 
> errors in the varstore image.
> 
> 
>>
>> Ard,
>>
>> Is the fix urgent or not for you?
>>
> 
> Not really. But fwupdate is shipping as part of many distros, so I guess 
> others may run into it as well.
> 
>> I may want to wait for Ruiyu’s back to take some look at the detail of it.
>>
> 
> That is fine.
> 
>> At the same time, you may help check the code flow in some detail if 
>> you have free time, I think that will be helpful. J
>>
> 
> OK.
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

2017-10-04 Thread Ard Biesheuvel

On 4 October 2017 at 15:40, Laszlo Ersek  wrote:
> On 10/04/17 15:54, Ard Biesheuvel wrote:
>> On 4 October 2017 at 14:49, Zeng, Star  wrote:
>>> Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it will be
>>> rejected by MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that
>>> will check the VariableName against UEFI spec “Table 13. Global Variables”
>>> if the VendorGuid is gEfiGlobalVariableGuid.
>>>
>>>
>>>
>>> I would suspect there is a bug at other place if the code ends up calling
>>> this function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext".
>>>
>>
>> That still does not mean you should ASSERT() here. The state of the
>> variable store != the internals of the code, and so it should be
>> considered external input to some extent.
>
> At least under some circumstances, I disagree with this. The assumption
> that the variable store can be written only by privileged firmware
> routines is core to Secure Boot, for example.
>

That is true. But the firmware that wrote to the varstore may be a
different version from the one reading it.

>> ASSERTs are meant to catch
>> programming errors, not errors in the varstore image.
>
> I agree.
>
> However, as a corollary to the above, if said "privileged routines" are
> supposed to catch all invalid inputs passed to gRT->SetVariable(), then
> the rest of the firmware is right to assume that the contents of the
> variable store are valid. If it is found invalid at some point, then it
> is indeed due to a programming error (somewhere in the
> gRT->SetVariable() machinery, that is), so the ASSERT() is justified.
>
> Another example in support of this argument is the Fault Tolerant Write
> machinery -- the firmware tries very hard to recover from power loss
> during a varstore update. If, on reboot, the error proves
> non-recoverable (i.e. we cannot even roll back to a previous pristine
> state), then that can be considered a bug (or design error) in FTW.
>
>
> That said, I agree with the patch. BmCharToUint() explicitly documents
> "conversion failed" as a return condition, and both functions that call
> BmCharToUint(), namely EfiBootManagerIsValidLoadOptionVariableName() and
> BmIsKeyOptionVariable(), check for that return condition.
>

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

2017-10-04 Thread Laszlo Ersek

On 10/04/17 15:54, Ard Biesheuvel wrote:
> On 4 October 2017 at 14:49, Zeng, Star  wrote:
>> Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it will be
>> rejected by MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that
>> will check the VariableName against UEFI spec “Table 13. Global Variables”
>> if the VendorGuid is gEfiGlobalVariableGuid.
>>
>>
>>
>> I would suspect there is a bug at other place if the code ends up calling
>> this function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext".
>>
> 
> That still does not mean you should ASSERT() here. The state of the
> variable store != the internals of the code, and so it should be
> considered external input to some extent.

At least under some circumstances, I disagree with this. The assumption
that the variable store can be written only by privileged firmware
routines is core to Secure Boot, for example.

> ASSERTs are meant to catch
> programming errors, not errors in the varstore image.

I agree.

However, as a corollary to the above, if said "privileged routines" are
supposed to catch all invalid inputs passed to gRT->SetVariable(), then
the rest of the firmware is right to assume that the contents of the
variable store are valid. If it is found invalid at some point, then it
is indeed due to a programming error (somewhere in the
gRT->SetVariable() machinery, that is), so the ASSERT() is justified.

Another example in support of this argument is the Fault Tolerant Write
machinery -- the firmware tries very hard to recover from power loss
during a varstore update. If, on reboot, the error proves
non-recoverable (i.e. we cannot even roll back to a previous pristine
state), then that can be considered a bug (or design error) in FTW.

That said, I agree with the patch. BmCharToUint() explicitly documents
"conversion failed" as a return condition, and both functions that call
BmCharToUint(), namely EfiBootManagerIsValidLoadOptionVariableName() and
BmIsKeyOptionVariable(), check for that return condition.

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

[edk2] [PATCH 5/5] IntelFrameworkModulePkg FwVolDxe: Get FV auth status propagated from PEI

2017-10-04 Thread Star Zeng

FV3 HOB was introduced by new (>= 1.5) PI spec, it is intended to
be used to propagate PEI-phase FV authentication status to DXE.
This patch is to update FwVolDxe to get the authentication status
propagated from PEI-phase to DXE by FV3 HOB when producing FV
protocol.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../Universal/FirmwareVolume/FwVolDxe/FwVol.c  | 73 --
 .../FirmwareVolume/FwVolDxe/FwVolDriver.h  |  3 +-
 .../Universal/FirmwareVolume/FwVolDxe/FwVolDxe.inf |  4 +-
 3 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c 
b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
index 65a292db6b91..91fcd4721244 100644
--- a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
+++ b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c
@@ -4,7 +4,7 @@
   Layers on top of Firmware Block protocol to produce a file abstraction
   of FV based files.
 
-  Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions
@@ -195,7 +195,7 @@ FreeFvDeviceResource (
 /**
 
   Firmware volume inherits authentication status from the FV image file and 
section(in another firmware volume)
-  where it came from.
+  where it came from or propagated from PEI-phase.
 
   @param  FvDevice  A pointer to the FvDevice.
 
@@ -205,26 +205,30 @@ FwVolInheritAuthenticationStatus (
   IN FV_DEVICE  *FvDevice
   )
 {
-  EFI_STATUSStatus;
-  EFI_FIRMWARE_VOLUME_HEADER*CachedFvHeader;
-  EFI_FIRMWARE_VOLUME_EXT_HEADER*CachedFvExtHeader;
-  EFI_FIRMWARE_VOLUME2_PROTOCOL *ParentFvProtocol;
-  UINTN Key;
-  EFI_GUID  FileNameGuid;
-  EFI_FV_FILETYPE   FileType;
-  EFI_FV_FILE_ATTRIBUTESFileAttributes;
-  UINTN FileSize;
-  EFI_SECTION_TYPE  SectionType;
-  UINT32AuthenticationStatus;
-  EFI_FIRMWARE_VOLUME_HEADER*FvHeader;
-  EFI_FIRMWARE_VOLUME_EXT_HEADER*FvExtHeader;
-  UINTN BufferSize;
-
-  CachedFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) FvDevice->CachedFv;
+  EFI_STATUSStatus;
+  EFI_FIRMWARE_VOLUME_HEADER*CachedFvHeader;
+  EFI_FIRMWARE_VOLUME_EXT_HEADER*CachedFvExtHeader;
+  EFI_FIRMWARE_VOLUME2_PROTOCOL *ParentFvProtocol;
+  UINTN Key;
+  EFI_GUID  FileNameGuid;
+  EFI_FV_FILETYPE   FileType;
+  EFI_FV_FILE_ATTRIBUTESFileAttributes;
+  UINTN FileSize;
+  EFI_SECTION_TYPE  SectionType;
+  UINT32AuthenticationStatus;
+  EFI_FIRMWARE_VOLUME_HEADER*FvHeader;
+  EFI_FIRMWARE_VOLUME_EXT_HEADER*FvExtHeader;
+  UINTN BufferSize;
+  EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL*Fvb;
+  EFI_FVB_ATTRIBUTES_2  FvbAttributes;
+  EFI_PHYSICAL_ADDRESS  BaseAddress;
+  EFI_PEI_HOB_POINTERS  Fv3Hob;
 
   if (FvDevice->Fv.ParentHandle != NULL) {
+CachedFvHeader = (EFI_FIRMWARE_VOLUME_HEADER *) (UINTN) FvDevice->CachedFv;
+
 //
-// By Parent Handle, find out the FV image file and section(in another 
firmware volume) where the firmware volume came from 
+// By Parent Handle, find out the FV image file and section(in another 
firmware volume) where the firmware volume came from
 //
 Status = gBS->HandleProtocol (FvDevice->Fv.ParentHandle, 
&gEfiFirmwareVolume2ProtocolGuid, (VOID **) &ParentFvProtocol);
 if (!EFI_ERROR (Status) && (ParentFvProtocol != NULL)) {
@@ -258,7 +262,7 @@ FwVolInheritAuthenticationStatus (
 if (!EFI_ERROR (Status)) {
   if ((FvHeader->FvLength == CachedFvHeader->FvLength) &&
   (FvHeader->ExtHeaderOffset == CachedFvHeader->ExtHeaderOffset)) {
-if (FvHeader->ExtHeaderOffset !=0) {
+if (FvHeader->ExtHeaderOffset != 0) {
   //
   // Both FVs contain extension header, then compare their FV Name 
GUID
   //
@@ -292,6 +296,35 @@ FwVolInheritAuthenticationStatus (
 }
   } while (TRUE);
 }
+  } else {
+Fvb = FvDevice->Fvb;
+
+Status  = Fvb->GetAttributes (Fvb, &FvbAttributes);
+if (EFI_ERROR (Status)) {
+  return;
+}
+
+if ((FvbAttributes & EFI_FVB2_MEMORY_MAPPED) != 0) {
+  //
+  // Get volume base address
+  //
+  Status = Fvb->GetPhysicalAddress (Fvb, &BaseAddress);
+  if (EFI_ERROR (Status)) {
+

[edk2] [PATCH 4/5] MdeModulePkg Core: Propagate PEI-phase FV authentication status to DXE

2017-10-04 Thread Star Zeng

FV3 HOB was introduced by new (>= 1.5) PI spec, it is intended to
be used to propagate PEI-phase FV authentication status to DXE.
This patch is to update PeiCore to build FV3 HOB with the
authentication status and DxeCore to get the authentication
status from FV3 HOB when producing FVB Protocol.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   | 41 ---
 MdeModulePkg/Core/Dxe/FwVol/FwVol.c   | 13 -
 MdeModulePkg/Core/Dxe/FwVolBlock/FwVolBlock.c | 23 +++
 MdeModulePkg/Core/Pei/FwVol/FwVol.c   |  9 ++
 4 files changed, 69 insertions(+), 17 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c 
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 91e94a78d205..433cca3a800c 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -380,10 +380,43 @@ DxeMain (
   }
 }
 for (Hob.Raw = HobStart; !END_OF_HOB_LIST(Hob); Hob.Raw = 
GET_NEXT_HOB(Hob)) {
-  if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV2) {
-DEBUG ((DEBUG_INFO | DEBUG_LOAD, "FV2 Hob   0x%0lx - 
0x%0lx\n", Hob.FirmwareVolume2->BaseAddress, Hob.FirmwareVolume2->BaseAddress + 
Hob.FirmwareVolume2->Length - 1));
-  } else if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV) {
-DEBUG ((DEBUG_INFO | DEBUG_LOAD, "FV Hob0x%0lx - 
0x%0lx\n", Hob.FirmwareVolume->BaseAddress, Hob.FirmwareVolume->BaseAddress + 
Hob.FirmwareVolume->Length - 1));
+  if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV) {
+DEBUG ((
+  DEBUG_INFO | DEBUG_LOAD,
+  "FV Hob0x%0lx - 0x%0lx\n",
+  Hob.FirmwareVolume->BaseAddress,
+  Hob.FirmwareVolume->BaseAddress + Hob.FirmwareVolume->Length - 1
+  ));
+  } else if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV2) {
+DEBUG ((
+  DEBUG_INFO | DEBUG_LOAD,
+  "FV2 Hob   0x%0lx - 0x%0lx\n",
+  Hob.FirmwareVolume2->BaseAddress,
+  Hob.FirmwareVolume2->BaseAddress + Hob.FirmwareVolume2->Length - 1
+  ));
+DEBUG ((
+  DEBUG_INFO | DEBUG_LOAD,
+  "  %g - %g\n",
+  &Hob.FirmwareVolume2->FvName,
+  &Hob.FirmwareVolume2->FileName
+  ));
+  } else if (GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_FV3) {
+DEBUG ((
+  DEBUG_INFO | DEBUG_LOAD,
+  "FV3 Hob   0x%0lx - 0x%0lx - 0x%x - 0x%x\n",
+  Hob.FirmwareVolume3->BaseAddress,
+  Hob.FirmwareVolume3->BaseAddress + Hob.FirmwareVolume3->Length - 1,
+  Hob.FirmwareVolume3->AuthenticationStatus,
+  Hob.FirmwareVolume3->ExtractedFv
+  ));
+if (Hob.FirmwareVolume3->ExtractedFv) {
+  DEBUG ((
+DEBUG_INFO | DEBUG_LOAD,
+"  %g - %g\n",
+&Hob.FirmwareVolume3->FvName,
+&Hob.FirmwareVolume3->FileName
+));
+}
   }
 }
   DEBUG_CODE_END ();
diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c 
b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
index fe12d6e0ac30..2f5867b59d90 100644
--- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
+++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c
@@ -3,7 +3,7 @@
   Layers on top of Firmware Block protocol to produce a file abstraction
   of FV based files.
 
-Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -711,13 +711,10 @@ NotifyFwVolBlock (
   FvDevice->FwVolHeader = FwVolHeader;
   FvDevice->IsFfs3Fv= CompareGuid (&FwVolHeader->FileSystemGuid, 
&gEfiFirmwareFileSystem3Guid);
   FvDevice->Fv.ParentHandle = Fvb->ParentHandle;
-
-  if (Fvb->ParentHandle != NULL) {
-//
-// Inherit the authentication status from FVB.
-//
-FvDevice->AuthenticationStatus = GetFvbAuthenticationStatus (Fvb);
-  }
+  //
+  // Inherit the authentication status from FVB.
+  //
+  FvDevice->AuthenticationStatus = GetFvbAuthenticationStatus (Fvb);
   
   if (!EFI_ERROR (FvCheck (FvDevice))) {
 //
diff --git a/MdeModulePkg/Core/Dxe/FwVolBlock/FwVolBlock.c 
b/MdeModulePkg/Core/Dxe/FwVolBlock/FwVolBlock.c
index bc7b34140f84..f7fb18ae15df 100644
--- a/MdeModulePkg/Core/Dxe/FwVolBlock/FwVolBlock.c
+++ b/MdeModulePkg/Core/Dxe/FwVolBlock/FwVolBlock.c
@@ -4,7 +4,7 @@
   It consumes FV HOBs and creates read-only Firmare Volume Block protocol
   instances for each of them.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and ma

[edk2] [PATCH 2/5] MdePkg HobLib: Add BuildFv3Hob API

2017-10-04 Thread Star Zeng

Add BuildFv3Hob API in HobLib.h and implement the API
in HobLib instances PeiHobLib, DxeHobLib and DxeCoreHobLib.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdePkg/Include/Library/HobLib.h   | 34 --
 MdePkg/Library/DxeCoreHobLib/HobLib.c | 35 ++-
 MdePkg/Library/DxeHobLib/HobLib.c | 32 +
 MdePkg/Library/PeiHobLib/HobLib.c | 54 ++-
 4 files changed, 151 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Library/HobLib.h b/MdePkg/Include/Library/HobLib.h
index fc48703826c5..6f1f7b3f5f20 100644
--- a/MdePkg/Include/Library/HobLib.h
+++ b/MdePkg/Include/Library/HobLib.h
@@ -6,9 +6,9 @@
   defined in the PI Specification.
   A HOB is a Hand-Off Block, defined in the Framework architecture, that
   allows the PEI phase to pass information to the DXE phase. HOBs are position
-  independent and can be relocated easily to different memory memory locations.
+  independent and can be relocated easily to different memory locations.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -343,6 +343,36 @@ BuildFv2Hob (
   );
 
 /**
+  Builds a EFI_HOB_TYPE_FV3 HOB.
+
+  This function builds a EFI_HOB_TYPE_FV3 HOB.
+  It can only be invoked during PEI phase;
+  for DXE phase, it will ASSERT() since PEI HOB is read-only for DXE phase.
+
+  If there is no additional space for HOB creation, then ASSERT().
+  If the FvImage buffer is not at its required alignment, then ASSERT().
+
+  @param BaseAddressThe base address of the Firmware Volume.
+  @param Length The size of the Firmware Volume in bytes.
+  @param AuthenticationStatus   The authentication status.
+  @param ExtractedFvTRUE if the FV was extracted as a file within 
another firmware volume.
+FALSE otherwise.
+  @param FvName The name of the Firmware Volume. Valid only if 
IsExtractedFv is TRUE
+  @param FileName   The name of the file. Valid only if 
IsExtractedFv is TRUE
+
+**/
+VOID
+EFIAPI
+BuildFv3Hob (
+  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
+  IN  UINT64  Length,
+  IN  UINT32  AuthenticationStatus,
+  IN  BOOLEAN ExtractedFv,
+  IN CONSTEFI_GUID*FvName, OPTIONAL
+  IN CONSTEFI_GUID*FileName OPTIONAL
+  );
+
+/**
   Builds a Capsule Volume HOB.
 
   This function builds a Capsule Volume HOB.
diff --git a/MdePkg/Library/DxeCoreHobLib/HobLib.c 
b/MdePkg/Library/DxeCoreHobLib/HobLib.c
index 2e5fb1c6ee55..2f4f4b473386 100644
--- a/MdePkg/Library/DxeCoreHobLib/HobLib.c
+++ b/MdePkg/Library/DxeCoreHobLib/HobLib.c
@@ -1,7 +1,7 @@
 /** @file
   HOB Library implementation for DxeCore driver.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -433,6 +433,39 @@ BuildFv2Hob (
 }
 
 /**
+  Builds a EFI_HOB_TYPE_FV3 HOB.
+
+  This function builds a EFI_HOB_TYPE_FV3 HOB.
+  It can only be invoked during PEI phase;
+  for DXE phase, it will ASSERT() since PEI HOB is read-only for DXE phase.
+
+  If there is no additional space for HOB creation, then ASSERT().
+  If the FvImage buffer is not at its required alignment, then ASSERT().
+
+  @param BaseAddressThe base address of the Firmware Volume.
+  @param Length The size of the Firmware Volume in bytes.
+  @param AuthenticationStatus   The authentication status.
+  @param ExtractedFvTRUE if the FV was extracted as a file within 
another firmware volume.
+FALSE otherwise.
+  @param FvName The name of the Firmware Volume. Valid only if 
IsExtractedFv is TRUE
+  @param FileName   The name of the file. Valid only if 
IsExtractedFv is TRUE
+
+**/
+VOID
+EFIAPI
+BuildFv3Hob (
+  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
+  IN  UINT64  Length,
+  IN  UINT32  AuthenticationStatus,
+  IN  BOOLEAN ExtractedFv,
+  IN CONSTEFI_GUID*FvName, OPTIONAL
+  IN CONSTEFI_GUID*FileName OPTIONAL
+  )
+{
+  ASSERT (FALSE);
+}
+
+/**
   Builds a Capsule Volume HOB.
 
   This function builds a Capsule Volume

[edk2] [PATCH 3/5] IntelFrameworkPkg PeiHobLibFramework: Implement BuildFv3Hob

2017-10-04 Thread Star Zeng

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../Library/PeiHobLibFramework/HobLib.c| 54 +-
 1 file changed, 53 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkPkg/Library/PeiHobLibFramework/HobLib.c 
b/IntelFrameworkPkg/Library/PeiHobLibFramework/HobLib.c
index 223e56a9db12..c7e6d4b75aba 100644
--- a/IntelFrameworkPkg/Library/PeiHobLibFramework/HobLib.c
+++ b/IntelFrameworkPkg/Library/PeiHobLibFramework/HobLib.c
@@ -5,7 +5,7 @@
  This library instance uses EFI_HOB_TYPE_CV defined in Intel framework HOB 
specification v0.9
  to implement HobLib BuildCvHob() API. 
 
-Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -612,6 +612,58 @@ BuildFv2Hob (
 }
 
 /**
+  Builds a EFI_HOB_TYPE_FV3 HOB.
+
+  This function builds a EFI_HOB_TYPE_FV3 HOB.
+  It can only be invoked during PEI phase;
+  for DXE phase, it will ASSERT() since PEI HOB is read-only for DXE phase.
+
+  If there is no additional space for HOB creation, then ASSERT().
+  If the FvImage buffer is not at its required alignment, then ASSERT().
+
+  @param BaseAddressThe base address of the Firmware Volume.
+  @param Length The size of the Firmware Volume in bytes.
+  @param AuthenticationStatus   The authentication status.
+  @param ExtractedFvTRUE if the FV was extracted as a file within 
another firmware volume.
+FALSE otherwise.
+  @param FvName The name of the Firmware Volume. Valid only if 
IsExtractedFv is TRUE
+  @param FileName   The name of the file. Valid only if 
IsExtractedFv is TRUE
+
+**/
+VOID
+EFIAPI
+BuildFv3Hob (
+  IN  EFI_PHYSICAL_ADDRESSBaseAddress,
+  IN  UINT64  Length,
+  IN  UINT32  AuthenticationStatus,
+  IN  BOOLEAN ExtractedFv,
+  IN CONSTEFI_GUID*FvName, OPTIONAL
+  IN CONSTEFI_GUID*FileName OPTIONAL
+  )
+{
+  EFI_HOB_FIRMWARE_VOLUME3  *Hob;
+
+  if (!InternalCheckFvAlignment (BaseAddress, Length)) {
+ASSERT (FALSE);
+return;
+  }
+
+  Hob = InternalPeiCreateHob (EFI_HOB_TYPE_FV3, (UINT16) sizeof 
(EFI_HOB_FIRMWARE_VOLUME3));
+  if (Hob == NULL) {
+return;
+  }
+
+  Hob->BaseAddress  = BaseAddress;
+  Hob->Length   = Length;
+  Hob->AuthenticationStatus = AuthenticationStatus;
+  Hob->ExtractedFv  = ExtractedFv;
+  if (ExtractedFv) {
+CopyGuid (&Hob->FvName, FvName);
+CopyGuid (&Hob->FileName, FileName);
+  }
+}
+
+/**
   Builds a Capsule Volume HOB.
 
   This function builds a Capsule Volume HOB.
-- 
2.13.3.windows.1

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

[edk2] [PATCH 1/5] MdePkg PiHob.h: Add FV3 HOB definitions

2017-10-04 Thread Star Zeng

Follow PI 1.6 spec to add FV3 HOB definitions

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdePkg/Include/Pi/PiHob.h | 43 +--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Pi/PiHob.h b/MdePkg/Include/Pi/PiHob.h
index 29467e79d59c..4c90998bc03c 100644
--- a/MdePkg/Include/Pi/PiHob.h
+++ b/MdePkg/Include/Pi/PiHob.h
@@ -1,7 +1,7 @@
 /** @file
   HOB related definitions in PI.
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under
 the terms and conditions of the BSD License that accompanies this distribution.
 The full text of the license may be found at
@@ -11,7 +11,7 @@ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS 
IS" BASIS,
 WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
   @par Revision Reference:
-  PI Version 1.4a
+  PI Version 1.6
 
 **/
 
@@ -31,6 +31,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define EFI_HOB_TYPE_FV2  0x0009
 #define EFI_HOB_TYPE_LOAD_PEIM_UNUSED 0x000A
 #define EFI_HOB_TYPE_UEFI_CAPSULE 0x000B
+#define EFI_HOB_TYPE_FV3  0x000C
 #define EFI_HOB_TYPE_UNUSED   0xFFFE
 #define EFI_HOB_TYPE_END_OF_HOB_LIST  0x
 
@@ -399,6 +400,43 @@ typedef struct {
   EFI_GUIDFileName;
 } EFI_HOB_FIRMWARE_VOLUME2;
 
+///
+/// Details the location of a firmware volume that was extracted
+/// from a file within another firmware volume.
+///
+typedef struct {
+  ///
+  /// The HOB generic header. Header.HobType = EFI_HOB_TYPE_FV3.
+  ///
+  EFI_HOB_GENERIC_HEADER  Header;
+  ///
+  /// The physical memory-mapped base address of the firmware volume.
+  ///
+  EFI_PHYSICAL_ADDRESSBaseAddress;
+  ///
+  /// The length in bytes of the firmware volume.
+  ///
+  UINT64  Length;
+  ///
+  /// The authentication status.
+  ///
+  UINT32  AuthenticationStatus;
+  ///
+  /// TRUE if the FV was extracted as a file within another firmware volume.
+  /// FALSE otherwise.
+  ///
+  BOOLEAN ExtractedFv;
+  ///
+  /// The name of the firmware volume.
+  /// Valid only if IsExtractedFv is TRUE.
+  ///
+  EFI_GUIDFvName;
+  ///
+  /// The name of the firmware file that contained this firmware volume.
+  /// Valid only if IsExtractedFv is TRUE.
+  ///
+  EFI_GUIDFileName;
+} EFI_HOB_FIRMWARE_VOLUME3;
 
 ///
 /// Describes processor information, such as address space and I/O space 
capabilities.
@@ -469,6 +507,7 @@ typedef union {
   EFI_HOB_GUID_TYPE   *Guid;
   EFI_HOB_FIRMWARE_VOLUME *FirmwareVolume;
   EFI_HOB_FIRMWARE_VOLUME2*FirmwareVolume2;
+  EFI_HOB_FIRMWARE_VOLUME3*FirmwareVolume3;
   EFI_HOB_CPU *Cpu;
   EFI_HOB_MEMORY_POOL *Pool;
   EFI_HOB_UEFI_CAPSULE*Capsule;
-- 
2.13.3.windows.1

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

[edk2] [PATCH 0/5] Propagate PEI-phase FV authentication status to DXE

2017-10-04 Thread Star Zeng

FV3 HOB was introduced by new (>= 1.5) PI spec, it is intended to
be used to propagate PEI-phase FV authentication status to DXE.
Check the separated patches for the details.


Cc: Liming Gao 

Star Zeng (5):
  MdePkg PiHob.h: Add FV3 HOB definitions
  MdePkg HobLib: Add BuildFv3Hob API
  IntelFrameworkPkg PeiHobLibFramework: Implement BuildFv3Hob
  MdeModulePkg Core: Propagate PEI-phase FV authentication status to DXE
  IntelFrameworkModulePkg FwVolDxe: Get FV auth status propagated from
PEI

 .../Universal/FirmwareVolume/FwVolDxe/FwVol.c  | 73 --
 .../FirmwareVolume/FwVolDxe/FwVolDriver.h  |  3 +-
 .../Universal/FirmwareVolume/FwVolDxe/FwVolDxe.inf |  4 +-
 .../Library/PeiHobLibFramework/HobLib.c| 54 +++-
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c| 41 ++--
 MdeModulePkg/Core/Dxe/FwVol/FwVol.c| 13 ++--
 MdeModulePkg/Core/Dxe/FwVolBlock/FwVolBlock.c  | 23 +--
 MdeModulePkg/Core/Pei/FwVol/FwVol.c|  9 +++
 MdePkg/Include/Library/HobLib.h| 34 +-
 MdePkg/Include/Pi/PiHob.h  | 43 -
 MdePkg/Library/DxeCoreHobLib/HobLib.c  | 35 ++-
 MdePkg/Library/DxeHobLib/HobLib.c  | 32 ++
 MdePkg/Library/PeiHobLib/HobLib.c  | 54 +++-
 13 files changed, 371 insertions(+), 47 deletions(-)

-- 
2.13.3.windows.1

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

2017-10-04 Thread Zeng, Star

Thanks for confirming the urgency.

I have no strong motivation to keep/remove the ASSERT, I would like Ruiyu to 
argue and make the decision.
I mainly want the issue (the code ends up calling this 
function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext") could be 
root caused.

Thanks,
Star
-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Wednesday, October 4, 2017 9:54 PM
To: Zeng, Star 
Cc: Yao, Jiewen ; Ni, Ruiyu ; 
edk2-devel@lists.01.org; Dong, Eric ; 
leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 
'BootNext' varname

On 4 October 2017 at 14:49, Zeng, Star  wrote:
> Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it 
> will be rejected by 
> MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that will check the 
> VariableName against UEFI spec “Table 13. Global Variables”
> if the VendorGuid is gEfiGlobalVariableGuid.
>
>
>
> I would suspect there is a bug at other place if the code ends up 
> calling this function(EfiBootManagerIsValidLoadOptionVariableName) on 
> L"BootNext".
>

That still does not mean you should ASSERT() here. The state of the variable 
store != the internals of the code, and so it should be considered external 
input to some extent. ASSERTs are meant to catch programming errors, not errors 
in the varstore image.

>
> Ard,
>
> Is the fix urgent or not for you?
>

Not really. But fwupdate is shipping as part of many distros, so I guess others 
may run into it as well.

> I may want to wait for Ruiyu’s back to take some look at the detail of it.
>

That is fine.

> At the same time, you may help check the code flow in some detail if 
> you have free time, I think that will be helpful. J
>

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

2017-10-04 Thread Ard Biesheuvel

On 4 October 2017 at 14:49, Zeng, Star  wrote:
> Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it will be
> rejected by MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that
> will check the VariableName against UEFI spec “Table 13. Global Variables”
> if the VendorGuid is gEfiGlobalVariableGuid.
>
>
>
> I would suspect there is a bug at other place if the code ends up calling
> this function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext".
>

That still does not mean you should ASSERT() here. The state of the
variable store != the internals of the code, and so it should be
considered external input to some extent. ASSERTs are meant to catch
programming errors, not errors in the varstore image.

>
> Ard,
>
> Is the fix urgent or not for you?
>

Not really. But fwupdate is shipping as part of many distros, so I
guess others may run into it as well.

> I may want to wait for Ruiyu’s back to take some look at the detail of it.
>

That is fine.

> At the same time, you may help check the code flow in some detail if you
> have free time, I think that will be helpful. J
>

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

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 'BootNext' varname

2017-10-04 Thread Zeng, Star

Creating Boot000@ with gEfiGlobalVariableGuid can not succeed as it will be 
rejected by MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf that will 
check the VariableName against UEFI spec "Table 13. Global Variables" if the 
VendorGuid is gEfiGlobalVariableGuid.

I would suspect there is a bug at other place if the code ends up calling this 
function(EfiBootManagerIsValidLoadOptionVariableName) on L"BootNext".

Ard,
Is the fix urgent or not for you?
I may want to wait for Ruiyu's back to take some look at the detail of it.
At the same time, you may help check the code flow in some detail if you have 
free time, I think that will be helpful. :)

Thanks,
Star
From: Yao, Jiewen
Sent: Wednesday, October 4, 2017 8:18 AM
To: Ard Biesheuvel ; Zeng, Star 
Cc: Ni, Ruiyu ; edk2-devel@lists.01.org; Dong, Eric 
; leif.lindh...@linaro.org
Subject: RE: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 
'BootNext' varname

I agree. If creating Boot000@ can hit this ASSERT, this ASSERT must be removed. 
Error handling must be used to handle such case.

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Wednesday, October 4, 2017 8:00 AM
To: Zeng, Star mailto:star.z...@intel.com>>
Cc: Ni, Ruiyu mailto:ruiyu...@intel.com>>; 
edk2-devel@lists.01.org; Dong, Eric 
mailto:eric.d...@intel.com>>; 
leif.lindh...@linaro.org
Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: don't ASSERT on 
'BootNext' varname

On 4 October 2017 at 00:56, Zeng, Star 
mailto:star.z...@intel.com>> wrote:
> Hi Ard,
>
> To me, the ASSERT there seems on purpose to help catch the misuse of that 
> interface.
> Could you share the case you met the ASSERT?
>

When using the 'fwupdate' Linux tool to perform capsule updates,
BootNext is set to the id of the Boot### variable it creates to run
fwupx64.efi, which executes in UEFI context.

I haven't looked in great detail how exactly the code ends up calling
this function on L"BootNext", but the ASSERT () is wrong, because it
is called on variable names that are modifiable externally.

For example, if I create a variable Boot000@ from the UEFI Shell, the
firmware should not crash.

> Given that interface is an open API of UefiBootManagerLib, some comments for 
> the behavior of ASSERT may can be added to be more clear.
>

I still think the assert should be removed.

> Cc Ruiyu who is the expert of this part code.
>

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

Re: [edk2] [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency

2017-10-04 Thread Ladi Prosek

On Wed, Oct 4, 2017 at 12:39 PM, Laszlo Ersek  wrote:
> On 10/04/17 03:18, Yao, Jiewen wrote:
>> Thanks. Laszlo.
>> This patch looks good to me in general.
>>
>> One minor comment is below:
>> MorLockInitAtEndOfDxe()
>> +  if (!mMorLockInitializationRequired) {
>> +//
>> +// The EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL has never been installed, 
>> thus
>> +// the variable write service is unavailable. Do nothing.
>> +//
>> +return;
>> +  }
>> I think it is an illegal case. I would add ASSERT before return.
>
> OK, I will replace the sentence "Do nothing." with "This should never
> happen.", and also add ASSERT (FALSE) above the "return".
>
>> And I hope Ladi can help us double confirm if this patch fixed the device 
>> guard problem. :-)
>
> Right!

I have tested the patch and can confirm that it fixes the Windows
device guard problem.

Tested-by: Ladi Prosek 

> Ladi stated in the RHBZ that he had built OVMF; I'll talk with him
> off-list regardless, to see if I can help with anything.

This was my first time building OVMF on my machine (the builds I had
done before filing the RHBZ were all using Red Hat's internal build
service). The only thing that threw me off was -D SMM_REQUIRE which I
was initially missing so I couldn't reproduce the issue on baseline.

> Thank you Jiewen!
> Laszlo

Thank you Laszlo and Jiewen for the very quick turnaround on this!
Ladi

>
>>
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, October 4, 2017 5:28 AM
>>> To: edk2-devel-01 
>>> Cc: Dong, Eric ; Yao, Jiewen ; 
>>> Ladi
>>> Prosek ; Zeng, Star 
>>> Subject: [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock
>>> inconsistency
>>>
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: mor_lock_init_at_end_of_dxe
>>>
>>> This patch set fixes the issue reported in the following items:
>>>
>>> * Inconsistent MOR control variables exposed by OVMF, breaks Windows
>>>   Device Guard
>>>
>>>   https://bugzilla.redhat.com/show_bug.cgi?id=1496170
>>>
>>> * VariableSmm MorLockInit(): create MORLock only if / after MOR exists
>>>
>>>   https://bugzilla.tianocore.org/show_bug.cgi?id=727
>>>
>>> Patches #1 through #3 are cleanups.
>>>
>>> Patch #4 is a small helper patch for patch #5.
>>>
>>> Patch #5 is the actual fix, following Jiewen's suggestions from the
>>> edk2-devel thread
>>>
>>> * [edk2] multiple levels of support for MOR / MORLock
>>>
>>>   https://lists.01.org/pipermail/edk2-devel/2017-September/015444.html
>>>   https://lists.01.org/pipermail/edk2-devel/2017-October/015530.html
>>>
>>> Patch #6 is a workaround for some OSes (minimally Fedora 24-26, and some
>>> Debian versions) that create the MOR variable even if the platform
>>> doesn't offer it up-front. This patch also follows Jiewen's suggestion
>>> from the same edk2-devel thread.
>>>
>>> (
>>>
>>> BTW, at Paolo's recommendation, I've now reported this kernel issue for
>>> Fedora, under
>>>
>>> * incorrect downstream-only Platform Reset Attack Mitigation patch in
>>>   the F24-F26 kernels
>>>
>>>   https://bugzilla.redhat.com/show_bug.cgi?id=1498159
>>>
>>> )
>>>
>>> I've checked this set for basic regressions, using OVMF, normal boot and
>>> S3 suspend/resume:
>>>
>>> * Q35, SMM, IA32:
>>>   - Fedora 25 -- verified patch #6 specifically
>>>
>>> * i440fx, no SMM, X64:
>>>   - Fedora 24
>>>
>>> * Q35, SMM, IA32X64:
>>>   - Fedora 26 -- verified patch #6 specifically
>>>   - Windows 7
>>>   - Windows 8.1
>>>   - Windows 10
>>>   - Windows Server 2008 R2
>>>   - Windows Server 2012 R2
>>>
>>> I didn't / couldn't test this set in the following two environments:
>>>
>>> - on platforms where TcgMor.inf is included in the firmware, and the MOR
>>>   variable exists genuinely,
>>>
>>> - in the nested virt setup where Ladi reported the Device Guard
>>>   breakage. (If I understand correctly, ATM this requires additional
>>>   host kernel (KVM) patches.)
>>>
>>> Test results / feedback from those envs would be appreciated.
>>>
>>> Cc: Eric Dong 
>>> Cc: Jiewen Yao 
>>> Cc: Ladi Prosek 
>>> Cc: Star Zeng 
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (6):
>>>   MdeModulePkg/Variable/RuntimeDxe: move SecureBootHook() decl to new
>>> header
>>>   MdeModulePkg/Variable/RuntimeDxe: move MOR func. declarations to
>>> header
>>>   MdeModulePkg/Variable/RuntimeDxe: introduce MorLockInitAtEndOfDxe()
>>> hook
>>>   MdeModulePkg/Variable/RuntimeDxe: permit MorLock deletion for passthru
>>> req
>>>   MdeModulePkg/Variable/RuntimeDxe: delay MorLock creation until
>>> EndOfDxe
>>>   MdeModulePkg/Variable/RuntimeDxe: delete and lock OS-created MOR
>>> variable
>>>
>>>  MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
>>> |   2 +
>>>  MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h|
>>> 89 ++
>>>  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
>>> |  45 +++--
>>>  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
>>> | 173 +

Re: [edk2] [PATCH v2 0/9] Enhanced SMM support to AMD-based x86 systems.

2017-10-04 Thread Laszlo Ersek

On 10/04/17 03:22, Kinney, Michael D wrote:
> Leo,
> 
> The general design issue here is that a PCD and PCD macro were
> added to a UefiCpuPkg include file Include/Register/SmramSaveStateMap.h.
> This means any library or module that includes this package include
> file will break the build until the PCD is added to the INF file for
> that library or module.
> 
> The INF file for a module is supposed to express the PCDs that the
> library or module sources access.  We do not expect #include
> statements from package include files to require adding additional
> PCDs.
> 
> Updating the code for these values to be detected or configurable
> is a good idea.
> 
> One option is to update the C code that currently uses the #define
> names to use the PCD access function directly, instead of adding
> the PcdGetxxx() to the Include/Register/SmramSaveStateMap.h file.
> 
> Jiewen's idea to remove the PCD and detect the offset from CPUID
> also looks like a reasonable approach.

In addition to the above, I'd like to request a bit more "telling"
subject lines for the patches. Using the OVMF patch as example,

  OvmfPkg: SmmCpuFeaturesLib library.

is not really helpful; I'd consider the following an improvement:

  OvmfPkg/SmmCpuFeaturesLib: consume SMRAM Save State Map Offset PCD

66 characters -- we generally limit subjects to 74.

(Of course I realize the patch might entirely be replaced in the next
version, based on Jiewen's and Mike's feedback -- that's OK with me, I
just wanted to give an example.)

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

Re: [edk2] [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock inconsistency

2017-10-04 Thread Laszlo Ersek

On 10/04/17 03:18, Yao, Jiewen wrote:
> Thanks. Laszlo.
> This patch looks good to me in general.
> 
> One minor comment is below:
> MorLockInitAtEndOfDxe()
> +  if (!mMorLockInitializationRequired) {
> +//
> +// The EFI_SMM_FAULT_TOLERANT_WRITE_PROTOCOL has never been installed, 
> thus
> +// the variable write service is unavailable. Do nothing.
> +//
> +return;
> +  }
> I think it is an illegal case. I would add ASSERT before return.

OK, I will replace the sentence "Do nothing." with "This should never
happen.", and also add ASSERT (FALSE) above the "return".

> And I hope Ladi can help us double confirm if this patch fixed the device 
> guard problem. :-)

Right!

Ladi stated in the RHBZ that he had built OVMF; I'll talk with him
off-list regardless, to see if I can help with anything.

Thank you Jiewen!
Laszlo


> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, October 4, 2017 5:28 AM
>> To: edk2-devel-01 
>> Cc: Dong, Eric ; Yao, Jiewen ; 
>> Ladi
>> Prosek ; Zeng, Star 
>> Subject: [PATCH 0/6] MdeModulePkg/VariableSmm: fix MOR / MorLock
>> inconsistency
>>
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: mor_lock_init_at_end_of_dxe
>>
>> This patch set fixes the issue reported in the following items:
>>
>> * Inconsistent MOR control variables exposed by OVMF, breaks Windows
>>   Device Guard
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1496170
>>
>> * VariableSmm MorLockInit(): create MORLock only if / after MOR exists
>>
>>   https://bugzilla.tianocore.org/show_bug.cgi?id=727
>>
>> Patches #1 through #3 are cleanups.
>>
>> Patch #4 is a small helper patch for patch #5.
>>
>> Patch #5 is the actual fix, following Jiewen's suggestions from the
>> edk2-devel thread
>>
>> * [edk2] multiple levels of support for MOR / MORLock
>>
>>   https://lists.01.org/pipermail/edk2-devel/2017-September/015444.html
>>   https://lists.01.org/pipermail/edk2-devel/2017-October/015530.html
>>
>> Patch #6 is a workaround for some OSes (minimally Fedora 24-26, and some
>> Debian versions) that create the MOR variable even if the platform
>> doesn't offer it up-front. This patch also follows Jiewen's suggestion
>> from the same edk2-devel thread.
>>
>> (
>>
>> BTW, at Paolo's recommendation, I've now reported this kernel issue for
>> Fedora, under
>>
>> * incorrect downstream-only Platform Reset Attack Mitigation patch in
>>   the F24-F26 kernels
>>
>>   https://bugzilla.redhat.com/show_bug.cgi?id=1498159
>>
>> )
>>
>> I've checked this set for basic regressions, using OVMF, normal boot and
>> S3 suspend/resume:
>>
>> * Q35, SMM, IA32:
>>   - Fedora 25 -- verified patch #6 specifically
>>
>> * i440fx, no SMM, X64:
>>   - Fedora 24
>>
>> * Q35, SMM, IA32X64:
>>   - Fedora 26 -- verified patch #6 specifically
>>   - Windows 7
>>   - Windows 8.1
>>   - Windows 10
>>   - Windows Server 2008 R2
>>   - Windows Server 2012 R2
>>
>> I didn't / couldn't test this set in the following two environments:
>>
>> - on platforms where TcgMor.inf is included in the firmware, and the MOR
>>   variable exists genuinely,
>>
>> - in the nested virt setup where Ladi reported the Device Guard
>>   breakage. (If I understand correctly, ATM this requires additional
>>   host kernel (KVM) patches.)
>>
>> Test results / feedback from those envs would be appreciated.
>>
>> Cc: Eric Dong 
>> Cc: Jiewen Yao 
>> Cc: Ladi Prosek 
>> Cc: Star Zeng 
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (6):
>>   MdeModulePkg/Variable/RuntimeDxe: move SecureBootHook() decl to new
>> header
>>   MdeModulePkg/Variable/RuntimeDxe: move MOR func. declarations to
>> header
>>   MdeModulePkg/Variable/RuntimeDxe: introduce MorLockInitAtEndOfDxe()
>> hook
>>   MdeModulePkg/Variable/RuntimeDxe: permit MorLock deletion for passthru
>> req
>>   MdeModulePkg/Variable/RuntimeDxe: delay MorLock creation until
>> EndOfDxe
>>   MdeModulePkg/Variable/RuntimeDxe: delete and lock OS-created MOR
>> variable
>>
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/Measurement.c
>> |   2 +
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/PrivilegePolymorphic.h|
>> 89 ++
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c
>> |  45 +++--
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c
>> | 173 ++--
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c|
>> 51 --
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.h|
>> 2 +
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c |
>> 2 +
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf|
>> 1 +
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
>> |   2 +
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
>> |   4 +
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c
>> |  16 +-
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
>> |   1 +
>>  1

Re: [edk2] [PATCH edk2-platforms] Platform/Hisilicon: fix D02 driver indentation errors

2017-10-04 Thread Leif Lindholm

Note to self: --subject-prefix="PATCH edk2-platforms" goes on git
format-patch command line, not send-email.

Grabbing some coffee.

/
Leif

On Wed, Oct 04, 2017 at 09:12:33AM +0100, Leif Lindholm wrote:
> When building with a somewhat recent toolchain (GCC 6.3), the D02
> platform fails due to (the implicit) -Werror=misleading-indentation.
> 
> Cc: Heyi Guo 
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm 
> ---
>  Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c |  4 ++--
>  Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c | 10 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c 
> b/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
> index d876565a7d..b18b56ddb2 100644
> --- a/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
> +++ b/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
> @@ -497,8 +497,8 @@ STATIC VOID hisi_sas_v1_init(struct hisi_hba *hba, 
> PLATFORM_SAS_PROTOCOL *plat)
>  !(dma_rx_status & DMA_RX_STATUS_BUSY))
>  break;
>  
> -// Wait for status change in polling
> -NanoSecondDelay (100);
> +  // Wait for status change in polling
> +  NanoSecondDelay (100);
>  }
>}
>  
> diff --git a/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c 
> b/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
> index 3581b41c90..3739a36e64 100644
> --- a/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
> +++ b/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
> @@ -570,7 +570,7 @@ EFI_STATUS AssertPciePcsReset(UINT32 HostBridgeNum,UINT32 
> Port)
>if (pcs_local_status_checked)
>  DEBUG((EFI_D_ERROR, "pcs local reset status read failed\n"));
>  
> -count = 0;
> +  count = 0;
>do {
>  MicroSecondDelay(1000);
>  count ++;
> @@ -583,7 +583,7 @@ EFI_STATUS AssertPciePcsReset(UINT32 HostBridgeNum,UINT32 
> Port)
>if (hilink_status_checked)
>  DEBUG((EFI_D_ERROR, "error:pcs assert reset failed\n"));
>  
> -return EFI_SUCCESS;
> +  return EFI_SUCCESS;
>  }
>  
>  EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, UINT32 Port)
> @@ -616,7 +616,7 @@ EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, 
> UINT32 Port)
>if (pcs_local_status_checked)
>  DEBUG((EFI_D_ERROR, "pcs deassert reset failed!\n"));
>  
> -count = 0;
> +  count = 0;
>do {
>  MicroSecondDelay(1000);
>  RegRead(pcie_subctrl_base[HostBridgeNum] + 
> PCIE_SUBCTRL_SC_PCIE_HILINK_PCS_RESET_ST_REG, hilink_reset_status);
> @@ -627,7 +627,7 @@ EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, 
> UINT32 Port)
>if (hilink_status_checked)
>  DEBUG((EFI_D_ERROR, "pcs deassert reset failed!\n"));
>  
> -return EFI_SUCCESS;
> +  return EFI_SUCCESS;
>  }
>  
>  VOID PcieGen3Config(UINT32 HostBridgeNum, UINT32 Port)
> @@ -777,7 +777,7 @@ EFI_STATUS HisiPcieClockCtrl(UINT32 soctype, UINT32 
> HostBridgeNum, UINT32 Port,
>if (clock_status_checked)
>  DEBUG((EFI_D_ERROR, "clock operation failed!\n"));
>  
> -return EFI_SUCCESS;
> +  return EFI_SUCCESS;
>  }
>  
>  VOID PcieSpdSet(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port, UINT8 Spd)
> -- 
> 2.11.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] Platform/Hisilicon: fix D02 driver indentation errors

2017-10-04 Thread Ard Biesheuvel

On 4 October 2017 at 09:12, Leif Lindholm  wrote:
> When building with a somewhat recent toolchain (GCC 6.3), the D02
> platform fails due to (the implicit) -Werror=misleading-indentation.
>
> Cc: Heyi Guo 
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Leif Lindholm 

Reviewed-by: Ard Biesheuvel 

> ---
>  Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c |  4 ++--
>  Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c | 10 +-
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c 
> b/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
> index d876565a7d..b18b56ddb2 100644
> --- a/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
> +++ b/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
> @@ -497,8 +497,8 @@ STATIC VOID hisi_sas_v1_init(struct hisi_hba *hba, 
> PLATFORM_SAS_PROTOCOL *plat)
>  !(dma_rx_status & DMA_RX_STATUS_BUSY))
>  break;
>
> -// Wait for status change in polling
> -NanoSecondDelay (100);
> +  // Wait for status change in polling
> +  NanoSecondDelay (100);
>  }
>}
>
> diff --git a/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c 
> b/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
> index 3581b41c90..3739a36e64 100644
> --- a/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
> +++ b/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
> @@ -570,7 +570,7 @@ EFI_STATUS AssertPciePcsReset(UINT32 HostBridgeNum,UINT32 
> Port)
>if (pcs_local_status_checked)
>  DEBUG((EFI_D_ERROR, "pcs local reset status read failed\n"));
>
> -count = 0;
> +  count = 0;
>do {
>  MicroSecondDelay(1000);
>  count ++;
> @@ -583,7 +583,7 @@ EFI_STATUS AssertPciePcsReset(UINT32 HostBridgeNum,UINT32 
> Port)
>if (hilink_status_checked)
>  DEBUG((EFI_D_ERROR, "error:pcs assert reset failed\n"));
>
> -return EFI_SUCCESS;
> +  return EFI_SUCCESS;
>  }
>
>  EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, UINT32 Port)
> @@ -616,7 +616,7 @@ EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, 
> UINT32 Port)
>if (pcs_local_status_checked)
>  DEBUG((EFI_D_ERROR, "pcs deassert reset failed!\n"));
>
> -count = 0;
> +  count = 0;
>do {
>  MicroSecondDelay(1000);
>  RegRead(pcie_subctrl_base[HostBridgeNum] + 
> PCIE_SUBCTRL_SC_PCIE_HILINK_PCS_RESET_ST_REG, hilink_reset_status);
> @@ -627,7 +627,7 @@ EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, 
> UINT32 Port)
>if (hilink_status_checked)
>  DEBUG((EFI_D_ERROR, "pcs deassert reset failed!\n"));
>
> -return EFI_SUCCESS;
> +  return EFI_SUCCESS;
>  }
>
>  VOID PcieGen3Config(UINT32 HostBridgeNum, UINT32 Port)
> @@ -777,7 +777,7 @@ EFI_STATUS HisiPcieClockCtrl(UINT32 soctype, UINT32 
> HostBridgeNum, UINT32 Port,
>if (clock_status_checked)
>  DEBUG((EFI_D_ERROR, "clock operation failed!\n"));
>
> -return EFI_SUCCESS;
> +  return EFI_SUCCESS;
>  }
>
>  VOID PcieSpdSet(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port, UINT8 Spd)
> --
> 2.11.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] Platform/Hisilicon: fix D02 driver indentation errors

2017-10-04 Thread Leif Lindholm

When building with a somewhat recent toolchain (GCC 6.3), the D02
platform fails due to (the implicit) -Werror=misleading-indentation.

Cc: Heyi Guo 

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Leif Lindholm 
---
 Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c |  4 ++--
 Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c | 10 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c 
b/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
index d876565a7d..b18b56ddb2 100644
--- a/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
+++ b/Silicon/Hisilicon/Drivers/SasV1Dxe/SasV1Dxe.c
@@ -497,8 +497,8 @@ STATIC VOID hisi_sas_v1_init(struct hisi_hba *hba, 
PLATFORM_SAS_PROTOCOL *plat)
 !(dma_rx_status & DMA_RX_STATUS_BUSY))
 break;
 
-// Wait for status change in polling
-NanoSecondDelay (100);
+  // Wait for status change in polling
+  NanoSecondDelay (100);
 }
   }
 
diff --git a/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c 
b/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
index 3581b41c90..3739a36e64 100644
--- a/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
+++ b/Silicon/Hisilicon/Pv660/Drivers/PcieInitDxe/PcieInitLib.c
@@ -570,7 +570,7 @@ EFI_STATUS AssertPciePcsReset(UINT32 HostBridgeNum,UINT32 
Port)
   if (pcs_local_status_checked)
 DEBUG((EFI_D_ERROR, "pcs local reset status read failed\n"));
 
-count = 0;
+  count = 0;
   do {
 MicroSecondDelay(1000);
 count ++;
@@ -583,7 +583,7 @@ EFI_STATUS AssertPciePcsReset(UINT32 HostBridgeNum,UINT32 
Port)
   if (hilink_status_checked)
 DEBUG((EFI_D_ERROR, "error:pcs assert reset failed\n"));
 
-return EFI_SUCCESS;
+  return EFI_SUCCESS;
 }
 
 EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, UINT32 Port)
@@ -616,7 +616,7 @@ EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, 
UINT32 Port)
   if (pcs_local_status_checked)
 DEBUG((EFI_D_ERROR, "pcs deassert reset failed!\n"));
 
-count = 0;
+  count = 0;
   do {
 MicroSecondDelay(1000);
 RegRead(pcie_subctrl_base[HostBridgeNum] + 
PCIE_SUBCTRL_SC_PCIE_HILINK_PCS_RESET_ST_REG, hilink_reset_status);
@@ -627,7 +627,7 @@ EFI_STATUS DeassertPciePcsReset(UINT32 HostBridgeNum, 
UINT32 Port)
   if (hilink_status_checked)
 DEBUG((EFI_D_ERROR, "pcs deassert reset failed!\n"));
 
-return EFI_SUCCESS;
+  return EFI_SUCCESS;
 }
 
 VOID PcieGen3Config(UINT32 HostBridgeNum, UINT32 Port)
@@ -777,7 +777,7 @@ EFI_STATUS HisiPcieClockCtrl(UINT32 soctype, UINT32 
HostBridgeNum, UINT32 Port,
   if (clock_status_checked)
 DEBUG((EFI_D_ERROR, "clock operation failed!\n"));
 
-return EFI_SUCCESS;
+  return EFI_SUCCESS;
 }
 
 VOID PcieSpdSet(UINT32 soctype, UINT32 HostBridgeNum, UINT32 Port, UINT8 Spd)
-- 
2.11.0

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

39 matches

Mail list logo