public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdePkg/Uefi: pack EFI_CAPSULE_TABLE struct
@ 2017-03-03 12:16 Ard Biesheuvel
  2017-03-03 13:24 ` Yao, Jiewen
  0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2017-03-03 12:16 UTC (permalink / raw)
  To: edk2-devel, jiewen.yao
  Cc: liming.gao, leif.lindholm, michael.d.kinney, Ard Biesheuvel

The UEFI specification describes the per-capsule type configuration
table entry as follows:

  The EFI System Table entry must use the GUID from the CapsuleGuid
  field of the EFI_CAPSULE_HEADER. The EFI System Table entry must point
  to an array of capsules that contain the same CapsuleGuid value. The
  array must be prefixed by a UINT32 that represents the size of the array
  of capsules.

In the current EDK2 implementation, this is translated into the following

  typedef struct {
    ///
    /// the size of the array of capsules.
    ///
    UINT32   CapsuleArrayNumber;
    ///
    /// Point to an array of capsules that contain the same CapsuleGuid value.
    ///
    VOID*    CapsulePtr[1];
  } EFI_CAPSULE_TABLE;

Note that this implements an array of capsule *pointers* rather than an
array of capsules. Also, it lacks the #pragma pack(1), resulting in
padding to be added after the CapsuleArrayNumber.

So let's bring this code in line with the UEFI spec and
- put the *size* of the array in the leading UINT32 rather than the number
  of entries,
- pack the struct to remove any padding on 64-bit architectures
- replace the array of pointers with an array of capsule headers.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 EdkCompatibilityPkg/Foundation/Framework/Include/EfiCapsule.h | 6 ++++--
 IntelFrameworkModulePkg/Universal/BdsDxe/Capsules.c           | 8 +++++---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c  | 8 +++++---
 MdePkg/Include/Uefi/UefiSpec.h                                | 8 +++++---
 4 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/EdkCompatibilityPkg/Foundation/Framework/Include/EfiCapsule.h b/EdkCompatibilityPkg/Foundation/Framework/Include/EfiCapsule.h
index cae8aec1619e..7026a1876961 100644
--- a/EdkCompatibilityPkg/Foundation/Framework/Include/EfiCapsule.h
+++ b/EdkCompatibilityPkg/Foundation/Framework/Include/EfiCapsule.h
@@ -54,10 +54,12 @@ typedef struct {
   UINT32    CapsuleImageSize;
 } EFI_CAPSULE_HEADER;
 
+#pragma pack(1)
 typedef struct {
-  UINT32   CapsuleArrayNumber;
-  VOID*    CapsulePtr[1];
+  UINT32              CapsuleArraySize;
+  EFI_CAPSULE_HEADER  Capsule[1];
 } EFI_CAPSULE_TABLE;
+#pragma pack()
 
 //
 // This struct is deprecated because VendorTable entries physical address will not be fixed up when 
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/Capsules.c b/IntelFrameworkModulePkg/Universal/BdsDxe/Capsules.c
index 6c7fc7ced4c9..91d21f1d7218 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/Capsules.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/Capsules.c
@@ -172,11 +172,13 @@ BdsProcessCapsules (
       }
     }
     if (CapsuleNumber != 0) {
-      Size = sizeof(EFI_CAPSULE_TABLE) + (CapsuleNumber - 1) * sizeof(VOID*);  
+      Size = sizeof(EFI_CAPSULE_TABLE) + (CapsuleNumber - 1) * sizeof(EFI_CAPSULE_HEADER);
       CapsuleTable = AllocateRuntimePool (Size);
       ASSERT (CapsuleTable != NULL);
-      CapsuleTable->CapsuleArrayNumber =  CapsuleNumber;
-      CopyMem(&CapsuleTable->CapsulePtr[0], CapsulePtrCache, CapsuleNumber * sizeof(VOID*));
+      CapsuleTable->CapsuleArraySize = Size - sizeof(EFI_CAPSULE_TABLE);
+      for (Index = 0; Index < CapsuleNumber; Index++) {
+        CopyMem(&CapsuleTable->Capsule[Index], CapsulePtrCache[Index], sizeof(EFI_CAPSULE_TABLE));
+      }
       Status = gBS->InstallConfigurationTable (&CapsuleGuidCache[CacheIndex], (VOID*)CapsuleTable);
       ASSERT_EFI_ERROR (Status);
     }
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
index ba3ff90b9f73..45a0026acacc 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
@@ -277,14 +277,16 @@ PopulateCapsuleInConfigurationTable (
       }
     }
     if (CapsuleNumber != 0) {
-      Size = sizeof(EFI_CAPSULE_TABLE) + (CapsuleNumber - 1) * sizeof(VOID*);
+      Size = sizeof(EFI_CAPSULE_TABLE) + (CapsuleNumber - 1) * sizeof(EFI_CAPSULE_HEADER);
       CapsuleTable = AllocateRuntimePool (Size);
       if (CapsuleTable == NULL) {
         DEBUG ((DEBUG_ERROR, "Allocate CapsuleTable (%g) fail!\n", &CapsuleGuidCache[CacheIndex]));
         continue;
       }
-      CapsuleTable->CapsuleArrayNumber =  CapsuleNumber;
-      CopyMem(&CapsuleTable->CapsulePtr[0], CapsulePtrCache, CapsuleNumber * sizeof(VOID*));
+      CapsuleTable->CapsuleArraySize = Size - sizeof(EFI_CAPSULE_TABLE);
+      for (Index = 0; Index < CapsuleNumber; Index++) {
+        CopyMem(&CapsuleTable->Capsule[Index], CapsulePtrCache[Index], sizeof(EFI_CAPSULE_TABLE));
+      }
       Status = gBS->InstallConfigurationTable (&CapsuleGuidCache[CacheIndex], (VOID*)CapsuleTable);
       if (EFI_ERROR (Status)) {
         DEBUG ((DEBUG_ERROR, "InstallConfigurationTable (%g) fail!\n", &CapsuleGuidCache[CacheIndex]));
diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
index 57cb4e804f70..ad9dfefbccf8 100644
--- a/MdePkg/Include/Uefi/UefiSpec.h
+++ b/MdePkg/Include/Uefi/UefiSpec.h
@@ -1630,16 +1630,18 @@ typedef struct {
 /// that contain the same CapsuleGuid value. The array must be
 /// prefixed by a UINT32 that represents the size of the array of capsules.
 ///
+#pragma pack(1)
 typedef struct {
   ///
   /// the size of the array of capsules.
   ///
-  UINT32   CapsuleArrayNumber;
+  UINT32   CapsuleArraySize;
   ///
-  /// Point to an array of capsules that contain the same CapsuleGuid value.
+  /// Array of capsules that contain the same CapsuleGuid value.
   ///
-  VOID*    CapsulePtr[1];
+  EFI_CAPSULE_HEADER    Capsule[1];
 } EFI_CAPSULE_TABLE;
+#pragma pack()
 
 #define CAPSULE_FLAGS_PERSIST_ACROSS_RESET          0x00010000
 #define CAPSULE_FLAGS_POPULATE_SYSTEM_TABLE         0x00020000
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-03-03 13:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-03 12:16 [PATCH] MdePkg/Uefi: pack EFI_CAPSULE_TABLE struct Ard Biesheuvel
2017-03-03 13:24 ` Yao, Jiewen
2017-03-03 13:45   ` Ard Biesheuvel

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