[edk2] EFI hanging at EfiBootManagerRefreshAllBootOption

[Saqib Khan]

Hi ,
I am working on a Boot manager
I have following set-up
in USB i have directory  EFI->BOOT->Bootx64.efi(my EFI)


My EFI does load an image, before that I am trying to
EfiBootManagerRefreshAllBootOption () I am trying to refresh all but option
.but it hangs on  EfiBootManagerRefreshAllBootOption () .

I have shared code  I have bold and underlined where it is hanging, any
idea what might be the reason?


Here is Code :

EFI_STATUS
EFIAPI
BootManagerMenuEntry (
  IN EFI_HANDLEImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{

EFI_BOOT_MANAGER_LOAD_OPTION*BootOption;
UINTN   BootOptionCount;
EFI_LEGACY_BIOS_PROTOCOL  *LegacyBios;
EFI_BOOT_LOGO_PROTOCOL  *BootLogo;
EFI_STATUSStatus;
UINTN BootVariableSize;

  CHAR16*Name;
  UINTN NewNameSize;
  UINTN NameSize;
 UINTN i;

EFI_GUID VarGuid;
VOID* Value;
UINTN Index;

EFI_DEVICE_PATH_PROTOCOL *DevicePathProtocol;

EFI_LOADED_IMAGE_PROTOCOL* LoadedImageProtocol;



  BootLogo = NULL;
  Status = gBS->LocateProtocol (&gEfiBootLogoProtocolGuid, NULL, (VOID **)
&BootLogo);
  if (!EFI_ERROR (Status) && (BootLogo != NULL)) {
Status = BootLogo->SetBootLogo (BootLogo, NULL, 0, 0, 0, 0);
ASSERT_EFI_ERROR (Status);
  }

  gBS->SetWatchdogTimer (0x, 0x, 0x, NULL);

  gStringPackHandle = HiiAddPackages (
 &gEfiCallerIdGuid,
 gImageHandle,
 BootManagerMenuAppStrings,
 NULL
 );
  ASSERT (gStringPackHandle != NULL);



Print(L"ConnectBootManager\n");
EfiBootManagerConnectAll ();
Print(L"Connected BootManager\n");

*->EfiBootManagerRefreshAllBootOption ();   *//Here it gets hang
  Print(L"All option refreshed\n");

  BootOption = EfiBootManagerGetLoadOptions (&BootOptionCount,
LoadOptionTypeBoot);
  Print(L"Alloption refreshed fdfdf\n");
  //legacybootoptionrefresh();
  //LegacyBootManager(BootOption);

//EfiBootManagerRegisterLegacyBootSupport(legacybootoptionrefresh,LegacyBootManager



Status = gBS->LocateProtocol (&gEfiLegacyBiosProtocolGuid, NULL, (VOID
**) &LegacyBios);
  if (EFI_ERROR (Status)) {
//
// If no LegacyBios protocol we do not support legacy boot
//
Print(L"Boot not suported [%X]\n",Status);
//BootOption->Status = EFI_UNSUPPORTED;
   // return;
  }








//UINTN ExitDataSizePtr;
//UINTN HandleCount;
//UINTN ScratchBufferSize = 0;
//EFI_HANDLE *HandleBuffer;

//EFI_BOOT_LOGO_PROTOCOL  *BootLogo;

Status=gBS->HandleProtocol(
ImageHandle,
&gEfiLoadedImageProtocolGuid,
(VOID**)&LoadedImageProtocol
);


Status=gBS->HandleProtocol(
LoadedImageProtocol->DeviceHandle,
&gEfiDevicePathProtocolGuid,
(VOID**)&DevicePathProtocol
);
Print (L"Image device : %s\n",ConvertDevicePathToText
(DevicePathProtocol,TRUE,TRUE));
Print (L"Image file: %s\n",ConvertDevicePathToText
(LoadedImageProtocol->FilePath,TRUE,TRUE));
Print (L"Image Base: %X\n",LoadedImageProtocol->ImageBase);
Print (L"Image Size: %X\n",LoadedImageProtocol->ImageSize);

Value =NULL ;
/*Print All Boot option*/

 //Print all BOOT Load Options
  NameSize = sizeof(CHAR16);
  Name = AllocateZeroPool(NameSize);
  for (i=0; ;i++ ){
  NewNameSize = NameSize;
  //search all EFI variables
  Status = gRT->GetNextVariableName (&NewNameSize, Name, &VarGuid);
if (Status == EFI_BUFFER_TOO_SMALL) {
  Name = ReallocatePool (NameSize, NewNameSize, Name);
  Status = gRT->GetNextVariableName (&NewNameSize, Name, &VarGuid);
  NameSize = NewNameSize;
}
//
if (Status == EFI_NOT_FOUND) {
  break;
}
//skip if not Global variable
if (!CompareGuid(&VarGuid, &gEfiGlobalVariableGuid))
continue;
//check BOOT variable
if(!StrnCmp(Name, L"Boot", 4) &&
isitDigit(Name[4]) && isitDigit(Name[5]) &&
isitDigit(Name[6]) && isitDigit(Name[7]))
{
Print(L"%s:", Name);
//get BOOT
   // Status = gRT->GetVariable((CHAR16 *)Name,
&gEfiGlobalVariableGuid, &BootVariableSize, NULL);
Status = gRT->GetVariable ((CHAR16 *)Name, &gEfiGlobalVariableGuid,
NULL, &BootVariableSize, Value);
//print attribute

//print EFI_LOAD_OPTION description
Print(L" %s",(CHAR16 *)(Name+3));
Print(L"\n");

}
  }


LoadImageFromFile (L"\\Hello.efi");

SystemTable->ConOut->OutputString (
SystemTable->ConOut,
L"\n\r\n\r\n\rHit any key to exit\n\r");
SystemTable->BootServices->WaitForEvent (
1,
&(SystemTable->ConIn->WaitForKey),
&Index);




  return Status;

}


-- 
Regards
Saqib Ahmed Khanzada
__

Re: [edk2] EFI hanging at EfiBootManagerRefreshAllBootOption

[Saqib Khan]

[Packages]
  MdePkg/MdePkg.dec
  MdeModulePkg/MdeModulePkg.dec
  IntelFrameworkPkg/IntelFrameworkPkg.dec
  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
  ShellPkg/ShellPkg.dec

[LibraryClasses]
  HiiLib
  DebugLib
  UefiLib
  MemoryAllocationLib
  UefiBootServicesTableLib
  UefiApplicationEntryPoint
  UefiBootManagerLib
  LegacyBootManagerLib


I have figured out that if I add LegacyBootManagerLib it hangs at
EfiBootManagerRefreshAllBootOption() . any idea why this behaviour?

I want to boot legacy so i need legacyBootManagerLib...

On Wed, Oct 5, 2016 at 4:35 PM, Saqib Khan  wrote:

> Hi ,
> I am working on a Boot manager
> I have following set-up
> in USB i have directory  EFI->BOOT->Bootx64.efi(my EFI)
>
>
> My EFI does load an image, before that I am trying to
> EfiBootManagerRefreshAllBootOption () I am trying to refresh all but
> option .but it hangs on  EfiBootManagerRefreshAllBootOption () .
>
> I have shared code  I have bold and underlined where it is hanging, any
> idea what might be the reason?
>
>
> Here is Code :
>
> EFI_STATUS
> EFIAPI
> BootManagerMenuEntry (
>   IN EFI_HANDLEImageHandle,
>   IN EFI_SYSTEM_TABLE  *SystemTable
>   )
> {
>
> EFI_BOOT_MANAGER_LOAD_OPTION*BootOption;
> UINTN   BootOptionCount;
> EFI_LEGACY_BIOS_PROTOCOL  *LegacyBios;
> EFI_BOOT_LOGO_PROTOCOL  *BootLogo;
> EFI_STATUSStatus;
> UINTN BootVariableSize;
>
>   CHAR16*Name;
>   UINTN NewNameSize;
>   UINTN NameSize;
>  UINTN i;
>
> EFI_GUID VarGuid;
> VOID* Value;
> UINTN Index;
>
> EFI_DEVICE_PATH_PROTOCOL *DevicePathProtocol;
>
> EFI_LOADED_IMAGE_PROTOCOL* LoadedImageProtocol;
>
>
>
>   BootLogo = NULL;
>   Status = gBS->LocateProtocol (&gEfiBootLogoProtocolGuid, NULL, (VOID **)
> &BootLogo);
>   if (!EFI_ERROR (Status) && (BootLogo != NULL)) {
> Status = BootLogo->SetBootLogo (BootLogo, NULL, 0, 0, 0, 0);
> ASSERT_EFI_ERROR (Status);
>   }
>
>   gBS->SetWatchdogTimer (0x, 0x, 0x, NULL);
>
>   gStringPackHandle = HiiAddPackages (
>  &gEfiCallerIdGuid,
>  gImageHandle,
>  BootManagerMenuAppStrings,
>  NULL
>  );
>   ASSERT (gStringPackHandle != NULL);
>
>
>
> Print(L"ConnectBootManager\n");
> EfiBootManagerConnectAll ();
> Print(L"Connected BootManager\n");
>
> *->EfiBootManagerRefreshAllBootOption ();   *//Here it gets hang
>   Print(L"All option refreshed\n");
>
>   BootOption = EfiBootManagerGetLoadOptions (&BootOptionCount,
> LoadOptionTypeBoot);
>   Print(L"Alloption refreshed fdfdf\n");
>   //legacybootoptionrefresh();
>   //LegacyBootManager(BootOption);
>   //EfiBootManagerRegisterLegacyBootSupport(legacybootoptionrefresh,
> LegacyBootManager
>
>
>
> Status = gBS->LocateProtocol (&gEfiLegacyBiosProtocolGuid, NULL, (VOID
> **) &LegacyBios);
>   if (EFI_ERROR (Status)) {
> //
> // If no LegacyBios protocol we do not support legacy boot
> //
> Print(L"Boot not suported [%X]\n",Status);
> //BootOption->Status = EFI_UNSUPPORTED;
>// return;
>   }
>
>
>
>
>
>
>
>
> //UINTN ExitDataSizePtr;
> //UINTN HandleCount;
> //UINTN ScratchBufferSize = 0;
> //EFI_HANDLE *HandleBuffer;
>
> //EFI_BOOT_LOGO_PROTOCOL  *BootLogo;
>
> Status=gBS->HandleProtocol(
> ImageHandle,
> &gEfiLoadedImageProtocolGuid,
> (VOID**)&LoadedImageProtocol
> );
>
>
> Status=gBS->HandleProtocol(
> LoadedImageProtocol->DeviceHandle,
> &gEfiDevicePathProtocolGuid,
> (VOID**)&DevicePathProtocol
> );
> Print (L"Image device : %s\n",ConvertDevicePathToText
> (DevicePathProtocol,TRUE,TRUE));
> Print (L"Image file: %s\n",ConvertDevicePathToText
> (LoadedImageProtocol->FilePath,TRUE,TRUE));
> Print (L"Image Base: %X\n",LoadedImageProtocol->ImageBase);
> Print (L"Image Size: %X\n",LoadedImageProtocol->ImageSize);
>
> Value =NULL ;
> /*Print All Boot option*/
>
>  //Print all BOOT Load Options
>   NameSize = sizeof(CHAR16);
>   Name = AllocateZeroPool(NameSize);
>   for (i=0; ;i++ ){
>   NewNameSize = NameSize;
>   //search all EFI variables
>   Status = gRT->GetNextVariableName (&NewNameSize, Name, &VarGuid);
> if (Status == EFI_BUFFER_TOO_SMALL) {
>   Name = ReallocatePool (NameSize, NewNameSize, Name);
>   Status = gRT->GetNextVariableName (&NewNameSize, Name, &VarGuid);
>   NameSize = NewNameSize;
> }
> //
> if (Status == EFI_NOT_FOUND) {
>   break;
> }
> //skip if not Global variable
> if (!CompareGuid(&VarGuid, &gEfiGlobalVariableGuid))
> continue;
> //check BOOT variable
> if(!StrnCmp(Name, L"Boot", 4) &&
> isitDigit(N

[edk2] UefiBootManagerLib and LegacyBootManagerLib issue

[Saqib Khan]

Hi,

when i import both lib in my project my EFI hangs at
EfiRefreshAllBootOptions, i removed LegacyBootManager and it worked fine .i
need both lib as i need to boot legacy from EFI, how this issue can be
resolved?


here is piece of inf file

[Packages]
  MdePkg/MdePkg.dec
  MdeModulePkg/MdeModulePkg.dec
  IntelFrameworkPkg/
IntelFrameworkPkg.dec
  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
  ShellPkg/ShellPkg.dec

[LibraryClasses]
  HiiLib
  DebugLib
  UefiLib
  MemoryAllocationLib
  UefiBootServicesTableLib
  UefiApplicationEntryPoint
  UefiBootManagerLib
  LegacyBootManagerLib


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

Re: [edk2] [RFC V2] EDK2 Platform Proposal

[Richardson, Brian]

This link is correct ...
https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015 

If you cannot get the link to work, go to this link first ...
https://github.com/tianocore/edk2-platforms 
... then click on the 'Branch' button and select 'minnowboard-max-udk2015' from 
the dropdown list.

Thanks ... br
---
Brian Richardson, Senior Technical Marketing Engineer, Intel Software
brian.richard...@intel.com -- http://evangelists.intel.com/bio/Brian_Richardson_

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Bhupesh 
Sharma
Sent: Wednesday, October 5, 2016 2:11 AM
To: Kinney, Michael D ; edk2-devel@lists.01.org
Subject: Re: [edk2] [RFC V2] EDK2 Platform Proposal

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael D
> Sent: Friday, September 23, 2016 2:25 AM

> Hello,
> 
> 
> 
> Here is the V2 version of the proposal for the edk2-platforms repo.
> 
> 
> 
> Changes from V1:
> 
> 
> 
> * edk2-platform is not a fork of edk2.
> 
> * edk2-platforms branches contain CPU, Chipset, SoC, and platform 
> specific
> 
>   packages
> 
> * edk2-plaforms/master contains all open platforms that are synced 
> with
> 
>   edk2/master.
> 
> * Each edk2-platforms branch may support many platforms (not just one)
> 
> * Use PACKAGES_PATH to do builds using packages from multiple 
> repositories
> 
> * Update edk2-platforms branch naming to clearly identify platforms 
> that
> 
>   are considered stable and platforms that are under active 
> development.
> 
> * edk2 developers may be required to verify platforms in 
> edk2-platforms
> 
>   builds as part of test criteria.  Especially platforms that are 
> intended
> 
>   to be used with edk2/master in edk2-platforms/stable-* branches.
> 
> 
> 
> =
> 
> 
> 
> Similar to edk2-staging, we also have a need to manage platforms
> 
> that have been ported to edk2.  Jordan has created a repository
> 
> called edk2-platforms and has created a branch for the
> 
> minnowboard-max that uses a validated release of the UDK 2015 for
> 
> the dependent packages:
> 
> 
> 
> https://github.com/tianocore/edk2-platforms
> 
> 
> 
> https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-
> udk2015

[Bhupesh] This link seems broken. I get a 404 error while trying to acess this 
branch. Does it require some special privileges.

Regards,
Bhupesh

[snip..]
___
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] [RFC V2] EDK2 Platform Proposal

[Sakar Arora]

Hi,

A few questions here.

What is the proposed directory structure for the edk2-patforms branches? 
Will it be the same as in 
https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015, which 
is just the same as edk2?
Or, will it be the similar to platform specific part of the directory structure 
proposed in https://github.com/mdkinney/edk2/tree/NewDirStruct_V3 ?
Or, similar to https://git.linaro.org/uefi/OpenPlatformPkg.git/tree?
Or, left to platform developers' discretion?

Thanks,
Sakar
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Bhupesh 
Sharma
Sent: Wednesday, October 05, 2016 11:41 AM
To: Kinney, Michael D ; edk2-devel@lists.01.org
Subject: Re: [edk2] [RFC V2] EDK2 Platform Proposal

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Kinney, Michael D
> Sent: Friday, September 23, 2016 2:25 AM

> Hello,
> 
> 
> 
> Here is the V2 version of the proposal for the edk2-platforms repo.
> 
> 
> 
> Changes from V1:
> 
> 
> 
> * edk2-platform is not a fork of edk2.
> 
> * edk2-platforms branches contain CPU, Chipset, SoC, and platform 
> specific
> 
>   packages
> 
> * edk2-plaforms/master contains all open platforms that are synced 
> with
> 
>   edk2/master.
> 
> * Each edk2-platforms branch may support many platforms (not just one)
> 
> * Use PACKAGES_PATH to do builds using packages from multiple 
> repositories
> 
> * Update edk2-platforms branch naming to clearly identify platforms 
> that
> 
>   are considered stable and platforms that are under active 
> development.
> 
> * edk2 developers may be required to verify platforms in 
> edk2-platforms
> 
>   builds as part of test criteria.  Especially platforms that are 
> intended
> 
>   to be used with edk2/master in edk2-platforms/stable-* branches.
> 
> 
> 
> =
> 
> 
> 
> Similar to edk2-staging, we also have a need to manage platforms
> 
> that have been ported to edk2.  Jordan has created a repository
> 
> called edk2-platforms and has created a branch for the
> 
> minnowboard-max that uses a validated release of the UDK 2015 for
> 
> the dependent packages:
> 
> 
> 
> https://github.com/tianocore/edk2-platforms
> 
> 
> 
> https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-
> udk2015

[Bhupesh] This link seems broken. I get a 404 error while trying to acess this 
branch. Does it require some special privileges.

Regards,
Bhupesh

[snip..]
___
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 V2] BaseTools: support the NOOPT target with the GCC tool chains

[Laszlo Ersek]

On 10/05/16 03:30, Yonghong Zhu wrote:
> Update the tools_def.template to add NOOPT support with GCC tool chains.
> 
> Cc: Liming Gao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Conf/tools_def.template | 28 
>  1 file changed, 28 insertions(+)

I thought I understood what was going on, but apparently I was wrong
about that.

In this patch, we add or modify:
- NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay
- NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay

So that part is fine with me. But then we also add / modify:
- NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS)
- NOOPT_GCC5_ARM_DLINK_FLAGS

First I thought the latter set of changes was unnecessary, because "ld"
didn't use "-O". I checked the manual, and I was wrong: "ld" does know /
use "-O". So those changes are fine, I guess.

But then: is the patch *complete*? Because I can see some more DLINK
stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore
those? For example:

*_GCC5_IA32_DLINK_FLAGS  = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os
-Wl,-m,elf_i386,--oformat=elf32-i386


*_GCC5_X64_DLINK_FLAGS   = DEF(GCC5_X64_DLINK_FLAGS) -Os

Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include
-flto. (I don't know if "-flto" hampers source level debugging or not.)

Is there a way for BaseTools to dump the complete, expanded (i.e., no
macro references and no asterisks) macro list?

Thanks
Laszlo


> diff --git a/BaseTools/Conf/tools_def.template 
> b/BaseTools/Conf/tools_def.template
> index 5414454..2c0dcd6 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -4333,10 +4333,11 @@ NOOPT_DDK3790xASL_IPF_DLINK_FLAGS= /NOLOGO 
> /NODEFAULTLIB /LTCG /DLL /OPT:REF
>  *_*_*_OBJCOPY_FLAGS = objcopy not needed for
>  *_*_*_SYMRENAME_PATH= echo
>  *_*_*_SYMRENAME_FLAGS   = Symbol renaming not needed for
>  DEBUG_*_*_OBJCOPY_ADDDEBUGFLAG = 
> --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_NAME).debug
>  RELEASE_*_*_OBJCOPY_ADDDEBUGFLAG   =
> +NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG = 
> --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_NAME).debug
>  
>  DEFINE GCC_ALL_CC_FLAGS= -g -Os -fshort-wchar 
> -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h 
> -fno-common
>  DEFINE GCC_IA32_CC_FLAGS   = DEF(GCC_ALL_CC_FLAGS) -m32 
> -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 
> -mno-stack-arg-probe
>  DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
> -Wno-address -mno-stack-arg-probe
>  DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
> -minline-int-divide-min-latency
> @@ -4767,10 +4768,11 @@ DEFINE GCC5_AARCH64_ASLDLINK_FLAGS   = 
> DEF(GCC49_AARCH64_ASLDLINK_FLAGS)
>  *_GCC46_IA32_OBJCOPY_FLAGS= 
>  *_GCC46_IA32_NASM_FLAGS   = -f elf32
>  
>DEBUG_GCC46_IA32_CC_FLAGS   = DEF(GCC46_IA32_CC_FLAGS) -Os
>  RELEASE_GCC46_IA32_CC_FLAGS   = DEF(GCC46_IA32_CC_FLAGS) -Os 
> -Wno-unused-but-set-variable
> +  NOOPT_GCC46_IA32_CC_FLAGS   = DEF(GCC46_IA32_CC_FLAGS) -O0
>  
>  ##
>  # GCC46 X64 definitions
>  ##
>  *_GCC46_X64_OBJCOPY_PATH = DEF(GCC46_X64_PREFIX)objcopy
> @@ -4794,10 +4796,11 @@ RELEASE_GCC46_IA32_CC_FLAGS   = 
> DEF(GCC46_IA32_CC_FLAGS) -Os -Wno-unused-but
>  *_GCC46_X64_OBJCOPY_FLAGS= 
>  *_GCC46_X64_NASM_FLAGS   = -f elf64
>  
>DEBUG_GCC46_X64_CC_FLAGS   = DEF(GCC46_X64_CC_FLAGS)
>  RELEASE_GCC46_X64_CC_FLAGS   = DEF(GCC46_X64_CC_FLAGS) 
> -Wno-unused-but-set-variable
> +  NOOPT_GCC46_X64_CC_FLAGS   = DEF(GCC46_X64_CC_FLAGS) -O0
>  
>  ##
>  # GCC46 ARM definitions
>  ##
>  *_GCC46_ARM_OBJCOPY_PATH = echo
> @@ -4826,10 +4829,11 @@ RELEASE_GCC46_X64_CC_FLAGS   = 
> DEF(GCC46_X64_CC_FLAGS) -Wno-unused-but-set-v
>  *_GCC46_ARM_VFRPP_FLAGS  = $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
> DEF(GCC_VFRPP_FLAGS)
>  *_GCC46_ARM_CC_XIPFLAGS  = DEF(GCC46_ARM_CC_XIPFLAGS)
>  
>DEBUG_GCC46_ARM_CC_FLAGS   = DEF(GCC46_ARM_CC_FLAGS) -O0
>  RELEASE_GCC46_ARM_CC_FLAGS   = DEF(GCC46_ARM_CC_FLAGS) 
> -Wno-unused-but-set-variable
> +  NOOPT_GCC46_ARM_CC_FLAGS   = DEF(GCC46_ARM_CC_FLAGS) -O0
>  
>  
> 
>  #
>  # GCC 4.7 - This configuration is used to compile under Linux to produce
>  #   PE/COFF binaries using GCC 4.7.
> @@ -4873,10 +4877,11 @@ RELEASE_GCC46_ARM_CC_FLAGS   = 
> DEF(GCC46_ARM_CC_FLAGS) -Wno-unused-but-set-v
>  *_GCC47_IA32_OBJCOPY_FLAGS= 
>  *_GCC47_IA32_NASM_FLAGS   = -f elf32
>  
>DEBUG_GCC47_IA32_CC_FLAGS   = DEF(GCC47_IA32_CC_FLAGS) -Os
>  RELEASE_GCC47_IA32_CC_FLAGS   = DEF(GCC47_IA32_CC_FLAGS) -Os 
> -Wno-unused-but-set-variable
> +  NOOPT_GCC47_IA32_CC_FLAGS

[edk2] TE relocations

[valerij zaporogeci]

Hi everyone. I have a problem with understanding the description of TE
files and its relocations.
The specifications says this:
"17.2 XIP Images
For execute-in-place (XIP) images that do not require relocations,
loading a TE image simply
requires that the loader adjust the image’s entry point from the value
specified in the
EFI_TE_IMAGE_HEADER. For example, if the image (and thus the TE
header) resides at memory
location LoadedImageAddress, then the actual entry for the driver is
computed as follows:
EntryPoint = LoadedImageAddress + sizeof (EFI_TE_IMAGE_HEADER)
+
((EFI_TE_IMAGE_HEADER *)LoadedImageAddress)–>
AddressOfEntryPoint – ((EFI_TE_IMAGE_HEADER *)
LoadedImageAddress)–>StrippedSize;"

But it looks not as simple as "simply" adjusting the only
AddressOfEntryPoint. What "do not require relocations means" here?
Suppose an Image has ImageBase set to X in its header, and it IS at
the address X in XIP memory. Will it need relocations? For PE it
won't, but it seems it is not the case for TE. Since not only
AddressOfEntryPoint is affected, also CodeBase, sections RVAs and all
base relocations too. They all are lying not at their RVA in XIP
memory, because TE has a smaller header and screws up even
FileAlignment of sections. Doesn't it?
The adjustment in the quote uses StrippedSize and it is described as
what was removed from PE:

"The StrippedSize should be set to the number of bytes removed from
the start of the original
image, which will typically include the MS-DOS, COFF, and optional
headers, as well as the section headers."

But does it take into account section alignment by FileAlignment?
(SectionAlignment==FileAlignment in this case). Because for example if
FileAlignment = 200h, then in PE, section for code would be sitting at
n*200h from the beginning of the file and from ImageBase in XIP
memory.
Here is how specification describes PE->TE tranformation process:
"• Create an EFI_TE_IMAGE_HEADER in memory
• Parse the PE/COFF headers in an existing image and extract the
necessary fields to fill in  the EFI_TE_IMAGE_HEADER
• Fill in the signature and stripped size fields in the EFI_TE_IMAGE_HEADER
• Write out the EFI_TE_IMAGE_HEADER to a new binary file
• Write out the contents of the original image, less the stripped
headers, to the output file"

This process would place any section at different file offset
(dispecting alignment) than in PE, and as a result for XIP - different
RVA from ImageBase. For XIP this would mean that such a file requires
relocation, even though its actual ImageBase is exactly what is
written in the file. For non-XIP it would mean that the loader would
need section headers to know what displacement to put the section to
in memory to not break addresses. But they are stripped! Otherwise, if
it just blindly copies what is after TE header, RVAs would be broken,
all addresses would be broken, as for XIP.
In fact this means any TE file loaded at its preferred ImageBase will
need relocations. And for non-XIP. section header cannot be stripped.
Then what TE files would not need relocations?
Did I understand right?
Thank you for answers.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] OVMF.fd and placement of EfiBootServicesData

[Laszlo Ersek]

On 10/05/16 04:47, spam collector wrote:
> 
> - Original Message -
>> From: "Laszlo Ersek" 
>> To: "spam collector" 
>> Cc: edk2-de...@ml01.01.org
>> Sent: Tuesday, October 4, 2016 10:22:34 AM
>> Subject: Re: [edk2] OVMF.fd and placement of EfiBootServicesData

>> * The first stage improvement is the following command line:
>>
>>   qemu-system-x86_64 -pflash OVMF.fd
> 
> This did not work either with or without the NvVars file present.

I think I'll need a more complete issue riport then -- what is your
exact QEMU command line, and what OVMF binary are you using?

>> * The second stage improvement is the following command line:
>>
>>   # create the virtual machine's private variable store from the
>>   # pristine variable store template, if the former doesn't exist yet,
>>   # or has been lost for some reason
>>   if ! [ -e GUEST_1_VARS.fd ]; then
>> cp OVMF_VARS.fd GUEST_1_VARS.fd
>>   fi
>>
>>   qemu-system-x86_64 \
>> -drive if=pflash,format=raw,unit=0,readonly,file=OVMF_CODE.fd \
>> -drive if=pflash,format=raw,unit=1,file=GUEST_1_VARS.fd \
>> ...
>>
>> In this case, the OVMF_CODE.fd and OVMF_VARS.fd are used separately.
>> (You can find both files under the Build directory; plus Gerd Hoffmann's
>> RPM files  also package them.)
> 
> For the life of me I could not find any directory called "Build".

The Build directory exists in the source edk2 tree, if you build the
firmware yourself.

>  I did find a few RPM files and one of them contained:
>   OVMF_CODE-pure-efi.fd
>  and
>   OVMF_VARS-pure-efi.fd

Yes, they are from Gerd's "edk2.git-ovmf-ia32" package (since you
mention IA32), from , and they are
recommended to use.

> However, no matter the emulation (32- or 64-bit), neither worked and
> would get as far as printing the '.' just before the 
>   .FLOPPY failed
>   .CD-ROM failed
>   etc.
> lines and go no further.

These messages mean the following: you are running QEMU in a default
configuration, with a default NIC, a default floppy, a default CD-ROM,
and so on. OVMF automatically generates boot options for these (in some
unspecified order), and then tries to boot those options. Specifically
the dots with the long delay mean that OVMF is trying to PXE boot from
your virtual NIC.

There are two ways around this:

- First, if you have a dedicated device you want to boot off of, use
  the "bootindex" property I mentioned earlier.
  - Specifically, if you want to boot the UEFI shell like this, then
use "/usr/share/edk2.git/ovmf-ia32/UefiShell.iso" with an ide-cd or
scsi-cd device.

- Second, you can pass "-nodefaults" to QEMU. This option will prevent
  the automatic creation of the NIC, the floppy, the CD-ROM -- and a
  lot more actually, such as keyboard, mouse, VGA too --; in turn OVMF
  should not create boot options for these devices either, and should
  drop you directly to the shell.

  Assuming of course that your firmware binary includes the shell in
  the first place. If it doesn't, then you can only use "UefiShell.iso".

I recommend trying the following (32-bit command line), with Gerd's package:

  FW_BIN=/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd
  VARS_TMPL=/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd
  VARS=myvars.fd
  SHELL_ISO=/usr/share/edk2.git/ovmf-ia32/UefiShell.iso
  if ! [ -e "$VARS" ]; then
cp -v -- "$VARS_TMPL" "$VARS"
  fi

  qemu-system-i386 \
-m 2048 \
-machine accel=kvm \
\
-drive if=pflash,format=raw,unit=0,readonly,file="$FW_BIN" \
-drive if=pflash,format=raw,unit=1,file="$VARS" \
\
-device virtio-scsi-pci,id=scsi0 \
-drive if=none,format=raw,readonly,file="$SHELL_ISO",id=shell \
-device scsi-cd,bus=scsi0.0,drive=shell,bootindex=1 \
\
-chardev file,id=debugfile,path=ovmf.log \
-device isa-debugcon,iobase=0x402,chardev=debugfile

This will boot the shell for you, and even save the OVMF debug log in
the file "ovmf.log".

> would you please specify a specific URL that contains the two files
> you speak of, whether it be a listing of those files, a .gz file, or
> even a .rpm file.

If you GNU/Linux distro of choice is RPM-based, simply follow the
instrutions at .

Otherwise, download the "edk2.git-ovmf-ia32" or "edk2.git-ovmf-x64" RPM
file (as needed) from , and
extract it with:

  rpm2cpio FILENAME | pax -r -v

The extracted files you want are:

  usr/share/edk2.git/ovmf-*/OVMF_CODE-pure-efi.fd
  usr/share/edk2.git/ovmf-*/OVMF_VARS-pure-efi.fd
  usr/share/edk2.git/ovmf-*/UefiShell.iso

Thanks
Laszlo

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Carsey, Jaben]

Tapan,

Could this be a side effect of your patch?  Should we allow the UefiShellLib to 
continue without this protocol and then return an error only when the OpenFile 
is called?

Also, it looks like we never properly initialize mUnicodeCollationProtocol.  We 
check for NULL in Constructor, but nothing else...

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Supreeth Venkatesh
> Sent: Tuesday, October 04, 2016 3:51 PM
> To: edk2-devel-01 
> Cc: Leif Lindholm 
> Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the
> Reference Platform
> Importance: High
> 
> All,
> 
> Recently, I updated to latest edk2 master (yesterday's) and I am actually
> encountering assert in ShellPkg/Library/UefiShellLib/UefiShellLib.c
> 
> if (mUnicodeCollationProtocol == NULL) {
> Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL,
> (VOID**)&mUnicodeCollationProtocol);
> ASSERT_EFI_ERROR (Status);
>   }
> 
> It was working few weeks back and has now stopped working.
> It's probably because  the platform is unable to locate this protocol
> "UnicodeCollationProtocol".
> Is Anyone else facing the same issue?
> Does the new ShellPkg requires additional packages/infs to be included in
> the platform description file to work with latest changes  to get past this
> error?
> 
> Thanks,
> Supreeth
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended 
> recipient,
> please notify the sender immediately and do not disclose the contents to any
> other person, use it for any purpose, or store or copy the information in any
> medium. Thank you.
> ___
> 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] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Shah, Tapan]

It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is 
necessary for even other Shell libraries other than UefiShellLib in order to 
have Shell parser and other command line processing to work properly. That's 
why I added ASSERT if UefiShellLib fails to locate the protocol.
 
Reference platform should have EnglishDxe module to have this protocol 
installed properly.

  MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com] 
Sent: Wednesday, October 05, 2016 10:41 AM
To: Supreeth Venkatesh ; edk2-devel-01 
; Shah, Tapan 
Cc: Leif Lindholm ; Carsey, Jaben 

Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

Tapan,

Could this be a side effect of your patch?  Should we allow the UefiShellLib to 
continue without this protocol and then return an error only when the OpenFile 
is called?

Also, it looks like we never properly initialize mUnicodeCollationProtocol.  We 
check for NULL in Constructor, but nothing else...

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Supreeth Venkatesh
> Sent: Tuesday, October 04, 2016 3:51 PM
> To: edk2-devel-01 
> Cc: Leif Lindholm 
> Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source 
> on the Reference Platform
> Importance: High
> 
> All,
> 
> Recently, I updated to latest edk2 master (yesterday's) and I am 
> actually encountering assert in 
> ShellPkg/Library/UefiShellLib/UefiShellLib.c
> 
> if (mUnicodeCollationProtocol == NULL) {
> Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, 
> NULL, (VOID**)&mUnicodeCollationProtocol);
> ASSERT_EFI_ERROR (Status);
>   }
> 
> It was working few weeks back and has now stopped working.
> It's probably because  the platform is unable to locate this protocol 
> "UnicodeCollationProtocol".
> Is Anyone else facing the same issue?
> Does the new ShellPkg requires additional packages/infs to be included 
> in the platform description file to work with latest changes  to get 
> past this error?
> 
> Thanks,
> Supreeth
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose 
> the contents to any other person, use it for any purpose, or store or 
> copy the information in any medium. Thank you.
> ___
> 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] UefiBootManagerLib and LegacyBootManagerLib issue

[Saqib Khan]

I have found that  it just dont return from

*mBmRefreshLegacyBootOption (); .*
have a look at code. let me know the possible cause of it ...
I need urgent help

EfiBootManagerRefreshAllBootOption (
  VOID
  )
{
  EFI_STATUSStatus;
  EFI_BOOT_MANAGER_LOAD_OPTION  *NvBootOptions;
  UINTN NvBootOptionCount;
  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
  UINTN BootOptionCount;
  UINTN Index;
  Print(L"indside refresh\n");
  //
  // Optionally refresh the legacy boot option
  //
  if (mBmRefreshLegacyBootOption != NULL) {
  Print(L"Before legacy refresh \n");
*mBmRefreshLegacyBootOption ();  *//this method does not return
Print(L"legacy refresh complete\n");
  }

On Wed, Oct 5, 2016 at 5:51 PM, Saqib Khan  wrote:

> Hi,
>
> when i import both lib in my project my EFI hangs at
> EfiRefreshAllBootOptions, i removed LegacyBootManager and it worked fine .i
> need both lib as i need to boot legacy from EFI, how this issue can be
> resolved?
>
>
> here is piece of inf file
>
> [Packages]
>   MdePkg/MdePkg.dec
>   MdeModulePkg/MdeModulePkg.dec
>   IntelFrameworkPkg/
> IntelFrameworkPkg.dec
>   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>   ShellPkg/ShellPkg.dec
>
> [LibraryClasses]
>   HiiLib
>   DebugLib
>   UefiLib
>   MemoryAllocationLib
>   UefiBootServicesTableLib
>   UefiApplicationEntryPoint
>   UefiBootManagerLib
>   LegacyBootManagerLib
>
>
> --
> Regards
> Saqib Ahmed Khanzada
>



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

Re: [edk2] [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains

[Ard Biesheuvel]

On 5 October 2016 at 15:48, Laszlo Ersek  wrote:
> On 10/05/16 03:30, Yonghong Zhu wrote:
>> Update the tools_def.template to add NOOPT support with GCC tool chains.
>>
>> Cc: Liming Gao 
>> Cc: Laszlo Ersek 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Yonghong Zhu 
>> ---
>>  BaseTools/Conf/tools_def.template | 28 
>>  1 file changed, 28 insertions(+)
>
> I thought I understood what was going on, but apparently I was wrong
> about that.
>
> In this patch, we add or modify:
> - NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay
> - NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay
>
> So that part is fine with me. But then we also add / modify:
> - NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS)
> - NOOPT_GCC5_ARM_DLINK_FLAGS
>
> First I thought the latter set of changes was unnecessary, because "ld"
> didn't use "-O". I checked the manual, and I was wrong: "ld" does know /
> use "-O". So those changes are fine, I guess.
>

Yes, especially under LTO, in which case code generation is performed
during the link stage, which should adhere to the same rules as the
compiler. This not only applies to -O, but also to things like
-march/-mcpu and -mstrict-align. This is why we pass all CFLAGS to the
linker for the GCC5 LTO builds.

> But then: is the patch *complete*? Because I can see some more DLINK
> stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore
> those? For example:
>
> *_GCC5_IA32_DLINK_FLAGS  = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os
> -Wl,-m,elf_i386,--oformat=elf32-i386
>
>
> *_GCC5_X64_DLINK_FLAGS   = DEF(GCC5_X64_DLINK_FLAGS) -Os
>
> Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include
> -flto. (I don't know if "-flto" hampers source level debugging or not.)
>

The GCC man page documents -flto as being a bad idea, i.e.,

"""
Link-time optimization does not work well with generation of debugging
information.  Combining -flto with -g is currently experimental and
expected to produce unexpected results.
"""

(which raises a philosophical question as well, i.e., to which extent
expected unexpected results are still unexpected results. But I
digress ...)

Another note: the DEBUG build for ARM and AARCH64 is essentially NOOPT
already, not DEBUG. How does this patch intend to deal with that?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] TE relocations

[Andrew Fish]

> On Oct 5, 2016, at 8:08 AM, valerij zaporogeci  wrote:
> 
> Hi everyone. I have a problem with understanding the description of TE
> files and its relocations.

TE is basically PE/COFF with a smaller header. Unlike ELF (Mach-O, etc.) 
PE/COFF includes the headers in the executable image. The TE header is 
basically the bits required by EFI and the adjustments (so you can calculate 
the location the PE/COFF header would have started, since the sections are 
relative to that in the PE/COFF debug info) needed by the debugger to turn it 
into a PE/COFF. 

The only reason TE exists is to save size in the ROM for XIP code. The build 
tools zero out the unused parts of the PE/COFF header so they tend to compress 
well and TE is not needed. 

> The specifications says this:
> "17.2 XIP Images
> For execute-in-place (XIP) images that do not require relocations,
> loading a TE image simply
> requires that the loader adjust the image’s entry point from the value
> specified in the
> EFI_TE_IMAGE_HEADER. For example, if the image (and thus the TE
> header) resides at memory
> location LoadedImageAddress, then the actual entry for the driver is
> computed as follows:
> EntryPoint = LoadedImageAddress + sizeof (EFI_TE_IMAGE_HEADER)
> +
> ((EFI_TE_IMAGE_HEADER *)LoadedImageAddress)–>
> AddressOfEntryPoint – ((EFI_TE_IMAGE_HEADER *)
> LoadedImageAddress)–>StrippedSize;"
> 
> But it looks not as simple as "simply" adjusting the only
> AddressOfEntryPoint. What "do not require relocations means" here?

PE/COFF images are linked at a specific address and can execute directly if 
they are run from that address. If a PE/COFF image is loaded/executed from a 
different address the relocations must be applied to adjust all the absolute 
references in the image to the new addresses. 

The edk2 links PE/COFF images at zero (or for things like ELF they add a pad 
for the PE/COFF header ~0x220). When an FV is constructed and XIP (SEC, PEIM, 
and PEI CORE) images are placed in the FV these PE/COFF images get relocated to 
the address in the FV (ROM address). 

If the code is XIP then there is no need for relocations as that code can run 
from any address. 

I think this section is making the point that even if the image does not 
contain any relocations (all code is PC relative for example) you still have to 
adjust the EntryPoint to point to ROM address of the entry point. 

> Suppose an Image has ImageBase set to X in its header, and it IS at
> the address X in XIP memory. Will it need relocations?

No. 

> For PE it
> won't, but it seems it is not the case for TE. Since not only
> AddressOfEntryPoint is affected, also CodeBase, sections RVAs and all
> base relocations too. They all are lying not at their RVA in XIP
> memory, because TE has a smaller header and screws up even
> FileAlignment of sections. Doesn't it?

No. The TE header lets you compute the location of the PE/COFF header as this 
is required to make debuggers happy. So part of the TE construction is making 
sure the virtual start of the PE/COFF header is aligned correctly. You also 
need to make sure that the file alignment and section alignment are the same. 
For most toolchains 0x20 is uses as the file and section alignment to save 
space.

Note: 0x20 was picked as that was the smallest value we could get to work with 
Visual Studio "back in the day."

> The adjustment in the quote uses StrippedSize and it is described as
> what was removed from PE:
> 
> "The StrippedSize should be set to the number of bytes removed from
> the start of the original
> image, which will typically include the MS-DOS, COFF, and optional
> headers, as well as the section headers."
> 
> But does it take into account section alignment by FileAlignment?

Yes as I pointed out section alignment and FileAlignment has to be the same for 
XIP to work. 

> (SectionAlignment==FileAlignment in this case). Because for example if
> FileAlignment = 200h, then in PE, section for code would be sitting at
> n*200h from the beginning of the file and from ImageBase in XIP
> memory.
> Here is how specification describes PE->TE tranformation process:
> "• Create an EFI_TE_IMAGE_HEADER in memory
> • Parse the PE/COFF headers in an existing image and extract the
> necessary fields to fill in  the EFI_TE_IMAGE_HEADER
> • Fill in the signature and stripped size fields in the EFI_TE_IMAGE_HEADER
> • Write out the EFI_TE_IMAGE_HEADER to a new binary file
> • Write out the contents of the original image, less the stripped
> headers, to the output file"
> 
> This process would place any section at different file offset
> (dispecting alignment) than in PE, and as a result for XIP - different
> RVA from ImageBase. For XIP this would mean that such a file requires
> relocation, even though its actual ImageBase is exactly what is
> written in the file. For non-XIP it would mean that the loader would
> need section headers to know what displacement to put the section to
> in memory to not break addresses. But they are

Re: [edk2] [PATCH] IntelFsp2Pkg/Tools: Add PE32 section rebasing support

[Yarlagadda, Satya P]

Reviewed-by: Satya Yarlagadda 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Maurice 
Ma
Sent: Wednesday, October 05, 2016 5:49 AM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen 
Subject: [edk2] [PATCH] IntelFsp2Pkg/Tools: Add PE32 section rebasing support

The current SplitFspBin.py can only support TE image format rebasing in an FSP 
binary. This patch adds PE32 image format rebasing support.

Cc: Jiewen Yao 
Cc: Giri P Mudusuru 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Maurice Ma 
---
 IntelFsp2Pkg/Tools/SplitFspBin.py | 174 +++---
 1 file changed, 145 insertions(+), 29 deletions(-)

diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py 
b/IntelFsp2Pkg/Tools/SplitFspBin.py
index ef759f0dc46a..e4c3aa6d0b28 100644
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -159,12 +159,102 @@ class EFI_TE_IMAGE_HEADER(Structure):
 ('DataDirectoryDebug',  EFI_IMAGE_DATA_DIRECTORY)
 ]
 
+class EFI_IMAGE_DOS_HEADER(Structure):
+_fields_ = [
+('e_magic',  c_uint16),
+('e_cblp',   c_uint16),
+('e_cp', c_uint16),
+('e_crlc',   c_uint16),
+('e_cparhdr',c_uint16),
+('e_minalloc',   c_uint16),
+('e_maxalloc',   c_uint16),
+('e_ss', c_uint16),
+('e_sp', c_uint16),
+('e_csum',   c_uint16),
+('e_ip', c_uint16),
+('e_cs', c_uint16),
+('e_lfarlc', c_uint16),
+('e_ovno',   c_uint16),
+('e_res',ARRAY(c_uint16, 4)),
+('e_oemid',  c_uint16),
+('e_oeminfo',c_uint16),
+('e_res2',   ARRAY(c_uint16, 10)),
+('e_lfanew', c_uint16)
+]
+
+class EFI_IMAGE_FILE_HEADER(Structure):
+_fields_ = [
+('Machine',   c_uint16),
+('NumberOfSections',  c_uint16),
+('TimeDateStamp', c_uint32),
+('PointerToSymbolTable',  c_uint32),
+('NumberOfSymbols',   c_uint32),
+('SizeOfOptionalHeader',  c_uint16),
+('Characteristics',   c_uint16)
+]
+
 class PE_RELOC_BLOCK_HEADER(Structure):
 _fields_ = [
 ('PageRVA',  c_uint32),
 ('BlockSize',c_uint32)
 ]
 
+class EFI_IMAGE_OPTIONAL_HEADER32(Structure):
+_fields_ = [
+('Magic', c_uint16),
+('MajorLinkerVersion',c_uint8),
+('MinorLinkerVersion',c_uint8),
+('SizeOfCode',c_uint32),
+('SizeOfInitializedData', c_uint32),
+('SizeOfUninitializedData',   c_uint32),
+('AddressOfEntryPoint',   c_uint32),
+('BaseOfCode',c_uint32),
+('BaseOfData',c_uint32),
+('ImageBase', c_uint32),
+('SectionAlignment',  c_uint32),
+('FileAlignment', c_uint32),
+('MajorOperatingSystemVersion',   c_uint16),
+('MinorOperatingSystemVersion',   c_uint16),
+('MajorImageVersion', c_uint16),
+('MinorImageVersion', c_uint16),
+('MajorSubsystemVersion', c_uint16),
+('MinorSubsystemVersion', c_uint16),
+('Win32VersionValue', c_uint32),
+('SizeOfImage',   c_uint32),
+('SizeOfHeaders', c_uint32),
+('CheckSum' , c_uint32),
+('Subsystem', c_uint16),
+('DllCharacteristics',c_uint16),
+('SizeOfStackReserve',c_uint32),
+('SizeOfStackCommit' ,c_uint32),
+('SizeOfHeapReserve', c_uint32),
+('SizeOfHeapCommit' , c_uint32),
+('LoaderFlags' ,  c_uint32),
+('NumberOfRvaAndSizes',   c_uint32),
+('DataDirectory', ARRAY(EFI_IMAGE_DATA_DIRECTORY, 16))
+]
+
+class EFI_IMAGE_NT_HEADERS32(Structure):
+_fields_ = [
+('Signature',c_uint32),
+('FileHeader',   EFI_IMAGE_FILE_HEADER),
+('OptionalHeader',   EFI_IMAGE_OPTIONAL_HEADER32)
+]
+
+
+class EFI_IMAGE_DIRECTORY_ENTRY:
+EXPORT = 0
+IMPORT = 1
+RESOURCE   = 2
+EXCEPTION  = 3
+SECURITY   = 4
+BASERELOC  = 5
+DEBUG  = 6
+COPYRIGHT  = 7
+GLOBALPTR  = 8
+TLS= 9
+LOAD_CONFIG= 10
+
 c

Re: [edk2] [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains

[Laszlo Ersek]

On 10/05/16 18:06, Ard Biesheuvel wrote:
> On 5 October 2016 at 15:48, Laszlo Ersek  wrote:
>> On 10/05/16 03:30, Yonghong Zhu wrote:
>>> Update the tools_def.template to add NOOPT support with GCC tool chains.
>>>
>>> Cc: Liming Gao 
>>> Cc: Laszlo Ersek 
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Yonghong Zhu 
>>> ---
>>>  BaseTools/Conf/tools_def.template | 28 
>>>  1 file changed, 28 insertions(+)
>>
>> I thought I understood what was going on, but apparently I was wrong
>> about that.
>>
>> In this patch, we add or modify:
>> - NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay
>> - NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay
>>
>> So that part is fine with me. But then we also add / modify:
>> - NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS)
>> - NOOPT_GCC5_ARM_DLINK_FLAGS
>>
>> First I thought the latter set of changes was unnecessary, because "ld"
>> didn't use "-O". I checked the manual, and I was wrong: "ld" does know /
>> use "-O". So those changes are fine, I guess.
>>
> 
> Yes, especially under LTO, in which case code generation is performed
> during the link stage, which should adhere to the same rules as the
> compiler. This not only applies to -O, but also to things like
> -march/-mcpu and -mstrict-align. This is why we pass all CFLAGS to the
> linker for the GCC5 LTO builds.
> 
>> But then: is the patch *complete*? Because I can see some more DLINK
>> stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore
>> those? For example:
>>
>> *_GCC5_IA32_DLINK_FLAGS  = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os
>> -Wl,-m,elf_i386,--oformat=elf32-i386
>>
>>
>> *_GCC5_X64_DLINK_FLAGS   = DEF(GCC5_X64_DLINK_FLAGS) -Os
>>
>> Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include
>> -flto. (I don't know if "-flto" hampers source level debugging or not.)
>>
> 
> The GCC man page documents -flto as being a bad idea, i.e.,
> 
> """
> Link-time optimization does not work well with generation of debugging
> information.  Combining -flto with -g is currently experimental and
> expected to produce unexpected results.
> """
> 
> (which raises a philosophical question as well, i.e., to which extent
> expected unexpected results are still unexpected results. But I
> digress ...)
> 
> Another note: the DEBUG build for ARM and AARCH64 is essentially NOOPT
> already, not DEBUG. How does this patch intend to deal with that?

It just copies the DEBUG settings to NOOPT (via deep copy, not by
reference). I believe that's OK.

Thanks
Laszlo

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

Re: [edk2] [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains

[Ard Biesheuvel]

On 5 October 2016 at 18:56, Laszlo Ersek  wrote:
> On 10/05/16 18:06, Ard Biesheuvel wrote:
>> On 5 October 2016 at 15:48, Laszlo Ersek  wrote:
>>> On 10/05/16 03:30, Yonghong Zhu wrote:
 Update the tools_def.template to add NOOPT support with GCC tool chains.

 Cc: Liming Gao 
 Cc: Laszlo Ersek 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Yonghong Zhu 
 ---
  BaseTools/Conf/tools_def.template | 28 
  1 file changed, 28 insertions(+)
>>>
>>> I thought I understood what was going on, but apparently I was wrong
>>> about that.
>>>
>>> In this patch, we add or modify:
>>> - NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay
>>> - NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay
>>>
>>> So that part is fine with me. But then we also add / modify:
>>> - NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS)
>>> - NOOPT_GCC5_ARM_DLINK_FLAGS
>>>
>>> First I thought the latter set of changes was unnecessary, because "ld"
>>> didn't use "-O". I checked the manual, and I was wrong: "ld" does know /
>>> use "-O". So those changes are fine, I guess.
>>>
>>
>> Yes, especially under LTO, in which case code generation is performed
>> during the link stage, which should adhere to the same rules as the
>> compiler. This not only applies to -O, but also to things like
>> -march/-mcpu and -mstrict-align. This is why we pass all CFLAGS to the
>> linker for the GCC5 LTO builds.
>>
>>> But then: is the patch *complete*? Because I can see some more DLINK
>>> stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore
>>> those? For example:
>>>
>>> *_GCC5_IA32_DLINK_FLAGS  = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os
>>> -Wl,-m,elf_i386,--oformat=elf32-i386
>>>
>>>
>>> *_GCC5_X64_DLINK_FLAGS   = DEF(GCC5_X64_DLINK_FLAGS) -Os
>>>
>>> Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include
>>> -flto. (I don't know if "-flto" hampers source level debugging or not.)
>>>
>>
>> The GCC man page documents -flto as being a bad idea, i.e.,
>>
>> """
>> Link-time optimization does not work well with generation of debugging
>> information.  Combining -flto with -g is currently experimental and
>> expected to produce unexpected results.
>> """
>>
>> (which raises a philosophical question as well, i.e., to which extent
>> expected unexpected results are still unexpected results. But I
>> digress ...)
>>
>> Another note: the DEBUG build for ARM and AARCH64 is essentially NOOPT
>> already, not DEBUG. How does this patch intend to deal with that?
>
> It just copies the DEBUG settings to NOOPT (via deep copy, not by
> reference). I believe that's OK.
>

OK, fair enough. Leif and I can look into this in the future.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 2/2] QuarkSocPkg/QncSmmDispatcher: Fix use after free issue

[Michael Kinney]

Update Quark North Cluster SMM dispatcher logic to handle
case where an SMI handler unregisters itself.

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

This issue was reproduced using the unit test at:

https://github.com/mdkinney/edk2/tree/Bug51/Reproduce

An ASSERT() is generated the 4th time the periodic SMI
handler is triggered when the periodic SMI handler
unregisters itself.  After applying this patch, the
DEBUG() message from the periodic SMI handler in this
unit test is generated 4 times, the periodic SMI handler
is unregistered, and the UEFI Shell works as expected.

Cc: Kelly Steele 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 .../Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h   |  2 ++
 .../Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c   | 41 +++---
 2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h 
b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h
index 797be16..ccf9dae 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h
@@ -387,6 +387,8 @@ struct _DATABASE_RECORD {
   UINT32Signature;
   LIST_ENTRYLink;
 
+  BOOLEAN   Processed;
+
   //
   // Status and Enable bit description
   //
diff --git 
a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c 
b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
index 4783406..c2f75f8 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
@@ -592,6 +592,7 @@ QNCSmmCoreDispatcher (
   BOOLEAN ChildWasDispatched;
 
   DATABASE_RECORD *RecordInDb;
+  DATABASE_RECORD ActiveRecordInDb;
   LIST_ENTRY  *LinkInDb;
   DATABASE_RECORD *RecordToExhaust;
   LIST_ENTRY  *LinkToExhaust;
@@ -614,6 +615,16 @@ QNCSmmCoreDispatcher (
   ChildWasDispatched= FALSE;
 
   //
+  // Mark all child handlers as not processed
+  //
+  LinkInDb = GetFirstNode (&mPrivateData.CallbackDataBase);
+  while (!IsNull (&mPrivateData.CallbackDataBase, LinkInDb)) {
+RecordInDb = DATABASE_RECORD_FROM_LINK (LinkInDb);
+RecordInDb->Processed = FALSE;
+LinkInDb = GetNextNode (&mPrivateData.CallbackDataBase, LinkInDb);
+  }
+
+  //
   // Preserve Index registers
   //
   SaveState ();
@@ -634,6 +645,12 @@ QNCSmmCoreDispatcher (
 
   while ((!IsNull (&mPrivateData.CallbackDataBase, LinkInDb)) && 
(ResetListSearch == FALSE)) {
 RecordInDb = DATABASE_RECORD_FROM_LINK (LinkInDb);
+//
+// Make a copy of the record that contains an active SMI source,
+// because un-register maybe invoked in callback function and
+// RecordInDb maybe released
+//
+CopyMem (&ActiveRecordInDb, RecordInDb, sizeof (ActiveRecordInDb));
 
 //
 // look for the first active source
@@ -663,6 +680,13 @@ QNCSmmCoreDispatcher (
   //
   while (!IsNull (&mPrivateData.CallbackDataBase, LinkToExhaust)) {
 RecordToExhaust = DATABASE_RECORD_FROM_LINK (LinkToExhaust);
+LinkToExhaust = GetNextNode (&mPrivateData.CallbackDataBase, 
LinkToExhaust);
+if (RecordToExhaust->Processed) {
+  //
+  // Record has already been processed.  Continue with next child 
handler.
+  //
+  continue;
+}
 
 if (CompareSources (&RecordToExhaust->SrcDesc, &ActiveSource)) {
   //
@@ -692,6 +716,11 @@ QNCSmmCoreDispatcher (
 ContextsMatch = TRUE;
   }
 
+  //
+  // Mark this child handler so it will not be processed again
+  //
+  RecordToExhaust->Processed = TRUE;
+
   if (ContextsMatch) {
 
 if (RecordToExhaust->BufferSize != 0) {
@@ -720,11 +749,13 @@ QNCSmmCoreDispatcher (
   SxChildWasDispatched = TRUE;
 }
   }
+  //
+  // Can not use RecordInDb after this point because Callback may 
have unregistered RecordInDb
+  // Restart processing of SMI handlers from the begining of the 
linked list because the
+  // state of the linked listed may have been modified due to 
unregister actions in the Callback.
+  //
+  LinkToExhaust = GetFirstNode (&mPrivateData.CallbackDataBase);
 }
-//
-// Get next record in DB
-//
-LinkToExhaust = GetNextNode (&mPrivateData.CallbackDataBase, 
&RecordToExhaust->Link);
   }
 
   if (RecordInDb->ClearSource == NULL) {
@@ -736,7 +767,7 @@ QNCSmmCoreDispatcher (
 //
 // This source requires special handling to clear
   

[edk2] [Patch 0/2] QuarkSocPkg/QncSmmDispatcher: Fix SMI Handler ASSERTs()

[Michael Kinney]

This series fixes the following two issues:

QuarkSocPkg QncSmmDispatcher passes incorrect context to SMI handler
  https://bugzilla.tianocore.org/show_bug.cgi?id=136

QuarkSockg Use after free in QNCSmmCoreDispatcher
  https://bugzilla.tianocore.org/show_bug.cgi?id=51

These issues can be reproduced using the unit test available in the following 
branch that registers a periodic SMI that is triggered every 8 seconds and 
unregisters itself after the periodic SMI handler has been triggered 4 times.

https://github.com/mdkinney/edk2/tree/Bug51/Reproduce

Cc: Kelly Steele 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 

Michael Kinney (2):
  QuarkSocPkg/QncSmmDispatcher: Fix context passed to SMI handlers
  QuarkSocPkg/QncSmmDispatcher: Fix use after free issue

 .../QncSmmDispatcher/QNC/QNCSmmPeriodicTimer.c |  4 +-
 .../Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h   |  9 ++--
 .../Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c   | 51 +-
 3 files changed, 49 insertions(+), 15 deletions(-)

-- 
2.6.3.windows.1

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

[edk2] [Patch 1/2] QuarkSocPkg/QncSmmDispatcher: Fix context passed to SMI handlers

[Michael Kinney]

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

1) Add CallbackContext field to the DATABASE_RECORD structure that
   is set to the RegisterContent value passed to QNCSmmCoreRegister().
   This is the content that must be passed to the SMI handler when
   its source is triggered.

2) Update usage of ChildContext field in the DATABASE_RECOD to use
   CopyMem() instead of structure assignments to avoid compiler
   use of memcpy() intrinsics

This issue was reproduced using the unit test at:

https://github.com/mdkinney/edk2/tree/Bug51/Reproduce

An ASSERT() is generated the first time the periodic SMI
handler is triggered.  After applying this patch, the
DEBUG() messages from the periodic SMI handler in this
unit test are generated.

Cc: Kelly Steele 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 .../Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmPeriodicTimer.c  |  4 ++--
 .../QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h |  7 ---
 .../QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c | 10 +-
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git 
a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmPeriodicTimer.c
 
b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmPeriodicTimer.c
index 1d1030c..670ca91 100644
--- 
a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmPeriodicTimer.c
+++ 
b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNC/QNCSmmPeriodicTimer.c
@@ -1,7 +1,7 @@
 /** @file
 File to contain all the hardware specific stuff for the Periodical Timer 
dispatch protocol.
 
-Copyright (c) 2013-2015 Intel Corporation.
+Copyright (c) 2013-2016 Intel Corporation.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -177,7 +177,7 @@ PeriodicTimerGetContext (
 // Update the elapsed time w/ the data from our tables
 //
 Record->CommBuffer.PeriodicTimer.ElapsedTime += TimerInterval->Interval;
-*HwContext = Record->ChildContext;
+CopyMem (HwContext, &Record->ChildContext, sizeof (QNC_SMM_CONTEXT));
   }
 }
 
diff --git a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h 
b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h
index 892294f..797be16 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmm.h
@@ -1,7 +1,7 @@
 /** @file
 Prototypes and defines for the QNC SMM Dispatcher.
 
-Copyright (c) 2013-2015 Intel Corporation.
+Copyright (c) 2013-2016 Intel Corporation.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -396,8 +396,9 @@ struct _DATABASE_RECORD {
   // Callback function
   //
   EFI_SMM_HANDLER_ENTRY_POINT2  Callback;
-  QNC_SMM_CONTEXT   ChildContext;
-  QNC_SMM_BUFFER CommBuffer;
+  QNC_SMM_CONTEXT   ChildContext;
+  VOID  *CallbackContext;
+  QNC_SMM_BUFFERCommBuffer;
   UINTN BufferSize;
 
   //
diff --git 
a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c 
b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
index ba8c721..4783406 100644
--- a/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
+++ b/QuarkSocPkg/QuarkNorthCluster/Smm/DxeSmm/QncSmmDispatcher/QNCSmmCore.c
@@ -2,7 +2,7 @@
 This driver is responsible for the registration of child drivers
 and the abstraction of the QNC SMI sources.
 
-Copyright (c) 2013-2015 Intel Corporation.
+Copyright (c) 2013-2016 Intel Corporation.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -351,7 +351,8 @@ Returns:
   // Gather information about the registration request
   //
   Record->Callback  = DispatchFunction;
-  Record->ChildContext  = *RegisterContext;
+  Record->CallbackContext   = RegisterContext;
+  CopyMem (&Record->ChildContext, RegisterContext, sizeof (QNC_SMM_CONTEXT));
 
   Qualified = QUALIFIED_PROTOCOL_FROM_GENERIC (This);
 
@@ -407,7 +408,7 @@ Returns:
   //
   // Update ChildContext again as SwSmiInputValue has been changed
   //
-  Record->ChildContext = *RegisterContext;
+  CopyMem (&Record->ChildContext, RegisterContext, sizeof 
(QNC_SMM_CONTEXT));
 }
 
 //
@@ -688,7 +689,6 @@ QNCSmmCoreDispatcher (
 // it supplied in registration.  Simply pass back what it gave 
us.
 //
 ASSERT (RecordToExhaust->Callback != NULL);
-Context   = RecordToExhaust->ChildContext;
 ContextsMatch = TRUE;
   }
 
@@ -710,7 +710,7 @@ QNCSmmCoreDispatcher (
 
 RecordTo

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Daniil Egranov]

I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
included to the platform build. If UefiShellLib has such dependency on 
the protocol then according to EDKII Module Writer's Guide you need to 
specify the dependency on protocol in the module .inf to ensure the 
drivers proper load sequence.


8.6 Dependency Expressions
A dependency expression specifies the protocols that the DXE driver 
requires to

execute. In EDK II, it is specified in the [Depex] section of INF file.

The following dependency in UefiShellLib.inf fixes ASSERT problem:

[Depex]
  gEfiUnicodeCollation2ProtocolGuid


Thanks,

Daniil


On 10/05/2016 11:02 AM, Shah, Tapan wrote:

It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is 
necessary for even other Shell libraries other than UefiShellLib in order to 
have Shell parser and other command line processing to work properly. That's 
why I added ASSERT if UefiShellLib fails to locate the protocol.
  
Reference platform should have EnglishDxe module to have this protocol installed properly.


   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com]
Sent: Wednesday, October 05, 2016 10:41 AM
To: Supreeth Venkatesh ; edk2-devel-01 
; Shah, Tapan 
Cc: Leif Lindholm ; Carsey, Jaben 

Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

Tapan,

Could this be a side effect of your patch?  Should we allow the UefiShellLib to 
continue without this protocol and then return an error only when the OpenFile 
is called?

Also, it looks like we never properly initialize mUnicodeCollationProtocol.  We 
check for NULL in Constructor, but nothing else...

-Jaben


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Supreeth Venkatesh
Sent: Tuesday, October 04, 2016 3:51 PM
To: edk2-devel-01 
Cc: Leif Lindholm 
Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source
on the Reference Platform
Importance: High

All,

Recently, I updated to latest edk2 master (yesterday's) and I am
actually encountering assert in
ShellPkg/Library/UefiShellLib/UefiShellLib.c

if (mUnicodeCollationProtocol == NULL) {
 Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
NULL, (VOID**)&mUnicodeCollationProtocol);
 ASSERT_EFI_ERROR (Status);
   }

It was working few weeks back and has now stopped working.
It's probably because  the platform is unable to locate this protocol
"UnicodeCollationProtocol".
Is Anyone else facing the same issue?
Does the new ShellPkg requires additional packages/infs to be included
in the platform description file to work with latest changes  to get
past this error?

Thanks,
Supreeth
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose
the contents to any other person, use it for any purpose, or store or
copy the information in any medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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


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

Re: [edk2] TE relocations

[valerij zaporogeci]

Thank you, Andrew, I guess I understood. PE/TE images get relocated at
FV creation time, they execute directly from FV and they are linked at
where they really will lie in XIP memory. And the alignment value
chosen is not that minimum of 512 byte, mentioned in the PE
specification. Now it's clear. The only thing I still don't get is
that:
Suppose you have your input PE image, which you need to translate in
TE. It's linked at 0. It has a code section and it is lying (in the
file) at the first available 20h aligned offset right after all the
headers. Now you make translation into TE following the specification
algorithm. It says you should strip away all not needed stuff from the
beginning of the PE file, then put TE header instead (28h byte) and
right after it you should place all the remaining PE content. But
constructed this way TE, will break absolute referencies in the code.
Suppose some code references some data, using its address: address =
:0 +  + . Where
DataSectionBase is the offset of the beginning of the section from the
ImageBase, thus its RVA. ImageBase remains the same - 0, SectionOffset
too, but _actual_ DataSectionBase, would not be the same for the newly
created TE, because stripping got the data section closer to the file
beginning. Every reference to the data would be broken. Base
relocation at the FV creation won't help, since it changes ImageBase
(from 0 to "ROM address"), it doesn't fix DataSectionBase mismatch. If
after PE->TE translation, "somehow" DataSectionBase remains the same,
then what space saving it is? :)
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Tim Lewis]

Daniil --

Per your point about modules: Adding a dependency expression to a library 
should NOT have any effect on an application, since applications do not have 
dependency expressions. 

So this would indicate that you are trying to link a driver against the 
UefiShellLib. Is this correct?

This is valid for drivers that produce the new Dynamic commands. That is why 
the UefiShellLib INF allows DXE_RUNTIME_DRIVER and DXE_DRIVER. 
DXE_RUNTIME_DRIVER and DXE_DRIVER should call ShellInitialize() (this is what 
the Shell does for commands). Unfortunately, the UnicodeCollation initializer 
is not a part of this.

Jaben --- I suggest that moving the UnicodeCollation initializer into 
ShellLibConstructorWorker would probably solve the problem for a 
well-constructed shell extension driver. 

Tim 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
Egranov
Sent: Wednesday, October 05, 2016 12:24 PM
To: Shah, Tapan ; Carsey, Jaben ; 
Supreeth Venkatesh ; edk2-devel-01 

Cc: Leif Lindholm 
Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
included to the platform build. If UefiShellLib has such dependency on the 
protocol then according to EDKII Module Writer's Guide you need to specify the 
dependency on protocol in the module .inf to ensure the drivers proper load 
sequence.

8.6 Dependency Expressions
A dependency expression specifies the protocols that the DXE driver requires to 
execute. In EDK II, it is specified in the [Depex] section of INF file.

The following dependency in UefiShellLib.inf fixes ASSERT problem:

[Depex]
   gEfiUnicodeCollation2ProtocolGuid


Thanks,

Daniil


On 10/05/2016 11:02 AM, Shah, Tapan wrote:
> It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is 
> necessary for even other Shell libraries other than UefiShellLib in order to 
> have Shell parser and other command line processing to work properly. That's 
> why I added ASSERT if UefiShellLib fails to locate the protocol.
>   
> Reference platform should have EnglishDxe module to have this protocol 
> installed properly.
>
>
> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>
> -Original Message-
> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> Sent: Wednesday, October 05, 2016 10:41 AM
> To: Supreeth Venkatesh ; edk2-devel-01 
> ; Shah, Tapan 
> Cc: Leif Lindholm ; Carsey, Jaben 
> 
> Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on 
> the Reference Platform
>
> Tapan,
>
> Could this be a side effect of your patch?  Should we allow the UefiShellLib 
> to continue without this protocol and then return an error only when the 
> OpenFile is called?
>
> Also, it looks like we never properly initialize mUnicodeCollationProtocol.  
> We check for NULL in Constructor, but nothing else...
>
> -Jaben
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> Of Supreeth Venkatesh
>> Sent: Tuesday, October 04, 2016 3:51 PM
>> To: edk2-devel-01 
>> Cc: Leif Lindholm 
>> Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source 
>> on the Reference Platform
>> Importance: High
>>
>> All,
>>
>> Recently, I updated to latest edk2 master (yesterday's) and I am 
>> actually encountering assert in 
>> ShellPkg/Library/UefiShellLib/UefiShellLib.c
>>
>> if (mUnicodeCollationProtocol == NULL) {
>>  Status = gBS->LocateProtocol 
>> (&gEfiUnicodeCollation2ProtocolGuid,
>> NULL, (VOID**)&mUnicodeCollationProtocol);
>>  ASSERT_EFI_ERROR (Status);
>>}
>>
>> It was working few weeks back and has now stopped working.
>> It's probably because  the platform is unable to locate this protocol 
>> "UnicodeCollationProtocol".
>> Is Anyone else facing the same issue?
>> Does the new ShellPkg requires additional packages/infs to be 
>> included in the platform description file to work with latest changes  
>> to get past this error?
>>
>> Thanks,
>> Supreeth
>> IMPORTANT NOTICE: The contents of this email and any attachments are 
>> confidential and may also be privileged. If you are not the intended 
>> recipient, please notify the sender immediately and do not disclose 
>> the contents to any other person, use it for any purpose, or store or 
>> copy the information in any medium. Thank you.
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Andrew Fish]

> On Oct 5, 2016, at 12:24 PM, Daniil Egranov  wrote:
> 
> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
> included to the platform build. If UefiShellLib has such dependency on the 
> protocol then according to EDKII Module Writer's Guide you need to specify 
> the dependency on protocol in the module .inf to ensure the drivers proper 
> load sequence.
> 
> 8.6 Dependency Expressions
> A dependency expression specifies the protocols that the DXE driver requires 
> to
> execute. In EDK II, it is specified in the [Depex] section of INF file.
> 

The Dependency Expression is for DXE Drivers that are dispatched by the DXE 
Core. A UEFI Driver from an option ROM or an Application does not get 
dispatched by the dispatch and the Depex will not help. The Depex ends up being 
a section in the FV and it has nothing to do with the PE/COFF image of the an 
application or option ROM. 

IMHO the shell should try as hard as possible to function and should not ASSERT 
if some newer Protocol is missing.  

Thanks,

Andre wFish


> The following dependency in UefiShellLib.inf fixes ASSERT problem:
> 
> [Depex]
>  gEfiUnicodeCollation2ProtocolGuid
> 
> 
> Thanks,
> 
> Daniil
> 
> 
> On 10/05/2016 11:02 AM, Shah, Tapan wrote:
>> It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is 
>> necessary for even other Shell libraries other than UefiShellLib in order to 
>> have Shell parser and other command line processing to work properly. That's 
>> why I added ASSERT if UefiShellLib fails to locate the protocol.
>>  Reference platform should have EnglishDxe module to have this protocol 
>> installed properly.
>> 
>>   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>> 
>> -Original Message-
>> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
>> Sent: Wednesday, October 05, 2016 10:41 AM
>> To: Supreeth Venkatesh ; edk2-devel-01 
>> ; Shah, Tapan 
>> Cc: Leif Lindholm ; Carsey, Jaben 
>> 
>> Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the 
>> Reference Platform
>> 
>> Tapan,
>> 
>> Could this be a side effect of your patch?  Should we allow the UefiShellLib 
>> to continue without this protocol and then return an error only when the 
>> OpenFile is called?
>> 
>> Also, it looks like we never properly initialize mUnicodeCollationProtocol.  
>> We check for NULL in Constructor, but nothing else...
>> 
>> -Jaben
>> 
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>> Supreeth Venkatesh
>>> Sent: Tuesday, October 04, 2016 3:51 PM
>>> To: edk2-devel-01 
>>> Cc: Leif Lindholm 
>>> Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source
>>> on the Reference Platform
>>> Importance: High
>>> 
>>> All,
>>> 
>>> Recently, I updated to latest edk2 master (yesterday's) and I am
>>> actually encountering assert in
>>> ShellPkg/Library/UefiShellLib/UefiShellLib.c
>>> 
>>> if (mUnicodeCollationProtocol == NULL) {
>>> Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
>>> NULL, (VOID**)&mUnicodeCollationProtocol);
>>> ASSERT_EFI_ERROR (Status);
>>>   }
>>> 
>>> It was working few weeks back and has now stopped working.
>>> It's probably because  the platform is unable to locate this protocol
>>> "UnicodeCollationProtocol".
>>> Is Anyone else facing the same issue?
>>> Does the new ShellPkg requires additional packages/infs to be included
>>> in the platform description file to work with latest changes  to get
>>> past this error?
>>> 
>>> Thanks,
>>> Supreeth
>>> IMPORTANT NOTICE: The contents of this email and any attachments are
>>> confidential and may also be privileged. If you are not the intended
>>> recipient, please notify the sender immediately and do not disclose
>>> the contents to any other person, use it for any purpose, or store or
>>> copy the information in any medium. Thank you.
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 21:48, Andrew Fish wrote:
> 
>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov  wrote:
>>
>> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
>> included to the platform build. If UefiShellLib has such dependency on the 
>> protocol then according to EDKII Module Writer's Guide you need to specify 
>> the dependency on protocol in the module .inf to ensure the drivers proper 
>> load sequence.
>>
>> 8.6 Dependency Expressions
>> A dependency expression specifies the protocols that the DXE driver requires 
>> to
>> execute. In EDK II, it is specified in the [Depex] section of INF file.
>>
> 
> The Dependency Expression is for DXE Drivers that are dispatched by the DXE 
> Core. A UEFI Driver from an option ROM or an Application does not get 
> dispatched by the dispatch and the Depex will not help. The Depex ends up 
> being a section in the FV and it has nothing to do with the PE/COFF image of 
> the an application or option ROM. 
> 
> IMHO the shell should try as hard as possible to function and should not 
> ASSERT if some newer Protocol is missing.  

(Background: commit 583448b441650.)

In this specific case, the protocol dependency seems possible to eliminate:

- Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
includes the CharToUpper() function, which is minimal -- could even be
copied or moved over -- and can translate the ASCII subset of UCS-2,

- once the ASCII characters of the input string have been translated to
upper case, the result could be searched for / compared against L"NULL"
using BaseLib.h APIs.

(Given that L"NULL" only contains characters from the ASCII subset, it
suffices to upper-case ASCII chars in the input string. No non-ASCII
character in the BMP can translate to ASCII N/U/L via upcasing, even
with the real collation protocol (I trust), and no other ASCII
characters than n/u/l will translate to N/U/L. This means that we won't
miss an instance of NULL because we didn't upcate it (because it was
non-ascii), and it also means we won't mistakenly match something
non-NULL as NULL.)

Just my two cents.

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Shah, Tapan]

How about moving protocol locate from constructor to the actual function level 
and returning an error if it fails to locate (See attached patch file).

Tapan

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, October 05, 2016 3:18 PM
To: Andrew Fish ; Daniil Egranov 
Cc: Carsey, Jaben ; edk2-devel-01 
; Leif Lindholm ; Shah, Tapan 

Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

On 10/05/16 21:48, Andrew Fish wrote:
> 
>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov  wrote:
>>
>> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
>> included to the platform build. If UefiShellLib has such dependency on the 
>> protocol then according to EDKII Module Writer's Guide you need to specify 
>> the dependency on protocol in the module .inf to ensure the drivers proper 
>> load sequence.
>>
>> 8.6 Dependency Expressions
>> A dependency expression specifies the protocols that the DXE driver 
>> requires to execute. In EDK II, it is specified in the [Depex] section of 
>> INF file.
>>
> 
> The Dependency Expression is for DXE Drivers that are dispatched by the DXE 
> Core. A UEFI Driver from an option ROM or an Application does not get 
> dispatched by the dispatch and the Depex will not help. The Depex ends up 
> being a section in the FV and it has nothing to do with the PE/COFF image of 
> the an application or option ROM. 
> 
> IMHO the shell should try as hard as possible to function and should not 
> ASSERT if some newer Protocol is missing.  

(Background: commit 583448b441650.)

In this specific case, the protocol dependency seems possible to eliminate:

- Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
includes the CharToUpper() function, which is minimal -- could even be copied 
or moved over -- and can translate the ASCII subset of UCS-2,

- once the ASCII characters of the input string have been translated to upper 
case, the result could be searched for / compared against L"NULL"
using BaseLib.h APIs.

(Given that L"NULL" only contains characters from the ASCII subset, it suffices 
to upper-case ASCII chars in the input string. No non-ASCII character in the 
BMP can translate to ASCII N/U/L via upcasing, even with the real collation 
protocol (I trust), and no other ASCII characters than n/u/l will translate to 
N/U/L. This means that we won't miss an instance of NULL because we didn't 
upcate it (because it was non-ascii), and it also means we won't mistakenly 
match something non-NULL as NULL.)

Just my two cents.

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Carsey, Jaben]

That should work.  We also need to initialize the variable to NULL in the 
constructor.

Do we need to be case insensitive for this?  Can we use memcmp instead of the 
prototol?

-Jaben

> -Original Message-
> From: Shah, Tapan [mailto:tapands...@hpe.com]
> Sent: Wednesday, October 05, 2016 1:25 PM
> To: Laszlo Ersek ; Andrew Fish ;
> Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> Importance: High
> 
> How about moving protocol locate from constructor to the actual function
> level and returning an error if it fails to locate (See attached patch file).
> 
> Tapan
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, October 05, 2016 3:18 PM
> To: Andrew Fish ; Daniil Egranov
> 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm ; Shah, Tapan
> 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> 
> On 10/05/16 21:48, Andrew Fish wrote:
> >
> >> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
> wrote:
> >>
> >> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is
> included to the platform build. If UefiShellLib has such dependency on the
> protocol then according to EDKII Module Writer's Guide you need to specify
> the dependency on protocol in the module .inf to ensure the drivers proper
> load sequence.
> >>
> >> 8.6 Dependency Expressions
> >> A dependency expression specifies the protocols that the DXE driver
> >> requires to execute. In EDK II, it is specified in the [Depex] section of 
> >> INF
> file.
> >>
> >
> > The Dependency Expression is for DXE Drivers that are dispatched by the
> DXE Core. A UEFI Driver from an option ROM or an Application does not get
> dispatched by the dispatch and the Depex will not help. The Depex ends up
> being a section in the FV and it has nothing to do with the PE/COFF image of
> the an application or option ROM.
> >
> > IMHO the shell should try as hard as possible to function and should not
> ASSERT if some newer Protocol is missing.
> 
> (Background: commit 583448b441650.)
> 
> In this specific case, the protocol dependency seems possible to eliminate:
> 
> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
> includes the CharToUpper() function, which is minimal -- could even be
> copied or moved over -- and can translate the ASCII subset of UCS-2,
> 
> - once the ASCII characters of the input string have been translated to upper
> case, the result could be searched for / compared against L"NULL"
> using BaseLib.h APIs.
> 
> (Given that L"NULL" only contains characters from the ASCII subset, it
> suffices to upper-case ASCII chars in the input string. No non-ASCII character
> in the BMP can translate to ASCII N/U/L via upcasing, even with the real
> collation protocol (I trust), and no other ASCII characters than n/u/l will
> translate to N/U/L. This means that we won't miss an instance of NULL
> because we didn't upcate it (because it was non-ascii), and it also means we
> won't mistakenly match something non-NULL as NULL.)
> 
> Just my two cents.
> 
> Laszlo

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Shah, Tapan]

Yes, it has to be case insensitive per the latest Shell spec.

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com] 
Sent: Wednesday, October 05, 2016 3:35 PM
To: Shah, Tapan ; Laszlo Ersek ; Andrew 
Fish ; Daniil Egranov 
Cc: edk2-devel-01 ; Leif Lindholm 
; Carsey, Jaben 
Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

That should work.  We also need to initialize the variable to NULL in the 
constructor.

Do we need to be case insensitive for this?  Can we use memcmp instead of the 
prototol?

-Jaben

> -Original Message-
> From: Shah, Tapan [mailto:tapands...@hpe.com]
> Sent: Wednesday, October 05, 2016 1:25 PM
> To: Laszlo Ersek ; Andrew Fish ; 
> Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> Importance: High
> 
> How about moving protocol locate from constructor to the actual 
> function level and returning an error if it fails to locate (See attached 
> patch file).
> 
> Tapan
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, October 05, 2016 3:18 PM
> To: Andrew Fish ; Daniil Egranov 
> 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm ; Shah, Tapan 
> 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> 
> On 10/05/16 21:48, Andrew Fish wrote:
> >
> >> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
> >> 
> wrote:
> >>
> >> I have the same ASSERT issue on Juno platform even the 
> >> EnglishDxe.inf is
> included to the platform build. If UefiShellLib has such dependency on 
> the protocol then according to EDKII Module Writer's Guide you need to 
> specify the dependency on protocol in the module .inf to ensure the 
> drivers proper load sequence.
> >>
> >> 8.6 Dependency Expressions
> >> A dependency expression specifies the protocols that the DXE driver 
> >> requires to execute. In EDK II, it is specified in the [Depex] 
> >> section of INF
> file.
> >>
> >
> > The Dependency Expression is for DXE Drivers that are dispatched by 
> > the
> DXE Core. A UEFI Driver from an option ROM or an Application does not 
> get dispatched by the dispatch and the Depex will not help. The Depex 
> ends up being a section in the FV and it has nothing to do with the 
> PE/COFF image of the an application or option ROM.
> >
> > IMHO the shell should try as hard as possible to function and should 
> > not
> ASSERT if some newer Protocol is missing.
> 
> (Background: commit 583448b441650.)
> 
> In this specific case, the protocol dependency seems possible to eliminate:
> 
> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
> includes the CharToUpper() function, which is minimal -- could even be 
> copied or moved over -- and can translate the ASCII subset of UCS-2,
> 
> - once the ASCII characters of the input string have been translated 
> to upper case, the result could be searched for / compared against L"NULL"
> using BaseLib.h APIs.
> 
> (Given that L"NULL" only contains characters from the ASCII subset, it 
> suffices to upper-case ASCII chars in the input string. No non-ASCII 
> character in the BMP can translate to ASCII N/U/L via upcasing, even 
> with the real collation protocol (I trust), and no other ASCII 
> characters than n/u/l will translate to N/U/L. This means that we 
> won't miss an instance of NULL because we didn't upcate it (because it 
> was non-ascii), and it also means we won't mistakenly match something 
> non-NULL as NULL.)
> 
> Just my two cents.
> 
> Laszlo

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Tim Lewis]

Jaben --

In which cases would you have the Shell protocol present and not have the 
Unicode Collation protocol?

By my count, the Shell itself cannot function without it (see 
ProcessCommandLine()). 

Tim


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Carsey, 
Jaben
Sent: Wednesday, October 05, 2016 1:35 PM
To: Shah, Tapan ; Laszlo Ersek ; Andrew 
Fish ; Daniil Egranov 
Cc: Carsey, Jaben ; edk2-devel-01 
; Leif Lindholm 
Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

That should work.  We also need to initialize the variable to NULL in the 
constructor.

Do we need to be case insensitive for this?  Can we use memcmp instead of the 
prototol?

-Jaben

> -Original Message-
> From: Shah, Tapan [mailto:tapands...@hpe.com]
> Sent: Wednesday, October 05, 2016 1:25 PM
> To: Laszlo Ersek ; Andrew Fish ; 
> Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> Importance: High
> 
> How about moving protocol locate from constructor to the actual 
> function level and returning an error if it fails to locate (See attached 
> patch file).
> 
> Tapan
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, October 05, 2016 3:18 PM
> To: Andrew Fish ; Daniil Egranov 
> 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm ; Shah, Tapan 
> 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> 
> On 10/05/16 21:48, Andrew Fish wrote:
> >
> >> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
> >> 
> wrote:
> >>
> >> I have the same ASSERT issue on Juno platform even the 
> >> EnglishDxe.inf is
> included to the platform build. If UefiShellLib has such dependency on 
> the protocol then according to EDKII Module Writer's Guide you need to 
> specify the dependency on protocol in the module .inf to ensure the 
> drivers proper load sequence.
> >>
> >> 8.6 Dependency Expressions
> >> A dependency expression specifies the protocols that the DXE driver 
> >> requires to execute. In EDK II, it is specified in the [Depex] 
> >> section of INF
> file.
> >>
> >
> > The Dependency Expression is for DXE Drivers that are dispatched by 
> > the
> DXE Core. A UEFI Driver from an option ROM or an Application does not 
> get dispatched by the dispatch and the Depex will not help. The Depex 
> ends up being a section in the FV and it has nothing to do with the 
> PE/COFF image of the an application or option ROM.
> >
> > IMHO the shell should try as hard as possible to function and should 
> > not
> ASSERT if some newer Protocol is missing.
> 
> (Background: commit 583448b441650.)
> 
> In this specific case, the protocol dependency seems possible to eliminate:
> 
> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
> includes the CharToUpper() function, which is minimal -- could even be 
> copied or moved over -- and can translate the ASCII subset of UCS-2,
> 
> - once the ASCII characters of the input string have been translated 
> to upper case, the result could be searched for / compared against L"NULL"
> using BaseLib.h APIs.
> 
> (Given that L"NULL" only contains characters from the ASCII subset, it 
> suffices to upper-case ASCII chars in the input string. No non-ASCII 
> character in the BMP can translate to ASCII N/U/L via upcasing, even 
> with the real collation protocol (I trust), and no other ASCII 
> characters than n/u/l will translate to N/U/L. This means that we 
> won't miss an instance of NULL because we didn't upcate it (because it 
> was non-ascii), and it also means we won't mistakenly match something 
> non-NULL as NULL.)
> 
> Just my two cents.
> 
> 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] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 22:24, Shah, Tapan wrote:
> How about moving protocol locate from constructor to the actual function 
> level and returning an error if it fails to locate (See attached patch file).

What function is that precisely? The hunk headers in the patch don't show.

If it is a function that is part of some file operation, then, although
the patch is an improvement because the shell won't ASSERT(), it's still
not perfect, because lack of the protocol will break that file operation
completely.

... The containing function seems to be ShellOpenFileByName(). Seems
like a quite central function to render inoperable if the collation
protocol is not present.

Actually, I'm quite confused about conformance to specific versions of
the UEFI spec. In UEFI v2.6, section "2.6.2 Platform-Specific Elements",
point 5, we have

If a platform includes the ability to boot from a disk device, then
the EFI_BLOCK_IO_PROTOCOL, the EFI_DISK_IO_PROTOCOL, the
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL, and the
EFI_UNICODE_COLLATION_PROTOCOL are required.

In fact this requirement goes back to UEFI v2.3.1c at least. (I didn't
bother to check earlier releases of the spec.) By depending on the
protocol in the shell, we say that the current upstream shell is not
required to open files by name on pre-v2.3.1c systems (or perhaps on
systems older than v2.3.1c), and on systems that cannot boot from a disk
device.

Is this something we can say? How is this kind of compatibility tracked
in edk2?

Thanks
Laszlo

> 
> Tapan
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Wednesday, October 05, 2016 3:18 PM
> To: Andrew Fish ; Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01 
> ; Leif Lindholm ; Shah, Tapan 
> 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on 
> the Reference Platform
> 
> On 10/05/16 21:48, Andrew Fish wrote:
>> > 
>>> >> On Oct 5, 2016, at 12:24 PM, Daniil Egranov  
>>> >> wrote:
>>> >>
>>> >> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
>>> >> included to the platform build. If UefiShellLib has such dependency on 
>>> >> the protocol then according to EDKII Module Writer's Guide you need to 
>>> >> specify the dependency on protocol in the module .inf to ensure the 
>>> >> drivers proper load sequence.
>>> >>
>>> >> 8.6 Dependency Expressions
>>> >> A dependency expression specifies the protocols that the DXE driver 
>>> >> requires to execute. In EDK II, it is specified in the [Depex] section 
>>> >> of INF file.
>>> >>
>> > 
>> > The Dependency Expression is for DXE Drivers that are dispatched by the 
>> > DXE Core. A UEFI Driver from an option ROM or an Application does not get 
>> > dispatched by the dispatch and the Depex will not help. The Depex ends up 
>> > being a section in the FV and it has nothing to do with the PE/COFF image 
>> > of the an application or option ROM. 
>> > 
>> > IMHO the shell should try as hard as possible to function and should not 
>> > ASSERT if some newer Protocol is missing.  
> (Background: commit 583448b441650.)
> 
> In this specific case, the protocol dependency seems possible to eliminate:
> 
> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
> includes the CharToUpper() function, which is minimal -- could even be copied 
> or moved over -- and can translate the ASCII subset of UCS-2,
> 
> - once the ASCII characters of the input string have been translated to upper 
> case, the result could be searched for / compared against L"NULL"
> using BaseLib.h APIs.
> 
> (Given that L"NULL" only contains characters from the ASCII subset, it 
> suffices to upper-case ASCII chars in the input string. No non-ASCII 
> character in the BMP can translate to ASCII N/U/L via upcasing, even with the 
> real collation protocol (I trust), and no other ASCII characters than n/u/l 
> will translate to N/U/L. This means that we won't miss an instance of NULL 
> because we didn't upcate it (because it was non-ascii), and it also means we 
> won't mistakenly match something non-NULL as NULL.)
> 
> Just my two cents.
> 
> Laszlo
> 
> 
> uefishelllib_fix.diff
> 
> 
> Index: ShellPkg/Library/UefiShellLib/UefiShellLib.c
> ===
> --- ShellPkg/Library/UefiShellLib/UefiShellLib.c  (revision 22692)
> +++ ShellPkg/Library/UefiShellLib/UefiShellLib.c  (working copy)
> @@ -292,8 +292,6 @@
>IN EFI_SYSTEM_TABLE  *SystemTable
>)
>  {
> -  EFI_STATUS  Status;
> -
>mEfiShellEnvironment2   = NULL;
>gEfiShellProtocol   = NULL;
>gEfiShellParametersProtocol = NULL;
> @@ -300,11 +298,6 @@
>mEfiShellInterface  = NULL;
>mEfiShellEnvironment2Handle = NULL;
>  
> -  if (mUnicodeCollationProtocol == NULL) {
> -Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, 
> (VOID**)&mUnicodeCollationProtocol);
> -ASSERT_EFI_ERROR (Status);
> -  }
> -
>//
>// verify th

[edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor

[Tapan Shah]

Move gEfiUnicodeCollation2ProtocolGuid protocol outside of UefiShellLib 
constructor
function.
Locate gEfiUnicodeCollation2ProtocolGuid protocol in ShellOpenFileByName() which
consumes this protocol API.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tapan Shah 
---
 ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
index e47d535..53f54e1 100644
--- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
+++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
@@ -292,18 +292,12 @@ ShellLibConstructor (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_STATUS  Status;
-
   mEfiShellEnvironment2   = NULL;
   gEfiShellProtocol   = NULL;
   gEfiShellParametersProtocol = NULL;
   mEfiShellInterface  = NULL;
   mEfiShellEnvironment2Handle = NULL;
-
-  if (mUnicodeCollationProtocol == NULL) {
-Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, 
(VOID**)&mUnicodeCollationProtocol);
-ASSERT_EFI_ERROR (Status);
-  }
+  mUnicodeCollationProtocol   = NULL;
 
   //
   // verify that auto initialize is not set false
@@ -730,6 +724,14 @@ ShellOpenFileByName(
FileHandle,
OpenMode);
 
+if (mUnicodeCollationProtocol == NULL) {
+  Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL, 
(VOID**)&mUnicodeCollationProtocol);
+  if (EFI_ERROR (Status)) {
+gEfiShellProtocol->CloseFile (*FileHandle);
+return Status;
+  }
+}
+
 if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, 
(CHAR16*)FileName, L"NUL") != 0) &&
 (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, 
(CHAR16*)FileName, L"NULL") != 0) &&
  !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){
-- 
1.9.5.msysgit.0

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 22:44, Tim Lewis wrote:
> Jaben --
> 
> In which cases would you have the Shell protocol present and not have the 
> Unicode Collation protocol?

That's exactly the question!

According to the spec, at least if the system cannot boot from a disk
device, then the collation protocol need not be present.

> By my count, the Shell itself cannot function without it (see 
> ProcessCommandLine()). 

In which case though I don't understand why Daniil experiences this
change (= commit 583448b441650) as a regression on Juno. If the shell
couldn't function without the collation protocol even before commit
583448b441650, then the shell must never have run on Juno -- because it
appears to lack collation -- and then this change cannot be a regression.

Thanks
Laszlo


> 
> Tim
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Carsey, Jaben
> Sent: Wednesday, October 05, 2016 1:35 PM
> To: Shah, Tapan ; Laszlo Ersek ; 
> Andrew Fish ; Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01 
> ; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on 
> the Reference Platform
> 
> That should work.  We also need to initialize the variable to NULL in the 
> constructor.
> 
> Do we need to be case insensitive for this?  Can we use memcmp instead of the 
> prototol?
> 
> -Jaben
> 
>> -Original Message-
>> From: Shah, Tapan [mailto:tapands...@hpe.com]
>> Sent: Wednesday, October 05, 2016 1:25 PM
>> To: Laszlo Ersek ; Andrew Fish ; 
>> Daniil Egranov 
>> Cc: Carsey, Jaben ; edk2-devel-01 > de...@ml01.01.org>; Leif Lindholm 
>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
>> source on the Reference Platform
>> Importance: High
>>
>> How about moving protocol locate from constructor to the actual 
>> function level and returning an error if it fails to locate (See attached 
>> patch file).
>>
>> Tapan
>>
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, October 05, 2016 3:18 PM
>> To: Andrew Fish ; Daniil Egranov 
>> 
>> Cc: Carsey, Jaben ; edk2-devel-01 > de...@ml01.01.org>; Leif Lindholm ; Shah, Tapan 
>> 
>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
>> source on the Reference Platform
>>
>> On 10/05/16 21:48, Andrew Fish wrote:
>>>
 On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
 
>> wrote:

 I have the same ASSERT issue on Juno platform even the 
 EnglishDxe.inf is
>> included to the platform build. If UefiShellLib has such dependency on 
>> the protocol then according to EDKII Module Writer's Guide you need to 
>> specify the dependency on protocol in the module .inf to ensure the 
>> drivers proper load sequence.

 8.6 Dependency Expressions
 A dependency expression specifies the protocols that the DXE driver 
 requires to execute. In EDK II, it is specified in the [Depex] 
 section of INF
>> file.

>>>
>>> The Dependency Expression is for DXE Drivers that are dispatched by 
>>> the
>> DXE Core. A UEFI Driver from an option ROM or an Application does not 
>> get dispatched by the dispatch and the Depex will not help. The Depex 
>> ends up being a section in the FV and it has nothing to do with the 
>> PE/COFF image of the an application or option ROM.
>>>
>>> IMHO the shell should try as hard as possible to function and should 
>>> not
>> ASSERT if some newer Protocol is missing.
>>
>> (Background: commit 583448b441650.)
>>
>> In this specific case, the protocol dependency seems possible to eliminate:
>>
>> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
>> includes the CharToUpper() function, which is minimal -- could even be 
>> copied or moved over -- and can translate the ASCII subset of UCS-2,
>>
>> - once the ASCII characters of the input string have been translated 
>> to upper case, the result could be searched for / compared against L"NULL"
>> using BaseLib.h APIs.
>>
>> (Given that L"NULL" only contains characters from the ASCII subset, it 
>> suffices to upper-case ASCII chars in the input string. No non-ASCII 
>> character in the BMP can translate to ASCII N/U/L via upcasing, even 
>> with the real collation protocol (I trust), and no other ASCII 
>> characters than n/u/l will translate to N/U/L. This means that we 
>> won't miss an instance of NULL because we didn't upcate it (because it 
>> was non-ascii), and it also means we won't mistakenly match something 
>> non-NULL as NULL.)
>>
>> Just my two cents.
>>
>> 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
> 

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

Re: [edk2] TE relocations

[Andrew Fish]

> On Oct 5, 2016, at 12:24 PM, valerij zaporogeci  wrote:
> 
> Thank you, Andrew, I guess I understood. PE/TE images get relocated at
> FV creation time, they execute directly from FV and they are linked at
> where they really will lie in XIP memory.

Pedantically speaking PE/TE images are not linked at FV creation time they are 
relocated to the XIP address. For the most part this is the same thing that EFI 
does when it loads the PE/TE image into malloced memory. 

> And the alignment value
> chosen is not that minimum of 512 byte, mentioned in the PE
> specification. Now it's clear. The only thing I still don't get is
> that:
> Suppose you have your input PE image, which you need to translate in
> TE. It's linked at 0. It has a code section and it is lying (in the
> file) at the first available 20h aligned offset right after all the
> headers. Now you make translation into TE following the specification
> algorithm. It says you should strip away all not needed stuff from the
> beginning of the PE file, then put TE header instead (28h byte) and
> right after it you should place all the remaining PE content. But
> constructed this way TE, will break absolute referencies in the code.
> Suppose some code references some data, using its address: address =
> :0 +  + . Where
> DataSectionBase is the offset of the beginning of the section from the
> ImageBase, thus its RVA. ImageBase remains the same - 0, SectionOffset
> too, but _actual_ DataSectionBase, would not be the same for the newly
> created TE, because stripping got the data section closer to the file
> beginning. Every reference to the data would be broken. Base
> relocation at the FV creation won't help, since it changes ImageBase
> (from 0 to "ROM address"), it doesn't fix DataSectionBase mismatch. If
> after PE->TE translation, "somehow" DataSectionBase remains the same,
> then what space saving it is? :)


I don't think you are Groking the simplicity inherent in the TE design. The 
image layout of a PE/COFF or TE image is the same.

>From low address to hight address:
IMAGE_HEADER
SECTION_TABLE
.text SECTION
.data SECTION
.reloc SECTION
.debug SECTION

The 1st section after the SECTION_TABLE is going to start on a section 
alignment boundary, and each subsequent section will start on the aligned 
boundary. 

>From the PE/COFF point of view it is only the system that cares about the 
>PE/COFF header not really the code that executes. So the image does not really 
>care that it is TE or PE/COFF. I'll use P for PE/COFF header and T for TE 
>header to make my point. 

P
P
P
PT

SECTION_TABLE
---
.text SECTION
.data SECTION
.reloc SECTION
.debug SECTION

This layout is the basic reason that debuggers work even though they don't 
understand TE. When you tell a debugger the start location of a TE image you 
actually give the debugger the start of P. There is info in the TE header that 
lets you calculate the location that would have been the start of the PE/COFF 
header. Conceptually it is the space before the TE image (P but no T) that is 
being saved in the ROM by using the TE format. 

Feel free to look at the code, 
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
 

 , that decodes PE/COFF and TE.

Thanks,

Andrew Fish

PS It is a little tricky to actually look at TE images in the build output. All 
the  modules get constructed as PE/COFF. If you search the Build/ directories 
for .te files these are a TE Section in the FV, thus they start with a 4 byte 
section header. If you remove the 1st 4 bytes you get a TE Image. 


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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Andrew Fish]

> On Oct 5, 2016, at 1:58 PM, Laszlo Ersek  wrote:
> 
> On 10/05/16 22:44, Tim Lewis wrote:
>> Jaben --
>> 
>> In which cases would you have the Shell protocol present and not have the 
>> Unicode Collation protocol?
> 
> That's exactly the question!
> 
> According to the spec, at least if the system cannot boot from a disk
> device, then the collation protocol need not be present.
> 
>> By my count, the Shell itself cannot function without it (see 
>> ProcessCommandLine()). 
> 
> In which case though I don't understand why Daniil experiences this
> change (= commit 583448b441650) as a regression on Juno. If the shell
> couldn't function without the collation protocol even before commit
> 583448b441650, then the shell must never have run on Juno -- because it
> appears to lack collation -- and then this change cannot be a regression.
> 

Looks like he has a DXE Driver that consume the ShellLib. 

Thanks,

Andrew Fish

> Thanks
> Laszlo
> 
> 
>> 
>> Tim
>> 
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Carsey, Jaben
>> Sent: Wednesday, October 05, 2016 1:35 PM
>> To: Shah, Tapan ; Laszlo Ersek ; 
>> Andrew Fish ; Daniil Egranov 
>> Cc: Carsey, Jaben ; edk2-devel-01 
>> ; Leif Lindholm 
>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on 
>> the Reference Platform
>> 
>> That should work.  We also need to initialize the variable to NULL in the 
>> constructor.
>> 
>> Do we need to be case insensitive for this?  Can we use memcmp instead of 
>> the prototol?
>> 
>> -Jaben
>> 
>>> -Original Message-
>>> From: Shah, Tapan [mailto:tapands...@hpe.com]
>>> Sent: Wednesday, October 05, 2016 1:25 PM
>>> To: Laszlo Ersek ; Andrew Fish ; 
>>> Daniil Egranov 
>>> Cc: Carsey, Jaben ; edk2-devel-01 >> de...@ml01.01.org>; Leif Lindholm 
>>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
>>> source on the Reference Platform
>>> Importance: High
>>> 
>>> How about moving protocol locate from constructor to the actual 
>>> function level and returning an error if it fails to locate (See attached 
>>> patch file).
>>> 
>>> Tapan
>>> 
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, October 05, 2016 3:18 PM
>>> To: Andrew Fish ; Daniil Egranov 
>>> 
>>> Cc: Carsey, Jaben ; edk2-devel-01 >> de...@ml01.01.org>; Leif Lindholm ; Shah, Tapan 
>>> 
>>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
>>> source on the Reference Platform
>>> 
>>> On 10/05/16 21:48, Andrew Fish wrote:
 
> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
> 
>>> wrote:
> 
> I have the same ASSERT issue on Juno platform even the 
> EnglishDxe.inf is
>>> included to the platform build. If UefiShellLib has such dependency on 
>>> the protocol then according to EDKII Module Writer's Guide you need to 
>>> specify the dependency on protocol in the module .inf to ensure the 
>>> drivers proper load sequence.
> 
> 8.6 Dependency Expressions
> A dependency expression specifies the protocols that the DXE driver 
> requires to execute. In EDK II, it is specified in the [Depex] 
> section of INF
>>> file.
> 
 
 The Dependency Expression is for DXE Drivers that are dispatched by 
 the
>>> DXE Core. A UEFI Driver from an option ROM or an Application does not 
>>> get dispatched by the dispatch and the Depex will not help. The Depex 
>>> ends up being a section in the FV and it has nothing to do with the 
>>> PE/COFF image of the an application or option ROM.
 
 IMHO the shell should try as hard as possible to function and should 
 not
>>> ASSERT if some newer Protocol is missing.
>>> 
>>> (Background: commit 583448b441650.)
>>> 
>>> In this specific case, the protocol dependency seems possible to eliminate:
>>> 
>>> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
>>> includes the CharToUpper() function, which is minimal -- could even be 
>>> copied or moved over -- and can translate the ASCII subset of UCS-2,
>>> 
>>> - once the ASCII characters of the input string have been translated 
>>> to upper case, the result could be searched for / compared against L"NULL"
>>> using BaseLib.h APIs.
>>> 
>>> (Given that L"NULL" only contains characters from the ASCII subset, it 
>>> suffices to upper-case ASCII chars in the input string. No non-ASCII 
>>> character in the BMP can translate to ASCII N/U/L via upcasing, even 
>>> with the real collation protocol (I trust), and no other ASCII 
>>> characters than n/u/l will translate to N/U/L. This means that we 
>>> won't miss an instance of NULL because we didn't upcate it (because it 
>>> was non-ascii), and it also means we won't mistakenly match something 
>>> non-NULL as NULL.)
>>> 
>>> Just my two cents.
>>> 
>>> Laszlo
>> 
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.o

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Daniil Egranov]

On 10/05/2016 02:48 PM, Andrew Fish wrote:

On Oct 5, 2016, at 12:24 PM, Daniil Egranov  wrote:

I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
included to the platform build. If UefiShellLib has such dependency on the 
protocol then according to EDKII Module Writer's Guide you need to specify the 
dependency on protocol in the module .inf to ensure the drivers proper load 
sequence.

8.6 Dependency Expressions
A dependency expression specifies the protocols that the DXE driver requires to
execute. In EDK II, it is specified in the [Depex] section of INF file.


The Dependency Expression is for DXE Drivers that are dispatched by the DXE 
Core. A UEFI Driver from an option ROM or an Application does not get 
dispatched by the dispatch and the Depex will not help. The Depex ends up being 
a section in the FV and it has nothing to do with the PE/COFF image of the an 
application or option ROM.

IMHO the shell should try as hard as possible to function and should not ASSERT 
if some newer Protocol is missing.

Thanks,

Andre wFish
I had to put a context of the ASSERT message in the original message to 
make it more clear:


add-symbol-file 
/home/user1/workspace/juno_16.09/uefi/edk2/Build/ArmJuno/DEBUG_GCC49/AARCH64/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe/DEBUG/ArmJunoDxe.dll 
0xF8AB9000

Loading driver at 0x000F8AB8000 EntryPoint=0x000F8AB9048 ArmJunoDxe.efi

ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [ArmJunoDxe] 
/home/danegr01/workspace/juno_16.09/uefi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c(305): 
!EFI_ERROR (Status)


If driver includes a module which has dependency on one of the 
protocols, should the driver know about this dependency? Probably not. 
It should be inherited from the module. Adding [Depex] to UefiShellLib 
corrected dispatching ArmJunoDxe and EnglishDxe by the Dxe Core.






The following dependency in UefiShellLib.inf fixes ASSERT problem:

[Depex]
  gEfiUnicodeCollation2ProtocolGuid


Thanks,

Daniil


On 10/05/2016 11:02 AM, Shah, Tapan wrote:

It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is 
necessary for even other Shell libraries other than UefiShellLib in order to 
have Shell parser and other command line processing to work properly. That's 
why I added ASSERT if UefiShellLib fails to locate the protocol.
  Reference platform should have EnglishDxe module to have this protocol 
installed properly.

   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com]
Sent: Wednesday, October 05, 2016 10:41 AM
To: Supreeth Venkatesh ; edk2-devel-01 
; Shah, Tapan 
Cc: Leif Lindholm ; Carsey, Jaben 

Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

Tapan,

Could this be a side effect of your patch?  Should we allow the UefiShellLib to 
continue without this protocol and then return an error only when the OpenFile 
is called?

Also, it looks like we never properly initialize mUnicodeCollationProtocol.  We 
check for NULL in Constructor, but nothing else...

-Jaben


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Supreeth Venkatesh
Sent: Tuesday, October 04, 2016 3:51 PM
To: edk2-devel-01 
Cc: Leif Lindholm 
Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source
on the Reference Platform
Importance: High

All,

Recently, I updated to latest edk2 master (yesterday's) and I am
actually encountering assert in
ShellPkg/Library/UefiShellLib/UefiShellLib.c

if (mUnicodeCollationProtocol == NULL) {
 Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
NULL, (VOID**)&mUnicodeCollationProtocol);
 ASSERT_EFI_ERROR (Status);
   }

It was working few weeks back and has now stopped working.
It's probably because  the platform is unable to locate this protocol
"UnicodeCollationProtocol".
Is Anyone else facing the same issue?
Does the new ShellPkg requires additional packages/infs to be included
in the platform description file to work with latest changes  to get
past this error?

Thanks,
Supreeth
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended
recipient, please notify the sender immediately and do not disclose
the contents to any other person, use it for any purpose, or store or
copy the information in any medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

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

___
edk2-devel maili

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 22:53, Daniil Egranov wrote:
> 
> 
> On 10/05/2016 02:48 PM, Andrew Fish wrote:
>>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
>>> wrote:
>>>
>>> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf
>>> is included to the platform build. If UefiShellLib has such
>>> dependency on the protocol then according to EDKII Module Writer's
>>> Guide you need to specify the dependency on protocol in the module
>>> .inf to ensure the drivers proper load sequence.
>>>
>>> 8.6 Dependency Expressions
>>> A dependency expression specifies the protocols that the DXE driver
>>> requires to
>>> execute. In EDK II, it is specified in the [Depex] section of INF file.
>>>
>> The Dependency Expression is for DXE Drivers that are dispatched by
>> the DXE Core. A UEFI Driver from an option ROM or an Application does
>> not get dispatched by the dispatch and the Depex will not help. The
>> Depex ends up being a section in the FV and it has nothing to do with
>> the PE/COFF image of the an application or option ROM.
>>
>> IMHO the shell should try as hard as possible to function and should
>> not ASSERT if some newer Protocol is missing.
>>
>> Thanks,
>>
>> Andre wFish
> I had to put a context of the ASSERT message in the original message to
> make it more clear:
> 
> add-symbol-file
> /home/user1/workspace/juno_16.09/uefi/edk2/Build/ArmJuno/DEBUG_GCC49/AARCH64/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe/DEBUG/ArmJunoDxe.dll
> 0xF8AB9000
> Loading driver at 0x000F8AB8000 EntryPoint=0x000F8AB9048 ArmJunoDxe.efi
> 
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe]
> /home/danegr01/workspace/juno_16.09/uefi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c(305):
> !EFI_ERROR (Status)
> 
> If driver includes a module which has dependency on one of the
> protocols, should the driver know about this dependency? Probably not.
> It should be inherited from the module. Adding [Depex] to UefiShellLib
> corrected dispatching ArmJunoDxe and EnglishDxe by the Dxe Core.

You are right; Tim confirmed UefiShellLib is valid to use in
DXE_RUNTIME_DRIVER and DXE_DRIVER modules, and ArmJunoDxe is a DXE_DRIVER.

This additional information also explains why you see the change as a
regression, but have had a working shell all this time -- due to the
missing depex, ArmJunoDxe got dispatched earlier and experienced the
regression, but the shell app itself (being a UEFI application) found
the collation protocol (provided by EnglishDxe) just fine.

So, your suggestion about the depex solves the problem for drivers, on
platforms that provide the collation protocol, but it doesn't solve the
(separate) problem of the shell app proper, on platforms that don't
provide the collation protocol at all.

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Tim Lewis]

Is it possible that Daniil has UnicodeCollation and not UnicodeCollation2? The 
shell itself can handle this case, but the ShellLib cannot.

Tim

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, October 05, 2016 1:58 PM
To: Tim Lewis ; Carsey, Jaben ; 
Shah, Tapan ; Andrew Fish ; Daniil Egranov 

Cc: edk2-devel-01 ; Leif Lindholm 

Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

On 10/05/16 22:44, Tim Lewis wrote:
> Jaben --
> 
> In which cases would you have the Shell protocol present and not have the 
> Unicode Collation protocol?

That's exactly the question!

According to the spec, at least if the system cannot boot from a disk device, 
then the collation protocol need not be present.

> By my count, the Shell itself cannot function without it (see 
> ProcessCommandLine()). 

In which case though I don't understand why Daniil experiences this change (= 
commit 583448b441650) as a regression on Juno. If the shell couldn't function 
without the collation protocol even before commit 583448b441650, then the shell 
must never have run on Juno -- because it appears to lack collation -- and then 
this change cannot be a regression.

Thanks
Laszlo


> 
> Tim
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Carsey, Jaben
> Sent: Wednesday, October 05, 2016 1:35 PM
> To: Shah, Tapan ; Laszlo Ersek 
> ; Andrew Fish ; Daniil Egranov 
> 
> Cc: Carsey, Jaben ; edk2-devel-01 
> ; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> 
> That should work.  We also need to initialize the variable to NULL in the 
> constructor.
> 
> Do we need to be case insensitive for this?  Can we use memcmp instead of the 
> prototol?
> 
> -Jaben
> 
>> -Original Message-
>> From: Shah, Tapan [mailto:tapands...@hpe.com]
>> Sent: Wednesday, October 05, 2016 1:25 PM
>> To: Laszlo Ersek ; Andrew Fish ; 
>> Daniil Egranov 
>> Cc: Carsey, Jaben ; edk2-devel-01 > de...@ml01.01.org>; Leif Lindholm 
>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
>> source on the Reference Platform
>> Importance: High
>>
>> How about moving protocol locate from constructor to the actual 
>> function level and returning an error if it fails to locate (See attached 
>> patch file).
>>
>> Tapan
>>
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Wednesday, October 05, 2016 3:18 PM
>> To: Andrew Fish ; Daniil Egranov 
>> 
>> Cc: Carsey, Jaben ; edk2-devel-01 > de...@ml01.01.org>; Leif Lindholm ; Shah, 
>> Tapan 
>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
>> source on the Reference Platform
>>
>> On 10/05/16 21:48, Andrew Fish wrote:
>>>
 On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
 
>> wrote:

 I have the same ASSERT issue on Juno platform even the 
 EnglishDxe.inf is
>> included to the platform build. If UefiShellLib has such dependency 
>> on the protocol then according to EDKII Module Writer's Guide you 
>> need to specify the dependency on protocol in the module .inf to 
>> ensure the drivers proper load sequence.

 8.6 Dependency Expressions
 A dependency expression specifies the protocols that the DXE driver 
 requires to execute. In EDK II, it is specified in the [Depex] 
 section of INF
>> file.

>>>
>>> The Dependency Expression is for DXE Drivers that are dispatched by 
>>> the
>> DXE Core. A UEFI Driver from an option ROM or an Application does not 
>> get dispatched by the dispatch and the Depex will not help. The Depex 
>> ends up being a section in the FV and it has nothing to do with the 
>> PE/COFF image of the an application or option ROM.
>>>
>>> IMHO the shell should try as hard as possible to function and should 
>>> not
>> ASSERT if some newer Protocol is missing.
>>
>> (Background: commit 583448b441650.)
>>
>> In this specific case, the protocol dependency seems possible to eliminate:
>>
>> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
>> includes the CharToUpper() function, which is minimal -- could even 
>> be copied or moved over -- and can translate the ASCII subset of 
>> UCS-2,
>>
>> - once the ASCII characters of the input string have been translated 
>> to upper case, the result could be searched for / compared against L"NULL"
>> using BaseLib.h APIs.
>>
>> (Given that L"NULL" only contains characters from the ASCII subset, 
>> it suffices to upper-case ASCII chars in the input string. No 
>> non-ASCII character in the BMP can translate to ASCII N/U/L via 
>> upcasing, even with the real collation protocol (I trust), and no 
>> other ASCII characters than n/u/l will translate to N/U/L. This means 
>> that we won't miss an instance of NULL because we didn't upcate it 
>> (because it was non-ascii), and it also means we won't mistakenly 
>> match something n

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Andrew Fish]

> On Oct 5, 2016, at 1:53 PM, Daniil Egranov  wrote:
> 
> 
> 
> On 10/05/2016 02:48 PM, Andrew Fish wrote:
>>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov  wrote:
>>> 
>>> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf is 
>>> included to the platform build. If UefiShellLib has such dependency on the 
>>> protocol then according to EDKII Module Writer's Guide you need to specify 
>>> the dependency on protocol in the module .inf to ensure the drivers proper 
>>> load sequence.
>>> 
>>> 8.6 Dependency Expressions
>>> A dependency expression specifies the protocols that the DXE driver 
>>> requires to
>>> execute. In EDK II, it is specified in the [Depex] section of INF file.
>>> 
>> The Dependency Expression is for DXE Drivers that are dispatched by the DXE 
>> Core. A UEFI Driver from an option ROM or an Application does not get 
>> dispatched by the dispatch and the Depex will not help. The Depex ends up 
>> being a section in the FV and it has nothing to do with the PE/COFF image of 
>> the an application or option ROM.
>> 
>> IMHO the shell should try as hard as possible to function and should not 
>> ASSERT if some newer Protocol is missing.
>> 
>> Thanks,
>> 
>> Andre wFish
> I had to put a context of the ASSERT message in the original message to make 
> it more clear:
> 
> add-symbol-file 
> /home/user1/workspace/juno_16.09/uefi/edk2/Build/ArmJuno/DEBUG_GCC49/AARCH64/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe/DEBUG/ArmJunoDxe.dll
>  0xF8AB9000
> Loading driver at 0x000F8AB8000 EntryPoint=0x000F8AB9048 ArmJunoDxe.efi
> 
> ASSERT_EFI_ERROR (Status = Not Found)
> ASSERT [ArmJunoDxe] 
> /home/danegr01/workspace/juno_16.09/uefi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c(305):
>  !EFI_ERROR (Status)
> 
> If driver includes a module which has dependency on one of the protocols, 
> should the driver know about this dependency? Probably not. It should be 
> inherited from the module. Adding [Depex] to UefiShellLib corrected 
> dispatching ArmJunoDxe and EnglishDxe by the Dxe Core.
> 

Daniil,

Sorry, I was assuming the UefiShellLib only supported UEFI Applications. Thats 
what I get for not looking at the code :(, my bad. 

https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellLib/UefiShellLib.inf#L23
 

  LIBRARY_CLASS  = ShellLib|UEFI_APPLICATION UEFI_DRIVER 
DXE_RUNTIME_DRIVER DXE_DRIVER

Given the library supports DXE_RUNTIME_DRIVER and DXE_DRIVER I think you are 
right it should publish a Depex if it has a hard dependency. 

I'm guessing that your DXE Driver is dispatching prior UEFI Driver that 
publishes the protocol. 

Thanks,

Andrew Fish


>> 
>> 
>>> The following dependency in UefiShellLib.inf fixes ASSERT problem:
>>> 
>>> [Depex]
>>>  gEfiUnicodeCollation2ProtocolGuid
>>> 
>>> 
>>> Thanks,
>>> 
>>> Daniil
>>> 
>>> 
>>> On 10/05/2016 11:02 AM, Shah, Tapan wrote:
 It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is 
 necessary for even other Shell libraries other than UefiShellLib in order 
 to have Shell parser and other command line processing to work properly. 
 That's why I added ASSERT if UefiShellLib fails to locate the protocol.
  Reference platform should have EnglishDxe module to have this protocol 
 installed properly.
 
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
 
 -Original Message-
 From: Carsey, Jaben [mailto:jaben.car...@intel.com]
 Sent: Wednesday, October 05, 2016 10:41 AM
 To: Supreeth Venkatesh ; edk2-devel-01 
 ; Shah, Tapan 
 Cc: Leif Lindholm ; Carsey, Jaben 
 
 Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the 
 Reference Platform
 
 Tapan,
 
 Could this be a side effect of your patch?  Should we allow the 
 UefiShellLib to continue without this protocol and then return an error 
 only when the OpenFile is called?
 
 Also, it looks like we never properly initialize 
 mUnicodeCollationProtocol.  We check for NULL in Constructor, but nothing 
 else...
 
 -Jaben
 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Supreeth Venkatesh
> Sent: Tuesday, October 04, 2016 3:51 PM
> To: edk2-devel-01 
> Cc: Leif Lindholm 
> Subject: [edk2] Assert in ShellPkg with latest tianocore edk2 source
> on the Reference Platform
> Importance: High
> 
> All,
> 
> Recently, I updated to latest edk2 master (yesterday's) and I am
> actually encountering assert in
> ShellPkg/Library/UefiShellLib/UefiShellLib.c
> 
> if (mUnicodeCollationProtocol == NULL) {
> Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
> NULL, (VOID**)&mUnicodeCollationProtocol);
> ASSERT_EFI_ERROR 

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 22:59, Andrew Fish wrote:
> 
>> On Oct 5, 2016, at 1:58 PM, Laszlo Ersek  wrote:
>>
>> On 10/05/16 22:44, Tim Lewis wrote:
>>> Jaben --
>>>
>>> In which cases would you have the Shell protocol present and not have the 
>>> Unicode Collation protocol?
>>
>> That's exactly the question!
>>
>> According to the spec, at least if the system cannot boot from a disk
>> device, then the collation protocol need not be present.
>>
>>> By my count, the Shell itself cannot function without it (see 
>>> ProcessCommandLine()). 
>>
>> In which case though I don't understand why Daniil experiences this
>> change (= commit 583448b441650) as a regression on Juno. If the shell
>> couldn't function without the collation protocol even before commit
>> 583448b441650, then the shell must never have run on Juno -- because it
>> appears to lack collation -- and then this change cannot be a regression.
>>
> 
> Looks like he has a DXE Driver that consume the ShellLib. 

That's it, thank you. Daniil named ArmJunoDxe.efi in his second (?) email.

Laszlo

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Tim Lewis]

If he has a DXE driver that consumes the shell lib (a valid case), then my 
original suggestion stands: move it to ShellLibConstructorWorker. This is where 
the ShellLib's internal pointers to the Shell protocol are initialized.

Tim

-Original Message-
From: af...@apple.com [mailto:af...@apple.com] 
Sent: Wednesday, October 05, 2016 1:59 PM
To: Laszlo Ersek 
Cc: Tim Lewis ; Carsey, Jaben ; 
Shah, Tapan ; Daniil Egranov ; 
edk2-devel-01 ; Leif Lindholm 
Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform


> On Oct 5, 2016, at 1:58 PM, Laszlo Ersek  wrote:
> 
> On 10/05/16 22:44, Tim Lewis wrote:
>> Jaben --
>> 
>> In which cases would you have the Shell protocol present and not have the 
>> Unicode Collation protocol?
> 
> That's exactly the question!
> 
> According to the spec, at least if the system cannot boot from a disk 
> device, then the collation protocol need not be present.
> 
>> By my count, the Shell itself cannot function without it (see 
>> ProcessCommandLine()). 
> 
> In which case though I don't understand why Daniil experiences this 
> change (= commit 583448b441650) as a regression on Juno. If the shell 
> couldn't function without the collation protocol even before commit 
> 583448b441650, then the shell must never have run on Juno -- because 
> it appears to lack collation -- and then this change cannot be a regression.
> 

Looks like he has a DXE Driver that consume the ShellLib. 

Thanks,

Andrew Fish

> Thanks
> Laszlo
> 
> 
>> 
>> Tim
>> 
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> Of Carsey, Jaben
>> Sent: Wednesday, October 05, 2016 1:35 PM
>> To: Shah, Tapan ; Laszlo Ersek 
>> ; Andrew Fish ; Daniil Egranov 
>> 
>> Cc: Carsey, Jaben ; edk2-devel-01 
>> ; Leif Lindholm 
>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
>> source on the Reference Platform
>> 
>> That should work.  We also need to initialize the variable to NULL in the 
>> constructor.
>> 
>> Do we need to be case insensitive for this?  Can we use memcmp instead of 
>> the prototol?
>> 
>> -Jaben
>> 
>>> -Original Message-
>>> From: Shah, Tapan [mailto:tapands...@hpe.com]
>>> Sent: Wednesday, October 05, 2016 1:25 PM
>>> To: Laszlo Ersek ; Andrew Fish ; 
>>> Daniil Egranov 
>>> Cc: Carsey, Jaben ; edk2-devel-01 >> de...@ml01.01.org>; Leif Lindholm 
>>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
>>> source on the Reference Platform
>>> Importance: High
>>> 
>>> How about moving protocol locate from constructor to the actual 
>>> function level and returning an error if it fails to locate (See attached 
>>> patch file).
>>> 
>>> Tapan
>>> 
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, October 05, 2016 3:18 PM
>>> To: Andrew Fish ; Daniil Egranov 
>>> 
>>> Cc: Carsey, Jaben ; edk2-devel-01 >> de...@ml01.01.org>; Leif Lindholm ; Shah, 
>>> Tapan 
>>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
>>> source on the Reference Platform
>>> 
>>> On 10/05/16 21:48, Andrew Fish wrote:
 
> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
> 
>>> wrote:
> 
> I have the same ASSERT issue on Juno platform even the 
> EnglishDxe.inf is
>>> included to the platform build. If UefiShellLib has such dependency 
>>> on the protocol then according to EDKII Module Writer's Guide you 
>>> need to specify the dependency on protocol in the module .inf to 
>>> ensure the drivers proper load sequence.
> 
> 8.6 Dependency Expressions
> A dependency expression specifies the protocols that the DXE 
> driver requires to execute. In EDK II, it is specified in the 
> [Depex] section of INF
>>> file.
> 
 
 The Dependency Expression is for DXE Drivers that are dispatched by 
 the
>>> DXE Core. A UEFI Driver from an option ROM or an Application does 
>>> not get dispatched by the dispatch and the Depex will not help. The 
>>> Depex ends up being a section in the FV and it has nothing to do 
>>> with the PE/COFF image of the an application or option ROM.
 
 IMHO the shell should try as hard as possible to function and 
 should not
>>> ASSERT if some newer Protocol is missing.
>>> 
>>> (Background: commit 583448b441650.)
>>> 
>>> In this specific case, the protocol dependency seems possible to eliminate:
>>> 
>>> - Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.c
>>> includes the CharToUpper() function, which is minimal -- could even 
>>> be copied or moved over -- and can translate the ASCII subset of 
>>> UCS-2,
>>> 
>>> - once the ASCII characters of the input string have been translated 
>>> to upper case, the result could be searched for / compared against L"NULL"
>>> using BaseLib.h APIs.
>>> 
>>> (Given that L"NULL" only contains characters from the ASCII subset, 
>>> it suffices to upper-case ASCII chars in the input st

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Carsey, Jaben]

All,
What are your thoughts about Tapan's suggestion to move the protocol location 
to the only function that needs it?

Andrew,

We cannot publish a dependency.  There are DXE driver dynamic shell commands 
that launch before the shell runs and produce the appropriate protocol so that 
the shell will call into them.  These drivers must be allowed to successfully 
launch before the shell runs.

Danil,

Can you elaborate what your DXE driver is doing with the library?


-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Wednesday, October 05, 2016 2:06 PM
> To: Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@lists.01.org>; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> Importance: High
> 
> 
> > On Oct 5, 2016, at 1:53 PM, Daniil Egranov 
> wrote:
> >
> >
> >
> > On 10/05/2016 02:48 PM, Andrew Fish wrote:
> >>> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
> wrote:
> >>>
> >>> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf
> is included to the platform build. If UefiShellLib has such dependency on the
> protocol then according to EDKII Module Writer's Guide you need to specify
> the dependency on protocol in the module .inf to ensure the drivers proper
> load sequence.
> >>>
> >>> 8.6 Dependency Expressions
> >>> A dependency expression specifies the protocols that the DXE driver
> requires to
> >>> execute. In EDK II, it is specified in the [Depex] section of INF file.
> >>>
> >> The Dependency Expression is for DXE Drivers that are dispatched by the
> DXE Core. A UEFI Driver from an option ROM or an Application does not get
> dispatched by the dispatch and the Depex will not help. The Depex ends up
> being a section in the FV and it has nothing to do with the PE/COFF image of
> the an application or option ROM.
> >>
> >> IMHO the shell should try as hard as possible to function and should not
> ASSERT if some newer Protocol is missing.
> >>
> >> Thanks,
> >>
> >> Andre wFish
> > I had to put a context of the ASSERT message in the original message to
> make it more clear:
> >
> > add-symbol-file
> /home/user1/workspace/juno_16.09/uefi/edk2/Build/ArmJuno/DEBUG_GC
> C49/AARCH64/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJuno
> Dxe/DEBUG/ArmJunoDxe.dll 0xF8AB9000
> > Loading driver at 0x000F8AB8000 EntryPoint=0x000F8AB9048
> ArmJunoDxe.efi
> >
> > ASSERT_EFI_ERROR (Status = Not Found)
> > ASSERT [ArmJunoDxe]
> /home/danegr01/workspace/juno_16.09/uefi/edk2/ShellPkg/Library/UefiSh
> ellLib/UefiShellLib.c(305): !EFI_ERROR (Status)
> >
> > If driver includes a module which has dependency on one of the protocols,
> should the driver know about this dependency? Probably not. It should be
> inherited from the module. Adding [Depex] to UefiShellLib corrected
> dispatching ArmJunoDxe and EnglishDxe by the Dxe Core.
> >
> 
> Daniil,
> 
> Sorry, I was assuming the UefiShellLib only supported UEFI Applications.
> Thats what I get for not looking at the code :(, my bad.
> 
> https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellL
> ib/UefiShellLib.inf#L23
>  lLib/UefiShellLib.inf#L23>
>   LIBRARY_CLASS  = ShellLib|UEFI_APPLICATION UEFI_DRIVER
> DXE_RUNTIME_DRIVER DXE_DRIVER
> 
> Given the library supports DXE_RUNTIME_DRIVER and DXE_DRIVER I think
> you are right it should publish a Depex if it has a hard dependency.
> 
> I'm guessing that your DXE Driver is dispatching prior UEFI Driver that
> publishes the protocol.
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> >>
> >>
> >>> The following dependency in UefiShellLib.inf fixes ASSERT problem:
> >>>
> >>> [Depex]
> >>>  gEfiUnicodeCollation2ProtocolGuid
> >>>
> >>>
> >>> Thanks,
> >>>
> >>> Daniil
> >>>
> >>>
> >>> On 10/05/2016 11:02 AM, Shah, Tapan wrote:
>  It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is
> necessary for even other Shell libraries other than UefiShellLib in order to
> have Shell parser and other command line processing to work properly.
> That's why I added ASSERT if UefiShellLib fails to locate the protocol.
>   Reference platform should have EnglishDxe module to have this
> protocol installed properly.
> 
> 
> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> 
>  -Original Message-
>  From: Carsey, Jaben [mailto:jaben.car...@intel.com]
>  Sent: Wednesday, October 05, 2016 10:41 AM
>  To: Supreeth Venkatesh ; edk2-
> devel-01 ; Shah, Tapan 
>  Cc: Leif Lindholm ; Carsey, Jaben
> 
>  Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the
> Reference Platform
> 
>  Tapan,
> 
>  Could this be a side effect of your patch?  Should we allow the
> UefiShellLib to continue without this protocol and then return an error only
> when the OpenFile 

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 23:05, Andrew Fish wrote:

> I'm guessing that your DXE Driver is dispatching prior UEFI Driver that 
> publishes the protocol. 

BTW... Why is EnglishDxe a UEFI_DRIVER rather than a DXE_DRIVER? It
doesn't conform to the UEFI driver model. (I'm not saying it *must* not
be a UEFI_DRIVER, but what reason does it have for being one?) It also
calls no runtime services, and only the
InstallMultipleProtocolInterfaces() boot service; so I don't think it
relies on all of the architectural protocols either.

Thanks
Laszlo

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Andrew Fish]

> On Oct 5, 2016, at 2:15 PM, Carsey, Jaben  wrote:
> 
> All,
> What are your thoughts about Tapan's suggestion to move the protocol location 
> to the only function that needs it?
> 
> Andrew,
> 
> We cannot publish a dependency.  There are DXE driver dynamic shell commands 
> that launch before the shell runs and produce the appropriate protocol so 
> that the shell will call into them.  These drivers must be allowed to 
> successfully launch before the shell runs.
> 

Jaben,

I'm not saying it was good change, but the patch added a hard dependency on 
a protocol so a depex was required for correctness. I agree deferring the work 
is a better solution. 

Thanks,

Andrew Fish


> Danil,
> 
> Can you elaborate what your DXE driver is doing with the library?
> 
> 
> -Jaben
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Andrew Fish
>> Sent: Wednesday, October 05, 2016 2:06 PM
>> To: Daniil Egranov 
>> Cc: Carsey, Jaben ; edk2-devel-01 > de...@lists.01.org>; Leif Lindholm 
>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
>> the Reference Platform
>> Importance: High
>> 
>> 
>>> On Oct 5, 2016, at 1:53 PM, Daniil Egranov 
>> wrote:
>>> 
>>> 
>>> 
>>> On 10/05/2016 02:48 PM, Andrew Fish wrote:
> On Oct 5, 2016, at 12:24 PM, Daniil Egranov 
>> wrote:
> 
> I have the same ASSERT issue on Juno platform even the EnglishDxe.inf
>> is included to the platform build. If UefiShellLib has such dependency on the
>> protocol then according to EDKII Module Writer's Guide you need to specify
>> the dependency on protocol in the module .inf to ensure the drivers proper
>> load sequence.
> 
> 8.6 Dependency Expressions
> A dependency expression specifies the protocols that the DXE driver
>> requires to
> execute. In EDK II, it is specified in the [Depex] section of INF file.
> 
 The Dependency Expression is for DXE Drivers that are dispatched by the
>> DXE Core. A UEFI Driver from an option ROM or an Application does not get
>> dispatched by the dispatch and the Depex will not help. The Depex ends up
>> being a section in the FV and it has nothing to do with the PE/COFF image of
>> the an application or option ROM.
 
 IMHO the shell should try as hard as possible to function and should not
>> ASSERT if some newer Protocol is missing.
 
 Thanks,
 
 Andre wFish
>>> I had to put a context of the ASSERT message in the original message to
>> make it more clear:
>>> 
>>> add-symbol-file
>> /home/user1/workspace/juno_16.09/uefi/edk2/Build/ArmJuno/DEBUG_GC
>> C49/AARCH64/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJuno
>> Dxe/DEBUG/ArmJunoDxe.dll 0xF8AB9000
>>> Loading driver at 0x000F8AB8000 EntryPoint=0x000F8AB9048
>> ArmJunoDxe.efi
>>> 
>>> ASSERT_EFI_ERROR (Status = Not Found)
>>> ASSERT [ArmJunoDxe]
>> /home/danegr01/workspace/juno_16.09/uefi/edk2/ShellPkg/Library/UefiSh
>> ellLib/UefiShellLib.c(305): !EFI_ERROR (Status)
>>> 
>>> If driver includes a module which has dependency on one of the protocols,
>> should the driver know about this dependency? Probably not. It should be
>> inherited from the module. Adding [Depex] to UefiShellLib corrected
>> dispatching ArmJunoDxe and EnglishDxe by the Dxe Core.
>>> 
>> 
>> Daniil,
>> 
>> Sorry, I was assuming the UefiShellLib only supported UEFI Applications.
>> Thats what I get for not looking at the code :(, my bad.
>> 
>> https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellL
>> ib/UefiShellLib.inf#L23
>> > lLib/UefiShellLib.inf#L23>
>>  LIBRARY_CLASS  = ShellLib|UEFI_APPLICATION UEFI_DRIVER
>> DXE_RUNTIME_DRIVER DXE_DRIVER
>> 
>> Given the library supports DXE_RUNTIME_DRIVER and DXE_DRIVER I think
>> you are right it should publish a Depex if it has a hard dependency.
>> 
>> I'm guessing that your DXE Driver is dispatching prior UEFI Driver that
>> publishes the protocol.
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>> 
 
 
> The following dependency in UefiShellLib.inf fixes ASSERT problem:
> 
> [Depex]
> gEfiUnicodeCollation2ProtocolGuid
> 
> 
> Thanks,
> 
> Daniil
> 
> 
> On 10/05/2016 11:02 AM, Shah, Tapan wrote:
>> It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is
>> necessary for even other Shell libraries other than UefiShellLib in order to
>> have Shell parser and other command line processing to work properly.
>> That's why I added ASSERT if UefiShellLib fails to locate the protocol.
>> Reference platform should have EnglishDxe module to have this
>> protocol installed properly.
>> 
>> 
>> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>> 
>> -Original Message-
>> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
>> Sent: Wednesday, October 05, 2016 10:41 AM
>> To: Sup

[edk2] Urgent help -UefiBootManagerLib and LegacyBootManagerLib issue

[Saqib Khan]

Hi all,i need urgent help regarding this issue.

> On 05-Oct-2016, at 9:05 PM, Saqib Khan  wrote:
> 
> 
> I have found that  it just dont return from  mBmRefreshLegacyBootOption (); .
> 
> have a look at code. let me know the possible cause of it ...
> I need urgent help
> 
> EfiBootManagerRefreshAllBootOption (
>   VOID
>   )
> {
>   EFI_STATUSStatus;
>   EFI_BOOT_MANAGER_LOAD_OPTION  *NvBootOptions;
>   UINTN NvBootOptionCount;
>   EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
>   UINTN BootOptionCount;
>   UINTN Index;
>   Print(L"indside refresh\n");
>   //
>   // Optionally refresh the legacy boot option
>   //
>   if (mBmRefreshLegacyBootOption != NULL) {
>   Print(L"Before legacy refresh \n");
> mBmRefreshLegacyBootOption ();  //this method does not return 
> Print(L"legacy refresh complete\n");
>   }
> 
>> On Wed, Oct 5, 2016 at 5:51 PM, Saqib Khan  wrote:
>> Hi,
>> 
>> when i import both lib in my project my EFI hangs at 
>> EfiRefreshAllBootOptions, i removed LegacyBootManager and it worked fine .i 
>> need both lib as i need to boot legacy from EFI, how this issue can be 
>> resolved?
>> 
>> 
>> here is piece of inf file
>> 
>> [Packages]
>>   MdePkg/MdePkg.dec
>>   MdeModulePkg/MdeModulePkg.dec
>>   IntelFrameworkPkg/
>> IntelFrameworkPkg.dec
>>   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>>   ShellPkg/ShellPkg.dec
>> 
>> [LibraryClasses]
>>   HiiLib
>>   DebugLib
>>   UefiLib
>>   MemoryAllocationLib
>>   UefiBootServicesTableLib
>>   UefiApplicationEntryPoint
>>   UefiBootManagerLib
>>   LegacyBootManagerLib
>> 
>> 
>> -- 
>> Regards
>> Saqib Ahmed Khanzada
> 
> 
> 
> -- 
> Regards
> Saqib Ahmed Khanzada
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Carsey, Jaben]

That is not enough.  That gets called by the constructor.  We need to not check 
in either constructor or the constructor worker.

That difference only matters for the shell binary itself.

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Tim Lewis
> Sent: Wednesday, October 05, 2016 2:07 PM
> To: af...@apple.com; Laszlo Ersek 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> Importance: High
> 
> If he has a DXE driver that consumes the shell lib (a valid case), then my
> original suggestion stands: move it to ShellLibConstructorWorker. This is
> where the ShellLib's internal pointers to the Shell protocol are initialized.
> 
> Tim
> 
> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Wednesday, October 05, 2016 1:59 PM
> To: Laszlo Ersek 
> Cc: Tim Lewis ; Carsey, Jaben
> ; Shah, Tapan ; Daniil
> Egranov ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> 
> 
> > On Oct 5, 2016, at 1:58 PM, Laszlo Ersek  wrote:
> >
> > On 10/05/16 22:44, Tim Lewis wrote:
> >> Jaben --
> >>
> >> In which cases would you have the Shell protocol present and not have
> the Unicode Collation protocol?
> >
> > That's exactly the question!
> >
> > According to the spec, at least if the system cannot boot from a disk
> > device, then the collation protocol need not be present.
> >
> >> By my count, the Shell itself cannot function without it (see
> ProcessCommandLine()).
> >
> > In which case though I don't understand why Daniil experiences this
> > change (= commit 583448b441650) as a regression on Juno. If the shell
> > couldn't function without the collation protocol even before commit
> > 583448b441650, then the shell must never have run on Juno -- because
> > it appears to lack collation -- and then this change cannot be a regression.
> >
> 
> Looks like he has a DXE Driver that consume the ShellLib.
> 
> Thanks,
> 
> Andrew Fish
> 
> > Thanks
> > Laszlo
> >
> >
> >>
> >> Tim
> >>
> >>
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> >> Of Carsey, Jaben
> >> Sent: Wednesday, October 05, 2016 1:35 PM
> >> To: Shah, Tapan ; Laszlo Ersek
> >> ; Andrew Fish ; Daniil Egranov
> >> 
> >> Cc: Carsey, Jaben ; edk2-devel-01
> >> ; Leif Lindholm 
> >> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2
> >> source on the Reference Platform
> >>
> >> That should work.  We also need to initialize the variable to NULL in the
> constructor.
> >>
> >> Do we need to be case insensitive for this?  Can we use memcmp instead
> of the prototol?
> >>
> >> -Jaben
> >>
> >>> -Original Message-
> >>> From: Shah, Tapan [mailto:tapands...@hpe.com]
> >>> Sent: Wednesday, October 05, 2016 1:25 PM
> >>> To: Laszlo Ersek ; Andrew Fish
> ;
> >>> Daniil Egranov 
> >>> Cc: Carsey, Jaben ; edk2-devel-01  >>> de...@ml01.01.org>; Leif Lindholm 
> >>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2
> >>> source on the Reference Platform
> >>> Importance: High
> >>>
> >>> How about moving protocol locate from constructor to the actual
> >>> function level and returning an error if it fails to locate (See attached
> patch file).
> >>>
> >>> Tapan
> >>>
> >>> -Original Message-
> >>> From: Laszlo Ersek [mailto:ler...@redhat.com]
> >>> Sent: Wednesday, October 05, 2016 3:18 PM
> >>> To: Andrew Fish ; Daniil Egranov
> >>> 
> >>> Cc: Carsey, Jaben ; edk2-devel-01  >>> de...@ml01.01.org>; Leif Lindholm ; Shah,
> >>> Tapan 
> >>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2
> >>> source on the Reference Platform
> >>>
> >>> On 10/05/16 21:48, Andrew Fish wrote:
> 
> > On Oct 5, 2016, at 12:24 PM, Daniil Egranov
> > 
> >>> wrote:
> >
> > I have the same ASSERT issue on Juno platform even the
> > EnglishDxe.inf is
> >>> included to the platform build. If UefiShellLib has such dependency
> >>> on the protocol then according to EDKII Module Writer's Guide you
> >>> need to specify the dependency on protocol in the module .inf to
> >>> ensure the drivers proper load sequence.
> >
> > 8.6 Dependency Expressions
> > A dependency expression specifies the protocols that the DXE
> > driver requires to execute. In EDK II, it is specified in the
> > [Depex] section of INF
> >>> file.
> >
> 
>  The Dependency Expression is for DXE Drivers that are dispatched by
>  the
> >>> DXE Core. A UEFI Driver from an option ROM or an Application does
> >>> not get dispatched by the dispatch and the Depex will not help. The
> >>> Depex ends up being a section in the FV and it has nothing to do
> >>> with the PE/COFF image of the an application or option ROM.
> 
>  IMHO the shell

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 23:15, Carsey, Jaben wrote:
> All,
> What are your thoughts about Tapan's suggestion to move the protocol location 
> to the only function that needs it?

As I wrote elsewhere in the thread, that will eliminate the assert, but
it will break ShellOpenFileByName() for good (under the same
circumstances where the assert is triggered now).

Is a non-functional ShellOpenFileByName() acceptable, if the platform
doesn't provide the collation protocol? In other words, is it okay to
fail to open any file by name just because we can't compare the filename
against NULL because the collation protocol is missing?

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Tim Lewis]

Jaben --

Here is the relevant piece of code. There is *no way* for Unicode Collation to 
execute without the Shell Protocol existing! There is no way for the Shell 
Protocol to exist without (at least) Unicode Collation or Unicode Collation 2 
from existing.

So adding a check to the function does not help it, because in every case that 
counts (where shell protocol is produced by some sort of standard shell), 
Unicode Collation already exists.

Status = gEfiShellProtocol->OpenFileByName(FileName,
   FileHandle,
   OpenMode);
if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, 
(CHAR16*)FileName, L"NUL") != 0) &&
(mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol, 
(CHAR16*)FileName, L"NULL") != 0) &&
 !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){
  FileInfo = FileFunctionMap.GetFileInfo(*FileHandle); << WITHOUT A SHELL 
IT WILL FAIL HERE
  ASSERT(FileInfo != NULL);
  FileInfo->Attribute = Attributes;
  Status2 = FileFunctionMap.SetFileInfo(*FileHandle, FileInfo);
  FreePool(FileInfo);
  if (EFI_ERROR (Status2)) {
gEfiShellProtocol->CloseFile(*FileHandle);
  }
  Status = Status2;
}



-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com] 
Sent: Wednesday, October 05, 2016 2:17 PM
To: Tim Lewis ; af...@apple.com; Laszlo Ersek 

Cc: edk2-devel-01 ; Leif Lindholm 
; Carsey, Jaben 
Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the 
Reference Platform

That is not enough.  That gets called by the constructor.  We need to not check 
in either constructor or the constructor worker.

That difference only matters for the shell binary itself.

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Tim Lewis
> Sent: Wednesday, October 05, 2016 2:07 PM
> To: af...@apple.com; Laszlo Ersek 
> Cc: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> Importance: High
> 
> If he has a DXE driver that consumes the shell lib (a valid case), 
> then my original suggestion stands: move it to 
> ShellLibConstructorWorker. This is where the ShellLib's internal pointers to 
> the Shell protocol are initialized.
> 
> Tim
> 
> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Wednesday, October 05, 2016 1:59 PM
> To: Laszlo Ersek 
> Cc: Tim Lewis ; Carsey, Jaben 
> ; Shah, Tapan ; Daniil 
> Egranov ; edk2-devel-01  de...@ml01.01.org>; Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
> source on the Reference Platform
> 
> 
> > On Oct 5, 2016, at 1:58 PM, Laszlo Ersek  wrote:
> >
> > On 10/05/16 22:44, Tim Lewis wrote:
> >> Jaben --
> >>
> >> In which cases would you have the Shell protocol present and not 
> >> have
> the Unicode Collation protocol?
> >
> > That's exactly the question!
> >
> > According to the spec, at least if the system cannot boot from a 
> > disk device, then the collation protocol need not be present.
> >
> >> By my count, the Shell itself cannot function without it (see
> ProcessCommandLine()).
> >
> > In which case though I don't understand why Daniil experiences this 
> > change (= commit 583448b441650) as a regression on Juno. If the 
> > shell couldn't function without the collation protocol even before 
> > commit 583448b441650, then the shell must never have run on Juno -- 
> > because it appears to lack collation -- and then this change cannot be a 
> > regression.
> >
> 
> Looks like he has a DXE Driver that consume the ShellLib.
> 
> Thanks,
> 
> Andrew Fish
> 
> > Thanks
> > Laszlo
> >
> >
> >>
> >> Tim
> >>
> >>
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> >> Of Carsey, Jaben
> >> Sent: Wednesday, October 05, 2016 1:35 PM
> >> To: Shah, Tapan ; Laszlo Ersek 
> >> ; Andrew Fish ; Daniil Egranov 
> >> 
> >> Cc: Carsey, Jaben ; edk2-devel-01 
> >> ; Leif Lindholm 
> >> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 
> >> source on the Reference Platform
> >>
> >> That should work.  We also need to initialize the variable to NULL 
> >> in the
> constructor.
> >>
> >> Do we need to be case insensitive for this?  Can we use memcmp 
> >> instead
> of the prototol?
> >>
> >> -Jaben
> >>
> >>> -Original Message-
> >>> From: Shah, Tapan [mailto:tapands...@hpe.com]
> >>> Sent: Wednesday, October 05, 2016 1:25 PM
> >>> To: Laszlo Ersek ; Andrew Fish
> ;
> >>> Daniil Egranov 
> >>> Cc: Carsey, Jaben ; edk2-devel-01  >>> de...@ml01.01.org>; Leif Lindholm 
> >>> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 
> >>> source on the Reference Platform
> >>> Importance: High
> >>>
> >>> How about moving protocol l

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Kinney, Michael D]

Laszlo,

There are many types of UEFI Drivers and they are enumerated in the
UEFI Driver Writer's Guide.  Some follow the UEFI Driver Model and
some do not.

A driver that only consumes/produces UEFI services/protocols without a depex
should be a UEFI Driver.  If there is a depex or use of any PI 
services/protocols
then it must be one of the DXE driver types.

We actually prefer to categorize drivers as UEFI Driver's if possible, because 
that maximizes the drivers's compatibility with UEFI conformant platforms.

Mike


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Wednesday, October 5, 2016 2:18 PM
> To: Andrew Fish ; Daniil Egranov 
> Cc: Carsey, Jaben ; edk2-devel-01 
> ;
> Leif Lindholm 
> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on 
> the
> Reference Platform
> 
> On 10/05/16 23:05, Andrew Fish wrote:
> 
> > I'm guessing that your DXE Driver is dispatching prior UEFI Driver that 
> > publishes the
> protocol.
> 
> BTW... Why is EnglishDxe a UEFI_DRIVER rather than a DXE_DRIVER? It
> doesn't conform to the UEFI driver model. (I'm not saying it *must* not
> be a UEFI_DRIVER, but what reason does it have for being one?) It also
> calls no runtime services, and only the
> InstallMultipleProtocolInterfaces() boot service; so I don't think it
> relies on all of the architectural protocols either.
> 
> 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] Urgent help -UefiBootManagerLib and LegacyBootManagerLib issue

[Andrew Fish]

> On Oct 5, 2016, at 2:23 PM, Saqib Khan  wrote:
> 
> 
> 
> Hi all,i need urgent help regarding this issue.
> 


Saqib,

You likely have a bug in your CSM. So that is your gEfiLegacyBiosProtocolGuid 
implementation and all the 16-bit legacy BIOS code. 

So you should contact the people you got your CSM from. 

Thanks,

Andrew Fish

>> On 05-Oct-2016, at 9:05 PM, Saqib Khan  wrote:
>> 
>> 
>> I have found that  it just dont return from  mBmRefreshLegacyBootOption (); .
>> 
>> have a look at code. let me know the possible cause of it ...
>> I need urgent help
>> 
>> EfiBootManagerRefreshAllBootOption (
>>  VOID
>>  )
>> {
>>  EFI_STATUSStatus;
>>  EFI_BOOT_MANAGER_LOAD_OPTION  *NvBootOptions;
>>  UINTN NvBootOptionCount;
>>  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
>>  UINTN BootOptionCount;
>>  UINTN Index;
>>  Print(L"indside refresh\n");
>>  //
>>  // Optionally refresh the legacy boot option
>>  //
>>  if (mBmRefreshLegacyBootOption != NULL) {
>>  Print(L"Before legacy refresh \n");
>>mBmRefreshLegacyBootOption ();  //this method does not return 
>>Print(L"legacy refresh complete\n");
>>  }
>> 
>>> On Wed, Oct 5, 2016 at 5:51 PM, Saqib Khan  wrote:
>>> Hi,
>>> 
>>> when i import both lib in my project my EFI hangs at 
>>> EfiRefreshAllBootOptions, i removed LegacyBootManager and it worked fine .i 
>>> need both lib as i need to boot legacy from EFI, how this issue can be 
>>> resolved?
>>> 
>>> 
>>> here is piece of inf file
>>> 
>>> [Packages]
>>>  MdePkg/MdePkg.dec
>>>  MdeModulePkg/MdeModulePkg.dec
>>>  IntelFrameworkPkg/
>>> IntelFrameworkPkg.dec
>>>  IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>>>  ShellPkg/ShellPkg.dec
>>> 
>>> [LibraryClasses]
>>>  HiiLib
>>>  DebugLib
>>>  UefiLib
>>>  MemoryAllocationLib
>>>  UefiBootServicesTableLib
>>>  UefiApplicationEntryPoint
>>>  UefiBootManagerLib
>>>  LegacyBootManagerLib
>>> 
>>> 
>>> -- 
>>> Regards
>>> Saqib Ahmed Khanzada
>> 
>> 
>> 
>> -- 
>> Regards
>> Saqib Ahmed Khanzada
> ___
> 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] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Daniil Egranov]

On 10/05/2016 04:15 PM, Carsey, Jaben wrote:

All,
What are your thoughts about Tapan's suggestion to move the protocol location 
to the only function that needs it?

Andrew,

We cannot publish a dependency.  There are DXE driver dynamic shell commands 
that launch before the shell runs and produce the appropriate protocol so that 
the shell will call into them.  These drivers must be allowed to successfully 
launch before the shell runs.

Danil,

Can you elaborate what your DXE driver is doing with the library?


As I can see, it used by ArmJunoDxe for registering new shell command:

  // Install dynamic Shell command to run baremetal binaries.
  Status = ShellDynCmdRunAxfInstall (ImageHandle);
  if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "ArmJunoDxe: Failed to install 
ShellDynCmdRunAxf\n"));

  }





-Jaben


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Andrew Fish
Sent: Wednesday, October 05, 2016 2:06 PM
To: Daniil Egranov 
Cc: Carsey, Jaben ; edk2-devel-01 ; Leif Lindholm 
Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
the Reference Platform
Importance: High



On Oct 5, 2016, at 1:53 PM, Daniil Egranov 

wrote:



On 10/05/2016 02:48 PM, Andrew Fish wrote:

On Oct 5, 2016, at 12:24 PM, Daniil Egranov 

wrote:

I have the same ASSERT issue on Juno platform even the EnglishDxe.inf

is included to the platform build. If UefiShellLib has such dependency on the
protocol then according to EDKII Module Writer's Guide you need to specify
the dependency on protocol in the module .inf to ensure the drivers proper
load sequence.

8.6 Dependency Expressions
A dependency expression specifies the protocols that the DXE driver

requires to

execute. In EDK II, it is specified in the [Depex] section of INF file.


The Dependency Expression is for DXE Drivers that are dispatched by the

DXE Core. A UEFI Driver from an option ROM or an Application does not get
dispatched by the dispatch and the Depex will not help. The Depex ends up
being a section in the FV and it has nothing to do with the PE/COFF image of
the an application or option ROM.

IMHO the shell should try as hard as possible to function and should not

ASSERT if some newer Protocol is missing.

Thanks,

Andre wFish

I had to put a context of the ASSERT message in the original message to

make it more clear:

add-symbol-file

/home/user1/workspace/juno_16.09/uefi/edk2/Build/ArmJuno/DEBUG_GC
C49/AARCH64/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJuno
Dxe/DEBUG/ArmJunoDxe.dll 0xF8AB9000

Loading driver at 0x000F8AB8000 EntryPoint=0x000F8AB9048

ArmJunoDxe.efi

ASSERT_EFI_ERROR (Status = Not Found)
ASSERT [ArmJunoDxe]

/home/danegr01/workspace/juno_16.09/uefi/edk2/ShellPkg/Library/UefiSh
ellLib/UefiShellLib.c(305): !EFI_ERROR (Status)

If driver includes a module which has dependency on one of the protocols,

should the driver know about this dependency? Probably not. It should be
inherited from the module. Adding [Depex] to UefiShellLib corrected
dispatching ArmJunoDxe and EnglishDxe by the Dxe Core.
Daniil,

Sorry, I was assuming the UefiShellLib only supported UEFI Applications.
Thats what I get for not looking at the code :(, my bad.

https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiShellL
ib/UefiShellLib.inf#L23

   LIBRARY_CLASS  = ShellLib|UEFI_APPLICATION UEFI_DRIVER
DXE_RUNTIME_DRIVER DXE_DRIVER

Given the library supports DXE_RUNTIME_DRIVER and DXE_DRIVER I think
you are right it should publish a Depex if it has a hard dependency.

I'm guessing that your DXE Driver is dispatching prior UEFI Driver that
publishes the protocol.

Thanks,

Andrew Fish





The following dependency in UefiShellLib.inf fixes ASSERT problem:

[Depex]
  gEfiUnicodeCollation2ProtocolGuid


Thanks,

Daniil


On 10/05/2016 11:02 AM, Shah, Tapan wrote:

It's possible. But I think gEfiUnicodeCollation2ProtocolGuid protocol is

necessary for even other Shell libraries other than UefiShellLib in order to
have Shell parser and other command line processing to work properly.
That's why I added ASSERT if UefiShellLib fails to locate the protocol.

  Reference platform should have EnglishDxe module to have this

protocol installed properly.



MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf

-Original Message-
From: Carsey, Jaben [mailto:jaben.car...@intel.com]
Sent: Wednesday, October 05, 2016 10:41 AM
To: Supreeth Venkatesh ; edk2-

devel-01 ; Shah, Tapan 

Cc: Leif Lindholm ; Carsey, Jaben



Subject: RE: Assert in ShellPkg with latest tianocore edk2 source on the

Reference Platform

Tapan,

Could this be a side effect of your patch?  Should we allow the

UefiShellLib to continue without this protocol and then return an error only
when the OpenFile is called?

Also, it looks like we never properly initialize

mUnicodeColl

Re: [edk2] TE relocations

[valerij zaporogeci]

Thank you very much, Andrew.
The last question. Regarding that scheme you depicted, which entity
will have ImageBase address when TE image is loaded in the memory? A
'conceptual' stripped Pe headers, or Te header?
Which is placed (even conceptually) at ImageBase address?
P <--- This? (would have been here if presented)
P
P
PT <--- Or this? (really is put at ImageBase)

SECTION_TABLE
---
.text SECTION
.data SECTION
.reloc SECTION
.debug SECTION

Because looking at the AddressOfEntryPoint adjustment shows TE header
is placed at ImageBase address. But then again all sections (and the
addresses of referenced symbols) get shifted at the delta value
(StrippedSize - SizeOf(TE_HEADER)). The same as AddressOfEntryPoint.
If ImageBase points to non-existent Pe headers, before TE header, then
everything is fine with adresses in sections, but AddressOfEntryPoint
doesn't need to be adjusted as well contrary to what specification
says.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Laszlo Ersek]

On 10/05/16 23:34, Kinney, Michael D wrote:
> Laszlo,
> 
> There are many types of UEFI Drivers and they are enumerated in the
> UEFI Driver Writer's Guide.  Some follow the UEFI Driver Model and
> some do not.
> 
> A driver that only consumes/produces UEFI services/protocols without a depex
> should be a UEFI Driver.  If there is a depex or use of any PI 
> services/protocols
> then it must be one of the DXE driver types.
> 
> We actually prefer to categorize drivers as UEFI Driver's if possible, 
> because 
> that maximizes the drivers's compatibility with UEFI conformant platforms.

Thanks!

It's been a while since I last looked at that part of the guide (chapter
6, UEFI Driver Categories). I guess EnglishDxe then qualifies as a
Service Driver.

... Interestingly, when people asked about automatically running
applications (not drivers) for various kinds of init work, without
staying resident, I occasionally thought about recommending a driver
module that would return an error code intentionally, even on success.
And now I see there's actually a category of drivers for this, called
"Initializing Drivers". So now I wonder why we needed SysPrep after
all... Probably because their order had to be specified in some way.

The Root Bridge Driver category is also intersting (under "UEFI Driver
Categories"); the core PciHostBridgeDxe in edk2 is a DXE_DRIVER. (It
does have an elaborate depex though, and not just on architectural
protocols.)

Thanks!
Laszlo


>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Laszlo Ersek
>> Sent: Wednesday, October 5, 2016 2:18 PM
>> To: Andrew Fish ; Daniil Egranov 
>> Cc: Carsey, Jaben ; edk2-devel-01 
>> ;
>> Leif Lindholm 
>> Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on 
>> the
>> Reference Platform
>>
>> On 10/05/16 23:05, Andrew Fish wrote:
>>
>>> I'm guessing that your DXE Driver is dispatching prior UEFI Driver that 
>>> publishes the
>> protocol.
>>
>> BTW... Why is EnglishDxe a UEFI_DRIVER rather than a DXE_DRIVER? It
>> doesn't conform to the UEFI driver model. (I'm not saying it *must* not
>> be a UEFI_DRIVER, but what reason does it have for being one?) It also
>> calls no runtime services, and only the
>> InstallMultipleProtocolInterfaces() boot service; so I don't think it
>> relies on all of the architectural protocols either.
>>
>> 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] TE relocations

[Andrew Fish]

> On Oct 5, 2016, at 2:45 PM, valerij zaporogeci  wrote:
> 
> Thank you very much, Andrew.
> The last question. Regarding that scheme you depicted, which entity
> will have ImageBase address when TE image is loaded in the memory? A
> 'conceptual' stripped Pe headers, or Te header?
> Which is placed (even conceptually) at ImageBase address?
> P <--- This? (would have been here if presented)
> P
> P
> PT <--- Or this? (really is put at ImageBase)
> 
> SECTION_TABLE
> ---
> .text SECTION
> .data SECTION
> .reloc SECTION
> .debug SECTION
> 

The ImageBase is the same for PE/COFF and TE. 
In the code ImageAddress points to the start of T or P (well P can have a DOS 
header prepended etc). I think a lot of the code operates on ImageAddress and 
thus needs the adjustment. 

Thanks,

Andrew Fish

> Because looking at the AddressOfEntryPoint adjustment shows TE header
> is placed at ImageBase address. But then again all sections (and the
> addresses of referenced symbols) get shifted at the delta value
> (StrippedSize - SizeOf(TE_HEADER)). The same as AddressOfEntryPoint.
> If ImageBase points to non-existent Pe headers, before TE header, then
> everything is fine with adresses in sections, but AddressOfEntryPoint
> doesn't need to be adjusted as well contrary to what specification
> says.

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

Re: [edk2] Assert in ShellPkg with latest tianocore edk2 source on the Reference Platform

[Carsey, Jaben]

Laszlo,

As Tim clarified, the API already fail when the shell protocol was not present 
and the shell already needs the UnicodeCollation protocol.

The goal here is not to allow the library to function without the shell 
protocol (and thereby the UnicodeCollation protocol).  The goal is to allow the 
constructor to complete successfully. So that a DXE driver (or other driver) 
can be loaded into memory and wait for the shell to be present to increase 
shell functionality.

The reasons is that a non-functional ShellOpenFileByName() in the time gap 
between them is ok since it will fail as soon as the lib calls into the shell 
anyways.  This was already true since the protocol was not present for the API 
to work with...

-Jaben

> -Original Message-
> From: Tim Lewis [mailto:tim.le...@insyde.com]
> Sent: Wednesday, October 05, 2016 2:33 PM
> To: Carsey, Jaben ; af...@apple.com; Laszlo Ersek
> 
> Cc: edk2-devel-01 ; Leif Lindholm
> 
> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> Importance: High
> 
> Jaben --
> 
> Here is the relevant piece of code. There is *no way* for Unicode Collation to
> execute without the Shell Protocol existing! There is no way for the Shell
> Protocol to exist without (at least) Unicode Collation or Unicode Collation 2
> from existing.
> 
> So adding a check to the function does not help it, because in every case that
> counts (where shell protocol is produced by some sort of standard shell),
> Unicode Collation already exists.
> 
> Status = gEfiShellProtocol->OpenFileByName(FileName,
>FileHandle,
>OpenMode);
> if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol,
> (CHAR16*)FileName, L"NUL") != 0) &&
> (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol,
> (CHAR16*)FileName, L"NULL") != 0) &&
>  !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){
>   FileInfo = FileFunctionMap.GetFileInfo(*FileHandle); << WITHOUT A
> SHELL IT WILL FAIL HERE
>   ASSERT(FileInfo != NULL);
>   FileInfo->Attribute = Attributes;
>   Status2 = FileFunctionMap.SetFileInfo(*FileHandle, FileInfo);
>   FreePool(FileInfo);
>   if (EFI_ERROR (Status2)) {
> gEfiShellProtocol->CloseFile(*FileHandle);
>   }
>   Status = Status2;
> }
> 
> 
> 
> -Original Message-
> From: Carsey, Jaben [mailto:jaben.car...@intel.com]
> Sent: Wednesday, October 05, 2016 2:17 PM
> To: Tim Lewis ; af...@apple.com; Laszlo Ersek
> 
> Cc: edk2-devel-01 ; Leif Lindholm
> ; Carsey, Jaben 
> Subject: RE: [edk2] Assert in ShellPkg with latest tianocore edk2 source on
> the Reference Platform
> 
> That is not enough.  That gets called by the constructor.  We need to not
> check in either constructor or the constructor worker.
> 
> That difference only matters for the shell binary itself.
> 
> -Jaben
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Tim Lewis
> > Sent: Wednesday, October 05, 2016 2:07 PM
> > To: af...@apple.com; Laszlo Ersek 
> > Cc: Carsey, Jaben ; edk2-devel-01  > de...@ml01.01.org>; Leif Lindholm 
> > Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2
> > source on the Reference Platform
> > Importance: High
> >
> > If he has a DXE driver that consumes the shell lib (a valid case),
> > then my original suggestion stands: move it to
> > ShellLibConstructorWorker. This is where the ShellLib's internal pointers to
> the Shell protocol are initialized.
> >
> > Tim
> >
> > -Original Message-
> > From: af...@apple.com [mailto:af...@apple.com]
> > Sent: Wednesday, October 05, 2016 1:59 PM
> > To: Laszlo Ersek 
> > Cc: Tim Lewis ; Carsey, Jaben
> > ; Shah, Tapan ; Daniil
> > Egranov ; edk2-devel-01  > de...@ml01.01.org>; Leif Lindholm 
> > Subject: Re: [edk2] Assert in ShellPkg with latest tianocore edk2
> > source on the Reference Platform
> >
> >
> > > On Oct 5, 2016, at 1:58 PM, Laszlo Ersek  wrote:
> > >
> > > On 10/05/16 22:44, Tim Lewis wrote:
> > >> Jaben --
> > >>
> > >> In which cases would you have the Shell protocol present and not
> > >> have
> > the Unicode Collation protocol?
> > >
> > > That's exactly the question!
> > >
> > > According to the spec, at least if the system cannot boot from a
> > > disk device, then the collation protocol need not be present.
> > >
> > >> By my count, the Shell itself cannot function without it (see
> > ProcessCommandLine()).
> > >
> > > In which case though I don't understand why Daniil experiences this
> > > change (= commit 583448b441650) as a regression on Juno. If the
> > > shell couldn't function without the collation protocol even before
> > > commit 583448b441650, then the shell must never have run on Juno --
> > > because it appears to lack collation -- and then this change cannot be a
> regression.
> > >
> >
> > Look

Re: [edk2] [Patch V2] BaseTools: support the NOOPT target with the GCC tool chains

[B Cran]

Tested-by: Bruce Cran 

Sent from my iPhone

> On Oct 5, 2016, at 12:06 PM, Ard Biesheuvel  wrote:
> 
>> On 5 October 2016 at 18:56, Laszlo Ersek  wrote:
>>> On 10/05/16 18:06, Ard Biesheuvel wrote:
 On 5 October 2016 at 15:48, Laszlo Ersek  wrote:
> On 10/05/16 03:30, Yonghong Zhu wrote:
> Update the tools_def.template to add NOOPT support with GCC tool chains.
> 
> Cc: Liming Gao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
> BaseTools/Conf/tools_def.template | 28 
> 1 file changed, 28 insertions(+)
 
 I thought I understood what was going on, but apparently I was wrong
 about that.
 
 In this patch, we add or modify:
 - NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG -- okay
 - NOOPT_GCC*_(IA32|X64|ARM|AARCH64)_CC_FLAGS -- okay
 
 So that part is fine with me. But then we also add / modify:
 - NOOPT_GCC(49|5)_AARCH64_DLINK_(FLAGS|XIPFLAGS)
 - NOOPT_GCC5_ARM_DLINK_FLAGS
 
 First I thought the latter set of changes was unnecessary, because "ld"
 didn't use "-O". I checked the manual, and I was wrong: "ld" does know /
 use "-O". So those changes are fine, I guess.
 
>>> 
>>> Yes, especially under LTO, in which case code generation is performed
>>> during the link stage, which should adhere to the same rules as the
>>> compiler. This not only applies to -O, but also to things like
>>> -march/-mcpu and -mstrict-align. This is why we pass all CFLAGS to the
>>> linker for the GCC5 LTO builds.
>>> 
 But then: is the patch *complete*? Because I can see some more DLINK
 stuff, for IA32 and X64 (not just ARM and AARCH64). Is it okay to ignore
 those? For example:
 
 *_GCC5_IA32_DLINK_FLAGS  = DEF(GCC5_IA32_X64_DLINK_FLAGS) -Os
 -Wl,-m,elf_i386,--oformat=elf32-i386
 
 
 *_GCC5_X64_DLINK_FLAGS   = DEF(GCC5_X64_DLINK_FLAGS) -Os
 
 Where GCC5_X64_DLINK_FLAGS and GCC5_IA32_X64_DLINK_FLAGS even include
 -flto. (I don't know if "-flto" hampers source level debugging or not.)
 
>>> 
>>> The GCC man page documents -flto as being a bad idea, i.e.,
>>> 
>>> """
>>> Link-time optimization does not work well with generation of debugging
>>> information.  Combining -flto with -g is currently experimental and
>>> expected to produce unexpected results.
>>> """
>>> 
>>> (which raises a philosophical question as well, i.e., to which extent
>>> expected unexpected results are still unexpected results. But I
>>> digress ...)
>>> 
>>> Another note: the DEBUG build for ARM and AARCH64 is essentially NOOPT
>>> already, not DEBUG. How does this patch intend to deal with that?
>> 
>> It just copies the DEBUG settings to NOOPT (via deep copy, not by
>> reference). I believe that's OK.
>> 
> 
> OK, fair enough. Leif and I can look into this in the future.
> ___
> 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 V2] BaseTools: support the NOOPT target with the GCC tool chains

[Bruce Cran]

On 10/04/2016 07:30 PM, Yonghong Zhu wrote:


Update the tools_def.template to add NOOPT support with GCC tool chains.

Cc: Liming Gao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 


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

Tested with both GCC49 and GCC5 toolchain settings (using gcc 6.2.1 for 
both) and verified that both have functional debugging, though gdb skips 
around with GCC5 as expected due to LTCG.


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

Re: [edk2] TE relocations

[valerij zaporogeci]

>> The ImageBase is the same for PE/COFF and TE.
>> In the code ImageAddress points to the start of T or P (well P can have a 
>> DOS header
>> prepended etc). I think a lot of the code operates on ImageAddress and thus 
>> needs the
>> adjustment.

Well, I don't want to abuse your attention. Just last try to explain
the incosistency here I can not resolve.
Suppose we have some imaginable ISA instruction somewhere in code:
LOAD r1, [0x402f04bc]
and 0x402f04bc is the address of some symbol, resolved by linker.
let's parse this address. Let ImageBase be 0x402f, and data
section offset be 0x400 and finally data item offset in the section be
0xbc. Data section is also at 0x400 from the file beginning, since
sectionalignment==filealignment, which means the layout is the same.
When it is loaded at 0x402f000, everything works. Data section is at
0x400 from there, and our variable is at 0xbc from the section start.
Now, we make TE from it. Now, the data section in the TE file is NOT
at 0x400 from the file beginning (it is closer). And when (and if) TE
is loaded such that TE header is placed in memory at THE SAME
ImageBase address as the original PE would have been, the referenced
variable will not be at 402f04bc. And the code, referencing address
0x402f4bc, would get something else instead of this variable content.
This is not the case in the reality. But why? The PI specification
recipe, with only AddressOfEntryPoint adjustment and without
adjustment of anything else referenced (in the code) should result in
this incostistency.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of UefiShellLib constructor

[Carsey, Jaben]

And pushed.
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Tapan Shah [mailto:tapands...@hpe.com]
> Sent: Wednesday, October 05, 2016 1:58 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Tapan Shah
> 
> Subject: [PATCH] ShellPkg: Move UnicodeCollation2 Protcol locate out of
> UefiShellLib constructor
> Importance: High
> 
> Move gEfiUnicodeCollation2ProtocolGuid protocol outside of UefiShellLib
> constructor
> function.
> Locate gEfiUnicodeCollation2ProtocolGuid protocol in
> ShellOpenFileByName() which
> consumes this protocol API.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Tapan Shah 
> ---
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c | 16 +---
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index e47d535..53f54e1 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -292,18 +292,12 @@ ShellLibConstructor (
>IN EFI_SYSTEM_TABLE  *SystemTable
>)
>  {
> -  EFI_STATUS  Status;
> -
>mEfiShellEnvironment2   = NULL;
>gEfiShellProtocol   = NULL;
>gEfiShellParametersProtocol = NULL;
>mEfiShellInterface  = NULL;
>mEfiShellEnvironment2Handle = NULL;
> -
> -  if (mUnicodeCollationProtocol == NULL) {
> -Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid, NULL,
> (VOID**)&mUnicodeCollationProtocol);
> -ASSERT_EFI_ERROR (Status);
> -  }
> +  mUnicodeCollationProtocol   = NULL;
> 
>//
>// verify that auto initialize is not set false
> @@ -730,6 +724,14 @@ ShellOpenFileByName(
> FileHandle,
> OpenMode);
> 
> +if (mUnicodeCollationProtocol == NULL) {
> +  Status = gBS->LocateProtocol (&gEfiUnicodeCollation2ProtocolGuid,
> NULL, (VOID**)&mUnicodeCollationProtocol);
> +  if (EFI_ERROR (Status)) {
> +gEfiShellProtocol->CloseFile (*FileHandle);
> +return Status;
> +  }
> +}
> +
>  if ((mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol,
> (CHAR16*)FileName, L"NUL") != 0) &&
>  (mUnicodeCollationProtocol->StriColl (mUnicodeCollationProtocol,
> (CHAR16*)FileName, L"NULL") != 0) &&
>   !EFI_ERROR(Status) && ((OpenMode & EFI_FILE_MODE_CREATE) != 0)){
> --
> 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/GenFds: Support FMP Payload files generated during build

[Michael Kinney]

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

This patch allows FILE DATA statements in [FmpPayload] sections
to refer to a file that does not exist at the time the FDF file
is parsed.  There are cases where these FILE DATA statements refer
to FD or FV images that are generated as part of the build and those
FD or FV image files are not present when FDF file is parsed.  These
files are required to be present when the FMP Payload is
generated.

Skip the file present verification step for FILE DATA statements in
[FmpPayload] sections if the file path referenced is in the
$(OUTPUT_DIRECTORY).  Perform this verification step when the
FMP Payload is generated.

Cc: Kelly Steele 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 BaseTools/Source/Python/GenFds/Capsule.py |  8 ++
 BaseTools/Source/Python/GenFds/CapsuleData.py |  9 +++
 BaseTools/Source/Python/GenFds/FdfParser.py   | 38 ++-
 3 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/Capsule.py 
b/BaseTools/Source/Python/GenFds/Capsule.py
index c98c054..31c414f 100644
--- a/BaseTools/Source/Python/GenFds/Capsule.py
+++ b/BaseTools/Source/Python/GenFds/Capsule.py
@@ -28,6 +28,8 @@ from struct import pack
 from GenFds import FindExtendTool
 from Common import EdkLogger
 from Common.BuildToolError import *
+from Common.Misc import PathClass
+from Common.String import NormPath
 
 
 T_CHAR_LF = '\n'
@@ -142,6 +144,12 @@ class Capsule (CapsuleClassObject) :
 File.close()
 for fmp in self.FmpPayloadList:
 if fmp.Certificate_Guid:
+#
+# Verify that ImageFile exists
+#
+ErrorCode, ErrorInfo = PathClass(NormPath(fmp.ImageFile), 
GenFdsGlobalVariable.WorkSpaceDir).Validate()
+if ErrorCode != 0:
+EdkLogger.error("GenFds", ErrorCode, ExtraData=ErrorInfo)
 ExternalTool, ExternalOption = FindExtendTool([], 
GenFdsGlobalVariable.ArchList, fmp.Certificate_Guid)
 CmdOption = ''
 CapInputFile = fmp.ImageFile
diff --git a/BaseTools/Source/Python/GenFds/CapsuleData.py 
b/BaseTools/Source/Python/GenFds/CapsuleData.py
index 07cc198..7cbe351 100644
--- a/BaseTools/Source/Python/GenFds/CapsuleData.py
+++ b/BaseTools/Source/Python/GenFds/CapsuleData.py
@@ -22,6 +22,9 @@ from struct import pack
 import os
 from Common.Misc import SaveFileOnChange
 import uuid
+from Common import EdkLogger
+from Common.Misc import PathClass
+from Common.String import NormPath
 
 ## base class for capsule data
 #
@@ -194,6 +197,12 @@ class CapsulePayload(CapsuleData):
 ImageFileSize += 32
 VendorFileSize = 0
 if self.VendorCodeFile:
+#
+# Verify that VendorCodeFile exists
+#
+ErrorCode, ErrorInfo = PathClass(NormPath(self.VendorCodeFile), 
GenFdsGlobalVariable.WorkSpaceDir).Validate()
+if ErrorCode != 0:
+EdkLogger.error("GenFds", ErrorCode, ExtraData=ErrorInfo)
 VendorFileSize = os.path.getsize(self.VendorCodeFile)
 
 #
diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py 
b/BaseTools/Source/Python/GenFds/FdfParser.py
index 02ae7c9..2038a37 100644
--- a/BaseTools/Source/Python/GenFds/FdfParser.py
+++ b/BaseTools/Source/Python/GenFds/FdfParser.py
@@ -3037,17 +3037,21 @@ class FdfParser:
 ## __VerifyFile
 #
 #Check if file exists or not:
-#  If current phase if GenFds, the file must exist;
-#  If current phase is AutoGen and the file is not in 
$(OUTPUT_DIRECTORY), the file must exist
+#  If current phase is AutoGen or FmpPayload is True, and the file is 
not in $(OUTPUT_DIRECTORY),
+#  then the file must exist
 #@param FileName: File path to be verified.
+#@param FmpPayload: If True, then FileName is from a raw file 
statement in an [FmpPayload] section
+#   and FileName is only required to be present if 
FileName is not in OUTPUT_DIRECTORY
 #
-def __VerifyFile(self, FileName):
+def __VerifyFile(self, FileName, FmpPayload = False):
 if FileName.replace('$(WORKSPACE)', '').find('$') != -1:
 return
-if not GlobalData.gAutoGenPhase or not 
self.__GetMacroValue("OUTPUT_DIRECTORY") in FileName:
-ErrorCode, ErrorInfo = PathClass(NormPath(FileName), 
GenFdsGlobalVariable.WorkSpaceDir).Validate()
-if ErrorCode != 0:
-EdkLogger.error("GenFds", ErrorCode, ExtraData=ErrorInfo)
+if GlobalData.gAutoGenPhase or FmpPayload:
+if self.__GetMacroValue("OUTPUT_DIRECTORY") in FileName:
+return
+ErrorCode, ErrorInfo = PathClass(NormPath(FileName), 
GenFdsGlobalVariable.WorkSpaceDir).Validate()
+if ErrorCode != 0:
+EdkLogger.error("Ge

Re: [edk2] TE relocations

[Andrew Fish]

> On Oct 5, 2016, at 4:25 PM, valerij zaporogeci  wrote:
> 
>>> The ImageBase is the same for PE/COFF and TE.
>>> In the code ImageAddress points to the start of T or P (well P can have a 
>>> DOS header
>>> prepended etc). I think a lot of the code operates on ImageAddress and thus 
>>> needs the
>>> adjustment.
> 
> Well, I don't want to abuse your attention.

Well I'm doing it off the top of my head so I'm not doing a lot of research. If 
I get a chance at some point I'll re-read the PI spec and see if is a bug in 
the specification since I can file an ECR to get it fixed. 

> Just last try to explain
> the incosistency here I can not resolve.
> Suppose we have some imaginable ISA instruction somewhere in code:
> LOAD r1, [0x402f04bc]
> and 0x402f04bc is the address of some symbol, resolved by linker.
> let's parse this address. Let ImageBase be 0x402f, and data
> section offset be 0x400 and finally data item offset in the section be
> 0xbc. Data section is also at 0x400 from the file beginning, since
> sectionalignment==filealignment, which means the layout is the same.
> When it is loaded at 0x402f000, everything works. Data section is at
> 0x400 from there, and our variable is at 0xbc from the section start.
> Now, we make TE from it. Now, the data section in the TE file is NOT
> at 0x400 from the file beginning (it is closer). And when (and if) TE
> is loaded such that TE header is placed in memory at THE SAME
> ImageBase address as the original PE would have been, the referenced
> variable will not be at 402f04bc. And the code, referencing address
> 0x402f4bc, would get something else instead of this variable content.
> This is not the case in the reality. But why? The PI specification
> recipe, with only AddressOfEntryPoint adjustment and without
> adjustment of anything else referenced (in the code) should result in
> this incostistency.

Quick answer is sectionalignment==filealignment for XIP is from the PE/COFF 
image point of view. From a TE point of view the FileAlignment has to get 
adjusted. But please remember TE is really just a shortened version of the 
PE/COFF header, so other than references to those header values it is still the 
PE/COFF image. 

So I have to ask why are you so interested in TE?

Thanks,

Andrew Fish


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

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

[edk2] [PATCH] ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe: Set Marvell Yukon MAC address

[Daniil Egranov]

The patch reads a valid MAC address form the Juno IOFPGA registers 
and pushes it into onboard Marvell Yukon NIC.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 .../ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c | 141 +
 .../Drivers/ArmJunoDxe/ArmJunoDxeInternal.h|  12 ++
 2 files changed, 153 insertions(+)

diff --git a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c 
b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
index b97f044..0c5fbd0 100644
--- a/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
+++ b/ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/ArmJunoDxe.c
@@ -17,6 +17,8 @@
 
 #include 
 #include 
+#include 
+#include 
 
 #include 
 #include 
@@ -68,6 +70,142 @@ STATIC CONST EFI_PCI_ROOT_BRIDGE_DEVICE_PATH 
mPciRootComplexDevicePath = {
 
 EFI_EVENT mAcpiRegistration = NULL;
 
+UINT32 SwapUINT32(UINT32 value)
+{
+  value = ((value << 8) & 0xFF00FF00 ) | ((value >> 8) & 0xFF00FF );
+  return (value << 16) | (value >> 16);
+}
+
+/**
+  The function reads MAC address from Juno IOFPGA registers and writes it
+  into Marvell Yukon NIC.
+**/
+STATIC
+EFI_STATUS
+ArmJunoSetNetworkMAC()
+{
+
+  EFI_STATUS  Status;
+  UINTN   HandleCount;
+  EFI_HANDLE  *HandleBuffer;
+  UINTN   HIndex;
+  EFI_PCI_IO_PROTOCOL*PciIo;
+  UINT64  PciID;
+  UINT32  MacHigh;
+  UINT32  MacLow;
+  UINT32  PciRegBase;
+  UINT64  OldPciAttributes;
+  UINT64  AttrSupports;
+  UINT8   *PciBarAttributes;
+
+  Status = gBS->LocateHandleBuffer (ByProtocol,
+&gEfiPciIoProtocolGuid,
+NULL, &HandleCount, &HandleBuffer);
+
+  if (!EFI_ERROR (Status)) {
+for (HIndex = 0; HIndex < HandleCount; ++HIndex) {
+  Status = gBS->OpenProtocol (
+  HandleBuffer[HIndex],
+  &gEfiPciIoProtocolGuid,
+  (VOID **) &PciIo,
+  NULL,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+
+  if (EFI_ERROR (Status)) {
+continue;
+  }
+
+  Status = PciIo->Pci.Read (
+PciIo,
+EfiPciIoWidthUint32,
+PCI_VENDOR_ID_OFFSET,
+1,
+&PciID
+);
+
+  if (EFI_ERROR (Status)) {
+continue;
+  }
+
+  if ((PciID & 0x) == JUNO_MARVELL_YUKON_ID) {
+
+// Read MAC address from IOFPGA
+MacHigh= MmioRead32 (ARM_JUNO_SYS_PCIGBE_H);
+MacLow = MmioRead32 (ARM_JUNO_SYS_PCIGBE_L);
+
+Status = PciIo->Attributes (
+  PciIo,
+  EfiPciIoAttributeOperationGet,
+  0,
+  &OldPciAttributes
+  );
+
+if (EFI_ERROR (Status)) {
+  continue;
+}
+
+Status = PciIo->Attributes (
+  PciIo,
+  EfiPciIoAttributeOperationSupported,
+  0,
+  &AttrSupports
+  );
+
+if (!EFI_ERROR (Status)) {
+  AttrSupports &= EFI_PCI_DEVICE_ENABLE;
+  Status = PciIo->Attributes (
+PciIo,
+EfiPciIoAttributeOperationEnable,
+AttrSupports,
+NULL
+);
+
+  Status = PciIo->GetBarAttributes (PciIo, 0, &AttrSupports, 
(VOID**)&PciBarAttributes);
+  if (!EFI_ERROR (Status) && (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR 
*)PciBarAttributes)->Desc == ACPI_ADDRESS_SPACE_DESCRIPTOR)) {
+if (((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR 
*)PciBarAttributes)->ResType == ACPI_ADDRESS_SPACE_TYPE_MEM) {
+  if (!(((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR 
*)PciBarAttributes)->SpecificFlag & ACPI_SPECFLAG_PREFETCHABLE)) {
+PciRegBase = ((EFI_ACPI_ADDRESS_SPACE_DESCRIPTOR 
*)PciBarAttributes)->AddrRangeMin;
+
+// Clear Software Reset
+MmioWrite16 (PciRegBase + R_CONTROL_STATUS, CS_RESET_CLR);
+
+// Convert to Marvell MAC Address register format
+MacHigh = SwapUINT32 ((MacHigh & 0x) << 16 |
+ (MacLow & 0x) >> 16);
+MacLow = SwapUINT32 (MacLow) >> 16;
+
+// Set MAC Address
+MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_ENABLE);
+MmioWrite32 (PciRegBase + R_MAC, MacHigh);
+MmioWrite32 (PciRegBase + R_MAC_MAINT, MacHigh);
+MmioWrite32 (PciRegBase + R_MAC + 4, MacLow);
+MmioWrite32 (PciRegBase + R_MAC_MAINT + 4, MacLow);
+MmioWrite8 (PciRegBase + R_TST_CTRL_1, TST_CFG_WRITE_DISA

Re: [edk2] OVMF.fd and placement of EfiBootServicesData

[spam collector]

- Original Message -
> From: "Laszlo Ersek" 
> To: "spam collector" 
> Cc: edk2-de...@ml01.01.org
> Sent: Wednesday, October 5, 2016 8:28:46 AM
> Subject: Re: [edk2] OVMF.fd and placement of EfiBootServicesData
> 
> On 10/05/16 04:47, spam collector wrote:
> > 
> > - Original Message -
> >> From: "Laszlo Ersek" 
> >> To: "spam collector" 
> >> Cc: edk2-de...@ml01.01.org
> >> Sent: Tuesday, October 4, 2016 10:22:34 AM
> >> Subject: Re: [edk2] OVMF.fd and placement of EfiBootServicesData
> 
> >> * The first stage improvement is the following command line:
> >>
> >>   qemu-system-x86_64 -pflash OVMF.fd
> > 
> > This did not work either with or without the NvVars file present.
> 
> I think I'll need a more complete issue riport then -- what is your
> exact QEMU command line, and what OVMF binary are you using?

"..\qemu-system-i386w.exe" -m 256 -localtime 
  -drive file=uefi.bin.vhd,format=raw,if=ide,media=disk,index=0 
  -pflash ..\OVMF.fd -parallel file:para.txt

Remember that I am running this in WinXPSP3.  Also, the version of
OVMF.fd is from http://www.tianocore.org/ovmf/ and I just noticed
that that page has been recently changed.  You use to be able to 
download the OVMF.fd directly from that page.  It was r15214, which 
I found out yesterday, is an older version.

> >  I did find a few RPM files and one of them contained:
> >   OVMF_CODE-pure-efi.fd
> >  and
> >   OVMF_VARS-pure-efi.fd
> 
> Yes, they are from Gerd's "edk2.git-ovmf-ia32" package (since you
> mention IA32), from , and they are
> recommended to use.

I also could not get these to work.  I will do a little more 
research into why not and be sure to let you know.

> Otherwise, download the "edk2.git-ovmf-ia32" or "edk2.git-ovmf-x64" RPM
> file (as needed) from , and
> extract it with:
> 
>   rpm2cpio FILENAME | pax -r -v
> 
> The extracted files you want are:
> 
>   usr/share/edk2.git/ovmf-*/OVMF_CODE-pure-efi.fd
>   usr/share/edk2.git/ovmf-*/OVMF_VARS-pure-efi.fd
>   usr/share/edk2.git/ovmf-*/UefiShell.iso

These are the three files I used and could not get the shell to load.
I will try a few other things and get back with you.

Please note that I am using the Windows port of QEMU from 
https://qemu.weilnetz.de/ on a WinXP host, if that makes any 
difference.

Also, one more question.  I can make my GPT image either have a legacy
MBR with two entries pointing to one partition each or I can have a
Protected MBR that encompasses the whole GPT, both partitions, and the
backup items.

With the r15214 version and:

"..\qemu-system-i386w.exe" -m 256 -localtime 
  -drive file=uefi.bin.vhd,format=raw,if=ide,media=disk,index=0 
  -bios ..\OVMF.fd -parallel file:para.txt

it would boot to the shell either way and load and run my BOOT.efi file
just fine, whether I had a Legacy MBR or a PMBR.  Does this make a
difference with the new image(s) you mention above.

Also, please note that the hard drive image is a VHD image, a
raw image with a single sector at the end for the VHD info so that
Oracles VM Box will boot it.  This means that the backup GPT header
is *not* the last sector of the disk.  The

  -bios OVMF.fd

version boots it just fine in QEMU, but I thought I would mention
this in case the new code expects it to be in the last sector.

Thank you.  I will get back with you on my findings.

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

Re: [edk2] OVMF.fd and placement of EfiBootServicesData

[spam collector]

- Original Message -
> From: "Laszlo Ersek" 
> To: "spam collector" 
> Cc: edk2-de...@ml01.01.org
> Sent: Wednesday, October 5, 2016 8:28:46 AM
> Subject: Re: [edk2] OVMF.fd and placement of EfiBootServicesData
>
> I recommend trying the following (32-bit command line), with Gerd's package:
> 
>   FW_BIN=/usr/share/edk2.git/ovmf-ia32/OVMF_CODE-pure-efi.fd
>   VARS_TMPL=/usr/share/edk2.git/ovmf-ia32/OVMF_VARS-pure-efi.fd
>   VARS=myvars.fd
>   SHELL_ISO=/usr/share/edk2.git/ovmf-ia32/UefiShell.iso
>   if ! [ -e "$VARS" ]; then
> cp -v -- "$VARS_TMPL" "$VARS"
>   fi
> 
>   qemu-system-i386 \
> -m 2048 \
> -machine accel=kvm \
> \
> -drive if=pflash,format=raw,unit=0,readonly,file="$FW_BIN" \
> -drive if=pflash,format=raw,unit=1,file="$VARS" \
> \
> -device virtio-scsi-pci,id=scsi0 \
> -drive if=none,format=raw,readonly,file="$SHELL_ISO",id=shell \
> -device scsi-cd,bus=scsi0.0,drive=shell,bootindex=1 \
> \
> -chardev file,id=debugfile,path=ovmf.log \
> -device isa-debugcon,iobase=0x402,chardev=debugfile
> 
> This will boot the shell for you, and even save the OVMF debug log in
> the file "ovmf.log".

I used the following command line (again, a WinXP DOS Session .BAT file):

"..\qemu-system-i386w.exe" -m 256 -drive 
if=pflash,format=raw,unit=0,readonly,file=..\OVMF_CODE.fd -drive 
if=pflash,format=raw,unit=1,file=..\OVMF_VARS.fd -device 
virtio-scsi-pci,id=scsi0 -drive 
if=none,format=raw,readonly,file=..\UefiShell.iso,id=shell -device 
scsi-cd,bus=scsi0.0,drive=shell,bootindex=1 
-chardev file,id=debugfile,path=ovmf.log -device 
isa-debugcon,iobase=0x402,chardev=debugfile -drive 
file=uefi.bin.vhd,format=raw,if=ide,media=disk,index=0

The shell booted up as expected.  However, my BOOT.efi file is now a:

  "image is not an application"

I don't think I mentioned before, my BOOT.efi is an OS loader, not 
a UEFI driver or other type application.  Sorry, I should have mentioned
that earlier, hope that did not confuse you.

My BOOT.efi file has a value of 0x0B as the Subsystem value.

   PE Optional Header: Subsystem: 0x000B

I removed the UefiShell.iso part:

"..\qemu-system-i386w.exe" -m 256 -drive 
if=pflash,format=raw,unit=0,readonly,file=..\OVMF_CODE.fd -drive 
if=pflash,format=raw,unit=1,file=..\OVMF_VARS.fd -device 
virtio-scsi-pci,id=scsi0 -chardev 
file,id=debugfile,path=ovmf.log -device 
isa-debugcon,iobase=0x402,chardev=debugfile -drive 
file=uefi.bin.vhd,format=raw,if=ide,media=disk,index=0

and the shell was loaded, with the same result, "not an application".

Anyway, thanks for your help.  I will figure this out, I am sure of it :-)

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