[Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.

public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed

* [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
@ 2020-02-27  1:58 Siyuan, Fu
  2020-02-27  2:05 ` Liming Gao
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Siyuan, Fu @ 2020-02-27  1:58 UTC (permalink / raw)
  To: devel; +Cc: Ray Ni, Rangasai V Chaganty, Liming Gao

This patch fixes compiler error introduced by commit
b0099a39bd.

BZ: https://tianocore.acgmultimedia.com/show_bug.cgi?id=2449
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
---
 .../Feature/ShadowMicrocode/ShadowMicrocodePei.c                | 2 +-
 .../Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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/MicrocodeShadowInfoHob.h b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
index d887b39123..1daae1234a 100644
--- a/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  1:58 [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error Siyuan, Fu
@ 2020-02-27  2:05 ` Liming Gao
  2020-02-27  4:22 ` Ni, Ray
  2020-02-27  5:25 ` [edk2-devel] " Michael D Kinney
  2 siblings, 0 replies; 12+ messages in thread
From: Liming Gao @ 2020-02-27  2:05 UTC (permalink / raw)
  To: Fu, Siyuan, devel@edk2.groups.io; +Cc: Ni, Ray, Chaganty, Rangasai V

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Fu, Siyuan <siyuan.fu@intel.com>
> Sent: Thursday, February 27, 2020 9:58 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [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=2449
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
> ---
>  .../Feature/ShadowMicrocode/ShadowMicrocodePei.c                | 2 +-
>  .../Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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/MicrocodeShadowInfoHob.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
> index d887b39123..1daae1234a 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  1:58 [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error Siyuan, Fu
  2020-02-27  2:05 ` Liming Gao
@ 2020-02-27  4:22 ` Ni, Ray
  2020-02-27  5:25 ` [edk2-devel] " Michael D Kinney
  2 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2020-02-27  4:22 UTC (permalink / raw)
  To: Fu, Siyuan, devel@edk2.groups.io; +Cc: Chaganty, Rangasai V, Gao, Liming

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: Fu, Siyuan <siyuan.fu@intel.com>
> Sent: Thursday, February 27, 2020 9:58 AM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: [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=2449
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@intel.com>
> ---
>  .../Feature/ShadowMicrocode/ShadowMicrocodePei.c                | 2 +-
>  .../Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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/MicrocodeShadowInfoHob.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
> index d887b39123..1daae1234a 100644
> --- a/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.h
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Guid/MicrocodeShadowInfoHob.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


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  1:58 [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error Siyuan, Fu
  2020-02-27  2:05 ` Liming Gao
  2020-02-27  4:22 ` Ni, Ray
@ 2020-02-27  5:25 ` Michael D Kinney
  2020-02-27  5:38   ` Liming Gao
  2 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2020-02-27  5:25 UTC (permalink / raw)
  To: devel@edk2.groups.io, Fu, Siyuan, Kinney, Michael D
  Cc: Ni, Ray, Chaganty, Rangasai V, Gao, Liming

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.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Gao, Liming
> <liming.gao@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.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Signed-off-by: Siyuan Fu <siyuan.fu@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
> 
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  5:25 ` [edk2-devel] " Michael D Kinney
@ 2020-02-27  5:38   ` Liming Gao
  2020-02-27  5:53     ` Michael D Kinney
  0 siblings, 1 reply; 12+ messages in thread
From: Liming Gao @ 2020-02-27  5:38 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Fu, Siyuan
  Cc: Ni, Ray, Chaganty, Rangasai V

Mike:
  I find GCC and CLANG will report the error for the empty struct. 

d:\allpkg\edk2-platforms\Silicon\Intel\IntelSiliconPkg\Include\Guid/MicrocodeShadowInfoHob.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.kinney@intel.com>
> Sent: Thursday, February 27, 2020 1:25 PM
> To: devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Gao, Liming <liming.gao@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.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > <liming.gao@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.ni@intel.com>
> > Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Signed-off-by: Siyuan Fu <siyuan.fu@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
> >
> >
> > 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  5:38   ` Liming Gao
@ 2020-02-27  5:53     ` Michael D Kinney
  2020-02-27  5:58       ` Liming Gao
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2020-02-27  5:53 UTC (permalink / raw)
  To: Gao, Liming, devel@edk2.groups.io, Fu, Siyuan, Kinney, Michael D
  Cc: Ni, Ray, Chaganty, Rangasai V

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.gao@intel.com>
> Sent: Wednesday, February 26, 2020 9:38 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.kinney@intel.com>
> > Sent: Thursday, February 27, 2020 1:25 PM
> > To: devel@edk2.groups.io; Fu, Siyuan
> <siyuan.fu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@intel.com>; Gao, Liming
> <liming.gao@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.ni@intel.com>; Chaganty, Rangasai
> V
> > > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > > <liming.gao@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.ni@intel.com>
> > > Cc: Rangasai V Chaganty
> <rangasai.v.chaganty@intel.com>
> > > Cc: Liming Gao <liming.gao@intel.com>
> > > Signed-off-by: Siyuan Fu <siyuan.fu@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
> > >
> > >
> > > 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  5:53     ` Michael D Kinney
@ 2020-02-27  5:58       ` Liming Gao
  2020-02-27  6:03         ` Michael D Kinney
  0 siblings, 1 reply; 12+ messages in thread
From: Liming Gao @ 2020-02-27  5:58 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kinney, Michael D, Fu, Siyuan
  Cc: Ni, Ray, Chaganty, Rangasai V

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.gao@intel.com>; devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@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.gao@intel.com>
> > Sent: Wednesday, February 26, 2020 9:38 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.kinney@intel.com>
> > > Sent: Thursday, February 27, 2020 1:25 PM
> > > To: devel@edk2.groups.io; Fu, Siyuan
> > <siyuan.fu@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > <liming.gao@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.ni@intel.com>; Chaganty, Rangasai
> > V
> > > > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > > > <liming.gao@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.ni@intel.com>
> > > > Cc: Rangasai V Chaganty
> > <rangasai.v.chaganty@intel.com>
> > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > Signed-off-by: Siyuan Fu <siyuan.fu@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
> > > >
> > > >
> > > >
> 
> 
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  5:58       ` Liming Gao
@ 2020-02-27  6:03         ` Michael D Kinney
  2020-02-27  6:52           ` Liming Gao
  2020-02-27 15:42           ` Andrew Fish
  0 siblings, 2 replies; 12+ messages in thread
From: Michael D Kinney @ 2020-02-27  6:03 UTC (permalink / raw)
  To: Gao, Liming, devel@edk2.groups.io, Fu, Siyuan, Kinney, Michael D
  Cc: Ni, Ray, Chaganty, Rangasai V

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.gao@intel.com>
> Sent: Wednesday, February 26, 2020 9:58 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.gao@intel.com>;
> devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>;
> Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> <rangasai.v.chaganty@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.gao@intel.com>
> > > Sent: Wednesday, February 26, 2020 9:38 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > devel@edk2.groups.io; Fu, Siyuan
> <siyuan.fu@intel.com>
> > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
> V
> > > <rangasai.v.chaganty@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.kinney@intel.com>
> > > > Sent: Thursday, February 27, 2020 1:25 PM
> > > > To: devel@edk2.groups.io; Fu, Siyuan
> > > <siyuan.fu@intel.com>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
> Rangasai V
> > > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > > <liming.gao@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.ni@intel.com>; Chaganty,
> Rangasai
> > > V
> > > > > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > > > > <liming.gao@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.ni@intel.com>
> > > > > Cc: Rangasai V Chaganty
> > > <rangasai.v.chaganty@intel.com>
> > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > Signed-off-by: Siyuan Fu <siyuan.fu@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
> > > > >
> > > > >
> > > > >
> >
> >
> > 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  6:03         ` Michael D Kinney
@ 2020-02-27  6:52           ` Liming Gao
  2020-02-27 15:42           ` Andrew Fish
  1 sibling, 0 replies; 12+ messages in thread
From: Liming Gao @ 2020-02-27  6:52 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Fu, Siyuan
  Cc: Ni, Ray, Chaganty, Rangasai V, Gao, Liming

Mike:
  Current solution is to define the flexible structure array with zero. This style is supported by VS/GCC/CLANG compiler. StructurePcd also uses this solution (here is my example https://github.com/lgao4/edk2/blob/StructurePcd/TestPkg/Include/Guid/Test2.h#L32). Edk2 also have the similar usage. So, I suggest to use this solution and fix GCC failure in ShadowMicrocodePei now. Next, we can investigate the compiler flags in GCC5 and CLANGPDB for the empty flexible structure array.

Thanks
Liming
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Thursday, February 27, 2020 2:04 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Subject: RE: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
> 
> 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.gao@intel.com>
> > Sent: Wednesday, February 26, 2020 9:58 PM
> > To: devel@edk2.groups.io; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Fu, Siyuan
> > <siyuan.fu@intel.com>
> > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.gao@intel.com>;
> > devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>;
> > Kinney, Michael D
> > > <michael.d.kinney@intel.com>
> > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> > <rangasai.v.chaganty@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.gao@intel.com>
> > > > Sent: Wednesday, February 26, 2020 9:38 PM
> > > > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > > > devel@edk2.groups.io; Fu, Siyuan
> > <siyuan.fu@intel.com>
> > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
> > V
> > > > <rangasai.v.chaganty@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.kinney@intel.com>
> > > > > Sent: Thursday, February 27, 2020 1:25 PM
> > > > > To: devel@edk2.groups.io; Fu, Siyuan
> > > > <siyuan.fu@intel.com>; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>
> > > > > Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
> > Rangasai V
> > > > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > > > <liming.gao@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.ni@intel.com>; Chaganty,
> > Rangasai
> > > > V
> > > > > > <rangasai.v.chaganty@intel.com>; Gao, Liming
> > > > > > <liming.gao@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.ni@intel.com>
> > > > > > Cc: Rangasai V Chaganty
> > > > <rangasai.v.chaganty@intel.com>
> > > > > > Cc: Liming Gao <liming.gao@intel.com>
> > > > > > Signed-off-by: Siyuan Fu <siyuan.fu@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
> > > > > >
> > > > > >
> > > > > >
> > >
> > >
> > > 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27  6:03         ` Michael D Kinney
  2020-02-27  6:52           ` Liming Gao
@ 2020-02-27 15:42           ` Andrew Fish
  2020-03-01 23:41             ` Michael D Kinney
  1 sibling, 1 reply; 12+ messages in thread
From: Andrew Fish @ 2020-02-27 15:42 UTC (permalink / raw)
  To: devel, michael.d.kinney
  Cc: Gao, Liming, Fu, Siyuan, Ni, Ray, Chaganty, Rangasai V

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?
> On Feb 26, 2020, at 10:03 PM, Michael D Kinney <michael.d.kinney@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.gao@intel.com>
>> Sent: Wednesday, February 26, 2020 9:58 PM
>> To: devel@edk2.groups.io; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Fu, Siyuan
>> <siyuan.fu@intel.com>
>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
>> <rangasai.v.chaganty@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.gao@intel.com>;
>> devel@edk2.groups.io; Fu, Siyuan <siyuan.fu@intel.com>;
>> Kinney, Michael D
>>> <michael.d.kinney@intel.com>
>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
>> <rangasai.v.chaganty@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.gao@intel.com>
>>>> Sent: Wednesday, February 26, 2020 9:38 PM
>>>> To: Kinney, Michael D <michael.d.kinney@intel.com>;
>>>> devel@edk2.groups.io; Fu, Siyuan
>> <siyuan.fu@intel.com>
>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
>> V
>>>> <rangasai.v.chaganty@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.kinney@intel.com>
>>>>> Sent: Thursday, February 27, 2020 1:25 PM
>>>>> To: devel@edk2.groups.io; Fu, Siyuan
>>>> <siyuan.fu@intel.com>; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>
>>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
>> Rangasai V
>>>> <rangasai.v.chaganty@intel.com>; Gao, Liming
>>>> <liming.gao@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.ni@intel.com>; Chaganty,
>> Rangasai
>>>> V
>>>>>> <rangasai.v.chaganty@intel.com>; Gao, Liming
>>>>>> <liming.gao@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.ni@intel.com>
>>>>>> Cc: Rangasai V Chaganty
>>>> <rangasai.v.chaganty@intel.com>
>>>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>>>> Signed-off-by: Siyuan Fu <siyuan.fu@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
>>>>>> 
>>>>>> 
>>>>>> 
>>> 
>>> 
>>> 
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-02-27 15:42           ` Andrew Fish
@ 2020-03-01 23:41             ` Michael D Kinney
  2020-03-02  3:47               ` Andrew Fish
  0 siblings, 1 reply; 12+ messages in thread
From: Michael D Kinney @ 2020-03-01 23:41 UTC (permalink / raw)
  To: afish@apple.com, devel@edk2.groups.io, Kinney, Michael D
  Cc: Gao, Liming, Fu, Siyuan, Ni, Ray, Chaganty, Rangasai V

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

> -----Original Message-----
> From: afish@apple.com <afish@apple.com>
> Sent: Thursday, February 27, 2020 7:43 AM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Gao, Liming <liming.gao@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Chaganty, Rangasai V <rangasai.v.chaganty@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.kinney@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.gao@intel.com>
> >> Sent: Wednesday, February 26, 2020 9:58 PM
> >> To: devel@edk2.groups.io; Kinney, Michael D
> >> <michael.d.kinney@intel.com>; Fu, Siyuan
> >> <siyuan.fu@intel.com>
> >> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
> >> <rangasai.v.chaganty@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.gao@intel.com>;
> >> devel@edk2.groups.io; Fu, Siyuan
> <siyuan.fu@intel.com>;
> >> Kinney, Michael D
> >>> <michael.d.kinney@intel.com>
> >>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
> V
> >> <rangasai.v.chaganty@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.gao@intel.com>
> >>>> Sent: Wednesday, February 26, 2020 9:38 PM
> >>>> To: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> >>>> devel@edk2.groups.io; Fu, Siyuan
> >> <siyuan.fu@intel.com>
> >>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
> >> V
> >>>> <rangasai.v.chaganty@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.kinney@intel.com>
> >>>>> Sent: Thursday, February 27, 2020 1:25 PM
> >>>>> To: devel@edk2.groups.io; Fu, Siyuan
> >>>> <siyuan.fu@intel.com>; Kinney, Michael D
> >>>> <michael.d.kinney@intel.com>
> >>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
> >> Rangasai V
> >>>> <rangasai.v.chaganty@intel.com>; Gao, Liming
> >>>> <liming.gao@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.ni@intel.com>; Chaganty,
> >> Rangasai
> >>>> V
> >>>>>> <rangasai.v.chaganty@intel.com>; Gao, Liming
> >>>>>> <liming.gao@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.ni@intel.com>
> >>>>>> Cc: Rangasai V Chaganty
> >>>> <rangasai.v.chaganty@intel.com>
> >>>>>> Cc: Liming Gao <liming.gao@intel.com>
> >>>>>> Signed-off-by: Siyuan Fu <siyuan.fu@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
> >>>>>>
> >>>>>>
> >>>>>>
> >>>
> >>>
> >>>
> >
> >
> > 
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [edk2-devel] [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error.
  2020-03-01 23:41             ` Michael D Kinney
@ 2020-03-02  3:47               ` Andrew Fish
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Fish @ 2020-03-02  3:47 UTC (permalink / raw)
  To: devel, Mike Kinney; +Cc: Gao, Liming, Fu, Siyuan, Ni, Ray, Chaganty, Rangasai V

[-- Attachment #1: Type: text/plain, Size: 10121 bytes --]


> On Mar 1, 2020, at 3:41 PM, Michael D Kinney <michael.d.kinney@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: afish@apple.com <mailto:afish@apple.com> <afish@apple.com <mailto:afish@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.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
>> Cc: Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>; Fu, Siyuan
>> <siyuan.fu@intel.com <mailto:siyuan.fu@intel.com>>; Ni, Ray <ray.ni@intel.com <mailto:ray.ni@intel.com>>;
>> Chaganty, Rangasai V <rangasai.v.chaganty@intel.com <mailto:rangasai.v.chaganty@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.kinney@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.gao@intel.com>
>>>> Sent: Wednesday, February 26, 2020 9:58 PM
>>>> To: devel@edk2.groups.io; Kinney, Michael D
>>>> <michael.d.kinney@intel.com>; Fu, Siyuan
>>>> <siyuan.fu@intel.com>
>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V
>>>> <rangasai.v.chaganty@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.gao@intel.com>;
>>>> devel@edk2.groups.io; Fu, Siyuan
>> <siyuan.fu@intel.com>;
>>>> Kinney, Michael D
>>>>> <michael.d.kinney@intel.com>
>>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
>> V
>>>> <rangasai.v.chaganty@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.gao@intel.com>
>>>>>> Sent: Wednesday, February 26, 2020 9:38 PM
>>>>>> To: Kinney, Michael D
>> <michael.d.kinney@intel.com>;
>>>>>> devel@edk2.groups.io; Fu, Siyuan
>>>> <siyuan.fu@intel.com>
>>>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai
>>>> V
>>>>>> <rangasai.v.chaganty@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.kinney@intel.com>
>>>>>>> Sent: Thursday, February 27, 2020 1:25 PM
>>>>>>> To: devel@edk2.groups.io; Fu, Siyuan
>>>>>> <siyuan.fu@intel.com>; Kinney, Michael D
>>>>>> <michael.d.kinney@intel.com>
>>>>>>> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty,
>>>> Rangasai V
>>>>>> <rangasai.v.chaganty@intel.com>; Gao, Liming
>>>>>> <liming.gao@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.ni@intel.com>; Chaganty,
>>>> Rangasai
>>>>>> V
>>>>>>>> <rangasai.v.chaganty@intel.com>; Gao, Liming
>>>>>>>> <liming.gao@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.ni@intel.com>
>>>>>>>> Cc: Rangasai V Chaganty
>>>>>> <rangasai.v.chaganty@intel.com>
>>>>>>>> Cc: Liming Gao <liming.gao@intel.com>
>>>>>>>> Signed-off-by: Siyuan Fu <siyuan.fu@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
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>> 
>>> 
>>> 
>>> 
> 
> 


[-- Attachment #2: Type: text/html, Size: 38988 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-03-02  3:47 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-02-27  1:58 [Patch] IntelSiliconPkg/ShadowMicrocodePei: Fix GCC build error Siyuan, Fu
2020-02-27  2:05 ` Liming Gao
2020-02-27  4:22 ` Ni, Ray
2020-02-27  5:25 ` [edk2-devel] " Michael D Kinney
2020-02-27  5:38   ` Liming Gao
2020-02-27  5:53     ` Michael D Kinney
2020-02-27  5:58       ` Liming Gao
2020-02-27  6:03         ` Michael D Kinney
2020-02-27  6:52           ` Liming Gao
2020-02-27 15:42           ` Andrew Fish
2020-03-01 23:41             ` Michael D Kinney
2020-03-02  3:47               ` Andrew Fish

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox