[PATCH edk2-staging 00/20] IntelUndiPkg/XGigUndiDxe: fix GCC / ARM build issues

[PATCH edk2-staging 00/20] IntelUndiPkg/XGigUndiDxe: fix GCC / ARM build issues

[Ard Biesheuvel]

This series updates the XGigUndiDxe driver so it can be built with GCC
for X64, ARM and AARCH64 (and probably IA32 but I did not test that)

Ard Biesheuvel (20):
  IntelUndiPkg/XGigUndiDxe: create GCC alternatives for MSFT build
    options
  IntelUndiPkg/XGigUndiDxe: move MSFT warning overrides to INF file
  IntelUndiPkg/XGigUndiDxe: consistently use forward slashes as path
    separators
  IntelUndiPkg/XGigUndiDxe: move BRAND_STRUCT declaration after type
    definition
  IntelUndiPkg/XGigUndiDxe: add missing VOID** cast
  IntelUndiPkg/XGigUndiDxe: add missing UINT8* cast
  IntelUndiPkg/XGigUndiDxe: drop definition of gImageHandle
  IntelUndiPkg/XGigUndiDxe: add missing braces to GUID literals
  IntelUndiPkg/XGigUndiDxe: fix incorrect use of CPP token pasting
  IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference
  IntelUndiPkg/XGigUndiDxe: cast XgbeMemCopy () args to correct pointer
    type
  IntelUndiPkg/XGigUndiDxe: don't take address of cast expression
  IntelUndiPkg/XGigUndiDxe: drop locally defined ASSERT() macro
  IntelUndiPkg/XGigUndiDxe: redefine UNREFERENCED_nPARAMETER macros for
    GCC
  IntelUndiPkg/XGigUndiDxe: use intermediate UINTN casts for pointers
  IntelUndiPkg/XGigUndiDxe: add missing EFIAPI modifiers
  IntelUndiPkg/XGigUndiDxe: drop unused variables
  IntelUndiPkg/XGigUndiDxe: set MDEPKG_NDEBUG only for RELEASE builds
  IntelUndiPkg/XGigUndiDxe: drop separate debug macros for DBG_LVL
  IntelUndiPkg/XGigUndiDxe: avoid unused var warnings for
    ERROR_REPORTn()

 IntelUndiPkg/XGigUndiDxe/AdapterInformation.c |  3 ++
 IntelUndiPkg/XGigUndiDxe/ComponentName.c      |  2 +
 IntelUndiPkg/XGigUndiDxe/ComponentName.h      |  1 +
 IntelUndiPkg/XGigUndiDxe/Decode.c             |  5 +--
 IntelUndiPkg/XGigUndiDxe/DeviceSupport.h      |  6 +--
 IntelUndiPkg/XGigUndiDxe/Dma.c                |  8 ++--
 IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c  |  5 ++-
 IntelUndiPkg/XGigUndiDxe/DriverHealth.c       |  2 +
 IntelUndiPkg/XGigUndiDxe/Hii.c                | 10 ++---
 IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c     |  4 --
 IntelUndiPkg/XGigUndiDxe/Init.c               | 10 ++---
 IntelUndiPkg/XGigUndiDxe/NVDataStruc.h        |  6 +--
 IntelUndiPkg/XGigUndiDxe/StartStop.c          |  2 +
 IntelUndiPkg/XGigUndiDxe/StartStop.h          |  4 +-
 IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf      | 29 +++++++-----
 IntelUndiPkg/XGigUndiDxe/Xgbe.c               | 18 ++++----
 IntelUndiPkg/XGigUndiDxe/Xgbe.h               | 27 ++----------
 IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h        | 44 +++----------------
 IntelUndiPkg/XGigUndiDxe/ixgbe_type.h         |  8 ++++
 19 files changed, 78 insertions(+), 116 deletions(-)

-- 
2.17.1

[PATCH edk2-staging 01/20] IntelUndiPkg/XGigUndiDxe: create GCC alternatives for MSFT build options

[Ard Biesheuvel]

Prefix the existing MSFT-only build option overrides with MSFT: and
create the GCC: counterparts so we can build this code with GCC as
well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
index 9e0e4aff0885..ab9c64dac7e8 100644
--- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
+++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
@@ -44,19 +44,24 @@ UNLOAD_IMAGE         = UnloadXGigUndiDriver
 
 [BuildOptions.common]
 
-*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G
+MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G
+GCC:*_*_*_CC_FLAGS = -DUNDI_10G
+GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
 
 [BuildOptions.X64]
 
-*_*_*_CC_FLAGS = /D EFIX64 
+MSFT:*_*_*_CC_FLAGS = /D EFIX64
+GCC:*_*_*_CC_FLAGS = -DEFIX64
 
 [BuildOptions.IPF]
 
-*_*_*_CC_FLAGS = /D EFI64
+MSFT:*_*_*_CC_FLAGS = /D EFI64
+GCC:*_*_*_CC_FLAGS = -DEFI64
 
 [BuildOptions.IA32]
 
-*_*_*_CC_FLAGS = /D EFI32
+MSFT:*_*_*_CC_FLAGS = /D EFI32
+GCC:*_*_*_CC_FLAGS = -DEFI32
 
 [sources.common]
  InventoryStrings.uni
-- 
2.17.1

Re: [PATCH edk2-staging 01/20] IntelUndiPkg/XGigUndiDxe: create GCC alternatives for MSFT build options

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Prefix the existing MSFT-only build option overrides with MSFT: and
> create the GCC: counterparts so we can build this code with GCC as
> well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> index 9e0e4aff0885..ab9c64dac7e8 100644
> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> @@ -44,19 +44,24 @@ UNLOAD_IMAGE         = UnloadXGigUndiDriver
>  
>  [BuildOptions.common]
>  
> -*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G
> +MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G
> +GCC:*_*_*_CC_FLAGS = -DUNDI_10G
> +GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
>  
>  [BuildOptions.X64]
>  
> -*_*_*_CC_FLAGS = /D EFIX64 
> +MSFT:*_*_*_CC_FLAGS = /D EFIX64
> +GCC:*_*_*_CC_FLAGS = -DEFIX64
>  
>  [BuildOptions.IPF]
>  
> -*_*_*_CC_FLAGS = /D EFI64
> +MSFT:*_*_*_CC_FLAGS = /D EFI64
> +GCC:*_*_*_CC_FLAGS = -DEFI64
>  
>  [BuildOptions.IA32]
>  
> -*_*_*_CC_FLAGS = /D EFI32
> +MSFT:*_*_*_CC_FLAGS = /D EFI32
> +GCC:*_*_*_CC_FLAGS = -DEFI32
>  
>  [sources.common]
>   InventoryStrings.uni

[PATCH edk2-staging 02/20] IntelUndiPkg/XGigUndiDxe: move MSFT warning overrides to INF file

[Ard Biesheuvel]

GCC chokes on the unknown MSVC specific #pragmas used for suppressing
warnings, so remove them and use the INF BuildOptions section instead.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf |  4 ++--
 IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h   | 10 ----------
 2 files changed, 2 insertions(+), 12 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
index ab9c64dac7e8..7ccc52bc9869 100644
--- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
+++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
@@ -44,7 +44,7 @@ UNLOAD_IMAGE         = UnloadXGigUndiDriver
 
 [BuildOptions.common]
 
-MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G
+MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G /wd4244 /wd4206 /wd4189
 GCC:*_*_*_CC_FLAGS = -DUNDI_10G
 GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
 
@@ -60,7 +60,7 @@ GCC:*_*_*_CC_FLAGS = -DEFI64
 
 [BuildOptions.IA32]
 
-MSFT:*_*_*_CC_FLAGS = /D EFI32
+MSFT:*_*_*_CC_FLAGS = /D EFI32 /wd4305
 GCC:*_*_*_CC_FLAGS = -DEFI32
 
 [sources.common]
diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
index 6fa0d1605c7c..368f40811904 100644
--- a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
+++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
@@ -29,16 +29,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #ifndef IXGBE_OSDEP_H_
 #define IXGBE_OSDEP_H_
 
-#pragma warning(disable : 4244)
-#pragma warning(disable : 4206)
-#pragma warning(disable : 4189)
-
-#ifdef EFI32
-
-// Remove truncation warning in type cast when some 64 bit variables are converted to 32-bit pointers
-#pragma warning(disable : 4305)
-#endif /* EFI32 */
-
 #ifndef EFI_SPECIFICATION_VERSION
 #define EFI_SPECIFICATION_VERSION 0x00020000
 #endif /* EFI_SPECIFICATION_VERSION */
-- 
2.17.1

Re: [PATCH edk2-staging 02/20] IntelUndiPkg/XGigUndiDxe: move MSFT warning overrides to INF file

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> GCC chokes on the unknown MSVC specific #pragmas used for suppressing
> warnings, so remove them and use the INF BuildOptions section
> instead.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf |  4 ++--
>  IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h   | 10 ----------
>  2 files changed, 2 insertions(+), 12 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> index ab9c64dac7e8..7ccc52bc9869 100644
> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> @@ -44,7 +44,7 @@ UNLOAD_IMAGE         = UnloadXGigUndiDriver
>  
>  [BuildOptions.common]
>  
> -MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G
> +MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G /wd4244
> /wd4206 /wd4189
>  GCC:*_*_*_CC_FLAGS = -DUNDI_10G
>  GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
>  
> @@ -60,7 +60,7 @@ GCC:*_*_*_CC_FLAGS = -DEFI64
>  
>  [BuildOptions.IA32]
>  
> -MSFT:*_*_*_CC_FLAGS = /D EFI32
> +MSFT:*_*_*_CC_FLAGS = /D EFI32 /wd4305
>  GCC:*_*_*_CC_FLAGS = -DEFI32
>  
>  [sources.common]
> diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> index 6fa0d1605c7c..368f40811904 100644
> --- a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> +++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> @@ -29,16 +29,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  #ifndef IXGBE_OSDEP_H_
>  #define IXGBE_OSDEP_H_
>  
> -#pragma warning(disable : 4244)
> -#pragma warning(disable : 4206)
> -#pragma warning(disable : 4189)
> -
> -#ifdef EFI32
> -
> -// Remove truncation warning in type cast when some 64 bit variables
> are converted to 32-bit pointers
> -#pragma warning(disable : 4305)
> -#endif /* EFI32 */
> -
>  #ifndef EFI_SPECIFICATION_VERSION
>  #define EFI_SPECIFICATION_VERSION 0x00020000
>  #endif /* EFI_SPECIFICATION_VERSION */

[PATCH edk2-staging 03/20] IntelUndiPkg/XGigUndiDxe: consistently use forward slashes as path separators

[Ard Biesheuvel]

Replace backslashes in paths with forward slashes to be compatible with
non-Windows OSes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Decode.c        |  2 +-
 IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 14 +++++++-------
 IntelUndiPkg/XGigUndiDxe/Xgbe.h          |  2 +-
 IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h   |  4 ++--
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Decode.c b/IntelUndiPkg/XGigUndiDxe/Decode.c
index 70fbce64c64f..5f37ce254872 100644
--- a/IntelUndiPkg/XGigUndiDxe/Decode.c
+++ b/IntelUndiPkg/XGigUndiDxe/Decode.c
@@ -28,7 +28,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ***************************************************************************/
 
 #include "Xgbe.h"
