Re: [edk2] [PATCH 1/2] BaseTools ARM AARCH64: pass CC flags to linker for XIP modules as well

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, August 11, 2016 7:14 PM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; Gao, Liming
> ; Zhu, Yonghong 
> Cc: Ard Biesheuvel 
> Subject: [edk2] [PATCH 1/2] BaseTools ARM AARCH64: pass CC flags to linker
> for XIP modules as well
> 
> Commit 478f50990a ("BaseTools GCC: add the compiler flags to the linker
> command line") added the compiler flags to the linker command line,
> which is required for LTO to function correctly, since it involves code
> generation at link time.
> 
> This patch failed to update the build rules for XIP modules on AARCH64,
> which not only requires the ordinary CC flags but also the XIP CC flags
> to prevent the LTO backend to, e.g., emit code that does not adhere to
> the strict alignment rules we impose for code that may execute with the
> MMU off.
> 
> So update the XIP link rules as well. Since AARCH64 and ARM are not
> supported by any toolchains in the GCCLD build rule family, drop the
> reference to GCCLD while we're at it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Conf/build_rule.template | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Conf/build_rule.template
> b/BaseTools/Conf/build_rule.template
> index 6191957e0e70..7e2c6a96583d 100755
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -310,7 +310,7 @@
>  "$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH) -filelist
> $(STATIC_LIBRARY_FILES_LIST)  $(DLINK2_FLAGS)
> 
> 
> -[Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64,
> Static-Library-File.PEIM.AARCH64]
> +[Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64,
> Static-Library-File.PEIM.AARCH64,Static-Library-File.SEC.ARM, Static-Library-
> File.PEI_CORE.ARM, Static-Library-File.PEIM.ARM]
>  
>  *.lib
> 
> @@ -320,8 +320,8 @@
>  
>  $(DEBUG_DIR)(+)$(MODULE_NAME).dll
> 
> -
> -"$(DLINK)" -o ${dst} $(DLINK_FLAGS) $(DLINK_XIPFLAGS) -Wl,--start-
> group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(DLINK2_FLAGS)
> +
> +"$(DLINK)" -o ${dst} $(DLINK_FLAGS) $(DLINK_XIPFLAGS) -Wl,--start-
> group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS)
> $(CC_XIPFLAGS) $(DLINK2_FLAGS)
>  "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
> 
> 
> --
> 2.7.4
> 
> ___
> 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 2/2] BaseTools ARM AARCH64: drop redundant compiler arguments

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, August 11, 2016 7:14 PM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; Gao, Liming
> ; Zhu, Yonghong 
> Cc: Ard Biesheuvel 
> Subject: [edk2] [PATCH 2/2] BaseTools ARM AARCH64: drop redundant
> compiler arguments
> 
> The ARM and AARCH64 CC_FLAGS definitions include both
> GCC_ALL_CC_FLAGS
> and GCC44_ALL_CC_FLAGS, resulting in many of the compiler arguments
> being passed twice. Since the CLANG35 definitions do not refer to
> GCC44_ALL_CC_FLAGS, drop the reference for GCCx as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Conf/tools_def.template | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Conf/tools_def.template
> b/BaseTools/Conf/tools_def.template
> index 2002c4c0598b..d6d3ed380668 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -4400,7 +4400,7 @@ DEFINE GCC46_X64_DLINK_FLAGS =
> DEF(GCC45_X64_DLINK_FLAGS)
>  DEFINE GCC46_X64_DLINK2_FLAGS= DEF(GCC45_X64_DLINK2_FLAGS)
>  DEFINE GCC46_ASM_FLAGS   = DEF(GCC45_ASM_FLAGS)
>  DEFINE GCC46_ARM_ASM_FLAGS   = $(ARCHASM_FLAGS)
> $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian
> -DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS)
> $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS)
> DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -save-
> temps
> +DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS)
> $(PLATFORM_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-
> relocations -save-temps
>  DEFINE GCC46_ARM_CC_XIPFLAGS = -
> D__ARM_FEATURE_UNALIGNED=0
>  DEFINE GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) -
> Wl,--oformat=elf32-littlearm
>  DEFINE GCC46_ARM_DLINK2_FLAGS=
> DEF(GCC_DLINK2_FLAGS_COMMON) -Wl,--
> defsym=PECOFF_HEADER_SIZE=0x220
> @@ -4419,7 +4419,7 @@ DEFINE GCC47_ARM_ASM_FLAGS   =
> DEF(GCC46_ARM_ASM_FLAGS)
>  DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS)
> $(PLATFORM_FLAGS) DEF(GCC_ASM_FLAGS) -mlittle-endian
>  DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS)
>  DEFINE GCC47_ARM_CC_XIPFLAGS = DEF(GCC_ARM_CC_XIPFLAGS)
> -DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS)
> $(PLATFORM_FLAGS) DEF(GCC44_ALL_CC_FLAGS) -mcmodel=large
> DEF(GCC_AARCH64_CC_FLAGS) -save-temps
> +DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS)
> $(PLATFORM_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -
> save-temps
>  DEFINE GCC47_AARCH64_CC_XIPFLAGS =
> DEF(GCC_AARCH64_CC_XIPFLAGS)
>  DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
>  DEFINE GCC47_ARM_DLINK2_FLAGS= DEF(GCC46_ARM_DLINK2_FLAGS)
> --
> 2.7.4
> 
> ___
> 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 3/3] MdePkg RVCT: add definition of UNREACHABLE

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Friday, August 12, 2016 2:21 AM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; eug...@hp.com; Gao,
> Liming 
> Cc: Ard Biesheuvel 
> Subject: [edk2] [PATCH 3/3] MdePkg RVCT: add definition of UNREACHABLE
> 
> The RVCT compiler in --gnu mode appears to simply strip of the __builtin
> prefix when it encounters calls to __builtin_xxx() functions, and so
> the __builtin_unreachable() we emit for GCC results in linker errors
> regarding undefined references against 'unreachable()'.
> 
> So define UNREACHABLE() to a NOP instead.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdePkg/Include/Arm/ProcessorBind.h | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MdePkg/Include/Arm/ProcessorBind.h
> b/MdePkg/Include/Arm/ProcessorBind.h
> index c2482c2f50f0..5ee7465c05a3 100644
> --- a/MdePkg/Include/Arm/ProcessorBind.h
> +++ b/MdePkg/Include/Arm/ProcessorBind.h
> @@ -28,6 +28,13 @@
>  #pragma pack()
>  #endif
> 
> +//
> +// RVCT does not support the __builtin_unreachable() macro
> +//
> +#ifdef __ARMCC_VERSION
> +#define UNREACHABLE()
> +#endif
> +
>  #if _MSC_EXTENSIONS
>//
>// use Microsoft* C complier dependent integer width types
> --
> 2.7.4
> 
> ___
> 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 2/3] BaseTools RVCT: ignore various RVC diagnostics

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Friday, August 12, 2016 2:21 AM
> To: edk2-devel@lists.01.org; leif.lindh...@linaro.org; eug...@hp.com; Gao,
> Liming 
> Cc: Ard Biesheuvel 
> Subject: [PATCH 2/3] BaseTools RVCT: ignore various RVC diagnostics
> 
> This updates the RVCT CC flags so various diagnostics that trigger
> warnings-as-errors are silenced. In particular, RVCT complains about
> missing newlines at the end of source files, mixing of enums and int
> values and return statements followed by a break, all of which occur
> in the Tianocore codebase, but none of which are actual errors in the
> code. So just silence them.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Conf/tools_def.template | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/BaseTools/Conf/tools_def.template
> b/BaseTools/Conf/tools_def.template
> index 2002c4c0598b..e3d70133caaa 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -7449,7 +7449,7 @@ RELEASE_XCODE5_X64_CC_FLAGS   = -target
> x86_64-pc-win32-macho -c-Os   -W
> 
> ##
> ##
> 
>  DEFINE RVCT_ALL_ASM_FLAGS   = --diag_suppress=1786 --
> diag_error=warning --apcs /interwork
> -DEFINE RVCT_ALL_CC_FLAGS= --c90 --no_autoinline --asm --gnu --apcs
> /interwork --signed_chars --no_unaligned_access --split_sections --
> enum_is_int --preinclude AutoGen.h --diag_suppress=186 --diag_warning
> 167 --diag_error=warning --diag_style=ide --protect_stack
> +DEFINE RVCT_ALL_CC_FLAGS= --c90 --no_autoinline --asm --gnu --apcs
> /interwork --signed_chars --no_unaligned_access --split_sections --
> enum_is_int --preinclude AutoGen.h --diag_suppress=186,188,1,111,68 --
> diag_warning 167 --diag_error=warning --diag_style=ide --protect_stack
>  DEFINE RVCT_ALL_DLINK_FLAGS = --no_scanlib --no_exceptions --
> datacompressor off --strict --symbols --diag_style=ide --no_legacyalign --
> scatter $(EDK_TOOLS_PATH)/Scripts/Rvct-Align32.sct
> 
> 
> ##
> ##
> --
> 2.7.4

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

[edk2] [Patch] MdePkg: Fix guid conflict.

[Eric Dong]

Update Image Decoder Protocol GUID value to fix GUID
conflict with EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_GUID.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
Cc: Liming Gao 
Cc: Cecil Sheng 
---
 MdePkg/Include/Protocol/ImageDecoder.h | 10 +-
 MdePkg/MdePkg.dec  | 14 +++---
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Protocol/ImageDecoder.h 
b/MdePkg/Include/Protocol/ImageDecoder.h
index f1985bc..aebb813 100644
--- a/MdePkg/Include/Protocol/ImageDecoder.h
+++ b/MdePkg/Include/Protocol/ImageDecoder.h
@@ -18,8 +18,16 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 
+//
+// In UEFI 2.6 spec,this guid value is duplicate with
+// EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_GUID. Now update this guid value to
+// avoid the duplicate guid issue. So its value is not consistent with
+// UEFI spec definition now. We have proposed to update UEFI spec to
+// use this new guid. After new spec released, we will remove this
+// comments.
+//
 #define EFI_HII_IMAGE_DECODER_PROTOCOL_GUID \
-  { 0x2f707ebb, 0x4a1a, 0x11d4, {0x9a,0x38,0x00,0x90,0x27,0x3f,0xc1,0x4d}}
+  {0x9e66f251, 0x727c, 0x418c, { 0xbf, 0xd6, 0xc2, 0xb4, 0x25, 0x28, 0x18, 
0xea }}
 
 
 #define EFI_HII_IMAGE_DECODER_NAME_JPEG_GUID \
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 458d568..606e2f1 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -393,7 +393,7 @@
 
   ## Include/Guid/MemoryOverwriteControl.h
   gEfiMemoryOverwriteControlDataGuid = { 0xe20939be, 0x32d4, 0x41be, {0xa1, 
0x50, 0x89, 0x7f, 0x85, 0xd4, 0x98, 0x29 }}
-  
+
   ## Include/IndustryStandard/MemoryOverwriteRequestControlLock.h
   gEfiMemoryOverwriteRequestControlLockGuid = { 0xBB983CCF, 0x151D, 0x40E1, 
{0xA0, 0x7B, 0x4A, 0x17, 0xBE, 0x16, 0x82, 0x92}}
 
@@ -1384,7 +1384,7 @@
 
   ## Include/Protocol/TrEEProtocol.h
   gEfiTrEEProtocolGuid   = {0x607f766c, 0x7455, 0x42be, { 0x93, 0x0b, 
0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f }}
-  
+
   ## Include/Protocol/Tcg2Protocol.h
   gEfiTcg2ProtocolGuid   = {0x607f766c, 0x7455, 0x42be, { 0x93, 0x0b, 
0xe4, 0xd7, 0x6d, 0xb2, 0x72, 0x0f }}
   gEfiTcg2FinalEventsTableGuid   = {0x1e2ed096, 0x30e2, 0x4254, { 0xbd, 0x89, 
0x86, 0x3b, 0xbe, 0xf8, 0x23, 0x25 }}
@@ -1620,7 +1620,15 @@
   gEfiRamDiskProtocolGuid  = { 0xab38a0df, 0x6873, 0x44a9, { 0x87, 
0xe6, 0xd4, 0xeb, 0x56, 0x14, 0x84, 0x49 }}
 
   ## Include/Protocol/ImageDecoder.h
-  gEfiHiiImageDecoderProtocolGuid  = { 0x2f707ebb, 0x4a1a, 0x11d4, { 0x9a, 
0x38, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d }}
+  ##
+  ## In UEFI 2.6 spec,this guid value is duplicate with
+  ## EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_GUID. Now update this guid value to
+  ## avoid the duplicate guid issue. So its value is not consistent with
+  ## UEFI spec definition now. We have proposed to update UEFI spec to
+  ## use this new guid. After new spec released, we will remove this
+  ## comments.
+  ##
+  gEfiHiiImageDecoderProtocolGuid  = { 0x9e66f251, 0x727c, 0x418c, { 0xbf, 
0xd6, 0xc2, 0xb4, 0x25, 0x28, 0x18, 0xea }}
 
   ## Include/Protocol/HiiImageEx.h
   gEfiHiiImageExProtocolGuid   = { 0x1a1241e6, 0x8f19, 0x41a9, { 0xbc, 
0xe,  0xe8, 0xef, 0x39, 0xe0, 0x65, 0x46 }}
-- 
2.6.4.windows.1

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

Re: [edk2] DHCP Automatic Configure at Driver Connect

[Wu, Jiaxin]

> In our situation DHCP is being set on a previous boot and we are now
> observing DHCP being attempted on every boot (since the controllers are
> connected).  So this is consistent with the behavior you describe - even
> though the default was originally static/manual the fact that we configured
> DHCP once causes this to be stored to NVRAM and perform dhcp process at
> every boot.

Yes, that's because you have changed the policy to DHCP.

> 
> The issue is not just a performance issue around DHCP timing.  Even with a
> static IP address set the fact that the network interface comes "up" at each
> boot is problematic because our requirement is only to enable the network
> interface when directed to by an application.

In my memory, we only talked about the performance issue before. Here, I 
understand your requirement: 1) The network interface should be in 
*un-configured* status at each boot time until the third part configuration; 2) 
the policy setting should be ignored, right? I don't think it's reasonable. I 
know the old Ip4Config did as you said here, but currently, the IPv4 default 
policy concept is new enrolled by Ip4Config2 protocol, the device policy should 
be in one state no matter static or DHCP. 

> 
> Modifying the IP configuration dynamically to suppress the interface coming
> up at every boot is also problematic because it means we would need to store
> the IP address configuration in another NVRAM location.  In other words, the
> combining of the IP address settings storage *and* the policy of whether to
> configure at boot or wait until explicitly requested is problematic - we 
> really
> would like to control these settings independently.  Is that possible within 
> the
> scope of the spec?  Could we just have a PCD that suppresses the automatic
> configure at boot under any circumstance?

Actually, if you want to recover the Ipv4 setting to the *un-configured* status 
(Note here: policy should be always set, *un-configured* means no IP address 
configuration), Ip4Config2 provide the ability to change the default policy. As 
git commit version SHA-1: 7648748e99eeeadec38fda7568adb260c4acc861 described, 
"This update let the other platform drivers have chance to change the default 
config data by consume Ip4Config2Protocol. "  So, you don't need to set another 
NVRAM location to do that, that means additional DXE_DRIVER can add in your 
platform ahead to change the default config data. The only operation in this 
DXE_DRIVER is to register a notify for Ip4Config2 protocol to change the 
default policy. That's also the reason why we don't want to enroll PCD to 
change it, you know two interface to do the same thing is not a good idea.  


Thanks,
Jiaxin


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Cohen, Eugene
> Sent: Thursday, August 11, 2016 10:23 PM
> To: Wu, Jiaxin ; Ye, Ting ; edk2-
> de...@lists.01.org
> Subject: Re: [edk2] DHCP Automatic Configure at Driver Connect
> 
> Ting and Jiaxin, thank you both for clarifying.
> 
> In our situation DHCP is being set on a previous boot and we are now
> observing DHCP being attempted on every boot (since the controllers are
> connected).  So this is consistent with the behavior you describe - even
> though the default was originally static/manual the fact that we configured
> DHCP once causes this to be stored to NVRAM and perform dhcp process at
> every boot.
> 
> The issue is not just a performance issue around DHCP timing.  Even with a
> static IP address set the fact that the network interface comes "up" at each
> boot is problematic because our requirement is only to enable the network
> interface when directed to by an application.
> 
> Modifying the IP configuration dynamically to suppress the interface coming
> up at every boot is also problematic because it means we would need to store
> the IP address configuration in another NVRAM location.  In other words, the
> combining of the IP address settings storage *and* the policy of whether to
> configure at boot or wait until explicitly requested is problematic - we 
> really
> would like to control these settings independently.  Is that possible within 
> the
> scope of the spec?  Could we just have a PCD that suppresses the automatic
> configure at boot under any circumstance?
> 
> Thanks,
> 
> Eugene
> 
> > -Original Message-
> > From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> > Sent: Thursday, August 11, 2016 12:00 AM
> > To: Ye, Ting ; Cohen, Eugene ;
> > edk2-devel@lists.01.org
> > Subject: RE: DHCP Automatic Configure at Driver Connect
> >
> > Thanks Ting's more background clarification.
> >
> > I assume the difference you mentioned is between "SHA-1:
> > 3d0a49ad47619c30c84bbee8a33f54b64dddbcec" and "SHA-1:
> > 7648748e99eeeadec38fda7568adb260c4acc861". The two commits does
> cause
> > the different behavior as Ting said below. Git 

Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency

[Zeng, Star]

On 2016/8/11 21:41, Brian J. Johnson wrote:

On 08/10/2016 09:37 PM, Star Zeng wrote:

Minimize the code overhead between the two TSC reads by adding
new internal API to calculate TSC Frequency instead of reusing
MicroSecondDelay ().


Why not just use the MSR_PLATFORM_INFO (0xce) register bits 15..8, on
CPUs where it's available, to read the TSC frequency directly?


Per my understanding, not every generation CPU has this 
MSR_PLATFORM_INFO (0xce) register defined, and also MSR_PLATFORM_INFO 
(0xce) register bits 15..8 is just the ratio, then the radio needs to 
multiply external clock frequency which may be different for various 
generation CPU.
I believe the library's position stands as a generic library and not for 
specific CPU.



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

Re: [edk2] IP4 Config Troubles with DHCP

[Wu, Jiaxin]

Thanks Eugene, I will try to reproduce the issue and dig it out. Any process 
will inform you.

Best Regards!
Jiaxin

> -Original Message-
> From: Cohen, Eugene [mailto:eug...@hp.com]
> Sent: Thursday, August 11, 2016 10:31 PM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Subject: RE: IP4 Config Troubles with DHCP
> 
> Jianxin,
> 
> > I want to confirm with you the steps to reproduce the issue:
> >
> > 1. Set policy to DHCP.
> > 2. If DHCP process is not complete yet, then run one App to invoke the
> > UDP4 Configure with "UseDefaultAddress = TRUE" (loop to keep calling
> > Udp4->Configure until Ip4Mode.IsConfigured changes to TRUE)
> > 3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to
> > TRUE  failure here!!!
> >
> > Above steps right?
> 
> Yes, that summarizes it well.
> 
> > Actually, you don't need to retry the UDP configuration loop according
> > the Ip4Mode.IsConfigured flag. You are only recommended to set a timer
> > to check the mapping status after the configuration:
> 
> This is exactly what we used to do.  Recently it stopped working which is why
> we now have to re-do service binding creation and configuration.
> 
> > If DHCP process succeed, Ip4Mode.IsConfigured should be updated. If
> > not, any bug may be existed.
> 
> From the traces I've looked at I can clearly see that the DHCP process as
> finished (lease acquired) and that Ip4Mode.IsConfigured is still FALSE.  I'll 
> send
> you a detailed trace of this separately.
> 
> So yes, I think there is a bug that crept in.
> 
> Eugene
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPkg/CompilerIntrinsicsLib: replace memcpy and memset with C code

[Andrew Fish]

> On Aug 11, 2016, at 2:50 PM, Cohen, Eugene  wrote:
> 
>>> Why does memcpy performance matter?  In addition to the overall
>> memcpy stuff scattered around C code we have an instance that is
>> particularly sensitive to memcpy performance.  For DMA operations
>> when invoking double-buffering or access to portions of a buffer that
>> is common mapped (i.e. uncached on non-coherent DMA systems) the
>> impact of a non-optimized memcpy is enormous compared to the
>> optimized ones because the penalty is amplified by orders of
>> magnitude due to uncached memory access latency.
>>> 
>> 
>> That code would be using CopyMem(), no? This only serves the
>> compiler
>> generated calls, which are few since Tianocore does not allow
>> initialized locals.
> 
> I see and agree that should minimize the impact.   I guess I'll ask the naive 
> question.  Could the BaseMemoryLib and CompilerIntrinsicsLib share the same 
> stuff?
> 

Eugene,

I think if a CompilerIntrinsicsLib implementation consumes the BaseMemoryLib 
class (lists it in the INF) then I think it should just work.

Thanks,

Andrew Fish

> ___
> 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/3] ArmPkg/CompilerIntrinsicsLib: replace memcpy and memset with C code

[Ard Biesheuvel]

On 11 August 2016 at 23:50, Cohen, Eugene  wrote:
>> > Why does memcpy performance matter?  In addition to the overall
>> memcpy stuff scattered around C code we have an instance that is
>> particularly sensitive to memcpy performance.  For DMA operations
>> when invoking double-buffering or access to portions of a buffer that
>> is common mapped (i.e. uncached on non-coherent DMA systems) the
>> impact of a non-optimized memcpy is enormous compared to the
>> optimized ones because the penalty is amplified by orders of
>> magnitude due to uncached memory access latency.
>> >
>>
>> That code would be using CopyMem(), no? This only serves the
>> compiler
>> generated calls, which are few since Tianocore does not allow
>> initialized locals.
>
> I see and agree that should minimize the impact.   I guess I'll ask the naive 
> question.  Could the BaseMemoryLib and CompilerIntrinsicsLib share the same 
> stuff?
>

Let me look into that
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPkg/CompilerIntrinsicsLib: replace memcpy and memset with C code

[Cohen, Eugene]

> > Why does memcpy performance matter?  In addition to the overall
> memcpy stuff scattered around C code we have an instance that is
> particularly sensitive to memcpy performance.  For DMA operations
> when invoking double-buffering or access to portions of a buffer that
> is common mapped (i.e. uncached on non-coherent DMA systems) the
> impact of a non-optimized memcpy is enormous compared to the
> optimized ones because the penalty is amplified by orders of
> magnitude due to uncached memory access latency.
> >
> 
> That code would be using CopyMem(), no? This only serves the
> compiler
> generated calls, which are few since Tianocore does not allow
> initialized locals.

I see and agree that should minimize the impact.   I guess I'll ask the naive 
question.  Could the BaseMemoryLib and CompilerIntrinsicsLib share the same 
stuff?

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

Re: [edk2] [PATCH 1/3] ArmPkg/CompilerIntrinsicsLib: replace memcpy and memset with C code

[Ard Biesheuvel]

On 11 August 2016 at 23:34, Cohen, Eugene  wrote:
>> This replaces the various implementations of memset and memcpy,
>> including the ARM RTABI ones (__aeabi_mem[set|clr]_[|4|8]) with
>> a single C implementation for each. The ones we have are either not
>> very sophisticated (ARM), or they are too sophisticated (memcpy() on
>> AARCH64, which may perform unaligned accesses) or already coded in
>> C
>> (memset on AArch64).
>
> Ard,
>
> I'm concerned about the performance impact of this change... there's a reason 
> for all that complexity and it's to optimize performance.
>
> Why does memcpy performance matter?  In addition to the overall memcpy stuff 
> scattered around C code we have an instance that is particularly sensitive to 
> memcpy performance.  For DMA operations when invoking double-buffering or 
> access to portions of a buffer that is common mapped (i.e. uncached on 
> non-coherent DMA systems) the impact of a non-optimized memcpy is enormous 
> compared to the optimized ones because the penalty is amplified by orders of 
> magnitude due to uncached memory access latency.
>

