public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Carsey, Jaben" <jaben.carsey@intel.com>
To: Leif Lindholm <leif.lindholm@linaro.org>
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>,
	"Carsey, Jaben" <jaben.carsey@intel.com>
Subject: Re: [PATCH v6 21/23] Applications/FirmwareUpdate: Add 'fupdate' comand to shell
Date: Wed, 16 Nov 2016 21:31:22 +0000	[thread overview]
Message-ID: <CB6E33457884FA40993F35157061515C54AEB52D@FMSMSX103.amr.corp.intel.com> (raw)
In-Reply-To: <20161116173553.GB27644@bivouac.eciton.net>

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
> > > >>

  reply	other threads:[~2016-11-16 21:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [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 [this message]
2016-11-16 21:48             ` Marcin Wojtas
2016-11-16 23:58               ` Marcin Wojtas
2016-11-17  0:57                 ` Carsey, Jaben

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CB6E33457884FA40993F35157061515C54AEB52D@FMSMSX103.amr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox