From: "Yuwei Chen" <yuwei.chen@intel.com>
To: "Lin, Jason1" <jason1.lin@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Feng, Bob C" <bob.c.feng@intel.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>,
"Oram, Isaac W" <isaac.w.oram@intel.com>,
"Chaganty, Rangasai V" <rangasai.v.chaganty@intel.com>,
"Chiang, Dakota" <dakota.chiang@intel.com>
Subject: Re: [PATCH v3 2/3] [edk2-platforms] Silicon/Intel/FitGen: Reduce the typecasting and pointer usage
Date: Mon, 4 Jul 2022 01:37:17 +0000 [thread overview]
Message-ID: <MW5PR11MB590641859DD4F43516A8AC0196BE9@MW5PR11MB5906.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220701151005.189-2-jason1.lin@intel.com>
Reviewed-by: Yuwei Chen<yuwei.chen@intel.com>
> -----Original Message-----
> From: Lin, Jason1 <jason1.lin@intel.com>
> Sent: Friday, July 1, 2022 11:10 PM
> To: devel@edk2.groups.io
> Cc: Lin, Jason1 <jason1.lin@intel.com>; Feng, Bob C <bob.c.feng@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Chen, Christine
> <yuwei.chen@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Chiang, Dakota
> <dakota.chiang@intel.com>
> Subject: [PATCH v3 2/3] [edk2-platforms] Silicon/Intel/FitGen: Reduce the
> typecasting and pointer usage
>
> From: Jason1 Lin <jason1.lin@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3958
>
> FitGen tool exists lots of typecasting and pointer usage.
> This code change is used to reduce these in FillFitTable () and GetFitEntryInfo
> ().
> To make code more clearly and easy to read.
>
> Signed-off-by: Jason1 Lin <jason1.lin@intel.com>
> Cc: Bob Feng <bob.c.feng@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Yuwei Chen <yuwei.chen@intel.com>
> Cc: Isaac W Oram <isaac.w.oram@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Dakota Chiang <dakota.chiang@intel.com>
> ---
> Silicon/Intel/Tools/FitGen/FitGen.c | 125 ++++++++++++--------
> 1 file changed, 78 insertions(+), 47 deletions(-)
>
> diff --git a/Silicon/Intel/Tools/FitGen/FitGen.c
> b/Silicon/Intel/Tools/FitGen/FitGen.c
> index eac8fa8715..01b4f82518 100644
> --- a/Silicon/Intel/Tools/FitGen/FitGen.c
> +++ b/Silicon/Intel/Tools/FitGen/FitGen.c
> @@ -2768,6 +2768,7 @@ Returns:
> { FIRMWARE_INTERFACE_TABLE_ENTRY *FitEntry; UINT32
> FitIndex;+ UINT32 FitEntrySizeValue; UINT32
> Index; UINT8 Checksum; UINTN SubIndex;@@
> -2788,27 +2789,35 @@ Returns:
> // // 2. FitHeader //- FitEntry[FitIndex].Address = *(UINT64
> *)"_FIT_ ";- *(UINT32 *)&FitEntry[FitIndex].Size[0] =
> gFitTableContext.FitEntryNumber;- FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.FitHeaderVersion;- FitEntry[FitIndex].Type
> = FIT_TABLE_TYPE_HEADER;- FitEntry[FitIndex].C_V = 1;+
> FitEntrySizeValue = gFitTableContext.FitEntryNumber;+
> FitEntry[FitIndex].Address = *(UINT64 *)"_FIT_ ";+ FitEntry[FitIndex].Size[0]
> = (UINT8)FitEntrySizeValue;+ FitEntry[FitIndex].Size[1] =
> (UINT8)(FitEntrySizeValue >> 8);+ FitEntry[FitIndex].Size[2] =
> (UINT8)(FitEntrySizeValue >> 16);+ FitEntry[FitIndex].Rsvd = 0;+
> FitEntry[FitIndex].Version = (UINT16)gFitTableContext.FitHeaderVersion;+
> FitEntry[FitIndex].Type = FIT_TABLE_TYPE_HEADER;+
> FitEntry[FitIndex].C_V = 1; // // Checksum will be updated later... //-
> FitEntry[FitIndex].Checksum = 0;+ FitEntry[FitIndex].Checksum = 0; //
> // 3. Microcode // FitIndex++; for (Index = 0; Index <
> gFitTableContext.MicrocodeNumber; Index++) {- FitEntry[FitIndex].Address
> = gFitTableContext.Microcode[Index].Address;- *(UINT32
> *)&FitEntry[FitIndex].Size[0] = 0; //gFitTableContext.Microcode[Index].Size /
> 16;- FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.MicrocodeVersion;- FitEntry[FitIndex].Type
> = FIT_TABLE_TYPE_MICROCODE;- FitEntry[FitIndex].C_V = 0;-
> FitEntry[FitIndex].Checksum = 0;+ FitEntrySizeValue = 0; //
> gFitTableContext.Microcode[Index].Size / 16+ FitEntry[FitIndex].Address =
> gFitTableContext.Microcode[Index].Address;+ FitEntry[FitIndex].Size[0] =
> (UINT8)FitEntrySizeValue;+ FitEntry[FitIndex].Size[1] =
> (UINT8)(FitEntrySizeValue >> 8);+ FitEntry[FitIndex].Size[2] =
> (UINT8)(FitEntrySizeValue >> 16);+ FitEntry[FitIndex].Rsvd = 0;+
> FitEntry[FitIndex].Version = (UINT16)gFitTableContext.MicrocodeVersion;+
> FitEntry[FitIndex].Type = FIT_TABLE_TYPE_MICROCODE;+
> FitEntry[FitIndex].C_V = 0;+ FitEntry[FitIndex].Checksum = 0;
> FitIndex++; } @@ -2816,12 +2825,16 @@ Returns:
> // 4. StartupAcm // for (Index = 0; Index <
> gFitTableContext.StartupAcmNumber; Index++) {-
> FitEntry[FitIndex].Address =
> gFitTableContext.StartupAcm[Index].Address;- *(UINT32
> *)&FitEntry[FitIndex].Size[0] = 0; //gFitTableContext.StartupAcm.Size / 16;-
> FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.StartupAcmVersion;- FitEntry[FitIndex].Type
> = FIT_TABLE_TYPE_STARTUP_ACM;- FitEntry[FitIndex].C_V = 0;-
> FitEntry[FitIndex].Checksum = 0;+ FitEntrySizeValue = 0; //
> gFitTableContext.StartupAcm.Size / 16+ FitEntry[FitIndex].Address =
> gFitTableContext.StartupAcm[Index].Address;+ FitEntry[FitIndex].Size[0] =
> (UINT8)FitEntrySizeValue;+ FitEntry[FitIndex].Size[1] =
> (UINT8)(FitEntrySizeValue >> 8);+ FitEntry[FitIndex].Size[2] =
> (UINT8)(FitEntrySizeValue >> 16);+ FitEntry[FitIndex].Rsvd = 0;+
> FitEntry[FitIndex].Version = (UINT16)gFitTableContext.StartupAcmVersion;+
> FitEntry[FitIndex].Type = FIT_TABLE_TYPE_STARTUP_ACM;+
> FitEntry[FitIndex].C_V = 0;+ FitEntry[FitIndex].Checksum = 0;
> FitIndex++; } @@ -2829,19 +2842,23 @@ Returns:
> // 4.5. DiagnosticAcm // if (gFitTableContext.DiagnstAcm.Address != 0) {-
> FitEntry[FitIndex].Address = gFitTableContext.DiagnstAcm.Address;-
> *(UINT32 *)&FitEntry[FitIndex].Size[0] = 0;- FitEntry[FitIndex].Version
> = (UINT16)gFitTableContext.DiagnstAcmVersion;- FitEntry[FitIndex].Type
> = FIT_TABLE_TYPE_DIAGNST_ACM;- FitEntry[FitIndex].C_V = 0;-
> FitEntry[FitIndex].Checksum = 0;+ FitEntrySizeValue = 0; //
> gFitTableContext.DiagnstAcmVersion.Size / 16+ FitEntry[FitIndex].Address
> = gFitTableContext.DiagnstAcm.Address;+ FitEntry[FitIndex].Size[0] =
> (UINT8)FitEntrySizeValue;+ FitEntry[FitIndex].Size[1] =
> (UINT8)(FitEntrySizeValue >> 8);+ FitEntry[FitIndex].Size[2] =
> (UINT8)(FitEntrySizeValue >> 16);+ FitEntry[FitIndex].Rsvd = 0;+
> FitEntry[FitIndex].Version = (UINT16)gFitTableContext.DiagnstAcmVersion;+
> FitEntry[FitIndex].Type = FIT_TABLE_TYPE_DIAGNST_ACM;+
> FitEntry[FitIndex].C_V = 0;+ FitEntry[FitIndex].Checksum = 0;
> FitIndex++; } // // 5. BiosModule // //- // BiosModule segments order
> needs to be put from low addresss to high for Btg requirement+ //
> BiosModule segments order needs to be put from low address to high for Btg
> requirement // if (gFitTableContext.BiosModuleNumber > 1) { for (Index
> = 0; Index < (UINTN)gFitTableContext.BiosModuleNumber - 1; Index++){@@ -
> 2855,12 +2872,16 @@ Returns:
> } } for (Index = 0; Index < gFitTableContext.BiosModuleNumber; Index++)
> {- FitEntry[FitIndex].Address =
> gFitTableContext.BiosModule[Index].Address;- *(UINT32
> *)&FitEntry[FitIndex].Size[0] = gFitTableContext.BiosModule[Index].Size / 16;-
> FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.BiosModuleVersion;- FitEntry[FitIndex].Type
> = FIT_TABLE_TYPE_BIOS_MODULE;- FitEntry[FitIndex].C_V = 0;-
> FitEntry[FitIndex].Checksum = 0;+ FitEntrySizeValue =
> gFitTableContext.BiosModule[Index].Size / 16;+ FitEntry[FitIndex].Address
> = gFitTableContext.BiosModule[Index].Address;+ FitEntry[FitIndex].Size[0]
> = (UINT8)FitEntrySizeValue;+ FitEntry[FitIndex].Size[1] =
> (UINT8)(FitEntrySizeValue >> 8);+ FitEntry[FitIndex].Size[2] =
> (UINT8)(FitEntrySizeValue >> 16);+ FitEntry[FitIndex].Rsvd = 0;+
> FitEntry[FitIndex].Version = (UINT16)gFitTableContext.BiosModuleVersion;+
> FitEntry[FitIndex].Type = FIT_TABLE_TYPE_BIOS_MODULE;+
> FitEntry[FitIndex].C_V = 0;+ FitEntry[FitIndex].Checksum = 0;
> FitIndex++; } @@ -2868,15 +2889,18 @@ Returns:
> // 6. Optional module // for (Index = 0; Index <
> gFitTableContext.OptionalModuleNumber; Index++) {-
> FitEntry[FitIndex].Address =
> gFitTableContext.OptionalModule[Index].Address;- *(UINT32
> *)&FitEntry[FitIndex].Size[0] = gFitTableContext.OptionalModule[Index].Size;-
> FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.OptionalModule[Index].Version;-
> FitEntry[FitIndex].Type =
> (UINT8)gFitTableContext.OptionalModule[Index].Type;+ FitEntrySizeValue
> = gFitTableContext.OptionalModule[Index].Size;+ FitEntry[FitIndex].Address
> = gFitTableContext.OptionalModule[Index].Address;+
> FitEntry[FitIndex].Size[0] = (UINT8)FitEntrySizeValue;+
> FitEntry[FitIndex].Size[1] = (UINT8)(FitEntrySizeValue >> 8);+
> FitEntry[FitIndex].Size[2] = (UINT8)(FitEntrySizeValue >> 16);+
> FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.OptionalModule[Index].Version;+
> FitEntry[FitIndex].Type =
> (UINT8)gFitTableContext.OptionalModule[Index].Type; if
> (FitEntry[FitIndex].Type == FIT_TABLE_TYPE_CSE_SECURE_BOOT) {-
> FitEntry[FitIndex].Rsvd =
> (UINT8)gFitTableContext.OptionalModule[Index].SubType;+
> FitEntry[FitIndex].Rsvd =
> (UINT8)gFitTableContext.OptionalModule[Index].SubType; }-
> FitEntry[FitIndex].C_V = 0;- FitEntry[FitIndex].Checksum = 0;+
> FitEntry[FitIndex].C_V = 0;+ FitEntry[FitIndex].Checksum = 0;
> FitIndex++; } @@ -2884,12 +2908,16 @@ Returns:
> // 7. Port module // for (Index = 0; Index <
> gFitTableContext.PortModuleNumber; Index++) {-
> FitEntry[FitIndex].Address =
> gFitTableContext.PortModule[Index].Address +
> ((UINT64)gFitTableContext.PortModule[Index].Size << 32);- *(UINT32
> *)&FitEntry[FitIndex].Size[0] = 0;- FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.PortModule[Index].Version;-
> FitEntry[FitIndex].Type =
> (UINT8)gFitTableContext.PortModule[Index].Type;- FitEntry[FitIndex].C_V
> = 0;- FitEntry[FitIndex].Checksum = 0;+ FitEntrySizeValue = 0;+
> FitEntry[FitIndex].Address = gFitTableContext.PortModule[Index].Address +
> ((UINT64)gFitTableContext.PortModule[Index].Size << 32);+
> FitEntry[FitIndex].Size[0] = (UINT8)FitEntrySizeValue;+
> FitEntry[FitIndex].Size[1] = (UINT8)(FitEntrySizeValue >> 8);+
> FitEntry[FitIndex].Size[2] = (UINT8)(FitEntrySizeValue >> 16);+
> FitEntry[FitIndex].Rsvd = 0;+ FitEntry[FitIndex].Version =
> (UINT16)gFitTableContext.PortModule[Index].Version;+
> FitEntry[FitIndex].Type =
> (UINT8)gFitTableContext.PortModule[Index].Type;+ FitEntry[FitIndex].C_V
> = 0;+ FitEntry[FitIndex].Checksum = 0; FitIndex++; } @@ -3130,6 +3158,7
> @@ Returns:
> --*/ { FIRMWARE_INTERFACE_TABLE_ENTRY *FitEntry;+ UINT32
> FitEntrySizeValue; UINT32 FitIndex; UINT32
> FitTableOffset; @@ -3155,7 +3184,8 @@ Returns:
> if (FitEntry[FitIndex].Type != FIT_TABLE_TYPE_HEADER) { return 0; }-
> gFitTableContext.FitEntryNumber = *(UINT32 *)&FitEntry[FitIndex].Size[0];+
> FitEntrySizeValue = (((UINT32)FitEntry[FitIndex].Size[2]) << 16) +
> (((UINT32)FitEntry[FitIndex].Size[1]) << 8) +
> ((UINT32)FitEntry[FitIndex].Size[0]);+ gFitTableContext.FitEntryNumber =
> FitEntrySizeValue; gFitTableContext.FitHeaderVersion =
> FitEntry[FitIndex].Version; //@@ -3163,6 +3193,7 @@ Returns:
> // FitIndex++; for (; FitIndex < gFitTableContext.FitEntryNumber;
> FitIndex++) {+ FitEntrySizeValue = (((UINT32)FitEntry[FitIndex].Size[2]) << 16)
> + (((UINT32)FitEntry[FitIndex].Size[1]) << 8) +
> ((UINT32)FitEntry[FitIndex].Size[0]); switch (FitEntry[FitIndex].Type)
> { case FIT_TABLE_TYPE_MICROCODE:
> gFitTableContext.Microcode[gFitTableContext.MicrocodeNumber].Address =
> (UINT32)FitEntry[FitIndex].Address;@@ -3175,7 +3206,7 @@ Returns:
> break; case FIT_TABLE_TYPE_BIOS_MODULE:
> gFitTableContext.BiosModule[gFitTableContext.BiosModuleNumber].Address
> = (UINT32)FitEntry[FitIndex].Address;-
> gFitTableContext.BiosModule[gFitTableContext.BiosModuleNumber].Size =
> *(UINT32 *)&FitEntry[FitIndex].Size[0] * 16;+
> gFitTableContext.BiosModule[gFitTableContext.BiosModuleNumber].Size =
> FitEntrySizeValue * 16; gFitTableContext.BiosModuleVersion
> = FitEntry[FitIndex].Version; gFitTableContext.BiosModuleNumber ++;
> break;@@ -3192,7 +3223,7 @@ Returns:
> // Not Port Configure, pass through default: // Others
> gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber
> ].Address = (UINT32)FitEntry[FitIndex].Address;-
> gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber
> ].Size = *(UINT32 *)&FitEntry[FitIndex].Size[0];+
> gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber
> ].Size = FitEntrySizeValue;
> gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber
> ].Version = FitEntry[FitIndex].Version;
> gFitTableContext.OptionalModule[gFitTableContext.OptionalModuleNumber
> ].Type = FitEntry[FitIndex].Type;
> gFitTableContext.OptionalModuleNumber ++;--
> 2.37.0.windows.1
next prev parent reply other threads:[~2022-07-04 1:37 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-01 15:10 [PATCH v3 1/3] [edk2-platforms] Silicon/Intel/FitGen: Support multiple Startup ACM Type 2 entries in FitGen tool Lin, Jason1
2022-07-01 15:10 ` [PATCH v3 2/3] [edk2-platforms] Silicon/Intel/FitGen: Reduce the typecasting and pointer usage Lin, Jason1
2022-07-01 18:34 ` Oram, Isaac W
2022-07-04 1:37 ` Yuwei Chen [this message]
2022-07-01 15:10 ` [PATCH v3 3/3] [edk2-platforms] Silicon/Intel/FitGen: Support Startup ACM entries (Type 2) 0x200 Version Lin, Jason1
2022-07-01 18:34 ` Oram, Isaac W
2022-07-04 1:37 ` Yuwei Chen
2022-07-01 18:34 ` [PATCH v3 1/3] [edk2-platforms] Silicon/Intel/FitGen: Support multiple Startup ACM Type 2 entries in FitGen tool Oram, Isaac W
2022-07-04 1:35 ` [edk2-devel] " Yuwei Chen
2022-07-05 13:06 ` Bob Feng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MW5PR11MB590641859DD4F43516A8AC0196BE9@MW5PR11MB5906.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox