subject:"\[edk2\] portability of ShellPkg"

Re: [edk2] portability of ShellPkg

2018-09-06 Thread Andrew Fish

Mike,

More possible ideas

I think from a 10,000 foot level the issue is the Public definition of the HOB 
lib calls out the ASSERT behavior 

5) Make the ASSERT() conditional via a PCD and update the HOB lib definition. 
For platforms that make HOBs options GetHobList() must be called 1st, and non 
of the other APIs can be called on platforms that don't support HOBs. That has 
the upside of only needing to make the Lib constructor and GetHobList() ASSERTs 
conditional onPCDs. 

6) Split the existing UefiBootManagerLib in a backwards compatible way by 
making a new UefiBootLib. UefiBootManagerLib.h can include UefiBootLib.h and 
UefiBootManagerLib.inf could depend on UefiBootLib. This would only require 
adding UefiBootLib to the library class of existing DSC files. 

Thanks,

Andrew Fish

> On Sep 6, 2018, at 10:17 AM, Kinney, Michael D  
> wrote:
> 
> Ray,
> 
> I have a few ideas here.
> 
> 1) Add a new UefiHobLib instance that is only for 
>   use by UEFI Drivers and UEFI Applications.  It fails
>   gracefully if the HOB List is not in the UEFI System
>   Configuration Table.  BootMode if HobList is not present
>   can be BOOT_WITH_FULL_CONFIGURATION.
> 
> 2) Remove ASSERT() from the CONSTRUCTOR and a return
>   NULL from the Get*() functions if mHobList is NULL.
>   BootMode can be BOOT_WITH_FULL_CONFIGURATION if
>   mHobList is NULL.
> 
> 3) Update UEFI Shell to not use the UefiBootManagerLib.
>   This would add duplicate code to the UEFI Shell.
> 
> 4) Update UefiBootManagerLib to not use the HobLib.
>   Instead look for HobList in UEFI System Configuration
>   Table and fail gracefully if it is not present.
>   This would add some duplicate code to the 
>   UefiBootManagerLib to parse the Hob List.
> 
> Best regards,
> 
> Mike
> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, September 6, 2018 2:57 AM
>> To: Ni, Ruiyu ; Andrew Fish
>> 
>> Cc: Leif Lindholm ; edk2-
>> de...@lists.01.org; Carsey, Jaben
>> ; Alexander Graf
>> ; Heinrich Schuchardt
>> ; AKASHI Takahiro
>> ; Kinney, Michael D
>> 
>> Subject: Re: portability of ShellPkg
>> 
>> On 09/06/18 04:34, Ni, Ruiyu wrote:
>>> On 9/6/2018 3:47 AM, Andrew Fish wrote:
 
 Laszlo,
 
 gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg
>> concept used to
 give the DXE Core hints on how to reduce
>> fragmentation in the memory
 map. Typically there is code in PEI that creates a
>> HOB and may consume
 a variable written by the BDS. This library seems to
>> be the generic
 way to do it on an edk2 platform.
 
 Thus this library is not just PI but edk2
>> MdeModulePkg specific in
 some of its assumptions. In general this
>> UefiBootManagerLib seems
 focused on construction and edk2 BDS. It would
>> probably make more
 sense to break out the UEFI Spec related bits so they
>> could be used in
 generic UEFI Applications.
 
 Thanks,
 
 Andrew Fish
 
 
>>> 
>>> Andrew,
>>> I agree refactoring UefiBootManagerLib to separate the
>> pure UEFI, pure
>>> PI and EDKII specific looks much more cleaner.
>>> So far the PI related bits in UefiBootManagerLib may
>> include:
>>> 1. Hob access for S4 support (memory type information
>> HOB).
>>> 2. Boot from Firmware Volume support.
>>> 
>>> But that requires introducing two or more library
>> classes so affecting
>>> all existing platforms. EfiCreateEventLegacyBootEx()
>> in UefiLib also
>>> touches PI event gEfiEventLegacyBootGuid.
>>> 
>>> And I think the value of refactor might be small.
>>> 
>>> I think root cause of this problem is not
>> UefiBootManagerLib includes
>>> PI, it's the assertion in DxeHobLib.
>>> So I am thinking maybe a very light fix is to remove
>> the constructor of
>>> DxeHobLib.
>>> 
>>> I talked with Liming about this and he suggested that
>> instead of
>>> removing the constructor, it's safer to just remove
>> the assertion in the
>>> constructor. Because removing the constructor of
>> HobLib may cause
>>> AutoGen process generates a different order of library
>> constructors
>>> calling sequence, which may break the platform.
>>> 
>>> So I propose to just remove the assertion in DxeHobLib
>> constructor.
>>> Thoughts?
>> 
>> I think keeping the constructor itself is important and
>> a good idea.
>> 
>> I also think that we should *perhaps* keep the assertion
>> *somewhere*,
>> just not in the constructor. Because, at least some of
>> the HobLib APIs
>> cannot return errors (and their callers expect them to
>> succeed at all
>> times). This suggests we should still trip assertions
>> when a HobLib API
>> is called *in practice* on a non-PI UEFI platform.
>> 
>> Thanks
>> Laszlo

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

Re: [edk2] portability of ShellPkg

2018-09-06 Thread Kinney, Michael D

Ray,

I have a few ideas here.

1) Add a new UefiHobLib instance that is only for 
   use by UEFI Drivers and UEFI Applications.  It fails
   gracefully if the HOB List is not in the UEFI System
   Configuration Table.  BootMode if HobList is not present
   can be BOOT_WITH_FULL_CONFIGURATION.

2) Remove ASSERT() from the CONSTRUCTOR and a return
   NULL from the Get*() functions if mHobList is NULL.
   BootMode can be BOOT_WITH_FULL_CONFIGURATION if
   mHobList is NULL.

3) Update UEFI Shell to not use the UefiBootManagerLib.
   This would add duplicate code to the UEFI Shell.

4) Update UefiBootManagerLib to not use the HobLib.
   Instead look for HobList in UEFI System Configuration
   Table and fail gracefully if it is not present.
   This would add some duplicate code to the 
   UefiBootManagerLib to parse the Hob List.

Best regards,

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, September 6, 2018 2:57 AM
> To: Ni, Ruiyu ; Andrew Fish
> 
> Cc: Leif Lindholm ; edk2-
> de...@lists.01.org; Carsey, Jaben
> ; Alexander Graf
> ; Heinrich Schuchardt
> ; AKASHI Takahiro
> ; Kinney, Michael D
> 
> Subject: Re: portability of ShellPkg
> 
> On 09/06/18 04:34, Ni, Ruiyu wrote:
> > On 9/6/2018 3:47 AM, Andrew Fish wrote:
> >>
> >> Laszlo,
> >>
> >> gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg
> concept used to
> >> give the DXE Core hints on how to reduce
> fragmentation in the memory
> >> map. Typically there is code in PEI that creates a
> HOB and may consume
> >> a variable written by the BDS. This library seems to
> be the generic
> >> way to do it on an edk2 platform.
> >>
> >> Thus this library is not just PI but edk2
> MdeModulePkg specific in
> >> some of its assumptions. In general this
> UefiBootManagerLib seems
> >> focused on construction and edk2 BDS. It would
> probably make more
> >> sense to break out the UEFI Spec related bits so they
> could be used in
> >> generic UEFI Applications.
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>
> >
> > Andrew,
> > I agree refactoring UefiBootManagerLib to separate the
> pure UEFI, pure
> > PI and EDKII specific looks much more cleaner.
> > So far the PI related bits in UefiBootManagerLib may
> include:
> > 1. Hob access for S4 support (memory type information
> HOB).
> > 2. Boot from Firmware Volume support.
> >
> > But that requires introducing two or more library
> classes so affecting
> > all existing platforms. EfiCreateEventLegacyBootEx()
> in UefiLib also
> > touches PI event gEfiEventLegacyBootGuid.
> >
> > And I think the value of refactor might be small.
> >
> > I think root cause of this problem is not
> UefiBootManagerLib includes
> > PI, it's the assertion in DxeHobLib.
> > So I am thinking maybe a very light fix is to remove
> the constructor of
> > DxeHobLib.
> >
> > I talked with Liming about this and he suggested that
> instead of
> > removing the constructor, it's safer to just remove
> the assertion in the
> > constructor. Because removing the constructor of
> HobLib may cause
> > AutoGen process generates a different order of library
> constructors
> > calling sequence, which may break the platform.
> >
> > So I propose to just remove the assertion in DxeHobLib
> constructor.
> > Thoughts?
> 
> I think keeping the constructor itself is important and
> a good idea.
> 
> I also think that we should *perhaps* keep the assertion
> *somewhere*,
> just not in the constructor. Because, at least some of
> the HobLib APIs
> cannot return errors (and their callers expect them to
> succeed at all
> times). This suggests we should still trip assertions
> when a HobLib API
> is called *in practice* on a non-PI UEFI platform.
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] portability of ShellPkg

2018-09-06 Thread Laszlo Ersek

On 09/06/18 04:34, Ni, Ruiyu wrote:
> On 9/6/2018 3:47 AM, Andrew Fish wrote:
>>
>> Laszlo,
>>
>> gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg concept used to
>> give the DXE Core hints on how to reduce fragmentation in the memory
>> map. Typically there is code in PEI that creates a HOB and may consume
>> a variable written by the BDS. This library seems to be the generic
>> way to do it on an edk2 platform.
>>
>> Thus this library is not just PI but edk2 MdeModulePkg specific in
>> some of its assumptions. In general this UefiBootManagerLib seems
>> focused on construction and edk2 BDS. It would probably make more
>> sense to break out the UEFI Spec related bits so they could be used in
>> generic UEFI Applications.
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>
> 
> Andrew,
> I agree refactoring UefiBootManagerLib to separate the pure UEFI, pure
> PI and EDKII specific looks much more cleaner.
> So far the PI related bits in UefiBootManagerLib may include:
> 1. Hob access for S4 support (memory type information HOB).
> 2. Boot from Firmware Volume support.
> 
> But that requires introducing two or more library classes so affecting
> all existing platforms. EfiCreateEventLegacyBootEx() in UefiLib also
> touches PI event gEfiEventLegacyBootGuid.
> 
> And I think the value of refactor might be small.
> 
> I think root cause of this problem is not UefiBootManagerLib includes
> PI, it's the assertion in DxeHobLib.
> So I am thinking maybe a very light fix is to remove the constructor of
> DxeHobLib.
> 
> I talked with Liming about this and he suggested that instead of
> removing the constructor, it's safer to just remove the assertion in the
> constructor. Because removing the constructor of HobLib may cause
> AutoGen process generates a different order of library constructors
> calling sequence, which may break the platform.
> 
> So I propose to just remove the assertion in DxeHobLib constructor.
> Thoughts?

I think keeping the constructor itself is important and a good idea.

I also think that we should *perhaps* keep the assertion *somewhere*,
just not in the constructor. Because, at least some of the HobLib APIs
cannot return errors (and their callers expect them to succeed at all
times). This suggests we should still trip assertions when a HobLib API
is called *in practice* on a non-PI UEFI platform.

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

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Ni, Ruiyu


On 9/6/2018 3:47 AM, Andrew Fish wrote:


Laszlo,

gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg concept used to 
give the DXE Core hints on how to reduce fragmentation in the memory 
map. Typically there is code in PEI that creates a HOB and may consume a 
variable written by the BDS. This library seems to be the generic way to 
do it on an edk2 platform.


Thus this library is not just PI but edk2 MdeModulePkg specific in some 
of its assumptions. In general this UefiBootManagerLib seems focused on 
construction and edk2 BDS. It would probably make more sense to break 
out the UEFI Spec related bits so they could be used in generic UEFI 
Applications.


Thanks,

Andrew Fish




Andrew,
I agree refactoring UefiBootManagerLib to separate the pure UEFI, pure
PI and EDKII specific looks much more cleaner.
So far the PI related bits in UefiBootManagerLib may include:
1. Hob access for S4 support (memory type information HOB).
2. Boot from Firmware Volume support.

But that requires introducing two or more library classes so affecting
all existing platforms. EfiCreateEventLegacyBootEx() in UefiLib also 
touches PI event gEfiEventLegacyBootGuid.


And I think the value of refactor might be small.

I think root cause of this problem is not UefiBootManagerLib includes
PI, it's the assertion in DxeHobLib.
So I am thinking maybe a very light fix is to remove the constructor of
DxeHobLib.

I talked with Liming about this and he suggested that instead of 
removing the constructor, it's safer to just remove the assertion in the 
constructor. Because removing the constructor of HobLib may cause 
AutoGen process generates a different order of library constructors 
calling sequence, which may break the platform.


So I propose to just remove the assertion in DxeHobLib constructor.
Thoughts?



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

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Carsey, Jaben

But that is the line Leif wants to remove... the library from the components 
section.  Looks just like MdePkg to me... what is different?

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Wednesday, September 05, 2018 11:33 AM
> To: Carsey, Jaben 
> Cc: Ni, Ruiyu ; Heinrich Schuchardt
> ; edk2-devel@lists.01.org; Alexander Graf
> ; AKASHI Takahiro ; Kinney,
> Michael D ; Laszlo Ersek
> 
> Subject: Re: [edk2] portability of ShellPkg
> Importance: High
> 
> Jaben,
> 
> Sorry to test compile a library just list the library in the [Components] 
> section
> of the DSC file. MdePkg/MdePkg.dsc is an example of this.
> 
> Thanks,
> 
> Andrew Fish
> 
> > On Sep 5, 2018, at 11:23 AM, Carsey, Jaben 
> wrote:
> >
> > Aha. So that is very different from a non NULL library when listed in the
> components section.  The goal is to test compile the library but not use I
> think.  Is there any way to do that?
> >
> > Jaben
> >
> > On Sep 5, 2018, at 11:21 AM, Andrew Fish  <mailto:af...@apple.com>> wrote:
> >
> >>
> >>
> >>> On Sep 5, 2018, at 11:05 AM, Carsey, Jaben  <mailto:jaben.car...@intel.com>> wrote:
> >>>
> >>> But a NULL lib listed in components section shouldn’t be linked in to
> anything...
> >>>
> >>
> >> Jaben,
> >>
> >> A NULL library class means force it to be linked in.
> >>
> >> ShellPkg/ShellPkg.dsc:70:  # [LibraryClasses.ARM] and NULL mean link this
> library into all ARM images.
> >> ShellPkg/ShellPkg.dsc:72:
> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >> ShellPkg/ShellPkg.dsc:75:
> NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> >> ShellPkg/ShellPkg.dsc:78:
> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> >> ShellPkg/ShellPkg.dsc:110:
> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Comma
> ndsLib.inf
> >> ShellPkg/ShellPkg.dsc:111:
> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1Comma
> ndsLib.inf
> >> ShellPkg/ShellPkg.dsc:112:
> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3Comma
> ndsLib.inf
> >> ShellPkg/ShellPkg.dsc:114:
> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Com
> mandsLib.inf
> >> ShellPkg/ShellPkg.dsc:115:
> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1Com
> mandsLib.inf
> >> ShellPkg/ShellPkg.dsc:116:
> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Com
> mandsLib.inf
> >> ShellPkg/ShellPkg.dsc:117:
> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1
> CommandsLib.inf
> >> ShellPkg/ShellPkg.dsc:118:
> NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2
> CommandsLib.inf
> >>
> >> Libraries can get pulled in via other libraries. The only way to tell for 
> >> sure is
> to look at the build report.
> >>
> >> $ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
> >> $ cat report.txt | grep HobLib
> >> /Volumes/Case/UDK2018/MdePkg/Library/DxeHobLib/DxeHobLib.inf
> >> {HobLib:  C = HobLibConstructor Time = 19ms}
> >>
> >> You can comment out the HobLib reference in the ShellPkg.dsc file and
> figure out who is using it "
> ffHobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf"
> >> $ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
> >> ...
> >> build.py...
> >> /Volumes/Case/UDK2018/ShellPkg/ShellPkg.dsc(...): error 4000: Instance
> of library class [HobLib] is not found
> >> in
> [/Volumes/Case/UDK2018/MdeModulePkg/Library/UefiBootManagerLib/Ue
> fiBootManagerLib.inf] [X64]
> >> consumed by module
> [/Volumes/Case/UDK2018/ShellPkg/Application/Shell/Shell.inf]
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>> Unless is listed below with the shell INF also, it just test compiles 
> >>> it...
> >>>
> >>> Or so I thought.
> >>>
> >>> On Sep 5, 2018, at 11:04 AM, Andrew Fish  <mailto:af...@apple.com>> wrote:
> >>>
> >>>>
> >>>>
> >>>>> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben  <mailto:jaben.car...@intel.com>> wrote:
> >>>>>
> >>>>> How does removing a lib from the components section affect the shell
> binary output?
> >>

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Laszlo Ersek

On 09/05/18 19:25, Leif Lindholm 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?

Such errors can be narrowed down by removing the resolution altogether
from the DSC file, for the library class in question. Then the build
will fail and the error message will report the dependency chain.

With the following patch:

> diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc
> index 59dd07e0ae62..ebd7f23a8bee 100644
> --- a/ShellPkg/ShellPkg.dsc
> +++ b/ShellPkg/ShellPkg.dsc
> @@ -58,7 +58,6 @@ [LibraryClasses.common]
>IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
>
>
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
>
> DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
>DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf

and the following command:

 build -a X64 -p ShellPkg/ShellPkg.dsc \
   -m ShellPkg/Application/Shell/Shell.inf

(obviously one can use any other suitable architecture with "-a"), the
error is:

> ShellPkg/ShellPkg.dsc(...): error 4000: Instance of library class [HobLib] is 
> not found
> in [MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf] 
> [X64]
> consumed by module [ShellPkg/Application/Shell/Shell.inf]

Some parts still have to be filled in manually (to "meet in the
middle"), but ultimately, the dependencies go like this:

(1) UefiShellDebug1CommandsLib.inf depends on the BcfgCommandLib class,
for implementing the BCFG shell command (as required by the shell
spec, for the Debug1 (and Install1) profiles.

(2) ShellPkg.dsc resolves the BcfgCommandLib class to the sole instance
in edk2, namely
"ShellPkg/Library/UefiShellBcfgCommandLib/UefiShellBcfgCommandLib.inf".

(3) This library instance depends on the UefiBootManagerLib class (for
the EfiBootManagerVariableToLoadOption(),
EfiBootManagerLoadOptionToVariable(), and
EfiBootManagerFreeLoadOption() APIs).

(4) The edk2 tree provides one instance of the UefiBootManagerLib class,
and ShellPkg.dsc indeed contains that resolution: namely to
"MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf".

(5) This library instance depends on the HobLib class. (More on this
later.)

(6) ShellPkg.dsc resolves the HobLib class to

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Andrew Fish

Jaben,

Sorry to test compile a library just list the library in the [Components] 
section of the DSC file. MdePkg/MdePkg.dsc is an example of this. 

Thanks,

Andrew Fish

> On Sep 5, 2018, at 11:23 AM, Carsey, Jaben  wrote:
> 
> Aha. So that is very different from a non NULL library when listed in the 
> components section.  The goal is to test compile the library but not use I 
> think.  Is there any way to do that?
> 
> Jaben
> 
> On Sep 5, 2018, at 11:21 AM, Andrew Fish  > wrote:
> 
>> 
>> 
>>> On Sep 5, 2018, at 11:05 AM, Carsey, Jaben >> > wrote:
>>> 
>>> But a NULL lib listed in components section shouldn’t be linked in to 
>>> anything...
>>> 
>> 
>> Jaben,
>> 
>> A NULL library class means force it to be linked in. 
>> 
>> ShellPkg/ShellPkg.dsc:70:  # [LibraryClasses.ARM] and NULL mean link this 
>> library into all ARM images.
>> ShellPkg/ShellPkg.dsc:72:  
>> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>> ShellPkg/ShellPkg.dsc:75:  
>> NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> ShellPkg/ShellPkg.dsc:78:  
>> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>> ShellPkg/ShellPkg.dsc:110:  
>> NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:111:  
>> NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:112:  
>> NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:114:  
>> NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:115:  
>> NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:116:  
>> NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:117:  
>> NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
>> ShellPkg/ShellPkg.dsc:118:  
>> NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf
>> 
>> Libraries can get pulled in via other libraries. The only way to tell for 
>> sure is to look at the build report. 
>> 
>> $ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
>> $ cat report.txt | grep HobLib
>> /Volumes/Case/UDK2018/MdePkg/Library/DxeHobLib/DxeHobLib.inf
>> {HobLib:  C = HobLibConstructor Time = 19ms}
>> 
>> You can comment out the HobLib reference in the ShellPkg.dsc file and figure 
>> out who is using it " ffHobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf"
>> $ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
>> ...
>> build.py...
>> /Volumes/Case/UDK2018/ShellPkg/ShellPkg.dsc(...): error 4000: Instance of 
>> library class [HobLib] is not found
>> in 
>> [/Volumes/Case/UDK2018/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf]
>>  [X64]
>> consumed by module 
>> [/Volumes/Case/UDK2018/ShellPkg/Application/Shell/Shell.inf]
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> Unless is listed below with the shell INF also, it just test compiles it...
>>> 
>>> Or so I thought.
>>> 
>>> On Sep 5, 2018, at 11:04 AM, Andrew Fish >> > wrote:
>>> 
 
 
> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben  > 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.

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Carsey, Jaben

Aha. So that is very different from a non NULL library when listed in the 
components section.  The goal is to test compile the library but not use I 
think.  Is there any way to do that?

Jaben

On Sep 5, 2018, at 11:21 AM, Andrew Fish 
mailto:af...@apple.com>> wrote:



On Sep 5, 2018, at 11:05 AM, Carsey, Jaben 
mailto:jaben.car...@intel.com>> wrote:

But a NULL lib listed in components section shouldn’t be linked in to 
anything...


Jaben,

A NULL library class means force it to be linked in.

ShellPkg/ShellPkg.dsc:70:  # [LibraryClasses.ARM] and NULL mean link this 
library into all ARM images.
ShellPkg/ShellPkg.dsc:72:  
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
ShellPkg/ShellPkg.dsc:75:  
NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
ShellPkg/ShellPkg.dsc:78:  
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
ShellPkg/ShellPkg.dsc:110:  
NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
ShellPkg/ShellPkg.dsc:111:  
NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
ShellPkg/ShellPkg.dsc:112:  
NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
ShellPkg/ShellPkg.dsc:114:  
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
ShellPkg/ShellPkg.dsc:115:  
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
ShellPkg/ShellPkg.dsc:116:  
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
ShellPkg/ShellPkg.dsc:117:  
NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
ShellPkg/ShellPkg.dsc:118:  
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf

Libraries can get pulled in via other libraries. The only way to tell for sure 
is to look at the build report.

$ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
$ cat report.txt | grep HobLib
/Volumes/Case/UDK2018/MdePkg/Library/DxeHobLib/DxeHobLib.inf
{HobLib:  C = HobLibConstructor Time = 19ms}

You can comment out the HobLib reference in the ShellPkg.dsc file and figure 
out who is using it " ffHobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf"
$ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
...
build.py...
/Volumes/Case/UDK2018/ShellPkg/ShellPkg.dsc(...): error 4000: Instance of 
library class [HobLib] is not found
in 
[/Volumes/Case/UDK2018/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf]
 [X64]
consumed by module [/Volumes/Case/UDK2018/ShellPkg/Application/Shell/Shell.inf]

Thanks,

Andrew Fish

Unless is listed below with the shell INF also, it just test compiles it...

Or so I thought.

On Sep 5, 2018, at 11:04 AM, Andrew Fish 
mailto:af...@apple.com>> wrote:



On Sep 5, 2018, at 10:30 AM, Carsey, Jaben 
mailto:jaben.car...@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 (, );
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

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Andrew Fish

> On Sep 5, 2018, at 11:05 AM, Carsey, Jaben  wrote:
> 
> But a NULL lib listed in components section shouldn’t be linked in to 
> anything...
> 

Jaben,

A NULL library class means force it to be linked in. 

ShellPkg/ShellPkg.dsc:70:  # [LibraryClasses.ARM] and NULL mean link this 
library into all ARM images.
ShellPkg/ShellPkg.dsc:72:  
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
ShellPkg/ShellPkg.dsc:75:  
NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
ShellPkg/ShellPkg.dsc:78:  
NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
ShellPkg/ShellPkg.dsc:110:  
NULL|ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.inf
ShellPkg/ShellPkg.dsc:111:  
NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLib.inf
ShellPkg/ShellPkg.dsc:112:  
NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLib.inf
ShellPkg/ShellPkg.dsc:114:  
NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.inf
ShellPkg/ShellPkg.dsc:115:  
NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib.inf
ShellPkg/ShellPkg.dsc:116:  
NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf
ShellPkg/ShellPkg.dsc:117:  
NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.inf
ShellPkg/ShellPkg.dsc:118:  
NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib.inf

Libraries can get pulled in via other libraries. The only way to tell for sure 
is to look at the build report. 

$ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
$ cat report.txt | grep HobLib
/Volumes/Case/UDK2018/MdePkg/Library/DxeHobLib/DxeHobLib.inf
{HobLib:  C = HobLibConstructor Time = 19ms}

You can comment out the HobLib reference in the ShellPkg.dsc file and figure 
out who is using it " ffHobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf"
$ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 --report-file=report.txt
...
build.py...
/Volumes/Case/UDK2018/ShellPkg/ShellPkg.dsc(...): error 4000: Instance of 
library class [HobLib] is not found
in 
[/Volumes/Case/UDK2018/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf]
 [X64]
consumed by module 
[/Volumes/Case/UDK2018/ShellPkg/Application/Shell/Shell.inf]

Thanks,

Andrew Fish

> Unless is listed below with the shell INF also, it just test compiles it...
> 
> Or so I thought.
> 
> On Sep 5, 2018, at 11:04 AM, Andrew Fish  > wrote:
> 
>> 
>> 
>>> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben >> > 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 (, );
>> 
>> 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,
>>

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Carsey, Jaben

But a NULL lib listed in components section shouldn’t be linked in to 
anything...

Unless is listed below with the shell INF also, it just test compiles it...

Or so I thought.

On Sep 5, 2018, at 11:04 AM, Andrew Fish 
mailto:af...@apple.com>> wrote:



On Sep 5, 2018, at 10:30 AM, Carsey, Jaben 
mailto:jaben.car...@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 (, );
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 
mailto:leif.lindh...@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

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Andrew Fish

> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben  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 (, );
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_HANDLEImageHandle,
  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  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
>>

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Leif Lindholm

On Wed, Sep 05, 2018 at 05:30:23PM +, Carsey, Jaben wrote:
> How does removing a lib from the components section affect the shell binary 
> output?

Maybe it doesn't and I'm barking up the wrong tree? Unfortunately,
the only thing that means is we don't have a trivial workaround.

/
Leif

> Is the asset at compile time?
> 
> Jaben
> 
> > On Sep 5, 2018, at 10:26 AM, Leif Lindholm  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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] portability of ShellPkg

2018-09-05 Thread Carsey, Jaben

How does removing a lib from the components section affect the shell binary 
output?

Is the asset at compile time?

Jaben

> On Sep 5, 2018, at 10:26 AM, Leif Lindholm  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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] portability of ShellPkg

2018-09-05 Thread Leif Lindholm

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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

Re: [edk2] portability of ShellPkg

[edk2] portability of ShellPkg

14 matches

Site Navigation

Mail list logo

Footer information