public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [platforms PATCH 0/4] Armada capsule support
@ 2018-06-01 14:32 Marcin Wojtas
  2018-06-01 14:32 ` [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address Marcin Wojtas
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 14:32 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jinghua, jaz, davidsn

Hi,

Hereby I submit a capsule support for Marvell Armada platforms.
Capsule preparation must be done in two steps, it requires additional
build flag (-D CAPSULE_ENABLE), so by default nothing changes. Also the
solution is using generic EDK2 drivers and libraries, so all
wiki/howtos from Tianocore are valid. Verified from within UEFI
(CapsuleApp.efi) and standard distro fwupdate (on Debian and Fedora).
More details can be found in the commit logs.

The code is also available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/capsule-upstream-r20180601

David Sniatkiwicz (1):
  Marvell/Armada7k8k: Introduce capsule FW update implementation

Marcin Wojtas (3):
  Marvell/Armada70x0Db: Shift main FV from 0x0 address
  Marvell/Aramda7k8k: Enable PEI booting stage
  Marvell/Armada7k8k: Wire up capsule support

 Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf                                                       |  96 ++++++-
 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc                                                |  46 ++++
 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf                                                |  70 ++++++
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                                                        |  81 +++++-
 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c           | 261 ++++++++++++++++++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf         |  52 ++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc    |  81 ++++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf     |  50 ++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c    |  74 ++++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  26 ++
 10 files changed, 829 insertions(+), 8 deletions(-)
 create mode 100644 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc
 create mode 100644 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf
 create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
 create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
 create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
 create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
 create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
 create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini

-- 
2.7.4



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address
  2018-06-01 14:32 [platforms PATCH 0/4] Armada capsule support Marcin Wojtas
@ 2018-06-01 14:32 ` Marcin Wojtas
  2018-06-01 15:26   ` Ard Biesheuvel
  2018-06-01 14:32 ` [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage Marcin Wojtas
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 14:32 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jinghua, jaz, davidsn

When using PEI phase, UEFI interprets 0x0 address
of boot FV as an error. In order to avoid it, shift
it to 0x1000 and put a hardcoded 'jump to 0x1000' at
offset 0x0. This patch is a preparation for using PEI
by Armada platforms.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
index befb107..69cb4cd 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
@@ -49,7 +49,21 @@ NumBlocks     = 0x400
 #
 ################################################################################
 
-0x00000000|0x00100000
+#
+# UEFI has trouble dealing with FVs that reside at physical address 0x0.
+# So instead, put a hardcoded 'jump to 0x1000' at offset 0x0, and put the
+# real FV at offset 0x1000
+#
+0x00000000|0x00001000
+DATA = {
+!if $(ARCH) == AARCH64
+  0x00, 0x04, 0x00, 0x14   # 'b 0x1000' in AArch64 ASM
+!else
+  0xfe, 0x03, 0x00, 0xea   # 'b 0x1000' in AArch32 ASM
+!endif
+}
+
+0x00001000|0x000ff000
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
@@ -191,7 +205,6 @@ FvNameGuid         = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
 # PEI phase firmware volume
 [FV.FVMAIN_COMPACT]
 FvAlignment        = 8
-FvForceRebase      = TRUE
 ERASE_POLARITY     = 1
 MEMORY_MAPPED      = TRUE
 STICKY_WRITE       = TRUE
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage
  2018-06-01 14:32 [platforms PATCH 0/4] Armada capsule support Marcin Wojtas
  2018-06-01 14:32 ` [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address Marcin Wojtas
@ 2018-06-01 14:32 ` Marcin Wojtas
  2018-06-01 15:30   ` Ard Biesheuvel
  2018-06-01 14:32 ` [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation Marcin Wojtas
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 14:32 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jinghua, jaz, davidsn

PEI phase will allow to use more robust platform initialization,
with new features like the capsule support. Wire up all
dependencies for that purpose.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 15 ++++++--
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  | 40 ++++++++++++++++++--
 2 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
index 69cb4cd..bf04f4d 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
@@ -63,7 +63,7 @@ DATA = {
 !endif
 }
 
-0x00001000|0x000ff000
+0x00001000|0x001ff000
 gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
 FV = FVMAIN_COMPACT
 
@@ -221,7 +221,14 @@ READ_STATUS        = TRUE
 READ_LOCK_CAP      = TRUE
 READ_LOCK_STATUS   = TRUE
 
-  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
+  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+  INF MdeModulePkg/Core/Pei/PeiMain.inf
+  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
+  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
+  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
     SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
@@ -264,14 +271,14 @@ READ_LOCK_STATUS   = TRUE
 
 [Rule.Common.PEI_CORE]
   FILE PEI_CORE = $(NAMED_GUID) {
-    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
+    TE     TE Align = Auto              $(INF_OUTPUT)/$(MODULE_NAME).efi
     UI     STRING ="$(MODULE_NAME)" Optional
   }
 
 [Rule.Common.PEIM]
   FILE PEIM = $(NAMED_GUID) {
      PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
-     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
+     TE       TE Align = Auto           $(INF_OUTPUT)/$(MODULE_NAME).efi
      UI       STRING="$(MODULE_NAME)" Optional
   }
 
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 4129742..195b6b7 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -145,13 +145,28 @@
   MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
   HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
   PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
-  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
   ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
 
 [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
-  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
+
+[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
+  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
+  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
+  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
+  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
+  PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
+
+[LibraryClasses.common.PEI_CORE]
+  PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
+
+[LibraryClasses.common.PEIM]
+  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
+  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -336,8 +351,13 @@
   # ARM Pcds
   gArmTokenSpaceGuid.PcdSystemMemoryBase|0
   gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
+
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
 
+  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
+
   # Secure region reservation
   gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x4000000
   gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x0200000
@@ -364,7 +384,21 @@
 [Components.common]
 
   # PEI Phase modules
-  ArmPlatformPkg/PrePi/PeiUniCore.inf
+  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
+  MdeModulePkg/Core/Pei/PeiMain.inf
+  MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
+  ArmPlatformPkg/PlatformPei/PlatformPeim.inf
+  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
+  ArmPkg/Drivers/CpuPei/CpuPei.inf
+  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
+    <LibraryClasses>
+      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
+  }
+
 
   # DXE
   MdeModulePkg/Core/Dxe/DxeMain.inf {
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation
  2018-06-01 14:32 [platforms PATCH 0/4] Armada capsule support Marcin Wojtas
  2018-06-01 14:32 ` [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address Marcin Wojtas
  2018-06-01 14:32 ` [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage Marcin Wojtas
@ 2018-06-01 14:32 ` Marcin Wojtas
  2018-06-01 15:32   ` Ard Biesheuvel
  2018-06-01 14:32 ` [platforms PATCH 4/4] Marvell/Armada7k8k: Wire up capsule support Marcin Wojtas
  2018-06-01 15:34 ` [platforms PATCH 0/4] Armada " Ard Biesheuvel
  4 siblings, 1 reply; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 14:32 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jinghua, jaz, davidsn

From: David Sniatkiwicz <davidsn@marvell.com>

This patch adds necessary code that allows to update
firmware on Armada7k8k platforms, using generic gRT->UpdateCapsule,
i.e.
  * PlatformFlashAccessLib implementation to write data to SPI flash
  * SystemFirmwareDescriptor for FMP protocol
  * SystemFirmwareUpdateConfig to specify binary description
    within SystemFirmwareFile

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: David Sniatkiwicz <davidsn@marvell.com>
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c           | 261 ++++++++++++++++++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf         |  52 ++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc    |  81 ++++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf     |  50 ++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c    |  74 ++++++
 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  26 ++
 6 files changed, 544 insertions(+)

diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
new file mode 100644
index 0000000..47d3205
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
@@ -0,0 +1,261 @@
+/** @file
+  Platform flash device access library for Marvell Armada 7k8k Platforms
+
+  Copyright (c) 2018 Marvell International Ltd.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiDxe.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PlatformFlashAccessLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+#include <Protocol/Spi.h>
+#include <Protocol/SpiFlash.h>
+
+#define MAIN_HDR_MAGIC        0xB105B002
+
+STATIC MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol;
+STATIC MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
+
+typedef struct {              // Bytes
+  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_FIRMWARE_IMAGE_HEADER;
+
+STATIC
+EFI_STATUS
+SpiFlashProbe (
+  IN SPI_DEVICE    *SpiFlash
+  )
+{
+  EFI_STATUS       Status;
+
+  // Read SPI flash ID
+  Status = SpiFlashProtocol->ReadId (SpiFlash, FALSE);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  Status = SpiFlashProtocol->Init (SpiFlashProtocol, SpiFlash);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Cannot initialize flash device\n", __FUNCTION__));
+    return Status;
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+CheckImageHeader (
+  IN OUT VOID     *ImageBuffer
+  )
+{
+  MV_FIRMWARE_IMAGE_HEADER *Header;
+  UINT32 HeaderLength, Checksum, ChecksumBackup;
+
+  Header = (MV_FIRMWARE_IMAGE_HEADER *)ImageBuffer;
+  HeaderLength = Header->PrologSize;
+  ChecksumBackup = Header->PrologChecksum;
+
+  // Compare magic number
+  if (Header->Magic != MAIN_HDR_MAGIC) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Bad Image magic 0x%08x != 0x%08x\n",
+      __FUNCTION__,
+      Header->Magic,
+      MAIN_HDR_MAGIC));
+    return EFI_VOLUME_CORRUPTED;
+  }
+
+  // The checksum field is discarded from calculation
+  Header->PrologChecksum = 0;
+
+  Checksum = CalculateSum32 ((UINT32 *)Header, HeaderLength);
+  if (Checksum != ChecksumBackup) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Bad Image checksum. 0x%x != 0x%x\n",
+      __FUNCTION__,
+      Checksum,
+      ChecksumBackup));
+    return EFI_VOLUME_CORRUPTED;
+  }
+
+  // Restore checksum backup
+  Header->PrologChecksum = ChecksumBackup;
+
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Perform flash write operation.
+
+  @param[in] FirmwareType      The type of firmware.
+  @param[in] FlashAddress      The address of flash device to be accessed.
+  @param[in] FlashAddressType  The type of flash device address.
+  @param[in] Buffer            The pointer to the data buffer.
+  @param[in] Length            The length of data buffer in bytes.
+
+  @retval EFI_SUCCESS           The operation returns successfully.
+  @retval EFI_WRITE_PROTECTED   The flash device is read only.
+  @retval EFI_UNSUPPORTED       The flash device access is unsupported.
+  @retval EFI_INVALID_PARAMETER The input parameter is not valid.
+**/
+EFI_STATUS
+EFIAPI
+PerformFlashWrite (
+  IN PLATFORM_FIRMWARE_TYPE       FirmwareType,
+  IN EFI_PHYSICAL_ADDRESS         FlashAddress,
+  IN FLASH_ADDRESS_TYPE           FlashAddressType,
+  IN VOID                         *Buffer,
+  IN UINTN                        Length
+  )
+{
+  EFI_STATUS              Status;
+  VOID                    *ImageBuffer;
+  SPI_DEVICE              *SpiFlash = NULL;
+  BOOLEAN                 BufferAligned = TRUE;
+
+  // Verify Firmware data
+  if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: only FlashAddressTypeAbsoluteAddress supported\n",
+      __FUNCTION__));
+
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (FirmwareType != PlatformFirmwareTypeSystemFirmware) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: only PlatformFirmwareTypeSystemFirmware supported\n",
+      __FUNCTION__));
+
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Locate SPI protocols
+  Status = gBS->LocateProtocol (&gMarvellSpiFlashProtocolGuid,
+                  NULL,
+                  (VOID **)&SpiFlashProtocol);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Cannot locate SpiFlash protocol\n",
+      __FUNCTION__));
+    return Status;
+  }
+
+  Status = gBS->LocateProtocol (&gMarvellSpiMasterProtocolGuid,
+                  NULL,
+                  (VOID **)&SpiMasterProtocol);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Cannot locate SpiMaster protocol\n",
+      __FUNCTION__));
+    return Status;
+  }
+
+  //
+  // Counting checksum in the header verification requires
+  // the buffer address alignment.
+  // It is not guaranteed by the generic capsule handling code,
+  // so use an auxiliary buffer in such case.
+  //
+  if (((UINTN) Buffer & 0x3) != 0) {
+    ImageBuffer = AllocateCopyPool (Length, Buffer);
+    if (ImageBuffer == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+    BufferAligned = FALSE;
+  } else {
+    ImageBuffer = Buffer;
+  }
+
+  // Check image checksum and magic
+  Status = CheckImageHeader (ImageBuffer);
+  if (EFI_ERROR (Status)) {
+    goto HeaderError;
+  }
+
+  // Setup and probe SPI flash
+  SpiFlash = SpiMasterProtocol->SetupDevice (SpiMasterProtocol,
+            SpiFlash,
+            PcdGet32 (PcdSpiFlashCs),
+            PcdGet32 (PcdSpiFlashMode));
+  if (SpiFlash == NULL) {
+    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate SPI device!\n", __FUNCTION__));
+    Status = EFI_DEVICE_ERROR;
+    goto HeaderError;
+  }
+
+  Status = SpiFlashProbe (SpiFlash);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Error while performing SPI flash probe\n",
+      __FUNCTION__));
+    goto FlashProbeError;
+  }
+
+  // Update firmware image in flash at offset 0x0
+  Status = SpiFlashProtocol->Update (SpiFlash, 0, Length, (UINT8 *)ImageBuffer);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "%a: Error while performing flash update\n",
+      __FUNCTION__));
+    goto FlashProbeError;
+  }
+
+  DEBUG ((DEBUG_ERROR,
+    "%a: Update %d bytes at offset 0x0 succeeded!\n",
+    __FUNCTION__,
+    Length));
+
+  // Release resources
+  SpiMasterProtocol->FreeDevice (SpiFlash);
+
+  if (!BufferAligned) {
+    FreePool (ImageBuffer);
+  }
+
+  return EFI_SUCCESS;
+
+FlashProbeError:
+  SpiMasterProtocol->FreeDevice (SpiFlash);
+HeaderError:
+  if (!BufferAligned) {
+    FreePool (ImageBuffer);
+  }
+
+  return Status;
+}
diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
new file mode 100644
index 0000000..fd94759
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
@@ -0,0 +1,52 @@
+## @file
+#  Platform flash device access library.
+#
+#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
+#  Copyright (c) 2018, Marvell International, Ltd. All rights reserved.<BR>
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010019
+  BASE_NAME                      = PlatformFlashAccessLib
+  FILE_GUID                      = c3f314d8-2995-4f0c-a8d6-e10298de4bde
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = PlatformFlashAccessLib|DXE_DRIVER
+
+[Sources]
+  PlatformFlashAccessLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  SignedCapsulePkg/SignedCapsulePkg.dec
+  Silicon/Marvell/Marvell.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  DxeServicesTableLib
+  MemoryAllocationLib
+  PcdLib
+  UefiBootServicesTableLib
+  UefiLib
+  UefiRuntimeServicesTableLib
+
+[Pcd]
+ gMarvellTokenSpaceGuid.PcdSpiFlashCs
+ gMarvellTokenSpaceGuid.PcdSpiFlashMode
+
+[Protocols]
+ gMarvellSpiFlashProtocolGuid
+ gMarvellSpiMasterProtocolGuid
diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
new file mode 100644
index 0000000..fbccdc2
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
@@ -0,0 +1,81 @@
+/** @file
+  System Firmware descriptor.
+
+  Copyright (c) 2018, Marvell International Limited. All rights reserved.
+  Copyright (c) 2018, Linaro Limited. All rights reserved.
+  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiPei.h>
+#include <Guid/EdkiiSystemFmpCapsule.h>
+#include <Protocol/FirmwareManagement.h>
+
+#define PACKAGE_VERSION                     0xFFFFFFFF
+#define PACKAGE_VERSION_STRING              L"Unknown"
+
+#define CURRENT_FIRMWARE_VERSION            0x00000002
+#define CURRENT_FIRMWARE_VERSION_STRING     L"0x00000002"
+#define LOWEST_SUPPORTED_FIRMWARE_VERSION   0x00000001
+
+#define IMAGE_ID                            SIGNATURE_64('M','V','E', 'B', 'U', '_', 'I', 'D')
+#define IMAGE_ID_STRING                     L"MvebuPlatform"
+
+// PcdSystemFmpCapsuleImageTypeIdGuid
+#define IMAGE_TYPE_ID_GUID                  { 0x757fc475, 0x6b22, 0x4482, { 0x86, 0x8e, 0xde, 0xd2, 0x86, 0xf3, 0x09, 0x40 } }
+
+typedef struct {
+  EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR  Descriptor;
+  // real string data
+  CHAR16                                  ImageIdNameStr[ARRAY_SIZE (IMAGE_ID_STRING)];
+  CHAR16                                  VersionNameStr[ARRAY_SIZE (CURRENT_FIRMWARE_VERSION_STRING)];
+  CHAR16                                  PackageVersionNameStr[ARRAY_SIZE (PACKAGE_VERSION_STRING)];
+} IMAGE_DESCRIPTOR;
+
+IMAGE_DESCRIPTOR mImageDescriptor =
+{
+  {
+    EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE,
+    sizeof (EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR),
+    sizeof (IMAGE_DESCRIPTOR),
+    PACKAGE_VERSION,                                       // PackageVersion
+    OFFSET_OF (IMAGE_DESCRIPTOR, PackageVersionNameStr),   // PackageVersionName
+    1,                                                     // ImageIndex;
+    {0x0},                                                 // Reserved
+    IMAGE_TYPE_ID_GUID,                                    // ImageTypeId;
+    IMAGE_ID,                                              // ImageId;
+    OFFSET_OF (IMAGE_DESCRIPTOR, ImageIdNameStr),          // ImageIdName;
+    CURRENT_FIRMWARE_VERSION,                              // Version;
+    OFFSET_OF (IMAGE_DESCRIPTOR, VersionNameStr),          // VersionName;
+    {0x0},                                                 // Reserved2
+    FixedPcdGet32 (PcdFdSize),                             // Size;
+    IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
+      IMAGE_ATTRIBUTE_RESET_REQUIRED |
+      IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
+      IMAGE_ATTRIBUTE_IN_USE,                              // AttributesSupported;
+    IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
+      IMAGE_ATTRIBUTE_RESET_REQUIRED |
+      IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
+      IMAGE_ATTRIBUTE_IN_USE,                              // AttributesSetting;
+    0x0,                                                   // Compatibilities;
+    LOWEST_SUPPORTED_FIRMWARE_VERSION,                     // LowestSupportedImageVersion;
+    0x00000000,                                            // LastAttemptVersion;
+    0,                                                     // LastAttemptStatus;
+    {0x0},                                                 // Reserved3
+    0,                                                     // HardwareInstance;
+  },
+  // real string data
+  {IMAGE_ID_STRING},
+  {CURRENT_FIRMWARE_VERSION_STRING},
+  {PACKAGE_VERSION_STRING},
+};
+
+VOID* CONST ReferenceAcpiTable = &mImageDescriptor;
diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
new file mode 100644
index 0000000..e6967b2
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
@@ -0,0 +1,50 @@
+## @file
+#  System Firmware descriptor.
+#
+#  Copyright (c) 2018, Marvell International Limited. All rights reserved.
+#  Copyright (c) 2018, Linaro Limited. All rights reserved.
+#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = SystemFirmwareDescriptor
+  FILE_GUID                      = 90B2B846-CA6D-4D6E-A8D3-C140A8E110AC
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = SystemFirmwareDescriptorPeimEntry
+
+[Sources]
+  SystemFirmwareDescriptorPei.c
+  SystemFirmwareDescriptor.aslc
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  SignedCapsulePkg/SignedCapsulePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  PcdLib
+  PeimEntryPoint
+  PeiServicesLib
+
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFdSize
+
+[Pcd]
+  gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
+
+[Depex]
+  TRUE
diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
new file mode 100644
index 0000000..c55c4d9
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
@@ -0,0 +1,74 @@
+/** @file
+  System Firmware descriptor producer.
+
+  Copyright (c) 2018, Marvell International, Ltd. All rights reserved.
+  Copyright (c) 2018, Linaro Limited. All rights reserved.
+  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiPei.h>
+#include <Guid/EdkiiSystemFmpCapsule.h>
+#include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Protocol/FirmwareManagement.h>
+
+/**
+  Entrypoint for SystemFirmwareDescriptor PEIM.
+
+  @param[in]  FileHandle  Handle of the file being invoked.
+  @param[in]  PeiServices Describes the list of possible PEI Services.
+
+  @retval EFI_SUCCESS            PPI successfully installed.
+**/
+EFI_STATUS
+EFIAPI
+SystemFirmwareDescriptorPeimEntry (
+  IN EFI_PEI_FILE_HANDLE     FileHandle,
+  IN CONST EFI_PEI_SERVICES  **PeiServices
+  )
+{
+  EFI_STATUS                              Status;
+  EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR  *Descriptor;
+  UINTN                                   Size;
+  UINTN                                   Index;
+  UINT32                                  AuthenticationStatus;
+
+  //
+  // Search RAW section.
+  //
+
+  Index = 0;
+  while (TRUE) {
+    Status = PeiServicesFfsFindSectionData3 (EFI_SECTION_RAW,
+               Index,
+               FileHandle,
+               (VOID **)&Descriptor,
+               &AuthenticationStatus);
+    if (EFI_ERROR (Status)) {
+      // Should not happen, must something wrong in FDF.
+      ASSERT(FALSE);
+      return EFI_NOT_FOUND;
+    }
+    if (Descriptor->Signature == EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE) {
+      break;
+    }
+    Index++;
+  }
+
+  DEBUG ((DEBUG_INFO, "EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR size - 0x%x\n", Descriptor->Length));
+
+  Size = Descriptor->Length;
+  PcdSetPtrS (PcdEdkiiSystemFirmwareImageDescriptor, &Size, Descriptor);
+
+  return EFI_SUCCESS;
+}
diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
new file mode 100644
index 0000000..fb0bd0b
--- /dev/null
+++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
@@ -0,0 +1,26 @@
+## @file
+#
+#  Copyright (c) 2018, Marvell International Ltd.<BR>
+#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Head]
+NumOfUpdate = 1
+NumOfRecovery = 0
+Update0 = MvFvMain
+
+[MvFvMain]
+FirmwareType  = 0             # SystemFirmware
+AddressType   = 1             # 0 - relative address, 1 - absolute address.
+BaseAddress   = 0x0           # Base address offset on flash
+Length        = 0x00300000    # Length, need to fix the length
+ImageOffset   = 0x00100000    # Image offset of this SystemFirmware image
+FileGuid      = b3890e02-c46b-4970-9536-57787a9e06c7  # PcdEdkiiSystemFirmwareFileGuid
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [platforms PATCH 4/4] Marvell/Armada7k8k: Wire up capsule support
  2018-06-01 14:32 [platforms PATCH 0/4] Armada capsule support Marcin Wojtas
                   ` (2 preceding siblings ...)
  2018-06-01 14:32 ` [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation Marcin Wojtas
@ 2018-06-01 14:32 ` Marcin Wojtas
  2018-06-01 15:34 ` [platforms PATCH 0/4] Armada " Ard Biesheuvel
  4 siblings, 0 replies; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 14:32 UTC (permalink / raw)
  To: edk2-devel; +Cc: leif.lindholm, ard.biesheuvel, mw, jsd, jinghua, jaz, davidsn

All required components are in place, so we can now
add all necessary dependencies to build and use capsule support
for Armada7k8k platforms. It is conditionally enabled
with '-D CAPSULE_ENABLE' flag added during build time.

Because the capsule generation must be sequential,
due to boot requirements and glueing all binaries
(BLE, ARM-TF, BL33) externally, introduce additional
.dsc and .fdf file solely for creating the capsule.
Prior to this step 'flash-image.bin' binary must be placed
under Platform/Marvell path.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Signed-off-by: David Sniatkiwicz <davidsn@marvell.com>
---
 Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf        | 66 ++++++++++++++++++
 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc | 46 +++++++++++++
 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf | 70 ++++++++++++++++++++
 Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc         | 41 ++++++++++++
 4 files changed, 223 insertions(+)

diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
index bf04f4d..e5e5443 100644
--- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
@@ -201,6 +201,15 @@ FvNameGuid         = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
   INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
   INF MdeModulePkg/Application/UiApp/UiApp.inf
 
+!if $(CAPSULE_ENABLE)
+  # Firmware update
+  INF MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf
+  INF SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf
+  FILE FREEFORM = PCD(gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiPkcs7TestPublicKeyFileGuid) {
+     SECTION RAW = BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer
+     SECTION UI = "Pkcs7TestRoot"
+  }
+!endif
 
 # PEI phase firmware volume
 [FV.FVMAIN_COMPACT]
@@ -228,6 +237,11 @@ READ_LOCK_STATUS   = TRUE
   INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
   INF ArmPkg/Drivers/CpuPei/CpuPei.inf
   INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+!if $(CAPSULE_ENABLE)
+  INF MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+  INF MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+  INF RuleOverride = FMP_IMAGE_DESC Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
+!endif
   INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
@@ -236,6 +250,49 @@ READ_LOCK_STATUS   = TRUE
     }
   }
 
