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 <jaben.car...@intel.com> > Cc: Ni, Ruiyu <ruiyu...@intel.com>; Heinrich Schuchardt > <xypron.g...@gmx.de>; edk2-devel@lists.01.org; Alexander Graf > <ag...@suse.de>; AKASHI Takahiro <takahiro.aka...@linaro.org>; Kinney, > Michael D <michael.d.kin...@intel.com>; Laszlo Ersek > <ler...@redhat.com> > 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 <jaben.car...@intel.com> > 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 <af...@apple.com > <mailto:af...@apple.com>> wrote: > > > >> > >> > >>> On Sep 5, 2018, at 11:05 AM, Carsey, Jaben <jaben.car...@intel.com > <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 <af...@apple.com > <mailto:af...@apple.com>> wrote: > >>> > >>>> > >>>> > >>>>> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben <jaben.car...@intel.com > <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/DxeHobLi > b/HobLib.c#L54 > <https://github.com/tianocore/edk2/blob/master/MdePkg/Library/DxeHob > Lib/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.lindh...@linaro.org > <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 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/UefiShellDriver1CommandsL > ib.inf > >>>>>> > ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLi > b.inf > >>>>>> - > ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1Commands > Lib.inf > >>>>>> > ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comm > andsLib.inf > >>>>>> > ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comm > andsLib.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-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
