Re: [edk2] [PATCH] IntelFsp2Pkg: Skip loading Microcode if MicrocodeCodeSize is zero

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Yarlagadda, Satya P
> Sent: Tuesday, July 26, 2016 9:14 AM
> To: edk2-devel@lists.01.org
> Cc: Mudusuru, Giri P ; Yao, Jiewen
> 
> Subject: [PATCH] IntelFsp2Pkg: Skip loading Microcode if MicrocodeCodeSize
> is zero
> 
> During asm to Nasm conversion, we missed the code to skip loading the
> microcode and return success if the size is zero. Added additional check to
> report error if the microcode size is not zero but less than 2 kB.
> 
> Cc: Giri P Mudusuru 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Satya Yarlagadda 
> ---
>  IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> index 55ee85a..6e86023 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> @@ -172,7 +172,15 @@ ASM_PFX(LoadMicrocodeDefault):
> cmpeax, 0
> jz ParamError
> movesp, eax
> -
> +
> +   ; skip loading Microcode if the MicrocodeCodeSize is zero
> +   ; and report error if size is less than 2k
> +   moveax, dword [esp +
> LoadMicrocodeParams.MicrocodeCodeSize]
> +   cmpeax, 0
> +   jz Exit2
> +   cmpeax, 0800h
> +   jl ParamError
> +
> movesi, dword [esp +
> LoadMicrocodeParams.MicrocodeCodeAddr]
> cmpesi, 0
> jnzCheckMainHeader
> --
> 2.9.2.windows.1

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

