* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell [not found] ` <CAKv+Gu9KvJVGgORjxnLzuGVswimavRg8h1ZXKC+=Fv48RLJGEQ@mail.gmail.com> @ 2016-11-16 16:13 ` Marcin Wojtas 2016-11-16 17:05 ` Carsey, Jaben 0 siblings, 1 reply; 7+ messages in thread From: Marcin Wojtas @ 2016-11-16 16:13 UTC (permalink / raw) To: Ard Biesheuvel Cc: linaro-uefi, Leif Lindholm, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dąbroś, Bartosz Szczepanek, edk2-devel-01, Carsey, Jaben, ruiyu.ni + Jaben, Ruiyu and edk2 list Hi Ard, Internal Shell header ("../../../ShellPkg/Application/Shell/Shell.h") was added ONLY for being able to execute shell commands (tftp and our custom SPI flash control) from within a new command. This is now done with: RunShellCommand (TftpCmd, &Status); And it works fine. Replacing above with: ShellExecute (&ImageHandle, TftpCmd, FALSE, NULL, &Status); allow to get rid of relative include and seems more appropriate, but there is completely no effect of calling it. I checked in edk2 sources and see no relation between above problem and registering our command with help of ShellDynamicCommand protocol. Unless I don't know something. Anyway, I'll apreciate any hint, how to solve the issue in a nice way. Best regards, Marcin 2016-11-15 17:09 GMT+01:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > On 10 July 2016 at 00:21, Marcin Wojtas <mw@semihalf.com> wrote: >> From: Jan Dąbroś <jsd@semihalf.com> >> >> 'fupdate' command performs updating firmware from file placed on local >> filesystem or on TFTP server. >> It uses tftp and sf commands. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jan Dabros <jsd@semihalf.com> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > Please use the Shell dynamic command protocol to add commands to the > shell. This will remove the need for including internal Shell headers, > and even allow your commands to work with prebuilt Shell binaries. > > MdePkg/Include/Protocol/ShellDynamicCommand.h > > Thanks, > Ard. > > >> --- >> Applications/FirmwareUpdate/FUpdate.c | 448 ++++++++++++++++++++++++++++++++ >> Applications/FirmwareUpdate/FUpdate.inf | 68 +++++ >> Applications/FirmwareUpdate/FUpdate.uni | Bin 0 -> 5838 bytes >> Platforms/Marvell/Marvell.dec | 1 + >> 4 files changed, 517 insertions(+) >> create mode 100644 Applications/FirmwareUpdate/FUpdate.c >> create mode 100644 Applications/FirmwareUpdate/FUpdate.inf >> create mode 100644 Applications/FirmwareUpdate/FUpdate.uni >> >> diff --git a/Applications/FirmwareUpdate/FUpdate.c b/Applications/FirmwareUpdate/FUpdate.c >> new file mode 100644 >> index 0000000..359a4ac >> --- /dev/null >> +++ b/Applications/FirmwareUpdate/FUpdate.c >> @@ -0,0 +1,448 @@ >> +/******************************************************************************* >> +Copyright (C) 2016 Marvell International Ltd. >> + >> +Marvell BSD License Option >> + >> +If you received this File from Marvell, you may opt to use, redistribute and/or >> +modify this File under the following licensing terms. >> +Redistribution and use in source and binary forms, with or without modification, >> +are permitted provided that the following conditions are met: >> + >> +* Redistributions of source code must retain the above copyright notice, >> + this list of conditions and the following disclaimer. >> + >> +* Redistributions in binary form must reproduce the above copyright >> + notice, this list of conditions and the following disclaimer in the >> + documentation and/or other materials provided with the distribution. >> + >> +* Neither the name of Marvell nor the names of its contributors may be >> + used to endorse or promote products derived from this software without >> + specific prior written permission. >> + >> +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND >> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED >> +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE >> +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR >> +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES >> +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; >> +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON >> +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT >> +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS >> +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> + >> +*******************************************************************************/ >> +#include <Uefi.h> >> +#include <ShellBase.h> >> + >> +#include <Library/BaseLib.h> >> +#include <Library/DebugLib.h> >> +#include <Library/MemoryAllocationLib.h> >> +#include <Library/ShellCommandLib.h> >> +#include <Library/ShellLib.h> >> +#include <Library/UefiLib.h> >> +#include <Library/PrintLib.h> >> +#include <Library/UefiLib.h> >> +#include <Library/ShellCEntryLib.h> >> +#include <Library/HiiLib.h> >> +#include <Guid/ShellLibHiiGuid.h> >> +#include <Library/UefiBootServicesTableLib.h> >> +#include <Library/FileHandleLib.h> >> +// FIXME: Remove including relative path >> +#include "../../../ShellPkg/Application/Shell/Shell.h" >> + >> +#define CMD_NAME_STRING L"fupdate" >> + >> +#define TFTP_CMD_STRING L"tftp " >> +#define SPACE_STRING L" " >> +#define SF_PROBE_CMD_STRING L"sf probe" >> +#define SF_WRITE_CMD_STRING L"sf updatefile " >> +#define SF_LOAD_ADDR_STRING L"0x0" >> + >> +#define MAIN_HDR_MAGIC 0xB105B002 >> + >> +typedef struct { >> + UINT32 Magic; // 0-3 >> + UINT32 PrologSize; // 4-7 >> + UINT32 PrologChecksum; // 8-11 >> + UINT32 BootImageSize; // 12-15 >> + UINT32 BootImageChecksum; // 16-19 >> + UINT32 Reserved0; // 20-23 >> + UINT32 LoadAddr; // 24-27 >> + UINT32 ExecAddr; // 28-31 >> + UINT8 UartConfig; // 32 >> + UINT8 Baudrate; // 33 >> + UINT8 ExtCount; // 34 >> + UINT8 AuxFlags; // 35 >> + UINT32 IoArg0; // 36-39 >> + UINT32 IoArg1; // 40-43 >> + UINT32 IoArg2; // 43-47 >> + UINT32 IoArg3; // 48-51 >> + UINT32 Reserved1; // 52-55 >> + UINT32 Reserved2; // 56-59 >> + UINT32 Reserved3; // 60-63 >> +} MV_IMAGE_HEADER; >> + >> +STATIC >> +UINT32 >> +CountChecksum ( >> + UINT32 *Start, >> + UINT32 Length >> + ) >> +{ >> + UINT32 Sum = 0; >> + UINT32 *Startp = Start; >> + >> + do { >> + Sum += *Startp; >> + Startp++; >> + Length -= 4; >> + } while (Length > 0); >> + >> + return Sum; >> +} >> + >> +STATIC >> +EFI_STATUS >> +CheckImageHeader ( >> + IN VOID *ImageHeader >> + ) >> +{ >> + MV_IMAGE_HEADER *Header; >> + UINT32 HeaderLength, Checksum, ChecksumBackup; >> + >> + Header = (MV_IMAGE_HEADER *) ImageHeader; >> + HeaderLength = Header->PrologSize; >> + ChecksumBackup = Header->PrologChecksum; >> + >> + // Compare magic number >> + if (Header->Magic != MAIN_HDR_MAGIC) { >> + Print (L"%s: Bad Image magic 0x%08x != 0x%08x\n", CMD_NAME_STRING, >> + Header->Magic, MAIN_HDR_MAGIC); >> + return EFI_DEVICE_ERROR; >> + } >> + >> + // The checksum field is discarded from calculation >> + Header->PrologChecksum = 0; >> + >> + Checksum = CountChecksum((UINT32 *)Header, HeaderLength); >> + if (Checksum != ChecksumBackup) { >> + Print (L"%s: Bad Image checksum. 0x%x != 0x%x\n", CMD_NAME_STRING, Checksum, >> + ChecksumBackup); >> + return EFI_DEVICE_ERROR; >> + } >> + >> + // Restore checksum backup >> + Header->PrologChecksum = ChecksumBackup; >> + >> + return 0; >> +} >> + >> +STATIC >> +EFI_STATUS >> +CheckFirmwareImage ( >> + CONST CHAR16* FirmwareImage >> + ) >> +{ >> + EFI_STATUS Status; >> + VOID *FileBuffer; >> + UINT64 OpenMode; >> + UINTN FileSize; >> + SHELL_FILE_HANDLE FileHandle = NULL; >> + >> + OpenMode = EFI_FILE_MODE_READ; >> + >> + Status = ShellOpenFileByName (FirmwareImage, &FileHandle, OpenMode, 0); >> + if (EFI_ERROR (Status)) { >> + Print (L"%s: Cannot open Image file\n", CMD_NAME_STRING); >> + return EFI_DEVICE_ERROR; >> + } >> + >> + Status = FileHandleGetSize (FileHandle, &FileSize); >> + if (EFI_ERROR (Status)) { >> + Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING); >> + } >> + >> + FileBuffer = AllocateZeroPool (FileSize); >> + >> + // Read Image header into buffer >> + Status = FileHandleRead (FileHandle, &FileSize, FileBuffer); >> + if (EFI_ERROR (Status)) { >> + Print (L"%s: Cannot read Image file header\n", CMD_NAME_STRING); >> + ShellCloseFile (&FileHandle); >> + FreePool (FileBuffer); >> + return EFI_DEVICE_ERROR; >> + } >> + >> + Status = CheckImageHeader (FileBuffer); >> + if (EFI_ERROR(Status)) { >> + return EFI_DEVICE_ERROR; >> + } >> + >> + FreePool (FileBuffer); >> + >> + return EFI_SUCCESS; >> +} >> + >> +STATIC >> +CONST CHAR16 * >> +FileNameFromFilePath ( >> + CONST CHAR16 *RemoteFilePath >> + ) >> +{ >> + CONST CHAR16 *Walker; >> + >> + // Gather FileName from FilePath >> + Walker = RemoteFilePath + StrLen (RemoteFilePath); >> + while ((--Walker) >= RemoteFilePath) { >> + if ((*Walker == L'\\') || (*Walker == L'/')) { >> + break; >> + } >> + } >> + >> + return (Walker + 1); >> +} >> + >> +STATIC >> +CONST CHAR16* >> +PrepareFile ( >> + LIST_ENTRY *CheckPackage >> + ) >> +{ >> + EFI_STATUS Status; >> + CONST CHAR16 *ValueStr; >> + >> + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-f"); >> + if (ValueStr == NULL) { >> + Print (L"%s: No LocalFilePath parameter!\n", CMD_NAME_STRING); >> + return NULL; >> + } else { >> + Status = ShellIsFile (ValueStr); >> + if (EFI_ERROR(Status)) { >> + Print (L"%s: Wrong LocalFilePath parameter!\n", CMD_NAME_STRING); >> + return NULL; >> + } >> + } >> + return ValueStr; >> +} >> + >> +CONST CHAR16 gShellFUpdateFileName[] = L"ShellCommand"; >> +EFI_HANDLE gShellFUpdateHiiHandle = NULL; >> +EFI_HANDLE gShellFUpdateHiiHandle; >> + >> +STATIC CONST SHELL_PARAM_ITEM ParamList[] = { >> + {L"help", TypeFlag}, >> + {L"-t", TypeFlag}, >> + {L"-f", TypeValue}, >> + {NULL , TypeMax} >> + }; >> + >> +/** >> + Return the file name of the help text file if not using HII. >> + >> + @return The string pointer to the file name. >> +**/ >> +CONST CHAR16* >> +EFIAPI >> +ShellCommandGetManFileNameFUpdate ( >> + VOID >> + ) >> +{ >> + >> + return gShellFUpdateFileName; >> +} >> + >> +VOID >> +FUpdateUsage ( >> + VOID >> + ) >> +{ >> + Print (L"\nFirmware update command\n" >> + "fupdate [-f <LocalFilePath>] [-t <Host> <RemoteFilePath>]\n\n" >> + "LocalFilePath - path to local firmware image file\n" >> + "Host - IP number of TFTP server\n" >> + "RemoteFilePath - path to firmware image file on TFTP server\n" >> + "Examples:\n" >> + "Update firmware from file fs2:Uefi.img\n" >> + " fupdate -f fs2:Uefi.img\n" >> + "Update firmware from file path/Uefi.img TFTP server with IP " >> + "address 10.0.0.200\n" >> + " fupdate -t 10.0.0.200 path/Uefi.img\n" >> + ); >> +} >> + >> +SHELL_STATUS >> +EFIAPI >> +ShellCommandRunFUpdate ( >> + IN EFI_HANDLE ImageHandle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + EFI_STATUS Status; >> + LIST_ENTRY *CheckPackage; >> + CHAR16 *ProblemParam, *TftpCmd = NULL, *SfCmd = NULL; >> + CONST CHAR16 *RemoteFilePath, *Host, *FileToWrite; >> + UINT8 CmdLen; >> + BOOLEAN FileFlag, TftpFlag; >> + >> + Status = ShellInitialize (); >> + if (EFI_ERROR (Status)) { >> + Print (L"%s: Error while initializinf Shell\n", CMD_NAME_STRING); >> + ASSERT_EFI_ERROR (Status); >> + return SHELL_ABORTED; >> + } >> + >> + Status = ShellCommandLineParse (ParamList, &CheckPackage, &ProblemParam, >> + TRUE); >> + if (EFI_ERROR (Status)) { >> + Print (L"Parse error!\n"); >> + } >> + >> + if (ShellCommandLineGetFlag (CheckPackage, L"help")) { >> + FUpdateUsage(); >> + return EFI_SUCCESS; >> + } >> + >> + FileFlag = ShellCommandLineGetFlag (CheckPackage, L"-f"); >> + TftpFlag = ShellCommandLineGetFlag (CheckPackage, L"-t"); >> + >> + if (!FileFlag && !TftpFlag) { >> + Print (L"%s: Please specify -f or -t flag\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } else if (FileFlag && !TftpFlag) { >> + // Prepare local file to be burned into flash >> + FileToWrite = PrepareFile (CheckPackage); >> + if (FileToWrite == NULL) { >> + Print (L"%s: Error while preparing file for burn\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + } else if (TftpFlag && !FileFlag) { >> + Host = ShellCommandLineGetRawValue (CheckPackage, 1); >> + if (Host == NULL) { >> + Print (L"%s: No Host parameter!\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + RemoteFilePath = ShellCommandLineGetRawValue (CheckPackage, 2); >> + if (RemoteFilePath == NULL) { >> + Print (L"%s: No remote_file_path parameter!\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + // Gather firmware image name from remote filepath >> + FileToWrite = FileNameFromFilePath (RemoteFilePath); >> + >> + // Allocate buffer for tftp command string >> + CmdLen = StrSize (TFTP_CMD_STRING) + StrSize (SPACE_STRING) + >> + StrSize (Host) + StrSize (RemoteFilePath); >> + TftpCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); >> + if (TftpCmd == NULL) { >> + Print (L"%s: Cannot allocate memory\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + // Concatenate parameters and form tftp command string >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), TFTP_CMD_STRING); >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)Host); >> + // Insert space >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)RemoteFilePath); >> + >> + RunShellCommand (TftpCmd, &Status); >> + FreePool (TftpCmd); >> + if (EFI_ERROR(Status)) { >> + Print (L"%s: Error while performing tftp command\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + } else { >> + Print (L"%s: Both -f and -t flag specified, please choose one\n", >> + CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + // Check image checksum and magic >> + Status = CheckFirmwareImage (FileToWrite); >> + if (EFI_ERROR(Status)) { >> + Print (L"%s: Wrong firmware Image\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + // Probe spi bus >> + RunShellCommand (SF_PROBE_CMD_STRING, &Status); >> + if (EFI_ERROR(Status)) { >> + Print (L"%s: Error while performing sf probe\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + // Allocate buffer for sf command string >> + CmdLen = StrSize (SF_WRITE_CMD_STRING) + StrSize (FileToWrite) + >> + StrSize (SPACE_STRING) + StrSize (SF_LOAD_ADDR_STRING); >> + SfCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); >> + if (SfCmd == NULL) { >> + Print (L"%s: Cannot allocate memory\n"); >> + return SHELL_ABORTED; >> + } >> + >> + // Concatenate parameters and form command string >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SF_WRITE_CMD_STRING); >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)FileToWrite); >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SF_LOAD_ADDR_STRING); >> + >> + // Update firmware image in flash >> + RunShellCommand (SfCmd, &Status); >> + FreePool (SfCmd); >> + if (EFI_ERROR(Status)) { >> + Print (L"%s: Error while performing sf update\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + return EFI_SUCCESS; >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +ShellFUpdateCommandConstructor ( >> + IN EFI_HANDLE ImageHandle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + EFI_STATUS Status; >> + >> + gShellFUpdateHiiHandle = NULL; >> + >> + gShellFUpdateHiiHandle = HiiAddPackages ( >> + &gShellFUpdateHiiGuid, gImageHandle, >> + UefiShellFUpdateCommandLibStrings, NULL >> + ); >> + if (gShellFUpdateHiiHandle == NULL) { >> + Print (L"%s: Cannot add Hii package\n", CMD_NAME_STRING); >> + return EFI_DEVICE_ERROR; >> + } >> + >> + Status = ShellCommandRegisterCommandName ( >> + CMD_NAME_STRING, ShellCommandRunFUpdate, ShellCommandGetManFileNameFUpdate, >> + 0, CMD_NAME_STRING, TRUE , gShellFUpdateHiiHandle, >> + STRING_TOKEN (STR_GET_HELP_FUPDATE) >> + ); >> + if (EFI_ERROR(Status)) { >> + Print (L"%s: Error while registering command\n", CMD_NAME_STRING); >> + return SHELL_ABORTED; >> + } >> + >> + return EFI_SUCCESS; >> +} >> + >> +EFI_STATUS >> +EFIAPI >> +ShellFUpdateCommandDestructor ( >> + IN EFI_HANDLE ImageHandle, >> + IN EFI_SYSTEM_TABLE *SystemTable >> + ) >> +{ >> + >> + if (gShellFUpdateHiiHandle != NULL) { >> + HiiRemovePackages (gShellFUpdateHiiHandle); >> + } >> + return EFI_SUCCESS; >> +} >> diff --git a/Applications/FirmwareUpdate/FUpdate.inf b/Applications/FirmwareUpdate/FUpdate.inf >> new file mode 100644 >> index 0000000..53b5305 >> --- /dev/null >> +++ b/Applications/FirmwareUpdate/FUpdate.inf >> @@ -0,0 +1,68 @@ >> +# >> +# Marvell BSD License Option >> +# >> +# If you received this File from Marvell, you may opt to use, redistribute >> +# and/or modify this File under the following licensing terms. >> +# Redistribution and use in source and binary forms, with or without >> +# modification, are permitted provided that the following conditions are met: >> +# >> +# * Redistributions of source code must retain the above copyright notice, >> +# this list of conditions and the following disclaimer. >> +# >> +# * Redistributions in binary form must reproduce the above copyright >> +# notice, this list of conditions and the following disclaimer in the >> +# documentation and/or other materials provided with the distribution. >> +# >> +# * Neither the name of Marvell nor the names of its contributors may be >> +# used to endorse or promote products derived from this software without >> +# specific prior written permission. >> +# >> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" >> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE >> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE >> +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE >> +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL >> +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR >> +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER >> +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, >> +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE >> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. >> +# >> + >> +[Defines] >> + INF_VERSION = 0x00010006 >> + BASE_NAME = UefiShellFUpdateCommandLib >> + FILE_GUID = 470292b2-926b-4ed8-8080-be7a260db627 >> + MODULE_TYPE = UEFI_APPLICATION >> + VERSION_STRING = 0.1 >> + LIBRARY_CLASS = NULL|UEFI_APPLICATION UEFI_DRIVER >> + CONSTRUCTOR = ShellFUpdateCommandConstructor >> + DESTRUCTOR = ShellFUpdateCommandDestructor >> + >> +[Sources] >> + FUpdate.c >> + FUpdate.uni >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + ShellPkg/ShellPkg.dec >> + MdeModulePkg/MdeModulePkg.dec >> + OpenPlatformPkg/Platforms/Marvell/Marvell.dec >> + >> +[LibraryClasses] >> + UefiLib >> + UefiBootServicesTableLib >> + MemoryAllocationLib >> + BaseLib >> + BaseMemoryLib >> + DebugLib >> + ShellCommandLib >> + ShellLib >> + UefiLib >> + UefiRuntimeServicesTableLib >> + PcdLib >> + HiiLib >> + FileHandleLib >> + >> +[Guids] >> + gShellFUpdateHiiGuid >> diff --git a/Applications/FirmwareUpdate/FUpdate.uni b/Applications/FirmwareUpdate/FUpdate.uni >> new file mode 100644 >> index 0000000000000000000000000000000000000000..6143c0580f1d1ef8ce4fe619a1df1ce380adf2c0 >> GIT binary patch >> literal 5838 >> zcmd6r?N1v=5XSd&rTz~m@+FOuK&z@BXdB5G6OhbHeI_(&s)*P?EX6>LDdL}R`}}4Y >> z_Fim)B2|h`_TBEy&OY<Z%kJL4|E#8GU-k8E`X+VLMY>G4X_hAGEPYJ#RHUcs{Z98E >> z(pvgH{iLVebS#O#(@FZIt4KTPT#_g1JJ*}J#$MPH@A_(w)60Z*e$BBPsZ5(DH%TMu >> zI8RslFVenngr*p~lTP*KR@$$1pGl4p6GJjq&s>Nn8egO>-9MS0Q^{SVsru?OKUYs^ >> znhN!;+Src8b3GNB10={X)7Ui6^*l)*^bUIqjXT$yRmsoHx~ZO!JT-lb@LbynxN^K` >> zIn^kbyht}1doJb+wK5;k=NwOk%lRBE#O3S<wy>ix&4tFo-prWGG9T<MUWEtn%*5H1 >> zSq~;Gfgeyu8$Ga<Z3c1n|8rvmo!EG(e{`Y?JMh<~dM@O>iCW&nJZN~Bn~_GrJG$Ij >> zd>%7RrE8wn?<20zyXy5!t%sOH*M&t|ohxPqCl(VMpv1PhX2wGq^RXfXyO3p6Go#Mc >> z2Xdm<S*qfT=Nvu6g3sr;me~&{8O5Tg@+clDy1)!mvr2Bp%yEAVQ*Qa<_-Qib{vL}x >> zEYC<E=8NnPMAdtpMc3xp|Nrt8nS_5%EXt~=;(06|k7OwvQyp_&MAmuLAl6(HxkV-N >> zET8D<lf1qtOCkd^gYD|%d6DaG;EFrG-4iy??Rq_;ii11ypk6r(+2fVKanpABoH=Nv >> z<&j#gnvCR4)Z7mbaOf4-W15N#lkkcLM+avfRa7Cb$<-)x6l5)Rn~X$TUK4VTk?W_@ >> za3fp1Vg<`y=axm`l~uNpw4drXy2AteUMrc`D;^ahSHUaU(DczTZEGzpC0A<w+e*W9 >> zlsc+aJ@t71fm-|Njar-P-BkZZdaHj%BGuK~1N96f@mhUD^=+%KZ}J1v-I3oJiR_*v >> zkljd!dfL}Bv#)4Y?##KOyN`eYo|vzfe%Cx8D@~G-$nUDRue)P&^aC3;wBFKIlpSg5 >> zO72iNcz$iYj)AD$H{9si)o*yG4ScM|VQYOh&@Gyeoy3+#?Mll)vu@v^>B#0C!#XsJ >> zf|V_saqr%;En@`^3fYD?u%s*OeQVKssNMs~l^TU4ynqMmO0%@1ClG9^+QQSG{;?A# >> z-{^-Q@H*)Fy0VpvJi0GSK?vv0{BmDzNRD0Djx=xRPRnho`V*v}?g|GSgK$w_S8s?s >> z@PZWCQnoXa8N7$+=-tHvY`itODpHvGO}x1#A6ZCqN25B52jU8RPYubJ$BO3*-oiRO >> z;7of=l<H^yiARTeCejMczOQRj{lPA12bOhRJ(cNa!`7EnL_ZOBs8-J`#SZv^9k<$X >> zcx*8~`ic3{Y>Z5Td2kGL9g117k*Dmy$>bxwBVzD<S9AER<a0zY(pBGjzJP<wb;~0j >> zat46)NHg>_BmbVy#A%ML&V78q8Fhnr$<bBU=9YU^87}oEdC(K*Aj0od2E5>-sRWMZ >> zNblhn9*8MgUOOV6qBc<l+)_Lm7Dv@^zN)=wg0E)Hg^|ck=D^1t#Rb(DByjF*hHkF` >> z;HuY_9BtH}dYtDK`s+R&t{O-1w3>cRPwrb!Y%j@~g;kdG85b+{3yV!w6Iy3>qPfzZ >> zX{9ojT`eo5pLsm0-dh|D(|hxLUpeqz+>;BQS42nq$~dmIf}d&5CJLW-MHLu-u?x7? >> zdOVi2?;F<Cx1yOQLNu-H0}g9a>}aO8dtoo~QX}3;{+%qy`l;i@HPx)wdSBPauAx3( >> z>^nf%KE)1&a40S~$+4?pPs1L58H!T5z)Evq`BJvzidIjPmp|l5{Q8S>lszr`Qdr`A >> zePHZ;Os~?Pl0b59lG~ERx38?lX)E?;i$*TP4(h0Xk3mjUJT#X_%?n8_X0gin<(*~h >> zvDtgF@4S(hIi(+~mG|D+OX0buJ?&^+)>Dm0=76ngbhQ19reJ%S)<S+^oo4sxJJnmY >> zGY6%`kVg`g?m$uXqa?6%tocvmOTIZQvTJ!v1RCa!I0-gk(Id$@uiD7@iqnDTaIo8+ >> zyV>(n7Ls@DKFB<FqO7V-xpu@iN36HTd@g76i6Zb~*Ok7Rt*dfT7boHmyUtp=;i;K( >> zo^g-W@FnOXzE@lLs*zExG5O1n$4E0i3Vj`1la=w~d*Rw6t)*%#8qJA``utI=&gXax >> z|FhF$H#bn-d!tC;obk9gsE#aZW7`pd*NC^C^!K$wv6t-H-o^KX5&g~k(7qbL-AH!% >> z2@qzF6)k>Vp2}uwYkVd1H)P>oTCvDQf3r9rtC-mFvyX}sR8P6g7q^*ax)F|8UHrt% >> QHyJ;b`f6NX>FQbZ9}*T`00000 >> >> literal 0 >> HcmV?d00001 >> >> diff --git a/Platforms/Marvell/Marvell.dec b/Platforms/Marvell/Marvell.dec >> index e63add4..f9d2660 100644 >> --- a/Platforms/Marvell/Marvell.dec >> +++ b/Platforms/Marvell/Marvell.dec >> @@ -54,6 +54,7 @@ >> >> gShellEepromHiiGuid = { 0xb2f4c714, 0x147f, 0x4ff7, { 0x82, 0x1b, 0xce, 0x7b, 0x91, 0x7f, 0x5f, 0x2f } } >> gShellSfHiiGuid = { 0x03a67756, 0x8cde, 0x4638, { 0x82, 0x34, 0x4a, 0x0f, 0x6d, 0x58, 0x81, 0x39 } } >> + gShellFUpdateHiiGuid = { 0x9b5d2176, 0x590a, 0x49db, { 0x89, 0x5d, 0x4a, 0x70, 0xfe, 0xad, 0xbe, 0x24 } } >> >> [PcdsFixedAtBuild.common] >> #MPP >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell 2016-11-16 16:13 ` [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell Marcin Wojtas @ 2016-11-16 17:05 ` Carsey, Jaben 2016-11-16 17:35 ` Leif Lindholm 0 siblings, 1 reply; 7+ messages in thread From: Carsey, Jaben @ 2016-11-16 17:05 UTC (permalink / raw) To: Marcin Wojtas, Ard Biesheuvel Cc: linaro-uefi, Leif Lindholm, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dabros, Bartosz Szczepanek, edk2-devel-01, Ni, Ruiyu, Carsey, Jaben Marcin and Ard, It sounds like there is a goal to have a UEFI Application call into an internal shell command. There is no documented method to programmatically call an internal command. This is not a goal of the current shell implementation. ShellExecute explicitly does not allow running of internal commands (per UEFI Shell Specification 2.2). Thus I don’t think using ShellDynamicCommandProtocol will help. That protocol is for allowing a driver add a command to the shell's list of available commands. -Jaben > -----Original Message----- > From: Marcin Wojtas [mailto:mw@semihalf.com] > Sent: Wednesday, November 16, 2016 8:13 AM > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: linaro-uefi <linaro-uefi@lists.linaro.org>; Leif Lindholm > <leif.lindholm@linaro.org>; Neta Zur Hershkovits <neta@marvell.com>; > Yehuda Yitschak <yehuday@marvell.com>; Haim Boot > <hayim@marvell.com>; Jan Dąbroś <jsd@semihalf.com>; Bartosz > Szczepanek <bsz@semihalf.com>; edk2-devel-01 <edk2-devel@lists.01.org>; > Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com> > Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' > comand to shell > Importance: High > > + Jaben, Ruiyu and edk2 list > > Hi Ard, > > Internal Shell header ("../../../ShellPkg/Application/Shell/Shell.h") > was added ONLY for being able to execute shell commands (tftp and our > custom SPI flash control) from within a new command. This is now done > with: > RunShellCommand (TftpCmd, &Status); > And it works fine. > > Replacing above with: > ShellExecute (&ImageHandle, TftpCmd, FALSE, NULL, &Status); > allow to get rid of relative include and seems more appropriate, but > there is completely no effect of calling it. > > I checked in edk2 sources and see no relation between above problem > and registering our command with help of ShellDynamicCommand protocol. > Unless I don't know something. Anyway, I'll apreciate any hint, how to > solve the issue in a nice way. > > Best regards, > Marcin > > 2016-11-15 17:09 GMT+01:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > > On 10 July 2016 at 00:21, Marcin Wojtas <mw@semihalf.com> wrote: > >> From: Jan Dąbroś <jsd@semihalf.com> > >> > >> 'fupdate' command performs updating firmware from file placed on local > >> filesystem or on TFTP server. > >> It uses tftp and sf commands. > >> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Jan Dabros <jsd@semihalf.com> > >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > > Please use the Shell dynamic command protocol to add commands to the > > shell. This will remove the need for including internal Shell headers, > > and even allow your commands to work with prebuilt Shell binaries. > > > > MdePkg/Include/Protocol/ShellDynamicCommand.h > > > > Thanks, > > Ard. > > > > > >> --- > >> Applications/FirmwareUpdate/FUpdate.c | 448 > ++++++++++++++++++++++++++++++++ > >> Applications/FirmwareUpdate/FUpdate.inf | 68 +++++ > >> Applications/FirmwareUpdate/FUpdate.uni | Bin 0 -> 5838 bytes > >> Platforms/Marvell/Marvell.dec | 1 + > >> 4 files changed, 517 insertions(+) > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.c > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.inf > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.uni > >> > >> diff --git a/Applications/FirmwareUpdate/FUpdate.c > b/Applications/FirmwareUpdate/FUpdate.c > >> new file mode 100644 > >> index 0000000..359a4ac > >> --- /dev/null > >> +++ b/Applications/FirmwareUpdate/FUpdate.c > >> @@ -0,0 +1,448 @@ > >> > +/********************************************************* > ********************** > >> +Copyright (C) 2016 Marvell International Ltd. > >> + > >> +Marvell BSD License Option > >> + > >> +If you received this File from Marvell, you may opt to use, redistribute > and/or > >> +modify this File under the following licensing terms. > >> +Redistribution and use in source and binary forms, with or without > modification, > >> +are permitted provided that the following conditions are met: > >> + > >> +* Redistributions of source code must retain the above copyright notice, > >> + this list of conditions and the following disclaimer. > >> + > >> +* Redistributions in binary form must reproduce the above copyright > >> + notice, this list of conditions and the following disclaimer in the > >> + documentation and/or other materials provided with the distribution. > >> + > >> +* Neither the name of Marvell nor the names of its contributors may be > >> + used to endorse or promote products derived from this software > without > >> + specific prior written permission. > >> + > >> +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS "AS IS" AND > >> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > TO, THE IMPLIED > >> +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > PURPOSE ARE > >> +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > CONTRIBUTORS BE LIABLE FOR > >> +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > CONSEQUENTIAL DAMAGES > >> +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE > GOODS OR SERVICES; > >> +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > HOWEVER CAUSED AND ON > >> +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR > TORT > >> +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > THE USE OF THIS > >> +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > >> + > >> > +********************************************************* > **********************/ > >> +#include <Uefi.h> > >> +#include <ShellBase.h> > >> + > >> +#include <Library/BaseLib.h> > >> +#include <Library/DebugLib.h> > >> +#include <Library/MemoryAllocationLib.h> > >> +#include <Library/ShellCommandLib.h> > >> +#include <Library/ShellLib.h> > >> +#include <Library/UefiLib.h> > >> +#include <Library/PrintLib.h> > >> +#include <Library/UefiLib.h> > >> +#include <Library/ShellCEntryLib.h> > >> +#include <Library/HiiLib.h> > >> +#include <Guid/ShellLibHiiGuid.h> > >> +#include <Library/UefiBootServicesTableLib.h> > >> +#include <Library/FileHandleLib.h> > >> +// FIXME: Remove including relative path > >> +#include "../../../ShellPkg/Application/Shell/Shell.h" > >> + > >> +#define CMD_NAME_STRING L"fupdate" > >> + > >> +#define TFTP_CMD_STRING L"tftp " > >> +#define SPACE_STRING L" " > >> +#define SF_PROBE_CMD_STRING L"sf probe" > >> +#define SF_WRITE_CMD_STRING L"sf updatefile " > >> +#define SF_LOAD_ADDR_STRING L"0x0" > >> + > >> +#define MAIN_HDR_MAGIC 0xB105B002 > >> + > >> +typedef struct { > >> + UINT32 Magic; // 0-3 > >> + UINT32 PrologSize; // 4-7 > >> + UINT32 PrologChecksum; // 8-11 > >> + UINT32 BootImageSize; // 12-15 > >> + UINT32 BootImageChecksum; // 16-19 > >> + UINT32 Reserved0; // 20-23 > >> + UINT32 LoadAddr; // 24-27 > >> + UINT32 ExecAddr; // 28-31 > >> + UINT8 UartConfig; // 32 > >> + UINT8 Baudrate; // 33 > >> + UINT8 ExtCount; // 34 > >> + UINT8 AuxFlags; // 35 > >> + UINT32 IoArg0; // 36-39 > >> + UINT32 IoArg1; // 40-43 > >> + UINT32 IoArg2; // 43-47 > >> + UINT32 IoArg3; // 48-51 > >> + UINT32 Reserved1; // 52-55 > >> + UINT32 Reserved2; // 56-59 > >> + UINT32 Reserved3; // 60-63 > >> +} MV_IMAGE_HEADER; > >> + > >> +STATIC > >> +UINT32 > >> +CountChecksum ( > >> + UINT32 *Start, > >> + UINT32 Length > >> + ) > >> +{ > >> + UINT32 Sum = 0; > >> + UINT32 *Startp = Start; > >> + > >> + do { > >> + Sum += *Startp; > >> + Startp++; > >> + Length -= 4; > >> + } while (Length > 0); > >> + > >> + return Sum; > >> +} > >> + > >> +STATIC > >> +EFI_STATUS > >> +CheckImageHeader ( > >> + IN VOID *ImageHeader > >> + ) > >> +{ > >> + MV_IMAGE_HEADER *Header; > >> + UINT32 HeaderLength, Checksum, ChecksumBackup; > >> + > >> + Header = (MV_IMAGE_HEADER *) ImageHeader; > >> + HeaderLength = Header->PrologSize; > >> + ChecksumBackup = Header->PrologChecksum; > >> + > >> + // Compare magic number > >> + if (Header->Magic != MAIN_HDR_MAGIC) { > >> + Print (L"%s: Bad Image magic 0x%08x != 0x%08x\n", > CMD_NAME_STRING, > >> + Header->Magic, MAIN_HDR_MAGIC); > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + // The checksum field is discarded from calculation > >> + Header->PrologChecksum = 0; > >> + > >> + Checksum = CountChecksum((UINT32 *)Header, HeaderLength); > >> + if (Checksum != ChecksumBackup) { > >> + Print (L"%s: Bad Image checksum. 0x%x != 0x%x\n", > CMD_NAME_STRING, Checksum, > >> + ChecksumBackup); > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + // Restore checksum backup > >> + Header->PrologChecksum = ChecksumBackup; > >> + > >> + return 0; > >> +} > >> + > >> +STATIC > >> +EFI_STATUS > >> +CheckFirmwareImage ( > >> + CONST CHAR16* FirmwareImage > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + VOID *FileBuffer; > >> + UINT64 OpenMode; > >> + UINTN FileSize; > >> + SHELL_FILE_HANDLE FileHandle = NULL; > >> + > >> + OpenMode = EFI_FILE_MODE_READ; > >> + > >> + Status = ShellOpenFileByName (FirmwareImage, &FileHandle, > OpenMode, 0); > >> + if (EFI_ERROR (Status)) { > >> + Print (L"%s: Cannot open Image file\n", CMD_NAME_STRING); > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + Status = FileHandleGetSize (FileHandle, &FileSize); > >> + if (EFI_ERROR (Status)) { > >> + Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING); > >> + } > >> + > >> + FileBuffer = AllocateZeroPool (FileSize); > >> + > >> + // Read Image header into buffer > >> + Status = FileHandleRead (FileHandle, &FileSize, FileBuffer); > >> + if (EFI_ERROR (Status)) { > >> + Print (L"%s: Cannot read Image file header\n", CMD_NAME_STRING); > >> + ShellCloseFile (&FileHandle); > >> + FreePool (FileBuffer); > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + Status = CheckImageHeader (FileBuffer); > >> + if (EFI_ERROR(Status)) { > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + FreePool (FileBuffer); > >> + > >> + return EFI_SUCCESS; > >> +} > >> + > >> +STATIC > >> +CONST CHAR16 * > >> +FileNameFromFilePath ( > >> + CONST CHAR16 *RemoteFilePath > >> + ) > >> +{ > >> + CONST CHAR16 *Walker; > >> + > >> + // Gather FileName from FilePath > >> + Walker = RemoteFilePath + StrLen (RemoteFilePath); > >> + while ((--Walker) >= RemoteFilePath) { > >> + if ((*Walker == L'\\') || (*Walker == L'/')) { > >> + break; > >> + } > >> + } > >> + > >> + return (Walker + 1); > >> +} > >> + > >> +STATIC > >> +CONST CHAR16* > >> +PrepareFile ( > >> + LIST_ENTRY *CheckPackage > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + CONST CHAR16 *ValueStr; > >> + > >> + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-f"); > >> + if (ValueStr == NULL) { > >> + Print (L"%s: No LocalFilePath parameter!\n", CMD_NAME_STRING); > >> + return NULL; > >> + } else { > >> + Status = ShellIsFile (ValueStr); > >> + if (EFI_ERROR(Status)) { > >> + Print (L"%s: Wrong LocalFilePath parameter!\n", > CMD_NAME_STRING); > >> + return NULL; > >> + } > >> + } > >> + return ValueStr; > >> +} > >> + > >> +CONST CHAR16 gShellFUpdateFileName[] = L"ShellCommand"; > >> +EFI_HANDLE gShellFUpdateHiiHandle = NULL; > >> +EFI_HANDLE gShellFUpdateHiiHandle; > >> + > >> +STATIC CONST SHELL_PARAM_ITEM ParamList[] = { > >> + {L"help", TypeFlag}, > >> + {L"-t", TypeFlag}, > >> + {L"-f", TypeValue}, > >> + {NULL , TypeMax} > >> + }; > >> + > >> +/** > >> + Return the file name of the help text file if not using HII. > >> + > >> + @return The string pointer to the file name. > >> +**/ > >> +CONST CHAR16* > >> +EFIAPI > >> +ShellCommandGetManFileNameFUpdate ( > >> + VOID > >> + ) > >> +{ > >> + > >> + return gShellFUpdateFileName; > >> +} > >> + > >> +VOID > >> +FUpdateUsage ( > >> + VOID > >> + ) > >> +{ > >> + Print (L"\nFirmware update command\n" > >> + "fupdate [-f <LocalFilePath>] [-t <Host> <RemoteFilePath>]\n\n" > >> + "LocalFilePath - path to local firmware image file\n" > >> + "Host - IP number of TFTP server\n" > >> + "RemoteFilePath - path to firmware image file on TFTP server\n" > >> + "Examples:\n" > >> + "Update firmware from file fs2:Uefi.img\n" > >> + " fupdate -f fs2:Uefi.img\n" > >> + "Update firmware from file path/Uefi.img TFTP server with IP " > >> + "address 10.0.0.200\n" > >> + " fupdate -t 10.0.0.200 path/Uefi.img\n" > >> + ); > >> +} > >> + > >> +SHELL_STATUS > >> +EFIAPI > >> +ShellCommandRunFUpdate ( > >> + IN EFI_HANDLE ImageHandle, > >> + IN EFI_SYSTEM_TABLE *SystemTable > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + LIST_ENTRY *CheckPackage; > >> + CHAR16 *ProblemParam, *TftpCmd = NULL, *SfCmd = NULL; > >> + CONST CHAR16 *RemoteFilePath, *Host, *FileToWrite; > >> + UINT8 CmdLen; > >> + BOOLEAN FileFlag, TftpFlag; > >> + > >> + Status = ShellInitialize (); > >> + if (EFI_ERROR (Status)) { > >> + Print (L"%s: Error while initializinf Shell\n", CMD_NAME_STRING); > >> + ASSERT_EFI_ERROR (Status); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + Status = ShellCommandLineParse (ParamList, &CheckPackage, > &ProblemParam, > >> + TRUE); > >> + if (EFI_ERROR (Status)) { > >> + Print (L"Parse error!\n"); > >> + } > >> + > >> + if (ShellCommandLineGetFlag (CheckPackage, L"help")) { > >> + FUpdateUsage(); > >> + return EFI_SUCCESS; > >> + } > >> + > >> + FileFlag = ShellCommandLineGetFlag (CheckPackage, L"-f"); > >> + TftpFlag = ShellCommandLineGetFlag (CheckPackage, L"-t"); > >> + > >> + if (!FileFlag && !TftpFlag) { > >> + Print (L"%s: Please specify -f or -t flag\n", CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } else if (FileFlag && !TftpFlag) { > >> + // Prepare local file to be burned into flash > >> + FileToWrite = PrepareFile (CheckPackage); > >> + if (FileToWrite == NULL) { > >> + Print (L"%s: Error while preparing file for burn\n", > CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + } else if (TftpFlag && !FileFlag) { > >> + Host = ShellCommandLineGetRawValue (CheckPackage, 1); > >> + if (Host == NULL) { > >> + Print (L"%s: No Host parameter!\n", CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + RemoteFilePath = ShellCommandLineGetRawValue (CheckPackage, 2); > >> + if (RemoteFilePath == NULL) { > >> + Print (L"%s: No remote_file_path parameter!\n", > CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + // Gather firmware image name from remote filepath > >> + FileToWrite = FileNameFromFilePath (RemoteFilePath); > >> + > >> + // Allocate buffer for tftp command string > >> + CmdLen = StrSize (TFTP_CMD_STRING) + StrSize (SPACE_STRING) + > >> + StrSize (Host) + StrSize (RemoteFilePath); > >> + TftpCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); > >> + if (TftpCmd == NULL) { > >> + Print (L"%s: Cannot allocate memory\n", CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + // Concatenate parameters and form tftp command string > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), TFTP_CMD_STRING); > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)Host); > >> + // Insert space > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 > *)RemoteFilePath); > >> + > >> + RunShellCommand (TftpCmd, &Status); > >> + FreePool (TftpCmd); > >> + if (EFI_ERROR(Status)) { > >> + Print (L"%s: Error while performing tftp command\n", > CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + } else { > >> + Print (L"%s: Both -f and -t flag specified, please choose one\n", > >> + CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + // Check image checksum and magic > >> + Status = CheckFirmwareImage (FileToWrite); > >> + if (EFI_ERROR(Status)) { > >> + Print (L"%s: Wrong firmware Image\n", CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + // Probe spi bus > >> + RunShellCommand (SF_PROBE_CMD_STRING, &Status); > >> + if (EFI_ERROR(Status)) { > >> + Print (L"%s: Error while performing sf probe\n", > CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + // Allocate buffer for sf command string > >> + CmdLen = StrSize (SF_WRITE_CMD_STRING) + StrSize (FileToWrite) + > >> + StrSize (SPACE_STRING) + StrSize (SF_LOAD_ADDR_STRING); > >> + SfCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); > >> + if (SfCmd == NULL) { > >> + Print (L"%s: Cannot allocate memory\n"); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + // Concatenate parameters and form command string > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SF_WRITE_CMD_STRING); > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)FileToWrite); > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SF_LOAD_ADDR_STRING); > >> + > >> + // Update firmware image in flash > >> + RunShellCommand (SfCmd, &Status); > >> + FreePool (SfCmd); > >> + if (EFI_ERROR(Status)) { > >> + Print (L"%s: Error while performing sf update\n", > CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + return EFI_SUCCESS; > >> +} > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +ShellFUpdateCommandConstructor ( > >> + IN EFI_HANDLE ImageHandle, > >> + IN EFI_SYSTEM_TABLE *SystemTable > >> + ) > >> +{ > >> + EFI_STATUS Status; > >> + > >> + gShellFUpdateHiiHandle = NULL; > >> + > >> + gShellFUpdateHiiHandle = HiiAddPackages ( > >> + &gShellFUpdateHiiGuid, gImageHandle, > >> + UefiShellFUpdateCommandLibStrings, NULL > >> + ); > >> + if (gShellFUpdateHiiHandle == NULL) { > >> + Print (L"%s: Cannot add Hii package\n", CMD_NAME_STRING); > >> + return EFI_DEVICE_ERROR; > >> + } > >> + > >> + Status = ShellCommandRegisterCommandName ( > >> + CMD_NAME_STRING, ShellCommandRunFUpdate, > ShellCommandGetManFileNameFUpdate, > >> + 0, CMD_NAME_STRING, TRUE , gShellFUpdateHiiHandle, > >> + STRING_TOKEN (STR_GET_HELP_FUPDATE) > >> + ); > >> + if (EFI_ERROR(Status)) { > >> + Print (L"%s: Error while registering command\n", > CMD_NAME_STRING); > >> + return SHELL_ABORTED; > >> + } > >> + > >> + return EFI_SUCCESS; > >> +} > >> + > >> +EFI_STATUS > >> +EFIAPI > >> +ShellFUpdateCommandDestructor ( > >> + IN EFI_HANDLE ImageHandle, > >> + IN EFI_SYSTEM_TABLE *SystemTable > >> + ) > >> +{ > >> + > >> + if (gShellFUpdateHiiHandle != NULL) { > >> + HiiRemovePackages (gShellFUpdateHiiHandle); > >> + } > >> + return EFI_SUCCESS; > >> +} > >> diff --git a/Applications/FirmwareUpdate/FUpdate.inf > b/Applications/FirmwareUpdate/FUpdate.inf > >> new file mode 100644 > >> index 0000000..53b5305 > >> --- /dev/null > >> +++ b/Applications/FirmwareUpdate/FUpdate.inf > >> @@ -0,0 +1,68 @@ > >> +# > >> +# Marvell BSD License Option > >> +# > >> +# If you received this File from Marvell, you may opt to use, redistribute > >> +# and/or modify this File under the following licensing terms. > >> +# Redistribution and use in source and binary forms, with or without > >> +# modification, are permitted provided that the following conditions are > met: > >> +# > >> +# * Redistributions of source code must retain the above copyright > notice, > >> +# this list of conditions and the following disclaimer. > >> +# > >> +# * Redistributions in binary form must reproduce the above copyright > >> +# notice, this list of conditions and the following disclaimer in the > >> +# documentation and/or other materials provided with the distribution. > >> +# > >> +# * Neither the name of Marvell nor the names of its contributors may > be > >> +# used to endorse or promote products derived from this software > without > >> +# specific prior written permission. > >> +# > >> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > CONTRIBUTORS "AS IS" > >> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > LIMITED TO, THE > >> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > PARTICULAR PURPOSE ARE > >> +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > CONTRIBUTORS BE LIABLE > >> +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > CONSEQUENTIAL > >> +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > SUBSTITUTE GOODS OR > >> +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS > INTERRUPTION) HOWEVER > >> +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, > STRICT LIABILITY, > >> +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY > WAY OUT OF THE USE > >> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > DAMAGE. > >> +# > >> + > >> +[Defines] > >> + INF_VERSION = 0x00010006 > >> + BASE_NAME = UefiShellFUpdateCommandLib > >> + FILE_GUID = 470292b2-926b-4ed8-8080-be7a260db627 > >> + MODULE_TYPE = UEFI_APPLICATION > >> + VERSION_STRING = 0.1 > >> + LIBRARY_CLASS = NULL|UEFI_APPLICATION UEFI_DRIVER > >> + CONSTRUCTOR = ShellFUpdateCommandConstructor > >> + DESTRUCTOR = ShellFUpdateCommandDestructor > >> + > >> +[Sources] > >> + FUpdate.c > >> + FUpdate.uni > >> + > >> +[Packages] > >> + MdePkg/MdePkg.dec > >> + ShellPkg/ShellPkg.dec > >> + MdeModulePkg/MdeModulePkg.dec > >> + OpenPlatformPkg/Platforms/Marvell/Marvell.dec > >> + > >> +[LibraryClasses] > >> + UefiLib > >> + UefiBootServicesTableLib > >> + MemoryAllocationLib > >> + BaseLib > >> + BaseMemoryLib > >> + DebugLib > >> + ShellCommandLib > >> + ShellLib > >> + UefiLib > >> + UefiRuntimeServicesTableLib > >> + PcdLib > >> + HiiLib > >> + FileHandleLib > >> + > >> +[Guids] > >> + gShellFUpdateHiiGuid > >> diff --git a/Applications/FirmwareUpdate/FUpdate.uni > b/Applications/FirmwareUpdate/FUpdate.uni > >> new file mode 100644 > >> index > 0000000000000000000000000000000000000000..6143c0580f1d1ef8ce4fe619a1 > df1ce380adf2c0 > >> GIT binary patch > >> literal 5838 > >> > zcmd6r?N1v=5XSd&rTz~m@+FOuK&z@BXdB5G6OhbHeI_(&s)*P?EX6>LDdL} > R`}}4Y > >> z_Fim)B2|h`_TBEy&OY<Z%kJL4|E#8GU- > k8E`X+VLMY>G4X_hAGEPYJ#RHUcs{Z98E > >> > z(pvgH{iLVebS#O#(@FZIt4KTPT#_g1JJ*}J#$MPH@A_(w)60Z*e$BBPsZ5(DH% > TMu > >> zI8RslFVenngr*p~lTP*KR@$$1pGl4p6GJjq&s>Nn8egO>- > 9MS0Q^{SVsru?OKUYs^ > >> znhN!;+Src8b3GNB10={X)7Ui6^*l)*^bUIqjXT$yRmsoHx~ZO!JT- > lb@LbynxN^K` > >> zIn^kbyht}1doJb+wK5;k=NwOk%lRBE#O3S<wy>ix&4tFo- > prWGG9T<MUWEtn%*5H1 > >> > zSq~;Gfgeyu8$Ga<Z3c1n|8rvmo!EG(e{`Y?JMh<~dM@O>iCW&nJZN~Bn~_Gr > JG$Ij > >> > zd>%7RrE8wn?<20zyXy5!t%sOH*M&t|ohxPqCl(VMpv1PhX2wGq^RXfXyO3p > 6Go#Mc > >> z2Xdm<S*qfT=Nvu6g3sr;me~&{8O5Tg@+clDy1)!mvr2Bp%yEAVQ*Qa<_- > Qib{vL}x > >> > zEYC<E=8NnPMAdtpMc3xp|Nrt8nS_5%EXt~=;(06|k7OwvQyp_&MAmuLAl6( > HxkV-N > >> zET8D<lf1qtOCkd^gYD|%d6DaG;EFrG- > 4iy??Rq_;ii11ypk6r(+2fVKanpABoH=Nv > >> z<&j#gnvCR4)Z7mbaOf4-W15N#lkkcLM+avfRa7Cb$<- > )x6l5)Rn~X$TUK4VTk?W_@ > >> > za3fp1Vg<`y=axm`l~uNpw4drXy2AteUMrc`D;^ahSHUaU(DczTZEGzpC0A<w+ > e*W9 > >> zlsc+aJ@t71fm-|Njar-P-BkZZdaHj%BGuK~1N96f@mhUD^=+%KZ}J1v- > I3oJiR_*v > >> zkljd!dfL}Bv#)4Y?##KOyN`eYo|vzfe%Cx8D@~G- > $nUDRue)P&^aC3;wBFKIlpSg5 > >> > zO72iNcz$iYj)AD$H{9si)o*yG4ScM|VQYOh&@Gyeoy3+#?Mll)vu@v^>B#0C!# > XsJ > >> > zf|V_saqr%;En@`^3fYD?u%s*OeQVKssNMs~l^TU4ynqMmO0%@1ClG9^+Q > QSG{;?A# > >> z-{^- > Q@H*)Fy0VpvJi0GSK?vv0{BmDzNRD0Djx=xRPRnho`V*v}?g|GSgK$w_S8s?s > >> z@PZWCQnoXa8N7$+=- > tHvY`itODpHvGO}x1#A6ZCqN25B52jU8RPYubJ$BO3*-oiRO > >> z;7of=l<H^yiARTeCejMczOQRj{lPA12bOhRJ(cNa!`7EnL_ZOBs8- > J`#SZv^9k<$X > >> > zcx*8~`ic3{Y>Z5Td2kGL9g117k*Dmy$>bxwBVzD<S9AER<a0zY(pBGjzJP<wb;~ > 0j > >> > zat46)NHg>_BmbVy#A%ML&V78q8Fhnr$<bBU=9YU^87}oEdC(K*Aj0od2E5>- > sRWMZ > >> > zNblhn9*8MgUOOV6qBc<l+)_Lm7Dv@^zN)=wg0E)Hg^|ck=D^1t#Rb(DByjF* > hHkF` > >> z;HuY_9BtH}dYtDK`s+R&t{O- > 1w3>cRPwrb!Y%j@~g;kdG85b+{3yV!w6Iy3>qPfzZ > >> zX{9ojT`eo5pLsm0-dh|D(|hxLUpeqz+>;BQS42nq$~dmIf}d&5CJLW-MHLu- > u?x7? > >> zdOVi2?;F<Cx1yOQLNu- > H0}g9a>}aO8dtoo~QX}3;{+%qy`l;i@HPx)wdSBPauAx3( > >> z>^nf%KE)1&a40S~$+4?pPs1L58H!T5z)Evq`BJvzidIjPmp|l5{Q8S>lszr`Qdr`A > >> > zePHZ;Os~?Pl0b59lG~ERx38?lX)E?;i$*TP4(h0Xk3mjUJT#X_%?n8_X0gin<(*~h > >> > zvDtgF@4S(hIi(+~mG|D+OX0buJ?&^+)>Dm0=76ngbhQ19reJ%S)<S+^oo4sxJJ > nmY > >> zGY6%`kVg`g?m$uXqa?6%tocvmOTIZQvTJ!v1RCa!I0- > gk(Id$@uiD7@iqnDTaIo8+ > >> zyV>(n7Ls@DKFB<FqO7V- > xpu@iN36HTd@g76i6Zb~*Ok7Rt*dfT7boHmyUtp=;i;K( > >> zo^g- > W@FnOXzE@lLs*zExG5O1n$4E0i3Vj`1la=w~d*Rw6t)*%#8qJA``utI=&gXax > >> z|FhF$H#bn-d!tC;obk9gsE#aZW7`pd*NC^C^!K$wv6t-H-o^KX5&g~k(7qbL- > AH!% > >> z2@qzF6)k>Vp2}uwYkVd1H)P>oTCvDQf3r9rtC- > mFvyX}sR8P6g7q^*ax)F|8UHrt% > >> QHyJ;b`f6NX>FQbZ9}*T`00000 > >> > >> literal 0 > >> HcmV?d00001 > >> > >> diff --git a/Platforms/Marvell/Marvell.dec > b/Platforms/Marvell/Marvell.dec > >> index e63add4..f9d2660 100644 > >> --- a/Platforms/Marvell/Marvell.dec > >> +++ b/Platforms/Marvell/Marvell.dec > >> @@ -54,6 +54,7 @@ > >> > >> gShellEepromHiiGuid = { 0xb2f4c714, 0x147f, 0x4ff7, { 0x82, 0x1b, 0xce, > 0x7b, 0x91, 0x7f, 0x5f, 0x2f } } > >> gShellSfHiiGuid = { 0x03a67756, 0x8cde, 0x4638, { 0x82, 0x34, 0x4a, 0x0f, > 0x6d, 0x58, 0x81, 0x39 } } > >> + gShellFUpdateHiiGuid = { 0x9b5d2176, 0x590a, 0x49db, { 0x89, 0x5d, > 0x4a, 0x70, 0xfe, 0xad, 0xbe, 0x24 } } > >> > >> [PcdsFixedAtBuild.common] > >> #MPP > >> -- > >> 1.8.3.1 > >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell 2016-11-16 17:05 ` Carsey, Jaben @ 2016-11-16 17:35 ` Leif Lindholm 2016-11-16 21:31 ` Carsey, Jaben 0 siblings, 1 reply; 7+ messages in thread From: Leif Lindholm @ 2016-11-16 17:35 UTC (permalink / raw) To: Carsey, Jaben Cc: Marcin Wojtas, Ard Biesheuvel, linaro-uefi, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dabros, Bartosz Szczepanek, edk2-devel-01, Ni, Ruiyu Hi Jaben, I've seen a few different solutions to the same problem of providing platform-specific "shorthand" commands, my least favourite one being embedding a locally hacked EBL. I guess we're ending up with something halfway between an application and a script. Is it your view that this should always be implemented as a standalone application, to be called from the UI config menus? Basically, is there an expected usage pattern that people keep missing? Regards, Leif On Wed, Nov 16, 2016 at 05:05:36PM +0000, Carsey, Jaben wrote: > Marcin and Ard, > > It sounds like there is a goal to have a UEFI Application call into an internal shell command. > > There is no documented method to programmatically call an internal > command. This is not a goal of the current shell > implementation. ShellExecute explicitly does not allow running of > internal commands (per UEFI Shell Specification 2.2). > > Thus I don’t think using ShellDynamicCommandProtocol will help. > That protocol is for allowing a driver add a command to the shell's > list of available commands. > > -Jaben > > > -----Original Message----- > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > Sent: Wednesday, November 16, 2016 8:13 AM > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Cc: linaro-uefi <linaro-uefi@lists.linaro.org>; Leif Lindholm > > <leif.lindholm@linaro.org>; Neta Zur Hershkovits <neta@marvell.com>; > > Yehuda Yitschak <yehuday@marvell.com>; Haim Boot > > <hayim@marvell.com>; Jan Dąbroś <jsd@semihalf.com>; Bartosz > > Szczepanek <bsz@semihalf.com>; edk2-devel-01 <edk2-devel@lists.01.org>; > > Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com> > > Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' > > comand to shell > > Importance: High > > > > + Jaben, Ruiyu and edk2 list > > > > Hi Ard, > > > > Internal Shell header ("../../../ShellPkg/Application/Shell/Shell.h") > > was added ONLY for being able to execute shell commands (tftp and our > > custom SPI flash control) from within a new command. This is now done > > with: > > RunShellCommand (TftpCmd, &Status); > > And it works fine. > > > > Replacing above with: > > ShellExecute (&ImageHandle, TftpCmd, FALSE, NULL, &Status); > > allow to get rid of relative include and seems more appropriate, but > > there is completely no effect of calling it. > > > > I checked in edk2 sources and see no relation between above problem > > and registering our command with help of ShellDynamicCommand protocol. > > Unless I don't know something. Anyway, I'll apreciate any hint, how to > > solve the issue in a nice way. > > > > Best regards, > > Marcin > > > > 2016-11-15 17:09 GMT+01:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>: > > > On 10 July 2016 at 00:21, Marcin Wojtas <mw@semihalf.com> wrote: > > >> From: Jan Dąbroś <jsd@semihalf.com> > > >> > > >> 'fupdate' command performs updating firmware from file placed on local > > >> filesystem or on TFTP server. > > >> It uses tftp and sf commands. > > >> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Jan Dabros <jsd@semihalf.com> > > >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > > > > Please use the Shell dynamic command protocol to add commands to the > > > shell. This will remove the need for including internal Shell headers, > > > and even allow your commands to work with prebuilt Shell binaries. > > > > > > MdePkg/Include/Protocol/ShellDynamicCommand.h > > > > > > Thanks, > > > Ard. > > > > > > > > >> --- > > >> Applications/FirmwareUpdate/FUpdate.c | 448 > > ++++++++++++++++++++++++++++++++ > > >> Applications/FirmwareUpdate/FUpdate.inf | 68 +++++ > > >> Applications/FirmwareUpdate/FUpdate.uni | Bin 0 -> 5838 bytes > > >> Platforms/Marvell/Marvell.dec | 1 + > > >> 4 files changed, 517 insertions(+) > > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.c > > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.inf > > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.uni > > >> > > >> diff --git a/Applications/FirmwareUpdate/FUpdate.c > > b/Applications/FirmwareUpdate/FUpdate.c > > >> new file mode 100644 > > >> index 0000000..359a4ac > > >> --- /dev/null > > >> +++ b/Applications/FirmwareUpdate/FUpdate.c > > >> @@ -0,0 +1,448 @@ > > >> > > +/********************************************************* > > ********************** > > >> +Copyright (C) 2016 Marvell International Ltd. > > >> + > > >> +Marvell BSD License Option > > >> + > > >> +If you received this File from Marvell, you may opt to use, redistribute > > and/or > > >> +modify this File under the following licensing terms. > > >> +Redistribution and use in source and binary forms, with or without > > modification, > > >> +are permitted provided that the following conditions are met: > > >> + > > >> +* Redistributions of source code must retain the above copyright notice, > > >> + this list of conditions and the following disclaimer. > > >> + > > >> +* Redistributions in binary form must reproduce the above copyright > > >> + notice, this list of conditions and the following disclaimer in the > > >> + documentation and/or other materials provided with the distribution. > > >> + > > >> +* Neither the name of Marvell nor the names of its contributors may be > > >> + used to endorse or promote products derived from this software > > without > > >> + specific prior written permission. > > >> + > > >> +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > > CONTRIBUTORS "AS IS" AND > > >> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED > > TO, THE IMPLIED > > >> +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR > > PURPOSE ARE > > >> +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > > CONTRIBUTORS BE LIABLE FOR > > >> +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > > CONSEQUENTIAL DAMAGES > > >> +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE > > GOODS OR SERVICES; > > >> +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > > HOWEVER CAUSED AND ON > > >> +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR > > TORT > > >> +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF > > THE USE OF THIS > > >> +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > >> + > > >> > > +********************************************************* > > **********************/ > > >> +#include <Uefi.h> > > >> +#include <ShellBase.h> > > >> + > > >> +#include <Library/BaseLib.h> > > >> +#include <Library/DebugLib.h> > > >> +#include <Library/MemoryAllocationLib.h> > > >> +#include <Library/ShellCommandLib.h> > > >> +#include <Library/ShellLib.h> > > >> +#include <Library/UefiLib.h> > > >> +#include <Library/PrintLib.h> > > >> +#include <Library/UefiLib.h> > > >> +#include <Library/ShellCEntryLib.h> > > >> +#include <Library/HiiLib.h> > > >> +#include <Guid/ShellLibHiiGuid.h> > > >> +#include <Library/UefiBootServicesTableLib.h> > > >> +#include <Library/FileHandleLib.h> > > >> +// FIXME: Remove including relative path > > >> +#include "../../../ShellPkg/Application/Shell/Shell.h" > > >> + > > >> +#define CMD_NAME_STRING L"fupdate" > > >> + > > >> +#define TFTP_CMD_STRING L"tftp " > > >> +#define SPACE_STRING L" " > > >> +#define SF_PROBE_CMD_STRING L"sf probe" > > >> +#define SF_WRITE_CMD_STRING L"sf updatefile " > > >> +#define SF_LOAD_ADDR_STRING L"0x0" > > >> + > > >> +#define MAIN_HDR_MAGIC 0xB105B002 > > >> + > > >> +typedef struct { > > >> + UINT32 Magic; // 0-3 > > >> + UINT32 PrologSize; // 4-7 > > >> + UINT32 PrologChecksum; // 8-11 > > >> + UINT32 BootImageSize; // 12-15 > > >> + UINT32 BootImageChecksum; // 16-19 > > >> + UINT32 Reserved0; // 20-23 > > >> + UINT32 LoadAddr; // 24-27 > > >> + UINT32 ExecAddr; // 28-31 > > >> + UINT8 UartConfig; // 32 > > >> + UINT8 Baudrate; // 33 > > >> + UINT8 ExtCount; // 34 > > >> + UINT8 AuxFlags; // 35 > > >> + UINT32 IoArg0; // 36-39 > > >> + UINT32 IoArg1; // 40-43 > > >> + UINT32 IoArg2; // 43-47 > > >> + UINT32 IoArg3; // 48-51 > > >> + UINT32 Reserved1; // 52-55 > > >> + UINT32 Reserved2; // 56-59 > > >> + UINT32 Reserved3; // 60-63 > > >> +} MV_IMAGE_HEADER; > > >> + > > >> +STATIC > > >> +UINT32 > > >> +CountChecksum ( > > >> + UINT32 *Start, > > >> + UINT32 Length > > >> + ) > > >> +{ > > >> + UINT32 Sum = 0; > > >> + UINT32 *Startp = Start; > > >> + > > >> + do { > > >> + Sum += *Startp; > > >> + Startp++; > > >> + Length -= 4; > > >> + } while (Length > 0); > > >> + > > >> + return Sum; > > >> +} > > >> + > > >> +STATIC > > >> +EFI_STATUS > > >> +CheckImageHeader ( > > >> + IN VOID *ImageHeader > > >> + ) > > >> +{ > > >> + MV_IMAGE_HEADER *Header; > > >> + UINT32 HeaderLength, Checksum, ChecksumBackup; > > >> + > > >> + Header = (MV_IMAGE_HEADER *) ImageHeader; > > >> + HeaderLength = Header->PrologSize; > > >> + ChecksumBackup = Header->PrologChecksum; > > >> + > > >> + // Compare magic number > > >> + if (Header->Magic != MAIN_HDR_MAGIC) { > > >> + Print (L"%s: Bad Image magic 0x%08x != 0x%08x\n", > > CMD_NAME_STRING, > > >> + Header->Magic, MAIN_HDR_MAGIC); > > >> + return EFI_DEVICE_ERROR; > > >> + } > > >> + > > >> + // The checksum field is discarded from calculation > > >> + Header->PrologChecksum = 0; > > >> + > > >> + Checksum = CountChecksum((UINT32 *)Header, HeaderLength); > > >> + if (Checksum != ChecksumBackup) { > > >> + Print (L"%s: Bad Image checksum. 0x%x != 0x%x\n", > > CMD_NAME_STRING, Checksum, > > >> + ChecksumBackup); > > >> + return EFI_DEVICE_ERROR; > > >> + } > > >> + > > >> + // Restore checksum backup > > >> + Header->PrologChecksum = ChecksumBackup; > > >> + > > >> + return 0; > > >> +} > > >> + > > >> +STATIC > > >> +EFI_STATUS > > >> +CheckFirmwareImage ( > > >> + CONST CHAR16* FirmwareImage > > >> + ) > > >> +{ > > >> + EFI_STATUS Status; > > >> + VOID *FileBuffer; > > >> + UINT64 OpenMode; > > >> + UINTN FileSize; > > >> + SHELL_FILE_HANDLE FileHandle = NULL; > > >> + > > >> + OpenMode = EFI_FILE_MODE_READ; > > >> + > > >> + Status = ShellOpenFileByName (FirmwareImage, &FileHandle, > > OpenMode, 0); > > >> + if (EFI_ERROR (Status)) { > > >> + Print (L"%s: Cannot open Image file\n", CMD_NAME_STRING); > > >> + return EFI_DEVICE_ERROR; > > >> + } > > >> + > > >> + Status = FileHandleGetSize (FileHandle, &FileSize); > > >> + if (EFI_ERROR (Status)) { > > >> + Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING); > > >> + } > > >> + > > >> + FileBuffer = AllocateZeroPool (FileSize); > > >> + > > >> + // Read Image header into buffer > > >> + Status = FileHandleRead (FileHandle, &FileSize, FileBuffer); > > >> + if (EFI_ERROR (Status)) { > > >> + Print (L"%s: Cannot read Image file header\n", CMD_NAME_STRING); > > >> + ShellCloseFile (&FileHandle); > > >> + FreePool (FileBuffer); > > >> + return EFI_DEVICE_ERROR; > > >> + } > > >> + > > >> + Status = CheckImageHeader (FileBuffer); > > >> + if (EFI_ERROR(Status)) { > > >> + return EFI_DEVICE_ERROR; > > >> + } > > >> + > > >> + FreePool (FileBuffer); > > >> + > > >> + return EFI_SUCCESS; > > >> +} > > >> + > > >> +STATIC > > >> +CONST CHAR16 * > > >> +FileNameFromFilePath ( > > >> + CONST CHAR16 *RemoteFilePath > > >> + ) > > >> +{ > > >> + CONST CHAR16 *Walker; > > >> + > > >> + // Gather FileName from FilePath > > >> + Walker = RemoteFilePath + StrLen (RemoteFilePath); > > >> + while ((--Walker) >= RemoteFilePath) { > > >> + if ((*Walker == L'\\') || (*Walker == L'/')) { > > >> + break; > > >> + } > > >> + } > > >> + > > >> + return (Walker + 1); > > >> +} > > >> + > > >> +STATIC > > >> +CONST CHAR16* > > >> +PrepareFile ( > > >> + LIST_ENTRY *CheckPackage > > >> + ) > > >> +{ > > >> + EFI_STATUS Status; > > >> + CONST CHAR16 *ValueStr; > > >> + > > >> + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-f"); > > >> + if (ValueStr == NULL) { > > >> + Print (L"%s: No LocalFilePath parameter!\n", CMD_NAME_STRING); > > >> + return NULL; > > >> + } else { > > >> + Status = ShellIsFile (ValueStr); > > >> + if (EFI_ERROR(Status)) { > > >> + Print (L"%s: Wrong LocalFilePath parameter!\n", > > CMD_NAME_STRING); > > >> + return NULL; > > >> + } > > >> + } > > >> + return ValueStr; > > >> +} > > >> + > > >> +CONST CHAR16 gShellFUpdateFileName[] = L"ShellCommand"; > > >> +EFI_HANDLE gShellFUpdateHiiHandle = NULL; > > >> +EFI_HANDLE gShellFUpdateHiiHandle; > > >> + > > >> +STATIC CONST SHELL_PARAM_ITEM ParamList[] = { > > >> + {L"help", TypeFlag}, > > >> + {L"-t", TypeFlag}, > > >> + {L"-f", TypeValue}, > > >> + {NULL , TypeMax} > > >> + }; > > >> + > > >> +/** > > >> + Return the file name of the help text file if not using HII. > > >> + > > >> + @return The string pointer to the file name. > > >> +**/ > > >> +CONST CHAR16* > > >> +EFIAPI > > >> +ShellCommandGetManFileNameFUpdate ( > > >> + VOID > > >> + ) > > >> +{ > > >> + > > >> + return gShellFUpdateFileName; > > >> +} > > >> + > > >> +VOID > > >> +FUpdateUsage ( > > >> + VOID > > >> + ) > > >> +{ > > >> + Print (L"\nFirmware update command\n" > > >> + "fupdate [-f <LocalFilePath>] [-t <Host> <RemoteFilePath>]\n\n" > > >> + "LocalFilePath - path to local firmware image file\n" > > >> + "Host - IP number of TFTP server\n" > > >> + "RemoteFilePath - path to firmware image file on TFTP server\n" > > >> + "Examples:\n" > > >> + "Update firmware from file fs2:Uefi.img\n" > > >> + " fupdate -f fs2:Uefi.img\n" > > >> + "Update firmware from file path/Uefi.img TFTP server with IP " > > >> + "address 10.0.0.200\n" > > >> + " fupdate -t 10.0.0.200 path/Uefi.img\n" > > >> + ); > > >> +} > > >> + > > >> +SHELL_STATUS > > >> +EFIAPI > > >> +ShellCommandRunFUpdate ( > > >> + IN EFI_HANDLE ImageHandle, > > >> + IN EFI_SYSTEM_TABLE *SystemTable > > >> + ) > > >> +{ > > >> + EFI_STATUS Status; > > >> + LIST_ENTRY *CheckPackage; > > >> + CHAR16 *ProblemParam, *TftpCmd = NULL, *SfCmd = NULL; > > >> + CONST CHAR16 *RemoteFilePath, *Host, *FileToWrite; > > >> + UINT8 CmdLen; > > >> + BOOLEAN FileFlag, TftpFlag; > > >> + > > >> + Status = ShellInitialize (); > > >> + if (EFI_ERROR (Status)) { > > >> + Print (L"%s: Error while initializinf Shell\n", CMD_NAME_STRING); > > >> + ASSERT_EFI_ERROR (Status); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + Status = ShellCommandLineParse (ParamList, &CheckPackage, > > &ProblemParam, > > >> + TRUE); > > >> + if (EFI_ERROR (Status)) { > > >> + Print (L"Parse error!\n"); > > >> + } > > >> + > > >> + if (ShellCommandLineGetFlag (CheckPackage, L"help")) { > > >> + FUpdateUsage(); > > >> + return EFI_SUCCESS; > > >> + } > > >> + > > >> + FileFlag = ShellCommandLineGetFlag (CheckPackage, L"-f"); > > >> + TftpFlag = ShellCommandLineGetFlag (CheckPackage, L"-t"); > > >> + > > >> + if (!FileFlag && !TftpFlag) { > > >> + Print (L"%s: Please specify -f or -t flag\n", CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } else if (FileFlag && !TftpFlag) { > > >> + // Prepare local file to be burned into flash > > >> + FileToWrite = PrepareFile (CheckPackage); > > >> + if (FileToWrite == NULL) { > > >> + Print (L"%s: Error while preparing file for burn\n", > > CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + } else if (TftpFlag && !FileFlag) { > > >> + Host = ShellCommandLineGetRawValue (CheckPackage, 1); > > >> + if (Host == NULL) { > > >> + Print (L"%s: No Host parameter!\n", CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + RemoteFilePath = ShellCommandLineGetRawValue (CheckPackage, 2); > > >> + if (RemoteFilePath == NULL) { > > >> + Print (L"%s: No remote_file_path parameter!\n", > > CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + // Gather firmware image name from remote filepath > > >> + FileToWrite = FileNameFromFilePath (RemoteFilePath); > > >> + > > >> + // Allocate buffer for tftp command string > > >> + CmdLen = StrSize (TFTP_CMD_STRING) + StrSize (SPACE_STRING) + > > >> + StrSize (Host) + StrSize (RemoteFilePath); > > >> + TftpCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); > > >> + if (TftpCmd == NULL) { > > >> + Print (L"%s: Cannot allocate memory\n", CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + // Concatenate parameters and form tftp command string > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), TFTP_CMD_STRING); > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)Host); > > >> + // Insert space > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 > > *)RemoteFilePath); > > >> + > > >> + RunShellCommand (TftpCmd, &Status); > > >> + FreePool (TftpCmd); > > >> + if (EFI_ERROR(Status)) { > > >> + Print (L"%s: Error while performing tftp command\n", > > CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + } else { > > >> + Print (L"%s: Both -f and -t flag specified, please choose one\n", > > >> + CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + // Check image checksum and magic > > >> + Status = CheckFirmwareImage (FileToWrite); > > >> + if (EFI_ERROR(Status)) { > > >> + Print (L"%s: Wrong firmware Image\n", CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + // Probe spi bus > > >> + RunShellCommand (SF_PROBE_CMD_STRING, &Status); > > >> + if (EFI_ERROR(Status)) { > > >> + Print (L"%s: Error while performing sf probe\n", > > CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + // Allocate buffer for sf command string > > >> + CmdLen = StrSize (SF_WRITE_CMD_STRING) + StrSize (FileToWrite) + > > >> + StrSize (SPACE_STRING) + StrSize (SF_LOAD_ADDR_STRING); > > >> + SfCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); > > >> + if (SfCmd == NULL) { > > >> + Print (L"%s: Cannot allocate memory\n"); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + // Concatenate parameters and form command string > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SF_WRITE_CMD_STRING); > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)FileToWrite); > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SF_LOAD_ADDR_STRING); > > >> + > > >> + // Update firmware image in flash > > >> + RunShellCommand (SfCmd, &Status); > > >> + FreePool (SfCmd); > > >> + if (EFI_ERROR(Status)) { > > >> + Print (L"%s: Error while performing sf update\n", > > CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + return EFI_SUCCESS; > > >> +} > > >> + > > >> +EFI_STATUS > > >> +EFIAPI > > >> +ShellFUpdateCommandConstructor ( > > >> + IN EFI_HANDLE ImageHandle, > > >> + IN EFI_SYSTEM_TABLE *SystemTable > > >> + ) > > >> +{ > > >> + EFI_STATUS Status; > > >> + > > >> + gShellFUpdateHiiHandle = NULL; > > >> + > > >> + gShellFUpdateHiiHandle = HiiAddPackages ( > > >> + &gShellFUpdateHiiGuid, gImageHandle, > > >> + UefiShellFUpdateCommandLibStrings, NULL > > >> + ); > > >> + if (gShellFUpdateHiiHandle == NULL) { > > >> + Print (L"%s: Cannot add Hii package\n", CMD_NAME_STRING); > > >> + return EFI_DEVICE_ERROR; > > >> + } > > >> + > > >> + Status = ShellCommandRegisterCommandName ( > > >> + CMD_NAME_STRING, ShellCommandRunFUpdate, > > ShellCommandGetManFileNameFUpdate, > > >> + 0, CMD_NAME_STRING, TRUE , gShellFUpdateHiiHandle, > > >> + STRING_TOKEN (STR_GET_HELP_FUPDATE) > > >> + ); > > >> + if (EFI_ERROR(Status)) { > > >> + Print (L"%s: Error while registering command\n", > > CMD_NAME_STRING); > > >> + return SHELL_ABORTED; > > >> + } > > >> + > > >> + return EFI_SUCCESS; > > >> +} > > >> + > > >> +EFI_STATUS > > >> +EFIAPI > > >> +ShellFUpdateCommandDestructor ( > > >> + IN EFI_HANDLE ImageHandle, > > >> + IN EFI_SYSTEM_TABLE *SystemTable > > >> + ) > > >> +{ > > >> + > > >> + if (gShellFUpdateHiiHandle != NULL) { > > >> + HiiRemovePackages (gShellFUpdateHiiHandle); > > >> + } > > >> + return EFI_SUCCESS; > > >> +} > > >> diff --git a/Applications/FirmwareUpdate/FUpdate.inf > > b/Applications/FirmwareUpdate/FUpdate.inf > > >> new file mode 100644 > > >> index 0000000..53b5305 > > >> --- /dev/null > > >> +++ b/Applications/FirmwareUpdate/FUpdate.inf > > >> @@ -0,0 +1,68 @@ > > >> +# > > >> +# Marvell BSD License Option > > >> +# > > >> +# If you received this File from Marvell, you may opt to use, redistribute > > >> +# and/or modify this File under the following licensing terms. > > >> +# Redistribution and use in source and binary forms, with or without > > >> +# modification, are permitted provided that the following conditions are > > met: > > >> +# > > >> +# * Redistributions of source code must retain the above copyright > > notice, > > >> +# this list of conditions and the following disclaimer. > > >> +# > > >> +# * Redistributions in binary form must reproduce the above copyright > > >> +# notice, this list of conditions and the following disclaimer in the > > >> +# documentation and/or other materials provided with the distribution. > > >> +# > > >> +# * Neither the name of Marvell nor the names of its contributors may > > be > > >> +# used to endorse or promote products derived from this software > > without > > >> +# specific prior written permission. > > >> +# > > >> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > > CONTRIBUTORS "AS IS" > > >> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > LIMITED TO, THE > > >> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > > PARTICULAR PURPOSE ARE > > >> +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > > CONTRIBUTORS BE LIABLE > > >> +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > > CONSEQUENTIAL > > >> +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > > SUBSTITUTE GOODS OR > > >> +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS > > INTERRUPTION) HOWEVER > > >> +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, > > STRICT LIABILITY, > > >> +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY > > WAY OUT OF THE USE > > >> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > DAMAGE. > > >> +# > > >> + > > >> +[Defines] > > >> + INF_VERSION = 0x00010006 > > >> + BASE_NAME = UefiShellFUpdateCommandLib > > >> + FILE_GUID = 470292b2-926b-4ed8-8080-be7a260db627 > > >> + MODULE_TYPE = UEFI_APPLICATION > > >> + VERSION_STRING = 0.1 > > >> + LIBRARY_CLASS = NULL|UEFI_APPLICATION UEFI_DRIVER > > >> + CONSTRUCTOR = ShellFUpdateCommandConstructor > > >> + DESTRUCTOR = ShellFUpdateCommandDestructor > > >> + > > >> +[Sources] > > >> + FUpdate.c > > >> + FUpdate.uni > > >> + > > >> +[Packages] > > >> + MdePkg/MdePkg.dec > > >> + ShellPkg/ShellPkg.dec > > >> + MdeModulePkg/MdeModulePkg.dec > > >> + OpenPlatformPkg/Platforms/Marvell/Marvell.dec > > >> + > > >> +[LibraryClasses] > > >> + UefiLib > > >> + UefiBootServicesTableLib > > >> + MemoryAllocationLib > > >> + BaseLib > > >> + BaseMemoryLib > > >> + DebugLib > > >> + ShellCommandLib > > >> + ShellLib > > >> + UefiLib > > >> + UefiRuntimeServicesTableLib > > >> + PcdLib > > >> + HiiLib > > >> + FileHandleLib > > >> + > > >> +[Guids] > > >> + gShellFUpdateHiiGuid > > >> diff --git a/Applications/FirmwareUpdate/FUpdate.uni > > b/Applications/FirmwareUpdate/FUpdate.uni > > >> new file mode 100644 > > >> index > > 0000000000000000000000000000000000000000..6143c0580f1d1ef8ce4fe619a1 > > df1ce380adf2c0 > > >> GIT binary patch > > >> literal 5838 > > >> > > zcmd6r?N1v=5XSd&rTz~m@+FOuK&z@BXdB5G6OhbHeI_(&s)*P?EX6>LDdL} > > R`}}4Y > > >> z_Fim)B2|h`_TBEy&OY<Z%kJL4|E#8GU- > > k8E`X+VLMY>G4X_hAGEPYJ#RHUcs{Z98E > > >> > > z(pvgH{iLVebS#O#(@FZIt4KTPT#_g1JJ*}J#$MPH@A_(w)60Z*e$BBPsZ5(DH% > > TMu > > >> zI8RslFVenngr*p~lTP*KR@$$1pGl4p6GJjq&s>Nn8egO>- > > 9MS0Q^{SVsru?OKUYs^ > > >> znhN!;+Src8b3GNB10={X)7Ui6^*l)*^bUIqjXT$yRmsoHx~ZO!JT- > > lb@LbynxN^K` > > >> zIn^kbyht}1doJb+wK5;k=NwOk%lRBE#O3S<wy>ix&4tFo- > > prWGG9T<MUWEtn%*5H1 > > >> > > zSq~;Gfgeyu8$Ga<Z3c1n|8rvmo!EG(e{`Y?JMh<~dM@O>iCW&nJZN~Bn~_Gr > > JG$Ij > > >> > > zd>%7RrE8wn?<20zyXy5!t%sOH*M&t|ohxPqCl(VMpv1PhX2wGq^RXfXyO3p > > 6Go#Mc > > >> z2Xdm<S*qfT=Nvu6g3sr;me~&{8O5Tg@+clDy1)!mvr2Bp%yEAVQ*Qa<_- > > Qib{vL}x > > >> > > zEYC<E=8NnPMAdtpMc3xp|Nrt8nS_5%EXt~=;(06|k7OwvQyp_&MAmuLAl6( > > HxkV-N > > >> zET8D<lf1qtOCkd^gYD|%d6DaG;EFrG- > > 4iy??Rq_;ii11ypk6r(+2fVKanpABoH=Nv > > >> z<&j#gnvCR4)Z7mbaOf4-W15N#lkkcLM+avfRa7Cb$<- > > )x6l5)Rn~X$TUK4VTk?W_@ > > >> > > za3fp1Vg<`y=axm`l~uNpw4drXy2AteUMrc`D;^ahSHUaU(DczTZEGzpC0A<w+ > > e*W9 > > >> zlsc+aJ@t71fm-|Njar-P-BkZZdaHj%BGuK~1N96f@mhUD^=+%KZ}J1v- > > I3oJiR_*v > > >> zkljd!dfL}Bv#)4Y?##KOyN`eYo|vzfe%Cx8D@~G- > > $nUDRue)P&^aC3;wBFKIlpSg5 > > >> > > zO72iNcz$iYj)AD$H{9si)o*yG4ScM|VQYOh&@Gyeoy3+#?Mll)vu@v^>B#0C!# > > XsJ > > >> > > zf|V_saqr%;En@`^3fYD?u%s*OeQVKssNMs~l^TU4ynqMmO0%@1ClG9^+Q > > QSG{;?A# > > >> z-{^- > > Q@H*)Fy0VpvJi0GSK?vv0{BmDzNRD0Djx=xRPRnho`V*v}?g|GSgK$w_S8s?s > > >> z@PZWCQnoXa8N7$+=- > > tHvY`itODpHvGO}x1#A6ZCqN25B52jU8RPYubJ$BO3*-oiRO > > >> z;7of=l<H^yiARTeCejMczOQRj{lPA12bOhRJ(cNa!`7EnL_ZOBs8- > > J`#SZv^9k<$X > > >> > > zcx*8~`ic3{Y>Z5Td2kGL9g117k*Dmy$>bxwBVzD<S9AER<a0zY(pBGjzJP<wb;~ > > 0j > > >> > > zat46)NHg>_BmbVy#A%ML&V78q8Fhnr$<bBU=9YU^87}oEdC(K*Aj0od2E5>- > > sRWMZ > > >> > > zNblhn9*8MgUOOV6qBc<l+)_Lm7Dv@^zN)=wg0E)Hg^|ck=D^1t#Rb(DByjF* > > hHkF` > > >> z;HuY_9BtH}dYtDK`s+R&t{O- > > 1w3>cRPwrb!Y%j@~g;kdG85b+{3yV!w6Iy3>qPfzZ > > >> zX{9ojT`eo5pLsm0-dh|D(|hxLUpeqz+>;BQS42nq$~dmIf}d&5CJLW-MHLu- > > u?x7? > > >> zdOVi2?;F<Cx1yOQLNu- > > H0}g9a>}aO8dtoo~QX}3;{+%qy`l;i@HPx)wdSBPauAx3( > > >> z>^nf%KE)1&a40S~$+4?pPs1L58H!T5z)Evq`BJvzidIjPmp|l5{Q8S>lszr`Qdr`A > > >> > > zePHZ;Os~?Pl0b59lG~ERx38?lX)E?;i$*TP4(h0Xk3mjUJT#X_%?n8_X0gin<(*~h > > >> > > zvDtgF@4S(hIi(+~mG|D+OX0buJ?&^+)>Dm0=76ngbhQ19reJ%S)<S+^oo4sxJJ > > nmY > > >> zGY6%`kVg`g?m$uXqa?6%tocvmOTIZQvTJ!v1RCa!I0- > > gk(Id$@uiD7@iqnDTaIo8+ > > >> zyV>(n7Ls@DKFB<FqO7V- > > xpu@iN36HTd@g76i6Zb~*Ok7Rt*dfT7boHmyUtp=;i;K( > > >> zo^g- > > W@FnOXzE@lLs*zExG5O1n$4E0i3Vj`1la=w~d*Rw6t)*%#8qJA``utI=&gXax > > >> z|FhF$H#bn-d!tC;obk9gsE#aZW7`pd*NC^C^!K$wv6t-H-o^KX5&g~k(7qbL- > > AH!% > > >> z2@qzF6)k>Vp2}uwYkVd1H)P>oTCvDQf3r9rtC- > > mFvyX}sR8P6g7q^*ax)F|8UHrt% > > >> QHyJ;b`f6NX>FQbZ9}*T`00000 > > >> > > >> literal 0 > > >> HcmV?d00001 > > >> > > >> diff --git a/Platforms/Marvell/Marvell.dec > > b/Platforms/Marvell/Marvell.dec > > >> index e63add4..f9d2660 100644 > > >> --- a/Platforms/Marvell/Marvell.dec > > >> +++ b/Platforms/Marvell/Marvell.dec > > >> @@ -54,6 +54,7 @@ > > >> > > >> gShellEepromHiiGuid = { 0xb2f4c714, 0x147f, 0x4ff7, { 0x82, 0x1b, 0xce, > > 0x7b, 0x91, 0x7f, 0x5f, 0x2f } } > > >> gShellSfHiiGuid = { 0x03a67756, 0x8cde, 0x4638, { 0x82, 0x34, 0x4a, 0x0f, > > 0x6d, 0x58, 0x81, 0x39 } } > > >> + gShellFUpdateHiiGuid = { 0x9b5d2176, 0x590a, 0x49db, { 0x89, 0x5d, > > 0x4a, 0x70, 0xfe, 0xad, 0xbe, 0x24 } } > > >> > > >> [PcdsFixedAtBuild.common] > > >> #MPP > > >> -- > > >> 1.8.3.1 > > >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell 2016-11-16 17:35 ` Leif Lindholm @ 2016-11-16 21:31 ` Carsey, Jaben 2016-11-16 21:48 ` Marcin Wojtas 0 siblings, 1 reply; 7+ messages in thread From: Carsey, Jaben @ 2016-11-16 21:31 UTC (permalink / raw) To: Leif Lindholm Cc: Marcin Wojtas, Ard Biesheuvel, linaro-uefi, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dabros, Bartosz Szczepanek, edk2-devel-01, Ni, Ruiyu, Carsey, Jaben Leif, I agree on EBL, but I have very little experience with EBL so I don’t want to discuss in detail as I am not the right person without more research. Specifically, my gut reaction is that needing a platform specific boot loader indicates that something has already gone wrong on that platform. However, this does not seem like a boot loader or an application at all. this is an internal shell command. The goal here seems to be to create a NULL library to add a new internal command to the UEFI Shell. This library gets compiled/linked into the shell itself. I feel that we have found a "new" use case that I encountered, but worked around in the past because all previous cases involved commands in the same library (there are interactions between Reconnect and Disconnect/Connect). I would say that a new API in the ShellCommandLib that links the UEFI Shell Application to the NULL libraries that make up the internal commands would be my first choice for implementation. I would lean to something like the function that Marcin already called. Maybe this? EFIAPI RunRegisteredCommand( CHAR16* CommandLine, EFI_STATUS *CommandReturnValue ) -Jaben > -----Original Message----- > From: Leif Lindholm [mailto:leif.lindholm@linaro.org] > Sent: Wednesday, November 16, 2016 9:36 AM > To: Carsey, Jaben <jaben.carsey@intel.com> > Cc: Marcin Wojtas <mw@semihalf.com>; Ard Biesheuvel > <ard.biesheuvel@linaro.org>; linaro-uefi <linaro-uefi@lists.linaro.org>; Neta > Zur Hershkovits <neta@marvell.com>; Yehuda Yitschak > <yehuday@marvell.com>; Haim Boot <hayim@marvell.com>; Jan Dabros > <jsd@semihalf.com>; Bartosz Szczepanek <bsz@semihalf.com>; edk2-devel- > 01 <edk2-devel@lists.01.org>; Ni, Ruiyu <ruiyu.ni@intel.com> > Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' > comand to shell > Importance: High > > Hi Jaben, > > I've seen a few different solutions to the same problem of providing > platform-specific "shorthand" commands, my least favourite one being > embedding a locally hacked EBL. > > I guess we're ending up with something halfway between an application > and a script. > > Is it your view that this should always be implemented as a standalone > application, to be called from the UI config menus? > > Basically, is there an expected usage pattern that people keep > missing? > > Regards, > > Leif > > On Wed, Nov 16, 2016 at 05:05:36PM +0000, Carsey, Jaben wrote: > > Marcin and Ard, > > > > It sounds like there is a goal to have a UEFI Application call into an internal > shell command. > > > > There is no documented method to programmatically call an internal > > command. This is not a goal of the current shell > > implementation. ShellExecute explicitly does not allow running of > > internal commands (per UEFI Shell Specification 2.2). > > > > Thus I don’t think using ShellDynamicCommandProtocol will help. > > That protocol is for allowing a driver add a command to the shell's > > list of available commands. > > > > -Jaben > > > > > -----Original Message----- > > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > > Sent: Wednesday, November 16, 2016 8:13 AM > > > To: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > > Cc: linaro-uefi <linaro-uefi@lists.linaro.org>; Leif Lindholm > > > <leif.lindholm@linaro.org>; Neta Zur Hershkovits <neta@marvell.com>; > > > Yehuda Yitschak <yehuday@marvell.com>; Haim Boot > > > <hayim@marvell.com>; Jan Dąbroś <jsd@semihalf.com>; Bartosz > > > Szczepanek <bsz@semihalf.com>; edk2-devel-01 <edk2- > devel@lists.01.org>; > > > Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com> > > > Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add > 'fupdate' > > > comand to shell > > > Importance: High > > > > > > + Jaben, Ruiyu and edk2 list > > > > > > Hi Ard, > > > > > > Internal Shell header ("../../../ShellPkg/Application/Shell/Shell.h") > > > was added ONLY for being able to execute shell commands (tftp and our > > > custom SPI flash control) from within a new command. This is now done > > > with: > > > RunShellCommand (TftpCmd, &Status); > > > And it works fine. > > > > > > Replacing above with: > > > ShellExecute (&ImageHandle, TftpCmd, FALSE, NULL, &Status); > > > allow to get rid of relative include and seems more appropriate, but > > > there is completely no effect of calling it. > > > > > > I checked in edk2 sources and see no relation between above problem > > > and registering our command with help of ShellDynamicCommand > protocol. > > > Unless I don't know something. Anyway, I'll apreciate any hint, how to > > > solve the issue in a nice way. > > > > > > Best regards, > > > Marcin > > > > > > 2016-11-15 17:09 GMT+01:00 Ard Biesheuvel > <ard.biesheuvel@linaro.org>: > > > > On 10 July 2016 at 00:21, Marcin Wojtas <mw@semihalf.com> wrote: > > > >> From: Jan Dąbroś <jsd@semihalf.com> > > > >> > > > >> 'fupdate' command performs updating firmware from file placed on > local > > > >> filesystem or on TFTP server. > > > >> It uses tftp and sf commands. > > > >> > > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > > >> Signed-off-by: Jan Dabros <jsd@semihalf.com> > > > >> Signed-off-by: Marcin Wojtas <mw@semihalf.com> > > > > > > > > Please use the Shell dynamic command protocol to add commands to > the > > > > shell. This will remove the need for including internal Shell headers, > > > > and even allow your commands to work with prebuilt Shell binaries. > > > > > > > > MdePkg/Include/Protocol/ShellDynamicCommand.h > > > > > > > > Thanks, > > > > Ard. > > > > > > > > > > > >> --- > > > >> Applications/FirmwareUpdate/FUpdate.c | 448 > > > ++++++++++++++++++++++++++++++++ > > > >> Applications/FirmwareUpdate/FUpdate.inf | 68 +++++ > > > >> Applications/FirmwareUpdate/FUpdate.uni | Bin 0 -> 5838 bytes > > > >> Platforms/Marvell/Marvell.dec | 1 + > > > >> 4 files changed, 517 insertions(+) > > > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.c > > > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.inf > > > >> create mode 100644 Applications/FirmwareUpdate/FUpdate.uni > > > >> > > > >> diff --git a/Applications/FirmwareUpdate/FUpdate.c > > > b/Applications/FirmwareUpdate/FUpdate.c > > > >> new file mode 100644 > > > >> index 0000000..359a4ac > > > >> --- /dev/null > > > >> +++ b/Applications/FirmwareUpdate/FUpdate.c > > > >> @@ -0,0 +1,448 @@ > > > >> > > > > +/********************************************************* > > > ********************** > > > >> +Copyright (C) 2016 Marvell International Ltd. > > > >> + > > > >> +Marvell BSD License Option > > > >> + > > > >> +If you received this File from Marvell, you may opt to use, > redistribute > > > and/or > > > >> +modify this File under the following licensing terms. > > > >> +Redistribution and use in source and binary forms, with or without > > > modification, > > > >> +are permitted provided that the following conditions are met: > > > >> + > > > >> +* Redistributions of source code must retain the above copyright > notice, > > > >> + this list of conditions and the following disclaimer. > > > >> + > > > >> +* Redistributions in binary form must reproduce the above copyright > > > >> + notice, this list of conditions and the following disclaimer in the > > > >> + documentation and/or other materials provided with the > distribution. > > > >> + > > > >> +* Neither the name of Marvell nor the names of its contributors may > be > > > >> + used to endorse or promote products derived from this software > > > without > > > >> + specific prior written permission. > > > >> + > > > >> +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > > > CONTRIBUTORS "AS IS" AND > > > >> +ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > LIMITED > > > TO, THE IMPLIED > > > >> +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > PARTICULAR > > > PURPOSE ARE > > > >> +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > > > CONTRIBUTORS BE LIABLE FOR > > > >> +ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > > > CONSEQUENTIAL DAMAGES > > > >> +(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE > > > GOODS OR SERVICES; > > > >> +LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) > > > HOWEVER CAUSED AND ON > > > >> +ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, > OR > > > TORT > > > >> +(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY > OUT OF > > > THE USE OF THIS > > > >> +SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. > > > >> + > > > >> > > > > +********************************************************* > > > **********************/ > > > >> +#include <Uefi.h> > > > >> +#include <ShellBase.h> > > > >> + > > > >> +#include <Library/BaseLib.h> > > > >> +#include <Library/DebugLib.h> > > > >> +#include <Library/MemoryAllocationLib.h> > > > >> +#include <Library/ShellCommandLib.h> > > > >> +#include <Library/ShellLib.h> > > > >> +#include <Library/UefiLib.h> > > > >> +#include <Library/PrintLib.h> > > > >> +#include <Library/UefiLib.h> > > > >> +#include <Library/ShellCEntryLib.h> > > > >> +#include <Library/HiiLib.h> > > > >> +#include <Guid/ShellLibHiiGuid.h> > > > >> +#include <Library/UefiBootServicesTableLib.h> > > > >> +#include <Library/FileHandleLib.h> > > > >> +// FIXME: Remove including relative path > > > >> +#include "../../../ShellPkg/Application/Shell/Shell.h" > > > >> + > > > >> +#define CMD_NAME_STRING L"fupdate" > > > >> + > > > >> +#define TFTP_CMD_STRING L"tftp " > > > >> +#define SPACE_STRING L" " > > > >> +#define SF_PROBE_CMD_STRING L"sf probe" > > > >> +#define SF_WRITE_CMD_STRING L"sf updatefile " > > > >> +#define SF_LOAD_ADDR_STRING L"0x0" > > > >> + > > > >> +#define MAIN_HDR_MAGIC 0xB105B002 > > > >> + > > > >> +typedef struct { > > > >> + UINT32 Magic; // 0-3 > > > >> + UINT32 PrologSize; // 4-7 > > > >> + UINT32 PrologChecksum; // 8-11 > > > >> + UINT32 BootImageSize; // 12-15 > > > >> + UINT32 BootImageChecksum; // 16-19 > > > >> + UINT32 Reserved0; // 20-23 > > > >> + UINT32 LoadAddr; // 24-27 > > > >> + UINT32 ExecAddr; // 28-31 > > > >> + UINT8 UartConfig; // 32 > > > >> + UINT8 Baudrate; // 33 > > > >> + UINT8 ExtCount; // 34 > > > >> + UINT8 AuxFlags; // 35 > > > >> + UINT32 IoArg0; // 36-39 > > > >> + UINT32 IoArg1; // 40-43 > > > >> + UINT32 IoArg2; // 43-47 > > > >> + UINT32 IoArg3; // 48-51 > > > >> + UINT32 Reserved1; // 52-55 > > > >> + UINT32 Reserved2; // 56-59 > > > >> + UINT32 Reserved3; // 60-63 > > > >> +} MV_IMAGE_HEADER; > > > >> + > > > >> +STATIC > > > >> +UINT32 > > > >> +CountChecksum ( > > > >> + UINT32 *Start, > > > >> + UINT32 Length > > > >> + ) > > > >> +{ > > > >> + UINT32 Sum = 0; > > > >> + UINT32 *Startp = Start; > > > >> + > > > >> + do { > > > >> + Sum += *Startp; > > > >> + Startp++; > > > >> + Length -= 4; > > > >> + } while (Length > 0); > > > >> + > > > >> + return Sum; > > > >> +} > > > >> + > > > >> +STATIC > > > >> +EFI_STATUS > > > >> +CheckImageHeader ( > > > >> + IN VOID *ImageHeader > > > >> + ) > > > >> +{ > > > >> + MV_IMAGE_HEADER *Header; > > > >> + UINT32 HeaderLength, Checksum, ChecksumBackup; > > > >> + > > > >> + Header = (MV_IMAGE_HEADER *) ImageHeader; > > > >> + HeaderLength = Header->PrologSize; > > > >> + ChecksumBackup = Header->PrologChecksum; > > > >> + > > > >> + // Compare magic number > > > >> + if (Header->Magic != MAIN_HDR_MAGIC) { > > > >> + Print (L"%s: Bad Image magic 0x%08x != 0x%08x\n", > > > CMD_NAME_STRING, > > > >> + Header->Magic, MAIN_HDR_MAGIC); > > > >> + return EFI_DEVICE_ERROR; > > > >> + } > > > >> + > > > >> + // The checksum field is discarded from calculation > > > >> + Header->PrologChecksum = 0; > > > >> + > > > >> + Checksum = CountChecksum((UINT32 *)Header, HeaderLength); > > > >> + if (Checksum != ChecksumBackup) { > > > >> + Print (L"%s: Bad Image checksum. 0x%x != 0x%x\n", > > > CMD_NAME_STRING, Checksum, > > > >> + ChecksumBackup); > > > >> + return EFI_DEVICE_ERROR; > > > >> + } > > > >> + > > > >> + // Restore checksum backup > > > >> + Header->PrologChecksum = ChecksumBackup; > > > >> + > > > >> + return 0; > > > >> +} > > > >> + > > > >> +STATIC > > > >> +EFI_STATUS > > > >> +CheckFirmwareImage ( > > > >> + CONST CHAR16* FirmwareImage > > > >> + ) > > > >> +{ > > > >> + EFI_STATUS Status; > > > >> + VOID *FileBuffer; > > > >> + UINT64 OpenMode; > > > >> + UINTN FileSize; > > > >> + SHELL_FILE_HANDLE FileHandle = NULL; > > > >> + > > > >> + OpenMode = EFI_FILE_MODE_READ; > > > >> + > > > >> + Status = ShellOpenFileByName (FirmwareImage, &FileHandle, > > > OpenMode, 0); > > > >> + if (EFI_ERROR (Status)) { > > > >> + Print (L"%s: Cannot open Image file\n", CMD_NAME_STRING); > > > >> + return EFI_DEVICE_ERROR; > > > >> + } > > > >> + > > > >> + Status = FileHandleGetSize (FileHandle, &FileSize); > > > >> + if (EFI_ERROR (Status)) { > > > >> + Print (L"%s: Cannot get Image file size\n", CMD_NAME_STRING); > > > >> + } > > > >> + > > > >> + FileBuffer = AllocateZeroPool (FileSize); > > > >> + > > > >> + // Read Image header into buffer > > > >> + Status = FileHandleRead (FileHandle, &FileSize, FileBuffer); > > > >> + if (EFI_ERROR (Status)) { > > > >> + Print (L"%s: Cannot read Image file header\n", > CMD_NAME_STRING); > > > >> + ShellCloseFile (&FileHandle); > > > >> + FreePool (FileBuffer); > > > >> + return EFI_DEVICE_ERROR; > > > >> + } > > > >> + > > > >> + Status = CheckImageHeader (FileBuffer); > > > >> + if (EFI_ERROR(Status)) { > > > >> + return EFI_DEVICE_ERROR; > > > >> + } > > > >> + > > > >> + FreePool (FileBuffer); > > > >> + > > > >> + return EFI_SUCCESS; > > > >> +} > > > >> + > > > >> +STATIC > > > >> +CONST CHAR16 * > > > >> +FileNameFromFilePath ( > > > >> + CONST CHAR16 *RemoteFilePath > > > >> + ) > > > >> +{ > > > >> + CONST CHAR16 *Walker; > > > >> + > > > >> + // Gather FileName from FilePath > > > >> + Walker = RemoteFilePath + StrLen (RemoteFilePath); > > > >> + while ((--Walker) >= RemoteFilePath) { > > > >> + if ((*Walker == L'\\') || (*Walker == L'/')) { > > > >> + break; > > > >> + } > > > >> + } > > > >> + > > > >> + return (Walker + 1); > > > >> +} > > > >> + > > > >> +STATIC > > > >> +CONST CHAR16* > > > >> +PrepareFile ( > > > >> + LIST_ENTRY *CheckPackage > > > >> + ) > > > >> +{ > > > >> + EFI_STATUS Status; > > > >> + CONST CHAR16 *ValueStr; > > > >> + > > > >> + ValueStr = ShellCommandLineGetValue (CheckPackage, L"-f"); > > > >> + if (ValueStr == NULL) { > > > >> + Print (L"%s: No LocalFilePath parameter!\n", > CMD_NAME_STRING); > > > >> + return NULL; > > > >> + } else { > > > >> + Status = ShellIsFile (ValueStr); > > > >> + if (EFI_ERROR(Status)) { > > > >> + Print (L"%s: Wrong LocalFilePath parameter!\n", > > > CMD_NAME_STRING); > > > >> + return NULL; > > > >> + } > > > >> + } > > > >> + return ValueStr; > > > >> +} > > > >> + > > > >> +CONST CHAR16 gShellFUpdateFileName[] = L"ShellCommand"; > > > >> +EFI_HANDLE gShellFUpdateHiiHandle = NULL; > > > >> +EFI_HANDLE gShellFUpdateHiiHandle; > > > >> + > > > >> +STATIC CONST SHELL_PARAM_ITEM ParamList[] = { > > > >> + {L"help", TypeFlag}, > > > >> + {L"-t", TypeFlag}, > > > >> + {L"-f", TypeValue}, > > > >> + {NULL , TypeMax} > > > >> + }; > > > >> + > > > >> +/** > > > >> + Return the file name of the help text file if not using HII. > > > >> + > > > >> + @return The string pointer to the file name. > > > >> +**/ > > > >> +CONST CHAR16* > > > >> +EFIAPI > > > >> +ShellCommandGetManFileNameFUpdate ( > > > >> + VOID > > > >> + ) > > > >> +{ > > > >> + > > > >> + return gShellFUpdateFileName; > > > >> +} > > > >> + > > > >> +VOID > > > >> +FUpdateUsage ( > > > >> + VOID > > > >> + ) > > > >> +{ > > > >> + Print (L"\nFirmware update command\n" > > > >> + "fupdate [-f <LocalFilePath>] [-t <Host> > <RemoteFilePath>]\n\n" > > > >> + "LocalFilePath - path to local firmware image file\n" > > > >> + "Host - IP number of TFTP server\n" > > > >> + "RemoteFilePath - path to firmware image file on TFTP server\n" > > > >> + "Examples:\n" > > > >> + "Update firmware from file fs2:Uefi.img\n" > > > >> + " fupdate -f fs2:Uefi.img\n" > > > >> + "Update firmware from file path/Uefi.img TFTP server with IP " > > > >> + "address 10.0.0.200\n" > > > >> + " fupdate -t 10.0.0.200 path/Uefi.img\n" > > > >> + ); > > > >> +} > > > >> + > > > >> +SHELL_STATUS > > > >> +EFIAPI > > > >> +ShellCommandRunFUpdate ( > > > >> + IN EFI_HANDLE ImageHandle, > > > >> + IN EFI_SYSTEM_TABLE *SystemTable > > > >> + ) > > > >> +{ > > > >> + EFI_STATUS Status; > > > >> + LIST_ENTRY *CheckPackage; > > > >> + CHAR16 *ProblemParam, *TftpCmd = NULL, *SfCmd = NULL; > > > >> + CONST CHAR16 *RemoteFilePath, *Host, *FileToWrite; > > > >> + UINT8 CmdLen; > > > >> + BOOLEAN FileFlag, TftpFlag; > > > >> + > > > >> + Status = ShellInitialize (); > > > >> + if (EFI_ERROR (Status)) { > > > >> + Print (L"%s: Error while initializinf Shell\n", CMD_NAME_STRING); > > > >> + ASSERT_EFI_ERROR (Status); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + Status = ShellCommandLineParse (ParamList, &CheckPackage, > > > &ProblemParam, > > > >> + TRUE); > > > >> + if (EFI_ERROR (Status)) { > > > >> + Print (L"Parse error!\n"); > > > >> + } > > > >> + > > > >> + if (ShellCommandLineGetFlag (CheckPackage, L"help")) { > > > >> + FUpdateUsage(); > > > >> + return EFI_SUCCESS; > > > >> + } > > > >> + > > > >> + FileFlag = ShellCommandLineGetFlag (CheckPackage, L"-f"); > > > >> + TftpFlag = ShellCommandLineGetFlag (CheckPackage, L"-t"); > > > >> + > > > >> + if (!FileFlag && !TftpFlag) { > > > >> + Print (L"%s: Please specify -f or -t flag\n", CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } else if (FileFlag && !TftpFlag) { > > > >> + // Prepare local file to be burned into flash > > > >> + FileToWrite = PrepareFile (CheckPackage); > > > >> + if (FileToWrite == NULL) { > > > >> + Print (L"%s: Error while preparing file for burn\n", > > > CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + } else if (TftpFlag && !FileFlag) { > > > >> + Host = ShellCommandLineGetRawValue (CheckPackage, 1); > > > >> + if (Host == NULL) { > > > >> + Print (L"%s: No Host parameter!\n", CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + RemoteFilePath = ShellCommandLineGetRawValue > (CheckPackage, 2); > > > >> + if (RemoteFilePath == NULL) { > > > >> + Print (L"%s: No remote_file_path parameter!\n", > > > CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + // Gather firmware image name from remote filepath > > > >> + FileToWrite = FileNameFromFilePath (RemoteFilePath); > > > >> + > > > >> + // Allocate buffer for tftp command string > > > >> + CmdLen = StrSize (TFTP_CMD_STRING) + StrSize (SPACE_STRING) > + > > > >> + StrSize (Host) + StrSize (RemoteFilePath); > > > >> + TftpCmd = (CHAR16 *) AllocateZeroPool (CmdLen + > sizeof(CHAR16)); > > > >> + if (TftpCmd == NULL) { > > > >> + Print (L"%s: Cannot allocate memory\n", CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + // Concatenate parameters and form tftp command string > > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), TFTP_CMD_STRING); > > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)Host); > > > >> + // Insert space > > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); > > > >> + StrCatS (TftpCmd, CmdLen / sizeof(CHAR16), (CHAR16 > > > *)RemoteFilePath); > > > >> + > > > >> + RunShellCommand (TftpCmd, &Status); > > > >> + FreePool (TftpCmd); > > > >> + if (EFI_ERROR(Status)) { > > > >> + Print (L"%s: Error while performing tftp command\n", > > > CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + } else { > > > >> + Print (L"%s: Both -f and -t flag specified, please choose one\n", > > > >> + CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + // Check image checksum and magic > > > >> + Status = CheckFirmwareImage (FileToWrite); > > > >> + if (EFI_ERROR(Status)) { > > > >> + Print (L"%s: Wrong firmware Image\n", CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + // Probe spi bus > > > >> + RunShellCommand (SF_PROBE_CMD_STRING, &Status); > > > >> + if (EFI_ERROR(Status)) { > > > >> + Print (L"%s: Error while performing sf probe\n", > > > CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + // Allocate buffer for sf command string > > > >> + CmdLen = StrSize (SF_WRITE_CMD_STRING) + StrSize (FileToWrite) > + > > > >> + StrSize (SPACE_STRING) + StrSize (SF_LOAD_ADDR_STRING); > > > >> + SfCmd = (CHAR16 *) AllocateZeroPool (CmdLen + sizeof(CHAR16)); > > > >> + if (SfCmd == NULL) { > > > >> + Print (L"%s: Cannot allocate memory\n"); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + // Concatenate parameters and form command string > > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), > SF_WRITE_CMD_STRING); > > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), (CHAR16 *)FileToWrite); > > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), SPACE_STRING); > > > >> + StrCatS (SfCmd, CmdLen / sizeof(CHAR16), > SF_LOAD_ADDR_STRING); > > > >> + > > > >> + // Update firmware image in flash > > > >> + RunShellCommand (SfCmd, &Status); > > > >> + FreePool (SfCmd); > > > >> + if (EFI_ERROR(Status)) { > > > >> + Print (L"%s: Error while performing sf update\n", > > > CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + return EFI_SUCCESS; > > > >> +} > > > >> + > > > >> +EFI_STATUS > > > >> +EFIAPI > > > >> +ShellFUpdateCommandConstructor ( > > > >> + IN EFI_HANDLE ImageHandle, > > > >> + IN EFI_SYSTEM_TABLE *SystemTable > > > >> + ) > > > >> +{ > > > >> + EFI_STATUS Status; > > > >> + > > > >> + gShellFUpdateHiiHandle = NULL; > > > >> + > > > >> + gShellFUpdateHiiHandle = HiiAddPackages ( > > > >> + &gShellFUpdateHiiGuid, gImageHandle, > > > >> + UefiShellFUpdateCommandLibStrings, NULL > > > >> + ); > > > >> + if (gShellFUpdateHiiHandle == NULL) { > > > >> + Print (L"%s: Cannot add Hii package\n", CMD_NAME_STRING); > > > >> + return EFI_DEVICE_ERROR; > > > >> + } > > > >> + > > > >> + Status = ShellCommandRegisterCommandName ( > > > >> + CMD_NAME_STRING, ShellCommandRunFUpdate, > > > ShellCommandGetManFileNameFUpdate, > > > >> + 0, CMD_NAME_STRING, TRUE , gShellFUpdateHiiHandle, > > > >> + STRING_TOKEN (STR_GET_HELP_FUPDATE) > > > >> + ); > > > >> + if (EFI_ERROR(Status)) { > > > >> + Print (L"%s: Error while registering command\n", > > > CMD_NAME_STRING); > > > >> + return SHELL_ABORTED; > > > >> + } > > > >> + > > > >> + return EFI_SUCCESS; > > > >> +} > > > >> + > > > >> +EFI_STATUS > > > >> +EFIAPI > > > >> +ShellFUpdateCommandDestructor ( > > > >> + IN EFI_HANDLE ImageHandle, > > > >> + IN EFI_SYSTEM_TABLE *SystemTable > > > >> + ) > > > >> +{ > > > >> + > > > >> + if (gShellFUpdateHiiHandle != NULL) { > > > >> + HiiRemovePackages (gShellFUpdateHiiHandle); > > > >> + } > > > >> + return EFI_SUCCESS; > > > >> +} > > > >> diff --git a/Applications/FirmwareUpdate/FUpdate.inf > > > b/Applications/FirmwareUpdate/FUpdate.inf > > > >> new file mode 100644 > > > >> index 0000000..53b5305 > > > >> --- /dev/null > > > >> +++ b/Applications/FirmwareUpdate/FUpdate.inf > > > >> @@ -0,0 +1,68 @@ > > > >> +# > > > >> +# Marvell BSD License Option > > > >> +# > > > >> +# If you received this File from Marvell, you may opt to use, > redistribute > > > >> +# and/or modify this File under the following licensing terms. > > > >> +# Redistribution and use in source and binary forms, with or without > > > >> +# modification, are permitted provided that the following conditions > are > > > met: > > > >> +# > > > >> +# * Redistributions of source code must retain the above copyright > > > notice, > > > >> +# this list of conditions and the following disclaimer. > > > >> +# > > > >> +# * Redistributions in binary form must reproduce the above > copyright > > > >> +# notice, this list of conditions and the following disclaimer in the > > > >> +# documentation and/or other materials provided with the > distribution. > > > >> +# > > > >> +# * Neither the name of Marvell nor the names of its contributors > may > > > be > > > >> +# used to endorse or promote products derived from this software > > > without > > > >> +# specific prior written permission. > > > >> +# > > > >> +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND > > > CONTRIBUTORS "AS IS" > > > >> +# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT > NOT > > > LIMITED TO, THE > > > >> +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A > > > PARTICULAR PURPOSE ARE > > > >> +# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR > > > CONTRIBUTORS BE LIABLE > > > >> +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR > > > CONSEQUENTIAL > > > >> +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF > > > SUBSTITUTE GOODS OR > > > >> +# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS > > > INTERRUPTION) HOWEVER > > > >> +# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN > CONTRACT, > > > STRICT LIABILITY, > > > >> +# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN > ANY > > > WAY OUT OF THE USE > > > >> +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH > > > DAMAGE. > > > >> +# > > > >> + > > > >> +[Defines] > > > >> + INF_VERSION = 0x00010006 > > > >> + BASE_NAME = UefiShellFUpdateCommandLib > > > >> + FILE_GUID = 470292b2-926b-4ed8-8080-be7a260db627 > > > >> + MODULE_TYPE = UEFI_APPLICATION > > > >> + VERSION_STRING = 0.1 > > > >> + LIBRARY_CLASS = NULL|UEFI_APPLICATION UEFI_DRIVER > > > >> + CONSTRUCTOR = ShellFUpdateCommandConstructor > > > >> + DESTRUCTOR = ShellFUpdateCommandDestructor > > > >> + > > > >> +[Sources] > > > >> + FUpdate.c > > > >> + FUpdate.uni > > > >> + > > > >> +[Packages] > > > >> + MdePkg/MdePkg.dec > > > >> + ShellPkg/ShellPkg.dec > > > >> + MdeModulePkg/MdeModulePkg.dec > > > >> + OpenPlatformPkg/Platforms/Marvell/Marvell.dec > > > >> + > > > >> +[LibraryClasses] > > > >> + UefiLib > > > >> + UefiBootServicesTableLib > > > >> + MemoryAllocationLib > > > >> + BaseLib > > > >> + BaseMemoryLib > > > >> + DebugLib > > > >> + ShellCommandLib > > > >> + ShellLib > > > >> + UefiLib > > > >> + UefiRuntimeServicesTableLib > > > >> + PcdLib > > > >> + HiiLib > > > >> + FileHandleLib > > > >> + > > > >> +[Guids] > > > >> + gShellFUpdateHiiGuid > > > >> diff --git a/Applications/FirmwareUpdate/FUpdate.uni > > > b/Applications/FirmwareUpdate/FUpdate.uni > > > >> new file mode 100644 > > > >> index > > > > 0000000000000000000000000000000000000000..6143c0580f1d1ef8ce4fe619a1 > > > df1ce380adf2c0 > > > >> GIT binary patch > > > >> literal 5838 > > > >> > > > > zcmd6r?N1v=5XSd&rTz~m@+FOuK&z@BXdB5G6OhbHeI_(&s)*P?EX6>LDdL} > > > R`}}4Y > > > >> z_Fim)B2|h`_TBEy&OY<Z%kJL4|E#8GU- > > > k8E`X+VLMY>G4X_hAGEPYJ#RHUcs{Z98E > > > >> > > > > z(pvgH{iLVebS#O#(@FZIt4KTPT#_g1JJ*}J#$MPH@A_(w)60Z*e$BBPsZ5(DH% > > > TMu > > > >> zI8RslFVenngr*p~lTP*KR@$$1pGl4p6GJjq&s>Nn8egO>- > > > 9MS0Q^{SVsru?OKUYs^ > > > >> znhN!;+Src8b3GNB10={X)7Ui6^*l)*^bUIqjXT$yRmsoHx~ZO!JT- > > > lb@LbynxN^K` > > > >> zIn^kbyht}1doJb+wK5;k=NwOk%lRBE#O3S<wy>ix&4tFo- > > > prWGG9T<MUWEtn%*5H1 > > > >> > > > > zSq~;Gfgeyu8$Ga<Z3c1n|8rvmo!EG(e{`Y?JMh<~dM@O>iCW&nJZN~Bn~_Gr > > > JG$Ij > > > >> > > > > zd>%7RrE8wn?<20zyXy5!t%sOH*M&t|ohxPqCl(VMpv1PhX2wGq^RXfXyO3p > > > 6Go#Mc > > > >> > z2Xdm<S*qfT=Nvu6g3sr;me~&{8O5Tg@+clDy1)!mvr2Bp%yEAVQ*Qa<_- > > > Qib{vL}x > > > >> > > > > zEYC<E=8NnPMAdtpMc3xp|Nrt8nS_5%EXt~=;(06|k7OwvQyp_&MAmuLAl6( > > > HxkV-N > > > >> zET8D<lf1qtOCkd^gYD|%d6DaG;EFrG- > > > 4iy??Rq_;ii11ypk6r(+2fVKanpABoH=Nv > > > >> z<&j#gnvCR4)Z7mbaOf4-W15N#lkkcLM+avfRa7Cb$<- > > > )x6l5)Rn~X$TUK4VTk?W_@ > > > >> > > > > za3fp1Vg<`y=axm`l~uNpw4drXy2AteUMrc`D;^ahSHUaU(DczTZEGzpC0A<w+ > > > e*W9 > > > >> zlsc+aJ@t71fm-|Njar-P-BkZZdaHj%BGuK~1N96f@mhUD^=+%KZ}J1v- > > > I3oJiR_*v > > > >> zkljd!dfL}Bv#)4Y?##KOyN`eYo|vzfe%Cx8D@~G- > > > $nUDRue)P&^aC3;wBFKIlpSg5 > > > >> > > > > zO72iNcz$iYj)AD$H{9si)o*yG4ScM|VQYOh&@Gyeoy3+#?Mll)vu@v^>B#0C!# > > > XsJ > > > >> > > > > zf|V_saqr%;En@`^3fYD?u%s*OeQVKssNMs~l^TU4ynqMmO0%@1ClG9^+Q > > > QSG{;?A# > > > >> z-{^- > > > > Q@H*)Fy0VpvJi0GSK?vv0{BmDzNRD0Djx=xRPRnho`V*v}?g|GSgK$w_S8s?s > > > >> z@PZWCQnoXa8N7$+=- > > > tHvY`itODpHvGO}x1#A6ZCqN25B52jU8RPYubJ$BO3*-oiRO > > > >> z;7of=l<H^yiARTeCejMczOQRj{lPA12bOhRJ(cNa!`7EnL_ZOBs8- > > > J`#SZv^9k<$X > > > >> > > > > zcx*8~`ic3{Y>Z5Td2kGL9g117k*Dmy$>bxwBVzD<S9AER<a0zY(pBGjzJP<wb;~ > > > 0j > > > >> > > > > zat46)NHg>_BmbVy#A%ML&V78q8Fhnr$<bBU=9YU^87}oEdC(K*Aj0od2E5>- > > > sRWMZ > > > >> > > > > zNblhn9*8MgUOOV6qBc<l+)_Lm7Dv@^zN)=wg0E)Hg^|ck=D^1t#Rb(DByjF* > > > hHkF` > > > >> z;HuY_9BtH}dYtDK`s+R&t{O- > > > 1w3>cRPwrb!Y%j@~g;kdG85b+{3yV!w6Iy3>qPfzZ > > > >> zX{9ojT`eo5pLsm0-dh|D(|hxLUpeqz+>;BQS42nq$~dmIf}d&5CJLW- > MHLu- > > > u?x7? > > > >> zdOVi2?;F<Cx1yOQLNu- > > > H0}g9a>}aO8dtoo~QX}3;{+%qy`l;i@HPx)wdSBPauAx3( > > > >> > z>^nf%KE)1&a40S~$+4?pPs1L58H!T5z)Evq`BJvzidIjPmp|l5{Q8S>lszr`Qdr`A > > > >> > > > > zePHZ;Os~?Pl0b59lG~ERx38?lX)E?;i$*TP4(h0Xk3mjUJT#X_%?n8_X0gin<(*~h > > > >> > > > > zvDtgF@4S(hIi(+~mG|D+OX0buJ?&^+)>Dm0=76ngbhQ19reJ%S)<S+^oo4sxJJ > > > nmY > > > >> zGY6%`kVg`g?m$uXqa?6%tocvmOTIZQvTJ!v1RCa!I0- > > > gk(Id$@uiD7@iqnDTaIo8+ > > > >> zyV>(n7Ls@DKFB<FqO7V- > > > xpu@iN36HTd@g76i6Zb~*Ok7Rt*dfT7boHmyUtp=;i;K( > > > >> zo^g- > > > > W@FnOXzE@lLs*zExG5O1n$4E0i3Vj`1la=w~d*Rw6t)*%#8qJA``utI=&gXax > > > >> z|FhF$H#bn-d!tC;obk9gsE#aZW7`pd*NC^C^!K$wv6t-H- > o^KX5&g~k(7qbL- > > > AH!% > > > >> z2@qzF6)k>Vp2}uwYkVd1H)P>oTCvDQf3r9rtC- > > > mFvyX}sR8P6g7q^*ax)F|8UHrt% > > > >> QHyJ;b`f6NX>FQbZ9}*T`00000 > > > >> > > > >> literal 0 > > > >> HcmV?d00001 > > > >> > > > >> diff --git a/Platforms/Marvell/Marvell.dec > > > b/Platforms/Marvell/Marvell.dec > > > >> index e63add4..f9d2660 100644 > > > >> --- a/Platforms/Marvell/Marvell.dec > > > >> +++ b/Platforms/Marvell/Marvell.dec > > > >> @@ -54,6 +54,7 @@ > > > >> > > > >> gShellEepromHiiGuid = { 0xb2f4c714, 0x147f, 0x4ff7, { 0x82, 0x1b, > 0xce, > > > 0x7b, 0x91, 0x7f, 0x5f, 0x2f } } > > > >> gShellSfHiiGuid = { 0x03a67756, 0x8cde, 0x4638, { 0x82, 0x34, 0x4a, > 0x0f, > > > 0x6d, 0x58, 0x81, 0x39 } } > > > >> + gShellFUpdateHiiGuid = { 0x9b5d2176, 0x590a, 0x49db, { 0x89, 0x5d, > > > 0x4a, 0x70, 0xfe, 0xad, 0xbe, 0x24 } } > > > >> > > > >> [PcdsFixedAtBuild.common] > > > >> #MPP > > > >> -- > > > >> 1.8.3.1 > > > >> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell 2016-11-16 21:31 ` Carsey, Jaben @ 2016-11-16 21:48 ` Marcin Wojtas 2016-11-16 23:58 ` Marcin Wojtas 0 siblings, 1 reply; 7+ messages in thread From: Marcin Wojtas @ 2016-11-16 21:48 UTC (permalink / raw) To: Carsey, Jaben Cc: Leif Lindholm, Ard Biesheuvel, linaro-uefi, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dabros, Bartosz Szczepanek, edk2-devel-01, Ni, Ruiyu Hi Jaben, Thank you for your input. > > I agree on EBL, but I have very little experience with EBL so I don’t want to discuss in detail as I am not the right person without more research. Specifically, my gut reaction is that needing a platform specific boot loader indicates that something has already gone wrong on that platform. > > However, this does not seem like a boot loader or an application at all. this is an internal shell command. The goal here seems to be to create a NULL library to add a new internal command to the UEFI Shell. This library gets compiled/linked into the shell itself. Indeed, it's nothing similar to the bootloader whatsoever. This command simply enables updating firmware in SPI flash directly from local path or from tftp. > > I feel that we have found a "new" use case that I encountered, but worked around in the past because all previous cases involved commands in the same library (there are interactions between Reconnect and Disconnect/Connect). Right, however Reconnect is easy, as it simply calls gBS callbacks, whose definition are also in the same location. > > I would say that a new API in the ShellCommandLib that links the UEFI Shell Application to the NULL libraries that make up the internal commands would be my first choice for implementation. I would lean to something like the function that Marcin already called. Maybe this? > > EFIAPI > RunRegisteredCommand( > CHAR16* CommandLine, > EFI_STATUS *CommandReturnValue > ) I'll try this one and let know. Thanks, Marcin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell 2016-11-16 21:48 ` Marcin Wojtas @ 2016-11-16 23:58 ` Marcin Wojtas 2016-11-17 0:57 ` Carsey, Jaben 0 siblings, 1 reply; 7+ messages in thread From: Marcin Wojtas @ 2016-11-16 23:58 UTC (permalink / raw) To: Carsey, Jaben Cc: Leif Lindholm, Ard Biesheuvel, linaro-uefi, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dabros, Bartosz Szczepanek, edk2-devel-01, Ni, Ruiyu Hi Jaben, I haven't found RunRegisteredCommand in newest edk2. Can you please point exactly what you meant? As an alternative I checked ShellCommandRunCommandHandler - this is in fact a library helper function and it's nested deep down in RunShellCommand, which is for now the only working option. ShellCommandRunCommandHandler requires a lot of additional processing of the command line. All is done in Application/Shell code. Is there any chance to expose RunShellCommand (or equivalent), so that it can be used in a nice way, not with including multi "../../" relative path header? I expected, that the commands, which are in fact some wrappers or they mix multiple others is pretty much of a standard, I'm pretty surprised there are so huge difficulties in EDK2. I'm wondering of if there are any other options to be used here (unless we accept, what we have for now:) ). Best regards, Marcin 2016-11-16 22:48 GMT+01:00 Marcin Wojtas <mw@semihalf.com>: > Hi Jaben, > > Thank you for your input. > >> >> I agree on EBL, but I have very little experience with EBL so I don’t want to discuss in detail as I am not the right person without more research. Specifically, my gut reaction is that needing a platform specific boot loader indicates that something has already gone wrong on that platform. >> >> However, this does not seem like a boot loader or an application at all. this is an internal shell command. The goal here seems to be to create a NULL library to add a new internal command to the UEFI Shell. This library gets compiled/linked into the shell itself. > > Indeed, it's nothing similar to the bootloader whatsoever. This > command simply enables updating firmware in SPI flash directly from > local path or from tftp. > >> >> I feel that we have found a "new" use case that I encountered, but worked around in the past because all previous cases involved commands in the same library (there are interactions between Reconnect and Disconnect/Connect). > > Right, however Reconnect is easy, as it simply calls gBS callbacks, > whose definition are also in the same location. > >> >> I would say that a new API in the ShellCommandLib that links the UEFI Shell Application to the NULL libraries that make up the internal commands would be my first choice for implementation. I would lean to something like the function that Marcin already called. Maybe this? >> >> EFIAPI >> RunRegisteredCommand( >> CHAR16* CommandLine, >> EFI_STATUS *CommandReturnValue >> ) > > I'll try this one and let know. > > Thanks, > Marcin ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell 2016-11-16 23:58 ` Marcin Wojtas @ 2016-11-17 0:57 ` Carsey, Jaben 0 siblings, 0 replies; 7+ messages in thread From: Carsey, Jaben @ 2016-11-17 0:57 UTC (permalink / raw) To: Marcin Wojtas Cc: Leif Lindholm, Ard Biesheuvel, linaro-uefi, Neta Zur Hershkovits, Yehuda Yitschak, Haim Boot, Jan Dabros, Bartosz Szczepanek, edk2-devel-01, Ni, Ruiyu, Carsey, Jaben > -----Original Message----- > From: Marcin Wojtas [mailto:mw@semihalf.com] > Sent: Wednesday, November 16, 2016 3:58 PM > To: Carsey, Jaben <jaben.carsey@intel.com> > Cc: Leif Lindholm <leif.lindholm@linaro.org>; Ard Biesheuvel > <ard.biesheuvel@linaro.org>; linaro-uefi <linaro-uefi@lists.linaro.org>; Neta Zur > Hershkovits <neta@marvell.com>; Yehuda Yitschak <yehuday@marvell.com>; > Haim Boot <hayim@marvell.com>; Jan Dabros <jsd@semihalf.com>; Bartosz > Szczepanek <bsz@semihalf.com>; edk2-devel-01 <edk2-devel@lists.01.org>; Ni, > Ruiyu <ruiyu.ni@intel.com> > Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' > comand to shell > Importance: High > > Hi Jaben, > > I haven't found RunRegisteredCommand in newest edk2. Can you please point > exactly what you meant? > I meant add a new function like that. It does not exist at all now. > As an alternative I checked ShellCommandRunCommandHandler - this is in fact a > library helper function and it's nested deep down in RunShellCommand, which is > for now the only working option. > ShellCommandRunCommandHandler requires a lot of additional processing of > the command line. All is done in Application/Shell code. What's the difference that you need? I see this: looks like you would need a command line (which I assume you have), the return value (which I assume you want), and control over whether you want LastError changed based on this (I would suggest not, but...) RETURN_STATUS EFIAPI ShellCommandRunCommandHandler ( IN CONST CHAR16 *CommandString, IN OUT SHELL_STATUS *RetVal, IN OUT BOOLEAN *CanAffectLE OPTIONAL ) The main difference between that and the RunShellCommand is things like file redirection, trimming space, alias replacement, and things that feel very "command-line" processing. I would think that your TFTP command line would be less needy of fixing up. > > Is there any chance to expose RunShellCommand (or equivalent), so that it can > be used in a nice way, not with including multi "../../" > relative path header? > > I expected, that the commands, which are in fact some wrappers or they mix > multiple others is pretty much of a standard, I'm pretty surprised there are so > huge difficulties in EDK2. I'm wondering of if there are any other options to be > used here (unless we accept, what we have for > now:) ). I think we have less wrapping and less mixing that it appears at first. Those that are related are (I believe) in the same lib and as such, just call each other. > > Best regards, > Marcin > > 2016-11-16 22:48 GMT+01:00 Marcin Wojtas <mw@semihalf.com>: > > Hi Jaben, > > > > Thank you for your input. > > > >> > >> I agree on EBL, but I have very little experience with EBL so I don’t want to > discuss in detail as I am not the right person without more research. Specifically, > my gut reaction is that needing a platform specific boot loader indicates that > something has already gone wrong on that platform. > >> > >> However, this does not seem like a boot loader or an application at all. this is > an internal shell command. The goal here seems to be to create a NULL library > to add a new internal command to the UEFI Shell. This library gets > compiled/linked into the shell itself. > > > > Indeed, it's nothing similar to the bootloader whatsoever. This > > command simply enables updating firmware in SPI flash directly from > > local path or from tftp. > > > >> > >> I feel that we have found a "new" use case that I encountered, but worked > around in the past because all previous cases involved commands in the same > library (there are interactions between Reconnect and Disconnect/Connect). > > > > Right, however Reconnect is easy, as it simply calls gBS callbacks, > > whose definition are also in the same location. > > > >> > >> I would say that a new API in the ShellCommandLib that links the UEFI Shell > Application to the NULL libraries that make up the internal commands would be > my first choice for implementation. I would lean to something like the function > that Marcin already called. Maybe this? > >> > >> EFIAPI > >> RunRegisteredCommand( > >> CHAR16* CommandLine, > >> EFI_STATUS *CommandReturnValue > >> ) > > > > I'll try this one and let know. > > > > Thanks, > > Marcin ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-11-17 0:57 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1468110107-18979-1-git-send-email-mw@semihalf.com>
[not found] ` <1468110107-18979-22-git-send-email-mw@semihalf.com>
[not found] ` <CAKv+Gu9KvJVGgORjxnLzuGVswimavRg8h1ZXKC+=Fv48RLJGEQ@mail.gmail.com>
2016-11-16 16:13 ` [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell Marcin Wojtas
2016-11-16 17:05 ` Carsey, Jaben
2016-11-16 17:35 ` Leif Lindholm
2016-11-16 21:31 ` Carsey, Jaben
2016-11-16 21:48 ` Marcin Wojtas
2016-11-16 23:58 ` Marcin Wojtas
2016-11-17 0:57 ` Carsey, Jaben
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox