date:20180528

[edk2] [Patch] MdeModulePkg BootManagerMenuApp: Update usage info for BootLogo protocol

2018-05-28 Thread Liming Gao

BootLogo protocol is not always required. If it is installed,
BootManagerMenuApp can work.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Ruiyu Ni 
---
 MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf 
b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
index dd60ef4..392a970 100644
--- a/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
+++ b/MdeModulePkg/Application/BootManagerMenuApp/BootManagerMenuApp.inf
@@ -4,7 +4,7 @@
 #  The application pops up a menu showing all the boot options referenced by
 #  BootOrder NV variable and user can choose to boot from one of them.
 #  
-#  Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -51,7 +51,7 @@
 [Guids]
 
 [Protocols]
-  gEfiBootLogoProtocolGuid  ## CONSUMES
+  gEfiBootLogoProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiLoadedImageDevicePathProtocolGuid ## CONSUMES
 
 [Pcd]
-- 
2.8.0.windows.1

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

Re: [edk2] [PATCH] OvmfPkg: raise DXEFV size to 11 MB

2018-05-28 Thread Gao, Liming

Laszlo:
  OvmfPkgIa32.fdf, OvmfPkgX64.fdf and OvmfPkgIa32X64.fdf are almost same. I 
suggest to use the single FDF for them. If so, this change is only made once. 

  For X64 only module in FDF, we can use below style to include it. 

!if $(E1000_ENABLE) && "X64" in $(ARCH)
  FILE DRIVER = 5D695E11-9B3F-4b83-B25F-4A8D5D69BE07 {
SECTION PE32 = Intel3.5/EFIX64/E3522X2.EFI
  }
!endif

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Tuesday, May 29, 2018 2:50 AM
>To: edk2-devel-01 
>Cc: Justen, Jordan L ; Gary Lin ;
>Ard Biesheuvel 
>Subject: [edk2] [PATCH] OvmfPkg: raise DXEFV size to 11 MB
>
>Almost exactly two years after commit 2f7b34b20842f, we've grown out the
>10MB DXEFV:
>
>> build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -b NOOPT -t GCC48 \
>>   -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D TLS_ENABLE -D
>E1000_ENABLE \
>>   -D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE
>>
>> [...]
>>
>> GenFv: ERROR 3000: Invalid
>>   the required fv image size 0xa28d48 exceeds the set fv image size
>>   0xa0
>
>Raise the DXEFV size to 11MB.
>
>(For builds that don't need this DXEFV bump, I've checked the
>FVMAIN_COMPACT increase stemming from the additional 1MB padding,
>using
>NOOPT + GCC48 + FD_SIZE_2MB, and no other "-D" flags. In the IA32 build,
>FVMAIN_COMPACT grows by 232 bytes. In the IA32X64 build,
>FVMAIN_COMPACT
>shrinks by 64 bytes. In the X64 build, FVMAIN_COMPACT shrinks by 376
>bytes.)
>
>Cc: Ard Biesheuvel 
>Cc: Gary Lin 
>Cc: Jordan Justen 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Laszlo Ersek 
>---
>
>Notes:
>- repo & branch: https://github.com/lersek/edk2.git ; dxefv_11mb
>
>- regression-tested with the "crash" tool for vmcore analysis
>
>- regression-tested using S3 suspend/resume with my usual guests,
>  including Linux, Windows, i440fx, q35, SMM etc
>
> OvmfPkg/OvmfPkgIa32.fdf| 6 +++---
> OvmfPkg/OvmfPkgIa32X64.fdf | 6 +++---
> OvmfPkg/OvmfPkgX64.fdf | 6 +++---
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
>diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
>index 0427ded49239..b199713925fe 100644
>--- a/OvmfPkg/OvmfPkgIa32.fdf
>+++ b/OvmfPkg/OvmfPkgIa32.fdf
>@@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>
> [FD.MEMFD]
> BaseAddress   = $(MEMFD_BASE_ADDRESS)
>-Size  = 0xB0
>+Size  = 0xC0
> ErasePolarity = 1
> BlockSize = 0x1
>-NumBlocks = 0xB0
>+NumBlocks = 0xC0
>
> 0x00|0x006000
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgT
>okenSpaceGuid.PcdOvmfSecPageTablesSize
>@@ -89,7 +89,7 @@ [FD.MEMFD]
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgToke
>nSpaceGuid.PcdOvmfPeiMemFvSize
> FV = PEIFV
>
>-0x10|0xA0
>+0x10|0xB0
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTok
>enSpaceGuid.PcdOvmfDxeMemFvSize
> FV = DXEFV
>
>diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
>index 6df47f48cd2c..4ebf64b2b9dc 100644
>--- a/OvmfPkg/OvmfPkgIa32X64.fdf
>+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
>@@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>
> [FD.MEMFD]
> BaseAddress   = $(MEMFD_BASE_ADDRESS)
>-Size  = 0xB0
>+Size  = 0xC0
> ErasePolarity = 1
> BlockSize = 0x1
>-NumBlocks = 0xB0
>+NumBlocks = 0xC0
>
> 0x00|0x006000
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgT
>okenSpaceGuid.PcdOvmfSecPageTablesSize
>@@ -89,7 +89,7 @@ [FD.MEMFD]
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgToke
>nSpaceGuid.PcdOvmfPeiMemFvSize
> FV = PEIFV
>
>-0x10|0xA0
>+0x10|0xB0
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTok
>enSpaceGuid.PcdOvmfDxeMemFvSize
> FV = DXEFV
>
>diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
>index 2e2a1749b5d2..9ca96f928287 100644
>--- a/OvmfPkg/OvmfPkgX64.fdf
>+++ b/OvmfPkg/OvmfPkgX64.fdf
>@@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>
> [FD.MEMFD]
> BaseAddress   = $(MEMFD_BASE_ADDRESS)
>-Size  = 0xB0
>+Size  = 0xC0
> ErasePolarity = 1
> BlockSize = 0x1
>-NumBlocks = 0xB0
>+NumBlocks = 0xC0
>
> 0x00|0x006000
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgT
>okenSpaceGuid.PcdOvmfSecPageTablesSize
>@@ -89,7 +89,7 @@ [FD.MEMFD]
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgToke
>nSpaceGuid.PcdOvmfPeiMemFvSize
> FV = PEIFV
>
>-0x10|0xA0
>+0x10|0xB0
>
>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTok
>enSpaceGuid.PcdOvmfDxeMemFvSize
> FV = DXEFV
>
>--
>2.14.1.3.gb7cf6e02401b
>
>___
>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] smm lock query

2018-05-28 Thread Abhishek Singh

Thanks, Jiewen and Star. I was able to figure out that smrr was preventing
me from accessing the contents of smram. It seems to me that this violates
the EFI_MM_ACCESS_PROTOCOL  protocol. I have talked to the edk2-platform
maintainers about this on a private thread, but, of course, I may be
mistaken.

I agree that smm is a runtime environment, but you must also agree that
there is an initialization or loading phase in which all the smi handlers
are loaded into smram. Theoretically, new handlers may keep getting
installed as smi's are received but I doubt that this is the case. There
must be a point at which the code (not necessarily the data) is supposed to
be fixed in smram. My guess is that that point is after the SMI handlers
have responded to the SmmReadyToLock event, but I would like to know if you
disagree.

I am definitely not seeking to add the smram dumping code as a production
feature. I am merely interested in using it for my research.

I may have to resort to writing an smm driver, as you suggest Jiewen, but
currently I am just trying to dump smram contents from smm ipl, having
disabled smrr in SmmCpuFeaturesLib. The problem, in both cases, is that the
smram range is quite large (around 8 MB) and cannot fit in a single UEFI
variable. Do you have any suggestions on how to actually dump out this
large range?

Thanks again,
Abhishek


On Mon, May 28, 2018 at 10:21 PM, Yao, Jiewen  wrote:

> Let me share my thought.
>
> 1)  From interface point of view, ReadyToLock means it is the last
> time to lock. But it does not mean it must be open before.
>
> As implementation choice, a platform MAY lock it earlier.
>
>
>
> Also SMRR may force the SMRAM invisible to outside SMRAM, even with D_OPEN
> set.
>
>
>
> 2)  Dumping SMRAM exposes the secret inside of SMRAM, I would treat
> it as debug feature only, not a production.
>
>
>
> If you want to debug, you can add a debug SMM driver and expose an
> interface to copy SMRAM content out.
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
> *From:* Zeng, Star
> *Sent:* Monday, May 28, 2018 6:16 PM
> *To:* Abhishek Singh ; Marvin H?user <
> marvin.haeu...@outlook.com>
> *Cc:* edk2-devel@lists.01.org; Laszlo Ersek ;
> af...@apple.com; Ni, Ruiyu ; Dong, Eric <
> eric.d...@intel.com>; Zeng, Star ; Yao, Jiewen <
> jiewen@intel.com>
> *Subject:* RE: [edk2] smm lock query
>
>
>
> I do not see issue according to the spec.
>
> Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
>
> DxeSmmReadyToLock event is to notify DXE handlers.
>
> Modules are responsible to lock or protect their resource and effect the
> appropriate protections in their notification handlers.
>
> SmmIplGuidedEventNotify is used to inform SMM environment to signal
> SmmReadyToLock.
>
> SmmReadyToLock event is to notify SMM handlers.
>
>
>
> DXE handlers could not touch SMRAM (after SMRAM is locked or even after
> SMRR is configured in PiSmmCpuDxeSmm if I know it is correct).
>
>
>
> “This protocol in tandem with the *End of DXE Even*t facilitates
> transition of the platform from the environment where all of the components
> are under the authority of the platform manufacturer to the environment
> where third party extensible modules such as UEFI drivers and UEFI
> applications are executed.
>
> The protocol is published immediately after signaling of the *End of DXE
> Event*.
>
> PI modules that need to lock or protect their resources in anticipation of
> the invocation of 3rd party extensible modules should register for
> notification on installation of this protocol and effect the appropriate
> protections in their notification handlers. For example, PI platform code
> may choose to use notification handler to lock MM by invoking 
> *EFI_MM_ACCESS_PROTOCOL.Lock()
> *function.”
>
>
>
>
>
> SMM environment is a *runtime* environment. SMRAM will be even changed
> after SmmReadyToLock, for example, by SMM handler for SMM communication
> from DXE.
>
>
>
>
>
> Thanks,
>
> Star
>
> *From:* Abhishek Singh [mailto:a...@cs.unc.edu ]
> *Sent:* Tuesday, May 29, 2018 2:02 AM
> *To:* Marvin Häuser 
> *Cc:* edk2-devel@lists.01.org; Laszlo Ersek ;
> af...@apple.com; Ni, Ruiyu ; Dong, Eric <
> eric.d...@intel.com>; Zeng, Star 
> *Subject:* Re: [edk2] smm lock query
>
>
>
> Thank you everyone for your inputs and clarifications. They are helping me
> to better understand the uefi code, to which I am very new. I do not mean
> to hijack the thread: so please continue your discussions about whether the
> implementation matches the spec.
>
>
>
> However, I want to state why I am interested in the IPL code. For my
> research, I wish to dump the contents of SMRAM when it has reached steady
> state, i.e., all the drivers have made changes to smram and it has been
> locked. The current implementation (smm ipl) locks smram when it receives
> the SmmReadyToLock event and then propagates the event to the smm drivers
> that make further changes to smram. Unfortunately, I cannot take a snapshot
> of smram af

[edk2] [Patch] OvmfPkg BasePciCapLib: Fix VS build failure

2018-05-28 Thread Liming Gao

Fix VS warning C4244: 'function': conversion from 'UINT32' to 'UINT16',
possible loss of data.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Laszlo Ersek 
---
 OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c 
b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
index c059264..54e7409 100644
--- a/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
+++ b/OvmfPkg/Library/BasePciCapLib/BasePciCapLib.c
@@ -613,8 +613,8 @@ PciCapListInit (
   }
 
   Status = InsertPciCap (OutCapList, CapHdrOffsets, PciCapExtended,
- ExtendedCapHdr.CapabilityId, ExtendedCapHdrOffset,
- ExtendedCapHdr.CapabilityVersion);
+ (UINT16) ExtendedCapHdr.CapabilityId, ExtendedCapHdrOffset,
+ (UINT8) ExtendedCapHdr.CapabilityVersion);
   if (RETURN_ERROR (Status)) {
 goto FreeCapHdrOffsets;
   }
-- 
2.8.0.windows.1

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

Re: [edk2] smm lock query

2018-05-28 Thread Yao, Jiewen

Let me share my thought.

1)  From interface point of view, ReadyToLock means it is the last time to 
lock. But it does not mean it must be open before.
As implementation choice, a platform MAY lock it earlier.

Also SMRR may force the SMRAM invisible to outside SMRAM, even with D_OPEN set.


2)  Dumping SMRAM exposes the secret inside of SMRAM, I would treat it as 
debug feature only, not a production.

If you want to debug, you can add a debug SMM driver and expose an interface to 
copy SMRAM content out.

Thank you
Yao Jiewen

From: Zeng, Star
Sent: Monday, May 28, 2018 6:16 PM
To: Abhishek Singh ; Marvin H?user 
Cc: edk2-devel@lists.01.org; Laszlo Ersek ; af...@apple.com; 
Ni, Ruiyu ; Dong, Eric ; Zeng, Star 
; Yao, Jiewen 
Subject: RE: [edk2] smm lock query

I do not see issue according to the spec.
Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
DxeSmmReadyToLock event is to notify DXE handlers.
Modules are responsible to lock or protect their resource and effect the 
appropriate protections in their notification handlers.
SmmIplGuidedEventNotify is used to inform SMM environment to signal 
SmmReadyToLock.
SmmReadyToLock event is to notify SMM handlers.

DXE handlers could not touch SMRAM (after SMRAM is locked or even after SMRR is 
configured in PiSmmCpuDxeSmm if I know it is correct).

“This protocol in tandem with the End of DXE Event facilitates transition of 
the platform from the environment where all of the components are under the 
authority of the platform manufacturer to the environment where third party 
extensible modules such as UEFI drivers and UEFI applications are executed.

The protocol is published immediately after signaling of the End of DXE Event.
PI modules that need to lock or protect their resources in anticipation of the 
invocation of 3rd party extensible modules should register for notification on 
installation of this protocol and effect the appropriate protections in their 
notification handlers. For example, PI platform code may choose to use 
notification handler to lock MM by invoking EFI_MM_ACCESS_PROTOCOL.Lock() 
function.”


SMM environment is a *runtime* environment. SMRAM will be even changed after 
SmmReadyToLock, for example, by SMM handler for SMM communication from DXE.


Thanks,
Star
From: Abhishek Singh [mailto:a...@cs.unc.edu]
Sent: Tuesday, May 29, 2018 2:02 AM
To: Marvin Häuser 
mailto:marvin.haeu...@outlook.com>>
Cc: edk2-devel@lists.01.org; Laszlo Ersek 
mailto:ler...@redhat.com>>; 
af...@apple.com; Ni, Ruiyu 
mailto:ruiyu...@intel.com>>; Dong, Eric 
mailto:eric.d...@intel.com>>; Zeng, Star 
mailto:star.z...@intel.com>>
Subject: Re: [edk2] smm lock query

Thank you everyone for your inputs and clarifications. They are helping me to 
better understand the uefi code, to which I am very new. I do not mean to 
hijack the thread: so please continue your discussions about whether the 
implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my research, 
I wish to dump the contents of SMRAM when it has reached steady state, i.e., 
all the drivers have made changes to smram and it has been locked. The current 
implementation (smm ipl) locks smram when it receives the SmmReadyToLock event 
and then propagates the event to the smm drivers that make further changes to 
smram. Unfortunately, I cannot take a snapshot of smram after it has been 
locked! Thus, I have solved this issue by propagating the event to the smm 
drivers first (using SmmIplGuidedEventNotify), then opening access to and 
dumping the contents of SMRAM, and finally closing access to and locking smram. 
Would it be fair to say that this would give me the fully initialized contents 
of smram?



My second observation is that despite opening access to smram, I am unable to 
access its contents, which is a violation of the EFI_MM_ACCESS_PROTOCOL.Open() 
description in the spec, which says: "This function “opens” MMRAM so that it is 
visible while not inside of MM." Note that I am working with minnowboard 
firmware release 0.97. So some of the binaries like SmmAccess.efi are provided 
by Intel, while others have been built from the edk source tree: thus, this may 
not be an EDK issue. Please suggest further steps and/or workarounds. Should I 
contact edk2-platforms maintainers, or start a new thread here for this issue?

-Abhishek


On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser 
mailto:marvin.haeu...@outlook.com>> wrote:
Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock 
event and therefor is aware.
However, they might rely on the protocol's description that the resources are 
about(!) to be locked and code accordingly, not considering the event 
characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against 
edk2's actions, or add

Re: [edk2] [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

2018-05-28 Thread Zeng, Star

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Monday, May 28, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel 
Subject: [edk2] [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: 
use AllocatePeiAccessiblePages

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to AllocatePeiAccessiblePages, 
which boils down to the same on X64, but does not crash non-X64 systems that 
lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
   | 51 
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 |  1 +
 2 files changed, 10 insertions(+), 42 deletions(-)

diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
index e719e9e482cb..ded817f37301 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firmwa
+++ rePerformanceDxe.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include  @@ -179,46 +180,6 @@ 
FpdtAcpiTableChecksum (
   Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);  }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN   Size
-  )
-{
-  UINTN Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUSStatus;
-  VOID  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0x;
-
-  Status = gBS->AllocatePages (
-  AllocateMaxAddress,
-  EfiReservedMemoryType,
-  Pages,
-  &Address
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-Buffer = (VOID *) (UINTN) Address;
-ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Callback function upon VariableArchProtocol and LockBoxProtocol
   to allocate S3 performance table memory and save the pointer to LockBox.
@@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
 //
 // Fail to allocate at specified address, continue to allocate at any 
address.
 //
-mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
+mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+ 
EfiReservedMemoryType,
+ EFI_SIZE_TO_PAGES 
(sizeof (S3_PERFORMANCE_TABLE))
+ );
   }
   DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 0x%x\n", 
mAcpiS3PerformanceTable));
   if (mAcpiS3PerformanceTable != NULL) { @@ -368,7 +332,10 @@ 
InstallFirmwarePerformanceDataTable (
   //
   // Fail to allocate at specified address, continue to allocate at any 
address.
   //
-  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+   
EfiReservedMemoryType,
+   
EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
+   );
 }
 DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 0x%x\n", 
mAcpiBootPerformanceTable));
 if (mAcpiBootPerformanceTable == NULL) { diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
index 8757bbd0aaa9..3d2dd6eb732f 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
+++ b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/Firmwa
+++ rePerformanceDxe.inf
@@ -44,6 +44,7 @@ [LibraryClasses]
   UefiRuntimeServicesTableLib
   BaseLib
   DebugLib
+  DxeServicesLib
   TimerLib
   BaseMemoryLib
   MemoryAllocationLib
--
2.17.0

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

Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

2018-05-28 Thread Zeng, Star

I think " be accessible by PEI after a warm reboot " should be " be accessible 
by PEI after resuming from S3 ".
You can update it when pushing without need to send a new patch if other has no 
comment to the code part.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Monday, May 28, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel 
Subject: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce 
AllocatePeiAccessiblePages routine

Add a routine to DxeServicesLib that abstracts the allocation of memory
that should be accessible by PEI after a warm reboot. We will use it to
replace open coded implementations that limit the address to < 4 GB,
which may not be possible on non-Intel systems that have no 32-bit
addressable memory at all.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Include/Library/DxeServicesLib.h  | 23 ++-
 MdePkg/Library/DxeServicesLib/Allocate.c | 54 +++
 MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
 MdePkg/Library/DxeServicesLib/X64/Allocate.c | 69 
 4 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/DxeServicesLib.h 
b/MdePkg/Include/Library/DxeServicesLib.h
index 7c1c62236d96..20aee68af558 100644
--- a/MdePkg/Include/Library/DxeServicesLib.h
+++ b/MdePkg/Include/Library/DxeServicesLib.h
@@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
   OUT   EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
   );
 
-#endif
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
 
