Re: [edk2] Regarding UefiDriverEntryPoint unload handler

2017-06-14 Thread Marvin Häuser
Hey Mike,

"if UnloadImage() is called and the Unload field is NULL, it returns an 
EFI_UNSUPPORTED."
This is the part I missed, my example is impossible in this case.

Thanks,
Marvin.

-Ursprüngliche Nachricht-
Von: Kinney, Michael D [mailto:michael.d.kin...@intel.com] 
Gesendet: Donnerstag, 15. Juni 2017 04:51
An: Marvin H?user ; edk2-devel@lists.01.org; 
Kinney, Michael D 
Betreff: RE: Regarding UefiDriverEntryPoint unload handler

Hi Marvin,

Yes.  This is intentional.  If MODULE_UNLOAD is not provided, then the module 
can never be unloaded.  
The LoadImage() services initializes the Unload field to NULL, and if 
UnloadImage() is called and the Unload field is NULL, it returns an 
EFI_UNSUPPORTED.

I do not think the case you describer is possible.  If 
ProcessModuleUnloadList() returns an error, then 
ProcessLibraryDestructorList()is not called.

Since the module is not unloaded, it is still in allocated memory.  How would 
the protocol point to a freed buffer?

Mike

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Marvin 
H?user
Sent: Wednesday, June 14, 2017 6:41 PM
To: edk2-devel@lists.01.org
Subject: [edk2] Regarding UefiDriverEntryPoint unload handler

Dear developers,

While performing some research tasks, I noticed that when 
UefiDriverEntryPoint's _gDriverUnloadImageCount is 0 (only MODULE_UNLOAD 
entries are count, DESTRUCTOR as they are used with libraries are not, as far 
as I can see), EFI_LOADED_IMAGE_PROTOCOL.Unload is not set, even if libraries 
with destructors are included by the built module.
Is this intentional, so, is a module without a MODULE_UNLOAD property in its 
build file considered a module that does not support being unloaded? If so, why 
is EFI_LOADED_IMAGE_PROTOCOL.Unload not set to a dummy that returns an EFI 
error code?

For example, DxeDebugPrintErrorLevelLib installs a protocol interface in its 
CONSTRUCTOR function. When this library is included in a module without a 
MODULE_UNLOAD property and that module is unloaded, the 
DxeDebugPrintErrorLevelLib DESTRUCTOR function, which uninstalls the interface, 
is never called and hence the interface remains in the protocol database, 
despite it pointing to a memory location that is now free. If it is called, the 
behavior is obviously undefined.

Did I understand something incorrectly, are these modules not supposed to be 
unloadable or is this a bug?

Thank you,
Marvin.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch 1/2 V2] BaseTools: add /Gw to CC_FLAGS for VS2013 and higher tool chain tags

2017-06-14 Thread Kinney, Michael D

Series Reviewed-by: Michael D Kinney 

Mike

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Wednesday, June 14, 2017 12:35 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [edk2] [Patch 1/2 V2] BaseTools: add /Gw to CC_FLAGS for VS2013 and 
higher tool chain tags

The /Gw flag does a better job at size optimization than use of the
GLOBAL_REMOVE_IF_UNREFERENCED macro that is currently used for VS20xx
tool chains to remove unreferenced global variables.
This patch add /Gw to CC_FLAGS for VS2013 and higher tool chain tags.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Conf/tools_def.template | 96 +++
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 04a1bcb..6f73bd7 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3167,13 +3167,13 @@ NOOPT_VS2012x86xASL_X64_DLINK_FLAGS= /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT
 *_VS2013_IA32_ASLCC_PATH  = DEF(VS2013_BIN)\cl.exe
 *_VS2013_IA32_ASLPP_PATH  = DEF(VS2013_BIN)\cl.exe
 *_VS2013_IA32_ASLDLINK_PATH   = DEF(VS2013_BIN)\link.exe
 
   *_VS2013_IA32_MAKE_FLAGS= /nologo
-  DEBUG_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm
-RELEASE_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od
+  DEBUG_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw
+RELEASE_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Gw
 
   DEBUG_VS2013_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 RELEASE_VS2013_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd
 NOOPT_VS2013_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 
@@ -3199,13 +3199,13 @@ NOOPT_VS2013_IA32_DLINK_FLAGS = /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT:REF
 *_VS2013_X64_DLINK_PATH= DEF(VS2013_BINX64)\link.exe
 *_VS2013_X64_ASLCC_PATH= DEF(VS2013_BINX64)\cl.exe
 *_VS2013_X64_ASLPP_PATH= DEF(VS2013_BINX64)\cl.exe
 *_VS2013_X64_ASLDLINK_PATH = DEF(VS2013_BINX64)\link.exe
 
-  DEBUG_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm
-RELEASE_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013_X64_CC_FLAGS   = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Od
+  DEBUG_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Gw
+RELEASE_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013_X64_CC_FLAGS   = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Od /Gw
 
   DEBUG_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd /Zi
 RELEASE_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd
 NOOPT_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd /Zi
 
@@ -3285,13 +3285,13 @@ NOOPT_VS2013_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF /OPT
 *_VS2013xASL_IA32_ASLCC_PATH= DEF(VS2013_BIN)\cl.exe
 *_VS2013xASL_IA32_ASLPP_PATH= DEF(VS2013_BIN)\cl.exe
 *_VS2013xASL_IA32_ASLDLINK_PATH = DEF(VS2013_BIN)\link.exe
 
   *_VS2013xASL_IA32_MAKE_FLAGS  = /nologo
-  DEBUG_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm
-RELEASE_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013xASL_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od
+  DEBUG_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw
+RELEASE_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013xASL_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Gw
 
   DEBUG_VS2013xASL_IA32_ASM_FLAGS   = /nologo /c /WX /W

Re: [edk2] Regarding UefiDriverEntryPoint unload handler

2017-06-14 Thread Kinney, Michael D
Hi Marvin,

