> On Mar 1, 2020, at 3:41 PM, Michael D Kinney wrote: > > Andrew, > > Thanks. I did not realize this was a struct with > a single flexible array member. I agree that does > not make sense. > > The top level struct here is a HOB that must be in > contiguous memory. The HOB can vary in size based on > MicrocodeCount and StorageType. We are trying to make > it easy to write the C code that produces and consumes > this HOB. Using flexible array members helps improve > the readability when structures have this attribute. > > For the storage type defined, the StorageContext is a > single UINT64 value. Other storage type GUIDs may have > more complex StorageContext structures. This array > of structures is guaranteed to start on an 8-byte > boundary, so the use case is to cast the start of > the StorageContext to the defined type and access > the array if structures and their fields. > > Do you have any suggestions on how to make this > better? Mike, The flexible array member can not be in a struct by its self or a member of a union (I looked into that, but clang warns on that too). It has to be a the end of a structure with size. If we can't build a compound structure with the preamble data as that data varies too much the best I can think of is this: Add the known size element to enforce alignment, and make the flexible size member legal C. Add a macro that converts the pointer to raw data to the FLEXIBLE_ARRAY. Basically you need to subtract the fixed size element to get the pointer to the flexible array aligned and indexable. typedef struct { UINT64 ForceAlign; UINT8 Array[]; } FLEXIBLE_ARRAY; FLEXIBLE_ARRAY *gFlex = FLEXIBLE_ARRAY_ALIGN (RawData); Now FLEXIBLE_ARRAY.Array[] is an index into the RawData, and the definition of the FLEXIBLE_ARRAY_ALIGN can explain the trick and why it is required or standard C. Then from the C code it looks more like code that requires aligning a buffer? Sorry that is the best I've got. But at the end of the day a pointer is also a flexible array in C so that is the other option. Thanks, Andrew Fish > > Mike > >> -----Original Message----- >> From: afish@apple.com > >> Sent: Thursday, February 27, 2020 7:43 AM >> To: devel@edk2.groups.io ; Kinney, Michael D >> > >> Cc: Gao, Liming >; Fu, Siyuan >> >; Ni, Ray >; >> Chaganty, Rangasai V > >> Subject: Re: [edk2-devel] [Patch] >> IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build >> error. >> >> Mike, >> >> Flexible array members must be the last element of a >> struct but they can not be the only element. >> >> This is non standard behavior from the compilers that >> are not throwing the error. >> >> Why not just use a pointer? >> >> Sent from my iPhone >> >>> On Feb 26, 2020, at 10:03 PM, Michael D Kinney >> wrote: >>> >>> Liming, >>> >>> This does not make sense. Those compilers >>> should support C99 flexible array members. >>> >>> Structured PCDs also require use of flexible >>> array members. >>> >>> We need to make sure the compiler flags for >>> those tool chain are correct to support flexible >>> array members. >>> >>> Mike >>> >>>> -----Original Message----- >>>> From: Gao, Liming >>>> Sent: Wednesday, February 26, 2020 9:58 PM >>>> To: devel@edk2.groups.io; Kinney, Michael D >>>> ; Fu, Siyuan >>>> >>>> Cc: Ni, Ray ; Chaganty, Rangasai V >>>> >>>> Subject: RE: [edk2-devel] [Patch] >>>> IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build >>>> error. >>>> >>>> Mike: >>>> I find this issue on GCC5 tool chain tag with GCC >> 5.5 >>>> and CLANGPDB tool chain tag with CLANG 9.0.0 >>>> >>>> Thanks >>>> Liming >>>>> -----Original Message----- >>>>> From: devel@edk2.groups.io >> On >>>> Behalf Of Michael D Kinney >>>>> Sent: Thursday, February 27, 2020 1:54 PM >>>>> To: Gao, Liming ; >>>> devel@edk2.groups.io; Fu, Siyuan >> ; >>>> Kinney, Michael D >>>>> >>>>> Cc: Ni, Ray ; Chaganty, Rangasai >> V >>>> >>>>> Subject: Re: [edk2-devel] [Patch] >>>> IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build >>>> error. >>>>> >>>>> Which GCC and CLANG tool chain tags? >>>>> >>>>> Flexible array member is a standard C feature >>>>> documented in C99. >>>>> >>>>> Mike >>>>> >>>>>> -----Original Message----- >>>>>> From: Gao, Liming >>>>>> Sent: Wednesday, February 26, 2020 9:38 PM >>>>>> To: Kinney, Michael D >> ; >>>>>> devel@edk2.groups.io; Fu, Siyuan >>>> >>>>>> Cc: Ni, Ray ; Chaganty, Rangasai >>>> V >>>>>> >>>>>> Subject: RE: [edk2-devel] [Patch] >>>>>> IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build >>>>>> error. >>>>>> >>>>>> Mike: >>>>>> I find GCC and CLANG will report the error for >>>> the >>>>>> empty struct. >>>>>> >>>>>> d:\allpkg\edk2- >>>>>> >>>> >> platforms\Silicon\Intel\IntelSiliconPkg\Include\Guid/Mi >>>>>> crocodeShadowInfoHob.h:61:11: error: flexible >> array >>>>>> member 'MicrocodeAddressInFlash' not allowed in >>>>>> otherwise empty struct >>>>>> UINT64 MicrocodeAddressInFlash[]; >>>>>> ^ >>>>>> 1 error generated. >>>>>> >>>>>> Thanks >>>>>> Liming >>>>>>> -----Original Message----- >>>>>>> From: Kinney, Michael D >>>> >>>>>>> Sent: Thursday, February 27, 2020 1:25 PM >>>>>>> To: devel@edk2.groups.io; Fu, Siyuan >>>>>> ; Kinney, Michael D >>>>>> >>>>>>> Cc: Ni, Ray ; Chaganty, >>>> Rangasai V >>>>>> ; Gao, Liming >>>>>> >>>>>>> Subject: RE: [edk2-devel] [Patch] >>>>>> IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build >>>>>> error. >>>>>>> >>>>>>> What compiler does not like the flexible array >>>>>>> member syntax []. >>>>>>> >>>>>>> Mike >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: devel@edk2.groups.io >>>> >>>>>> On >>>>>>>> Behalf Of Siyuan, Fu >>>>>>>> Sent: Wednesday, February 26, 2020 5:58 PM >>>>>>>> To: devel@edk2.groups.io >>>>>>>> Cc: Ni, Ray ; Chaganty, >>>> Rangasai >>>>>> V >>>>>>>> ; Gao, Liming >>>>>>>> >>>>>>>> Subject: [edk2-devel] [Patch] >>>>>>>> IntelSiliconPkg/ShadowMicrocodePei: Fix GCC >>>> build >>>>>>>> error. >>>>>>>> >>>>>>>> This patch fixes compiler error introduced by >>>>>> commit >>>>>>>> b0099a39bd. >>>>>>>> >>>>>>>> BZ: >>>>>>>> >>>>>> >>>> >> https://tianocore.acgmultimedia.com/show_bug.cgi?id=244 >>>>>>>> 9 >>>>>>>> Cc: Ray Ni >>>>>>>> Cc: Rangasai V Chaganty >>>>>> >>>>>>>> Cc: Liming Gao >>>>>>>> Signed-off-by: Siyuan Fu >>>>>>>> --- >>>>>>>> >>>> .../Feature/ShadowMicrocode/ShadowMicrocodePei.c >>>>>>>> | 2 +- >>>>>>>> >>>>>>>> >>>>>> >>>> >> .../Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowI >>>>>>>> nfoHob.h | 2 +- >>>>>>>> 2 files changed, 2 insertions(+), 2 >>>> deletions(-) >>>>>>>> >>>>>>>> diff --git >>>>>>>> >>>>>> >>>> >> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode >>>>>>>> /ShadowMicrocodePei.c >>>>>>>> >>>>>> >>>> >> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode >>>>>>>> /ShadowMicrocodePei.c >>>>>>>> index 7e4084247e..8d6574f667 100644 >>>>>>>> --- >>>>>>>> >>>>>> >>>> >> a/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode >>>>>>>> /ShadowMicrocodePei.c >>>>>>>> +++ >>>>>>>> >>>>>> >>>> >> b/Silicon/Intel/IntelSiliconPkg/Feature/ShadowMicrocode >>>>>>>> /ShadowMicrocodePei.c >>>>>>>> @@ -247,7 +247,7 @@ ShadowMicrocodePatchWorker >>>> ( >>>>>>>> (VOID *) Patches[Index].Address, >>>>>>>> Patches[Index].Size >>>>>>>> ); >>>>>>>> - MicrocodeAddressInMemory[Index] = (UINT64) >>>>>> Walker; >>>>>>>> + MicrocodeAddressInMemory[Index] = (UINT64) >>>>>> (UINTN) >>>>>>>> Walker; >>>>>>>> Flashcontext- >>>>> MicrocodeAddressInFlash[Index] >>>>>> = >>>>>>>> (UINT64) Patches[Index].Address; >>>>>>>> Walker += Patches[Index].Size; >>>>>>>> } >>>>>>>> diff --git >>>>>>>> >>>>>> >>>> >> a/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS >>>>>>>> hadowInfoHob.h >>>>>>>> >>>>>> >>>> >> b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS >>>>>>>> hadowInfoHob.h >>>>>>>> index d887b39123..1daae1234a 100644 >>>>>>>> --- >>>>>>>> >>>>>> >>>> >> a/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS >>>>>>>> hadowInfoHob.h >>>>>>>> +++ >>>>>>>> >>>>>> >>>> >> b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeS >>>>>>>> hadowInfoHob.h >>>>>>>> @@ -58,7 +58,7 @@ typedef struct { >>>>>>>> // microcode patch address on flash. The >>>> address >>>>>> is >>>>>>>> placed in same >>>>>>>> // order as the microcode patches in >>>>>>>> MicrocodeAddrInMemory. >>>>>>>> // >>>>>>>> - UINT64 MicrocodeAddressInFlash[]; >>>>>>>> + UINT64 MicrocodeAddressInFlash[0]; >>>>>>>> } EFI_MICROCODE_STORAGE_TYPE_FLASH_CONTEXT; >>>>>>>> >>>>>>>> #endif >>>>>>>> -- >>>>>>>> 2.19.1.windows.1 >>>>>>>> >>>>>>>> >>>>>>>> >>>>> >>>>> >>>>> >>> >>> >>> >>> > >