[edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV.
Aaron Li
aaron.li at intel.com
Tue Dec 22 01:46:15 UTC 2020
Hi Liming,
I had replied under your comments.
I'll send the v3 version of this patch.
Best,
Aaron
-----Original Message-----
From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of gaoliming
Sent: Tuesday, December 22, 2020 9:37 AM
To: devel at edk2.groups.io; Li, Aaron <aaron.li at intel.com>
Cc: Feng, Bob C <bob.c.feng at intel.com>
Subject: 回复: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV.
Aaron:
I add my comment.
Thanks
Liming
> -----邮件原件-----
> 发件人: bounce+27952+69301+4905953+8761045 at groups.io
> <bounce+27952+69301+4905953+8761045 at groups.io> 代表 Aaron Li
> 发送时间: 2020年12月21日 14:26
> 收件人: devel at edk2.groups.io
> 抄送: Bob Feng <bob.c.feng at intel.com>; Liming Gao
> <gaoliming at byosoft.com.cn>
> 主题: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode
support
> force mode for multiple FV.
>
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3122
>
> Adding "-LF"/"-lf" option to support slot mode without FFS GUID check.
> It will support the scenario that multiple Microcode FV with different FFS
> GUID enable slot mode.
> The size of slot should be 16 byte-aligned, and larger than every
> microcode.
>
> Signed-off-by: Aaron Li <aaron.li at intel.com>
> Cc: Bob Feng <bob.c.feng at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> ---
> Silicon/Intel/Tools/FitGen/FitGen.c | 39 +++++++++++++-------
> Silicon/Intel/Tools/FitGen/FitGen.h | 2 +-
> 2 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/Silicon/Intel/Tools/FitGen/FitGen.c
> b/Silicon/Intel/Tools/FitGen/FitGen.c
> index e9541c1e95cb..6f7ddedf7e5f 100644
> --- a/Silicon/Intel/Tools/FitGen/FitGen.c
> +++ b/Silicon/Intel/Tools/FitGen/FitGen.c
> @@ -333,9 +333,10 @@ Returns:
> "\t[-F <FitTablePointerOffset>] [-F <FitTablePointerOffset>]
[-V
> <FitHeaderVersion>]\n"
>
> "\t[-NA]\n"
>
> "\t[-A <MicrocodeAlignment>]\n"
>
> - "\t[-REMAP <TopFlashAddress>\n"
>
> + "\t[-REMAP <TopFlashAddress>\n"
>
> "\t[-CLEAR]\n"
>
> "\t[-L <MicrocodeSlotSize> <MicrocodeFfsGuid>]\n"
>
> + "\t[-LF <MicrocodeSlotSize>]\n"
>
> "\t[-I <BiosInfoGuid>]\n"
>
> "\t[-S <StartupAcmAddress
> StartupAcmSize>|<StartupAcmGuid>] [-V <StartupAcmVersion>]\n"
>
> "\t[-U <DiagnstAcmAddress>|<DiagnstAcmGuid>]\n"
>
> @@ -366,6 +367,7 @@ Returns:
> printf ("\tMicrocodeGuid - Guid of Microcode Module.\n");
>
> printf ("\tMicrocodeSlotSize - Occupied region size of each
> Microcode binary.\n");
>
> printf ("\tMicrocodeFfsGuid - Guid of FFS which is used to save
> Microcode binary");
>
> + printf ("\t-LF - Microcode Slot mode without FFS
> check, treat all Microcode FV as slot mode. In this case the Microcode FV
> should only contain one FFS.\n");
>
> printf ("\t-NA - No 0x800 aligned Microcode
> requirement. No -NA means Microcode is aligned with option
> MicrocodeAlignment value.\n");
>
> printf ("\tMicrocodeAlignment - HEX value of Microcode alignment.
> Ignored if \"-NA\" is specified. Default value is 0x800. The Microcode
update
> data must start at a 16-byte aligned linear address.\n");
>
> printf ("\tRecordType - FIT entry record type. User should
> ensure it is ordered.\n");
>
> @@ -882,11 +884,13 @@ Returns:
> UINTN FitEntryNumber;
>
> BOOLEAN BiosInfoExist;
>
> BOOLEAN SlotMode;
>
> + BOOLEAN SlotModeForce;
>
> BIOS_INFO_HEADER *BiosInfo;
>
> BIOS_INFO_STRUCT *BiosInfoStruct;
>
> UINTN BiosInfoIndex;
>
>
>
> - SlotMode = FALSE;
>
> + SlotMode = FALSE;
>
> + SlotModeForce = FALSE;
>
>
>
> //
>
> // Init index
>
> @@ -1031,7 +1035,9 @@ Returns:
> //
>
> if ((Index + 1 >= argc) ||
>
> ((strcmp (argv[Index], "-L") != 0) &&
>
> - (strcmp (argv[Index], "-l") != 0)) ) {
>
> + (strcmp (argv[Index], "-l") != 0) &&
>
> + (strcmp (argv[Index], "-LF") != 0) &&
>
> + (strcmp (argv[Index], "-lf") != 0))) {
>
> //
>
> // Bypass
>
> //
>
> @@ -1039,18 +1045,21 @@ Returns:
> } else {
>
> SlotSize = xtoi (argv[Index + 1]);
>
>
>
> - if (SlotSize == 0) {
>
> - printf ("Invalid slotsize = %d\n", SlotSize);
>
> + if (SlotSize & 0xF) {
>
> + printf ("Invalid slotsize = 0x%x, Microcode data must start at a
> 16-byte aligned linear address!\n", SlotSize);
>
> return 0;
>
> }
>
If SlotSize is zero, is it valid or not?
-- Slot size should not be zero, I'll change the code.
> -
>
> - SlotMode = IsGuidData(argv[Index + 2], &MicrocodeFfsGuid);
>
> - if (!SlotMode) {
>
> - printf ("Need a ffs GUID for search uCode ffs\n");
>
> - return 0;
>
> + if (strcmp (argv[Index], "-LF") == 0 || strcmp (argv[Index], "-lf")
== 0) {
>
> + SlotModeForce = TRUE;
>
> + Index += 2;
>
> + } else {
>
> + SlotMode = IsGuidData(argv[Index + 2], &MicrocodeFfsGuid);
>
> + if (!SlotMode) {
>
> + printf ("Need a ffs GUID for search uCode ffs\n");
>
> + return 0;
>
> + }
>
> + Index += 3;
>
> }
>
> -
>
> - Index += 3;
>
> }
>
>
>
> //
>
> @@ -1219,6 +1228,10 @@ Returns:
> gFitTableContext.FitEntryNumber++;
>
>
>
> if (SlotSize != 0) {
>
> + if (SlotSize < MicrocodeSize) {
>
> + printf ("Parameter incorrect, Slot size: %x is too
small
> for Microcode size: %x!\n", SlotSize, MicrocodeSize);
>
> + return 0;
>
> + }
What purpose is for this new check?
-- The slot size should be larger than microcode size, since the microcode is contained by slot.
Thanks
Liming
>
> MicrocodeBuffer += SlotSize;
>
> } else {
>
> MicrocodeBuffer += MicrocodeSize;
>
> @@ -1228,7 +1241,7 @@ Returns:
> ///
>
> /// Check the remaining buffer
>
> ///
>
> - if (((UINT32)(MicrocodeBuffer - MicrocodeFileBuffer) <
> MicrocodeFileSize) && SlotMode != 0) {
>
> + if (((UINT32)(MicrocodeBuffer - MicrocodeFileBuffer) <
> MicrocodeFileSize) && (SlotMode || SlotModeForce)) {
>
> for (Walker = MicrocodeBuffer; Walker <
> MicrocodeFileBuffer + MicrocodeFileSize; Walker++) {
>
> if (*Walker != 0xFF) {
>
> printf ("Error: detect non-spare space after uCode
> array, please check uCode array!\n");
>
> diff --git a/Silicon/Intel/Tools/FitGen/FitGen.h
> b/Silicon/Intel/Tools/FitGen/FitGen.h
> index 435fc26209da..5add6a8870e9 100644
> --- a/Silicon/Intel/Tools/FitGen/FitGen.h
> +++ b/Silicon/Intel/Tools/FitGen/FitGen.h
> @@ -31,7 +31,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> // Utility version information
>
> //
>
> #define UTILITY_MAJOR_VERSION 0
>
> -#define UTILITY_MINOR_VERSION 63
>
> +#define UTILITY_MINOR_VERSION 64
>
> #define UTILITY_DATE __DATE__
>
>
>
> //
>
> --
> 2.29.2.windows.2
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#69301): https://edk2.groups.io/g/devel/message/69301
> Mute This Topic: https://groups.io/mt/79120990/4905953
> Group Owner: devel+owner at edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [gaoliming at byosoft.com.cn]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69360): https://edk2.groups.io/g/devel/message/69360
Mute This Topic: https://groups.io/mt/79140786/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list