* [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV. @ 2020-12-21 6:25 Aaron Li 2020-12-22 1:37 ` 回复: [edk2-devel] " gaoliming 0 siblings, 1 reply; 5+ messages in thread From: Aaron Li @ 2020-12-21 6:25 UTC (permalink / raw) To: devel; +Cc: Bob Feng, Liming Gao 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@intel.com> Cc: Bob Feng <bob.c.feng@intel.com> Cc: Liming Gao <gaoliming@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; } - - 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; + } 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* 回复: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV. 2020-12-21 6:25 [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV Aaron Li @ 2020-12-22 1:37 ` gaoliming 2020-12-22 1:46 ` Aaron Li 2020-12-22 1:46 ` Bob Feng 0 siblings, 2 replies; 5+ messages in thread From: gaoliming @ 2020-12-22 1:37 UTC (permalink / raw) To: devel, aaron.li; +Cc: 'Bob Feng' Aaron: I add my comment. Thanks Liming > -----邮件原件----- > 发件人: bounce+27952+69301+4905953+8761045@groups.io > <bounce+27952+69301+4905953+8761045@groups.io> 代表 Aaron Li > 发送时间: 2020年12月21日 14:26 > 收件人: devel@edk2.groups.io > 抄送: Bob Feng <bob.c.feng@intel.com>; Liming Gao > <gaoliming@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@intel.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <gaoliming@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? > - > > - 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? 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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [gaoliming@byosoft.com.cn] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV. 2020-12-22 1:37 ` 回复: [edk2-devel] " gaoliming @ 2020-12-22 1:46 ` Aaron Li 2020-12-22 1:46 ` Bob Feng 1 sibling, 0 replies; 5+ messages in thread From: Aaron Li @ 2020-12-22 1:46 UTC (permalink / raw) To: devel@edk2.groups.io, gaoliming@byosoft.com.cn; +Cc: Feng, Bob C Hi Liming, I had replied under your comments. I'll send the v3 version of this patch. Best, Aaron -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming Sent: Tuesday, December 22, 2020 9:37 AM To: devel@edk2.groups.io; Li, Aaron <aaron.li@intel.com> Cc: Feng, Bob C <bob.c.feng@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@groups.io > <bounce+27952+69301+4905953+8761045@groups.io> 代表 Aaron Li > 发送时间: 2020年12月21日 14:26 > 收件人: devel@edk2.groups.io > 抄送: Bob Feng <bob.c.feng@intel.com>; Liming Gao > <gaoliming@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@intel.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <gaoliming@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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [gaoliming@byosoft.com.cn] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV. 2020-12-22 1:37 ` 回复: [edk2-devel] " gaoliming 2020-12-22 1:46 ` Aaron Li @ 2020-12-22 1:46 ` Bob Feng 2020-12-22 1:56 ` Aaron Li 1 sibling, 1 reply; 5+ messages in thread From: Bob Feng @ 2020-12-22 1:46 UTC (permalink / raw) To: devel@edk2.groups.io, gaoliming@byosoft.com.cn, Li, Aaron Aaron, I have the same questions. Thanks, Bob -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming Sent: Tuesday, December 22, 2020 9:37 AM To: devel@edk2.groups.io; Li, Aaron <aaron.li@intel.com> Cc: Feng, Bob C <bob.c.feng@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@groups.io > <bounce+27952+69301+4905953+8761045@groups.io> 代表 Aaron Li > 发送时间: 2020年12月21日 14:26 > 收件人: devel@edk2.groups.io > 抄送: Bob Feng <bob.c.feng@intel.com>; Liming Gao > <gaoliming@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@intel.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <gaoliming@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? > - > > - 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? 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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [gaoliming@byosoft.com.cn] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV. 2020-12-22 1:46 ` Bob Feng @ 2020-12-22 1:56 ` Aaron Li 0 siblings, 0 replies; 5+ messages in thread From: Aaron Li @ 2020-12-22 1:56 UTC (permalink / raw) To: Feng, Bob C, devel@edk2.groups.io, gaoliming@byosoft.com.cn Hi Bob & Liming, I had replied in previous mail. If SlotSize is zero, is it valid or not? -- Slot size should not be zero, I'll change the code. What purpose is for this new check? -- The slot size should be larger than microcode size, since the microcode is contained by slot. Do you agree with adding this new check, if so, I'll not remove this in my next patch. Thanks. Best, Aaron -----Original Message----- From: Feng, Bob C <bob.c.feng@intel.com> Sent: Tuesday, December 22, 2020 9:47 AM To: devel@edk2.groups.io; gaoliming@byosoft.com.cn; Li, Aaron <aaron.li@intel.com> Subject: RE: [edk2-devel] [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV. Aaron, I have the same questions. Thanks, Bob -----Original Message----- From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming Sent: Tuesday, December 22, 2020 9:37 AM To: devel@edk2.groups.io; Li, Aaron <aaron.li@intel.com> Cc: Feng, Bob C <bob.c.feng@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@groups.io > <bounce+27952+69301+4905953+8761045@groups.io> 代表 Aaron Li > 发送时间: 2020年12月21日 14:26 > 收件人: devel@edk2.groups.io > 抄送: Bob Feng <bob.c.feng@intel.com>; Liming Gao > <gaoliming@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@intel.com> > Cc: Bob Feng <bob.c.feng@intel.com> > Cc: Liming Gao <gaoliming@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? > - > > - 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? 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@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub > [gaoliming@byosoft.com.cn] > -=-=-=-=-=-= > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-12-22 1:56 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-12-21 6:25 [PATCH v2 1/1] Silicon/FitGen: Enhance Slot mode support force mode for multiple FV Aaron Li 2020-12-22 1:37 ` 回复: [edk2-devel] " gaoliming 2020-12-22 1:46 ` Aaron Li 2020-12-22 1:46 ` Bob Feng 2020-12-22 1:56 ` Aaron Li
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox