From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-x22e.google.com (mail-wr0-x22e.google.com [IPv6:2a00:1450:400c:c0c::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 41D7F82212 for ; Fri, 3 Mar 2017 04:16:56 -0800 (PST) Received: by mail-wr0-x22e.google.com with SMTP id g10so72392659wrg.2 for ; Fri, 03 Mar 2017 04:16:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=wfV/TrABPDrr2IOAYeZQJdCYoQsQ/f/QZzjx1wPwFro=; b=FJdrk1RzIshu6kJKEBoQfG2BFqcCRINk9uqRdpYEQjhkMXJK90UB5bzbCoAagVGUW7 vQWpgCAN2uglR3XHhfGxwMHjRLoLcVLliZgFkNSY+0R47/fb2UH76D56F3NNVgJoKWRp 4eKsMZCycbcSYvDCNNFcY/b9gyp0EzDEsrxQU= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=wfV/TrABPDrr2IOAYeZQJdCYoQsQ/f/QZzjx1wPwFro=; b=uTtZ9Uy3VWD+Y5GZkR8Qm4F0N+/Oi50aKCqZCgL8BztyZPnrXxkLRmv9BiRBBSx1Bt UrGHSFWWey2VmYh0SHuqtIhJrq+LdDyjIHmLApu2t1OYbhALCM79MJzNokmwwPa8vy6J vZk2bus1ta0SquV3XxI6NNKKDJ/9VLShTv9/UPt40e5fWfBBtiRh1WP8BbSRTVPpWbc/ zOUVyNCmocJcMT1ihgI58IVC1r6W7N1LvPIqESvKBdGMeMTOrS+odo154nXDSaKtdR8O QCUxyw7wkyezDRJX5M6L71IDyN8YBgiE+OxrAcEoWkUYcvKukQtk+QmWin3MOxhVt0Gr ASmg== X-Gm-Message-State: AMke39kdTULgDEReMm+7BxiLsqG5kDmEa2nPswrAvBd20e8g0556Zazj3gRMf9QS9H0ZxUSm X-Received: by 10.223.172.77 with SMTP id v71mr2698780wrc.131.1488543414531; Fri, 03 Mar 2017 04:16:54 -0800 (PST) Received: from localhost.localdomain ([105.147.1.203]) by smtp.gmail.com with ESMTPSA id w125sm2815104wme.27.2017.03.03.04.16.52 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 03 Mar 2017 04:16:53 -0800 (PST) From: Ard Biesheuvel To: edk2-devel@lists.01.org, jiewen.yao@intel.com Cc: liming.gao@intel.com, leif.lindholm@linaro.org, michael.d.kinney@intel.com, Ard Biesheuvel Date: Fri, 3 Mar 2017 12:16:48 +0000 Message-Id: <1488543408-27921-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.7.4 Subject: [PATCH] MdePkg/Uefi: pack EFI_CAPSULE_TABLE struct X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 03 Mar 2017 12:16:56 -0000 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 --- 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