+  @param[in]  MemoryTypeThe memory type to allocate
+  @param[in]  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTNPages
+  );
+
+#endif
diff --git a/MdePkg/Library/DxeServicesLib/Allocate.c 
b/MdePkg/Library/DxeServicesLib/Allocate.c
new file mode 100644
index ..4d118f766d49
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/Allocate.c
@@ -0,0 +1,54 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, 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 which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
+
+  @param[in]  MemoryTypeThe memory type to allocate
+  @param[in]  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTNPages
+  )
+{
+  EFI_STATUS  Status;
+  EFI_PHYSICAL_ADDRESSMemory;
+
+  if (Pages == 0) {
+return NULL;
+  }
+
+  Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf 
b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
index bd2faf2f6f2d..50ae24f8ee22 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
@@ -27,12 +27,18 @@ [Defines]
   LIBRARY_CLASS  = DxeServicesLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
UEFI_DRIVER
 
 #
-#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
+#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC ARM AARCH64
 #
 
 [Sources]
   DxeServicesLib.c
 
+[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
+  Allocate.c
+
+[Sources.X64]
+  X64/Allocate.c
+
 [Packages]
   MdePkg/MdePkg.dec
 
@@ -44,6 +50,

Re: [edk2] [Patch] SecurityPkg/Tcg2Smm: Correct function parameter attribute

2018-05-28 Thread Long, Qin

Reviewed-by: Long Qin 


Best Regards & Thanks,
LONG, Qin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zhang, Chao B
> Sent: Monday, May 28, 2018 10:10 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Long, Qin 
> Subject: [edk2] [Patch] SecurityPkg/Tcg2Smm: Correct function parameter
> attribute
> 
> Correct UpdatePossibleResource parameter attribute to align to comment
> 
> Change-Id: Id8f8be975f0e8666573decc3fbaaf326b7767ba8
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Long Qin 
> Cc: Yao Jiewen 
> Reviewed-by: Chao Zhang 
> Signed-off-by: Zhang, Chao B 
> ---
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index 3e0a68999a..f0c92462cf 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -315,14 +315,14 @@ UpdatePPVersion (
>@return  patch status.
> 
>  **/
>  EFI_STATUS
>  UpdatePossibleResource (
> -  IN  EFI_ACPI_DESCRIPTION_HEADER*Table,
> -  IN  UINT32 *IrqBuffer,
> -  IN  UINT32 IrqBuffserSize,
> -  OUT BOOLEAN*IsShortFormPkgLength
> +  IN OUT  EFI_ACPI_DESCRIPTION_HEADER*Table,
> +  IN  UINT32 *IrqBuffer,
> +  IN  UINT32 IrqBuffserSize,
> +  OUT BOOLEAN*IsShortFormPkgLength
>)
>  {
>UINT8   *DataPtr;
>UINT8   *DataEndPtr;
>UINT32  NewPkgLength;
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] smm lock query

2018-05-28 Thread Zeng, Star

I do not see issue according to the spec.
Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
DxeSmmReadyToLock event is to notify DXE handlers.
Modules are responsible to lock or protect their resource and effect the 
appropriate protections in their notification handlers.
SmmIplGuidedEventNotify is used to inform SMM environment to signal 
SmmReadyToLock.
SmmReadyToLock event is to notify SMM handlers.

DXE handlers could not touch SMRAM (after SMRAM is locked or even after SMRR is 
configured in PiSmmCpuDxeSmm if I know it is correct).

“This protocol in tandem with the End of DXE Event facilitates transition of 
the platform from the environment where all of the components are under the 
authority of the platform manufacturer to the environment where third party 
extensible modules such as UEFI drivers and UEFI applications are executed.

The protocol is published immediately after signaling of the End of DXE Event.
PI modules that need to lock or protect their resources in anticipation of the 
invocation of 3rd party extensible modules should register for notification on 
installation of this protocol and effect the appropriate protections in their 
notification handlers. For example, PI platform code may choose to use 
notification handler to lock MM by invoking EFI_MM_ACCESS_PROTOCOL.Lock() 
function.”

SMM environment is a *runtime* environment. SMRAM will be even changed after 
SmmReadyToLock, for example, by SMM handler for SMM communication from DXE.

Thanks,
Star
From: Abhishek Singh [mailto:a...@cs.unc.edu]
Sent: Tuesday, May 29, 2018 2:02 AM
To: Marvin Häuser 
Cc: edk2-devel@lists.01.org; Laszlo Ersek ; af...@apple.com; 
Ni, Ruiyu ; Dong, Eric ; Zeng, Star 

Subject: Re: [edk2] smm lock query

Thank you everyone for your inputs and clarifications. They are helping me to 
better understand the uefi code, to which I am very new. I do not mean to 
hijack the thread: so please continue your discussions about whether the 
implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my research, 
I wish to dump the contents of SMRAM when it has reached steady state, i.e., 
all the drivers have made changes to smram and it has been locked. The current 
implementation (smm ipl) locks smram when it receives the SmmReadyToLock event 
and then propagates the event to the smm drivers that make further changes to 
smram. Unfortunately, I cannot take a snapshot of smram after it has been 
locked! Thus, I have solved this issue by propagating the event to the smm 
drivers first (using SmmIplGuidedEventNotify), then opening access to and 
dumping the contents of SMRAM, and finally closing access to and locking smram. 
Would it be fair to say that this would give me the fully initialized contents 
of smram?

My second observation is that despite opening access to smram, I am unable to 
access its contents, which is a violation of the EFI_MM_ACCESS_PROTOCOL.Open() 
description in the spec, which says: "This function “opens” MMRAM so that it is 
visible while not inside of MM." Note that I am working with minnowboard 
firmware release 0.97. So some of the binaries like SmmAccess.efi are provided 
by Intel, while others have been built from the edk source tree: thus, this may 
not be an EDK issue. Please suggest further steps and/or workarounds. Should I 
contact edk2-platforms maintainers, or start a new thread here for this issue?

-Abhishek

On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser 
mailto:marvin.haeu...@outlook.com>> wrote:
Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock 
event and therefor is aware.
However, they might rely on the protocol's description that the resources are 
about(!) to be locked and code accordingly, not considering the event 
characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against 
edk2's actions, or additional code might be supplied by third parties that do 
not have tree code access (which, by integration, would be "platform binaries" 
by the definition applying here).

Therefor I would still ask everyone to consider figuring out a solution to this 
discrepancy from the specification, such as the internal "dummy event" I 
proposed.

Thanks,
Marvin.

> -Original Message-
> From: Laszlo Ersek mailto:ler...@redhat.com>>
> Sent: Monday, May 28, 2018 12:58 PM
> To: Andrew Fish mailto:af...@apple.com>>; Marvin H?user
> mailto:marvin.haeu...@outlook.com>>
> Cc: edk2-devel@lists.01.org; Abhishek Singh 
> mailto:a...@cs.unc.edu>>;
> ruiyu...@intel.com; 
> eric.d...@intel.com; 
> star.z...@intel.com
> Subject: Re: [edk2] smm lock query
>
> On 05/27/18 22:44, Andrew Fish wrote:
> >
> >
> >> On May 27, 2018, at 9:47 AM, Marvin H?user
> mailto:marvin.haeu...@ou

Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

2018-05-28 Thread Gao, Liming

The patch is good to me.

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Monday, May 28, 2018 11:00 PM
>To: Ard Biesheuvel ; edk2-devel@lists.01.org
>Subject: Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce
>AllocatePeiAccessiblePages routine
>
>On 05/28/18 16:40, Ard Biesheuvel wrote:
>> Add a routine to DxeServicesLib that abstracts the allocation of memory
>> that should be accessible by PEI after a warm reboot. We will use it to
>> replace open coded implementations that limit the address to < 4 GB,
>> which may not be possible on non-Intel systems that have no 32-bit
>> addressable memory at all.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  MdePkg/Include/Library/DxeServicesLib.h  | 23 ++-
>>  MdePkg/Library/DxeServicesLib/Allocate.c | 54 +++
>>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
>>  MdePkg/Library/DxeServicesLib/X64/Allocate.c | 69
>
>>  4 files changed, 155 insertions(+), 2 deletions(-)
>
>Reviewed-by: Laszlo Ersek 
>
>Thanks,
>Laszlo
>
>>
>> diff --git a/MdePkg/Include/Library/DxeServicesLib.h
>b/MdePkg/Include/Library/DxeServicesLib.h
>> index 7c1c62236d96..20aee68af558 100644
>> --- a/MdePkg/Include/Library/DxeServicesLib.h
>> +++ b/MdePkg/Include/Library/DxeServicesLib.h
>> @@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
>>OUT   EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
>>);
>>
>> -#endif
>> +/**
>> +  Allocates one or more 4KB pages of a given type from a memory region
>that is
>> +  accessible to PEI.
>> +
>> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
>> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
>> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
>> +  memory remaining to satisfy the request, then NULL is returned.
>>
>> +  @param[in]  MemoryTypeThe memory type to allocate
>> +  @param[in]  Pages The number of 4 KB pages to allocate.
>> +
>> +  @return A pointer to the allocated buffer or NULL if allocation fails.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +AllocatePeiAccessiblePages (
>> +  IN EFI_MEMORY_TYPE  MemoryType,
>> +  IN UINTNPages
>> +  );
>> +
>> +#endif
>> diff --git a/MdePkg/Library/DxeServicesLib/Allocate.c
>b/MdePkg/Library/DxeServicesLib/Allocate.c
>> new file mode 100644
>> index ..4d118f766d49
>> --- /dev/null
>> +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
>> @@ -0,0 +1,54 @@
>> +/** @file
>> +  DxeServicesLib memory allocation routines
>> +
>> +  Copyright (c) 2018, 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 which accompanies
>this
>> +  distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php.
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include 
>> +#include 
>> +#include 
>> +
>> +/**
>> +  Allocates one or more 4KB pages of a given type from a memory region
>that is
>> +  accessible to PEI.
>> +
>> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
>> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
>> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
>> +  memory remaining to satisfy the request, then NULL is returned.
>> +
>> +  @param[in]  MemoryTypeThe memory type to allocate
>> +  @param[in]  Pages The number of 4 KB pages to allocate.
>> +
>> +  @return A pointer to the allocated buffer or NULL if allocation fails.
>> +
>> +**/
>> +VOID *
>> +EFIAPI
>> +AllocatePeiAccessiblePages (
>> +  IN EFI_MEMORY_TYPE  MemoryType,
>> +  IN UINTNPages
>> +  )
>> +{
>> +  EFI_STATUS  Status;
>> +  EFI_PHYSICAL_ADDRESSMemory;
>> +
>> +  if (Pages == 0) {
>> +return NULL;
>> +  }
>> +
>> +  Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages,
>&Memory);
>> +  if (EFI_ERROR (Status)) {
>> +return NULL;
>> +  }
>> +  return (VOID *)(UINTN)Memory;
>> +}
>> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> index bd2faf2f6f2d..50ae24f8ee22 100644
>> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>> @@ -27,12 +27,18 @@ [Defines]
>>LIBRARY_CLASS  = DxeServicesLib|DXE_CORE DXE_DRIVER
>DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE
>UEFI_APPLICATION UEFI_DRIVER
>>
>>  #
>> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
>> +#  VALI

Re: [edk2] [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

2018-05-28 Thread Zeng, Star

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Monday, May 28, 2018 10:40 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel 
Subject: [edk2] [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use 
AllocatePeiAccessiblePages

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to AllocatePeiAccessiblePages, 
which boils down to the same on X64, but does not crash non-X64 systems that 
lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 48 
+++-
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 71d624fc9ce9..68b29ac5a9e2 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -165,46 +165,6 @@ IsKnownID (
   }
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN   Size
-  )
-{
-  UINTN Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUSStatus;
-  VOID  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0x;
-
-  Status = gBS->AllocatePages (
-  AllocateMaxAddress,
-  EfiReservedMemoryType,
-  Pages,
-  &Address
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-Buffer = (VOID *) (UINTN) Address;
-ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Allocate buffer for Boot Performance table.
 
@@ -348,7 +308,13 @@ AllocateBootPerformanceTable (
 //
 // Fail to allocate at specified address, continue to allocate at any 
address.
 //
-mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+ 
EfiReservedMemoryType,
+ EFI_SIZE_TO_PAGES 
(BootPerformanceDataSize)
+ );
+if (mAcpiBootPerformanceTable != NULL) {
+  ZeroMem (mAcpiBootPerformanceTable, BootPerformanceDataSize);
+}
   }
   DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table 
address = 0x%x\n", mAcpiBootPerformanceTable));
 
--
2.17.0

___
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] CryptoPkg: Remove deprecated function usage in X509GetCommonName()

2018-05-28 Thread Ye, Ting

Reviewed-by: Ye Ting  

-Original Message-
From: Long, Qin 
Sent: Thursday, May 24, 2018 4:11 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; michael.tur...@microsoft.com
Subject: [PATCH] CryptoPkg: Remove deprecated function usage in 
X509GetCommonName()

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

X509_NAME_get_text_by_NID() used in X509GetCommonName() implementation is one 
legacy function which have various limitations. The returned data may be not 
usable  when the target cert contains multicharacter string type like a 
BMPString or a UTF8String.
This patch replaced the legacy function usage with more general
X509_NAME_get_index_by_NID() / X509_NAME_get_entry() APIs for X509 CommonName 
retrieving.

Tests: Validated the commonName retrieving with test certificates
   containing PrintableString or BMPString data.

Cc: Ye Ting 
Cc: Michael Turner 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Long Qin 
---
 CryptoPkg/Include/Library/BaseCryptLib.h  |  4 +-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c | 53 ++-
 CryptoPkg/Library/BaseCryptLib/Pk/CryptX509Null.c |  4 +-
 3 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/CryptoPkg/Include/Library/BaseCryptLib.h 
b/CryptoPkg/Include/Library/BaseCryptLib.h
index 027ea09feb..dc6aaf0635 100644
--- a/CryptoPkg/Include/Library/BaseCryptLib.h
+++ b/CryptoPkg/Include/Library/BaseCryptLib.h
@@ -4,7 +4,7 @@
   primitives (Hash Serials, HMAC, RSA, Diffie-Hellman, etc) for UEFI security
   functionality enabling.
 
-Copyright (c) 2009 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -2177,7 +2177,7 
@@ X509GetSubjectName (
   @param[in]  Cert Pointer to the DER-encoded X509 certificate.
   @param[in]  CertSize Size of the X509 certificate in bytes.
   @param[out] CommonName   Buffer to contain the retrieved certificate 
common
-   name string. At most CommonNameSize bytes 
will be
+   name string (UTF8). At most 
+ CommonNameSize bytes will be
written and the string will be null 
terminated. May be
NULL in order to determine the size buffer 
needed.
   @param[in,out]  CommonNameSize   The size in bytes of the CommonName buffer 
on input,
diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c 
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
index 56e66308ae..c137df357f 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c
@@ -1,7 +1,7 @@
 /** @file
   X.509 Certificate Handler Wrapper Implementation over OpenSSL.
 
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -303,7 +303,7 @@ 
_Exit:
   @param[in]  Cert Pointer to the DER-encoded X509 certificate.
   @param[in]  CertSize Size of the X509 certificate in bytes.
   @param[out] CommonName   Buffer to contain the retrieved certificate 
common
-   name string. At most CommonNameSize bytes 
will be
+   name string (UTF8). At most 
+ CommonNameSize bytes will be
written and the string will be null 
terminated. May be
NULL in order to determine the size buffer 
needed.
   @param[in,out]  CommonNameSize   The size in bytes of the CommonName buffer 
on input,
@@ -332,13 +332,18 @@ X509GetCommonName (
   IN OUT  UINTN*CommonNameSize
   )
 {
-  RETURN_STATUS  ReturnStatus;
-  BOOLEANStatus;
-  X509   *X509Cert;
-  X509_NAME  *X509Name;
-  INTN   Length;
+  RETURN_STATUSReturnStatus;
+  BOOLEAN  Status;
+  X509 *X509Cert;
+  X509_NAME*X509Name;
+  INT32Index;
+  INTN Length;
+  X509_NAME_ENTRY  *Entry;
+  ASN1_STRING  *EntryData;
+  UINT8*UTF8Name;
 
   ReturnStatus = RETURN_INVALID_PARAMETER;
+  UTF8Name = NULL;
 
   //
   // Check input parameters.
@@ -378,8 +383,8 @@ X509GetCommonName (
   //
   // Retrieve the CommonName information from X.509 Subject
   //
-  Length = (INTN) X509_NAME_get_text_by_NID (X509Name, NID_commonName, 
CommonName, (int)(*CommonNameSize));
-  if (Length < 0) {
+  Index = X509_NAME_get_index_by_NID (X509Name, NID_commonNam

Re: [edk2] [Patch 0/3] Use comparison logic to check UINTN parameter in GetBestLanguage API

2018-05-28 Thread Gao, Liming

Marvin:
  Current solution is to keep max compatibility. 

Thanks
Liming
>-Original Message-
>From: Marvin H?user [mailto:marvin.haeu...@outlook.com]
>Sent: Monday, May 28, 2018 4:11 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star ; Gao, Liming 
>Subject: RE: [edk2] [Patch 0/3] Use comparison logic to check UINTN
>parameter in GetBestLanguage API
>
>Hey Star and Liming,
>
>May I propose re-considering the introduction of a third named parameter to
>reflect the first Language passed?
>This would 1) have the advantage that the BOOLEAN parameter can remain as
>is, which increases readability and
>2) require at least two parameters related to the language list passed. Having
>to write "NULL, NULL" is way more obvious than just having to write "NULL"
>when (accidentally?) not passing any language.
>
>And error caused by this change would be calls that do not pass an expected
>amount of parameters for the call to make sense.
>
>Thanks,
>Marvin.
>
>> -Original Message-
>> From: edk2-devel  On Behalf Of Zeng,
>> Star
>> Sent: Monday, May 28, 2018 9:54 AM
>> To: Gao, Liming ; edk2-devel@lists.01.org
>> Cc: Zeng, Star 
>> Subject: Re: [edk2] [Patch 0/3] Use comparison logic to check UINTN
>> parameter in GetBestLanguage API
>>
>> Reviewed-by: Star Zeng 
>>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Liming Gao
>> Sent: Monday, May 28, 2018 3:31 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [Patch 0/3] Use comparison logic to check UINTN parameter
>> in GetBestLanguage API
>>
>> Liming Gao (3):
>>   MdePkg UefiLib: Use comparison logic to check UINTN parameter
>>   IntelFrameworkPkg UefiLib: Use comparison logic to check UINTN
>> parameter
>>   MdeModulePkg Variable: Use comparison logic to check UINTN parameter
>>
>>  IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c| 6 +++---
>>  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 8
>> 
>>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 8 
>>  MdePkg/Library/UefiLib/UefiLib.c| 6 +++---
>>  4 files changed, 14 insertions(+), 14 deletions(-)
>>
>> --
>> 2.8.0.windows.1
>>
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/5] Remove X86 ASM and S files

2018-05-28 Thread Gao, Liming

Mike:
  This patch serials remove ASM from libraries, and keep S files in Libraries 
until new NASM is updated. I understand only S file optimization feature 
depends on NASM file. Right? 

  Yes. I will update the patch to update copyright to 2018. 

Thanks
Liming
>-Original Message-
>From: Kinney, Michael D
>Sent: Tuesday, May 29, 2018 8:42 AM
>To: Gao, Liming ; edk2-devel@lists.01.org; Kinney,
>Michael D 
>Subject: RE: [edk2] [Patch 0/5] Remove X86 ASM and S files
>
>Liming,
>
>This patch series includes removal of ASM and S files
>from libraries.  Should those be removes from this series
>until NASM is updated?
>
>I also see updates to INF files without updates to the
>Copyright.
>
>Thanks,
>
>Mike
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-
>> boun...@lists.01.org] On Behalf Of Liming Gao
>> Sent: Sunday, May 13, 2018 7:32 AM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [Patch 0/5] Remove X86 ASM and S files
>>
>> For IA32 and X64, NASM has replaced ASM and S files.
>> Rmove ASM from all modules.
>> Remove S files from the drivers only.
>> After NASM is updated, S files can be removed from
>> Library.
>>
>> Liming Gao (5):
>>   IntelFrameworkModulePkg: Remove X86 ASM and S files
>>   MdeModulePkg: Remove X86 ASM and S files
>>   MdePkg: Remove X86 ASM and S files
>>   SourceLevelDebugPkg: Remove X86 ASM and S files
>>   UefiCpuPkg: Remove X86 ASM and S files
>>
>>  .../Csm/LegacyBiosDxe/IA32/InterruptTable.S|
>> 67 ---
>>  .../Csm/LegacyBiosDxe/IA32/InterruptTable.asm  |
>> 73 ---
>>  .../Csm/LegacyBiosDxe/LegacyBiosDxe.inf|
>> 4 -
>>  .../Csm/LegacyBiosDxe/X64/InterruptTable.S |
>> 72 ---
>>  .../Csm/LegacyBiosDxe/X64/InterruptTable.asm   |
>> 71 ---
>>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf|
>> 2 -
>>  MdeModulePkg/Core/DxeIplPeim/Ia32/IdtVectorAsm.S   |
>> 80 ---
>>  MdeModulePkg/Core/DxeIplPeim/Ia32/IdtVectorAsm.asm |
>> 88 ---
>>  .../BootScriptExecutorDxe.inf  |
>> 4 -
>>  .../Acpi/BootScriptExecutorDxe/IA32/S3Asm.S|
>> 66 ---
>>  .../Acpi/BootScriptExecutorDxe/IA32/S3Asm.asm  |
>> 71 ---
>>  .../Acpi/BootScriptExecutorDxe/X64/S3Asm.S |
>> 130 -
>>  .../Acpi/BootScriptExecutorDxe/X64/S3Asm.asm   |
>> 135 -
>>  MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf   |
>> 2 -
>>  .../Universal/CapsulePei/X64/PageFaultHandler.S|
>> 81 ---
>>  .../Universal/CapsulePei/X64/PageFaultHandler.asm  |
>> 87 ---
>>  .../Universal/DebugSupportDxe/DebugSupportDxe.inf  |
>> 4 -
>>  .../Universal/DebugSupportDxe/Ia32/AsmFuncs.S  |
>> 407 --
>>  .../Universal/DebugSupportDxe/Ia32/AsmFuncs.asm|
>> 509 --
>>  .../Universal/DebugSupportDxe/X64/AsmFuncs.S   |
>> 551 ---
>>  .../Universal/DebugSupportDxe/X64/AsmFuncs.asm |
>> 596 -
>>  MdeModulePkg/Universal/EbcDxe/EbcDebugger.inf  |
>> 4 -
>>  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf   |
>> 4 -
>>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcLowLevel.S   |
>> 83 ---
>>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcLowLevel.asm |
>> 207 ---
>>  MdeModulePkg/Universal/EbcDxe/X64/EbcLowLevel.S|
>> 147 -
>>  MdeModulePkg/Universal/EbcDxe/X64/EbcLowLevel.asm  |
>> 246 -
>>  MdePkg/Library/BaseCpuLib/BaseCpuLib.inf   |
>> 4 -
>>  MdePkg/Library/BaseCpuLib/Ia32/CpuFlushTlb.asm |
>> 40 --
>>  MdePkg/Library/BaseCpuLib/Ia32/CpuSleep.asm|
>> 39 --
>>  MdePkg/Library/BaseCpuLib/X64/CpuFlushTlb.asm  |
>> 38 --
>>  MdePkg/Library/BaseCpuLib/X64/CpuSleep.asm |
>> 37 --
>>  .../BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf  |
>> 2 -
>>  MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm  |
>> 141 -
>>  MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm   |
>> 127 -
>>  MdePkg/Library/BaseLib/BaseLib.inf |
>> 246 -
>>  MdePkg/Library/BaseLib/Ia32/ARShiftU64.asm |
>> 48 --
>>  MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm  |
>> 40 --
>>  MdePkg/Library/BaseLib/Ia32/CpuId.asm  |
>> 66 ---
>>  MdePkg/Library/BaseLib/Ia32/CpuIdEx.asm|
>> 68 ---
>>  MdePkg/Library/BaseLib/Ia32/CpuPause.asm   |
>> 40 --
>>  MdePkg/Library/BaseLib/Ia32/DisableCache.asm   |
>> 45 --
>>  MdePkg/Library/BaseLib/Ia32/DisableInterrupts.asm  |
>> 40 --
>>  MdePkg/Library/BaseLib/Ia32/DisablePaging32.asm|
>> 57 --
>>  MdePkg/Library/BaseLib/Ia32/DivU64x32.asm  |
>> 46 --
>>  MdePkg/Library/BaseLib/Ia32/DivU64x32Remainder.asm |
>> 51 --
>>  MdePkg/Library/BaseLib/Ia32/DivU64x64Remainder.asm |
>> 92 
>>  MdePkg/Library/BaseLib/Ia32/EnableCache.asm|
>> 45 --
>>  .../BaseLib/Ia32/EnableDisableInterrupts.asm   |
>> 41 --
>>  MdePkg/Library/BaseLib/Ia32/EnableInterrupts.asm   |
>> 40 --
>>  MdePkg/Library/BaseLib/Ia32/EnablePaging32.asm |
>> 57 --
>>  MdePkg/Library/BaseLib/Ia32/EnablePaging64.asm |
>> 68 ---
>>  MdePkg/Library/BaseLib/Ia

Re: [edk2] [PATCH] OvmfPkg: raise DXEFV size to 11 MB

2018-05-28 Thread Gary Lin

On Mon, May 28, 2018 at 08:49:56PM +0200, Laszlo Ersek wrote:
> Almost exactly two years after commit 2f7b34b20842f, we've grown out the
> 10MB DXEFV:
> 
> > build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -b NOOPT -t GCC48 \
> >   -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D TLS_ENABLE -D E1000_ENABLE \
> >   -D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE
> >
> > [...]
> >
> > GenFv: ERROR 3000: Invalid
> >   the required fv image size 0xa28d48 exceeds the set fv image size
> >   0xa0
> 
> Raise the DXEFV size to 11MB.
> 
> (For builds that don't need this DXEFV bump, I've checked the
> FVMAIN_COMPACT increase stemming from the additional 1MB padding, using
> NOOPT + GCC48 + FD_SIZE_2MB, and no other "-D" flags. In the IA32 build,
> FVMAIN_COMPACT grows by 232 bytes. In the IA32X64 build, FVMAIN_COMPACT
> shrinks by 64 bytes. In the X64 build, FVMAIN_COMPACT shrinks by 376
> bytes.)
> 
> Cc: Ard Biesheuvel 
> Cc: Gary Lin 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
> 
Reviewed-by: Gary Lin 

> Notes:
> - repo & branch: https://github.com/lersek/edk2.git ; dxefv_11mb
> 
> - regression-tested with the "crash" tool for vmcore analysis
> 
> - regression-tested using S3 suspend/resume with my usual guests,
>   including Linux, Windows, i440fx, q35, SMM etc
> 
>  OvmfPkg/OvmfPkgIa32.fdf| 6 +++---
>  OvmfPkg/OvmfPkgIa32X64.fdf | 6 +++---
>  OvmfPkg/OvmfPkgX64.fdf | 6 +++---
>  3 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 0427ded49239..b199713925fe 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>  
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size  = 0xB0
> +Size  = 0xC0
>  ErasePolarity = 1
>  BlockSize = 0x1
> -NumBlocks = 0xB0
> +NumBlocks = 0xC0
>  
>  0x00|0x006000
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +89,7 @@ [FD.MEMFD]
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>  
> -0x10|0xA0
> +0x10|0xB0
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>  
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 6df47f48cd2c..4ebf64b2b9dc 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>  
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size  = 0xB0
> +Size  = 0xC0
>  ErasePolarity = 1
>  BlockSize = 0x1
> -NumBlocks = 0xB0
> +NumBlocks = 0xC0
>  
>  0x00|0x006000
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +89,7 @@ [FD.MEMFD]
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>  
> -0x10|0xA0
> +0x10|0xB0
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>  
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 2e2a1749b5d2..9ca96f928287 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>  
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size  = 0xB0
> +Size  = 0xC0
>  ErasePolarity = 1
>  BlockSize = 0x1
> -NumBlocks = 0xB0
> +NumBlocks = 0xC0
>  
>  0x00|0x006000
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +89,7 @@ [FD.MEMFD]
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>  
> -0x10|0xA0
> +0x10|0xB0
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>  
> -- 
> 2.14.1.3.gb7cf6e02401b
> 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/5] Remove X86 ASM and S files

2018-05-28 Thread Kinney, Michael D

Liming,

This patch series includes removal of ASM and S files
from libraries.  Should those be removes from this series
until NASM is updated?

I also see updates to INF files without updates to the
Copyright.

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Liming Gao
> Sent: Sunday, May 13, 2018 7:32 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/5] Remove X86 ASM and S files
> 
> For IA32 and X64, NASM has replaced ASM and S files.
> Rmove ASM from all modules.
> Remove S files from the drivers only.
> After NASM is updated, S files can be removed from
> Library.
> 
> Liming Gao (5):
>   IntelFrameworkModulePkg: Remove X86 ASM and S files
>   MdeModulePkg: Remove X86 ASM and S files
>   MdePkg: Remove X86 ASM and S files
>   SourceLevelDebugPkg: Remove X86 ASM and S files
>   UefiCpuPkg: Remove X86 ASM and S files
> 
>  .../Csm/LegacyBiosDxe/IA32/InterruptTable.S|
> 67 ---
>  .../Csm/LegacyBiosDxe/IA32/InterruptTable.asm  |
> 73 ---
>  .../Csm/LegacyBiosDxe/LegacyBiosDxe.inf|
> 4 -
>  .../Csm/LegacyBiosDxe/X64/InterruptTable.S |
> 72 ---
>  .../Csm/LegacyBiosDxe/X64/InterruptTable.asm   |
> 71 ---
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf|
> 2 -
>  MdeModulePkg/Core/DxeIplPeim/Ia32/IdtVectorAsm.S   |
> 80 ---
>  MdeModulePkg/Core/DxeIplPeim/Ia32/IdtVectorAsm.asm |
> 88 ---
>  .../BootScriptExecutorDxe.inf  |
> 4 -
>  .../Acpi/BootScriptExecutorDxe/IA32/S3Asm.S|
> 66 ---
>  .../Acpi/BootScriptExecutorDxe/IA32/S3Asm.asm  |
> 71 ---
>  .../Acpi/BootScriptExecutorDxe/X64/S3Asm.S |
> 130 -
>  .../Acpi/BootScriptExecutorDxe/X64/S3Asm.asm   |
> 135 -
>  MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf   |
> 2 -
>  .../Universal/CapsulePei/X64/PageFaultHandler.S|
> 81 ---
>  .../Universal/CapsulePei/X64/PageFaultHandler.asm  |
> 87 ---
>  .../Universal/DebugSupportDxe/DebugSupportDxe.inf  |
> 4 -
>  .../Universal/DebugSupportDxe/Ia32/AsmFuncs.S  |
> 407 --
>  .../Universal/DebugSupportDxe/Ia32/AsmFuncs.asm|
> 509 --
>  .../Universal/DebugSupportDxe/X64/AsmFuncs.S   |
> 551 ---
>  .../Universal/DebugSupportDxe/X64/AsmFuncs.asm |
> 596 -
>  MdeModulePkg/Universal/EbcDxe/EbcDebugger.inf  |
> 4 -
>  MdeModulePkg/Universal/EbcDxe/EbcDxe.inf   |
> 4 -
>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcLowLevel.S   |
> 83 ---
>  MdeModulePkg/Universal/EbcDxe/Ia32/EbcLowLevel.asm |
> 207 ---
>  MdeModulePkg/Universal/EbcDxe/X64/EbcLowLevel.S|
> 147 -
>  MdeModulePkg/Universal/EbcDxe/X64/EbcLowLevel.asm  |
> 246 -
>  MdePkg/Library/BaseCpuLib/BaseCpuLib.inf   |
> 4 -
>  MdePkg/Library/BaseCpuLib/Ia32/CpuFlushTlb.asm |
> 40 --
>  MdePkg/Library/BaseCpuLib/Ia32/CpuSleep.asm|
> 39 --
>  MdePkg/Library/BaseCpuLib/X64/CpuFlushTlb.asm  |
> 38 --
>  MdePkg/Library/BaseCpuLib/X64/CpuSleep.asm |
> 37 --
>  .../BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf  |
> 2 -
>  MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifo.asm  |
> 141 -
>  MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifo.asm   |
> 127 -
>  MdePkg/Library/BaseLib/BaseLib.inf |
> 246 -
>  MdePkg/Library/BaseLib/Ia32/ARShiftU64.asm |
> 48 --
>  MdePkg/Library/BaseLib/Ia32/CpuBreakpoint.asm  |
> 40 --
>  MdePkg/Library/BaseLib/Ia32/CpuId.asm  |
> 66 ---
>  MdePkg/Library/BaseLib/Ia32/CpuIdEx.asm|
> 68 ---
>  MdePkg/Library/BaseLib/Ia32/CpuPause.asm   |
> 40 --
>  MdePkg/Library/BaseLib/Ia32/DisableCache.asm   |
> 45 --
>  MdePkg/Library/BaseLib/Ia32/DisableInterrupts.asm  |
> 40 --
>  MdePkg/Library/BaseLib/Ia32/DisablePaging32.asm|
> 57 --
>  MdePkg/Library/BaseLib/Ia32/DivU64x32.asm  |
> 46 --
>  MdePkg/Library/BaseLib/Ia32/DivU64x32Remainder.asm |
> 51 --
>  MdePkg/Library/BaseLib/Ia32/DivU64x64Remainder.asm |
> 92 
>  MdePkg/Library/BaseLib/Ia32/EnableCache.asm|
> 45 --
>  .../BaseLib/Ia32/EnableDisableInterrupts.asm   |
> 41 --
>  MdePkg/Library/BaseLib/Ia32/EnableInterrupts.asm   |
> 40 --
>  MdePkg/Library/BaseLib/Ia32/EnablePaging32.asm |
> 57 --
>  MdePkg/Library/BaseLib/Ia32/EnablePaging64.asm |
> 68 ---
>  MdePkg/Library/BaseLib/Ia32/FlushCacheLine.asm |
> 55 --
>  MdePkg/Library/BaseLib/Ia32/FxRestore.asm  |
> 42 --
>  MdePkg/Library/BaseLib/Ia32/FxSave.asm |
> 42 --
>  MdePkg/Library/BaseLib/Ia32/Invd.asm   |
> 40 --
>  MdePkg/Library/BaseLib/Ia32/LRotU64.asm|
> 51 --
>  MdePkg/Library/BaseLib/Ia32/LShiftU64.asm  |
> 48 --
>  MdePkg/Library/BaseLib/Ia32/LongJump.asm   |
> 46 --
>  MdePkg/Library/BaseLib/Ia32/ModU64x32.asm  |
> 45 --
>  MdePkg/Library/BaseLib/Ia32/Monitor.asm|
> 45 --
>  MdePkg/Library/BaseLib/Ia32/MultU64x32.asm |
> 43 --
>  Md

Re: [edk2] PciSegmentInfoLib instances

2018-05-28 Thread Bill Paul

Of all the gin joints in all the towns in all the world, Ni, Ruiyu had to walk 
into mine at 19:55 on Sunday 27 May 2018 and say:

> No. There is no such instance.
> 
> My understanding:
> Segment is just to separate the PCI devices to different groups.
> Each group of devices use the continuous BUS/IO/MMIO resource.
> Each group has a BASE PCIE address that can be used to access PCIE
> configuration in MMIO way.

This makes it sound like an either/or design choice that a hardware designer 
can make simply for some kind of convenience, and I don't think that's the 
case.

Segments typically indicate completely separate host/PCI interfaces. For 
example, I've seen older Intel boards with both 32-bit/33MHz slots and 64-
bit/66MHz slots. This was not done with a bridge: each set of slots was tied 
to a completely separate host PCI bridge and hence each was a separate 
segment. This was required in order to support legacy 32-bit/33MHz devices 
without forcing the 64-bit/66MHz devices down to 33MHz as well.

With PCIe, on platforms other than Intel, each root complex would also be a 
separate segment. Each root complex would have its own bus/dev/func namespace, 
its own configuration space access method, and its own portion of the physical 
address space into which to map BARs. This means that you could have two or 
more different devices with the same bus/dev/func identifier tuple, meaning 
they are not unique on a platform-wide basis. 

At the hardware level, PCIe may be implemented similarly on Intel too, but 
they hide some of the details from you. The major difference is that even in 
cases where you may have multiple PCIe channels, they all share the same 
bus/dev/func namespace so that you can pretend the bus/dev/func tuples are 
unique platform-wide. The case where you would need to advertise multiple 
segments arises where there's some technical roadblock that prevents 
implementing this illusion of a single namespace in a completely transparent 
way.

In the case of the 32-bit/64-bit hybrid design I mentioned above, scanning the 
bus starting from bus0/dev0/func0 would only allow you to automatically 
discover the 32-bit devices because there was no bridge between the 32-bit and 
64-bit spaces. The hardware allows you to issue configuration accesses to both 
spaces using the same 0xcf8/0xcfc registers, but in order to autodiscover the 
64-bit devices, you needed know ahead of time to also scan starting at 
bus1/dev0/func0. But the only way to know to do that was to check the 
advertised segments in the ACPI device table and honor their starting bus 
numbers.
 
> So with the above understanding, even a platform which has single segment
> can be implemented as a multiple segments platform.

I would speculate this might only be true on Intel. :) Intel is the only 
platform that creates the illusion of a single bus/dev/func namespace for 
multiple PCI "hoses," and it only does that for backward compatibility 
purposes (i.e. to make Windows happy). Without that gimmick, each segment 
would be a separate tree rooted at bus0/dev0/func0, and there wouldn't be much 
point to doing that if you only had a single root complex.

-Bill
 
> Thanks/Ray
> 
> > -Original Message-
> > From: edk2-devel  On Behalf Of Laszlo
> > Ersek
> > Sent: Wednesday, May 23, 2018 3:38 PM
> > To: Ni, Ruiyu 
> > Cc: edk2-devel-01 
> > Subject: [edk2] PciSegmentInfoLib instances
> > 
> > Hi Ray,
> > 
> > do you know of any open source, non-Null, PciSegmentInfoLib instance?
> > (Possibly outside of edk2?)
> > 
> > More precisely, it's not the PciSegmentInfoLib instance itself that's of
> > particular interest, but the hardware and the platform support code that
> > offer multiple PCIe segments.
> > 
> > Thanks
> > Laszlo
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
-- 
=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River Systems
=
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [edk2-platforms Patch v4 4/6] Styx/PlatformFlashAccessLib: Add progress API

2018-05-28 Thread Michael D Kinney

From: "Kinney, Michael D" 

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

Add PerformFlashWriteWithProgress() to the PlatformFlashAccessLib.
This allows the platform to inform the user of progress when a
firmware storage device is being updated with a new firmware
image.

This is the minimal update to this library implementation to
keep everything building and preserve any existing progress
indication.  Additional updates are required to use the
Progress() API passed into PerformFlashWriteWithProgress().

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 .../StyxPlatformFlashAccessLib.c   | 70 +++---
 1 file changed, 63 insertions(+), 7 deletions(-)

diff --git 
a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
 
b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
index a94373bb4b..38f1830b5c 100644
--- 
a/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
+++ 
b/Silicon/AMD/Styx/Library/StyxPlatformFlashAccessLib/StyxPlatformFlashAccessLib.c
@@ -2,6 +2,7 @@
   Platform flash device access library for AMD Styx
 
   Copyright (c) 2017, Linaro, Ltd. All rights reserved.
+  Copyright (c) 2018, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -27,13 +28,29 @@ STATIC CONST UINT64 mFlashMaxSize = FixedPcdGet64 
(PcdFdSize);
 STATIC CONST UINTN mBlockSize = SIZE_64KB;
 
 /**
-  Perform flash write operation.
+  Perform flash write operation with progress indicator.  The start and end
+  completion percentage values are passed into this function.  If the requested
+  flash write operation is broken up, then completion percentage between the
+  start and end values may be passed to the provided Progress function.  The
+  caller of this function is required to call the Progress function for the
+  start and end completion percentage values.  This allows the Progress,
+  StartPercentage, and EndPercentage parameters to be ignored if the requested
+  flash write operation can not be broken up
 
   @param[in] FirmwareType  The type of firmware.
   @param[in] FlashAddress  The address of flash device to be accessed.
   @param[in] FlashAddressType  The type of flash device address.
   @param[in] BufferThe pointer to the data buffer.
   @param[in] LengthThe length of data buffer in bytes.
+  @param[in] Progress  A function used report the progress of the
+   firmware update.  This is an optional parameter
+   that may be NULL.
+  @param[in] StartPercentage   The start completion percentage value that may
+   be used to report progress during the flash
+   write operation.
+  @param[in] EndPercentage The end completion percentage value that may
+   be used to report progress during the flash
+   write operation.
 
   @retval EFI_SUCCESS   The operation returns successfully.
   @retval EFI_WRITE_PROTECTED   The flash device is read only.
@@ -42,12 +59,15 @@ STATIC CONST UINTN mBlockSize = SIZE_64KB;
 **/
 EFI_STATUS
 EFIAPI
-PerformFlashWrite (
-  IN PLATFORM_FIRMWARE_TYPE   FirmwareType,
-  IN EFI_PHYSICAL_ADDRESS FlashAddress,
-  IN FLASH_ADDRESS_TYPE   FlashAddressType,
-  IN VOID *Buffer,
-  IN UINTNLength
+PerformFlashWriteWithProgress (
+  IN PLATFORM_FIRMWARE_TYPE FirmwareType,
+  IN EFI_PHYSICAL_ADDRESS   FlashAddress,
+  IN FLASH_ADDRESS_TYPE FlashAddressType,
+  IN VOID   *Buffer,
+  IN UINTN  Length,
+  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress,OPTIONAL
+  IN UINTN  StartPercentage,
+  IN UINTN  EndPercentage
   )
 {
   EFI_STATUS  Status;
@@ -122,3 +142,39 @@ PerformFlashWrite (
 
   return EFI_SUCCESS;
 }
+
+/**
+  Perform flash write operation.
+
+  @param[in] FirmwareType  The type of firmware.
+  @param[in] FlashAddress  The address of flash device to be accessed.
+  @param[in] FlashAddressType  The type of flash device address.
+  @param[in] BufferThe pointer to the data buffer.
+  @param[in] LengthThe length of data buffer in bytes.
+
+  @retval EFI_SUCCESS   The operation returns successfully.
+  @retval EFI_WRITE_PROTECTED   The flash device is read only.
+  @retval EFI_UNSUPPORTED   The flash device access is unsupported.
+  @retval EFI_INVALID_PARAMETER The input

[edk2] [edk2-platforms Patch v4 1/6] AMD/OverdriveBoard: Add DisplayUpdateProgressLib mapping

2018-05-28 Thread Michael D Kinney

From: "Kinney, Michael D" 

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

Based on content from the following branch/commits:
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc 
b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
index 5e564f66b8..aad5f472e4 100644
--- a/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
+++ b/Platform/AMD/OverdriveBoard/OverdriveBoard.dsc
@@ -125,6 +125,7 @@ [LibraryClasses.common]
   
RealTimeClockLib|Silicon/AMD/Styx/Library/RealTimeClockLib/RealTimeClockLib.inf
 
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+  
DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.inf
 !if $(DO_CAPSULE) == TRUE
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
-- 
2.14.2.windows.3

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

[edk2] [edk2-platforms Patch v4 0/6] Add DisplayUpdateProgressLib to platforms

2018-05-28 Thread Michael D Kinney

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

Based on content from:

https://github.com/Microsoft/MS_UEFI/blob/share/MsCapsuleSupport/MsCapsuleUpdatePkg/Include/Library/DisplayUpdateProgressLib.h
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsuleUpdatePkg/Library/DisplayUpdateProgressGraphicsLib
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsuleUpdatePkg/Library/DisplayUpdateProgressTextLib

Updates for V4
==
* Add following to commit messages based on feedback from:

  https://lists.01.org/pipermail/edk2-devel/2018-April/023737.html

This is the minimal update to this library implementation to
keep everything building and preserve any existing progress
indication.  Additional updates are required to use the
Progress() API passed into PerformFlashWriteWithProgress().


Updates for V3
==
* Add Version field to EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL
* Break up patch series into 4 smaller patch series to handle dependencies
  between the edk2 repository and the edk2-platforms repository.
  + Patch series for edk2 repo that adds DisplayUpdateProgressLib class and 
instances.  Defines the EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL.
Adds PerformFlashWriteWithProgress() API to the PlatformFlashAccessLib.
  + Patch series for platforms in edk2-platforms that use capsules to add the
DisplayUpdateProgressLib mapping to the DSC files and add the
PerformFlashWriteWithProgress() API implementation to the
PlatformFlashAccessLib implementations.
  + Patch series for platforms in edk2 that use capsules to add the
DisplayUpdateProgressLib mapping to the DSC files and add the
PerformFlashWriteWithProgress() API implementation to the
PlatformFlashAccessLib implementations.
  + Patch for edk2 that adds the use of the DisplayUpateProgressLib and the 
PerformFlashWriteWithProgress() API .

Updates for V2
==
* Change DisplayUpdateProgressGraphicsLib to DisplayUpdateProgressLibGraphics
* Change DisplayUpdateProgressTextLib to DisplayUpdateProgressLibText
* Clarify that color in Firmware Management Progress Protocol is the foreground 
color
* Add missing parameters to PerformFlashWriteWithProgress() function header.
* Update PerformFlashWriteWithProgress() function header describing the use of
  the start and end percentage values.
* Update QuarkPlatformPkg PerformFlashWriteWithProgress() to call Progress() for
  the end precentage.
* Update Vlv2Tbl2DevicePkg PerformFlashWriteWithProgress() to call Progress()
  for the end precentage.

Add DisplayUpdateProgressLib class along implementations for both graphical
(Graphics Output Protocol based) and text (Simple Text Output Protocol based)
consoles.  Also add the EDK II Firmware Management Progress Protocol that is an
optional protocol that provides the progress bar color and a watchdog timeout
value thaty can be used when a firmware image is updated in a firmware device.

* Add progress support to DxeCapsuleLibFmp
* Add progress support to SystemFirmwareUpdateDxe
* Add progress support to PlatformFlashAccessLib class and instances.
* Reduce Print() calls during a firmware update.

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1

Kinney, Michael D (6):
  AMD/OverdriveBoard: Add DisplayUpdateProgressLib mapping
  Socionext/DeveloperBox: Add DisplayUpdateProgressLib mapping
  Socionext/SynQuacerEvalBoard: Add DisplayUpdateProgressLib mapping
  Styx/PlatformFlashAccessLib: Add progress API
  Hisilicon/PlatformFlashAccessLib: Add progress API
  SynQuacer/PlatformFlashAccessLib: Add progress API

 Platform/AMD/OverdriveBoard/OverdriveBoard.dsc |  1 +
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc   |  1 +
 .../SynQuacerEvalBoard/SynQuacerEvalBoard.dsc  |  1 +
 .../StyxPlatformFlashAccessLib.c   | 70 +--
 .../PlatformFlashAccessLibDxe.c| 71 +---
 .../SynQuacerPlatformFlashAccessLib.c  | 78 +++---
 6 files changed, 196 insertions(+), 26 deletions(-)

-- 
2.14.2.windows.3

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

[edk2] [edk2-platforms Patch v4 5/6] Hisilicon/PlatformFlashAccessLib: Add progress API

2018-05-28 Thread Michael D Kinney

From: "Kinney, Michael D" 

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

Add PerformFlashWriteWithProgress() to the PlatformFlashAccessLib.
This allows the platform to inform the user of progress when a
firmware storage device is being updated with a new firmware
image.

This is the minimal update to this library implementation to
keep everything building and preserve any existing progress
indication.  Additional updates are required to use the
Progress() API passed into PerformFlashWriteWithProgress().

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 .../PlatformFlashAccessLibDxe.c| 71 +++---
 1 file changed, 63 insertions(+), 8 deletions(-)

diff --git 
a/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c 
b/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c
index 62da61c79b..585f7ef0e8 100644
--- 
a/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c
+++ 
b/Silicon/Hisilicon/Library/PlatformFlashAccessLib/PlatformFlashAccessLibDxe.c
@@ -3,7 +3,7 @@
 
   Copyright (c) 2018, Hisilicon Limited. All rights reserved.
   Copyright (c) 2018, Linaro Limited. All rights reserved.
-  Copyright (c) 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -30,13 +30,29 @@ STATIC EFI_PHYSICAL_ADDRESS mSFCMEM0BaseAddress;
 STATIC HISI_SPI_FLASH_PROTOCOL *mSpiProtocol;
 
 /**
-  Perform flash write opreation.
+  Perform flash write operation with progress indicator.  The start and end
+  completion percentage values are passed into this function.  If the requested
+  flash write operation is broken up, then completion percentage between the
+  start and end values may be passed to the provided Progress function.  The
+  caller of this function is required to call the Progress function for the
+  start and end completion percentage values.  This allows the Progress,
+  StartPercentage, and EndPercentage parameters to be ignored if the requested
+  flash write operation can not be broken up
 
   @param[in] FirmwareType  The type of firmware.
   @param[in] FlashAddress  The address of flash device to be accessed.
   @param[in] FlashAddressType  The type of flash device address.
   @param[in] BufferThe pointer to the data buffer.
   @param[in] LengthThe length of data buffer in bytes.
+  @param[in] Progress  A function used report the progress of the
+   firmware update.  This is an optional parameter
+   that may be NULL.
+  @param[in] StartPercentage   The start completion percentage value that may
+   be used to report progress during the flash
+   write operation.
+  @param[in] EndPercentage The end completion percentage value that may
+   be used to report progress during the flash
+   write operation.
 
   @retval EFI_SUCCESS   The operation returns successfully.
   @retval EFI_WRITE_PROTECTED   The flash device is read only.
@@ -45,12 +61,15 @@ STATIC HISI_SPI_FLASH_PROTOCOL *mSpiProtocol;
 **/
 EFI_STATUS
 EFIAPI
-PerformFlashWrite (
-  IN PLATFORM_FIRMWARE_TYPE   FirmwareType,
-  IN EFI_PHYSICAL_ADDRESS FlashAddress,
-  IN FLASH_ADDRESS_TYPE   FlashAddressType,
-  IN VOID *Buffer,
-  IN UINTNLength
+PerformFlashWriteWithProgress (
+  IN PLATFORM_FIRMWARE_TYPE FirmwareType,
+  IN EFI_PHYSICAL_ADDRESS   FlashAddress,
+  IN FLASH_ADDRESS_TYPE FlashAddressType,
+  IN VOID   *Buffer,
+  IN UINTN  Length,
+  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress,OPTIONAL
+  IN UINTN  StartPercentage,
+  IN UINTN  EndPercentage
   )
 {
   UINT32   RomAddress;
@@ -83,6 +102,42 @@ PerformFlashWrite (
   return Status;
 }
 
+/**
+  Perform flash write operation.
+
+  @param[in] FirmwareType  The type of firmware.
+  @param[in] FlashAddress  The address of flash device to be accessed.
+  @param[in] FlashAddressType  The type of flash device address.
+  @param[in] BufferThe pointer to the data buffer.
+  @param[in] LengthThe length of data buffer in bytes.
+
+  @retval EFI_SUCCESS   The operation returns successfully.
+  @retval EFI_WRITE_PROTECTED   The flash device is read only.
+  @retval EFI_UNSUPPORTED   The flash device acce

[edk2] [edk2-platforms Patch v4 6/6] SynQuacer/PlatformFlashAccessLib: Add progress API

2018-05-28 Thread Michael D Kinney

From: "Kinney, Michael D" 

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

Add PerformFlashWriteWithProgress() to the PlatformFlashAccessLib.
This allows the platform to inform the user of progress when a
firmware storage device is being updated with a new firmware
image.

This is the minimal update to this library implementation to
keep everything building and preserve any existing progress
indication.  Additional updates are required to use the
Progress() API passed into PerformFlashWriteWithProgress().

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 .../SynQuacerPlatformFlashAccessLib.c  | 78 +++---
 1 file changed, 67 insertions(+), 11 deletions(-)

diff --git 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
index fbb8f1f9e4..4cf8318a93 100644
--- 
a/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
+++ 
b/Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.c
@@ -2,6 +2,7 @@
   Platform flash device access library for Socionext SynQuacer
 
   Copyright (c) 2016, Linaro, Ltd. All rights reserved.
+  Copyright (c) 2018, Intel Corporation. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -117,13 +118,29 @@ GetFvbByAddress (
 }
 
 /**
-  Perform flash write operation.
+  Perform flash write operation with progress indicator.  The start and end
+  completion percentage values are passed into this function.  If the requested
+  flash write operation is broken up, then completion percentage between the
+  start and end values may be passed to the provided Progress function.  The
+  caller of this function is required to call the Progress function for the
+  start and end completion percentage values.  This allows the Progress,
+  StartPercentage, and EndPercentage parameters to be ignored if the requested
+  flash write operation can not be broken up
 
   @param[in] FirmwareType  The type of firmware.
   @param[in] FlashAddress  The address of flash device to be accessed.
   @param[in] FlashAddressType  The type of flash device address.
   @param[in] BufferThe pointer to the data buffer.
   @param[in] LengthThe length of data buffer in bytes.
+  @param[in] Progress  A function used report the progress of the
+   firmware update.  This is an optional parameter
+   that may be NULL.
+  @param[in] StartPercentage   The start completion percentage value that may
+   be used to report progress during the flash
+   write operation.
+  @param[in] EndPercentage The end completion percentage value that may
+   be used to report progress during the flash
+   write operation.
 
   @retval EFI_SUCCESS   The operation returns successfully.
   @retval EFI_WRITE_PROTECTED   The flash device is read only.
@@ -132,12 +149,15 @@ GetFvbByAddress (
 **/
 EFI_STATUS
 EFIAPI
-PerformFlashWrite (
-  IN PLATFORM_FIRMWARE_TYPE   FirmwareType,
-  IN EFI_PHYSICAL_ADDRESS FlashAddress,
-  IN FLASH_ADDRESS_TYPE   FlashAddressType,
-  IN VOID *Buffer,
-  IN UINTNLength
+PerformFlashWriteWithProgress (
+  IN PLATFORM_FIRMWARE_TYPE FirmwareType,
+  IN EFI_PHYSICAL_ADDRESS   FlashAddress,
+  IN FLASH_ADDRESS_TYPE FlashAddressType,
+  IN VOID   *Buffer,
+  IN UINTN  Length,
+  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress,OPTIONAL
+  IN UINTN  StartPercentage,
+  IN UINTN  EndPercentage
   )
 {
   EFI_FIRMWARE_VOLUME_BLOCK_PROTOCOL  *Fvb;
@@ -150,7 +170,7 @@ PerformFlashWrite (
   EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION Black;
   EFI_GRAPHICS_OUTPUT_BLT_PIXEL_UNION White;
   UINTN   Resolution;
-  UINTN   Progress;
+  UINTN   CurrentProgress;
   BOOLEAN HaveBootGraphics;
 
   Black.Raw = 0x;
@@ -228,7 +248,7 @@ PerformFlashWrite (
 
   if (HaveBootGraphics) {
 Resolution = (BlockSize * 100) / Length + 1;
-Progress = 0;
+CurrentProgress = 0;
 
 Status = BootLogoUpdateProgress (White.Pixel, Black.Pixel,
L"Updating firmware - please wait", Black.Pixel, 100, 0);
@@ -268,8 +288,8 @@ Per

[edk2] [edk2-platforms Patch v4 3/6] Socionext/SynQuacerEvalBoard: Add DisplayUpdateProgressLib mapping

2018-05-28 Thread Michael D Kinney

From: "Kinney, Michael D" 

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

Based on content from the following branch/commits:
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc 
b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
index aa34fb075d..402319bdfe 100644
--- a/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
+++ b/Platform/Socionext/SynQuacerEvalBoard/SynQuacerEvalBoard.dsc
@@ -171,6 +171,7 @@ [LibraryClasses.common.DXE_DRIVER]
   # Firmware update
   #
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+  
DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.inf
   
EdkiiSystemCapsuleLib|SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
   
FmpAuthenticationLib|SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.inf
   
PlatformFlashAccessLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
-- 
2.14.2.windows.3

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

[edk2] [edk2-platforms Patch v4 2/6] Socionext/DeveloperBox: Add DisplayUpdateProgressLib mapping

2018-05-28 Thread Michael D Kinney

From: "Kinney, Michael D" 

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

Based on content from the following branch/commits:
https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport

Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 24b2925bf9..b4f87deb5b 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -174,6 +174,7 @@ [LibraryClasses.common.DXE_DRIVER]
   # Firmware update
   #
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+  
DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProgressLibGraphics.inf
   
EdkiiSystemCapsuleLib|SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
   
FmpAuthenticationLib|SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.inf
   
PlatformFlashAccessLib|Silicon/Socionext/SynQuacer/Library/SynQuacerPlatformFlashAccessLib/SynQuacerPlatformFlashAccessLib.inf
-- 
2.14.2.windows.3

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

Re: [edk2] [Patch] SecurityPkg/Tcg2Smm: Correct function parameter attribute

2018-05-28 Thread Yao, Jiewen

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang,
> Chao B
> Sent: Monday, May 28, 2018 7:10 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Long, Qin 
> Subject: [edk2] [Patch] SecurityPkg/Tcg2Smm: Correct function parameter
> attribute
> 
> Correct UpdatePossibleResource parameter attribute to align to comment
> 
> Change-Id: Id8f8be975f0e8666573decc3fbaaf326b7767ba8
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Long Qin 
> Cc: Yao Jiewen 
> Reviewed-by: Chao Zhang 
> Signed-off-by: Zhang, Chao B 
> ---
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index 3e0a68999a..f0c92462cf 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -315,14 +315,14 @@ UpdatePPVersion (
>@return  patch status.
> 
>  **/
>  EFI_STATUS
>  UpdatePossibleResource (
> -  IN  EFI_ACPI_DESCRIPTION_HEADER*Table,
> -  IN  UINT32 *IrqBuffer,
> -  IN  UINT32 IrqBuffserSize,
> -  OUT BOOLEAN*IsShortFormPkgLength
> +  IN OUT  EFI_ACPI_DESCRIPTION_HEADER*Table,
> +  IN  UINT32 *IrqBuffer,
> +  IN  UINT32 IrqBuffserSize,
> +  OUT BOOLEAN*IsShortFormPkgLength
>)
>  {
>UINT8   *DataPtr;
>UINT8   *DataEndPtr;
>UINT32  NewPkgLength;
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch v3 3/3] SignedCapsulePkg/PlatformFlashAccessLib: Add progress API

2018-05-28 Thread Yao, Jiewen

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Kinney, Michael D
> Sent: Thursday, May 24, 2018 11:16 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen 
> Subject: [Patch v3 3/3] SignedCapsulePkg/PlatformFlashAccessLib: Add progress
> API
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=801
> 
> Add a new API to the PlatformFlashAccessLib that passes
> in an optional Progress() function along with a start and
> end percentage to call the Progress() function with.
> If the Progress() function is not NULL, then it is the
> Progress() function that was passed into the Firmware
> Management Protocol SetImage() services and is used
> to update the user on the progress as a firmware device
> is updated with a firmware image.
> 
> Implementations of the PlatformFlashAccessLib are
> recommended to call the Progress() function as work
> is performed to update to contents of a firmware
> storage device.
> 
> Cc: Jiewen Yao 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  .../Include/Library/PlatformFlashAccessLib.h   | 49 ++-
>  .../PlatformFlashAccessLibNull.c   | 70
> +++---
>  2 files changed, 110 insertions(+), 9 deletions(-)
> 
> diff --git a/SignedCapsulePkg/Include/Library/PlatformFlashAccessLib.h
> b/SignedCapsulePkg/Include/Library/PlatformFlashAccessLib.h
> index 0a8858e4c4..e3308251c2 100644
> --- a/SignedCapsulePkg/Include/Library/PlatformFlashAccessLib.h
> +++ b/SignedCapsulePkg/Include/Library/PlatformFlashAccessLib.h
> @@ -1,7 +1,7 @@
>  /** @file
>Platform flash device access library.
> 
> -  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -16,6 +16,8 @@
>  #ifndef __PLATFORM_FLASH_ACCESS_LIB_H__
>  #define __PLATFORM_FLASH_ACCESS_LIB_H__
> 
> +#include 
> +
>  typedef enum {
>FlashAddressTypeRelativeAddress,
>FlashAddressTypeAbsoluteAddress,
> @@ -31,7 +33,7 @@ typedef enum {
>  } PLATFORM_FIRMWARE_TYPE;
> 
>  /**
> -  Perform flash write opreation.
> +  Perform flash write operation.
> 
>@param[in] FirmwareType  The type of firmware.
>@param[in] FlashAddress  The address of flash device to be accessed.
> @@ -54,4 +56,47 @@ PerformFlashWrite (
>IN UINTNLength
>);
> 
> +/**
> +  Perform flash write operation with progress indicator.  The start and end
> +  completion percentage values are passed into this function.  If the
> requested
> +  flash write operation is broken up, then completion percentage between the
> +  start and end values may be passed to the provided Progress function.  The
> +  caller of this function is required to call the Progress function for the
> +  start and end completion percentage values.  This allows the Progress,
> +  StartPercentage, and EndPercentage parameters to be ignored if the
> requested
> +  flash write operation can not be broken up
> +
> +  @param[in] FirmwareType  The type of firmware.
> +  @param[in] FlashAddress  The address of flash device to be accessed.
> +  @param[in] FlashAddressType  The type of flash device address.
> +  @param[in] BufferThe pointer to the data buffer.
> +  @param[in] LengthThe length of data buffer in bytes.
> +  @param[in] Progress  A function used report the progress of the
> +   firmware update.  This is an optional
> parameter
> +   that may be NULL.
> +  @param[in] StartPercentage   The start completion percentage value that
> may
> +   be used to report progress during the flash
> +   write operation.
> +  @param[in] EndPercentage The end completion percentage value that
> may
> +   be used to report progress during the flash
> +   write operation.
> +
> +  @retval EFI_SUCCESS   The operation returns successfully.
> +  @retval EFI_WRITE_PROTECTED   The flash device is read only.
> +  @retval EFI_UNSUPPORTED   The flash device access is unsupported.
> +  @retval EFI_INVALID_PARAMETER The input parameter is not valid.
> +**/
> +EFI_STATUS
> +EFIAPI
> +PerformFlashWriteWithProgress (
> +  IN PLATFORM_FIRMWARE_TYPE FirmwareType,
> +  IN EFI_PHYSICAL_ADDRESS   FlashAddress,
> +  IN FLASH_ADDRESS_TYPE
> FlashAddressType,
> +  IN VOID   *Buffer,
> +  IN UINTN  Length,
> +  IN EFI_FIRMWARE_MANAGEMENT_UPDATE_IMAGE_PROGRESS  Progress,
> OPTIONAL
> +  IN UINTN

Re: [edk2] [Patch v3 3/4] QuarkPlatformPkg: Add DisplayUpdateProgressLib mapping

2018-05-28 Thread Yao, Jiewen

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael D Kinney
> Sent: Thursday, May 24, 2018 11:23 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D 
> Subject: [edk2] [Patch v3 3/4] QuarkPlatformPkg: Add
> DisplayUpdateProgressLib mapping
> 
> From: "Kinney, Michael D" 
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=801
> 
> Based on content from the following branch/commits:
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport
> 
> Cc: Sean Brogan 
> Cc: Kelly Steele 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  QuarkPlatformPkg/Quark.dsc | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/QuarkPlatformPkg/Quark.dsc b/QuarkPlatformPkg/Quark.dsc
> index a43a5595d4..14142087bd 100644
> --- a/QuarkPlatformPkg/Quark.dsc
> +++ b/QuarkPlatformPkg/Quark.dsc
> @@ -241,6 +241,7 @@ [LibraryClasses]
> 
> FmpAuthenticationLib|MdeModulePkg/Library/FmpAuthenticationLibNull/Fmp
> AuthenticationLibNull.inf
>IniParsingLib|SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
> 
> PlatformFlashAccessLib|QuarkPlatformPkg/Feature/Capsule/Library/PlatformFl
> ashAccessLib/PlatformFlashAccessLibDxe.inf
> +
> DisplayUpdateProgressLib|MdeModulePkg/Library/DisplayUpdateProgressLibTe
> xt/DisplayUpdateProgressLibText.inf
> 
>  [LibraryClasses.common.SEC]
>#
> --
> 2.14.2.windows.3
> 
> ___
> 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] smm lock query

2018-05-28 Thread Abhishek Singh

Thank you everyone for your inputs and clarifications. They are helping me
to better understand the uefi code, to which I am very new. I do not mean
to hijack the thread: so please continue your discussions about whether the
implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my
research, I wish to dump the contents of SMRAM when it has reached steady
state, i.e., all the drivers have made changes to smram and it has been
locked. The current implementation (smm ipl) locks smram when it receives
the SmmReadyToLock event and then propagates the event to the smm drivers
that make further changes to smram. Unfortunately, I cannot take a snapshot
of smram after it has been locked! Thus, I have solved this issue by
propagating the event to the smm drivers first (using
SmmIplGuidedEventNotify), then opening access to and dumping the contents
of SMRAM, and finally closing access to and locking smram. Would it be fair
to say that this would give me the fully initialized contents of smram?

My second observation is that despite opening access to smram, I am unable
to access its contents, which is a violation of the
EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This
function “opens” MMRAM so that it is visible while not inside of MM." Note
that I am working with minnowboard firmware release 0.97. So some of the
binaries like SmmAccess.efi are provided by Intel, while others have been
built from the edk source tree: thus, this may not be an EDK issue. Please
suggest further steps and/or workarounds. Should I contact edk2-platforms
maintainers, or start a new thread here for this issue?

-Abhishek

On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser 
wrote:

> Hello Andrew and Laszlo,
>
> Thanks for your comments!
> Of course I'm with you that it is the platform that signals the
> SmmReadyToLock event and therefor is aware.
> However, they might rely on the protocol's description that the resources
> are about(!) to be locked and code accordingly, not considering the event
> characteristic of the handler in PiSmmIpl.
> The code might be written by different people, not especially reviewed
> against edk2's actions, or additional code might be supplied by third
> parties that do not have tree code access (which, by integration, would be
> "platform binaries" by the definition applying here).
>
> Therefor I would still ask everyone to consider figuring out a solution to
> this discrepancy from the specification, such as the internal "dummy event"
> I proposed.
>
> Thanks,
> Marvin.
>
> > -Original Message-
> > From: Laszlo Ersek 
> > Sent: Monday, May 28, 2018 12:58 PM
> > To: Andrew Fish ; Marvin H?user
> > 
> > Cc: edk2-devel@lists.01.org; Abhishek Singh ;
> > ruiyu...@intel.com; eric.d...@intel.com; star.z...@intel.com
> > Subject: Re: [edk2] smm lock query
> >
> > On 05/27/18 22:44, Andrew Fish wrote:
> > >
> > >
> > >> On May 27, 2018, at 9:47 AM, Marvin H?user
> >  wrote:
> > >>
> > >> Good day Abhishek,
> > >>
> > >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> > (exposes the ReadyToLock protocol) and Laszlo for his high-quality
> answers.
> > >>
> > >> Strictly speaking you are, right, because of the description for the
> MM
> > protocol:
> > >> "Indicates that MM resources and services that should not be used by
> the
> > third party code are about[Marvin: (!)] to be locked."
> > >> Practically however, I don't see any issue with the current
> > implementation. Code inside MMRAM is not affected directly by the lock,
> it is
> > just notified.
> > >> However, either the code or the specification should be slightly
> updated
> > to be in sync. A code update might require review of the caller
> assumptions,
> > just to be sure.
> > >>
> > >> I have a different concern though and hope I'm actually overlooking
> > something.
> > >> If I understand the code correctly, it is the Platform BDS that
> exposes the
> > (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> > lock SMM resources based on the event.
> > >> Because of latter being an event however, I don't think it is, or can
> be,
> > guaranteed to be the last event group member executing.
> > >> When it is not the last, the "about to be locked" part is not true
> for any
> > subsequent callbacks, that could actually be a risky break of the
> specification
> > - if it is.
> > >> If it is a break of the specification, I can only think of letting
> Platform BDS
> > expose an "internal" event group, which is only caught by PiSmmIpl, which
> > then drives the actual SmmReadyToLock flow.
> > >> This would require updates to all platform trees and hence I would
> > propose a temporary backwards-compatibility.
> > >>
> > >> Any comments? Did I overlook something (I hope)?
> > >>
> > >
> > > Mavvin,
> > >
> > > You are correct there is no guarantee of order in events. Thanks for
> cc'ing
> > the right folks, as I don't remember all the low level detai

Re: [edk2] [Patch v3 0/3] Add DisplayUpdateProgressLib for capsules

2018-05-28 Thread Yao, Jiewen

Thank you to add Version.

Patch 1,2,3,4 reviewed-by: jiewen@intel.com

Thank you
Yao Jiewen

> -Original Message-
> From: Kinney, Michael D
> Sent: Thursday, May 24, 2018 11:16 PM
> To: edk2-devel@lists.01.org
> Cc: Sean Brogan ; Zeng, Star
> ; Dong, Eric ; Yao, Jiewen
> ; Wei, David ; Guo, Mang
> ; Steele, Kelly 
> Subject: [Patch v3 0/3] Add DisplayUpdateProgressLib for capsules
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=801
> 
> Based on content from:
> 
> https://github.com/Microsoft/MS_UEFI/blob/share/MsCapsuleSupport/MsCaps
> uleUpdatePkg/Include/Library/DisplayUpdateProgressLib.h
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsu
> leUpdatePkg/Library/DisplayUpdateProgressGraphicsLib
> https://github.com/Microsoft/MS_UEFI/tree/share/MsCapsuleSupport/MsCapsu
> leUpdatePkg/Library/DisplayUpdateProgressTextLib
> 
> Updates for V3
> ==
> * Add Version field to EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL
> * Break up patch series into 4 smaller patch series to handle dependencies
>   between the edk2 repository and the edk2-platforms repository.
>   + Patch series for edk2 repo that adds DisplayUpdateProgressLib class and
> instances.  Defines the
> EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL.
> Adds PerformFlashWriteWithProgress() API to the PlatformFlashAccessLib.
>   + Patch series for platforms in edk2-platforms that use capsules to add the
> DisplayUpdateProgressLib mapping to the DSC files and add the
> PerformFlashWriteWithProgress() API implementation to the
> PlatformFlashAccessLib implementations.
>   + Patch series for platforms in edk2 that use capsules to add the
> DisplayUpdateProgressLib mapping to the DSC files and add the
> PerformFlashWriteWithProgress() API implementation to the
> PlatformFlashAccessLib implementations.
>   + Patch for edk2 that adds the use of the DisplayUpateProgressLib and the
> PerformFlashWriteWithProgress() API .
> 
> Updates for V2
> ==
> * Change DisplayUpdateProgressGraphicsLib to
> DisplayUpdateProgressLibGraphics
> * Change DisplayUpdateProgressTextLib to DisplayUpdateProgressLibText
> * Clarify that color in Firmware Management Progress Protocol is the
> foreground color
> * Add missing parameters to PerformFlashWriteWithProgress() function header.
> * Update PerformFlashWriteWithProgress() function header describing the use
> of
>   the start and end percentage values.
> * Update QuarkPlatformPkg PerformFlashWriteWithProgress() to call Progress()
> for
>   the end precentage.
> * Update Vlv2Tbl2DevicePkg PerformFlashWriteWithProgress() to call Progress()
>   for the end precentage.
> 
> Add DisplayUpdateProgressLib class along implementations for both graphical
> (Graphics Output Protocol based) and text (Simple Text Output Protocol based)
> consoles.  Also add the EDK II Firmware Management Progress Protocol that is
> an
> optional protocol that provides the progress bar color and a watchdog timeout
> value thaty can be used when a firmware image is updated in a firmware device.
> 
> * Add progress support to DxeCapsuleLibFmp
> * Add progress support to SystemFirmwareUpdateDxe
> * Add progress support to PlatformFlashAccessLib class and instances.
> * Reduce Print() calls during a firmware update.
> 
> Cc: Sean Brogan 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: David Wei 
> Cc: Mang Guo 
> Cc: Kelly Steele 
> 
> Signed-off-by: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> 
> Michael D Kinney (3):
>   MdeModulePkg: Add DisplayUpdateProgressLib class
>   MdeModulePkg: Add DisplayUpdateProgressLib instances
>   SignedCapsulePkg/PlatformFlashAccessLib: Add progress API
> 
>  .../Include/Library/DisplayUpdateProgressLib.h |  65 +++
>  .../Include/Protocol/FirmwareManagementProgress.h  |  55 +++
>  .../DisplayUpdateProgressLibGraphics.c | 475
> +
>  .../DisplayUpdateProgressLibGraphics.inf   |  60 +++
>  .../DisplayUpdateProgressLibGraphics.uni   |  18 +
>  .../DisplayUpdateProgressLibText.c | 174 
>  .../DisplayUpdateProgressLibText.inf   |  53 +++
>  .../DisplayUpdateProgressLibText.uni   |  18 +
>  MdeModulePkg/MdeModulePkg.dec  |  11 +
>  MdeModulePkg/MdeModulePkg.dsc  |   3 +
>  .../Include/Library/PlatformFlashAccessLib.h   |  49 ++-
>  .../PlatformFlashAccessLibNull.c   |  70 ++-
>  12 files changed, 1042 insertions(+), 9 deletions(-)
>  create mode 100644
> MdeModulePkg/Include/Library/DisplayUpdateProgressLib.h
>  create mode 100644
> MdeModulePkg/Include/Protocol/FirmwareManagementProgress.h
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProg
> ressLibGraphics.c
>  create mode 100644
> MdeModulePkg/Library/DisplayUpdateProgressLibGraphics/DisplayUpdateProg
> ressLibGraphics.inf
>  create mode 100644
>

Re: [edk2] [PATCH] OvmfPkg: raise DXEFV size to 11 MB

2018-05-28 Thread Ard Biesheuvel

On 28 May 2018 at 20:49, Laszlo Ersek  wrote:
> Almost exactly two years after commit 2f7b34b20842f, we've grown out the
> 10MB DXEFV:
>
>> build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -b NOOPT -t GCC48 \
>>   -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D TLS_ENABLE -D E1000_ENABLE \
>>   -D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE
>>
>> [...]
>>
>> GenFv: ERROR 3000: Invalid
>>   the required fv image size 0xa28d48 exceeds the set fv image size
>>   0xa0
>
> Raise the DXEFV size to 11MB.
>
> (For builds that don't need this DXEFV bump, I've checked the
> FVMAIN_COMPACT increase stemming from the additional 1MB padding, using
> NOOPT + GCC48 + FD_SIZE_2MB, and no other "-D" flags. In the IA32 build,
> FVMAIN_COMPACT grows by 232 bytes. In the IA32X64 build, FVMAIN_COMPACT
> shrinks by 64 bytes. In the X64 build, FVMAIN_COMPACT shrinks by 376
> bytes.)
>
> Cc: Ard Biesheuvel 
> Cc: Gary Lin 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 

Reviewed-by: Ard Biesheuvel 

> ---
>
> Notes:
> - repo & branch: https://github.com/lersek/edk2.git ; dxefv_11mb
>
> - regression-tested with the "crash" tool for vmcore analysis
>
> - regression-tested using S3 suspend/resume with my usual guests,
>   including Linux, Windows, i440fx, q35, SMM etc
>
>  OvmfPkg/OvmfPkgIa32.fdf| 6 +++---
>  OvmfPkg/OvmfPkgIa32X64.fdf | 6 +++---
>  OvmfPkg/OvmfPkgX64.fdf | 6 +++---
>  3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 0427ded49239..b199713925fe 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size  = 0xB0
> +Size  = 0xC0
>  ErasePolarity = 1
>  BlockSize = 0x1
> -NumBlocks = 0xB0
> +NumBlocks = 0xC0
>
>  0x00|0x006000
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +89,7 @@ [FD.MEMFD]
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>
> -0x10|0xA0
> +0x10|0xB0
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 6df47f48cd2c..4ebf64b2b9dc 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size  = 0xB0
> +Size  = 0xC0
>  ErasePolarity = 1
>  BlockSize = 0x1
> -NumBlocks = 0xB0
> +NumBlocks = 0xC0
>
>  0x00|0x006000
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +89,7 @@ [FD.MEMFD]
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>
> -0x10|0xA0
> +0x10|0xB0
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 2e2a1749b5d2..9ca96f928287 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -68,10 +68,10 @@ [FD.OVMF_CODE]
>
>  [FD.MEMFD]
>  BaseAddress   = $(MEMFD_BASE_ADDRESS)
> -Size  = 0xB0
> +Size  = 0xC0
>  ErasePolarity = 1
>  BlockSize = 0x1
> -NumBlocks = 0xB0
> +NumBlocks = 0xC0
>
>  0x00|0x006000
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
> @@ -89,7 +89,7 @@ [FD.MEMFD]
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
>  FV = PEIFV
>
> -0x10|0xA0
> +0x10|0xB0
>  
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
>
> --
> 2.14.1.3.gb7cf6e02401b
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] OvmfPkg: raise DXEFV size to 11 MB

2018-05-28 Thread Laszlo Ersek

Almost exactly two years after commit 2f7b34b20842f, we've grown out the
10MB DXEFV:

> build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -b NOOPT -t GCC48 \
>   -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D TLS_ENABLE -D E1000_ENABLE \
>   -D HTTP_BOOT_ENABLE -D NETWORK_IP6_ENABLE
>
> [...]
>
> GenFv: ERROR 3000: Invalid
>   the required fv image size 0xa28d48 exceeds the set fv image size
>   0xa0

Raise the DXEFV size to 11MB.

(For builds that don't need this DXEFV bump, I've checked the
FVMAIN_COMPACT increase stemming from the additional 1MB padding, using
NOOPT + GCC48 + FD_SIZE_2MB, and no other "-D" flags. In the IA32 build,
FVMAIN_COMPACT grows by 232 bytes. In the IA32X64 build, FVMAIN_COMPACT
shrinks by 64 bytes. In the X64 build, FVMAIN_COMPACT shrinks by 376
bytes.)

Cc: Ard Biesheuvel 
Cc: Gary Lin 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
- repo & branch: https://github.com/lersek/edk2.git ; dxefv_11mb

- regression-tested with the "crash" tool for vmcore analysis

- regression-tested using S3 suspend/resume with my usual guests,
  including Linux, Windows, i440fx, q35, SMM etc

 OvmfPkg/OvmfPkgIa32.fdf| 6 +++---
 OvmfPkg/OvmfPkgIa32X64.fdf | 6 +++---
 OvmfPkg/OvmfPkgX64.fdf | 6 +++---
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 0427ded49239..b199713925fe 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -68,10 +68,10 @@ [FD.OVMF_CODE]
 
 [FD.MEMFD]
 BaseAddress   = $(MEMFD_BASE_ADDRESS)
-Size  = 0xB0
+Size  = 0xC0
 ErasePolarity = 1
 BlockSize = 0x1
-NumBlocks = 0xB0
+NumBlocks = 0xC0
 
 0x00|0x006000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
@@ -89,7 +89,7 @@ [FD.MEMFD]
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
 FV = PEIFV
 
-0x10|0xA0
+0x10|0xB0
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = DXEFV
 
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 6df47f48cd2c..4ebf64b2b9dc 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -68,10 +68,10 @@ [FD.OVMF_CODE]
 
 [FD.MEMFD]
 BaseAddress   = $(MEMFD_BASE_ADDRESS)
-Size  = 0xB0
+Size  = 0xC0
 ErasePolarity = 1
 BlockSize = 0x1
-NumBlocks = 0xB0
+NumBlocks = 0xC0
 
 0x00|0x006000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
@@ -89,7 +89,7 @@ [FD.MEMFD]
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
 FV = PEIFV
 
-0x10|0xA0
+0x10|0xB0
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = DXEFV
 
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 2e2a1749b5d2..9ca96f928287 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -68,10 +68,10 @@ [FD.OVMF_CODE]
 
 [FD.MEMFD]
 BaseAddress   = $(MEMFD_BASE_ADDRESS)
-Size  = 0xB0
+Size  = 0xC0
 ErasePolarity = 1
 BlockSize = 0x1
-NumBlocks = 0xB0
+NumBlocks = 0xC0
 
 0x00|0x006000
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
@@ -89,7 +89,7 @@ [FD.MEMFD]
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
 FV = PEIFV
 
-0x10|0xA0
+0x10|0xB0
 
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
 FV = DXEFV
 
-- 
2.14.1.3.gb7cf6e02401b

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

Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

2018-05-28 Thread Marvin Häuser

Hey Jeff,

Of course I meant to introduce a “gEdkiiCpuS3DataAvailablePpiGuid” or similar 
to create a Depex on.

Thanks,
Marvin.

From: Fan Jeff 
Sent: Monday, May 28, 2018 4:19 PM
To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
Cc: ler...@redhat.com; eric.d...@intel.com
Subject: Re:RE: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

Marvin,

Thanks your reply. i have thought my mail hasn't sent out just now.

Adding CpuS3DataPei depends on wether we need to suppoert S3 without DXE, i 
think.

Even we add CpuS3DataPei, we cannot assume the dispatch order between 
CpuFeaturesPei and CpuS3DataPei from core code view. So,we cannot remove those 
code to produce PCD if it does not exist.

Thanks!
Jeff

发自我的小米手机
在 Marvin Häuser 
mailto:marvin.haeu...@outlook.com>>，2018年5月28日 
下午9:55写道：
Hey Jeff,

Thanks for looking into it!

Maybe both should be implemented (PEI and additional DXE Depex) and leave it to 
the platform maintainer, as with CpuFeaturesPei vs CpuFeaturesDxe?
If the platform PEI happens to not consume PCD, PcdPei would need to be 
introduced just to support “CpuS3DataPei”.
On the other hand, when e.g. CpuFeaturesPei is used anyway, it makes good sense 
to choose CpuS3DataPei over CpuS3DataDxe and remove the PCD allocation code 
from CpuFeaturesPei.

Regards,
Marvin.

From: Fan Jeff mailto:vanjeff_...@hotmail.com>>
Sent: Monday, May 28, 2018 11:51 AM
To: Laszlo Ersek mailto:ler...@redhat.com>>; Marvin Häuser 
mailto:marvin.haeu...@outlook.com>>
Cc: edk2-devel@lists.01.org; 
eric.d...@intel.com
Subject: 答复: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

Hi,

The current implementation assumes CpuS3DataDxe was dispatched before 
CpuFeaturesDxe. I do not remember clearly why I made this assumption before. 
(It maybe only due to CpuS3DataDxe was just dispatched firstly on all my 
validation platforms.),

I agree this is one bug.  Simply, we could implement one AllocateAcpiCpuData() 
in DXE instance as PEI instance.

Thanks!

Jeff

From: edk2-devel 
mailto:edk2-devel-boun...@lists.01.org>> on 
behalf of Laszlo Ersek mailto:ler...@redhat.com>>
Sent: Friday, May 25, 2018 7:40:32 PM
To: Marvin Häuser
Cc: edk2-devel@lists.01.org; 
eric.d...@intel.com
Subject: Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

On 05/25/18 12:54, Marvin H?user wrote:
> Good day,
>
> While I was inspecting CpuS3DataDxe and the modules depending on its
> PCD PcdCpuS3DataAddress,

(Side remark: see e.g. the commit message on 92b87f1c8c0b, "OvmfPkg:
build CpuS3DataDxe for -D SMM_REQUIRE", 2015-11-30.)

> I noticed that DxeRegisterCpuFeaturesLib seemingly has an asserted
> dependency on the PCD being ready when it its executed. I did neither
> see a Depex entry, nor an event callback ensuring CpuS3DataDxe has
> been loaded, neither exposed by CpuS3DataDxe, nor consumed by this
> library.
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c#L211

"DxeRegisterCpuFeaturesLib.inf" has a depex on
"gEdkiiCpuFeaturesSetDoneGuid".

No module in the open source edk2 tree produces this protocol GUID, thus
I think this library instance is unusable without other, out-of-tree,
modules. I assume that one of those modules satisfies the dependency
somehow.

Note that CpuS3DataDxe is a platform driver [1]; it is possible that the
platform that includes DxeRegisterCpuFeaturesLib in a driver *also*
includes such a CpuS3DataDxe variant that populates the PCD and then
installs gEdkiiCpuFeaturesSetDoneGuid.

[1] I suggest reviewing the message of commit bfec5efa56ca
("UefiCpuPkg/CpuS3DataDxe: Add module to initialize ACPI_CPU_DATA for
S3", 2015-11-25).

In fact, the series that added "DxeRegisterCpuFeaturesLib.inf" (with the
depex mentioned above) *also* modified CpuS3DataDxe: see [2] and [3].

[2] 8b371e93f206 ("UefiCpuPkg/CpuS3DataDxe: Consume the existing
PcdCpuS3DataAddress", 2017-03-22)

[3] "[edk2] [PATCH 00/11] Add CPU features driver"
https://bugzilla.tianocore.org/show_bug.cgi?id=421
http://mid.mail-archive.com/20170309083553.6016-1-jeff.fan@intel.com

This suggests that there is an out-of-tree module that populates
PcdCpuS3DataAddress before *both* CpuS3DataDxe and
DxeRegisterCpuFeaturesLib access the PCD. For achieving this kind of
ordering, it would be enough for a driver to first populate the PCD, and
then install "gEfiMpServiceProtocolGuid", as both
"DxeRegisterCpuFeaturesLib.inf" and "CpuS3DataDxe.inf" depend on that.

> Is there anything I'm missing that ensures the execution of
> CpuS3DataDxe prior to executing the dependent code? If not, should
> there be a dummy protocol exposed? PiSmmCpuDxeSmm also retrieves this
> PCD, however safely quits when it has not been set. However, this
> could cause unexpected behavior when the PCD is set after this code
>

Re: [edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

2018-05-28 Thread Laszlo Ersek

On 05/28/18 16:40, Ard Biesheuvel wrote:
> Add a routine to DxeServicesLib that abstracts the allocation of memory
> that should be accessible by PEI after a warm reboot. We will use it to
> replace open coded implementations that limit the address to < 4 GB,
> which may not be possible on non-Intel systems that have no 32-bit
> addressable memory at all.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdePkg/Include/Library/DxeServicesLib.h  | 23 ++-
>  MdePkg/Library/DxeServicesLib/Allocate.c | 54 +++
>  MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
>  MdePkg/Library/DxeServicesLib/X64/Allocate.c | 69 
>  4 files changed, 155 insertions(+), 2 deletions(-)

Reviewed-by: Laszlo Ersek 

Thanks,
Laszlo

> 
> diff --git a/MdePkg/Include/Library/DxeServicesLib.h 
> b/MdePkg/Include/Library/DxeServicesLib.h
> index 7c1c62236d96..20aee68af558 100644
> --- a/MdePkg/Include/Library/DxeServicesLib.h
> +++ b/MdePkg/Include/Library/DxeServicesLib.h
> @@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
>OUT   EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
>);
>  
> -#endif
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that 
> is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
>  
> +  @param[in]  MemoryTypeThe memory type to allocate
> +  @param[in]  Pages The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTNPages
> +  );
> +
> +#endif
> diff --git a/MdePkg/Library/DxeServicesLib/Allocate.c 
> b/MdePkg/Library/DxeServicesLib/Allocate.c
> new file mode 100644
> index ..4d118f766d49
> --- /dev/null
> +++ b/MdePkg/Library/DxeServicesLib/Allocate.c
> @@ -0,0 +1,54 @@
> +/** @file
> +  DxeServicesLib memory allocation routines
> +
> +  Copyright (c) 2018, 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 which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +
> +/**
> +  Allocates one or more 4KB pages of a given type from a memory region that 
> is
> +  accessible to PEI.
> +
> +  Allocates the number of 4KB pages of type 'MemoryType' and returns a
> +  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
> +  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
> +  memory remaining to satisfy the request, then NULL is returned.
> +
> +  @param[in]  MemoryTypeThe memory type to allocate
> +  @param[in]  Pages The number of 4 KB pages to allocate.
> +
> +  @return A pointer to the allocated buffer or NULL if allocation fails.
> +
> +**/
> +VOID *
> +EFIAPI
> +AllocatePeiAccessiblePages (
> +  IN EFI_MEMORY_TYPE  MemoryType,
> +  IN UINTNPages
> +  )
> +{
> +  EFI_STATUS  Status;
> +  EFI_PHYSICAL_ADDRESSMemory;
> +
> +  if (Pages == 0) {
> +return NULL;
> +  }
> +
> +  Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
> +  if (EFI_ERROR (Status)) {
> +return NULL;
> +  }
> +  return (VOID *)(UINTN)Memory;
> +}
> diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf 
> b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> index bd2faf2f6f2d..50ae24f8ee22 100644
> --- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> +++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> @@ -27,12 +27,18 @@ [Defines]
>LIBRARY_CLASS  = DxeServicesLib|DXE_CORE DXE_DRIVER 
> DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
> UEFI_DRIVER
>  
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC ARM AARCH64
>  #
>  
>  [Sources]
>DxeServicesLib.c
>  
> +[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
> +  Allocate.c
> +
> +[Sources.X64]
> +  X64/Allocate.c
> +
>  [Packages]
>MdePkg/MdePkg.dec
>  
> @@ -44,6 +50,9 @@ [LibraryClasses]
>UefiLib
>UefiBootServicesTableLib
>  
> +[LibraryClasses.X64]
> +  HobLib
> +
>  [Guids]
>gEfiFileInfoGuid

[edk2] [PATCH v3 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages

2018-05-28 Thread Ard Biesheuvel

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to
AllocatePeiAccessiblePages, which boils down to the same on X64,
but does not crash non-X64 systems that lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 48 
+++-
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 71d624fc9ce9..68b29ac5a9e2 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -165,46 +165,6 @@ IsKnownID (
   }
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN   Size
-  )
-{
-  UINTN Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUSStatus;
-  VOID  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0x;
-
-  Status = gBS->AllocatePages (
-  AllocateMaxAddress,
-  EfiReservedMemoryType,
-  Pages,
-  &Address
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-Buffer = (VOID *) (UINTN) Address;
-ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Allocate buffer for Boot Performance table.
 
@@ -348,7 +308,13 @@ AllocateBootPerformanceTable (
 //
 // Fail to allocate at specified address, continue to allocate at any 
address.
 //
-mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+ 
EfiReservedMemoryType,
+ EFI_SIZE_TO_PAGES 
(BootPerformanceDataSize)
+ );
+if (mAcpiBootPerformanceTable != NULL) {
+  ZeroMem (mAcpiBootPerformanceTable, BootPerformanceDataSize);
+}
   }
   DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table 
address = 0x%x\n", mAcpiBootPerformanceTable));
 
-- 
2.17.0

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

[edk2] [PATCH v3 1/5] OvmfPkg/PlatformBootManagerLib: add missing report status code call

2018-05-28 Thread Ard Biesheuvel

Consumers of status code reports may rely on a status code to be
reported when the ReadyToBoot event is signalled. For instance,
FirmwarePerformanceDxe will fail to install the FPDT ACPI table
in this case. So add the missing call.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c   | 4 
 2 files changed, 5 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 4b72c44bcf0a..d355d0440efd 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -56,6 +56,7 @@ [LibraryClasses]
   QemuFwCfgS3Lib
   LoadLinuxLib
   QemuBootOrderLib
+  ReportStatusCodeLib
   UefiLib
   Tcg2PhysicalPresenceLib
 
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c 
b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
index ef728dfdeb60..f20df9533fda 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -149,6 +150,9 @@ TryRunningQemuKernel (
   //
   EfiSignalEventReadyToBoot();
 
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
+(EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   Status = LoadLinux (KernelBuf, SetupBuf);
 
 FreeAndReturn:
-- 
2.17.0

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

[edk2] [PATCH v3 0/5] Abstract allocation of PEI accessible memory

2018-05-28 Thread Ard Biesheuvel

At the moment, FirmwarePerformanceTableDataDxe or DxeCorePerformanceLib
are unusable on systems such as AMD Seattle, because they unconditionally
attempt to allocate memory below 4 GB, and ASSERT() if this fails. On AMD
Seattle, no 32-bit addressable DRAM exists, and so the driver will always
assert, and crash a running DEBUG build.

The reason for this is that some platforms (i.e., X64 builds consisting of
a 32-bit PEI stage and 64-bit remaining stages) require these allocations
to be below 4 GB, and for some reason, open coding this practice throughout
the code without regard for other architectures has been regarded as an
acceptable approach.

Instead, I would like to propose the approach implemented in this series:
- create an abstracted AllocatePeiAccessiblePages() routine in DxeServicesLib
  that simply allocates pages from any region on all architectures except X64,
  and allocate below 4 GB for X64
- update the various call sites with calls to this new function.

The above is implemented in patches #3 - #6. Patches #1 and #2 are fixes
for issues that I observed in ArmVirtPkg and OvmfPkg while working on
these patches.

Code can be found here:
https://github.com/ardbiesheuvel/edk2/tree/allocate-pei-v3

Changes since v2:
- move AllocatePeiAccessiblePages() to a separate .c file, and create a special
  X64 version (#3)
- add back ZeroMem() call to #4
- add Laszlo's ack to #4 - #5 (but not to #3 since it has been updated)

Changes since v1:
- add Laszlo's ack to #1 - #2
- move EfiAllocatePeiPages() to DxeServicesLib and drop Efi prefix
- update implementation to take EfiFreeMemoryTop into account when built
  for X64

Ard Biesheuvel (5):
  OvmfPkg/PlatformBootManagerLib: add missing report status code call
  ArmVirtPkg/PlatformBootManagerLib: add missing report status code call
  MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine
  MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages
  MdeModulePkg/FirmwarePerformanceDataTableDxe: use
AllocatePeiAccessiblePages

 .../PlatformBootManagerLib.inf|  1 +
 .../PlatformBootManagerLib/QemuKernel.c   |  4 ++
 .../DxeCorePerformanceLib.c   | 48 ++---
 .../FirmwarePerformanceDxe.c  | 51 +++---
 .../FirmwarePerformanceDxe.inf|  1 +
 MdePkg/Include/Library/DxeServicesLib.h   | 23 ++-
 MdePkg/Library/DxeServicesLib/Allocate.c  | 54 +++
 .../Library/DxeServicesLib/DxeServicesLib.inf | 11 ++-
 MdePkg/Library/DxeServicesLib/X64/Allocate.c  | 69 +++
 .../PlatformBootManagerLib.inf|  1 +
 .../PlatformBootManagerLib/QemuKernel.c   |  4 ++
 11 files changed, 182 insertions(+), 85 deletions(-)
 create mode 100644 MdePkg/Library/DxeServicesLib/Allocate.c
 create mode 100644 MdePkg/Library/DxeServicesLib/X64/Allocate.c

-- 
2.17.0

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

[edk2] [PATCH v3 5/5] MdeModulePkg/FirmwarePerformanceDataTableDxe: use AllocatePeiAccessiblePages

2018-05-28 Thread Ard Biesheuvel

Replace the call to and implementation of the function
FpdtAllocateReservedMemoryBelow4G() with a call to
AllocatePeiAccessiblePages, which boils down to the same on X64,
but does not crash non-X64 systems that lack memory below 4 GB.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
   | 51 
 
MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 |  1 +
 2 files changed, 10 insertions(+), 42 deletions(-)

diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
index e719e9e482cb..ded817f37301 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
+++ 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c
@@ -32,6 +32,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -179,46 +180,6 @@ FpdtAcpiTableChecksum (
   Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
 }
 
-/**
-  Allocate EfiReservedMemoryType below 4G memory address.
-
-  This function allocates EfiReservedMemoryType below 4G memory address.
-
-  @param[in]  Size   Size of memory to allocate.
-
-  @return Allocated address for output.
-
-**/
-VOID *
-FpdtAllocateReservedMemoryBelow4G (
-  IN UINTN   Size
-  )
-{
-  UINTN Pages;
-  EFI_PHYSICAL_ADDRESS  Address;
-  EFI_STATUSStatus;
-  VOID  *Buffer;
-
-  Buffer  = NULL;
-  Pages   = EFI_SIZE_TO_PAGES (Size);
-  Address = 0x;
-
-  Status = gBS->AllocatePages (
-  AllocateMaxAddress,
-  EfiReservedMemoryType,
-  Pages,
-  &Address
-  );
-  ASSERT_EFI_ERROR (Status);
-
-  if (!EFI_ERROR (Status)) {
-Buffer = (VOID *) (UINTN) Address;
-ZeroMem (Buffer, Size);
-  }
-
-  return Buffer;
-}
-
 /**
   Callback function upon VariableArchProtocol and LockBoxProtocol
   to allocate S3 performance table memory and save the pointer to LockBox.
@@ -287,7 +248,10 @@ FpdtAllocateS3PerformanceTableMemory (
 //
 // Fail to allocate at specified address, continue to allocate at any 
address.
 //
-mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (sizeof (S3_PERFORMANCE_TABLE));
+mAcpiS3PerformanceTable = (S3_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+ 
EfiReservedMemoryType,
+ EFI_SIZE_TO_PAGES 
(sizeof (S3_PERFORMANCE_TABLE))
+ );
   }
   DEBUG ((EFI_D_INFO, "FPDT: ACPI S3 Performance Table address = 0x%x\n", 
mAcpiS3PerformanceTable));
   if (mAcpiS3PerformanceTable != NULL) {
@@ -368,7 +332,10 @@ InstallFirmwarePerformanceDataTable (
   //
   // Fail to allocate at specified address, continue to allocate at any 
address.
   //
-  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize);
+  mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) 
AllocatePeiAccessiblePages (
+   
EfiReservedMemoryType,
+   
EFI_SIZE_TO_PAGES (BootPerformanceDataSize)
+   );
 }
 DEBUG ((DEBUG_INFO, "FPDT: ACPI Boot Performance Table address = 0x%x\n", 
mAcpiBootPerformanceTable));
 if (mAcpiBootPerformanceTable == NULL) {
diff --git 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
index 8757bbd0aaa9..3d2dd6eb732f 100644
--- 
a/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
+++ 
b/MdeModulePkg/Universal/Acpi/FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf
@@ -44,6 +44,7 @@ [LibraryClasses]
   UefiRuntimeServicesTableLib
   BaseLib
   DebugLib
+  DxeServicesLib
   TimerLib
   BaseMemoryLib
   MemoryAllocationLib
-- 
2.17.0

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

[edk2] [PATCH v3 3/5] MdePkg/DxeServicesLib: introduce AllocatePeiAccessiblePages routine

2018-05-28 Thread Ard Biesheuvel

Add a routine to DxeServicesLib that abstracts the allocation of memory
that should be accessible by PEI after a warm reboot. We will use it to
replace open coded implementations that limit the address to < 4 GB,
which may not be possible on non-Intel systems that have no 32-bit
addressable memory at all.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Include/Library/DxeServicesLib.h  | 23 ++-
 MdePkg/Library/DxeServicesLib/Allocate.c | 54 +++
 MdePkg/Library/DxeServicesLib/DxeServicesLib.inf | 11 +++-
 MdePkg/Library/DxeServicesLib/X64/Allocate.c | 69 
 4 files changed, 155 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Include/Library/DxeServicesLib.h 
b/MdePkg/Include/Library/DxeServicesLib.h
index 7c1c62236d96..20aee68af558 100644
--- a/MdePkg/Include/Library/DxeServicesLib.h
+++ b/MdePkg/Include/Library/DxeServicesLib.h
@@ -305,5 +305,26 @@ GetFileDevicePathFromAnyFv (
   OUT   EFI_DEVICE_PATH_PROTOCOL  **FvFileDevicePath
   );
 
-#endif
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
 
+  @param[in]  MemoryTypeThe memory type to allocate
+  @param[in]  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTNPages
+  );
+
+#endif
diff --git a/MdePkg/Library/DxeServicesLib/Allocate.c 
b/MdePkg/Library/DxeServicesLib/Allocate.c
new file mode 100644
index ..4d118f766d49
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/Allocate.c
@@ -0,0 +1,54 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, 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 which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+
+/**
+  Allocates one or more 4KB pages of a given type from a memory region that is
+  accessible to PEI.
+
+  Allocates the number of 4KB pages of type 'MemoryType' and returns a
+  pointer to the allocated buffer.  The buffer returned is aligned on a 4KB
+  boundary.  If Pages is 0, then NULL is returned.  If there is not enough
+  memory remaining to satisfy the request, then NULL is returned.
+
+  @param[in]  MemoryTypeThe memory type to allocate
+  @param[in]  Pages The number of 4 KB pages to allocate.
+
+  @return A pointer to the allocated buffer or NULL if allocation fails.
+
+**/
+VOID *
+EFIAPI
+AllocatePeiAccessiblePages (
+  IN EFI_MEMORY_TYPE  MemoryType,
+  IN UINTNPages
+  )
+{
+  EFI_STATUS  Status;
+  EFI_PHYSICAL_ADDRESSMemory;
+
+  if (Pages == 0) {
+return NULL;
+  }
+
+  Status = gBS->AllocatePages (AllocateAnyPages, MemoryType, Pages, &Memory);
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+  return (VOID *)(UINTN)Memory;
+}
diff --git a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf 
b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
index bd2faf2f6f2d..50ae24f8ee22 100644
--- a/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+++ b/MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
@@ -27,12 +27,18 @@ [Defines]
   LIBRARY_CLASS  = DxeServicesLib|DXE_CORE DXE_DRIVER 
DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER SMM_CORE UEFI_APPLICATION 
UEFI_DRIVER
 
 #
-#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
+#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC ARM AARCH64
 #
 
 [Sources]
   DxeServicesLib.c
 
+[Sources.IA32, Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
+  Allocate.c
+
+[Sources.X64]
+  X64/Allocate.c
+
 [Packages]
   MdePkg/MdePkg.dec
 
@@ -44,6 +50,9 @@ [LibraryClasses]
   UefiLib
   UefiBootServicesTableLib
 
+[LibraryClasses.X64]
+  HobLib
+
 [Guids]
   gEfiFileInfoGuid  ## SOMETIMES_CONSUMES ## 
UNDEFINED
 
diff --git a/MdePkg/Library/DxeServicesLib/X64/Allocate.c 
b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
new file mode 100644
index ..b6d34ba20881
--- /dev/null
+++ b/MdePkg/Library/DxeServicesLib/X64/Allocate.c
@@ -0,0 +1,69 @@
+/** @file
+  DxeServicesLib memory allocation routines
+
+  Copyright (c) 2018, Linaro, Ltd. All r

[edk2] [PATCH v3 2/5] ArmVirtPkg/PlatformBootManagerLib: add missing report status code call

2018-05-28 Thread Ard Biesheuvel

Consumers of status code reports may rely on a status code to be
reported when the ReadyToBoot event is signalled. For instance,
FirmwarePerformanceDxe will fail to install the FPDT ACPI table
in this case. So add the missing call.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
 ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c   | 4 
 2 files changed, 5 insertions(+)

diff --git 
a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index d6c1ef95dc44..0cbc82f5d27d 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -53,6 +53,7 @@ [LibraryClasses]
   PrintLib
   QemuBootOrderLib
   QemuFwCfgLib
+  ReportStatusCodeLib
   UefiBootManagerLib
   UefiBootServicesTableLib
   UefiLib
diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c 
b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
index ac47d21e71c8..7b59f57eb19f 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c
@@ -20,6 +20,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1072,6 +1073,9 @@ TryRunningQemuKernel (
   //
   EfiSignalEventReadyToBoot();
 
+  REPORT_STATUS_CODE (EFI_PROGRESS_CODE,
+(EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_TO_BOOT_EVENT));
+
   //
   // Start the image.
   //
-- 
2.17.0

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

Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

2018-05-28 Thread Fan Jeff

Marvin,

Thanks your reply. i have thought my mail hasn't sent out just now.

Adding CpuS3DataPei depends on wether we need to suppoert S3 without DXE, i 
think.

Even we add CpuS3DataPei, we cannot assume the dispatch order between 
CpuFeaturesPei and CpuS3DataPei from core code view. So,we cannot remove those 
code to produce PCD if it does not exist.

Thanks!
Jeff

发自我的小米手机
在 Marvin Häuser ，2018年5月28日 下午9:55写道：
Hey Jeff,

Thanks for looking into it!

Maybe both should be implemented (PEI and additional DXE Depex) and leave it to 
the platform maintainer, as with CpuFeaturesPei vs CpuFeaturesDxe?
If the platform PEI happens to not consume PCD, PcdPei would need to be 
introduced just to support “CpuS3DataPei”.
On the other hand, when e.g. CpuFeaturesPei is used anyway, it makes good sense 
to choose CpuS3DataPei over CpuS3DataDxe and remove the PCD allocation code 
from CpuFeaturesPei.

Regards,
Marvin.

From: Fan Jeff 
Sent: Monday, May 28, 2018 11:51 AM
To: Laszlo Ersek ; Marvin Häuser 
Cc: edk2-devel@lists.01.org; eric.d...@intel.com
Subject: 答复: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

Hi,

The current implementation assumes CpuS3DataDxe was dispatched before 
CpuFeaturesDxe. I do not remember clearly why I made this assumption before. 
(It maybe only due to CpuS3DataDxe was just dispatched firstly on all my 
validation platforms.),

I agree this is one bug.  Simply, we could implement one AllocateAcpiCpuData() 
in DXE instance as PEI instance.

Thanks!

Jeff

From: edk2-devel 
mailto:edk2-devel-boun...@lists.01.org>> on 
behalf of Laszlo Ersek mailto:ler...@redhat.com>>
Sent: Friday, May 25, 2018 7:40:32 PM
To: Marvin Häuser
Cc: edk2-devel@lists.01.org; 
eric.d...@intel.com
Subject: Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

On 05/25/18 12:54, Marvin H?user wrote:
> Good day,
>
> While I was inspecting CpuS3DataDxe and the modules depending on its
> PCD PcdCpuS3DataAddress,

(Side remark: see e.g. the commit message on 92b87f1c8c0b, "OvmfPkg:
build CpuS3DataDxe for -D SMM_REQUIRE", 2015-11-30.)

> I noticed that DxeRegisterCpuFeaturesLib seemingly has an asserted
> dependency on the PCD being ready when it its executed. I did neither
> see a Depex entry, nor an event callback ensuring CpuS3DataDxe has
> been loaded, neither exposed by CpuS3DataDxe, nor consumed by this
> library.
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c#L211

"DxeRegisterCpuFeaturesLib.inf" has a depex on
"gEdkiiCpuFeaturesSetDoneGuid".

No module in the open source edk2 tree produces this protocol GUID, thus
I think this library instance is unusable without other, out-of-tree,
modules. I assume that one of those modules satisfies the dependency
somehow.

Note that CpuS3DataDxe is a platform driver [1]; it is possible that the
platform that includes DxeRegisterCpuFeaturesLib in a driver *also*
includes such a CpuS3DataDxe variant that populates the PCD and then
installs gEdkiiCpuFeaturesSetDoneGuid.

[1] I suggest reviewing the message of commit bfec5efa56ca
("UefiCpuPkg/CpuS3DataDxe: Add module to initialize ACPI_CPU_DATA for
S3", 2015-11-25).

In fact, the series that added "DxeRegisterCpuFeaturesLib.inf" (with the
depex mentioned above) *also* modified CpuS3DataDxe: see [2] and [3].

[2] 8b371e93f206 ("UefiCpuPkg/CpuS3DataDxe: Consume the existing
PcdCpuS3DataAddress", 2017-03-22)

[3] "[edk2] [PATCH 00/11] Add CPU features driver"
https://bugzilla.tianocore.org/show_bug.cgi?id=421
http://mid.mail-archive.com/20170309083553.6016-1-jeff.fan@intel.com

This suggests that there is an out-of-tree module that populates
PcdCpuS3DataAddress before *both* CpuS3DataDxe and
DxeRegisterCpuFeaturesLib access the PCD. For achieving this kind of
ordering, it would be enough for a driver to first populate the PCD, and
then install "gEfiMpServiceProtocolGuid", as both
"DxeRegisterCpuFeaturesLib.inf" and "CpuS3DataDxe.inf" depend on that.

> Is there anything I'm missing that ensures the execution of
> CpuS3DataDxe prior to executing the dependent code? If not, should
> there be a dummy protocol exposed? PiSmmCpuDxeSmm also retrieves this
> PCD, however safely quits when it has not been set. However, this
> could cause unexpected behavior when the PCD is set after this code
> has been executed. I did not notice any dependency satisfaction
> actions here either.

The ordering between CpuS3DataDxe and PiSmmCpuDxeSmm is safe; it's
orchestrated by Platform BDS. See commit 92b87f1c8c0b above.

> Furthermore, not directly related to this dependency issue, the DXE
> code obviously does not implement AllocateAcpiCpuData() entirely.

More precisely, the DXE code expects AllocateAcpiCpuData() never to be
called; i.e., when the common "RegisterCpuFeaturesLib.c" source file is
executed in DXE, the expectation is that it never

[edk2] [Patch] SecurityPkg/Tcg2Smm: Correct function parameter attribute

2018-05-28 Thread Zhang, Chao B

Correct UpdatePossibleResource parameter attribute to align to comment

Change-Id: Id8f8be975f0e8666573decc3fbaaf326b7767ba8
Contributed-under: TianoCore Contribution Agreement 1.1
Cc: Long Qin 
Cc: Yao Jiewen 
Reviewed-by: Chao Zhang 
Signed-off-by: Zhang, Chao B 
---
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 3e0a68999a..f0c92462cf 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -315,14 +315,14 @@ UpdatePPVersion (
   @return  patch status.
 
 **/
 EFI_STATUS
 UpdatePossibleResource (
-  IN  EFI_ACPI_DESCRIPTION_HEADER*Table,
-  IN  UINT32 *IrqBuffer,
-  IN  UINT32 IrqBuffserSize,
-  OUT BOOLEAN*IsShortFormPkgLength
+  IN OUT  EFI_ACPI_DESCRIPTION_HEADER*Table,
+  IN  UINT32 *IrqBuffer,
+  IN  UINT32 IrqBuffserSize,
+  OUT BOOLEAN*IsShortFormPkgLength
   )
 {
   UINT8   *DataPtr;
   UINT8   *DataEndPtr;
   UINT32  NewPkgLength;
-- 
2.16.2.windows.1

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

Re: [edk2] smm lock query

2018-05-28 Thread Marvin Häuser

Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock 
event and therefor is aware.
However, they might rely on the protocol's description that the resources are 
about(!) to be locked and code accordingly, not considering the event 
characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against 
edk2's actions, or additional code might be supplied by third parties that do 
not have tree code access (which, by integration, would be "platform binaries" 
by the definition applying here).

Therefor I would still ask everyone to consider figuring out a solution to this 
discrepancy from the specification, such as the internal "dummy event" I 
proposed.

Thanks,
Marvin.

> -Original Message-
> From: Laszlo Ersek 
> Sent: Monday, May 28, 2018 12:58 PM
> To: Andrew Fish ; Marvin H?user
> 
> Cc: edk2-devel@lists.01.org; Abhishek Singh ;
> ruiyu...@intel.com; eric.d...@intel.com; star.z...@intel.com
> Subject: Re: [edk2] smm lock query
> 
> On 05/27/18 22:44, Andrew Fish wrote:
> >
> >
> >> On May 27, 2018, at 9:47 AM, Marvin H?user
>  wrote:
> >>
> >> Good day Abhishek,
> >>
> >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> >>
> >> Strictly speaking you are, right, because of the description for the MM
> protocol:
> >> "Indicates that MM resources and services that should not be used by the
> third party code are about[Marvin: (!)] to be locked."
> >> Practically however, I don't see any issue with the current
> implementation. Code inside MMRAM is not affected directly by the lock, it is
> just notified.
> >> However, either the code or the specification should be slightly updated
> to be in sync. A code update might require review of the caller assumptions,
> just to be sure.
> >>
> >> I have a different concern though and hope I'm actually overlooking
> something.
> >> If I understand the code correctly, it is the Platform BDS that exposes the
> (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> lock SMM resources based on the event.
> >> Because of latter being an event however, I don't think it is, or can be,
> guaranteed to be the last event group member executing.
> >> When it is not the last, the "about to be locked" part is not true for any
> subsequent callbacks, that could actually be a risky break of the 
> specification
> - if it is.
> >> If it is a break of the specification, I can only think of letting 
> >> Platform BDS
> expose an "internal" event group, which is only caught by PiSmmIpl, which
> then drives the actual SmmReadyToLock flow.
> >> This would require updates to all platform trees and hence I would
> propose a temporary backwards-compatibility.
> >>
> >> Any comments? Did I overlook something (I hope)?
> >>
> >
> > Mavvin,
> >
> > You are correct there is no guarantee of order in events. Thanks for cc'ing
> the right folks, as I don't remember all the low level details...
> >
> > In general the idea behind the MM code is it only comes from the platform,
> then by definition that code should be aware when the platform was going
> to lock MM. In a practical sense any MM module that had a depex evaluate
> to true would have dispatched in DXE prior to BDS being launched. In general
> BDS is the code that enumerates PCI and connects devices, thus there is no
> chance for 3rd party software to run before that point in the boot. So in an
> abstract sense that lock represents the end of DXE dispatch.
> 
> This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> installed by Plaform BDS before any non-platform binaries get a chance to
> run. In terms of the current PlatformBootManagerLib interfaces, that means
> the protocol should be installed from
> PlatformBootManagerBeforeConsole() (as noted on the API declaration
> itself).
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

2018-05-28 Thread Marvin Häuser

Hey Jeff,

Thanks for looking into it!

Maybe both should be implemented (PEI and additional DXE Depex) and leave it to 
the platform maintainer, as with CpuFeaturesPei vs CpuFeaturesDxe?
If the platform PEI happens to not consume PCD, PcdPei would need to be 
introduced just to support “CpuS3DataPei”.
On the other hand, when e.g. CpuFeaturesPei is used anyway, it makes good sense 
to choose CpuS3DataPei over CpuS3DataDxe and remove the PCD allocation code 
from CpuFeaturesPei.

Regards,
Marvin.

From: Fan Jeff 
Sent: Monday, May 28, 2018 11:51 AM
To: Laszlo Ersek ; Marvin Häuser 
Cc: edk2-devel@lists.01.org; eric.d...@intel.com
Subject: 答复: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

Hi,

The current implementation assumes CpuS3DataDxe was dispatched before 
CpuFeaturesDxe. I do not remember clearly why I made this assumption before. 
(It maybe only due to CpuS3DataDxe was just dispatched firstly on all my 
validation platforms.),

I agree this is one bug.  Simply, we could implement one AllocateAcpiCpuData() 
in DXE instance as PEI instance.

Thanks!

Jeff

From: edk2-devel 
mailto:edk2-devel-boun...@lists.01.org>> on 
behalf of Laszlo Ersek mailto:ler...@redhat.com>>
Sent: Friday, May 25, 2018 7:40:32 PM
To: Marvin Häuser
Cc: edk2-devel@lists.01.org; 
eric.d...@intel.com
Subject: Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

On 05/25/18 12:54, Marvin H?user wrote:
> Good day,
>
> While I was inspecting CpuS3DataDxe and the modules depending on its
> PCD PcdCpuS3DataAddress,

(Side remark: see e.g. the commit message on 92b87f1c8c0b, "OvmfPkg:
build CpuS3DataDxe for -D SMM_REQUIRE", 2015-11-30.)

> I noticed that DxeRegisterCpuFeaturesLib seemingly has an asserted
> dependency on the PCD being ready when it its executed. I did neither
> see a Depex entry, nor an event callback ensuring CpuS3DataDxe has
> been loaded, neither exposed by CpuS3DataDxe, nor consumed by this
> library.
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c#L211

"DxeRegisterCpuFeaturesLib.inf" has a depex on
"gEdkiiCpuFeaturesSetDoneGuid".

No module in the open source edk2 tree produces this protocol GUID, thus
I think this library instance is unusable without other, out-of-tree,
modules. I assume that one of those modules satisfies the dependency
somehow.

Note that CpuS3DataDxe is a platform driver [1]; it is possible that the
platform that includes DxeRegisterCpuFeaturesLib in a driver *also*
includes such a CpuS3DataDxe variant that populates the PCD and then
installs gEdkiiCpuFeaturesSetDoneGuid.

[1] I suggest reviewing the message of commit bfec5efa56ca
("UefiCpuPkg/CpuS3DataDxe: Add module to initialize ACPI_CPU_DATA for
S3", 2015-11-25).

In fact, the series that added "DxeRegisterCpuFeaturesLib.inf" (with the
depex mentioned above) *also* modified CpuS3DataDxe: see [2] and [3].

[2] 8b371e93f206 ("UefiCpuPkg/CpuS3DataDxe: Consume the existing
PcdCpuS3DataAddress", 2017-03-22)

[3] "[edk2] [PATCH 00/11] Add CPU features driver"
https://bugzilla.tianocore.org/show_bug.cgi?id=421
http://mid.mail-archive.com/20170309083553.6016-1-jeff.fan@intel.com

This suggests that there is an out-of-tree module that populates
PcdCpuS3DataAddress before *both* CpuS3DataDxe and
DxeRegisterCpuFeaturesLib access the PCD. For achieving this kind of
ordering, it would be enough for a driver to first populate the PCD, and
then install "gEfiMpServiceProtocolGuid", as both
"DxeRegisterCpuFeaturesLib.inf" and "CpuS3DataDxe.inf" depend on that.

> Is there anything I'm missing that ensures the execution of
> CpuS3DataDxe prior to executing the dependent code? If not, should
> there be a dummy protocol exposed? PiSmmCpuDxeSmm also retrieves this
> PCD, however safely quits when it has not been set. However, this
> could cause unexpected behavior when the PCD is set after this code
> has been executed. I did not notice any dependency satisfaction
> actions here either.

The ordering between CpuS3DataDxe and PiSmmCpuDxeSmm is safe; it's
orchestrated by Platform BDS. See commit 92b87f1c8c0b above.

> Furthermore, not directly related to this dependency issue, the DXE
> code obviously does not implement AllocateAcpiCpuData() entirely.

More precisely, the DXE code expects AllocateAcpiCpuData() never to be
called; i.e., when the common "RegisterCpuFeaturesLib.c" source file is
executed in DXE, the expectation is that it never reaches the call to
AllocateAcpiCpuData().

> Hence, the if-branch following its call, will either add another layer
> of firing ASSERTs, or it will plainly do nothing. Maybe it could be
> moved into the current AllocateAcpiCpuData() function and it be
> renamed accordingly?
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c#L526

Sorry, I don't understand

Re: [edk2] 答复: CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

2018-05-28 Thread Fan Jeff

Hi,

The current implementation assumes CpuS3DataDxe was dispatched before 
CpuFeaturesDxe. I do not remember clearly why I made this assumption before. 
(It maybe only due to CpuS3DataDxe was just dispatched firstly on all my 
validation platforms.),

I agree this is one bug.  Simply, we could implement one AllocateAcpiCpuData() 
in DXE instance as PEI instance.

Thanks!

Jeff

发自我的小米手机
在 Fan Jeff ，2018年5月28日 下午5:50写道：

Hi,

The current implementation assumes CpuS3DataDxe was dispatched before 
CpuFeaturesDxe. I do not remember clearly why I made this assumption before. 
(It maybe only due to CpuS3DataDxe was just dispatched firstly on all my 
validation platforms.),

I agree this is one bug.  Simply, we could implement one AllocateAcpiCpuData() 
in DXE instance as PEI instance.

Thanks!

Jeff

From: edk2-devel  on behalf of Laszlo Ersek 

Sent: Friday, May 25, 2018 7:40:32 PM
To: Marvin Häuser
Cc: edk2-devel@lists.01.org; eric.d...@intel.com
Subject: Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

On 05/25/18 12:54, Marvin H?user wrote:
> Good day,
>
> While I was inspecting CpuS3DataDxe and the modules depending on its
> PCD PcdCpuS3DataAddress,

(Side remark: see e.g. the commit message on 92b87f1c8c0b, "OvmfPkg:
build CpuS3DataDxe for -D SMM_REQUIRE", 2015-11-30.)

> I noticed that DxeRegisterCpuFeaturesLib seemingly has an asserted
> dependency on the PCD being ready when it its executed. I did neither
> see a Depex entry, nor an event callback ensuring CpuS3DataDxe has
> been loaded, neither exposed by CpuS3DataDxe, nor consumed by this
> library.
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c#L211

"DxeRegisterCpuFeaturesLib.inf" has a depex on
"gEdkiiCpuFeaturesSetDoneGuid".

No module in the open source edk2 tree produces this protocol GUID, thus
I think this library instance is unusable without other, out-of-tree,
modules. I assume that one of those modules satisfies the dependency
somehow.

Note that CpuS3DataDxe is a platform driver [1]; it is possible that the
platform that includes DxeRegisterCpuFeaturesLib in a driver *also*
includes such a CpuS3DataDxe variant that populates the PCD and then
installs gEdkiiCpuFeaturesSetDoneGuid.

[1] I suggest reviewing the message of commit bfec5efa56ca
("UefiCpuPkg/CpuS3DataDxe: Add module to initialize ACPI_CPU_DATA for
S3", 2015-11-25).

In fact, the series that added "DxeRegisterCpuFeaturesLib.inf" (with the
depex mentioned above) *also* modified CpuS3DataDxe: see [2] and [3].

[2] 8b371e93f206 ("UefiCpuPkg/CpuS3DataDxe: Consume the existing
PcdCpuS3DataAddress", 2017-03-22)

[3] "[edk2] [PATCH 00/11] Add CPU features driver"
https://bugzilla.tianocore.org/show_bug.cgi?id=421
http://mid.mail-archive.com/20170309083553.6016-1-jeff.fan@intel.com

This suggests that there is an out-of-tree module that populates
PcdCpuS3DataAddress before *both* CpuS3DataDxe and
DxeRegisterCpuFeaturesLib access the PCD. For achieving this kind of
ordering, it would be enough for a driver to first populate the PCD, and
then install "gEfiMpServiceProtocolGuid", as both
"DxeRegisterCpuFeaturesLib.inf" and "CpuS3DataDxe.inf" depend on that.

> Is there anything I'm missing that ensures the execution of
> CpuS3DataDxe prior to executing the dependent code? If not, should
> there be a dummy protocol exposed? PiSmmCpuDxeSmm also retrieves this
> PCD, however safely quits when it has not been set. However, this
> could cause unexpected behavior when the PCD is set after this code
> has been executed. I did not notice any dependency satisfaction
> actions here either.

The ordering between CpuS3DataDxe and PiSmmCpuDxeSmm is safe; it's
orchestrated by Platform BDS. See commit 92b87f1c8c0b above.

> Furthermore, not directly related to this dependency issue, the DXE
> code obviously does not implement AllocateAcpiCpuData() entirely.

More precisely, the DXE code expects AllocateAcpiCpuData() never to be
called; i.e., when the common "RegisterCpuFeaturesLib.c" source file is
executed in DXE, the expectation is that it never reaches the call to
AllocateAcpiCpuData().

> Hence, the if-branch following its call, will either add another layer
> of firing ASSERTs, or it will plainly do nothing. Maybe it could be
> moved into the current AllocateAcpiCpuData() function and it be
> renamed accordingly?
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c#L526

Sorry, I don't understand your point -- CpuRegisterTableWriteWorker() is
used in both PEI and DXE, and it's implemented for the general case.
When it runs in DXE, the expectation is apparently that
AllocateAcpiCpuData() will never be needed / reached, hence the
ASSERT(FALSE) stub implementation for the latter, in
"DxeRegisterCpuFeaturesLib.c".

Oh wait, I think you mistyped your point. The "if" that you refer to
does not *follow* the

[edk2] [PATCH] CorebootModulePkg/CbSupportDxe: Enable ACPI via cmd register

2018-05-28 Thread Matt Delco

The existing code sets the SCI_EN bit directly, which is a violation
of the ACPI spec ("It is the responsibility of the hardware to set or
reset this bit. OSPM always preserves this bit position"). The proper
way to cause this bit to bit set is to reference the FADT table and
write the value of ACPI_ENABLE to SMI_CMD.  The SMI will in turn set
the SCI_EN bit.

Prior to this change ACPI events were not being delivered because
ACPI was not properly enabled and the OS also did not attempt
to enable ACPI since it sees that SCI_EN is already set.  After this
change I observed that ACPI events were now being delivered properly.

Cc: Maurice Ma 
Cc: Prince Agyeman 
Cc: Benjamin You 

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Matt Delco 
---
 CorebootModulePkg/CbSupportDxe/CbSupportDxe.c | 79 ---
 .../CbSupportDxe/CbSupportDxe.inf |  1 +
 2 files changed, 68 insertions(+), 12 deletions(-)

diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
index c526c9e871..4c7917ff2a 100755
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.c
@@ -14,7 +14,10 @@
 **/
 #include "CbSupportDxe.h"

-UINTN mPmCtrlReg = 0;
+BOOLEAN mSmiPortFound = FALSE;
+UINTN mSmiCommandPort = 0;
+UINT8 mAcpiEnable = 0;
+
 /**
   Reserve MMIO/IO resource in GCD

@@ -107,9 +110,63 @@ OnReadyToBoot (
   //
   // Enable SCI
   //
-  IoOr16 (mPmCtrlReg, BIT0);
+  if (!mSmiPortFound) {
+DEBUG ((DEBUG_ERROR, "SMI port not known so can not enable SCI\n"));
+  } else {
+IoWrite8 (mSmiCommandPort, mAcpiEnable);
+DEBUG ((DEBUG_ERROR, "Enable ACPI at 0x%lx with 0x%x before boot\n",
(UINT64)mSmiCommandPort, (UINT32)mAcpiEnable));
+  }
+}
+
+/**
+  Lookup port and value for enabling ACPI
+
+  @param[in] SystemTableA pointer to the EFI System Table.

-  DEBUG ((EFI_D_ERROR, "Enable SCI bit at 0x%lx before boot\n",
(UINT64)mPmCtrlReg));
+**/
+VOID
+FindAcpiFadtTableInfo (
+  IN EFI_SYSTEM_TABLE   *SystemTable
+  )
+{
+  EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
+  EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
+  EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *Fadt;
+  UINTN Index;
+  Rsdp  = NULL;
+  Rsdt  = NULL;
+  Fadt  = NULL;
+
+  for (Index = 0; Index < SystemTable->NumberOfTableEntries; Index++) {
+if (CompareGuid (&(SystemTable->ConfigurationTable[Index].VendorGuid),
&gEfiAcpi20TableGuid)) {
+  Rsdp = SystemTable->ConfigurationTable[Index].VendorTable;
+  break;
+}
+  }
+  if (Rsdp == NULL) {
+return;
+  }
+
+  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN) Rsdp->RsdtAddress;
+  if (Rsdt == NULL || Rsdt->Signature !=
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
+return;
+  }
+  for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < Rsdt->Length;
Index = Index + sizeof (UINT32)) {
+UINT32 Data32  = *(UINT32 *) ((UINT8 *) Rsdt + Index);
+Fadt= (EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE *) (UINT32 *)
(UINTN) Data32;
+if (Fadt->Header.Signature ==
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+  break;
+}
+  }
+
+  if (Fadt == NULL || Fadt->Header.Signature !=
EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+return;
+  }
+
+  mSmiPortFound = TRUE;
+  mSmiCommandPort = Fadt->SmiCmd;
+  mAcpiEnable = Fadt->AcpiEnable;
+  return;
 }

 /**
@@ -133,7 +190,6 @@ CbDxeEntryPoint (
   EFI_EVENT  ReadyToBootEvent;
   EFI_HOB_GUID_TYPE  *GuidHob;
   SYSTEM_TABLE_INFO  *pSystemTableInfo;
-  ACPI_BOARD_INFO*pAcpiBoardInfo;
   FRAME_BUFFER_INFO  *FbInfo;

   Status = EFI_SUCCESS;
@@ -172,14 +228,14 @@ CbDxeEntryPoint (
   }

   //
-  // Find the acpi board information guid hob
+  // Find the SMI port in the FADT table in acpi
   //
-  GuidHob = GetFirstGuidHob (&gUefiAcpiBoardInfoGuid);
-  ASSERT (GuidHob != NULL);
-  pAcpiBoardInfo = (ACPI_BOARD_INFO *)GET_GUID_HOB_DATA (GuidHob);
-
-  mPmCtrlReg = (UINTN)pAcpiBoardInfo->PmCtrlRegBase;
-  DEBUG ((EFI_D_ERROR, "PmCtrlReg at 0x%lx\n", (UINT64)mPmCtrlReg));
+  FindAcpiFadtTableInfo(SystemTable);
+  if (mSmiPortFound) {
+DEBUG ((DEBUG_ERROR, "Found ACPI enable info in FADT: 0x%lx 0x%x\n",
(UINT64)mSmiCommandPort, (UINT32)mAcpiEnable));
+  } else {
+DEBUG ((DEBUG_ERROR, "Failed to find FADT info for enabling ACPI\n"));
+  }

   //
   // Find the frame buffer information and update PCDs
@@ -212,4 +268,3 @@ CbDxeEntryPoint (

   return EFI_SUCCESS;
 }
-
diff --git a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
index 99245183ea..e334dafdb7 100644
--- a/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+++ b/CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
@@ -51,6 +51,7 @@

 [Guids]
   gEfiAcpiTableGuid
+  gEfiAcpi20TableGuid
   gEfiSmbiosTableGuid
   gUefiSystemTableInfoGuid
   gUefiAcpiBoardInfoGuid
-- 
2.17.0.921.gf22659ad46-goog
__

Re: [edk2] smm lock query

2018-05-28 Thread Laszlo Ersek

On 05/27/18 22:44, Andrew Fish wrote:
> 
> 
>> On May 27, 2018, at 9:47 AM, Marvin H?user  
>> wrote:
>>
>> Good day Abhishek,
>>
>> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect 
>> (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
>>
>> Strictly speaking you are, right, because of the description for the MM 
>> protocol:
>> "Indicates that MM resources and services that should not be used by the 
>> third party code are about[Marvin: (!)] to be locked."
>> Practically however, I don't see any issue with the current implementation. 
>> Code inside MMRAM is not affected directly by the lock, it is just notified.
>> However, either the code or the specification should be slightly updated to 
>> be in sync. A code update might require review of the caller assumptions, 
>> just to be sure.
>>
>> I have a different concern though and hope I'm actually overlooking 
>> something.
>> If I understand the code correctly, it is the Platform BDS that exposes the 
>> (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and lock SMM 
>> resources based on the event.
>> Because of latter being an event however, I don't think it is, or can be, 
>> guaranteed to be the last event group member executing.
>> When it is not the last, the "about to be locked" part is not true for any 
>> subsequent callbacks, that could actually be a risky break of the 
>> specification - if it is.
>> If it is a break of the specification, I can only think of letting Platform 
>> BDS expose an "internal" event group, which is only caught by PiSmmIpl, 
>> which then drives the actual SmmReadyToLock flow.
>> This would require updates to all platform trees and hence I would propose a 
>> temporary backwards-compatibility.
>>
>> Any comments? Did I overlook something (I hope)?
>>
> 
> Mavvin,
> 
> You are correct there is no guarantee of order in events. Thanks for cc'ing 
> the right folks, as I don't remember all the low level details...
> 
> In general the idea behind the MM code is it only comes from the platform, 
> then by definition that code should be aware when the platform was going to 
> lock MM. In a practical sense any MM module that had a depex evaluate to true 
> would have dispatched in DXE prior to BDS being launched. In general BDS is 
> the code that enumerates PCI and connects devices, thus there is no chance 
> for 3rd party software to run before that point in the boot. So in an 
> abstract sense that lock represents the end of DXE dispatch.

This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
installed by Plaform BDS before any non-platform binaries get a chance
to run. In terms of the current PlatformBootManagerLib interfaces, that
means the protocol should be installed from
PlatformBootManagerBeforeConsole() (as noted on the API declaration itself).

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

[edk2] 答复: CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

2018-05-28 Thread Fan Jeff

Hi,

The current implementation assumes CpuS3DataDxe was dispatched before 
CpuFeaturesDxe. I do not remember clearly why I made this assumption before. 
(It maybe only due to CpuS3DataDxe was just dispatched firstly on all my 
validation platforms.),

I agree this is one bug.  Simply, we could implement one AllocateAcpiCpuData() 
in DXE instance as PEI instance.

Thanks!

Jeff

From: edk2-devel  on behalf of Laszlo Ersek 

Sent: Friday, May 25, 2018 7:40:32 PM
To: Marvin Häuser
Cc: edk2-devel@lists.01.org; eric.d...@intel.com
Subject: Re: [edk2] CpuS3DataDxe / DxeRegisterCpuFeaturesLib dependency.

On 05/25/18 12:54, Marvin H?user wrote:
> Good day,
>
> While I was inspecting CpuS3DataDxe and the modules depending on its
> PCD PcdCpuS3DataAddress,

(Side remark: see e.g. the commit message on 92b87f1c8c0b, "OvmfPkg:
build CpuS3DataDxe for -D SMM_REQUIRE", 2015-11-30.)

> I noticed that DxeRegisterCpuFeaturesLib seemingly has an asserted
> dependency on the PCD being ready when it its executed. I did neither
> see a Depex entry, nor an event callback ensuring CpuS3DataDxe has
> been loaded, neither exposed by CpuS3DataDxe, nor consumed by this
> library.
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c#L211

"DxeRegisterCpuFeaturesLib.inf" has a depex on
"gEdkiiCpuFeaturesSetDoneGuid".

No module in the open source edk2 tree produces this protocol GUID, thus
I think this library instance is unusable without other, out-of-tree,
modules. I assume that one of those modules satisfies the dependency
somehow.

Note that CpuS3DataDxe is a platform driver [1]; it is possible that the
platform that includes DxeRegisterCpuFeaturesLib in a driver *also*
includes such a CpuS3DataDxe variant that populates the PCD and then
installs gEdkiiCpuFeaturesSetDoneGuid.

[1] I suggest reviewing the message of commit bfec5efa56ca
("UefiCpuPkg/CpuS3DataDxe: Add module to initialize ACPI_CPU_DATA for
S3", 2015-11-25).

In fact, the series that added "DxeRegisterCpuFeaturesLib.inf" (with the
depex mentioned above) *also* modified CpuS3DataDxe: see [2] and [3].

[2] 8b371e93f206 ("UefiCpuPkg/CpuS3DataDxe: Consume the existing
PcdCpuS3DataAddress", 2017-03-22)

[3] "[edk2] [PATCH 00/11] Add CPU features driver"
https://bugzilla.tianocore.org/show_bug.cgi?id=421
http://mid.mail-archive.com/20170309083553.6016-1-jeff.fan@intel.com

This suggests that there is an out-of-tree module that populates
PcdCpuS3DataAddress before *both* CpuS3DataDxe and
DxeRegisterCpuFeaturesLib access the PCD. For achieving this kind of
ordering, it would be enough for a driver to first populate the PCD, and
then install "gEfiMpServiceProtocolGuid", as both
"DxeRegisterCpuFeaturesLib.inf" and "CpuS3DataDxe.inf" depend on that.

> Is there anything I'm missing that ensures the execution of
> CpuS3DataDxe prior to executing the dependent code? If not, should
> there be a dummy protocol exposed? PiSmmCpuDxeSmm also retrieves this
> PCD, however safely quits when it has not been set. However, this
> could cause unexpected behavior when the PCD is set after this code
> has been executed. I did not notice any dependency satisfaction
> actions here either.

The ordering between CpuS3DataDxe and PiSmmCpuDxeSmm is safe; it's
orchestrated by Platform BDS. See commit 92b87f1c8c0b above.

> Furthermore, not directly related to this dependency issue, the DXE
> code obviously does not implement AllocateAcpiCpuData() entirely.

More precisely, the DXE code expects AllocateAcpiCpuData() never to be
called; i.e., when the common "RegisterCpuFeaturesLib.c" source file is
executed in DXE, the expectation is that it never reaches the call to
AllocateAcpiCpuData().

> Hence, the if-branch following its call, will either add another layer
> of firing ASSERTs, or it will plainly do nothing. Maybe it could be
> moved into the current AllocateAcpiCpuData() function and it be
> renamed accordingly?
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c#L526

Sorry, I don't understand your point -- CpuRegisterTableWriteWorker() is
used in both PEI and DXE, and it's implemented for the general case.
When it runs in DXE, the expectation is apparently that
AllocateAcpiCpuData() will never be needed / reached, hence the
ASSERT(FALSE) stub implementation for the latter, in
"DxeRegisterCpuFeaturesLib.c".

Oh wait, I think you mistyped your point. The "if" that you refer to
does not *follow* the call to AllocateAcpiCpuData(). It *precedes*
(guards) it. What the "if" follows is the PcdGet64() call, for
PcdCpuS3DataAddress. In DXE, that PcdGet64() is expected to return a
nonzero value, hence AllocateAcpiCpuData() is never called, and the
assertions about the return value of AllocateAcpiCpuData() are
irrelevant (unreached).

Thanks
Laszlo
___
edk2-devel mailing list
edk2-deve

Re: [edk2] [Patch 0/3] Use comparison logic to check UINTN parameter in GetBestLanguage API

2018-05-28 Thread Marvin H?user

Hey Star and Liming,

May I propose re-considering the introduction of a third named parameter to 
reflect the first Language passed?
This would 1) have the advantage that the BOOLEAN parameter can remain as is, 
which increases readability and
2) require at least two parameters related to the language list passed. Having 
to write "NULL, NULL" is way more obvious than just having to write "NULL" when 
(accidentally?) not passing any language.

And error caused by this change would be calls that do not pass an expected 
amount of parameters for the call to make sense.

Thanks,
Marvin.

> -Original Message-
> From: edk2-devel  On Behalf Of Zeng,
> Star
> Sent: Monday, May 28, 2018 9:54 AM
> To: Gao, Liming ; edk2-devel@lists.01.org
> Cc: Zeng, Star 
> Subject: Re: [edk2] [Patch 0/3] Use comparison logic to check UINTN
> parameter in GetBestLanguage API
> 
> Reviewed-by: Star Zeng 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Liming Gao
> Sent: Monday, May 28, 2018 3:31 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/3] Use comparison logic to check UINTN parameter
> in GetBestLanguage API
> 
> Liming Gao (3):
>   MdePkg UefiLib: Use comparison logic to check UINTN parameter
>   IntelFrameworkPkg UefiLib: Use comparison logic to check UINTN
> parameter
>   MdeModulePkg Variable: Use comparison logic to check UINTN parameter
> 
>  IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c| 6 +++---
>  MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 8
> 
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 8 
>  MdePkg/Library/UefiLib/UefiLib.c| 6 +++---
>  4 files changed, 14 insertions(+), 14 deletions(-)
> 
> --
> 2.8.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/3] Use comparison logic to check UINTN parameter in GetBestLanguage API

2018-05-28 Thread Zeng, Star

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Monday, May 28, 2018 3:31 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/3] Use comparison logic to check UINTN parameter in 
GetBestLanguage API

Liming Gao (3):
  MdePkg UefiLib: Use comparison logic to check UINTN parameter
  IntelFrameworkPkg UefiLib: Use comparison logic to check UINTN
parameter
  MdeModulePkg Variable: Use comparison logic to check UINTN parameter

 IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c| 6 +++---
 MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 8 
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 8 
 MdePkg/Library/UefiLib/UefiLib.c| 6 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.8.0.windows.1

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

[edk2] [Patch 0/3] Use comparison logic to check UINTN parameter in GetBestLanguage API

2018-05-28 Thread Liming Gao

Liming Gao (3):
  MdePkg UefiLib: Use comparison logic to check UINTN parameter
  IntelFrameworkPkg UefiLib: Use comparison logic to check UINTN
parameter
  MdeModulePkg Variable: Use comparison logic to check UINTN parameter

 IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c| 6 +++---
 MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 8 
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 8 
 MdePkg/Library/UefiLib/UefiLib.c| 6 +++---
 4 files changed, 14 insertions(+), 14 deletions(-)

-- 
2.8.0.windows.1

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

[edk2] [Patch 2/3] IntelFrameworkPkg UefiLib: Use comparison logic to check UINTN parameter

2018-05-28 Thread Liming Gao

Commit cb96e7d4f7afdbaef0706f9251ae479639d85a28 changes the input parameter
from BOOLEAN to UINTN. Its comparison logic should be updated.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c 
b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
index 1d71f47..819795b 100644
--- a/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
+++ b/IntelFrameworkPkg/Library/FrameworkUefiLib/UefiLib.c
@@ -1510,7 +1510,7 @@ GetBestLanguage (
 //
 // If in RFC 4646 mode, then determine the length of the first RFC 4646 
language code in Language
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   for (LanguageLength = 0; Language[LanguageLength] != 0 && 
Language[LanguageLength] != ';'; LanguageLength++);
 }
 
@@ -1525,7 +1525,7 @@ GetBestLanguage (
 //
 // In RFC 4646 mode, then Loop through all language codes in 
SupportedLanguages
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   //
   // Skip ';' characters in Supported
   //
@@ -1557,7 +1557,7 @@ GetBestLanguage (
 }
   }
 
-  if (Iso639Language) {
+  if (Iso639Language != 0) {
 //
 // If ISO 639 mode, then each language can only be tested once
 //
-- 
2.8.0.windows.1

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

[edk2] [Patch 3/3] MdeModulePkg Variable: Use comparison logic to check UINTN parameter

2018-05-28 Thread Liming Gao

Commit 180ac200da84785989443b06bcfa5db343c0bf7e changes the input parameter
from BOOLEAN to UINTN. Its comparison logic should be updated.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c | 8 
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c   | 8 
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c 
b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
index 8ad2c71..e37d0cd 100644
--- a/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
+++ b/MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariable.c
@@ -599,7 +599,7 @@ VariableGetBestLanguage (
 //
 // If in RFC 4646 mode, then determine the length of the first RFC 4646 
language code in Language
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   for (LanguageLength = 0; Language[LanguageLength] != 0 && 
Language[LanguageLength] != ';'; LanguageLength++);
 }
 
@@ -614,7 +614,7 @@ VariableGetBestLanguage (
 //
 // In RFC 4646 mode, then Loop through all language codes in 
SupportedLanguages
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   //
   // Skip ';' characters in Supported
   //
@@ -636,13 +636,13 @@ VariableGetBestLanguage (
 if (AsciiStrnCmp (Supported, Language, LanguageLength) == 0) {
   VA_END (Args);
 
-  Buffer = Iso639Language ? mVariableModuleGlobal->Lang : 
mVariableModuleGlobal->PlatformLang;
+  Buffer = (Iso639Language != 0) ? mVariableModuleGlobal->Lang : 
mVariableModuleGlobal->PlatformLang;
   Buffer[CompareLength] = '\0';
   return CopyMem (Buffer, Supported, CompareLength);
 }
   }
 
-  if (Iso639Language) {
+  if (Iso639Language != 0) {
 //
 // If ISO 639 mode, then each language can only be tested once
 //
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 5eff808..6073315 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -1588,7 +1588,7 @@ VariableGetBestLanguage (
 //
 // If in RFC 4646 mode, then determine the length of the first RFC 4646 
language code in Language
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   for (LanguageLength = 0; Language[LanguageLength] != 0 && 
Language[LanguageLength] != ';'; LanguageLength++);
 }
 
@@ -1603,7 +1603,7 @@ VariableGetBestLanguage (
 //
 // In RFC 4646 mode, then Loop through all language codes in 
SupportedLanguages
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   //
   // Skip ';' characters in Supported
   //
@@ -1625,13 +1625,13 @@ VariableGetBestLanguage (
 if (AsciiStrnCmp (Supported, Language, LanguageLength) == 0) {
   VA_END (Args);
 
-  Buffer = Iso639Language ? mVariableModuleGlobal->Lang : 
mVariableModuleGlobal->PlatformLang;
+  Buffer = (Iso639Language != 0) ? mVariableModuleGlobal->Lang : 
mVariableModuleGlobal->PlatformLang;
   Buffer[CompareLength] = '\0';
   return CopyMem (Buffer, Supported, CompareLength);
 }
   }
 
-  if (Iso639Language) {
+  if (Iso639Language != 0) {
 //
 // If ISO 639 mode, then each language can only be tested once
 //
-- 
2.8.0.windows.1

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

[edk2] [Patch 1/3] MdePkg UefiLib: Use comparison logic to check UINTN parameter

2018-05-28 Thread Liming Gao

Commit d2aafe1e410c80d1046f2d1e743055882ead8489 changes the input parameter
from BOOLEAN to UINTN. Its comparison logic should be updated.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Michael Kinney 
---
 MdePkg/Library/UefiLib/UefiLib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Library/UefiLib/UefiLib.c b/MdePkg/Library/UefiLib/UefiLib.c
index 23faa63..888519d 100644
--- a/MdePkg/Library/UefiLib/UefiLib.c
+++ b/MdePkg/Library/UefiLib/UefiLib.c
@@ -1538,7 +1538,7 @@ GetBestLanguage (
 //
 // If in RFC 4646 mode, then determine the length of the first RFC 4646 
language code in Language
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   for (LanguageLength = 0; Language[LanguageLength] != 0 && 
Language[LanguageLength] != ';'; LanguageLength++);
 }
 
@@ -1553,7 +1553,7 @@ GetBestLanguage (
 //
 // In RFC 4646 mode, then Loop through all language codes in 
SupportedLanguages
 //
-if (!Iso639Language) {
+if (Iso639Language == 0) {
   //
   // Skip ';' characters in Supported
   //
@@ -1585,7 +1585,7 @@ GetBestLanguage (
 }
   }
 
-  if (Iso639Language) {
+  if (Iso639Language != 0) {
 //
 // If ISO 639 mode, then each language can only be tested once
 //
-- 
2.8.0.windows.1

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

53 matches

Mail list logo