Yes.  This is intentional.  If MODULE_UNLOAD is not provided, then the module 
can never be unloaded.  
The LoadImage() services initializes the Unload field to NULL, and if 
UnloadImage() is called and
the Unload field is NULL, it returns an EFI_UNSUPPORTED.

I do not think the case you describer is possible.  If 
ProcessModuleUnloadList() returns an error,
then ProcessLibraryDestructorList()is not called.

Since the module is not unloaded, it is still in allocated memory.  How would 
the protocol point
to a freed buffer?

Mike

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Marvin 
H?user
Sent: Wednesday, June 14, 2017 6:41 PM
To: edk2-devel@lists.01.org
Subject: [edk2] Regarding UefiDriverEntryPoint unload handler

Dear developers,

While performing some research tasks, I noticed that when 
UefiDriverEntryPoint's _gDriverUnloadImageCount is 0 (only MODULE_UNLOAD 
entries are count, DESTRUCTOR as they are used with libraries are not, as far 
as I can see), EFI_LOADED_IMAGE_PROTOCOL.Unload is not set, even if libraries 
with destructors are included by the built module.
Is this intentional, so, is a module without a MODULE_UNLOAD property in its 
build file considered a module that does not support being unloaded? If so, why 
is EFI_LOADED_IMAGE_PROTOCOL.Unload not set to a dummy that returns an EFI 
error code?

For example, DxeDebugPrintErrorLevelLib installs a protocol interface in its 
CONSTRUCTOR function. When this library is included in a module without a 
MODULE_UNLOAD property and that module is unloaded, the 
DxeDebugPrintErrorLevelLib DESTRUCTOR function, which uninstalls the interface, 
is never called and hence the interface remains in the protocol database, 
despite it pointing to a memory location that is now free. If it is called, the 
behavior is obviously undefined.

Did I understand something incorrectly, are these modules not supposed to be 
unloadable or is this a bug?

Thank you,
Marvin.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] Regarding UefiDriverEntryPoint unload handler

2017-06-14 Thread Marvin H?user
Dear developers,

While performing some research tasks, I noticed that when 
UefiDriverEntryPoint's _gDriverUnloadImageCount is 0 (only MODULE_UNLOAD 
entries are count, DESTRUCTOR as they are used with libraries are not, as far 
as I can see), EFI_LOADED_IMAGE_PROTOCOL.Unload is not set, even if libraries 
with destructors are included by the built module.
Is this intentional, so, is a module without a MODULE_UNLOAD property in its 
build file considered a module that does not support being unloaded? If so, why 
is EFI_LOADED_IMAGE_PROTOCOL.Unload not set to a dummy that returns an EFI 
error code?

For example, DxeDebugPrintErrorLevelLib installs a protocol interface in its 
CONSTRUCTOR function. When this library is included in a module without a 
MODULE_UNLOAD property and that module is unloaded, the 
DxeDebugPrintErrorLevelLib DESTRUCTOR function, which uninstalls the interface, 
is never called and hence the interface remains in the protocol database, 
despite it pointing to a memory location that is now free. If it is called, the 
behavior is obviously undefined.

Did I understand something incorrectly, are these modules not supposed to be 
unloadable or is this a bug?

Thank you,
Marvin.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] MdePkg/Cper.h: Update Firmware Error Record per UEFI 2.7

2017-06-14 Thread Hao Wu
This commit updates the Firmware Error Record related definitions
according to UEFI 2.7 spec Section N.2.10 Table 281:

a. Adds definitions for 2 Firmware Error Record types
b. Update the structure EFI_FIRMWARE_ERROR_DATA

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdePkg/Include/Guid/Cper.h | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Guid/Cper.h b/MdePkg/Include/Guid/Cper.h
index 88e3a5874f..5ddd4c715e 100644
--- a/MdePkg/Include/Guid/Cper.h
+++ b/MdePkg/Include/Guid/Cper.h
@@ -1,7 +1,7 @@
 /** @file
   GUIDs and definitions used for Common Platform Error Record.
 
-  Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -12,7 +12,7 @@
   WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 
   @par Revision Reference:
-  GUIDs defined in UEFI 2.6 Specification.
+  GUIDs defined in UEFI 2.7 Specification.
 
 **/
 
@@ -1088,16 +1088,21 @@ typedef struct {
 
 ///
 /// Identifies the type of firmware error record
-///
+///@{
 #define EFI_FIRMWARE_ERROR_TYPE_IPF_SAL  0x00
+#define EFI_FIRMWARE_ERROR_TYPE_SOC_TYPE10x01
+#define EFI_FIRMWARE_ERROR_TYPE_SOC_TYPE20x02
+///@}
 
 ///
 /// Firmware Error Record Section
 ///
 typedef struct {
   UINT8   ErrorType;
-  UINT8   Resv1[7];
+  UINT8   Revision;
+  UINT8   Resv1[6];
   UINT64  RecordId;
+  EFI_GUIDRecordIdGuid;
 } EFI_FIRMWARE_ERROR_DATA;
 
 ///
-- 
2.12.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] migration of OpenPlatformPkg to tianocore

2017-06-14 Thread Leif Lindholm
Hi Mike,

Yes, this way I am able to build all of my packages! Many thanks!

This also works with the simplified PACKAGES_PATH usage model I had
initially been trying for (just point to edk2-platforms and
edk2-non-osi top level directories).

I have pushed updated and slightly cleaned up versions to
devel-OpenPlatformPkg branches on edk2-platforms and edk2-non-osi.

I'm working on updated helper scripts in my uefi-tools repository and
some documentation for this, and will post a new thread when I have
this ready.

/
Leif

On Tue, Jun 13, 2017 at 11:13:08PM +, Kinney, Michael D wrote:
> Hi Leif,
> 
> I pulled the latest versions of the repos and I was able to complete
> a build with no errors and without using symbolic links.
> 
> The change I made is to set WORKSPACE to the directory immediately
> above the repos and set PACKAGES_PATH to directories that contain the
> packages required for the platform to build.
> 
> This is the same technique used to build QuarkPlatformPkg that uses
> content from both the edk2 and the edk2-non-osi repos.  See the
> Readme.md in the QuarkPlatformPkg with instructions for setting up
> the build env for Linux and Windows for an example.
> 
> https://github.com/tianocore/edk2/tree/master/QuarkPlatformPkg
> 
> I wrote a small shell script that I put in the directory above the
> repos to setup the build environment for the JunoPkg platform.
> 
> export GCC5_AARCH64_PREFIX=aarch64-linux-gnu-
> 
> export WORKSPACE=$PWD
> 
> export PACKAGES_PATH=\
> $WORKSPACE/edk2:\
> $WORKSPACE/edk2-platforms/Platform/ARM:\
> $WORKSPACE/edk2-non-osi
> 
> cd edk2
> make -C BaseTools
> . edksetup.sh BaseTools
> 
> build -a AARCH64 -t GCC5 -p JunoPkg/ArmJuno.dsc
> 
> === end of build log ===
> Generate Region at Offset 0x0
>Region Size = 0xF8000
>Region Name = FV
> 
> Generating FVMAIN_COMPACT FV
> 
> Generating FVMAIN FV
> 
> 
> 
> 
> 
> Generate Region at Offset 0x0
>Region Size = 0xF8000
>Region Name = FV
> 
> GUID cross reference file can be found at 
> /home/mdkinney/GitHub/tianocore/Build/ArmJuno/DEBUG_GCC5/FV/Guid.xref
> 
> FV Space Information
> FVMAIN_COMPACT [94%Full] 1015808 total, 956200 used, 59608 free
> FVMAIN [99%Full] 8237696 total, 8237688 used, 8 free
> 
> - Done -
> Build end time: 18:49:57, Jun.13 2017
> Build total time: 00:04:22
> 
> 
> 
> If you want to support building any of the platforms, then
> PACKAGES_PATH can be set as follows:
> 
> export PACKAGES_PATH=\
> $WORKSPACE/edk2:\
> $WORKSPACE/edk2-platforms/Silicon/Hisilicon:\
> $WORKSPACE/edk2-platforms/Silicon/AMD:\
> $WORKSPACE/edk2-platforms/Platform/Hisilicon:\
> $WORKSPACE/edk2-platforms/Platform/Marvell:\
> $WORKSPACE/edk2-platforms/Platform/ARM:\
> $WORKSPACE/edk2-non-osi/Silicon/Intel:\
> $WORKSPACE/edk2-non-osi/Silicon/AMD/Styx:
> 
> Best regards,
> 
> Mike
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Wednesday, June 7, 2017 7:58 AM
> To: Yao, Jiewen 
> Cc: edk2-devel@lists.01.org; Ryan Harkin ; Ard 
> Biesheuvel ; Chenhui Sun ; 
> Andrew Fish ; Alan Ott ; Richardson, 
> Brian ; Duran, Leo ; 
> haojian.zhu...@linaro.org; Linaro UEFI ; 
> Kinney, Michael D ; Heyi Guo 
> Subject: Re: [edk2] [RFC] migration of OpenPlatformPkg to tianocore
> 
> Hi all,
> 
> OK, so I have now updated both edk2-non-osi and edk2-platforms
> (devel-OpenPlatformPkg branches) with package renaming, and updating
> .dsc/.fdf files to . 
> 
> It appears the problems I'm facing are mainly caused by the GenFds
> stage not seeing a view consistent with the actual compilation stage.
> 
> To demonstrate, I build the Juno platform:
> $ . edksetup.sh
> $ make -C BaseTools
> $ PACKAGES_PATH=/work/git/edk2-platforms/Platform/ARM \
>GCC5_AARCH64_PREFIX=aarch64-linux-gnu- build -n 9 -a AARCH64 \
>-t GCC5 -p JunoPkg/ArmJuno.dsc -b RELEASE
> 
> This build fails with:
> ---
> Fd File Name:BL33_AP_UEFI
> 
> Generate Region at Offset 0x0
>Region Size = 0xF8000
>   Region Name = FV
> 
> Generating FVMAIN_COMPACT FV
> 
> Generating FVMAIN FV
> 
> 
> GenFds.py...
>  : error F003: Output file for RAW section could not be found for
>  JunoPkg/AcpiTables/AcpiTables.inf
> 
> 
> 
> build.py...
>  : error 7000: Failed to execute command
>GenFds -f /work/git/edk2-platforms/Platform/ARM/JunoPkg/ArmJuno.fdf
>--conf=/work/git/edk2/Conf -o
>/work/git/edk2/Build/ArmJuno/RELEASE_GCC5 -t GCC5 -b RELEASE -p
>/work/git/edk2-platforms/Platform/ARM/JunoPkg/ArmJuno.dsc -a
>AARCH64 -D "EFI_SOURCE=/work/git/edk2/EdkCompatibilityPkg" -D
>"EDK_SOURCE=/work/git/edk2/EdkCompatibilityPkg" -D
>"TOOL_CHAIN_TAG=GCC5" -D "TOOLCHAIN=GCC5" -D "TARGET=RELEASE" -D
>"FAMILY=GCC" -D "WORKSPACE=/work/git/edk2" -D
>"EDK_TOOLS_PATH=/work/git/edk2/BaseTools"

Re: [edk2] [PATCH v2] EmbeddedPkg/MmcDxe: Add alignment for ECSD data

2017-06-14 Thread Leif Lindholm
On Wed, Jun 14, 2017 at 10:50:02AM +0800, Jun Nie wrote:
> >> Sorry, just see your email because that thread is not highlighted for
> >> new email in gmail for unknown reason.
> >> I have concern that "UINT64 VENDOR_SPECIFIC_FIELD[8]" cannot secure
> >> the ECSD alignment because it is not the first member.
> >
> > It being the first member or not has no relevance.
> > By my count, it starts at offset 64, with all preceding entries being
> > UINT8.
> 
>   Maybe you are right. I had have concern that if preceding member
>   of ECSDData, CSDData, is not 8 bytes aligned in the tail, UINT8
>   RESERVED_1[] may start from non 8 bytes aligned address.

The alignment of any struct must match the highest alignment
requirement of its members. If you add a 64-bit field to ECSDData,
that increases the alignment requirements for the ECSDData struct to
64 bits, which increases the alignment requirements for the CARD_INFO
struct to 64 bits. The compiler will automatically insert padding as
required to maintain this.

> >> Changing the first member to "UINT64 RESERVED_1[2]" shall secure the
> >> alignment.
> >
> > Relying on a reserved field for setting alignment constraints risks
> > having to find an alternative method at some point in the future, so I
> > agree this is not a preferred solution.
> >
> >> But I preferred Pad method. It is more readable if all ECSD member
> >> are UINT8 type.
> >
> > I confess am not familiar enough with eMMC to make a clean call here,
> > but if real alignment constrains exist, using UINT8 is actively
> > misleading, not to mention incorrect. If there is no real alignment
> > constraint, this is not the code that needs fixing.
> 
> 512 bytes ECSD data may be read either by CPU or DMA via dedicated
>  eMMC command. DMA may require alignment for the structure and
> DMA on different SoC may require different alignment. Hikey DMA is
> OK with 4 bytes alignment, while ZTE SoC require 8 bytes alignment.
> And pad must not be added in the 512 bytes structure body by compiler.
> So all members in ECSD are defined as UINT8 is safe.

There is no magic to the padding, it is 100% predictable. So the only
safety added is the false sense of not having to consider details of
alignment requirements.

> >> It is also more clear to add alignment info in
> >> CARD_INFO, just before ECSD member.
> >
> > Why is it more clear to apply alignment constraints at point of use
> > instead of point of definition?
> 
> Adding alignment attribute in definition is best way for the
> constraint, but I do not know how to make all compilers happy, just
> like my version 1 patch. It is more or less obscure to change
> VENDOR_SPECIFIC_FIELD or RESERVED_1 to type UINT64.

The benefit of inserting UINT64 types is that this is exactly the
mechanism the C language provides for resolving this type of issue.
But there could be other reasons that would not be preferable. (Say,
if the VENDOR_SPECIFIC_FIELD is expected to hold a text string.)

> I confess that adding a pad before ECSD usage is compromised method. It
> serve as a reminder for developers too, and allow all members of ECSD are
> defined with UINT8.
> 
> Another method is to define ECSDData as a pointer and allocate
> buffer for it. Which method do you prefer?

That would certainly be a much cleaner solution.
It would also be more likely to remain usable if we come across future
controllers with even higher alignment requirements.

It would also let us resolve this issue without me having to fully
understand why the Buffer argument to MMC_READBLOCKDATA is explicitly
a UINT32 * :)

So - yes please, if you could do that change instead, I would be happy
to use that.

> > Is there any chance you can share the EFI_MMC_HOST_PROTOCOL
> > implementation that triggers this error?
> 
> DWMMC controller on ZTE/Sanchip SoC will read corrupted data with current
> DwMmc driver in edk2 96boards git repo. I just add several small patches to
> enable it on new SoC. I will send all these changes in near future. Or
> what detail information do you need? I can share it here.

If you could paste the link to the commit(s) on github, that would
satisfy my curiosity.

Best Regards,

Leif

> Jun
> 
> >
> > /
> > Leif
> >
> >> I do not get point of Andrew, maybe he share the same concern.
> >>
> >> Jun
> >>
> >> >
> >> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> >> Signed-off-by: Jun Nie 
> >> >> ---
> >> >>  EmbeddedPkg/Universal/MmcDxe/Mmc.h | 1 +
> >> >>  1 file changed, 1 insertion(+)
> >> >>
> >> >> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
> >> >> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> >> >> index 8a7d5a3..6e3ab17 100644
> >> >> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> >> >> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> >> >> @@ -319,6 +319,7 @@ typedef struct  {
> >> >>OCR   OCRData;
> >> >>CID   CIDData;
> >> >>CSD   CSDData;
> >> >> +  UINT64Pad;  // For 8 bytes alignment 
> >> >> of ECSDData
> >> >>

[edk2] [Patch 0/2] Add HTTP Boot Callback Protocol support.

2017-06-14 Thread Fu Siyuan
Cc: Ye Ting 
Cc: Wu Jiaxin 

Fu Siyuan (2):
  MdePkg: Add header file for HTTP Boot Callback protocol in UEFI 2.7.
  NetworkPkg/HttpBootDxe: Add HTTP Boot Callback protocol support.

 MdePkg/Include/Protocol/HttpBootCallback.h | 100 +
 MdePkg/MdePkg.dec  |   7 +-
 NetworkPkg/HttpBootDxe/HttpBootClient.c|  67 -
 NetworkPkg/HttpBootDxe/HttpBootClient.h|   4 +-
 NetworkPkg/HttpBootDxe/HttpBootDhcp4.c |  26 +++-
 NetworkPkg/HttpBootDxe/HttpBootDhcp6.c | 106 --
 NetworkPkg/HttpBootDxe/HttpBootDxe.h   |  14 ++
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf |   3 +-
 NetworkPkg/HttpBootDxe/HttpBootImpl.c  | 221 +++--
 NetworkPkg/HttpBootDxe/HttpBootImpl.h  |   2 +
 NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  29 
 NetworkPkg/HttpBootDxe/HttpBootSupport.h   |  34 +
 12 files changed, 551 insertions(+), 62 deletions(-)
 create mode 100644 MdePkg/Include/Protocol/HttpBootCallback.h

-- 
2.13.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch 1/2] MdePkg: Add header file for HTTP Boot Callback protocol in UEFI 2.7.

2017-06-14 Thread Fu Siyuan
Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdePkg/Include/Protocol/HttpBootCallback.h | 100 +
 MdePkg/MdePkg.dec  |   7 +-
 2 files changed, 105 insertions(+), 2 deletions(-)
 create mode 100644 MdePkg/Include/Protocol/HttpBootCallback.h

diff --git a/MdePkg/Include/Protocol/HttpBootCallback.h 
b/MdePkg/Include/Protocol/HttpBootCallback.h
new file mode 100644
index 00..7542b30e03
--- /dev/null
+++ b/MdePkg/Include/Protocol/HttpBootCallback.h
@@ -0,0 +1,100 @@
+/** @file
+  This file defines the EFI HTTP Boot Callback Protocol interface.
+
+  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution. The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  @par Revision Reference:
+  This Protocol is introduced in UEFI Specification 2.7
+
+**/
+
+#ifndef __EFI_HTTP_BOOT_CALLBACK_H__
+#define __EFI_HTTP_BOOT_CALLBACK_H__
+
+#define EFI_HTTP_BOOT_CALLBACK_PROTOCOL_GUID \
+  { \
+0xba23b311, 0x343d, 0x11e6, {0x91, 0x85, 0x58, 0x20, 0xb1, 0xd6, 0x52, 
0x99} \
+  }
+
+typedef struct _EFI_HTTP_BOOT_CALLBACK_PROTOCOL  
EFI_HTTP_BOOT_CALLBACK_PROTOCOL;
+
+///
+/// EFI_HTTP_BOOT_CALLBACK_DATA_TYPE
+///
+typedef enum {
+  ///
+  /// Data points to a DHCP4 packet which is about to transmit or has received.
+  ///
+  HttpBootDhcp4,
+  ///
+  /// Data points to a DHCP6 packet which is about to be transmit or has 
received.
+  ///
+  HttpBootDhcp6,
+  ///
+  /// Data points to an EFI_HTTP_MESSAGE structure, whichcontians a HTTP 
request message
+  /// to be transmitted.
+  ///
+  HttpBootHttpRequest,
+  ///
+  /// Data points to an EFI_HTTP_MESSAGE structure, which contians a received 
HTTP 
+  /// response message.
+  ///
+  HttpBootHttpResponse,
+  ///
+  /// Part of the entity body has been received from the HTTP server. Data 
points to the
+  /// buffer of the entity body data.
+  ///
+  HttpBootHttpEntityBody,
+  HttpBootTypeMax
+} EFI_HTTP_BOOT_CALLBACK_DATA_TYPE;
+
+/**
+  Callback function that is invoked when the HTTP Boot driver is about to 
transmit or has received a
+  packet.
+
+  This function is invoked when the HTTP Boot driver is about to transmit or 
has received packet.
+  Parameters DataType and Received specify the type of event and the format of 
the buffer pointed
+  to by Data. Due to the polling nature of UEFI device drivers, this callback 
function should not
+  execute for more than 5 ms.
+  The returned status code determines the behavior of the HTTP Boot driver.
+
+  @param[in]  ThisPointer to the 
EFI_HTTP_BOOT_CALLBACK_PROTOCOL instance.
+  @param[in]  DataTypeThe event that occurs in the current state.
+  @param[in]  ReceivedTRUE if the callback is being invoked due to 
a receive event.
+  FALSE if the callback is being invoked due 
to a transmit event.
+  @param[in]  DataLength  The length in bytes of the buffer pointed to 
by Data.
+  @param[in]  DataA pointer to the buffer of data, the data 
type is specified by
+  DataType.
+  
+  @retval EFI_SUCCESS Tells the HTTP Boot driver to continue the 
HTTP Boot process.
+  @retval EFI_ABORTED Tells the HTTP Boot driver to abort the 
current HTTP Boot process.
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EFI_HTTP_BOOT_CALLBACK) (
+  IN EFI_HTTP_BOOT_CALLBACK_PROTOCOL*This,
+  IN EFI_HTTP_BOOT_CALLBACK_DATA_TYPE   DataType,
+  IN BOOLEANReceived,
+  IN UINT32 DataLength,
+  IN VOID   *Data   OPTIONAL
+ );
+
+///
+/// EFI HTTP Boot Callback Protocol is invoked when the HTTP Boot driver is 
about to transmit or 
+/// has received a packet. The EFI HTTP Boot Callback Protocol must be 
installed on the same handle
+/// as the Load File Protocol for the HTTP Boot.
+///
+struct _EFI_HTTP_BOOT_CALLBACK_PROTOCOL {
+  EFI_HTTP_BOOT_CALLBACK Callback;
+};
+
+extern EFI_GUID gEfiHttpBootCallbackProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 7a7504b7a3..a55e8ce1d9 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -2,7 +2,7 @@
 # This Package provides all definitions, library classes and libraries 
instances.
 #
 # It also provides the definitions(including PPIs/PROTOCOLs/GUIDs) of
-# EFI1.10/UEFI2.6/PI1.4 and some Industry Standards.
+# EFI1.10/UEFI2.7/PI1.4 and some Industry Standards.
 #
 # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved

[edk2] [Patch 2/2] NetworkPkg/HttpBootDxe: Add HTTP Boot Callback protocol support.

2017-06-14 Thread Fu Siyuan
This patch updates the HTTP Boot driver to install a default HTTP Callback 
protocol
if the platform doesn't provide one. This callback implementation will print the
boot file download progress in percentage format.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootClient.c  |  67 +-
 NetworkPkg/HttpBootDxe/HttpBootClient.h  |   4 +-
 NetworkPkg/HttpBootDxe/HttpBootDhcp4.c   |  26 +++-
 NetworkPkg/HttpBootDxe/HttpBootDhcp6.c   | 106 +--
 NetworkPkg/HttpBootDxe/HttpBootDxe.h |  14 ++
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf   |   3 +-
 NetworkPkg/HttpBootDxe/HttpBootImpl.c| 221 +--
 NetworkPkg/HttpBootDxe/HttpBootImpl.h|   2 +
 NetworkPkg/HttpBootDxe/HttpBootSupport.c |  29 
 NetworkPkg/HttpBootDxe/HttpBootSupport.h |  34 +
 10 files changed, 446 insertions(+), 60 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c 
b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 99db3d5505..68f5a49bad 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -233,7 +233,6 @@ HttpBootDhcp4ExtractUriInfo (
   //
   // All boot informations are valid here.
   //
-  AsciiPrint ("\n  URI: %a", Private->BootFileUri);
 
   //
   // Update the device path to include the IP and boot URI information.
@@ -401,7 +400,7 @@ HttpBootDhcp6ExtractUriInfo (
   //
   // All boot informations are valid here.
   //
-  AsciiPrint ("\n  URI: %a", Private->BootFileUri);
+
   //
   // Update the device path to include the IP and boot URI information.
   //
@@ -452,6 +451,40 @@ HttpBootDiscoverBootInfo (
 }
 
 /**
+  HttpIo Callback function which will be invoked when specified 
HTTP_IO_CALLBACK_EVENT happened.
+
+  @param[in]EventType  Indicate the Event type that occurs in the 
current callback.
+  @param[in]MessageHTTP message which will be send to, or just 
received from HTTP server.
+  @param[in]ContextThe Callback Context pointer.
+  
+  @retval EFI_SUCCESS  Tells the HttpIo to continue the HTTP process.
+  @retval Others   Tells the HttpIo to abort the current HTTP 
process.
+**/
+EFI_STATUS
+EFIAPI
+HttpBootHttpIoCallback (
+  IN  HTTP_IO_CALLBACK_EVENTEventType,
+  IN  EFI_HTTP_MESSAGE  *Message,
+  IN  VOID  *Context
+  )
+{
+  HTTP_BOOT_PRIVATE_DATA   *Private;
+  EFI_STATUS   Status;
+  Private = (HTTP_BOOT_PRIVATE_DATA *) Context;
+  if (Private->HttpBootCallback != NULL) {
+Status = Private->HttpBootCallback->Callback (
+   Private->HttpBootCallback,
+   EventType == HttpIoRequest ? HttpBootHttpRequest : 
HttpBootHttpResponse,
+   EventType == HttpIoRequest ? FALSE : TRUE,
+   sizeof (EFI_HTTP_MESSAGE),
+   (VOID *) Message
+   );
+return Status;
+  }
+  return EFI_SUCCESS;
+}
+
+/**
   Create a HttpIo instance for the file download.
 
   @param[in]PrivateThe pointer to the driver's private data.
@@ -490,6 +523,8 @@ HttpBootCreateHttpIo (
  Private->Controller,
  Private->UsingIpv6 ? IP_VERSION_6 : IP_VERSION_4,
  &ConfigData,
+ HttpBootHttpIoCallback,
+ (VOID *) Private,
  &Private->HttpIo
  );
   if (EFI_ERROR (Status)) {
@@ -686,6 +721,8 @@ HttpBootGetBootFileCallback (
 {
   HTTP_BOOT_CALLBACK_DATA  *CallbackData;
   HTTP_BOOT_ENTITY_DATA*NewEntityData;
+  EFI_STATUS   Status;
+  EFI_HTTP_BOOT_CALLBACK_PROTOCOL   *HttpBootCallback;
 
   //
   // We only care about the entity data.
@@ -695,6 +732,19 @@ HttpBootGetBootFileCallback (
   }
 
   CallbackData = (HTTP_BOOT_CALLBACK_DATA *) Context;
+  HttpBootCallback = CallbackData->Private->HttpBootCallback;
+  if (HttpBootCallback != NULL) {
+Status = HttpBootCallback->Callback (
+   HttpBootCallback,
+   HttpBootHttpEntityBody,
+   TRUE,
+   (UINT32)Length,
+   Data
+   );
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+  }
   //
   // Copy data if caller has provided a buffer.
   //
@@ -977,6 +1027,7 @@ HttpBootGetBootFile (
   Context.Buffer = Buffer;
   Context.BufferSize = *BufferSize;
   Context.Cache  = Cache;
+  Context.Private= Private;
   Status = HttpInitMsgParser (
  HeaderOnly? HttpMethodHead : HttpMethodGet,
  ResponseData->Response.StatusCode,
@@ -1032,6 +1083,18 @@ HttpBootGetBootFile (
   goto ERROR_6;
 }
 ReceivedSize += ResponseBody.BodyLength;
+if (Private->HttpBootCallback != NULL) {
+  Status = Private->HttpBootCallback->Callback (
+ Private->HttpBootCallback,
+ HttpBootHttpEntityBody,
+ TRUE,
+ (UINT3

Re: [edk2] [Patch 1/2] BaseTools: add /Gw to CC_FLAGS for VS2013 and higher tool chain tags

2017-06-14 Thread Zhu, Yonghong
Hi Mike,

Thanks. I only checked it build pass, but missed "iec: Command line warning: 
ignoring unknown option '-Gw'" in the log.
I sent out a V2 that not add /Gw for EBC.

Best Regards,
Zhu Yonghong


-Original Message-
From: Kinney, Michael D 
Sent: Wednesday, June 14, 2017 2:51 PM
To: Zhu, Yonghong ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: RE: [edk2] [Patch 1/2] BaseTools: add /Gw to CC_FLAGS for VS2013 and 
higher tool chain tags

Yonghong,

I do not think the EBC compiler supports /Gw.  Did you test that?

Mike

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Tuesday, June 13, 2017 9:47 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [edk2] [Patch 1/2] BaseTools: add /Gw to CC_FLAGS for VS2013 and 
higher tool chain tags

The /Gw flag does a better job at size optimization than use of the 
GLOBAL_REMOVE_IF_UNREFERENCED macro that is currently used for VS20xx tool 
chains to remove unreferenced global variables.
This patch add /Gw to CC_FLAGS for VS2013 and higher tool chain tags.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Conf/tools_def.template | 110 +++---
 1 file changed, 55 insertions(+), 55 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 04a1bcb..aa7ff2f 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3167,13 +3167,13 @@ NOOPT_VS2012x86xASL_X64_DLINK_FLAGS= /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT
 *_VS2013_IA32_ASLCC_PATH  = DEF(VS2013_BIN)\cl.exe
 *_VS2013_IA32_ASLPP_PATH  = DEF(VS2013_BIN)\cl.exe
 *_VS2013_IA32_ASLDLINK_PATH   = DEF(VS2013_BIN)\link.exe
 
   *_VS2013_IA32_MAKE_FLAGS= /nologo
-  DEBUG_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm
-RELEASE_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od
+  DEBUG_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw
+RELEASE_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Gw
 
   DEBUG_VS2013_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 RELEASE_VS2013_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd
 NOOPT_VS2013_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 
@@ -3199,13 +3199,13 @@ NOOPT_VS2013_IA32_DLINK_FLAGS = /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT:REF
 *_VS2013_X64_DLINK_PATH= DEF(VS2013_BINX64)\link.exe
 *_VS2013_X64_ASLCC_PATH= DEF(VS2013_BINX64)\cl.exe
 *_VS2013_X64_ASLPP_PATH= DEF(VS2013_BINX64)\cl.exe
 *_VS2013_X64_ASLDLINK_PATH = DEF(VS2013_BINX64)\link.exe
 
-  DEBUG_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm
-RELEASE_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013_X64_CC_FLAGS   = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Od
+  DEBUG_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Gw
+RELEASE_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013_X64_CC_FLAGS   = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Od /Gw
 
   DEBUG_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd /Zi
 RELEASE_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd
 NOOPT_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd /Zi
 
@@ -3230,11 +3230,11 @@ NOOPT_VS2013_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF /OPT
 *_VS2013_EBC_SLINK_PATH  = DEF(VS2013_BIN)\link.exe
 *_VS2013_EBC_DLINK_PATH  = DEF(VS2013_BIN)\link.exe
 
 *_VS2013_EBC_MAKE_FLAGS  = /nologo
 *_VS2013_EBC_PP_FLAGS= /nologo /E /TC /FIAutoGen.h
-*_VS2013_EBC_CC_FLAGS= /nologo /c /WX /W3 /FIAutoGen.h 
/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT)
+*_VS2013_EBC_CC_FLAGS= /nologo /c /WX /W3 /FIAutoGen.h 
/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT) /Gw
 *_VS2013_EBC_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE 
/FI$(MODULE_NAME)StrDefs.h
 *_VS2013_EBC_SLINK_FLAGS = /lib /NOLOGO /MACHINE:EBC
 *_VS2013_EBC_DLINK_FLAGS

[edk2] [Patch 1/2 V2] BaseTools: add /Gw to CC_FLAGS for VS2013 and higher tool chain tags

2017-06-14 Thread Yonghong Zhu
The /Gw flag does a better job at size optimization than use of the
GLOBAL_REMOVE_IF_UNREFERENCED macro that is currently used for VS20xx
tool chains to remove unreferenced global variables.
This patch add /Gw to CC_FLAGS for VS2013 and higher tool chain tags.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Conf/tools_def.template | 96 +++
 1 file changed, 48 insertions(+), 48 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 04a1bcb..6f73bd7 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -3167,13 +3167,13 @@ NOOPT_VS2012x86xASL_X64_DLINK_FLAGS= /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT
 *_VS2013_IA32_ASLCC_PATH  = DEF(VS2013_BIN)\cl.exe
 *_VS2013_IA32_ASLPP_PATH  = DEF(VS2013_BIN)\cl.exe
 *_VS2013_IA32_ASLDLINK_PATH   = DEF(VS2013_BIN)\link.exe
 
   *_VS2013_IA32_MAKE_FLAGS= /nologo
-  DEBUG_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm
-RELEASE_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od
+  DEBUG_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw
+RELEASE_VS2013_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Gw
 
   DEBUG_VS2013_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 RELEASE_VS2013_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd
 NOOPT_VS2013_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 
@@ -3199,13 +3199,13 @@ NOOPT_VS2013_IA32_DLINK_FLAGS = /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT:REF
 *_VS2013_X64_DLINK_PATH= DEF(VS2013_BINX64)\link.exe
 *_VS2013_X64_ASLCC_PATH= DEF(VS2013_BINX64)\cl.exe
 *_VS2013_X64_ASLPP_PATH= DEF(VS2013_BINX64)\cl.exe
 *_VS2013_X64_ASLDLINK_PATH = DEF(VS2013_BINX64)\link.exe
 
-  DEBUG_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm
-RELEASE_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013_X64_CC_FLAGS   = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Od
+  DEBUG_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Gw
+RELEASE_VS2013_X64_CC_FLAGS = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/O1b2s /GL /Gy /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013_X64_CC_FLAGS   = /nologo /c /WX /GS- /W4 /Gs32768 /D UNICODE 
/Gy /FIAutoGen.h /EHs-c- /GR- /GF /Zi /Gm /Od /Gw
 
   DEBUG_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd /Zi
 RELEASE_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd
 NOOPT_VS2013_X64_ASM_FLAGS= /nologo /c /WX /W3 /Cx /Zd /Zi
 
@@ -3285,13 +3285,13 @@ NOOPT_VS2013_X64_DLINK_FLAGS  = /NOLOGO /NODEFAULTLIB 
/IGNORE:4001 /OPT:REF /OPT
 *_VS2013xASL_IA32_ASLCC_PATH= DEF(VS2013_BIN)\cl.exe
 *_VS2013xASL_IA32_ASLPP_PATH= DEF(VS2013_BIN)\cl.exe
 *_VS2013xASL_IA32_ASLDLINK_PATH = DEF(VS2013_BIN)\link.exe
 
   *_VS2013xASL_IA32_MAKE_FLAGS  = /nologo
-  DEBUG_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm
-RELEASE_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF
-NOOPT_VS2013xASL_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od
+  DEBUG_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Gw
+RELEASE_VS2013xASL_IA32_CC_FLAGS= /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw
+NOOPT_VS2013xASL_IA32_CC_FLAGS  = /nologo /arch:IA32 /c /WX /GS- /W4 
/Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Zi /Gm /Od /Gw
 
   DEBUG_VS2013xASL_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 RELEASE_VS2013xASL_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /Cx /coff /Zd
 NOOPT_VS2013xASL_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd /Zi
 
@@ -3317,13 +3317,13 @@ NOOPT_VS2013xASL_IA32_DLINK_FLAGS   = /NOLOGO 
/NODEFAULTLIB /IGNORE:4001 /OPT:RE
 *_VS2013xASL_X64_DLINK_PATH= DEF(VS2013_BINX64)\link.exe
 *_VS20

Re: [edk2] HTTP Boot failed to download NBP file if it is .iso type

2017-06-14 Thread Gary Lin
On Tue, Jun 13, 2017 at 09:31:49AM +, Santhapur Naveen wrote:
> Hi Siyuan,
> 
> Thank you for your reply.
> And regarding the OS installation, we are able to download SUSE iso (>3 GB) 
> from the HTTP server. But the install didn't happen.
> May I ask you what could be possible reason? Is there anything else I've had 
> missed, please let me know.
Which version of SLE are you using?

SUSE Linux Enterprise supports EFI_RAM_DISK_PROTOCOL since 12 SP2, so
any image older than 12 SP2 would not work. Per my test, as long as the
firmware loads the iso into memory, the iso works as an additional
CD-ROM, and the firmware can boot into the iso directly.

Cheers,

Gary Lin

> 
> Regards,
> Naveen
> 
> -Original Message-
> From: Fu, Siyuan [mailto:siyuan...@intel.com] 
> Sent: Tuesday, June 13, 2017 8:17 AM
> To: Santhapur Naveen; Karunakar P; edk2-devel@lists.01.org
> Subject: RE: HTTP Boot failed to download NBP file if it is .iso type
> 
> Hi, Karunakar and Naveen
> 
> Status 0023 is EFI_HTTP_ERROR, which means the HTTP server replied an 
> HTTP error. The HTTP error code is placed in 
> HttpIo->RspToken.Message->Data.Response->StatusCode, and will be displayed in 
> HttpBootPrintErrorMessage() function.
> 
> If a downloaded NBP is a RAM disk image, the BDS will try to find a boot file 
> inside it according UEFI 2.7 section 3.5.1 "Boot via the Simple File 
> Protocol".
> 
> Best Regards,
> Siyuan
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Santhapur Naveen
> Sent: Saturday, June 10, 2017 1:49 AM
> To: Karunakar P; edk2-devel@lists.01.org
> Subject: Re: [edk2] HTTP Boot failed to download NBP file if it is .iso type
> 
> Even if we are able to download an ISO file successfully, how will 
> EFI_RAM_DISK_PROTOCOL comes to know what is the efi that needs to be used?
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Karunakar P
> Sent: Friday, June 09, 2017 9:04 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] HTTP Boot failed to download NBP file if it is .iso type
> 
> Hi All,
> 
> We have facing an issue with HTTP boot.
> 
> [Issue]
> HTTP Boot failed to download NBP file if it is an .iso type
> 
> [Reproduction Steps]
> 
> 1.  Configure HTTP Server in Ubuntu environment.
> 
> 2.  Place any iso image as NBP file.
> 
> 3.  Perform UEFI HTTPv4 boot.
> 
> [Result]
> DHCP process was success, But Failed to download NBP file.
> 
> [Observations]
> 
> 1.  As per UEFI spec "23.7.1 Boot from URL" (UEFI 2.6, page 1222).
> 
> Here is what the section says about binary image returned by HTTP server:
> 
> "...the binary image [..] is a UEFI-formatted executable[...], or it could be 
> mounted as a RAM disk which contains a UEFI-compliant file system (see 
> Section 12.3)."
> 
> We're interested in exploring second scenario, when downloaded image is a 
> UEFI-compliant file system.
> 
> Section "23.7.3.1 Device Path" on page 1226 provides examples of image URL: 
> http://192.168.1.100/boot.iso
> 
> The specification also says that "the HTTP Boot driver will register RAM disk 
> with the downloaded NBP, by appending a RamDisk device node to the device 
> path above, like...".
> 
> 
> 
> HttpBootDxe is doing this.But NBP file itself failing to download in the case 
> of iso image.
> 
> 
> 
> 2.  HTTP boot fails in the following function
> 
> 
> 
> HttpBootGetBootFile() ->
> 
> 
> 
> EFI_STATUS
> 
> HttpBootGetBootFile (
> 
>   IN HTTP_BOOT_PRIVATE_DATA   *Private,
> 
>   IN BOOLEAN  HeaderOnly,
> 
>   IN OUT UINTN*BufferSize,
> 
>  OUT UINT8*Buffer,
> 
>  OUT HTTP_BOOT_IMAGE_TYPE *ImageType
> 
>   )
> 
> {
> 
> .
> 
> .
> 
> .
> 
>  Status = HttpIoRecvResponse (
> 
>  &Private->HttpIo,
> 
>  TRUE,
> 
>  ResponseData
> 
>  );
> 
> // Here the Status value is Success and ResponseData->Status = 0023
> 
> 
> 
>   if (EFI_ERROR (Status) || EFI_ERROR (ResponseData->Status)) {
> 
> if (EFI_ERROR (ResponseData->Status)) {
> 
>   StatusCode = HttpIo->RspToken.Message->Data.Response->StatusCode;
> 
>   HttpBootPrintErrorMessage (StatusCode);
> 
>   Status = ResponseData->Status;
> 
> // Here Status = 0023
> 
> 
> 
> }
> 
> goto ERROR_5;// goto ERROR_5
> 
>   }
> 
> .
> 
> .
> 
> .
> 
> }
> 
> 
> 
> Note:
> We have HTTP server configured in Ubuntu Environment.
> 
> Could you please look into it.
> 
> 
> Thanks,
> karunakar
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> This e-mail is intended for the use of the addressee only and may contain 
> privileged, confidential, or proprietary information that is exempt from 
> disclosure under law. If you have received this message in error, please 
> inform us promptly by reply e-mail, th