From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jaben.carsey@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 8AA802111B78D for ; Wed, 5 Sep 2018 11:05:33 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Sep 2018 11:05:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.53,334,1531810800"; d="scan'208,217";a="70839565" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga008.jf.intel.com with ESMTP; 05 Sep 2018 11:05:31 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 5 Sep 2018 11:05:31 -0700 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.126]) by fmsmsx115.amr.corp.intel.com ([169.254.4.182]) with mapi id 14.03.0319.002; Wed, 5 Sep 2018 11:05:31 -0700 From: "Carsey, Jaben" To: Andrew Fish CC: Leif Lindholm , "edk2-devel@lists.01.org" , "Ni, Ruiyu" , Alexander Graf , Heinrich Schuchardt , AKASHI Takahiro , "Kinney, Michael D" , Laszlo Ersek Thread-Topic: portability of ShellPkg Thread-Index: AQHURT2FSII4bnoVI0+D2t3UnpHUkaTh8cUsgAB+t4D//4sZHA== Date: Wed, 5 Sep 2018 18:05:30 +0000 Message-ID: References: <20180905172546.hxc2vqn6pgmr2zqs@bivouac.eciton.net> <9FF5C684-0DA6-45B0-93D8-F6358ACF1C7A@intel.com>, In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: MIME-Version: 1.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:05:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable But a NULL lib listed in components section shouldn=92t be linked in to any= thing... 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. L= eif'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/HobL= ib.c#L54 VOID *mHobList =3D 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 ret= rieved through the EFI System Table by looking up theHOB list GUID in the System C= onfiguration Table. Since the System Configuration Table does not exist that the time t= he DXE Core is launched, the DXE Core uses a global variable from the DXE Core Ent= ry 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 =3D=3D NULL) { Status =3D EfiGetSystemConfigurationTable (&gEfiHobListGuid, &mHobL= ist); ASSERT_EFI_ERROR (Status); ASSERT (mHobList !=3D 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 st= arting 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 curre= nt 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 !=3D NULL); Hob.Raw =3D (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 =3D=3D Type) { return Hob.Raw; } Hob.Raw =3D 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 ShellPkg/Library/UefiShellDriver1CommandsLib/UefiShellDriver1CommandsLib.i= nf ShellPkg/Library/UefiShellInstall1CommandsLib/UefiShellInstall1CommandsLib= .inf - ShellPkg/Library/UefiShellDebug1CommandsLib/UefiShellDebug1CommandsLib.i= nf 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