+!if $(CAPSULE_ENABLE)
+[FV.SystemFirmwareDescriptor]
+FvAlignment        = 8
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  INF RuleOverride = FMP_IMAGE_DESC Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
+
+[FV.CapsuleDispatchFv]
+FvAlignment        = 8
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  INF  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf
+
+!endif
+
 ################################################################################
 #
 # Rules are use with the [FV] section's module INF type to define
@@ -331,3 +388,12 @@ READ_LOCK_STATUS   = TRUE
     UI     STRING ="$(MODULE_NAME)" Optional
     PE32   PE32                         $(INF_OUTPUT)/$(MODULE_NAME).efi
   }
+
+[Rule.Common.PEIM.FMP_IMAGE_DESC]
+  FILE PEIM = $(NAMED_GUID) {
+     RAW BIN                  |.acpi
+     PEI_DEPEX PEI_DEPEX Optional        $(INF_OUTPUT)/$(MODULE_NAME).depex
+     PE32      PE32    Align=4K          $(INF_OUTPUT)/$(MODULE_NAME).efi
+     UI       STRING="$(MODULE_NAME)" Optional
+     VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
+  }
diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc b/Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc
new file mode 100644
index 0000000..a7d1382
--- /dev/null
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc
@@ -0,0 +1,46 @@
+#Copyright (C) 2018 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.
+#
+################################################################################
+#
+# Defines Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+  PLATFORM_NAME                  = Armada70x0Db
+  PLATFORM_GUID                  = 982e2ab1-26ca-4617-92e6-061bd6ba9ae3
+  PLATFORM_VERSION               = 0.1
+  DSC_SPECIFICATION              = 0x00010019
+  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)-$(ARCH)
+  SUPPORTED_ARCHITECTURES        = AARCH64|ARM
+  BUILD_TARGETS                  = DEBUG|RELEASE
+  SKUID_IDENTIFIER               = DEFAULT
+  FLASH_DEFINITION               = Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf
diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf
new file mode 100644
index 0000000..e5a9c68
--- /dev/null
+++ b/Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf
@@ -0,0 +1,70 @@
+#
+#  Copyright (C) Marvell International Ltd. and its affiliates
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD License
+#  which accompanies this distribution.  The full text of the license may be found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+
+[FD.Armada_Capsule]
+BaseAddress   = 0x00000000 #|gArmTokenSpaceGuid.PcdFdBaseAddress  # The base address of the Firmware in NOR Flash.
+Size          = 0x00400000 #|gArmTokenSpaceGuid.PcdFdSize         # The size in bytes of the FLASH Device
+ErasePolarity = 1
+
+0x00000000|0x00010000
+FILE = $(WORKSPACE)/$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/FV/SYSTEMFIRMWAREDESCRIPTOR.Fv
+
+0x00100000|0x00300000
+FILE = Platform/Marvell/flash-image.bin
+
+[FV.SystemFirmwareUpdateCargo]
+FvAlignment        = 8
+ERASE_POLARITY     = 1
+MEMORY_MAPPED      = TRUE
+STICKY_WRITE       = TRUE
+LOCK_CAP           = TRUE
+LOCK_STATUS        = TRUE
+WRITE_DISABLED_CAP = TRUE
+WRITE_ENABLED_CAP  = TRUE
+WRITE_STATUS       = TRUE
+WRITE_LOCK_CAP     = TRUE
+WRITE_LOCK_STATUS  = TRUE
+READ_DISABLED_CAP  = TRUE
+READ_ENABLED_CAP   = TRUE
+READ_STATUS        = TRUE
+READ_LOCK_CAP      = TRUE
+READ_LOCK_STATUS   = TRUE
+
+  FILE RAW = b3890e02-c46b-4970-9536-57787a9e06c7 { # PcdEdkiiSystemFirmwareFileGuid
+     FD = Armada_Capsule
+  }
+
+  FILE RAW = ce57b167-b0e4-41e8-a897-5f4feb781d40 { # gEdkiiSystemFmpCapsuleDriverFvFileGuid
+    $(WORKSPACE)/$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/FV/CAPSULEDISPATCHFV.Fv
+  }
+
+  FILE RAW = 812136D3-4D3A-433A-9418-29BB9BF78F6E { # gEdkiiSystemFmpCapsuleConfigFileGuid
+    Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
+  }
+
+[FmpPayload.FmpPayloadSystemFirmwarePkcs7]
+IMAGE_HEADER_INIT_VERSION = 0x02
+IMAGE_TYPE_ID             = 757fc475-6b22-4482-868e-ded286f30940 # PcdSystemFmpCapsuleImageTypeIdGuid
+IMAGE_INDEX               = 0x1
+HARDWARE_INSTANCE         = 0x0
+MONOTONIC_COUNT           = 0x1
+CERTIFICATE_GUID          = 4AAFD29D-68DF-49EE-8AA9-347D375665A7 # PKCS7
+
+  FV = SystemFirmwareUpdateCargo
+
+[Capsule.MvFirmwareUpdateCapsuleFmpPkcs7]
+CAPSULE_GUID                = 6dcbd5ed-e82d-4c44-bda1-7194199ad92a # gEfiFmpCapsuleGuid
+CAPSULE_HEADER_SIZE         = 0x20
+CAPSULE_HEADER_INIT_VERSION = 0x1
+
+  FMP_PAYLOAD = FmpPayloadSystemFirmwarePkcs7
+
diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
index 195b6b7..6e4cc8b 100644
--- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
+++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
@@ -42,8 +42,23 @@
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
   DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
 