That code would be using CopyMem(), no? This only serves the compiler
generated calls, which are few since Tianocore does not allow
initialized locals.

> So I would ask that before a change like this is brought in that we 
> characterize the cached-cached and cached-uncached (and perhaps unaligned 
> cached-cached) performance across the implementations.  Based on my 
> experience I'm expecting both cases will take a massive performance hit.
>
> From your commit message I'm inferring that the problem you're solving is to 
> play nice in environments that can't tolerate unaligned access like when the 
> MMU is off.  I get that - and I think a variant of the library that plays 
> nice in these limited cases makes sense.  However, I don't think we should 
> drag down the performance down of the rest of the environment where we spend 
> the vast majority of our time executing.
>
> Eugene
>
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/3] ArmPkg/CompilerIntrinsicsLib: replace memcpy and memset with C code

[Cohen, Eugene]

> This replaces the various implementations of memset and memcpy,
> including the ARM RTABI ones (__aeabi_mem[set|clr]_[|4|8]) with
> a single C implementation for each. The ones we have are either not
> very sophisticated (ARM), or they are too sophisticated (memcpy() on
> AARCH64, which may perform unaligned accesses) or already coded in
> C
> (memset on AArch64).

Ard,

I'm concerned about the performance impact of this change... there's a reason 
for all that complexity and it's to optimize performance.

Why does memcpy performance matter?  In addition to the overall memcpy stuff 
scattered around C code we have an instance that is particularly sensitive to 
memcpy performance.  For DMA operations when invoking double-buffering or 
access to portions of a buffer that is common mapped (i.e. uncached on 
non-coherent DMA systems) the impact of a non-optimized memcpy is enormous 
compared to the optimized ones because the penalty is amplified by orders of 
magnitude due to uncached memory access latency.

So I would ask that before a change like this is brought in that we 
characterize the cached-cached and cached-uncached (and perhaps unaligned 
cached-cached) performance across the implementations.  Based on my experience 
I'm expecting both cases will take a massive performance hit.

>From your commit message I'm inferring that the problem you're solving is to 
>play nice in environments that can't tolerate unaligned access like when the 
>MMU is off.  I get that - and I think a variant of the library that plays nice 
>in these limited cases makes sense.  However, I don't think we should drag 
>down the performance down of the rest of the environment where we spend the 
>vast majority of our time executing.

Eugene



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

[edk2] [PATCH 3/3] MdePkg RVCT: add definition of UNREACHABLE

[Ard Biesheuvel]

The RVCT compiler in --gnu mode appears to simply strip of the __builtin
prefix when it encounters calls to __builtin_xxx() functions, and so
the __builtin_unreachable() we emit for GCC results in linker errors
regarding undefined references against 'unreachable()'.

So define UNREACHABLE() to a NOP instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Include/Arm/ProcessorBind.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MdePkg/Include/Arm/ProcessorBind.h 
b/MdePkg/Include/Arm/ProcessorBind.h
index c2482c2f50f0..5ee7465c05a3 100644
--- a/MdePkg/Include/Arm/ProcessorBind.h
+++ b/MdePkg/Include/Arm/ProcessorBind.h
@@ -28,6 +28,13 @@
 #pragma pack()
 #endif
 
+//
+// RVCT does not support the __builtin_unreachable() macro
+//
+#ifdef __ARMCC_VERSION
+#define UNREACHABLE()
+#endif
+
 #if _MSC_EXTENSIONS 
   //
   // use Microsoft* C complier dependent integer width types 
-- 
2.7.4

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

[edk2] [PATCH 2/3] BaseTools RVCT: ignore various RVC diagnostics

[Ard Biesheuvel]

This updates the RVCT CC flags so various diagnostics that trigger
warnings-as-errors are silenced. In particular, RVCT complains about
missing newlines at the end of source files, mixing of enums and int
values and return statements followed by a break, all of which occur
in the Tianocore codebase, but none of which are actual errors in the
code. So just silence them.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2002c4c0598b..e3d70133caaa 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -7449,7 +7449,7 @@ RELEASE_XCODE5_X64_CC_FLAGS   = -target 
x86_64-pc-win32-macho -c-Os   -W
 

 
 DEFINE RVCT_ALL_ASM_FLAGS   = --diag_suppress=1786 --diag_error=warning --apcs 
/interwork
-DEFINE RVCT_ALL_CC_FLAGS= --c90 --no_autoinline --asm --gnu --apcs 
/interwork --signed_chars --no_unaligned_access --split_sections --enum_is_int 
--preinclude AutoGen.h --diag_suppress=186 --diag_warning 167 
--diag_error=warning --diag_style=ide --protect_stack
+DEFINE RVCT_ALL_CC_FLAGS= --c90 --no_autoinline --asm --gnu --apcs 
/interwork --signed_chars --no_unaligned_access --split_sections --enum_is_int 
--preinclude AutoGen.h --diag_suppress=186,188,1,111,68 --diag_warning 167 
--diag_error=warning --diag_style=ide --protect_stack
 DEFINE RVCT_ALL_DLINK_FLAGS = --no_scanlib --no_exceptions --datacompressor 
off --strict --symbols --diag_style=ide --no_legacyalign --scatter 
$(EDK_TOOLS_PATH)/Scripts/Rvct-Align32.sct
 
 

-- 
2.7.4

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

[edk2] [PATCH 1/3] ArmPkg/CompilerIntrinsicsLib: replace memcpy and memset with C code

[Ard Biesheuvel]

This replaces the various implementations of memset and memcpy,
including the ARM RTABI ones (__aeabi_mem[set|clr]_[|4|8]) with
a single C implementation for each. The ones we have are either not
very sophisticated (ARM), or they are too sophisticated (memcpy() on
AARCH64, which may perform unaligned accesses) or already coded in C
(memset on AArch64).

The Tianocore codebase mandates the explicit use of its SetMem() and
CopyMem() equivalents, of which various implementations exist for use
in different contexts (PEI, DXE). Few compiler generated references to
these functions should remain, and so our implementations in this BASE
library should be small and usable with the MMU off.

So replace them with a simple C implementation that builds correctly
on GCC/AARCH64, CLANG/AARCH64, GCC/ARM, CLANG/ARM and RVCT/ARM.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/CompilerIntrinsicsLib/AArch64/memcpy.S  | 119 

 ArmPkg/Library/CompilerIntrinsicsLib/AArch64/memset.c  |  25 
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/memcpy.S  |  32 --
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/memcpy.asm|  40 ---
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/memcpy4.asm   |  60 --
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S  |  61 --
 ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.asm|  50 
 ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf |  11 +-
 ArmPkg/Library/CompilerIntrinsicsLib/memcpy.c  |  43 +++
 ArmPkg/Library/CompilerIntrinsicsLib/memset.c  |  55 +
 10 files changed, 101 insertions(+), 395 deletions(-)

diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/AArch64/memcpy.S 
b/ArmPkg/Library/CompilerIntrinsicsLib/AArch64/memcpy.S
deleted file mode 100644
index 4dd6cf207754..
--- a/ArmPkg/Library/CompilerIntrinsicsLib/AArch64/memcpy.S
+++ /dev/null
@@ -1,119 +0,0 @@
-/*
- * Copyright (c) 2011 - 2013, ARM Ltd
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *notice, this list of conditions and the following disclaimer in the
- *documentation and/or other materials provided with the distribution.
- * 3. The name of the company may not be used to endorse or promote
- *products derived from this software without specific prior written
- *permission.
- *
- * THIS SOFTWARE IS PROVIDED BY ARM LTD ``AS IS'' AND ANY EXPRESS OR IMPLIED
- * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
- * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
- * IN NO EVENT SHALL ARM LTD BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED
- * TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
- * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
- * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
- * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include 
-
-// Taken from Newlib BSD implementation.
-ASM_FUNC(memcpy)
-// Copy dst to x6, so we can preserve return value.
-mov x6, x0
-
-// NOTE: although size_t is unsigned, this code uses signed
-// comparisons on x2 so relies on nb never having its top bit
-// set. In practice this is not going to be a real problem.
-
-// Require at least 64 bytes to be worth aligning.
-cmp x2, #64
-blt qwordcopy
-
-// Compute offset to align destination to 16 bytes.
-neg x3, x0
-and x3, x3, 15
-
-cbz x3, blockcopy   // offset == 0 is likely
-
-// We know there is at least 64 bytes to be done, so we
-// do a 16 byte misaligned copy at first and then later do
-// all 16-byte aligned copies.  Some bytes will be copied
-// twice, but there's no harm in that since memcpy does not
-// guarantee correctness on overlap.
-
-sub x2, x2, x3  // nb -= offset
-ldp x4, x5, [x1]
-add x1, x1, x3
-stp x4, x5, [x6]
-add x6, x6, x3
-
-// The destination pointer is now qword (16 byte) aligned.
-// (The src pointer might be.)
-
-blockcopy:
-// Copy 64 bytes at a time.
-subsx2, x2, #64
-blt 3f
-2:  subsx2, x2, #64
-ldp x4, x5, [x1,#0]

Re: [edk2] [PATCH 04/11] CorebootPayloadPkg/ResetSystemLib: Implement ResetPlatformSpecific

[Ma, Maurice]

It looks good to me. 
Reviewed by: Maurice Ma 

Regards,
Maurice

-Original Message-
From: Ni, Ruiyu 
Sent: Tuesday, August 09, 2016 10:56 PM
To: edk2-devel@lists.01.org
Cc: Ma, Maurice; Agyeman, Prince
Subject: [PATCH 04/11] CorebootPayloadPkg/ResetSystemLib: Implement 
ResetPlatformSpecific

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Maurice Ma 
Cc: Prince Agyeman 
---
 .../Library/ResetSystemLib/ResetSystemLib.c| 23 +-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.c 
b/CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.c
index 55f5609..f6621bf 100644
--- a/CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.c
+++ b/CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.c
@@ -1,7 +1,7 @@
 /** @file
   Reset System Library functions for coreboot
 
-  Copyright (c) 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2014 - 2016, 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 @@ -161,3 +161,24 @@ EnterS3WithImmediateWake (
   AcpiPmControl (5);
   ASSERT (FALSE);
 }
+
+/**
+  This function causes a systemwide reset. The exact type of the reset 
+is defined
+  by the EFI_GUID that follows the Null-terminated Unicode string passed into 
ResetData.
+  If the platform does not recognize the EFI_GUID in ResetData the 
+platform must pick a
+  supported reset type to perform.The platform may optionally log the 
+parameters from
+  any non-normal reset that occurs.
+
+  @param[in]  DataSize   The size, in bytes, of ResetData.
+  @param[in]  ResetData  The data buffer starts with a Null-terminated string, 
followed
+ by the EFI_GUID.
+**/
+VOID
+EFIAPI
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID*ResetData
+  )
+{
+  ResetCold ();
+}
--
2.9.0.windows.1

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

[edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Leo Duran]

Account for rebase of FV section containing VTF file on IA32/IA64.
This supports cases where the reset vector may not be set at 0xFFF0.

For example, FV section defined as:
[FV.FvSecPei]
  FvBaseAddress = $(FV_BOOT_BASE)
  FvForceRebase = TRUE

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Leo Duran 
---
 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFv/GenFvInternalLib.c 
b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
index 7c839e2..8c2827b 100644
--- a/BaseTools/Source/C/GenFv/GenFvInternalLib.c
+++ b/BaseTools/Source/C/GenFv/GenFvInternalLib.c
@@ -2770,11 +2770,13 @@ Returns:
   //
   // Update reset vector (SALE_ENTRY for IPF)
   // Now for IA32 and IA64 platform, the fv which has bsf file must have 
the 
-  // EndAddress of 0x. Thus, only this type fv needs to update the 
  
-  // reset vector. If the PEI Core is found, the VTF file will probably 
get  
-  // corrupted by updating the entry point.
  
+  // EndAddress of 0x (unless the section was rebased).
+  // Thus, only this type fv needs to update the  reset vector.
+  // If the PEI Core is found, the VTF file will probably get
+  // corrupted by updating the entry point.
   //
-  if ((mFvDataInfo.BaseAddress + mFvDataInfo.Size) == 
FV_IMAGES_TOP_ADDRESS) {   
+  if ((mFvDataInfo.ForceRebase == 1) ||
+(mFvDataInfo.BaseAddress + mFvDataInfo.Size) == FV_IMAGES_TOP_ADDRESS) 
{
 Status = UpdateResetVector (, , 
VtfFileImage);
 if (EFI_ERROR(Status)) {   
   Error (NULL, 0, 3000, "Invalid", "Could not update the reset 
vector.");
-- 
1.9.1

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

[edk2] [PATCH] BaseTools/Source/C/GenFv/GenFvInternalLib.c

[Leo Duran]

We have a situation where an x86 processor does not start fetching code
at the traditional hardware reset vector address 0xFFF0.
The implication is that the hidden base address part of the CS register
is initialized with a value different than the traditional 0x.

In our case:
CS = 0xF000 (as expected)
EIP = 0xFFF0 (as expected)
CSLIMIT = 0x (as expected)
CSBASE = 0x (where  is not the traditional 0x)

Thus in our case execution starts at: CSBASE+EIP = 0xFFF0.

To account for this behavior, we define an FDF address override
to force a rebase of the FV section containing the VTF file.
For example, last FV section defined as:
[FV.FvSecPei]
FvBaseAddress = $(FV_BOOT_BASE)
FvForceRebase = TRUE

However, when the GenFv tool parses that section it is unware
of the possible rebasing, and assumes the section ends at the
traditional 4G-byte boundary. This patch solves this by simply
adding a check for a possible rebase scenario.

Leo Duran (1):
  BaseTools/Source/C/GenFv/GenFvInternalLib.c

 BaseTools/Source/C/GenFv/GenFvInternalLib.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

-- 
1.9.1

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

Re: [edk2] DHCP Automatic Configure at Driver Connect

[Samer El Haj Mahmoud]

I agree that this is problematic. I thought last time we discussed this, we did 
try to request a PCD to set the default policy. I recall some hesitation in 
introducing a new PCD, but do not remember the details. I will try to dig up 
that thread.

Thanks,
--Samer


Samer El-Haj-Mahmoud
SESM - OS / SW Architect
Systems Management Development, Data Center Group
Lenovo United States
+1.919.908.5833
+1.512.659.1523
smahm...@lenovo.com
 

Lenovo.com /us 
Twitter | Facebook | Instagram | Blogs | Forums






-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Cohen, 
Eugene
Sent: Thursday, August 11, 2016 10:23 AM
To: Wu, Jiaxin ; Ye, Ting ; 
edk2-devel@lists.01.org
Subject: Re: [edk2] DHCP Automatic Configure at Driver Connect

Ting and Jiaxin, thank you both for clarifying.

In our situation DHCP is being set on a previous boot and we are now observing 
DHCP being attempted on every boot (since the controllers are connected).  So 
this is consistent with the behavior you describe - even though the default was 
originally static/manual the fact that we configured DHCP once causes this to 
be stored to NVRAM and perform dhcp process at every boot.

The issue is not just a performance issue around DHCP timing.  Even with a 
static IP address set the fact that the network interface comes "up" at each 
boot is problematic because our requirement is only to enable the network 
interface when directed to by an application.

Modifying the IP configuration dynamically to suppress the interface coming up 
at every boot is also problematic because it means we would need to store the 
IP address configuration in another NVRAM location.  In other words, the 
combining of the IP address settings storage *and* the policy of whether to 
configure at boot or wait until explicitly requested is problematic - we really 
would like to control these settings independently.  Is that possible within 
the scope of the spec?  Could we just have a PCD that suppresses the automatic 
configure at boot under any circumstance?

Thanks,

Eugene

> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Thursday, August 11, 2016 12:00 AM
> To: Ye, Ting ; Cohen, Eugene ; 
> edk2-devel@lists.01.org
> Subject: RE: DHCP Automatic Configure at Driver Connect
> 
> Thanks Ting's more background clarification.
> 
> I assume the difference you mentioned is between "SHA-1:
> 3d0a49ad47619c30c84bbee8a33f54b64dddbcec" and "SHA-1:
> 7648748e99eeeadec38fda7568adb260c4acc861". The two commits does cause 
> the different behavior as Ting said below. Git version 3d0a49ad will 
> only set the policy to dhcp but don't trigger D.O.R.A while 7648748e 
> always trigger D.O.R.A if policy is DHCP.
> 
> Version 7648748e commit is also the current behavior of Ip4Config2:
> DHCP policy together with D.O.R.A process, which is similar to the old 
> Ip4Config behavior. The version 3d0a49ad did was trying to resolve the 
> Ip4Dxe performance but it's not workable for IPv6, so we reverted it.
> 
> Thanks,
> Jiaxin
> 
> > -Original Message-
> > From: Ye, Ting
> > Sent: Thursday, August 11, 2016 11:03 AM
> > To: Cohen, Eugene ; edk2-devel@lists.01.org;
> Wu, Jiaxin
> > 
> > Subject: RE: DHCP Automatic Configure at Driver Connect
> >
> > Hi Eugene,
> >
> > Actually this is exactly the problem Samer raised to the mailing 
> > list in
> Aug 2015.
> > We ever fixed it with following patch:
> >
> > SHA-1: 3d0a49ad47619c30c84bbee8a33f54b64dddbcec
> >
> > * MdeModulePkg: Fix issue about current Ip4Dxe implementation
> for DHCP
> > DORA process
> >
> > DHCP policy is applied as default at boot time on all NICs in the
> system, which
> > results in all NIC ports attempting DHCP and trying to acquire IP
> addresses
> > during boot.
> > Ip4 driver should only set dhcp as default policy, and not trigger
> DORA at
> > driver binding start(). We should start DORA until one IP child is
> configured to
> > use default address.
> >
> > Later HP raised the same performance impact in IPv6 stack. We
> realized we
> > couldn't use the same logic to defer DHCP6 SARR process.
> > Instead, we discussed the issue in spec group and we removed the 
> > restriction from UEFI specification that the default policy should 
> > be Ip4Config2PolicyDhcp or Ip6ConfigPolicyAutomatic. It's up to 
> > implementation's choice.
> > The EDKII implementation was later updated that the default policy
> was
> > changed to Ip4Config2PolicyStatic and IP6ConfigPolicyManual. Also
> the
> > previous change was reverted, in order to keep IP4/IP6 solution
> consistent.
> > See patch (also reviewed by Samer):
> >
> > SHA-1: 7648748e99eeeadec38fda7568adb260c4acc861
> >
> > * MdeModulePkg: Change the default IPv4 config policy
> >
> > Git version '3d0a49ad' commit provided a scenario to resolve the 
> > performance issue for IPv4, but 

Re: [edk2] DHCP Automatic Configure at Driver Connect

[Cohen, Eugene]

Ting and Jiaxin, thank you both for clarifying.

In our situation DHCP is being set on a previous boot and we are now observing 
DHCP being attempted on every boot (since the controllers are connected).  So 
this is consistent with the behavior you describe - even though the default was 
originally static/manual the fact that we configured DHCP once causes this to 
be stored to NVRAM and perform dhcp process at every boot.

The issue is not just a performance issue around DHCP timing.  Even with a 
static IP address set the fact that the network interface comes "up" at each 
boot is problematic because our requirement is only to enable the network 
interface when directed to by an application.

Modifying the IP configuration dynamically to suppress the interface coming up 
at every boot is also problematic because it means we would need to store the 
IP address configuration in another NVRAM location.  In other words, the 
combining of the IP address settings storage *and* the policy of whether to 
configure at boot or wait until explicitly requested is problematic - we really 
would like to control these settings independently.  Is that possible within 
the scope of the spec?  Could we just have a PCD that suppresses the automatic 
configure at boot under any circumstance?

Thanks,

Eugene

> -Original Message-
> From: Wu, Jiaxin [mailto:jiaxin...@intel.com]
> Sent: Thursday, August 11, 2016 12:00 AM
> To: Ye, Ting ; Cohen, Eugene ;
> edk2-devel@lists.01.org
> Subject: RE: DHCP Automatic Configure at Driver Connect
> 
> Thanks Ting's more background clarification.
> 
> I assume the difference you mentioned is between "SHA-1:
> 3d0a49ad47619c30c84bbee8a33f54b64dddbcec" and "SHA-1:
> 7648748e99eeeadec38fda7568adb260c4acc861". The two commits
> does cause the different behavior as Ting said below. Git version
> 3d0a49ad will only set the policy to dhcp but don't trigger D.O.R.A while
> 7648748e always trigger D.O.R.A if policy is DHCP.
> 
> Version 7648748e commit is also the current behavior of Ip4Config2:
> DHCP policy together with D.O.R.A process, which is similar to the old
> Ip4Config behavior. The version 3d0a49ad did was trying to resolve the
> Ip4Dxe performance but it's not workable for IPv6, so we reverted it.
> 
> Thanks,
> Jiaxin
> 
> > -Original Message-
> > From: Ye, Ting
> > Sent: Thursday, August 11, 2016 11:03 AM
> > To: Cohen, Eugene ; edk2-devel@lists.01.org;
> Wu, Jiaxin
> > 
> > Subject: RE: DHCP Automatic Configure at Driver Connect
> >
> > Hi Eugene,
> >
> > Actually this is exactly the problem Samer raised to the mailing list in
> Aug 2015.
> > We ever fixed it with following patch:
> >
> > SHA-1: 3d0a49ad47619c30c84bbee8a33f54b64dddbcec
> >
> > * MdeModulePkg: Fix issue about current Ip4Dxe implementation
> for DHCP
> > DORA process
> >
> > DHCP policy is applied as default at boot time on all NICs in the
> system, which
> > results in all NIC ports attempting DHCP and trying to acquire IP
> addresses
> > during boot.
> > Ip4 driver should only set dhcp as default policy, and not trigger
> DORA at
> > driver binding start(). We should start DORA until one IP child is
> configured to
> > use default address.
> >
> > Later HP raised the same performance impact in IPv6 stack. We
> realized we
> > couldn't use the same logic to defer DHCP6 SARR process.
> > Instead, we discussed the issue in spec group and we removed the
> > restriction from UEFI specification that the default policy should be
> > Ip4Config2PolicyDhcp or Ip6ConfigPolicyAutomatic. It's up to
> > implementation's choice.
> > The EDKII implementation was later updated that the default policy
> was
> > changed to Ip4Config2PolicyStatic and IP6ConfigPolicyManual. Also
> the
> > previous change was reverted, in order to keep IP4/IP6 solution
> consistent.
> > See patch (also reviewed by Samer):
> >
> > SHA-1: 7648748e99eeeadec38fda7568adb260c4acc861
> >
> > * MdeModulePkg: Change the default IPv4 config policy
> >
> > Git version '3d0a49ad' commit provided a scenario to resolve the
> > performance issue for IPv4, but it's not workable for IPv6. To avoid
> IPv4 and
> > IPv6 inconsistency, we decided to revert that version fix.
> >
> > If so, the default policy for Ip4Config2 is Ip4Config2PolicyDhcp, which
> results
> > in all NIC ports attempting DHCP. So, this patch is used to changes the
> the
> > default IPv4 config policy to Ip4Config2PolicyStatic and also defer the
> SetData
> > operation after Ip4Config2Protocol installed. This update let the
> other
> > platform drivers have chance to change the default config data by
> consume
> > Ip4Config2Protocol.
> >
> >
> > Current implementation recommends that the system should stay in
> default
> > policy - static to not initialize DHCP process, unless the system needs
> to start
> > DHCP -- it should change the policy to IP4Config2PolicyDhcp.
> >
> > Best Regards,
> > Ye Ting
> >
> >
> > 

Re: [edk2] [PATCH] PcAtChipsetPkg AcpiTimerLib: Get more accurate TSC Frequency

[Brian J. Johnson]

On 08/10/2016 09:37 PM, Star Zeng wrote:

Minimize the code overhead between the two TSC reads by adding
new internal API to calculate TSC Frequency instead of reusing
MicroSecondDelay ().


Why not just use the MSR_PLATFORM_INFO (0xce) register bits 15..8, on 
CPUs where it's available, to read the TSC frequency directly?

--

Brian J. Johnson



  My statements are my own, are not authorized by SGI, and do not
  necessarily represent SGI’s positions.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/2] BaseTools ARM AARCH64: pass CC flags to linker for XIP modules as well

[Leif Lindholm]

On Thu, Aug 11, 2016 at 01:14:14PM +0200, Ard Biesheuvel wrote:
> Commit 478f50990a ("BaseTools GCC: add the compiler flags to the linker
> command line") added the compiler flags to the linker command line,
> which is required for LTO to function correctly, since it involves code
> generation at link time.
> 
> This patch failed to update the build rules for XIP modules on AARCH64,
> which not only requires the ordinary CC flags but also the XIP CC flags
> to prevent the LTO backend to, e.g., emit code that does not adhere to
> the strict alignment rules we impose for code that may execute with the
> MMU off.
> 
> So update the XIP link rules as well. Since AARCH64 and ARM are not
> supported by any toolchains in the GCCLD build rule family, drop the
> reference to GCCLD while we're at it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  BaseTools/Conf/build_rule.template | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/BaseTools/Conf/build_rule.template 
> b/BaseTools/Conf/build_rule.template
> index 6191957e0e70..7e2c6a96583d 100755
> --- a/BaseTools/Conf/build_rule.template
> +++ b/BaseTools/Conf/build_rule.template
> @@ -310,7 +310,7 @@
>  "$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH) -filelist 
> $(STATIC_LIBRARY_FILES_LIST)  $(DLINK2_FLAGS)
>  
>  
> -[Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64, 
> Static-Library-File.PEIM.AARCH64]
> +[Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64, 
> Static-Library-File.PEIM.AARCH64,Static-Library-File.SEC.ARM, 
> Static-Library-File.PEI_CORE.ARM, Static-Library-File.PEIM.ARM]
>  
>  *.lib
>  
> @@ -320,8 +320,8 @@
>  
>  $(DEBUG_DIR)(+)$(MODULE_NAME).dll
>  
> -
> -"$(DLINK)" -o ${dst} $(DLINK_FLAGS) $(DLINK_XIPFLAGS) 
> -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(DLINK2_FLAGS)
> +
> +"$(DLINK)" -o ${dst} $(DLINK_FLAGS) $(DLINK_XIPFLAGS) 
> -Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS) 
> $(CC_XIPFLAGS) $(DLINK2_FLAGS)
>  "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
>  
>  
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 2/2] BaseTools ARM AARCH64: drop redundant compiler arguments

[Leif Lindholm]

On Thu, Aug 11, 2016 at 01:14:15PM +0200, Ard Biesheuvel wrote:
> The ARM and AARCH64 CC_FLAGS definitions include both GCC_ALL_CC_FLAGS
> and GCC44_ALL_CC_FLAGS, resulting in many of the compiler arguments
> being passed twice. Since the CLANG35 definitions do not refer to
> GCC44_ALL_CC_FLAGS, drop the reference for GCCx as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  BaseTools/Conf/tools_def.template | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Conf/tools_def.template 
> b/BaseTools/Conf/tools_def.template
> index 2002c4c0598b..d6d3ed380668 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -4400,7 +4400,7 @@ DEFINE GCC46_X64_DLINK_FLAGS = 
> DEF(GCC45_X64_DLINK_FLAGS)
>  DEFINE GCC46_X64_DLINK2_FLAGS= DEF(GCC45_X64_DLINK2_FLAGS)
>  DEFINE GCC46_ASM_FLAGS   = DEF(GCC45_ASM_FLAGS)
>  DEFINE GCC46_ARM_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
> DEF(GCC_ASM_FLAGS) -mlittle-endian
> -DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
> DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector 
> -mword-relocations -save-temps
> +DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
> DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -save-temps
>  DEFINE GCC46_ARM_CC_XIPFLAGS = -D__ARM_FEATURE_UNALIGNED=0
>  DEFINE GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) 
> -Wl,--oformat=elf32-littlearm
>  DEFINE GCC46_ARM_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
> -Wl,--defsym=PECOFF_HEADER_SIZE=0x220
> @@ -4419,7 +4419,7 @@ DEFINE GCC47_ARM_ASM_FLAGS   = 
> DEF(GCC46_ARM_ASM_FLAGS)
>  DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
> DEF(GCC_ASM_FLAGS) -mlittle-endian
>  DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS)
>  DEFINE GCC47_ARM_CC_XIPFLAGS = DEF(GCC_ARM_CC_XIPFLAGS)
> -DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
> DEF(GCC44_ALL_CC_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps
> +DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
> -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps
>  DEFINE GCC47_AARCH64_CC_XIPFLAGS = DEF(GCC_AARCH64_CC_XIPFLAGS)
>  DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
>  DEFINE GCC47_ARM_DLINK2_FLAGS= DEF(GCC46_ARM_DLINK2_FLAGS)
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC] Image definition file for HII Image package

[Gao, Liming]

Hi, all
  This proposal is to add support of HII Image package generation in edk2 
BaseTools.  Like uni file for HII string package, it includes the following 
things. Please help review and raise your comments.

1)  New file type *.idf Image definition file to describe HII image 
resource. It is the ASCII text file, and includes one or more "#image IMAGE_ID 
[TRANSPARENT] ImageFileName".

a.   IMAGE_ID is macro name. IMAGE_TOKEN (IMAGE_ID) is referred in other 
source files.

b.  TRANSPARENT is optional. If it is specified, BaseTools will add TRANS 
image block type for the input ImageFile. UEFI spec doesn't define TRANS JPG 
and PNG block type. So, this flag is ignored for JPG and PNG image.

c.   ImageFileName is also listed in [Sources] section of INF, and its file 
extension is one of .bmp, .jpg, .png (case insensitive).

d.  BaseTools parses idf files and gets every image file, then bases on 
file extension to generate the different image block data, last combine them 
and output HII image package.

   i.  If more 
than one idf files are listed in [Sources] section of one INF, they will be 
merged and parsed.

 ii.  If the 
image file is not found or its file extension is unsupported, BaseTools will 
report build error.

iii.  If the 
file extension is bmp, BaseTools will convert it convert it to 
EFI_HII_IIBT_IMAGE_BLOCK and EFI_HII_IMAGE_PALETTE_INFO.

   iv.  If the file 
extension is jpg, BaseTools will only check 'JFIF' signature at offset 6 in the 
image file, then add it into EFI_HII_IIBT_JPEG_BLOCK.

 v.  If the 
file extension is  png, BaseTools directly adds it into EFI_HII_IIBT_PNG_BLOCK 
without any check.

2)  New IMAGE_TOKEN macro is used to refer to IMAGE_ID.

3)  New AutoGen header file $(MODULE_NAME)ImgDefs.h to include the 
generated ImageId definition.

4)  New $(MODULE_NAME)Idf.hpk or $(MODULE_NAME)Images are generated as the 
output binary HII image package.

Sample.idf:
#image IMG_LOGO TRANSPARENT  Logo.bmp
#image IMG_FULL_LOGO   Logo.jpg
#image IMG_OEM_LOGOLogo.png

Thanks
Liming

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

Re: [edk2] [PATCH 0/6] Move to ASM_FUNC() macro, and clean up some asm as well

[Ard Biesheuvel]

On 11 August 2016 at 12:25, Leif Lindholm  wrote:
> On Thu, Aug 11, 2016 at 12:08:15PM +0200, Ard Biesheuvel wrote:
>> This moves the asm files in OpenPlatformPkg to the new ASM_FUNC() macro,
>> which annotates functions in a way that allows the linker to drop code
>> that is not actually used anywhere. It is analogous to -ffunction-sections
>> for GCC.
>>
>> Since there are some cargo culted asm patterns that are very clunky and
>> inefficient, clean those up as well (i.e., LoadConstantToReg())
>
> For the series:
> Reviewed-by: Leif Lindholm 
>

Thanks, pushed with a minor tweak to #2 to prevent further breakage
with RVCT (given that the upstream is broken with RVCT to begin with)

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

Re: [edk2] [PATCH 00/26] ARM assembler cleanup series

[Ard Biesheuvel]

On 11 August 2016 at 13:27, Ard Biesheuvel  wrote:
> On 11 August 2016 at 12:18, Leif Lindholm  wrote:
>> Nice bit of cleanup, thanks!
>>
>> For the ones I haven't commented on already:
>> Reviewed-by: Leif Lindholm 
>>
>> (And for the BeagleBoardPkg as well, unless Andrew objects.)
>>
>
> Committed as
>
> 2b47cdc9364f ArmLib: remove ArmReplaceLiveTranslationEntry() implementation
> 820d07abadf1 ArmPkg: add missing ArmMmuLib resolution to ArmPkg.dsc
> 874883a49d0e ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and
> other asm macros
> 66edb631f8a2 ArmPlatformPkg RVCT: drop dependency on GCC macro library
> d2d0e27c7668 ArmPkg: introduce ASM_FUNC, MOV32/MOV64 and ADRL/LDRL macros
> 16a9fe2ca9cc ArmVirt/PrePi: make jump to CEntryPoint relative
> dfc2838892e4 ArmVirtPkg: clean up assembly source files
> 5e32710023e2 ArmPkg/ArmSmcLibNull: move to generic C implementation
> 136df8b8b2bb ArmPkg/ArmCpuLib: switch to ASM_FUNC() asm macro
> f0883e35dea7 ArmPkg/ArmGicV3: switch to ASM_FUNC() asm macro
> de656e666c61 ArmPkg/ArmHvcLib: switch to ASM_FUNC() asm macro
> 0efaa42f6e06 ArmPkg/ArmLib: switch to ASM_FUNC() asm macro
> e4d37ada015f ArmPkg/ArmMmuLib: switch to ASM_FUNC() asm macro
> 86a4d91bda59 ArmPkg/ArmSmcLib: switch to ASM_FUNC() asm macro
> 8ca934aab50b ArmPkg/BaseMemoryLibSm: switch to ASM_FUNC() asm macro
> 7589d9dbcfbf ArmPkg/BaseMemoryLibVstm: switch to ASM_FUNC() asm macro
> 903e31242d01 ArmPkg/CompilerIntrinsicsLib: switch to ASM_FUNC() asm macro
> 22b080c78c7a ArmPkg/SemihostLib: switch to ASM_FUNC() asm macro
> b8f76eaec25e BeagleBoardPkg: add missing ArmMmuLib resolution
> a0f56915a02c ArmPlatformPkg/ArmJunoLib: switch to ASM_FUNC() asm macro
> d2fa09a13487 ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro
> 13dc7fa5a082 ArmPlatformPkg/PrePeiCore: switch to ASM_FUNC() asm macro
> 04209b53549b ArmPlatformPkg/ArmVExpressPkg: switch to ASM_FUNC() asm macro
> c17ae4cf8e07 ArmPlatformPkg/ArmPlatformLibNull: switch to ASM_FUNC() asm macro
> 926059304e83 ArmPlatformPkg/ArmPlatformStackLib: switch to ASM_FUNC() asm 
> macro
>
> (with the comments addressed)
>
> Thanks,
> Ard.
>

Oh, and in case anyone is curious:

ArmVirtQemu / RELEASE_GCC5

before
FVMAIN_COMPACT [34%Full] 2093056 total, 723448 used, 1369608 free
FVMAIN [99%Full] 3670400 total, 3670376 used, 24 free

after
FVMAIN_COMPACT [33%Full] 2093056 total, 709376 used, 1383680 free
FVMAIN [99%Full] 3652736 total, 3652680 used, 56 free
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 00/26] ARM assembler cleanup series

[Ard Biesheuvel]

On 11 August 2016 at 12:18, Leif Lindholm  wrote:
> Nice bit of cleanup, thanks!
>
> For the ones I haven't commented on already:
> Reviewed-by: Leif Lindholm 
>
> (And for the BeagleBoardPkg as well, unless Andrew objects.)
>

Committed as

2b47cdc9364f ArmLib: remove ArmReplaceLiveTranslationEntry() implementation
820d07abadf1 ArmPkg: add missing ArmMmuLib resolution to ArmPkg.dsc
874883a49d0e ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and
other asm macros
66edb631f8a2 ArmPlatformPkg RVCT: drop dependency on GCC macro library
d2d0e27c7668 ArmPkg: introduce ASM_FUNC, MOV32/MOV64 and ADRL/LDRL macros
16a9fe2ca9cc ArmVirt/PrePi: make jump to CEntryPoint relative
dfc2838892e4 ArmVirtPkg: clean up assembly source files
5e32710023e2 ArmPkg/ArmSmcLibNull: move to generic C implementation
136df8b8b2bb ArmPkg/ArmCpuLib: switch to ASM_FUNC() asm macro
f0883e35dea7 ArmPkg/ArmGicV3: switch to ASM_FUNC() asm macro
de656e666c61 ArmPkg/ArmHvcLib: switch to ASM_FUNC() asm macro
0efaa42f6e06 ArmPkg/ArmLib: switch to ASM_FUNC() asm macro
e4d37ada015f ArmPkg/ArmMmuLib: switch to ASM_FUNC() asm macro
86a4d91bda59 ArmPkg/ArmSmcLib: switch to ASM_FUNC() asm macro
8ca934aab50b ArmPkg/BaseMemoryLibSm: switch to ASM_FUNC() asm macro
7589d9dbcfbf ArmPkg/BaseMemoryLibVstm: switch to ASM_FUNC() asm macro
903e31242d01 ArmPkg/CompilerIntrinsicsLib: switch to ASM_FUNC() asm macro
22b080c78c7a ArmPkg/SemihostLib: switch to ASM_FUNC() asm macro
b8f76eaec25e BeagleBoardPkg: add missing ArmMmuLib resolution
a0f56915a02c ArmPlatformPkg/ArmJunoLib: switch to ASM_FUNC() asm macro
d2fa09a13487 ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro
13dc7fa5a082 ArmPlatformPkg/PrePeiCore: switch to ASM_FUNC() asm macro
04209b53549b ArmPlatformPkg/ArmVExpressPkg: switch to ASM_FUNC() asm macro
c17ae4cf8e07 ArmPlatformPkg/ArmPlatformLibNull: switch to ASM_FUNC() asm macro
926059304e83 ArmPlatformPkg/ArmPlatformStackLib: switch to ASM_FUNC() asm macro

(with the comments addressed)

Thanks,
Ard.



> On Wed, Aug 10, 2016 at 05:17:36PM +0200, Ard Biesheuvel wrote:
>> As requested by Eugene, this series introduces a new ASM_FUNC preprocessor
>> macro that emits functions into separate sections, allowing the linker to
>> get rid of the code that ends up unused in the module.
>>
>> Note that using a native GNU as macro turned out to be problematic, due
>> to our use of Trim, and the fact that not all versions of GNU as honour
>> the -I option, making both #include (preprocessor) and .include (GNU as)
>> unusable to include files with shared macro definitions.
>>
>> Since we're making a clean spot, let's introduce some other utility macros
>> as well, and clean up the various assembler files to use it. In particular,
>> clean up various patterns involving LoadConstantToReg(), including the gem
>>
>>   LoadConstantToReg (_gPcd_FixedAtBuild_, rN)
>>   ldr  rN, [rN]
>>
>> which performs two memory reads, including one that is subject to runtime
>> relocation, to load a compile time constant into a register. Note that this
>> is the definition of LoadConstantReg() we use for Clang, even on AARCH64:
>>
>>   // load _Reg with _Data
>>   #define LoadConstantToReg(_Data, _Reg)\
>> ldr  _Reg, 1f ; \
>> b2f   ; \
>>   .align(8)   ; \
>>   1:\
>> .8byte (_Data); \
>>   2:
>>
>> Other changes involve constant folding, i.e.,
>>
>>  // Stack for the secondary core = Number of Cores - 1
>>   -  LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0)
>>   -  sub   x0, x0, #1
>>   -  LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1)
>>   -  mul   x1, x1, x0
>>   +  MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * 
>> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
>>
>> and
>>
>>   -  LoadConstantToReg (FixedPcdGet64(PcdCPUCoresStackBase), r1)
>>   -  LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2)
>>   -  add   r1, r1, r2
>>   +  MOV32 (r1, FixedPcdGet64(PcdCPUCoresStackBase) + 
>> FixedPcdGet32(PcdCPUCorePrimaryStackSize))
>>
>> (where r2 is dead after the add)
>>
>> Code can be found here
>> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/arm-asm-cleanup2
>>
>> Ard Biesheuvel (26):
>>   ArmLib: remove ArmReplaceLiveTranslationEntry() implementation
>>   ArmPkg: add missing ArmMmuLib resolution to ArmPkg.dsc
>>   ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and other asm macros
>>   ArmPlatformPkg RVCT: drop dependency on GCC macro library
>>   ArmPkg: introduce ASM_FUNC, MOV32/MOV64 and ADRL/LDRL macros
>>   ArmVirt/PrePi: make jump to CEntryPoint relative
>>   ArmVirtPkg: clean up assembly source files
>>   ArmPkg/ArmSmcLibNull: move to generic C implementation
>>   ArmPkg/ArmCpuLib: switch to ASM_FUNC() asm macro
>>   ArmPkg/ArmGicV3: switch to ASM_FUNC() asm macro
>>   

[edk2] [PATCH 1/2] BaseTools ARM AARCH64: pass CC flags to linker for XIP modules as well

[Ard Biesheuvel]

Commit 478f50990a ("BaseTools GCC: add the compiler flags to the linker
command line") added the compiler flags to the linker command line,
which is required for LTO to function correctly, since it involves code
generation at link time.

This patch failed to update the build rules for XIP modules on AARCH64,
which not only requires the ordinary CC flags but also the XIP CC flags
to prevent the LTO backend to, e.g., emit code that does not adhere to
the strict alignment rules we impose for code that may execute with the
MMU off.

So update the XIP link rules as well. Since AARCH64 and ARM are not
supported by any toolchains in the GCCLD build rule family, drop the
reference to GCCLD while we're at it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/build_rule.template | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Conf/build_rule.template 
b/BaseTools/Conf/build_rule.template
index 6191957e0e70..7e2c6a96583d 100755
--- a/BaseTools/Conf/build_rule.template
+++ b/BaseTools/Conf/build_rule.template
@@ -310,7 +310,7 @@
 "$(DLINK)" $(DLINK_FLAGS) -o ${dst} $(DLINK_SPATH) -filelist 
$(STATIC_LIBRARY_FILES_LIST)  $(DLINK2_FLAGS)
 
 
-[Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64, 
Static-Library-File.PEIM.AARCH64]
+[Static-Library-File.SEC.AARCH64, Static-Library-File.PEI_CORE.AARCH64, 
Static-Library-File.PEIM.AARCH64,Static-Library-File.SEC.ARM, 
Static-Library-File.PEI_CORE.ARM, Static-Library-File.PEIM.ARM]
 
 *.lib
 
@@ -320,8 +320,8 @@
 
 $(DEBUG_DIR)(+)$(MODULE_NAME).dll
 
-
-"$(DLINK)" -o ${dst} $(DLINK_FLAGS) $(DLINK_XIPFLAGS) 
-Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(DLINK2_FLAGS)
+
+"$(DLINK)" -o ${dst} $(DLINK_FLAGS) $(DLINK_XIPFLAGS) 
-Wl,--start-group,@$(STATIC_LIBRARY_FILES_LIST),--end-group $(CC_FLAGS) 
$(CC_XIPFLAGS) $(DLINK2_FLAGS)
 "$(OBJCOPY)" $(OBJCOPY_FLAGS) ${dst}
 
 
-- 
2.7.4

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

[edk2] [PATCH 2/2] BaseTools ARM AARCH64: drop redundant compiler arguments

[Ard Biesheuvel]

The ARM and AARCH64 CC_FLAGS definitions include both GCC_ALL_CC_FLAGS
and GCC44_ALL_CC_FLAGS, resulting in many of the compiler arguments
being passed twice. Since the CLANG35 definitions do not refer to
GCC44_ALL_CC_FLAGS, drop the reference for GCCx as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2002c4c0598b..d6d3ed380668 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4400,7 +4400,7 @@ DEFINE GCC46_X64_DLINK_FLAGS = 
DEF(GCC45_X64_DLINK_FLAGS)
 DEFINE GCC46_X64_DLINK2_FLAGS= DEF(GCC45_X64_DLINK2_FLAGS)
 DEFINE GCC46_ASM_FLAGS   = DEF(GCC45_ASM_FLAGS)
 DEFINE GCC46_ARM_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
-DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) DEF(GCC_ARM_CC_FLAGS) -fstack-protector 
-mword-relocations -save-temps
+DEFINE GCC46_ARM_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ARM_CC_FLAGS) -fstack-protector -mword-relocations -save-temps
 DEFINE GCC46_ARM_CC_XIPFLAGS = -D__ARM_FEATURE_UNALIGNED=0
 DEFINE GCC46_ARM_DLINK_FLAGS = DEF(GCC_ARM_DLINK_FLAGS) 
-Wl,--oformat=elf32-littlearm
 DEFINE GCC46_ARM_DLINK2_FLAGS= DEF(GCC_DLINK2_FLAGS_COMMON) 
-Wl,--defsym=PECOFF_HEADER_SIZE=0x220
@@ -4419,7 +4419,7 @@ DEFINE GCC47_ARM_ASM_FLAGS   = 
DEF(GCC46_ARM_ASM_FLAGS)
 DEFINE GCC47_AARCH64_ASM_FLAGS   = $(ARCHASM_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC_ASM_FLAGS) -mlittle-endian
 DEFINE GCC47_ARM_CC_FLAGS= DEF(GCC46_ARM_CC_FLAGS)
 DEFINE GCC47_ARM_CC_XIPFLAGS = DEF(GCC_ARM_CC_XIPFLAGS)
-DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
DEF(GCC44_ALL_CC_FLAGS) -mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps
+DEFINE GCC47_AARCH64_CC_FLAGS= $(ARCHCC_FLAGS) $(PLATFORM_FLAGS) 
-mcmodel=large DEF(GCC_AARCH64_CC_FLAGS) -save-temps
 DEFINE GCC47_AARCH64_CC_XIPFLAGS = DEF(GCC_AARCH64_CC_XIPFLAGS)
 DEFINE GCC47_ARM_DLINK_FLAGS = DEF(GCC46_ARM_DLINK_FLAGS)
 DEFINE GCC47_ARM_DLINK2_FLAGS= DEF(GCC46_ARM_DLINK2_FLAGS)
-- 
2.7.4

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

Re: [edk2] [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Thursday, August 11, 2016 5:54 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Gao, Liming 
> Subject: [edk2] [PATCH v2] MdeModulePkg/Browser: Enhance the logic
> when getting value from AltResp
> 
> This patch is to enhance SetupBrowser to handle following two cases:
> 1. When searching BlockName in AltResp, the hex digits of related
> BlockName
>in AltResp may be in uppercase.
> 2. When converting the Value in AltResp to HiiValue, the length of value
>string is bigger than the length of StorageWidth of the question.
> 
> Notes:
> V2:
> - Add some comments to make the logic clear.
> 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> Reviewed-by: Eric Dong 
> ---
>  MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 61
> +++---
>  1 file changed, 35 insertions(+), 26 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> index 66c4313..cd3c8cc 100644
> --- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> +++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
> @@ -1427,44 +1427,48 @@ BufferToValue (
>}
>TempChar = *StringPtr;
>*StringPtr = L'\0';
> 
>LengthStr = StrLen (Value);
> +
> +  //
> +  // Value points to a Unicode hexadecimal string, we need to convert the
> string to the value with CHAR16/UINT8...type.
> +  // When generating the Value string, we follow this rule: 1 byte -> 2
> Unicode characters (for string: 2 byte(CHAR16) ->4 Unicode characters).
> +  // So the maximum value string length of a question is : Question-
> >StorageWidth * 2.
> +  // If the value string length > Question->StorageWidth * 2, only set the
> string length as Question->StorageWidth * 2, then convert.
> +  //
> +  if (LengthStr > (UINTN) Question->StorageWidth * 2) {
> +Length = (UINTN) Question->StorageWidth * 2;
> +  } else {
> +Length = LengthStr;
> +  }
> +
>Status= EFI_SUCCESS;
>if (!IsBufferStorage && IsString) {
>  //
>  // Convert Config String to Unicode String, e.g "0041004200430044" =>
> "ABCD"
>  // Add string tail char L'\0' into Length
>  //
> -Length= Question->StorageWidth + sizeof (CHAR16);
> -if (Length < ((LengthStr / 4 + 1) * 2)) {
> -  Status = EFI_BUFFER_TOO_SMALL;
> -} else {
> -  DstBuf = (CHAR16 *) Dst;
> -  ZeroMem (TemStr, sizeof (TemStr));
> -  for (Index = 0; Index < LengthStr; Index += 4) {
> -StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 
> 4);
> -DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
> -  }
> -  //
> -  // Add tailing L'\0' character
> -  //
> -  DstBuf[Index/4] = L'\0';
> +DstBuf = (CHAR16 *) Dst;
> +ZeroMem (TemStr, sizeof (TemStr));
> +for (Index = 0; Index < Length; Index += 4) {
> +  StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
> +  DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
>  }
> +//
> +// Add tailing L'\0' character
> +//
> +DstBuf[Index/4] = L'\0';
>} else {
> -if (Question->StorageWidth < ((LengthStr + 1) / 2)) {
> -  Status = EFI_BUFFER_TOO_SMALL;
> -} else {
> -  ZeroMem (TemStr, sizeof (TemStr));
> -  for (Index = 0; Index < LengthStr; Index ++) {
> -TemStr[0] = Value[LengthStr - Index - 1];
> -DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
> -if ((Index & 1) == 0) {
> -  Dst [Index/2] = DigitUint8;
> -} else {
> -  Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
> -}
> +ZeroMem (TemStr, sizeof (TemStr));
> +for (Index = 0; Index < Length; Index ++) {
> +  TemStr[0] = Value[LengthStr - Index - 1];
> +  DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
> +  if ((Index & 1) == 0) {
> +Dst [Index/2] = DigitUint8;
> +  } else {
> +Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
>}
>  }
>}
> 
>*StringPtr = TempChar;
> @@ -3755,10 +3759,15 @@ GetOffsetFromConfigResp (
>//
>// Type is EFI_HII_VARSTORE_EFI_VARIABLE or
> EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER
>//
> 
>//
> +  // Convert all hex digits in ConfigResp to lower case before searching.
> +  //
> +  HiiToLower (ConfigResp);
> +
> +  //
>// 1. Directly use Question->BlockName to find.
>//
>RequestElement = StrStr (ConfigResp, Question->BlockName);
>if (RequestElement != NULL) {
>  //
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> 

Re: [edk2] [PATCH 0/6] Move to ASM_FUNC() macro, and clean up some asm as well

[Leif Lindholm]

On Thu, Aug 11, 2016 at 12:08:15PM +0200, Ard Biesheuvel wrote:
> This moves the asm files in OpenPlatformPkg to the new ASM_FUNC() macro,
> which annotates functions in a way that allows the linker to drop code
> that is not actually used anywhere. It is analogous to -ffunction-sections
> for GCC.
> 
> Since there are some cargo culted asm patterns that are very clunky and
> inefficient, clean those up as well (i.e., LoadConstantToReg())

For the series:
Reviewed-by: Leif Lindholm 

> Ard Biesheuvel (6):
>   Platforms/BeagleBoard: remove unreferenced Sec.inf module
>   Platforms/BeagleBoard/BeagleBoardLib: switch to ASM_FUNC() asm macro
>   Platforms/Styx: remove unused AmdStyxSecLib
>   Platforms/Styx: switch to ASM_FUNC() asm macro
>   Platforms/Hisilicon/ArmPlatformLibPv660: switch to ASM_FUNC() asm
> macro
>   Platforms/Marvell/Armada70x0Lib: switch to ASM_FUNC() asm macro
> 
>  Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S 
>  |  44 +--
>  Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc 
>  |   1 -
>  Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S   
>  |  58 ++--
>  Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf 
>  |   7 +-
>  Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf  
>  |   1 +
>  Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S 
>  |  73 -
>  Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S  
>  | 182 -
>  Platforms/AMD/Styx/Library/AmdStyxSecLib/AmdStyxSecLib.inf   
>  |  50 
>  Platforms/AMD/Styx/Library/AmdStyxSecLib/StyxSec.c   
>  |  96 ---
>  Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc 
>  |   1 -
>  Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S   
>  |  32 +--
>  
> Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
>  |  16 +-
>  Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S
>  |  85 --
>  Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm  
>  |  89 --
>  Platforms/TexasInstruments/BeagleBoard/Sec/Cache.c   
>  |  80 --
>  Platforms/TexasInstruments/BeagleBoard/Sec/Clock.c   
>  |  70 -
>  Platforms/TexasInstruments/BeagleBoard/Sec/LzmaDecompress.h  
>  | 103 ---
>  Platforms/TexasInstruments/BeagleBoard/Sec/PadConfiguration.c
>  | 282 
>  Platforms/TexasInstruments/BeagleBoard/Sec/Sec.c 
>  | 186 -
>  Platforms/TexasInstruments/BeagleBoard/Sec/Sec.inf   
>  |  74 -
>  20 files changed, 42 insertions(+), 1488 deletions(-)
>  delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S
>  delete mode 100644 
> Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S
>  delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/AmdStyxSecLib.inf
>  delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/StyxSec.c
>  delete mode 100644 
> Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S
>  delete mode 100644 
> Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm
>  delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Cache.c
>  delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Clock.c
>  delete mode 100644 
> Platforms/TexasInstruments/BeagleBoard/Sec/LzmaDecompress.h
>  delete mode 100644 
> Platforms/TexasInstruments/BeagleBoard/Sec/PadConfiguration.c
>  delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.c
>  delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.inf
> 
> -- 
> 2.7.4
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 00/26] ARM assembler cleanup series

[Leif Lindholm]

Nice bit of cleanup, thanks!

For the ones I haven't commented on already:
Reviewed-by: Leif Lindholm 

(And for the BeagleBoardPkg as well, unless Andrew objects.)

On Wed, Aug 10, 2016 at 05:17:36PM +0200, Ard Biesheuvel wrote:
> As requested by Eugene, this series introduces a new ASM_FUNC preprocessor
> macro that emits functions into separate sections, allowing the linker to
> get rid of the code that ends up unused in the module.
> 
> Note that using a native GNU as macro turned out to be problematic, due
> to our use of Trim, and the fact that not all versions of GNU as honour
> the -I option, making both #include (preprocessor) and .include (GNU as)
> unusable to include files with shared macro definitions.
> 
> Since we're making a clean spot, let's introduce some other utility macros
> as well, and clean up the various assembler files to use it. In particular,
> clean up various patterns involving LoadConstantToReg(), including the gem
> 
>   LoadConstantToReg (_gPcd_FixedAtBuild_, rN)
>   ldr  rN, [rN]
> 
> which performs two memory reads, including one that is subject to runtime
> relocation, to load a compile time constant into a register. Note that this
> is the definition of LoadConstantReg() we use for Clang, even on AARCH64:
> 
>   // load _Reg with _Data
>   #define LoadConstantToReg(_Data, _Reg)\
> ldr  _Reg, 1f ; \
> b2f   ; \
>   .align(8)   ; \
>   1:\
> .8byte (_Data); \
>   2:
> 
> Other changes involve constant folding, i.e.,
> 
>  // Stack for the secondary core = Number of Cores - 1
>   -  LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0)
>   -  sub   x0, x0, #1
>   -  LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1)
>   -  mul   x1, x1, x0
>   +  MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * 
> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
> 
> and
> 
>   -  LoadConstantToReg (FixedPcdGet64(PcdCPUCoresStackBase), r1)
>   -  LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), r2)
>   -  add   r1, r1, r2
>   +  MOV32 (r1, FixedPcdGet64(PcdCPUCoresStackBase) + 
> FixedPcdGet32(PcdCPUCorePrimaryStackSize))
> 
> (where r2 is dead after the add)
> 
> Code can be found here
> https://git.linaro.org/people/ard.biesheuvel/uefi-next.git/shortlog/refs/heads/arm-asm-cleanup2
> 
> Ard Biesheuvel (26):
>   ArmLib: remove ArmReplaceLiveTranslationEntry() implementation
>   ArmPkg: add missing ArmMmuLib resolution to ArmPkg.dsc
>   ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and other asm macros
>   ArmPlatformPkg RVCT: drop dependency on GCC macro library
>   ArmPkg: introduce ASM_FUNC, MOV32/MOV64 and ADRL/LDRL macros
>   ArmVirt/PrePi: make jump to CEntryPoint relative
>   ArmVirtPkg: clean up assembly source files
>   ArmPkg/ArmSmcLibNull: move to generic C implementation
>   ArmPkg/ArmCpuLib: switch to ASM_FUNC() asm macro
>   ArmPkg/ArmGicV3: switch to ASM_FUNC() asm macro
>   ArmPkg/ArmHvcLib: switch to ASM_FUNC() asm macro
>   ArmPkg/ArmLib: switch to ASM_FUNC() asm macro
>   ArmPkg/ArmMmuLib: switch to ASM_FUNC() asm macro
>   ArmPkg/ArmSmcLib: switch to ASM_FUNC() asm macro
>   ArmPkg/BaseMemoryLibSm: switch to ASM_FUNC() asm macro
>   ArmPkg/BaseMemoryLibVstm: switch to ASM_FUNC() asm macro
>   ArmPkg/CompilerIntrinsicsLib: switch to ASM_FUNC() asm macro
>   ArmPkg/SemihostLib: switch to ASM_FUNC() asm macro
>   BeagleBoardPkg: remove unused Sec.inf module
>   BeagleBoardPkg: add missing ArmMmuLib resolution
>   ArmPlatformPkg/ArmJunoLib: switch to ASM_FUNC() asm macro
>   ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro
>   ArmPlatformPkg/PrePeiCore: switch to ASM_FUNC() asm macro
>   ArmPlatformPkg/ArmVExpressPkg: switch to ASM_FUNC() asm macro
>   ArmPlatformPkg/ArmPlatformLibNull: switch to ASM_FUNC() asm macro
>   ArmPlatformPkg/ArmPlatformStackLib: switch to ASM_FUNC() asm macro
> 
>  ArmPkg/ArmPkg.dsc
> |   4 +
>  ArmPkg/Drivers/ArmCpuLib/ArmCortexA5xLib/AArch64/ArmCortexA5xHelper.S
> |   9 +-
>  ArmPkg/Drivers/ArmCpuLib/ArmCortexA9Lib/ArmCortexA9Helper.S  
> |   9 +-
>  ArmPkg/Drivers/ArmGic/GicV3/AArch64/ArmGicV3.S   
> |  28 +-
>  ArmPkg/Drivers/ArmGic/GicV3/Arm/ArmGicV3.S   
> |  28 +-
>  ArmPkg/Include/AsmMacroIoLib.h   
> | 232 ++--
>  ArmPkg/Include/AsmMacroIoLib.inc 
> |  54 
>  ArmPkg/Include/AsmMacroIoLibV8.h 
> |  20 +-
>  ArmPkg/Library/ArmHvcLib/AArch64/ArmHvc.S
> |   9 +-
>  ArmPkg/Library/ArmHvcLib/Arm/ArmHvc.S
> |  10 +-
>  

[edk2] [PATCH 1/6] Platforms/BeagleBoard: remove unreferenced Sec.inf module

[Ard Biesheuvel]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S   |  85 
--
 Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm |  89 
--
 Platforms/TexasInstruments/BeagleBoard/Sec/Cache.c  |  80 
--
 Platforms/TexasInstruments/BeagleBoard/Sec/Clock.c  |  70 -
 Platforms/TexasInstruments/BeagleBoard/Sec/LzmaDecompress.h | 103 
---
 Platforms/TexasInstruments/BeagleBoard/Sec/PadConfiguration.c   | 282 

 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.c| 186 
-
 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.inf  |  74 -
 8 files changed, 969 deletions(-)

diff --git a/Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S 
b/Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S
deleted file mode 100644
index b656c1e040c5..
--- a/Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S
+++ /dev/null
@@ -1,85 +0,0 @@
-#--
-#
-# Copyright (c) 2008 - 2009, Apple Inc. 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.
-#
-#--
-
-#include 
-#include 
-
-.text
-.align 3
-
-.globl ASM_PFX(CEntryPoint)
-GCC_ASM_EXPORT(_ModuleEntryPoint)
-
-ASM_PFX(_ModuleEntryPoint):
-
-  //Disable L2 cache
-  mrc p15, 0, r0, c1, c0, 1   // read Auxiliary Control Register
-  bic r0, r0, #0x0002 // disable L2 cache
-  mcr p15, 0, r0, c1, c0, 1   // store Auxiliary Control Register
-
-  //Enable Strict alignment checking & Instruction cache
-  mrc p15, 0, r0, c1, c0, 0
-  bic r0, r0, #0x2300 /* clear bits 13, 9:8 (--V- --RS) */
-  bic r0, r0, #0x0005 /* clear bits 0, 2 ( -C-M) */
-  orr r0, r0, #0x0002 /* set bit 1 (A) Align */
-  orr r0, r0, #0x1000 /* set bit 12 (I) enable I-Cache */
-  mcr p15, 0, r0, c1, c0, 0
-
-  // Enable NEON register in case folks want to use them for optimizations 
(CopyMem)
-  mrc p15, 0, r0, c1, c0, 2
-  orr r0, r0, #0x00f0   // Enable VPF access (V* instructions)
-  mcr p15, 0, r0, c1, c0, 2
-  mov r0, #0x4000   // Set EN bit in FPEXC
-  mcr p10,#0x7,r0,c8,c0,#0  // msr FPEXC,r0 in ARM assembly
-
-
-  // Set CPU vectors to start of DRAM
-  LoadConstantToReg (FixedPcdGet32(PcdCpuVectorBaseAddress) ,r0) // Get vector 
base
-  mcr p15, 0, r0, c12, c0, 0
-  isb   // Sync changes to control registers
-
-  // Fill vector table with branchs to current pc (jmp $)
-  ldr r1, ShouldNeverGetHere
-  movsr2, #0
-FillVectors:
-  str r1, [r0, r2]
-  addsr2, r2, #4
-  cmp r2, #32
-  bne FillVectors
-
-  /* before we call C code, lets setup the stack pointer in internal RAM */
-stack_pointer_setup:
-
-  //
-  // Set stack based on PCD values. Need to do it this way to make C code work
-  // when it runs from FLASH.
-  //
-  LoadConstantToReg (FixedPcdGet32(PcdPrePiStackBase) ,r2)/* stack base 
arg2  */
-  LoadConstantToReg (FixedPcdGet32(PcdPrePiStackSize) ,r3)/* stack size 
arg3  */
-  add r4, r2, r3
-
-  //Enter SVC mode and set up SVC stack pointer
-  mov r0,#0x13|0x80|0x40
-  msr CPSR_c,r0
-  mov r13,r4
-
-  // Call C entry point
-  LoadConstantToReg (FixedPcdGet32(PcdMemorySize) ,r1)/* memory size arg1  
*/
-  LoadConstantToReg (FixedPcdGet32(PcdMemoryBase) ,r0)/* memory size arg0  
   */
-  blx  ASM_PFX(CEntryPoint) /* Assume C code is thumb*/
-
-ShouldNeverGetHere:
-  /* _CEntryPoint should never return */
-  b   ShouldNeverGetHere
-
diff --git 
a/Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm 
b/Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm
deleted file mode 100644
index 63174d4b8437..
--- a/Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm
+++ /dev/null
@@ -1,89 +0,0 @@
-//--
-//
-// Copyright (c) 2008 - 2009, Apple Inc. 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
-// 

[edk2] [PATCH 3/6] Platforms/Styx: remove unused AmdStyxSecLib

[Ard Biesheuvel]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc|   1 -
 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S|  73 
 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S | 182 

 Platforms/AMD/Styx/Library/AmdStyxSecLib/AmdStyxSecLib.inf  |  50 --
 Platforms/AMD/Styx/Library/AmdStyxSecLib/StyxSec.c  |  96 ---
 Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc|   1 -
 6 files changed, 403 deletions(-)

diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc 
b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
index 6b9446a1387d..01ad69708237 100644
--- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
+++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc
@@ -154,7 +154,6 @@ DEFINE DO_KCS= 0
 
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
   ArmLib|ArmPkg/Library/ArmLib/AArch64/AArch64LibSec.inf
-  
ArmPlatformSecLib|OpenPlatformPkg/Platforms/AMD/Styx/Library/AmdStyxSecLib/AmdStyxSecLib.inf
   
ArmPlatformLib|OpenPlatformPkg/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf
 
   
ArmPlatformSecExtraActionLib|ArmPlatformPkg/Library/DebugSecExtraActionLib/DebugSecExtraActionLib.inf
diff --git a/Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S 
b/Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S
deleted file mode 100644
index 617a6ff0cff0..
--- a/Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S
+++ /dev/null
@@ -1,73 +0,0 @@
-//
-//  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
-//  Copyright (c) 2014 - 2016, AMD Inc. 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.
-//
-//**
-//  Derived from:
-//   
ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibRTSM/AArch64/GicV3.S
-//
-//**
-
-
-#include 
-
-// Register definitions used by GCC for GICv3 access.
-// These are defined by ARMCC, so keep them in the GCC specific code for now.
-#define ICC_SRE_EL2 S3_4_C12_C9_5
-#define ICC_SRE_EL3 S3_6_C12_C12_5
-#define ICC_CTLR_EL1S3_0_C12_C12_4
-#define ICC_CTLR_EL3S3_6_C12_C12_4
-#define ICC_PMR_EL1 S3_0_C4_C6_0
-
-.text
-.align 3
-
-GCC_ASM_EXPORT(InitializeGicV3)
-
-/* Initialize GICv3 to expose it as a GICv2 as UEFI does not support GICv3 yet 
*/
-ASM_PFX(InitializeGicV3):
-  // We have a GICv3. UEFI still uses the GICv2 mode. We must do enough setup
-  // to allow Linux to use GICv3 if it chooses.
-
-  // In order to setup NS side we need to enable it first.
-  mrs x0, scr_el3
-  orr x0, x0, #1
-  msr scr_el3, x0
-
-  // Enable SRE at EL3 and ICC_SRE_EL2 access
-  mov x0, #((1 << 3) | (1 << 0))  // Enable | SRE
-  mrs x1, ICC_SRE_EL3
-  orr x1, x1, x0
-  msr ICC_SRE_EL3, x1
-  isb
-
-  // Enable SRE at EL2 and ICC_SRE_EL1 access..
-  mrs x1, ICC_SRE_EL2
-  orr x1, x1, x0
-  msr ICC_SRE_EL2, x1
-  isb
-
-  // Configure CPU interface
-  msr ICC_CTLR_EL3, xzr
-  isb
-  msr ICC_CTLR_EL1, xzr
-  isb
-
-  // The MemoryMap view and Register view may not be consistent, So Set PMR 
again.
-  mov w1, #1 << 7// allow NS access to GICC_PMR
-  msr ICC_PMR_EL1, x1
-  isb
-
-  // Remove the SCR.NS bit
-  mrs x0, scr_el3
-  bic x0, x0, #1
-  msr scr_el3, x0
-  ret
diff --git a/Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S 
b/Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S
deleted file mode 100644
index c9e316366eb6..
--- a/Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S
+++ /dev/null
@@ -1,182 +0,0 @@
-//
-//  Copyright (c) 2011 - 2014, ARM Limited. All rights reserved.
-//  Copyright (c) 2014 - 2016, AMD Inc. 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.
-//
-//**
-//  Derived from:
-//   
ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibRTSM/AArch64/RTSMBoot.S
-//
-//**
-
-#include 
-#include 
-#include 
-#include 
-
-.text
-.align 3
-
-GCC_ASM_EXPORT(ArmPlatformSecBootAction)
-GCC_ASM_EXPORT(ArmPlatformSecBootMemoryInit)
-GCC_ASM_EXPORT(ArmSecMpCoreSecondariesWrite)

[edk2] [PATCH 5/6] Platforms/Hisilicon/ArmPlatformLibPv660: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

Annotate functions with ASM_FUNC() so that they are emitted into
separate sections.

While we're at it, clean up some inefficient uses of LoadConstantToReg()
(which require runtime relocation).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S | 44 
+---
 1 file changed, 10 insertions(+), 34 deletions(-)

diff --git a/Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S 
b/Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S
index 3b545f188f64..3422df279c73 100644
--- a/Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S
+++ b/Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S
@@ -16,59 +16,35 @@
 //
 
 #include 
-#include 
 #include 
-#include 
-#include 
 
-.text
-.align 2
-
-GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
-GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
-GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
-GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
-GCC_ASM_EXPORT(ArmGetCpuCountPerCluster)
-
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCore)
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask)
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdCoreCount)
-
-ASM_PFX(ArmPlatformPeiBootAction):
+ASM_FUNC(ArmPlatformPeiBootAction)
   ret
 
 //UINTN
 //ArmPlatformGetPrimaryCoreMpId (
 //  VOID
 //  );
-ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCore, x0)
-  ldrh   w0, [x0]
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
+  MOV32 (w0, FixedPcdGet32(PcdArmPrimaryCore))
   ret
 
 # IN None
 # OUT x0 = number of cores present in the system
-ASM_PFX(ArmGetCpuCountPerCluster):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdCoreCount, x0)
-  ldrh  w0, [x0]
+ASM_FUNC(ArmGetCpuCountPerCluster)
+  MOV32 (w0, FixedPcdGet32(PcdCoreCount))
   ret
 
 //UINTN
 //ArmPlatformIsPrimaryCore (
 //  IN UINTN MpId
 //  );
-ASM_PFX(ArmPlatformIsPrimaryCore):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask, x1)
-  ldrh  w1, [x1]
+ASM_FUNC(ArmPlatformIsPrimaryCore)
+  MOV32 (w1, FixedPcdGet32(PcdArmPrimaryCoreMask))
   and   x0, x0, x1
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCore, x1)
-  ldrh  w1, [x1]
+  MOV32 (w1, FixedPcdGet32(PcdArmPrimaryCore))
   cmp   w0, w1
-  b.ne  1f
-  mov   x0, #1
-  ret
-1:
-  mov   x0, #0
+  cset  x0, eq
   ret
 
 //UINTN
@@ -76,7 +52,7 @@ ASM_PFX(ArmPlatformIsPrimaryCore):
 //  IN UINTN MpId
 //  );
 // With this function: CorePos = (ClusterId * 4) + CoreId
-ASM_PFX(ArmPlatformGetCorePosition):
+ASM_FUNC(ArmPlatformGetCorePosition)
   and   x1, x0, #ARM_CORE_MASK
   and   x0, x0, #ARM_CLUSTER_MASK
   add   x0, x1, x0, LSR #6
-- 
2.7.4

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

[edk2] [PATCH 4/6] Platforms/Styx: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

Annotate functions with ASM_FUNC() so that they are emitted into
separate sections.

While we're at it, clean up some inefficient uses of LoadConstantToReg()
and other indirect absolute references (which require runtime relocation).
Since this requires fixed PCDs to be declared as such in the module .inf,
update those as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S  | 58 
++--
 Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf|  7 ++-
 Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf |  1 +
 3 files changed, 21 insertions(+), 45 deletions(-)

diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S 
b/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S
index 0f9822a86989..a3ac60282706 100644
--- a/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S
+++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S
@@ -17,25 +17,8 @@
 #   ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMHelper.S
 #
 #**/
-
 #include 
-#include 
 #include 
-#include 
-#include 
-
-.text
-.align 2
-
-GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
-GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
-GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
-GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
-GCC_ASM_EXPORT(ArmGetCpuCountPerCluster)
-
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCore)
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask)
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdCoreCount)
 
 PrimaryCoreMpid:  .word0x0
 PrimaryCoreBoot:  .word0x0
@@ -44,18 +27,15 @@ PrimaryCoreBoot:  .word0x0
 //ArmPlatformPeiBootAction (
 //  VOID
 //  );
-ASM_PFX(ArmPlatformPeiBootAction):
+ASM_FUNC(ArmPlatformPeiBootAction)
+  ldr  w0, PrimaryCoreBoot
+  cbnz w0, 1f
+
   // Save the primary CPU MPID
-  ldr  x1, =PrimaryCoreBoot
-  ldrh w0, [x1]
-  cmp  wzr, w0
-  b.ne 1f
   mrs  x0, mpidr_el1
-  ldr  x1, =PrimaryCoreMpid
-  str  w0, [x1]
-  mov  w0, 1
-  ldr  x1, =PrimaryCoreBoot
-  str  w0, [x1]
+  adr  x2, PrimaryCoreMpid
+  mov  w1, #1
+  stp  w0, w1, [x2]
 1:
   ret
 
@@ -63,34 +43,28 @@ ASM_PFX(ArmPlatformPeiBootAction):
 //ArmPlatformGetPrimaryCoreMpId (
 //  VOID
 //  );
-ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
-  ldr   x0, =PrimaryCoreMpid
-  ldrh  w0, [x0]
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
+  ldr   w0, PrimaryCoreMpid
   ret
 
 # IN None
 # OUT x0 = number of cores present in the system
-ASM_PFX(ArmGetCpuCountPerCluster):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdCoreCount, x0)
-  ldrh  w0, [x0]
+ASM_FUNC(ArmGetCpuCountPerCluster)
+  MOV32 (w0, FixedPcdGet32 (PcdCoreCount))
   ret
 
 //UINTN
 //ArmPlatformIsPrimaryCore (
 //  IN UINTN MpId
 //  );
-ASM_PFX(ArmPlatformIsPrimaryCore):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask, x1)
-  ldrh  w1, [x1]
+ASM_FUNC(ArmPlatformIsPrimaryCore)
+  MOV32 (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
   and   x0, x0, x1
 
-  ldr   x1, =PrimaryCoreMpid
-  ldrh  w1, [x1]
+  ldr   w1, PrimaryCoreMpid
 
   cmp   w0, w1
-  mov   x0, #1
-  mov   x1, #0
-  csel  x0, x0, x1, eq
+  cset  x0, eq
   ret
 
 //UINTN
@@ -98,7 +72,7 @@ ASM_PFX(ArmPlatformIsPrimaryCore):
 //  IN UINTN MpId
 //  );
 // With this function: CorePos = (ClusterId * 2) + CoreId
-ASM_PFX(ArmPlatformGetCorePosition):
+ASM_FUNC(ArmPlatformGetCorePosition)
   and   x1, x0, #ARM_CORE_MASK
   and   x0, x0, #ARM_CLUSTER_MASK
   add   x0, x1, x0, LSR #7
diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf 
b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf
index c6e42e4482fe..4a6469ee016c 100644
--- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf
+++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf
@@ -63,14 +63,15 @@
   gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmTokenSpaceGuid.PcdFdBaseAddress
 
+  gAmdStyxTokenSpaceGuid.PcdTrustedFWMemoryBase
+  gAmdStyxTokenSpaceGuid.PcdTrustedFWMemorySize
+
+[FixedPcd]
   gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
   gArmTokenSpaceGuid.PcdArmPrimaryCore
 
   gArmPlatformTokenSpaceGuid.PcdCoreCount
   gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase
 
-  gAmdStyxTokenSpaceGuid.PcdTrustedFWMemoryBase
-  gAmdStyxTokenSpaceGuid.PcdTrustedFWMemorySize
-
 [Depex]
   gAmdStyxPlatInitPpiGuid
diff --git a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf 
b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf
index fefd3ee6999b..0b9b6287168d 100644
--- a/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf
+++ b/Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf
@@ -61,6 +61,7 @@
   gArmTokenSpaceGuid.PcdSystemMemorySize
   gArmTokenSpaceGuid.PcdFvBaseAddress
 
+[FixedPcd]
   gArmTokenSpaceGuid.PcdArmPrimaryCoreMask
   gArmTokenSpaceGuid.PcdArmPrimaryCore
 
-- 
2.7.4

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

[edk2] [PATCH 6/6] Platforms/Marvell/Armada70x0Lib: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

Annotate functions with ASM_FUNC() so that they are emitted into
separate sections.

While we're at it, clean up some inefficient uses of LoadConstantToReg()
(which require runtime relocation).

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S | 
32 +---
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git 
a/Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
b/Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
index 4ad435c9d044..9265636ca5cf 100644
--- a/Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
+++ b/Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S
@@ -15,18 +15,7 @@
 #include 
 #include 
 
-.text
-.align 2
-
-GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
-GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
-GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
-GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
-
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCore)
-GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask)
-
-ASM_PFX(ArmPlatformPeiBootAction):
+ASM_FUNC(ArmPlatformPeiBootAction)
   ret
 
 //UINTN
@@ -34,7 +23,7 @@ ASM_PFX(ArmPlatformPeiBootAction):
 //  IN UINTN MpId
 //  );
 // With this function: CorePos = (ClusterId * 4) + CoreId
-ASM_PFX(ArmPlatformGetCorePosition):
+ASM_FUNC(ArmPlatformGetCorePosition)
   and   x1, x0, #ARM_CORE_MASK
   and   x0, x0, #ARM_CLUSTER_MASK
   add   x0, x1, x0, LSR #6
@@ -44,23 +33,18 @@ ASM_PFX(ArmPlatformGetCorePosition):
 //ArmPlatformGetPrimaryCoreMpId (
 //  VOID
 //  );
-ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCore, x0)
-  ldrh  w0, [x0]
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
+  MOV32 (w0, FixedPcdGet32(PcdArmPrimaryCore))
   ret
 
 //UINTN
 //ArmPlatformIsPrimaryCore (
 //  IN UINTN MpId
 //  );
-ASM_PFX(ArmPlatformIsPrimaryCore):
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask, x1)
-  ldrh  w1, [x1]
+ASM_FUNC(ArmPlatformIsPrimaryCore)
+  MOV32 (w1, FixedPcdGet32(PcdArmPrimaryCoreMask))
   and   x0, x0, x1
-  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCore, x1)
-  ldrh  w1, [x1]
+  MOV32 (w1, FixedPcdGet32(PcdArmPrimaryCore))
   cmp   w0, w1
-  mov   x0, #1
-  mov   x1, #0
-  csel  x0, x0, x1, eq
+  cset  x0, eq
   ret
-- 
2.7.4

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

[edk2] [PATCH 2/6] Platforms/BeagleBoard/BeagleBoardLib: switch to ASM_FUNC() asm macro

[Ard Biesheuvel]

Annotate functions with ASM_FUNC() so that they are emitted into
separate sections.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 
Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
 | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git 
a/Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
 
b/Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
index f15792546448..958f0029935c 100644
--- 
a/Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
+++ 
b/Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
@@ -12,34 +12,24 @@
 #
 
 #include 
-#include 
-
-.text
-.align 2
-
-GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
-GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
-GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
-
-GCC_ASM_IMPORT(ArmReadMpidr)
 
 //UINTN
 //ArmPlatformIsPrimaryCore (
 //  IN UINTN MpId
 //  );
-ASM_PFX(ArmPlatformIsPrimaryCore):
+ASM_FUNC(ArmPlatformIsPrimaryCore)
   // BeagleBoard has a single core. We must always return 1.
   mov   r0, #1
   bxlr
 
-ASM_PFX(ArmPlatformPeiBootAction):
+ASM_FUNC(ArmPlatformPeiBootAction)
   bxlr
 
 //UINTN
 //ArmPlatformGetPrimaryCoreMpId (
 //  VOID
 //  );
-ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
+ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
   // The BeagleBoard is a uniprocessor platform. The MPIDR of primary core is
   // always the MPIDR of the calling CPU.
   b   ASM_PFX(ArmReadMpidr)
-- 
2.7.4

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

[edk2] [PATCH 0/6] Move to ASM_FUNC() macro, and clean up some asm as well

[Ard Biesheuvel]

This moves the asm files in OpenPlatformPkg to the new ASM_FUNC() macro,
which annotates functions in a way that allows the linker to drop code
that is not actually used anywhere. It is analogous to -ffunction-sections
for GCC.

Since there are some cargo culted asm patterns that are very clunky and
inefficient, clean those up as well (i.e., LoadConstantToReg())

Ard Biesheuvel (6):
  Platforms/BeagleBoard: remove unreferenced Sec.inf module
  Platforms/BeagleBoard/BeagleBoardLib: switch to ASM_FUNC() asm macro
  Platforms/Styx: remove unused AmdStyxSecLib
  Platforms/Styx: switch to ASM_FUNC() asm macro
  Platforms/Hisilicon/ArmPlatformLibPv660: switch to ASM_FUNC() asm
macro
  Platforms/Marvell/Armada70x0Lib: switch to ASM_FUNC() asm macro

 Chips/Hisilicon/Library/ArmPlatformLibPv660/AArch64/Helper.S   
   |  44 +--
 Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc   
   |   1 -
 Platforms/AMD/Styx/Library/AmdStyxLib/AArch64/Helper.S 
   |  58 ++--
 Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLib.inf   
   |   7 +-
 Platforms/AMD/Styx/Library/AmdStyxLib/AmdStyxLibSec.inf
   |   1 +
 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S   
   |  73 -
 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S
   | 182 -
 Platforms/AMD/Styx/Library/AmdStyxSecLib/AmdStyxSecLib.inf 
   |  50 
 Platforms/AMD/Styx/Library/AmdStyxSecLib/StyxSec.c 
   |  96 ---
 Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc   
   |   1 -
 Platforms/Marvell/Armada/Library/Armada70x0Lib/AArch64/ArmPlatformHelper.S 
   |  32 +--
 
Platforms/TexasInstruments/BeagleBoard/Library/BeagleBoardLib/BeagleBoardHelper.S
 |  16 +-
 Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S  
   |  85 --
 Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm
   |  89 --
 Platforms/TexasInstruments/BeagleBoard/Sec/Cache.c 
   |  80 --
 Platforms/TexasInstruments/BeagleBoard/Sec/Clock.c 
   |  70 -
 Platforms/TexasInstruments/BeagleBoard/Sec/LzmaDecompress.h
   | 103 ---
 Platforms/TexasInstruments/BeagleBoard/Sec/PadConfiguration.c  
   | 282 
 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.c   
   | 186 -
 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.inf 
   |  74 -
 20 files changed, 42 insertions(+), 1488 deletions(-)
 delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/GicV3.S
 delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/AArch64/StyxBoot.S
 delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/AmdStyxSecLib.inf
 delete mode 100644 Platforms/AMD/Styx/Library/AmdStyxSecLib/StyxSec.c
 delete mode 100644 
Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.S
 delete mode 100644 
Platforms/TexasInstruments/BeagleBoard/Sec/Arm/ModuleEntryPoint.asm
 delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Cache.c
 delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Clock.c
 delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/LzmaDecompress.h
 delete mode 100644 
Platforms/TexasInstruments/BeagleBoard/Sec/PadConfiguration.c
 delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.c
 delete mode 100644 Platforms/TexasInstruments/BeagleBoard/Sec/Sec.inf

-- 
2.7.4

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

[edk2] [PATCH v2] MdeModulePkg/Browser: Enhance the logic when getting value from AltResp

[Dandan Bi]

This patch is to enhance SetupBrowser to handle following two cases:
1. When searching BlockName in AltResp, the hex digits of related BlockName
   in AltResp may be in uppercase.
2. When converting the Value in AltResp to HiiValue, the length of value
   string is bigger than the length of StorageWidth of the question.

Notes:
V2:
- Add some comments to make the logic clear.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 
---
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 61 +++---
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index 66c4313..cd3c8cc 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -1427,44 +1427,48 @@ BufferToValue (
   }
   TempChar = *StringPtr;
   *StringPtr = L'\0';
 
   LengthStr = StrLen (Value);
+
+  //
+  // Value points to a Unicode hexadecimal string, we need to convert the 
string to the value with CHAR16/UINT8...type.
+  // When generating the Value string, we follow this rule: 1 byte -> 2 
Unicode characters (for string: 2 byte(CHAR16) ->4 Unicode characters).
+  // So the maximum value string length of a question is : 
Question->StorageWidth * 2.
+  // If the value string length > Question->StorageWidth * 2, only set the 
string length as Question->StorageWidth * 2, then convert.
+  //
+  if (LengthStr > (UINTN) Question->StorageWidth * 2) {
+Length = (UINTN) Question->StorageWidth * 2;
+  } else {
+Length = LengthStr;
+  }
+
   Status= EFI_SUCCESS;
   if (!IsBufferStorage && IsString) {
 //
 // Convert Config String to Unicode String, e.g "0041004200430044" => 
"ABCD"
 // Add string tail char L'\0' into Length
 //
-Length= Question->StorageWidth + sizeof (CHAR16);
-if (Length < ((LengthStr / 4 + 1) * 2)) {
-  Status = EFI_BUFFER_TOO_SMALL;
-} else {
-  DstBuf = (CHAR16 *) Dst;
-  ZeroMem (TemStr, sizeof (TemStr));
-  for (Index = 0; Index < LengthStr; Index += 4) {
-StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
-DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
-  }
-  //
-  // Add tailing L'\0' character
-  //
-  DstBuf[Index/4] = L'\0';
+DstBuf = (CHAR16 *) Dst;
+ZeroMem (TemStr, sizeof (TemStr));
+for (Index = 0; Index < Length; Index += 4) {
+  StrnCpyS (TemStr, sizeof (TemStr) / sizeof (CHAR16), Value + Index, 4);
+  DstBuf[Index/4] = (CHAR16) StrHexToUint64 (TemStr);
 }
+//
+// Add tailing L'\0' character
+//
+DstBuf[Index/4] = L'\0';
   } else {
-if (Question->StorageWidth < ((LengthStr + 1) / 2)) {
-  Status = EFI_BUFFER_TOO_SMALL;
-} else {
-  ZeroMem (TemStr, sizeof (TemStr));
-  for (Index = 0; Index < LengthStr; Index ++) {
-TemStr[0] = Value[LengthStr - Index - 1];
-DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
-if ((Index & 1) == 0) {
-  Dst [Index/2] = DigitUint8;
-} else {
-  Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
-}
+ZeroMem (TemStr, sizeof (TemStr));
+for (Index = 0; Index < Length; Index ++) {
+  TemStr[0] = Value[LengthStr - Index - 1];
+  DigitUint8 = (UINT8) StrHexToUint64 (TemStr);
+  if ((Index & 1) == 0) {
+Dst [Index/2] = DigitUint8;
+  } else {
+Dst [Index/2] = (UINT8) ((DigitUint8 << 4) + Dst [Index/2]);
   }
 }
   }
 
   *StringPtr = TempChar;
@@ -3755,10 +3759,15 @@ GetOffsetFromConfigResp (
   //
   // Type is EFI_HII_VARSTORE_EFI_VARIABLE or 
EFI_HII_VARSTORE_EFI_VARIABLE_BUFFER
   //
 
   //
+  // Convert all hex digits in ConfigResp to lower case before searching.
+  //
+  HiiToLower (ConfigResp);
+
+  //
   // 1. Directly use Question->BlockName to find.
   //
   RequestElement = StrStr (ConfigResp, Question->BlockName);
   if (RequestElement != NULL) {
 //
-- 
1.9.5.msysgit.1

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

Re: [edk2] [patch] MdeModulePkg: Refine codes of iSCSI driver

[Dong, Eric]

Reviewed-by:  Eric Dong 

> -Original Message-
> From: Zhang, Lubo
> Sent: Thursday, August 11, 2016 3:19 PM
> To: Dong, Eric
> Cc: Ye, Ting; Fu, Siyuan; edk2-devel@lists.01.org
> Subject: RE: [edk2] [patch] MdeModulePkg: Refine codes of iSCSI driver
> 
> Hi Eric
> Do you have any comments for this patch.
> 
> Best regards
> Lubo
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang 
> Lubo
> Sent: Thursday, August 11, 2016 10:28 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Dong, 
> Eric 
> Subject: [edk2] [patch] MdeModulePkg: Refine codes of iSCSI driver
> 
> The RSDT is only used when the bios need to support ACPI 1.0 version. When 
> change PcdAcpiExposedTableVersions to 0x3C, it will not
> support ACPI 1.0. The default is 0x3E.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Eric Dong 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> ---
>  .../Universal/Network/IScsiDxe/IScsiIbft.c | 35 
> --
>  1 file changed, 20 insertions(+), 15 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c 
> b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c
> index e5f685f..45d89a6 100644
> --- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c
> +++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c
> @@ -434,42 +434,42 @@ IScsiPublishIbft (
>EFI_ACPI_ISCSI_BOOT_FIRMWARE_TABLE_HEADER *Table;
>UINTN HandleCount;
>EFI_HANDLE*HandleBuffer;
>UINT8 *Heap;
>UINT8 Checksum;
> -  UINTN Index;
>EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
>EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
> +  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt;
> +
> +  Rsdt = NULL;
> +  Xsdt = NULL;
> 
>Status = gBS->LocateProtocol (, NULL, (VOID 
> **));
>if (EFI_ERROR (Status)) {
>  return ;
>}
> 
> 
>//
>// Find ACPI table RSD_PTR from system table
>//
> -  for (Index = 0, Rsdp = NULL; Index < gST->NumberOfTableEntries; Index++) {
> -if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
> ) ||
> -  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
> ) ||
> -  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
> )
> -  ) {
> -  //
> -  // A match was found.
> -  //
> -  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *) 
> gST->ConfigurationTable[Index].VendorTable;
> -  break;
> -}
> +  Status = EfiGetSystemConfigurationTable (, (VOID
> + **) );  if (EFI_ERROR (Status)) {
> +Status = EfiGetSystemConfigurationTable (,
> + (VOID **) );
>}
> 
> -  if (Rsdp == NULL) {
> +  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>  return ;
> -  } else {
> +  } else if (Rsdp->Revision >= 
> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 
> 0) {
> +Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress;
> + } else if (Rsdp->RsdtAddress != 0) {
>  Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress;
>}
> 
> +  if ((Xsdt == NULL) && (Rsdt == NULL)) {
> +return ;
> +  }
> 
>if (mIbftInstalled) {
>  Status = AcpiTableProtocol->UninstallAcpiTable (
>AcpiTableProtocol,
>mTableKey @@ -504,11 +504,16 @@ 
> IScsiPublishIbft (
>Heap = (UINT8 *) Table + IBFT_HEAP_OFFSET;
> 
>//
>// Fill in the various section of the iSCSI Boot Firmware Table.
>//
> -  IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);
> +  if (Rsdp->Revision >= 
> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) {
> +IScsiInitIbfTableHeader (Table, Xsdt->OemId, >OemTableId);  }
> + else {
> +IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);  }
> +
>IScsiInitControlSection (Table, HandleCount);
>IScsiFillInitiatorSection (Table, , HandleBuffer[0]);
>IScsiFillNICAndTargetSections (Table, , HandleCount, HandleBuffer);
> 
>Checksum = CalculateCheckSum8((UINT8 *)Table, Table->Length);
> --
> 1.9.5.msysgit.1
> 
> ___
> 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 26/26] ArmPlatformPkg/ArmPlatformStackLib: switch to ASM_FUNC() asm macro

[Leif Lindholm]

On Wed, Aug 10, 2016 at 05:18:02PM +0200, Ard Biesheuvel wrote:
> Annotate functions with ASM_FUNC() so that they are emitted into
> separate sections.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S | 
> 35 +---
>  ArmPlatformPkg/Library/ArmPlatformStackLib/Arm/ArmPlatformStackLib.S | 
> 25 +++---
>  2 files changed, 12 insertions(+), 48 deletions(-)
> 
> diff --git 
> a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S 
> b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> index 485017f62013..65d7d6c6d686 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> +++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/AArch64/ArmPlatformStackLib.S
> @@ -12,21 +12,6 @@
>  //
>  
>  #include 
> -#include 
> -#include 
> -
> -.text
> -.align 3
> -
> -GCC_ASM_EXPORT(ArmPlatformStackSet)
> -GCC_ASM_EXPORT(ArmPlatformStackSetPrimary)
> -GCC_ASM_EXPORT(ArmPlatformStackSetSecondary)
> -
> -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore)
> -GCC_ASM_IMPORT(ArmPlatformGetCorePosition)
> -GCC_ASM_IMPORT(ArmPlatformGetPrimaryCoreMpId)
> -
> -GCC_ASM_IMPORT(gPcd_FixedAtBuild_PcdCoreCount)
>  
>  //VOID
>  //ArmPlatformStackSet (
> @@ -35,7 +20,7 @@ GCC_ASM_IMPORT(gPcd_FixedAtBuild_PcdCoreCount)
>  //  IN UINTN PrimaryStackSize,
>  //  IN UINTN SecondaryStackSize
>  //  );
> -ASM_PFX(ArmPlatformStackSet):
> +ASM_FUNC(ArmPlatformStackSet)
>// Save parameters
>mov   x6, x3
>mov   x5, x2
> @@ -59,10 +44,10 @@ ASM_PFX(ArmPlatformStackSet):
>// Restore the Link register
>mov   x30, x7
>  
> -  // Should be ASM_PFX(ArmPlatformStackSetPrimary) but generate linker error 
> 'unsupported ELF EM_AARCH64'
> -  b.eq  ArmPlatformStackSetPrimaryL
> -  // Should be ASM_PFX(ArmPlatformStackSetSecondary) but generate linker 
> error 'unsupported ELF EM_AARCH64'

Worth mentioning removing this hack as well?
Other than that -
Also replacing LoadConstantToReg. Add that to commit message and:
Reviewed-by: Leif Lindholm 

> -  b.ne  ArmPlatformStackSetSecondaryL
> +  b.ne  0f
> +
> +  b ASM_PFX(ArmPlatformStackSetPrimary)
> +0:b ASM_PFX(ArmPlatformStackSetSecondary)
>  
>  //VOID
>  //ArmPlatformStackSetPrimary (
> @@ -71,8 +56,7 @@ ASM_PFX(ArmPlatformStackSet):
>  //  IN UINTN PrimaryStackSize,
>  //  IN UINTN SecondaryStackSize
>  //  );
> -ArmPlatformStackSetPrimaryL:
> -ASM_PFX(ArmPlatformStackSetPrimary):
> +ASM_FUNC(ArmPlatformStackSetPrimary)
>// Save the Link register
>mov   x4, x30
>  
> @@ -80,9 +64,7 @@ ASM_PFX(ArmPlatformStackSetPrimary):
>add   x0, x0, x2
>  
>// Compute SecondaryCoresCount * SecondaryCoreStackSize
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdCoreCount, x1)
> -  ldr   w1, [x1]
> -  sub   x1, x1, #1
> +  MOV32 (w1, FixedPcdGet32(PcdCoreCount) - 1)
>mul   x3, x3, x1
>  
>// Set Primary Stack ((StackBase + PrimaryStackSize) + 
> (SecondaryCoresCount * SecondaryCoreStackSize))
> @@ -97,8 +79,7 @@ ASM_PFX(ArmPlatformStackSetPrimary):
>  //  IN UINTN PrimaryStackSize,
>  //  IN UINTN SecondaryStackSize
>  //  );
> -ArmPlatformStackSetSecondaryL:
> -ASM_PFX(ArmPlatformStackSetSecondary):
> +ASM_FUNC(ArmPlatformStackSetSecondary)
>// Save the Link register
>mov   x4, x30
>mov   sp, x0
> diff --git 
> a/ArmPlatformPkg/Library/ArmPlatformStackLib/Arm/ArmPlatformStackLib.S 
> b/ArmPlatformPkg/Library/ArmPlatformStackLib/Arm/ArmPlatformStackLib.S
> index 96e925981fca..bdd7a27b7cf9 100644
> --- a/ArmPlatformPkg/Library/ArmPlatformStackLib/Arm/ArmPlatformStackLib.S
> +++ b/ArmPlatformPkg/Library/ArmPlatformStackLib/Arm/ArmPlatformStackLib.S
> @@ -12,21 +12,6 @@
>  //
>  
>  #include 
> -#include 
> -#include 
> -
> -.text
> -.align 3
> -
> -GCC_ASM_EXPORT(ArmPlatformStackSet)
> -GCC_ASM_EXPORT(ArmPlatformStackSetPrimary)
> -GCC_ASM_EXPORT(ArmPlatformStackSetSecondary)
> -
> -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore)
> -GCC_ASM_IMPORT(ArmPlatformGetCorePosition)
> -GCC_ASM_IMPORT(ArmPlatformGetPrimaryCoreMpId)
> -
> -GCC_ASM_IMPORT(gPcd_FixedAtBuild_PcdCoreCount)
>  
>  //VOID
>  //ArmPlatformStackSet (
> @@ -35,7 +20,7 @@ GCC_ASM_IMPORT(gPcd_FixedAtBuild_PcdCoreCount)
>  //  IN UINTN PrimaryStackSize,
>  //  IN UINTN SecondaryStackSize
>  //  );
> -ASM_PFX(ArmPlatformStackSet):
> +ASM_FUNC(ArmPlatformStackSet)
>// Save parameters
>mov   r6, r3
>mov   r5, r2
> @@ -69,16 +54,14 @@ ASM_PFX(ArmPlatformStackSet):
>  //  IN UINTN PrimaryStackSize,
>  //  IN UINTN SecondaryStackSize
>  //  );
> -ASM_PFX(ArmPlatformStackSetPrimary):
> +ASM_FUNC(ArmPlatformStackSetPrimary)
>mov   r4, lr
>  
>// Add stack of primary stack to StackBase
>add   r0, r0, r2
>  
>// Compute SecondaryCoresCount * SecondaryCoreStackSize
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdCoreCount, r1)
> -  

Re: [edk2] [PATCH 24/26] ArmPlatformPkg/ArmVExpressPkg: switch to ASM_FUNC() asm macro

[Leif Lindholm]

On Wed, Aug 10, 2016 at 05:18:00PM +0200, Ard Biesheuvel wrote:
> Annotate functions with ASM_FUNC() so that they are emitted into
> separate sections.

Also replacing LoadConstantToReg. Add that to commit message and:
Reviewed-by: Leif Lindholm 

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  
> ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA15-A7/CTA15-A7Helper.S 
> | 22 ---
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA9x4/CTA9x4Helper.S
>  | 28 -
>  
> ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/AArch64/RTSMHelper.S 
> | 38 +-
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/Arm/RTSMHelper.S
>  | 41 ++--
>  ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressSecLibCTA9x4/CTA9x4Boot.S   
>  | 23 ---
>  5 files changed, 41 insertions(+), 111 deletions(-)
> 
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA15-A7/CTA15-A7Helper.S
>  
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA15-A7/CTA15-A7Helper.S
> index 20bfe52610e3..3719a5ace604 100644
> --- 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA15-A7/CTA15-A7Helper.S
> +++ 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA15-A7/CTA15-A7Helper.S
> @@ -16,22 +16,14 @@
>  
>  #include 
>  
> -.text
> -.align 2
> -
> -GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> -GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> -GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> -GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> -
> -ASM_PFX(ArmPlatformPeiBootAction):
> +ASM_FUNC(ArmPlatformPeiBootAction)
>bxlr
>  
>  //UINTN
>  //ArmPlatformGetCorePosition (
>  //  IN UINTN MpId
>  //  );
> -ASM_PFX(ArmPlatformGetCorePosition):
> +ASM_FUNC(ArmPlatformGetCorePosition)
>and   r1, r0, #ARM_CORE_MASK
>and   r0, r0, #ARM_CLUSTER_MASK
>add   r0, r1, r0, LSR #7
> @@ -41,10 +33,10 @@ ASM_PFX(ArmPlatformGetCorePosition):
>  //ArmPlatformIsPrimaryCore (
>  //  IN UINTN MpId
>  //  );
> -ASM_PFX(ArmPlatformIsPrimaryCore):
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
>// Extract cpu_id and cluster_id from ARM_SCC_CFGREG48
>// with cpu_id[0:3] and cluster_id[4:7]
> -  LoadConstantToReg (ARM_CTA15A7_SCC_CFGREG48, r1)
> +  MOV32 (r1, ARM_CTA15A7_SCC_CFGREG48)
>ldr   r1, [r1]
>lsr   r1, #24
>  
> @@ -58,7 +50,7 @@ ASM_PFX(ArmPlatformIsPrimaryCore):
>orr   r1, r1, r2
>  
>// Keep the Cluster ID and Core ID from the MPID
> -  LoadConstantToReg (ARM_CLUSTER_MASK | ARM_CORE_MASK, r2)
> +  MOV32 (r2, ARM_CLUSTER_MASK | ARM_CORE_MASK)
>and   r0, r0, r2
>  
>// Compare mpid and boot cpu from ARM_SCC_CFGREG48
> @@ -71,10 +63,10 @@ ASM_PFX(ArmPlatformIsPrimaryCore):
>  //ArmPlatformGetPrimaryCoreMpId (
>  //  VOID
>  //  );
> -ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
>// Extract cpu_id and cluster_id from ARM_SCC_CFGREG48
>// with cpu_id[0:3] and cluster_id[4:7]
> -  LoadConstantToReg (ARM_CTA15A7_SCC_CFGREG48, r0)
> +  MOV32 (r0, ARM_CTA15A7_SCC_CFGREG48)
>ldr   r0, [r0]
>lsr   r0, #24
>  
> diff --git 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA9x4/CTA9x4Helper.S 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA9x4/CTA9x4Helper.S
> index c4aee741a602..f95d2f43d665 100644
> --- 
> a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA9x4/CTA9x4Helper.S
> +++ 
> b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibCTA9x4/CTA9x4Helper.S
> @@ -14,36 +14,22 @@
>  #include 
>  #include 
>  
> -.text
> -.align 2
> -
> -GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> -GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> -GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> -GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> -
> -GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCore)
> -GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask)
> -
>  //UINTN
>  //ArmPlatformGetPrimaryCoreMpId (
>  //  VOID
>  //  );
> -ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCore, r0)
> -  ldr   r0, [r0]
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  MOV32  (r0, FixedPcdGet32 (PcdArmPrimaryCore))
>bxlr
>  
>  //UINTN
>  //ArmPlatformIsPrimaryCore (
>  //  IN UINTN MpId
>  //  );
> -ASM_PFX(ArmPlatformIsPrimaryCore):
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask, r1)
> -  ldr   r1, [r1]
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32  (r1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
>and   r0, r0, r1
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCore, r1)
> -  ldr   r1, [r1]
> +  MOV32  (r1, FixedPcdGet32 (PcdArmPrimaryCore))
>cmp   r0, r1
>moveq r0, #1
>movne r0, #0
> @@ -53,11 +39,11 @@ ASM_PFX(ArmPlatformIsPrimaryCore):
>  //ArmPlatformGetCorePosition (
>  //  IN UINTN MpId
>  //  );
> -ASM_PFX(ArmPlatformGetCorePosition):
> 

[edk2] [PATCH 0/4] Fix GCC Minnowboard FSP build

[Gary Lin]

This series of patches fixes the build errors when using GCC with
"MINNOW2_FSP_BUILD = TRUE".

I followed the release notes(*) and the coreboot wiki(**) to generate
Vlv2MiscBinariesPkg/FspBinary/FvFsp.bin. Although the firmware image was built,
it didn't work for me.

Any suggestions are welcome.

(*) 
https://firmware.intel.com/sites/default/files/MinnowBoard_MAX-Rel_0_93-ReleaseNotes.txt
(**) http://wiki.minnowboard.org/Coreboot

Gary Lin (4):
  Vlv2TbltDevicePkg/FspSupport: Fix GCC build errors
  Vlv2TbltDevicePkg/SecFspPlatformSecLibVlv2: Add assembly code for GCC
  Vlv2TbltDevicePkg/PlatformFspLib: Fix the include path
  Vlv2TbltDevicePkg: Add RAW file type to Rule.Common.SEC.BINARY

 .../PeiFspHobProcessLibVlv2/FspHobProcessLibVlv2.c |   5 +-
 .../FspPlatformSecLibVlv2.inf  |  13 +-
 .../Ia32/AsmSaveSecContext.S   |  43 +++
 .../Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h|  48 +++
 .../SecFspPlatformSecLibVlv2/Ia32/PeiCoreEntry.S   | 130 
 .../SecFspPlatformSecLibVlv2/Ia32/SecEntry.S   | 328 +
 .../SecFspPlatformSecLibVlv2/PlatformInit.c|   4 +-
 .../Library/SecFspPlatformSecLibVlv2/UartInit.c|   6 +-
 .../Library/PlatformFspLib/PlatformFspLib.c|   2 +-
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf   |   6 +-
 10 files changed, 571 insertions(+), 14 deletions(-)
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/AsmSaveSecContext.S
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/PeiCoreEntry.S
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/SecEntry.S

-- 
2.9.2

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

Re: [edk2] [PATCH 22/26] ArmPlatformPkg/PrePi: switch to ASM_FUNC() asm macro

[Leif Lindholm]

On Wed, Aug 10, 2016 at 05:17:58PM +0200, Ard Biesheuvel wrote:
> Annotate functions with ASM_FUNC() so that they are emitted into
> separate sections.

Also replacing LoadConstantToReg. Add that to commit message and:
Reviewed-by: Leif Lindholm 

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S | 49 ++-
>  ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S | 50 ++--
>  2 files changed, 29 insertions(+), 70 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S 
> b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> index 9538c70a237c..d0530a874726 100644
> --- a/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> +++ b/ArmPlatformPkg/PrePi/AArch64/ModuleEntryPoint.S
> @@ -12,24 +12,10 @@
>  //
>  
>  #include 
> -#include 
> -#include 
> -#include 
> -
> -.text
> -.align 3
> -
> -GCC_ASM_IMPORT(ArmPlatformIsPrimaryCore)
> -GCC_ASM_IMPORT(ArmReadMpidr)
> -GCC_ASM_IMPORT(ArmPlatformPeiBootAction)
> -GCC_ASM_IMPORT(ArmPlatformStackSet)
> -GCC_ASM_EXPORT(_ModuleEntryPoint)
> -ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
>  
> -StartupAddr:  .8byte ASM_PFX(CEntryPoint)
> -ASM_PFX(mSystemMemoryEnd):.8byte 0
> +ASM_GLOBAL ASM_PFX(mSystemMemoryEnd)
>  
> -ASM_PFX(_ModuleEntryPoint):
> +ASM_FUNC(_ModuleEntryPoint)
>// Do early platform specific actions
>blASM_PFX(ArmPlatformPeiBootAction)
>  
> @@ -49,10 +35,8 @@ _SystemMemoryEndInit:
>cmp   x1, #0
>bne   _SetupStackPosition
>  
> -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemoryBase), x1)
> -  LoadConstantToReg (FixedPcdGet64(PcdSystemMemorySize), x2)
> -  sub   x2, x2, #1
> -  add   x1, x1, x2
> +  MOV64 (x1, FixedPcdGet64(PcdSystemMemoryBase) + 
> FixedPcdGet64(PcdSystemMemorySize) - 1)
> +
>// Update the global variable
>adr   x2, mSystemMemoryEnd
>str   x1, [x2]
> @@ -61,13 +45,13 @@ _SetupStackPosition:
>// r1 = SystemMemoryTop
>  
>// Calculate Top of the Firmware Device
> -  LoadConstantToReg (FixedPcdGet64(PcdFdBaseAddress), x2)
> -  LoadConstantToReg (FixedPcdGet32(PcdFdSize), x3)
> +  MOV64 (x2, FixedPcdGet64(PcdFdBaseAddress))
> +  MOV32 (x3, FixedPcdGet32(PcdFdSize) - 1)
>sub   x3, x3, #1
>add   x3, x3, x2  // x3 = FdTop = PcdFdBaseAddress + PcdFdSize
>  
>// UEFI Memory Size (stacks are allocated in this region)
> -  LoadConstantToReg (FixedPcdGet32(PcdSystemMemoryUefiRegionSize), x4)
> +  MOV32 (x4, FixedPcdGet32(PcdSystemMemoryUefiRegionSize))
>  
>//
>// Reserve the memory for the UEFI region (contain stacks on its top)
> @@ -98,9 +82,7 @@ _SetupAlignedStack:
>  _SetupOverflowStack:
>// Case memory at the top of the address space. Ensure the top of the 
> stack is EFI_PAGE_SIZE
>// aligned (4KB)
> -  LoadConstantToReg (EFI_PAGE_MASK, x11)
> -  and   x11, x11, x1
> -  sub   x1, x1, x11
> +  and   x1, x1, ~EFI_PAGE_MASK
>  
>  _GetBaseUefiMemory:
>// Calculate the Base of the UEFI Memory
> @@ -109,22 +91,19 @@ _GetBaseUefiMemory:
>  _GetStackBase:
>// r1 = The top of the Mpcore Stacks
>// Stack for the primary core = PrimaryCoreStack
> -  LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2)
> +  MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize))
>sub   x12, x1, x2
>  
>// Stack for the secondary core = Number of Cores - 1
> -  LoadConstantToReg (FixedPcdGet32(PcdCoreCount), x0)
> -  sub   x0, x0, #1
> -  LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x1)
> -  mul   x1, x1, x0
> +  MOV32 (x1, (FixedPcdGet32(PcdCoreCount) - 1) * 
> FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
>sub   x12, x12, x1
>  
>// x12 = The base of the MpCore Stacks (primary stack & secondary stacks)
>mov   x0, x12
>mov   x1, x10
>//ArmPlatformStackSet(StackBase, MpId, PrimaryStackSize, 
> SecondaryStackSize)
> -  LoadConstantToReg (FixedPcdGet32(PcdCPUCorePrimaryStackSize), x2)
> -  LoadConstantToReg (FixedPcdGet32(PcdCPUCoreSecondaryStackSize), x3)
> +  MOV32 (x2, FixedPcdGet32(PcdCPUCorePrimaryStackSize))
> +  MOV32 (x3, FixedPcdGet32(PcdCPUCoreSecondaryStackSize))
>blASM_PFX(ArmPlatformStackSet)
>  
>// Is it the Primary Core ?
> @@ -140,7 +119,7 @@ _PrepareArguments:
>  
>// Move sec startup address into a data register
>// Ensure we're jumping to FV version of the code (not boot remapped alias)
> -  ldr   x4, StartupAddr
> +  ldr   x4, =ASM_PFX(CEntryPoint)
>  
>// Jump to PrePiCore C code
>//x0 = MpId
> @@ -150,3 +129,5 @@ _PrepareArguments:
>  
>  _NeverReturn:
>b _NeverReturn
> +
> +ASM_PFX(mSystemMemoryEnd):.8byte 0
> diff --git a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S 
> b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> index 1311efc5cb2c..b7127ce9fb4c 100644
> --- a/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> +++ b/ArmPlatformPkg/PrePi/Arm/ModuleEntryPoint.S
> @@ 

[edk2] [PATCH 3/4] Vlv2TbltDevicePkg/PlatformFspLib: Fix the include path

[Gary Lin]

Cc: David Wei 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c 
b/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c
index 1306399..747b6a9 100644
--- a/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c
+++ b/Vlv2TbltDevicePkg/Library/PlatformFspLib/PlatformFspLib.c
@@ -14,7 +14,7 @@
 **/
 #include "PiPei.h"
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
-- 
2.9.2

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

[edk2] [PATCH 4/4] Vlv2TbltDevicePkg: Add RAW file type to Rule.Common.SEC.BINARY

[Gary Lin]

For GCC, it's using the reset vector in
IntelFspWrapperPkg/FspWrapperSecCore/Vtf0/Bin/ResetVec.ia32.raw.
Add the RAW file type to Rule.Common.SEC.BINARY in PlatformPkgGcc.fdf,
so that GenFds can handle the raw file.

Cc: David Wei 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 Vlv2TbltDevicePkg/PlatformPkgGcc.fdf | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf 
b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
index e9292ca..33f2038 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
+++ b/Vlv2TbltDevicePkg/PlatformPkgGcc.fdf
@@ -839,7 +839,11 @@ [Rule.Common.SEC]
 [Rule.Common.SEC.BINARY]
   FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
 PE32  PE32Align = 8   |.efi
-RAW BIN   Align = 16  |.com
+!if $(MINNOW2_FSP_BUILD) == TRUE
+RAW   RAW |.raw
+!else
+RAW   BIN Align = 16  |.com
+!endif
   }
 
 [Rule.Common.PEI_CORE]
-- 
2.9.2

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

[edk2] [PATCH 2/4] Vlv2TbltDevicePkg/SecFspPlatformSecLibVlv2: Add assembly code for GCC

[Gary Lin]

The original *.asm files are for Visual Studio. Copy the GAS assembly
code from IntelFspWrapperPkg/Library/SecPeiFspPlatformSecLibSample/Ia32/
so that GCC can compile those code.

Cc: David Wei 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Gary Lin 
---
 .../FspPlatformSecLibVlv2.inf  |  13 +-
 .../Ia32/AsmSaveSecContext.S   |  43 +++
 .../Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h|  48 +++
 .../SecFspPlatformSecLibVlv2/Ia32/PeiCoreEntry.S   | 130 
 .../SecFspPlatformSecLibVlv2/Ia32/SecEntry.S   | 328 +
 5 files changed, 558 insertions(+), 4 deletions(-)
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/AsmSaveSecContext.S
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/PeiCoreEntry.S
 create mode 100644 
Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/SecEntry.S

diff --git 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/FspPlatformSecLibVlv2.inf
 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/FspPlatformSecLibVlv2.inf
index 3d9b135..0b01cad 100644
--- 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/FspPlatformSecLibVlv2.inf
+++ 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/FspPlatformSecLibVlv2.inf
@@ -47,10 +47,15 @@ [Sources]
   UartInit.c
 
 [Sources.IA32]
-  Ia32/SecEntry.asm
-  Ia32/PeiCoreEntry.asm
-  Ia32/AsmSaveSecContext.asm
-  Ia32/Stack.asm
+  Ia32/SecEntry.asm | MSFT
+  Ia32/PeiCoreEntry.asm | MSFT
+  Ia32/AsmSaveSecContext.asm | MSFT
+  Ia32/Stack.asm | MSFT
+
+  Ia32/SecEntry.S | GCC
+  Ia32/PeiCoreEntry.S | GCC
+  Ia32/AsmSaveSecContext.S | GCC
+  Ia32/Stack.S | GCC
 
 

 #
diff --git 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/AsmSaveSecContext.S
 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/AsmSaveSecContext.S
new file mode 100644
index 000..3838cc8
--- /dev/null
+++ 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/AsmSaveSecContext.S
@@ -0,0 +1,43 @@
+#--
+#
+# Copyright (c) 2014, 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.
+#
+# Module Name:
+#
+#  AsmSaveSecContext.S
+#
+# Abstract:
+#
+#   Save Sec Conext before call FspInit API
+#
+#--
+
+#
+#  MMX Usage:
+#  MM0 = BIST State
+#  MM5 = Save time-stamp counter value high32bit
+#  MM6 = Save time-stamp counter value low32bit.
+#
+#  It should be same as SecEntry.asm and PeiCoreEntry.asm.
+#
+
+ASM_GLOBAL ASM_PFX(AsmSaveBistValue)
+ASM_PFX(AsmSaveBistValue):
+  movl4(%esp), %eax
+  movd%eax, %mm0
+  ret
+
+ASM_GLOBAL ASM_PFX(AsmSaveTickerValue)
+ASM_PFX(AsmSaveTickerValue):
+  movl4(%esp), %eax
+  movd%eax, %mm6
+  movl8(%esp), %eax
+  movd%eax, %mm5
+  ret
diff --git 
a/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h 
b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h
new file mode 100644
index 000..e145b4e
--- /dev/null
+++ b/Vlv2TbltDevicePkg/FspSupport/Library/SecFspPlatformSecLibVlv2/Ia32/Fsp.h
@@ -0,0 +1,48 @@
+/** @file
+  Fsp related definitions
+
+  Copyright (c) 2014, 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.
+
+**/
+
+#ifndef __FSP_H__
+#define __FSP_H__
+
+//
+// Fv Header
+//
+#define FVH_SIGINATURE_OFFSET 0x28
+#define FVH_SIGINATURE_VALID_VALUE0x4856465F  // valid signature:_FVH
+#define FVH_HEADER_LENGTH_OFFSET  0x30
+#define FVH_EXTHEADER_OFFSET_OFFSET   0x34
+#define FVH_EXTHEADER_SIZE_OFFSET 0x10
+
+//
+// 

Re: [edk2] [PATCH 21/26] ArmPlatformPkg/ArmJunoLib: switch to ASM_FUNC() asm macro

[Leif Lindholm]

On Wed, Aug 10, 2016 at 05:17:57PM +0200, Ard Biesheuvel wrote:
> Annotate functions with ASM_FUNC() so that they are emitted into
> separate sections.