Re: [edk2] [PATCH] BaseTools/toolsetup.bat: Fix bug caused by 'CONF_PATH' not defined

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, July 26, 2016 1:40 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Zhu, Yonghong
> ; Gao, Liming 
> Subject: [PATCH] BaseTools/toolsetup.bat: Fix bug caused by 'CONF_PATH'
> not defined
> 
> In batch script files, setting a variable in an 'if' block will only take
> effect after the 'if' block.
> 
> This commit fixes the issue of using the variable 'CONF_PATH' right after
> it is being set in an 'if' block.
> 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu 
> ---
>  BaseTools/toolsetup.bat | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/BaseTools/toolsetup.bat b/BaseTools/toolsetup.bat
> index c1ab9bc..a64938d 100755
> --- a/BaseTools/toolsetup.bat
> +++ b/BaseTools/toolsetup.bat
> @@ -162,14 +162,14 @@ if not defined WORKSPACE (
> 
>  if not defined CONF_PATH (
>set CONF_PATH=%WORKSPACE%\Conf
> +)
> 
> -  if NOT exist %CONF_PATH% (
> -if defined PACKAGES_PATH (
> -  for %%i IN (%PACKAGES_PATH%) DO (
> -if exist %%~fi\Conf (
> -  set CONF_PATH=%%i\Conf
> -  goto CopyConf
> -)
> +if NOT exist %CONF_PATH% (
> +  if defined PACKAGES_PATH (
> +for %%i IN (%PACKAGES_PATH%) DO (
> +  if exist %%~fi\Conf (
> +set CONF_PATH=%%i\Conf
> +goto CopyConf
>)
>  )
>)
> --
> 1.9.5.msysgit.0

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

[edk2] [PATCH] BaseTools/toolsetup.bat: Fix bug caused by 'CONF_PATH' not defined

[Hao Wu]

In batch script files, setting a variable in an 'if' block will only take
effect after the 'if' block.

This commit fixes the issue of using the variable 'CONF_PATH' right after
it is being set in an 'if' block.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 BaseTools/toolsetup.bat | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/BaseTools/toolsetup.bat b/BaseTools/toolsetup.bat
index c1ab9bc..a64938d 100755
--- a/BaseTools/toolsetup.bat
+++ b/BaseTools/toolsetup.bat
@@ -162,14 +162,14 @@ if not defined WORKSPACE (
 
 if not defined CONF_PATH (
   set CONF_PATH=%WORKSPACE%\Conf
+)
 
-  if NOT exist %CONF_PATH% (
-if defined PACKAGES_PATH (
-  for %%i IN (%PACKAGES_PATH%) DO (
-if exist %%~fi\Conf (
-  set CONF_PATH=%%i\Conf
-  goto CopyConf
-)
+if NOT exist %CONF_PATH% (
+  if defined PACKAGES_PATH (
+for %%i IN (%PACKAGES_PATH%) DO (
+  if exist %%~fi\Conf (
+set CONF_PATH=%%i\Conf
+goto CopyConf
   )
 )
   )
-- 
1.9.5.msysgit.0

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

Re: [edk2] USB Control Transfers

[Tian, Feng]

Hi, Ken

What I mean is:
For example, you initialize the buffer to full 0xFF. After control transfer 
complete, you can get the updated buffer. Then you can check the first byte to 
see if bType field is not equal to 3. If it's true, then it means you receive a 
valid HID report desc. (This thought assumes your usb device treat value 3 as 
reserved like HID spec said).

As for interface design, you can send your concern to USWG to see if there is 
other feedback.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ken 
Taylor
Sent: Tuesday, July 26, 2016 10:35 AM
To: Tian, Feng ; edk2-devel@lists.01.org
Subject: Re: [edk2] USB Control Transfers

Hi Feng,

Yes, I could initialize the buffer to 0xff or 0.  I am concerned that I would 
need to do the transaction twice to be sure I have the right length, however. 
The last byte(s) could be either 0 or 0xff, so how else could I be sure I have 
found the correct end of the transfer?

I am concerned that requesting the same report multiple times could break some 
devices.  I have seen such bad behavior in the past on other USB devices such 
as cheap USB flash drives.  I know that is USB mass storage and not HID, but 
I'm looking at a category of similarly cheap devices and shortcuts may have 
been taken.

If this is not in the UEFI specification, I would suggest that this is an error 
in the specification.  For example, even the C standard "fread" function 
returns how much data has been read independently from the buffer/request size, 
and this is a similar function in which the amount of data delivered may be 
less than the size of the buffer supplied.

Regards,
-Ken.

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: Monday, July 25, 2016 7:20 PM
To: Ken Taylor; edk2-devel@lists.01.org
Cc: Tian, Feng
Subject: RE: USB Control Transfers

Hi, Taylor

As far as I know, there is no UEFI interface to return actual transfer length 
for usb control transfer.

As for reusing Request parameter, UEFI spec doesn't say we need update the 
actual transfer length through this. So EDKII usb implementation doesn't update 
it.

For your case, is it possible to initialize your buffer with 0xFF and then 
check the validity of the return buffer through Format & Size field?

typedef struct {
  UINT16Format;
  UINT8 Size;
  UINT8 Type;
  UINT8 Tag;
  HID_DATA  Data;
} HID_ITEM;

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ken 
Taylor
Sent: Tuesday, July 26, 2016 9:23 AM
To: edk2-devel@lists.01.org
Subject: [edk2] USB Control Transfers

Hi All,

I am working on a USB HID device driver.  The devices I want to support aren't 
legacy (keyboard or mouse) and at least some of them don't appear to support 
HID descriptors.  I need to parse the first Report descriptor, but because of 
the lack of the HID descriptor, I can't know how long any of the report 
descriptors are going to be in advance of the transaction that requests the 
descriptor.

I have no problem fetching the first report descriptor using 
UsbIoControlTransfer with an extra large buffer to contain the report 
descriptor (just in case).  The devices I'm working on just fill that report 
buffer part way and stop at the end of the report descriptor.

Unfortunately, there doesn't seem to be any mechanism to determine how many 
bytes of data a device has actually transferred to the buffer during a control 
transfer after that transfer completes. The USB IO control transfer function 
takes UINTN DataLength, not a pointer to DataLength.  I might expect that the 
UsbIo protocol would patch the Length field in the EFI_USB_DEVICE_REQUEST 
structure to reflect the actual transfer length after the fact, but that 
doesn't seem to happen either.

Am I overlooking some hidden interface here, or am I really going to be forced 
to use trial reads and buffer scanning to determine how much of the buffer 
pointed to by "Data" is actually filled by the device during a control 
transfer?  This is really bugging me, as I know there's a mechanism for host 
controllers to report this information... I've written USB host controller and 
bus drivers for other BIOS implementations, and I remember this being possible.

For reference, here's the definition of the control transfer function from the 
UsbIo.h protocol definition:

/**
  This function is used to manage a USB device with a control transfer pipe. A 
control transfer is
  typically used to perform device initialization and configuration.

  @param  This  A pointer to the EFI_USB_IO_PROTOCOL instance.
  @param  Request   A pointer to the USB device request that will 
be sent to the USB
device.
  @param  Direction Indicates the data direction.
  @param  Timeout   Indicating the transfer should be completed 
within this time frame.
 

Re: [edk2] [patch] MdeModulePkg: add generic SataController driver.

[Mudusuru, Giri P]

Reviewed-by: Giri P Mudusuru  

> -Original Message-
> From: Chan, Amy
> Sent: Monday, July 25, 2016 8:14 PM
> To: Tian, Feng ; Kinney, Michael D
> ; Mudusuru, Giri P 
> Cc: edk2-devel@lists.01.org
> Subject: RE: [patch] MdeModulePkg: add generic SataController driver.
> 
> Reviewed-by: Chan, Amy 
> 
> Thanks
> Amy
> 
> > -Original Message-
> > From: Tian, Feng
> > Sent: Wednesday, July 20, 2016 1:07 PM
> > To: Kinney, Michael D ; Mudusuru, Giri P
> > ; Chan, Amy 
> > Cc: edk2-devel@lists.01.org
> > Subject: [patch] MdeModulePkg: add generic SataController driver.
> >
> > This SataController driver is used to provide a generic solution
> > for those IDE/SATA/AHCI spec compliance host controllers.
> >
> > If there is special reqirement on timing setting or something else,
> > user could override this driver to customize a specific one.
> >
> > Cc: Michael Kinney 
> > Cc: Amy Chan 
> > Cc: Giri P Mudusuru 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Feng Tian 
> > ---
> >  .../Bus/Pci/SataControllerDxe/ComponentName.c  |  177 
> >  .../Bus/Pci/SataControllerDxe/SataController.c | 1021
> > 
> >  .../Bus/Pci/SataControllerDxe/SataController.h |  536 ++
> >  .../Pci/SataControllerDxe/SataControllerDxe.inf|   58 ++
> >  .../Pci/SataControllerDxe/SataControllerDxe.uni|   22 +
> >  .../SataControllerDxe/SataControllerDxeExtra.uni   |   20 +
> >  MdeModulePkg/MdeModulePkg.dsc  |1 +
> >  7 files changed, 1835 insertions(+)
> >  create mode 100644
> > MdeModulePkg/Bus/Pci/SataControllerDxe/ComponentName.c
> >  create mode 100644
> > MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
> >  create mode 100644
> > MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.h
> >  create mode 100644
> > MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
> >  create mode 100644
> > MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.uni
> >  create mode 100644
> > MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxeExtra.uni
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/ComponentName.c
> > b/MdeModulePkg/Bus/Pci/SataControllerDxe/ComponentName.c
> > new file mode 100644
> > index 000..467ad11
> > --- /dev/null
> > +++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/ComponentName.c
> > @@ -0,0 +1,177 @@
> > +/** @file
> > +  UEFI Component Name(2) protocol implementation for Sata Controller
> > driver.
> > +
> > +  Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
> > +  This program and the accompanying materials
> > +  are licensed and made available under the terms and conditions of the BSD
> > License
> > +  which accompanies this distribution.  The full text of the license may be
> > found at
> > +  http://opensource.org/licenses/bsd-license.php.
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> > BASIS,
> > +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > EXPRESS OR IMPLIED.
> > +
> > +**/
> > +
> > +#include "SataController.h"
> > +
> > +//
> > +/// EFI Component Name Protocol
> > +///
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_COMPONENT_NAME_PROTOCOL
> > gSataControllerComponentName = {
> > +  SataControllerComponentNameGetDriverName,
> > +  SataControllerComponentNameGetControllerName,
> > +  "eng"
> > +};
> > +
> > +//
> > +/// EFI Component Name 2 Protocol
> > +///
> > +GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_COMPONENT_NAME2_PROTOCOL
> > gSataControllerComponentName2 = {
> > +  (EFI_COMPONENT_NAME2_GET_DRIVER_NAME)
> > SataControllerComponentNameGetDriverName,
> > +  (EFI_COMPONENT_NAME2_GET_CONTROLLER_NAME)
> > SataControllerComponentNameGetControllerName,
> > +  "en"
> > +};
> > +
> > +//
> > +/// Driver Name Strings
> > +///
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE
> > mSataControllerDriverNameTable[] = {
> > +  {
> > +"eng;en",
> > +(CHAR16 *)L"Sata Controller Init Driver"
> > +  },
> > +  {
> > +NULL,
> > +NULL
> > +  }
> > +};
> > +
> > +///
> > +/// Controller Name Strings
> > +///
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_UNICODE_STRING_TABLE
> > mSataControllerControllerNameTable[] = {
> > +  {
> > +"eng;en",
> > +(CHAR16 *)L"Sata Controller"
> > +  },
> > +  {
> > +NULL,
> > +NULL
> > +  }
> > +};
> > +
> > +/**
> > +  Retrieves a Unicode string that is the user readable name of the UEFI 
> > Driver.
> > +
> > +  @param This   A pointer to the
> > EFI_COMPONENT_NAME_PROTOCOL instance.
> > +  @param Language   A pointer to a three character ISO 639-2
> > language identifier.
> > +This is the language of the driver name 
> > that that the
> > caller
> > +is requesting, and it must match one of 
> > the languages
> > specified
> > +in SupportedLanguages.  The number of 
> > languages
> > supported by a
> > + 

Re: [edk2] [Patch] BaseTools: report error if source module INF is only list in FDF file

[Gao, Liming]

Reviewed-by: Liming Gao 

-Original Message-
From: Zhu, Yonghong 
Sent: Wednesday, July 20, 2016 2:16 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [Patch] BaseTools: report error if source module INF is only list in 
FDF file

If source module INF is not listed in DSC, it will not be built. And it
is listed in FDF, GenFds will fail to find its build output. To reminder
user this issue early, build tool should report failure to user in early
phase.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py  | 28 
 BaseTools/Source/Python/GenFds/FdfParser.py | 15 +++
 2 files changed, 43 insertions(+)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 8da441f..9a95014 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -327,10 +327,38 @@ class WorkspaceAutoGen(AutoGen):
 self.FdfProfile = Fdf.Profile
 for fvname in self.FvTargetList:
 if fvname.upper() not in self.FdfProfile.FvDict:
 EdkLogger.error("build", OPTION_VALUE_INVALID,
 "No such an FV in FDF file: %s" % fvname)
+
+for key in self.FdfProfile.InfDict:
+if key == 'ArchTBD':
+Platform_cache = {}
+for Arch in self.ArchList:
+Platform_cache[Arch] = 
self.BuildDatabase[self.MetaFile, Arch, Target, Toolchain]
+for Inf in self.FdfProfile.InfDict[key]:
+ModuleFile = PathClass(NormPath(Inf), 
GlobalData.gWorkspace, Arch)
+for Arch in self.ArchList:
+if ModuleFile in Platform_cache[Arch].Modules:
+break
+else:
+ModuleData = self.BuildDatabase[ModuleFile, Arch, 
Target, Toolchain]
+if not ModuleData.IsBinaryModule:
+EdkLogger.error('build', PARSER_ERROR, "Module 
%s NOT found in DSC file; Is it really a binary module?" % ModuleFile)
+
+else:
+for Arch in self.ArchList:
+if Arch == key:
+Platform = self.BuildDatabase[self.MetaFile, Arch, 
Target, Toolchain]
+for Inf in self.FdfProfile.InfDict[key]:
+ModuleFile = PathClass(NormPath(Inf), 
GlobalData.gWorkspace, Arch)
+if ModuleFile in Platform.Modules:
+continue
+ModuleData = self.BuildDatabase[ModuleFile, 
Arch, Target, Toolchain]
+if not ModuleData.IsBinaryModule:
+EdkLogger.error('build', PARSER_ERROR, 
"Module %s NOT found in DSC file; Is it really a binary module?" % ModuleFile)
+
 else:
 PcdSet = {}
 ModuleList = []
 self.FdfProfile = None
 if self.FdTargetList:
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 56303e1..8709cfc 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -229,10 +229,11 @@ class FileProfile :
 EdkLogger.error("FdfParser", FILE_OPEN_FAILURE, ExtraData=FileName)
 
 
 self.PcdDict = {}
 self.InfList = []
+self.InfDict = {'ArchTBD':[]}
 # ECC will use this Dict and List information
 self.PcdFileLineDict = {}
 self.InfFileLineList = []
 
 self.FdDict = {}
@@ -2470,10 +2471,17 @@ class FdfParser:
 
 if not ffsInf.InfFileName in self.Profile.InfList:
 self.Profile.InfList.append(ffsInf.InfFileName)
 FileLineTuple = GetRealFileLine(self.FileName, 
self.CurrentLineNumber)
 self.Profile.InfFileLineList.append(FileLineTuple)
+if ffsInf.UseArch:
+if ffsInf.UseArch not in self.Profile.InfDict:
+self.Profile.InfDict[ffsInf.UseArch] = [ffsInf.InfFileName]
+else:
+
self.Profile.InfDict[ffsInf.UseArch].append(ffsInf.InfFileName)
+else:
+self.Profile.InfDict['ArchTBD'].append(ffsInf.InfFileName)
 
 if self.__IsToken('|'):
 if self.__IsKeyword('RELOCS_STRIPPED'):
 ffsInf.KeepReloc = False
 elif self.__IsKeyword('RELOCS_RETAINED'):
@@ -4349,10 +4357,17 @@ class FdfParser:
 
 if not ffsInf.InfFileName in self.Profile.InfList:
 self.Profile.InfList.append(ffsInf.InfFileName)
 FileLineTuple = GetRealFileLine(self.FileName, 
self.CurrentLi

Re: [edk2] [Patch v3 34/40] OvmfPkg: Add MpInitLib reference in DSC files.

[Fan, Jeff]

Laszlo,

I am using TortoiseGit to create the patches. I can see the patch only includes 
the diff of DSC file as below in .patch file. I attached it for your reference.

@@ -213,6 +213,7 @@
   DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/SecPeiDebugAgentLib.inf
 !endif
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
+  MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -292,6 +293,7 @@
   DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
+  MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf

If you cannot see the diff part, what did you see in this patch through your 
mail application?

Thanks!
Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 11:44 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Justen, Jordan L; Tian, Feng
Subject: Re: [edk2] [Patch v3 34/40] OvmfPkg: Add MpInitLib reference in DSC 
files.

On 07/25/16 04:52, Jeff Fan wrote:
> This update is for CpuMpPei&CpuDxe consuming MP Initialize library.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  OvmfPkg/OvmfPkgIa32.dsc| 2 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
>  OvmfPkg/OvmfPkgX64.dsc | 2 ++
>  3 files changed, 6 insertions(+)

Please update your git configuration according to the following:

*
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05

  git config diff.ini.xfuncname '^\[[A-Za-z0-9_., ]+]'

* and
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-09

i.e., please add the following to " .git/info/attributes":

*.dec diff=ini
*.dsc diff=ini
*.dsc.inc diff=ini
*.fdf diff=ini
*.fdf.inc diff=ini
*.inf diff=ini

If you do that, then I'll be able to see from the patch itself what sections in 
the DSC file are being modified.

Thanks
Laszlo

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

Re: [edk2] [Patch v3 06/40] UefiCpuPkg/MpInitLib: Add AP assembly code and MP_CPU_EXCHANGE_INFO

[Fan, Jeff]

Laszlo,

I added my comments after [Jeff] for your each comment.

Thanks!
Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 11:33 PM
To: Fan, Jeff
Cc: edk2-de...@ml01.01.org; Kinney, Michael D; Tian, Feng
Subject: Re: [edk2] [Patch v3 06/40] UefiCpuPkg/MpInitLib: Add AP assembly code 
and MP_CPU_EXCHANGE_INFO

Jeff,

On 07/25/16 04:52, Jeff Fan wrote:
> Add assembly code for AP reset vector and the definition of 
> MP_CPU_EXCHANGE_INFO that are used to exchange the data between C code 
> and assembly code when AP wake up.
> 
> v3:
>   1. Rename NumApsExecutingLoction to NumApsExecutingLocation
>   2. Add whitespace after ; in .nasm file
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf  |   8 ++
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc|  37 +
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 170 +++
>  UefiCpuPkg/Library/MpInitLib/MpLib.h   |  24 
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf  |   8 ++
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc |  39 ++
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 178 
> +
>  7 files changed, 464 insertions(+)
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm

Patches like this (= patches that move code around, possibly tweaking
it) are extremely hard to review without dedicated help from the submitter. 
Such as:

- when copying a structure definition from another file, please spell
  out the origin of the structure definition. Example:
  MP_CPU_EXCHANGE_INFO comes from "UefiCpuPkg/CpuMpPei/CpuMpPei.h" with
  minimal changes.

- The patch should be please formatted with "--find-copies-harder". This
  will cause git to identify the best source file that may be the origin
  for a copy, and then express the new file as (copy + diff), rather
  than brand new code.

For example, I can look at this patch
(c661d4b85b748f03ed3e47c2b8424f797bdc0aaf) using git directly, from your repo, 
with the following options:

  git show --find-copies-harder --stat=1000 --stat-graph-width=20 \
c661d4b85b748f03ed3e47c2b8424f797bdc0aaf

It produces the following diffstat:

 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf|   8 ++
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Ia32/MpEqu.inc|   8 +-
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Ia32/MpFuncs.nasm | 129 
+-
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  24 
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf|   8 ++
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/X64/MpEqu.inc |  10 +-
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/X64/MpFuncs.nasm  | 141 
+++-
 7 files changed, 101 insertions(+), 227 deletions(-)

This tells me where the new files come from, and I can focus on the differences 
only. Otherwise, I have to review all of the code as brand new code, which is 
*immensely* more work.

Now, using the --find-copies-harder switch, I can see a number of unjustified 
differences immediately:

- "NumApsExecutingLoction" is replaced with "NumApsExecutingLocation".
  This is a typo fix, and should be in a separate patch.

- EnableExecuteDisableLocation and Cr3Location are new. The commit
  message should describe these fields in detail, and why they are
  necessary. (In practice, how we differ from CpuMpPei here.)

- The whitespace after ";" is actually a bad idea for this patch. It is
  in the same category as typo fixes: they can be justified, of course,
  but only as separate patches, so that they aren't intermixed with
  semantic changes.

- I see a few more changes of the same class (invoke -> Invoke, never ->
  Never)

I'm not a UefiCpuPkg maintainer, so technically my review is not necessary 
here. If you CC'd me so that I *test* the series, I'm happy to do it, and 
report back with results. However, if you'd like me to
*review* the series, then I'll need a *lot* more detail in the commit messages 
-- in practice you'll have to document your entire thought process. Every 
decision that is based on your knowledge with CpuMpPei and CpuMpDxe (which 
could be obvious to other UefiCpuPkg maintainers) will have to be spelled out 
for me in the commit messages.
[Jeff] I think you provided great comments to improve this serial of patches. 
Thanks! 
 I could try to use something like (copy+diff) to refine the patch.

Do you want me to review the series, or are you okay if I just test it?
[Jeff] Sure on both.:-)

Thanks
Laszlo
___
edk2-deve

Re: [edk2] [Patch 4/8] MdeModulePkg UefiBootManagerLib: Enhance EfiBootManagerFindLoadOption

[Ni, Ruiyu]

Liming,
Can you modify the patch to avoid changing the behavior of 
EfiBootManagerFindLoadOption()?

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Liming Gao
> Sent: Monday, July 25, 2016 10:00 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: [edk2] [Patch 4/8] MdeModulePkg UefiBootManagerLib: Enhance
> EfiBootManagerFindLoadOption
> 
> The same boot file may have two different device paths. One is its FV File
> device path, another is LoadFile FV device path. These two paths includes the
> same NameGuid. So, EfiBootManagerFindLoadOption() is updated to
> compare their NameGuid.
> 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  .../Library/UefiBootManagerLib/BmLoadOption.c  | 44
> --
>  1 file changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> index 9af98de..f9650ff 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
> @@ -506,6 +506,29 @@ EfiBootManagerInitializeLoadOption (
>return EFI_SUCCESS;
>  }
> 
> +/**
> +  Find NameGuid from the input device path.
> +
> +  @param  DevicePath Input device path.
> +
> +  @retval NULL  NameGuid can't be found in the input device path.
> +  @retval NameGuid  Pointer to NameGuid found in the input device path.
> +**/
> +EFI_GUID *
> +BmFileNameGuidFromFilePath (
> +  EFI_DEVICE_PATH_PROTOCOL *DevicePath
> +)
> +{
> +  EFI_HANDLE  FvHandle;
> +  EFI_STATUS  Status;
> +
> +  Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid,
> + &DevicePath, &FvHandle);  if (!EFI_ERROR (Status)) {
> +return EfiGetNameGuidFromFwVolDevicePathNode ((CONST
> + MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *) DevicePath);  }
> +
> +  return NULL;
> +}
> 
>  /**
>Return the index of the load option in the load option array.
> @@ -529,15 +552,30 @@ EfiBootManagerFindLoadOption (
>)
>  {
>UINTN Index;
> +  EFI_GUID  *KeyNameGuid;
> +  EFI_GUID  *NameGuid;
> +
> +  KeyNameGuid = NULL;
> +  if (Key->OptionType == LoadOptionTypeBoot) {
> +KeyNameGuid = BmFileNameGuidFromFilePath (Key->FilePath);  }
> 
>for (Index = 0; Index < Count; Index++) {
> -if ((Key->OptionType == Array[Index].OptionType) &&
> -(Key->Attributes == Array[Index].Attributes) &&
> +if (Key->OptionType == Array[Index].OptionType) {
> +  if ((Key->Attributes == Array[Index].Attributes) &&
>  (StrCmp (Key->Description, Array[Index].Description) == 0) &&
>  (CompareMem (Key->FilePath, Array[Index].FilePath,
> GetDevicePathSize (Key->FilePath)) == 0) &&
>  (Key->OptionalDataSize == Array[Index].OptionalDataSize) &&
>  (CompareMem (Key->OptionalData, Array[Index].OptionalData, Key-
> >OptionalDataSize) == 0)) {
> -  return (INTN) Index;
> +return (INTN) Index;
> +  }
> +
> +  if (KeyNameGuid != NULL) {
> +NameGuid = BmFileNameGuidFromFilePath (Array[Index].FilePath);
> +if ((NameGuid != NULL) && CompareGuid (KeyNameGuid, NameGuid))
> {
> +  return (INTN) Index;
> +}
> +  }
>  }
>}
> 
> --
> 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

Re: [edk2] [Patch v3 05/40] UefiCpuPkg/MpInitLib: Add two instances PeiMpInitLib and DxeMpInitLib

[Fan, Jeff]

Laszlo,

I added my comments after [Jeff] for your each comment.

Thanks!
Jeff
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Monday, July 25, 2016 10:27 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Tian, Feng
Subject: Re: [edk2] [Patch v3 05/40] UefiCpuPkg/MpInitLib: Add two instances 
PeiMpInitLib and DxeMpInitLib

On 07/25/16 04:52, Jeff Fan wrote:
> Add two MP Initialize Library instances PeiMpInitLib.inf and 
> DxeMpInitLib.inf with NULL implementation.
> 
> v3:
>   1. Rename MpInitLibSwitchBsp to MpInitLibSwitchBSP to match PI spec
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  61 ++  
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.uni |  22 +++
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   | 266 +
>  UefiCpuPkg/Library/MpInitLib/MpLib.c  | 119 
>  UefiCpuPkg/Library/MpInitLib/MpLib.h  |  40 
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  61 ++  
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.uni |  22 +++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 269 
> ++
>  UefiCpuPkg/UefiCpuPkg.dsc |   3 +-
>  9 files changed, 862 insertions(+), 1 deletion(-)  create mode 100644 
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.uni
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLib.c
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLib.h
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.uni
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c

(1) A Null implementation that does not fulfill the promises of the lib class 
interfaces should not invariably return EFI_SUCCESS in my opinion.
Instead, the MpInitLibInitialize() function should return EFI_UNSUPPORTED, and 
all other APIs should return EFI_NOT_READY.

I realize this is purely cosmetic as the implementation will change in the rest 
of the series.
[Jeff] Agree to use EFI_UNSUPPORTED.

(2) I see that some, but not all, of the APIs are kept in the common
(shared) code, while the rest is specialized to PEI vs. DXE. I understand that 
this split lays the foundation for the future implementation, but it should be 
documented in the commit message -- in particular it should be announced in 
this patch *why* the functions are distributed like this.
[Jeff] Agree to add comments in commit log to make a declaration on it.

(3) Once the comment / naming issues are fixed in the library class header file 
that I mentioned earlier, this patch should be adapted.

(In fact I wouldn't mind if these library instances omitted those huge comments 
altogether. I know it wouldn't conform to the edk2 coding style, but comment 
blocks of *this* size just make it harder to read the code; and the mindless 
copying just perpetuates errors in the comments.
This patch adds 800+ lines of code, without any functionality at all.)
[Jeff] I think I could simply the functions header size for those internal 
functions. But for external APIs, I prefer to keep the comments.

I think (2) is quite important. The other two comments are not crazy important.

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

Re: [edk2] [Patch v3 04/40] UefiCpuPkg/MpInitLib: Add MP Initialize library class definition

[Fan, Jeff]

Laszlo,

I added my comments after [Jeff] for your each comment.

Thanks!
Jeff
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 9:53 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Tian, Feng
Subject: Re: [edk2] [Patch v3 04/40] UefiCpuPkg/MpInitLib: Add MP Initialize 
library class definition

On 07/25/16 04:52, Jeff Fan wrote:
> MP Initialize library provides basic functionalities to do APs 
> initialization, to manage MP information and to wakeup APs to execute AP task.
> 
> It could be consumed by CPU MP PEI or DXE drivers to provide CPU MP 
> PPI/Protocol services.
> 
> v3:
>   1. Add whitespace after MpInitLibInitialize
>   2. Rename MpInitLibSwitchBsp to MpInitLibSwitchBSP to match PI spec
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/Include/Library/MpInitLib.h | 352 
> +
>  UefiCpuPkg/UefiCpuPkg.dec  |   4 +
>  2 files changed, 356 insertions(+)
>  create mode 100644 UefiCpuPkg/Include/Library/MpInitLib.h

(1) I guess "MpServicesLib" would be a better library class name, but it's not 
a big issue.

If you decide to change the name, then a number of other locations have to be 
changed as well (for example, the include guard).
[Jeff]  I agree MpServicesLib is good name. 
  Actually,  we have one plan to add one new library based on MP 
Services PPI/Protocols to provide basic MP functions. MpServiceLib is better 
for this module and located at MdePkg.
  So, MpServciesLib could be used by Pei/Dxe module/library that want 
to consume MP PPI/Protocol services.
  MpInitLib will be used by CpuPei/Dxe module that want to produce MP 
PPI/Protocol services.

> diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h 
> b/UefiCpuPkg/Include/Library/MpInitLib.h
> new file mode 100644
> index 000..4ae02fb
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Library/MpInitLib.h
> @@ -0,0 +1,352 @@
> +/** @file
> +  Multiple-Processor initialization Library.
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.  
> + This program and the accompanying materials  are licensed and made 
> + available under the terms and conditions of the BSD License  which 
> + accompanies this distribution.  The full text of the license may be 
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" 
> + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS 
> OR IMPLIED.
> +
> +**/
> +
> +#ifndef __MP_INIT_LIB_H__
> +#define __MP_INIT_LIB_H__
> +
> +#include 
> +
> +/**
> +  MP Initialize Library initialization.
> +
> +  This service will allocate AP reset vector and wakeup all APs to do 
> + APs  initialization.
> +
> +  This service must be invoked before all other MP Initialize Library  
> + service are invoked.
> +
> +  @retval  EFI_SUCCESS   MP initialization succeeds.
> +  @retval  OthersMP initialization fails.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MpInitLibInitialize (
> +  VOID
> +  );
> +
> +/**
> +  Retrieves the number of logical processor in the platform and the 
> +number of
> +  those logical processors that are enabled on this boot. This 
> +service may only
> +  be called from the BSP.
> +
> +  @param[out] NumberOfProcessors  Pointer to the total number of 
> logical
> +  processors in the system, 
> including the BSP
> +  and disabled APs.
> +  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled 
> logical
> +  processors that exist in system, 
> including
> +  the BSP.
> +
> +  @retval EFI_SUCCESS The number of logical processors and 
> enabled 
> +  logical processors was retrieved.
> +  @retval EFI_DEVICE_ERRORThe calling processor is an AP.
> +  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and 
> NumberOfEnabledProcessors
> +  is NULL.

(2) "and" should be "or"
[Jeff] It should be "and".  MpInitLib allow each of parameter is NULL.

> +  @retval EFI_NOT_READY   MP Initialize Library is not initialized.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MpInitLibGetNumberOfProcessors (
> +  OUT UINTN *NumberOfProcessors,   OPTIONAL
> +  OUT UINTN *NumberOfEnabledProcessors OPTIONAL
> +  );
> +
> +/**
> +  Gets detailed MP-related information on the requested processor at 
> +the
> +  instant this call is made. This service may only be called from the BSP.
> +
> +  @param[in]  ProcessorNumber   The handle number of processor.
> +  @param[out] ProcessorInfoBuf

Re: [edk2] USB Control Transfers

[Ken Taylor]

Hi Feng,

Yes, I could initialize the buffer to 0xff or 0.  I am concerned that I would 
need to do the transaction twice to be sure I have the right length, however. 
The last byte(s) could be either 0 or 0xff, so how else could I be sure I have 
found the correct end of the transfer?

I am concerned that requesting the same report multiple times could break some 
devices.  I have seen such bad behavior in the past on other USB devices such 
as cheap USB flash drives.  I know that is USB mass storage and not HID, but 
I'm looking at a category of similarly cheap devices and shortcuts may have 
been taken.

If this is not in the UEFI specification, I would suggest that this is an error 
in the specification.  For example, even the C standard "fread" function 
returns how much data has been read independently from the buffer/request size, 
and this is a similar function in which the amount of data delivered may be 
less than the size of the buffer supplied.

Regards,
-Ken.

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: Monday, July 25, 2016 7:20 PM
To: Ken Taylor; edk2-devel@lists.01.org
Cc: Tian, Feng
Subject: RE: USB Control Transfers

Hi, Taylor

As far as I know, there is no UEFI interface to return actual transfer length 
for usb control transfer.

As for reusing Request parameter, UEFI spec doesn't say we need update the 
actual transfer length through this. So EDKII usb implementation doesn't update 
it.

For your case, is it possible to initialize your buffer with 0xFF and then 
check the validity of the return buffer through Format & Size field?

typedef struct {
  UINT16Format;
  UINT8 Size;
  UINT8 Type;
  UINT8 Tag;
  HID_DATA  Data;
} HID_ITEM;

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ken 
Taylor
Sent: Tuesday, July 26, 2016 9:23 AM
To: edk2-devel@lists.01.org
Subject: [edk2] USB Control Transfers

Hi All,

I am working on a USB HID device driver.  The devices I want to support aren't 
legacy (keyboard or mouse) and at least some of them don't appear to support 
HID descriptors.  I need to parse the first Report descriptor, but because of 
the lack of the HID descriptor, I can't know how long any of the report 
descriptors are going to be in advance of the transaction that requests the 
descriptor.

I have no problem fetching the first report descriptor using 
UsbIoControlTransfer with an extra large buffer to contain the report 
descriptor (just in case).  The devices I'm working on just fill that report 
buffer part way and stop at the end of the report descriptor.

Unfortunately, there doesn't seem to be any mechanism to determine how many 
bytes of data a device has actually transferred to the buffer during a control 
transfer after that transfer completes. The USB IO control transfer function 
takes UINTN DataLength, not a pointer to DataLength.  I might expect that the 
UsbIo protocol would patch the Length field in the EFI_USB_DEVICE_REQUEST 
structure to reflect the actual transfer length after the fact, but that 
doesn't seem to happen either.

Am I overlooking some hidden interface here, or am I really going to be forced 
to use trial reads and buffer scanning to determine how much of the buffer 
pointed to by "Data" is actually filled by the device during a control 
transfer?  This is really bugging me, as I know there's a mechanism for host 
controllers to report this information... I've written USB host controller and 
bus drivers for other BIOS implementations, and I remember this being possible.

For reference, here's the definition of the control transfer function from the 
UsbIo.h protocol definition:

/**
  This function is used to manage a USB device with a control transfer pipe. A 
control transfer is
  typically used to perform device initialization and configuration.

  @param  This  A pointer to the EFI_USB_IO_PROTOCOL instance.
  @param  Request   A pointer to the USB device request that will 
be sent to the USB
device.
  @param  Direction Indicates the data direction.
  @param  Timeout   Indicating the transfer should be completed 
within this time frame.
The units are in milliseconds.
  @param  Data  A pointer to the buffer of data that will be 
transmitted to USB
device or received from USB device.
  @param  DataLengthThe size, in bytes, of the data buffer 
specified by Data.
  @param  StatusA pointer to the result of the USB transfer.

  @retval EFI_SUCCESS   The control transfer has been successfully 
executed.
  @retval EFI_DEVICE_ERROR  The transfer failed. The transfer status is 
returned in Status.
  @retval EFI_INVALID_PARAMETE  One or more parameters are invalid.
  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a 
la

Re: [edk2] [PATCH v3] [BaseTools/Scripts]: Preserve hii section in GCC binaries

[Zhu, Yonghong]

Pushed it 03630a81488ca30c384c09034caa1a70afb80a0d 

Best Regards,
Zhu Yonghong


-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Friday, July 22, 2016 3:34 PM
To: Thomas Palmer ; edk2-de...@ml01.01.org
Cc: Zhu, Yonghong ; Gao, Liming ; 
af...@apple.com; joseph.shiffl...@hpe.com; Bruce Cran 
Subject: Re: [PATCH v3] [BaseTools/Scripts]: Preserve hii section in GCC 
binaries

On 07/22/16 04:56, Thomas Palmer wrote:
> According to UEFI spec:
> Once an image is loaded, LoadImage() installs 
> EFI_HII_PACKAGE_LIST_PROTOCOL on the handle if the image contains a 
> custom PE/COFF resource with the type 'HII'. The protocol's interface 
> pointer points to the HII package list which is contained in the 
> resource's data.
> 
> This is controlled by the UEFI_HII_RESOURCE_SECTION define in the INF 
> file.  When present the HII resource is linked with the module binary.
> 
> Unfortunately GCC-built binaries have been stripping the .hii section 
> entirely.  See  "[edk2] HII gEfiHiiPackageListProtocolGuid problem 
> with  GCC48(VS2012x86 works)"
> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/13438
> http://thread.gmane.org/gmane.comp.bios.tianocore.devel/14899
> 
> This patch tells the linker to preserve the .hii sections
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  BaseTools/Scripts/GccBase.lds | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/BaseTools/Scripts/GccBase.lds 
> b/BaseTools/Scripts/GccBase.lds index 32310bc..7e4cdde 100644
> --- a/BaseTools/Scripts/GccBase.lds
> +++ b/BaseTools/Scripts/GccBase.lds
> @@ -4,6 +4,7 @@
>  
>Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
>Copyright (c) 2015, Linaro Ltd. All rights reserved.
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  
>This program and the accompanying materials are licensed and made 
> available under
>the terms and conditions of the BSD License that accompanies this 
> distribution.
> @@ -57,6 +58,10 @@ SECTIONS {
>  *(.rela .rela.*)
>}
>  
> +  .hii : ALIGN(CONSTANT(COMMONPAGESIZE)) {
> +KEEP (*(.hii))
> +  }
> +
>/DISCARD/ : {
>  *(.note.GNU-stack)
>  *(.gnu_debuglink)
> 

Acked-by: Laszlo Ersek 

Please also add the following tags from v2 when committing this patch, because 
the code has not changed, and the commit message changed only to add more 
information:

Reviewed-by: Bruce Cran 
Tested-by: Bruce Cran 

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

Re: [edk2] USB Control Transfers

[Tian, Feng]

Hi, Taylor

As far as I know, there is no UEFI interface to return actual transfer length 
for usb control transfer.

As for reusing Request parameter, UEFI spec doesn't say we need update the 
actual transfer length through this. So EDKII usb implementation doesn't update 
it.

For your case, is it possible to initialize your buffer with 0xFF and then 
check the validity of the return buffer through Format & Size field?

typedef struct {
  UINT16Format;
  UINT8 Size;
  UINT8 Type;
  UINT8 Tag;
  HID_DATA  Data;
} HID_ITEM;

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ken 
Taylor
Sent: Tuesday, July 26, 2016 9:23 AM
To: edk2-devel@lists.01.org
Subject: [edk2] USB Control Transfers

Hi All,

I am working on a USB HID device driver.  The devices I want to support aren't 
legacy (keyboard or mouse) and at least some of them don't appear to support 
HID descriptors.  I need to parse the first Report descriptor, but because of 
the lack of the HID descriptor, I can't know how long any of the report 
descriptors are going to be in advance of the transaction that requests the 
descriptor.

I have no problem fetching the first report descriptor using 
UsbIoControlTransfer with an extra large buffer to contain the report 
descriptor (just in case).  The devices I'm working on just fill that report 
buffer part way and stop at the end of the report descriptor.

Unfortunately, there doesn't seem to be any mechanism to determine how many 
bytes of data a device has actually transferred to the buffer during a control 
transfer after that transfer completes. The USB IO control transfer function 
takes UINTN DataLength, not a pointer to DataLength.  I might expect that the 
UsbIo protocol would patch the Length field in the EFI_USB_DEVICE_REQUEST 
structure to reflect the actual transfer length after the fact, but that 
doesn't seem to happen either.

Am I overlooking some hidden interface here, or am I really going to be forced 
to use trial reads and buffer scanning to determine how much of the buffer 
pointed to by "Data" is actually filled by the device during a control 
transfer?  This is really bugging me, as I know there's a mechanism for host 
controllers to report this information... I've written USB host controller and 
bus drivers for other BIOS implementations, and I remember this being possible.

For reference, here's the definition of the control transfer function from the 
UsbIo.h protocol definition:

/**
  This function is used to manage a USB device with a control transfer pipe. A 
control transfer is
  typically used to perform device initialization and configuration.

  @param  This  A pointer to the EFI_USB_IO_PROTOCOL instance.
  @param  Request   A pointer to the USB device request that will 
be sent to the USB
device.
  @param  Direction Indicates the data direction.
  @param  Timeout   Indicating the transfer should be completed 
within this time frame.
The units are in milliseconds.
  @param  Data  A pointer to the buffer of data that will be 
transmitted to USB
device or received from USB device.
  @param  DataLengthThe size, in bytes, of the data buffer 
specified by Data.
  @param  StatusA pointer to the result of the USB transfer.

  @retval EFI_SUCCESS   The control transfer has been successfully 
executed.
  @retval EFI_DEVICE_ERROR  The transfer failed. The transfer status is 
returned in Status.
  @retval EFI_INVALID_PARAMETE  One or more parameters are invalid.
  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a 
lack of resources.
  @retval EFI_TIMEOUT   The control transfer fails due to timeout.

**/
typedef
EFI_STATUS
(EFIAPI *EFI_USB_IO_CONTROL_TRANSFER)(
  IN EFI_USB_IO_PROTOCOL*This,
  IN EFI_USB_DEVICE_REQUEST *Request,
  IN EFI_USB_DATA_DIRECTION Direction,
  IN UINT32 Timeout,
  IN OUT VOID   *Data OPTIONAL,
  IN UINTN  DataLength  OPTIONAL,
  OUT UINT32*Status
);

Regards,
-Ken Taylor
Phoenix Technologies,  Ltd.
___
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] IntelFsp2Pkg: Skip loading Microcode if MicrocodeCodeSize is zero

[Mudusuru, Giri P]

Reviewed-by: Giri P Mudusuru  

> -Original Message-
> From: Yarlagadda, Satya P
> Sent: Monday, July 25, 2016 6:14 PM
> To: edk2-devel@lists.01.org
> Cc: Mudusuru, Giri P ; Yao, Jiewen
> 
> Subject: [PATCH] IntelFsp2Pkg: Skip loading Microcode if MicrocodeCodeSize is
> zero
> 
> During asm to Nasm conversion, we missed the code to skip loading the
> microcode and return success if the size is zero. Added additional check to
> report error if the microcode size is not zero but less than 2 kB.
> 
> Cc: Giri P Mudusuru 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Satya Yarlagadda 
> ---
>  IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> index 55ee85a..6e86023 100644
> --- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> +++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
> @@ -172,7 +172,15 @@ ASM_PFX(LoadMicrocodeDefault):
> cmpeax, 0
> jz ParamError
> movesp, eax
> -
> +
> +   ; skip loading Microcode if the MicrocodeCodeSize is zero
> +   ; and report error if size is less than 2k
> +   moveax, dword [esp + LoadMicrocodeParams.MicrocodeCodeSize]
> +   cmpeax, 0
> +   jz Exit2
> +   cmpeax, 0800h
> +   jl ParamError
> +
> movesi, dword [esp + LoadMicrocodeParams.MicrocodeCodeAddr]
> cmpesi, 0
> jnzCheckMainHeader
> --
> 2.9.2.windows.1

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

Re: [edk2] [Patch v3 03/40] UefiCpuPkg/CpuS3DataDxe: Move StartupVector allocation to EndOfDxe()

[Fan, Jeff]

Laszlo,

Agree, I will update it in v4. 

Jeff
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 9:24 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Tian, Feng
Subject: Re: [edk2] [Patch v3 03/40] UefiCpuPkg/CpuS3DataDxe: Move 
StartupVector allocation to EndOfDxe()

On 07/25/16 04:52, Jeff Fan wrote:
> Currently, we will allocate StartupVector buffer under 1MB at entry 
> point function. But some modules may allocate some hard code address under 
> 1MB.
> For example, LegacyBiosDxe driver tries to manage some legacy range 
> under 640KB.
> 
> To avoid the conflicts, we move StartupVector buffer allocation to End 
> Of DXE event callback function.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c  | 39 
> ++--
>  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf |  2 +-
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c 
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> index 9fb47dc..9e76cae 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> @@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, 
> so this module does not  support hot plug CPUs.  This module can be 
> copied into a CPU specific package  and customized if these additional 
> features are required.
>  
> -Copyright (c) 2013 - 2015, Intel Corporation. All rights 
> reserved.
> +Copyright (c) 2013 - 2016, Intel Corporation. All rights 
> +reserved.
>  Copyright (c) 2015, Red Hat, Inc.
>  
>  This program and the accompanying materials @@ -45,6 +45,8 @@ typedef 
> struct {
>IA32_DESCRIPTOR   IdtrProfile;
>  } ACPI_CPU_DATA_EX;
>  
> +ACPI_CPU_DATA  *mAcpiCpuData;
> +
>  /**
>Allocate EfiACPIMemoryNVS below 4G memory address.
>  
> @@ -84,18 +86,31 @@ AllocateAcpiNvsMemoryBelow4G (
>  /**
>Callback function executed when the EndOfDxe event group is signaled.
>  
> -  We delay saving the MTRR settings until BDS signals EndOfDxe.
> +  We delay allocating StartupVector and saving the MTRR settings until BDS 
> signals EndOfDxe.
>  
>@param[in]  EventEvent whose notification function is being invoked.
>@param[out] Context  Pointer to the MTRR_SETTINGS buffer to fill in.
>  **/
>  VOID
>  EFIAPI
> -SaveMtrrsOnEndOfDxe (
> +CpuS3DataOnEndOfDxe (
>IN  EFI_EVENT  Event,
>OUT VOID   *Context
>)
>  {
> +  EFI_STATUS  Status;
> +  //
> +  // Allocate a 4KB reserved page below 1MB  //  
> + mAcpiCpuData->StartupVector = BASE_1MB - 1;  Status = 
> + gBS->AllocatePages (
> +  AllocateMaxAddress,
> +  EfiReservedMemoryType,
> +  1,
> +  &mAcpiCpuData->StartupVector
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +
>DEBUG ((EFI_D_VERBOSE, "%a\n", __FUNCTION__));
>MtrrGetAllMtrrs (Context);
>  
> @@ -162,18 +177,6 @@ CpuS3DataInitialize (
>ASSERT_EFI_ERROR (Status);
>  
>//
> -  // Allocate a 4KB reserved page below 1MB
> -  //
> -  AcpiCpuData->StartupVector = BASE_1MB - 1;
> -  Status = gBS->AllocatePages (
> -  AllocateMaxAddress,
> -  EfiReservedMemoryType,
> -  1,
> -  &AcpiCpuData->StartupVector
> -  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  //
>// Get the number of CPUs
>//
>Status = MpServices->GetNumberOfProcessors ( @@ -255,17 +258,19 @@ 
> CpuS3DataInitialize (
>  
>//
>// Register EFI_END_OF_DXE_EVENT_GROUP_GUID event.
> -  // The notification function saves MTRRs for ACPI_CPU_DATA
> +  // The notification function allocates StartupVector and saves 
> + MTRRs for ACPI_CPU_DATA
>//
>Status = gBS->CreateEventEx (
>EVT_NOTIFY_SIGNAL,
>TPL_CALLBACK,
> -  SaveMtrrsOnEndOfDxe,
> +  CpuS3DataOnEndOfDxe,
>&AcpiCpuDataEx->MtrrTable,
>&gEfiEndOfDxeEventGroupGuid,
>&Event
>);
>ASSERT_EFI_ERROR (Status);
>  
> +  mAcpiCpuData = AcpiCpuData;
> +
>return EFI_SUCCESS;
>  }
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf 
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> index 857e12b..608e19f 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> @@ -9,7 +9,7 @@
>  #  support hot plug CPUs.  This module can be copied into a CPU 
> specific package  #  and customized if these additional features are required.
>  #
> -#  Copyright (c) 2013-2015, Intel Corporation. All rights 
> reserved.
> +#  Copyright (c) 2013-2016, Intel Corporation. All rights 
> +reserved.
>  #  Copyright (c) 2015, Red Hat, Inc.
>  #
> 

Re: [edk2] [Patch v3 02/40] UefiCpuPkg/MpInitLib: Add microcode definitions defined in IA32 SDM

[Fan, Jeff]

Laszlo,

Good catch on #1

For #2, #pack(1), the structure in this patch should have no padding data 
between each fields. But I agree to add #pack(1) to make it more clear.

Thanks!
Jeff
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 9:11 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Tian, Feng
Subject: Re: [edk2] [Patch v3 02/40] UefiCpuPkg/MpInitLib: Add microcode 
definitions defined in IA32 SDM

On 07/25/16 04:52, Jeff Fan wrote:
> Add microcode definitions defined in Intel(R) 64 and IA-32 
> Architectures Software Developer's Manual Volume 3A, Section 9.11.
> 
> v3:
>   1. Update SDM date to June, 2016
>   2. Mention BCD format in CPU_MICROCODE_DATE
>   3. Rename ProcessorChecksum to Checksum to match SDM.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  UefiCpuPkg/Include/Register/Microcode.h | 196 
> 
>  1 file changed, 196 insertions(+)
>  create mode 100644 UefiCpuPkg/Include/Register/Microcode.h
> 
> diff --git a/UefiCpuPkg/Include/Register/Microcode.h 
> b/UefiCpuPkg/Include/Register/Microcode.h
> new file mode 100644
> index 000..1f6cd47
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Register/Microcode.h
> @@ -0,0 +1,196 @@
> +/** @file
> +  Microcode Definitions.
> +
> +  Microcode Definitions based on contents of the
> +  Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> +Volume 3A, Section 9.11  Microcode Definitions
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.  
> + This program and the accompanying materials  are licensed and made 
> + available under the terms and conditions of the BSD License  which 
> + accompanies this distribution.  The full text of the license may be 
> + found at  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" 
> + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS 
> OR IMPLIED.
> +
> +  @par Specification Reference:
> +  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, 
> + Volume 3A,  June 2016, Chapter 9 Processor Management and Initialization, 
> Section 9-11.
> +
> +**/
> +
> +#ifndef __MICROCODE_H__
> +#define __MICROCODE_H__
> +
> +///
> +/// CPU Microcode Date in BCD format
> +///
> +typedef union {
> +  struct {
> +UINT32   Year:16;
> +UINT32   Day:8;
> +UINT32   Month:8;
> +  } Bits;
> +  UINT32 Uint32;
> +} CPU_MICROCODE_DATE;
> +
> +///
> +/// CPU Microcode Processor Signature format /// typedef union {
> +  struct {
> +UINT32   Stepping:4;
> +UINT32   Model:4;
> +UINT32   Family:4;
> +UINT32   Type:2;
> +UINT32   Reserved1:2;
> +UINT32   ExtendedModel:4;
> +UINT32   ExtendedFamily:8;
> +UINT32   Reserved2:4;
> +  } Bits;
> +  UINT32 Uint32;
> +} CPU_MICROCODE_PROCESSOR_SIGNATURE;
> +
> +///
> +/// Microcode Update Format definition /// typedef struct {
> +  ///
> +  /// Version number of the update header
> +  ///
> +  UINT32HeaderVersion;
> +  ///
> +  /// Unique version number for the update, the basis for the update
> +  /// signature provided by the processor to indicate the current 
> +update
> +  /// functioning within the processor. Used by the BIOS to 
> +authenticate
> +  /// the update and verify that the processor loads successfully. 
> +The
> +  /// value in this field cannot be used for processor stepping 
> +identification
> +  /// alone. This is a signed 32-bit number.
> +  ///
> +  UINT32UpdateRevision;
> +  ///
> +  /// Date of the update creation in binary format: mmdd (e.g.
> +  /// 07/18/98 is 07181998H).
> +  ///
> +  CPU_MICROCODE_DATEDate;
> +  ///
> +  /// Extended family, extended model, type, family, model, and 
> +stepping
> +  /// of processor that requires this particular update revision 
> +(e.g.,
> +  /// 0650H). Each microcode update is designed specifically for 
> +a
> +  /// given extended family, extended model, type, family, model, and
> +  /// stepping of the processor.
> +  /// The BIOS uses the processor signature field in conjunction with 
> +the
> +  /// CPUID instruction to determine whether or not an update is
> +  /// appropriate to load on a processor. The information encoded 
> +within
> +  /// this field exactly corresponds to the bit representations 
> +returned by
> +  /// the CPUID instruction.
> +  ///
> +  CPU_MICROCODE_PROCESSOR_SIGNATURE ProcessorSignature;
> +  ///
> +  /// Checksum of Update Data and Header. Used to verify the 
> +integrity of
> +  /// the update header and data. Checksum is correct when the
> +  /// summation of all the DWORDs (including the extended Processor
> +  /// Signature Table) that comprise the microcode update result in
> +  /// H.
> +  ///
> +  UIN

Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode

[Gao, Liming]

Ard:
  Another information. I try Steven change in 
https://github.com/shijunjing/edk2/tree/llvm_v2. With his patch, build -p 
OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -DSECURE_BOOT_ENABLE=TRUE -a IA32 -a X64 can 
pass build. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Gao, Liming
> Sent: Monday, July 25, 2016 10:12 PM
> To: Ard Biesheuvel 
> Cc: Justen, Jordan L ; edk2-devel@lists.01.org;
> ler...@redhat.com; leif.lindh...@linaro.org
> Subject: Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO
> mode
> 
> Without -DSECURE_BOOT_ENABLE=TRUE, OVMF on GCC5 in LTO mode can
> pass build.
> 
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Monday, July 25, 2016 10:04 PM
> To: Gao, Liming 
> Cc: Justen, Jordan L ; edk2-devel@lists.01.org;
> leif.lindh...@linaro.org; ler...@redhat.com
> Subject: Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO
> mode
> 
> 
> > On 25 jul. 2016, at 15:21, Gao, Liming wrote:
> >
> > Ard:
> > After apply these patches, I try to build OvmfPkg with SECURE enable on
> GCC5 in LTO mode.
> 
> 
> Did you also try it without?
> 
> > It reports build failure. Have you tried such build before?
> >
> 
> No I have not.
> 
> > Build command: build -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 -
> DSECURE_BOOT_ENABLE=TRUE -a IA32 -a X64
> > Build error message, seemly OpenSsl can't be linked with LTO.
> >
> 
> I will investigate. It indeed looks like an incompatibility with lto, I 
> should check
> whether Arm is affected as well.
> 
> Thanks,
> Ard.
> 
> > "gcc" -o
> /home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeM
> odulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStub
> Dxe.dll -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x40 -Wl,--
> entry,_ModuleEntryPoint -u _ModuleEntryPoint -Wl,-
> Map,/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/M
> deModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/Security
> StubDxe.map -Wl,-melf_x86_64,--oformat=elf64-x86-64 -Wl,--start-
> group,@/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64
> /MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/OUTPUT/stat
> ic_library_files.lst,--end-group -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 -
> Wl,--
> script=/home/hwu/work/lgao4/AllPkg/edk2/BaseTools/Scripts/GccBase.lds
> > /tmp/cc9uBgrd.ltrans0.ltrans.o: In function `DxeImageVerificationHandler':
> > :(.text+0x1f13): undefined reference to `malloc'
> > :(.text+0x1f7a): undefined reference to `free'
> > :(.text+0x1f92): undefined reference to `malloc'
> > :(.text+0x1fb9): undefined reference to `free'
> > :(.text+0x1fe3): undefined reference to `free'
> > :(.text+0x2027): undefined reference to `malloc'
> > :(.text+0x20bf): undefined reference to `free'
> > :(.text+0x2116): undefined reference to `free'
> > :(.text+0x212d): undefined reference to `free'
> > :(.text+0x213a): undefined reference to `free'
> > :(.text+0x21f7): undefined reference to `free'
> > /tmp/cc9uBgrd.ltrans0.ltrans.o::(.text+0x2204): more undefined
> references to `free' follow
> > /tmp/cc9uBgrd.ltrans8.ltrans.o: In function
> `default_realloc_ex.lto_priv.190':
> > :(.text+0x1): undefined reference to `realloc'
> > /tmp/cc9uBgrd.ltrans8.ltrans.o: In function
> `default_malloc_ex.lto_priv.187':
> > :(.text+0x6): undefined reference to `malloc'
> > /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `CRYPTO_free':
> > :(.text+0x613): undefined reference to `free'
> > /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `WrapPkcs7Data':
> > :(.text.unlikely+0x72): undefined reference to `malloc'
> > /tmp/cc9uBgrd.ltrans17.ltrans.o: In function `OPENSSL_gmtime':
> > :(.text+0x176a): undefined reference to `malloc'
> > /tmp/cc9uBgrd.ltrans18.ltrans.o: In function `RSA_free':
> > :(.text+0x123a): undefined reference to `free'
> > /tmp/cc9uBgrd.ltrans20.ltrans.o: In function `qsort':
> > :(.text+0x1368): undefined reference to `malloc'
> > :(.text+0x13bc): undefined reference to `malloc'
> > :(.text+0x13b4): undefined reference to `free'
> > /tmp/cc9uBgrd.ltrans27.ltrans.o: In function
> `CRYPTO_realloc_clean.constprop.153':
> > :(.text+0x20e): undefined reference to `free'
> > /tmp/cc9uBgrd.ltrans27.ltrans.o:(.data.rel.ro.local+0x578): undefined
> reference to `free'
> > /usr/bin/ld:
> /home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeM
> odulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStub
> Dxe.dll: protected symbol `realloc' isn't defined
> > /usr/bin/ld: final link failed: Bad value
> > collect2: error: ld returned 1 exit status
> > make: ***
> [/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeM
> odulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStub
> Dxe.dll] Error 1
> >
> >
> > build.py...
> > : error 7000: Failed to execute command
> > make tbuild
> [/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeM
> odulePkg/Universal/SecurityStubDxe/Secur

Re: [edk2] [Patch] Update edksetup.bat to check NASM system environment variable

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Yonghong Zhu
> Sent: Friday, July 15, 2016 5:42 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [Patch] Update edksetup.bat to check NASM system
> environment variable
> 
> If the NASM_PREFIX variable is not set, it would report warning message.
> If there exist the C:\nasm\nasm.exe file, it would set the NASM_PREFIX
> variable to C:\nasm\.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  edksetup.bat | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/edksetup.bat b/edksetup.bat
> index e065b64..f066d86 100755
> --- a/edksetup.bat
> +++ b/edksetup.bat
> @@ -123,10 +123,11 @@ if exist %EDK_TOOLS_PATH%\Source set
> BASE_TOOLS_PATH=%EDK_TOOLS_PATH%
> 
>  :checkBaseTools
>  IF NOT EXIST "%EDK_TOOLS_PATH%\toolsetup.bat" goto BadBaseTools
>  call %EDK_TOOLS_PATH%\toolsetup.bat %*
>  if /I "%1"=="Reconfig" shift
> +goto check_NASM
>  goto check_cygwin
> 
>  :BadBaseTools
>@REM
>REM Need the BaseTools Package in order to build
> @@ -139,10 +140,19 @@ goto check_cygwin
>@echo   set EDK_TOOLS_PATH=C:\MyTools\BaseTools
>@echo The setup script, toolsetup.bat must reside in this folder.
>@echo.
>goto end
> 
> +:check_NASM
> +if not defined NASM_PREFIX (
> +@echo.
> +@echo !!! WARNING !!! NASM_PREFIX environment variable is not set
> +@if exist "C:\nasm\nasm.exe" @set "NASM_PREFIX=C:\nasm\"
> +@if exist "C:\nasm\nasm.exe" @echo   Found nasm.exe, setting the
> environment variable to C:\nasm\
> +@if not exist "C:\nasm\nasm.exe" echo   Attempting to build modules
> that require NASM will fail.
> +)
> +
>  :check_cygwin
>  if defined CYGWIN_HOME (
>if not exist "%CYGWIN_HOME%" (
>  @echo.
>  @echo !!! WARNING !!! CYGWIN_HOME not found, gcc build may not be
> used !!!
> --
> 2.6.1.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] OVMF and passed usb 3. controller

[Tian, Feng]

If the renesas usb 3.0 host controller follows XHCI spec, then EDKII XHCI 
driver could be used to manage it.

Thanks
Feng

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 5:07 PM
To: Eugene Chekanskiy ; edk2-de...@ml01.01.org
Cc: Gerd Hoffmann ; Tian, Feng ; Alex 
Williamson 
Subject: Re: [edk2] OVMF and passed usb 3. controller

On 07/22/16 15:25, Eugene Chekanskiy wrote:
> Hello everyone. Just wondering if default builds of ovmf form 
> http://www.tianocore.org/ovmf/ has support of renesas usb 3.0 
> controller and if we can enable it in custom build.

First, please don't use the binary from .
It is incredibly old (Feb 9 2014). Instead, I recommend 
 for anything "bleeding edge".

(Unless you are willing to build your own OVMF, of course, in which case I 
recommend that instead).

Second, OVMF includes the "MdeModulePkg/Bus/Pci/XhciDxe" driver. I assume you 
assign your Renesas host controller to the virtual machine; is that right? If 
your host controller is supported by XhciDxe otherwise (IOW it would work in a 
purely physical setup), then I think it should work with device assignment as 
well.

CC: Gerd for owning kraxel.org and for USB knowledge
CC: Feng for maintaining XhciDxe
CC: Alex for any possible quirks with USB host controller assignment

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

[edk2] USB Control Transfers

[Ken Taylor]

Hi All,

I am working on a USB HID device driver.  The devices I want to support aren't 
legacy (keyboard or mouse) and at least some of them don't appear to support 
HID descriptors.  I need to parse the first Report descriptor, but because of 
the lack of the HID descriptor, I can't know how long any of the report 
descriptors are going to be in advance of the transaction that requests the 
descriptor.

I have no problem fetching the first report descriptor using 
UsbIoControlTransfer with an extra large buffer to contain the report 
descriptor (just in case).  The devices I'm working on just fill that report 
buffer part way and stop at the end of the report descriptor.

Unfortunately, there doesn't seem to be any mechanism to determine how many 
bytes of data a device has actually transferred to the buffer during a control 
transfer after that transfer completes. The USB IO control transfer function 
takes UINTN DataLength, not a pointer to DataLength.  I might expect that the 
UsbIo protocol would patch the Length field in the EFI_USB_DEVICE_REQUEST 
structure to reflect the actual transfer length after the fact, but that 
doesn't seem to happen either.

Am I overlooking some hidden interface here, or am I really going to be forced 
to use trial reads and buffer scanning to determine how much of the buffer 
pointed to by "Data" is actually filled by the device during a control 
transfer?  This is really bugging me, as I know there's a mechanism for host 
controllers to report this information... I've written USB host controller and 
bus drivers for other BIOS implementations, and I remember this being possible.

For reference, here's the definition of the control transfer function from the 
UsbIo.h protocol definition:

/**
  This function is used to manage a USB device with a control transfer pipe. A 
control transfer is
  typically used to perform device initialization and configuration.

  @param  This  A pointer to the EFI_USB_IO_PROTOCOL instance.
  @param  Request   A pointer to the USB device request that will 
be sent to the USB
device.
  @param  Direction Indicates the data direction.
  @param  Timeout   Indicating the transfer should be completed 
within this time frame.
The units are in milliseconds.
  @param  Data  A pointer to the buffer of data that will be 
transmitted to USB
device or received from USB device.
  @param  DataLengthThe size, in bytes, of the data buffer 
specified by Data.
  @param  StatusA pointer to the result of the USB transfer.

  @retval EFI_SUCCESS   The control transfer has been successfully 
executed.
  @retval EFI_DEVICE_ERROR  The transfer failed. The transfer status is 
returned in Status.
  @retval EFI_INVALID_PARAMETE  One or more parameters are invalid.
  @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a 
lack of resources.
  @retval EFI_TIMEOUT   The control transfer fails due to timeout.

**/
typedef
EFI_STATUS
(EFIAPI *EFI_USB_IO_CONTROL_TRANSFER)(
  IN EFI_USB_IO_PROTOCOL*This,
  IN EFI_USB_DEVICE_REQUEST *Request,
  IN EFI_USB_DATA_DIRECTION Direction,
  IN UINT32 Timeout,
  IN OUT VOID   *Data OPTIONAL,
  IN UINTN  DataLength  OPTIONAL,
  OUT UINT32*Status
);

Regards,
-Ken Taylor
Phoenix Technologies,  Ltd.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] IntelFsp2Pkg: Skip loading Microcode if MicrocodeCodeSize is zero

[Satya Yarlagadda]

During asm to Nasm conversion, we missed the code to skip loading the
microcode and return success if the size is zero. Added additional check to
report error if the microcode size is not zero but less than 2 kB.

Cc: Giri P Mudusuru 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Satya Yarlagadda 
---
 IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm 
b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
index 55ee85a..6e86023 100644
--- a/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
+++ b/IntelFsp2Pkg/FspSecCore/Ia32/FspApiEntryT.nasm
@@ -172,7 +172,15 @@ ASM_PFX(LoadMicrocodeDefault):
cmpeax, 0
jz ParamError
movesp, eax
-   
+
+   ; skip loading Microcode if the MicrocodeCodeSize is zero
+   ; and report error if size is less than 2k
+   moveax, dword [esp + LoadMicrocodeParams.MicrocodeCodeSize]
+   cmpeax, 0
+   jz Exit2
+   cmpeax, 0800h
+   jl ParamError
+
movesi, dword [esp + LoadMicrocodeParams.MicrocodeCodeAddr]
cmpesi, 0
jnzCheckMainHeader
-- 
2.9.2.windows.1

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

Re: [edk2] AArch64 XIP Recipe

[Cohen, Eugene]

Ard,

> OK, sorry for the monologue, but I just had an idea: we could build
> everything with the small model, and convert ADRP instructions to ADR
> instructions in GenFw if the section alignment < 4 KB. This will cause
> build failures if this conversion is not possible due to the fact that
> the binary exceeds 1 MB, but this means we only need to set the 4 KB
> page size for modules where this can reasonably be expected (i.e.,
> UEFI_APPLICATION modules) Everything else will be just as small as
> the
> small model.
> 
> I will cook something up, but don't expect anything before next week.

Fascinating idea - this effectively turns 'small' into 'tiny' but does so after 
linking.  You could even make the logic dynamic based on the size of the image.

It would be great to get this as an optimization into GCC's ld even.

Thanks!

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

Re: [edk2] AArch64 XIP Recipe

[Cohen, Eugene]

> Indeed.  The Shell in RELEASE mode builds fine with the tiny model
> though. Since the Shell is the only problematic module (afaik), it
> would suffice to simply build the shell components in RELEASE mode
> unconditionally, assuming that you are not interested in debugging the
> shell and your platform at the same time.

Yes, this works.  Another workaround is to do separate build invocations for 
the XIP and non-XIP modules with different memory models for each and thereby 
get different sets of libraries.

> This is not a toolchain bug, but an ISA problem, and so it is not
> going away, unfortunately.

Right, a combination of the ISA's ADRP and the fact that toolchains 
unfortunately choose to use it.  :)

For what it's worth I did an experiment forcing the 'large' memory model and 
given the 4KB page alignment effects on XIP placement it turned out to be much 
more efficient - 18KB of growth versus 327KB.

If GCC could somehow link tiny libraries into a small/large module without 
generating relocation displacement errors from the tiny components that can no 
longer reach their counterparts it could solve this nicely - then you only pay 
for what you use.

Eugene

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Monday, July 25, 2016 10:28 AM
> To: Cohen, Eugene 
> Cc: edk2-devel@lists.01.org
> Subject: Re: AArch64 XIP Recipe
> 
> On 25 July 2016 at 18:23, Cohen, Eugene  wrote:
> > Doing some git archaeology, I suspect that stuff started to go bad for
> us
> > around this commit:
> >
> >
> >
> > Revision: f37d891c1b870b294964adf65f619a661700fcab
> >
> > Author: Ard Biesheuvel 
> >
> > Date: 3/25/2016 5:33:28 AM
> >
> > Message:
> >
> > BaseTools AARCH64: move DEBUG GCC49 to the small code model
> >
> >
> >
> > When building AARCH64 platforms that include a Shell binary built
> from
> >
> > source, we run into trouble when using the tiny code model for
> DEBUG
> >
> > builds. The reason is that the Shell binary built in DEBUG mode
> exceeds
> >
> > the 1 MB range of the ADR instruction, so anything that gets pulled
> into
> >
> > the final link of the Shell binary either needs to be built with the small
> >
> > or large model, or needs to be sorted in some way to put the ADR
> references
> >
> > close to their targets.
> >
> >
> >
> > The rationale with this commit was that not only does the shell itself
> need
> > to use the small model (to address the 1MB displacement limitation
> of the
> > ADR instruction) but every library it pulls in must as well.  In our
> testing
> > before this commit we did not see a problem, what did you see (if
> you can
> > remember)?
> >
> 
> The Shell failed to link due to the fact that the BASE libraries it
> depends on were built with the tiny model
> 
> >
> >
> > But any time the small memory model is used the common page size
> is set to
> > 4KB per the requirement stated in this change to BaseTools
> Elf64Convert.c:
> >
> >
> >
> > Revision: 24d610e67752ac0325c7027e2fea2f8f2ff110e2
> >
> > Author: Ard Biesheuvel 
> >
> > Date: 8/10/2015 1:55:18 AM
> >
> > Message:
> >
> > BaseTools/GenFw: allow AArch64 tiny and small code model
> relocations
> >
> >
> >
> > The AArch64 small C model makes extensive use of ADRP/ADD and
> >
> > ADRP/{LDR,STR} pairs to emit PC-relative symbol references with
> >
> > a +/- 4 GB range. Since the relocation pair splits the relative
> >
> > offset into a relative page offset and an absolute offset into
> >
> > a 4 KB page, we need to take extra care to ensure that the target
> >
> > of the relocation preserves its alignment relative to a 4 KB
> >
> > alignment boundary.
> >
> >
> >
> > So since the shell requires all libraries to be built in the small memory
> > model and all small memory model components must have 4KB
> alignment, this
> > means that any component shared between XIP and the shell will
> get a 4KB
> > alignment treatment.  As discussed before this just eats up
> flash/sram space
> > in XIP regions since the FVs get padded out to meet this
> requirement.  This
> > results in an untenable situation.
> >
> 
> Indeed.  The Shell in RELEASE mode builds fine with the tiny model
> though. Since the Shell is the only problematic module (afaik), it
> would suffice to simply build the shell components in RELEASE mode
> unconditionally, assuming that you are not interested in debugging the
> shell and your platform at the same time.
> 
> 
> >
> >
> > As I look at the history of this, this looks like a limitation in GCC where
> > it doesn't emit the proper relocations for the page offset so as a
> > workaround the page relationship of the code and data must be
> preserved
> > across image conversion and relocation.  If this is fixed in GCC then
> can't
> > we just let the relocations fix this?  If so, is it too outrageous to
> > suggest fixing the linker?
> >
> 
> This is not a toolchain bug, but an ISA problem, and so it is not
> going away, unfortunately.
> 
> --
> Ard.
_

Re: [edk2] [PATCH v3 3/6] BaseTools UNIXGCC ELFGCC CYGGCC: clone GCC build rule family into GCCLD

[Jordan Justen]

I think you should make the build_rule changes in one patch, and the
tools_def changes in the second. GCCLD, in this patch, will not work
for using GCC as the linker.

-Jordan

On 2016-07-23 02:03:18, Ard Biesheuvel wrote:
> Before we can make non-backward compatible changes to the GCC build rules
> regarding the use of the 'gcc' binary as the linker, clone the existing
> GCC build rules into a 'GCCLD' build rule family, and move the legacy
> toolchains UNIXGCC, CYGGCC, CYGGCCxASL and ELFGCC over to it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Conf/build_rule.template | 28 ++--
>  BaseTools/Conf/tools_def.template  |  4 +++
>  2 files changed, 18 insertions(+), 14 deletions(-)
> 
> diff --git a/BaseTools/Conf/build_rule.template 
> b/BaseTools/Conf/build_rule.template
> index 91bcc1828cb5..3fea4f456118 100644
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -130,7 +130,7 @@
>  
>  "$(CC)" /Fo${dst} $(CC_FLAGS) $(INC) ${src}
>  
> -
> +
>  # For RVCTCYGWIN CC_FLAGS must be first to work around pathing issues
>  "$(CC)" $(CC_FLAGS) -o ${dst} $(INC) ${src}
>  
> @@ -156,7 +156,7 @@
>  
>  "$(CC)" /Fo${dst} $(CC_FLAGS) $(INC) ${src}
>  
> -
> +
>  # For RVCTCYGWIN CC_FLAGS must be first to work around pathing issues
>  "$(CC)" $(CC_FLAGS) -o ${dst} $(INC) ${src}
>  "$(SYMRENAME)" $(SYMRENAME_FLAGS) ${dst}
> @@ -171,7 +171,7 @@
>  
>  $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>  
> -
> +
>  "$(CC)" $(CC_FLAGS) $(CC_XIPFLAGS) -o ${dst} $(INC) ${src}
>  
>  [C-Header-File]
> @@ -187,7 +187,7 @@
>  
>  ?.asm, ?.Asm, ?.ASM
>  
> -
> +
>  ?.S, ?.s
>  
>  
> @@ -201,7 +201,7 @@
>  Trim --source-code --convert-hex --trim-long -o 
> ${d_path}(+)${s_base}.iii ${d_path}(+)${s_base}.i
>  "$(ASM)" /Fo${dst} $(ASM_FLAGS) /I${s_path} $(INC) 
> ${d_path}(+)${s_base}.iii
>  
> -
> +
>  "$(PP)" $(PP_FLAGS) $(INC) ${src} > ${d_path}(+)${s_base}.i
>  Trim --trim-long --source-code -o ${d_path}(+)${s_base}.iii 
> ${d_path}(+)${s_base}.i
>  # For RVCTCYGWIN ASM_FLAGS must be first to work around pathing 
> issues
> @@ -265,7 +265,7 @@
>  
>  "$(SLINK)" $(SLINK_FLAGS) /OUT:${dst} @$(OBJECT_FILES_LIST)
>  
> -
> +
>  "$(SLINK)" -cr ${dst} $(SLINK_FLAGS) @$(OBJECT_FILES_LIST)
>  
>  
> @@ -291,7 +291,7 @@
>  
>  "$(DLINK)" /OUT:${dst} $(DLINK_FLAGS) $(DLINK_SPATH) 
> @$(STATIC_LIBRARY_FILES_LIST)
>  
> -
> +
>  "$(DLINK)" -o ${dst} $(DLINK_FLAGS) --start-group $(DLINK_SPATH) 
> @$(STATIC_LIBRARY_FILES_LIST) --end-group $(DLINK2_FLAGS)
>  "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
>  
> @@ -319,7 +319,7 @@
>  
>  "$(DLINK)" $(DLINK_FLAGS) $(DLINK_SPATH) 
> @$(STATIC_LIBRARY_FILES_LIST)
>  
> -
> +
>  "$(DLINK)" $(DLINK_FLAGS) --start-group $(DLINK_SPATH) 
> @$(STATIC_LIBRARY_FILES_LIST) --end-group $(DLINK2_FLAGS)
>  
>  
> @@ -346,7 +346,7 @@
>  $(CP) ${dst} $(BIN_DIR)(+)$(MODULE_NAME_GUID).efi
>  -$(CP) $(DEBUG_DIR)(+)*.map $(OUTPUT_DIR)
>  -$(CP) $(DEBUG_DIR)(+)*.pdb $(OUTPUT_DIR) 
> -
> +
>  $(CP) ${src} $(DEBUG_DIR)(+)$(MODULE_NAME).debug
>  $(OBJCOPY) --strip-unneeded -R .eh_frame ${src}
>  
> @@ -402,7 +402,7 @@
>  Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}. 
> $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii 
>  "$(ASL)" $(ASL_FLAGS) $(ASL_OUTFLAGS)${dst} 
> $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.
>  
> -
> +
>  Trim --asl-file -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i -i 
> $(INC_LIST) ${src}
>  "$(ASLPP)" $(ASLPP_FLAGS) $(INC) -I${s_path} 
> $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.i > 
> $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii
>  Trim --source-code -l -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}. 
> $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.iii 
> @@ -423,7 +423,7 @@
>  "$(ASLDLINK)" /OUT:$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll 
> $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>  "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll 
> $(GENFW_FLAGS)
>  
> -
> +
>  "$(ASLCC)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj $(CC_FLAGS) 
> $(ASLCC_FLAGS) $(INC) ${src}
>  "$(ASLDLINK)" -o $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll 
> $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>  "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll 
> $(GENFW_FLAGS)
> @@ -443,7 +443,7 @@
>  "$(ASLDLINK)" /OUT:$(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll 
> $(ASLDLINK_FLAGS) $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.obj
>  "$(GENFW)" -o ${dst} -c $(OUTPUT_DIR)(+)${s_dir}(+)${s_base}.dll 
> $(GENFW_FLAGS)
>  
> -
> +
>  "$(

[edk2] [BaseTools] is there a reason we have *.pyd files checked in?

[Andrew Fish]

Are these .pyd files checked in for a reason? Seems like they have not been 
updated since 2013? 

BaseTools/Source/Python/Common/PyUtility.pyd
BaseTools/Source/Python/Eot/EfiCompressor.pyd
BaseTools/Source/Python/Eot/LzmaCompressor.pyd

~/work/src/edk2(master)>git log BaseTools/Source/Python/Common/PyUtility.pyd
commit 4afd3d042215afe68d00b9ab8c32f063a3a1c03f
Author: Liming Gao 
Date:   Fri Aug 23 02:18:16 2013 +

Sync BaseTool trunk (version r2599) into EDKII BaseTools.

Signed-off-by: Liming Gao 
Reviewed-by: Heshen Chen 


git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@14591 
6f19259b-4bc3-4df7-8a09-765794883524

commit 30fdf1140b8d1ce93f3821d986fa165552023440
Author: lgao4 
Date:   Fri Jul 17 09:10:31 2009 +

Check In tool source code based on Build tool project revision r1655.

git-svn-id: https://edk2.svn.sourceforge.net/svnroot/edk2/trunk/edk2@8964 
6f19259b-4bc3-4df7-8a09-765794883524


Thanks,

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

Re: [edk2] AArch64 XIP Recipe

[Ard Biesheuvel]

On 25 July 2016 at 21:25, Ard Biesheuvel  wrote:
>
>
>> On 25 jul. 2016, at 18:28, Ard Biesheuvel  wrote:
>>
>>> On 25 July 2016 at 18:23, Cohen, Eugene  wrote:
>>> Doing some git archaeology, I suspect that stuff started to go bad for us
>>> around this commit:
>>>
>>>
>>>
>>> Revision: f37d891c1b870b294964adf65f619a661700fcab
>>>
>>> Author: Ard Biesheuvel 
>>>
>>> Date: 3/25/2016 5:33:28 AM
>>>
>>> Message:
>>>
>>> BaseTools AARCH64: move DEBUG GCC49 to the small code model
>>>
>>>
>>>
>>> When building AARCH64 platforms that include a Shell binary built from
>>>
>>> source, we run into trouble when using the tiny code model for DEBUG
>>>
>>> builds. The reason is that the Shell binary built in DEBUG mode exceeds
>>>
>>> the 1 MB range of the ADR instruction, so anything that gets pulled into
>>>
>>> the final link of the Shell binary either needs to be built with the small
>>>
>>> or large model, or needs to be sorted in some way to put the ADR references
>>>
>>> close to their targets.
>>>
>>>
>>>
>>> The rationale with this commit was that not only does the shell itself need
>>> to use the small model (to address the 1MB displacement limitation of the
>>> ADR instruction) but every library it pulls in must as well.  In our testing
>>> before this commit we did not see a problem, what did you see (if you can
>>> remember)?
>>
>> The Shell failed to link due to the fact that the BASE libraries it
>> depends on were built with the tiny model
>>
>>>
>>>
>>> But any time the small memory model is used the common page size is set to
>>> 4KB per the requirement stated in this change to BaseTools Elf64Convert.c:
>>>
>>>
>>>
>>> Revision: 24d610e67752ac0325c7027e2fea2f8f2ff110e2
>>>
>>> Author: Ard Biesheuvel 
>>>
>>> Date: 8/10/2015 1:55:18 AM
>>>
>>> Message:
>>>
>>> BaseTools/GenFw: allow AArch64 tiny and small code model relocations
>>>
>>>
>>>
>>> The AArch64 small C model makes extensive use of ADRP/ADD and
>>>
>>> ADRP/{LDR,STR} pairs to emit PC-relative symbol references with
>>>
>>> a +/- 4 GB range. Since the relocation pair splits the relative
>>>
>>> offset into a relative page offset and an absolute offset into
>>>
>>> a 4 KB page, we need to take extra care to ensure that the target
>>>
>>> of the relocation preserves its alignment relative to a 4 KB
>>>
>>> alignment boundary.
>>>
>>>
>>>
>>> So since the shell requires all libraries to be built in the small memory
>>> model and all small memory model components must have 4KB alignment, this
>>> means that any component shared between XIP and the shell will get a 4KB
>>> alignment treatment.  As discussed before this just eats up flash/sram space
>>> in XIP regions since the FVs get padded out to meet this requirement.  This
>>> results in an untenable situation.
>>
>> Indeed.  The Shell in RELEASE mode builds fine with the tiny model
>> though. Since the Shell is the only problematic module (afaik), it
>> would suffice to simply build the shell components in RELEASE mode
>> unconditionally, assuming that you are not interested in debugging the
>> shell and your platform at the same time.
>>
>
> Alternatively, you could add -mcmodel=large to the CC_XIPFLAGS, which allows 
> you to set -z common-page-size 0x20 for SEC , PEIM and PEICORE modules
>

OK, sorry for the monologue, but I just had an idea: we could build
everything with the small model, and convert ADRP instructions to ADR
instructions in GenFw if the section alignment < 4 KB. This will cause
build failures if this conversion is not possible due to the fact that
the binary exceeds 1 MB, but this means we only need to set the 4 KB
page size for modules where this can reasonably be expected (i.e.,
UEFI_APPLICATION modules) Everything else will be just as small as the
small model.

I will cook something up, but don't expect anything before next week.

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

Re: [edk2] AArch64 XIP Recipe

[Ard Biesheuvel]

> On 25 jul. 2016, at 18:28, Ard Biesheuvel  wrote:
> 
>> On 25 July 2016 at 18:23, Cohen, Eugene  wrote:
>> Doing some git archaeology, I suspect that stuff started to go bad for us
>> around this commit:
>> 
>> 
>> 
>> Revision: f37d891c1b870b294964adf65f619a661700fcab
>> 
>> Author: Ard Biesheuvel 
>> 
>> Date: 3/25/2016 5:33:28 AM
>> 
>> Message:
>> 
>> BaseTools AARCH64: move DEBUG GCC49 to the small code model
>> 
>> 
>> 
>> When building AARCH64 platforms that include a Shell binary built from
>> 
>> source, we run into trouble when using the tiny code model for DEBUG
>> 
>> builds. The reason is that the Shell binary built in DEBUG mode exceeds
>> 
>> the 1 MB range of the ADR instruction, so anything that gets pulled into
>> 
>> the final link of the Shell binary either needs to be built with the small
>> 
>> or large model, or needs to be sorted in some way to put the ADR references
>> 
>> close to their targets.
>> 
>> 
>> 
>> The rationale with this commit was that not only does the shell itself need
>> to use the small model (to address the 1MB displacement limitation of the
>> ADR instruction) but every library it pulls in must as well.  In our testing
>> before this commit we did not see a problem, what did you see (if you can
>> remember)?
> 
> The Shell failed to link due to the fact that the BASE libraries it
> depends on were built with the tiny model
> 
>> 
>> 
>> But any time the small memory model is used the common page size is set to
>> 4KB per the requirement stated in this change to BaseTools Elf64Convert.c:
>> 
>> 
>> 
>> Revision: 24d610e67752ac0325c7027e2fea2f8f2ff110e2
>> 
>> Author: Ard Biesheuvel 
>> 
>> Date: 8/10/2015 1:55:18 AM
>> 
>> Message:
>> 
>> BaseTools/GenFw: allow AArch64 tiny and small code model relocations
>> 
>> 
>> 
>> The AArch64 small C model makes extensive use of ADRP/ADD and
>> 
>> ADRP/{LDR,STR} pairs to emit PC-relative symbol references with
>> 
>> a +/- 4 GB range. Since the relocation pair splits the relative
>> 
>> offset into a relative page offset and an absolute offset into
>> 
>> a 4 KB page, we need to take extra care to ensure that the target
>> 
>> of the relocation preserves its alignment relative to a 4 KB
>> 
>> alignment boundary.
>> 
>> 
>> 
>> So since the shell requires all libraries to be built in the small memory
>> model and all small memory model components must have 4KB alignment, this
>> means that any component shared between XIP and the shell will get a 4KB
>> alignment treatment.  As discussed before this just eats up flash/sram space
>> in XIP regions since the FVs get padded out to meet this requirement.  This
>> results in an untenable situation.
> 
> Indeed.  The Shell in RELEASE mode builds fine with the tiny model
> though. Since the Shell is the only problematic module (afaik), it
> would suffice to simply build the shell components in RELEASE mode
> unconditionally, assuming that you are not interested in debugging the
> shell and your platform at the same time.
> 

Alternatively, you could add -mcmodel=large to the CC_XIPFLAGS, which allows 
you to set -z common-page-size 0x20 for SEC , PEIM and PEICORE modules


> 
>> 
>> 
>> As I look at the history of this, this looks like a limitation in GCC where
>> it doesn't emit the proper relocations for the page offset so as a
>> workaround the page relationship of the code and data must be preserved
>> across image conversion and relocation.  If this is fixed in GCC then can't
>> we just let the relocations fix this?  If so, is it too outrageous to
>> suggest fixing the linker?
> 
> This is not a toolchain bug, but an ISA problem, and so it is not
> going away, unfortunately.
> 
> -- 
> Ard.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] OvmfPkg/Sec: Handle bigger FV sizes

[Palmer, Thomas]

Thank you Laszlo, I'll resend the patch out later today or tomorrow.

Thomas

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Monday, July 25, 2016 3:36 AM
To: Palmer, Thomas ; edk2-de...@ml01.01.org
Cc: jordan.l.jus...@intel.com; Shifflett, Joseph 
Subject: Re: [PATCH] OvmfPkg/Sec: Handle bigger FV sizes

On 07/22/16 17:47, Thomas Palmer wrote:
> Ovmf's SecMain needs to handle the EFI_COMMON_SECTION_HEADER2 header 
> so that larger images can be created. Use IS_SECTION2 and 
> SECTION2_SIZE macros to calculate accurate image sizes when 
> appropriate.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  OvmfPkg/Sec/SecMain.c | 25 ++---
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index 
> a12e676..464de10 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -2,6 +2,7 @@
>Main SEC phase code.  Transitions to PEI.
>  
>Copyright (c) 2008 - 2015, Intel Corporation. All rights 
> reserved.
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of 
> the BSD License @@ -332,11 +333,13 @@ DecompressMemFvs (
>UINT32AuthenticationStatus;
>VOID  *OutputBuffer;
>VOID  *ScratchBuffer;
> -  EFI_FIRMWARE_VOLUME_IMAGE_SECTION *FvSection;
> +  EFI_COMMON_SECTION_HEADER *FvSection;
>EFI_FIRMWARE_VOLUME_HEADER*PeiMemFv;
>EFI_FIRMWARE_VOLUME_HEADER*DxeMemFv;
> +  UINT32FvHeaderSize;
> +  UINT32FvSectionSize;
>  
> -  FvSection = (EFI_FIRMWARE_VOLUME_IMAGE_SECTION*) NULL;
> +  FvSection = (EFI_COMMON_SECTION_HEADER*) NULL;
>  
>Status = FindFfsFileAndSection (
>   *Fv,
> @@ -386,7 +389,7 @@ DecompressMemFvs (
>   OutputBufferSize,
>   EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>   0,
> - (EFI_COMMON_SECTION_HEADER**) &FvSection
> + &FvSection
>   );
>if (EFI_ERROR (Status)) {
>  DEBUG ((EFI_D_ERROR, "Unable to find PEI FV section\n")); @@ 
> -411,7 +414,7 @@ DecompressMemFvs (
>   OutputBufferSize,
>   EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>   1,
> - (EFI_COMMON_SECTION_HEADER**) &FvSection
> + &FvSection
>   );
>if (EFI_ERROR (Status)) {
>  DEBUG ((EFI_D_ERROR, "Unable to find DXE FV section\n")); @@ 
> -419,11 +422,19 @@ DecompressMemFvs (
>}
>  
>ASSERT (FvSection->Type == EFI_SECTION_FIRMWARE_VOLUME_IMAGE);
> -  ASSERT (SECTION_SIZE (FvSection) ==
> -  (PcdGet32 (PcdOvmfDxeMemFvSize) + sizeof (*FvSection)));
> +
> +  if (IS_SECTION2 (FvSection)) {
> +FvSectionSize = SECTION2_SIZE (FvSection);
> +FvHeaderSize = sizeof (EFI_COMMON_SECTION_HEADER2);  } else {
> +FvSectionSize = SECTION_SIZE (FvSection);
> +FvHeaderSize = sizeof (EFI_COMMON_SECTION_HEADER);  }
> +
> +  ASSERT (FvSectionSize == (PcdGet32 (PcdOvmfDxeMemFvSize) + 
> + FvHeaderSize));
>  
>DxeMemFv = (EFI_FIRMWARE_VOLUME_HEADER*)(UINTN) PcdGet32 
> (PcdOvmfDxeMemFvBase);
> -  CopyMem (DxeMemFv, (VOID*) (FvSection + 1), PcdGet32 
> (PcdOvmfDxeMemFvSize));
> +  CopyMem (DxeMemFv, (VOID*) ((UINTN)FvSection + FvHeaderSize), 
> + PcdGet32 (PcdOvmfDxeMemFvSize));
>  
>if (DxeMemFv->Signature != EFI_FVH_SIGNATURE) {
>  DEBUG ((EFI_D_ERROR, "Extracted FV at %p does not have FV header 
> signature\n", DxeMemFv));
> 

(1) please split this patch in two: The first patch should only do the 
EFI_FIRMWARE_VOLUME_IMAGE_SECTION --> EFI_COMMON_SECTION_HEADER replacement 
(which is no change in behavior, as the former is a typedef to the latter), 
drop the superfluous casts, etc. The second patch should add the section2 
handling.

(2) EFI_COMMON_SECTION_HEADER.Size can describe a section that is almost 16MB 
in size. In OvmfPkg our DXEFV is currently 10MB in size (from commit 
2f7b34b20842f).

I think you must be building a number of proprietary (or otherwise
downstream-only) modules into OVMF for not fitting into 16MB. That's okay, but 
the use case must be clearly described in the commit message of patch 2, so 
that looking at the code later, we understand why it's there, without any use 
case for it within the open source edk2 tree.

So, in the commit message of patch 2, please mention
- that EFI_COMMON_SECTION_HEADER2.ExtendedSize is only necessary when the DXEFV 
size exceeds approximately 16MB, which is currently a downstream-only situation,
- and that the build tools automatically generate
EFI_COMMON_SECTION_HEADER2 when necessary.

(3) The subject line of patch #2 should name DXEFV rather than just "FV".

Looks good to me otherwise.

Thanks!
Laszlo

__

Re: [edk2] [PATCH v3 0/4] ArmPlatformPkg/ArmJunoPkg: Adds SMBIOS data for ARM Juno

[Jeremy Linton]

On 07/22/2016 11:38 AM, Ryan Harkin wrote:

Hi Jeremy,

Can I give you a kick re: this patch series?

It's been sitting in my landing team tree for months and hasn't been
resubmitted to reflect the move of Juno to OpenPlatformPkg.


I will resubmit it, I wasn't sure if it was "wanted" given all the 
hard-coded tables. Although, now that there is something of a random 
number generator, maybe it's worth the effort to generate some of the 
serial numbers/etc dynamically.






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

Re: [edk2] AArch64 XIP Recipe

[Cohen, Eugene]

Doing some git archaeology, I suspect that stuff started to go bad for us 
around this commit:



Revision: f37d891c1b870b294964adf65f619a661700fcab

Author: Ard Biesheuvel 

Date: 3/25/2016 5:33:28 AM

Message:

BaseTools AARCH64: move DEBUG GCC49 to the small code model



When building AARCH64 platforms that include a Shell binary built from

source, we run into trouble when using the tiny code model for DEBUG

builds. The reason is that the Shell binary built in DEBUG mode exceeds

the 1 MB range of the ADR instruction, so anything that gets pulled into

the final link of the Shell binary either needs to be built with the small

or large model, or needs to be sorted in some way to put the ADR references

close to their targets.



The rationale with this commit was that not only does the shell itself need to 
use the small model (to address the 1MB displacement limitation of the ADR 
instruction) but every library it pulls in must as well.  In our testing before 
this commit we did not see a problem, what did you see (if you can remember)?



But any time the small memory model is used the common page size is set to 4KB 
per the requirement stated in this change to BaseTools Elf64Convert.c:



Revision: 24d610e67752ac0325c7027e2fea2f8f2ff110e2

Author: Ard Biesheuvel 

Date: 8/10/2015 1:55:18 AM

Message:

BaseTools/GenFw: allow AArch64 tiny and small code model relocations



The AArch64 small C model makes extensive use of ADRP/ADD and

ADRP/{LDR,STR} pairs to emit PC-relative symbol references with

a +/- 4 GB range. Since the relocation pair splits the relative

offset into a relative page offset and an absolute offset into

a 4 KB page, we need to take extra care to ensure that the target

of the relocation preserves its alignment relative to a 4 KB

alignment boundary.



So since the shell requires all libraries to be built in the small memory model 
and all small memory model components must have 4KB alignment, this means that 
any component shared between XIP and the shell will get a 4KB alignment 
treatment.  As discussed before this just eats up flash/sram space in XIP 
regions since the FVs get padded out to meet this requirement.  This results in 
an untenable situation.



As I look at the history of this, this looks like a limitation in GCC where it 
doesn't emit the proper relocations for the page offset so as a workaround the 
page relationship of the code and data must be preserved across image 
conversion and relocation.  If this is fixed in GCC then can't we just let the 
relocations fix this?  If so, is it too outrageous to suggest fixing the linker?



Thanks,



Eugene





> -Original Message-

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf

> Of Cohen, Eugene

> Sent: Monday, July 25, 2016 9:02 AM

> To: Ard Biesheuvel 

> Cc: edk2-devel@lists.01.org

> Subject: [edk2] AArch64 XIP Recipe

>

> Ard,

>

> I'm in the midst of syncing up with master and am running into XIP size

> issues once again.  My XIP regions are picking up 4KB alignments and

> blowing up.

>

> I need a way in the same build to get the following:

>

> A. XIP Regions (SEC, PEI): tiny memory model, common page size 0x20,

> relocatable exceptions

> B. Non-XIP Regions (DXE): small memory model, common page size

> 0x1000, in-place exceptions

>

> It looks like the current tools_def stuff for GCC on AArch64 assumes

> the 4KB common page size in all cases.

>

> Furthermore, what is the recommend procedure for enabling the large

> memory model as required for large applications?  Do you recommend

> a specific inf or dsc override?

>

> Thanks,

>

> Eugene

>

>

> ___

> 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] AArch64 XIP Recipe

[Ard Biesheuvel]

On 25 July 2016 at 18:23, Cohen, Eugene  wrote:
> Doing some git archaeology, I suspect that stuff started to go bad for us
> around this commit:
>
>
>
> Revision: f37d891c1b870b294964adf65f619a661700fcab
>
> Author: Ard Biesheuvel 
>
> Date: 3/25/2016 5:33:28 AM
>
> Message:
>
> BaseTools AARCH64: move DEBUG GCC49 to the small code model
>
>
>
> When building AARCH64 platforms that include a Shell binary built from
>
> source, we run into trouble when using the tiny code model for DEBUG
>
> builds. The reason is that the Shell binary built in DEBUG mode exceeds
>
> the 1 MB range of the ADR instruction, so anything that gets pulled into
>
> the final link of the Shell binary either needs to be built with the small
>
> or large model, or needs to be sorted in some way to put the ADR references
>
> close to their targets.
>
>
>
> The rationale with this commit was that not only does the shell itself need
> to use the small model (to address the 1MB displacement limitation of the
> ADR instruction) but every library it pulls in must as well.  In our testing
> before this commit we did not see a problem, what did you see (if you can
> remember)?
>

The Shell failed to link due to the fact that the BASE libraries it
depends on were built with the tiny model

>
>
> But any time the small memory model is used the common page size is set to
> 4KB per the requirement stated in this change to BaseTools Elf64Convert.c:
>
>
>
> Revision: 24d610e67752ac0325c7027e2fea2f8f2ff110e2
>
> Author: Ard Biesheuvel 
>
> Date: 8/10/2015 1:55:18 AM
>
> Message:
>
> BaseTools/GenFw: allow AArch64 tiny and small code model relocations
>
>
>
> The AArch64 small C model makes extensive use of ADRP/ADD and
>
> ADRP/{LDR,STR} pairs to emit PC-relative symbol references with
>
> a +/- 4 GB range. Since the relocation pair splits the relative
>
> offset into a relative page offset and an absolute offset into
>
> a 4 KB page, we need to take extra care to ensure that the target
>
> of the relocation preserves its alignment relative to a 4 KB
>
> alignment boundary.
>
>
>
> So since the shell requires all libraries to be built in the small memory
> model and all small memory model components must have 4KB alignment, this
> means that any component shared between XIP and the shell will get a 4KB
> alignment treatment.  As discussed before this just eats up flash/sram space
> in XIP regions since the FVs get padded out to meet this requirement.  This
> results in an untenable situation.
>

Indeed.  The Shell in RELEASE mode builds fine with the tiny model
though. Since the Shell is the only problematic module (afaik), it
would suffice to simply build the shell components in RELEASE mode
unconditionally, assuming that you are not interested in debugging the
shell and your platform at the same time.


>
>
> As I look at the history of this, this looks like a limitation in GCC where
> it doesn't emit the proper relocations for the page offset so as a
> workaround the page relationship of the code and data must be preserved
> across image conversion and relocation.  If this is fixed in GCC then can't
> we just let the relocations fix this?  If so, is it too outrageous to
> suggest fixing the linker?
>

This is not a toolchain bug, but an ISA problem, and so it is not
going away, unfortunately.

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

Re: [edk2] [Patch v3 34/40] OvmfPkg: Add MpInitLib reference in DSC files.

[Laszlo Ersek]

On 07/25/16 04:52, Jeff Fan wrote:
> This update is for CpuMpPei&CpuDxe consuming MP Initialize library.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  OvmfPkg/OvmfPkgIa32.dsc| 2 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
>  OvmfPkg/OvmfPkgX64.dsc | 2 ++
>  3 files changed, 6 insertions(+)

Please update your git configuration according to the following:

*
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05

  git config diff.ini.xfuncname '^\[[A-Za-z0-9_., ]+]'

* and
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-09

i.e., please add the following to " .git/info/attributes":

*.dec diff=ini
*.dsc diff=ini
*.dsc.inc diff=ini
*.fdf diff=ini
*.fdf.inc diff=ini
*.inf diff=ini

If you do that, then I'll be able to see from the patch itself what
sections in the DSC file are being modified.

Thanks
Laszlo

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

Re: [edk2] [Patch v3 06/40] UefiCpuPkg/MpInitLib: Add AP assembly code and MP_CPU_EXCHANGE_INFO

[Laszlo Ersek]

Jeff,

On 07/25/16 04:52, Jeff Fan wrote:
> Add assembly code for AP reset vector and the definition of 
> MP_CPU_EXCHANGE_INFO
> that are used to exchange the data between C code and assembly code when AP 
> wake
> up.
> 
> v3:
>   1. Rename NumApsExecutingLoction to NumApsExecutingLocation
>   2. Add whitespace after ; in .nasm file
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf  |   8 ++
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc|  37 +
>  UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm | 170 +++
>  UefiCpuPkg/Library/MpInitLib/MpLib.h   |  24 
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf  |   8 ++
>  UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc |  39 ++
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm  | 178 
> +
>  7 files changed, 464 insertions(+)
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm

Patches like this (= patches that move code around, possibly tweaking
it) are extremely hard to review without dedicated help from the
submitter. Such as:

- when copying a structure definition from another file, please spell
  out the origin of the structure definition. Example:
  MP_CPU_EXCHANGE_INFO comes from "UefiCpuPkg/CpuMpPei/CpuMpPei.h" with
  minimal changes.

- The patch should be please formatted with "--find-copies-harder". This
  will cause git to identify the best source file that may be the origin
  for a copy, and then express the new file as (copy + diff), rather
  than brand new code.

For example, I can look at this patch
(c661d4b85b748f03ed3e47c2b8424f797bdc0aaf) using git directly, from your
repo, with the following options:

  git show --find-copies-harder --stat=1000 --stat-graph-width=20 \
c661d4b85b748f03ed3e47c2b8424f797bdc0aaf

It produces the following diffstat:

 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf|   8 ++
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Ia32/MpEqu.inc|   8 +-
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/Ia32/MpFuncs.nasm | 129 
+-
 UefiCpuPkg/Library/MpInitLib/MpLib.h |  24 
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf|   8 ++
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/X64/MpEqu.inc |  10 +-
 UefiCpuPkg/{CpuMpPei => Library/MpInitLib}/X64/MpFuncs.nasm  | 141 
+++-
 7 files changed, 101 insertions(+), 227 deletions(-)

This tells me where the new files come from, and I can focus on the
differences only. Otherwise, I have to review all of the code as brand
new code, which is *immensely* more work.

Now, using the --find-copies-harder switch, I can see a number of
unjustified differences immediately:

- "NumApsExecutingLoction" is replaced with "NumApsExecutingLocation".
  This is a typo fix, and should be in a separate patch.

- EnableExecuteDisableLocation and Cr3Location are new. The commit
  message should describe these fields in detail, and why they are
  necessary. (In practice, how we differ from CpuMpPei here.)

- The whitespace after ";" is actually a bad idea for this patch. It is
  in the same category as typo fixes: they can be justified, of course,
  but only as separate patches, so that they aren't intermixed with
  semantic changes.

- I see a few more changes of the same class (invoke -> Invoke, never ->
  Never)

I'm not a UefiCpuPkg maintainer, so technically my review is not
necessary here. If you CC'd me so that I *test* the series, I'm happy to
do it, and report back with results. However, if you'd like me to
*review* the series, then I'll need a *lot* more detail in the commit
messages -- in practice you'll have to document your entire thought
process. Every decision that is based on your knowledge with CpuMpPei
and CpuMpDxe (which could be obvious to other UefiCpuPkg maintainers)
will have to be spelled out for me in the commit messages.

Do you want me to review the series, or are you okay if I just test it?

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

[edk2] AArch64 XIP Recipe

[Cohen, Eugene]

Ard,

I'm in the midst of syncing up with master and am running into XIP size issues 
once again.  My XIP regions are picking up 4KB alignments and blowing up.

I need a way in the same build to get the following:

A. XIP Regions (SEC, PEI): tiny memory model, common page size 0x20, 
relocatable exceptions
B. Non-XIP Regions (DXE): small memory model, common page size 0x1000, in-place 
exceptions

It looks like the current tools_def stuff for GCC on AArch64 assumes the 4KB 
common page size in all cases.

Furthermore, what is the recommend procedure for enabling the large memory 
model as required for large applications?  Do you recommend a specific inf or 
dsc override?

Thanks,

Eugene


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

Re: [edk2] [Patch v3 05/40] UefiCpuPkg/MpInitLib: Add two instances PeiMpInitLib and DxeMpInitLib

[Laszlo Ersek]

On 07/25/16 04:52, Jeff Fan wrote:
> Add two MP Initialize Library instances PeiMpInitLib.inf and DxeMpInitLib.inf
> with NULL implementation.
> 
> v3:
>   1. Rename MpInitLibSwitchBsp to MpInitLibSwitchBSP to match PI spec
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  61 ++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.uni |  22 +++
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   | 266 +
>  UefiCpuPkg/Library/MpInitLib/MpLib.c  | 119 
>  UefiCpuPkg/Library/MpInitLib/MpLib.h  |  40 
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  61 ++
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.uni |  22 +++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 269 
> ++
>  UefiCpuPkg/UefiCpuPkg.dsc |   3 +-
>  9 files changed, 862 insertions(+), 1 deletion(-)
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.uni
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLib.c
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/MpLib.h
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.uni
>  create mode 100644 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c

(1) A Null implementation that does not fulfill the promises of the lib
class interfaces should not invariably return EFI_SUCCESS in my opinion.
Instead, the MpInitLibInitialize() function should return
EFI_UNSUPPORTED, and all other APIs should return EFI_NOT_READY.

I realize this is purely cosmetic as the implementation will change in
the rest of the series.

(2) I see that some, but not all, of the APIs are kept in the common
(shared) code, while the rest is specialized to PEI vs. DXE. I
understand that this split lays the foundation for the future
implementation, but it should be documented in the commit message -- in
particular it should be announced in this patch *why* the functions are
distributed like this.

(3) Once the comment / naming issues are fixed in the library class
header file that I mentioned earlier, this patch should be adapted.

(In fact I wouldn't mind if these library instances omitted those huge
comments altogether. I know it wouldn't conform to the edk2 coding
style, but comment blocks of *this* size just make it harder to read the
code; and the mindless copying just perpetuates errors in the comments.
This patch adds 800+ lines of code, without any functionality at all.)

I think (2) is quite important. The other two comments are not crazy
important.

Thanks
Laszlo

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

Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode

[Gao, Liming]

Without -DSECURE_BOOT_ENABLE=TRUE, OVMF on GCC5 in LTO mode can pass build.

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Monday, July 25, 2016 10:04 PM
To: Gao, Liming 
Cc: Justen, Jordan L ; edk2-devel@lists.01.org; 
leif.lindh...@linaro.org; ler...@redhat.com
Subject: Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode


> On 25 jul. 2016, at 15:21, Gao, Liming wrote:
>
> Ard:
> After apply these patches, I try to build OvmfPkg with SECURE enable on GCC5 
> in LTO mode.


Did you also try it without?

> It reports build failure. Have you tried such build before?
>

No I have not.

> Build command: build -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 
> -DSECURE_BOOT_ENABLE=TRUE -a IA32 -a X64
> Build error message, seemly OpenSsl can't be linked with LTO.
>

I will investigate. It indeed looks like an incompatibility with lto, I should 
check whether Arm is affected as well.

Thanks,
Ard.

> "gcc" -o 
> /home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll
>  -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x40 
> -Wl,--entry,_ModuleEntryPoint -u _ModuleEntryPoint 
> -Wl,-Map,/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.map
>  -Wl,-melf_x86_64,--oformat=elf64-x86-64 
> -Wl,--start-group,@/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/OUTPUT/static_library_files.lst,--end-group
>  -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 
> -Wl,--script=/home/hwu/work/lgao4/AllPkg/edk2/BaseTools/Scripts/GccBase.lds
> /tmp/cc9uBgrd.ltrans0.ltrans.o: In function `DxeImageVerificationHandler':
> :(.text+0x1f13): undefined reference to `malloc'
> :(.text+0x1f7a): undefined reference to `free'
> :(.text+0x1f92): undefined reference to `malloc'
> :(.text+0x1fb9): undefined reference to `free'
> :(.text+0x1fe3): undefined reference to `free'
> :(.text+0x2027): undefined reference to `malloc'
> :(.text+0x20bf): undefined reference to `free'
> :(.text+0x2116): undefined reference to `free'
> :(.text+0x212d): undefined reference to `free'
> :(.text+0x213a): undefined reference to `free'
> :(.text+0x21f7): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans0.ltrans.o::(.text+0x2204): more undefined references to 
> `free' follow
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `default_realloc_ex.lto_priv.190':
> :(.text+0x1): undefined reference to `realloc'
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `default_malloc_ex.lto_priv.187':
> :(.text+0x6): undefined reference to `malloc'
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `CRYPTO_free':
> :(.text+0x613): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `WrapPkcs7Data':
> :(.text.unlikely+0x72): undefined reference to `malloc'
> /tmp/cc9uBgrd.ltrans17.ltrans.o: In function `OPENSSL_gmtime':
> :(.text+0x176a): undefined reference to `malloc'
> /tmp/cc9uBgrd.ltrans18.ltrans.o: In function `RSA_free':
> :(.text+0x123a): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans20.ltrans.o: In function `qsort':
> :(.text+0x1368): undefined reference to `malloc'
> :(.text+0x13bc): undefined reference to `malloc'
> :(.text+0x13b4): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans27.ltrans.o: In function 
> `CRYPTO_realloc_clean.constprop.153':
> :(.text+0x20e): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans27.ltrans.o:(.data.rel.ro.local+0x578): undefined 
> reference to `free'
> /usr/bin/ld: 
> /home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll:
>  protected symbol `realloc' isn't defined
> /usr/bin/ld: final link failed: Bad value
> collect2: error: ld returned 1 exit status
> make: *** 
> [/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll]
>  Error 1
>
>
> build.py...
> : error 7000: Failed to execute command
> make tbuild 
> [/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe]
>
>
> build.py...
> : error F002: Failed to build module
> /home/hwu/work/lgao4/AllPkg/edk2/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  [X64, GCC5, DEBUG]
>
> Thanks
> Liming
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Saturday, July 23, 2016 5:03 PM
>> To: edk2-devel@lists.01.org; 
>> ler...@redhat.com; Gao, Liming
>> ; Shi, Steven ; Zhu,
>> Yonghong ; Justen, Jordan L
>>
>> Cc: leif.lindh...@linaro.org; Ard Biesheuvel
>> Subject: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode
>>
>> This v3 to introduce GCC5 is now a 6 piece series, i

[edk2] [Patch 2/8] MdeModulePkg UefiBootManagerLib: Support LoadFile Protocol based on FV

[Liming Gao]

New LoadFileOnFv2 driver will install LoadFile protocol based on FV file.
Update UefiBootManagerLib to find BootMenuApp with LoadFile protocol.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 116 +++
 1 file changed, 76 insertions(+), 40 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 18259e9..d5818ca 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1940,6 +1940,7 @@ BmEnumerateBootOptions (
   UINTN Removable;
   UINTN Index;
   CHAR16*Description;
+  UINT32BootAttributes;
 
   ASSERT (BootOptionCount != NULL);
 
@@ -2059,7 +2060,7 @@ BmEnumerateBootOptions (
   }
 
   //
-  // Parse load file, assuming UEFI Network boot option
+  // Parse load file protocol
   //
   gBS->LocateHandleBuffer (
  ByProtocol,
@@ -2078,11 +2079,19 @@ BmEnumerateBootOptions (
 );
 ASSERT (BootOptions != NULL);
 
+//
+// If LoadFile includes BootMenuApp, its boot attribue will be set to APP 
and HIDDEN.
+//
+BootAttributes = LOAD_OPTION_ACTIVE;
+if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+  BootAttributes = LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | 
LOAD_OPTION_HIDDEN;
+}
+
 Status = EfiBootManagerInitializeLoadOption (
&BootOptions[(*BootOptionCount)++],
LoadOptionNumberUnassigned,
LoadOptionTypeBoot,
-   LOAD_OPTION_ACTIVE,
+   BootAttributes,
Description,
DevicePathFromHandle (Handles[Index]),
NULL,
@@ -2197,51 +2206,78 @@ BmRegisterBootManagerMenu (
   EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
   EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage;
   MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;
+  UINTN  HandleCount;
+  EFI_HANDLE *Handles;
+  UINTN  Index;
   VOID   *Data;
   UINTN  DataSize;
 
-  Data = NULL;
-  Status = GetSectionFromFv (
- PcdGetPtr (PcdBootManagerMenuFile),
- EFI_SECTION_PE32,
- 0,
- (VOID **) &Data,
- &DataSize
- );
-  if (Data != NULL) {
-FreePool (Data);
-  }
-  if (EFI_ERROR (Status)) {
-DEBUG ((EFI_D_WARN, "[Bds]BootManagerMenu FFS section can not be found, 
skip its boot option registration\n"));
-return EFI_NOT_FOUND;
-  }
-
+  DevicePath = NULL;
   //
-  // Get BootManagerMenu application's description from EFI User Interface 
Section.
+  // Try to find BootMenuApp from LoadFile protocol
   //
-  Status = GetSectionFromFv (
- PcdGetPtr (PcdBootManagerMenuFile),
- EFI_SECTION_USER_INTERFACE,
- 0,
- (VOID **) &Description,
- &DescriptionLength
- );
-  if (EFI_ERROR (Status)) {
-Description = NULL;
+  gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEfiLoadFileProtocolGuid,
+ NULL,
+ &HandleCount,
+ &Handles
+ );
+  for (Index = 0; Index < HandleCount; Index++) {
+if (BmIsBootMenuAppFilePath (DevicePathFromHandle (Handles[Index]))) {
+  DevicePath  = DuplicateDevicePath (DevicePathFromHandle 
(Handles[Index]));
+  Description = BmGetBootDescription (Handles[Index]);
+  break;
+}
+  }
+  if (HandleCount != 0) {
+FreePool (Handles);
   }
 
-  EfiInitializeFwVolDevicepathNode (&FileNode, PcdGetPtr 
(PcdBootManagerMenuFile));
-  Status = gBS->HandleProtocol (
-  gImageHandle,
-  &gEfiLoadedImageProtocolGuid,
-  (VOID **) &LoadedImage
-  );
-  ASSERT_EFI_ERROR (Status);
-  DevicePath = AppendDevicePathNode (
- DevicePathFromHandle (LoadedImage->DeviceHandle),
- (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
- );
-  ASSERT (DevicePath != NULL);
+  if (DevicePath == NULL) {
+Data = NULL;
+Status = GetSectionFromFv (
+   PcdGetPtr (PcdBootManagerMenuFile),
+   EFI_SECTION_PE32,
+   0,
+   (VOID **) &Data,
+   &DataSize
+   );
+if (Data != NULL) {
+  FreePool (Data);
+}
+if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_WARN, "[Bds]BootManagerMenu FFS section can not be found, 
skip its boot option registration\n"));
+  return EFI_NOT_FOUND;
+}
+
+//
+// Get BootManagerMenu application's description from EFI User Interface 
Section.
+//
+Status = GetSectionFromFv (
+   PcdGetPtr (PcdBootManagerMenuFil

Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode

[Ard Biesheuvel]

> On 25 jul. 2016, at 15:21, Gao, Liming  wrote:
> 
> Ard:
> After apply these patches, I try to build OvmfPkg with SECURE enable on GCC5 
> in LTO mode.


Did you also try it without?

> It reports build failure. Have you tried such build before?
> 

No I have not.

> Build command: build -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 
> -DSECURE_BOOT_ENABLE=TRUE -a IA32 -a X64
> Build error message, seemly OpenSsl can't be linked with LTO.
> 

I will investigate. It indeed looks like an incompatibility with lto, I should 
check whether Arm is affected as well.

Thanks,
Ard.

> "gcc" -o 
> /home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll
>  -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x40 
> -Wl,--entry,_ModuleEntryPoint -u _ModuleEntryPoint 
> -Wl,-Map,/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.map
>  -Wl,-melf_x86_64,--oformat=elf64-x86-64 
> -Wl,--start-group,@/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/OUTPUT/static_library_files.lst,--end-group
>  -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 
> -Wl,--script=/home/hwu/work/lgao4/AllPkg/edk2/BaseTools/Scripts/GccBase.lds
> /tmp/cc9uBgrd.ltrans0.ltrans.o: In function `DxeImageVerificationHandler':
> :(.text+0x1f13): undefined reference to `malloc'
> :(.text+0x1f7a): undefined reference to `free'
> :(.text+0x1f92): undefined reference to `malloc'
> :(.text+0x1fb9): undefined reference to `free'
> :(.text+0x1fe3): undefined reference to `free'
> :(.text+0x2027): undefined reference to `malloc'
> :(.text+0x20bf): undefined reference to `free'
> :(.text+0x2116): undefined reference to `free'
> :(.text+0x212d): undefined reference to `free'
> :(.text+0x213a): undefined reference to `free'
> :(.text+0x21f7): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans0.ltrans.o::(.text+0x2204): more undefined references to 
> `free' follow
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `default_realloc_ex.lto_priv.190':
> :(.text+0x1): undefined reference to `realloc'
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `default_malloc_ex.lto_priv.187':
> :(.text+0x6): undefined reference to `malloc'
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `CRYPTO_free':
> :(.text+0x613): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans8.ltrans.o: In function `WrapPkcs7Data':
> :(.text.unlikely+0x72): undefined reference to `malloc'
> /tmp/cc9uBgrd.ltrans17.ltrans.o: In function `OPENSSL_gmtime':
> :(.text+0x176a): undefined reference to `malloc'
> /tmp/cc9uBgrd.ltrans18.ltrans.o: In function `RSA_free':
> :(.text+0x123a): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans20.ltrans.o: In function `qsort':
> :(.text+0x1368): undefined reference to `malloc'
> :(.text+0x13bc): undefined reference to `malloc'
> :(.text+0x13b4): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans27.ltrans.o: In function 
> `CRYPTO_realloc_clean.constprop.153':
> :(.text+0x20e): undefined reference to `free'
> /tmp/cc9uBgrd.ltrans27.ltrans.o:(.data.rel.ro.local+0x578): undefined 
> reference to `free'
> /usr/bin/ld: 
> /home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll:
>  protected symbol `realloc' isn't defined
> /usr/bin/ld: final link failed: Bad value
> collect2: error: ld returned 1 exit status
> make: *** 
> [/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll]
>  Error 1
> 
> 
> build.py...
> : error 7000: Failed to execute command
> make tbuild 
> [/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe]
> 
> 
> build.py...
> : error F002: Failed to build module
> /home/hwu/work/lgao4/AllPkg/edk2/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
>  [X64, GCC5, DEBUG]
> 
> Thanks
> Liming
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Ard Biesheuvel
>> Sent: Saturday, July 23, 2016 5:03 PM
>> To: edk2-devel@lists.01.org; ler...@redhat.com; Gao, Liming
>> ; Shi, Steven ; Zhu,
>> Yonghong ; Justen, Jordan L
>> 
>> Cc: leif.lindh...@linaro.org; Ard Biesheuvel
>> Subject: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode
>> 
>> This v3 to introduce GCC5 is now a 6 piece series, including some
>> preparatory cleanup patches that allow all GCC4x and CLANG35 toolchains
>> to switch to using 'gcc' as the linker. This allows us to get rid of
>> the wrapper script to marshall ld arguments in order to make them
>> understandable by gcc, which is fragile and likely to cause problems in
>> the future.
>> 
>> Since there appears to be a natural split between the 'legacy' GCC
>> toolchains UNIXGCC, ELFGCC, and CYGGCC[xASL], both in term of supported
>> arc

[edk2] [Patch 5/8] MdeModulePkg LoadFileOnFv2: Add new LoadFileOnFv2 driver

[Liming Gao]

This driver searches APPLICATION in FV and installs LoadFile protocol
for every found one. Then, BDS will add BootOption for LoadFile protocol.

It provides the generic way to expose boot option for the internal
application, such as Shell. With this driver, PlatformBds doesn’t need
to specially handle Shell application.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../Universal/LoadFileOnFv2/LoadFileOnFv2.c| 421 +
 .../Universal/LoadFileOnFv2/LoadFileOnFv2.inf  |  68 
 .../Universal/LoadFileOnFv2/LoadFileOnFv2.uni  |  24 ++
 .../Universal/LoadFileOnFv2/LoadFileOnFv2Extra.uni |  18 +
 4 files changed, 531 insertions(+)
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.uni
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2Extra.uni

diff --git a/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c 
b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
new file mode 100644
index 000..6ef9fac
--- /dev/null
+++ b/MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
@@ -0,0 +1,421 @@
+/** @file
+  Produce Load File Protocol for UEFI Applications in Firmware Volumes
+
+  Copyright (c) 2011 - 2013, Intel Corporation
+  All rights reserved. This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE  SIGNATURE_32 ('l', 'f', 'f', 
'v')
+
+typedef struct {
+  UINTN  Signature;
+  EFI_LOAD_FILE_PROTOCOL LoadFile;
+  EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
+  EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
+  EFI_GUID   NameGuid;
+  LIST_ENTRY Link;
+} LOAD_FILE_ON_FV2_PRIVATE_DATA;
+
+#define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_THIS(a) CR (a, 
LOAD_FILE_ON_FV2_PRIVATE_DATA, LoadFile, 
LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE)
+#define LOAD_FILE_ON_FV2_PRIVATE_DATA_FROM_LINK(a) CR (a, 
LOAD_FILE_ON_FV2_PRIVATE_DATA, Link, LOAD_FILE_ON_FV2_PRIVATE_DATA_SIGNATURE)
+
+EFI_EVENT  mFvRegistration;
+LIST_ENTRY mPrivateDataList;
+
+/**
+  Causes the driver to load a specified file from firmware volume.
+
+  @param[in]  ThisProtocol instance pointer.
+  @param[in]  FilePathThe device specific path of the file to 
load.
+  @param[in]  BootPolicy  If TRUE, indicates that the request 
originates from the
+  boot manager is attempting to load 
FilePath as a boot
+  selection. If FALSE, then FilePath must 
match an exact file
+  to be loaded.
+  @param[in, out] BufferSize  On input the size of Buffer in bytes. On 
output with a return
+  code of EFI_SUCCESS, the amount of data 
transferred to
+  Buffer. On output with a return code of 
EFI_BUFFER_TOO_SMALL,
+  the size of Buffer required to retrieve 
the requested file.
+  @param[in]  Buffer  The memory buffer to transfer the file 
to. IF Buffer is NULL,
+  then no the size of the requested file 
is returned in
+  BufferSize.
+
+  @retval EFI_SUCCESS The file was loaded.
+  @retval EFI_UNSUPPORTED The device does not support the provided 
BootPolicy.
+  @retval EFI_INVALID_PARAMETER   FilePath is not a valid device path, or
+  BufferSize is NULL.
+  @retval EFI_DEVICE_ERRORThe file was not loaded due to a device 
error.
+  @retval EFI_NOT_FOUND   The file was not found.
+  @retval EFI_OUT_OF_RESOURCESAn allocation failure occurred.
+  @retval EFI_ACCESS_DENIED   The firmware volume is configured to
+  disallow reads.
+**/
+EFI_STATUS
+EFIAPI
+LoadFileOnFv2LoadFile (
+  IN EFI_LOAD_FILE_PROTOCOL*This,
+  IN EFI_DEVICE_PATH_PROTOCOL  *FilePath,
+  IN BOOLEAN   BootPolicy,
+  IN OUT UINTN *BufferSize,
+  IN VOID  *Buffer   OPTIONAL
+  )
+{
+  EFI_STATUS Status;
+  LOAD_FILE_ON_FV2_PRIVA

[edk2] [Patch 3/8] MdeModulePkg UefiBootManagerLib: Update LoadFile boot description

[Liming Gao]

Update boot description to support LoadFile protocol based on FV file.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../Library/UefiBootManagerLib/BmBootDescription.c | 47 ++
 1 file changed, 47 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
index 066ea80..f086764 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
@@ -455,6 +455,52 @@ BmGetNetworkDescription (
 }
 
 /**
+  Return the boot description for LoadFile
+
+  @param HandleController handle.
+
+  @return  The description string.
+**/
+CHAR16 *
+BmGetLoadFileDescription (
+  IN EFI_HANDLE  Handle
+  )
+{
+  EFI_STATUSStatus;
+  EFI_DEVICE_PATH_PROTOCOL  *FilePath;
+  EFI_DEVICE_PATH_PROTOCOL  *DevicePathNode;
+  CHAR16*Description;
+  EFI_LOAD_FILE_PROTOCOL*LoadFile;
+
+  Status = gBS->HandleProtocol (Handle, &gEfiLoadFileProtocolGuid, (VOID 
**)&LoadFile);
+  if (EFI_ERROR (Status)) {
+return NULL;
+  }
+
+  //
+  // Get the file name
+  //
+  Description = NULL;
+  Status = gBS->HandleProtocol (Handle, &gEfiDevicePathProtocolGuid, (VOID 
**)&FilePath);
+  if (!EFI_ERROR (Status)) {
+DevicePathNode = FilePath;
+while (!IsDevicePathEnd (DevicePathNode)) {
+  if (DevicePathNode->Type == MEDIA_DEVICE_PATH && DevicePathNode->SubType 
== MEDIA_FILEPATH_DP) {
+Description = (CHAR16 *)(DevicePathNode + 1);
+break;
+  }
+  DevicePathNode = NextDevicePathNode (DevicePathNode);
+}
+  }
+
+  if (Description != NULL) {
+return AllocateCopyPool (StrSize (Description), Description);
+  }
+
+  return NULL;
+}
+
+/**
   Return the boot description for the controller based on the type.
 
   @param HandleController handle.
@@ -559,6 +605,7 @@ BM_GET_BOOT_DESCRIPTION mBmBootDescriptionHandlers[] = {
   BmGetUsbDescription,
   BmGetDescriptionFromDiskInfo,
   BmGetNetworkDescription,
+  BmGetLoadFileDescription,
   BmGetMiscDescription
 };
 
-- 
2.8.0.windows.1

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

[edk2] [Patch 6/8] MdeModulePkg: Add new LoadFileOnFv2 in Package DSC for Build

[Liming Gao]

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/MdeModulePkg.dsc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 1e57389..8c0875b 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -405,6 +405,7 @@
   }
 
   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+  MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
 
 [Components.IA32, Components.X64, Components.IPF]  
   MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
-- 
2.8.0.windows.1

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

[edk2] [Patch 4/8] MdeModulePkg UefiBootManagerLib: Enhance EfiBootManagerFindLoadOption

[Liming Gao]

The same boot file may have two different device paths. One is its FV
File device path, another is LoadFile FV device path. These two paths
includes the same NameGuid. So, EfiBootManagerFindLoadOption() is updated
to compare their NameGuid.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../Library/UefiBootManagerLib/BmLoadOption.c  | 44 --
 1 file changed, 41 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 9af98de..f9650ff 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -506,6 +506,29 @@ EfiBootManagerInitializeLoadOption (
   return EFI_SUCCESS;
 }
 
+/**
+  Find NameGuid from the input device path.
+
+  @param  DevicePath Input device path.
+
+  @retval NULL  NameGuid can't be found in the input device path.
+  @retval NameGuid  Pointer to NameGuid found in the input device path.
+**/
+EFI_GUID *
+BmFileNameGuidFromFilePath (
+  EFI_DEVICE_PATH_PROTOCOL *DevicePath
+)
+{
+  EFI_HANDLE  FvHandle;
+  EFI_STATUS  Status;
+
+  Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid, 
&DevicePath, &FvHandle);
+  if (!EFI_ERROR (Status)) {
+return EfiGetNameGuidFromFwVolDevicePathNode ((CONST 
MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *) DevicePath);
+  }
+
+  return NULL;
+}
 
 /**
   Return the index of the load option in the load option array.
@@ -529,15 +552,30 @@ EfiBootManagerFindLoadOption (
   )
 {
   UINTN Index;
+  EFI_GUID  *KeyNameGuid;
+  EFI_GUID  *NameGuid;
+
+  KeyNameGuid = NULL;
+  if (Key->OptionType == LoadOptionTypeBoot) {
+KeyNameGuid = BmFileNameGuidFromFilePath (Key->FilePath);
+  }
 
   for (Index = 0; Index < Count; Index++) {
-if ((Key->OptionType == Array[Index].OptionType) &&
-(Key->Attributes == Array[Index].Attributes) &&
+if (Key->OptionType == Array[Index].OptionType) {
+  if ((Key->Attributes == Array[Index].Attributes) &&
 (StrCmp (Key->Description, Array[Index].Description) == 0) &&
 (CompareMem (Key->FilePath, Array[Index].FilePath, GetDevicePathSize 
(Key->FilePath)) == 0) &&
 (Key->OptionalDataSize == Array[Index].OptionalDataSize) &&
 (CompareMem (Key->OptionalData, Array[Index].OptionalData, 
Key->OptionalDataSize) == 0)) {
-  return (INTN) Index;
+return (INTN) Index;
+  }
+
+  if (KeyNameGuid != NULL) {
+NameGuid = BmFileNameGuidFromFilePath (Array[Index].FilePath);
+if ((NameGuid != NULL) && CompareGuid (KeyNameGuid, NameGuid)) {
+  return (INTN) Index;
+}
+  }
 }
   }
 
-- 
2.8.0.windows.1

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

[edk2] [Patch 7/8] Nt32Pkg: Add LoadFileOnFv2 driver in DSC/FDF

[Liming Gao]

After add LoadFileOnFv2, PlatformBootManagerLib doesn't need to find Shell
application from FV.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../PlatformBootManagerLib/PlatformBootManager.c   | 123 +++--
 Nt32Pkg/Nt32Pkg.dsc|   1 +
 Nt32Pkg/Nt32Pkg.fdf|   2 +-
 3 files changed, 19 insertions(+), 107 deletions(-)

diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c 
b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 007e18a..e2c3a4e 100644
--- a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -58,91 +58,6 @@ PlatformBootManagerDiagnostics (
 }
 
 /**
-  Return the index of the load option in the load option array.
-
-  The function consider two load options are equal when the 
-  OptionType, Attributes, Description, FilePath and OptionalData are equal.
-
-  @param KeyPointer to the load option to be found.
-  @param Array  Pointer to the array of load options to be found.
-  @param Count  Number of entries in the Array.
-
-  @retval -1  Key wasn't found in the Array.
-  @retval 0 ~ Count-1 The index of the Key in the Array.
-**/
-INTN
-PlatformFindLoadOption (
-  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Key,
-  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Array,
-  IN UINTN  Count
-  )
-{
-  UINTN Index;
-
-  for (Index = 0; Index < Count; Index++) {
-if ((Key->OptionType == Array[Index].OptionType) &&
-(Key->Attributes == Array[Index].Attributes) &&
-(StrCmp (Key->Description, Array[Index].Description) == 0) &&
-(CompareMem (Key->FilePath, Array[Index].FilePath, GetDevicePathSize 
(Key->FilePath)) == 0) &&
-(Key->OptionalDataSize == Array[Index].OptionalDataSize) &&
-(CompareMem (Key->OptionalData, Array[Index].OptionalData, 
Key->OptionalDataSize) == 0)) {
-  return (INTN) Index;
-}
-  }
-
-  return -1;
-}
-
-VOID
-PlatformRegisterFvBootOption (
-  EFI_GUID *FileGuid,
-  CHAR16   *Description,
-  UINT32   Attributes
-  )
-{
-  EFI_STATUSStatus;
-  UINTN OptionIndex;
-  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
-  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
-  UINTN BootOptionCount;
-  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
-  EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
-  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
-
-  Status = gBS->HandleProtocol (gImageHandle, &gEfiLoadedImageProtocolGuid, 
(VOID **) &LoadedImage);
-  ASSERT_EFI_ERROR (Status);
-
-  EfiInitializeFwVolDevicepathNode (&FileNode, FileGuid);
-  DevicePath = AppendDevicePathNode (
- DevicePathFromHandle (LoadedImage->DeviceHandle),
- (EFI_DEVICE_PATH_PROTOCOL *) &FileNode
- );
-
-  Status = EfiBootManagerInitializeLoadOption (
- &NewOption,
- LoadOptionNumberUnassigned,
- LoadOptionTypeBoot,
- Attributes,
- Description,
- DevicePath,
- NULL,
- 0
- );
-  if (!EFI_ERROR (Status)) {
-BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount, 
LoadOptionTypeBoot);
-
-OptionIndex = PlatformFindLoadOption (&NewOption, BootOptions, 
BootOptionCount);
-
-if (OptionIndex == -1) {
-  Status = EfiBootManagerAddLoadOptionVariable (&NewOption, (UINTN) -1);
-  ASSERT_EFI_ERROR (Status);
-}
-EfiBootManagerFreeLoadOption (&NewOption);
-EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
-  }
-}
-
-/**
   Do the platform specific action before the console is connected.
 
   Such as:
@@ -159,9 +74,6 @@ PlatformBootManagerBeforeConsole (
   UINTNIndex;
   EFI_STATUS   Status;
   WIN_NT_SYSTEM_CONFIGURATION  *Configuration;
-  EFI_INPUT_KEYEnter;
-  EFI_INPUT_KEYF2;
-  EFI_BOOT_MANAGER_LOAD_OPTION BootOption;
 
   GetVariable2 (L"Setup", &gEfiWinNtSystemConfigGuid, (VOID **) 
&Configuration, NULL);
   if (Configuration != NULL) {
@@ -200,24 +112,6 @@ PlatformBootManagerBeforeConsole (
   EfiBootManagerUpdateConsoleVariable (ErrOut, 
gPlatformConsole[Index].DevicePath, NULL);
 }
   }
-
-  //
-  // Register ENTER as CONTINUE key
-  //
-  Enter.ScanCode= SCAN_NULL;
-  Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
-  EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
-  //
-  // Map F2 to Boot Manager Menu
-  //
-  F2.ScanCode= SCAN_F2;
-  F2.UnicodeChar = CHAR_NULL;
-  EfiBootManagerGetBootManagerMenu (&BootOption);
-  EfiBootManagerAddKeyOptionVariable (NULL, (UINT16) BootOption.OptionNumber, 
0, &F2, NULL);
-  //
-  // Register UEFI Shell
-

[edk2] [Patch 8/8] Nt32Pkg: Make Shell as the first boot option

[Liming Gao]

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../PlatformBootManagerLib/PlatformBootManager.c   | 34 ++
 1 file changed, 34 insertions(+)

diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c 
b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index e2c3a4e..4b23eb1 100644
--- a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -115,6 +115,35 @@ PlatformBootManagerBeforeConsole (
 }
 
 /**
+  Returns the priority number.
+
+  @param BootOption
+**/
+UINTN
+BootOptionPriority (
+  CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOption
+  )
+{
+  //
+  // Make sure Shell is first
+  //
+  if (StrCmp (BootOption->Description, L"UEFI Shell") == 0) {
+return 0;
+  }
+  return 100;
+}
+
+INTN
+EFIAPI
+CompareBootOption (
+  CONST EFI_BOOT_MANAGER_LOAD_OPTION  *Left,
+  CONST EFI_BOOT_MANAGER_LOAD_OPTION  *Right
+  )
+{
+  return BootOptionPriority (Left) - BootOptionPriority (Right);
+}
+
+/**
   Do the platform specific action after the console is connected.
 
   Such as:
@@ -156,6 +185,11 @@ PlatformBootManagerAfterConsole (
   EfiBootManagerGetBootManagerMenu (&BootOption);
   EfiBootManagerAddKeyOptionVariable (NULL, (UINT16) BootOption.OptionNumber, 
0, &F2, NULL);
 
+  //
+  // Make Shell as the first boot option
+  //
+  EfiBootManagerSortLoadOptionVariable (LoadOptionTypeBoot, (SORT_COMPARE) 
CompareBootOption);
+
   PlatformBootManagerDiagnostics (QUICK, TRUE);
   
   PrintXY (10, 10, &White, &Black, L"F2to enter Boot Manager Menu. 
   ");
-- 
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/8] MdeModulePkg UefiBootManagerLib: Add BmIsBootMenuAppFilePath internal API

[Liming Gao]

This function abstracts the common logic to find BootMenuApp file.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 49 +++-
 1 file changed, 30 insertions(+), 19 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index bb38f00..18259e9 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1530,6 +1530,34 @@ EfiBootManagerGetLoadOptionBuffer (
 }
 
 /**
+  Check if it's a Device Path pointing to BootMenuApp.
+
+  @param  DevicePath Input device path.
+
+  @retval TRUE   The device path is BootMenuApp File Device Path.
+  @retval FALSE  The device path is NOT BootMenuApp File Device Path.
+**/
+BOOLEAN
+BmIsBootMenuAppFilePath (
+  EFI_DEVICE_PATH_PROTOCOL *DevicePath
+)
+{
+  EFI_HANDLE  FvHandle;
+  VOID*NameGuid;
+  EFI_STATUS  Status;
+
+  Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid, 
&DevicePath, &FvHandle);
+  if (!EFI_ERROR (Status)) {
+NameGuid = EfiGetNameGuidFromFwVolDevicePathNode ((CONST 
MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *) DevicePath);
+if (NameGuid != NULL) {
+  return CompareGuid (NameGuid, PcdGetPtr (PcdBootManagerMenuFile));
+}
+  }
+
+  return FALSE;
+}
+
+/**
   Attempt to boot the EFI boot option. This routine sets L"BootCurent" and
   also signals the EFI ready to boot event. If the device path for the option
   starts with a BBS device path a legacy boot is attempted via the registered 
@@ -1562,9 +1590,7 @@ EfiBootManagerBoot (
   UINTN OptionNumber;
   UINTN OriginalOptionNumber;
   EFI_DEVICE_PATH_PROTOCOL  *FilePath;
-  EFI_DEVICE_PATH_PROTOCOL  *Node;
   EFI_DEVICE_PATH_PROTOCOL  *RamDiskDevicePath;
-  EFI_HANDLEFvHandle;
   VOID  *FileBuffer;
   UINTN FileSize;
   EFI_BOOT_LOGO_PROTOCOL*BootLogo;
@@ -1619,12 +1645,7 @@ EfiBootManagerBoot (
   // 3. Signal the EVT_SIGNAL_READY_TO_BOOT event when we are about to load 
and execute
   //the boot option.
   //
-  Node   = BootOption->FilePath;
-  Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid, &Node, 
&FvHandle);
-  if (!EFI_ERROR (Status) && CompareGuid (
-EfiGetNameGuidFromFwVolDevicePathNode ((CONST 
MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *) Node),
-PcdGetPtr (PcdBootManagerMenuFile)
-)) {
+  if (BmIsBootMenuAppFilePath (BootOption->FilePath)) {
 DEBUG ((EFI_D_INFO, "[Bds] Booting Boot Manager Menu.\n"));
 BmStopHotkeyService (NULL, NULL);
   } else {
@@ -2272,20 +2293,11 @@ EfiBootManagerGetBootManagerMenu (
   UINTNBootOptionCount;
   EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions;
   UINTNIndex;
-  EFI_DEVICE_PATH_PROTOCOL *Node;
-  EFI_HANDLE   FvHandle;
   
   BootOptions = EfiBootManagerGetLoadOptions (&BootOptionCount, 
LoadOptionTypeBoot);
 
   for (Index = 0; Index < BootOptionCount; Index++) {
-Node   = BootOptions[Index].FilePath;
-Status = gBS->LocateDevicePath (&gEfiFirmwareVolume2ProtocolGuid, &Node, 
&FvHandle);
-if (!EFI_ERROR (Status)) {
-  if (CompareGuid (
-EfiGetNameGuidFromFwVolDevicePathNode ((CONST 
MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *) Node),
-PcdGetPtr (PcdBootManagerMenuFile)
-)
-  ) {
+if (BmIsBootMenuAppFilePath (BootOptions[Index].FilePath)) {
 Status = EfiBootManagerInitializeLoadOption (
BootOption,
BootOptions[Index].OptionNumber,
@@ -2298,7 +2310,6 @@ EfiBootManagerGetBootManagerMenu (
);
 ASSERT_EFI_ERROR (Status);
 break;
-  }
 }
   }
 
-- 
2.8.0.windows.1

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

[edk2] [Patch 0/8] MdeModulePkg: Add new LoadFileOnFv2

[Liming Gao]

Update MdeModulePkg UefiBootManagerLib to support LoadFile protocol based on FV.
Add new LoadFileOnFv2 driver into MdeModulePkg.
Apply LoadFileOnFv2 driver in NT32 platform. 

Liming Gao (8):
  MdeModulePkg UefiBootManagerLib: Add BmIsBootMenuAppFilePath internal
API
  MdeModulePkg UefiBootManagerLib: Support LoadFile Protocol based on FV
  MdeModulePkg UefiBootManagerLib: Update LoadFile boot description
  MdeModulePkg UefiBootManagerLib: Enhance EfiBootManagerFindLoadOption
  MdeModulePkg LoadFileOnFv2: Add new LoadFileOnFv2 driver
  MdeModulePkg: Add new LoadFileOnFv2 in Package DSC for Build
  Nt32Pkg: Add LoadFileOnFv2 driver in DSC/FDF
  Nt32Pkg: Make Shell as the first boot option

 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 165 +---
 .../Library/UefiBootManagerLib/BmBootDescription.c |  47 +++
 .../Library/UefiBootManagerLib/BmLoadOption.c  |  44 ++-
 MdeModulePkg/MdeModulePkg.dsc  |   1 +
 .../Universal/LoadFileOnFv2/LoadFileOnFv2.c| 421 +
 .../Universal/LoadFileOnFv2/LoadFileOnFv2.inf  |  68 
 .../Universal/LoadFileOnFv2/LoadFileOnFv2.uni  |  24 ++
 .../Universal/LoadFileOnFv2/LoadFileOnFv2Extra.uni |  18 +
 .../PlatformBootManagerLib/PlatformBootManager.c   | 151 +++-
 Nt32Pkg/Nt32Pkg.dsc|   1 +
 Nt32Pkg/Nt32Pkg.fdf|   2 +-
 11 files changed, 776 insertions(+), 166 deletions(-)
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.c
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.uni
 create mode 100644 MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2Extra.uni

-- 
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 v3 04/40] UefiCpuPkg/MpInitLib: Add MP Initialize library class definition

[Laszlo Ersek]

On 07/25/16 04:52, Jeff Fan wrote:
> MP Initialize library provides basic functionalities to do APs initialization,
> to manage MP information and to wakeup APs to execute AP task.
> 
> It could be consumed by CPU MP PEI or DXE drivers to provide CPU MP 
> PPI/Protocol
> services.
> 
> v3:
>   1. Add whitespace after MpInitLibInitialize
>   2. Rename MpInitLibSwitchBsp to MpInitLibSwitchBSP to match PI spec
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/Include/Library/MpInitLib.h | 352 
> +
>  UefiCpuPkg/UefiCpuPkg.dec  |   4 +
>  2 files changed, 356 insertions(+)
>  create mode 100644 UefiCpuPkg/Include/Library/MpInitLib.h

(1) I guess "MpServicesLib" would be a better library class name, but
it's not a big issue.

If you decide to change the name, then a number of other locations have
to be changed as well (for example, the include guard).

> diff --git a/UefiCpuPkg/Include/Library/MpInitLib.h 
> b/UefiCpuPkg/Include/Library/MpInitLib.h
> new file mode 100644
> index 000..4ae02fb
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Library/MpInitLib.h
> @@ -0,0 +1,352 @@
> +/** @file
> +  Multiple-Processor initialization Library.
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#ifndef __MP_INIT_LIB_H__
> +#define __MP_INIT_LIB_H__
> +
> +#include 
> +
> +/**
> +  MP Initialize Library initialization.
> +
> +  This service will allocate AP reset vector and wakeup all APs to do APs
> +  initialization.
> +
> +  This service must be invoked before all other MP Initialize Library
> +  service are invoked.
> +
> +  @retval  EFI_SUCCESS   MP initialization succeeds.
> +  @retval  OthersMP initialization fails.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MpInitLibInitialize (
> +  VOID
> +  );
> +
> +/**
> +  Retrieves the number of logical processor in the platform and the number of
> +  those logical processors that are enabled on this boot. This service may 
> only
> +  be called from the BSP.
> +
> +  @param[out] NumberOfProcessors  Pointer to the total number of 
> logical
> +  processors in the system, 
> including the BSP
> +  and disabled APs.
> +  @param[out] NumberOfEnabledProcessors   Pointer to the number of enabled 
> logical
> +  processors that exist in system, 
> including
> +  the BSP.
> +
> +  @retval EFI_SUCCESS The number of logical processors and 
> enabled 
> +  logical processors was retrieved.
> +  @retval EFI_DEVICE_ERRORThe calling processor is an AP.
> +  @retval EFI_INVALID_PARAMETER   NumberOfProcessors is NULL and 
> NumberOfEnabledProcessors
> +  is NULL.

(2) "and" should be "or"

> +  @retval EFI_NOT_READY   MP Initialize Library is not initialized.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MpInitLibGetNumberOfProcessors (
> +  OUT UINTN *NumberOfProcessors,   OPTIONAL
> +  OUT UINTN *NumberOfEnabledProcessors OPTIONAL
> +  );
> +
> +/**
> +  Gets detailed MP-related information on the requested processor at the
> +  instant this call is made. This service may only be called from the BSP.
> +
> +  @param[in]  ProcessorNumber   The handle number of processor.
> +  @param[out] ProcessorInfoBuffer   A pointer to the buffer where 
> information for
> +the requested processor is deposited.
> +  @param[out]  HealthDataReturn processor health data.

(3) Unaligned indentation.

Also, some description for the health data would be nice. Is it the same
as HealthFlag a little lower down in this patch?

> +
> +  @retval EFI_SUCCESS Processor information was returned.
> +  @retval EFI_DEVICE_ERRORThe calling processor is an AP.
> +  @retval EFI_INVALID_PARAMETER   ProcessorInfoBuffer is NULL.
> +  @retval EFI_NOT_FOUND   The processor with the handle specified by
> +  ProcessorNumber does not exist in the 
> platform.
> +  @retval EFI_NOT_READY   MP Initialize Library is not initialized.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +MpInitLibGetProcessorInfo (
> +  IN  UINTN

Re: [edk2] [Patch v3 03/40] UefiCpuPkg/CpuS3DataDxe: Move StartupVector allocation to EndOfDxe()

[Laszlo Ersek]

On 07/25/16 04:52, Jeff Fan wrote:
> Currently, we will allocate StartupVector buffer under 1MB at entry point
> function. But some modules may allocate some hard code address under 1MB.
> For example, LegacyBiosDxe driver tries to manage some legacy range under
> 640KB.
> 
> To avoid the conflicts, we move StartupVector buffer allocation to End Of
> DXE event callback function.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c  | 39 
> ++--
>  UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf |  2 +-
>  2 files changed, 23 insertions(+), 18 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c 
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> index 9fb47dc..9e76cae 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c
> @@ -9,7 +9,7 @@ number of CPUs reported by the MP Services Protocol, so this 
> module does not
>  support hot plug CPUs.  This module can be copied into a CPU specific package
>  and customized if these additional features are required.
>  
> -Copyright (c) 2013 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.
>  Copyright (c) 2015, Red Hat, Inc.
>  
>  This program and the accompanying materials
> @@ -45,6 +45,8 @@ typedef struct {
>IA32_DESCRIPTOR   IdtrProfile;
>  } ACPI_CPU_DATA_EX;
>  
> +ACPI_CPU_DATA  *mAcpiCpuData;
> +
>  /**
>Allocate EfiACPIMemoryNVS below 4G memory address.
>  
> @@ -84,18 +86,31 @@ AllocateAcpiNvsMemoryBelow4G (
>  /**
>Callback function executed when the EndOfDxe event group is signaled.
>  
> -  We delay saving the MTRR settings until BDS signals EndOfDxe.
> +  We delay allocating StartupVector and saving the MTRR settings until BDS 
> signals EndOfDxe.
>  
>@param[in]  EventEvent whose notification function is being invoked.
>@param[out] Context  Pointer to the MTRR_SETTINGS buffer to fill in.
>  **/
>  VOID
>  EFIAPI
> -SaveMtrrsOnEndOfDxe (
> +CpuS3DataOnEndOfDxe (
>IN  EFI_EVENT  Event,
>OUT VOID   *Context
>)
>  {
> +  EFI_STATUS  Status;
> +  //
> +  // Allocate a 4KB reserved page below 1MB
> +  //
> +  mAcpiCpuData->StartupVector = BASE_1MB - 1;
> +  Status = gBS->AllocatePages (
> +  AllocateMaxAddress,
> +  EfiReservedMemoryType,
> +  1,
> +  &mAcpiCpuData->StartupVector
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +
>DEBUG ((EFI_D_VERBOSE, "%a\n", __FUNCTION__));
>MtrrGetAllMtrrs (Context);
>  
> @@ -162,18 +177,6 @@ CpuS3DataInitialize (
>ASSERT_EFI_ERROR (Status);
>  
>//
> -  // Allocate a 4KB reserved page below 1MB
> -  //
> -  AcpiCpuData->StartupVector = BASE_1MB - 1;
> -  Status = gBS->AllocatePages (
> -  AllocateMaxAddress,
> -  EfiReservedMemoryType,
> -  1,
> -  &AcpiCpuData->StartupVector
> -  );
> -  ASSERT_EFI_ERROR (Status);
> -
> -  //
>// Get the number of CPUs
>//
>Status = MpServices->GetNumberOfProcessors (
> @@ -255,17 +258,19 @@ CpuS3DataInitialize (
>  
>//
>// Register EFI_END_OF_DXE_EVENT_GROUP_GUID event.
> -  // The notification function saves MTRRs for ACPI_CPU_DATA
> +  // The notification function allocates StartupVector and saves MTRRs for 
> ACPI_CPU_DATA
>//
>Status = gBS->CreateEventEx (
>EVT_NOTIFY_SIGNAL,
>TPL_CALLBACK,
> -  SaveMtrrsOnEndOfDxe,
> +  CpuS3DataOnEndOfDxe,
>&AcpiCpuDataEx->MtrrTable,
>&gEfiEndOfDxeEventGroupGuid,
>&Event
>);
>ASSERT_EFI_ERROR (Status);
>  
> +  mAcpiCpuData = AcpiCpuData;
> +
>return EFI_SUCCESS;
>  }
> diff --git a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf 
> b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> index 857e12b..608e19f 100644
> --- a/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> +++ b/UefiCpuPkg/CpuS3DataDxe/CpuS3DataDxe.inf
> @@ -9,7 +9,7 @@
>  #  support hot plug CPUs.  This module can be copied into a CPU specific 
> package
>  #  and customized if these additional features are required.
>  #
> -#  Copyright (c) 2013-2015, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2013-2016, Intel Corporation. All rights reserved.
>  #  Copyright (c) 2015, Red Hat, Inc.
>  #
>  #  This program and the accompanying materials
> 

I think I agree with this patch, but with it applied, the way data is
carried from the entry point function to the end-of-dxe callback is
awkward. The Context parameter is used as a pointer to
AcpiCpuDataEx->MtrrTable, but then we set mAcpiCpuData to AcpiCpuData
too. That's redundant and confusing IMO.

In

Re: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode

[Gao, Liming]

Ard:
After apply these patches, I try to build OvmfPkg with SECURE enable on GCC5 in 
LTO mode. It reports build failure. Have you tried such build before?

Build command: build -p OvmfPkg/OvmfPkgIa32X64.dsc -t GCC5 
-DSECURE_BOOT_ENABLE=TRUE -a IA32 -a X64
Build error message, seemly OpenSsl can't be linked with LTO.

"gcc" -o 
/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll
 -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x40 
-Wl,--entry,_ModuleEntryPoint -u _ModuleEntryPoint 
-Wl,-Map,/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.map
 -Wl,-melf_x86_64,--oformat=elf64-x86-64 
-Wl,--start-group,@/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/OUTPUT/static_library_files.lst,--end-group
 -Wl,--defsym=PECOFF_HEADER_SIZE=0x228 
-Wl,--script=/home/hwu/work/lgao4/AllPkg/edk2/BaseTools/Scripts/GccBase.lds
/tmp/cc9uBgrd.ltrans0.ltrans.o: In function `DxeImageVerificationHandler':
:(.text+0x1f13): undefined reference to `malloc'
:(.text+0x1f7a): undefined reference to `free'
:(.text+0x1f92): undefined reference to `malloc'
:(.text+0x1fb9): undefined reference to `free'
:(.text+0x1fe3): undefined reference to `free'
:(.text+0x2027): undefined reference to `malloc'
:(.text+0x20bf): undefined reference to `free'
:(.text+0x2116): undefined reference to `free'
:(.text+0x212d): undefined reference to `free'
:(.text+0x213a): undefined reference to `free'
:(.text+0x21f7): undefined reference to `free'
/tmp/cc9uBgrd.ltrans0.ltrans.o::(.text+0x2204): more undefined references to 
`free' follow
/tmp/cc9uBgrd.ltrans8.ltrans.o: In function `default_realloc_ex.lto_priv.190':
:(.text+0x1): undefined reference to `realloc'
/tmp/cc9uBgrd.ltrans8.ltrans.o: In function `default_malloc_ex.lto_priv.187':
:(.text+0x6): undefined reference to `malloc'
/tmp/cc9uBgrd.ltrans8.ltrans.o: In function `CRYPTO_free':
:(.text+0x613): undefined reference to `free'
/tmp/cc9uBgrd.ltrans8.ltrans.o: In function `WrapPkcs7Data':
:(.text.unlikely+0x72): undefined reference to `malloc'
/tmp/cc9uBgrd.ltrans17.ltrans.o: In function `OPENSSL_gmtime':
:(.text+0x176a): undefined reference to `malloc'
/tmp/cc9uBgrd.ltrans18.ltrans.o: In function `RSA_free':
:(.text+0x123a): undefined reference to `free'
/tmp/cc9uBgrd.ltrans20.ltrans.o: In function `qsort':
:(.text+0x1368): undefined reference to `malloc'
:(.text+0x13bc): undefined reference to `malloc'
:(.text+0x13b4): undefined reference to `free'
/tmp/cc9uBgrd.ltrans27.ltrans.o: In function 
`CRYPTO_realloc_clean.constprop.153':
:(.text+0x20e): undefined reference to `free'
/tmp/cc9uBgrd.ltrans27.ltrans.o:(.data.rel.ro.local+0x578): undefined reference 
to `free'
/usr/bin/ld: 
/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll:
 protected symbol `realloc' isn't defined
/usr/bin/ld: final link failed: Bad value
collect2: error: ld returned 1 exit status
make: *** 
[/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe/DEBUG/SecurityStubDxe.dll]
 Error 1


build.py...
: error 7000: Failed to execute command
make tbuild 
[/home/hwu/work/lgao4/AllPkg/Build/Ovmf3264/DEBUG_GCC5/X64/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe]


build.py...
: error F002: Failed to build module
/home/hwu/work/lgao4/AllPkg/edk2/MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 [X64, GCC5, DEBUG]

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Saturday, July 23, 2016 5:03 PM
> To: edk2-devel@lists.01.org; ler...@redhat.com; Gao, Liming
> ; Shi, Steven ; Zhu,
> Yonghong ; Justen, Jordan L
>
> Cc: leif.lindh...@linaro.org; Ard Biesheuvel
> Subject: [edk2] [PATCH v3 0/6] BaseTools: add support for GCC5 in LTO mode
>
> This v3 to introduce GCC5 is now a 6 piece series, including some
> preparatory cleanup patches that allow all GCC4x and CLANG35 toolchains
> to switch to using 'gcc' as the linker. This allows us to get rid of
> the wrapper script to marshall ld arguments in order to make them
> understandable by gcc, which is fragile and likely to cause problems in
> the future.
>
> Since there appears to be a natural split between the 'legacy' GCC
> toolchains UNIXGCC, ELFGCC, and CYGGCC[xASL], both in term of supported
> architectures [IA32, X64, IPF] vs [IA32, X64, ARM, AARCH64], and in
> terms of maintenance, these toolchains are not moved to using 'gcc' as
> the linker, and instead, a new BUILDRULEFAMILY is introduced called GCCLD
> that will retain the old behavior.
>
> The result is that GCC5 can align much more closely with its predecessors,
> making the expected maintenance burden of supporting GCC back to v4.4
> much lowe

Re: [edk2] [Patch v3 02/40] UefiCpuPkg/MpInitLib: Add microcode definitions defined in IA32 SDM

[Laszlo Ersek]

On 07/25/16 04:52, Jeff Fan wrote:
> Add microcode definitions defined in Intel(R) 64 and IA-32 Architectures
> Software Developer's Manual Volume 3A, Section 9.11.
> 
> v3:
>   1. Update SDM date to June, 2016
>   2. Mention BCD format in CPU_MICROCODE_DATE
>   3. Rename ProcessorChecksum to Checksum to match SDM.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  UefiCpuPkg/Include/Register/Microcode.h | 196 
> 
>  1 file changed, 196 insertions(+)
>  create mode 100644 UefiCpuPkg/Include/Register/Microcode.h
> 
> diff --git a/UefiCpuPkg/Include/Register/Microcode.h 
> b/UefiCpuPkg/Include/Register/Microcode.h
> new file mode 100644
> index 000..1f6cd47
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Register/Microcode.h
> @@ -0,0 +1,196 @@
> +/** @file
> +  Microcode Definitions.
> +
> +  Microcode Definitions based on contents of the
> +  Intel(R) 64 and IA-32 Architectures Software Developer's Manual
> +Volume 3A, Section 9.11  Microcode Definitions
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +  @par Specification Reference:
> +  Intel(R) 64 and IA-32 Architectures Software Developer's Manual, Volume 3A,
> +  June 2016, Chapter 9 Processor Management and Initialization, Section 9-11.
> +
> +**/
> +
> +#ifndef __MICROCODE_H__
> +#define __MICROCODE_H__
> +
> +///
> +/// CPU Microcode Date in BCD format
> +///
> +typedef union {
> +  struct {
> +UINT32   Year:16;
> +UINT32   Day:8;
> +UINT32   Month:8;
> +  } Bits;
> +  UINT32 Uint32;
> +} CPU_MICROCODE_DATE;
> +
> +///
> +/// CPU Microcode Processor Signature format
> +///
> +typedef union {
> +  struct {
> +UINT32   Stepping:4;
> +UINT32   Model:4;
> +UINT32   Family:4;
> +UINT32   Type:2;
> +UINT32   Reserved1:2;
> +UINT32   ExtendedModel:4;
> +UINT32   ExtendedFamily:8;
> +UINT32   Reserved2:4;
> +  } Bits;
> +  UINT32 Uint32;
> +} CPU_MICROCODE_PROCESSOR_SIGNATURE;
> +
> +///
> +/// Microcode Update Format definition
> +///
> +typedef struct {
> +  ///
> +  /// Version number of the update header
> +  ///
> +  UINT32HeaderVersion;
> +  ///
> +  /// Unique version number for the update, the basis for the update
> +  /// signature provided by the processor to indicate the current update
> +  /// functioning within the processor. Used by the BIOS to authenticate
> +  /// the update and verify that the processor loads successfully. The
> +  /// value in this field cannot be used for processor stepping 
> identification
> +  /// alone. This is a signed 32-bit number.
> +  ///
> +  UINT32UpdateRevision;
> +  ///
> +  /// Date of the update creation in binary format: mmdd (e.g.
> +  /// 07/18/98 is 07181998H).
> +  ///
> +  CPU_MICROCODE_DATEDate;
> +  ///
> +  /// Extended family, extended model, type, family, model, and stepping
> +  /// of processor that requires this particular update revision (e.g.,
> +  /// 0650H). Each microcode update is designed specifically for a
> +  /// given extended family, extended model, type, family, model, and
> +  /// stepping of the processor.
> +  /// The BIOS uses the processor signature field in conjunction with the
> +  /// CPUID instruction to determine whether or not an update is
> +  /// appropriate to load on a processor. The information encoded within
> +  /// this field exactly corresponds to the bit representations returned by
> +  /// the CPUID instruction.
> +  ///
> +  CPU_MICROCODE_PROCESSOR_SIGNATURE ProcessorSignature;
> +  ///
> +  /// Checksum of Update Data and Header. Used to verify the integrity of
> +  /// the update header and data. Checksum is correct when the
> +  /// summation of all the DWORDs (including the extended Processor
> +  /// Signature Table) that comprise the microcode update result in
> +  /// H.
> +  ///
> +  UINT32Checksum;
> +  ///
> +  /// Version number of the loader program needed to correctly load this
> +  /// update. The initial version is 0001H
> +  ///
> +  UINT32LoaderRevision;
> +  ///
> +  /// Platform type information is encoded in the lower 8 bits of this 4-
> +  /// byte field. Each bit represents a particular platform type for a given
> +  /// CPUID. The BIOS uses the processor flags field in conjunction with
> +  /// the platform Id bit

Re: [edk2] [Patch v3 01/40] UefiCpuPkg/LocalApic.h: Remove duplicated/conflicted definitions

[Laszlo Ersek]

On 07/25/16 04:52, Jeff Fan wrote:
> #define MSR_IA32_APIC_BASE_ADDRESS is duplicated with #define 
> MSR_IA32_APIC_BASE
> defined in UefiCpuPkg/Include/Register/ArchitecturalMsr.h, so we could remove 
> it
> and update the modules to use MSR_IA32_APIC_BASE from ArchitecturalMsr.h.
> 
> Structure MSR_IA32_APIC_BASE conflicts with #define MSR_IA32_APIC_BASE 
> defined in
> UefiCpuPkg/Include/Register/ArchitecturalMsr.h, so we could remove it and 
> update
> the modules to use structure MSR_IA32_APIC_BASE_REGISTER from 
> ArchitecturalMsr.h.
> 
> Cc: Michael Kinney 
> Cc: Feng Tian 
> Cc: Giri P Mudusuru 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Reviewed-by: Giri P Mudusuru 
> ---
>  UefiCpuPkg/CpuMpPei/CpuMpPei.h |  1 +
>  UefiCpuPkg/CpuMpPei/PeiMpServices.c| 20 -
>  UefiCpuPkg/Include/Register/LocalApic.h| 20 +
>  UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 29 ++--
>  .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c| 51 
> +++---
>  5 files changed, 53 insertions(+), 68 deletions(-)

Reviewed-by: Laszlo Ersek 

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

Re: [edk2] OVMF and passed usb 3. controller

[Laszlo Ersek]

On 07/22/16 15:25, Eugene Chekanskiy wrote:
> Hello everyone. Just wondering if default builds of ovmf form
> http://www.tianocore.org/ovmf/ has support of renesas usb 3.0 controller
> and if we can enable it in custom build.

First, please don't use the binary from .
It is incredibly old (Feb 9 2014). Instead, I recommend
 for anything "bleeding edge".

(Unless you are willing to build your own OVMF, of course, in which case
I recommend that instead).

Second, OVMF includes the "MdeModulePkg/Bus/Pci/XhciDxe" driver. I
assume you assign your Renesas host controller to the virtual machine;
is that right? If your host controller is supported by XhciDxe otherwise
(IOW it would work in a purely physical setup), then I think it should
work with device assignment as well.

CC: Gerd for owning kraxel.org and for USB knowledge
CC: Feng for maintaining XhciDxe
CC: Alex for any possible quirks with USB host controller assignment

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

Re: [edk2] [PATCH] OvmfPkg/Sec: Handle bigger FV sizes

[Laszlo Ersek]

On 07/22/16 17:47, Thomas Palmer wrote:
> Ovmf's SecMain needs to handle the EFI_COMMON_SECTION_HEADER2 header
> so that larger images can be created. Use IS_SECTION2 and
> SECTION2_SIZE macros to calculate accurate image sizes when
> appropriate.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Thomas Palmer 
> ---
>  OvmfPkg/Sec/SecMain.c | 25 ++---
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index a12e676..464de10 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -2,6 +2,7 @@
>Main SEC phase code.  Transitions to PEI.
>  
>Copyright (c) 2008 - 2015, Intel Corporation. All rights reserved.
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
>  
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD 
> License
> @@ -332,11 +333,13 @@ DecompressMemFvs (
>UINT32AuthenticationStatus;
>VOID  *OutputBuffer;
>VOID  *ScratchBuffer;
> -  EFI_FIRMWARE_VOLUME_IMAGE_SECTION *FvSection;
> +  EFI_COMMON_SECTION_HEADER *FvSection;
>EFI_FIRMWARE_VOLUME_HEADER*PeiMemFv;
>EFI_FIRMWARE_VOLUME_HEADER*DxeMemFv;
> +  UINT32FvHeaderSize;
> +  UINT32FvSectionSize;
>  
> -  FvSection = (EFI_FIRMWARE_VOLUME_IMAGE_SECTION*) NULL;
> +  FvSection = (EFI_COMMON_SECTION_HEADER*) NULL;
>  
>Status = FindFfsFileAndSection (
>   *Fv,
> @@ -386,7 +389,7 @@ DecompressMemFvs (
>   OutputBufferSize,
>   EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>   0,
> - (EFI_COMMON_SECTION_HEADER**) &FvSection
> + &FvSection
>   );
>if (EFI_ERROR (Status)) {
>  DEBUG ((EFI_D_ERROR, "Unable to find PEI FV section\n"));
> @@ -411,7 +414,7 @@ DecompressMemFvs (
>   OutputBufferSize,
>   EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
>   1,
> - (EFI_COMMON_SECTION_HEADER**) &FvSection
> + &FvSection
>   );
>if (EFI_ERROR (Status)) {
>  DEBUG ((EFI_D_ERROR, "Unable to find DXE FV section\n"));
> @@ -419,11 +422,19 @@ DecompressMemFvs (
>}
>  
>ASSERT (FvSection->Type == EFI_SECTION_FIRMWARE_VOLUME_IMAGE);
> -  ASSERT (SECTION_SIZE (FvSection) ==
> -  (PcdGet32 (PcdOvmfDxeMemFvSize) + sizeof (*FvSection)));
> +
> +  if (IS_SECTION2 (FvSection)) {
> +FvSectionSize = SECTION2_SIZE (FvSection);
> +FvHeaderSize = sizeof (EFI_COMMON_SECTION_HEADER2);
> +  } else {
> +FvSectionSize = SECTION_SIZE (FvSection);
> +FvHeaderSize = sizeof (EFI_COMMON_SECTION_HEADER);
> +  }
> +
> +  ASSERT (FvSectionSize == (PcdGet32 (PcdOvmfDxeMemFvSize) + FvHeaderSize));
>  
>DxeMemFv = (EFI_FIRMWARE_VOLUME_HEADER*)(UINTN) PcdGet32 
> (PcdOvmfDxeMemFvBase);
> -  CopyMem (DxeMemFv, (VOID*) (FvSection + 1), PcdGet32 
> (PcdOvmfDxeMemFvSize));
> +  CopyMem (DxeMemFv, (VOID*) ((UINTN)FvSection + FvHeaderSize), PcdGet32 
> (PcdOvmfDxeMemFvSize));
>  
>if (DxeMemFv->Signature != EFI_FVH_SIGNATURE) {
>  DEBUG ((EFI_D_ERROR, "Extracted FV at %p does not have FV header 
> signature\n", DxeMemFv));
> 

(1) please split this patch in two: The first patch should only do the
EFI_FIRMWARE_VOLUME_IMAGE_SECTION --> EFI_COMMON_SECTION_HEADER
replacement (which is no change in behavior, as the former is a typedef
to the latter), drop the superfluous casts, etc. The second patch should
add the section2 handling.

(2) EFI_COMMON_SECTION_HEADER.Size can describe a section that is almost
16MB in size. In OvmfPkg our DXEFV is currently 10MB in size (from
commit 2f7b34b20842f).

I think you must be building a number of proprietary (or otherwise
downstream-only) modules into OVMF for not fitting into 16MB. That's
okay, but the use case must be clearly described in the commit message
of patch 2, so that looking at the code later, we understand why it's
there, without any use case for it within the open source edk2 tree.

So, in the commit message of patch 2, please mention
- that EFI_COMMON_SECTION_HEADER2.ExtendedSize is only necessary when
the DXEFV size exceeds approximately 16MB, which is currently a
downstream-only situation,
- and that the build tools automatically generate
EFI_COMMON_SECTION_HEADER2 when necessary.

(3) The subject line of patch #2 should name DXEFV rather than just "FV".

Looks good to me otherwise.

Thanks!
Laszlo

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

Re: [edk2] [PATCH v3 2/6] ArmVirtPkg/ArmVirtPrePiUniCoreRelocatable: ignore .hash and .note sections

[Laszlo Ersek]

On 07/23/16 11:03, Ard Biesheuvel wrote:
> Newer versions of ld automatically emit .gnu.hash and .note.gnu.build-id
> sections, which are not listed in the linker script, and will end up
> breaking the build with an allocation conflict, e.g.,
> 
>   /usr/bin/aarch64-linux-gnu-ld: section .note.gnu.build-id loaded at
> [,0023] overlaps section .text loaded at
> [,00017dbf]
> 
> Since we don't require or care about these sections, update the linker
> script so that they are discarded. Note that this involves emitting the
> .note.gnu.build-id section into a non-allocatable segment to prevent the
> linker from noticing that it is being discarded (and subsequently
> complaining about it)
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/PrePi/Scripts/PrePi-PIE.lds | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/ArmVirtPkg/PrePi/Scripts/PrePi-PIE.lds 
> b/ArmVirtPkg/PrePi/Scripts/PrePi-PIE.lds
> index 832ad1474468..44df7840adfd 100644
> --- a/ArmVirtPkg/PrePi/Scripts/PrePi-PIE.lds
> +++ b/ArmVirtPkg/PrePi/Scripts/PrePi-PIE.lds
> @@ -30,8 +30,11 @@ SECTIONS
>  PROVIDE(__reloc_end = .);
>}
>  
> +  .note (INFO) : { *(.note.gnu.build-id) }
> +
>/DISCARD/ : {
>  *(.note.GNU-stack)
> +*(.gnu.hash)
>  *(.gnu_debuglink)
>  *(.interp)
>  *(.dynamic)
> 

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