-#include <Uefi\UEfiPxe.h>
+#include <Uefi/UefiPxe.h>
 
 // Forward declarations for UNDI function table
 /** This routine determines the operational state of the UNDI.  It updates the state flags in the
diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
index 7ccc52bc9869..beee8aa8134e 100644
--- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
+++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
@@ -110,13 +110,13 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
  Version.h
  HiiInternalLib.c
  HiiInternalLib.h
-  wol\wol.c
-  wol\wol.h
-  wol\wolfamily.c
-  wol\wolimpl.c
-  wol\wolimpl.h
-  wol\wolinfo.c
-  wol\wol_10G.c
+  wol/wol.c
+  wol/wol.h
+  wol/wolfamily.c
+  wol/wolimpl.c
+  wol/wolimpl.h
+  wol/wolinfo.c
+  wol/wol_10G.c
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.h b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
index 5ab088362f90..4472cfb4fa31 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
@@ -69,7 +69,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
 #include <Library/DevicePathLib.h>
-#include <Library\PrintLib.h>
+#include <Library/PrintLib.h>
 
 #include <IndustryStandard/Pci.h>
 
diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
index 368f40811904..e324b0539782 100644
--- a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
+++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
@@ -39,8 +39,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #include <Uefi.h>
 #include <Base.h>
-#include <Library\BaseLib.h>
-#include <Library\BaseMemoryLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
 #include <Library/PrintLib.h>
 
 #define CHAR            CHAR8
-- 
2.17.1

Re: [PATCH edk2-staging 03/20] IntelUndiPkg/XGigUndiDxe: consistently use forward slashes as path separators

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Replace backslashes in paths with forward slashes to be compatible
> with
> non-Windows OSes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Decode.c        |  2 +-
>  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 14 +++++++-------
>  IntelUndiPkg/XGigUndiDxe/Xgbe.h          |  2 +-
>  IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h   |  4 ++--
>  4 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Decode.c
> b/IntelUndiPkg/XGigUndiDxe/Decode.c
> index 70fbce64c64f..5f37ce254872 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Decode.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Decode.c
> @@ -28,7 +28,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  ********************************************************************
> *******/
>  
>  #include "Xgbe.h"
> -#include <Uefi\UEfiPxe.h>
> +#include <Uefi/UefiPxe.h>
>  
>  // Forward declarations for UNDI function table
>  /** This routine determines the operational state of the UNDI.  It
> updates the state flags in the
> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> index 7ccc52bc9869..beee8aa8134e 100644
> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> @@ -110,13 +110,13 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
>   Version.h
>   HiiInternalLib.c
>   HiiInternalLib.h
> -  wol\wol.c
> -  wol\wol.h
> -  wol\wolfamily.c
> -  wol\wolimpl.c
> -  wol\wolimpl.h
> -  wol\wolinfo.c
> -  wol\wol_10G.c
> +  wol/wol.c
> +  wol/wol.h
> +  wol/wolfamily.c
> +  wol/wolimpl.c
> +  wol/wolimpl.h
> +  wol/wolinfo.c
> +  wol/wol_10G.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> index 5ab088362f90..4472cfb4fa31 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> +++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> @@ -69,7 +69,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  #include <Library/UefiLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/DevicePathLib.h>
> -#include <Library\PrintLib.h>
> +#include <Library/PrintLib.h>
>  
>  #include <IndustryStandard/Pci.h>
>  
> diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> index 368f40811904..e324b0539782 100644
> --- a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> +++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
> @@ -39,8 +39,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  
>  #include <Uefi.h>
>  #include <Base.h>
> -#include <Library\BaseLib.h>
> -#include <Library\BaseMemoryLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/PrintLib.h>
>  
>  #define CHAR            CHAR8

[PATCH edk2-staging 04/20] IntelUndiPkg/XGigUndiDxe: move BRAND_STRUCT declaration after type definition

[Ard Biesheuvel]

Move the extern declaration of mBrandingTable[] after the definition of
the type. This solves a build issue with GCC.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/DeviceSupport.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h b/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
index e997983ba3e7..d501d9bb1371 100644
--- a/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
+++ b/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
@@ -33,9 +33,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 typedef struct BRAND_STRUCT_S BRAND_STRUCT;
 
-extern BRAND_STRUCT mBrandingTable[];
-extern UINTN        mBrandingTableSize;
-
 /* Defines */
 #define INVALID_VENDOR_ID     0xFFFF
 #define INVALID_SUBVENDOR_ID  0xFFFF
@@ -53,6 +50,9 @@ struct BRAND_STRUCT_S {
   CHAR16 *BrandString;
 };
 
+extern BRAND_STRUCT mBrandingTable[];
+extern UINTN        mBrandingTableSize;
+
 /* Function declarations */
 
 /** Returns pointer to current device's branding string (looks for best match)
-- 
2.17.1

Re: [PATCH edk2-staging 04/20] IntelUndiPkg/XGigUndiDxe: move BRAND_STRUCT declaration after type definition

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Move the extern declaration of mBrandingTable[] after the definition
> of
> the type. This solves a build issue with GCC.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/DeviceSupport.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
> b/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
> index e997983ba3e7..d501d9bb1371 100644
> --- a/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
> +++ b/IntelUndiPkg/XGigUndiDxe/DeviceSupport.h
> @@ -33,9 +33,6 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  
>  typedef struct BRAND_STRUCT_S BRAND_STRUCT;
>  
> -extern BRAND_STRUCT mBrandingTable[];
> -extern UINTN        mBrandingTableSize;
> -
>  /* Defines */
>  #define INVALID_VENDOR_ID     0xFFFF
>  #define INVALID_SUBVENDOR_ID  0xFFFF
> @@ -53,6 +50,9 @@ struct BRAND_STRUCT_S {
>    CHAR16 *BrandString;
>  };
>  
> +extern BRAND_STRUCT mBrandingTable[];
> +extern UINTN        mBrandingTableSize;
> +
>  /* Function declarations */
>  
>  /** Returns pointer to current device's branding string (looks for
> best match)

[PATCH edk2-staging 05/20] IntelUndiPkg/XGigUndiDxe: add missing VOID** cast

[Ard Biesheuvel]

Unlike Visual Studio, GCC does not permit implicit conversion between
a pointer-to-void-pointer and pointer to a typed pointer. So add the
explicit casts where necessary.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Hii.c  | 8 ++++----
 IntelUndiPkg/XGigUndiDxe/Init.c | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Hii.c b/IntelUndiPkg/XGigUndiDxe/Hii.c
index 857a475622f7..3ad7d61e493d 100644
--- a/IntelUndiPkg/XGigUndiDxe/Hii.c
+++ b/IntelUndiPkg/XGigUndiDxe/Hii.c
@@ -459,7 +459,7 @@ HiiOpenProtocol (
   Status = gBS->LocateProtocol (
                   &gEfiHiiDatabaseProtocolGuid,
                   NULL,
-                  &UndiPrivateData->HiiDatabase
+                  (VOID **)&UndiPrivateData->HiiDatabase
                 );
   if (EFI_ERROR (Status)) {
     DEBUGPRINT (CRITICAL, ("Error finding HII protocol: %r\n", Status));
@@ -472,7 +472,7 @@ HiiOpenProtocol (
   Status = gBS->LocateProtocol (
                   &gEfiHiiStringProtocolGuid,
                   NULL,
-                  &UndiPrivateData->HiiString
+                  (VOID **)&UndiPrivateData->HiiString
                 );
   if (EFI_ERROR (Status)) {
     DEBUGPRINT (CRITICAL, ("Error finding HII String protocol: %r\n", Status));
@@ -485,7 +485,7 @@ HiiOpenProtocol (
   Status = gBS->LocateProtocol (
                   &gEfiFormBrowser2ProtocolGuid,
                   NULL,
-                  &UndiPrivateData->FormBrowser2
+                  (VOID **)&UndiPrivateData->FormBrowser2
                 );
   if (EFI_ERROR (Status)) {
     DEBUGPRINT (CRITICAL, ("Error finding HII form browser protocol: %r\n", Status));
@@ -498,7 +498,7 @@ HiiOpenProtocol (
   Status = gBS->LocateProtocol (
                   &gEfiHiiConfigRoutingProtocolGuid,
                   NULL,
-                  &UndiPrivateData->HiiConfigRouting
+                  (VOID **)&UndiPrivateData->HiiConfigRouting
                 );
   if (EFI_ERROR (Status)) {
     DEBUGPRINT (CRITICAL, ("Error finding HII ConfigRouting protocol: %r\n", Status));
diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c b/IntelUndiPkg/XGigUndiDxe/Init.c
index bffe3c165866..84e06ea071c5 100644
--- a/IntelUndiPkg/XGigUndiDxe/Init.c
+++ b/IntelUndiPkg/XGigUndiDxe/Init.c
@@ -179,7 +179,7 @@ InitAppendMac2DevPath (
   Status = gBS->AllocatePool (
                   EfiBootServicesData,  // EfiRuntimeServicesData,
                   TotalPathLen,
-                  &DevicePtr
+                  (VOID **)&DevicePtr
                 );
 
   if (Status != EFI_SUCCESS) {
-- 
2.17.1

Re: [PATCH edk2-staging 05/20] IntelUndiPkg/XGigUndiDxe: add missing VOID** cast

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Unlike Visual Studio, GCC does not permit implicit conversion between
> a pointer-to-void-pointer and pointer to a typed pointer. So add the
> explicit casts where necessary.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Hii.c  | 8 ++++----
>  IntelUndiPkg/XGigUndiDxe/Init.c | 2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Hii.c
> b/IntelUndiPkg/XGigUndiDxe/Hii.c
> index 857a475622f7..3ad7d61e493d 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Hii.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Hii.c
> @@ -459,7 +459,7 @@ HiiOpenProtocol (
>    Status = gBS->LocateProtocol (
>                    &gEfiHiiDatabaseProtocolGuid,
>                    NULL,
> -                  &UndiPrivateData->HiiDatabase
> +                  (VOID **)&UndiPrivateData->HiiDatabase
>                  );
>    if (EFI_ERROR (Status)) {
>      DEBUGPRINT (CRITICAL, ("Error finding HII protocol: %r\n",
> Status));
> @@ -472,7 +472,7 @@ HiiOpenProtocol (
>    Status = gBS->LocateProtocol (
>                    &gEfiHiiStringProtocolGuid,
>                    NULL,
> -                  &UndiPrivateData->HiiString
> +                  (VOID **)&UndiPrivateData->HiiString
>                  );
>    if (EFI_ERROR (Status)) {
>      DEBUGPRINT (CRITICAL, ("Error finding HII String protocol:
> %r\n", Status));
> @@ -485,7 +485,7 @@ HiiOpenProtocol (
>    Status = gBS->LocateProtocol (
>                    &gEfiFormBrowser2ProtocolGuid,
>                    NULL,
> -                  &UndiPrivateData->FormBrowser2
> +                  (VOID **)&UndiPrivateData->FormBrowser2
>                  );
>    if (EFI_ERROR (Status)) {
>      DEBUGPRINT (CRITICAL, ("Error finding HII form browser protocol:
> %r\n", Status));
> @@ -498,7 +498,7 @@ HiiOpenProtocol (
>    Status = gBS->LocateProtocol (
>                    &gEfiHiiConfigRoutingProtocolGuid,
>                    NULL,
> -                  &UndiPrivateData->HiiConfigRouting
> +                  (VOID **)&UndiPrivateData->HiiConfigRouting
>                  );
>    if (EFI_ERROR (Status)) {
>      DEBUGPRINT (CRITICAL, ("Error finding HII ConfigRouting
> protocol: %r\n", Status));
> diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c
> b/IntelUndiPkg/XGigUndiDxe/Init.c
> index bffe3c165866..84e06ea071c5 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Init.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Init.c
> @@ -179,7 +179,7 @@ InitAppendMac2DevPath (
>    Status = gBS->AllocatePool (
>                    EfiBootServicesData,  // EfiRuntimeServicesData,
>                    TotalPathLen,
> -                  &DevicePtr
> +                  (VOID **)&DevicePtr
>                  );
>  
>    if (Status != EFI_SUCCESS) {

[PATCH edk2-staging 06/20] IntelUndiPkg/XGigUndiDxe: add missing UINT8* cast

[Ard Biesheuvel]

UINT8 and CHAR8 are not the same underlying type on all architectures,
so add an explicit cast where necessary.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Hii.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Hii.c b/IntelUndiPkg/XGigUndiDxe/Hii.c
index 3ad7d61e493d..3170580487d5 100644
--- a/IntelUndiPkg/XGigUndiDxe/Hii.c
+++ b/IntelUndiPkg/XGigUndiDxe/Hii.c
@@ -782,7 +782,7 @@ HiiSetMenuStrings (
 
       Status = ReadPbaString (
                  &UndiPrivateData->NicInfo,
-                 PBAString8,
+                 (UINT8 *)PBAString8,
                  MAX_PBA_STR_LENGTH
                );
       if (Status == EFI_SUCCESS) {
-- 
2.17.1

Re: [PATCH edk2-staging 06/20] IntelUndiPkg/XGigUndiDxe: add missing UINT8* cast

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> UINT8 and CHAR8 are not the same underlying type on all
> architectures,
> so add an explicit cast where necessary.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Hii.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Hii.c
> b/IntelUndiPkg/XGigUndiDxe/Hii.c
> index 3ad7d61e493d..3170580487d5 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Hii.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Hii.c
> @@ -782,7 +782,7 @@ HiiSetMenuStrings (
>  
>        Status = ReadPbaString (
>                   &UndiPrivateData->NicInfo,
> -                 PBAString8,
> +                 (UINT8 *)PBAString8,
>                   MAX_PBA_STR_LENGTH
>                 );
>        if (Status == EFI_SUCCESS) {

[PATCH edk2-staging 07/20] IntelUndiPkg/XGigUndiDxe: drop definition of gImageHandle

[Ard Biesheuvel]

Remove duplicate definition of gImageHandle, which is emitted by
the build tools as well.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Init.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c b/IntelUndiPkg/XGigUndiDxe/Init.c
index 84e06ea071c5..03e3942a1944 100644
--- a/IntelUndiPkg/XGigUndiDxe/Init.c
+++ b/IntelUndiPkg/XGigUndiDxe/Init.c
@@ -47,7 +47,6 @@ UINT16             mActiveChildren    = 0;
 EFI_EVENT          gEventNotifyExitBs;
 EFI_EVENT          gEventNotifyVirtual;
 
-EFI_HANDLE        gImageHandle;
 EFI_SYSTEM_TABLE *gSystemTable;
 
 EFI_GUID gEfiNiiPointerGuid = EFI_NII_POINTER_PROTOCOL_GUID;
@@ -502,7 +501,6 @@ InitializeXGigUndiDriver (
 {
   EFI_STATUS                 Status;
 
-  gImageHandle  = ImageHandle;
   gSystemTable  = SystemTable;
 
   Status = EfiLibInstallDriverBinding (ImageHandle, SystemTable, &gUndiDriverBinding, ImageHandle);
-- 
2.17.1

Re: [PATCH edk2-staging 07/20] IntelUndiPkg/XGigUndiDxe: drop definition of gImageHandle

[Ryszard Knop]

Hmm, is there a list/something I can generate to see which globals
build tools emit? There are some more variables I'd happily get rid
of, eg if I could drop gSystemTable and others.

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Remove duplicate definition of gImageHandle, which is emitted by
> the build tools as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Init.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c
> b/IntelUndiPkg/XGigUndiDxe/Init.c
> index 84e06ea071c5..03e3942a1944 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Init.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Init.c
> @@ -47,7 +47,6 @@ UINT16             mActiveChildren    = 0;
>  EFI_EVENT          gEventNotifyExitBs;
>  EFI_EVENT          gEventNotifyVirtual;
>  
> -EFI_HANDLE        gImageHandle;
>  EFI_SYSTEM_TABLE *gSystemTable;
>  
>  EFI_GUID gEfiNiiPointerGuid = EFI_NII_POINTER_PROTOCOL_GUID;
> @@ -502,7 +501,6 @@ InitializeXGigUndiDriver (
>  {
>    EFI_STATUS                 Status;
>  
> -  gImageHandle  = ImageHandle;
>    gSystemTable  = SystemTable;
>  
>    Status = EfiLibInstallDriverBinding (ImageHandle, SystemTable,
> &gUndiDriverBinding, ImageHandle);

Re: [PATCH edk2-staging 07/20] IntelUndiPkg/XGigUndiDxe: drop definition of gImageHandle

[Ard Biesheuvel]

On Wed, 30 Jan 2019 at 17:05, Ryszard Knop <ryszard.knop@linux.intel.com> wrote:
>
> Hmm, is there a list/something I can generate to see which globals
> build tools emit? There are some more variables I'd happily get rid
> of, eg if I could drop gSystemTable and others.
>

You can look at the contents of the various AutoGen.h files in the Build/ folder


> Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>
>
> On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> > Remove duplicate definition of gImageHandle, which is emitted by
> > the build tools as well.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > ---
> >  IntelUndiPkg/XGigUndiDxe/Init.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c
> > b/IntelUndiPkg/XGigUndiDxe/Init.c
> > index 84e06ea071c5..03e3942a1944 100644
> > --- a/IntelUndiPkg/XGigUndiDxe/Init.c
> > +++ b/IntelUndiPkg/XGigUndiDxe/Init.c
> > @@ -47,7 +47,6 @@ UINT16             mActiveChildren    = 0;
> >  EFI_EVENT          gEventNotifyExitBs;
> >  EFI_EVENT          gEventNotifyVirtual;
> >
> > -EFI_HANDLE        gImageHandle;
> >  EFI_SYSTEM_TABLE *gSystemTable;
> >
> >  EFI_GUID gEfiNiiPointerGuid = EFI_NII_POINTER_PROTOCOL_GUID;
> > @@ -502,7 +501,6 @@ InitializeXGigUndiDriver (
> >  {
> >    EFI_STATUS                 Status;
> >
> > -  gImageHandle  = ImageHandle;
> >    gSystemTable  = SystemTable;
> >
> >    Status = EfiLibInstallDriverBinding (ImageHandle, SystemTable,
> > &gUndiDriverBinding, ImageHandle);
>

Re: [PATCH edk2-staging 07/20] IntelUndiPkg/XGigUndiDxe: drop definition of gImageHandle

[Ryszard Knop]

Ah, thanks!

On Wed, 2019-01-30 at 17:06 +0100, Ard Biesheuvel wrote:
> On Wed, 30 Jan 2019 at 17:05, Ryszard Knop <
> ryszard.knop@linux.intel.com> wrote:
> > Hmm, is there a list/something I can generate to see which globals
> > build tools emit? There are some more variables I'd happily get rid
> > of, eg if I could drop gSystemTable and others.
> > 
> 
> You can look at the contents of the various AutoGen.h files in the
> Build/ folder
> 
> 
> > Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>
> > 
> > On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> > > Remove duplicate definition of gImageHandle, which is emitted by
> > > the build tools as well.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > > ---
> > >  IntelUndiPkg/XGigUndiDxe/Init.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > > 
> > > diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c
> > > b/IntelUndiPkg/XGigUndiDxe/Init.c
> > > index 84e06ea071c5..03e3942a1944 100644
> > > --- a/IntelUndiPkg/XGigUndiDxe/Init.c
> > > +++ b/IntelUndiPkg/XGigUndiDxe/Init.c
> > > @@ -47,7 +47,6 @@ UINT16             mActiveChildren    = 0;
> > >  EFI_EVENT          gEventNotifyExitBs;
> > >  EFI_EVENT          gEventNotifyVirtual;
> > > 
> > > -EFI_HANDLE        gImageHandle;
> > >  EFI_SYSTEM_TABLE *gSystemTable;
> > > 
> > >  EFI_GUID gEfiNiiPointerGuid = EFI_NII_POINTER_PROTOCOL_GUID;
> > > @@ -502,7 +501,6 @@ InitializeXGigUndiDriver (
> > >  {
> > >    EFI_STATUS                 Status;
> > > 
> > > -  gImageHandle  = ImageHandle;
> > >    gSystemTable  = SystemTable;
> > > 
> > >    Status = EfiLibInstallDriverBinding (ImageHandle, SystemTable,
> > > &gUndiDriverBinding, ImageHandle);

Re: [PATCH edk2-staging 07/20] IntelUndiPkg/XGigUndiDxe: drop definition of gImageHandle

[Andrew Fish]

> On Jan 30, 2019, at 8:05 AM, Ryszard Knop <ryszard.knop@linux.intel.com> wrote:
> 
> Hmm, is there a list/something I can generate to see which globals
> build tools emit? There are some more variables I'd happily get rid
> of, eg if I could drop gSystemTable and others.
> 

Ryszard,

You need to use libs by including the include file, and listing the lib in the INF. The library constructor initializes the globals. 

https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/DxeServicesTableLib.h <https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/DxeServicesTableLib.h>
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/UefiBootServicesTableLib.h <https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/UefiBootServicesTableLib.h>
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/UefiRuntimeServicesTableLib.h

For edk2 the library constructors get called prior calling your drivers entry point. 

Thanks,

Andrew Fish


> Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>
> 
> On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
>> Remove duplicate definition of gImageHandle, which is emitted by
>> the build tools as well.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>> ---
>> IntelUndiPkg/XGigUndiDxe/Init.c | 2 --
>> 1 file changed, 2 deletions(-)
>> 
>> diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c
>> b/IntelUndiPkg/XGigUndiDxe/Init.c
>> index 84e06ea071c5..03e3942a1944 100644
>> --- a/IntelUndiPkg/XGigUndiDxe/Init.c
>> +++ b/IntelUndiPkg/XGigUndiDxe/Init.c
>> @@ -47,7 +47,6 @@ UINT16             mActiveChildren    = 0;
>> EFI_EVENT          gEventNotifyExitBs;
>> EFI_EVENT          gEventNotifyVirtual;
>> 
>> -EFI_HANDLE        gImageHandle;
>> EFI_SYSTEM_TABLE *gSystemTable;
>> 
>> EFI_GUID gEfiNiiPointerGuid = EFI_NII_POINTER_PROTOCOL_GUID;
>> @@ -502,7 +501,6 @@ InitializeXGigUndiDriver (
>> {
>>   EFI_STATUS                 Status;
>> 
>> -  gImageHandle  = ImageHandle;
>>   gSystemTable  = SystemTable;
>> 
>>   Status = EfiLibInstallDriverBinding (ImageHandle, SystemTable,
>> &gUndiDriverBinding, ImageHandle);
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

[PATCH edk2-staging 08/20] IntelUndiPkg/XGigUndiDxe: add missing braces to GUID literals

[Ard Biesheuvel]

The Data4 member of the GUID/EFI_GUID struct type is an array of
UINT8, so literals require two sets of { } braces. Add them where
missing.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/NVDataStruc.h | 4 ++--
 IntelUndiPkg/XGigUndiDxe/StartStop.h   | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
index e5e669681b07..63dd841277ab 100644
--- a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
+++ b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
@@ -34,12 +34,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 #define XGBE_HII_FORM_GUID \
   { \
-    0x25fd9f0b, 0xa3ef, 0x4788, 0xa4, 0x0c, 0x81, 0x84, 0x9d, 0x17, 0x8a, 0x6c \
+    0x25fd9f0b, 0xa3ef, 0x4788, { 0xa4, 0x0c, 0x81, 0x84, 0x9d, 0x17, 0x8a, 0x6c } \
   }
 
 #define XGBE_HII_DATA_GUID \
   { \
-    0xe2c85968, 0x6906, 0x4b27, 0x9d, 0x09, 0x33, 0x43, 0xaf, 0x06, 0x46, 0x76 \
+    0xe2c85968, 0x6906, 0x4b27, { 0x9d, 0x09, 0x33, 0x43, 0xaf, 0x06, 0x46, 0x76 } \
   }
 
 
diff --git a/IntelUndiPkg/XGigUndiDxe/StartStop.h b/IntelUndiPkg/XGigUndiDxe/StartStop.h
index 03ad3dc9cecc..5dfc45c7de6a 100644
--- a/IntelUndiPkg/XGigUndiDxe/StartStop.h
+++ b/IntelUndiPkg/XGigUndiDxe/StartStop.h
@@ -32,8 +32,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include "Xgbe.h"
 
 #define EFI_DRIVER_STOP_PROTOCOL_GUID \
-  { 0x34d59603, 0x1428, 0x4429, 0xa4, 0x14, 0xe6, 0xb3, \
-    0xb5, 0xfd, 0x7d, 0xc1 }
+  { 0x34d59603, 0x1428, 0x4429, { 0xa4, 0x14, 0xe6, 0xb3, \
+    0xb5, 0xfd, 0x7d, 0xc1 } }
 
 typedef struct EFI_DRIVER_STOP_PROTOCOL_S  EFI_DRIVER_STOP_PROTOCOL;
 
-- 
2.17.1

Re: [PATCH edk2-staging 08/20] IntelUndiPkg/XGigUndiDxe: add missing braces to GUID literals

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> The Data4 member of the GUID/EFI_GUID struct type is an array of
> UINT8, so literals require two sets of { } braces. Add them where
> missing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/NVDataStruc.h | 4 ++--
>  IntelUndiPkg/XGigUndiDxe/StartStop.h   | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> index e5e669681b07..63dd841277ab 100644
> --- a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> +++ b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> @@ -34,12 +34,12 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  
>  #define XGBE_HII_FORM_GUID \
>    { \
> -    0x25fd9f0b, 0xa3ef, 0x4788, 0xa4, 0x0c, 0x81, 0x84, 0x9d, 0x17,
> 0x8a, 0x6c \
> +    0x25fd9f0b, 0xa3ef, 0x4788, { 0xa4, 0x0c, 0x81, 0x84, 0x9d,
> 0x17, 0x8a, 0x6c } \
>    }
>  
>  #define XGBE_HII_DATA_GUID \
>    { \
> -    0xe2c85968, 0x6906, 0x4b27, 0x9d, 0x09, 0x33, 0x43, 0xaf, 0x06,
> 0x46, 0x76 \
> +    0xe2c85968, 0x6906, 0x4b27, { 0x9d, 0x09, 0x33, 0x43, 0xaf,
> 0x06, 0x46, 0x76 } \
>    }
>  
>  
> diff --git a/IntelUndiPkg/XGigUndiDxe/StartStop.h
> b/IntelUndiPkg/XGigUndiDxe/StartStop.h
> index 03ad3dc9cecc..5dfc45c7de6a 100644
> --- a/IntelUndiPkg/XGigUndiDxe/StartStop.h
> +++ b/IntelUndiPkg/XGigUndiDxe/StartStop.h
> @@ -32,8 +32,8 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>  #include "Xgbe.h"
>  
>  #define EFI_DRIVER_STOP_PROTOCOL_GUID \
> -  { 0x34d59603, 0x1428, 0x4429, 0xa4, 0x14, 0xe6, 0xb3, \
> -    0xb5, 0xfd, 0x7d, 0xc1 }
> +  { 0x34d59603, 0x1428, 0x4429, { 0xa4, 0x14, 0xe6, 0xb3, \
> +    0xb5, 0xfd, 0x7d, 0xc1 } }
>  
>  typedef struct EFI_DRIVER_STOP_PROTOCOL_S  EFI_DRIVER_STOP_PROTOCOL;
>  

[PATCH edk2-staging 09/20] IntelUndiPkg/XGigUndiDxe: fix incorrect use of CPP token pasting

[Ard Biesheuvel]

The ## CPP token pasting operator is used to paste *tokens*, which
is not the same thing as pasting arbitrary macro arguments. Since a
token cannot contain . or ) characters in the first place, using
the ## operator here is wrong and unnecessary, so just remove it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/NVDataStruc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
index 63dd841277ab..c78d26da7869 100644
--- a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
+++ b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
@@ -113,7 +113,7 @@ typedef struct {
 
    @return   Width of given field is returned
 **/
-#define UNDI_CONFIG_WIDTH(Field) sizeof (UndiPrivateData->Configuration. ## Field ## )
+#define UNDI_CONFIG_WIDTH(Field) sizeof (UndiPrivateData->Configuration.Field)
 
 // General parameters
 #define     QUESTION_ID_EFI_DRIVER_VER                          0x1100
-- 
2.17.1

Re: [PATCH edk2-staging 09/20] IntelUndiPkg/XGigUndiDxe: fix incorrect use of CPP token pasting

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> The ## CPP token pasting operator is used to paste *tokens*, which
> is not the same thing as pasting arbitrary macro arguments. Since a
> token cannot contain . or ) characters in the first place, using
> the ## operator here is wrong and unnecessary, so just remove it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/NVDataStruc.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> index 63dd841277ab..c78d26da7869 100644
> --- a/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> +++ b/IntelUndiPkg/XGigUndiDxe/NVDataStruc.h
> @@ -113,7 +113,7 @@ typedef struct {
>  
>     @return   Width of given field is returned
>  **/
> -#define UNDI_CONFIG_WIDTH(Field) sizeof (UndiPrivateData-
> >Configuration. ## Field ## )
> +#define UNDI_CONFIG_WIDTH(Field) sizeof (UndiPrivateData-
> >Configuration.Field)
>  
>  // General parameters
>  #define     QUESTION_ID_EFI_DRIVER_VER                          0x11
> 00

[PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Ard Biesheuvel]

StdLibc should not be used in drivers (it has dependencies on Shell
protocols), but in fact, we don't appear to rely on it in the first
place, so just drop the reference.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
index beee8aa8134e..b5747565fbea 100644
--- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
+++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
@@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
   PrintLib
   UefiLib
   HiiLib
-  StdLibC
 
 [LibraryClasses.X64]
   
-- 
2.17.1

Re: [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Carsey, Jaben]

Agreed.  
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Wednesday, November 14, 2018 6:34 PM
> To: edk2-devel@lists.01.org
> Cc: Kacperski, Kamil <kamil.kacperski@intel.com>; Jin, Eric
> <eric.jin@intel.com>; Orlowski, Pawel <pawel.orlowski@intel.com>; Kinney,
> Michael D <michael.d.kinney@intel.com>; Hsiung, Harry L
> <harry.l.hsiung@intel.com>
> Subject: [edk2] [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop
> StdLibC library class reference
> Importance: High
> 
> StdLibc should not be used in drivers (it has dependencies on Shell
> protocols), but in fact, we don't appear to rely on it in the first
> place, so just drop the reference.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> index beee8aa8134e..b5747565fbea 100644
> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> @@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
>    PrintLib
>    UefiLib
>    HiiLib
> -  StdLibC
> 
>  [LibraryClasses.X64]
> 
> --
> 2.17.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Ryszard Knop]

That's actually not quite correct - we need this package to build on
IA32. It's named rather unfortunately, since it's not the EDK2 StdLibC,
but rather a package in this repository - see IntelUndiPkg/LibC. It
contains the bare minimum of functionality required to fix missing 64-
bit math/shifts on IA32 and missing memcpy/memset intrinsics. We can't
prevent MSVC from yielding memcpy/memset either, so this was the nasty
solution for build issues. You have included CompilerIntrinsicsLib for
the same reason, too :)

I'm not aware of any X64/IA32 equivalent of your CompilerIntrinsicsLib,
but I'd be happy to be proven wrong here. I'm off for the rest of the
week - I'll continue with reviews and merging early next week.

Thanks, Richard.

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> StdLibc should not be used in drivers (it has dependencies on Shell
> protocols), but in fact, we don't appear to rely on it in the first
> place, so just drop the reference.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> index beee8aa8134e..b5747565fbea 100644
> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> @@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
>    PrintLib
>    UefiLib
>    HiiLib
> -  StdLibC
>  
>  [LibraryClasses.X64]
>    

Re: [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Andrew Fish]

> On Jan 30, 2019, at 9:26 AM, Ryszard Knop <ryszard.knop@linux.intel.com> wrote:
> 
> That's actually not quite correct - we need this package to build on
> IA32. It's named rather unfortunately, since it's not the EDK2 StdLibC,
> but rather a package in this repository - see IntelUndiPkg/LibC. It
> contains the bare minimum of functionality required to fix missing 64-
> bit math/shifts on IA32 and missing memcpy/memset intrinsics. We can't
> prevent MSVC from yielding memcpy/memset either, so this was the nasty
> solution for build issues. You have included CompilerIntrinsicsLib for
> the same reason, too :)
> 

Ryszard,

For IA32/X64 we avoid the compiler intrinsic libs via the coding standard. 
1) If you don't assign something too large at execution time with an = the compiler will not inline memcpy()/memset()
2) BaseLib.h has all the math functions that generate intrinsics that your code can call explicitly: https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/BaseLib.h#L3533 <https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/BaseLib.h#L3533>

UINT64 X=0x100000000;
UINT64 Y=2;

So:
Y = X*Y;
should be:
Y = MultU64x64 (X, Y);

When ARM got added much later and some versions of ARM did not even have a divide instruction we gave up on trying to add more functions into all the existing IA32 code, and add the intrinsic lib. 

If we are going to add an intrinsic lib for x86 then we should probably add it to the MdePkg and it needs to support MSVC and GCC (as far as I can tell clang should work with the GCC intrinsics). 

Thanks,

Andrew Fish


> I'm not aware of any X64/IA32 equivalent of your CompilerIntrinsicsLib,
> but I'd be happy to be proven wrong here. I'm off for the rest of the
> week - I'll continue with reviews and merging early next week.
> 
> Thanks, Richard.
> 
> On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
>> StdLibc should not be used in drivers (it has dependencies on Shell
>> protocols), but in fact, we don't appear to rely on it in the first
>> place, so just drop the reference.
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
>> ---
>> IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
>> 1 file changed, 1 deletion(-)
>> 
>> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
>> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
>> index beee8aa8134e..b5747565fbea 100644
>> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
>> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
>> @@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
>>   PrintLib
>>   UefiLib
>>   HiiLib
>> -  StdLibC
>> 
>> [LibraryClasses.X64]
>> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Ryszard Knop]

Andrew,

Unfortunately, not assigning something too large or using math
functions is not an option for us, as we share a significant amount of
code with Linux/FreeBSD drivers and maintainers of that code don't want
changes similar to the ones below (especially that, for all the other
drivers, intrinsics just work). Intrinsic lib for IA32 and others would
be very much preferred (and one that just works on any architecture, so
that we wouldn't have to add extra arch-specific LibraryClasses).

Thanks, Richard

On Wed, 2019-01-30 at 10:34 -0800, Andrew Fish wrote:
> > On Jan 30, 2019, at 9:26 AM, Ryszard Knop <
> > ryszard.knop@linux.intel.com> wrote:
> > 
> > That's actually not quite correct - we need this package to build
> > on
> > IA32. It's named rather unfortunately, since it's not the EDK2
> > StdLibC,
> > but rather a package in this repository - see IntelUndiPkg/LibC. It
> > contains the bare minimum of functionality required to fix missing
> > 64-
> > bit math/shifts on IA32 and missing memcpy/memset intrinsics. We
> > can't
> > prevent MSVC from yielding memcpy/memset either, so this was the
> > nasty
> > solution for build issues. You have included CompilerIntrinsicsLib
> > for
> > the same reason, too :)
> > 
> 
> Ryszard,
> 
> For IA32/X64 we avoid the compiler intrinsic libs via the coding
> standard. 
> 1) If you don't assign something too large at execution time with an
> = the compiler will not inline memcpy()/memset()
> 2) BaseLib.h has all the math functions that generate intrinsics that
> your code can call explicitly: 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Library/BaseLib.h#L3533
> 
> UINT64 X=0x100000000;
> UINT64 Y=2;
> 
> So:
> Y = X*Y;
> should be:
> Y = MultU64x64 (X, Y);
> 
> When ARM got added much later and some versions of ARM did not even
> have a divide instruction we gave up on trying to add more functions
> into all the existing IA32 code, and add the intrinsic lib. 
> 
> If we are going to add an intrinsic lib for x86 then we should
> probably add it to the MdePkg and it needs to support MSVC and GCC
> (as far as I can tell clang should work with the GCC intrinsics). 
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> > I'm not aware of any X64/IA32 equivalent of your
> > CompilerIntrinsicsLib,
> > but I'd be happy to be proven wrong here. I'm off for the rest of
> > the
> > week - I'll continue with reviews and merging early next week.
> > 
> > Thanks, Richard.
> > 
> > On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> > > StdLibc should not be used in drivers (it has dependencies on
> > > Shell
> > > protocols), but in fact, we don't appear to rely on it in the
> > > first
> > > place, so just drop the reference.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > > ---
> > > IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
> > > 1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > index beee8aa8134e..b5747565fbea 100644
> > > --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > @@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
> > >   PrintLib
> > >   UefiLib
> > >   HiiLib
> > > -  StdLibC
> > > 
> > > [LibraryClasses.X64]
> > > 
> > 
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> 
> 

Re: [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Kinney, Michael D]

Hi Richard,

It is possible to update C code to prevent the use of compiler
intrinsic functions.  This is what we have done for EDK II modules
that are built for both IA32 and X64.

The one exception is the use of OpenSSL.  We prefer to use that
project "as is" as a git submodule, so modifying the OpenSSL
sources to prevent use of intrinsic functions was not practical.
The CryptoPkg has the minimum set of intrinsic functions required
For OpenSSL to build.

We do recognize that this issue is difficult for developers to
resolve because the techniques require generation of mixed C/asm
output files to track down the C statements that are generating
the intrinsic functions.

Similar to the ARM support for an intrinsic lib, it may be 
reasonable to add a small intrinsic lib for IA32 and X64 that
supports the intrinsic functions that are seen the most often.
May  require an intrinsic lib for each supported tool chain.

This would be a new feature that would take some effort to 
implement and validate.  Can you enter an Bugzilla for this
feature and list the specific intrinsic functions needed for
this driver?

Thanks,

Mike

> -----Original Message-----
> From: Ryszard Knop
> [mailto:ryszard.knop@linux.intel.com]
> Sent: Wednesday, January 30, 2019 9:27 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> devel@lists.01.org; Carsey, Jaben
> <jaben.carsey@intel.com>
> Cc: Kacperski, Kamil <kamil.kacperski@intel.com>; Jin,
> Eric <eric.jin@intel.com>; Orlowski, Pawel
> <pawel.orlowski@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Hsiung, Harry L
> <harry.l.hsiung@intel.com>
> Subject: Re: [edk2] [PATCH edk2-staging 10/20]
> IntelUndiPkg/XGigUndiDxe: drop StdLibC library class
> reference
> 
> That's actually not quite correct - we need this
> package to build on
> IA32. It's named rather unfortunately, since it's not
> the EDK2 StdLibC,
> but rather a package in this repository - see
> IntelUndiPkg/LibC. It
> contains the bare minimum of functionality required to
> fix missing 64-
> bit math/shifts on IA32 and missing memcpy/memset
> intrinsics. We can't
> prevent MSVC from yielding memcpy/memset either, so
> this was the nasty
> solution for build issues. You have included
> CompilerIntrinsicsLib for
> the same reason, too :)
> 
> I'm not aware of any X64/IA32 equivalent of your
> CompilerIntrinsicsLib,
> but I'd be happy to be proven wrong here. I'm off for
> the rest of the
> week - I'll continue with reviews and merging early
> next week.
> 
> Thanks, Richard.
> 
> On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela
> wrote:
> > StdLibc should not be used in drivers (it has
> dependencies on Shell
> > protocols), but in fact, we don't appear to rely on
> it in the first
> > place, so just drop the reference.
> >
> > Contributed-under: TianoCore Contribution Agreement
> 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel at
> linaro.org>
> > ---
> >  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > index beee8aa8134e..b5747565fbea 100644
> > --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > @@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
> >    PrintLib
> >    UefiLib
> >    HiiLib
> > -  StdLibC
> >
> >  [LibraryClasses.X64]
> >

Re: [PATCH edk2-staging 10/20] IntelUndiPkg/XGigUndiDxe: drop StdLibC library class reference

[Ryszard Knop]

Mike,

As mentioned in previous mails, we can't change some of our code to
meet the coding standard. I've filled BZ #1516 with everything we need,
plus what CryptoPkg provides for reference.

Thanks, Richard.

On Wed, 2019-01-30 at 20:58 +0000, Kinney, Michael D wrote:
> Hi Richard,
> 
> It is possible to update C code to prevent the use of compiler
> intrinsic functions.  This is what we have done for EDK II modules
> that are built for both IA32 and X64.
> 
> The one exception is the use of OpenSSL.  We prefer to use that
> project "as is" as a git submodule, so modifying the OpenSSL
> sources to prevent use of intrinsic functions was not practical.
> The CryptoPkg has the minimum set of intrinsic functions required
> For OpenSSL to build.
> 
> We do recognize that this issue is difficult for developers to
> resolve because the techniques require generation of mixed C/asm
> output files to track down the C statements that are generating
> the intrinsic functions.
> 
> Similar to the ARM support for an intrinsic lib, it may be 
> reasonable to add a small intrinsic lib for IA32 and X64 that
> supports the intrinsic functions that are seen the most often.
> May  require an intrinsic lib for each supported tool chain.
> 
> This would be a new feature that would take some effort to 
> implement and validate.  Can you enter an Bugzilla for this
> feature and list the specific intrinsic functions needed for
> this driver?
> 
> Thanks,
> 
> Mike
> 
> > -----Original Message-----
> > From: Ryszard Knop
> > [mailto:ryszard.knop@linux.intel.com]
> > Sent: Wednesday, January 30, 2019 9:27 AM
> > To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> > devel@lists.01.org; Carsey, Jaben
> > <jaben.carsey@intel.com>
> > Cc: Kacperski, Kamil <kamil.kacperski@intel.com>; Jin,
> > Eric <eric.jin@intel.com>; Orlowski, Pawel
> > <pawel.orlowski@intel.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Hsiung, Harry L
> > <harry.l.hsiung@intel.com>
> > Subject: Re: [edk2] [PATCH edk2-staging 10/20]
> > IntelUndiPkg/XGigUndiDxe: drop StdLibC library class
> > reference
> > 
> > That's actually not quite correct - we need this
> > package to build on
> > IA32. It's named rather unfortunately, since it's not
> > the EDK2 StdLibC,
> > but rather a package in this repository - see
> > IntelUndiPkg/LibC. It
> > contains the bare minimum of functionality required to
> > fix missing 64-
> > bit math/shifts on IA32 and missing memcpy/memset
> > intrinsics. We can't
> > prevent MSVC from yielding memcpy/memset either, so
> > this was the nasty
> > solution for build issues. You have included
> > CompilerIntrinsicsLib for
> > the same reason, too :)
> > 
> > I'm not aware of any X64/IA32 equivalent of your
> > CompilerIntrinsicsLib,
> > but I'd be happy to be proven wrong here. I'm off for
> > the rest of the
> > week - I'll continue with reviews and merging early
> > next week.
> > 
> > Thanks, Richard.
> > 
> > On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela
> > wrote:
> > > StdLibc should not be used in drivers (it has
> > dependencies on Shell
> > > protocols), but in fact, we don't appear to rely on
> > it in the first
> > > place, so just drop the reference.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement
> > 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel at
> > linaro.org>
> > > ---
> > >  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > index beee8aa8134e..b5747565fbea 100644
> > > --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> > > @@ -132,7 +132,6 @@ GCC:*_*_*_CC_FLAGS = -DEFI32
> > >    PrintLib
> > >    UefiLib
> > >    HiiLib
> > > -  StdLibC
> > > 
> > >  [LibraryClasses.X64]
> > > 

[PATCH edk2-staging 11/20] IntelUndiPkg/XGigUndiDxe: cast XgbeMemCopy () args to correct pointer type

[Ard Biesheuvel]

XgbeMemCopy () takes UINT8 pointers not INT8 pointers, so cast the
arguments to the correct type.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Xgbe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
index 9e3ee862fb4e..0c823efe8963 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
@@ -542,8 +542,8 @@ XgbeReceive (
 
       // Copy the packet from our list to the EFI buffer.
       XgbeMemCopy (
-        (INT8 *) (UINTN) CpbReceive->BufferAddr,
-        (INT8 *) (UINTN) ReceiveDescriptor->buffer_addr,
+        (UINT8 *) (UINTN) CpbReceive->BufferAddr,
+        (UINT8 *) (UINTN) ReceiveDescriptor->buffer_addr,
         TempLen
       );
 
-- 
2.17.1

Re: [PATCH edk2-staging 11/20] IntelUndiPkg/XGigUndiDxe: cast XgbeMemCopy () args to correct pointer type

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> XgbeMemCopy () takes UINT8 pointers not INT8 pointers, so cast the
> arguments to the correct type.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Xgbe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> index 9e3ee862fb4e..0c823efe8963 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> @@ -542,8 +542,8 @@ XgbeReceive (
>  
>        // Copy the packet from our list to the EFI buffer.
>        XgbeMemCopy (
> -        (INT8 *) (UINTN) CpbReceive->BufferAddr,
> -        (INT8 *) (UINTN) ReceiveDescriptor->buffer_addr,
> +        (UINT8 *) (UINTN) CpbReceive->BufferAddr,
> +        (UINT8 *) (UINTN) ReceiveDescriptor->buffer_addr,
>          TempLen
>        );
>  

[PATCH edk2-staging 12/20] IntelUndiPkg/XGigUndiDxe: don't take address of cast expression

[Ard Biesheuvel]

Taking the address of a cast expression is not permitted in C. Instead,
take the address of the variable, and cast the pointer to the desired
pointer type.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Xgbe.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
index 0c823efe8963..003c3b9065ec 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
@@ -1264,7 +1264,7 @@ XgbeTxRxConfigure (
   IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_RDBAL (0), (UINT32) (UINTN) (XgbeAdapter->RxRing.PhysicalAddress));
 
   MemAddr = (UINT64) (UINTN) XgbeAdapter->RxRing.PhysicalAddress;
-  MemPtr  = &((UINT32) MemAddr);
+  MemPtr  = (UINT32 *) &MemAddr;
   MemPtr++;
   IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_RDBAH (0), *MemPtr);
   DEBUGPRINT (XGBE, ("Rdbal0 %X\n", (UINT32) IXGBE_READ_REG (&XgbeAdapter->Hw, IXGBE_RDBAL (0))));
@@ -1337,7 +1337,7 @@ XgbeTxRxConfigure (
   XgbeAdapter->XmitDoneHead = 0;  // the last cleaned buffer
   IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_TDBAL (0), (UINT32) (XgbeAdapter->TxRing.PhysicalAddress));
   MemAddr = (UINT64) XgbeAdapter->TxRing.PhysicalAddress;
-  MemPtr  = &((UINT32) MemAddr);
+  MemPtr  = (UINT32 *) &MemAddr;
   MemPtr++;
   IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_TDBAH (0), *MemPtr);
   DEBUGPRINT (XGBE, ("TdBah0 %X\n", *MemPtr));
-- 
2.17.1

Re: [PATCH edk2-staging 12/20] IntelUndiPkg/XGigUndiDxe: don't take address of cast expression

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Taking the address of a cast expression is not permitted in C.
> Instead,
> take the address of the variable, and cast the pointer to the desired
> pointer type.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Xgbe.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> index 0c823efe8963..003c3b9065ec 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> @@ -1264,7 +1264,7 @@ XgbeTxRxConfigure (
>    IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_RDBAL (0), (UINT32)
> (UINTN) (XgbeAdapter->RxRing.PhysicalAddress));
>  
>    MemAddr = (UINT64) (UINTN) XgbeAdapter->RxRing.PhysicalAddress;
> -  MemPtr  = &((UINT32) MemAddr);
> +  MemPtr  = (UINT32 *) &MemAddr;
>    MemPtr++;
>    IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_RDBAH (0), *MemPtr);
>    DEBUGPRINT (XGBE, ("Rdbal0 %X\n", (UINT32) IXGBE_READ_REG
> (&XgbeAdapter->Hw, IXGBE_RDBAL (0))));
> @@ -1337,7 +1337,7 @@ XgbeTxRxConfigure (
>    XgbeAdapter->XmitDoneHead = 0;  // the last cleaned buffer
>    IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_TDBAL (0), (UINT32)
> (XgbeAdapter->TxRing.PhysicalAddress));
>    MemAddr = (UINT64) XgbeAdapter->TxRing.PhysicalAddress;
> -  MemPtr  = &((UINT32) MemAddr);
> +  MemPtr  = (UINT32 *) &MemAddr;
>    MemPtr++;
>    IXGBE_WRITE_REG (&XgbeAdapter->Hw, IXGBE_TDBAH (0), *MemPtr);
>    DEBUGPRINT (XGBE, ("TdBah0 %X\n", *MemPtr));

[PATCH edk2-staging 13/20] IntelUndiPkg/XGigUndiDxe: drop locally defined ASSERT() macro

[Ard Biesheuvel]

Drop the locally defined ASSERT() macro and include DebugLib instead.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h | 22 +-------------------
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
index e324b0539782..aa3b3543cb79 100644
--- a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
+++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
@@ -41,6 +41,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <Base.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
 #include <Library/PrintLib.h>
 
 #define CHAR            CHAR8
@@ -219,26 +220,6 @@ IxgbeReverseDword (
 
 typedef BOOLEAN boolean_t;
 
-#if 0
-#undef ASSERT
-#define ASSERT(x)
-#define DEBUGOUT  Aprint
-#define DEBUGOUT1 Aprint
-#define DEBUGOUT2 Aprint
-#define DEBUGOUT3 Aprint
-#define DEBUGOUT6 Aprint
-#define DEBUGOUT7 Aprint
-#else /* 1 */
-#undef ASSERT
-
-/** ASSERT macro left blank
-
-   @param[in]   x    Assert condition
-   
-   @return   None
-**/
-#define ASSERT(x)
-
 /** Macro wrapper for shared code DEBUGOUT statement,
    blank here
 
@@ -311,7 +292,6 @@ typedef BOOLEAN boolean_t;
    @retval   None
 **/
 #define DEBUGOUT7(s, a, b, c, d, e, f, g)
-#endif /* 0 */
 
 /** Macro wrapper for shared code EWARN macro,
    blank here
-- 
2.17.1

[PATCH edk2-staging 14/20] IntelUndiPkg/XGigUndiDxe: redefine UNREFERENCED_nPARAMETER macros for GCC

[Ard Biesheuvel]

Use (VOID) casts to silence unreferenced parameter warnings on GCC. The
existing macros generate 'statement with no effect' warnings instead,
which does not really help.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/ixgbe_type.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h b/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
index e3bb1a8a313e..f67bfbfc2a9f 100644
--- a/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
+++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
@@ -4411,11 +4411,19 @@ struct ixgbe_hw {
 #define IXGBE_NOT_IMPLEMENTED			0x7FFFFFFF
 
 #ifndef UNREFERENCED_XPARAMETER
+#ifdef _MSC_VER
 #define UNREFERENCED_XPARAMETER
 #define UNREFERENCED_1PARAMETER(_p) (_p);
 #define UNREFERENCED_2PARAMETER(_p, _q) (_p); (_q);
 #define UNREFERENCED_3PARAMETER(_p, _q, _r) (_p); (_q); (_r);
 #define UNREFERENCED_4PARAMETER(_p, _q, _r, _s) (_p); (_q); (_r); (_s);
+#else
+#define UNREFERENCED_1PARAMETER(_p) (VOID)(_p)
+#define UNREFERENCED_2PARAMETER(_p, _q) (VOID)(_p); (VOID)(_q);
+#define UNREFERENCED_3PARAMETER(_p, _q, _r) (VOID)(_p); (VOID)(_q); (VOID)(_r);
+#define UNREFERENCED_4PARAMETER(_p, _q, _r, _s) (VOID)(_p); (VOID)(_q); (VOID)(_r); (VOID)(_s);
+#define UNREFERENCED_5PARAMETER(_p, _q, _r, _s, _t) (VOID)(_p); (VOID)(_q); (VOID)(_r); (VOID)(_s); (VOID)(_t);
+#endif
 #endif
 #define IXGBE_FUSES0_GROUP(_i)		(0x11158 + ((_i) * 4))
 #define IXGBE_FUSES0_300MHZ		(1 << 5)
-- 
2.17.1

Re: [PATCH edk2-staging 14/20] IntelUndiPkg/XGigUndiDxe: redefine UNREFERENCED_nPARAMETER macros for GCC

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Use (VOID) casts to silence unreferenced parameter warnings on GCC.
> The
> existing macros generate 'statement with no effect' warnings instead,
> which does not really help.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/ixgbe_type.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
> b/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
> index e3bb1a8a313e..f67bfbfc2a9f 100644
> --- a/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
> +++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_type.h
> @@ -4411,11 +4411,19 @@ struct ixgbe_hw {
>  #define IXGBE_NOT_IMPLEMENTED			0x7FFFFFFF
>  
>  #ifndef UNREFERENCED_XPARAMETER
> +#ifdef _MSC_VER
>  #define UNREFERENCED_XPARAMETER
>  #define UNREFERENCED_1PARAMETER(_p) (_p);
>  #define UNREFERENCED_2PARAMETER(_p, _q) (_p); (_q);
>  #define UNREFERENCED_3PARAMETER(_p, _q, _r) (_p); (_q); (_r);
>  #define UNREFERENCED_4PARAMETER(_p, _q, _r, _s) (_p); (_q); (_r);
> (_s);
> +#else
> +#define UNREFERENCED_1PARAMETER(_p) (VOID)(_p)
> +#define UNREFERENCED_2PARAMETER(_p, _q) (VOID)(_p); (VOID)(_q);
> +#define UNREFERENCED_3PARAMETER(_p, _q, _r) (VOID)(_p); (VOID)(_q);
> (VOID)(_r);
> +#define UNREFERENCED_4PARAMETER(_p, _q, _r, _s) (VOID)(_p);
> (VOID)(_q); (VOID)(_r); (VOID)(_s);
> +#define UNREFERENCED_5PARAMETER(_p, _q, _r, _s, _t) (VOID)(_p);
> (VOID)(_q); (VOID)(_r); (VOID)(_s); (VOID)(_t);
> +#endif
>  #endif
>  #define IXGBE_FUSES0_GROUP(_i)		(0x11158 + ((_i) * 4))
>  #define IXGBE_FUSES0_300MHZ		(1 << 5)

[PATCH edk2-staging 15/20] IntelUndiPkg/XGigUndiDxe: use intermediate UINTN casts for pointers

[Ard Biesheuvel]

Use intermediate (UINTN) casts when casting pointers to UINT64.
This is needed to be able to build this code for 32-bit architectures
such as ARM or IA32.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Dma.c               | 8 ++++----
 IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c | 4 ++--
 IntelUndiPkg/XGigUndiDxe/Init.c              | 6 +++---
 IntelUndiPkg/XGigUndiDxe/Xgbe.c              | 8 ++++----
 IntelUndiPkg/XGigUndiDxe/Xgbe.h              | 4 ++--
 5 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Dma.c b/IntelUndiPkg/XGigUndiDxe/Dma.c
index c8588df96ef5..79a5cefe9f41 100644
--- a/IntelUndiPkg/XGigUndiDxe/Dma.c
+++ b/IntelUndiPkg/XGigUndiDxe/Dma.c
@@ -127,7 +127,7 @@ FREE_BUF_ON_ERROR:
   PciIo->FreeBuffer (
            PciIo,
            BytesToPages (DmaMapping->Size),
-           (VOID *) DmaMapping->UnmappedAddress
+           (VOID *)(UINTN)DmaMapping->UnmappedAddress
            );
   DmaMapping->Size = 0;
   DmaMapping->UnmappedAddress = 0;
@@ -173,7 +173,7 @@ UndiDmaFreeCommonBuffer (
   PciIo->FreeBuffer (
            PciIo,
            BytesToPages (DmaMapping->Size),
-           (VOID *) DmaMapping->UnmappedAddress
+           (VOID *)(UINTN)DmaMapping->UnmappedAddress
            );
 
   DmaMapping->UnmappedAddress = 0;
@@ -209,7 +209,7 @@ UndiDmaMapCommonBuffer (
   return PciIo->Map (
                   PciIo,
                   EfiPciIoOperationBusMasterCommonBuffer,
-                  (VOID *) DmaMapping->UnmappedAddress,
+                  (VOID *)(UINTN)DmaMapping->UnmappedAddress,
                   &DmaMapping->Size,
                   &DmaMapping->PhysicalAddress,
                   &DmaMapping->Mapping
@@ -243,7 +243,7 @@ UndiDmaMapMemoryRead (
   return PciIo->Map (
                   PciIo,
                   EfiPciIoOperationBusMasterRead,
-                  (VOID *) DmaMapping->UnmappedAddress,
+                  (VOID *)(UINTN)DmaMapping->UnmappedAddress,
                   &DmaMapping->Size,
                   &DmaMapping->PhysicalAddress,
                   &DmaMapping->Mapping
diff --git a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
index 66dc67a5400e..2f9c66771910 100644
--- a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
+++ b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
@@ -119,11 +119,11 @@ XgbeUndiRunPhyLoopback (
   DEBUGPRINT (DIAG, ("CpbReceive.BufferAddr allocated at %x\n", (UINTN) CpbReceive.BufferAddr));
 
   while (j < PHY_LOOPBACK_ITERATIONS) {
-    ZeroMem ((VOID *) CpbReceive.BufferAddr, RX_BUFFER_SIZE);
+    ZeroMem ((VOID *)(UINTN)CpbReceive.BufferAddr, RX_BUFFER_SIZE);
 
     Status = XgbeTransmit (
                XgbeAdapter,
-               (UINT64) &PxeCpbTransmit,
+               (UINT64)(UINTN)&PxeCpbTransmit,
                PXE_OPFLAGS_TRANSMIT_WHOLE
              );
 
diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c b/IntelUndiPkg/XGigUndiDxe/Init.c
index 03e3942a1944..c112db667148 100644
--- a/IntelUndiPkg/XGigUndiDxe/Init.c
+++ b/IntelUndiPkg/XGigUndiDxe/Init.c
@@ -260,7 +260,7 @@ InitUndiPxeStructInit (
                            PXE_ROMID_IMP_TX_COMPLETE_INT_SUPPORTED |
                            PXE_ROMID_IMP_PACKET_RX_INT_SUPPORTED;
 
-  PxePtr->EntryPoint    = (UINT64) UndiApiEntry;
+  PxePtr->EntryPoint    = (UINT64)(UINTN)UndiApiEntry;
   PxePtr->MinorVer      = PXE_ROMID_MINORVER_31;
 
   PxePtr->reserved2[0]  = 0;
@@ -1099,7 +1099,7 @@ InitUndiCallbackFunctions (
   NicInfo->MapMem      = (VOID *) 0;
   NicInfo->UnMapMem    = (VOID *) 0;
   NicInfo->SyncMem     = (VOID *) 0;
-  NicInfo->UniqueId   = (UINT64) NicInfo;
+  NicInfo->UniqueId   = (UINT64)(UINTN)NicInfo;
   NicInfo->VersionFlag = 0x31;
 }
 
@@ -1284,7 +1284,7 @@ InitNiiProtocol (
   EFI_STATUS Status;
 
 
-  NiiProtocol31->Id            = (UINT64) (mIxgbePxe31);
+  NiiProtocol31->Id            = (UINT64)(UINTN)(mIxgbePxe31);
 
   // IFcnt should be equal to the total number of physical ports - 1
   NiiProtocol31->IfNum         = mIxgbePxe31->IFcnt;
diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
index 003c3b9065ec..6769f2dc72ab 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
@@ -1237,7 +1237,7 @@ XgbeTxRxConfigure (
 
   ZeroMem (XgbeAdapter->TxBufferMappings, sizeof (XgbeAdapter->TxBufferMappings));
 
-  RxBuffer = (LOCAL_RX_BUFFER *) XgbeAdapter->RxBufferMapping.PhysicalAddress;
+  RxBuffer = (LOCAL_RX_BUFFER *)(UINTN)XgbeAdapter->RxBufferMapping.PhysicalAddress;
 
   DEBUGPRINT (
     XGBE, ("Local Rx Buffer %X size %X\n",
@@ -1407,17 +1407,17 @@ XgbeInitialize (
   TempBar     = NULL;
 
   ZeroMem (
-    (VOID *) XgbeAdapter->RxRing.UnmappedAddress,
+    (VOID *)(UINTN)XgbeAdapter->RxRing.UnmappedAddress,
     RX_RING_SIZE
     );
 
   ZeroMem (
-    (VOID *) XgbeAdapter->TxRing.UnmappedAddress,
+    (VOID *)(UINTN)XgbeAdapter->TxRing.UnmappedAddress,
     TX_RING_SIZE
     );
 
   ZeroMem (
-    (VOID *) XgbeAdapter->RxBufferMapping.UnmappedAddress,
+    (VOID *)(UINTN)XgbeAdapter->RxBufferMapping.UnmappedAddress,
     RX_BUFFERS_SIZE
     );
 
diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.h b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
index 4472cfb4fa31..2a904de5bf2f 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
@@ -221,7 +221,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    @return   Descriptor retrieved
 **/
 #define XGBE_RX_DESC(R, i)          \
-          (&(((struct ixgbe_legacy_rx_desc *) ((R)->UnmappedAddress))[i]))
+          (&(((struct ixgbe_legacy_rx_desc *)(UINTN)((R)->UnmappedAddress))[i]))
 
 /** Retrieves TX descriptor from TX ring structure
 
@@ -231,7 +231,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    @return   Descriptor retrieved
 **/
 #define XGBE_TX_DESC(R, i)          \
-          (&(((struct ixgbe_legacy_tx_desc *) ((R)->UnmappedAddress))[i]))
+          (&(((struct ixgbe_legacy_tx_desc *)(UINTN)((R)->UnmappedAddress))[i]))
 
 /** Retrieves UNDI_PRIVATE_DATA structure using NII Protocol 3.1 instance
 
-- 
2.17.1

Re: [PATCH edk2-staging 15/20] IntelUndiPkg/XGigUndiDxe: use intermediate UINTN casts for pointers

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Use intermediate (UINTN) casts when casting pointers to UINT64.
> This is needed to be able to build this code for 32-bit architectures
> such as ARM or IA32.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Dma.c               | 8 ++++----
>  IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c | 4 ++--
>  IntelUndiPkg/XGigUndiDxe/Init.c              | 6 +++---
>  IntelUndiPkg/XGigUndiDxe/Xgbe.c              | 8 ++++----
>  IntelUndiPkg/XGigUndiDxe/Xgbe.h              | 4 ++--
>  5 files changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Dma.c
> b/IntelUndiPkg/XGigUndiDxe/Dma.c
> index c8588df96ef5..79a5cefe9f41 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Dma.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Dma.c
> @@ -127,7 +127,7 @@ FREE_BUF_ON_ERROR:
>    PciIo->FreeBuffer (
>             PciIo,
>             BytesToPages (DmaMapping->Size),
> -           (VOID *) DmaMapping->UnmappedAddress
> +           (VOID *)(UINTN)DmaMapping->UnmappedAddress
>             );
>    DmaMapping->Size = 0;
>    DmaMapping->UnmappedAddress = 0;
> @@ -173,7 +173,7 @@ UndiDmaFreeCommonBuffer (
>    PciIo->FreeBuffer (
>             PciIo,
>             BytesToPages (DmaMapping->Size),
> -           (VOID *) DmaMapping->UnmappedAddress
> +           (VOID *)(UINTN)DmaMapping->UnmappedAddress
>             );
>  
>    DmaMapping->UnmappedAddress = 0;
> @@ -209,7 +209,7 @@ UndiDmaMapCommonBuffer (
>    return PciIo->Map (
>                    PciIo,
>                    EfiPciIoOperationBusMasterCommonBuffer,
> -                  (VOID *) DmaMapping->UnmappedAddress,
> +                  (VOID *)(UINTN)DmaMapping->UnmappedAddress,
>                    &DmaMapping->Size,
>                    &DmaMapping->PhysicalAddress,
>                    &DmaMapping->Mapping
> @@ -243,7 +243,7 @@ UndiDmaMapMemoryRead (
>    return PciIo->Map (
>                    PciIo,
>                    EfiPciIoOperationBusMasterRead,
> -                  (VOID *) DmaMapping->UnmappedAddress,
> +                  (VOID *)(UINTN)DmaMapping->UnmappedAddress,
>                    &DmaMapping->Size,
>                    &DmaMapping->PhysicalAddress,
>                    &DmaMapping->Mapping
> diff --git a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> index 66dc67a5400e..2f9c66771910 100644
> --- a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> +++ b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> @@ -119,11 +119,11 @@ XgbeUndiRunPhyLoopback (
>    DEBUGPRINT (DIAG, ("CpbReceive.BufferAddr allocated at %x\n",
> (UINTN) CpbReceive.BufferAddr));
>  
>    while (j < PHY_LOOPBACK_ITERATIONS) {
> -    ZeroMem ((VOID *) CpbReceive.BufferAddr, RX_BUFFER_SIZE);
> +    ZeroMem ((VOID *)(UINTN)CpbReceive.BufferAddr, RX_BUFFER_SIZE);
>  
>      Status = XgbeTransmit (
>                 XgbeAdapter,
> -               (UINT64) &PxeCpbTransmit,
> +               (UINT64)(UINTN)&PxeCpbTransmit,
>                 PXE_OPFLAGS_TRANSMIT_WHOLE
>               );
>  
> diff --git a/IntelUndiPkg/XGigUndiDxe/Init.c
> b/IntelUndiPkg/XGigUndiDxe/Init.c
> index 03e3942a1944..c112db667148 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Init.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Init.c
> @@ -260,7 +260,7 @@ InitUndiPxeStructInit (
>                             PXE_ROMID_IMP_TX_COMPLETE_INT_SUPPORTED |
>                             PXE_ROMID_IMP_PACKET_RX_INT_SUPPORTED;
>  
> -  PxePtr->EntryPoint    = (UINT64) UndiApiEntry;
> +  PxePtr->EntryPoint    = (UINT64)(UINTN)UndiApiEntry;
>    PxePtr->MinorVer      = PXE_ROMID_MINORVER_31;
>  
>    PxePtr->reserved2[0]  = 0;
> @@ -1099,7 +1099,7 @@ InitUndiCallbackFunctions (
>    NicInfo->MapMem      = (VOID *) 0;
>    NicInfo->UnMapMem    = (VOID *) 0;
>    NicInfo->SyncMem     = (VOID *) 0;
> -  NicInfo->UniqueId   = (UINT64) NicInfo;
> +  NicInfo->UniqueId   = (UINT64)(UINTN)NicInfo;
>    NicInfo->VersionFlag = 0x31;
>  }
>  
> @@ -1284,7 +1284,7 @@ InitNiiProtocol (
>    EFI_STATUS Status;
>  
>  
> -  NiiProtocol31->Id            = (UINT64) (mIxgbePxe31);
> +  NiiProtocol31->Id            = (UINT64)(UINTN)(mIxgbePxe31);
>  
>    // IFcnt should be equal to the total number of physical ports - 1
>    NiiProtocol31->IfNum         = mIxgbePxe31->IFcnt;
> diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> index 003c3b9065ec..6769f2dc72ab 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> @@ -1237,7 +1237,7 @@ XgbeTxRxConfigure (
>  
>    ZeroMem (XgbeAdapter->TxBufferMappings, sizeof (XgbeAdapter-
> >TxBufferMappings));
>  
> -  RxBuffer = (LOCAL_RX_BUFFER *) XgbeAdapter-
> >RxBufferMapping.PhysicalAddress;
> +  RxBuffer = (LOCAL_RX_BUFFER *)(UINTN)XgbeAdapter-
> >RxBufferMapping.PhysicalAddress;
>  
>    DEBUGPRINT (
>      XGBE, ("Local Rx Buffer %X size %X\n",
> @@ -1407,17 +1407,17 @@ XgbeInitialize (
>    TempBar     = NULL;
>  
>    ZeroMem (
> -    (VOID *) XgbeAdapter->RxRing.UnmappedAddress,
> +    (VOID *)(UINTN)XgbeAdapter->RxRing.UnmappedAddress,
>      RX_RING_SIZE
>      );
>  
>    ZeroMem (
> -    (VOID *) XgbeAdapter->TxRing.UnmappedAddress,
> +    (VOID *)(UINTN)XgbeAdapter->TxRing.UnmappedAddress,
>      TX_RING_SIZE
>      );
>  
>    ZeroMem (
> -    (VOID *) XgbeAdapter->RxBufferMapping.UnmappedAddress,
> +    (VOID *)(UINTN)XgbeAdapter->RxBufferMapping.UnmappedAddress,
>      RX_BUFFERS_SIZE
>      );
>  
> diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> index 4472cfb4fa31..2a904de5bf2f 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> +++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
> @@ -221,7 +221,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>     @return   Descriptor retrieved
>  **/
>  #define XGBE_RX_DESC(R, i)          \
> -          (&(((struct ixgbe_legacy_rx_desc *) ((R)-
> >UnmappedAddress))[i]))
> +          (&(((struct ixgbe_legacy_rx_desc *)(UINTN)((R)-
> >UnmappedAddress))[i]))
>  
>  /** Retrieves TX descriptor from TX ring structure
>  
> @@ -231,7 +231,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>     @return   Descriptor retrieved
>  **/
>  #define XGBE_TX_DESC(R, i)          \
> -          (&(((struct ixgbe_legacy_tx_desc *) ((R)-
> >UnmappedAddress))[i]))
> +          (&(((struct ixgbe_legacy_tx_desc *)(UINTN)((R)-
> >UnmappedAddress))[i]))
>  
>  /** Retrieves UNDI_PRIVATE_DATA structure using NII Protocol 3.1
> instance
>  

[PATCH edk2-staging 16/20] IntelUndiPkg/XGigUndiDxe: add missing EFIAPI modifiers

[Ard Biesheuvel]

When building for the X64 target using GCC, correct use of the
EFIAPI modifiers is essential to ensure that the correct calling
convention is used. So add the missing ones where appropriate.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/AdapterInformation.c | 3 +++
 IntelUndiPkg/XGigUndiDxe/ComponentName.c      | 2 ++
 IntelUndiPkg/XGigUndiDxe/ComponentName.h      | 1 +
 IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c  | 1 +
 IntelUndiPkg/XGigUndiDxe/DriverHealth.c       | 2 ++
 IntelUndiPkg/XGigUndiDxe/StartStop.c          | 2 ++
 6 files changed, 11 insertions(+)

diff --git a/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c b/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
index 26556e284381..b25ed7c4f022 100644
--- a/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
+++ b/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
@@ -123,6 +123,7 @@ GetIpv6SupportInformationBlock (
 **/
 STATIC
 EFI_STATUS
+EFIAPI
 GetInformation (
   IN  EFI_ADAPTER_INFORMATION_PROTOCOL *This,
   IN  EFI_GUID *                        InformationType,
@@ -188,6 +189,7 @@ GetInformation (
 **/
 STATIC
 EFI_STATUS
+EFIAPI
 SetInformation (
   IN  EFI_ADAPTER_INFORMATION_PROTOCOL *This,
   IN  EFI_GUID *                        InformationType,
@@ -234,6 +236,7 @@ SetInformation (
 **/
 STATIC
 EFI_STATUS
+EFIAPI
 GetSupportedTypes (
   IN  EFI_ADAPTER_INFORMATION_PROTOCOL *This,
   OUT EFI_GUID **                       InfoTypesBuffer,
diff --git a/IntelUndiPkg/XGigUndiDxe/ComponentName.c b/IntelUndiPkg/XGigUndiDxe/ComponentName.c
index 2972c28bda4e..eee59d7c14f4 100644
--- a/IntelUndiPkg/XGigUndiDxe/ComponentName.c
+++ b/IntelUndiPkg/XGigUndiDxe/ComponentName.c
@@ -116,6 +116,7 @@ ComponentNameInitializeControllerName (
                                     language specified by Language.
 **/
 EFI_STATUS
+EFIAPI
 ComponentNameGetDriverName (
   IN  EFI_COMPONENT_NAME_PROTOCOL *This,
   IN  CHAR8 *                      Language,
@@ -186,6 +187,7 @@ ComponentNameGetDriverName (
                                     language specified by Language.
 **/
 EFI_STATUS
+EFIAPI
 ComponentNameGetControllerName (
   IN  EFI_COMPONENT_NAME_PROTOCOL *                               This,
   IN  EFI_HANDLE                                                  ControllerHandle,
diff --git a/IntelUndiPkg/XGigUndiDxe/ComponentName.h b/IntelUndiPkg/XGigUndiDxe/ComponentName.h
index f63d58b8a18c..4f62e48379ed 100644
--- a/IntelUndiPkg/XGigUndiDxe/ComponentName.h
+++ b/IntelUndiPkg/XGigUndiDxe/ComponentName.h
@@ -67,6 +67,7 @@ ComponentNameInitializeControllerName (
                                     language specified by Language.
 **/
 EFI_STATUS
+EFIAPI
 ComponentNameGetDriverName (
   IN  EFI_COMPONENT_NAME_PROTOCOL *This,
   IN  CHAR8 *                      Language,
diff --git a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
index 2f9c66771910..77b7a954698b 100644
--- a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
+++ b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
@@ -370,6 +370,7 @@ XgbeUndiPhyLoopback (
                                          ChildHandle did not pass the diagnostic.
 **/
 EFI_STATUS
+EFIAPI
 XgbeUndiDriverDiagnosticsRunDiagnostics (
   IN EFI_DRIVER_DIAGNOSTICS_PROTOCOL *                                   This,
   IN EFI_HANDLE                                                          ControllerHandle,
diff --git a/IntelUndiPkg/XGigUndiDxe/DriverHealth.c b/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
index df6cfdf8a123..4b173bc8d8a9 100644
--- a/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
+++ b/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
@@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
    @retval      !EFI_SUCCESS      Failure to retrieve health status
 **/
 EFI_STATUS
+EFIAPI
 GetHealthStatus (
   IN  EFI_DRIVER_HEALTH_PROTOCOL *     This,
   IN  EFI_HANDLE                       ControllerHandle, OPTIONAL
@@ -145,6 +146,7 @@ GetHealthStatus (
    @retval   EFI_UNSUPPORTED   This function is unsupported
 **/
 EFI_STATUS
+EFIAPI
 Repair (
   IN  EFI_DRIVER_HEALTH_PROTOCOL                *This,
   IN  EFI_HANDLE                                ControllerHandle,
diff --git a/IntelUndiPkg/XGigUndiDxe/StartStop.c b/IntelUndiPkg/XGigUndiDxe/StartStop.c
index 156e927a35c6..c881c811aefb 100644
--- a/IntelUndiPkg/XGigUndiDxe/StartStop.c
+++ b/IntelUndiPkg/XGigUndiDxe/StartStop.c
@@ -38,6 +38,7 @@ EFI_GUID gEfiStartStopProtocolGuid = EFI_DRIVER_STOP_PROTOCOL_GUID;
    @retval   EFI_SUCCESS   Driver is stopped successfully
 **/
 EFI_STATUS
+EFIAPI
 StopDriver (
   IN EFI_DRIVER_STOP_PROTOCOL *This
   )
@@ -63,6 +64,7 @@ StopDriver (
    @retval   EFI_DEVICE_ERROR  Failed to initialize hardware
 **/
 EFI_STATUS
+EFIAPI
 StartDriver (
   IN EFI_DRIVER_STOP_PROTOCOL *This
   )
-- 
2.17.1

Re: [PATCH edk2-staging 16/20] IntelUndiPkg/XGigUndiDxe: add missing EFIAPI modifiers

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> When building for the X64 target using GCC, correct use of the
> EFIAPI modifiers is essential to ensure that the correct calling
> convention is used. So add the missing ones where appropriate.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/AdapterInformation.c | 3 +++
>  IntelUndiPkg/XGigUndiDxe/ComponentName.c      | 2 ++
>  IntelUndiPkg/XGigUndiDxe/ComponentName.h      | 1 +
>  IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c  | 1 +
>  IntelUndiPkg/XGigUndiDxe/DriverHealth.c       | 2 ++
>  IntelUndiPkg/XGigUndiDxe/StartStop.c          | 2 ++
>  6 files changed, 11 insertions(+)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
> b/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
> index 26556e284381..b25ed7c4f022 100644
> --- a/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
> +++ b/IntelUndiPkg/XGigUndiDxe/AdapterInformation.c
> @@ -123,6 +123,7 @@ GetIpv6SupportInformationBlock (
>  **/
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  GetInformation (
>    IN  EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>    IN  EFI_GUID *                        InformationType,
> @@ -188,6 +189,7 @@ GetInformation (
>  **/
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  SetInformation (
>    IN  EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>    IN  EFI_GUID *                        InformationType,
> @@ -234,6 +236,7 @@ SetInformation (
>  **/
>  STATIC
>  EFI_STATUS
> +EFIAPI
>  GetSupportedTypes (
>    IN  EFI_ADAPTER_INFORMATION_PROTOCOL *This,
>    OUT EFI_GUID **                       InfoTypesBuffer,
> diff --git a/IntelUndiPkg/XGigUndiDxe/ComponentName.c
> b/IntelUndiPkg/XGigUndiDxe/ComponentName.c
> index 2972c28bda4e..eee59d7c14f4 100644
> --- a/IntelUndiPkg/XGigUndiDxe/ComponentName.c
> +++ b/IntelUndiPkg/XGigUndiDxe/ComponentName.c
> @@ -116,6 +116,7 @@ ComponentNameInitializeControllerName (
>                                      language specified by Language.
>  **/
>  EFI_STATUS
> +EFIAPI
>  ComponentNameGetDriverName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL *This,
>    IN  CHAR8 *                      Language,
> @@ -186,6 +187,7 @@ ComponentNameGetDriverName (
>                                      language specified by Language.
>  **/
>  EFI_STATUS
> +EFIAPI
>  ComponentNameGetControllerName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL
> *                               This,
>    IN  EFI_HANDLE                                                  Co
> ntrollerHandle,
> diff --git a/IntelUndiPkg/XGigUndiDxe/ComponentName.h
> b/IntelUndiPkg/XGigUndiDxe/ComponentName.h
> index f63d58b8a18c..4f62e48379ed 100644
> --- a/IntelUndiPkg/XGigUndiDxe/ComponentName.h
> +++ b/IntelUndiPkg/XGigUndiDxe/ComponentName.h
> @@ -67,6 +67,7 @@ ComponentNameInitializeControllerName (
>                                      language specified by Language.
>  **/
>  EFI_STATUS
> +EFIAPI
>  ComponentNameGetDriverName (
>    IN  EFI_COMPONENT_NAME_PROTOCOL *This,
>    IN  CHAR8 *                      Language,
> diff --git a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> index 2f9c66771910..77b7a954698b 100644
> --- a/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> +++ b/IntelUndiPkg/XGigUndiDxe/DriverDiagnostics.c
> @@ -370,6 +370,7 @@ XgbeUndiPhyLoopback (
>                                           ChildHandle did not pass
> the diagnostic.
>  **/
>  EFI_STATUS
> +EFIAPI
>  XgbeUndiDriverDiagnosticsRunDiagnostics (
>    IN EFI_DRIVER_DIAGNOSTICS_PROTOCOL
> *                                   This,
>    IN
> EFI_HANDLE                                                          C
> ontrollerHandle,
> diff --git a/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
> b/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
> index df6cfdf8a123..4b173bc8d8a9 100644
> --- a/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
> +++ b/IntelUndiPkg/XGigUndiDxe/DriverHealth.c
> @@ -45,6 +45,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> POSSIBILITY OF SUCH DAMAGE.
>     @retval      !EFI_SUCCESS      Failure to retrieve health status
>  **/
>  EFI_STATUS
> +EFIAPI
>  GetHealthStatus (
>    IN  EFI_DRIVER_HEALTH_PROTOCOL *     This,
>    IN  EFI_HANDLE                       ControllerHandle, OPTIONAL
> @@ -145,6 +146,7 @@ GetHealthStatus (
>     @retval   EFI_UNSUPPORTED   This function is unsupported
>  **/
>  EFI_STATUS
> +EFIAPI
>  Repair (
>    IN  EFI_DRIVER_HEALTH_PROTOCOL                *This,
>    IN  EFI_HANDLE                                ControllerHandle,
> diff --git a/IntelUndiPkg/XGigUndiDxe/StartStop.c
> b/IntelUndiPkg/XGigUndiDxe/StartStop.c
> index 156e927a35c6..c881c811aefb 100644
> --- a/IntelUndiPkg/XGigUndiDxe/StartStop.c
> +++ b/IntelUndiPkg/XGigUndiDxe/StartStop.c
> @@ -38,6 +38,7 @@ EFI_GUID gEfiStartStopProtocolGuid =
> EFI_DRIVER_STOP_PROTOCOL_GUID;
>     @retval   EFI_SUCCESS   Driver is stopped successfully
>  **/
>  EFI_STATUS
> +EFIAPI
>  StopDriver (
>    IN EFI_DRIVER_STOP_PROTOCOL *This
>    )
> @@ -63,6 +64,7 @@ StopDriver (
>     @retval   EFI_DEVICE_ERROR  Failed to initialize hardware
>  **/
>  EFI_STATUS
> +EFIAPI
>  StartDriver (
>    IN EFI_DRIVER_STOP_PROTOCOL *This
>    )

[PATCH edk2-staging 17/20] IntelUndiPkg/XGigUndiDxe: drop unused variables

[Ard Biesheuvel]

Drop some variables that are defined, assigned but never referenced.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Decode.c         | 3 ---
 IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c | 4 ----
 IntelUndiPkg/XGigUndiDxe/Xgbe.c           | 2 --
 3 files changed, 9 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Decode.c b/IntelUndiPkg/XGigUndiDxe/Decode.c
index 5f37ce254872..c8dc3f4eb659 100644
--- a/IntelUndiPkg/XGigUndiDxe/Decode.c
+++ b/IntelUndiPkg/XGigUndiDxe/Decode.c
@@ -1454,14 +1454,11 @@ UndiStatus (
   )
 {
   PXE_DB_GET_STATUS *          DbPtr;
-  UINT16                       i;
   UINT32                       IntStatus;
   UINT16                       NumEntries;
   struct ixgbe_legacy_rx_desc *RxPtr;
   bool                         LinkUp;
 
-  i = 0;
-
   if (XgbeAdapter->DriverBusy) {
   
     //DEBUGPRINT (CRITICAL, ("ERROR: UndiStatus called when driver busy\n"));
diff --git a/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c b/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
index 2aacb63ca158..df5238a3b55a 100644
--- a/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
+++ b/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
@@ -197,7 +197,6 @@ GetNextRequestElement (
   )
 {
   EFI_STRING StringPtr;
-  EFI_STRING TmpPtr;
   EFI_STATUS Status;
   UINTN      Length;
   UINT8 *    TmpBuffer;
@@ -226,9 +225,6 @@ GetNextRequestElement (
     return NULL;
   }
 
-  // Back up the header of one <BlockName>
-  TmpPtr = StringPtr;
-
   StringPtr += StrLen (L"OFFSET=");
 
   // Get Offset
diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
index 6769f2dc72ab..b3dbbd2b1d08 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
@@ -1399,12 +1399,10 @@ XgbeInitialize (
   XGBE_DRIVER_DATA *XgbeAdapter
   )
 {
-  UINT32 *     TempBar;
   PXE_STATCODE PxeStatcode;
   EFI_STATUS   Status;
 
   PxeStatcode = PXE_STATCODE_SUCCESS;
-  TempBar     = NULL;
 
   ZeroMem (
     (VOID *)(UINTN)XgbeAdapter->RxRing.UnmappedAddress,
-- 
2.17.1

Re: [PATCH edk2-staging 17/20] IntelUndiPkg/XGigUndiDxe: drop unused variables

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Drop some variables that are defined, assigned but never referenced.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/Decode.c         | 3 ---
>  IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c | 4 ----
>  IntelUndiPkg/XGigUndiDxe/Xgbe.c           | 2 --
>  3 files changed, 9 deletions(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/Decode.c
> b/IntelUndiPkg/XGigUndiDxe/Decode.c
> index 5f37ce254872..c8dc3f4eb659 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Decode.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Decode.c
> @@ -1454,14 +1454,11 @@ UndiStatus (
>    )
>  {
>    PXE_DB_GET_STATUS *          DbPtr;
> -  UINT16                       i;
>    UINT32                       IntStatus;
>    UINT16                       NumEntries;
>    struct ixgbe_legacy_rx_desc *RxPtr;
>    bool                         LinkUp;
>  
> -  i = 0;
> -
>    if (XgbeAdapter->DriverBusy) {
>    
>      //DEBUGPRINT (CRITICAL, ("ERROR: UndiStatus called when driver
> busy\n"));
> diff --git a/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
> b/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
> index 2aacb63ca158..df5238a3b55a 100644
> --- a/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
> +++ b/IntelUndiPkg/XGigUndiDxe/HiiInternalLib.c
> @@ -197,7 +197,6 @@ GetNextRequestElement (
>    )
>  {
>    EFI_STRING StringPtr;
> -  EFI_STRING TmpPtr;
>    EFI_STATUS Status;
>    UINTN      Length;
>    UINT8 *    TmpBuffer;
> @@ -226,9 +225,6 @@ GetNextRequestElement (
>      return NULL;
>    }
>  
> -  // Back up the header of one <BlockName>
> -  TmpPtr = StringPtr;
> -
>    StringPtr += StrLen (L"OFFSET=");
>  
>    // Get Offset
> diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> index 6769f2dc72ab..b3dbbd2b1d08 100644
> --- a/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> +++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.c
> @@ -1399,12 +1399,10 @@ XgbeInitialize (
>    XGBE_DRIVER_DATA *XgbeAdapter
>    )
>  {
> -  UINT32 *     TempBar;
>    PXE_STATCODE PxeStatcode;
>    EFI_STATUS   Status;
>  
>    PxeStatcode = PXE_STATCODE_SUCCESS;
> -  TempBar     = NULL;
>  
>    ZeroMem (
>      (VOID *)(UINTN)XgbeAdapter->RxRing.UnmappedAddress,

[PATCH edk2-staging 18/20] IntelUndiPkg/XGigUndiDxe: set MDEPKG_NDEBUG only for RELEASE builds

[Ard Biesheuvel]

Only define the CPP macro MDEPKG_NDEBUG for the RELEASE target so
that debug features are functional otherwise.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
index b5747565fbea..e3201fb9881f 100644
--- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
+++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
@@ -44,7 +44,8 @@ UNLOAD_IMAGE         = UnloadXGigUndiDriver
 
 [BuildOptions.common]
 
-MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G /wd4244 /wd4206 /wd4189
+MSFT:*_*_*_CC_FLAGS = /FAcs /D UNDI_10G /wd4244 /wd4206 /wd4189
+MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
 GCC:*_*_*_CC_FLAGS = -DUNDI_10G
 GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
 
-- 
2.17.1

Re: [PATCH edk2-staging 18/20] IntelUndiPkg/XGigUndiDxe: set MDEPKG_NDEBUG only for RELEASE builds

[Ryszard Knop]

Reviewed-by: Ryszard Knop <ryszard.knop@linux.intel.com>

On Wed, 2018-11-14 at 18:33 -0800, ard.biesheuvela wrote:
> Only define the CPP macro MDEPKG_NDEBUG for the RELEASE target so
> that debug features are functional otherwise.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> index b5747565fbea..e3201fb9881f 100644
> --- a/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> +++ b/IntelUndiPkg/XGigUndiDxe/XGigUndiDxe.inf
> @@ -44,7 +44,8 @@ UNLOAD_IMAGE         = UnloadXGigUndiDriver
>  
>  [BuildOptions.common]
>  
> -MSFT:*_*_*_CC_FLAGS = /FAcs /D MDEPKG_NDEBUG /D UNDI_10G /wd4244
> /wd4206 /wd4189
> +MSFT:*_*_*_CC_FLAGS = /FAcs /D UNDI_10G /wd4244 /wd4206 /wd4189
> +MSFT:RELEASE_*_*_CC_FLAGS = /D MDEPKG_NDEBUG
>  GCC:*_*_*_CC_FLAGS = -DUNDI_10G
>  GCC:RELEASE_*_*_CC_FLAGS = -DMDEPKG_NDEBUG
>  

[PATCH edk2-staging 19/20] IntelUndiPkg/XGigUndiDxe: drop separate debug macros for DBG_LVL

[Ard Biesheuvel]

The DBG_LVL bit tests are already done at build time, so there is no
point in having separate definitions, since they only result in
unused variable warnings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/Xgbe.h | 21 --------------------
 1 file changed, 21 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/Xgbe.h b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
index 2a904de5bf2f..ecfcc1032e8b 100644
--- a/IntelUndiPkg/XGigUndiDxe/Xgbe.h
+++ b/IntelUndiPkg/XGigUndiDxe/Xgbe.h
@@ -1112,7 +1112,6 @@ WaitForEnter (
   );
 
 // This is the Macro Section
-#if DBG_LVL
 /** When specific debug level is currently set this macro
    prints debug message.
 
@@ -1137,26 +1136,6 @@ WaitForEnter (
   if ((DBG_LVL & Lvl) != 0) { \
     WaitForEnter (); \
   }
-#else /* NOT DBG_LVL */
-
-// Comment out the debug stuff
-/** When DBG_LVL is not defined leave occurences of DEBUGPRINT blank
-
-   @param[in]   Lvl   Debug level
-   @param[in]   Msg   Debug message
-
-   @return   None
-**/
-#define DEBUGPRINT(Lvl, Msg)
-
-/** When DBG_LVL is not defined leave occurences of DEBUGWAIT blank
-
-   @param[in]   Lvl   Debug level
-
-   @return   None
-**/
-#define DEBUGWAIT(Lvl)
-#endif /* DBG_LVL */
 
 /** Delays code execution for specified time in milliseconds
 
-- 
2.17.1

[PATCH edk2-staging 20/20] IntelUndiPkg/XGigUndiDxe: avoid unused var warnings for ERROR_REPORTn()

[Ard Biesheuvel]

We hit unused variable warnings since the various ERROR_REPORTn()
macros resolve to nothing. So instead, map them to the macros that
were created to prevent these warnings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
index aa3b3543cb79..02eb6fe7f67b 100644
--- a/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
+++ b/IntelUndiPkg/XGigUndiDxe/ixgbe_osdep.h
@@ -451,7 +451,7 @@ typedef BOOLEAN boolean_t;
 
    @retval  None
 **/
-#define ERROR_REPORT(S,A)
+#define ERROR_REPORT(S,A) UNREFERENCED_1PARAMETER(A)
 
 /** ERROR_REPORT1 wrapper macro for shared code,
    blank here
@@ -461,7 +461,7 @@ typedef BOOLEAN boolean_t;
 
    @retval  None
 **/
-#define ERROR_REPORT1(S,A)
+#define ERROR_REPORT1(S,A) UNREFERENCED_1PARAMETER(A)
 
 /** ERROR_REPORT2 wrapper macro for shared code,
    blank here
@@ -472,7 +472,7 @@ typedef BOOLEAN boolean_t;
 
    @retval  None
 **/
-#define ERROR_REPORT2(S,A,B)
+#define ERROR_REPORT2(S,A,B) UNREFERENCED_2PARAMETER(A,B)
 
 /** ERROR_REPORT3 wrapper macro for shared code,
    blank here
@@ -484,7 +484,7 @@ typedef BOOLEAN boolean_t;
 
    @retval  None
 **/
-#define ERROR_REPORT3(S,A,B,C)
+#define ERROR_REPORT3(S,A,B,C) UNREFERENCED_3PARAMETER(A,B,C)
 
 /** Macro for word conversion from CPU native
    to Big Endian
-- 
2.17.1