+!if $(CAPSULE_ENABLE)
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+  EdkiiSystemCapsuleLib|SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
+  FmpAuthenticationLib|SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.inf
+  IniParsingLib|SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
+  PlatformFlashAccessLib|Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
+!endif
+
 # Basic utility libraries
   BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+!if $(CAPSULE_ENABLE)
+  BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+!endif
   SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
   PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
   PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
@@ -199,7 +214,11 @@
 [LibraryClasses.common.DXE_RUNTIME_DRIVER]
   HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
   MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
+!if $(CAPSULE_ENABLE)
+  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
+!else
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
+!endif
 !if $(TARGET) != RELEASE
   DebugLib|MdePkg/Library/DxeRuntimeDebugLibSerialPort/DxeRuntimeDebugLibSerialPort.inf
 !endif
@@ -376,6 +395,13 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0xF93E0000
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
 
+!if $(CAPSULE_ENABLE)
+[PcdsDynamicExDefault.common.DEFAULT]
+  gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor|{0x0}|VOID*|0x100
+  gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareFileGuid|{0x02, 0x0e, 0x89, 0xb3, 0x6b, 0xc4, 0x70, 0x49, 0x95, 0x36, 0x57, 0x78, 0x7a, 0x9e, 0x06, 0xc7}
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSystemFmpCapsuleImageTypeIdGuid|{0x75, 0xc4, 0x7f, 0x75, 0x22, 0x6b, 0x82, 0x44, 0x86, 0x8e, 0xde, 0xd2, 0x86, 0xf3, 0x09, 0x40}
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
@@ -394,6 +420,11 @@
   ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
   ArmPkg/Drivers/CpuPei/CpuPei.inf
   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+!if $(CAPSULE_ENABLE)
+  MdeModulePkg/Universal/FaultTolerantWritePei/FaultTolerantWritePei.inf
+  MdeModulePkg/Universal/CapsulePei/CapsulePei.inf
+  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
+!endif
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
     <LibraryClasses>
       NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
@@ -503,6 +534,16 @@
       NULL|MdeModulePkg/Library/BootMaintenanceManagerUiLib/BootMaintenanceManagerUiLib.inf
   }
 
+!if $(CAPSULE_ENABLE)
+  MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf
+  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareReportDxe.inf
+  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf {
+    <LibraryClasses>
+      FmpAuthenticationLib|SecurityPkg/Library/FmpAuthenticationLibPkcs7/FmpAuthenticationLibPkcs7.inf
+  }
+  MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+!endif
+
   #
   # Variable services
   #
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address
  2018-06-01 14:32 ` [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address Marcin Wojtas
@ 2018-06-01 15:26   ` Ard Biesheuvel
  0 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 15:26 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, davidsn

On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
> When using PEI phase, UEFI interprets 0x0 address
> of boot FV as an error. In order to avoid it, shift
> it to 0x1000 and put a hardcoded 'jump to 0x1000' at
> offset 0x0. This patch is a preparation for using PEI
> by Armada platforms.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
> index befb107..69cb4cd 100644
> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
> @@ -49,7 +49,21 @@ NumBlocks     = 0x400
>  #
>  ################################################################################
>
> -0x00000000|0x00100000
> +#
> +# UEFI has trouble dealing with FVs that reside at physical address 0x0.
> +# So instead, put a hardcoded 'jump to 0x1000' at offset 0x0, and put the
> +# real FV at offset 0x1000
> +#
> +0x00000000|0x00001000
> +DATA = {
> +!if $(ARCH) == AARCH64
> +  0x00, 0x04, 0x00, 0x14   # 'b 0x1000' in AArch64 ASM
> +!else
> +  0xfe, 0x03, 0x00, 0xea   # 'b 0x1000' in AArch32 ASM
> +!endif
> +}
> +
> +0x00001000|0x000ff000
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>
> @@ -191,7 +205,6 @@ FvNameGuid         = 5eda4200-2c5f-43cb-9da3-0baf74b1b30c
>  # PEI phase firmware volume
>  [FV.FVMAIN_COMPACT]
>  FvAlignment        = 8
> -FvForceRebase      = TRUE
>  ERASE_POLARITY     = 1
>  MEMORY_MAPPED      = TRUE
>  STICKY_WRITE       = TRUE
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage
  2018-06-01 14:32 ` [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage Marcin Wojtas
@ 2018-06-01 15:30   ` Ard Biesheuvel
  2018-06-01 16:43     ` Marcin Wojtas
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 15:30 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, davidsn

On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
> PEI phase will allow to use more robust platform initialization,
> with new features like the capsule support. Wire up all
> dependencies for that purpose.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 15 ++++++--
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  | 40 ++++++++++++++++++--
>  2 files changed, 48 insertions(+), 7 deletions(-)
>
> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
> index 69cb4cd..bf04f4d 100644
> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
> @@ -63,7 +63,7 @@ DATA = {
>  !endif
>  }
>
> -0x00001000|0x000ff000
> +0x00001000|0x001ff000
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>  FV = FVMAIN_COMPACT
>
> @@ -221,7 +221,14 @@ READ_STATUS        = TRUE
>  READ_LOCK_CAP      = TRUE
>  READ_LOCK_STATUS   = TRUE
>
> -  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
> +  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +  INF MdeModulePkg/Core/Pei/PeiMain.inf
> +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
> +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
> +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>
>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
> @@ -264,14 +271,14 @@ READ_LOCK_STATUS   = TRUE
>
>  [Rule.Common.PEI_CORE]
>    FILE PEI_CORE = $(NAMED_GUID) {
> -    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
> +    TE     TE Align = Auto              $(INF_OUTPUT)/$(MODULE_NAME).efi
>      UI     STRING ="$(MODULE_NAME)" Optional
>    }
>
>  [Rule.Common.PEIM]
>    FILE PEIM = $(NAMED_GUID) {
>       PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
> -     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
> +     TE       TE Align = Auto           $(INF_OUTPUT)/$(MODULE_NAME).efi
>       UI       STRING="$(MODULE_NAME)" Optional
>    }
>
> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> index 4129742..195b6b7 100644
> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
> @@ -145,13 +145,28 @@
>    MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>    HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
>    PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>    ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>
>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
> -  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +
> +[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
> +  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> +  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> +  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> +  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> +  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
> +  PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
> +
> +[LibraryClasses.common.PEI_CORE]
> +  PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
> +
> +[LibraryClasses.common.PEIM]
> +  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> +  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>
>  [LibraryClasses.common.DXE_CORE]
>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
> @@ -336,8 +351,13 @@
>    # ARM Pcds
>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0
>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
> +
> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>
> +  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
> +

These were copy/pasted from SynQuacer, I suppose?

Ideally, these should point to some memory that is not exposed to the
OS, so that the PEI phase cannot corrupt a capsule image that has been
left in DRAM by the OS.

This only becomes relevant once we implement support for PSCI warm
reboot, otherwise DRAM will be cleared anyway. However, pointing these
into a random slice of main memory feels a little risky.

Do you have non-secure SRAM on this SoC?

>    # Secure region reservation
>    gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x4000000
>    gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x0200000
> @@ -364,7 +384,21 @@
>  [Components.common]
>
>    # PEI Phase modules
> -  ArmPlatformPkg/PrePi/PeiUniCore.inf
> +  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> +  MdeModulePkg/Core/Pei/PeiMain.inf
> +  MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
> +    <LibraryClasses>
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
> +  ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
> +  ArmPkg/Drivers/CpuPei/CpuPei.inf
> +  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
> +  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
> +    <LibraryClasses>
> +      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> +  }
> +
>
>    # DXE
>    MdeModulePkg/Core/Dxe/DxeMain.inf {
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation
  2018-06-01 14:32 ` [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation Marcin Wojtas
@ 2018-06-01 15:32   ` Ard Biesheuvel
  2018-06-01 16:02     ` Marcin Wojtas
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 15:32 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, davidsn

 On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
> From: David Sniatkiwicz <davidsn@marvell.com>
>
> This patch adds necessary code that allows to update
> firmware on Armada7k8k platforms, using generic gRT->UpdateCapsule,
> i.e.
>   * PlatformFlashAccessLib implementation to write data to SPI flash
>   * SystemFirmwareDescriptor for FMP protocol
>   * SystemFirmwareUpdateConfig to specify binary description
>     within SystemFirmwareFile
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: David Sniatkiwicz <davidsn@marvell.com>
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Please take the [very recent] changes made in
550de36633fec97d4f7467c02a33b3217f440ae9 into account here.
Unfortunately, there is no loop in your code from which you can report
progress, but you should at least implement the new entry point.


> ---
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c           | 261 ++++++++++++++++++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf         |  52 ++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc    |  81 ++++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf     |  50 ++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c    |  74 ++++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  26 ++
>  6 files changed, 544 insertions(+)
>
> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
> new file mode 100644
> index 0000000..47d3205
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
> @@ -0,0 +1,261 @@
> +/** @file
> +  Platform flash device access library for Marvell Armada 7k8k Platforms
> +
> +  Copyright (c) 2018 Marvell International Ltd.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <PiDxe.h>
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PlatformFlashAccessLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +
> +#include <Protocol/Spi.h>
> +#include <Protocol/SpiFlash.h>
> +
> +#define MAIN_HDR_MAGIC        0xB105B002
> +
> +STATIC MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol;
> +STATIC MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
> +
> +typedef struct {              // Bytes
> +  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_FIRMWARE_IMAGE_HEADER;
> +
> +STATIC
> +EFI_STATUS
> +SpiFlashProbe (
> +  IN SPI_DEVICE    *SpiFlash
> +  )
> +{
> +  EFI_STATUS       Status;
> +
> +  // Read SPI flash ID
> +  Status = SpiFlashProtocol->ReadId (SpiFlash, FALSE);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = SpiFlashProtocol->Init (SpiFlashProtocol, SpiFlash);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "%a: Cannot initialize flash device\n", __FUNCTION__));
> +    return Status;
> +  }
> +
> +  return EFI_SUCCESS;
> +}
> +
> +STATIC
> +EFI_STATUS
> +CheckImageHeader (
> +  IN OUT VOID     *ImageBuffer
> +  )
> +{
> +  MV_FIRMWARE_IMAGE_HEADER *Header;
> +  UINT32 HeaderLength, Checksum, ChecksumBackup;
> +
> +  Header = (MV_FIRMWARE_IMAGE_HEADER *)ImageBuffer;
> +  HeaderLength = Header->PrologSize;
> +  ChecksumBackup = Header->PrologChecksum;
> +
> +  // Compare magic number
> +  if (Header->Magic != MAIN_HDR_MAGIC) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: Bad Image magic 0x%08x != 0x%08x\n",
> +      __FUNCTION__,
> +      Header->Magic,
> +      MAIN_HDR_MAGIC));
> +    return EFI_VOLUME_CORRUPTED;
> +  }
> +
> +  // The checksum field is discarded from calculation
> +  Header->PrologChecksum = 0;
> +
> +  Checksum = CalculateSum32 ((UINT32 *)Header, HeaderLength);
> +  if (Checksum != ChecksumBackup) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: Bad Image checksum. 0x%x != 0x%x\n",
> +      __FUNCTION__,
> +      Checksum,
> +      ChecksumBackup));
> +    return EFI_VOLUME_CORRUPTED;
> +  }
> +
> +  // Restore checksum backup
> +  Header->PrologChecksum = ChecksumBackup;
> +
> +  return EFI_SUCCESS;
> +}
> +
> +
> +/**
> +  Perform flash write operation.
> +
> +  @param[in] FirmwareType      The type of firmware.
> +  @param[in] FlashAddress      The address of flash device to be accessed.
> +  @param[in] FlashAddressType  The type of flash device address.
> +  @param[in] Buffer            The pointer to the data buffer.
> +  @param[in] Length            The length of data buffer in bytes.
> +
> +  @retval EFI_SUCCESS           The operation returns successfully.
> +  @retval EFI_WRITE_PROTECTED   The flash device is read only.
> +  @retval EFI_UNSUPPORTED       The flash device access is unsupported.
> +  @retval EFI_INVALID_PARAMETER The input parameter is not valid.
> +**/
> +EFI_STATUS
> +EFIAPI
> +PerformFlashWrite (
> +  IN PLATFORM_FIRMWARE_TYPE       FirmwareType,
> +  IN EFI_PHYSICAL_ADDRESS         FlashAddress,
> +  IN FLASH_ADDRESS_TYPE           FlashAddressType,
> +  IN VOID                         *Buffer,
> +  IN UINTN                        Length
> +  )
> +{
> +  EFI_STATUS              Status;
> +  VOID                    *ImageBuffer;
> +  SPI_DEVICE              *SpiFlash = NULL;
> +  BOOLEAN                 BufferAligned = TRUE;
> +
> +  // Verify Firmware data
> +  if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: only FlashAddressTypeAbsoluteAddress supported\n",
> +      __FUNCTION__));
> +
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (FirmwareType != PlatformFirmwareTypeSystemFirmware) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: only PlatformFirmwareTypeSystemFirmware supported\n",
> +      __FUNCTION__));
> +
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Locate SPI protocols
> +  Status = gBS->LocateProtocol (&gMarvellSpiFlashProtocolGuid,
> +                  NULL,
> +                  (VOID **)&SpiFlashProtocol);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: Cannot locate SpiFlash protocol\n",
> +      __FUNCTION__));
> +    return Status;
> +  }
> +
> +  Status = gBS->LocateProtocol (&gMarvellSpiMasterProtocolGuid,
> +                  NULL,
> +                  (VOID **)&SpiMasterProtocol);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: Cannot locate SpiMaster protocol\n",
> +      __FUNCTION__));
> +    return Status;
> +  }
> +
> +  //
> +  // Counting checksum in the header verification requires
> +  // the buffer address alignment.
> +  // It is not guaranteed by the generic capsule handling code,
> +  // so use an auxiliary buffer in such case.
> +  //
> +  if (((UINTN) Buffer & 0x3) != 0) {
> +    ImageBuffer = AllocateCopyPool (Length, Buffer);
> +    if (ImageBuffer == NULL) {
> +      return EFI_OUT_OF_RESOURCES;
> +    }
> +    BufferAligned = FALSE;
> +  } else {
> +    ImageBuffer = Buffer;
> +  }
> +
> +  // Check image checksum and magic
> +  Status = CheckImageHeader (ImageBuffer);
> +  if (EFI_ERROR (Status)) {
> +    goto HeaderError;
> +  }
> +
> +  // Setup and probe SPI flash
> +  SpiFlash = SpiMasterProtocol->SetupDevice (SpiMasterProtocol,
> +            SpiFlash,
> +            PcdGet32 (PcdSpiFlashCs),
> +            PcdGet32 (PcdSpiFlashMode));
> +  if (SpiFlash == NULL) {
> +    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate SPI device!\n", __FUNCTION__));
> +    Status = EFI_DEVICE_ERROR;
> +    goto HeaderError;
> +  }
> +
> +  Status = SpiFlashProbe (SpiFlash);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: Error while performing SPI flash probe\n",
> +      __FUNCTION__));
> +    goto FlashProbeError;
> +  }
> +
> +  // Update firmware image in flash at offset 0x0
> +  Status = SpiFlashProtocol->Update (SpiFlash, 0, Length, (UINT8 *)ImageBuffer);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR,
> +      "%a: Error while performing flash update\n",
> +      __FUNCTION__));
> +    goto FlashProbeError;
> +  }
> +
> +  DEBUG ((DEBUG_ERROR,
> +    "%a: Update %d bytes at offset 0x0 succeeded!\n",
> +    __FUNCTION__,
> +    Length));
> +
> +  // Release resources
> +  SpiMasterProtocol->FreeDevice (SpiFlash);
> +
> +  if (!BufferAligned) {
> +    FreePool (ImageBuffer);
> +  }
> +
> +  return EFI_SUCCESS;
> +
> +FlashProbeError:
> +  SpiMasterProtocol->FreeDevice (SpiFlash);
> +HeaderError:
> +  if (!BufferAligned) {
> +    FreePool (ImageBuffer);
> +  }
> +
> +  return Status;
> +}
> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
> new file mode 100644
> index 0000000..fd94759
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
> @@ -0,0 +1,52 @@
> +## @file
> +#  Platform flash device access library.
> +#
> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
> +#  Copyright (c) 2018, Marvell International, Ltd. All rights reserved.<BR>
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010019
> +  BASE_NAME                      = PlatformFlashAccessLib
> +  FILE_GUID                      = c3f314d8-2995-4f0c-a8d6-e10298de4bde
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = PlatformFlashAccessLib|DXE_DRIVER
> +
> +[Sources]
> +  PlatformFlashAccessLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  SignedCapsulePkg/SignedCapsulePkg.dec
> +  Silicon/Marvell/Marvell.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  DxeServicesTableLib
> +  MemoryAllocationLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +  UefiLib
> +  UefiRuntimeServicesTableLib
> +
> +[Pcd]
> + gMarvellTokenSpaceGuid.PcdSpiFlashCs
> + gMarvellTokenSpaceGuid.PcdSpiFlashMode
> +
> +[Protocols]
> + gMarvellSpiFlashProtocolGuid
> + gMarvellSpiMasterProtocolGuid
> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
> new file mode 100644
> index 0000000..fbccdc2
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
> @@ -0,0 +1,81 @@
> +/** @file
> +  System Firmware descriptor.
> +
> +  Copyright (c) 2018, Marvell International Limited. All rights reserved.
> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <PiPei.h>
> +#include <Guid/EdkiiSystemFmpCapsule.h>
> +#include <Protocol/FirmwareManagement.h>
> +
> +#define PACKAGE_VERSION                     0xFFFFFFFF
> +#define PACKAGE_VERSION_STRING              L"Unknown"
> +
> +#define CURRENT_FIRMWARE_VERSION            0x00000002
> +#define CURRENT_FIRMWARE_VERSION_STRING     L"0x00000002"
> +#define LOWEST_SUPPORTED_FIRMWARE_VERSION   0x00000001
> +
> +#define IMAGE_ID                            SIGNATURE_64('M','V','E', 'B', 'U', '_', 'I', 'D')
> +#define IMAGE_ID_STRING                     L"MvebuPlatform"
> +
> +// PcdSystemFmpCapsuleImageTypeIdGuid
> +#define IMAGE_TYPE_ID_GUID                  { 0x757fc475, 0x6b22, 0x4482, { 0x86, 0x8e, 0xde, 0xd2, 0x86, 0xf3, 0x09, 0x40 } }
> +
> +typedef struct {
> +  EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR  Descriptor;
> +  // real string data
> +  CHAR16                                  ImageIdNameStr[ARRAY_SIZE (IMAGE_ID_STRING)];
> +  CHAR16                                  VersionNameStr[ARRAY_SIZE (CURRENT_FIRMWARE_VERSION_STRING)];
> +  CHAR16                                  PackageVersionNameStr[ARRAY_SIZE (PACKAGE_VERSION_STRING)];
> +} IMAGE_DESCRIPTOR;
> +
> +IMAGE_DESCRIPTOR mImageDescriptor =
> +{
> +  {
> +    EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE,
> +    sizeof (EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR),
> +    sizeof (IMAGE_DESCRIPTOR),
> +    PACKAGE_VERSION,                                       // PackageVersion
> +    OFFSET_OF (IMAGE_DESCRIPTOR, PackageVersionNameStr),   // PackageVersionName
> +    1,                                                     // ImageIndex;
> +    {0x0},                                                 // Reserved
> +    IMAGE_TYPE_ID_GUID,                                    // ImageTypeId;
> +    IMAGE_ID,                                              // ImageId;
> +    OFFSET_OF (IMAGE_DESCRIPTOR, ImageIdNameStr),          // ImageIdName;
> +    CURRENT_FIRMWARE_VERSION,                              // Version;
> +    OFFSET_OF (IMAGE_DESCRIPTOR, VersionNameStr),          // VersionName;
> +    {0x0},                                                 // Reserved2
> +    FixedPcdGet32 (PcdFdSize),                             // Size;
> +    IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
> +      IMAGE_ATTRIBUTE_RESET_REQUIRED |
> +      IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
> +      IMAGE_ATTRIBUTE_IN_USE,                              // AttributesSupported;
> +    IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
> +      IMAGE_ATTRIBUTE_RESET_REQUIRED |
> +      IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
> +      IMAGE_ATTRIBUTE_IN_USE,                              // AttributesSetting;
> +    0x0,                                                   // Compatibilities;
> +    LOWEST_SUPPORTED_FIRMWARE_VERSION,                     // LowestSupportedImageVersion;
> +    0x00000000,                                            // LastAttemptVersion;
> +    0,                                                     // LastAttemptStatus;
> +    {0x0},                                                 // Reserved3
> +    0,                                                     // HardwareInstance;
> +  },
> +  // real string data
> +  {IMAGE_ID_STRING},
> +  {CURRENT_FIRMWARE_VERSION_STRING},
> +  {PACKAGE_VERSION_STRING},
> +};
> +
> +VOID* CONST ReferenceAcpiTable = &mImageDescriptor;
> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> new file mode 100644
> index 0000000..e6967b2
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
> @@ -0,0 +1,50 @@
> +## @file
> +#  System Firmware descriptor.
> +#
> +#  Copyright (c) 2018, Marvell International Limited. All rights reserved.
> +#  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x0001001A
> +  BASE_NAME                      = SystemFirmwareDescriptor
> +  FILE_GUID                      = 90B2B846-CA6D-4D6E-A8D3-C140A8E110AC
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = SystemFirmwareDescriptorPeimEntry
> +
> +[Sources]
> +  SystemFirmwareDescriptorPei.c
> +  SystemFirmwareDescriptor.aslc
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  SignedCapsulePkg/SignedCapsulePkg.dec
> +
> +[LibraryClasses]
> +  DebugLib
> +  PcdLib
> +  PeimEntryPoint
> +  PeiServicesLib
> +
> +[FixedPcd]
> +  gArmTokenSpaceGuid.PcdFdSize
> +
> +[Pcd]
> +  gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
> +
> +[Depex]
> +  TRUE
> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
> new file mode 100644
> index 0000000..c55c4d9
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
> @@ -0,0 +1,74 @@
> +/** @file
> +  System Firmware descriptor producer.
> +
> +  Copyright (c) 2018, Marvell International, Ltd. All rights reserved.
> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <PiPei.h>
> +#include <Guid/EdkiiSystemFmpCapsule.h>
> +#include <Library/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PeiServicesLib.h>
> +#include <Protocol/FirmwareManagement.h>
> +
> +/**
> +  Entrypoint for SystemFirmwareDescriptor PEIM.
> +
> +  @param[in]  FileHandle  Handle of the file being invoked.
> +  @param[in]  PeiServices Describes the list of possible PEI Services.
> +
> +  @retval EFI_SUCCESS            PPI successfully installed.
> +**/
> +EFI_STATUS
> +EFIAPI
> +SystemFirmwareDescriptorPeimEntry (
> +  IN EFI_PEI_FILE_HANDLE     FileHandle,
> +  IN CONST EFI_PEI_SERVICES  **PeiServices
> +  )
> +{
> +  EFI_STATUS                              Status;
> +  EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR  *Descriptor;
> +  UINTN                                   Size;
> +  UINTN                                   Index;
> +  UINT32                                  AuthenticationStatus;
> +
> +  //
> +  // Search RAW section.
> +  //
> +
> +  Index = 0;
> +  while (TRUE) {
> +    Status = PeiServicesFfsFindSectionData3 (EFI_SECTION_RAW,
> +               Index,
> +               FileHandle,
> +               (VOID **)&Descriptor,
> +               &AuthenticationStatus);
> +    if (EFI_ERROR (Status)) {
> +      // Should not happen, must something wrong in FDF.
> +      ASSERT(FALSE);
> +      return EFI_NOT_FOUND;
> +    }
> +    if (Descriptor->Signature == EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE) {
> +      break;
> +    }
> +    Index++;
> +  }
> +
> +  DEBUG ((DEBUG_INFO, "EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR size - 0x%x\n", Descriptor->Length));
> +
> +  Size = Descriptor->Length;
> +  PcdSetPtrS (PcdEdkiiSystemFirmwareImageDescriptor, &Size, Descriptor);
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
> new file mode 100644
> index 0000000..fb0bd0b
> --- /dev/null
> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
> @@ -0,0 +1,26 @@
> +## @file
> +#
> +#  Copyright (c) 2018, Marvell International Ltd.<BR>
> +#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD License
> +#  which accompanies this distribution.  The full text of the license may be found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Head]
> +NumOfUpdate = 1
> +NumOfRecovery = 0
> +Update0 = MvFvMain
> +
> +[MvFvMain]
> +FirmwareType  = 0             # SystemFirmware
> +AddressType   = 1             # 0 - relative address, 1 - absolute address.
> +BaseAddress   = 0x0           # Base address offset on flash
> +Length        = 0x00300000    # Length, need to fix the length
> +ImageOffset   = 0x00100000    # Image offset of this SystemFirmware image
> +FileGuid      = b3890e02-c46b-4970-9536-57787a9e06c7  # PcdEdkiiSystemFirmwareFileGuid
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 0/4] Armada capsule support
  2018-06-01 14:32 [platforms PATCH 0/4] Armada capsule support Marcin Wojtas
                   ` (3 preceding siblings ...)
  2018-06-01 14:32 ` [platforms PATCH 4/4] Marvell/Armada7k8k: Wire up capsule support Marcin Wojtas
