> On Mar 1, 2020, at 3:41 PM, Michael D Kinney <michael.d.kin...@intel.com>
> 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: af...@apple.com <mailto:af...@apple.com> <af...@apple.com
>> <mailto:af...@apple.com>>
>> Sent: Thursday, February 27, 2020 7:43 AM
>> To: devel@edk2.groups.io <mailto:devel@edk2.groups.io>; Kinney, Michael D
>> <michael.d.kin...@intel.com <mailto:michael.d.kin...@intel.com>>
>> Cc: Gao, Liming <liming....@intel.com <mailto:liming....@intel.com>>; Fu,
>> Siyuan
>> <siyuan...@intel.com <mailto:siyuan...@intel.com>>; Ni, Ray
>> <ray...@intel.com <mailto:ray...@intel.com>>;
>> Chaganty, Rangasai V <rangasai.v.chaga...@intel.com
>> <mailto:rangasai.v.chaga...@intel.com>>
>> 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
>> <michael.d.kin...@intel.com> 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 <liming....@intel.com>
>>>> Sent: Wednesday, February 26, 2020 9:58 PM
>>>> To: devel@edk2.groups.io; Kinney, Michael D
>>>> <michael.d.kin...@intel.com>; Fu, Siyuan
>>>> <siyuan...@intel.com>
>>>> Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai V
>>>> <rangasai.v.chaga...@intel.com>
>>>> 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 <devel@edk2.groups.io>
>> On
>>>> Behalf Of Michael D Kinney
>>>>> Sent: Thursday, February 27, 2020 1:54 PM
>>>>> To: Gao, Liming <liming....@intel.com>;
>>>> devel@edk2.groups.io; Fu, Siyuan
>> <siyuan...@intel.com>;
>>>> Kinney, Michael D
>>>>> <michael.d.kin...@intel.com>
>>>>> Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai
>> V
>>>> <rangasai.v.chaga...@intel.com>
>>>>> 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 <liming....@intel.com>
>>>>>> Sent: Wednesday, February 26, 2020 9:38 PM
>>>>>> To: Kinney, Michael D
>> <michael.d.kin...@intel.com>;
>>>>>> devel@edk2.groups.io; Fu, Siyuan
>>>> <siyuan...@intel.com>
>>>>>> Cc: Ni, Ray <ray...@intel.com>; Chaganty, Rangasai
>>>> V
>>>>>> <rangasai.v.chaga...@intel.com>
>>>>>> 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
>>>> <michael.d.kin...@intel.com>
>>>>>>> Sent: Thursday, February 27, 2020 1:25 PM
>>>>>>> To: devel@edk2.groups.io; Fu, Siyuan
>>>>>> <siyuan...@intel.com>; Kinney, Michael D
>>>>>> <michael.d.kin...@intel.com>
>>>>>>> Cc: Ni, Ray <ray...@intel.com>; Chaganty,
>>>> Rangasai V
>>>>>> <rangasai.v.chaga...@intel.com>; Gao, Liming
>>>>>> <liming....@intel.com>
>>>>>>> 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
>>>> <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 <ray...@intel.com>; Chaganty,
>>>> Rangasai
>>>>>> V
>>>>>>>> <rangasai.v.chaga...@intel.com>; Gao, Liming
>>>>>>>> <liming....@intel.com>
>>>>>>>> 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 <ray...@intel.com>
>>>>>>>> Cc: Rangasai V Chaganty
>>>>>> <rangasai.v.chaga...@intel.com>
>>>>>>>> Cc: Liming Gao <liming....@intel.com>
>>>>>>>> Signed-off-by: Siyuan Fu <siyuan...@intel.com>
>>>>>>>> ---
>>>>>>>>
>>>> .../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
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>
>>>>>
>>>>>
>>>
>>>
>>>
>>>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55136): https://edk2.groups.io/g/devel/message/55136
Mute This Topic: https://groups.io/mt/71579115/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