Also replacing LoadConstantToReg. Add that to commit message and:
Reviewed-by: Leif Lindholm 

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/AArch64/ArmJunoHelper.S | 37 
> ++--
>  ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/Arm/ArmJunoHelper.S | 36 
> ++-
>  2 files changed, 21 insertions(+), 52 deletions(-)
> 
> diff --git 
> a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/AArch64/ArmJunoHelper.S 
> b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/AArch64/ArmJunoHelper.S
> index 73b249ca5ffd..4bdf08d1a98a 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/AArch64/ArmJunoHelper.S
> +++ b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/AArch64/ArmJunoHelper.S
> @@ -15,25 +15,12 @@
>  #include 
>  #include 
>  
> -.text
> -.align 3
> -
> -GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> -GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> -GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> -GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> -
> -GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask)
> -
> -
> -PrimaryCoreMpid:  .word0x0
> -
>  //UINTN
>  //ArmPlatformGetCorePosition (
>  //  IN UINTN MpId
>  //  );
>  // With this function: CorePos = (ClusterId * 2) + CoreId
> -ASM_PFX(ArmPlatformGetCorePosition):
> +ASM_FUNC(ArmPlatformGetCorePosition)
>and   x1, x0, #ARM_CORE_MASK
>and   x0, x0, #ARM_CLUSTER_MASK
>add   x0, x1, x0, LSR #7
> @@ -43,33 +30,29 @@ ASM_PFX(ArmPlatformGetCorePosition):
>  //ArmPlatformGetPrimaryCoreMpId (
>  //  VOID
>  //  );
> -ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
> -  ldr   x0, =PrimaryCoreMpid
> -  ldrh  w0, [x0]
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  ldr   w0, PrimaryCoreMpid
>ret
>  
>  //UINTN
>  //ArmPlatformIsPrimaryCore (
>  //  IN UINTN MpId
>  //  );
> -ASM_PFX(ArmPlatformIsPrimaryCore):
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask, x1)
> -  ldrh  w1, [x1]
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32 (w1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
>and   x0, x0, x1
>  
> -  ldr   x1, =PrimaryCoreMpid
> -  ldrh  w1, [x1]
> +  ldr   w1, PrimaryCoreMpid
>  
>cmp   w0, w1
> -  mov   x0, #1
> -  mov   x1, #0
> -  csel  x0, x0, x1, eq
> +  cset  x0, eq
>ret
>  
> -ASM_PFX(ArmPlatformPeiBootAction):
> +ASM_FUNC(ArmPlatformPeiBootAction)
>// The trusted firmware passes the primary CPU MPID through x0 register.
>// Save it in a variable.
> -  ldr  x1, =PrimaryCoreMpid
> +  adr  x1, PrimaryCoreMpid
>str  w0, [x1]
>ret
>  
> +PrimaryCoreMpid:  .word0x0
> diff --git a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/Arm/ArmJunoHelper.S 
> b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/Arm/ArmJunoHelper.S
> index 2efb5451b88b..a7e904eac697 100644
> --- a/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/Arm/ArmJunoHelper.S
> +++ b/ArmPlatformPkg/ArmJunoPkg/Library/ArmJunoLib/Arm/ArmJunoHelper.S
> @@ -12,22 +12,9 @@
>  *
>  **/
>  
> -#include 
> +#include 
>  #include 
>  
> -.text
> -.align 3
> -
> -GCC_ASM_EXPORT(ArmPlatformPeiBootAction)
> -GCC_ASM_EXPORT(ArmPlatformGetCorePosition)
> -GCC_ASM_EXPORT(ArmPlatformGetPrimaryCoreMpId)
> -GCC_ASM_EXPORT(ArmPlatformIsPrimaryCore)
> -
> -GCC_ASM_IMPORT(_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask)
> -
> -
> -PrimaryCoreMpid:  .word0x0
> -
>  //
>  // Return the core position from the value of its MpId register
>  //
> @@ -41,7 +28,7 @@ PrimaryCoreMpid:  .word0x0
>  //  IN UINTN MpId
>  //  );
>  // With this function: CorePos = (ClusterId * 2) + CoreId
> -ASM_PFX(ArmPlatformGetCorePosition):
> +ASM_FUNC(ArmPlatformGetCorePosition)
>and   r1, r0, #ARM_CORE_MASK
>and   r0, r0, #ARM_CLUSTER_MASK
>add   r0, r1, r0, LSR #7
> @@ -59,9 +46,8 @@ ASM_PFX(ArmPlatformGetCorePosition):
>  //ArmPlatformGetPrimaryCoreMpId (
>  //  VOID
>  //  );
> -ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
> -  ldr   r0, =PrimaryCoreMpid
> -  ldr   r0, [r0]
> +ASM_FUNC(ArmPlatformGetPrimaryCoreMpId)
> +  LDRL  (r0, PrimaryCoreMpid)
>bxlr
>  
>  //
> @@ -77,13 +63,11 @@ ASM_PFX(ArmPlatformGetPrimaryCoreMpId):
>  //ArmPlatformIsPrimaryCore (
>  //  IN UINTN MpId
>  //  );
> -ASM_PFX(ArmPlatformIsPrimaryCore):
> -  LoadConstantToReg (_gPcd_FixedAtBuild_PcdArmPrimaryCoreMask, r1)
> -  ldr   r1, [r1]
> +ASM_FUNC(ArmPlatformIsPrimaryCore)
> +  MOV32 (r1, FixedPcdGet32 (PcdArmPrimaryCoreMask))
>and   r0, r0, r1
>  
> -  ldr   r1, =PrimaryCoreMpid
> -  ldr   r1, [r1]
> +  LDRL  (r1, PrimaryCoreMpid)
>  
>cmp   r0, r1
>moveq r0, #1
> @@ -97,9 +81,11 @@ ASM_PFX(ArmPlatformIsPrimaryCore):
>  // or PrePeiCore modules. It allows to retrieve arguments passed to
>  // the UEFI firmware through the CPU registers.
>  //
> 

Re: [edk2] [PATCH 19/26] BeagleBoardPkg: remove unused Sec.inf module

[Leif Lindholm]

CC:ing Andrew, who wrote it.

On Wed, Aug 10, 2016 at 05:17:55PM +0200, Ard Biesheuvel wrote:
> This module is not referenced anywhere, so remove it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.S   |  85 --
>  BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.asm |  89 --
>  BeagleBoardPkg/Sec/Cache.c  |  79 --
>  BeagleBoardPkg/Sec/Clock.c  |  70 -
>  BeagleBoardPkg/Sec/LzmaDecompress.h | 103 ---
>  BeagleBoardPkg/Sec/PadConfiguration.c   | 282 
>  BeagleBoardPkg/Sec/Sec.c| 186 -
>  BeagleBoardPkg/Sec/Sec.inf  |  73 -
>  8 files changed, 967 deletions(-)
> 
> diff --git a/BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.S 
> b/BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.S
> deleted file mode 100644
> index b656c1e040c5..
> --- a/BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.S
> +++ /dev/null
> @@ -1,85 +0,0 @@
> -#--
> -#
> -# Copyright (c) 2008 - 2009, Apple Inc. 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.
> -#
> -#--
> -
> -#include 
> -#include 
> -
> -.text
> -.align 3
> -
> -.globl ASM_PFX(CEntryPoint)
> -GCC_ASM_EXPORT(_ModuleEntryPoint)
> -
> -ASM_PFX(_ModuleEntryPoint):
> -
> -  //Disable L2 cache
> -  mrc p15, 0, r0, c1, c0, 1   // read Auxiliary Control Register
> -  bic r0, r0, #0x0002 // disable L2 cache
> -  mcr p15, 0, r0, c1, c0, 1   // store Auxiliary Control Register
> -
> -  //Enable Strict alignment checking & Instruction cache
> -  mrc p15, 0, r0, c1, c0, 0
> -  bic r0, r0, #0x2300 /* clear bits 13, 9:8 (--V- --RS) */
> -  bic r0, r0, #0x0005 /* clear bits 0, 2 ( -C-M) */
> -  orr r0, r0, #0x0002 /* set bit 1 (A) Align */
> -  orr r0, r0, #0x1000 /* set bit 12 (I) enable I-Cache */
> -  mcr p15, 0, r0, c1, c0, 0
> -
> -  // Enable NEON register in case folks want to use them for optimizations 
> (CopyMem)
> -  mrc p15, 0, r0, c1, c0, 2
> -  orr r0, r0, #0x00f0   // Enable VPF access (V* instructions)
> -  mcr p15, 0, r0, c1, c0, 2
> -  mov r0, #0x4000   // Set EN bit in FPEXC
> -  mcr p10,#0x7,r0,c8,c0,#0  // msr FPEXC,r0 in ARM assembly
> -
> -
> -  // Set CPU vectors to start of DRAM
> -  LoadConstantToReg (FixedPcdGet32(PcdCpuVectorBaseAddress) ,r0) // Get 
> vector base
> -  mcr p15, 0, r0, c12, c0, 0
> -  isb   // Sync changes to control registers
> -
> -  // Fill vector table with branchs to current pc (jmp $)
> -  ldr r1, ShouldNeverGetHere
> -  movsr2, #0
> -FillVectors:
> -  str r1, [r0, r2]
> -  addsr2, r2, #4
> -  cmp r2, #32
> -  bne FillVectors
> -
> -  /* before we call C code, lets setup the stack pointer in internal RAM */
> -stack_pointer_setup:
> -
> -  //
> -  // Set stack based on PCD values. Need to do it this way to make C code 
> work
> -  // when it runs from FLASH.
> -  //
> -  LoadConstantToReg (FixedPcdGet32(PcdPrePiStackBase) ,r2)/* stack base 
> arg2  */
> -  LoadConstantToReg (FixedPcdGet32(PcdPrePiStackSize) ,r3)/* stack size 
> arg3  */
> -  add r4, r2, r3
> -
> -  //Enter SVC mode and set up SVC stack pointer
> -  mov r0,#0x13|0x80|0x40
> -  msr CPSR_c,r0
> -  mov r13,r4
> -
> -  // Call C entry point
> -  LoadConstantToReg (FixedPcdGet32(PcdMemorySize) ,r1)/* memory size 
> arg1  */
> -  LoadConstantToReg (FixedPcdGet32(PcdMemoryBase) ,r0)/* memory size 
> arg0 */
> -  blx  ASM_PFX(CEntryPoint) /* Assume C code is thumb*/
> -
> -ShouldNeverGetHere:
> -  /* _CEntryPoint should never return */
> -  b   ShouldNeverGetHere
> -
> diff --git a/BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.asm 
> b/BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.asm
> deleted file mode 100644
> index 63174d4b8437..
> --- a/BeagleBoardPkg/Sec/Arm/ModuleEntryPoint.asm
> +++ /dev/null
> @@ -1,89 +0,0 @@
> -//--
> -//
> -// Copyright (c) 2008 - 2009, Apple Inc. 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. 

Re: [edk2] [PATCH 03/26] ArmPkg/AsmMacroIoLib: remove unused obsolete MMIO and other asm macros

[Leif Lindholm]

On Wed, Aug 10, 2016 at 07:26:46PM +0200, Ard Biesheuvel wrote:
> On 10 August 2016 at 19:04, Leif Lindholm  wrote:
> > On Wed, Aug 10, 2016 at 05:17:39PM +0200, Ard Biesheuvel wrote:
> >> This removes the various Mmio ASM macros that are not used anywhere in
> >> the code, and removes some variants of LoadConstant... () that are not
> >> used anywhere either.
> >
> > If you say something about how the Mmio* functions are redundant due
> > to the MdePkg implementations:
> 
> No, they are not. These are asm implementations, and completely unused.

Yes, but due to the unfortunate naming of these macros conflicting
with real functions existing in IoLib, this patch looks potentially
horrifying at first glance. Hence I would prefer that to be mentioned
in the commit message.

/
Leif

> > Reviewed-by: Leif Lindholm 
> >
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>  ArmPkg/Include/AsmMacroIoLib.h   | 213 
> >>  ArmPkg/Include/AsmMacroIoLib.inc |  54 -
> >>  2 files changed, 267 deletions(-)
> >>
> >> diff --git a/ArmPkg/Include/AsmMacroIoLib.h 
> >> b/ArmPkg/Include/AsmMacroIoLib.h
> >> index f94dcc619f7a..551b87803d19 100644
> >> --- a/ArmPkg/Include/AsmMacroIoLib.h
> >> +++ b/ArmPkg/Include/AsmMacroIoLib.h
> >> @@ -24,88 +24,6 @@
> >>  //  ldr reg, =expr does not work with current Apple tool chain. So do the 
> >> work our selves
> >>  //
> >>
> >> -// returns _Data in R0 and _Address in R1
> >> -#define MmioWrite32(_Address, _Data) \
> >> -  ldr  r1, [pc, #8] ;\
> >> -  ldr  r0, [pc, #8] ;\
> >> -  str  r0, [r1] ;\
> >> -  b1f   ;\
> >> -  .long (_Address)  ;\
> >> -  .long (_Data) ;\
> >> -1:
> >> -
> >> -// returns _Data in R0 and _Address in R1, and _OrData in r2
> >> -#define MmioOr32(_Address, _OrData) \
> >> -  ldr  r1, [pc, #16];   \
> >> -  ldr  r2, [pc, #16];   \
> >> -  ldr  r0, [r1] ;   \
> >> -  orr  r0, r0, r2   ;   \
> >> -  str  r0, [r1] ;   \
> >> -  b1f   ;   \
> >> -  .long (_Address)  ;   \
> >> -  .long (_OrData)   ;   \
> >> -1:
> >> -
> >> -// returns _Data in R0 and _Address in R1, and _OrData in r2
> >> -#define MmioAnd32(_Address, _AndData) \
> >> -  ldr  r1, [pc, #16]; \
> >> -  ldr  r2, [pc, #16]; \
> >> -  ldr  r0, [r1] ; \
> >> -  and  r0, r0, r2   ; \
> >> -  str  r0, [r1] ; \
> >> -  b1f   ; \
> >> -  .long (_Address)  ; \
> >> -  .long (_AndData)   ; \
> >> -1:
> >> -
> >> -// returns result in R0, _Address in R1, and _OrData in r2
> >> -#define MmioAndThenOr32(_Address, _AndData, _OrData)  \
> >> -  ldr  r1, [pc, #24]; \
> >> -  ldr  r0, [r1] ; \
> >> -  ldr  r2, [pc, #20]; \
> >> -  and  r0, r0, r2   ; \
> >> -  ldr  r2, [pc, #16]; \
> >> -  orr  r0, r0, r2   ; \
> >> -  str  r0, [r1] ; \
> >> -  b1f   ; \
> >> -  .long (_Address)  ; \
> >> -  .long (_AndData)  ; \
> >> -  .long (_OrData)   ; \
> >> -1:
> >> -
> >> -// returns _Data in _Reg and _Address in R1
> >> -#define MmioWriteFromReg32(_Address, _Reg) \
> >> -  ldr  r1, [pc, #4] ;  \
> >> -  str  _Reg, [r1]   ;  \
> >> -  b1f   ;  \
> >> -  .long (_Address)  ;  \
> >> -1:
> >> -
> >> -
> >> -// returns _Data in R0 and _Address in R1
> >> -#define MmioRead32(_Address)   \
> >> -  ldr  r1, [pc, #4] ;  \
> >> -  ldr  r0, [r1] ;  \
> >> -  b1f   ;  \
> >> -  .long (_Address)  ;  \
> >> -1:
> >> -
> >> -// returns _Data in Reg and _Address in R1
> >> -#define MmioReadToReg32(_Address, _Reg) \
> >> -  ldr  r1, [pc, #4] ;   \
> >> -  ldr  _Reg, [r1]   ;   \
> >> -  b1f   ;   \
> >> -  .long (_Address)  ;   \
> >> -1:
> >> -
> >> -
> >> -// load R0 with _Data
> >> -#define LoadConstant(_Data)  \
> >> -  ldr  r0, [pc, #0] ;\
> >> -  b1f   ;\
> >> -  .long (_Data) ;\
> >> -1:
> >> -
> >>  // load _Reg with _Data
> >>  #define LoadConstantToReg(_Data, _Reg)  \
> >>ldr  _Reg, [pc, #0]   ;   \
> >> @@ -113,91 +31,8 @@
> >>.long (_Data) ;   

Re: [edk2] [PATCH] CryptoPkg: Fix capitalization of path name in Patch-HOWTO.txt

[Wu, Jiaxin]

Commit version: 

SHA-1: 34a4babec8df5c1f5bf86f1cc83b3cc20016c62c

Reviewed-By: Wu Jiaxin 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Long, Qin
> Sent: Sunday, August 7, 2016 10:03 PM
> To: Thomas Huth ; edk2-de...@ml01.01.org
> Cc: Ye, Ting 
> Subject: Re: [edk2] [PATCH] CryptoPkg: Fix capitalization of path name in
> Patch-HOWTO.txt
> 
> Exactly. Thanks.
> 
> Reviewed-by: Qin Long 
> 
> 
> > -Original Message-
> > From: Thomas Huth [mailto:th...@redhat.com]
> > Sent: Saturday, August 6, 2016 4:51 AM
> > To: edk2-de...@ml01.01.org
> > Cc: Long, Qin ; Ye, Ting 
> > Subject: [PATCH] CryptoPkg: Fix capitalization of path name in
> > Patch-HOWTO.txt
> >
> > It's "OpensslLib", not "OpenSslLib" - not a big issue, but the typo is
> > annoying when trying to copy-n-paste the path name to use it on the
> > command line on Linux.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Thomas Huth 
> > ---
> >  CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> > b/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> > index f836736..91098b9 100644
> > --- a/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> > +++ b/CryptoPkg/Library/OpensslLib/Patch-HOWTO.txt
> > @@ -32,7 +32,7 @@ cryptography. This patch will enable openssl building
> under UEFI environment.
> >"openssl-1.0.2h.tar.gz" or rename the local downloaded file with
> ".tar.tar"
> >extension to ".tar.gz".
> >
> > -2.  Extract TAR into CryptoPkg/Library/OpenSslLib/openssl-1.0.2h
> > +2.  Extract TAR into CryptoPkg/Library/OpensslLib/openssl-1.0.2h
> >
> >  NOTE: If you use WinZip to unpack the openssl source in Windows,
> please
> >uncheck the WinZip smart CR/LF conversion option (WINZIP:
> > Options -->
> > --
> > 1.8.3.1
> 
> ___
> 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] CryptoPkg: Fix "responsiblity" typos

[Wu, Jiaxin]

Commit version: 

SHA-1: 210abffdca9015986a78d883647c85706fc39ee3

Reviewed-By: Wu Jiaxin 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Long, Qin
> Sent: Sunday, August 7, 2016 10:04 PM
> To: Thomas Huth ; edk2-de...@ml01.01.org
> Cc: Ye, Ting 
> Subject: Re: [edk2] [PATCH] CryptoPkg: Fix "responsiblity" typos
> 
> Reviewed-by: Qin Long 
> 
> Best Regards & Thanks,
> LONG, Qin
> 
> > -Original Message-
> > From: Thomas Huth [mailto:th...@redhat.com]
> > Sent: Saturday, August 6, 2016 5:00 AM
> > To: edk2-de...@ml01.01.org
> > Cc: Long, Qin ; Ye, Ting 
> > Subject: [PATCH] CryptoPkg: Fix "responsiblity" typos
> >
> > It's "responsibility", not "responsiblity".
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Thomas Huth 
> > ---
> >  CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c   | 10 
> > +
> -
> >  CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c   | 10 +-
> 
> >  .../BaseCryptLibRuntimeCryptProtocol/Pk/CryptPkcs7VerifyNull.c | 10
> > +-
> >  3 files changed, 15 insertions(+), 15 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
> > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
> > index efa3796..4dd1625 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7Verify.c
> > @@ -238,10 +238,10 @@ _Exit:
> >@param[in]  P7Data   Pointer to the PKCS#7 message to verify.
> >@param[in]  P7Length Length of the PKCS#7 message in bytes.
> >@param[out] CertStackPointer to Signer's certificates retrieved from
> P7Data.
> > -   It's caller's responsiblity to free the buffer.
> > +   It's caller's responsibility to free the buffer.
> >@param[out] StackLength  Length of signer's certificates in bytes.
> >@param[out] TrustedCert  Pointer to a trusted certificate from Signer's
> certificates.
> > -   It's caller's responsiblity to free the buffer.
> > +   It's caller's responsibility to free the buffer.
> >@param[out] CertLength   Length of the trusted certificate in bytes.
> >
> >@retval  TRUEThe operation is finished successfully.
> > @@ -436,10 +436,10 @@ Pkcs7FreeSigners (
> >@param[in]  P7DataPointer to the PKCS#7 message.
> >@param[in]  P7Length  Length of the PKCS#7 message in bytes.
> >@param[out] SignerChainCerts  Pointer to the certificates list chained to
> signer's
> > -certificate. It's caller's responsiblity 
> > to free the buffer.
> > +certificate. It's caller's responsibility 
> > to free the buffer.
> >@param[out] ChainLength   Length of the chained certificates list 
> > buffer
> in bytes.
> >@param[out] UnchainCerts  Pointer to the unchained certificates 
> > lists.
> It's caller's
> > -responsiblity to free the buffer.
> > +responsibility to free the buffer.
> >@param[out] UnchainLength Length of the unchained certificates list
> buffer in bytes.
> >
> >@retval  TRUE The operation is finished successfully.
> > @@ -905,7 +905,7 @@ _Exit:
> >@param[in]   P7Data   Pointer to the PKCS#7 signed data to process.
> >@param[in]   P7Length Length of the PKCS#7 signed data in bytes.
> >@param[out]  Content  Pointer to the extracted content from the
> PKCS#7 signedData.
> > -It's caller's responsiblity to free the buffer.
> > +It's caller's responsibility to free the 
> > buffer.
> >@param[out]  ContentSize  The size of the extracted content in bytes.
> >
> >@retval TRUE  The P7Data was correctly formatted for 
> > processing.
> > diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
> > b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
> > index 05433ff..d09fd54 100644
> > --- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
> > +++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyNull.c
> > @@ -25,10 +25,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> >@param[in]  P7Data   Pointer to the PKCS#7 message to verify.
> >@param[in]  P7Length Length of the PKCS#7 message in bytes.
> >@param[out] CertStackPointer to Signer's certificates retrieved from
> P7Data.
> > -   It's caller's responsiblity to free the buffer.
> > +   It's caller's responsibility to free the buffer.
> >@param[out] StackLength  Length of signer's certificates in 

Re: [edk2] [patch] MdeModulePkg: Refine codes of iSCSI driver

[Zhang, Lubo]

Hi Eric
Do you have any comments for this patch.

Best regards
Lubo 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang 
Lubo
Sent: Thursday, August 11, 2016 10:28 AM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Dong, Eric 

Subject: [edk2] [patch] MdeModulePkg: Refine codes of iSCSI driver

The RSDT is only used when the bios need to support ACPI 1.0 version. When 
change PcdAcpiExposedTableVersions to 0x3C, it will not support ACPI 1.0. The 
default is 0x3E.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Eric Dong 
Cc: Ye Ting 
Cc: Fu Siyuan 
---
 .../Universal/Network/IScsiDxe/IScsiIbft.c | 35 --
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c 
b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c
index e5f685f..45d89a6 100644
--- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c
+++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiIbft.c
@@ -434,42 +434,42 @@ IScsiPublishIbft (
   EFI_ACPI_ISCSI_BOOT_FIRMWARE_TABLE_HEADER *Table;
   UINTN HandleCount;
   EFI_HANDLE*HandleBuffer;
   UINT8 *Heap;
   UINT8 Checksum;
-  UINTN Index;
   EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
   EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
+  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt;
+
+  Rsdt = NULL;
+  Xsdt = NULL;
 
   Status = gBS->LocateProtocol (, NULL, (VOID 
**));
   if (EFI_ERROR (Status)) {
 return ;
   }
 
 
   //
   // Find ACPI table RSD_PTR from system table
   //
-  for (Index = 0, Rsdp = NULL; Index < gST->NumberOfTableEntries; Index++) {
-if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
)
-  ) {
-  //
-  // A match was found.
-  //
-  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *) 
gST->ConfigurationTable[Index].VendorTable;
-  break;
-}
+  Status = EfiGetSystemConfigurationTable (, (VOID 
+ **) );  if (EFI_ERROR (Status)) {
+Status = EfiGetSystemConfigurationTable (, 
+ (VOID **) );
   }
 
-  if (Rsdp == NULL) {
+  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
 return ;
-  } else {
+  } else if (Rsdp->Revision >= 
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 
0) {
+Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress;  
+ } else if (Rsdp->RsdtAddress != 0) {
 Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress;
   }
 
+  if ((Xsdt == NULL) && (Rsdt == NULL)) {
+return ;
+  }
 
   if (mIbftInstalled) {
 Status = AcpiTableProtocol->UninstallAcpiTable (
   AcpiTableProtocol,
   mTableKey @@ -504,11 +504,16 @@ 
IScsiPublishIbft (
   Heap = (UINT8 *) Table + IBFT_HEAP_OFFSET;
 
   //
   // Fill in the various section of the iSCSI Boot Firmware Table.
   //
-  IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);
+  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) 
{
+IScsiInitIbfTableHeader (Table, Xsdt->OemId, >OemTableId);  } 
+ else {
+IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);  }
+
   IScsiInitControlSection (Table, HandleCount);
   IScsiFillInitiatorSection (Table, , HandleBuffer[0]);
   IScsiFillNICAndTargetSections (Table, , HandleCount, HandleBuffer);
 
   Checksum = CalculateCheckSum8((UINT8 *)Table, Table->Length);
--
1.9.5.msysgit.1

___
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] NetworkPkg: Refine codes of iSCSI driver

[Zhang, Lubo]

Ok, I will update the log.

Thanks
Lubo 

-Original Message-
From: Ye, Ting 
Sent: Thursday, August 11, 2016 3:11 PM
To: Zhang, Lubo ; Dong, Eric 
Cc: Fu, Siyuan ; edk2-devel@lists.01.org
Subject: RE: [patch] NetworkPkg: Refine codes of iSCSI driver

So how about adding your below sentences to the log message when you check in? 
I think it is more clear why we introduce the change to iSCSI. 
Others are good to me.

Reviewed-by: Ye Ting  

-Original Message-
From: Zhang, Lubo
Sent: Thursday, August 11, 2016 3:01 PM
To: Ye, Ting ; Dong, Eric 
Cc: Fu, Siyuan ; edk2-devel@lists.01.org
Subject: RE: [patch] NetworkPkg: Refine codes of iSCSI driver

  From my understanding, the default PcdAcpiExposedTableVersions is 0x3E, 
it can support ACPI table above version 1.0.but when changing the PCD to 0x3C, 
table 1.0 will not be installed and supported . However  the original code 
   Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress 
..
   IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);
   always assume the Rsdt is not NULL which is used for table 1.0. So we 
should distinguish which table version we find. If table version above 1.0, we 
use the Rsdp->XsdtAddress instead of Rsdp->RsdtAddress.


Best Regards
Lubo 

-Original Message-
From: Ye, Ting
Sent: Thursday, August 11, 2016 2:14 PM
To: Zhang, Lubo ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Fu, Siyuan 
Subject: RE: [patch] NetworkPkg: Refine codes of iSCSI driver

Hi Lubo,

Could you please describe the changes introduced by this patch in your log 
message?

Thanks,
Ting

-Original Message-
From: Zhang, Lubo
Sent: Thursday, August 11, 2016 10:27 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Ye, Ting ; Fu, Siyuan 

Subject: [patch] NetworkPkg: Refine codes of iSCSI driver

The RSDT is only used when the bios need to support ACPI 1.0 version. When 
change PcdAcpiExposedTableVersions to 0x3C, it will not support ACPI 1.0. The 
default is 0x3E.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Eric Dong 
Cc: Ye Ting 
Cc: Fu Siyuan 
---
 NetworkPkg/IScsiDxe/IScsiIbft.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiIbft.c b/NetworkPkg/IScsiDxe/IScsiIbft.c 
index 3c179bf..27d098d 100644
--- a/NetworkPkg/IScsiDxe/IScsiIbft.c
+++ b/NetworkPkg/IScsiDxe/IScsiIbft.c
@@ -454,42 +454,42 @@ IScsiPublishIbft (
   EFI_STATUSStatus;
   EFI_ACPI_TABLE_PROTOCOL   *AcpiTableProtocol;
   EFI_ACPI_ISCSI_BOOT_FIRMWARE_TABLE_HEADER *Table;
   EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
   EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
+  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt;
   UINT8 *Heap;
   UINT8 Checksum;
-  UINTN Index;
 
+  Rsdt = NULL;
+  Xsdt = NULL;
 
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
   if (EFI_ERROR (Status)) {
 return ;
   }
 
   //
   // Find ACPI table RSD_PTR from the system table.
   //
-  for (Index = 0, Rsdp = NULL; Index < gST->NumberOfTableEntries; Index++) {
-if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
)
-  ) {
-  //
-  // A match was found.
-  //
-  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *) 
gST->ConfigurationTable[Index].VendorTable;
-  break;
-}
+  Status = EfiGetSystemConfigurationTable (, (VOID
+ **) );  if (EFI_ERROR (Status)) {
+Status = EfiGetSystemConfigurationTable (, 
+ (VOID **) );
   }
 
-  if (Rsdp == NULL) {
+  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
 return ;
-  } else {
+  } else if (Rsdp->Revision >= 
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 
0) {
+Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress; } 
+ else if (Rsdp->RsdtAddress != 0) {
 Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress;
   }
 
+  if ((Xsdt == NULL) && (Rsdt == NULL)) {
+return ;
+  }
+
   if (mIbftInstalled) {
 Status = AcpiTableProtocol->UninstallAcpiTable (
   AcpiTableProtocol,
   mTableKey
   );
@@ -518,11 +518,16 @@ IScsiPublishIbft (
   Heap = (UINT8 *) Table + IBFT_HEAP_OFFSET;
 
   //
   // Fill 

Re: [edk2] [patch] BaseTool/VfrCompile: Remove reset button opcode in CheckQuestionOpCode

[Dong, Eric]

Reviewed-by: Eric Dong 

> -Original Message-
> From: Bi, Dandan
> Sent: Wednesday, August 10, 2016 4:54 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming; Dong, Eric
> Subject: [patch] BaseTool/VfrCompile: Remove reset button opcode in 
> CheckQuestionOpCode
> 
> "EFI_IFR_RESET_BUTTON_OP" is a statement, not a question,
> so remove it from function CheckQuestionOpCode.
> 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp 
> b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
> index db1e4bd..0b7b8b1 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrFormPkg.cpp
> @@ -1199,11 +1199,10 @@ CIfrRecordInfoDB::CheckQuestionOpCode (
>case EFI_IFR_STRING_OP:
>case EFI_IFR_DATE_OP:
>case EFI_IFR_TIME_OP:
>case EFI_IFR_ORDERED_LIST_OP:
>case EFI_IFR_REF_OP:
> -  case EFI_IFR_RESET_BUTTON_OP:
>  return TRUE;
>default:
>  return FALSE;
>}
>  }
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [patch] NetworkPkg: Refine codes of iSCSI driver

[Ye, Ting]

So how about adding your below sentences to the log message when you check in? 
I think it is more clear why we introduce the change to iSCSI. 
Others are good to me.

Reviewed-by: Ye Ting  

-Original Message-
From: Zhang, Lubo 
Sent: Thursday, August 11, 2016 3:01 PM
To: Ye, Ting ; Dong, Eric 
Cc: Fu, Siyuan ; edk2-devel@lists.01.org
Subject: RE: [patch] NetworkPkg: Refine codes of iSCSI driver

  From my understanding, the default PcdAcpiExposedTableVersions is 0x3E, 
it can support ACPI table above version 1.0.but when changing the PCD to 0x3C, 
table 1.0 will not be installed and supported . However  the original code 
   Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress 
..
   IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);
   always assume the Rsdt is not NULL which is used for table 1.0. So we 
should distinguish which table version we find. If table version above 1.0, we 
use the Rsdp->XsdtAddress instead of Rsdp->RsdtAddress.


Best Regards
Lubo 

-Original Message-
From: Ye, Ting
Sent: Thursday, August 11, 2016 2:14 PM
To: Zhang, Lubo ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Fu, Siyuan 
Subject: RE: [patch] NetworkPkg: Refine codes of iSCSI driver

Hi Lubo,

Could you please describe the changes introduced by this patch in your log 
message?

Thanks,
Ting

-Original Message-
From: Zhang, Lubo
Sent: Thursday, August 11, 2016 10:27 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Ye, Ting ; Fu, Siyuan 

Subject: [patch] NetworkPkg: Refine codes of iSCSI driver

The RSDT is only used when the bios need to support ACPI 1.0 version. When 
change PcdAcpiExposedTableVersions to 0x3C, it will not support ACPI 1.0. The 
default is 0x3E.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Eric Dong 
Cc: Ye Ting 
Cc: Fu Siyuan 
---
 NetworkPkg/IScsiDxe/IScsiIbft.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiIbft.c b/NetworkPkg/IScsiDxe/IScsiIbft.c 
index 3c179bf..27d098d 100644
--- a/NetworkPkg/IScsiDxe/IScsiIbft.c
+++ b/NetworkPkg/IScsiDxe/IScsiIbft.c
@@ -454,42 +454,42 @@ IScsiPublishIbft (
   EFI_STATUSStatus;
   EFI_ACPI_TABLE_PROTOCOL   *AcpiTableProtocol;
   EFI_ACPI_ISCSI_BOOT_FIRMWARE_TABLE_HEADER *Table;
   EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
   EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
+  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt;
   UINT8 *Heap;
   UINT8 Checksum;
-  UINTN Index;
 
+  Rsdt = NULL;
+  Xsdt = NULL;
 
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
   if (EFI_ERROR (Status)) {
 return ;
   }
 
   //
   // Find ACPI table RSD_PTR from the system table.
   //
-  for (Index = 0, Rsdp = NULL; Index < gST->NumberOfTableEntries; Index++) {
-if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
)
-  ) {
-  //
-  // A match was found.
-  //
-  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *) 
gST->ConfigurationTable[Index].VendorTable;
-  break;
-}
+  Status = EfiGetSystemConfigurationTable (, (VOID
+ **) );  if (EFI_ERROR (Status)) {
+Status = EfiGetSystemConfigurationTable (, 
+ (VOID **) );
   }
 
-  if (Rsdp == NULL) {
+  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
 return ;
-  } else {
+  } else if (Rsdp->Revision >= 
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 
0) {
+Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress; } 
+ else if (Rsdp->RsdtAddress != 0) {
 Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress;
   }
 
+  if ((Xsdt == NULL) && (Rsdt == NULL)) {
+return ;
+  }
+
   if (mIbftInstalled) {
 Status = AcpiTableProtocol->UninstallAcpiTable (
   AcpiTableProtocol,
   mTableKey
   );
@@ -518,11 +518,16 @@ IScsiPublishIbft (
   Heap = (UINT8 *) Table + IBFT_HEAP_OFFSET;
 
   //
   // Fill in the various section of the iSCSI Boot Firmware Table.
   //
-  IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);
+  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) 
{
+IScsiInitIbfTableHeader (Table, Xsdt->OemId, >OemTableId);  } 
+ else {
+IScsiInitIbfTableHeader (Table, 

[edk2] FW: IP4 Config Troubles with DHCP

[Wu, Jiaxin]

Eugene,

I want to confirm with you the steps to reproduce the issue:

1. Set policy to DHCP.
2. If DHCP process is not complete yet, then run one App to invoke the UDP4 
Configure with "UseDefaultAddress = TRUE" (loop to keep calling Udp4->Configure 
until Ip4Mode.IsConfigured changes to TRUE)
3. Even DHCP succeed but Ip4Mode.IsConfigured flag never set to TRUE  
failure here!!!

Above steps right? 

Actually, you don't need to retry the UDP configuration loop according the 
Ip4Mode.IsConfigured flag. You are only recommended to set a timer to check the 
mapping status after the configuration:

For example:
  Status = Nlc->Udp4->Configure(Nlc->Udp4, >UdpConfig);
  if (EFI_ERROR (Status) && (Status != EFI_NO_MAPPING)) {
  return  Status;
  }
  if (Status == EFI_NO_MAPPING && !UdpGetMapping (Nlc->Udp4)) {
  return  Status;
  }

In UdpGetMapping () function, create one timer to check Ip4Mode.IsConfigured:

For example:
UdpGetMapping () {
  IsMapDone = FALSE;
  gBS->CreateEvent (EVT_TIMER, TPL_CALLBACK, NULL, NULL, );
  gBS->SetTimer (TimeoutEvent, TimerRelative, AnyValue);
  while (EFI_ERROR (gBS->CheckEvent (TimeoutEvent))) {
Udp4->Poll (Udp4);
Udp4->GetModeData (Udp4, , & Ip4Mode, NULL, NULL);
if (Ip4Mode.IsConfigured) {
  IsMapDone = TRUE;
  break;
}
  }
  return IsMapDone;
}

If DHCP process succeed, Ip4Mode.IsConfigured should be updated. If not, any 
bug may be existed.

Thanks,
Jiaxin

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Cohen, 
Eugene
Sent: Thursday, August 11, 2016 2:14 AM
To: edk2-devel@lists.01.org; Wu, Jiaxin 
Subject: [edk2] IP4 Config Troubles with DHCP

We have been running into an issue when trying to configure an interface as 
DHCP where if the DHCP process is not yet complete (Ip4Mode.IsConfigured is 
FALSE) the configure process will never succeed.

We have a case where we attempt to invoke the UDP4 Configure:

Status = Nlc->Udp4->Configure(Nlc->Udp4, >UdpConfig);

We had a retry loop where we keep calling Udp4->Configure until we finally see 
Ip4Mode.IsConfigured go TRUE (similar to what you see in Mtftp4GetMapping) - 
this has worked for many years but recently something broke this.   Now, even 
when DHCP succeeds the Ip4Mode.IsConfigured flag is set to FALSE.  

Only if we retry by destroying and re-creating new service binding children can 
we actually get this logic to succeed.  This logic is getting ridiculously 
complicated so I'm thinking there has to be a better way of doing this.

Do you have an example of specifically how a driver/app should handle the case 
where the DHCP process is not yet complete and wants to wait?

Thanks,

Eugene



___
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] NetworkPkg: Refine codes of iSCSI driver

[Ye, Ting]

Hi Lubo,

Could you please describe the changes introduced by this patch in your log 
message?

Thanks,
Ting

-Original Message-
From: Zhang, Lubo 
Sent: Thursday, August 11, 2016 10:27 AM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Ye, Ting ; Fu, Siyuan 

Subject: [patch] NetworkPkg: Refine codes of iSCSI driver

The RSDT is only used when the bios need to support ACPI 1.0 version. When 
change PcdAcpiExposedTableVersions to 0x3C, it will not support ACPI 1.0. The 
default is 0x3E.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Cc: Eric Dong 
Cc: Ye Ting 
Cc: Fu Siyuan 
---
 NetworkPkg/IScsiDxe/IScsiIbft.c | 35 ---
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiIbft.c b/NetworkPkg/IScsiDxe/IScsiIbft.c 
index 3c179bf..27d098d 100644
--- a/NetworkPkg/IScsiDxe/IScsiIbft.c
+++ b/NetworkPkg/IScsiDxe/IScsiIbft.c
@@ -454,42 +454,42 @@ IScsiPublishIbft (
   EFI_STATUSStatus;
   EFI_ACPI_TABLE_PROTOCOL   *AcpiTableProtocol;
   EFI_ACPI_ISCSI_BOOT_FIRMWARE_TABLE_HEADER *Table;
   EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER  *Rsdp;
   EFI_ACPI_DESCRIPTION_HEADER   *Rsdt;
+  EFI_ACPI_DESCRIPTION_HEADER   *Xsdt;
   UINT8 *Heap;
   UINT8 Checksum;
-  UINTN Index;
 
+  Rsdt = NULL;
+  Xsdt = NULL;
 
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
   if (EFI_ERROR (Status)) {
 return ;
   }
 
   //
   // Find ACPI table RSD_PTR from the system table.
   //
-  for (Index = 0, Rsdp = NULL; Index < gST->NumberOfTableEntries; Index++) {
-if (CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
) ||
-  CompareGuid (&(gST->ConfigurationTable[Index].VendorGuid), 
)
-  ) {
-  //
-  // A match was found.
-  //
-  Rsdp = (EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *) 
gST->ConfigurationTable[Index].VendorTable;
-  break;
-}
+  Status = EfiGetSystemConfigurationTable (, (VOID 
+ **) );  if (EFI_ERROR (Status)) {
+Status = EfiGetSystemConfigurationTable (, 
+ (VOID **) );
   }
 
-  if (Rsdp == NULL) {
+  if (EFI_ERROR (Status) || (Rsdp == NULL)) {
 return ;
-  } else {
+  } else if (Rsdp->Revision >= 
EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp->XsdtAddress != 
0) {
+Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->XsdtAddress;  
+ } else if (Rsdp->RsdtAddress != 0) {
 Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN) Rsdp->RsdtAddress;
   }
 
+  if ((Xsdt == NULL) && (Rsdt == NULL)) {
+return ;
+  }
+
   if (mIbftInstalled) {
 Status = AcpiTableProtocol->UninstallAcpiTable (
   AcpiTableProtocol,
   mTableKey
   );
@@ -518,11 +518,16 @@ IScsiPublishIbft (
   Heap = (UINT8 *) Table + IBFT_HEAP_OFFSET;
 
   //
   // Fill in the various section of the iSCSI Boot Firmware Table.
   //
-  IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);
+  if (Rsdp->Revision >= EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION) 
{
+IScsiInitIbfTableHeader (Table, Xsdt->OemId, >OemTableId);  } 
+ else {
+IScsiInitIbfTableHeader (Table, Rsdt->OemId, >OemTableId);  }
+
   IScsiInitControlSection (Table);
   IScsiFillInitiatorSection (Table, );
   IScsiFillNICAndTargetSections (Table, );
 
   Checksum = CalculateCheckSum8((UINT8 *)Table, Table->Length);
--
1.9.5.msysgit.1

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

Re: [edk2] [PATCH 11/11] MdeModulePkg/ResetSystemRuntimeDxe: Support EfiResetPlatformSpecific

[Jordan Justen]

On 2016-08-10 19:43:03, Ni, Ruiyu wrote:
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Jordan Justen
> > 
> > On 2016-08-09 22:56:11, Ruiyu Ni wrote:
> > > +indicate the reason for the system 
> > > reset. ResetData is only
> > > +valid if ResetStatus is something other 
> > > than EFI_SUCCESS
> > > +unless the ResetType is 
> > > EfiResetPlatformSpecific
> > > +where a minimum amount of ResetData is 
> > > always
> > required.
> > 
> > Most of the patches in this series have lines that are longer than 80 
> > columns.
> > Can you fix that?
> 
> Do we have rule to limit the line to 80 columns? I saw many files have long
> lines.
> Or I can change OvmfPkg changes to follow the 80 rule. What do you think?

Yes, it is in the code style document. You should update all patches
for it. It looks like most of them will just be copy/paste.

> 
> > 
> > I think you should move patch 11 before patch 09 "OvmfPkg: Use
> > MdeModulePkg/ResetSystemRuntimeDxe". I think this should allow reset to
> > continue working through the entire series for OVMF. (right?)
> 
> Reset works using my current order.
> But I agree moving #11 before #9 can move ResetPlatformSpecific support
> earilier, and group the ResetPlatformSpecific patches together. I will do 
> that.

Oh, I see. I guess this patch updates the ResetPlatformSpecific type,
so it probably won't be too important for OVMF. I think it might be a
little better earlier, but you can decide if you want to move it or
not.

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

Re: [edk2] DHCP Automatic Configure at Driver Connect

[Wu, Jiaxin]

Thanks Ting's more background clarification. 

I assume the difference you mentioned is between "SHA-1: 
3d0a49ad47619c30c84bbee8a33f54b64dddbcec" and "SHA-1: 
7648748e99eeeadec38fda7568adb260c4acc861". The two commits does cause the 
different behavior as Ting said below. Git version 3d0a49ad will only set the 
policy to dhcp but don't trigger D.O.R.A while 7648748e always trigger D.O.R.A 
if policy is DHCP.

Version 7648748e commit is also the current behavior of Ip4Config2: DHCP policy 
together with D.O.R.A process, which is similar to the old Ip4Config behavior. 
The version 3d0a49ad did was trying to resolve the Ip4Dxe performance but it's 
not workable for IPv6, so we reverted it. 

Thanks,
Jiaxin

> -Original Message-
> From: Ye, Ting
> Sent: Thursday, August 11, 2016 11:03 AM
> To: Cohen, Eugene ; edk2-devel@lists.01.org; Wu, Jiaxin
> 
> Subject: RE: DHCP Automatic Configure at Driver Connect
> 
> Hi Eugene,
> 
> Actually this is exactly the problem Samer raised to the mailing list in Aug 
> 2015.
> We ever fixed it with following patch:
> 
> SHA-1: 3d0a49ad47619c30c84bbee8a33f54b64dddbcec
> 
> * MdeModulePkg: Fix issue about current Ip4Dxe implementation for DHCP
> DORA process
> 
> DHCP policy is applied as default at boot time on all NICs in the system, 
> which
> results in all NIC ports attempting DHCP and trying to acquire IP addresses
> during boot.
> Ip4 driver should only set dhcp as default policy, and not trigger DORA at
> driver binding start(). We should start DORA until one IP child is configured 
> to
> use default address.
> 
> Later HP raised the same performance impact in IPv6 stack. We realized we
> couldn't use the same logic to defer DHCP6 SARR process.
> Instead, we discussed the issue in spec group and we removed the
> restriction from UEFI specification that the default policy should be
> Ip4Config2PolicyDhcp or Ip6ConfigPolicyAutomatic. It's up to
> implementation's choice.
> The EDKII implementation was later updated that the default policy was
> changed to Ip4Config2PolicyStatic and IP6ConfigPolicyManual. Also the
> previous change was reverted, in order to keep IP4/IP6 solution consistent.
> See patch (also reviewed by Samer):
> 
> SHA-1: 7648748e99eeeadec38fda7568adb260c4acc861
> 
> * MdeModulePkg: Change the default IPv4 config policy
> 
> Git version '3d0a49ad' commit provided a scenario to resolve the
> performance issue for IPv4, but it's not workable for IPv6. To avoid IPv4 and
> IPv6 inconsistency, we decided to revert that version fix.
> 
> If so, the default policy for Ip4Config2 is Ip4Config2PolicyDhcp, which 
> results
> in all NIC ports attempting DHCP. So, this patch is used to changes the the
> default IPv4 config policy to Ip4Config2PolicyStatic and also defer the 
> SetData
> operation after Ip4Config2Protocol installed. This update let the other
> platform drivers have chance to change the default config data by consume
> Ip4Config2Protocol.
> 
> 
> Current implementation recommends that the system should stay in default
> policy - static to not initialize DHCP process, unless the system needs to 
> start
> DHCP -- it should change the policy to IP4Config2PolicyDhcp.
> 
> Best Regards,
> Ye Ting
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Cohen, Eugene
> Sent: Thursday, August 11, 2016 2:17 AM
> To: edk2-devel@lists.01.org; Wu, Jiaxin 
> Subject: [edk2] DHCP Automatic Configure at Driver Connect
> 
> With this commit:
> 
> Revision: 1f6729ffe98095107ce82e67a4a0209674601a90
> Author: jiaxinwu 
> Date: 7/7/2015 2:19:55 AM
> Message:
> MdeModulePkg: Update Ip4Dxe driver to support Ip4Config2 protocol,
> 
> a new behavior seemed to come in to the network stack that was not
> present before: It appears now that as soon as the DHCP Service Binding
> protocol is installed the DHCP process will be initiated (see
> Ip4Config2OnDhcp4SbInstalled).  This differs from past behavior where DHCP
> would only occur if a driver or application specifically did Configure() on 
> the
> network interface.
> 
> For some systems this is problematic because they need to defer DHCP until
> it is certain that the network interface will be used for something.
> 
> Can you provide the reason for this change?  Can we have a policy (PCD) to
> disable this mode of operation?
> 
> Thanks,
> 
> Eugene
> 
> 
> ___
> 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