@ 2018-06-01 15:34 ` Ard Biesheuvel
  4 siblings, 0 replies; 15+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 15:34 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, davidsn

On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi,
>
> Hereby I submit a capsule support for Marvell Armada platforms.
> Capsule preparation must be done in two steps, it requires additional
> build flag (-D CAPSULE_ENABLE), so by default nothing changes. Also the
> solution is using generic EDK2 drivers and libraries, so all
> wiki/howtos from Tianocore are valid. Verified from within UEFI
> (CapsuleApp.efi) and standard distro fwupdate (on Debian and Fedora).
> More details can be found in the commit logs.
>
> The code is also available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/capsule-upstream-r20180601
>
> David Sniatkiwicz (1):
>   Marvell/Armada7k8k: Introduce capsule FW update implementation
>
> Marcin Wojtas (3):
>   Marvell/Armada70x0Db: Shift main FV from 0x0 address
>   Marvell/Aramda7k8k: Enable PEI booting stage
>   Marvell/Armada7k8k: Wire up capsule support
>

Very nice!

The code looks mostly fine to me, with a couple of minor comments.

>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf                                                       |  96 ++++++-
>  Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc                                                |  46 ++++
>  Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf                                                |  70 ++++++
>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc                                                        |  81 +++++-
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c           | 261 ++++++++++++++++++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf         |  52 ++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc    |  81 ++++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf     |  50 ++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c    |  74 ++++++
>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  26 ++
>  10 files changed, 829 insertions(+), 8 deletions(-)
>  create mode 100644 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.dsc
>  create mode 100644 Platform/Marvell/Armada70x0Db/Armada70x0DbCapsule.fdf
>  create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
>  create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
>  create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
>  create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
>  create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
>  create mode 100644 Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
>
> --
> 2.7.4
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation
  2018-06-01 15:32   ` Ard Biesheuvel
@ 2018-06-01 16:02     ` Marcin Wojtas
  2018-06-01 16:08       ` Ard Biesheuvel
  0 siblings, 1 reply; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 16:02 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, David Sniatkiwicz

Hi Ard,

2018-06-01 17:32 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>  On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
>> From: David Sniatkiwicz <davidsn@marvell.com>
>>
>> This patch adds necessary code that allows to update
>> firmware on Armada7k8k platforms, using generic gRT->UpdateCapsule,
>> i.e.
>>   * PlatformFlashAccessLib implementation to write data to SPI flash
>>   * SystemFirmwareDescriptor for FMP protocol
>>   * SystemFirmwareUpdateConfig to specify binary description
>>     within SystemFirmwareFile
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: David Sniatkiwicz <davidsn@marvell.com>
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>
> Please take the [very recent] changes made in
> 550de36633fec97d4f7467c02a33b3217f440ae9 into account here.
> Unfortunately, there is no loop in your code from which you can report
> progress, but you should at least implement the new entry point.
>

Actually there is a loop and current % of progress printed when
updating data in the SPI.
It is implemented in the SPI flash protocol callback.

https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c#L382

What would you recommend me to do in such case?

Thanks,
Marcin

>
>> ---
>>  Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c           | 261 ++++++++++++++++++++
>>  Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf         |  52 ++++
>>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc    |  81 ++++++
>>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf     |  50 ++++
>>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c    |  74 ++++++
>>  Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini |  26 ++
>>  6 files changed, 544 insertions(+)
>>
>> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
>> new file mode 100644
>> index 0000000..47d3205
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.c
>> @@ -0,0 +1,261 @@
>> +/** @file
>> +  Platform flash device access library for Marvell Armada 7k8k Platforms
>> +
>> +  Copyright (c) 2018 Marvell International Ltd.<BR>
>> +
>> +  This program and the accompanying materials
>> +  are licensed and made available under the terms and conditions of the BSD License
>> +  which accompanies this distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <PiDxe.h>
>> +
>> +#include <Library/BaseMemoryLib.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/DxeServicesTableLib.h>
>> +#include <Library/MemoryAllocationLib.h>
>> +#include <Library/PlatformFlashAccessLib.h>
>> +#include <Library/UefiBootServicesTableLib.h>
>> +#include <Library/UefiLib.h>
>> +
>> +#include <Protocol/Spi.h>
>> +#include <Protocol/SpiFlash.h>
>> +
>> +#define MAIN_HDR_MAGIC        0xB105B002
>> +
>> +STATIC MARVELL_SPI_FLASH_PROTOCOL *SpiFlashProtocol;
>> +STATIC MARVELL_SPI_MASTER_PROTOCOL *SpiMasterProtocol;
>> +
>> +typedef struct {              // Bytes
>> +  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_FIRMWARE_IMAGE_HEADER;
>> +
>> +STATIC
>> +EFI_STATUS
>> +SpiFlashProbe (
>> +  IN SPI_DEVICE    *SpiFlash
>> +  )
>> +{
>> +  EFI_STATUS       Status;
>> +
>> +  // Read SPI flash ID
>> +  Status = SpiFlashProtocol->ReadId (SpiFlash, FALSE);
>> +  if (EFI_ERROR (Status)) {
>> +    return Status;
>> +  }
>> +
>> +  Status = SpiFlashProtocol->Init (SpiFlashProtocol, SpiFlash);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "%a: Cannot initialize flash device\n", __FUNCTION__));
>> +    return Status;
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +STATIC
>> +EFI_STATUS
>> +CheckImageHeader (
>> +  IN OUT VOID     *ImageBuffer
>> +  )
>> +{
>> +  MV_FIRMWARE_IMAGE_HEADER *Header;
>> +  UINT32 HeaderLength, Checksum, ChecksumBackup;
>> +
>> +  Header = (MV_FIRMWARE_IMAGE_HEADER *)ImageBuffer;
>> +  HeaderLength = Header->PrologSize;
>> +  ChecksumBackup = Header->PrologChecksum;
>> +
>> +  // Compare magic number
>> +  if (Header->Magic != MAIN_HDR_MAGIC) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Bad Image magic 0x%08x != 0x%08x\n",
>> +      __FUNCTION__,
>> +      Header->Magic,
>> +      MAIN_HDR_MAGIC));
>> +    return EFI_VOLUME_CORRUPTED;
>> +  }
>> +
>> +  // The checksum field is discarded from calculation
>> +  Header->PrologChecksum = 0;
>> +
>> +  Checksum = CalculateSum32 ((UINT32 *)Header, HeaderLength);
>> +  if (Checksum != ChecksumBackup) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Bad Image checksum. 0x%x != 0x%x\n",
>> +      __FUNCTION__,
>> +      Checksum,
>> +      ChecksumBackup));
>> +    return EFI_VOLUME_CORRUPTED;
>> +  }
>> +
>> +  // Restore checksum backup
>> +  Header->PrologChecksum = ChecksumBackup;
>> +
>> +  return EFI_SUCCESS;
>> +}
>> +
>> +
>> +/**
>> +  Perform flash write operation.
>> +
>> +  @param[in] FirmwareType      The type of firmware.
>> +  @param[in] FlashAddress      The address of flash device to be accessed.
>> +  @param[in] FlashAddressType  The type of flash device address.
>> +  @param[in] Buffer            The pointer to the data buffer.
>> +  @param[in] Length            The length of data buffer in bytes.
>> +
>> +  @retval EFI_SUCCESS           The operation returns successfully.
>> +  @retval EFI_WRITE_PROTECTED   The flash device is read only.
>> +  @retval EFI_UNSUPPORTED       The flash device access is unsupported.
>> +  @retval EFI_INVALID_PARAMETER The input parameter is not valid.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +PerformFlashWrite (
>> +  IN PLATFORM_FIRMWARE_TYPE       FirmwareType,
>> +  IN EFI_PHYSICAL_ADDRESS         FlashAddress,
>> +  IN FLASH_ADDRESS_TYPE           FlashAddressType,
>> +  IN VOID                         *Buffer,
>> +  IN UINTN                        Length
>> +  )
>> +{
>> +  EFI_STATUS              Status;
>> +  VOID                    *ImageBuffer;
>> +  SPI_DEVICE              *SpiFlash = NULL;
>> +  BOOLEAN                 BufferAligned = TRUE;
>> +
>> +  // Verify Firmware data
>> +  if (FlashAddressType != FlashAddressTypeAbsoluteAddress) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: only FlashAddressTypeAbsoluteAddress supported\n",
>> +      __FUNCTION__));
>> +
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  if (FirmwareType != PlatformFirmwareTypeSystemFirmware) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: only PlatformFirmwareTypeSystemFirmware supported\n",
>> +      __FUNCTION__));
>> +
>> +    return EFI_INVALID_PARAMETER;
>> +  }
>> +
>> +  // Locate SPI protocols
>> +  Status = gBS->LocateProtocol (&gMarvellSpiFlashProtocolGuid,
>> +                  NULL,
>> +                  (VOID **)&SpiFlashProtocol);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Cannot locate SpiFlash protocol\n",
>> +      __FUNCTION__));
>> +    return Status;
>> +  }
>> +
>> +  Status = gBS->LocateProtocol (&gMarvellSpiMasterProtocolGuid,
>> +                  NULL,
>> +                  (VOID **)&SpiMasterProtocol);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Cannot locate SpiMaster protocol\n",
>> +      __FUNCTION__));
>> +    return Status;
>> +  }
>> +
>> +  //
>> +  // Counting checksum in the header verification requires
>> +  // the buffer address alignment.
>> +  // It is not guaranteed by the generic capsule handling code,
>> +  // so use an auxiliary buffer in such case.
>> +  //
>> +  if (((UINTN) Buffer & 0x3) != 0) {
>> +    ImageBuffer = AllocateCopyPool (Length, Buffer);
>> +    if (ImageBuffer == NULL) {
>> +      return EFI_OUT_OF_RESOURCES;
>> +    }
>> +    BufferAligned = FALSE;
>> +  } else {
>> +    ImageBuffer = Buffer;
>> +  }
>> +
>> +  // Check image checksum and magic
>> +  Status = CheckImageHeader (ImageBuffer);
>> +  if (EFI_ERROR (Status)) {
>> +    goto HeaderError;
>> +  }
>> +
>> +  // Setup and probe SPI flash
>> +  SpiFlash = SpiMasterProtocol->SetupDevice (SpiMasterProtocol,
>> +            SpiFlash,
>> +            PcdGet32 (PcdSpiFlashCs),
>> +            PcdGet32 (PcdSpiFlashMode));
>> +  if (SpiFlash == NULL) {
>> +    DEBUG ((DEBUG_ERROR, "%a: Cannot allocate SPI device!\n", __FUNCTION__));
>> +    Status = EFI_DEVICE_ERROR;
>> +    goto HeaderError;
>> +  }
>> +
>> +  Status = SpiFlashProbe (SpiFlash);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Error while performing SPI flash probe\n",
>> +      __FUNCTION__));
>> +    goto FlashProbeError;
>> +  }
>> +
>> +  // Update firmware image in flash at offset 0x0
>> +  Status = SpiFlashProtocol->Update (SpiFlash, 0, Length, (UINT8 *)ImageBuffer);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR,
>> +      "%a: Error while performing flash update\n",
>> +      __FUNCTION__));
>> +    goto FlashProbeError;
>> +  }
>> +
>> +  DEBUG ((DEBUG_ERROR,
>> +    "%a: Update %d bytes at offset 0x0 succeeded!\n",
>> +    __FUNCTION__,
>> +    Length));
>> +
>> +  // Release resources
>> +  SpiMasterProtocol->FreeDevice (SpiFlash);
>> +
>> +  if (!BufferAligned) {
>> +    FreePool (ImageBuffer);
>> +  }
>> +
>> +  return EFI_SUCCESS;
>> +
>> +FlashProbeError:
>> +  SpiMasterProtocol->FreeDevice (SpiFlash);
>> +HeaderError:
>> +  if (!BufferAligned) {
>> +    FreePool (ImageBuffer);
>> +  }
>> +
>> +  return Status;
>> +}
>> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
>> new file mode 100644
>> index 0000000..fd94759
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/PlatformFlashAccessLib/PlatformFlashAccessLib.inf
>> @@ -0,0 +1,52 @@
>> +## @file
>> +#  Platform flash device access library.
>> +#
>> +#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
>> +#  Copyright (c) 2018, Marvell International, Ltd. All rights reserved.<BR>
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the BSD License
>> +#  which accompanies this distribution.  The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x00010019
>> +  BASE_NAME                      = PlatformFlashAccessLib
>> +  FILE_GUID                      = c3f314d8-2995-4f0c-a8d6-e10298de4bde
>> +  MODULE_TYPE                    = BASE
>> +  VERSION_STRING                 = 1.0
>> +  LIBRARY_CLASS                  = PlatformFlashAccessLib|DXE_DRIVER
>> +
>> +[Sources]
>> +  PlatformFlashAccessLib.c
>> +
>> +[Packages]
>> +  ArmPkg/ArmPkg.dec
>> +  EmbeddedPkg/EmbeddedPkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  SignedCapsulePkg/SignedCapsulePkg.dec
>> +  Silicon/Marvell/Marvell.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  BaseMemoryLib
>> +  DebugLib
>> +  DxeServicesTableLib
>> +  MemoryAllocationLib
>> +  PcdLib
>> +  UefiBootServicesTableLib
>> +  UefiLib
>> +  UefiRuntimeServicesTableLib
>> +
>> +[Pcd]
>> + gMarvellTokenSpaceGuid.PcdSpiFlashCs
>> + gMarvellTokenSpaceGuid.PcdSpiFlashMode
>> +
>> +[Protocols]
>> + gMarvellSpiFlashProtocolGuid
>> + gMarvellSpiMasterProtocolGuid
>> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
>> new file mode 100644
>> index 0000000..fbccdc2
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.aslc
>> @@ -0,0 +1,81 @@
>> +/** @file
>> +  System Firmware descriptor.
>> +
>> +  Copyright (c) 2018, Marvell International Limited. All rights reserved.
>> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
>> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials
>> +  are licensed and made available under the terms and conditions of the BSD License
>> +  which accompanies this distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <PiPei.h>
>> +#include <Guid/EdkiiSystemFmpCapsule.h>
>> +#include <Protocol/FirmwareManagement.h>
>> +
>> +#define PACKAGE_VERSION                     0xFFFFFFFF
>> +#define PACKAGE_VERSION_STRING              L"Unknown"
>> +
>> +#define CURRENT_FIRMWARE_VERSION            0x00000002
>> +#define CURRENT_FIRMWARE_VERSION_STRING     L"0x00000002"
>> +#define LOWEST_SUPPORTED_FIRMWARE_VERSION   0x00000001
>> +
>> +#define IMAGE_ID                            SIGNATURE_64('M','V','E', 'B', 'U', '_', 'I', 'D')
>> +#define IMAGE_ID_STRING                     L"MvebuPlatform"
>> +
>> +// PcdSystemFmpCapsuleImageTypeIdGuid
>> +#define IMAGE_TYPE_ID_GUID                  { 0x757fc475, 0x6b22, 0x4482, { 0x86, 0x8e, 0xde, 0xd2, 0x86, 0xf3, 0x09, 0x40 } }
>> +
>> +typedef struct {
>> +  EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR  Descriptor;
>> +  // real string data
>> +  CHAR16                                  ImageIdNameStr[ARRAY_SIZE (IMAGE_ID_STRING)];
>> +  CHAR16                                  VersionNameStr[ARRAY_SIZE (CURRENT_FIRMWARE_VERSION_STRING)];
>> +  CHAR16                                  PackageVersionNameStr[ARRAY_SIZE (PACKAGE_VERSION_STRING)];
>> +} IMAGE_DESCRIPTOR;
>> +
>> +IMAGE_DESCRIPTOR mImageDescriptor =
>> +{
>> +  {
>> +    EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE,
>> +    sizeof (EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR),
>> +    sizeof (IMAGE_DESCRIPTOR),
>> +    PACKAGE_VERSION,                                       // PackageVersion
>> +    OFFSET_OF (IMAGE_DESCRIPTOR, PackageVersionNameStr),   // PackageVersionName
>> +    1,                                                     // ImageIndex;
>> +    {0x0},                                                 // Reserved
>> +    IMAGE_TYPE_ID_GUID,                                    // ImageTypeId;
>> +    IMAGE_ID,                                              // ImageId;
>> +    OFFSET_OF (IMAGE_DESCRIPTOR, ImageIdNameStr),          // ImageIdName;
>> +    CURRENT_FIRMWARE_VERSION,                              // Version;
>> +    OFFSET_OF (IMAGE_DESCRIPTOR, VersionNameStr),          // VersionName;
>> +    {0x0},                                                 // Reserved2
>> +    FixedPcdGet32 (PcdFdSize),                             // Size;
>> +    IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
>> +      IMAGE_ATTRIBUTE_RESET_REQUIRED |
>> +      IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
>> +      IMAGE_ATTRIBUTE_IN_USE,                              // AttributesSupported;
>> +    IMAGE_ATTRIBUTE_IMAGE_UPDATABLE |
>> +      IMAGE_ATTRIBUTE_RESET_REQUIRED |
>> +      IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED |
>> +      IMAGE_ATTRIBUTE_IN_USE,                              // AttributesSetting;
>> +    0x0,                                                   // Compatibilities;
>> +    LOWEST_SUPPORTED_FIRMWARE_VERSION,                     // LowestSupportedImageVersion;
>> +    0x00000000,                                            // LastAttemptVersion;
>> +    0,                                                     // LastAttemptStatus;
>> +    {0x0},                                                 // Reserved3
>> +    0,                                                     // HardwareInstance;
>> +  },
>> +  // real string data
>> +  {IMAGE_ID_STRING},
>> +  {CURRENT_FIRMWARE_VERSION_STRING},
>> +  {PACKAGE_VERSION_STRING},
>> +};
>> +
>> +VOID* CONST ReferenceAcpiTable = &mImageDescriptor;
>> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
>> new file mode 100644
>> index 0000000..e6967b2
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptor.inf
>> @@ -0,0 +1,50 @@
>> +## @file
>> +#  System Firmware descriptor.
>> +#
>> +#  Copyright (c) 2018, Marvell International Limited. All rights reserved.
>> +#  Copyright (c) 2018, Linaro Limited. All rights reserved.
>> +#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>> +#
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the BSD License
>> +#  which accompanies this distribution.  The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +##
>> +
>> +[Defines]
>> +  INF_VERSION                    = 0x0001001A
>> +  BASE_NAME                      = SystemFirmwareDescriptor
>> +  FILE_GUID                      = 90B2B846-CA6D-4D6E-A8D3-C140A8E110AC
>> +  MODULE_TYPE                    = PEIM
>> +  VERSION_STRING                 = 1.0
>> +  ENTRY_POINT                    = SystemFirmwareDescriptorPeimEntry
>> +
>> +[Sources]
>> +  SystemFirmwareDescriptorPei.c
>> +  SystemFirmwareDescriptor.aslc
>> +
>> +[Packages]
>> +  ArmPkg/ArmPkg.dec
>> +  ArmPlatformPkg/ArmPlatformPkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +  SignedCapsulePkg/SignedCapsulePkg.dec
>> +
>> +[LibraryClasses]
>> +  DebugLib
>> +  PcdLib
>> +  PeimEntryPoint
>> +  PeiServicesLib
>> +
>> +[FixedPcd]
>> +  gArmTokenSpaceGuid.PcdFdSize
>> +
>> +[Pcd]
>> +  gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor
>> +
>> +[Depex]
>> +  TRUE
>> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
>> new file mode 100644
>> index 0000000..c55c4d9
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareDescriptor/SystemFirmwareDescriptorPei.c
>> @@ -0,0 +1,74 @@
>> +/** @file
>> +  System Firmware descriptor producer.
>> +
>> +  Copyright (c) 2018, Marvell International, Ltd. All rights reserved.
>> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
>> +  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials
>> +  are licensed and made available under the terms and conditions of the BSD License
>> +  which accompanies this distribution.  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +
>> +**/
>> +
>> +#include <PiPei.h>
>> +#include <Guid/EdkiiSystemFmpCapsule.h>
>> +#include <Library/DebugLib.h>
>> +#include <Library/PcdLib.h>
>> +#include <Library/PeiServicesLib.h>
>> +#include <Protocol/FirmwareManagement.h>
>> +
>> +/**
>> +  Entrypoint for SystemFirmwareDescriptor PEIM.
>> +
>> +  @param[in]  FileHandle  Handle of the file being invoked.
>> +  @param[in]  PeiServices Describes the list of possible PEI Services.
>> +
>> +  @retval EFI_SUCCESS            PPI successfully installed.
>> +**/
>> +EFI_STATUS
>> +EFIAPI
>> +SystemFirmwareDescriptorPeimEntry (
>> +  IN EFI_PEI_FILE_HANDLE     FileHandle,
>> +  IN CONST EFI_PEI_SERVICES  **PeiServices
>> +  )
>> +{
>> +  EFI_STATUS                              Status;
>> +  EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR  *Descriptor;
>> +  UINTN                                   Size;
>> +  UINTN                                   Index;
>> +  UINT32                                  AuthenticationStatus;
>> +
>> +  //
>> +  // Search RAW section.
>> +  //
>> +
>> +  Index = 0;
>> +  while (TRUE) {
>> +    Status = PeiServicesFfsFindSectionData3 (EFI_SECTION_RAW,
>> +               Index,
>> +               FileHandle,
>> +               (VOID **)&Descriptor,
>> +               &AuthenticationStatus);
>> +    if (EFI_ERROR (Status)) {
>> +      // Should not happen, must something wrong in FDF.
>> +      ASSERT(FALSE);
>> +      return EFI_NOT_FOUND;
>> +    }
>> +    if (Descriptor->Signature == EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR_SIGNATURE) {
>> +      break;
>> +    }
>> +    Index++;
>> +  }
>> +
>> +  DEBUG ((DEBUG_INFO, "EDKII_SYSTEM_FIRMWARE_IMAGE_DESCRIPTOR size - 0x%x\n", Descriptor->Length));
>> +
>> +  Size = Descriptor->Length;
>> +  PcdSetPtrS (PcdEdkiiSystemFirmwareImageDescriptor, &Size, Descriptor);
>> +
>> +  return EFI_SUCCESS;
>> +}
>> diff --git a/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
>> new file mode 100644
>> index 0000000..fb0bd0b
>> --- /dev/null
>> +++ b/Silicon/Marvell/Armada7k8k/Feature/Capsule/SystemFirmwareUpdateConfig/SystemFirmwareUpdateConfig.ini
>> @@ -0,0 +1,26 @@
>> +## @file
>> +#
>> +#  Copyright (c) 2018, Marvell International Ltd.<BR>
>> +#  Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
>> +#  This program and the accompanying materials
>> +#  are licensed and made available under the terms and conditions of the BSD License
>> +#  which accompanies this distribution.  The full text of the license may be found at
>> +#  http://opensource.org/licenses/bsd-license.php
>> +#
>> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>> +#
>> +##
>> +
>> +[Head]
>> +NumOfUpdate = 1
>> +NumOfRecovery = 0
>> +Update0 = MvFvMain
>> +
>> +[MvFvMain]
>> +FirmwareType  = 0             # SystemFirmware
>> +AddressType   = 1             # 0 - relative address, 1 - absolute address.
>> +BaseAddress   = 0x0           # Base address offset on flash
>> +Length        = 0x00300000    # Length, need to fix the length
>> +ImageOffset   = 0x00100000    # Image offset of this SystemFirmware image
>> +FileGuid      = b3890e02-c46b-4970-9536-57787a9e06c7  # PcdEdkiiSystemFirmwareFileGuid
>> --
>> 2.7.4
>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation
  2018-06-01 16:02     ` Marcin Wojtas
@ 2018-06-01 16:08       ` Ard Biesheuvel
  2018-06-01 16:20         ` Marcin Wojtas
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 16:08 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, David Sniatkiwicz

On 1 June 2018 at 18:02, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi Ard,
>
> 2018-06-01 17:32 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>  On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
>>> From: David Sniatkiwicz <davidsn@marvell.com>
>>>
>>> This patch adds necessary code that allows to update
>>> firmware on Armada7k8k platforms, using generic gRT->UpdateCapsule,
>>> i.e.
>>>   * PlatformFlashAccessLib implementation to write data to SPI flash
>>>   * SystemFirmwareDescriptor for FMP protocol
>>>   * SystemFirmwareUpdateConfig to specify binary description
>>>     within SystemFirmwareFile
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: David Sniatkiwicz <davidsn@marvell.com>
>>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>>
>> Please take the [very recent] changes made in
>> 550de36633fec97d4f7467c02a33b3217f440ae9 into account here.
>> Unfortunately, there is no loop in your code from which you can report
>> progress, but you should at least implement the new entry point.
>>
>
> Actually there is a loop and current % of progress printed when
> updating data in the SPI.
> It is implemented in the SPI flash protocol callback.
>
> https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c#L382
>
> What would you recommend me to do in such case?
>

You could find a way to pass the callback argument into your SPI flash
protocol method, perhaps by adding a new member to it?


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation
  2018-06-01 16:08       ` Ard Biesheuvel
@ 2018-06-01 16:20         ` Marcin Wojtas
  0 siblings, 0 replies; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 16:20 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, David Sniatkiwicz

2018-06-01 18:08 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 1 June 2018 at 18:02, Marcin Wojtas <mw@semihalf.com> wrote:
>> Hi Ard,
>>
>> 2018-06-01 17:32 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>>  On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
>>>> From: David Sniatkiwicz <davidsn@marvell.com>
>>>>
>>>> This patch adds necessary code that allows to update
>>>> firmware on Armada7k8k platforms, using generic gRT->UpdateCapsule,
>>>> i.e.
>>>>   * PlatformFlashAccessLib implementation to write data to SPI flash
>>>>   * SystemFirmwareDescriptor for FMP protocol
>>>>   * SystemFirmwareUpdateConfig to specify binary description
>>>>     within SystemFirmwareFile
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: David Sniatkiwicz <davidsn@marvell.com>
>>>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>>>
>>> Please take the [very recent] changes made in
>>> 550de36633fec97d4f7467c02a33b3217f440ae9 into account here.
>>> Unfortunately, there is no loop in your code from which you can report
>>> progress, but you should at least implement the new entry point.
>>>
>>
>> Actually there is a loop and current % of progress printed when
>> updating data in the SPI.
>> It is implemented in the SPI flash protocol callback.
>>
>> https://github.com/tianocore/edk2-platforms/blob/master/Silicon/Marvell/Drivers/Spi/MvSpiFlashDxe/MvSpiFlashDxe.c#L382
>>
>> What would you recommend me to do in such case?
>>
>
> You could find a way to pass the callback argument into your SPI flash
> protocol method, perhaps by adding a new member to it?

Good idea, will try it.

Thanks,
Marcin


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage
  2018-06-01 15:30   ` Ard Biesheuvel
@ 2018-06-01 16:43     ` Marcin Wojtas
  2018-06-01 16:57       ` Ard Biesheuvel
  0 siblings, 1 reply; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 16:43 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, David Sniatkiwicz

2018-06-01 17:30 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
>> PEI phase will allow to use more robust platform initialization,
>> with new features like the capsule support. Wire up all
>> dependencies for that purpose.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>> ---
>>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 15 ++++++--
>>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  | 40 ++++++++++++++++++--
>>  2 files changed, 48 insertions(+), 7 deletions(-)
>>
>> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>> index 69cb4cd..bf04f4d 100644
>> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>> @@ -63,7 +63,7 @@ DATA = {
>>  !endif
>>  }
>>
>> -0x00001000|0x000ff000
>> +0x00001000|0x001ff000
>>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>>  FV = FVMAIN_COMPACT
>>
>> @@ -221,7 +221,14 @@ READ_STATUS        = TRUE
>>  READ_LOCK_CAP      = TRUE
>>  READ_LOCK_STATUS   = TRUE
>>
>> -  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
>> +  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>> +  INF MdeModulePkg/Core/Pei/PeiMain.inf
>> +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>> +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
>> +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>> +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
>> +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>> +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>
>>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>> @@ -264,14 +271,14 @@ READ_LOCK_STATUS   = TRUE
>>
>>  [Rule.Common.PEI_CORE]
>>    FILE PEI_CORE = $(NAMED_GUID) {
>> -    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +    TE     TE Align = Auto              $(INF_OUTPUT)/$(MODULE_NAME).efi
>>      UI     STRING ="$(MODULE_NAME)" Optional
>>    }
>>
>>  [Rule.Common.PEIM]
>>    FILE PEIM = $(NAMED_GUID) {
>>       PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
>> -     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
>> +     TE       TE Align = Auto           $(INF_OUTPUT)/$(MODULE_NAME).efi
>>       UI       STRING="$(MODULE_NAME)" Optional
>>    }
>>
>> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>> index 4129742..195b6b7 100644
>> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>> @@ -145,13 +145,28 @@
>>    MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>>    HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
>>    PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>>    ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>>
>>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>> -  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>> +  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>> +
>> +[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
>> +  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>> +  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>> +  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
>> +  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>> +  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
>> +  PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>> +
>> +[LibraryClasses.common.PEI_CORE]
>> +  PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
>> +
>> +[LibraryClasses.common.PEIM]
>> +  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
>> +  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>>
>>  [LibraryClasses.common.DXE_CORE]
>>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>> @@ -336,8 +351,13 @@
>>    # ARM Pcds
>>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0
>>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
>> +
>> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>>
>> +  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
>> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
>> +
>
> These were copy/pasted from SynQuacer, I suppose?
>

SynQuacer and ARM VExpress use this value, I also checked ones used in
HiSilicon Platforms - I didn't notice any issue with neither of them.

> Ideally, these should point to some memory that is not exposed to the
> OS, so that the PEI phase cannot corrupt a capsule image that has been
> left in DRAM by the OS.
>
> This only becomes relevant once we implement support for PSCI warm
> reboot, otherwise DRAM will be cleared anyway. However, pointing these
> into a random slice of main memory feels a little risky.


Currently we have also a hole between 0xc0000000 - 4GB for the config
space, so how about squeezing those 64kB there?

Alternatively would you recommend to use value outside the DRAM space
(e.g. above max DRAM size)?

>
> Do you have non-secure SRAM on this SoC?

There are a couple of SRAMs (for offload CM3 CPUs), one configured as
secure. Not sure about others. Why do you ask?

Best regards,
Marcin



>
>>    # Secure region reservation
>>    gMarvellTokenSpaceGuid.PcdSecureRegionBase|0x4000000
>>    gMarvellTokenSpaceGuid.PcdSecureRegionSize|0x0200000
>> @@ -364,7 +384,21 @@
>>  [Components.common]
>>
>>    # PEI Phase modules
>> -  ArmPlatformPkg/PrePi/PeiUniCore.inf
>> +  ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>> +  MdeModulePkg/Core/Pei/PeiMain.inf
>> +  MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
>> +    <LibraryClasses>
>> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>> +  }
>> +  ArmPlatformPkg/PlatformPei/PlatformPeim.inf
>> +  ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>> +  ArmPkg/Drivers/CpuPei/CpuPei.inf
>> +  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>> +  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
>> +    <LibraryClasses>
>> +      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
>> +  }
>> +
>>
>>    # DXE
>>    MdeModulePkg/Core/Dxe/DxeMain.inf {
>> --
>> 2.7.4
>>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage
  2018-06-01 16:43     ` Marcin Wojtas
@ 2018-06-01 16:57       ` Ard Biesheuvel
  2018-06-01 17:18         ` Marcin Wojtas
  0 siblings, 1 reply; 15+ messages in thread
From: Ard Biesheuvel @ 2018-06-01 16:57 UTC (permalink / raw)
  To: Marcin Wojtas
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, David Sniatkiwicz

On 1 June 2018 at 18:43, Marcin Wojtas <mw@semihalf.com> wrote:
> 2018-06-01 17:30 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>> On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
>>> PEI phase will allow to use more robust platform initialization,
>>> with new features like the capsule support. Wire up all
>>> dependencies for that purpose.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>>> ---
>>>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 15 ++++++--
>>>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  | 40 ++++++++++++++++++--
>>>  2 files changed, 48 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>>> index 69cb4cd..bf04f4d 100644
>>> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>>> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>>> @@ -63,7 +63,7 @@ DATA = {
>>>  !endif
>>>  }
>>>
>>> -0x00001000|0x000ff000
>>> +0x00001000|0x001ff000
>>>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>>>  FV = FVMAIN_COMPACT
>>>
>>> @@ -221,7 +221,14 @@ READ_STATUS        = TRUE
>>>  READ_LOCK_CAP      = TRUE
>>>  READ_LOCK_STATUS   = TRUE
>>>
>>> -  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
>>> +  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>>> +  INF MdeModulePkg/Core/Pei/PeiMain.inf
>>> +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>>> +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
>>> +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>> +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
>>> +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>>> +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>>
>>>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>>>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>>> @@ -264,14 +271,14 @@ READ_LOCK_STATUS   = TRUE
>>>
>>>  [Rule.Common.PEI_CORE]
>>>    FILE PEI_CORE = $(NAMED_GUID) {
>>> -    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
>>> +    TE     TE Align = Auto              $(INF_OUTPUT)/$(MODULE_NAME).efi
>>>      UI     STRING ="$(MODULE_NAME)" Optional
>>>    }
>>>
>>>  [Rule.Common.PEIM]
>>>    FILE PEIM = $(NAMED_GUID) {
>>>       PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
>>> -     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
>>> +     TE       TE Align = Auto           $(INF_OUTPUT)/$(MODULE_NAME).efi
>>>       UI       STRING="$(MODULE_NAME)" Optional
>>>    }
>>>
>>> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>> index 4129742..195b6b7 100644
>>> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>> @@ -145,13 +145,28 @@
>>>    MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>>>    HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
>>>    PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>>> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>>>    ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>>>
>>>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>>> -  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>>>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>> +  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>>> +
>>> +[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
>>> +  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>>> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>> +  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>>> +  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
>>> +  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>>> +  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
>>> +  PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>>> +
>>> +[LibraryClasses.common.PEI_CORE]
>>> +  PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
>>> +
>>> +[LibraryClasses.common.PEIM]
>>> +  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
>>> +  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>>>
>>>  [LibraryClasses.common.DXE_CORE]
>>>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>>> @@ -336,8 +351,13 @@
>>>    # ARM Pcds
>>>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0
>>>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
>>> +
>>> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
>>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>>>
>>> +  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
>>> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
>>> +
>>
>> These were copy/pasted from SynQuacer, I suppose?
>>
>
> SynQuacer and ARM VExpress use this value, I also checked ones used in
> HiSilicon Platforms - I didn't notice any issue with neither of them.
>

SynQuacer and ARM VExpress are both platforms designed by ARM Ltd, and
both have non-secure SRAM at this offset.

>> Ideally, these should point to some memory that is not exposed to the
>> OS, so that the PEI phase cannot corrupt a capsule image that has been
>> left in DRAM by the OS.
>>
>> This only becomes relevant once we implement support for PSCI warm
>> reboot, otherwise DRAM will be cleared anyway. However, pointing these
>> into a random slice of main memory feels a little risky.
>
>
> Currently we have also a hole between 0xc0000000 - 4GB for the config
> space, so how about squeezing those 64kB there?
>
> Alternatively would you recommend to use value outside the DRAM space
> (e.g. above max DRAM size)?
>

No, this region should point to usable memory, and ideally memory that
the OS does not use. So the options are (in order of preference)
- put the PEI stack and heap in non-secure SRAM
- put the PEI stack and heap in a slice of DRAM that is marked as
'reserved' in the EFI memory map.
- put the PEI stack and heap in a slice of DRAM that is otherwise
protected from allocation until DxeCore launches.

When the OS calls UpdateCapsule(), it will put the capsule at an
arbitrary location in memory, and if this happens to be the same
region PEI uses for its stack and heap, you will corrupt the capsule
before the capsule PEIM gets a chance to preserve it.

At the moment, this does not make any difference given that we don't
implement warm reboot yet (but PSCI defines it now). Also, fwupdate in
Linux does not rely on UpdateCapsule() at OS runtime. However, M$
Windows does, and I assume you don't want to make your UEFI
implementation Linux-only?


>>
>> Do you have non-secure SRAM on this SoC?
>
> There are a couple of SRAMs (for offload CM3 CPUs), one configured as
> secure. Not sure about others. Why do you ask?
>

Because that is the preferred place for the PEI stack and heap.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage
  2018-06-01 16:57       ` Ard Biesheuvel
@ 2018-06-01 17:18         ` Marcin Wojtas
  0 siblings, 0 replies; 15+ messages in thread
From: Marcin Wojtas @ 2018-06-01 17:18 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: edk2-devel@lists.01.org, Leif Lindholm, Jan Dąbroś,
	Hua Jing, Grzegorz Jaszczyk, David Sniatkiwicz

2018-06-01 18:57 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> On 1 June 2018 at 18:43, Marcin Wojtas <mw@semihalf.com> wrote:
>> 2018-06-01 17:30 GMT+02:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
>>> On 1 June 2018 at 16:32, Marcin Wojtas <mw@semihalf.com> wrote:
>>>> PEI phase will allow to use more robust platform initialization,
>>>> with new features like the capsule support. Wire up all
>>>> dependencies for that purpose.
>>>>
>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
>>>> ---
>>>>  Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf | 15 ++++++--
>>>>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc  | 40 ++++++++++++++++++--
>>>>  2 files changed, 48 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>>>> index 69cb4cd..bf04f4d 100644
>>>> --- a/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>>>> +++ b/Platform/Marvell/Armada70x0Db/Armada70x0Db.fdf
>>>> @@ -63,7 +63,7 @@ DATA = {
>>>>  !endif
>>>>  }
>>>>
>>>> -0x00001000|0x000ff000
>>>> +0x00001000|0x001ff000
>>>>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
>>>>  FV = FVMAIN_COMPACT
>>>>
>>>> @@ -221,7 +221,14 @@ READ_STATUS        = TRUE
>>>>  READ_LOCK_CAP      = TRUE
>>>>  READ_LOCK_STATUS   = TRUE
>>>>
>>>> -  INF ArmPlatformPkg/PrePi/PeiUniCore.inf
>>>> +  INF ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
>>>> +  INF MdeModulePkg/Core/Pei/PeiMain.inf
>>>> +  INF MdeModulePkg/Universal/PCD/Pei/Pcd.inf
>>>> +  INF ArmPlatformPkg/PlatformPei/PlatformPeim.inf
>>>> +  INF ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf
>>>> +  INF ArmPkg/Drivers/CpuPei/CpuPei.inf
>>>> +  INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>>>> +  INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>>>>
>>>>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>>>>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>>>> @@ -264,14 +271,14 @@ READ_LOCK_STATUS   = TRUE
>>>>
>>>>  [Rule.Common.PEI_CORE]
>>>>    FILE PEI_CORE = $(NAMED_GUID) {
>>>> -    TE     TE                           $(INF_OUTPUT)/$(MODULE_NAME).efi
>>>> +    TE     TE Align = Auto              $(INF_OUTPUT)/$(MODULE_NAME).efi
>>>>      UI     STRING ="$(MODULE_NAME)" Optional
>>>>    }
>>>>
>>>>  [Rule.Common.PEIM]
>>>>    FILE PEIM = $(NAMED_GUID) {
>>>>       PEI_DEPEX PEI_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
>>>> -     TE       TE                        $(INF_OUTPUT)/$(MODULE_NAME).efi
>>>> +     TE       TE Align = Auto           $(INF_OUTPUT)/$(MODULE_NAME).efi
>>>>       UI       STRING="$(MODULE_NAME)" Optional
>>>>    }
>>>>
>>>> diff --git a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>>> index 4129742..195b6b7 100644
>>>> --- a/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>>> +++ b/Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>>>> @@ -145,13 +145,28 @@
>>>>    MemoryAllocationLib|EmbeddedPkg/Library/PrePiMemoryAllocationLib/PrePiMemoryAllocationLib.inf
>>>>    HobLib|EmbeddedPkg/Library/PrePiHobLib/PrePiHobLib.inf
>>>>    PrePiHobListPointerLib|ArmPlatformPkg/Library/PrePiHobListPointerLib/PrePiHobListPointerLib.inf
>>>> -  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>>>>    ArmGicArchLib|ArmPkg/Library/ArmGicArchSecLib/ArmGicArchSecLib.inf
>>>>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>>>>
>>>>  [LibraryClasses.common.SEC, LibraryClasses.common.PEIM]
>>>> -  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>>>>    BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>>> +  PlatformPeiLib|ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
>>>> +
>>>> +[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
>>>> +  PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>>>> +  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>>> +  HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>>>> +  PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
>>>> +  MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
>>>> +  ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
>>>> +  PeiServicesTablePointerLib|ArmPkg/Library/PeiServicesTablePointerLib/PeiServicesTablePointerLib.inf
>>>> +
>>>> +[LibraryClasses.common.PEI_CORE]
>>>> +  PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
>>>> +
>>>> +[LibraryClasses.common.PEIM]
>>>> +  PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
>>>> +  MemoryInitPeiLib|Silicon/Marvell/Armada7k8k/Library/Armada7k8kMemoryInitPeiLib/Armada7k8kMemoryInitPeiLib.inf
>>>>
>>>>  [LibraryClasses.common.DXE_CORE]
>>>>    HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
>>>> @@ -336,8 +351,13 @@
>>>>    # ARM Pcds
>>>>    gArmTokenSpaceGuid.PcdSystemMemoryBase|0
>>>>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x40000000
>>>> +
>>>> +  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|36
>>>>    gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
>>>>
>>>> +  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
>>>> +  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
>>>> +
>>>
>>> These were copy/pasted from SynQuacer, I suppose?
>>>
>>
>> SynQuacer and ARM VExpress use this value, I also checked ones used in
>> HiSilicon Platforms - I didn't notice any issue with neither of them.
>>
>
> SynQuacer and ARM VExpress are both platforms designed by ARM Ltd, and
> both have non-secure SRAM at this offset.
>
>>> Ideally, these should point to some memory that is not exposed to the
>>> OS, so that the PEI phase cannot corrupt a capsule image that has been
>>> left in DRAM by the OS.
>>>
>>> This only becomes relevant once we implement support for PSCI warm
>>> reboot, otherwise DRAM will be cleared anyway. However, pointing these
>>> into a random slice of main memory feels a little risky.
>>
>>
>> Currently we have also a hole between 0xc0000000 - 4GB for the config
>> space, so how about squeezing those 64kB there?
>>
>> Alternatively would you recommend to use value outside the DRAM space
>> (e.g. above max DRAM size)?
>>
>
> No, this region should point to usable memory, and ideally memory that
> the OS does not use. So the options are (in order of preference)
> - put the PEI stack and heap in non-secure SRAM
> - put the PEI stack and heap in a slice of DRAM that is marked as
> 'reserved' in the EFI memory map.
> - put the PEI stack and heap in a slice of DRAM that is otherwise
> protected from allocation until DxeCore launches.
>
> When the OS calls UpdateCapsule(), it will put the capsule at an
> arbitrary location in memory, and if this happens to be the same
> region PEI uses for its stack and heap, you will corrupt the capsule
> before the capsule PEIM gets a chance to preserve it.
>
> At the moment, this does not make any difference given that we don't
> implement warm reboot yet (but PSCI defines it now). Also, fwupdate in
> Linux does not rely on UpdateCapsule() at OS runtime. However, M$
> Windows does, and I assume you don't want to make your UEFI
> implementation Linux-only?
>

Thanks for explanation. Not sure if I will be able to use the SRAM,
otherwise I will extend the hole in DRAM, which is reserved for the
PSCI and use it as the PEI stack and heap.

>
>>>
>>> Do you have non-secure SRAM on this SoC?
>>
>> There are a couple of SRAMs (for offload CM3 CPUs), one configured as
>> secure. Not sure about others. Why do you ask?
>>
>
> Because that is the preferred place for the PEI stack and heap.


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2018-06-01 17:18 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-01 14:32 [platforms PATCH 0/4] Armada capsule support Marcin Wojtas
2018-06-01 14:32 ` [platforms PATCH 1/4] Marvell/Armada70x0Db: Shift main FV from 0x0 address Marcin Wojtas
2018-06-01 15:26   ` Ard Biesheuvel
2018-06-01 14:32 ` [platforms PATCH 2/4] Marvell/Aramda7k8k: Enable PEI booting stage Marcin Wojtas
2018-06-01 15:30   ` Ard Biesheuvel
2018-06-01 16:43     ` Marcin Wojtas
2018-06-01 16:57       ` Ard Biesheuvel
2018-06-01 17:18         ` Marcin Wojtas
2018-06-01 14:32 ` [platforms PATCH 3/4] Marvell/Armada7k8k: Introduce capsule FW update implementation Marcin Wojtas
2018-06-01 15:32   ` Ard Biesheuvel
2018-06-01 16:02     ` Marcin Wojtas
2018-06-01 16:08       ` Ard Biesheuvel
2018-06-01 16:20         ` Marcin Wojtas
2018-06-01 14:32 ` [platforms PATCH 4/4] Marvell/Armada7k8k: Wire up capsule support Marcin Wojtas
2018-06-01 15:34 ` [platforms PATCH 0/4] Armada " Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox