public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot
@ 2019-01-04 14:43 Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines Ard Biesheuvel
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

Wire up the various pieces so that the authenticated variable store
runs entirely in standalone MM context residing in a secure partition.

This primarily involves refactoring the platform's NOR flash driver so
we can build a version that can work in the standalone MM context.
Beyond that, it is just a matter of enabling all the boilerplate in
the .DSC and .FDF files.

Note that the resulting standalone MM firmware volume needs to be
wrapped in a FIP, which is not part of the build sequence.

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Masahisa Kojima <masahisa.kojima@linaro.org>

Ard Biesheuvel (7):
  Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
  Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant
  Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access
  Platform/DeveloperBox: create shared .DSC include file
  Platform/DeveloperBox: add .DSC/.FDF description of MM components
  Platform/DeveloperBox: add MM based UEFI secure boot support

 .../Socionext/DeveloperBox/DeveloperBox.dsc   |  304 +---
 .../DeveloperBox/DeveloperBox.dsc.inc         |  315 ++++
 .../Socionext/DeveloperBox/DeveloperBox.fdf   |   13 +
 .../Socionext/DeveloperBox/DeveloperBoxMm.dsc |  103 ++
 .../Socionext/DeveloperBox/DeveloperBoxMm.fdf |  161 ++
 .../SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf |    9 +-
 .../Drivers/Fip006Dxe/Fip006StandaloneMm.inf  |   71 +
 .../SynQuacer/Drivers/Fip006Dxe/NorFlash.c    | 1006 +++++++++++++
 .../Fip006Dxe/{NorFlashDxe.h => NorFlash.h}   |   93 +-
 .../Drivers/Fip006Dxe/NorFlashBlockIoDxe.c    |  138 --
 .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 1341 ++---------------
 .../{NorFlashFvbDxe.c => NorFlashFvb.c}       |  197 +--
 .../SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c |  182 +++
 13 files changed, 2076 insertions(+), 1857 deletions(-)
 create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
 create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
 create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
 create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
 create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
 rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h} (85%)
 delete mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
 rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} (76%)
 create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c

-- 
2.17.1



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

* [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-17  9:50   ` Leif Lindholm
  2019-01-04 14:43 ` [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces Ard Biesheuvel
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

The FIP006 NOR flash driver contains implementations of the block I/O
and disk I/O protocols, but never exposes them to other drivers (i.e.,
it never installs the protocol interfaces). So let's drop this code
altogether: the NOR flash is for code and variables, not for arbitrary
files.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf        |   3 -
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c | 138 ------------
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c        | 229 ++------------------
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h        |  51 +----
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c     |  40 ++--
 5 files changed, 37 insertions(+), 424 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
index bddb052c2dcc..b939aa689eef 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
@@ -24,7 +24,6 @@ [Defines]
   ENTRY_POINT                    = NorFlashInitialise
 
 [Sources]
-  NorFlashBlockIoDxe.c
   NorFlashDxe.c
   NorFlashFvbDxe.c
 
@@ -60,9 +59,7 @@ [Guids]
   gEfiVariableGuid
 
 [Protocols]
-  gEfiBlockIoProtocolGuid
   gEfiDevicePathProtocolGuid
-  gEfiDiskIoProtocolGuid
   gEfiFirmwareVolumeBlockProtocolGuid
 
 [FixedPcd]
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
deleted file mode 100644
index b41f5003217c..000000000000
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
+++ /dev/null
@@ -1,138 +0,0 @@
-/** @file  NorFlashBlockIoDxe.c
-
-  Copyright (c) 2011-2013, ARM Ltd. All rights reserved.<BR>
-  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
-  Copyright (c) 2017, Linaro, 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.
-
-**/
-
-#include <Library/BaseMemoryLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-
-#include "NorFlashDxe.h"
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.Reset
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoReset (
-  IN EFI_BLOCK_IO_PROTOCOL  *This,
-  IN BOOLEAN                ExtendedVerification
-  )
-{
-  NOR_FLASH_INSTANCE *Instance;
-
-  Instance = INSTANCE_FROM_BLKIO_THIS(This);
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReset(MediaId=0x%x)\n",
-    This->Media->MediaId));
-
-  return NorFlashReset (Instance);
-}
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.ReadBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoReadBlocks (
-  IN  EFI_BLOCK_IO_PROTOCOL   *This,
-  IN  UINT32                  MediaId,
-  IN  EFI_LBA                 Lba,
-  IN  UINTN                   BufferSizeInBytes,
-  OUT VOID                    *Buffer
-  )
-{
-  NOR_FLASH_INSTANCE  *Instance;
-  EFI_STATUS          Status;
-  EFI_BLOCK_IO_MEDIA  *Media;
-
-  if (This == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  Instance = INSTANCE_FROM_BLKIO_THIS(This);
-  Media = This->Media;
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes "
-    "(%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
-
-  if (!Media) {
-    Status = EFI_INVALID_PARAMETER;
-  } else if (!Media->MediaPresent) {
-    Status = EFI_NO_MEDIA;
-  } else if (Media->MediaId != MediaId) {
-    Status = EFI_MEDIA_CHANGED;
-  } else if ((Media->IoAlign > 2) &&
-             (((UINTN)Buffer & (Media->IoAlign - 1)) != 0)) {
-    Status = EFI_INVALID_PARAMETER;
-  } else {
-    Status = NorFlashReadBlocks (Instance, Lba, BufferSizeInBytes, Buffer);
-  }
-
-  return Status;
-}
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.WriteBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoWriteBlocks (
-  IN  EFI_BLOCK_IO_PROTOCOL   *This,
-  IN  UINT32                  MediaId,
-  IN  EFI_LBA                 Lba,
-  IN  UINTN                   BufferSizeInBytes,
-  IN  VOID                    *Buffer
-  )
-{
-  NOR_FLASH_INSTANCE  *Instance;
-  EFI_STATUS          Status;
-
-  Instance = INSTANCE_FROM_BLKIO_THIS(This);
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes "
-    "(%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, Buffer));
-
-  if( !This->Media->MediaPresent ) {
-    Status = EFI_NO_MEDIA;
-  } else if( This->Media->MediaId != MediaId ) {
-    Status = EFI_MEDIA_CHANGED;
-  } else if( This->Media->ReadOnly ) {
-    Status = EFI_WRITE_PROTECTED;
-  } else {
-    Status = NorFlashWriteBlocks (Instance,Lba,BufferSizeInBytes,Buffer);
-  }
-
-  return Status;
-}
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.FlushBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoFlushBlocks (
-  IN EFI_BLOCK_IO_PROTOCOL  *This
-  )
-{
-  // No Flush required for the NOR Flash driver
-  // because cache operations are not permitted.
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashBlockIoFlushBlocks: Function NOT IMPLEMENTED (not required).\n"));
-
-  // Nothing to do so just return without error
-  return EFI_SUCCESS;
-}
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
index 98f3e4d5a012..e52ab52d8cf7 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
@@ -338,7 +338,7 @@ NorFlashEraseSingleBlock (
   //
   if (EfiAtRuntime()) {
     BlockAddress -= Instance->RegionBaseAddress;
-    BlockAddress += Instance->OffsetLba * Instance->Media.BlockSize;
+    BlockAddress += Instance->OffsetLba * Instance->BlockSize;
   }
 
   NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
@@ -520,10 +520,6 @@ NorFlashWriteBlocks (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (Instance->Media.ReadOnly == TRUE) {
-    return EFI_WRITE_PROTECTED;
-  }
-
   // We must have some bytes to read
   DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: BufferSizeInBytes=0x%x\n",
     BufferSizeInBytes));
@@ -533,19 +529,19 @@ NorFlashWriteBlocks (
 
   // The size of the buffer must be a multiple of the block size
   DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: BlockSize in bytes =0x%x\n",
-    Instance->Media.BlockSize));
-  if ((BufferSizeInBytes % Instance->Media.BlockSize) != 0) {
+    Instance->BlockSize));
+  if ((BufferSizeInBytes % Instance->BlockSize) != 0) {
     return EFI_BAD_BUFFER_SIZE;
   }
 
   // All blocks must be within the device
-  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->Media.BlockSize ;
+  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->BlockSize ;
 
   DEBUG ((DEBUG_BLKIO,
     "NorFlashWriteBlocks: NumBlocks=%d, LastBlock=%ld, Lba=%ld.\n", NumBlocks,
-    Instance->Media.LastBlock, Lba));
+    Instance->LastBlock, Lba));
 
-  if ((Lba + NumBlocks) > (Instance->Media.LastBlock + 1)) {
+  if ((Lba + NumBlocks) > (Instance->LastBlock + 1)) {
     DEBUG ((DEBUG_ERROR,
       "NorFlashWriteBlocks: ERROR - Write will exceed last block.\n"));
     return EFI_INVALID_PARAMETER;
@@ -553,7 +549,7 @@ NorFlashWriteBlocks (
 
   ASSERT (((UINTN)Buffer % sizeof (UINT32)) == 0);
 
-  BlockSizeInWords = Instance->Media.BlockSize / 4;
+  BlockSizeInWords = Instance->BlockSize / 4;
 
   // Because the target *Buffer is a pointer to VOID, we must put
   // all the data into a pointer to a proper data type, so use *ReadBuffer
@@ -592,7 +588,7 @@ NorFlashReadBlocks (
 
   DEBUG ((DEBUG_BLKIO,
     "NorFlashReadBlocks: BufferSize=0x%xB BlockSize=0x%xB LastBlock=%ld, Lba=%ld.\n",
-    BufferSizeInBytes, Instance->Media.BlockSize, Instance->Media.LastBlock,
+    BufferSizeInBytes, Instance->BlockSize, Instance->LastBlock,
     Lba));
 
   // The buffer must be valid
@@ -606,14 +602,14 @@ NorFlashReadBlocks (
   }
 
   // The size of the buffer must be a multiple of the block size
-  if ((BufferSizeInBytes % Instance->Media.BlockSize) != 0) {
+  if ((BufferSizeInBytes % Instance->BlockSize) != 0) {
     return EFI_BAD_BUFFER_SIZE;
   }
 
   // All blocks must be within the device
-  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->Media.BlockSize ;
+  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->BlockSize ;
 
-  if ((Lba + NumBlocks) > (Instance->Media.LastBlock + 1)) {
+  if ((Lba + NumBlocks) > (Instance->LastBlock + 1)) {
     DEBUG ((DEBUG_ERROR,
       "NorFlashReadBlocks: ERROR - Read will exceed last block\n"));
     return EFI_INVALID_PARAMETER;
@@ -621,7 +617,7 @@ NorFlashReadBlocks (
 
   // Get the address to start reading from
   StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-                                        Instance->Media.BlockSize);
+                                        Instance->BlockSize);
 
   // Put the device into Read Array mode
   NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
@@ -654,7 +650,7 @@ NorFlashRead (
     return EFI_SUCCESS;
   }
 
-  if (((Lba * Instance->Media.BlockSize) + Offset + BufferSizeInBytes) >
+  if (((Lba * Instance->BlockSize) + Offset + BufferSizeInBytes) >
       Instance->Size) {
     DEBUG ((DEBUG_ERROR,
       "NorFlashRead: ERROR - Read will exceed device size.\n"));
@@ -663,7 +659,7 @@ NorFlashRead (
 
   // Get the address to start reading from
   StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-                                        Instance->Media.BlockSize);
+                                        Instance->BlockSize);
 
   // Put the device into Read Array mode
   NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
@@ -677,7 +673,7 @@ NorFlashRead (
 
 /*
   Write a full or portion of a block. It must not span block boundaries;
-  that is, Offset + *NumBytes <= Instance->Media.BlockSize.
+  that is, Offset + *NumBytes <= Instance->BlockSize.
 */
 EFI_STATUS
 NorFlashWriteSingleBlock (
@@ -711,16 +707,8 @@ NorFlashWriteSingleBlock (
     "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n",
     Lba, Offset, *NumBytes, Buffer));
 
-  // Detect WriteDisabled state
-  if (Instance->Media.ReadOnly == TRUE) {
-    DEBUG ((DEBUG_ERROR,
-      "NorFlashWriteSingleBlock: ERROR - Can not write: Device is in WriteDisabled state.\n"));
-    // It is in WriteDisabled state, return an error right away
-    return EFI_ACCESS_DENIED;
-  }
-
   // Cache the block size to avoid de-referencing pointers all the time
-  BlockSize = Instance->Media.BlockSize;
+  BlockSize = Instance->BlockSize;
 
   // The write must not span block boundaries.
   // We need to check each variable individually because adding two large
@@ -896,143 +884,6 @@ NorFlashWriteSingleBlock (
   return EFI_SUCCESS;
 }
 
-/*
-  Although DiskIoDxe will automatically install the DiskIO protocol whenever
-  we install the BlockIO protocol, its implementation is sub-optimal as it reads
-  and writes entire blocks using the BlockIO protocol. In fact we can access
-  NOR flash with a finer granularity than that, so we can improve performance
-  by directly producing the DiskIO protocol.
-*/
-
-/**
-  Read BufferSize bytes from Offset into Buffer.
-
-  @param  This                  Protocol instance pointer.
-  @param  MediaId               Id of the media, changes every time the media is
-                                replaced.
-  @param  Offset                The starting byte offset to read from
-  @param  BufferSize            Size of Buffer
-  @param  Buffer                Buffer containing read data
-
-  @retval EFI_SUCCESS           The data was read correctly from the device.
-  @retval EFI_DEVICE_ERROR      The device reported an error while performing
-                                the read.
-  @retval EFI_NO_MEDIA          There is no media in the device.
-  @retval EFI_MEDIA_CHNAGED     The MediaId does not matched the current device.
-  @retval EFI_INVALID_PARAMETER The read request contains device addresses that
-                                are not valid for the device.
-
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-NorFlashDiskIoReadDisk (
-  IN EFI_DISK_IO_PROTOCOL         *This,
-  IN UINT32                       MediaId,
-  IN UINT64                       DiskOffset,
-  IN UINTN                        BufferSize,
-  OUT VOID                        *Buffer
-  )
-{
-  NOR_FLASH_INSTANCE *Instance;
-  UINT32              BlockSize;
-  UINT32              BlockOffset;
-  EFI_LBA             Lba;
-
-  Instance = INSTANCE_FROM_DISKIO_THIS(This);
-
-  if (MediaId != Instance->Media.MediaId) {
-    return EFI_MEDIA_CHANGED;
-  }
-
-  BlockSize = Instance->Media.BlockSize;
-  Lba = (EFI_LBA) DivU64x32Remainder (DiskOffset, BlockSize, &BlockOffset);
-
-  return NorFlashRead (Instance, Lba, BlockOffset, BufferSize, Buffer);
-}
-
-/**
-  Writes a specified number of bytes to a device.
-
-  @param  This       Indicates a pointer to the calling context.
-  @param  MediaId    ID of the medium to be written.
-  @param  Offset     The starting byte offset on the logical block I/O device to
-                     write.
-  @param  BufferSize The size in bytes of Buffer. The number of bytes to write
-                     to the device.
-  @param  Buffer     A pointer to the buffer containing the data to be written.
-
-  @retval EFI_SUCCESS           The data was written correctly to the device.
-  @retval EFI_WRITE_PROTECTED   The device can not be written to.
-  @retval EFI_DEVICE_ERROR      The device reported an error while performing
-                                the write.
-  @retval EFI_NO_MEDIA          There is no media in the device.
-  @retval EFI_MEDIA_CHNAGED     The MediaId does not matched the current device.
-  @retval EFI_INVALID_PARAMETER The write request contains device addresses that
-                                are not valid for the device.
-
-**/
-STATIC
-EFI_STATUS
-EFIAPI
-NorFlashDiskIoWriteDisk (
-  IN EFI_DISK_IO_PROTOCOL         *This,
-  IN UINT32                       MediaId,
-  IN UINT64                       DiskOffset,
-  IN UINTN                        BufferSize,
-  IN VOID                         *Buffer
-  )
-{
-  NOR_FLASH_INSTANCE *Instance;
-  UINT32              BlockSize;
-  UINT32              BlockOffset;
-  EFI_LBA             Lba;
-  UINTN               RemainingBytes;
-  UINTN               WriteSize;
-  EFI_STATUS          Status;
-
-  Instance = INSTANCE_FROM_DISKIO_THIS(This);
-
-  if (MediaId != Instance->Media.MediaId) {
-    return EFI_MEDIA_CHANGED;
-  }
-
-  BlockSize = Instance->Media.BlockSize;
-  Lba = (EFI_LBA) DivU64x32Remainder (DiskOffset, BlockSize, &BlockOffset);
-
-  RemainingBytes = BufferSize;
-
-  // Write either all the remaining bytes, or the number of bytes that bring
-  // us up to a block boundary, whichever is less.
-  // (DiskOffset | (BlockSize - 1)) + 1) rounds DiskOffset up to the next
-  // block boundary (even if it is already on one).
-  WriteSize = MIN (RemainingBytes,
-                   ((DiskOffset | (BlockSize - 1)) + 1) - DiskOffset);
-
-  do {
-    if (WriteSize == BlockSize) {
-      // Write a full block
-      Status = NorFlashWriteFullBlock (Instance, Lba, Buffer,
-                 BlockSize / sizeof (UINT32));
-    } else {
-      // Write a partial block
-      Status = NorFlashWriteSingleBlock (Instance, Lba, BlockOffset, &WriteSize,
-                 Buffer);
-    }
-    if (EFI_ERROR (Status)) {
-      return Status;
-    }
-    // Now continue writing either all the remaining bytes or single blocks.
-    RemainingBytes -= WriteSize;
-    Buffer = (UINT8 *) Buffer + WriteSize;
-    Lba++;
-    BlockOffset = 0;
-    WriteSize = MIN (RemainingBytes, BlockSize);
-  } while (RemainingBytes);
-
-  return Status;
-}
-
 STATIC CONST NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = {
   NOR_FLASH_SIGNATURE, // Signature
   NULL, // Handle ... NEED TO BE FILLED
@@ -1044,37 +895,11 @@ STATIC CONST NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = {
   0, // DeviceBaseAddress ... NEED TO BE FILLED
   0, // RegionBaseAddress ... NEED TO BE FILLED
   0, // Size ... NEED TO BE FILLED
+  0, // BlockSize
+  0, // LastBlock
   0, // StartLba
   0, // OffsetLba
 
-  {
-    EFI_BLOCK_IO_PROTOCOL_REVISION2,  // Revision
-    NULL,                             // Media ... NEED TO BE FILLED
-    NorFlashBlockIoReset,             // Reset;
-    NorFlashBlockIoReadBlocks,        // ReadBlocks
-    NorFlashBlockIoWriteBlocks,       // WriteBlocks
-    NorFlashBlockIoFlushBlocks        // FlushBlocks
-  }, // BlockIoProtocol
-
-  {
-    0, // MediaId ... NEED TO BE FILLED
-    FALSE, // RemovableMedia
-    TRUE, // MediaPresent
-    FALSE, // LogicalPartition
-    FALSE, // ReadOnly
-    FALSE, // WriteCaching;
-    0, // BlockSize ... NEED TO BE FILLED
-    4, //  IoAlign
-    0, // LastBlock ... NEED TO BE FILLED
-    0, // LowestAlignedLba
-    1, // LogicalBlocksPerPhysicalBlock
-  }, //Media;
-
-  {
-    EFI_DISK_IO_PROTOCOL_REVISION, // Revision
-    NorFlashDiskIoReadDisk,        // ReadDisk
-    NorFlashDiskIoWriteDisk        // WriteDisk
-  },
   {
     FvbGetAttributes, // GetAttributes
     FvbSetAttributes, // SetAttributes
@@ -1143,12 +968,10 @@ NorFlashCreateInstance (
   Instance->HostRegisterBaseAddress = HostRegisterBase;
   Instance->DeviceBaseAddress       = NorFlashDeviceBase;
   Instance->RegionBaseAddress       = NorFlashRegionBase;
-  Instance->Size = NorFlashSize;
+  Instance->Size                    = NorFlashSize;
+  Instance->BlockSize               = BlockSize;
+  Instance->LastBlock               = (NorFlashSize / BlockSize) - 1;
 
-  Instance->BlockIoProtocol.Media = &Instance->Media;
-  Instance->Media.MediaId = Index;
-  Instance->Media.BlockSize = BlockSize;
-  Instance->Media.LastBlock = (NorFlashSize / BlockSize) - 1;
   Instance->OffsetLba = (NorFlashRegionBase - NorFlashDeviceBase) / BlockSize;
 
   CopyGuid (&Instance->DevicePath.Vendor.Guid, &gEfiCallerIdGuid);
@@ -1263,16 +1086,6 @@ NorFlashVirtualNotifyEvent (
     EfiConvertPointer (0x0,
       (VOID**)&mNorFlashInstances[Index]->RegionBaseAddress);
 
-    // Convert BlockIo protocol
-    EfiConvertPointer (0x0,
-      (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.FlushBlocks);
-    EfiConvertPointer (0x0,
-      (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.ReadBlocks);
-    EfiConvertPointer (0x0,
-      (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.Reset);
-    EfiConvertPointer (0x0,
-      (VOID**)&mNorFlashInstances[Index]->BlockIoProtocol.WriteBlocks);
-
     // Convert Fvb
     EfiConvertPointer (0x0,
       (VOID**)&mNorFlashInstances[Index]->FvbProtocol.EraseBlocks);
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
index ef1257a64904..20e74b0320ce 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
@@ -102,13 +102,11 @@ struct _NOR_FLASH_INSTANCE {
   UINTN                               DeviceBaseAddress;
   UINTN                               RegionBaseAddress;
   UINTN                               Size;
+  UINTN                               BlockSize;
+  UINTN                               LastBlock;
   EFI_LBA                             StartLba;
   EFI_LBA                             OffsetLba;
 
-  EFI_BLOCK_IO_PROTOCOL               BlockIoProtocol;
-  EFI_BLOCK_IO_MEDIA                  Media;
-  EFI_DISK_IO_PROTOCOL                DiskIoProtocol;
-
   EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol;
   VOID*                               ShadowBuffer;
 
@@ -137,51 +135,6 @@ NorFlashWriteBuffer (
   IN UINT32                 *Buffer
   );
 
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.Reset
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoReset (
-  IN EFI_BLOCK_IO_PROTOCOL    *This,
-  IN BOOLEAN                  ExtendedVerification
-  );
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.ReadBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoReadBlocks (
-  IN  EFI_BLOCK_IO_PROTOCOL   *This,
-  IN  UINT32                  MediaId,
-  IN  EFI_LBA                 Lba,
-  IN  UINTN                   BufferSizeInBytes,
-  OUT VOID                    *Buffer
-);
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.WriteBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoWriteBlocks (
-  IN  EFI_BLOCK_IO_PROTOCOL   *This,
-  IN  UINT32                  MediaId,
-  IN  EFI_LBA                 Lba,
-  IN  UINTN                   BufferSizeInBytes,
-  IN  VOID                    *Buffer
-);
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.FlushBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoFlushBlocks (
-  IN EFI_BLOCK_IO_PROTOCOL    *This
-);
-
 //
 // NorFlashFvbDxe.c
 //
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c
index ca3b1b5c34f8..776ec8a5437c 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c
@@ -91,8 +91,8 @@ InitializeFvAndVariableStoreHeaders (
   FirmwareVolumeHeader->HeaderLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) +
                                        sizeof(EFI_FV_BLOCK_MAP_ENTRY);
   FirmwareVolumeHeader->Revision = EFI_FVH_REVISION;
-  FirmwareVolumeHeader->BlockMap[0].NumBlocks = Instance->Media.LastBlock + 1;
-  FirmwareVolumeHeader->BlockMap[0].Length      = Instance->Media.BlockSize;
+  FirmwareVolumeHeader->BlockMap[0].NumBlocks = Instance->LastBlock + 1;
+  FirmwareVolumeHeader->BlockMap[0].Length      = Instance->BlockSize;
   FirmwareVolumeHeader->BlockMap[1].NumBlocks = 0;
   FirmwareVolumeHeader->BlockMap[1].Length      = 0;
   FirmwareVolumeHeader->Checksum = CalculateCheckSum16 (
@@ -223,14 +223,10 @@ FvbGetAttributes(
   Instance = INSTANCE_FROM_FVB_THIS(This);
 
   FlashFvbAttributes = EFI_FVB2_READ_ENABLED_CAP | EFI_FVB2_READ_STATUS |
+                       EFI_FVB2_WRITE_ENABLED_CAP | EFI_FVB2_WRITE_STATUS |
                        EFI_FVB2_STICKY_WRITE | EFI_FVB2_MEMORY_MAPPED |
                        EFI_FVB2_ERASE_POLARITY;
 
-  // Check if it is write protected
-  if (!Instance->Media.ReadOnly) {
-    FlashFvbAttributes |= EFI_FVB2_WRITE_STATUS | EFI_FVB2_WRITE_ENABLED_CAP;
-  }
-
   *Attributes = FlashFvbAttributes;
 
   DEBUG ((DEBUG_BLKIO, "FvbGetAttributes(0x%X)\n", *Attributes));
@@ -349,17 +345,17 @@ FvbGetBlockSize (
 
   DEBUG ((DEBUG_BLKIO,
     "FvbGetBlockSize(Lba=%ld, BlockSize=0x%x, LastBlock=%ld)\n", Lba,
-    Instance->Media.BlockSize, Instance->Media.LastBlock));
+    Instance->BlockSize, Instance->LastBlock));
 
-  if (Lba > Instance->Media.LastBlock) {
+  if (Lba > Instance->LastBlock) {
     DEBUG ((DEBUG_ERROR,
       "FvbGetBlockSize: ERROR - Parameter LBA %ld is beyond the last Lba (%ld).\n",
-      Lba, Instance->Media.LastBlock));
+      Lba, Instance->LastBlock));
     Status = EFI_INVALID_PARAMETER;
   } else {
     // This is easy because in this platform each NorFlash device has equal sized blocks.
-    *BlockSize = (UINTN) Instance->Media.BlockSize;
-    *NumberOfBlocks = (UINTN) (Instance->Media.LastBlock - Lba + 1);
+    *BlockSize = (UINTN) Instance->BlockSize;
+    *NumberOfBlocks = (UINTN) (Instance->LastBlock - Lba + 1);
 
     DEBUG ((DEBUG_BLKIO,
       "FvbGetBlockSize: *BlockSize=0x%x, *NumberOfBlocks=0x%x.\n", *BlockSize,
@@ -442,7 +438,7 @@ FvbRead (
   TempStatus = EFI_SUCCESS;
 
   // Cache the block size to avoid de-referencing pointers all the time
-  BlockSize = Instance->Media.BlockSize;
+  BlockSize = Instance->BlockSize;
 
   DEBUG ((DEBUG_BLKIO,
     "FvbRead: Check if (Offset=0x%x + NumBytes=0x%x) <= BlockSize=0x%x\n",
@@ -626,14 +622,6 @@ FvbEraseBlocks (
 
   Status = EFI_SUCCESS;
 
-  // Detect WriteDisabled state
-  if (Instance->Media.ReadOnly) {
-    // Firmware volume is in WriteDisabled state
-    DEBUG ((DEBUG_ERROR,
-      "FvbEraseBlocks: ERROR - Device is in WriteDisabled state.\n"));
-    return EFI_ACCESS_DENIED;
-  }
-
   // Before erasing, check the entire list of parameters to ensure
   // all specified blocks are valid
 
@@ -654,10 +642,10 @@ FvbEraseBlocks (
     // All blocks must be within range
     DEBUG ((DEBUG_BLKIO,
       "FvbEraseBlocks: Check if: ( StartingLba=%ld + NumOfLba=%d - 1 ) > LastBlock=%ld.\n",
-      Instance->StartLba + StartingLba, NumOfLba, Instance->Media.LastBlock));
+      Instance->StartLba + StartingLba, NumOfLba, Instance->LastBlock));
     if (NumOfLba == 0 ||
         (Instance->StartLba + StartingLba + NumOfLba - 1) >
-        Instance->Media.LastBlock) {
+        Instance->LastBlock) {
       VA_END (Args);
       DEBUG ((DEBUG_ERROR,
         "FvbEraseBlocks: ERROR - Lba range goes past the last Lba.\n"));
@@ -690,7 +678,7 @@ FvbEraseBlocks (
       // Get the physical address of Lba to erase
       BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress,
                        Instance->StartLba + StartingLba,
-                       Instance->Media.BlockSize);
+                       Instance->BlockSize);
 
       // Erase it
       DEBUG ((DEBUG_BLKIO, "FvbEraseBlocks: Erasing Lba=%ld @ 0x%08x.\n",
@@ -747,7 +735,7 @@ NorFlashFvbInitialize (
 
   DEBUG ((DEBUG_BLKIO,"NorFlashFvbInitialize\n"));
 
-  BlockSize = Instance->Media.BlockSize;
+  BlockSize = Instance->BlockSize;
 
   // FirmwareVolumeHeader->FvLength is declared to have the Variable area
   // AND the FTW working area AND the FTW Spare contiguous.
@@ -798,7 +786,7 @@ NorFlashFvbInitialize (
     FvbNumLba = (PcdGet32(PcdFlashNvStorageVariableSize) +
                  PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
                  PcdGet32(PcdFlashNvStorageFtwSpareSize)) /
-                Instance->Media.BlockSize;
+                Instance->BlockSize;
 
     Status = FvbEraseBlocks (&Instance->FvbProtocol, (EFI_LBA)0, FvbNumLba,
                EFI_LBA_LIST_TERMINATOR);
-- 
2.17.1



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

* [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-17 10:10   ` Leif Lindholm
  2019-01-04 14:43 ` [PATCH edk2-platforms 3/7] Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant Ard Biesheuvel
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

In preparation of creating a SMM version of the FIP006 NOR flash
driver, refactor the existing pieces into a core driver, the FVB
methods and the DXE instantiation code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf                       |    6 +-
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c                          | 1006 +++++++++++++++++
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h}       |   52 +-
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c                       | 1150 +++-----------------
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} |  161 +--
 5 files changed, 1194 insertions(+), 1181 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
index b939aa689eef..603641e0a68f 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
@@ -2,7 +2,7 @@
 #  Socionext FIP006 High-Speed SPI Controller with NOR Flash Driver
 #
 #  Copyright (c) 2017, Socionext Inc. All rights reserved.<BR>
-#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
+#  Copyright (c) 2017-2018, Linaro, 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
@@ -25,7 +25,9 @@ [Defines]
 
 [Sources]
   NorFlashDxe.c
-  NorFlashFvbDxe.c
+  NorFlash.c
+  NorFlash.h
+  NorFlashFvb.c
 
 [Packages]
   ArmPlatformPkg/ArmPlatformPkg.dec
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
new file mode 100644
index 000000000000..2134739bfba9
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
@@ -0,0 +1,1006 @@
+/** @file  NorFlashDxe.c
+
+  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2017, Socionext Inc. All rights reserved.<BR>
+  Copyright (c) 2017, Linaro, 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.
+
+**/
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/NorFlashInfoLib.h>
+#include <Library/PcdLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+
+#include "NorFlash.h"
+
+STATIC CONST UINT16 mFip006NullCmdSeq[] = {
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+};
+
+STATIC CONST CSDC_DEFINITION mN25qCSDCDefTable[] = {
+  // Identification Operations
+  { SPINOR_OP_RDID,     FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  // Register Operations
+  { SPINOR_OP_RDSR,     FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_WRSR,     FALSE, FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_ARRAY, TRUE,  FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_RDFSR,    FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_NVCFG, FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_VCR,   FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_EVCR,  FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  // Read Operations
+  { SPINOR_OP_READ_4B,  TRUE,  TRUE,  FALSE, FALSE, CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  // Write Operations
+  { SPINOR_OP_PP,       TRUE,  FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+  { SPINOR_OP_PP_1_1_4, TRUE,  FALSE, FALSE, TRUE,  CS_CFG_MBM_QUAD,
+                        CSDC_TRP_SINGLE },
+  // Erase Operations
+  { SPINOR_OP_SE,       FALSE, FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
+                        CSDC_TRP_SINGLE },
+};
+
+STATIC CONST NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = {
+  NOR_FLASH_SIGNATURE, // Signature
+  NULL, // Handle ... NEED TO BE FILLED
+
+  FALSE, // Initialized
+  NULL, // Initialize
+
+  0, // HostRegisterBaseAddress ... NEED TO BE FILLED
+  0, // DeviceBaseAddress ... NEED TO BE FILLED
+  0, // RegionBaseAddress ... NEED TO BE FILLED
+  0, // Size ... NEED TO BE FILLED
+  0, // BlockSize
+  0, // LastBlock
+  0, // StartLba
+  0, // OffsetLba
+
+  {
+    FvbGetAttributes, // GetAttributes
+    FvbSetAttributes, // SetAttributes
+    FvbGetPhysicalAddress,  // GetPhysicalAddress
+    FvbGetBlockSize,  // GetBlockSize
+    FvbRead,  // Read
+    FvbWrite, // Write
+    FvbEraseBlocks, // EraseBlocks
+    NULL, //ParentHandle
+  }, //  FvbProtoccol;
+
+  NULL, // ShadowBuffer
+  {
+    {
+      {
+        HARDWARE_DEVICE_PATH,
+        HW_VENDOR_DP,
+        {
+          (UINT8)(OFFSET_OF (NOR_FLASH_DEVICE_PATH, End)),
+          (UINT8)(OFFSET_OF (NOR_FLASH_DEVICE_PATH, End) >> 8)
+        }
+      },
+      { 0x0, 0x0, 0x0, { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } },
+    },
+    0,  // Index
+    {
+      END_DEVICE_PATH_TYPE,
+      END_ENTIRE_DEVICE_PATH_SUBTYPE,
+      { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
+    }
+  }, // DevicePath
+  0 // Flags
+};
+
+EFI_STATUS
+NorFlashCreateInstance (
+  IN UINTN                  HostRegisterBase,
+  IN UINTN                  NorFlashDeviceBase,
+  IN UINTN                  NorFlashRegionBase,
+  IN UINTN                  NorFlashSize,
+  IN UINT32                 Index,
+  IN UINT32                 BlockSize,
+  IN BOOLEAN                HasVarStore,
+  OUT NOR_FLASH_INSTANCE**  NorFlashInstance
+  )
+{
+  EFI_STATUS Status;
+  NOR_FLASH_INSTANCE* Instance;
+  NOR_FLASH_INFO *FlashInfo;
+  UINT8 JedecId[3];
+
+  ASSERT(NorFlashInstance != NULL);
+
+  Instance = AllocateRuntimeCopyPool (sizeof mNorFlashInstanceTemplate,
+                                      &mNorFlashInstanceTemplate);
+  if (Instance == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  Instance->HostRegisterBaseAddress = HostRegisterBase;
+  Instance->DeviceBaseAddress       = NorFlashDeviceBase;
+  Instance->RegionBaseAddress       = NorFlashRegionBase;
+  Instance->Size                    = NorFlashSize;
+  Instance->BlockSize               = BlockSize;
+  Instance->LastBlock               = (NorFlashSize / BlockSize) - 1;
+
+  Instance->OffsetLba = (NorFlashRegionBase - NorFlashDeviceBase) / BlockSize;
+
+  CopyGuid (&Instance->DevicePath.Vendor.Guid, &gEfiCallerIdGuid);
+  Instance->DevicePath.Index = (UINT8)Index;
+
+  NorFlashReset (Instance);
+
+  NorFlashReadID (Instance, JedecId);
+  Status = NorFlashGetInfo (JedecId, &FlashInfo, FALSE);
+  if (EFI_ERROR (Status)) {
+    goto FreeInstance;
+  }
+
+  NorFlashPrintInfo (FlashInfo);
+
+  Instance->Flags = 0;
+  if (FlashInfo->Flags & NOR_FLASH_WRITE_FSR) {
+    Instance->Flags = NOR_FLASH_POLL_FSR;
+  }
+
+  Instance->ShadowBuffer = AllocateRuntimePool (BlockSize);
+  if (Instance->ShadowBuffer == NULL) {
+    Status = EFI_OUT_OF_RESOURCES;
+    goto FreeInstance;
+  }
+
+  if (HasVarStore) {
+    Instance->Initialize = NorFlashFvbInitialize;
+  }
+
+  *NorFlashInstance = Instance;
+  FreePool (FlashInfo);
+  return EFI_SUCCESS;
+
+FreeInstance:
+  FreePool (Instance);
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+NorFlashSetHostCSDC (
+  IN  NOR_FLASH_INSTANCE    *Instance,
+  IN  BOOLEAN               ReadWrite,
+  IN  CONST UINT16          CSDC[ARRAY_SIZE (mFip006NullCmdSeq)]
+  )
+{
+  EFI_PHYSICAL_ADDRESS      Dst;
+  UINTN                     Index;
+
+  Dst = Instance->HostRegisterBaseAddress
+        + (ReadWrite ? FIP006_REG_CS_WR : FIP006_REG_CS_RD);
+  for (Index = 0; Index < ARRAY_SIZE (mFip006NullCmdSeq); Index++) {
+    MmioWrite16 (Dst + (Index << 1), CSDC[Index]);
+  }
+  return EFI_SUCCESS;
+}
+
+STATIC
+CONST CSDC_DEFINITION *
+NorFlashGetCmdDef (
+  IN  NOR_FLASH_INSTANCE    *Instance,
+  IN  UINT8                 Code
+  )
+{
+  CONST CSDC_DEFINITION *Cmd;
+  UINTN                 Index;
+
+  Cmd = NULL;
+  for (Index = 0; Index <  ARRAY_SIZE (mN25qCSDCDefTable); Index++) {
+    if (Code == mN25qCSDCDefTable[Index].Code) {
+      Cmd = &mN25qCSDCDefTable[Index];
+      break;
+    }
+  }
+  return Cmd;
+}
+
+STATIC
+EFI_STATUS
+GenCSDC (
+  IN  UINT8     Cmd,
+  IN  BOOLEAN   AddrAccess,
+  IN  BOOLEAN   AddrMode4Byte,
+  IN  BOOLEAN   HighZ,
+  IN  UINT8     TransferMode,
+  OUT UINT16    *CmdSeq
+  )
+{
+  UINTN         Index;
+
+  if (!CmdSeq) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Index = 0;
+  CopyMem (CmdSeq, mFip006NullCmdSeq, sizeof (mFip006NullCmdSeq));
+
+  CmdSeq[Index++] = CSDC (Cmd, CSDC_CONT_NON_CONTINUOUS, TransferMode,
+                          CSDC_DEC_LEAVE_ASIS);
+  if (AddrAccess) {
+    if (AddrMode4Byte) {
+      CmdSeq[Index++] = CSDC (CSDC_ADDRESS_31_24, CSDC_CONT_NON_CONTINUOUS,
+                              TransferMode, CSDC_DEC_DECODE);
+    }
+    CmdSeq[Index++] = CSDC (CSDC_ADDRESS_23_16, CSDC_CONT_NON_CONTINUOUS,
+                            TransferMode, CSDC_DEC_DECODE);
+    CmdSeq[Index++] = CSDC (CSDC_ADDRESS_15_8, CSDC_CONT_NON_CONTINUOUS,
+                            TransferMode, CSDC_DEC_DECODE);
+    CmdSeq[Index++] = CSDC (CSDC_ADDRESS_7_0, CSDC_CONT_NON_CONTINUOUS,
+                            TransferMode, CSDC_DEC_DECODE);
+  }
+  if (HighZ) {
+    CmdSeq[Index++] = CSDC (CSDC_HIGH_Z, CSDC_CONT_NON_CONTINUOUS,
+                            TransferMode, CSDC_DEC_DECODE);
+  }
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+NorFlashSetHostCommand (
+  IN  NOR_FLASH_INSTANCE    *Instance,
+  IN  UINT8                 Code
+  )
+{
+  CONST CSDC_DEFINITION     *Cmd;
+  UINT16                    CSDC[ARRAY_SIZE (mFip006NullCmdSeq)];
+
+  Cmd = NorFlashGetCmdDef (Instance, Code);
+  if (Cmd == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+  GenCSDC (
+      Cmd->Code,
+      Cmd->AddrAccess,
+      Cmd->AddrMode4Byte,
+      Cmd->HighZ,
+      Cmd->CsdcTrp,
+      CSDC
+      );
+  NorFlashSetHostCSDC (Instance, Cmd->ReadWrite, CSDC);
+  return EFI_SUCCESS;
+}
+
+STATIC
+UINT8
+NorFlashReadStatusRegister (
+  IN NOR_FLASH_INSTANCE     *Instance
+  )
+{
+  UINT8       StatusRegister;
+
+  NorFlashSetHostCommand (Instance, SPINOR_OP_RDSR);
+  StatusRegister = MmioRead8 (Instance->RegionBaseAddress);
+  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
+  return StatusRegister;
+}
+
+STATIC
+EFI_STATUS
+NorFlashWaitProgramErase (
+  IN NOR_FLASH_INSTANCE     *Instance
+  )
+{
+  BOOLEAN     SRegDone;
+  BOOLEAN     FSRegDone;
+
+  DEBUG ((DEBUG_BLKIO, "NorFlashWaitProgramErase()\n"));
+
+  do {
+    SRegDone = (NorFlashReadStatusRegister (Instance) & SPINOR_SR_WIP) == 0;
+    FSRegDone = TRUE;
+    if (Instance->Flags & NOR_FLASH_POLL_FSR) {
+      NorFlashSetHostCommand (Instance, SPINOR_OP_RDFSR);
+      FSRegDone = (MmioRead8 (Instance->RegionBaseAddress) &
+                   SPINOR_FSR_READY) != 0;
+    }
+  } while (!SRegDone || !FSRegDone);
+  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
+  return EFI_SUCCESS;
+}
+
+// TODO: implement lock checking
+STATIC
+BOOLEAN
+NorFlashBlockIsLocked (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN UINTN                  BlockAddress
+  )
+{
+  return FALSE;
+}
+
+// TODO: implement sector unlocking
+STATIC
+EFI_STATUS
+NorFlashUnlockSingleBlock (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN UINTN                  BlockAddress
+  )
+{
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+NorFlashUnlockSingleBlockIfNecessary (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN UINTN                  BlockAddress
+  )
+{
+  EFI_STATUS Status;
+
+  Status = EFI_SUCCESS;
+
+  if (NorFlashBlockIsLocked (Instance, BlockAddress) == TRUE) {
+    Status = NorFlashUnlockSingleBlock (Instance, BlockAddress);
+  }
+
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+NorFlashEnableWrite (
+  IN  NOR_FLASH_INSTANCE    *Instance
+  )
+{
+  EFI_STATUS      Status;
+  UINT8           StatusRegister;
+  UINTN           Retry;
+
+  DEBUG ((DEBUG_BLKIO, "NorFlashEnableWrite()\n"));
+
+  Status = EFI_DEVICE_ERROR;
+  Retry = NOR_FLASH_ERASE_RETRY;
+
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+  while (Retry > 0 && EFI_ERROR (Status)) {
+    MmioWrite8 (Instance->RegionBaseAddress, SPINOR_OP_WREN);
+    MemoryFence ();
+    StatusRegister = NorFlashReadStatusRegister (Instance);
+    Status = (StatusRegister & BIT1) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
+    Retry--;
+  }
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+NorFlashDisableWrite (
+  IN  NOR_FLASH_INSTANCE    *Instance
+  )
+{
+  EFI_STATUS      Status;
+  UINT8           StatusRegister;
+  UINTN           Retry;
+
+  DEBUG ((DEBUG_BLKIO, "NorFlashDisableWrite()\n"));
+
+  Status = EFI_DEVICE_ERROR;
+  Retry = NOR_FLASH_ERASE_RETRY;
+
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+  while (Retry > 0 && EFI_ERROR (Status)) {
+    MmioWrite8 (Instance->RegionBaseAddress, SPINOR_OP_WRDIS);
+    MemoryFence ();
+    StatusRegister = NorFlashReadStatusRegister (Instance);
+    Status = (StatusRegister & BIT1) ? EFI_DEVICE_ERROR : EFI_SUCCESS;
+    Retry--;
+  }
+  return Status;
+}
+
+/**
+ * The following function presumes that the block has already been unlocked.
+ **/
+STATIC
+EFI_STATUS
+NorFlashEraseSingleBlock (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN UINTN                  BlockAddress
+  )
+{
+
+  DEBUG ((DEBUG_BLKIO, "NorFlashEraseSingleBlock(BlockAddress=0x%08x)\n",
+    BlockAddress));
+
+  if (EFI_ERROR (NorFlashEnableWrite (Instance))) {
+    return EFI_DEVICE_ERROR;
+  }
+
+  //
+  // The virtual address chosen by the OS may have a different offset modulo
+  // 16 MB than the physical address, so we need to subtract the region base
+  // address before we can mask off a block index. Note that the relative
+  // offset between device base address and region base address may have changed
+  // as well, so we cannot use the device base address directly.
+  //
+  BlockAddress -= Instance->RegionBaseAddress;
+  BlockAddress += Instance->OffsetLba * Instance->BlockSize;
+
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+  MmioWrite32 (Instance->DeviceBaseAddress,
+               SwapBytes32 (BlockAddress & 0x00FFFFFF) | SPINOR_OP_SE);
+  NorFlashWaitProgramErase (Instance);
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+
+  if (EFI_ERROR (NorFlashDisableWrite (Instance))) {
+    return EFI_DEVICE_ERROR;
+  }
+  return EFI_SUCCESS;
+}
+
+/**
+ * This function unlock and erase an entire NOR Flash block.
+ **/
+EFI_STATUS
+NorFlashUnlockAndEraseSingleBlock (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN UINTN                  BlockAddress
+  )
+{
+  EFI_STATUS              Status;
+  UINTN                   Index;
+  NOR_FLASH_LOCK_CONTEXT  Lock;
+
+  NorFlashLock (&Lock);
+
+  Index = 0;
+  // The block erase might fail a first time (SW bug ?). Retry it ...
+  do {
+    // Unlock the block if we have to
+    Status = NorFlashUnlockSingleBlockIfNecessary (Instance, BlockAddress);
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+    Status = NorFlashEraseSingleBlock (Instance, BlockAddress);
+    Index++;
+  } while ((Index < NOR_FLASH_ERASE_RETRY) && (Status == EFI_WRITE_PROTECTED));
+
+  if (Index == NOR_FLASH_ERASE_RETRY) {
+    DEBUG ((DEBUG_ERROR,
+      "EraseSingleBlock(BlockAddress=0x%08x: Block Locked Error (try to erase %d times)\n",
+      BlockAddress,Index));
+  }
+
+  NorFlashUnlock (&Lock);
+
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+NorFlashWriteSingleWord (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN UINTN                  WordAddress,
+  IN UINT32                 WriteData
+  )
+{
+  EFI_STATUS            Status;
+
+  DEBUG ((DEBUG_BLKIO,
+    "NorFlashWriteSingleWord(WordAddress=0x%08x, WriteData=0x%08x)\n",
+    WordAddress, WriteData));
+
+  Status = EFI_SUCCESS;
+
+  if (EFI_ERROR (NorFlashEnableWrite (Instance))) {
+    return EFI_DEVICE_ERROR;
+  }
+  NorFlashSetHostCommand (Instance, SPINOR_OP_PP);
+  MmioWrite32 (WordAddress, WriteData);
+  NorFlashWaitProgramErase (Instance);
+
+  NorFlashDisableWrite (Instance);
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+  return Status;
+}
+
+STATIC
+EFI_STATUS
+NorFlashWriteFullBlock (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN EFI_LBA                Lba,
+  IN UINT32                 *DataBuffer,
+  IN UINT32                 BlockSizeInWords
+  )
+{
+  EFI_STATUS              Status;
+  UINTN                   WordAddress;
+  UINT32                  WordIndex;
+  UINTN                   BlockAddress;
+  NOR_FLASH_LOCK_CONTEXT  Lock;
+
+  Status = EFI_SUCCESS;
+
+  // Get the physical address of the block
+  BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
+                   BlockSizeInWords * 4);
+
+  // Start writing from the first address at the start of the block
+  WordAddress = BlockAddress;
+
+  NorFlashLock (&Lock);
+
+  Status = NorFlashUnlockAndEraseSingleBlock (Instance, BlockAddress);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "WriteSingleBlock: ERROR - Failed to Unlock and Erase the single block at 0x%X\n",
+      BlockAddress));
+    goto EXIT;
+  }
+
+  for (WordIndex=0;
+       WordIndex < BlockSizeInWords;
+       WordIndex++, DataBuffer++, WordAddress += 4) {
+    Status = NorFlashWriteSingleWord (Instance, WordAddress, *DataBuffer);
+    if (EFI_ERROR (Status)) {
+      goto EXIT;
+    }
+  }
+
+EXIT:
+  NorFlashUnlock (&Lock);
+
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "NOR FLASH Programming [WriteSingleBlock] failed at address 0x%08x. Exit Status = \"%r\".\n",
+      WordAddress, Status));
+  }
+  return Status;
+}
+
+EFI_STATUS
+NorFlashWriteBlocks (
+  IN NOR_FLASH_INSTANCE     *Instance,
+  IN EFI_LBA                Lba,
+  IN UINTN                  BufferSizeInBytes,
+  IN VOID                   *Buffer
+  )
+{
+  UINT32          *pWriteBuffer;
+  EFI_STATUS      Status = EFI_SUCCESS;
+  EFI_LBA         CurrentBlock;
+  UINT32          BlockSizeInWords;
+  UINT32          NumBlocks;
+  UINT32          BlockCount;
+
+  // The buffer must be valid
+  if (Buffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // We must have some bytes to read
+  DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: BufferSizeInBytes=0x%x\n",
+    BufferSizeInBytes));
+  if (BufferSizeInBytes == 0) {
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  // The size of the buffer must be a multiple of the block size
+  DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: BlockSize in bytes =0x%x\n",
+    Instance->BlockSize));
+  if ((BufferSizeInBytes % Instance->BlockSize) != 0) {
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  // All blocks must be within the device
+  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->BlockSize ;
+
+  DEBUG ((DEBUG_BLKIO,
+    "NorFlashWriteBlocks: NumBlocks=%d, LastBlock=%ld, Lba=%ld.\n", NumBlocks,
+    Instance->LastBlock, Lba));
+
+  if ((Lba + NumBlocks) > (Instance->LastBlock + 1)) {
+    DEBUG ((DEBUG_ERROR,
+      "NorFlashWriteBlocks: ERROR - Write will exceed last block.\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ASSERT (((UINTN)Buffer % sizeof (UINT32)) == 0);
+
+  BlockSizeInWords = Instance->BlockSize / 4;
+
+  // Because the target *Buffer is a pointer to VOID, we must put
+  // all the data into a pointer to a proper data type, so use *ReadBuffer
+  pWriteBuffer = (UINT32 *)Buffer;
+
+  CurrentBlock = Lba;
+  for (BlockCount = 0;
+       BlockCount < NumBlocks;
+       BlockCount++, CurrentBlock++, pWriteBuffer += BlockSizeInWords) {
+
+    DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: Writing block #%d\n",
+      (UINTN)CurrentBlock));
+
+    Status = NorFlashWriteFullBlock (Instance, CurrentBlock, pWriteBuffer,
+               BlockSizeInWords);
+
+    if (EFI_ERROR (Status)) {
+      break;
+    }
+  }
+
+  DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: Exit Status = \"%r\".\n", Status));
+  return Status;
+}
+
+EFI_STATUS
+NorFlashReadBlocks (
+  IN NOR_FLASH_INSTANCE   *Instance,
+  IN EFI_LBA              Lba,
+  IN UINTN                BufferSizeInBytes,
+  OUT VOID                *Buffer
+  )
+{
+  UINT32              NumBlocks;
+  UINTN               StartAddress;
+
+  DEBUG ((DEBUG_BLKIO,
+    "NorFlashReadBlocks: BufferSize=0x%xB BlockSize=0x%xB LastBlock=%ld, Lba=%ld.\n",
+    BufferSizeInBytes, Instance->BlockSize, Instance->LastBlock,
+    Lba));
+
+  // The buffer must be valid
+  if (Buffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Return if we have not any byte to read
+  if (BufferSizeInBytes == 0) {
+    return EFI_SUCCESS;
+  }
+
+  // The size of the buffer must be a multiple of the block size
+  if ((BufferSizeInBytes % Instance->BlockSize) != 0) {
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  // All blocks must be within the device
+  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->BlockSize ;
+
+  if ((Lba + NumBlocks) > (Instance->LastBlock + 1)) {
+    DEBUG ((DEBUG_ERROR,
+      "NorFlashReadBlocks: ERROR - Read will exceed last block\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Get the address to start reading from
+  StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
+                                        Instance->BlockSize);
+
+  // Put the device into Read Array mode
+  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+
+  // Readout the data
+  CopyMem(Buffer, (UINTN *)StartAddress, BufferSizeInBytes);
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+NorFlashRead (
+  IN NOR_FLASH_INSTANCE   *Instance,
+  IN EFI_LBA              Lba,
+  IN UINTN                Offset,
+  IN UINTN                BufferSizeInBytes,
+  OUT VOID                *Buffer
+  )
+{
+  UINTN  StartAddress;
+
+  // The buffer must be valid
+  if (Buffer == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Return if we have not any byte to read
+  if (BufferSizeInBytes == 0) {
+    return EFI_SUCCESS;
+  }
+
+  if (((Lba * Instance->BlockSize) + Offset + BufferSizeInBytes) >
+      Instance->Size) {
+    DEBUG ((DEBUG_ERROR,
+      "NorFlashRead: ERROR - Read will exceed device size.\n"));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  // Get the address to start reading from
+  StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
+                                        Instance->BlockSize);
+
+  // Put the device into Read Array mode
+  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+
+  // Readout the data
+  CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes);
+
+  return EFI_SUCCESS;
+}
+
+/*
+  Write a full or portion of a block. It must not span block boundaries;
+  that is, Offset + *NumBytes <= Instance->BlockSize.
+*/
+EFI_STATUS
+NorFlashWriteSingleBlock (
+  IN        NOR_FLASH_INSTANCE   *Instance,
+  IN        EFI_LBA               Lba,
+  IN        UINTN                 Offset,
+  IN OUT    UINTN                *NumBytes,
+  IN        UINT8                *Buffer
+  )
+{
+  EFI_STATUS  TempStatus;
+  UINT32      Tmp;
+  UINT32      TmpBuf;
+  UINT32      WordToWrite;
+  UINT32      Mask;
+  BOOLEAN     DoErase;
+  UINTN       BytesToWrite;
+  UINTN       CurOffset;
+  UINTN       WordAddr;
+  UINTN       BlockSize;
+  UINTN       BlockAddress;
+  UINTN       PrevBlockAddress;
+
+  PrevBlockAddress = 0;
+
+  if (!Instance->Initialized && Instance->Initialize) {
+    Instance->Initialize(Instance);
+  }
+
+  DEBUG ((DEBUG_BLKIO,
+    "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n",
+    Lba, Offset, *NumBytes, Buffer));
+
+  // Cache the block size to avoid de-referencing pointers all the time
+  BlockSize = Instance->BlockSize;
+
+  // The write must not span block boundaries.
+  // We need to check each variable individually because adding two large
+  // values together overflows.
+  if (Offset               >= BlockSize ||
+      *NumBytes            >  BlockSize ||
+      (Offset + *NumBytes) >  BlockSize) {
+    DEBUG ((DEBUG_ERROR,
+      "NorFlashWriteSingleBlock: ERROR - EFI_BAD_BUFFER_SIZE: (Offset=0x%x + NumBytes=0x%x) > BlockSize=0x%x\n",
+      Offset, *NumBytes, BlockSize ));
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  // We must have some bytes to write
+  if (*NumBytes == 0) {
+    DEBUG ((DEBUG_ERROR,
+      "NorFlashWriteSingleBlock: ERROR - EFI_BAD_BUFFER_SIZE: (Offset=0x%x + NumBytes=0x%x) > BlockSize=0x%x\n",
+      Offset, *NumBytes, BlockSize ));
+    return EFI_BAD_BUFFER_SIZE;
+  }
+
+  // Pick 128bytes as a good start for word operations as opposed to erasing the
+  // block and writing the data regardless if an erase is really needed.
+  // It looks like most individual NV variable writes are smaller than 128bytes.
+  if (*NumBytes <= 128) {
+    // Check to see if we need to erase before programming the data into NOR.
+    // If the destination bits are only changing from 1s to 0s we can just write.
+    // After a block is erased all bits in the block is set to 1.
+    // If any byte requires us to erase we just give up and rewrite all of it.
+    DoErase      = FALSE;
+    BytesToWrite = *NumBytes;
+    CurOffset    = Offset;
+
+    while (BytesToWrite > 0) {
+      // Read full word from NOR, splice as required. A word is the smallest
+      // unit we can write.
+      TempStatus = NorFlashRead (Instance, Lba, CurOffset & ~(0x3), sizeof(Tmp),
+                     &Tmp);
+      if (EFI_ERROR (TempStatus)) {
+        return EFI_DEVICE_ERROR;
+      }
+
+      // Physical address of word in NOR to write.
+      WordAddr = (CurOffset & ~(0x3)) +
+                 GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
+                   BlockSize);
+
+      // The word of data that is to be written.
+      TmpBuf = *((UINT32*)(Buffer + (*NumBytes - BytesToWrite)));
+
+      // First do word aligned chunks.
+      if ((CurOffset & 0x3) == 0) {
+        if (BytesToWrite >= 4) {
+          // Is the destination still in 'erased' state?
+          if (~Tmp != 0) {
+            // Check to see if we are only changing bits to zero.
+            if ((Tmp ^ TmpBuf) & TmpBuf) {
+              DoErase = TRUE;
+              break;
+            }
+          }
+          // Write this word to NOR
+          WordToWrite = TmpBuf;
+          CurOffset += sizeof(TmpBuf);
+          BytesToWrite -= sizeof(TmpBuf);
+        } else {
+          // BytesToWrite < 4. Do small writes and left-overs
+          Mask = ~((~0) << (BytesToWrite * 8));
+          // Mask out the bytes we want.
+          TmpBuf &= Mask;
+          // Is the destination still in 'erased' state?
+          if ((Tmp & Mask) != Mask) {
+            // Check to see if we are only changing bits to zero.
+            if ((Tmp ^ TmpBuf) & TmpBuf) {
+              DoErase = TRUE;
+              break;
+            }
+          }
+          // Merge old and new data. Write merged word to NOR
+          WordToWrite = (Tmp & ~Mask) | TmpBuf;
+          CurOffset += BytesToWrite;
+          BytesToWrite = 0;
+        }
+      } else {
+        // Do multiple words, but starting unaligned.
+        if (BytesToWrite > (4 - (CurOffset & 0x3))) {
+          Mask = ((~0) << ((CurOffset & 0x3) * 8));
+          // Mask out the bytes we want.
+          TmpBuf &= Mask;
+          // Is the destination still in 'erased' state?
+          if ((Tmp & Mask) != Mask) {
+            // Check to see if we are only changing bits to zero.
+            if ((Tmp ^ TmpBuf) & TmpBuf) {
+              DoErase = TRUE;
+              break;
+            }
+          }
+          // Merge old and new data. Write merged word to NOR
+          WordToWrite = (Tmp & ~Mask) | TmpBuf;
+          BytesToWrite -= (4 - (CurOffset & 0x3));
+          CurOffset += (4 - (CurOffset & 0x3));
+        } else {
+          // Unaligned and fits in one word.
+          Mask = (~((~0) << (BytesToWrite * 8))) << ((CurOffset & 0x3) * 8);
+          // Mask out the bytes we want.
+          TmpBuf = (TmpBuf << ((CurOffset & 0x3) * 8)) & Mask;
+          // Is the destination still in 'erased' state?
+          if ((Tmp & Mask) != Mask) {
+            // Check to see if we are only changing bits to zero.
+            if ((Tmp ^ TmpBuf) & TmpBuf) {
+              DoErase = TRUE;
+              break;
+            }
+          }
+          // Merge old and new data. Write merged word to NOR
+          WordToWrite = (Tmp & ~Mask) | TmpBuf;
+          CurOffset += BytesToWrite;
+          BytesToWrite = 0;
+        }
+      }
+
+      //
+      // Write the word to NOR.
+      //
+
+      BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
+        BlockSize);
+      if (BlockAddress != PrevBlockAddress) {
+        TempStatus = NorFlashUnlockSingleBlockIfNecessary (Instance,
+                       BlockAddress);
+        if (EFI_ERROR (TempStatus)) {
+          return EFI_DEVICE_ERROR;
+        }
+        PrevBlockAddress = BlockAddress;
+      }
+      TempStatus = NorFlashWriteSingleWord (Instance, WordAddr, WordToWrite);
+      if (EFI_ERROR (TempStatus)) {
+        return EFI_DEVICE_ERROR;
+      }
+    }
+    // Exit if we got here and could write all the data. Otherwise do the
+    // Erase-Write cycle.
+    if (!DoErase) {
+      return EFI_SUCCESS;
+    }
+  }
+
+  // Check we did get some memory. Buffer is BlockSize.
+  if (Instance->ShadowBuffer == NULL) {
+    DEBUG ((DEBUG_ERROR, "FvbWrite: ERROR - Buffer not ready\n"));
+    return EFI_DEVICE_ERROR;
+  }
+
+  // Read NOR Flash data into shadow buffer
+  TempStatus = NorFlashReadBlocks (Instance, Lba, BlockSize,
+                 Instance->ShadowBuffer);
+  if (EFI_ERROR (TempStatus)) {
+    // Return one of the pre-approved error statuses
+    return EFI_DEVICE_ERROR;
+  }
+
+  // Put the data at the appropriate location inside the buffer area
+  CopyMem ((VOID*)((UINTN)Instance->ShadowBuffer + Offset), Buffer, *NumBytes);
+
+  // Write the modified buffer back to the NorFlash
+  TempStatus = NorFlashWriteBlocks (Instance, Lba, BlockSize,
+                 Instance->ShadowBuffer);
+  if (EFI_ERROR (TempStatus)) {
+    // Return one of the pre-approved error statuses
+    return EFI_DEVICE_ERROR;
+  }
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+NorFlashReset (
+  IN  NOR_FLASH_INSTANCE *Instance
+  )
+{
+  FIP006_CS_CFG         CsCfg;
+
+  DEBUG ((DEBUG_BLKIO, "NorFlashReset()\n"));
+  CsCfg.Raw = MmioRead32 (Instance->HostRegisterBaseAddress +
+                          FIP006_REG_CS_CFG);
+  CsCfg.Reg.MBM = CS_CFG_MBM_SINGLE;
+  CsCfg.Reg.SRAM = CS_CFG_SRAM_RW;
+  MmioWrite32 (Instance->HostRegisterBaseAddress + FIP006_REG_CS_CFG,
+               CsCfg.Raw);
+  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
+  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+NorFlashReadID (
+  IN  NOR_FLASH_INSTANCE  *Instance,
+  OUT UINT8               JedecId[3]
+  )
+{
+  if (Instance == NULL || JedecId == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  NorFlashSetHostCommand (Instance, SPINOR_OP_RDID);
+  JedecId[0] = MmioRead8 (Instance->DeviceBaseAddress);
+  JedecId[1] = MmioRead8 (Instance->DeviceBaseAddress + 1);
+  JedecId[2] = MmioRead8 (Instance->DeviceBaseAddress + 2);
+  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
+  return EFI_SUCCESS;
+}
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
similarity index 88%
rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
index 20e74b0320ce..61b8e6a08fa0 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
@@ -27,11 +27,9 @@
 #include <Protocol/FirmwareVolumeBlock.h>
 
 #include <Library/DebugLib.h>
-#include <Library/DxeServicesTableLib.h>
 #include <Library/IoLib.h>
+#include <Library/MemoryAllocationLib.h>
 #include <Library/NorFlashPlatformLib.h>
-#include <Library/UefiLib.h>
-#include <Library/UefiRuntimeLib.h>
 
 #include "Fip006Reg.h"
 
@@ -112,13 +110,27 @@ struct _NOR_FLASH_INSTANCE {
 
   NOR_FLASH_DEVICE_PATH               DevicePath;
 
-  CONST CSDC_DEFINITION               *CmdTable;
-  UINTN                               CmdTableSize;
-
   UINT32                              Flags;
 #define NOR_FLASH_POLL_FSR      BIT0
 };
 
+typedef struct {
+  EFI_TPL         OriginalTPL;
+  BOOLEAN         InterruptsEnabled;
+} NOR_FLASH_LOCK_CONTEXT;
+
+VOID
+EFIAPI
+NorFlashLock (
+  NOR_FLASH_LOCK_CONTEXT    *Context
+  );
+
+VOID
+EFIAPI
+NorFlashUnlock (
+  NOR_FLASH_LOCK_CONTEXT    *Context
+  );
+
 EFI_STATUS
 NorFlashReadCfiData (
   IN  UINTN                   DeviceBaseAddress,
@@ -135,14 +147,34 @@ NorFlashWriteBuffer (
   IN UINT32                 *Buffer
   );
 
-//
-// NorFlashFvbDxe.c
-//
+extern UINTN     mFlashNvStorageVariableBase;
+
+EFI_STATUS
+NorFlashCreateInstance (
+  IN UINTN                  HostRegisterBase,
+  IN UINTN                  NorFlashDeviceBase,
+  IN UINTN                  NorFlashRegionBase,
+  IN UINTN                  NorFlashSize,
+  IN UINT32                 Index,
+  IN UINT32                 BlockSize,
+  IN BOOLEAN                HasVarStore,
+  OUT NOR_FLASH_INSTANCE**  NorFlashInstance
+  );
 
 EFI_STATUS
 EFIAPI
 NorFlashFvbInitialize (
-  IN NOR_FLASH_INSTANCE*                            Instance
+  IN NOR_FLASH_INSTANCE* Instance
+  );
+
+EFI_STATUS
+ValidateFvHeader (
+  IN  NOR_FLASH_INSTANCE *Instance
+  );
+
+EFI_STATUS
+InitializeFvAndVariableStoreHeaders (
+  IN NOR_FLASH_INSTANCE *Instance
   );
 
 EFI_STATUS
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
index e52ab52d8cf7..6c07799b22d8 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
@@ -15,15 +15,16 @@
 **/
 
 #include <Library/BaseMemoryLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/NorFlashInfoLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
+#include <Library/UefiRuntimeLib.h>
 
-#include "NorFlashDxe.h"
-
-STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
+#include "NorFlash.h"
 
 //
 // Global variable declarations
@@ -31,1035 +32,129 @@ STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
 STATIC NOR_FLASH_INSTANCE   **mNorFlashInstances;
 STATIC UINT32               mNorFlashDeviceCount;
 
-STATIC CONST UINT16 mFip006NullCmdSeq[] = {
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
-};
-
-STATIC CONST CSDC_DEFINITION mN25qCSDCDefTable[] = {
-  // Identification Operations
-  { SPINOR_OP_RDID,     FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  // Register Operations
-  { SPINOR_OP_RDSR,     FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_WRSR,     FALSE, FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_RD_ARRAY, TRUE,  FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_RDFSR,    FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_RD_NVCFG, FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_RD_VCR,   FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_RD_EVCR,  FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  // Read Operations
-  { SPINOR_OP_READ_4B,  TRUE,  TRUE,  FALSE, FALSE, CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  // Write Operations
-  { SPINOR_OP_PP,       TRUE,  FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-  { SPINOR_OP_PP_1_1_4, TRUE,  FALSE, FALSE, TRUE,  CS_CFG_MBM_QUAD,
-                        CSDC_TRP_SINGLE },
-  // Erase Operations
-  { SPINOR_OP_SE,       FALSE, FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
-                        CSDC_TRP_SINGLE },
-};
-
-STATIC
-EFI_STATUS
-NorFlashSetHostCSDC (
-  IN  NOR_FLASH_INSTANCE    *Instance,
-  IN  BOOLEAN               ReadWrite,
-  IN  CONST UINT16          CSDC[ARRAY_SIZE (mFip006NullCmdSeq)]
-  )
-{
-  EFI_PHYSICAL_ADDRESS      Dst;
-  UINTN                     Index;
-
-  Dst = Instance->HostRegisterBaseAddress
-        + (ReadWrite ? FIP006_REG_CS_WR : FIP006_REG_CS_RD);
-  for (Index = 0; Index < ARRAY_SIZE (mFip006NullCmdSeq); Index++) {
-    MmioWrite16 (Dst + (Index << 1), CSDC[Index]);
-  }
-  return EFI_SUCCESS;
-}
-
-STATIC
-CONST CSDC_DEFINITION *
-NorFlashGetCmdDef (
-  IN  NOR_FLASH_INSTANCE    *Instance,
-  IN  UINT8                 Code
-  )
-{
-  CONST CSDC_DEFINITION *Cmd;
-  UINTN                 Index;
-
-  Cmd = NULL;
-  for (Index = 0; Index <  Instance->CmdTableSize; Index++) {
-    if (Code == Instance->CmdTable[Index].Code) {
-      Cmd = &Instance->CmdTable[Index];
-      break;
-    }
-  }
-  return Cmd;
-}
-
-STATIC
-EFI_STATUS
-GenCSDC (
-  IN  UINT8     Cmd,
-  IN  BOOLEAN   AddrAccess,
-  IN  BOOLEAN   AddrMode4Byte,
-  IN  BOOLEAN   HighZ,
-  IN  UINT8     TransferMode,
-  OUT UINT16    *CmdSeq
-  )
-{
-  UINTN         Index;
-
-  if (!CmdSeq) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  Index = 0;
-  CopyMem (CmdSeq, mFip006NullCmdSeq, sizeof (mFip006NullCmdSeq));
-
-  CmdSeq[Index++] = CSDC (Cmd, CSDC_CONT_NON_CONTINUOUS, TransferMode,
-                          CSDC_DEC_LEAVE_ASIS);
-  if (AddrAccess) {
-    if (AddrMode4Byte) {
-      CmdSeq[Index++] = CSDC (CSDC_ADDRESS_31_24, CSDC_CONT_NON_CONTINUOUS,
-                              TransferMode, CSDC_DEC_DECODE);
-    }
-    CmdSeq[Index++] = CSDC (CSDC_ADDRESS_23_16, CSDC_CONT_NON_CONTINUOUS,
-                            TransferMode, CSDC_DEC_DECODE);
-    CmdSeq[Index++] = CSDC (CSDC_ADDRESS_15_8, CSDC_CONT_NON_CONTINUOUS,
-                            TransferMode, CSDC_DEC_DECODE);
-    CmdSeq[Index++] = CSDC (CSDC_ADDRESS_7_0, CSDC_CONT_NON_CONTINUOUS,
-                            TransferMode, CSDC_DEC_DECODE);
-  }
-  if (HighZ) {
-    CmdSeq[Index++] = CSDC (CSDC_HIGH_Z, CSDC_CONT_NON_CONTINUOUS,
-                            TransferMode, CSDC_DEC_DECODE);
-  }
-
-  return EFI_SUCCESS;
-}
-
-STATIC
-EFI_STATUS
-NorFlashSetHostCommand (
-  IN  NOR_FLASH_INSTANCE    *Instance,
-  IN  UINT8                 Code
-  )
-{
-  CONST CSDC_DEFINITION     *Cmd;
-  UINT16                    CSDC[ARRAY_SIZE (mFip006NullCmdSeq)];
-
-  Cmd = NorFlashGetCmdDef (Instance, Code);
-  if (Cmd == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-  GenCSDC (
-      Cmd->Code,
-      Cmd->AddrAccess,
-      Cmd->AddrMode4Byte,
-      Cmd->HighZ,
-      Cmd->CsdcTrp,
-      CSDC
-      );
-  NorFlashSetHostCSDC (Instance, Cmd->ReadWrite, CSDC);
-  return EFI_SUCCESS;
-}
-
-STATIC
-UINT8
-NorFlashReadStatusRegister (
-  IN NOR_FLASH_INSTANCE     *Instance
-  )
-{
-  UINT8       StatusRegister;
-
-  NorFlashSetHostCommand (Instance, SPINOR_OP_RDSR);
-  StatusRegister = MmioRead8 (Instance->RegionBaseAddress);
-  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
-  return StatusRegister;
-}
-
-STATIC
-EFI_STATUS
-NorFlashWaitProgramErase (
-  IN NOR_FLASH_INSTANCE     *Instance
-  )
-{
-  BOOLEAN     SRegDone;
-  BOOLEAN     FSRegDone;
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashWaitProgramErase()\n"));
-
-  do {
-    SRegDone = (NorFlashReadStatusRegister (Instance) & SPINOR_SR_WIP) == 0;
-    FSRegDone = TRUE;
-    if (Instance->Flags & NOR_FLASH_POLL_FSR) {
-      NorFlashSetHostCommand (Instance, SPINOR_OP_RDFSR);
-      FSRegDone = (MmioRead8 (Instance->RegionBaseAddress) &
-                   SPINOR_FSR_READY) != 0;
-    }
-  } while (!SRegDone || !FSRegDone);
-  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
-  return EFI_SUCCESS;
-}
-
-// TODO: implement lock checking
-STATIC
-BOOLEAN
-NorFlashBlockIsLocked (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN UINTN                  BlockAddress
-  )
-{
-  return FALSE;
-}
-
-// TODO: implement sector unlocking
-STATIC
-EFI_STATUS
-NorFlashUnlockSingleBlock (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN UINTN                  BlockAddress
-  )
-{
-  return EFI_SUCCESS;
-}
-
-STATIC
-EFI_STATUS
-NorFlashUnlockSingleBlockIfNecessary (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN UINTN                  BlockAddress
-  )
-{
-  EFI_STATUS Status;
-
-  Status = EFI_SUCCESS;
-
-  if (NorFlashBlockIsLocked (Instance, BlockAddress) == TRUE) {
-    Status = NorFlashUnlockSingleBlock (Instance, BlockAddress);
-  }
-
-  return Status;
-}
-
-STATIC
-EFI_STATUS
-NorFlashEnableWrite (
-  IN  NOR_FLASH_INSTANCE    *Instance
-  )
-{
-  EFI_STATUS      Status;
-  UINT8           StatusRegister;
-  UINTN           Retry;
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashEnableWrite()\n"));
-
-  Status = EFI_DEVICE_ERROR;
-  Retry = NOR_FLASH_ERASE_RETRY;
-
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-  while (Retry > 0 && EFI_ERROR (Status)) {
-    MmioWrite8 (Instance->RegionBaseAddress, SPINOR_OP_WREN);
-    MemoryFence ();
-    StatusRegister = NorFlashReadStatusRegister (Instance);
-    Status = (StatusRegister & BIT1) ? EFI_SUCCESS : EFI_DEVICE_ERROR;
-    Retry--;
-  }
-  return Status;
-}
+STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
 
-STATIC
 EFI_STATUS
-NorFlashDisableWrite (
-  IN  NOR_FLASH_INSTANCE    *Instance
+EFIAPI
+NorFlashFvbInitialize (
+  IN NOR_FLASH_INSTANCE* Instance
   )
 {
   EFI_STATUS      Status;
-  UINT8           StatusRegister;
-  UINTN           Retry;
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashDisableWrite()\n"));
-
-  Status = EFI_DEVICE_ERROR;
-  Retry = NOR_FLASH_ERASE_RETRY;
-
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-  while (Retry > 0 && EFI_ERROR (Status)) {
-    MmioWrite8 (Instance->RegionBaseAddress, SPINOR_OP_WRDIS);
-    MemoryFence ();
-    StatusRegister = NorFlashReadStatusRegister (Instance);
-    Status = (StatusRegister & BIT1) ? EFI_DEVICE_ERROR : EFI_SUCCESS;
-    Retry--;
-  }
-  return Status;
-}
-
-/**
- * The following function presumes that the block has already been unlocked.
- **/
-STATIC
-EFI_STATUS
-NorFlashEraseSingleBlock (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN UINTN                  BlockAddress
-  )
-{
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashEraseSingleBlock(BlockAddress=0x%08x)\n",
-    BlockAddress));
-
-  if (EFI_ERROR (NorFlashEnableWrite (Instance))) {
-    return EFI_DEVICE_ERROR;
-  }
+  UINT32          FvbNumLba;
+  EFI_BOOT_MODE   BootMode;
+  UINTN           RuntimeMmioRegionSize;
+  UINTN           BlockSize;
 
-  //
-  // The virtual address chosen by the OS may have a different offset modulo
-  // 16 MB than the physical address, so we need to subtract the region base
-  // address before we can mask off a block index. Note that the relative
-  // offset between device base address and region base address may have changed
-  // as well, so we cannot use the device base address directly.
-  //
-  if (EfiAtRuntime()) {
-    BlockAddress -= Instance->RegionBaseAddress;
-    BlockAddress += Instance->OffsetLba * Instance->BlockSize;
-  }
-
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-  MmioWrite32 (Instance->DeviceBaseAddress,
-               SwapBytes32 (BlockAddress & 0x00FFFFFF) | SPINOR_OP_SE);
-  NorFlashWaitProgramErase (Instance);
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-
-  if (EFI_ERROR (NorFlashDisableWrite (Instance))) {
-    return EFI_DEVICE_ERROR;
-  }
-  return EFI_SUCCESS;
-}
-
-/**
- * This function unlock and erase an entire NOR Flash block.
- **/
-EFI_STATUS
-NorFlashUnlockAndEraseSingleBlock (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN UINTN                  BlockAddress
-  )
-{
-  EFI_STATUS      Status;
-  UINTN           Index;
-  EFI_TPL         OriginalTPL;
-  BOOLEAN         InterruptsEnabled;
+  DEBUG ((DEBUG_BLKIO,"NorFlashFvbInitialize\n"));
 
-  OriginalTPL = 0;
-  InterruptsEnabled = FALSE;
+  BlockSize = Instance->BlockSize;
 
-  if (!EfiAtRuntime ()) {
-    // Raise TPL to TPL_HIGH to stop anyone from interrupting us.
-    OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+  // FirmwareVolumeHeader->FvLength is declared to have the Variable area
+  // AND the FTW working area AND the FTW Spare contiguous.
+  ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) +
+         PcdGet32(PcdFlashNvStorageVariableSize) ==
+         PcdGet32(PcdFlashNvStorageFtwWorkingBase));
+  ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) +
+         PcdGet32(PcdFlashNvStorageFtwWorkingSize) ==
+         PcdGet32(PcdFlashNvStorageFtwSpareBase));
+
+  // Check if the size of the area is at least one block size
+  ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) &&
+         (PcdGet32(PcdFlashNvStorageVariableSize) / BlockSize > 0));
+  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingSize) > 0) &&
+         (PcdGet32(PcdFlashNvStorageFtwWorkingSize) / BlockSize > 0));
+  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) &&
+         (PcdGet32(PcdFlashNvStorageFtwSpareSize) / BlockSize > 0));
+
+  // Ensure the Variable areas are aligned on block size boundaries
+  ASSERT((PcdGet32(PcdFlashNvStorageVariableBase) % BlockSize) == 0);
+  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingBase) % BlockSize) == 0);
+  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareBase) % BlockSize) == 0);
+
+
+  Instance->Initialized = TRUE;
+  mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
+
+  // Set the index of the first LBA for the FVB
+  Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) -
+                        Instance->RegionBaseAddress) / BlockSize;
+
+  BootMode = GetBootModeHob ();
+  if (BootMode == BOOT_WITH_DEFAULT_SETTINGS) {
+    Status = EFI_INVALID_PARAMETER;
   } else {
-    InterruptsEnabled = SaveAndDisableInterrupts ();
-  }
-
-  Index = 0;
-  // The block erase might fail a first time (SW bug ?). Retry it ...
-  do {
-    // Unlock the block if we have to
-    Status = NorFlashUnlockSingleBlockIfNecessary (Instance, BlockAddress);
-    if (EFI_ERROR (Status)) {
-      break;
+    // Determine if there is a valid header at the beginning of the NorFlash
+    Status = ValidateFvHeader (Instance);
+  }
+
+  // Install the Default FVB header if required
+  if (EFI_ERROR(Status)) {
+    // There is no valid header, so time to install one.
+    DEBUG ((DEBUG_INFO, "%a: The FVB Header is not valid.\n", __FUNCTION__));
+    DEBUG ((DEBUG_INFO, "%a: Installing a correct one for this volume.\n",
+      __FUNCTION__));
+
+    // Erase all the NorFlash that is reserved for variable storage
+    FvbNumLba = (PcdGet32(PcdFlashNvStorageVariableSize) +
+                 PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
+                 PcdGet32(PcdFlashNvStorageFtwSpareSize)) /
+                Instance->BlockSize;
+
+    Status = FvbEraseBlocks (&Instance->FvbProtocol, (EFI_LBA)0, FvbNumLba,
+               EFI_LBA_LIST_TERMINATOR);
+    if (EFI_ERROR(Status)) {
+      return Status;
     }
-    Status = NorFlashEraseSingleBlock (Instance, BlockAddress);
-    Index++;
-  } while ((Index < NOR_FLASH_ERASE_RETRY) && (Status == EFI_WRITE_PROTECTED));
-
-  if (Index == NOR_FLASH_ERASE_RETRY) {
-    DEBUG ((DEBUG_ERROR,
-      "EraseSingleBlock(BlockAddress=0x%08x: Block Locked Error (try to erase %d times)\n",
-      BlockAddress,Index));
-  }
 
-  if (!EfiAtRuntime ()) {
-    // Interruptions can resume.
-    gBS->RestoreTPL (OriginalTPL);
-  } else if (InterruptsEnabled) {
-    SetInterruptState (TRUE);
-  }
-
-  return Status;
-}
-
-STATIC
-EFI_STATUS
-NorFlashWriteSingleWord (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN UINTN                  WordAddress,
-  IN UINT32                 WriteData
-  )
-{
-  EFI_STATUS            Status;
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashWriteSingleWord(WordAddress=0x%08x, WriteData=0x%08x)\n",
-    WordAddress, WriteData));
-
-  Status = EFI_SUCCESS;
-
-  if (EFI_ERROR (NorFlashEnableWrite (Instance))) {
-    return EFI_DEVICE_ERROR;
-  }
-  NorFlashSetHostCommand (Instance, SPINOR_OP_PP);
-  MmioWrite32 (WordAddress, WriteData);
-  NorFlashWaitProgramErase (Instance);
-
-  NorFlashDisableWrite (Instance);
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-  return Status;
-}
-
-STATIC
-EFI_STATUS
-NorFlashWriteFullBlock (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN EFI_LBA                Lba,
-  IN UINT32                 *DataBuffer,
-  IN UINT32                 BlockSizeInWords
-  )
-{
-  EFI_STATUS    Status;
-  UINTN         WordAddress;
-  UINT32        WordIndex;
-  UINTN         BlockAddress;
-  EFI_TPL       OriginalTPL;
-  BOOLEAN       InterruptsEnabled;
-
-  Status = EFI_SUCCESS;
-  OriginalTPL = 0;
-  InterruptsEnabled = FALSE;
-
-  // Get the physical address of the block
-  BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-                   BlockSizeInWords * 4);
-
-  // Start writing from the first address at the start of the block
-  WordAddress = BlockAddress;
-
-  if (!EfiAtRuntime ()) {
-    // Raise TPL to TPL_HIGH to stop anyone from interrupting us.
-    OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
-  } else {
-    InterruptsEnabled = SaveAndDisableInterrupts ();
-  }
-
-  Status = NorFlashUnlockAndEraseSingleBlock (Instance, BlockAddress);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR,
-      "WriteSingleBlock: ERROR - Failed to Unlock and Erase the single block at 0x%X\n",
-      BlockAddress));
-    goto EXIT;
-  }
-
-  for (WordIndex=0;
-       WordIndex < BlockSizeInWords;
-       WordIndex++, DataBuffer++, WordAddress += 4) {
-    Status = NorFlashWriteSingleWord (Instance, WordAddress, *DataBuffer);
-    if (EFI_ERROR (Status)) {
-      goto EXIT;
+    // Install all appropriate headers
+    Status = InitializeFvAndVariableStoreHeaders (Instance);
+    if (EFI_ERROR(Status)) {
+      return Status;
     }
   }
 
-EXIT:
-  if (!EfiAtRuntime ()) {
-    // Interruptions can resume.
-    gBS->RestoreTPL (OriginalTPL);
-  } else if (InterruptsEnabled) {
-    SetInterruptState (TRUE);
-  }
-
+  //
+  // The driver implementing the variable read service can now be dispatched;
+  // the varstore headers are in place.
+  //
+  Status = gBS->InstallProtocolInterface (&gImageHandle,
+                  &gEdkiiNvVarStoreFormattedGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR,
-      "NOR FLASH Programming [WriteSingleBlock] failed at address 0x%08x. Exit Status = \"%r\".\n",
-      WordAddress, Status));
-  }
-  return Status;
-}
-
-EFI_STATUS
-NorFlashWriteBlocks (
-  IN NOR_FLASH_INSTANCE     *Instance,
-  IN EFI_LBA                Lba,
-  IN UINTN                  BufferSizeInBytes,
-  IN VOID                   *Buffer
-  )
-{
-  UINT32          *pWriteBuffer;
-  EFI_STATUS      Status = EFI_SUCCESS;
-  EFI_LBA         CurrentBlock;
-  UINT32          BlockSizeInWords;
-  UINT32          NumBlocks;
-  UINT32          BlockCount;
-
-  // The buffer must be valid
-  if (Buffer == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  // We must have some bytes to read
-  DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: BufferSizeInBytes=0x%x\n",
-    BufferSizeInBytes));
-  if (BufferSizeInBytes == 0) {
-    return EFI_BAD_BUFFER_SIZE;
-  }
-
-  // The size of the buffer must be a multiple of the block size
-  DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: BlockSize in bytes =0x%x\n",
-    Instance->BlockSize));
-  if ((BufferSizeInBytes % Instance->BlockSize) != 0) {
-    return EFI_BAD_BUFFER_SIZE;
-  }
-
-  // All blocks must be within the device
-  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->BlockSize ;
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashWriteBlocks: NumBlocks=%d, LastBlock=%ld, Lba=%ld.\n", NumBlocks,
-    Instance->LastBlock, Lba));
-
-  if ((Lba + NumBlocks) > (Instance->LastBlock + 1)) {
-    DEBUG ((DEBUG_ERROR,
-      "NorFlashWriteBlocks: ERROR - Write will exceed last block.\n"));
-    return EFI_INVALID_PARAMETER;
-  }
-
-  ASSERT (((UINTN)Buffer % sizeof (UINT32)) == 0);
-
-  BlockSizeInWords = Instance->BlockSize / 4;
-
-  // Because the target *Buffer is a pointer to VOID, we must put
-  // all the data into a pointer to a proper data type, so use *ReadBuffer
-  pWriteBuffer = (UINT32 *)Buffer;
-
-  CurrentBlock = Lba;
-  for (BlockCount = 0;
-       BlockCount < NumBlocks;
-       BlockCount++, CurrentBlock++, pWriteBuffer += BlockSizeInWords) {
-
-    DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: Writing block #%d\n",
-      (UINTN)CurrentBlock));
-
-    Status = NorFlashWriteFullBlock (Instance, CurrentBlock, pWriteBuffer,
-               BlockSizeInWords);
-
-    if (EFI_ERROR (Status)) {
-      break;
-    }
-  }
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashWriteBlocks: Exit Status = \"%r\".\n", Status));
-  return Status;
-}
-
-EFI_STATUS
-NorFlashReadBlocks (
-  IN NOR_FLASH_INSTANCE   *Instance,
-  IN EFI_LBA              Lba,
-  IN UINTN                BufferSizeInBytes,
-  OUT VOID                *Buffer
-  )
-{
-  UINT32              NumBlocks;
-  UINTN               StartAddress;
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashReadBlocks: BufferSize=0x%xB BlockSize=0x%xB LastBlock=%ld, Lba=%ld.\n",
-    BufferSizeInBytes, Instance->BlockSize, Instance->LastBlock,
-    Lba));
-
-  // The buffer must be valid
-  if (Buffer == NULL) {
-    return EFI_INVALID_PARAMETER;
+      "%a: Failed to install gEdkiiNvVarStoreFormattedGuid\n",
+      __FUNCTION__));
+      return Status;
   }
 
-  // Return if we have not any byte to read
-  if (BufferSizeInBytes == 0) {
-    return EFI_SUCCESS;
-  }
-
-  // The size of the buffer must be a multiple of the block size
-  if ((BufferSizeInBytes % Instance->BlockSize) != 0) {
-    return EFI_BAD_BUFFER_SIZE;
-  }
-
-  // All blocks must be within the device
-  NumBlocks = ((UINT32)BufferSizeInBytes) / Instance->BlockSize ;
-
-  if ((Lba + NumBlocks) > (Instance->LastBlock + 1)) {
-    DEBUG ((DEBUG_ERROR,
-      "NorFlashReadBlocks: ERROR - Read will exceed last block\n"));
-    return EFI_INVALID_PARAMETER;
-  }
-
-  // Get the address to start reading from
-  StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-                                        Instance->BlockSize);
-
-  // Put the device into Read Array mode
-  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-
-  // Readout the data
-  CopyMem(Buffer, (UINTN *)StartAddress, BufferSizeInBytes);
-
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-NorFlashRead (
-  IN NOR_FLASH_INSTANCE   *Instance,
-  IN EFI_LBA              Lba,
-  IN UINTN                Offset,
-  IN UINTN                BufferSizeInBytes,
-  OUT VOID                *Buffer
-  )
-{
-  UINTN  StartAddress;
-
-  // The buffer must be valid
-  if (Buffer == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  // Return if we have not any byte to read
-  if (BufferSizeInBytes == 0) {
-    return EFI_SUCCESS;
-  }
-
-  if (((Lba * Instance->BlockSize) + Offset + BufferSizeInBytes) >
-      Instance->Size) {
-    DEBUG ((DEBUG_ERROR,
-      "NorFlashRead: ERROR - Read will exceed device size.\n"));
-    return EFI_INVALID_PARAMETER;
-  }
-
-  // Get the address to start reading from
-  StartAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-                                        Instance->BlockSize);
-
-  // Put the device into Read Array mode
-  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-
-  // Readout the data
-  CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes);
-
-  return EFI_SUCCESS;
-}
-
-/*
-  Write a full or portion of a block. It must not span block boundaries;
-  that is, Offset + *NumBytes <= Instance->BlockSize.
-*/
-EFI_STATUS
-NorFlashWriteSingleBlock (
-  IN        NOR_FLASH_INSTANCE   *Instance,
-  IN        EFI_LBA               Lba,
-  IN        UINTN                 Offset,
-  IN OUT    UINTN                *NumBytes,
-  IN        UINT8                *Buffer
-  )
-{
-  EFI_STATUS  TempStatus;
-  UINT32      Tmp;
-  UINT32      TmpBuf;
-  UINT32      WordToWrite;
-  UINT32      Mask;
-  BOOLEAN     DoErase;
-  UINTN       BytesToWrite;
-  UINTN       CurOffset;
-  UINTN       WordAddr;
-  UINTN       BlockSize;
-  UINTN       BlockAddress;
-  UINTN       PrevBlockAddress;
-
-  PrevBlockAddress = 0;
-
-  if (!Instance->Initialized && Instance->Initialize) {
-    Instance->Initialize(Instance);
-  }
-
-  DEBUG ((DEBUG_BLKIO,
-    "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n",
-    Lba, Offset, *NumBytes, Buffer));
-
-  // Cache the block size to avoid de-referencing pointers all the time
-  BlockSize = Instance->BlockSize;
-
-  // The write must not span block boundaries.
-  // We need to check each variable individually because adding two large
-  // values together overflows.
-  if (Offset               >= BlockSize ||
-      *NumBytes            >  BlockSize ||
-      (Offset + *NumBytes) >  BlockSize) {
-    DEBUG ((DEBUG_ERROR,
-      "NorFlashWriteSingleBlock: ERROR - EFI_BAD_BUFFER_SIZE: (Offset=0x%x + NumBytes=0x%x) > BlockSize=0x%x\n",
-      Offset, *NumBytes, BlockSize ));
-    return EFI_BAD_BUFFER_SIZE;
-  }
-
-  // We must have some bytes to write
-  if (*NumBytes == 0) {
-    DEBUG ((DEBUG_ERROR,
-      "NorFlashWriteSingleBlock: ERROR - EFI_BAD_BUFFER_SIZE: (Offset=0x%x + NumBytes=0x%x) > BlockSize=0x%x\n",
-      Offset, *NumBytes, BlockSize ));
-    return EFI_BAD_BUFFER_SIZE;
-  }
-
-  // Pick 128bytes as a good start for word operations as opposed to erasing the
-  // block and writing the data regardless if an erase is really needed.
-  // It looks like most individual NV variable writes are smaller than 128bytes.
-  if (*NumBytes <= 128) {
-    // Check to see if we need to erase before programming the data into NOR.
-    // If the destination bits are only changing from 1s to 0s we can just write.
-    // After a block is erased all bits in the block is set to 1.
-    // If any byte requires us to erase we just give up and rewrite all of it.
-    DoErase      = FALSE;
-    BytesToWrite = *NumBytes;
-    CurOffset    = Offset;
-
-    while (BytesToWrite > 0) {
-      // Read full word from NOR, splice as required. A word is the smallest
-      // unit we can write.
-      TempStatus = NorFlashRead (Instance, Lba, CurOffset & ~(0x3), sizeof(Tmp),
-                     &Tmp);
-      if (EFI_ERROR (TempStatus)) {
-        return EFI_DEVICE_ERROR;
-      }
-
-      // Physical address of word in NOR to write.
-      WordAddr = (CurOffset & ~(0x3)) +
-                 GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-                   BlockSize);
-
-      // The word of data that is to be written.
-      TmpBuf = *((UINT32*)(Buffer + (*NumBytes - BytesToWrite)));
-
-      // First do word aligned chunks.
-      if ((CurOffset & 0x3) == 0) {
-        if (BytesToWrite >= 4) {
-          // Is the destination still in 'erased' state?
-          if (~Tmp != 0) {
-            // Check to see if we are only changing bits to zero.
-            if ((Tmp ^ TmpBuf) & TmpBuf) {
-              DoErase = TRUE;
-              break;
-            }
-          }
-          // Write this word to NOR
-          WordToWrite = TmpBuf;
-          CurOffset += sizeof(TmpBuf);
-          BytesToWrite -= sizeof(TmpBuf);
-        } else {
-          // BytesToWrite < 4. Do small writes and left-overs
-          Mask = ~((~0) << (BytesToWrite * 8));
-          // Mask out the bytes we want.
-          TmpBuf &= Mask;
-          // Is the destination still in 'erased' state?
-          if ((Tmp & Mask) != Mask) {
-            // Check to see if we are only changing bits to zero.
-            if ((Tmp ^ TmpBuf) & TmpBuf) {
-              DoErase = TRUE;
-              break;
-            }
-          }
-          // Merge old and new data. Write merged word to NOR
-          WordToWrite = (Tmp & ~Mask) | TmpBuf;
-          CurOffset += BytesToWrite;
-          BytesToWrite = 0;
-        }
-      } else {
-        // Do multiple words, but starting unaligned.
-        if (BytesToWrite > (4 - (CurOffset & 0x3))) {
-          Mask = ((~0) << ((CurOffset & 0x3) * 8));
-          // Mask out the bytes we want.
-          TmpBuf &= Mask;
-          // Is the destination still in 'erased' state?
-          if ((Tmp & Mask) != Mask) {
-            // Check to see if we are only changing bits to zero.
-            if ((Tmp ^ TmpBuf) & TmpBuf) {
-              DoErase = TRUE;
-              break;
-            }
-          }
-          // Merge old and new data. Write merged word to NOR
-          WordToWrite = (Tmp & ~Mask) | TmpBuf;
-          BytesToWrite -= (4 - (CurOffset & 0x3));
-          CurOffset += (4 - (CurOffset & 0x3));
-        } else {
-          // Unaligned and fits in one word.
-          Mask = (~((~0) << (BytesToWrite * 8))) << ((CurOffset & 0x3) * 8);
-          // Mask out the bytes we want.
-          TmpBuf = (TmpBuf << ((CurOffset & 0x3) * 8)) & Mask;
-          // Is the destination still in 'erased' state?
-          if ((Tmp & Mask) != Mask) {
-            // Check to see if we are only changing bits to zero.
-            if ((Tmp ^ TmpBuf) & TmpBuf) {
-              DoErase = TRUE;
-              break;
-            }
-          }
-          // Merge old and new data. Write merged word to NOR
-          WordToWrite = (Tmp & ~Mask) | TmpBuf;
-          CurOffset += BytesToWrite;
-          BytesToWrite = 0;
-        }
-      }
-
-      //
-      // Write the word to NOR.
-      //
-
-      BlockAddress = GET_NOR_BLOCK_ADDRESS (Instance->RegionBaseAddress, Lba,
-        BlockSize);
-      if (BlockAddress != PrevBlockAddress) {
-        TempStatus = NorFlashUnlockSingleBlockIfNecessary (Instance,
-                       BlockAddress);
-        if (EFI_ERROR (TempStatus)) {
-          return EFI_DEVICE_ERROR;
-        }
-        PrevBlockAddress = BlockAddress;
-      }
-      TempStatus = NorFlashWriteSingleWord (Instance, WordAddr, WordToWrite);
-      if (EFI_ERROR (TempStatus)) {
-        return EFI_DEVICE_ERROR;
-      }
-    }
-    // Exit if we got here and could write all the data. Otherwise do the
-    // Erase-Write cycle.
-    if (!DoErase) {
-      return EFI_SUCCESS;
-    }
-  }
-
-  // Check we did get some memory. Buffer is BlockSize.
-  if (Instance->ShadowBuffer == NULL) {
-    DEBUG ((DEBUG_ERROR, "FvbWrite: ERROR - Buffer not ready\n"));
-    return EFI_DEVICE_ERROR;
-  }
-
-  // Read NOR Flash data into shadow buffer
-  TempStatus = NorFlashReadBlocks (Instance, Lba, BlockSize,
-                 Instance->ShadowBuffer);
-  if (EFI_ERROR (TempStatus)) {
-    // Return one of the pre-approved error statuses
-    return EFI_DEVICE_ERROR;
-  }
-
-  // Put the data at the appropriate location inside the buffer area
-  CopyMem ((VOID*)((UINTN)Instance->ShadowBuffer + Offset), Buffer, *NumBytes);
-
-  // Write the modified buffer back to the NorFlash
-  TempStatus = NorFlashWriteBlocks (Instance, Lba, BlockSize,
-                 Instance->ShadowBuffer);
-  if (EFI_ERROR (TempStatus)) {
-    // Return one of the pre-approved error statuses
-    return EFI_DEVICE_ERROR;
-  }
-
-  return EFI_SUCCESS;
-}
-
-STATIC CONST NOR_FLASH_INSTANCE mNorFlashInstanceTemplate = {
-  NOR_FLASH_SIGNATURE, // Signature
-  NULL, // Handle ... NEED TO BE FILLED
-
-  FALSE, // Initialized
-  NULL, // Initialize
-
-  0, // HostRegisterBaseAddress ... NEED TO BE FILLED
-  0, // DeviceBaseAddress ... NEED TO BE FILLED
-  0, // RegionBaseAddress ... NEED TO BE FILLED
-  0, // Size ... NEED TO BE FILLED
-  0, // BlockSize
-  0, // LastBlock
-  0, // StartLba
-  0, // OffsetLba
-
-  {
-    FvbGetAttributes, // GetAttributes
-    FvbSetAttributes, // SetAttributes
-    FvbGetPhysicalAddress,  // GetPhysicalAddress
-    FvbGetBlockSize,  // GetBlockSize
-    FvbRead,  // Read
-    FvbWrite, // Write
-    FvbEraseBlocks, // EraseBlocks
-    NULL, //ParentHandle
-  }, //  FvbProtoccol;
-
-  NULL, // ShadowBuffer
-  {
-    {
-      {
-        HARDWARE_DEVICE_PATH,
-        HW_VENDOR_DP,
-        {
-          (UINT8)(OFFSET_OF (NOR_FLASH_DEVICE_PATH, End)),
-          (UINT8)(OFFSET_OF (NOR_FLASH_DEVICE_PATH, End) >> 8)
-        }
-      },
-      { 0x0, 0x0, 0x0, { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0 } },
-    },
-    0,  // Index
-    {
-      END_DEVICE_PATH_TYPE,
-      END_ENTIRE_DEVICE_PATH_SUBTYPE,
-      { sizeof (EFI_DEVICE_PATH_PROTOCOL), 0 }
-    }
-  }, // DevicePath
-
-  NULL, // CmdTable
-  0, // CmdTableSize
-  0 // Flags
-};
-
-STATIC
-EFI_STATUS
-NorFlashCreateInstance (
-  IN UINTN                  HostRegisterBase,
-  IN UINTN                  NorFlashDeviceBase,
-  IN UINTN                  NorFlashRegionBase,
-  IN UINTN                  NorFlashSize,
-  IN UINT32                 Index,
-  IN UINT32                 BlockSize,
-  IN BOOLEAN                HasVarStore,
-  IN CONST CSDC_DEFINITION  *CommandTable,
-  IN UINTN                  CommandTableSize,
-  OUT NOR_FLASH_INSTANCE**  NorFlashInstance
-  )
-{
-  EFI_STATUS Status;
-  NOR_FLASH_INSTANCE* Instance;
-  NOR_FLASH_INFO *FlashInfo;
-  UINT8 JedecId[3];
-
-  ASSERT(NorFlashInstance != NULL);
-
-  Instance = AllocateRuntimeCopyPool (sizeof mNorFlashInstanceTemplate,
-                                      &mNorFlashInstanceTemplate);
-  if (Instance == NULL) {
-    return EFI_OUT_OF_RESOURCES;
-  }
-
-  Instance->HostRegisterBaseAddress = HostRegisterBase;
-  Instance->DeviceBaseAddress       = NorFlashDeviceBase;
-  Instance->RegionBaseAddress       = NorFlashRegionBase;
-  Instance->Size                    = NorFlashSize;
-  Instance->BlockSize               = BlockSize;
-  Instance->LastBlock               = (NorFlashSize / BlockSize) - 1;
-
-  Instance->OffsetLba = (NorFlashRegionBase - NorFlashDeviceBase) / BlockSize;
-
-  CopyGuid (&Instance->DevicePath.Vendor.Guid, &gEfiCallerIdGuid);
-  Instance->DevicePath.Index = (UINT8)Index;
-
-  Instance->CmdTable = CommandTable;
-  Instance->CmdTableSize = CommandTableSize;
-  NorFlashReset (Instance);
-
-  NorFlashReadID (Instance, JedecId);
-  Status = NorFlashGetInfo (JedecId, &FlashInfo, FALSE);
-  if (EFI_ERROR (Status)) {
-    goto FreeInstance;
-  }
-
-  NorFlashPrintInfo (FlashInfo);
-
-  Instance->Flags = 0;
-  if (FlashInfo->Flags & NOR_FLASH_WRITE_FSR) {
-    Instance->Flags = NOR_FLASH_POLL_FSR;
-  }
+  //
+  // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME
+  //
+  RuntimeMmioRegionSize = Instance->Size;
 
-  Instance->ShadowBuffer = AllocateRuntimePool (BlockSize);;
-  if (Instance->ShadowBuffer == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto FreeInstance;
-  }
+  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo,
+                  Instance->RegionBaseAddress, RuntimeMmioRegionSize,
+                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+  ASSERT_EFI_ERROR (Status);
 
-  if (HasVarStore) {
-    Instance->Initialize = NorFlashFvbInitialize;
-  }
+  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo,
+                  Instance->DeviceBaseAddress, SIZE_4KB,
+                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+  ASSERT_EFI_ERROR (Status);
 
-  Status = gBS->InstallMultipleProtocolInterfaces (
-                &Instance->Handle,
-                &gEfiDevicePathProtocolGuid, &Instance->DevicePath,
-                &gEfiFirmwareVolumeBlockProtocolGuid, &Instance->FvbProtocol,
-                NULL
-                );
-  if (EFI_ERROR (Status)) {
-    goto FreeInstance;
-  }
+  Status = gDS->SetMemorySpaceAttributes (Instance->RegionBaseAddress,
+                  RuntimeMmioRegionSize, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+  ASSERT_EFI_ERROR (Status);
 
-  *NorFlashInstance = Instance;
-  FreePool (FlashInfo);
-  return EFI_SUCCESS;
+  Status = gDS->SetMemorySpaceAttributes (Instance->DeviceBaseAddress,
+                  SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
+  ASSERT_EFI_ERROR (Status);
 
-FreeInstance:
-  FreePool (Instance);
   return Status;
 }
 
-EFI_STATUS
-NorFlashReset (
-  IN  NOR_FLASH_INSTANCE *Instance
-  )
-{
-  FIP006_CS_CFG         CsCfg;
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashReset()\n"));
-  CsCfg.Raw = MmioRead32 (Instance->HostRegisterBaseAddress +
-                          FIP006_REG_CS_CFG); 
-  CsCfg.Reg.MBM = CS_CFG_MBM_SINGLE;
-  CsCfg.Reg.SRAM = CS_CFG_SRAM_RW;
-  MmioWrite32 (Instance->HostRegisterBaseAddress + FIP006_REG_CS_CFG,
-               CsCfg.Raw);
-  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
-  NorFlashSetHostCSDC (Instance, TRUE, mFip006NullCmdSeq);
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-NorFlashReadID (
-  IN  NOR_FLASH_INSTANCE  *Instance,
-  OUT UINT8               JedecId[3]
-  )
-{
-  if (Instance == NULL || JedecId == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  NorFlashSetHostCommand (Instance, SPINOR_OP_RDID);
-  JedecId[0] = MmioRead8 (Instance->DeviceBaseAddress);
-  JedecId[1] = MmioRead8 (Instance->DeviceBaseAddress + 1);
-  JedecId[2] = MmioRead8 (Instance->DeviceBaseAddress + 2);
-  NorFlashSetHostCommand (Instance, SPINOR_OP_READ_4B);
-  return EFI_SUCCESS;
-}
-
 /**
   Fixup internal data so that EFI can be call in virtual mode.
   Call the passed in Child Notify event and convert any pointers in
@@ -1078,6 +173,8 @@ NorFlashVirtualNotifyEvent (
 {
   UINTN Index;
 
+  EfiConvertPointer (0x0, (VOID**)&mFlashNvStorageVariableBase);
+
   for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
     EfiConvertPointer (0x0,
       (VOID**)&mNorFlashInstances[Index]->HostRegisterBaseAddress);
@@ -1105,8 +202,6 @@ NorFlashVirtualNotifyEvent (
     if (mNorFlashInstances[Index]->ShadowBuffer != NULL) {
       EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->ShadowBuffer);
     }
-
-    EfiConvertPointer (0x0, (VOID**)&mNorFlashInstances[Index]->CmdTable);
   }
 
   return;
@@ -1174,17 +269,24 @@ NorFlashInitialise (
       Index,
       NorFlashDevices[Index].BlockSize,
       ContainVariableStorage,
-      mN25qCSDCDefTable,
-      ARRAY_SIZE (mN25qCSDCDefTable),
       &mNorFlashInstances[Index]
     );
     if (EFI_ERROR (Status)) {
       DEBUG ((DEBUG_ERROR,
         "NorFlashInitialise: Fail to create instance for NorFlash[%d]\n",
         Index));
+      continue;
     }
+    Status = gBS->InstallMultipleProtocolInterfaces (
+                    &mNorFlashInstances[Index]->Handle,
+                    &gEfiDevicePathProtocolGuid, &mNorFlashInstances[Index]->DevicePath,
+                    &gEfiFirmwareVolumeBlockProtocolGuid, &mNorFlashInstances[Index]->FvbProtocol,
+                    NULL
+                    );
+    ASSERT_EFI_ERROR (Status);
   }
 
+
   //
   // Register for the virtual address change event
   //
@@ -1196,3 +298,31 @@ NorFlashInitialise (
 
   return Status;
 }
+
+VOID
+EFIAPI
+NorFlashLock (
+  NOR_FLASH_LOCK_CONTEXT    *Context
+  )
+{
+  if (!EfiAtRuntime ()) {
+    // Raise TPL to TPL_HIGH to stop anyone from interrupting us.
+    Context->OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
+  } else {
+    Context->InterruptsEnabled = SaveAndDisableInterrupts ();
+  }
+}
+
+VOID
+EFIAPI
+NorFlashUnlock (
+  NOR_FLASH_LOCK_CONTEXT    *Context
+  )
+{
+  if (!EfiAtRuntime ()) {
+    // Interruptions can resume.
+    gBS->RestoreTPL (Context->OriginalTPL);
+  } else if (Context->InterruptsEnabled) {
+    SetInterruptState (TRUE);
+  }
+}
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
similarity index 80%
rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c
rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
index 776ec8a5437c..30b7442d8947 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvb.c
@@ -21,16 +21,14 @@
 
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
-#include <Library/HobLib.h>
 #include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiLib.h>
 
-#include "NorFlashDxe.h"
+#include "NorFlash.h"
 
-STATIC EFI_EVENT mFvbVirtualAddrChangeEvent;
-STATIC UINTN     mFlashNvStorageVariableBase;
+UINTN     mFlashNvStorageVariableBase;
 
 ///
 /// The Firmware Volume Block Protocol is the low-level interface
@@ -49,7 +47,6 @@ STATIC UINTN     mFlashNvStorageVariableBase;
   @param[in]  Ptr - Location to initialise the headers
 
 **/
-STATIC
 EFI_STATUS
 InitializeFvAndVariableStoreHeaders (
   IN NOR_FLASH_INSTANCE *Instance
@@ -700,157 +697,3 @@ FvbEraseBlocks (
 EXIT:
   return Status;
 }
-
-/**
-  Fixup internal data so that EFI can be call in virtual mode.
-  Call the passed in Child Notify event and convert any pointers in
-  lib to virtual mode.
-
-  @param[in]    Event   The Event that is being processed
-  @param[in]    Context Event Context
-**/
-STATIC
-VOID
-EFIAPI
-FvbVirtualNotifyEvent (
-  IN EFI_EVENT        Event,
-  IN VOID             *Context
-  )
-{
-  EfiConvertPointer (0x0, (VOID**)&mFlashNvStorageVariableBase);
-  return;
-}
-
-EFI_STATUS
-EFIAPI
-NorFlashFvbInitialize (
-  IN NOR_FLASH_INSTANCE* Instance
-  )
-{
-  EFI_STATUS      Status;
-  UINT32          FvbNumLba;
-  EFI_BOOT_MODE   BootMode;
-  UINTN           RuntimeMmioRegionSize;
-  UINTN           BlockSize;
-
-  DEBUG ((DEBUG_BLKIO,"NorFlashFvbInitialize\n"));
-
-  BlockSize = Instance->BlockSize;
-
-  // FirmwareVolumeHeader->FvLength is declared to have the Variable area
-  // AND the FTW working area AND the FTW Spare contiguous.
-  ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) +
-         PcdGet32(PcdFlashNvStorageVariableSize) ==
-         PcdGet32(PcdFlashNvStorageFtwWorkingBase));
-  ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) +
-         PcdGet32(PcdFlashNvStorageFtwWorkingSize) ==
-         PcdGet32(PcdFlashNvStorageFtwSpareBase));
-
-  // Check if the size of the area is at least one block size
-  ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) &&
-         (PcdGet32(PcdFlashNvStorageVariableSize) / BlockSize > 0));
-  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingSize) > 0) &&
-         (PcdGet32(PcdFlashNvStorageFtwWorkingSize) / BlockSize > 0));
-  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) &&
-         (PcdGet32(PcdFlashNvStorageFtwSpareSize) / BlockSize > 0));
-
-  // Ensure the Variable areas are aligned on block size boundaries
-  ASSERT((PcdGet32(PcdFlashNvStorageVariableBase) % BlockSize) == 0);
-  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingBase) % BlockSize) == 0);
-  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareBase) % BlockSize) == 0);
-
-
-  Instance->Initialized = TRUE;
-  mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
-
-  // Set the index of the first LBA for the FVB
-  Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) -
-                        Instance->RegionBaseAddress) / BlockSize;
-
-  BootMode = GetBootModeHob ();
-  if (BootMode == BOOT_WITH_DEFAULT_SETTINGS) {
-    Status = EFI_INVALID_PARAMETER;
-  } else {
-    // Determine if there is a valid header at the beginning of the NorFlash
-    Status = ValidateFvHeader (Instance);
-  }
-
-  // Install the Default FVB header if required
-  if (EFI_ERROR(Status)) {
-    // There is no valid header, so time to install one.
-    DEBUG ((DEBUG_INFO, "%a: The FVB Header is not valid.\n", __FUNCTION__));
-    DEBUG ((DEBUG_INFO, "%a: Installing a correct one for this volume.\n",
-      __FUNCTION__));
-
-    // Erase all the NorFlash that is reserved for variable storage
-    FvbNumLba = (PcdGet32(PcdFlashNvStorageVariableSize) +
-                 PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
-                 PcdGet32(PcdFlashNvStorageFtwSpareSize)) /
-                Instance->BlockSize;
-
-    Status = FvbEraseBlocks (&Instance->FvbProtocol, (EFI_LBA)0, FvbNumLba,
-               EFI_LBA_LIST_TERMINATOR);
-    if (EFI_ERROR(Status)) {
-      return Status;
-    }
-
-    // Install all appropriate headers
-    Status = InitializeFvAndVariableStoreHeaders (Instance);
-    if (EFI_ERROR(Status)) {
-      return Status;
-    }
-  }
-
-  //
-  // The driver implementing the variable read service can now be dispatched;
-  // the varstore headers are in place.
-  //
-  Status = gBS->InstallProtocolInterface (&gImageHandle,
-                  &gEdkiiNvVarStoreFormattedGuid,
-                  EFI_NATIVE_INTERFACE,
-                  NULL);
-  if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR,
-      "%a: Failed to install gEdkiiNvVarStoreFormattedGuid\n",
-      __FUNCTION__));
-      return Status;
-  }
-
-  //
-  // Declare the Non-Volatile storage as EFI_MEMORY_RUNTIME
-  //
-  RuntimeMmioRegionSize = Instance->Size;
-
-  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo,
-                  Instance->RegionBaseAddress, RuntimeMmioRegionSize,
-                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
-  ASSERT_EFI_ERROR (Status);
-
-  Status = gDS->AddMemorySpace (EfiGcdMemoryTypeMemoryMappedIo,
-                  Instance->DeviceBaseAddress, SIZE_4KB,
-                  EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
-  ASSERT_EFI_ERROR (Status);
-
-  Status = gDS->SetMemorySpaceAttributes (Instance->RegionBaseAddress,
-                  RuntimeMmioRegionSize, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
-  ASSERT_EFI_ERROR (Status);
-
-  Status = gDS->SetMemorySpaceAttributes (Instance->DeviceBaseAddress,
-                  SIZE_4KB, EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
-  ASSERT_EFI_ERROR (Status);
-
-  //
-  // Register for the virtual address change event
-  //
-  Status = gBS->CreateEventEx (
-                  EVT_NOTIFY_SIGNAL,
-                  TPL_NOTIFY,
-                  FvbVirtualNotifyEvent,
-                  NULL,
-                  &gEfiEventVirtualAddressChangeGuid,
-                  &mFvbVirtualAddrChangeEvent
-                  );
-  ASSERT_EFI_ERROR (Status);
-
-  return Status;
-}
-- 
2.17.1



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

* [PATCH edk2-platforms 3/7] Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 4/7] Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access Ard Biesheuvel
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

Implement a variant of the FIP006 NOR flash driver that can execute
in standalone MM context. This is the foundation for hosting the
EFI authenticated variable store in the secure world.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf |  71 ++++++++
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c          | 182 ++++++++++++++++++++
 2 files changed, 253 insertions(+)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
new file mode 100644
index 000000000000..2dcbfd7db892
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
@@ -0,0 +1,71 @@
+## @file
+#  Socionext FIP006 High-Speed SPI Controller with NOR Flash Driver
+#
+#  Copyright (c) 2017, Socionext Inc. All rights reserved.<BR>
+#  Copyright (c) 2017-2018, Linaro, 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                    = 0x0001001A
+  BASE_NAME                      = Fip006Dxe
+  FILE_GUID                      = 1b041d85-9b44-442b-a583-5cf008ef9060
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 0.1
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  ENTRY_POINT                    = NorFlashInitialise
+
+[Sources]
+  NorFlashSmm.c
+  NorFlash.c
+  NorFlash.h
+  NorFlashFvb.c
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  IoLib
+  MemoryAllocationLib
+  MmServicesTableLib
+  NorFlashInfoLib
+  NorFlashPlatformLib
+  StandaloneMmDriverEntryPoint
+
+[Guids]
+  gEfiAuthenticatedVariableGuid
+  gEfiSystemNvDataFvGuid
+  gEfiVariableGuid
+
+[Protocols]
+  gEfiSmmFirmwareVolumeBlockProtocolGuid
+
+[FixedPcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+  gFip006DxeTokenSpaceGuid.PcdFip006DxeRegBaseAddress
+  gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress
+
+[Depex]
+  TRUE
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c
new file mode 100644
index 000000000000..bab3d9f4cd14
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c
@@ -0,0 +1,182 @@
+/** @file  NorFlashSmm.c
+
+  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2017, Socionext Inc. All rights reserved.<BR>
+  Copyright (c) 2017, Linaro, 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.
+
+**/
+
+#include <PiMm.h>
+
+#include <Library/MmServicesTableLib.h>
+
+#include "NorFlash.h"
+
+//
+// Global variable declarations
+//
+STATIC NOR_FLASH_INSTANCE   **mNorFlashInstances;
+STATIC UINT32               mNorFlashDeviceCount;
+
+EFI_STATUS
+EFIAPI
+NorFlashFvbInitialize (
+  IN NOR_FLASH_INSTANCE* Instance
+  )
+{
+  EFI_STATUS      Status;
+  UINT32          FvbNumLba;
+  UINTN           BlockSize;
+
+  DEBUG ((DEBUG_BLKIO,"NorFlashFvbInitialize\n"));
+
+  BlockSize = Instance->BlockSize;
+
+  // FirmwareVolumeHeader->FvLength is declared to have the Variable area
+  // AND the FTW working area AND the FTW Spare contiguous.
+  ASSERT (PcdGet32 (PcdFlashNvStorageVariableBase) +
+          PcdGet32 (PcdFlashNvStorageVariableSize) ==
+          PcdGet32 (PcdFlashNvStorageFtwWorkingBase));
+  ASSERT (PcdGet32 (PcdFlashNvStorageFtwWorkingBase) +
+          PcdGet32 (PcdFlashNvStorageFtwWorkingSize) ==
+          PcdGet32 (PcdFlashNvStorageFtwSpareBase));
+
+  // Check if the size of the area is at least one block size
+  ASSERT ((PcdGet32 (PcdFlashNvStorageVariableSize) > 0) &&
+          (PcdGet32 (PcdFlashNvStorageVariableSize) / BlockSize > 0));
+  ASSERT ((PcdGet32 (PcdFlashNvStorageFtwWorkingSize) > 0) &&
+          (PcdGet32 (PcdFlashNvStorageFtwWorkingSize) / BlockSize > 0));
+  ASSERT ((PcdGet32 (PcdFlashNvStorageFtwSpareSize) > 0) &&
+          (PcdGet32 (PcdFlashNvStorageFtwSpareSize) / BlockSize > 0));
+
+  // Ensure the Variable areas are aligned on block size boundaries
+  ASSERT ((PcdGet32 (PcdFlashNvStorageVariableBase) % BlockSize) == 0);
+  ASSERT ((PcdGet32 (PcdFlashNvStorageFtwWorkingBase) % BlockSize) == 0);
+  ASSERT ((PcdGet32 (PcdFlashNvStorageFtwSpareBase) % BlockSize) == 0);
+
+
+  Instance->Initialized = TRUE;
+  mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
+
+  // Set the index of the first LBA for the FVB
+  Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) -
+                        Instance->RegionBaseAddress) / BlockSize;
+
+  // Determine if there is a valid header at the beginning of the NorFlash
+  Status = ValidateFvHeader (Instance);
+  if (EFI_ERROR (Status)) {
+    // There is no valid header, so time to install one.
+    DEBUG ((DEBUG_INFO, "%a: The FVB Header is not valid.\n", __FUNCTION__));
+    DEBUG ((DEBUG_INFO, "%a: Installing a correct one for this volume.\n",
+      __FUNCTION__));
+
+    // Erase all the NorFlash that is reserved for variable storage
+    FvbNumLba = (PcdGet32(PcdFlashNvStorageVariableSize) +
+                 PcdGet32(PcdFlashNvStorageFtwWorkingSize) +
+                 PcdGet32(PcdFlashNvStorageFtwSpareSize)) /
+                Instance->BlockSize;
+
+    Status = FvbEraseBlocks (&Instance->FvbProtocol, (EFI_LBA)0, FvbNumLba,
+               EFI_LBA_LIST_TERMINATOR);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+
+    // Install all appropriate headers
+    Status = InitializeFvAndVariableStoreHeaders (Instance);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
+  }
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+EFIAPI
+NorFlashInitialise (
+  IN EFI_HANDLE            ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE   *MmSystemTable
+  )
+{
+  EFI_STATUS              Status;
+  UINT32                  Index;
+  NOR_FLASH_DESCRIPTION*  NorFlashDevices;
+  BOOLEAN                 ContainVariableStorage;
+
+  Status = NorFlashPlatformInitialization ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,
+      "NorFlashInitialise: Fail to initialize Nor Flash devices\n"));
+    return Status;
+  }
+
+  // Initialize NOR flash instances
+  Status = NorFlashPlatformGetDevices (&NorFlashDevices, &mNorFlashDeviceCount);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR,"NorFlashInitialise: Fail to get Nor Flash devices\n"));
+    return Status;
+  }
+
+  mNorFlashInstances = AllocatePool (sizeof(NOR_FLASH_INSTANCE*) *
+                                     mNorFlashDeviceCount);
+
+  for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
+    // Check if this NOR Flash device contain the variable storage region
+    ContainVariableStorage =
+        (NorFlashDevices[Index].RegionBaseAddress <=
+         PcdGet32 (PcdFlashNvStorageVariableBase)) &&
+        (PcdGet32 (PcdFlashNvStorageVariableBase) +
+         PcdGet32 (PcdFlashNvStorageVariableSize) <=
+        NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+
+    Status = NorFlashCreateInstance (
+      PcdGet32 (PcdFip006DxeRegBaseAddress),
+      NorFlashDevices[Index].DeviceBaseAddress,
+      NorFlashDevices[Index].RegionBaseAddress,
+      NorFlashDevices[Index].Size,
+      Index,
+      NorFlashDevices[Index].BlockSize,
+      ContainVariableStorage,
+      &mNorFlashInstances[Index]
+    );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_ERROR,
+        "NorFlashInitialise: Fail to create instance for NorFlash[%d]\n",
+        Index));
+      continue;
+    }
+    Status = gMmst->MmInstallProtocolInterface (
+                      &mNorFlashInstances[Index]->Handle,
+                      &gEfiSmmFirmwareVolumeBlockProtocolGuid,
+                      EFI_NATIVE_INTERFACE,
+                      &mNorFlashInstances[Index]->FvbProtocol
+                      );
+    ASSERT_EFI_ERROR (Status);
+  }
+
+  return Status;
+}
+
+VOID
+EFIAPI
+NorFlashLock (
+  NOR_FLASH_LOCK_CONTEXT    *Context
+  )
+{
+}
+
+VOID
+EFIAPI
+NorFlashUnlock (
+  NOR_FLASH_LOCK_CONTEXT    *Context
+  )
+{
+}
-- 
2.17.1



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

* [PATCH edk2-platforms 4/7] Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2019-01-04 14:43 ` [PATCH edk2-platforms 3/7] Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 5/7] Platform/DeveloperBox: create shared .DSC include file Ard Biesheuvel
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

This code may execute in SMM context, where unaligned accesses are
not permitted. So use ReadUnaligned32() instead of performing a
direct UINT32* cast.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
index 2134739bfba9..d45c8d9b35d2 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
@@ -841,7 +841,7 @@ NorFlashWriteSingleBlock (
                    BlockSize);
 
       // The word of data that is to be written.
-      TmpBuf = *((UINT32*)(Buffer + (*NumBytes - BytesToWrite)));
+      TmpBuf = ReadUnaligned32 ((UINT32 *)(Buffer + (*NumBytes - BytesToWrite)));
 
       // First do word aligned chunks.
       if ((CurOffset & 0x3) == 0) {
-- 
2.17.1



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

* [PATCH edk2-platforms 5/7] Platform/DeveloperBox: create shared .DSC include file
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2019-01-04 14:43 ` [PATCH edk2-platforms 4/7] Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-04 14:43 ` [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components Ard Biesheuvel
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

We are going to add a separate .DSC/.FDF combo for the standalone
MM components. So put all the pieces we will share in an include
file that both .DSC files can include.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc     | 280 +-----------------
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc | 312 ++++++++++++++++++++
 2 files changed, 317 insertions(+), 275 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index ed11aed798b7..56787a744157 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -28,11 +28,7 @@ [Defines]
   FLASH_DEFINITION               = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
   BUILD_NUMBER                   = 1
 
-[BuildOptions]
-  RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
-
-  # add ample padding to the DTC so we can apply 96boards mezzanine overlays
-  *_*_*_DTC_FLAGS = -p 1024
+!include Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
 
 [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
@@ -41,101 +37,6 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
 
-[LibraryClasses.common]
-  ArmPlatformLib|Silicon/Socionext/SynQuacer/Library/SynQuacerLib/SynQuacerLib.inf
-  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
-  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
-
-  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
-  FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
-
-!if $(TARGET) == RELEASE
-  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
-!else
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!endif
-  DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
-
-  BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
-
-  # Networking Requirements
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-
-  # ARM Architectural Libraries
-  CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
-  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
-  CpuExceptionHandlerLib|ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
-  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
-  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
-  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
-  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
-  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
-  ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
-  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
-  ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
-  OpteeLib|ArmPkg/Library/OpteeLib/OpteeLib.inf
-
-  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
-  BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
-  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
-  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
-  PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
-  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-  PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
-  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
-  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
-  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
-  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
-  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-
-  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
-  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
-  UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
-  DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
-  UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
-  DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
-  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
-  UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
-  HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
-  UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
-  ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
-  UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
-  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
-  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
-  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
-  UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
-  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
-
-  # BDS Libraries
-  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
-  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
-  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
-  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
-  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
-
-  PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
-  DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
-  DebugAgentTimerLib|EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf
-  PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
-  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
-  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
-
-  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
-  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
-
-  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
-  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
-
-  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
-  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
-  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
-  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
-
-  NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
-
 [LibraryClasses.common.SEC]
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
@@ -220,61 +121,12 @@ [LibraryClasses.common.DXE_RUNTIME_DRIVER]
 #
 ################################################################################
 
-[PcdsFeatureFlag]
-  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|TRUE
-
-  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
-  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|FALSE
-
-  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|TRUE
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|TRUE
-
-  # needed for NFIT tables installed by RamDiskDxe
-  gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
-
-[PcdsFixedAtBuild.common]
-!ifdef $(FIRMWARE_VENDOR)
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"$(FIRMWARE_VENDOR)"
-!endif
-
-  # non-secure SRAM
-  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
-  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
-  gSynQuacerTokenSpaceGuid.PcdDramInfoBase|0x2E00FFC0
-
-  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
-
-  gArmPlatformTokenSpaceGuid.PcdCoreCount|2
-  gArmPlatformTokenSpaceGuid.PcdClusterCount|12
-  gArmTokenSpaceGuid.PcdArmPrimaryCore|0x0
-!if $(ARCH) == AARCH64
-  gArmTokenSpaceGuid.PcdVFPEnabled|1
-!endif
-
-  ## PL011 - Serial Terminal
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2a400000
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
-  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
-  gArmPlatformTokenSpaceGuid.PL011UartInteger|0
-  gArmPlatformTokenSpaceGuid.PL011UartFractional|0
-  gArmPlatformTokenSpaceGuid.PL011UartClkInHz|62500000
-
-  #
-  # ARM Generic Interrupt Controller
-  #
-  gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
-  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x30400000
-
-  #
-  # Generic watchdog
-  #
-  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x2a440000
-  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x2a450000
-
+[PcdsFixedAtBuild]
   #
   # NETSEC Info
   #
+  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
+
   gNetsecDxeTokenSpaceGuid.PcdEncTxDescNum|128
   gNetsecDxeTokenSpaceGuid.PcdDecRxDescNum|128
   gNetsecDxeTokenSpaceGuid.PcdJumboPacket|0
@@ -286,121 +138,10 @@ [PcdsFixedAtBuild.common]
   gSynQuacerTokenSpaceGuid.PcdNetsecEepromBase|0x08080000
   gSynQuacerTokenSpaceGuid.PcdNetsecPhyAddress|7
 
-  gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections|TRUE
-
-  gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
-  gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
-  gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
-  gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000
-  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
-  gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
-  gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
-
-  # DEBUG_ASSERT_ENABLED       0x01
-  # DEBUG_PRINT_ENABLED        0x02
-  # DEBUG_CODE_ENABLED         0x04
-  # CLEAR_MEMORY_ENABLED       0x08
-  # ASSERT_BREAKPOINT_ENABLED  0x10
-  # ASSERT_DEADLOOP_ENABLED    0x20
-!if $(TARGET) == RELEASE
-  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x21
-!else
-  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x3f
-!endif
-
-  #  DEBUG_INIT      0x00000001  // Initialization
-  #  DEBUG_WARN      0x00000002  // Warnings
-  #  DEBUG_LOAD      0x00000004  // Load events
-  #  DEBUG_FS        0x00000008  // EFI File system
-  #  DEBUG_POOL      0x00000010  // Alloc & Free (pool)
-  #  DEBUG_PAGE      0x00000020  // Alloc & Free (page)
-  #  DEBUG_INFO      0x00000040  // Informational debug messages
-  #  DEBUG_DISPATCH  0x00000080  // PEI/DXE/SMM Dispatchers
-  #  DEBUG_VARIABLE  0x00000100  // Variable
-  #  DEBUG_BM        0x00000400  // Boot Manager
-  #  DEBUG_BLKIO     0x00001000  // BlkIo Driver
-  #  DEBUG_NET       0x00004000  // SNP Driver
-  #  DEBUG_UNDI      0x00010000  // UNDI Driver
-  #  DEBUG_LOADFILE  0x00020000  // LoadFile
-  #  DEBUG_EVENT     0x00080000  // Event messages
-  #  DEBUG_GCD       0x00100000  // Global Coherency Database changes
-  #  DEBUG_CACHE     0x00200000  // Memory range cachability changes
-  #  DEBUG_VERBOSE   0x00400000  // Detailed debug messages that may
-  #                              // significantly impact boot performance
-  #  DEBUG_ERROR     0x80000000  // Error
-  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
-
-  #
-  # Optional feature to help prevent EFI memory map fragments
-  # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
-  # Values are in EFI Pages (4K). DXE Core will make sure that
-  # at least this much of each type of memory can be allocated
-  # from a single memory range. This way you only end up with
-  # maximum of two fragements for each type in the memory map
-  # (the memory used, and the free memory that was prereserved
-  # but not used).
-  #
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|2000
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|1000
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|2000
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|20000
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
-  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
-
-  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
-
-  # use the TTY terminal type
-  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
-
-  # GUID of the UI app
-  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
-
-  #
-  # Enable strict image permissions for all images. (This applies
-  # only to images that were built with >= 4 KB section alignment.)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
-
-  #
-  # Enable NX memory protection for all non-code regions, including OEM and OS
-  # reserved ones, with the exception of LoaderData regions, of which OS loaders
-  # (i.e., GRUB) may assume that its contents are executable.
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
-
-  #
-  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
-  #
-  # Variable store
-  #
-  gFip006DxeTokenSpaceGuid.PcdFip006DxeRegBaseAddress|0x54800000
-  gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
-
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
-
-  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
-  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR
-  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId|0x4f524e4c # LNRO
-  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|1
-
   # set DIP switch DSW3-PIN1 (GPIO pin PD[0] on the SoC) to clear the varstore
   gSynQuacerTokenSpaceGuid.PcdClearSettingsGpioPin|0
 
-!if $(BUILD_NUMBER) > 1
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
-!endif
-  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
+  gSynQuacerTokenSpaceGuid.PcdDramInfoBase|0x2E00FFC0
 
   #
   # 96boards mezzanine support
@@ -424,24 +165,13 @@ [PcdsFixedAtBuild.common]
   g96BoardsTokenSpaceGuid.PcdGpioPinK|24
   g96BoardsTokenSpaceGuid.PcdGpioPinL|25
 
-[PcdsPatchableInModule]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
-  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
-
 [PcdsDynamicExDefault.common.DEFAULT]
   gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor|{0x0}|VOID*|0x100
   gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareFileGuid|{0xf7, 0x89, 0x9b, 0xe9, 0x20, 0xc1, 0x25, 0x4b, 0x4d, 0xb1, 0x83, 0x94, 0xed, 0xb0, 0xb4, 0xf5}
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSystemFmpCapsuleImageTypeIdGuid|{0xe5, 0x4c, 0xb9, 0x50, 0x63, 0x8b, 0x49, 0x48, 0x8a, 0xf4, 0xea, 0x47, 0x93, 0x56, 0xf0, 0xe3}
 
 [PcdsDynamicHii]
-  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
-
   gSynQuacerTokenSpaceGuid.PcdPlatformSettings|L"SynQuacerPlatformSettings"|gSynQuacerPlatformFormSetGuid|0x0|0x0|NV,BS
 
-[PcdsDynamicDefault]
-  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0000000000000000
-  gArmTokenSpaceGuid.PcdSystemMemorySize|0xFFFFFFFFFFFFFFFF
-
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
new file mode 100644
index 000000000000..56adc21d5caf
--- /dev/null
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
@@ -0,0 +1,312 @@
+#
+#  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+#  Copyright (c) 2017-2018, Linaro Limited. All rights reserved.
+#
+#  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 Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[BuildOptions]
+  RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
+
+  # add ample padding to the DTC so we can apply 96boards mezzanine overlays
+  *_*_*_DTC_FLAGS = -p 1024
+
+[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
+[BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
+  GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
+  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
+
+[LibraryClasses.common]
+  ArmPlatformLib|Silicon/Socionext/SynQuacer/Library/SynQuacerLib/SynQuacerLib.inf
+  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
+  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+
+  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
+  FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
+
+!if $(TARGET) == RELEASE
+  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+!else
+  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+!endif
+  DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
+
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
+
+  # Networking Requirements
+  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
+  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
+  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
+  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
+
+  # ARM Architectural Libraries
+  CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
+  DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
+  CpuExceptionHandlerLib|ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
+  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
+  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
+  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
+  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
+  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
+  ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
+  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
+  ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
+  OpteeLib|ArmPkg/Library/OpteeLib/OpteeLib.inf
+
+  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
+  BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
+  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
+  PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
+  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
+  PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
+  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
+  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
+  UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
+  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
+
+  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
+  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
+  UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
+  DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
+  UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
+  DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
+  UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
+  UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
+  HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
+  UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
+  ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
+  UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+  MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
+  DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
+  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
+  UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
+  UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
+
+  # BDS Libraries
+  UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+  PlatformBootManagerLib|ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
+  CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
+  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
+
+  PeCoffExtraActionLib|ArmPkg/Library/DebugPeCoffExtraActionLib/DebugPeCoffExtraActionLib.inf
+  DebugAgentLib|MdeModulePkg/Library/DebugAgentLibNull/DebugAgentLibNull.inf
+  DebugAgentTimerLib|EmbeddedPkg/Library/DebugAgentTimerLibNull/DebugAgentTimerLibNull.inf
+  PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
+  SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
+  PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
+
+  HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
+  TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
+
+  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
+  NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+  TlsLib|CryptoPkg/Library/TlsLib/TlsLib.inf
+  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+  PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
+
+  NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFeatureFlag]
+  gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob|TRUE
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutGopSupport|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdConOutUgaSupport|FALSE
+  gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport|FALSE
+
+  gArmTokenSpaceGuid.PcdArmReenterPeiForCapsuleWarmReboot|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSupportUpdateCapsuleReset|TRUE
+
+  # needed for NFIT tables installed by RamDiskDxe
+  gEfiMdeModulePkgTokenSpaceGuid.PcdInstallAcpiSdtProtocol|TRUE
+
+[PcdsFixedAtBuild.common]
+!ifdef $(FIRMWARE_VENDOR)
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"$(FIRMWARE_VENDOR)"
+!endif
+
+  # non-secure SRAM
+  gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x2E000000
+  gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0xFFC0
+
+  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|24
+
+  gArmPlatformTokenSpaceGuid.PcdCoreCount|2
+  gArmPlatformTokenSpaceGuid.PcdClusterCount|12
+  gArmTokenSpaceGuid.PcdArmPrimaryCore|0x0
+!if $(ARCH) == AARCH64
+  gArmTokenSpaceGuid.PcdVFPEnabled|1
+!endif
+
+  ## PL011 - Serial Terminal
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x2a400000
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
+  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultReceiveFifoDepth|0
+  gArmPlatformTokenSpaceGuid.PL011UartInteger|0
+  gArmPlatformTokenSpaceGuid.PL011UartFractional|0
+  gArmPlatformTokenSpaceGuid.PL011UartClkInHz|62500000
+
+  #
+  # ARM Generic Interrupt Controller
+  #
+  gArmTokenSpaceGuid.PcdGicDistributorBase|0x30000000
+  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0x30400000
+
+  #
+  # Generic watchdog
+  #
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x2a440000
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x2a450000
+
+  gEfiMdePkgTokenSpaceGuid.PcdMaximumUnicodeStringLength|1000000
+  gEfiMdePkgTokenSpaceGuid.PcdMaximumAsciiStringLength|1000000
+  gEfiMdePkgTokenSpaceGuid.PcdMaximumLinkedListLength|1000000
+  gEfiMdePkgTokenSpaceGuid.PcdSpinLockTimeout|10000000
+  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF
+  gEfiMdePkgTokenSpaceGuid.PcdPostCodePropertyMask|0
+  gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|320
+
+  # DEBUG_ASSERT_ENABLED       0x01
+  # DEBUG_PRINT_ENABLED        0x02
+  # DEBUG_CODE_ENABLED         0x04
+  # CLEAR_MEMORY_ENABLED       0x08
+  # ASSERT_BREAKPOINT_ENABLED  0x10
+  # ASSERT_DEADLOOP_ENABLED    0x20
+!if $(TARGET) == RELEASE
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x21
+!else
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x3f
+!endif
+
+  #  DEBUG_INIT      0x00000001  // Initialization
+  #  DEBUG_WARN      0x00000002  // Warnings
+  #  DEBUG_LOAD      0x00000004  // Load events
+  #  DEBUG_FS        0x00000008  // EFI File system
+  #  DEBUG_POOL      0x00000010  // Alloc & Free (pool)
+  #  DEBUG_PAGE      0x00000020  // Alloc & Free (page)
+  #  DEBUG_INFO      0x00000040  // Informational debug messages
+  #  DEBUG_DISPATCH  0x00000080  // PEI/DXE/SMM Dispatchers
+  #  DEBUG_VARIABLE  0x00000100  // Variable
+  #  DEBUG_BM        0x00000400  // Boot Manager
+  #  DEBUG_BLKIO     0x00001000  // BlkIo Driver
+  #  DEBUG_NET       0x00004000  // SNP Driver
+  #  DEBUG_UNDI      0x00010000  // UNDI Driver
+  #  DEBUG_LOADFILE  0x00020000  // LoadFile
+  #  DEBUG_EVENT     0x00080000  // Event messages
+  #  DEBUG_GCD       0x00100000  // Global Coherency Database changes
+  #  DEBUG_CACHE     0x00200000  // Memory range cachability changes
+  #  DEBUG_VERBOSE   0x00400000  // Detailed debug messages that may
+  #                              // significantly impact boot performance
+  #  DEBUG_ERROR     0x80000000  // Error
+  gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000004F
+
+  #
+  # Optional feature to help prevent EFI memory map fragments
+  # Turned on and off via: PcdPrePiProduceMemoryTypeInformationHob
+  # Values are in EFI Pages (4K). DXE Core will make sure that
+  # at least this much of each type of memory can be allocated
+  # from a single memory range. This way you only end up with
+  # maximum of two fragements for each type in the memory map
+  # (the memory used, and the free memory that was prereserved
+  # but not used).
+  #
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory|0
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS|0
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType|0
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData|2000
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesCode|1000
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesCode|2000
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiBootServicesData|20000
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
+  gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
+
+  # use the TTY terminal type
+  gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|4
+
+  # GUID of the UI app
+  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
+
+  #
+  # Enable strict image permissions for all images. (This applies
+  # only to images that were built with >= 4 KB section alignment.)
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
+
+  #
+  # Enable NX memory protection for all non-code regions, including OEM and OS
+  # reserved ones, with the exception of LoaderData regions, of which OS loaders
+  # (i.e., GRUB) may assume that its contents are executable.
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
+
+  #
+  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
+
+  #
+  # Variable store
+  #
+  gFip006DxeTokenSpaceGuid.PcdFip006DxeRegBaseAddress|0x54800000
+  gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress|0x08000000
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x08400000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00010000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x08410000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00010000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x08420000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00010000
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId|"SNI   "
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId|0x52434155514e5953 # SYNQUACR
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId|0x4f524e4c # LNRO
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|1
+
+!if $(BUILD_NUMBER) > 1
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString|L"$(BUILD_NUMBER)"
+!endif
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
+
+  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
+  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
+
+[PcdsPatchableInModule]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
+  gEfiMdeModulePkgTokenSpaceGuid.PcdVideoVerticalResolution|0
+
+[PcdsDynamicExDefault.common.DEFAULT]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSystemFmpCapsuleImageTypeIdGuid|{0xe5, 0x4c, 0xb9, 0x50, 0x63, 0x8b, 0x49, 0x48, 0x8a, 0xf4, 0xea, 0x47, 0x93, 0x56, 0xf0, 0xe3}
+
+[PcdsDynamicHii]
+  gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|5
+
+[PcdsDynamicDefault]
+  gArmTokenSpaceGuid.PcdSystemMemoryBase|0x0000000000000000
+  gArmTokenSpaceGuid.PcdSystemMemorySize|0xFFFFFFFFFFFFFFFF
-- 
2.17.1



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

* [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2019-01-04 14:43 ` [PATCH edk2-platforms 5/7] Platform/DeveloperBox: create shared .DSC include file Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-17 11:04   ` Leif Lindholm
  2019-01-04 14:43 ` [PATCH edk2-platforms 7/7] Platform/DeveloperBox: add MM based UEFI secure boot support Ard Biesheuvel
  2019-01-17 11:14 ` [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Leif Lindholm
  7 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

Create a pair of .DSC/.FDF files that describe the components and
the firmware volumes and flash device that will be dispatched into
a secure partition in the secure world to control the UEFI secure
variable store.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc     |   5 +-
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc |   7 +-
 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc   | 103 +++++++++++++
 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf   | 161 ++++++++++++++++++++
 4 files changed, 270 insertions(+), 6 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 56787a744157..666bd2716336 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -250,10 +250,7 @@ [Components.common]
   #
   # Variable services
   #
-  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf {
-    <LibraryClasses>
-      NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
-  }
+  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
     <LibraryClasses>
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
index 56adc21d5caf..87b2094cb356 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
@@ -125,6 +125,7 @@ [LibraryClasses.common]
   PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
 
   NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
+  NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
 
 ################################################################################
 #
@@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
 
-  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
-  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
+  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
 
 [PcdsPatchableInModule]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
new file mode 100644
index 000000000000..45cfd5645d9b
--- /dev/null
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
@@ -0,0 +1,103 @@
+#
+#  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+#  Copyright (c) 2017-2018, Linaro Limited. All rights reserved.
+#
+#  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 Section - statements that will be processed to create a Makefile.
+#
+################################################################################
+[Defines]
+  PLATFORM_NAME                  = DeveloperBoxMm
+  PLATFORM_GUID                  = cedcc3f9-fb42-45e6-b134-e7ca97abbffd
+  PLATFORM_VERSION               = 0.1
+  DSC_SPECIFICATION              = 0x0001001B
+  OUTPUT_DIRECTORY               = Build/$(PLATFORM_NAME)
+  SUPPORTED_ARCHITECTURES        = AARCH64
+  BUILD_TARGETS                  = DEBUG|RELEASE|NOOPT
+  SKUID_IDENTIFIER               = DEFAULT
+  FLASH_DEFINITION               = Platform/Socionext/DeveloperBox/$(PLATFORM_NAME).fdf
+  BUILD_NUMBER                   = 1
+
+!include Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+
+[LibraryClasses.common.MM_STANDALONE, LibraryClasses.common.MM_CORE_STANDALONE]
+  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
+  MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+  PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
+  StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+  StandaloneMmDriverEntryPoint|StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
+  StandaloneMmMmuLib|ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
+  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+
+  SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
+  PciLib|MdePkg/Library/BasePciLibPciExpress/BasePciLibPciExpress.inf
+  PciExpressLib|MdePkg/Library/BasePciExpressLib/BasePciExpressLib.inf
+
+[LibraryClasses.common.MM_CORE_STANDALONE]
+  HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
+  MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
+
+[LibraryClasses.common.MM_STANDALONE]
+  HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
+  MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
+  MmServicesTableLib|StandaloneMmPkg/Library/MmServicesTableLib/MmServicesTableLib.inf
+
+[BuildOptions.common.EDKII.MM_STANDALONE, BuildOptions.common.EDKII.MM_CORE_STANDALONE]
+  GCC:*_*_*_CC_FLAGS = -mstrict-align -mgeneral-regs-only
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
+################################################################################
+#
+# Pcd Section - list of all EDK II PCD Entries defined by this Platform
+#
+################################################################################
+
+[PcdsFixedAtBuild]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x51040000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|62500000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4
+
+[PcdsPatchableInModule]
+  gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x0
+
+################################################################################
+#
+# Components Section - list of all EDK II Modules needed by this Platform
+#
+################################################################################
+[Components.common]
+  #
+  # Standalone MM components
+  #
+  StandaloneMmPkg/Core/StandaloneMmCore.inf {
+    <LibraryClasses>
+      ExtractGuidedSectionLib|MdePkg/Library/BaseExtractGuidedSectionLib/BaseExtractGuidedSectionLib.inf
+      NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
+  }
+
+  StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
+  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf {
+    <LibraryClasses>
+      AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+      BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
+      VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+      NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+  }
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
new file mode 100644
index 000000000000..790a391080e6
--- /dev/null
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
@@ -0,0 +1,161 @@
+#
+#  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+#  Copyright (c) 2017-2018, Linaro Limited. All rights reserved.
+#
+#  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 Section
+# The [FD] Section is made up of the definition statements and a
+# description of what goes into  the Flash Device Image.  Each FD section
+# defines one flash "device" image.  A flash device image may be one of
+# the following: Removable media bootable image (like a boot floppy
+# image,) an Option ROM image (that would be "flashed" into an add-in
+# card,) a System "Flash"  image (that would be burned into a system's
+# flash) or an Update ("Capsule") image that will be used to update and
+# existing system flash.
+#
+################################################################################
+
+[FD.STANDALONE_MM]
+BaseAddress   = 0xfc000000
+Size          = 0x00040000
+ErasePolarity = 1
+
+# This one is tricky, it must be: BlockSize * NumBlocks = Size
+BlockSize     = 0x00010000
+NumBlocks     = 0x4
+
+################################################################################
+#
+# Following are lists of FD Region layout which correspond to the locations of different
+# images within the flash device.
+#
+# Regions must be defined in ascending order and may not overlap.
+#
+# A Layout Region start with a eight digit hex offset (leading "0x" required) followed by
+# the pipe "|" character, followed by the size of the region, also in hex with the leading
+# "0x" characters. Like:
+# Offset|Size
+# PcdOffsetCName|PcdSizeCName
+# RegionType <FV, DATA, or FILE>
+#
+################################################################################
+
+0x00000000|0x00040000
+FV = FvStandaloneMmCompact
+
+################################################################################
+#
+# FV Section
+#
+# [FV] section is used to define what components or modules are placed within a flash
+# device file.  This section also defines order the components and modules are positioned
+# within the image.  The [FV] section consists of define statements, set statements and
+# module statements.
+#
+################################################################################
+
+[FV.FvStandaloneMmCompact]
+FvNameGuid         = db7dcad2-de57-455d-a385-c297a6536cd8
+BlockSize          = 0x10000
+NumBlocks          = 0
+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 StandaloneMmPkg/Core/StandaloneMmCore.inf
+
+  FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
+    SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
+      SECTION FV_IMAGE = FVSTANDALONEMM
+    }
+  }
+
+[FV.FvStandaloneMm]
+FvNameGuid         = 2160589f-77f4-4cae-b9ba-0be99d2e33a4
+BlockSize          = 0x40
+NumBlocks          = 0         # This FV gets compressed so make it just big enough
+FvAlignment        = 8         # FV alignment and FV attributes setting.
+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 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
+  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+  INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+
+################################################################################
+#
+# Rules are use with the [FV] section's module INF type to define
+# how an FFS file is created for a given INF file. The following Rule are the default
+# rules for the different module type. User can add the customized rules to define the
+# content of the FFS file.
+#
+################################################################################
+
+
+############################################################################
+# Example of a DXE_DRIVER FFS file with a Checksum encapsulation section   #
+############################################################################
+#
+#[Rule.Common.DXE_DRIVER]
+#  FILE DRIVER = $(NAMED_GUID) {
+#    DXE_DEPEX    DXE_DEPEX               Optional $(INF_OUTPUT)/$(MODULE_NAME).depex
+#    COMPRESS PI_STD {
+#      GUIDED {
+#        PE32     PE32                    $(INF_OUTPUT)/$(MODULE_NAME).efi
+#        UI       STRING="$(MODULE_NAME)" Optional
+#        VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
+#      }
+#    }
+#  }
+#
+############################################################################
+
+[Rule.Common.MM_CORE_STANDALONE]
+  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
+    TE  TE Align = 4K                 $(INF_OUTPUT)/$(MODULE_NAME).efi
+  }
+
+[Rule.Common.MM_STANDALONE]
+  FILE MM_STANDALONE = $(NAMED_GUID) {
+    SMM_DEPEX SMM_DEPEX Optional       $(INF_OUTPUT)/$(MODULE_NAME).depex
+    PE32      PE32                     $(INF_OUTPUT)/$(MODULE_NAME).efi
+    UI        STRING="$(MODULE_NAME)" Optional
+    VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
+  }
-- 
2.17.1



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

* [PATCH edk2-platforms 7/7] Platform/DeveloperBox: add MM based UEFI secure boot support
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
                   ` (5 preceding siblings ...)
  2019-01-04 14:43 ` [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components Ard Biesheuvel
@ 2019-01-04 14:43 ` Ard Biesheuvel
  2019-01-17 11:14 ` [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Leif Lindholm
  7 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-04 14:43 UTC (permalink / raw)
  To: edk2-devel

This implements support for UEFI secure boot on DeveloperBox using
the standalone MM framework. This moves all of the software handling
of the UEFI authenticated variable store into the standalone MM
context residing in a secure partition.

Note that SynQuacer as configured today is not a truly secure
platform, since the NOR flash registers are accessible to the
non-secure world. However, from a software point of view, all
of the required pieces are in place. (In particular, it is no
longer possible for the OS to stub out authentication checks
in the validation code residing in RuntimeServicesCode regions)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 23 +++++++++++++++++++-
 Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 13 +++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 666bd2716336..d244048c5a6b 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -28,6 +28,8 @@ [Defines]
   FLASH_DEFINITION               = Platform/Socionext/DeveloperBox/DeveloperBox.fdf
   BUILD_NUMBER                   = 1
 
+  DEFINE SECURE_BOOT_ENABLE      = FALSE
+
 !include Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
 
 [BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
@@ -165,6 +167,13 @@ [PcdsFixedAtBuild]
   g96BoardsTokenSpaceGuid.PcdGpioPinK|24
   g96BoardsTokenSpaceGuid.PcdGpioPinL|25
 
+  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
+  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
+
+  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
+
 [PcdsDynamicExDefault.common.DEFAULT]
   gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor|{0x0}|VOID*|0x100
   gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareFileGuid|{0xf7, 0x89, 0x9b, 0xe9, 0x20, 0xc1, 0x25, 0x4b, 0x4d, 0xb1, 0x83, 0x94, 0xed, 0xb0, 0xb4, 0xf5}
@@ -223,7 +232,13 @@ [Components.common]
   }
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
+!if $(SECURE_BOOT_ENABLE) == TRUE
+    <LibraryClasses>
+      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+!endif
+  }
+
   ArmPkg/Drivers/TimerDxe/TimerDxe.inf
   ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
   MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
@@ -251,6 +266,7 @@ [Components.common]
   # Variable services
   #
   Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+!if $(SECURE_BOOT_ENABLE) == FALSE
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
     <LibraryClasses>
@@ -260,6 +276,11 @@ [Components.common]
       TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
       VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   }
+!else
+  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
 
   #
   # UEFI application (Shell Embedded Boot Loader)
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
index 4a234a36525e..7be40380efb4 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
@@ -51,7 +51,11 @@ [FD.SPI_NOR_IMAGE]
 ################################################################################
 
 0x00000000|0x00078000
+!if $(SECURE_BOOT_ENABLE) == FALSE
 FILE = Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin
+!else
+FILE = Platform/Socionext/DeveloperBox/fip_all_arm_tf_mm.bin
+!endif
 
 0x00078000|0x00008000
 FILE = $(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(ARCH)/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables/OUTPUT/Stage2Tables.bin
@@ -122,9 +126,15 @@ [FV.FvMain]
   #
   # Variable services
   #
+!if $(SECURE_BOOT_ENABLE) == FALSE
   INF Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
   INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+!else
+  INF ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
 
   #
   # UEFI applications
@@ -328,6 +338,9 @@ [FV.CapsuleDispatchFv]
 READ_LOCK_CAP      = TRUE
 READ_LOCK_STATUS   = TRUE
 
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  INF  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+!endif
   INF  SignedCapsulePkg/Universal/SystemFirmwareUpdate/SystemFirmwareUpdateDxe.inf
 
 [FV.SystemFirmwareUpdateCargo]
-- 
2.17.1



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

* Re: [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
  2019-01-04 14:43 ` [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines Ard Biesheuvel
@ 2019-01-17  9:50   ` Leif Lindholm
  2019-01-17 10:59     ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Leif Lindholm @ 2019-01-17  9:50 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Masahisa Kojima

On Fri, Jan 04, 2019 at 03:43:30PM +0100, Ard Biesheuvel wrote:
> The FIP006 NOR flash driver contains implementations of the block I/O
> and disk I/O protocols, but never exposes them to other drivers (i.e.,
> it never installs the protocol interfaces). So let's drop this code
> altogether: the NOR flash is for code and variables, not for arbitrary
> files.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Feel free to push this one in isolation - it's a useful improvement
anyway, may prevent someone from getting unfortunate ideas....

> ---
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf        |   3 -
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c | 138 ------------
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c        | 229 ++------------------
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h        |  51 +----
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c     |  40 ++--
>  5 files changed, 37 insertions(+), 424 deletions(-)


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

* Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-04 14:43 ` [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces Ard Biesheuvel
@ 2019-01-17 10:10   ` Leif Lindholm
  2019-01-17 11:27     ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Leif Lindholm @ 2019-01-17 10:10 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Masahisa Kojima

On Fri, Jan 04, 2019 at 03:43:31PM +0100, Ard Biesheuvel wrote:
> In preparation of creating a SMM version of the FIP006 NOR flash
> driver, refactor the existing pieces into a core driver, the FVB
> methods and the DXE instantiation code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I only have one nitpicky question on this patch:

> ---
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf                       |    6 +-
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c                          | 1006 +++++++++++++++++
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h}       |   52 +-
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c                       | 1150 +++-----------------
>  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} |  161 +--
>  5 files changed, 1194 insertions(+), 1181 deletions(-)
> 

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> similarity index 88%
> rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> index 20e74b0320ce..61b8e6a08fa0 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> @@ -27,11 +27,9 @@
>  #include <Protocol/FirmwareVolumeBlock.h>
>  
>  #include <Library/DebugLib.h>
> -#include <Library/DxeServicesTableLib.h>
>  #include <Library/IoLib.h>
> +#include <Library/MemoryAllocationLib.h>

Why add this include?
I'm not going to ask to move out the existing headers not actually
used by this file, but could we avoid adding new ones?

https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon
is actually a rule I agree with.

But also, aren't all of the users of this file already manually
including this one?

>  #include <Library/NorFlashPlatformLib.h>
> -#include <Library/UefiLib.h>
> -#include <Library/UefiRuntimeLib.h>
>  
>  #include "Fip006Reg.h"
>

> diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> index e52ab52d8cf7..6c07799b22d8 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> @@ -15,15 +15,16 @@
>  **/
>  
>  #include <Library/BaseMemoryLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/HobLib.h>
>  #include <Library/MemoryAllocationLib.h>

At least this one does.

>  #include <Library/NorFlashInfoLib.h>
>  #include <Library/PcdLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeLib.h>
>  
> -#include "NorFlashDxe.h"
> -
> -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
> +#include "NorFlash.h"


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

* Re: [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
  2019-01-17  9:50   ` Leif Lindholm
@ 2019-01-17 10:59     ` Ard Biesheuvel
  0 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-17 10:59 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 10:50, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 03:43:30PM +0100, Ard Biesheuvel wrote:
> > The FIP006 NOR flash driver contains implementations of the block I/O
> > and disk I/O protocols, but never exposes them to other drivers (i.e.,
> > it never installs the protocol interfaces). So let's drop this code
> > altogether: the NOR flash is for code and variables, not for arbitrary
> > files.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>
> Feel free to push this one in isolation - it's a useful improvement
> anyway, may prevent someone from getting unfortunate ideas....
>

Thanks - pushed as e48031fd75e6..944fbb62d684

> > ---
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf        |   3 -
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c | 138 ------------
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c        | 229 ++------------------
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h        |  51 +----
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c     |  40 ++--
> >  5 files changed, 37 insertions(+), 424 deletions(-)


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

* Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-04 14:43 ` [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components Ard Biesheuvel
@ 2019-01-17 11:04   ` Leif Lindholm
  2019-01-17 11:10     ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Leif Lindholm @ 2019-01-17 11:04 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Masahisa Kojima

On Fri, Jan 04, 2019 at 03:43:35PM +0100, Ard Biesheuvel wrote:
> Create a pair of .DSC/.FDF files that describe the components and
> the firmware volumes and flash device that will be dispatched into
> a secure partition in the secure world to control the UEFI secure
> variable store.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc     |   5 +-
>  Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc |   7 +-
>  Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc   | 103 +++++++++++++
>  Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf   | 161 ++++++++++++++++++++
>  4 files changed, 270 insertions(+), 6 deletions(-)
> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> index 56787a744157..666bd2716336 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> @@ -250,10 +250,7 @@ [Components.common]
>    #
>    # Variable services
>    #
> -  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf {
> -    <LibraryClasses>
> -      NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
> -  }
> +  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
>    MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>      <LibraryClasses>
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> index 56adc21d5caf..87b2094cb356 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> @@ -125,6 +125,7 @@ [LibraryClasses.common]
>    PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
>  
>    NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
> +  NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
>  
>  ################################################################################
>  #
> @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
>  !endif
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
>  
> -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE

So, I can see why you add this hard-wired for the purpose of testing.
But please, add a *very* conspicuous, and strongly worded, comment
statement preceding it.

> +
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>  
>  [PcdsPatchableInModule]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0


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

* Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-17 11:04   ` Leif Lindholm
@ 2019-01-17 11:10     ` Ard Biesheuvel
  2019-01-17 12:08       ` Leif Lindholm
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-17 11:10 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 12:04, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 03:43:35PM +0100, Ard Biesheuvel wrote:
> > Create a pair of .DSC/.FDF files that describe the components and
> > the firmware volumes and flash device that will be dispatched into
> > a secure partition in the secure world to control the UEFI secure
> > variable store.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  Platform/Socionext/DeveloperBox/DeveloperBox.dsc     |   5 +-
> >  Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc |   7 +-
> >  Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc   | 103 +++++++++++++
> >  Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf   | 161 ++++++++++++++++++++
> >  4 files changed, 270 insertions(+), 6 deletions(-)
> >
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > index 56787a744157..666bd2716336 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
> > @@ -250,10 +250,7 @@ [Components.common]
> >    #
> >    # Variable services
> >    #
> > -  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf {
> > -    <LibraryClasses>
> > -      NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
> > -  }
> > +  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
> >    MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> >    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
> >      <LibraryClasses>
> > diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > index 56adc21d5caf..87b2094cb356 100644
> > --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> > @@ -125,6 +125,7 @@ [LibraryClasses.common]
> >    PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> >
> >    NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
> > +  NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
> >
> >  ################################################################################
> >  #
> > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
> >  !endif
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> >
> > -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> > -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
>
> So, I can see why you add this hard-wired for the purpose of testing.
> But please, add a *very* conspicuous, and strongly worded, comment
> statement preceding it.
>

Well, I was talking to Peter about this the other day: according to
the spec, this setting should only matter before exit boot services,
and since this platform only supports serial and GOP consoles, one
could argue that only a physically present user could interact with it
before that time.

The obvious way of implementing this non-trivially on this platform is
to use a DIP switch, but that requires you to open the case to
enroll/delete the platform key. Perhaps that does not matter, and it
would in fact produce a less dangerous reference implementation.


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

* Re: [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot
  2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
                   ` (6 preceding siblings ...)
  2019-01-04 14:43 ` [PATCH edk2-platforms 7/7] Platform/DeveloperBox: add MM based UEFI secure boot support Ard Biesheuvel
@ 2019-01-17 11:14 ` Leif Lindholm
  2019-01-21 17:40   ` Ard Biesheuvel
  7 siblings, 1 reply; 24+ messages in thread
From: Leif Lindholm @ 2019-01-17 11:14 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel, Masahisa Kojima

On Fri, Jan 04, 2019 at 03:43:29PM +0100, Ard Biesheuvel wrote:
> Wire up the various pieces so that the authenticated variable store
> runs entirely in standalone MM context residing in a secure partition.
> 
> This primarily involves refactoring the platform's NOR flash driver so
> we can build a version that can work in the standalone MM context.
> Beyond that, it is just a matter of enabling all the boilerplate in
> the .DSC and .FDF files.
> 
> Note that the resulting standalone MM firmware volume needs to be
> wrapped in a FIP, which is not part of the build sequence.
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
> 
> Ard Biesheuvel (7):
>   Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
>   Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
>   Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant
>   Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access
>   Platform/DeveloperBox: create shared .DSC include file
>   Platform/DeveloperBox: add .DSC/.FDF description of MM components
>   Platform/DeveloperBox: add MM based UEFI secure boot support

For the patches I haven't commented on individually:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>  .../Socionext/DeveloperBox/DeveloperBox.dsc   |  304 +---
>  .../DeveloperBox/DeveloperBox.dsc.inc         |  315 ++++
>  .../Socionext/DeveloperBox/DeveloperBox.fdf   |   13 +
>  .../Socionext/DeveloperBox/DeveloperBoxMm.dsc |  103 ++
>  .../Socionext/DeveloperBox/DeveloperBoxMm.fdf |  161 ++
>  .../SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf |    9 +-
>  .../Drivers/Fip006Dxe/Fip006StandaloneMm.inf  |   71 +
>  .../SynQuacer/Drivers/Fip006Dxe/NorFlash.c    | 1006 +++++++++++++
>  .../Fip006Dxe/{NorFlashDxe.h => NorFlash.h}   |   93 +-
>  .../Drivers/Fip006Dxe/NorFlashBlockIoDxe.c    |  138 --
>  .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 1341 ++---------------
>  .../{NorFlashFvbDxe.c => NorFlashFvb.c}       |  197 +--
>  .../SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c |  182 +++
>  13 files changed, 2076 insertions(+), 1857 deletions(-)
>  create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
>  create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
>  create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
>  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
>  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
>  rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h} (85%)
>  delete mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
>  rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} (76%)
>  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c
> 
> -- 
> 2.17.1
> 


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

* Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-17 10:10   ` Leif Lindholm
@ 2019-01-17 11:27     ` Ard Biesheuvel
  2019-01-21 16:16       ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-17 11:27 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 11:10, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 03:43:31PM +0100, Ard Biesheuvel wrote:
> > In preparation of creating a SMM version of the FIP006 NOR flash
> > driver, refactor the existing pieces into a core driver, the FVB
> > methods and the DXE instantiation code.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> I only have one nitpicky question on this patch:
>
> > ---
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf                       |    6 +-
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c                          | 1006 +++++++++++++++++
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h}       |   52 +-
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c                       | 1150 +++-----------------
> >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} |  161 +--
> >  5 files changed, 1194 insertions(+), 1181 deletions(-)
> >
>
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > similarity index 88%
> > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > index 20e74b0320ce..61b8e6a08fa0 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > @@ -27,11 +27,9 @@
> >  #include <Protocol/FirmwareVolumeBlock.h>
> >
> >  #include <Library/DebugLib.h>
> > -#include <Library/DxeServicesTableLib.h>
> >  #include <Library/IoLib.h>
> > +#include <Library/MemoryAllocationLib.h>
>
> Why add this include?
> I'm not going to ask to move out the existing headers not actually
> used by this file, but could we avoid adding new ones?
>
> https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon
> is actually a rule I agree with.
>
> But also, aren't all of the users of this file already manually
> including this one?
>

Fair enough. I will drop the include here, and add it to whichever
source file requires it afterwards.

> >  #include <Library/NorFlashPlatformLib.h>
> > -#include <Library/UefiLib.h>
> > -#include <Library/UefiRuntimeLib.h>
> >
> >  #include "Fip006Reg.h"
> >
>
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > index e52ab52d8cf7..6c07799b22d8 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > @@ -15,15 +15,16 @@
> >  **/
> >
> >  #include <Library/BaseMemoryLib.h>
> > +#include <Library/DxeServicesTableLib.h>
> > +#include <Library/HobLib.h>
> >  #include <Library/MemoryAllocationLib.h>
>
> At least this one does.
>
> >  #include <Library/NorFlashInfoLib.h>
> >  #include <Library/PcdLib.h>
> >  #include <Library/UefiBootServicesTableLib.h>
> >  #include <Library/UefiLib.h>
> > +#include <Library/UefiRuntimeLib.h>
> >
> > -#include "NorFlashDxe.h"
> > -
> > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
> > +#include "NorFlash.h"


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

* Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-17 11:10     ` Ard Biesheuvel
@ 2019-01-17 12:08       ` Leif Lindholm
  2019-01-17 12:18         ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Leif Lindholm @ 2019-01-17 12:08 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, Jan 17, 2019 at 12:10:01PM +0100, Ard Biesheuvel wrote:
> > >  ################################################################################
> > >  #
> > > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
> > >  !endif
> > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> > >
> > > -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> > > -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> > > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> >
> > So, I can see why you add this hard-wired for the purpose of testing.
> > But please, add a *very* conspicuous, and strongly worded, comment
> > statement preceding it.
> 
> Well, I was talking to Peter about this the other day: according to
> the spec, this setting should only matter before exit boot services,
> and since this platform only supports serial and GOP consoles, one
> could argue that only a physically present user could interact with it
> before that time.

But that also makes the Pcd pointless.

> The obvious way of implementing this non-trivially on this platform is
> to use a DIP switch, but that requires you to open the case to
> enroll/delete the platform key. Perhaps that does not matter, and it
> would in fact produce a less dangerous reference implementation.

I would be totally OK with that.
I would also be totally OK with a DynamicPcd settable through the UI
(which is what most machines I come across have).

But I would also be cool with a sufficiently evil "here be dragons"
statement, pointing out that we don't care that much
*on*this*specific*system* because the NOR isn't actually hw protected
anyway, and this implementation is all about exercising the software
stack..

/
    Leif


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

* Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-17 12:08       ` Leif Lindholm
@ 2019-01-17 12:18         ` Ard Biesheuvel
  2019-01-21 16:57           ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-17 12:18 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 13:08, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Thu, Jan 17, 2019 at 12:10:01PM +0100, Ard Biesheuvel wrote:
> > > >  ################################################################################
> > > >  #
> > > > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
> > > >  !endif
> > > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> > > >
> > > > -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> > > > -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> > > > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> > >
> > > So, I can see why you add this hard-wired for the purpose of testing.
> > > But please, add a *very* conspicuous, and strongly worded, comment
> > > statement preceding it.
> >
> > Well, I was talking to Peter about this the other day: according to
> > the spec, this setting should only matter before exit boot services,
> > and since this platform only supports serial and GOP consoles, one
> > could argue that only a physically present user could interact with it
> > before that time.
>
> But that also makes the Pcd pointless.
>
> > The obvious way of implementing this non-trivially on this platform is
> > to use a DIP switch, but that requires you to open the case to
> > enroll/delete the platform key. Perhaps that does not matter, and it
> > would in fact produce a less dangerous reference implementation.
>
> I would be totally OK with that.
> I would also be totally OK with a DynamicPcd settable through the UI
> (which is what most machines I come across have).
>

That won't work for this implementation: the state of dynamic PCDs
does not propagate into the MM world (nor should it), and so no MM
driver implementing the dynamic PCD protocol exists.

> But I would also be cool with a sufficiently evil "here be dragons"
> statement, pointing out that we don't care that much
> *on*this*specific*system* because the NOR isn't actually hw protected
> anyway, and this implementation is all about exercising the software
> stack..
>

