From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=17.171.2.72; helo=ma1-aaemail-dr-lapp03.apple.com; envelope-from=afish@apple.com; receiver=edk2-devel@lists.01.org Received: from ma1-aaemail-dr-lapp03.apple.com (ma1-aaemail-dr-lapp03.apple.com [17.171.2.72]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 05D4E2111B78D for ; Wed, 5 Sep 2018 11:21:33 -0700 (PDT) Received: from pps.filterd (ma1-aaemail-dr-lapp03.apple.com [127.0.0.1]) by ma1-aaemail-dr-lapp03.apple.com (8.16.0.22/8.16.0.22) with SMTP id w85IH6Mi043275; Wed, 5 Sep 2018 11:21:27 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apple.com; h=mime-version : content-type : sender : from : message-id : subject : date : in-reply-to : cc : to : references; s=20180706; bh=edko/O8rjmkHUikxe8Hi5ukrQDlHPnTmViHqysprOco=; b=pS7CThdqZggtgdAMHZiTjrbOcOdUqGLEZDPJpv0tWejfizjy1ZwIIGOAjqDEx92bVBLT QPfOLOz9rEmZN3VD8vkyEBq/KZTG4YCVjHYrqC7JsUOTmEC5LuT5R6tjNp+G38h4M/jQ jA2DJjj70N4nUsegnNSUZTOyC/wQuNj+pdbp3hPI0Ka5gGsoE7KNmBZTYnsFWPHZ+Tk5 NioPo/cP98A8XMS/17WDOkWYy9mwglmkbB3uF3EGgeuhNia+qWCcQXbMJ1jknC9Wc123 iQwEsUEuNMvCA+Dq91SAv8coyVCW12LeC71PHTo86yr9vhiQBnXIvsA+PT5jqD8DO5ME bg== Received: from mr2-mtap-s01.rno.apple.com (mr2-mtap-s01.rno.apple.com [17.179.226.133]) by ma1-aaemail-dr-lapp03.apple.com with ESMTP id 2m7sryykcm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128 verify=NO); Wed, 05 Sep 2018 11:21:26 -0700 MIME-version: 1.0 Received: from nwk-mmpp-sz12.apple.com (nwk-mmpp-sz12.apple.com [17.128.115.204]) by mr2-mtap-s01.rno.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) with ESMTPS id <0PEL001UVHNODM40@mr2-mtap-s01.rno.apple.com>; Wed, 05 Sep 2018 11:21:24 -0700 (PDT) Received: from process_viserion-daemon.nwk-mmpp-sz12.apple.com by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) id <0PEL00E00HHQP200@nwk-mmpp-sz12.apple.com>; Wed, 05 Sep 2018 11:21:24 -0700 (PDT) X-Va-A: X-Va-T-CD: 81ca60fce39c2560b6c4a7e5841f9b8f X-Va-E-CD: 5cce92e979dc82d2dd4d76128a280c0f X-Va-R-CD: 0972fc81b48dbbdab5e0f3435d1cb1dc X-Va-CD: 0 X-Va-ID: 51489298-3b00-41c8-a9b7-73cff418c115 X-V-A: X-V-T-CD: 81ca60fce39c2560b6c4a7e5841f9b8f X-V-E-CD: 5cce92e979dc82d2dd4d76128a280c0f X-V-R-CD: 0972fc81b48dbbdab5e0f3435d1cb1dc X-V-CD: 0 X-V-ID: 1f44d62f-aa96-480d-8064-7c5e3186d7f6 Received: from process_milters-daemon.nwk-mmpp-sz12.apple.com by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) id <0PEL00D00HE8SE00@nwk-mmpp-sz12.apple.com>; Wed, 05 Sep 2018 11:21:22 -0700 (PDT) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:,, definitions=2018-09-05_10:,, signatures=0 X-Proofpoint-Scanner-Instance: nwk-grpmailp-qapp17.corp.apple.com-10000_instance1 Received: from [17.235.53.229] (unknown [17.235.53.229]) by nwk-mmpp-sz12.apple.com (Oracle Communications Messaging Server 8.0.2.3.20180614 64bit (built Jun 14 2018)) with ESMTPSA id <0PEL00JXEHM9IPB0@nwk-mmpp-sz12.apple.com>; Wed, 05 Sep 2018 11:20:35 -0700 (PDT) Sender: afish@apple.com From: Andrew Fish Message-id: <7ADF7CDD-1EE8-41ED-BAF5-7AFC9000345C@apple.com> Date: Wed, 05 Sep 2018 11:20:49 -0700 In-reply-to: Cc: Leif Lindholm , "edk2-devel@lists.01.org" , "Ni, Ruiyu" , Alexander Graf , Heinrich Schuchardt , AKASHI Takahiro , Mike Kinney , Laszlo Ersek To: "Carsey, Jaben" References: <20180905172546.hxc2vqn6pgmr2zqs@bivouac.eciton.net> <9FF5C684-0DA6-45B0-93D8-F6358ACF1C7A@intel.com> X-Mailer: Apple Mail (2.3445.6.18) X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:, , definitions=2018-09-05_10:, , signatures=0 X-Content-Filtered-By: Mailman/MimeDel 2.1.29 Subject: Re: portability of ShellPkg X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 05 Sep 2018 18:21:34 -0000 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable > On Sep 5, 2018, at 11:05 AM, Carsey, Jaben = wrote: >=20 > But a NULL lib listed in components section shouldn=E2=80=99t be = linked in to anything... >=20 Jaben, A NULL library class means force it to be linked in.=20 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/UefiShellLevel2CommandsLi= b.inf ShellPkg/ShellPkg.dsc:111: = NULL|ShellPkg/Library/UefiShellLevel1CommandsLib/UefiShellLevel1CommandsLi= b.inf ShellPkg/ShellPkg.dsc:112: = NULL|ShellPkg/Library/UefiShellLevel3CommandsLib/UefiShellLevel3CommandsLi= b.inf ShellPkg/ShellPkg.dsc:114: = NULL|ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1Commands= Lib.inf ShellPkg/ShellPkg.dsc:115: = NULL|ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1Comman= dsLib.inf ShellPkg/ShellPkg.dsc:116: = NULL|ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLi= b.inf ShellPkg/ShellPkg.dsc:117: = NULL|ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1Comman= dsLib.inf ShellPkg/ShellPkg.dsc:118: = NULL|ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comman= dsLib.inf Libraries can get pulled in via other libraries. The only way to tell = for sure is to look at the build report.=20 $ build -p ShellPkg/ShellPkg.dec -a X64 -t XCODE5 = --report-file=3Dreport.txt $ cat report.txt | grep HobLib /Volumes/Case/UDK2018/MdePkg/Library/DxeHobLib/DxeHobLib.inf {HobLib: C =3D HobLibConstructor Time =3D 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=3Dreport.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/UefiBootMan= agerLib.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... >=20 > Or so I thought. >=20 > On Sep 5, 2018, at 11:04 AM, Andrew Fish > wrote: >=20 >>=20 >>=20 >>> On Sep 5, 2018, at 10:30 AM, Carsey, Jaben > wrote: >>>=20 >>> How does removing a lib from the components section affect the shell = binary output? >>>=20 >>> Is the asset at compile time? >>=20 >> Jaben, >>=20 >> 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.=20 >>=20 >> The library has an explicit assumption that HOBs exist, and that is = not the case in Leif's platform.=20 >>=20 >> = https://github.com/tianocore/edk2/blob/master/MdePkg/Library/DxeHobLib/Hob= Lib.c#L54 = >>=20 >>=20 >> VOID *mHobList =3D=20 >> NULL; >>=20 >>=20 >> /** >>=20 >> Returns the pointer to the HOB list. >>=20 >>=20 >> This function returns the pointer to first HOB in the list. >>=20 >> For PEI phase, the PEI service GetHobList() can be used to retrieve = the pointer >>=20 >> to the HOB list. For the DXE phase, the HOB list pointer can be = retrieved through >>=20 >> the EFI System Table by looking up theHOB list GUID in the System = Configuration Table. >>=20 >> Since the System Configuration Table does not exist that the time the = DXE Core is >>=20 >> launched, the DXE Core uses a global variable from the DXE Core Entry = Point Library >>=20 >> to manage the pointer to the HOB list. >>=20 >>=20 >> If the pointer to the HOB list is NULL, then ASSERT(). >>=20 >>=20 >> This function also caches the pointer to the HOB list retrieved. >>=20 >>=20 >> @return The pointer to the HOB list. >>=20 >>=20 >> **/ >>=20 >> VOID * >>=20 >> EFIAPI >>=20 >> GetHobList ( >>=20 >> VOID >>=20 >> ) >>=20 >> { >>=20 >> EFI_STATUS Status; >>=20 >>=20 >> if (mHobList =3D=3D >> NULL) { >>=20 >> Status =3D=20 >> EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobList); >>=20 >> ASSERT_EFI_ERROR (Status); >>=20 >> ASSERT (mHobList !=3D >> NULL); >>=20 >> } >>=20 >> return mHobList; >>=20 >> } >>=20 >>=20 >> /** >>=20 >> The constructor function caches the pointer to HOB list by calling = GetHobList() >>=20 >> and will always return EFI_SUCCESS. >>=20 >>=20 >> @param ImageHandle The firmware allocated handle for the EFI image. >>=20 >> @param SystemTable A pointer to the EFI System Table. >>=20 >>=20 >> @retval EFI_SUCCESS The constructor successfully gets HobList. >>=20 >>=20 >> **/ >>=20 >> EFI_STATUS >>=20 >> EFIAPI >>=20 >> HobLibConstructor ( >>=20 >> IN EFI_HANDLE ImageHandle, >>=20 >> IN EFI_SYSTEM_TABLE *SystemTable >>=20 >> ) >>=20 >> { >>=20 >> GetHobList (); >>=20 >>=20 >> return EFI_SUCCESS; >>=20 >> } >>=20 >>=20 >>=20 >> /** >>=20 >> Returns the next instance of a HOB type from the starting HOB. >>=20 >>=20 >> This function searches the first instance of a HOB type from the = starting HOB pointer. >>=20 >> If there does not exist such HOB type from the starting HOB pointer, = it will return NULL. >>=20 >> In contrast with macro GET_NEXT_HOB(), this function does not skip = the starting HOB pointer >>=20 >> unconditionally: it returns HobStart back if HobStart itself meets = the requirement; >>=20 >> caller is required to use GET_NEXT_HOB() if it wishes to skip current = HobStart. >>=20 >>=20 >> If HobStart is NULL, then ASSERT(). >>=20 >>=20 >> @param Type The HOB type to return. >>=20 >> @param HobStart The starting HOB pointer to search from. >>=20 >>=20 >> @return The next instance of a HOB type from the starting HOB. >>=20 >>=20 >> **/ >>=20 >> VOID * >>=20 >> EFIAPI >>=20 >> GetNextHob ( >>=20 >> IN UINT16 Type, >>=20 >> IN CONST VOID *HobStart >>=20 >> ) >>=20 >> { >>=20 >> EFI_PEI_HOB_POINTERS Hob; >>=20 >>=20 >> ASSERT (HobStart !=3D >> NULL); >>=20 >>=20 >> Hob.Raw =3D (UINT8 *) HobStart; >>=20 >> // >>=20 >> // Parse the HOB list until end of list or matching type is found. >>=20 >> // >>=20 >> while (!END_OF_HOB_LIST (Hob)) { >>=20 >> if (Hob.Header->HobType =3D=3D Type) { >>=20 >> return Hob.Raw; >>=20 >> } >>=20 >> Hob.Raw =3D=20 >> GET_NEXT_HOB (Hob); >>=20 >> } >>=20 >> return >> NULL; >>=20 >> } >>=20 >> Thanks, >>=20 >> Andrew Fish >>=20 >>>=20 >>> Jaben >>>=20 >>>> On Sep 5, 2018, at 10:26 AM, Leif Lindholm = > wrote: >>>>=20 >>>> Hi all, >>>>=20 >>>> (This is partly a summary of discussions that have been held on IRC >>>> and offline, with Alex Graf and Mike Kinney.) >>>>=20 >>>> 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. >>>>=20 >>>> 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. >>>>=20 >>>> 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 :) >>>>=20 >>>> 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): >>>>=20 >>>> 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.i= nf >>>> = ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib= .inf >>>> - = ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.inf= >>>> = ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib= .inf >>>> = ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2CommandsLib= .inf >>>>=20 >>>> 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. >>>>=20 >>>> Thoughts? >>>>=20 >>>> / >>>> Leif >>=20