Queue 2024 Jan 18

OvmfPkg/VirtNorFlashDxe/VirtNorFlash.c

@@ -502,6 +502,38 @@ NorFlashRead (
   return EFI_SUCCESS;
 }
 
+STATIC
+EFI_STATUS
+NorFlashWriteSingleBlockWithErase (
+  IN        NOR_FLASH_INSTANCE  *Instance,
+  IN        EFI_LBA             Lba,
+  IN        UINTN               Offset,
+  IN OUT    UINTN               *NumBytes,
+  IN        UINT8               *Buffer
+  )
+{
+  EFI_STATUS  Status;
+
+  // Read NOR Flash data into shadow buffer
+  Status = NorFlashReadBlocks (Instance, Lba, Instance->BlockSize, Instance->ShadowBuffer);
+  if (EFI_ERROR (Status)) {
+    // 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
+  Status = NorFlashWriteBlocks (Instance, Lba, Instance->BlockSize, Instance->ShadowBuffer);
+  if (EFI_ERROR (Status)) {
+    // Return one of the pre-approved error statuses
+    return EFI_DEVICE_ERROR;
+  }
+
+  return EFI_SUCCESS;
+}
+
 /*
   Write a full or portion of a block. It must not span block boundaries; that is,
   Offset + *NumBytes <= Instance->BlockSize.
@@ -520,6 +552,8 @@ NorFlashWriteSingleBlock (
   UINTN       BlockSize;
   UINTN       BlockAddress;
   UINT8       *OrigData;
+  UINTN       Start, End;
+  UINT32      Index, Count;
 
   DEBUG ((DEBUG_BLKIO, "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Lba, Offset, *NumBytes, Buffer));
 
@@ -548,14 +582,37 @@ NorFlashWriteSingleBlock (
     return EFI_BAD_BUFFER_SIZE;
   }
 
-  // Pick P30_MAX_BUFFER_SIZE_IN_BYTES (== 128 bytes) 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 128 bytes.
-  // To avoid pathological cases were a 2 byte write is disregarded because it
-  // occurs right at a 128 byte buffered write alignment boundary, permit up to
-  // twice the max buffer size, and perform two writes if needed.
-  if ((*NumBytes + (Offset & BOUNDARY_OF_32_WORDS)) <= (2 * P30_MAX_BUFFER_SIZE_IN_BYTES)) {
+  // Pick 4 * P30_MAX_BUFFER_SIZE_IN_BYTES (== 512 bytes) as a good
+  // start for word operations as opposed to erasing the block and
+  // writing the data regardless if an erase is really needed.
+  //
+  // Many NV variable updates are small enough for a a single
+  // P30_MAX_BUFFER_SIZE_IN_BYTES block write.  In case the update is
+  // larger than a single block, or the update crosses a
+  // P30_MAX_BUFFER_SIZE_IN_BYTES boundary (as shown in the diagram
+  // below), or both, we might have to write two or more blocks.
+  //
+  //    0               128              256
+  //    [----------------|----------------]
+  //    ^         ^             ^         ^
+  //    |         |             |         |
+  //    |         |             |        End, the next "word" boundary beyond
+  //    |         |             |        the (logical) update
+  //    |         |             |
+  //    |         |     (Offset & BOUNDARY_OF_32_WORDS) + NumBytes;
+  //    |         |     i.e., the relative offset inside (or just past)
+  //    |         |     the *double-word* such that it is the
+  //    |         |     *exclusive* end of the (logical) update.
+  //    |         |
+  //    |         Offset & BOUNDARY_OF_32_WORDS; i.e., Offset within the "word";
+  //    |         this is where the (logical) update is supposed to start
+  //    |
+  //    Start = Offset & ~BOUNDARY_OF_32_WORDS; i.e., Offset truncated to "word" boundary
+
+  Start = Offset & ~BOUNDARY_OF_32_WORDS;
+  End   = ALIGN_VALUE (Offset + *NumBytes, P30_MAX_BUFFER_SIZE_IN_BYTES);
+
+  if ((End - Start) <= (4 * P30_MAX_BUFFER_SIZE_IN_BYTES)) {
     // 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.
@@ -565,8 +622,8 @@ NorFlashWriteSingleBlock (
     Status = NorFlashRead (
                Instance,
                Lba,
-               Offset & ~BOUNDARY_OF_32_WORDS,
-               (*NumBytes | BOUNDARY_OF_32_WORDS) + 1,
+               Start,
+               End - Start,
                Instance->ShadowBuffer
                );
     if (EFI_ERROR (Status)) {
@@ -581,8 +638,15 @@ NorFlashWriteSingleBlock (
     // contents, while checking whether the old version had any bits cleared
     // that we want to set. In that case, we will need to erase the block first.
     for (CurOffset = 0; CurOffset < *NumBytes; CurOffset++) {
-      if (~OrigData[CurOffset] & Buffer[CurOffset]) {
-        goto DoErase;
+      if (~(UINT32)OrigData[CurOffset] & (UINT32)Buffer[CurOffset]) {
+        Status = NorFlashWriteSingleBlockWithErase (
+                   Instance,
+                   Lba,
+                   Offset,
+                   NumBytes,
+                   Buffer
+                   );
+        return Status;
       }
 
       OrigData[CurOffset] = Buffer[CurOffset];
@@ -599,53 +663,34 @@ NorFlashWriteSingleBlock (
       goto Exit;
     }
 
-    Status = NorFlashWriteBuffer (
-               Instance,
-               BlockAddress + (Offset & ~BOUNDARY_OF_32_WORDS),
-               P30_MAX_BUFFER_SIZE_IN_BYTES,
-               Instance->ShadowBuffer
-               );
-    if (EFI_ERROR (Status)) {
-      goto Exit;
-    }
-
-    if ((*NumBytes + (Offset & BOUNDARY_OF_32_WORDS)) > P30_MAX_BUFFER_SIZE_IN_BYTES) {
-      BlockAddress += P30_MAX_BUFFER_SIZE_IN_BYTES;
-
+    Count = (End - Start) / P30_MAX_BUFFER_SIZE_IN_BYTES;
+    for (Index = 0; Index < Count; Index++) {
       Status = NorFlashWriteBuffer (
                  Instance,
-                 BlockAddress + (Offset & ~BOUNDARY_OF_32_WORDS),
+                 BlockAddress + Start + Index * P30_MAX_BUFFER_SIZE_IN_BYTES,
                  P30_MAX_BUFFER_SIZE_IN_BYTES,
-                 Instance->ShadowBuffer + P30_MAX_BUFFER_SIZE_IN_BYTES
+                 Instance->ShadowBuffer + Index * P30_MAX_BUFFER_SIZE_IN_BYTES
                  );
+      if (EFI_ERROR (Status)) {
+        goto Exit;
+      }
     }
-
-Exit:
-    // Put device back into Read Array mode
-    SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
-
+  } else {
+    Status = NorFlashWriteSingleBlockWithErase (
+               Instance,
+               Lba,
+               Offset,
+               NumBytes,
+               Buffer
+               );
     return Status;
   }
 
-DoErase:
-  // Read NOR Flash data into shadow buffer
-  Status = NorFlashReadBlocks (Instance, Lba, BlockSize, Instance->ShadowBuffer);
-  if (EFI_ERROR (Status)) {
-    // 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
-  Status = NorFlashWriteBlocks (Instance, Lba, BlockSize, Instance->ShadowBuffer);
-  if (EFI_ERROR (Status)) {
-    // Return one of the pre-approved error statuses
-    return EFI_DEVICE_ERROR;
-  }
+Exit:
+  // Put device back into Read Array mode
+  SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
 
-  return EFI_SUCCESS;
+  return Status;
 }
 
 EFI_STATUS

OvmfPkg/VirtNorFlashDxe/VirtNorFlash.h

@@ -61,7 +61,7 @@
 #define P30_MAX_BUFFER_SIZE_IN_BYTES  ((UINTN)128)
 #define P30_MAX_BUFFER_SIZE_IN_WORDS  (P30_MAX_BUFFER_SIZE_IN_BYTES/((UINTN)4))
 #define MAX_BUFFERED_PROG_ITERATIONS  10000000
-#define BOUNDARY_OF_32_WORDS          0x7F
+#define BOUNDARY_OF_32_WORDS          ((UINTN)0x7F)
 
 // CFI Addresses
 #define P30_CFI_ADDR_QUERY_UNIQUE_QRY  0x10

OvmfPkg/VirtNorFlashDxe/VirtNorFlashFvb.c

@@ -302,6 +302,11 @@ ValidateFvHeader (
       break;
     }
 
+    if (VarHeader->State == 0xff) {
+      DEBUG ((DEBUG_INFO, "%a: end of var list (unwritten state)\n", __func__));
+      break;
+    }
+
     VarName = NULL;
     switch (VarHeader->State) {
       // usage: State = VAR_HEADER_VALID_ONLY

SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c

@@ -200,7 +200,7 @@ Tcg2MeasureGptTable (
                      BlockIo->Media->BlockSize,
                      (UINT8 *)PrimaryHeader
                      );
-  if (EFI_ERROR (Status) || EFI_ERROR (SanitizeEfiPartitionTableHeader (PrimaryHeader, BlockIo))) {
+  if (EFI_ERROR (Status) || EFI_ERROR (Tpm2SanitizeEfiPartitionTableHeader (PrimaryHeader, BlockIo))) {
     DEBUG ((DEBUG_ERROR, "Failed to read Partition Table Header or invalid Partition Table Header!\n"));
     FreePool (PrimaryHeader);
     return EFI_DEVICE_ERROR;
@@ -209,7 +209,7 @@ Tcg2MeasureGptTable (
   //
   // Read the partition entry.
   //
-  Status = SanitizePrimaryHeaderAllocationSize (PrimaryHeader, &AllocSize);
+  Status = Tpm2SanitizePrimaryHeaderAllocationSize (PrimaryHeader, &AllocSize);
   if (EFI_ERROR (Status)) {
     FreePool (PrimaryHeader);
     return EFI_BAD_BUFFER_SIZE;
@@ -250,7 +250,7 @@ Tcg2MeasureGptTable (
   //
   // Prepare Data for Measurement (CcProtocol and Tcg2Protocol)
   //
-  Status = SanitizePrimaryHeaderGptEventSize (PrimaryHeader, NumberOfPartition, &TcgEventSize);
+  Status = Tpm2SanitizePrimaryHeaderGptEventSize (PrimaryHeader, NumberOfPartition, &TcgEventSize);
   if (EFI_ERROR (Status)) {
     FreePool (PrimaryHeader);
     FreePool (EntryPtr);
@@ -420,7 +420,7 @@ Tcg2MeasurePeImage (
   }
 
   FilePathSize = (UINT32)GetDevicePathSize (FilePath);
-  Status       = SanitizePeImageEventSize (FilePathSize, &EventSize);
+  Status       = Tpm2SanitizePeImageEventSize (FilePathSize, &EventSize);
   if (EFI_ERROR (Status)) {
     return EFI_UNSUPPORTED;
   }

SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.c

@@ -63,7 +63,7 @@
 **/
 EFI_STATUS
 EFIAPI
-SanitizeEfiPartitionTableHeader (
+Tpm2SanitizeEfiPartitionTableHeader (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN CONST EFI_BLOCK_IO_PROTOCOL       *BlockIo
   )
@@ -169,7 +169,7 @@ SanitizeEfiPartitionTableHeader (
 **/
 EFI_STATUS
 EFIAPI
-SanitizePrimaryHeaderAllocationSize (
+Tpm2SanitizePrimaryHeaderAllocationSize (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   OUT UINT32                           *AllocationSize
   )
@@ -221,7 +221,7 @@ SanitizePrimaryHeaderAllocationSize (
     One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePrimaryHeaderGptEventSize (
+Tpm2SanitizePrimaryHeaderGptEventSize (
   IN  CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN  UINTN                             NumberOfPartition,
   OUT UINT32                            *EventSize
@@ -292,7 +292,7 @@ SanitizePrimaryHeaderGptEventSize (
     One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePeImageEventSize (
+Tpm2SanitizePeImageEventSize (
   IN  UINT32  FilePathSize,
   OUT UINT32  *EventSize
   )

SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLibSanitization.h

@@ -54,7 +54,7 @@
 **/
 EFI_STATUS
 EFIAPI
-SanitizeEfiPartitionTableHeader (
+Tpm2SanitizeEfiPartitionTableHeader (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN CONST EFI_BLOCK_IO_PROTOCOL       *BlockIo
   );
@@ -78,7 +78,7 @@ SanitizeEfiPartitionTableHeader (
 **/
 EFI_STATUS
 EFIAPI
-SanitizePrimaryHeaderAllocationSize (
+Tpm2SanitizePrimaryHeaderAllocationSize (
   IN CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   OUT UINT32                           *AllocationSize
   );
@@ -107,7 +107,7 @@ SanitizePrimaryHeaderAllocationSize (
     One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePrimaryHeaderGptEventSize (
+Tpm2SanitizePrimaryHeaderGptEventSize (
   IN  CONST EFI_PARTITION_TABLE_HEADER  *PrimaryHeader,
   IN  UINTN                             NumberOfPartition,
   OUT UINT32                            *EventSize
@@ -131,7 +131,7 @@ SanitizePrimaryHeaderGptEventSize (
     One of the passed parameters was invalid.
 **/
 EFI_STATUS
-SanitizePeImageEventSize (
+Tpm2SanitizePeImageEventSize (
   IN  UINT32  FilePathSize,
   OUT UINT32  *EventSize
   );