I'll go with that for the time being.


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

* Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-17 11:27     ` Ard Biesheuvel
@ 2019-01-21 16:16       ` Ard Biesheuvel
  2019-01-21 16:46         ` Leif Lindholm
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-21 16:16 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 12:27, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 17 Jan 2019 at 11:10, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Fri, Jan 04, 2019 at 03:43:31PM +0100, Ard Biesheuvel wrote:
> > > In preparation of creating a SMM version of the FIP006 NOR flash
> > > driver, refactor the existing pieces into a core driver, the FVB
> > > methods and the DXE instantiation code.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >
> > I only have one nitpicky question on this patch:
> >
> > > ---
> > >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf                       |    6 +-
> > >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c                          | 1006 +++++++++++++++++
> > >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h}       |   52 +-
> > >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c                       | 1150 +++-----------------
> > >  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} |  161 +--
> > >  5 files changed, 1194 insertions(+), 1181 deletions(-)
> > >
> >
> > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > similarity index 88%
> > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > index 20e74b0320ce..61b8e6a08fa0 100644
> > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > @@ -27,11 +27,9 @@
> > >  #include <Protocol/FirmwareVolumeBlock.h>
> > >
> > >  #include <Library/DebugLib.h>
> > > -#include <Library/DxeServicesTableLib.h>
> > >  #include <Library/IoLib.h>
> > > +#include <Library/MemoryAllocationLib.h>
> >
> > Why add this include?
> > I'm not going to ask to move out the existing headers not actually
> > used by this file, but could we avoid adding new ones?
> >
> > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon
> > is actually a rule I agree with.
> >
> > But also, aren't all of the users of this file already manually
> > including this one?
> >
>
> Fair enough. I will drop the include here, and add it to whichever
> source file requires it afterwards.
>

That include turns out to be entirely redundant, so I will just drop it.

> > >  #include <Library/NorFlashPlatformLib.h>
> > > -#include <Library/UefiLib.h>
> > > -#include <Library/UefiRuntimeLib.h>
> > >
> > >  #include "Fip006Reg.h"
> > >
> >
> > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > index e52ab52d8cf7..6c07799b22d8 100644
> > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > @@ -15,15 +15,16 @@
> > >  **/
> > >
> > >  #include <Library/BaseMemoryLib.h>
> > > +#include <Library/DxeServicesTableLib.h>
> > > +#include <Library/HobLib.h>
> > >  #include <Library/MemoryAllocationLib.h>
> >
> > At least this one does.
> >
> > >  #include <Library/NorFlashInfoLib.h>
> > >  #include <Library/PcdLib.h>
> > >  #include <Library/UefiBootServicesTableLib.h>
> > >  #include <Library/UefiLib.h>
> > > +#include <Library/UefiRuntimeLib.h>
> > >
> > > -#include "NorFlashDxe.h"
> > > -
> > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
> > > +#include "NorFlash.h"


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

* Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-21 16:16       ` Ard Biesheuvel
@ 2019-01-21 16:46         ` Leif Lindholm
  2019-01-21 16:47           ` Ard Biesheuvel
  0 siblings, 1 reply; 24+ messages in thread
From: Leif Lindholm @ 2019-01-21 16:46 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote:
> > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > similarity index 88%
> > > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > index 20e74b0320ce..61b8e6a08fa0 100644
> > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > @@ -27,11 +27,9 @@
> > > >  #include <Protocol/FirmwareVolumeBlock.h>
> > > >
> > > >  #include <Library/DebugLib.h>
> > > > -#include <Library/DxeServicesTableLib.h>
> > > >  #include <Library/IoLib.h>
> > > > +#include <Library/MemoryAllocationLib.h>
> > >
> > > Why add this include?
> > > I'm not going to ask to move out the existing headers not actually
> > > used by this file, but could we avoid adding new ones?
> > >
> > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon
> > > is actually a rule I agree with.
> > >
> > > But also, aren't all of the users of this file already manually
> > > including this one?
> > >
> >
> > Fair enough. I will drop the include here, and add it to whichever
> > source file requires it afterwards.
> >
> 
> That include turns out to be entirely redundant, so I will just drop it.

Works for me.

> > > >  #include <Library/NorFlashPlatformLib.h>
> > > > -#include <Library/UefiLib.h>
> > > > -#include <Library/UefiRuntimeLib.h>
> > > >
> > > >  #include "Fip006Reg.h"
> > > >
> > >
> > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > index e52ab52d8cf7..6c07799b22d8 100644
> > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > @@ -15,15 +15,16 @@
> > > >  **/
> > > >
> > > >  #include <Library/BaseMemoryLib.h>
> > > > +#include <Library/DxeServicesTableLib.h>
> > > > +#include <Library/HobLib.h>
> > > >  #include <Library/MemoryAllocationLib.h>
> > >
> > > At least this one does.
> > >
> > > >  #include <Library/NorFlashInfoLib.h>
> > > >  #include <Library/PcdLib.h>
> > > >  #include <Library/UefiBootServicesTableLib.h>
> > > >  #include <Library/UefiLib.h>
> > > > +#include <Library/UefiRuntimeLib.h>
> > > >
> > > > -#include "NorFlashDxe.h"
> > > > -
> > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
> > > > +#include "NorFlash.h"


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

* Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-21 16:46         ` Leif Lindholm
@ 2019-01-21 16:47           ` Ard Biesheuvel
  2019-01-21 16:53             ` Leif Lindholm
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-21 16:47 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Mon, 21 Jan 2019 at 17:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote:
> > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > > similarity index 88%
> > > > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > > index 20e74b0320ce..61b8e6a08fa0 100644
> > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > > @@ -27,11 +27,9 @@
> > > > >  #include <Protocol/FirmwareVolumeBlock.h>
> > > > >
> > > > >  #include <Library/DebugLib.h>
> > > > > -#include <Library/DxeServicesTableLib.h>
> > > > >  #include <Library/IoLib.h>
> > > > > +#include <Library/MemoryAllocationLib.h>
> > > >
> > > > Why add this include?
> > > > I'm not going to ask to move out the existing headers not actually
> > > > used by this file, but could we avoid adding new ones?
> > > >
> > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon
> > > > is actually a rule I agree with.
> > > >
> > > > But also, aren't all of the users of this file already manually
> > > > including this one?
> > > >
> > >
> > > Fair enough. I will drop the include here, and add it to whichever
> > > source file requires it afterwards.
> > >
> >
> > That include turns out to be entirely redundant, so I will just drop it.
>
> Works for me.
>

Wonderful. Can I take that as an ack?

> > > > >  #include <Library/NorFlashPlatformLib.h>
> > > > > -#include <Library/UefiLib.h>
> > > > > -#include <Library/UefiRuntimeLib.h>
> > > > >
> > > > >  #include "Fip006Reg.h"
> > > > >
> > > >
> > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > > index e52ab52d8cf7..6c07799b22d8 100644
> > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > > @@ -15,15 +15,16 @@
> > > > >  **/
> > > > >
> > > > >  #include <Library/BaseMemoryLib.h>
> > > > > +#include <Library/DxeServicesTableLib.h>
> > > > > +#include <Library/HobLib.h>
> > > > >  #include <Library/MemoryAllocationLib.h>
> > > >
> > > > At least this one does.
> > > >
> > > > >  #include <Library/NorFlashInfoLib.h>
> > > > >  #include <Library/PcdLib.h>
> > > > >  #include <Library/UefiBootServicesTableLib.h>
> > > > >  #include <Library/UefiLib.h>
> > > > > +#include <Library/UefiRuntimeLib.h>
> > > > >
> > > > > -#include "NorFlashDxe.h"
> > > > > -
> > > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
> > > > > +#include "NorFlash.h"


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

* Re: [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  2019-01-21 16:47           ` Ard Biesheuvel
@ 2019-01-21 16:53             ` Leif Lindholm
  0 siblings, 0 replies; 24+ messages in thread
From: Leif Lindholm @ 2019-01-21 16:53 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Mon, Jan 21, 2019 at 05:47:10PM +0100, Ard Biesheuvel wrote:
> On Mon, 21 Jan 2019 at 17:46, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Mon, Jan 21, 2019 at 05:16:09PM +0100, Ard Biesheuvel wrote:
> > > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > > > similarity index 88%
> > > > > > rename from Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > > > > rename to Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > > > index 20e74b0320ce..61b8e6a08fa0 100644
> > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h
> > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.h
> > > > > > @@ -27,11 +27,9 @@
> > > > > >  #include <Protocol/FirmwareVolumeBlock.h>
> > > > > >
> > > > > >  #include <Library/DebugLib.h>
> > > > > > -#include <Library/DxeServicesTableLib.h>
> > > > > >  #include <Library/IoLib.h>
> > > > > > +#include <Library/MemoryAllocationLib.h>
> > > > >
> > > > > Why add this include?
> > > > > I'm not going to ask to move out the existing headers not actually
> > > > > used by this file, but could we avoid adding new ones?
> > > > >
> > > > > https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/v/release/2.20/5_source_files/53_include_files.html#534-include-files-may-include-only-those-headers-that-it-directly-depends-upon
> > > > > is actually a rule I agree with.
> > > > >
> > > > > But also, aren't all of the users of this file already manually
> > > > > including this one?
> > > > >
> > > >
> > > > Fair enough. I will drop the include here, and add it to whichever
> > > > source file requires it afterwards.
> > > >
> > >
> > > That include turns out to be entirely redundant, so I will just drop it.
> >
> > Works for me.
> >
> 
> Wonderful. Can I take that as an ack?

Whoops. Yes.
Acked-by: Leif Lindholm <leif.lindholm@linaro.org>

> > > > > >  #include <Library/NorFlashPlatformLib.h>
> > > > > > -#include <Library/UefiLib.h>
> > > > > > -#include <Library/UefiRuntimeLib.h>
> > > > > >
> > > > > >  #include "Fip006Reg.h"
> > > > > >
> > > > >
> > > > > > diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > > > index e52ab52d8cf7..6c07799b22d8 100644
> > > > > > --- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > > > +++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
> > > > > > @@ -15,15 +15,16 @@
> > > > > >  **/
> > > > > >
> > > > > >  #include <Library/BaseMemoryLib.h>
> > > > > > +#include <Library/DxeServicesTableLib.h>
> > > > > > +#include <Library/HobLib.h>
> > > > > >  #include <Library/MemoryAllocationLib.h>
> > > > >
> > > > > At least this one does.
> > > > >
> > > > > >  #include <Library/NorFlashInfoLib.h>
> > > > > >  #include <Library/PcdLib.h>
> > > > > >  #include <Library/UefiBootServicesTableLib.h>
> > > > > >  #include <Library/UefiLib.h>
> > > > > > +#include <Library/UefiRuntimeLib.h>
> > > > > >
> > > > > > -#include "NorFlashDxe.h"
> > > > > > -
> > > > > > -STATIC EFI_EVENT mNorFlashVirtualAddrChangeEvent;
> > > > > > +#include "NorFlash.h"


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

* Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-17 12:18         ` Ard Biesheuvel
@ 2019-01-21 16:57           ` Ard Biesheuvel
  2019-01-21 17:03             ` Leif Lindholm
  0 siblings, 1 reply; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-21 16:57 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 13:18, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Thu, 17 Jan 2019 at 13:08, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> >
> > On Thu, Jan 17, 2019 at 12:10:01PM +0100, Ard Biesheuvel wrote:
> > > > >  ################################################################################
> > > > >  #
> > > > > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
> > > > >  !endif
> > > > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> > > > >
> > > > > -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> > > > > -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> > > > > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> > > >
> > > > So, I can see why you add this hard-wired for the purpose of testing.
> > > > But please, add a *very* conspicuous, and strongly worded, comment
> > > > statement preceding it.
> > >
> > > Well, I was talking to Peter about this the other day: according to
> > > the spec, this setting should only matter before exit boot services,
> > > and since this platform only supports serial and GOP consoles, one
> > > could argue that only a physically present user could interact with it
> > > before that time.
> >
> > But that also makes the Pcd pointless.
> >
> > > The obvious way of implementing this non-trivially on this platform is
> > > to use a DIP switch, but that requires you to open the case to
> > > enroll/delete the platform key. Perhaps that does not matter, and it
> > > would in fact produce a less dangerous reference implementation.
> >
> > I would be totally OK with that.
> > I would also be totally OK with a DynamicPcd settable through the UI
> > (which is what most machines I come across have).
> >
>
> That won't work for this implementation: the state of dynamic PCDs
> does not propagate into the MM world (nor should it), and so no MM
> driver implementing the dynamic PCD protocol exists.
>
> > But I would also be cool with a sufficiently evil "here be dragons"
> > statement, pointing out that we don't care that much
> > *on*this*specific*system* because the NOR isn't actually hw protected
> > anyway, and this implementation is all about exercising the software
> > stack..
> >
>
> I'll go with that for the time being.

I'll add the following:

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
index f191edcb78dd..1ac8c8f95722 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
@@ -301,6 +301,13 @@
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)

+  #
+  # NOTE: this platform is not fully secure (the NOR flash is mapped
non-secure)
+  # and so the MM based secure boot implementation it provides should
be treated
+  # as a reference only. For this reason, it does not make a lot of sense to
+  # implement an elaborate PlatformSecureLib implementation that can assert
+  # physical presence, and instead, we'll stick with the default PCD based one.
+  #
   gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE

   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000


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

* Re: [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components
  2019-01-21 16:57           ` Ard Biesheuvel
@ 2019-01-21 17:03             ` Leif Lindholm
  0 siblings, 0 replies; 24+ messages in thread
From: Leif Lindholm @ 2019-01-21 17:03 UTC (permalink / raw)
  To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Mon, Jan 21, 2019 at 05:57:20PM +0100, Ard Biesheuvel wrote:
> On Thu, 17 Jan 2019 at 13:18, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > On Thu, 17 Jan 2019 at 13:08, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > >
> > > On Thu, Jan 17, 2019 at 12:10:01PM +0100, Ard Biesheuvel wrote:
> > > > > >  ################################################################################
> > > > > >  #
> > > > > > @@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
> > > > > >  !endif
> > > > > >    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> > > > > >
> > > > > > -  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC00000
> > > > > > -  gArmTokenSpaceGuid.PcdMmBufferSize|0x00200000
> > > > > > +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> > > > >
> > > > > So, I can see why you add this hard-wired for the purpose of testing.
> > > > > But please, add a *very* conspicuous, and strongly worded, comment
> > > > > statement preceding it.
> > > >
> > > > Well, I was talking to Peter about this the other day: according to
> > > > the spec, this setting should only matter before exit boot services,
> > > > and since this platform only supports serial and GOP consoles, one
> > > > could argue that only a physically present user could interact with it
> > > > before that time.
> > >
> > > But that also makes the Pcd pointless.
> > >
> > > > The obvious way of implementing this non-trivially on this platform is
> > > > to use a DIP switch, but that requires you to open the case to
> > > > enroll/delete the platform key. Perhaps that does not matter, and it
> > > > would in fact produce a less dangerous reference implementation.
> > >
> > > I would be totally OK with that.
> > > I would also be totally OK with a DynamicPcd settable through the UI
> > > (which is what most machines I come across have).
> > >
> >
> > That won't work for this implementation: the state of dynamic PCDs
> > does not propagate into the MM world (nor should it), and so no MM
> > driver implementing the dynamic PCD protocol exists.
> >
> > > But I would also be cool with a sufficiently evil "here be dragons"
> > > statement, pointing out that we don't care that much
> > > *on*this*specific*system* because the NOR isn't actually hw protected
> > > anyway, and this implementation is all about exercising the software
> > > stack..
> > >
> >
> > I'll go with that for the time being.
> 
> I'll add the following:
> 
> diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> index f191edcb78dd..1ac8c8f95722 100644
> --- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> +++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> @@ -301,6 +301,13 @@
>  !endif
>    gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
> 
> +  #
> +  # NOTE: this platform is not fully secure (the NOR flash is mapped
> non-secure)
> +  # and so the MM based secure boot implementation it provides should
> be treated
> +  # as a reference only. For this reason, it does not make a lot of sense to
> +  # implement an elaborate PlatformSecureLib implementation that can assert
> +  # physical presence, and instead, we'll stick with the default PCD based one.
> +  #
>    gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> 
>    gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000

Totally happy with that.
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


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

* Re: [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot
  2019-01-17 11:14 ` [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Leif Lindholm
@ 2019-01-21 17:40   ` Ard Biesheuvel
  0 siblings, 0 replies; 24+ messages in thread
From: Ard Biesheuvel @ 2019-01-21 17:40 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: edk2-devel@lists.01.org, Masahisa Kojima

On Thu, 17 Jan 2019 at 12:14, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>
> On Fri, Jan 04, 2019 at 03:43:29PM +0100, Ard Biesheuvel wrote:
> > Wire up the various pieces so that the authenticated variable store
> > runs entirely in standalone MM context residing in a secure partition.
> >
> > This primarily involves refactoring the platform's NOR flash driver so
> > we can build a version that can work in the standalone MM context.
> > Beyond that, it is just a matter of enabling all the boilerplate in
> > the .DSC and .FDF files.
> >
> > Note that the resulting standalone MM firmware volume needs to be
> > wrapped in a FIP, which is not part of the build sequence.
> >
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
> >
> > Ard Biesheuvel (7):
> >   Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
> >   Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
> >   Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant
> >   Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access
> >   Platform/DeveloperBox: create shared .DSC include file
> >   Platform/DeveloperBox: add .DSC/.FDF description of MM components
> >   Platform/DeveloperBox: add MM based UEFI secure boot support
>
> For the patches I haven't commented on individually:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
>

Thanks

Series pushed as 9b725b6ebb39..d571b43f8741

> >  .../Socionext/DeveloperBox/DeveloperBox.dsc   |  304 +---
> >  .../DeveloperBox/DeveloperBox.dsc.inc         |  315 ++++
> >  .../Socionext/DeveloperBox/DeveloperBox.fdf   |   13 +
> >  .../Socionext/DeveloperBox/DeveloperBoxMm.dsc |  103 ++
> >  .../Socionext/DeveloperBox/DeveloperBoxMm.fdf |  161 ++
> >  .../SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf |    9 +-
> >  .../Drivers/Fip006Dxe/Fip006StandaloneMm.inf  |   71 +
> >  .../SynQuacer/Drivers/Fip006Dxe/NorFlash.c    | 1006 +++++++++++++
> >  .../Fip006Dxe/{NorFlashDxe.h => NorFlash.h}   |   93 +-
> >  .../Drivers/Fip006Dxe/NorFlashBlockIoDxe.c    |  138 --
> >  .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 1341 ++---------------
> >  .../{NorFlashFvbDxe.c => NorFlashFvb.c}       |  197 +--
> >  .../SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c |  182 +++
> >  13 files changed, 2076 insertions(+), 1857 deletions(-)
> >  create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
> >  create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
> >  create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
> >  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
> >  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
> >  rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h} (85%)
> >  delete mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
> >  rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => NorFlashFvb.c} (76%)
> >  create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c
> >
> > --
> > 2.17.1
> >


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

end of thread, other threads:[~2019-01-21 17:40 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-04 14:43 [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Ard Biesheuvel
2019-01-04 14:43 ` [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines Ard Biesheuvel
2019-01-17  9:50   ` Leif Lindholm
2019-01-17 10:59     ` Ard Biesheuvel
2019-01-04 14:43 ` [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces Ard Biesheuvel
2019-01-17 10:10   ` Leif Lindholm
2019-01-17 11:27     ` Ard Biesheuvel
2019-01-21 16:16       ` Ard Biesheuvel
2019-01-21 16:46         ` Leif Lindholm
2019-01-21 16:47           ` Ard Biesheuvel
2019-01-21 16:53             ` Leif Lindholm
2019-01-04 14:43 ` [PATCH edk2-platforms 3/7] Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant Ard Biesheuvel
2019-01-04 14:43 ` [PATCH edk2-platforms 4/7] Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access Ard Biesheuvel
2019-01-04 14:43 ` [PATCH edk2-platforms 5/7] Platform/DeveloperBox: create shared .DSC include file Ard Biesheuvel
2019-01-04 14:43 ` [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components Ard Biesheuvel
2019-01-17 11:04   ` Leif Lindholm
2019-01-17 11:10     ` Ard Biesheuvel
2019-01-17 12:08       ` Leif Lindholm
2019-01-17 12:18         ` Ard Biesheuvel
2019-01-21 16:57           ` Ard Biesheuvel
2019-01-21 17:03             ` Leif Lindholm
2019-01-04 14:43 ` [PATCH edk2-platforms 7/7] Platform/DeveloperBox: add MM based UEFI secure boot support Ard Biesheuvel
2019-01-17 11:14 ` [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot Leif Lindholm
2019-01-21 17:40   ` Ard Biesheuvel

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