Re: portability of ShellPkg

[Andrew Fish <afish@apple.com>]

> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben <jaben.carsey@intel.com> wrote:
> 
> How does removing a lib from the components section affect the shell binary output?
> 
> Is the asset at compile time?

Jaben,

The issue is likely with the HOB lib constructor in the Shell iASSERTing. Leif's example platform supports UEFI, but not PI so it is not expected that HOBs exist. 

The library has an explicit assumption that HOBs exist, and that is not the case in Leif's platform. 

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/DxeHobLib/HobLib.c#L54

VOID  *mHobList = NULL;

/**
  Returns the pointer to the HOB list.
  This function returns the pointer to first HOB in the list.
  For PEI phase, the PEI service GetHobList() can be used to retrieve the pointer
  to the HOB list.  For the DXE phase, the HOB list pointer can be retrieved through
  the EFI System Table by looking up theHOB list GUID in the System Configuration Table.
  Since the System Configuration Table does not exist that the time the DXE Core is
  launched, the DXE Core uses a global variable from the DXE Core Entry Point Library
  to manage the pointer to the HOB list.
  If the pointer to the HOB list is NULL, then ASSERT().
  This function also caches the pointer to the HOB list retrieved.
  @return The pointer to the HOB list.
**/
VOID *
EFIAPI
GetHobList (
  VOID
  )
{
  EFI_STATUS  Status;

  if (mHobList == NULL) {
    Status = EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList);
    ASSERT_EFI_ERROR (Status);
    ASSERT (mHobList != NULL);
  }
  return mHobList;
}

/**
  The constructor function caches the pointer to HOB list by calling GetHobList()
  and will always return EFI_SUCCESS.
  @param  ImageHandle   The firmware allocated handle for the EFI image.
  @param  SystemTable   A pointer to the EFI System Table.
  @retval EFI_SUCCESS   The constructor successfully gets HobList.
**/
EFI_STATUS
EFIAPI
HobLibConstructor (
  IN EFI_HANDLE        ImageHandle,
  IN EFI_SYSTEM_TABLE  *SystemTable
  )
{
  GetHobList ();

  return EFI_SUCCESS;
}


/**
  Returns the next instance of a HOB type from the starting HOB.
  This function searches the first instance of a HOB type from the starting HOB pointer.
  If there does not exist such HOB type from the starting HOB pointer, it will return NULL.
  In contrast with macro GET_NEXT_HOB(), this function does not skip the starting HOB pointer
  unconditionally: it returns HobStart back if HobStart itself meets the requirement;
  caller is required to use GET_NEXT_HOB() if it wishes to skip current HobStart.
  If HobStart is NULL, then ASSERT().
  @param  Type          The HOB type to return.
  @param  HobStart      The starting HOB pointer to search from.
  @return The next instance of a HOB type from the starting HOB.
**/
VOID *
EFIAPI
GetNextHob (
  IN UINT16                 Type,
  IN CONST VOID             *HobStart
  )
{
  EFI_PEI_HOB_POINTERS  Hob;

  ASSERT (HobStart != NULL);

  Hob.Raw = (UINT8 *) HobStart;
  //
  // Parse the HOB list until end of list or matching type is found.
  //
  while (!END_OF_HOB_LIST (Hob)) {
    if (Hob.Header->HobType == Type) {
      return Hob.Raw;
    }
    Hob.Raw = GET_NEXT_HOB (Hob);
  }
  return NULL;
}

Thanks,

Andrew Fish

> 
> Jaben
> 
>> On Sep 5, 2018, at 10:26 AM, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> 
>> Hi all,
>> 
>> (This is partly a summary of discussions that have been held on IRC
>> and offline, with Alex Graf and Mike Kinney.)
>> 
>> The UEFI Shell, as produced by the contents of ShellPkg, is needed for
>> running the UEFI SCT. This has never been problematic before - but now
>> we are starting to run SCT on the U-Boot implementation of the UEFI
>> interfaces, certain implicit assumptions may need to be made explicit,
>> and perhaps reevaluated.
>> 
>> My feeling is the following:
>> - The MinUefiShell variant should be sufficient to run SCT.
>> - The UEFI Shell as provided by ShellPkg (any flavour) should run on
>> any valid UEFI implementation. Where underlying functionality is
>> missing for certain commands, those commands should be
>> degraded/disabled to let remaining commands function.
>> 
>> Ideally, I would like to see a Readme.md in ShellPkg, basically
>> providing a mission statement. I could write one, but I expect the
>> people who actually maintain it would be better suited :)
>> 
>> We currently have an issue with running the shell on U-Boot because
>> even MinUefiShell pulls in UefiShellDebug1CommandsLib.inf. This
>> appears to be inadvertent, since it is also included a few lines
>> further down inside an !ifndef $(NO_SHELL_PROFILES) guard.
>> So I would propose the following patch (and can send it out properly
>> if the maintainers agree):
>> 
>> diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
>> index 59dd07e0ae..c852abd3f7 100644
>> --- a/ShellPkg/ShellPkg.dsc
>> +++ b/ShellPkg/ShellPkg.dsc
>> @@ -101,7 +101,6 @@ [Components]
>>  ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>>  ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>>  ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>> -  ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>>  ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>>  ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
>> 
>> The reason this causes a problem is because this module has a
>> dependency on HobLib, which ASSERTS if it does not find any HOBs lying
>> around. Since HOBs are a PI concept rather than a UEFI concept,
>> ideally we would not terminate the shell if they are missing. However,
>> since the HobLib is generic to EDK2, we also shouldn't just go
>> stripping ASSERTs out of it. The above patch gives us a way of
>> unblocking the SCT on U-Boot UEFI while we consider what to do about
>> the bigger question.
>> 
>> Thoughts?
>> 
>> /
>>   Leif