Re: [edk2] [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data

2018-01-02 Thread Ard Biesheuvel
On 3 January 2018 at 05:09, Udit Kumar  wrote:
> Thanks Ard,
> This works for us as well
> Few comments inline
>
>
> Regards
> Udit
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Tuesday, January 02, 2018 9:21 PM
>> To: edk2-devel@lists.01.org
>> Cc: leif.lindh...@linaro.org; vladimir.olovyanni...@broadcom.com; Udit
>> Kumar ; Meenakshi Aggarwal
>> ; Ard Biesheuvel
>> 
>> Subject: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region
>> as boot services data
>>
>> Commit 8ae5fc182941 ("ArmPlatformPkg/MemoryInitPeiLib: don't reserve
>> primary FV in memory") deleted the code that removes the memory covering
>> the primary firmware volume from the memory map. The assumption was
>> that
>> this is no longer necessary now that we no longer expose compression and
>> PE/COFF loader library code from the PrePi module to DXE core.
>>
>> However, the FV is still declared, and so code may attempt to access it
>> anyway, which may cause unexpected results depending on whether the
>> memory has been reused for other purposes in the mean time.
>>
>> So reinstate the code that splits off the resource descriptor HOB that
>> describes the firmware device, but this time, don't mark the memory as
>> unusable, but create a memory allocation HOB that marks the region as
>> boot services data.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>> Vladimir, Udit, Meenakshi: please confirm whether this code works for you.
>>
>>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 74
>> 
>>  1 file changed, 74 insertions(+)
>>
>> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
>> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
>> index d03214b5df66..d1b5c0be9497 100644
>> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
>> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
>> @@ -70,7 +70,11 @@ MemoryPeim (
>>  {
>>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
>> +  UINT64   ResourceLength;
>>EFI_PEI_HOB_POINTERS NextHob;
>> +  EFI_PHYSICAL_ADDRESS FdTop;
>> +  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
>> +  EFI_PHYSICAL_ADDRESS ResourceTop;
>>BOOLEAN  Found;
>>
>>// Get Virtual Memory Map from the Platform Library
>> @@ -117,6 +121,76 @@ MemoryPeim (
>>  );
>>}
>>
>> +  //
>> +  // Reserve the memory space occupied by the firmware volume
>> +  //
>> +
>> +  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
>> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
>> (PcdSystemMemorySize);
>> +  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
>> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
>> +
>> +  // EDK2 does not have the concept of boot firmware copied into DRAM. To
>> avoid the DXE
>> +  // core to overwrite this area we must create a memory allocation HOB for
>> the region,
>> +  // but this only works if we split off the underlying resource descriptor 
>> as
>> well.
>> +  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase))
>> && (FdTop <= SystemMemoryTop)) {
>> +Found = FALSE;
>> +
>> +// Search for System Memory Hob that contains the firmware
>> +NextHob.Raw = GetHobList ();
>> +while ((NextHob.Raw = GetNextHob
>> (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
>> +  if ((NextHob.ResourceDescriptor->ResourceType ==
>> EFI_RESOURCE_SYSTEM_MEMORY) &&
>> +  (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor-
>> >PhysicalStart) &&
>> +  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
>> NextHob.ResourceDescriptor->ResourceLength))
>> +  {
>> +ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
>> +ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
>> +ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
>> ResourceLength;
>> +
>> +if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor-
>> >PhysicalStart) {
>> +  if (SystemMemoryTop != FdTop) {
>> +// Create the System Memory HOB for the firmware with the non-
>> present attribute
>
> Please correct comments, now this memory is present
>

Yes

>> +BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
>> +ResourceAttributes,
>> +PcdGet64 (PcdFdBaseAddress),
>> +PcdGet32 (PcdFdSize));
>> +
>> +// Top of the FD is system memory available for UEFI
>> +NextHob.ResourceDescriptor->PhysicalStart += 
>> PcdGet32(PcdFdSize);
>> +NextHob.ResourceDescriptor->ResourceLength -=
>> PcdGet32(PcdFdSize);
>> +  }
>> +   

Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: fix wrong base address set as Stack Guard

2018-01-02 Thread Wang, Jian J
Jiewen,

Please take a look at this patch.

Regards,
Jian


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Friday, December 29, 2017 4:37 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Yao, Jiewen ;
> Dong, Eric 
> Subject: [edk2] [PATCH] UefiCpuPkg/MpInitLib: fix wrong base address set as
> Stack Guard
> 
> The reason is that DXE part initialization will reuse the stack allocated
> at PEI phase, if MP was initialized before. Some code added to check this
> situation and use stack base address saved in HOB passed from PEI.
> 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 17 +++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index 40c1bf407a..05484c9ff3 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -295,6 +295,7 @@ InitMpGlobalData (
>UINTN   Index;
>EFI_GCD_MEMORY_SPACE_DESCRIPTOR MemDesc;
>UINTN   StackBase;
> +  CPU_INFO_IN_HOB *CpuInfoInHob;
> 
>SaveCpuMpData (CpuMpData);
> 
> @@ -314,9 +315,18 @@ InitMpGlobalData (
>ASSERT (FALSE);
>  }
> 
> -for (Index = 0; Index < CpuMpData->CpuCount; ++Index) {
> -  StackBase = CpuMpData->Buffer + Index * CpuMpData->CpuApStackSize;
> +//
> +// DXE will reuse stack allocated for APs at PEI phase if it's available.
> +// Let's check it here.
> +//
> +CpuInfoInHob = (CPU_INFO_IN_HOB *)(UINTN)CpuMpData->CpuInfoInHob;
> +if (CpuInfoInHob != NULL && CpuInfoInHob->ApTopOfStack != 0) {
> +  StackBase = CpuInfoInHob->ApTopOfStack;
> +} else {
> +  StackBase = CpuMpData->Buffer;
> +}
> 
> +for (Index = 0; Index < CpuMpData->CpuCount; ++Index) {
>Status = gDS->GetMemorySpaceDescriptor (StackBase, );
>ASSERT_EFI_ERROR (Status);
> 
> @@ -326,6 +336,9 @@ InitMpGlobalData (
>MemDesc.Attributes | EFI_MEMORY_RP
>);
>ASSERT_EFI_ERROR (Status);
> +
> +  DEBUG ((DEBUG_VERBOSE, "Stack Guard set at %x [cpu%d]!\n", StackBase,
> Index));
> +  StackBase += CpuMpData->CpuApStackSize;
>  }
>}
> 
> --
> 2.15.1.windows.2
> 
> ___
> 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] MdeModulePkg/DxeNetLib: Fix an error in packet length counting.

2018-01-02 Thread Wu, Jiaxin
Reviewed-by: Jiaxin Wu 


> -Original Message-
> From: Wang, Fan
> Sent: Wednesday, January 3, 2018 10:32 AM
> To: edk2-devel@lists.01.org
> Cc: Wang, Fan ; Fu, Siyuan ; Ye,
> Ting ; Wu, Jiaxin 
> Subject: [Patch 3/3] MdeModulePkg/DxeNetLib: Fix an error in packet length
> counting.
> 
> From: Wang Fan 
> 
> * In old implementation, the operation len-- assumes AsciiSPrint()
>   has counted NULL terminator, and it's not correct. This patch is
>   to fix this issue.
> 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Cc: Jiaxin Wu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index 90f17b7..cbce28f 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -401,22 +401,21 @@ SyslogBuildPacket (
>  Time.Day,
>  Time.Hour,
>  Time.Minute,
>  Time.Second
>  );
> -  Len--;
> 
>Len += (UINT32) AsciiSPrint (
>  Buf + Len,
>  BufLen - Len,
>  "Tiano %a: %a (Line: %d File: %a)",
>  Module,
>  Message,
>  Line,
>  File
>  );
> -  Len--;
> +  Len ++;
> 
>//
>// OK, patch the IP length/checksum and UDP length fields.
>//
>Len   += sizeof (EFI_UDP_HEADER);
> --
> 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 1/3] MdeModulePkg/DxeNetLib: Fix a potential issue in macro definition.

2018-01-02 Thread Wu, Jiaxin
Reviewed-by: Jiaxin Wu 


> -Original Message-
> From: Wang, Fan
> Sent: Wednesday, January 3, 2018 10:32 AM
> To: edk2-devel@lists.01.org
> Cc: Wang, Fan ; Fu, Siyuan ; Ye,
> Ting ; Wu, Jiaxin 
> Subject: [Patch 1/3] MdeModulePkg/DxeNetLib: Fix a potential issue in
> macro definition.
> 
> From: Wang Fan 
> 
> The NIC_ITEM_CONFIG_SIZE macro in DxeNetLib is defined as:
> sizeof (NIC_IP4_CONFIG_INFO) + sizeof (EFI_IP4_ROUTE_TABLE) *
> MAX_IP4_CONFIG_IN_VARIABLE. This macro should be surrounded
> with parenthesis to avoid being incorrectly used.
> 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Cc: Jiaxin Wu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index 26a80a7..0f00f79 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -36,11 +36,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>  #include 
>  #include 
>  #include 
>  #include 
> 
> -#define NIC_ITEM_CONFIG_SIZE   sizeof (NIC_IP4_CONFIG_INFO) + sizeof
> (EFI_IP4_ROUTE_TABLE) * MAX_IP4_CONFIG_IN_VARIABLE
> +#define NIC_ITEM_CONFIG_SIZE   (sizeof (NIC_IP4_CONFIG_INFO) + sizeof
> (EFI_IP4_ROUTE_TABLE) * MAX_IP4_CONFIG_IN_VARIABLE)
>  #define DEFAULT_ZERO_START ((UINTN) ~0)
> 
>  //
>  // All the supported IP4 maskes in host byte order.
>  //
> --
> 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 2/3] MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling.

2018-01-02 Thread Wu, Jiaxin
Reviewed-by: Jiaxin Wu 

> -Original Message-
> From: Wang, Fan
> Sent: Wednesday, January 3, 2018 10:32 AM
> To: edk2-devel@lists.01.org
> Cc: Wang, Fan ; Fu, Siyuan ; Ye,
> Ting ; Wu, Jiaxin 
> Subject: [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and
> ASSERT handling.
> 
> From: Wang Fan 
> 
> * Library API should check the input parameters before use, or
>   ASSERT to tell it has to meet some requirements. But in DxeNetLib,
>   not all functions follows this rule.
> * ASSERT shouldn't be used as error handling, add some handling code
>   for errors.
> * Add some ASSERT commence in function notes.
> 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Cc: Jiaxin Wu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>  MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 119
> +
>  1 file changed, 105 insertions(+), 14 deletions(-)
> 
> diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> index 0f00f79..90f17b7 100644
> --- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> +++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
> @@ -1,9 +1,9 @@
>  /** @file
>Network library.
> 
> -Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
>  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be
> found at
>  http://opensource.org/licenses/bsd-license.php
> @@ -196,10 +196,11 @@ SyslogLocateSnp (
>Transmit a syslog packet synchronously through SNP. The Packet
>already has the ethernet header prepended. This function should
>fill in the source MAC because it will try to locate a SNP each
>time it is called to avoid the problem if SNP is unloaded.
>This code snip is copied from MNP.
> +  If Packet is NULL, then ASSERT().
> 
>@param[in] Packet  The Syslog packet
>@param[in] Length  The length of the packet
> 
>@retval EFI_DEVICE_ERROR   Failed to locate a usable SNP protocol
> @@ -217,10 +218,12 @@ SyslogSendPacket (
>ETHER_HEAD  *Ether;
>EFI_STATUS  Status;
>EFI_EVENT   TimeoutEvent;
>UINT8   *TxBuf;
> 
> +  ASSERT (Packet != NULL);
> +
>Snp = SyslogLocateSnp ();
> 
>if (Snp == NULL) {
>  return EFI_DEVICE_ERROR;
>}
> @@ -308,11 +311,11 @@ ON_EXIT:
>@param[in]  Line  The line of code in the File that contains the 
> current log
>@param[in]  Message   The log message
>@param[in]  BufLenThe lenght of the Buf
>@param[out] Buf   The buffer to put the packet data
> 
> -  @return The length of the syslog packet built.
> +  @return The length of the syslog packet built, 0 represents no packet is
> built.
> 
>  **/
>  UINT32
>  SyslogBuildPacket (
>IN  UINT32Level,
> @@ -322,10 +325,11 @@ SyslogBuildPacket (
>IN  UINT8 *Message,
>IN  UINT32BufLen,
>OUT CHAR8 *Buf
>)
>  {
> +  EFI_STATUSStatus;
>ETHER_HEAD*Ether;
>IP4_HEAD  *Ip4;
>EFI_UDP_HEADER*Udp4;
>EFI_TIME  Time;
>UINT32Pri;
> @@ -377,12 +381,14 @@ SyslogBuildPacket (
> 
>//
>// Build the syslog message body with  Timestamp  machine module
> Message
>//
>Pri = ((NET_SYSLOG_FACILITY & 31) << 3) | (Level & 7);
> -  gRT->GetTime (, NULL);
> -  ASSERT ((Time.Month <= 12) && (Time.Month >= 1));
> +  Status = gRT->GetTime (, NULL);
> +  if (EFI_ERROR (Status)) {
> +return 0;
> +  }
> 
>//
>// Use %a to format the ASCII strings, %s to format UNICODE strings
>//
>Len  = 0;
> @@ -437,10 +443,12 @@ SyslogBuildPacket (
> __FILE__,
> __LINE__,
> NetDebugASPrint ("State transit to %a\n", Name)
>   )
> 
> +  If Format is NULL, then ASSERT().
> +
>@param Format  The ASCII format string.
>@param ... The variable length parameter whose format is determined
>   by the Format string.
> 
>@returnThe buffer containing the formatted message,
> @@ -455,10 +463,12 @@ NetDebugASPrint (
>)
>  {
>VA_LIST   Marker;
>CHAR8 *Buf;
> 
> +  ASSERT (Format != NULL);
> +
>Buf = (CHAR8 *) AllocatePool (NET_DEBUG_MSG_LEN);
> 
>if (Buf == NULL) {
>  return NULL;
>}
> @@ -481,11 +491,12 @@ NetDebugASPrint (
>@param File The file that contains 

Re: [edk2] [Patch] BaseTools: Fix compile error on VS2010

2018-01-02 Thread Gao, Liming
Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Yonghong Zhu
>Sent: Wednesday, January 03, 2018 1:48 PM
>To: edk2-devel@lists.01.org
>Subject: [edk2] [Patch] BaseTools: Fix compile error on VS2010
>
>VS2010 also defined RSIZE_MAX, so we undef it first.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Source/C/Common/CommonLib.h | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
>diff --git a/BaseTools/Source/C/Common/CommonLib.h
>b/BaseTools/Source/C/Common/CommonLib.h
>index 9da16e8..dccb192 100644
>--- a/BaseTools/Source/C/Common/CommonLib.h
>+++ b/BaseTools/Source/C/Common/CommonLib.h
>@@ -1,9 +1,9 @@
> /** @file
> Common library assistance routines.
>
>-Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
>+Copyright (c) 2004 - 2018, 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
>
>@@ -26,13 +26,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
> #define MAX_UINT64 ((UINT64)0xULL)
> #define MAX_UINT16  ((UINT16)0x)
> #define MAX_UINT8   ((UINT8)0xFF)
> #define ARRAY_SIZE(Array) (sizeof (Array) / sizeof ((Array)[0]))
> #define ASCII_RSIZE_MAX 100
>-#ifndef RSIZE_MAX
>+#undef RSIZE_MAX
> #define RSIZE_MAX 100
>-#endif
>
> #define IS_COMMA(a)((a) == L',')
> #define IS_HYPHEN(a)   ((a) == L'-')
> #define IS_DOT(a)  ((a) == L'.')
> #define IS_LEFT_PARENTH(a) ((a) == L'(')
>--
>2.6.1.windows.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


[edk2] [Patch] BaseTools: Fix compile error on VS2010

2018-01-02 Thread Yonghong Zhu
VS2010 also defined RSIZE_MAX, so we undef it first.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/C/Common/CommonLib.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.h 
b/BaseTools/Source/C/Common/CommonLib.h
index 9da16e8..dccb192 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -1,9 +1,9 @@
 /** @file
 Common library assistance routines.
 
-Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2018, 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
 
@@ -26,13 +26,12 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define MAX_UINT64 ((UINT64)0xULL)
 #define MAX_UINT16  ((UINT16)0x)
 #define MAX_UINT8   ((UINT8)0xFF)
 #define ARRAY_SIZE(Array) (sizeof (Array) / sizeof ((Array)[0]))
 #define ASCII_RSIZE_MAX 100
-#ifndef RSIZE_MAX
+#undef RSIZE_MAX
 #define RSIZE_MAX 100
-#endif
 
 #define IS_COMMA(a)((a) == L',')
 #define IS_HYPHEN(a)   ((a) == L'-')
 #define IS_DOT(a)  ((a) == L'.')
 #define IS_LEFT_PARENTH(a) ((a) == L'(')
-- 
2.6.1.windows.1

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


Re: [edk2] [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data

2018-01-02 Thread Udit Kumar
Thanks Ard, 
This works for us as well 
Few comments inline 


Regards
Udit

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, January 02, 2018 9:21 PM
> To: edk2-devel@lists.01.org
> Cc: leif.lindh...@linaro.org; vladimir.olovyanni...@broadcom.com; Udit
> Kumar ; Meenakshi Aggarwal
> ; Ard Biesheuvel
> 
> Subject: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region
> as boot services data
> 
> Commit 8ae5fc182941 ("ArmPlatformPkg/MemoryInitPeiLib: don't reserve
> primary FV in memory") deleted the code that removes the memory covering
> the primary firmware volume from the memory map. The assumption was
> that
> this is no longer necessary now that we no longer expose compression and
> PE/COFF loader library code from the PrePi module to DXE core.
> 
> However, the FV is still declared, and so code may attempt to access it
> anyway, which may cause unexpected results depending on whether the
> memory has been reused for other purposes in the mean time.
> 
> So reinstate the code that splits off the resource descriptor HOB that
> describes the firmware device, but this time, don't mark the memory as
> unusable, but create a memory allocation HOB that marks the region as
> boot services data.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Vladimir, Udit, Meenakshi: please confirm whether this code works for you.
> 
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 74
> 
>  1 file changed, 74 insertions(+)
> 
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index d03214b5df66..d1b5c0be9497 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,7 +70,11 @@ MemoryPeim (
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  UINT64   ResourceLength;
>EFI_PEI_HOB_POINTERS NextHob;
> +  EFI_PHYSICAL_ADDRESS FdTop;
> +  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> +  EFI_PHYSICAL_ADDRESS ResourceTop;
>BOOLEAN  Found;
> 
>// Get Virtual Memory Map from the Platform Library
> @@ -117,6 +121,76 @@ MemoryPeim (
>  );
>}
> 
> +  //
> +  // Reserve the memory space occupied by the firmware volume
> +  //
> +
> +  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> (PcdSystemMemorySize);
> +  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) +
> (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> +
> +  // EDK2 does not have the concept of boot firmware copied into DRAM. To
> avoid the DXE
> +  // core to overwrite this area we must create a memory allocation HOB for
> the region,
> +  // but this only works if we split off the underlying resource descriptor 
> as
> well.
> +  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase))
> && (FdTop <= SystemMemoryTop)) {
> +Found = FALSE;
> +
> +// Search for System Memory Hob that contains the firmware
> +NextHob.Raw = GetHobList ();
> +while ((NextHob.Raw = GetNextHob
> (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
> +  if ((NextHob.ResourceDescriptor->ResourceType ==
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> +  (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor-
> >PhysicalStart) &&
> +  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
> NextHob.ResourceDescriptor->ResourceLength))
> +  {
> +ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
> +ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> +ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
> ResourceLength;
> +
> +if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor-
> >PhysicalStart) {
> +  if (SystemMemoryTop != FdTop) {
> +// Create the System Memory HOB for the firmware with the non-
> present attribute

Please correct comments, now this memory is present 

> +BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> +ResourceAttributes,
> +PcdGet64 (PcdFdBaseAddress),
> +PcdGet32 (PcdFdSize));
> +
> +// Top of the FD is system memory available for UEFI
> +NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
> +NextHob.ResourceDescriptor->ResourceLength -=
> PcdGet32(PcdFdSize);
> +  }
> +} else {
> +  // Create the System Memory HOB for the firmware
> +  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> +  

Re: [edk2] [Patch] DSC Spec: Fix a typo error "enty" to "entry"

2018-01-02 Thread Gao, Liming
Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Tuesday, January 02, 2018 10:17 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Kinney, Michael D
>; Shaw, Kevin W 
>Subject: [Patch] DSC Spec: Fix a typo error "enty" to "entry"
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Cc: Kevin W Shaw 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> 2_dsc_overview/23_[defines]_section_processing.md | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/2_dsc_overview/23_[defines]_section_processing.md
>b/2_dsc_overview/23_[defines]_section_processing.md
>index 95c2893..a7e8680 100644
>--- a/2_dsc_overview/23_[defines]_section_processing.md
>+++ b/2_dsc_overview/23_[defines]_section_processing.md
>@@ -53,11 +53,11 @@ The format for entries in this section is:
>
> `Name = Value`
>
> If the `PREBUILD` and/or `POSTBUILD` entries are specified, value must be a
> tool that can be executed.  If the value contains space characters, then the
>-value must be a quoted string. The `PREBUILD` and `POSTBUILD` enty support
>+value must be a quoted string. The `PREBUILD` and `POSTBUILD` entry
>support
> multiple arguments, and tool will convert the arguments that are
>WORKSPACE or
> PACKAGES_PATH relative paths to absolute paths. Quotes may be used for
>arguments
> that have spaces or special characters. The `build` tool suspends processing 
> of
> the DSC file if the `PREBUILD` entry is present, calls the script, and either
> terminates or continues processing the DSC file depending on the exit code
>from
>--
>2.6.1.windows.1

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


Re: [edk2] [Patch] BaseTools CommonLib: Fix printf %llx issue on UINT64

2018-01-02 Thread Zhu, Yonghong
Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, January 3, 2018 8:34 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch] BaseTools CommonLib: Fix printf %llx issue on UINT64

UINT64 is defined as the different type for the different ARCHs. To let it work 
for all archs and compilers, add (unsigned long long) for the input value 
together with %llx.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/Common/PcdValueCommon.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c 
b/BaseTools/Source/C/Common/PcdValueCommon.c
index 6ca0994744..42f76ddbbc 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.c
+++ b/BaseTools/Source/C/Common/PcdValueCommon.c
@@ -266,11 +266,7 @@ Returns:
 sprintf(PcdList[Index].Value, "0x%08x", (UINT32)(Value & 0x));
 break;
   case PcdDataTypeUint64:
-#ifdef __GNUC__
-sprintf(PcdList[Index].Value, "0x%016lx", Value);
-#else
-sprintf(PcdList[Index].Value, "0x%016llx", Value);
-#endif
+sprintf(PcdList[Index].Value, "0x%016llx", (unsigned long 
+ long)Value);
 break;
   case PcdDataTypePointer:
 fprintf (stderr, "PCD %s.%s.%s.%s is structure.  Use PcdSetPtr()\n", 
SkuName, DefaultValueName, TokenSpaceGuidName, TokenName);
--
2.11.0.windows.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 edk2-platforms 0/4] NXP:LS2088A RDB Board Support

2018-01-02 Thread Meenakshi Aggarwal
Hi Ard,

Thanks for your time.

Below is the sequence in which we have sent the patches.

1. [PATCH edk2-platforms] [PATCH v3 0/9] Platform/NXP [27/11/2017] V3
 This patch-set adds support for LS1043 platform, it includes patches for 
platform initialization and all necessary drivers to support architecture 
protocols.

2. [PATCH 0/4] Platform/NXP-Adding NXP NOR IP [19/12/2017] V2
 This patch-set adds support for IFC NOR driver and enables it for LS1043 
Soc.

3. [PATCH 0/4] Platform/NXP-Add LS1046A RDB Board Support  [19/12/2017] V2
 This patch-set adds support for LS1046 platform, it includes patches for 
platform initialization and all necessary drivers to support architecture 
protocols.

4. [PATCH 0/3] Platform/NXP-Adding NXP MMC Host Driver [1/12/2017]  V1
 This patch-set adds support for MMC Host driver and enables it for LS1043 
Soc.

5. [PATCH edk2-platforms 0/3] Platform/NXP-Added NXP PCI Host Bridge Driver 
[22/12/2017] V1
 This patch-set adds support for PCI Host bridge driver and enables it for 
LS1043 Soc.
  V2 is in progress on basis of your comments.

6. [PATCH edk2-platforms 0/4] NXP:LS2088A RDB Board Support [22/12/2017] V1
 This patch-set adds support for LS2088 platform, it includes patches for 
platform initialization and all necessary drivers to support architecture 
protocols.

7. [PATCH edk2-platforms 0/3] Cover letter:Pci Emulation and SATA support 
[22/12/2017] V1
 This patch-set adds support for SATA on PCI Emulation layer and enables it 
for LS1046 Soc.
 V2 in progress on basis of your comment (using NonDiscoverablePciDevice 
Layer rather Pci Emulation layer).


Please let us know if you need any other information.

Thanks,
NXP Team

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, January 02, 2018 9:26 PM
> To: Wasim Khan ; Meenakshi Aggarwal
> 
> Cc: Leif Lindholm ; Kinney, Michael D
> ; edk2-devel@lists.01.org; Udit Kumar
> ; Varun Sethi 
> Subject: Re: [PATCH edk2-platforms 0/4] NXP:LS2088A RDB Board Support
> 
> Hello Wasim, Udit, Meenakshi,
> 
> I kind of lost track of all the patches you have been sending over the
> past month or so.
> Could one of you please send a quick summary of all the patches that
> are in flight at the moment, and for which platforms?
> 
> Thanks,
> Ard.
> 
> 
> On 22 December 2017 at 10:51, Wasim Khan  wrote:
> > In Silicon/NXP, we are keeping our SoC specific information and remaining
> code will be kept in Platform/NXP.
> >
> > Following patches will add support of NXP LS2088A RDB board in edk2-
> platforms.
> >
> > Platform/NXP/LS2088aRdbPkg will host .dsc and .fdf files to support
> compilation for LS2088A RDB board.
> >
> > Looking forward for your kind support in upstreaming LS2088 RDB board
> support in edk2-platforms.
> >
> > Wasim Khan (4):
> >   Platform/NXP: Add support for ArmPlatformLib
> >   Silicon/Maxim: Added Support for DS3232 RTC Library
> >   Silicon/NXP:SocLib support for initialization of peripherals
> >   Compilation : Add the fdf, dsc and dec files.
> >
> >  .../LS2088aRdbPkg/Include/Library/PlatformLib.h|  28 ++
> >  Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dec   |  29 ++
> >  Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc   |  95 ++
> >  Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf   | 197 +++
> >  .../Library/PlatformLib/ArmPlatformLib.c   | 106 ++
> >  .../Library/PlatformLib/ArmPlatformLib.inf |  80 +
> >  .../Library/PlatformLib/NxpQoriqLsHelper.S |  35 ++
> >  .../Library/PlatformLib/NxpQoriqLsMem.c| 196 +++
> >  Platform/NXP/LS2088aRdbPkg/VarStore.fdf.inc|  98 ++
> >  Platform/NXP/NxpQoriqLs.dec|   1 +
> >  Silicon/Maxim/Library/Ds3232RtcLib/Ds3232Rtc.h |  49 +++
> >  Silicon/Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.c  | 370
> +
> >  .../Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.dec|  31 ++
> >  .../Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.inf|  50 +++
> >  Silicon/NXP/Chassis/Chassis.c  |  32 ++
> >  Silicon/NXP/Chassis/Chassis.h  |  17 +
> >  Silicon/NXP/Chassis/Chassis3/Chassis3.dec  |  19 ++
> >  Silicon/NXP/Chassis/Chassis3/Errata.c  |  62 
> >  Silicon/NXP/Chassis/Chassis3/SerDes.h  |  92 +
> >  Silicon/NXP/Chassis/Chassis3/Soc.c | 171 ++
> >  Silicon/NXP/Chassis/Chassis3/Soc.h | 159 +
> >  Silicon/NXP/Chassis/LS2088aSocLib.inf  |  53 +++
> >  Silicon/NXP/LS2088A/Include/SocSerDes.h|  67 
> >  Silicon/NXP/LS2088A/LS2088A.dec|  22 ++
> >  Silicon/NXP/LS2088A/LS2088A.dsc| 101 ++
> >  25 files changed, 2160 insertions(+)
> >  

[edk2] [Patch 1/3] MdeModulePkg/DxeNetLib: Fix a potential issue in macro definition.

2018-01-02 Thread fanwang2
From: Wang Fan 

The NIC_ITEM_CONFIG_SIZE macro in DxeNetLib is defined as:
sizeof (NIC_IP4_CONFIG_INFO) + sizeof (EFI_IP4_ROUTE_TABLE) *
MAX_IP4_CONFIG_IN_VARIABLE. This macro should be surrounded
with parenthesis to avoid being incorrectly used.

Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Jiaxin Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c 
b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index 26a80a7..0f00f79 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -36,11 +36,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
 #include 
 
-#define NIC_ITEM_CONFIG_SIZE   sizeof (NIC_IP4_CONFIG_INFO) + sizeof 
(EFI_IP4_ROUTE_TABLE) * MAX_IP4_CONFIG_IN_VARIABLE
+#define NIC_ITEM_CONFIG_SIZE   (sizeof (NIC_IP4_CONFIG_INFO) + sizeof 
(EFI_IP4_ROUTE_TABLE) * MAX_IP4_CONFIG_IN_VARIABLE)
 #define DEFAULT_ZERO_START ((UINTN) ~0)
 
 //
 // All the supported IP4 maskes in host byte order.
 //
-- 
1.9.5.msysgit.1

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


[edk2] [Patch 3/3] MdeModulePkg/DxeNetLib: Fix an error in packet length counting.

2018-01-02 Thread fanwang2
From: Wang Fan 

* In old implementation, the operation len-- assumes AsciiSPrint()
  has counted NULL terminator, and it's not correct. This patch is
  to fix this issue.

Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Jiaxin Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c 
b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index 90f17b7..cbce28f 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -401,22 +401,21 @@ SyslogBuildPacket (
 Time.Day,
 Time.Hour,
 Time.Minute,
 Time.Second
 );
-  Len--;
 
   Len += (UINT32) AsciiSPrint (
 Buf + Len,
 BufLen - Len,
 "Tiano %a: %a (Line: %d File: %a)",
 Module,
 Message,
 Line,
 File
 );
-  Len--;
+  Len ++;
 
   //
   // OK, patch the IP length/checksum and UDP length fields.
   //
   Len   += sizeof (EFI_UDP_HEADER);
-- 
1.9.5.msysgit.1

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


[edk2] [Patch 0/3] Fix a set of issues for DxeNetLib

2018-01-02 Thread fanwang2
From: Wang Fan 

See each patch file for details.

Wang Fan (3):
  MdeModulePkg/DxeNetLib: Fix a potential issue in macro definition.
  MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling.
  MdeModulePkg/DxeNetLib: Fix an error in packet length counting.

 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 124 +
 1 file changed, 107 insertions(+), 17 deletions(-)

-- 
1.9.5.msysgit.1

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


[edk2] [Patch 2/3] MdeModulePkg/DxeNetLib: Add parameter check and ASSERT handling.

2018-01-02 Thread fanwang2
From: Wang Fan 

* Library API should check the input parameters before use, or
  ASSERT to tell it has to meet some requirements. But in DxeNetLib,
  not all functions follows this rule.
* ASSERT shouldn't be used as error handling, add some handling code
  for errors.
* Add some ASSERT commence in function notes.

Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Jiaxin Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 MdeModulePkg/Library/DxeNetLib/DxeNetLib.c | 119 +
 1 file changed, 105 insertions(+), 14 deletions(-)

diff --git a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c 
b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
index 0f00f79..90f17b7 100644
--- a/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
+++ b/MdeModulePkg/Library/DxeNetLib/DxeNetLib.c
@@ -1,9 +1,9 @@
 /** @file
   Network library.
 
-Copyright (c) 2005 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2018, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
@@ -196,10 +196,11 @@ SyslogLocateSnp (
   Transmit a syslog packet synchronously through SNP. The Packet
   already has the ethernet header prepended. This function should
   fill in the source MAC because it will try to locate a SNP each
   time it is called to avoid the problem if SNP is unloaded.
   This code snip is copied from MNP.
+  If Packet is NULL, then ASSERT().
 
   @param[in] Packet  The Syslog packet
   @param[in] Length  The length of the packet
 
   @retval EFI_DEVICE_ERROR   Failed to locate a usable SNP protocol
@@ -217,10 +218,12 @@ SyslogSendPacket (
   ETHER_HEAD  *Ether;
   EFI_STATUS  Status;
   EFI_EVENT   TimeoutEvent;
   UINT8   *TxBuf;
 
+  ASSERT (Packet != NULL);
+
   Snp = SyslogLocateSnp ();
 
   if (Snp == NULL) {
 return EFI_DEVICE_ERROR;
   }
@@ -308,11 +311,11 @@ ON_EXIT:
   @param[in]  Line  The line of code in the File that contains the current 
log
   @param[in]  Message   The log message
   @param[in]  BufLenThe lenght of the Buf
   @param[out] Buf   The buffer to put the packet data
 
-  @return The length of the syslog packet built.
+  @return The length of the syslog packet built, 0 represents no packet is 
built.
 
 **/
 UINT32
 SyslogBuildPacket (
   IN  UINT32Level,
@@ -322,10 +325,11 @@ SyslogBuildPacket (
   IN  UINT8 *Message,
   IN  UINT32BufLen,
   OUT CHAR8 *Buf
   )
 {
+  EFI_STATUSStatus;
   ETHER_HEAD*Ether;
   IP4_HEAD  *Ip4;
   EFI_UDP_HEADER*Udp4;
   EFI_TIME  Time;
   UINT32Pri;
@@ -377,12 +381,14 @@ SyslogBuildPacket (
 
   //
   // Build the syslog message body with  Timestamp  machine module Message
   //
   Pri = ((NET_SYSLOG_FACILITY & 31) << 3) | (Level & 7);
-  gRT->GetTime (, NULL);
-  ASSERT ((Time.Month <= 12) && (Time.Month >= 1));
+  Status = gRT->GetTime (, NULL);
+  if (EFI_ERROR (Status)) {
+return 0;
+  }
 
   //
   // Use %a to format the ASCII strings, %s to format UNICODE strings
   //
   Len  = 0;
@@ -437,10 +443,12 @@ SyslogBuildPacket (
__FILE__,
__LINE__,
NetDebugASPrint ("State transit to %a\n", Name)
  )
 
+  If Format is NULL, then ASSERT().
+
   @param Format  The ASCII format string.
   @param ... The variable length parameter whose format is determined
  by the Format string.
 
   @returnThe buffer containing the formatted message,
@@ -455,10 +463,12 @@ NetDebugASPrint (
   )
 {
   VA_LIST   Marker;
   CHAR8 *Buf;
 
+  ASSERT (Format != NULL);
+
   Buf = (CHAR8 *) AllocatePool (NET_DEBUG_MSG_LEN);
 
   if (Buf == NULL) {
 return NULL;
   }
@@ -481,11 +491,12 @@ NetDebugASPrint (
   @param File The file that contains the log.
   @param Line The exact line that contains the log.
   @param Message  The user message to log.
 
   @retval EFI_INVALID_PARAMETER Any input parameter is invalid.
-  @retval EFI_OUT_OF_RESOURCES  Failed to allocate memory for the packet
+  @retval EFI_OUT_OF_RESOURCES  Failed to allocate memory for the packet.
+  @retval EFI_DEVICE_ERROR  Device error occurs.
   @retval EFI_SUCCESS   The log is discard because that it is more 
verbose
 than the mNetDebugLevelMax. Or, it has been 
sent out.
 **/
 EFI_STATUS
 EFIAPI
@@ -502,11 +513,11 @@ NetDebugOutput (
   EFI_STATUS   

Re: [edk2] [PATCH] MdePkg: resolve bug 741

2018-01-02 Thread Gao, Liming
I choose to disable this warning first, because there are other cases in edk2 
project. One is PI S3SaveState protocol EFI_S3_SAVE_STATE_WRITE API. This case 
needs to update PI spec. It may take more time. 

typedef
EFI_STATUS
(EFIAPI *EFI_S3_SAVE_STATE_WRITE)(
   IN CONST EFI_S3_SAVE_STATE_PROTOCOL  *This,
   IN   UINT16  OpCode,
   ...
);

Thanks
Liming
> -Original Message-
> From: Zenith432 [mailto:zenith...@users.sourceforge.net]
> Sent: Sunday, December 10, 2017 5:33 AM
> To: edk2-devel@lists.01.org; Marvin Häuser 
> Cc: Kinney, Michael D ; Gao, Liming 
> 
> Subject: RE: [edk2] [PATCH] MdePkg: resolve bug 741
> 
> It's the package maintainer's choice.  As a practical matter, silencing the 
> warning also works because...
> 
> 1. clang is the only compiler that complains.  Even though it complains, it 
> generates correct code because it has __builtin
> implementation of va_start that takes register argument and stack granularity 
> into account.
> 2. In MdePkg/Include/Base.h there are __builtin implementations of VA_START 
> for __CC_ARM, GCC and clang which should all work
> despite the argument promotion.
> 3. For the other architectures (i.e. Windows) there's an implementation of 
> VA_START in Base.h that uses _INT_SIZE_OF for the
> parameter always a multiple of UINTN.  The Windows compilers also have 
> builtin forms of va_start, but this non-builtin
> implementation looks ok for all arguments actually passed as variadic in EDK2.
> 
> 
> On Sat, 12/9/17, Marvin Häuser  wrote:
> 
> ...
> It's your choice of course.
> ...
> 
>  Regards,
>  Marvin.
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] BaseTools/DevicePath: fix GCC build error in print_mem(), and clean it up

2018-01-02 Thread Zhu, Yonghong
Thanks.

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, January 03, 2018 1:17 AM
To: edk2-devel-01 
Cc: Gao, Liming ; Zhu, Yonghong 
Subject: [PATCH] BaseTools/DevicePath: fix GCC build error in print_mem(), and 
clean it up

Currently "BaseTools/Source/C/DevicePath/DevicePath.c" fails to build with
GCC48:

> DevicePath.c: In function 'print_mem':
> DevicePath.c:109:5: error: 'for' loop initial declarations are only 
> allowed in C99 mode
>  for (size_t i=0; i  ^
> DevicePath.c:109:5: note: use option -std=c99 or -std=gnu99 to compile 
> your code

In addition, the print_mem() function does not conform to the edk2 coding
style:

- we use CamelCase and no underscores in identifiers,
- the types and type qualifiers should follow the edk2 style,
- initialization as part of definition is forbidden for local variables.

Clean these up.

While updating the print_mem()/PrintMem() call sites, also remove the 
superfluous parentheses around the second argument.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Fixes: 7dbc50bd244d95fdc1741b9cfc561f0bfd724de1
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
Build-tested only (I don't have a DSC file with a device path PCD).

 BaseTools/Source/C/DevicePath/DevicePath.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c 
b/BaseTools/Source/C/DevicePath/DevicePath.c
index 4c87163209ab..76b8553b7145 100644
--- a/BaseTools/Source/C/DevicePath/DevicePath.c
+++ b/BaseTools/Source/C/DevicePath/DevicePath.c
@@ -103,11 +103,19 @@ Returns:
 }
 
 
-void print_mem(void const *vp, size_t n)
+STATIC
+VOID
+PrintMem (
+  CONST VOID *Buffer,
+  UINTN  Count
+  )
 {
-unsigned char const *p = vp;
-for (size_t i=0; iType == END_DEVICE_PATH_TYPE) && (DevicePath->SubType 
== END_ENTIRE_DEVICE_PATH_SUBTYPE)) )
   {
-print_mem(DevicePath, (DevicePath->Length[0] | DevicePath->Length[1] << 
8));
+PrintMem (DevicePath, DevicePath->Length[0] | DevicePath->Length[1] 
+ << 8);
 DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)DevicePath + 
(DevicePath->Length[0] | DevicePath->Length[1] << 8));
   }
-  print_mem(DevicePath, (DevicePath->Length[0] | DevicePath->Length[1] << 8));
+  PrintMem (DevicePath, DevicePath->Length[0] | DevicePath->Length[1] 
+ << 8);
   putchar('\n');
   return STATUS_SUCCESS;
 }
--
2.14.1.3.gb7cf6e02401b

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


[edk2] [Patch] BaseTools CommonLib: Fix printf %llx issue on UINT64

2018-01-02 Thread Liming Gao
UINT64 is defined as the different type for the different ARCHs. To
let it work for all archs and compilers, add (unsigned long long) for
the input value together with %llx.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 BaseTools/Source/C/Common/PcdValueCommon.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c 
b/BaseTools/Source/C/Common/PcdValueCommon.c
index 6ca0994744..42f76ddbbc 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.c
+++ b/BaseTools/Source/C/Common/PcdValueCommon.c
@@ -266,11 +266,7 @@ Returns:
 sprintf(PcdList[Index].Value, "0x%08x", (UINT32)(Value & 0x));
 break;
   case PcdDataTypeUint64:
-#ifdef __GNUC__
-sprintf(PcdList[Index].Value, "0x%016lx", Value);
-#else
-sprintf(PcdList[Index].Value, "0x%016llx", Value);
-#endif
+sprintf(PcdList[Index].Value, "0x%016llx", (unsigned long long)Value);
 break;
   case PcdDataTypePointer:
 fprintf (stderr, "PCD %s.%s.%s.%s is structure.  Use PcdSetPtr()\n", 
SkuName, DefaultValueName, TokenSpaceGuidName, TokenName);
-- 
2.11.0.windows.1

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


Re: [edk2] [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data

2018-01-02 Thread Vladimir Olovyannikov
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, January 2, 2018 7:51 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindh...@linaro.org; vladimir.olovyanni...@broadcom.com;
> udit.ku...@nxp.com; meenakshi.aggar...@nxp.com; Ard Biesheuvel
> Subject: [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV
> region as boot services data
>
> Commit 8ae5fc182941 ("ArmPlatformPkg/MemoryInitPeiLib: don't reserve
> primary FV in memory") deleted the code that removes the memory
> covering the primary firmware volume from the memory map. The
> assumption was that this is no longer necessary now that we no longer
> expose compression and PE/COFF loader library code from the PrePi module
> to DXE core.
>
> However, the FV is still declared, and so code may attempt to access it
> anyway, which may cause unexpected results depending on whether the
> memory has been reused for other purposes in the mean time.
>
> So reinstate the code that splits off the resource descriptor HOB that
> describes the firmware device, but this time, don't mark the memory as
> unusable, but create a memory allocation HOB that marks the region as
boot
> services data.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
> Vladimir, Udit, Meenakshi: please confirm whether this code works for
you.
Thanks Ard,
Works for me.

Vladimir
>
>  ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 74
> 
>  1 file changed, 74 insertions(+)
>
> diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> index d03214b5df66..d1b5c0be9497 100644
> --- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> +++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
> @@ -70,7 +70,11 @@ MemoryPeim (
>  {
>ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
>EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
> +  UINT64   ResourceLength;
>EFI_PEI_HOB_POINTERS NextHob;
> +  EFI_PHYSICAL_ADDRESS FdTop;
> +  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
> +  EFI_PHYSICAL_ADDRESS ResourceTop;
>BOOLEAN  Found;
>
>// Get Virtual Memory Map from the Platform Library @@ -117,6 +121,76
> @@ MemoryPeim (
>  );
>}
>
> +  //
> +  // Reserve the memory space occupied by the firmware volume  //
> +
> +  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> + (PcdSystemMemoryBase) + (EFI_PHYSICAL_ADDRESS)PcdGet64
> + (PcdSystemMemorySize);  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64
> + (PcdFdBaseAddress) + (EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
> +
> +  // EDK2 does not have the concept of boot firmware copied into DRAM.
> + To avoid the DXE  // core to overwrite this area we must create a
> + memory allocation HOB for the region,  // but this only works if we
split off
> the underlying resource descriptor as well.
> +  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64
> (PcdSystemMemoryBase)) && (FdTop <= SystemMemoryTop)) {
> +Found = FALSE;
> +
> +// Search for System Memory Hob that contains the firmware
> +NextHob.Raw = GetHobList ();
> +while ((NextHob.Raw = GetNextHob
> (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, NextHob.Raw)) != NULL) {
> +  if ((NextHob.ResourceDescriptor->ResourceType ==
> EFI_RESOURCE_SYSTEM_MEMORY) &&
> +  (PcdGet64 (PcdFdBaseAddress) >= NextHob.ResourceDescriptor-
> >PhysicalStart) &&
> +  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart +
> NextHob.ResourceDescriptor->ResourceLength))
> +  {
> +ResourceAttributes = NextHob.ResourceDescriptor-
> >ResourceAttribute;
> +ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
> +ResourceTop = NextHob.ResourceDescriptor->PhysicalStart +
> + ResourceLength;
> +
> +if (PcdGet64 (PcdFdBaseAddress) == NextHob.ResourceDescriptor-
> >PhysicalStart) {
> +  if (SystemMemoryTop != FdTop) {
> +// Create the System Memory HOB for the firmware with the
non-
> present attribute
> +BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> +ResourceAttributes,
> +PcdGet64 (PcdFdBaseAddress),
> +PcdGet32 (PcdFdSize));
> +
> +// Top of the FD is system memory available for UEFI
> +NextHob.ResourceDescriptor->PhysicalStart +=
PcdGet32(PcdFdSize);
> +NextHob.ResourceDescriptor->ResourceLength -=
> PcdGet32(PcdFdSize);
> +  }
> +} else {
> +  // Create the System Memory HOB for the firmware
> +  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
> +  ResourceAttributes,
> +  PcdGet64 (PcdFdBaseAddress),
> +  PcdGet32 (PcdFdSize));
> +
> +  // Update the 

[edk2] [PATCH] BaseTools/DevicePath: fix GCC build error in print_mem(), and clean it up

2018-01-02 Thread Laszlo Ersek
Currently "BaseTools/Source/C/DevicePath/DevicePath.c" fails to build with
GCC48:

> DevicePath.c: In function 'print_mem':
> DevicePath.c:109:5: error: 'for' loop initial declarations are only
> allowed in C99 mode
>  for (size_t i=0; i  ^
> DevicePath.c:109:5: note: use option -std=c99 or -std=gnu99 to compile
> your code

In addition, the print_mem() function does not conform to the edk2 coding
style:

- we use CamelCase and no underscores in identifiers,
- the types and type qualifiers should follow the edk2 style,
- initialization as part of definition is forbidden for local variables.

Clean these up.

While updating the print_mem()/PrintMem() call sites, also remove the
superfluous parentheses around the second argument.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Fixes: 7dbc50bd244d95fdc1741b9cfc561f0bfd724de1
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
Build-tested only (I don't have a DSC file with a device path PCD).

 BaseTools/Source/C/DevicePath/DevicePath.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c 
b/BaseTools/Source/C/DevicePath/DevicePath.c
index 4c87163209ab..76b8553b7145 100644
--- a/BaseTools/Source/C/DevicePath/DevicePath.c
+++ b/BaseTools/Source/C/DevicePath/DevicePath.c
@@ -103,11 +103,19 @@ Returns:
 }
 
 
-void print_mem(void const *vp, size_t n)
+STATIC
+VOID
+PrintMem (
+  CONST VOID *Buffer,
+  UINTN  Count
+  )
 {
-unsigned char const *p = vp;
-for (size_t i=0; iType == END_DEVICE_PATH_TYPE) && (DevicePath->SubType 
== END_ENTIRE_DEVICE_PATH_SUBTYPE)) )
   {
-print_mem(DevicePath, (DevicePath->Length[0] | DevicePath->Length[1] << 
8));
+PrintMem (DevicePath, DevicePath->Length[0] | DevicePath->Length[1] << 8);
 DevicePath = (EFI_DEVICE_PATH_PROTOCOL *)((UINT8 *)DevicePath + 
(DevicePath->Length[0] | DevicePath->Length[1] << 8));
   }
-  print_mem(DevicePath, (DevicePath->Length[0] | DevicePath->Length[1] << 8));
+  PrintMem (DevicePath, DevicePath->Length[0] | DevicePath->Length[1] << 8);
   putchar('\n');
   return STATUS_SUCCESS;
 }
-- 
2.14.1.3.gb7cf6e02401b

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


Re: [edk2] [PATCH edk2-platforms 0/4] NXP:LS2088A RDB Board Support

2018-01-02 Thread Ard Biesheuvel
Hello Wasim, Udit, Meenakshi,

I kind of lost track of all the patches you have been sending over the
past month or so.
Could one of you please send a quick summary of all the patches that
are in flight at the moment, and for which platforms?

Thanks,
Ard.


On 22 December 2017 at 10:51, Wasim Khan  wrote:
> In Silicon/NXP, we are keeping our SoC specific information and remaining 
> code will be kept in Platform/NXP.
>
> Following patches will add support of NXP LS2088A RDB board in edk2-platforms.
>
> Platform/NXP/LS2088aRdbPkg will host .dsc and .fdf files to support 
> compilation for LS2088A RDB board.
>
> Looking forward for your kind support in upstreaming LS2088 RDB board support 
> in edk2-platforms.
>
> Wasim Khan (4):
>   Platform/NXP: Add support for ArmPlatformLib
>   Silicon/Maxim: Added Support for DS3232 RTC Library
>   Silicon/NXP:SocLib support for initialization of peripherals
>   Compilation : Add the fdf, dsc and dec files.
>
>  .../LS2088aRdbPkg/Include/Library/PlatformLib.h|  28 ++
>  Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dec   |  29 ++
>  Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc   |  95 ++
>  Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf   | 197 +++
>  .../Library/PlatformLib/ArmPlatformLib.c   | 106 ++
>  .../Library/PlatformLib/ArmPlatformLib.inf |  80 +
>  .../Library/PlatformLib/NxpQoriqLsHelper.S |  35 ++
>  .../Library/PlatformLib/NxpQoriqLsMem.c| 196 +++
>  Platform/NXP/LS2088aRdbPkg/VarStore.fdf.inc|  98 ++
>  Platform/NXP/NxpQoriqLs.dec|   1 +
>  Silicon/Maxim/Library/Ds3232RtcLib/Ds3232Rtc.h |  49 +++
>  Silicon/Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.c  | 370 
> +
>  .../Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.dec|  31 ++
>  .../Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.inf|  50 +++
>  Silicon/NXP/Chassis/Chassis.c  |  32 ++
>  Silicon/NXP/Chassis/Chassis.h  |  17 +
>  Silicon/NXP/Chassis/Chassis3/Chassis3.dec  |  19 ++
>  Silicon/NXP/Chassis/Chassis3/Errata.c  |  62 
>  Silicon/NXP/Chassis/Chassis3/SerDes.h  |  92 +
>  Silicon/NXP/Chassis/Chassis3/Soc.c | 171 ++
>  Silicon/NXP/Chassis/Chassis3/Soc.h | 159 +
>  Silicon/NXP/Chassis/LS2088aSocLib.inf  |  53 +++
>  Silicon/NXP/LS2088A/Include/SocSerDes.h|  67 
>  Silicon/NXP/LS2088A/LS2088A.dec|  22 ++
>  Silicon/NXP/LS2088A/LS2088A.dsc| 101 ++
>  25 files changed, 2160 insertions(+)
>  create mode 100755 Platform/NXP/LS2088aRdbPkg/Include/Library/PlatformLib.h
>  create mode 100644 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dec
>  create mode 100755 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
>  create mode 100644 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
>  create mode 100644 
> Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/ArmPlatformLib.c
>  create mode 100644 
> Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
>  create mode 100644 
> Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/NxpQoriqLsHelper.S
>  create mode 100644 
> Platform/NXP/LS2088aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
>  create mode 100644 Platform/NXP/LS2088aRdbPkg/VarStore.fdf.inc
>  create mode 100644 Silicon/Maxim/Library/Ds3232RtcLib/Ds3232Rtc.h
>  create mode 100644 Silicon/Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.c
>  create mode 100644 Silicon/Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.dec
>  create mode 100644 Silicon/Maxim/Library/Ds3232RtcLib/Ds3232RtcLib.inf
>  create mode 100644 Silicon/NXP/Chassis/Chassis3/Chassis3.dec
>  create mode 100644 Silicon/NXP/Chassis/Chassis3/Errata.c
>  create mode 100644 Silicon/NXP/Chassis/Chassis3/SerDes.h
>  create mode 100644 Silicon/NXP/Chassis/Chassis3/Soc.c
>  create mode 100644 Silicon/NXP/Chassis/Chassis3/Soc.h
>  create mode 100644 Silicon/NXP/Chassis/LS2088aSocLib.inf
>  create mode 100644 Silicon/NXP/LS2088A/Include/SocSerDes.h
>  create mode 100644 Silicon/NXP/LS2088A/LS2088A.dec
>  create mode 100644 Silicon/NXP/LS2088A/LS2088A.dsc
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data

2018-01-02 Thread Ard Biesheuvel
Commit 8ae5fc182941 ("ArmPlatformPkg/MemoryInitPeiLib: don't reserve
primary FV in memory") deleted the code that removes the memory covering
the primary firmware volume from the memory map. The assumption was that
this is no longer necessary now that we no longer expose compression and
PE/COFF loader library code from the PrePi module to DXE core.

However, the FV is still declared, and so code may attempt to access it
anyway, which may cause unexpected results depending on whether the
memory has been reused for other purposes in the mean time.

So reinstate the code that splits off the resource descriptor HOB that
describes the firmware device, but this time, don't mark the memory as
unusable, but create a memory allocation HOB that marks the region as
boot services data.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
Vladimir, Udit, Meenakshi: please confirm whether this code works for you.

 ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c | 74 
 1 file changed, 74 insertions(+)

diff --git a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c 
b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
index d03214b5df66..d1b5c0be9497 100644
--- a/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
+++ b/ArmPlatformPkg/MemoryInitPei/MemoryInitPeiLib.c
@@ -70,7 +70,11 @@ MemoryPeim (
 {
   ARM_MEMORY_REGION_DESCRIPTOR *MemoryTable;
   EFI_RESOURCE_ATTRIBUTE_TYPE  ResourceAttributes;
+  UINT64   ResourceLength;
   EFI_PEI_HOB_POINTERS NextHob;
+  EFI_PHYSICAL_ADDRESS FdTop;
+  EFI_PHYSICAL_ADDRESS SystemMemoryTop;
+  EFI_PHYSICAL_ADDRESS ResourceTop;
   BOOLEAN  Found;
 
   // Get Virtual Memory Map from the Platform Library
@@ -117,6 +121,76 @@ MemoryPeim (
 );
   }
 
+  //
+  // Reserve the memory space occupied by the firmware volume
+  //
+
+  SystemMemoryTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemoryBase) + 
(EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdSystemMemorySize);
+  FdTop = (EFI_PHYSICAL_ADDRESS)PcdGet64 (PcdFdBaseAddress) + 
(EFI_PHYSICAL_ADDRESS)PcdGet32 (PcdFdSize);
+
+  // EDK2 does not have the concept of boot firmware copied into DRAM. To 
avoid the DXE
+  // core to overwrite this area we must create a memory allocation HOB for 
the region,
+  // but this only works if we split off the underlying resource descriptor as 
well.
+  if ((PcdGet64 (PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) && 
(FdTop <= SystemMemoryTop)) {
+Found = FALSE;
+
+// Search for System Memory Hob that contains the firmware
+NextHob.Raw = GetHobList ();
+while ((NextHob.Raw = GetNextHob (EFI_HOB_TYPE_RESOURCE_DESCRIPTOR, 
NextHob.Raw)) != NULL) {
+  if ((NextHob.ResourceDescriptor->ResourceType == 
EFI_RESOURCE_SYSTEM_MEMORY) &&
+  (PcdGet64 (PcdFdBaseAddress) >= 
NextHob.ResourceDescriptor->PhysicalStart) &&
+  (FdTop <= NextHob.ResourceDescriptor->PhysicalStart + 
NextHob.ResourceDescriptor->ResourceLength))
+  {
+ResourceAttributes = NextHob.ResourceDescriptor->ResourceAttribute;
+ResourceLength = NextHob.ResourceDescriptor->ResourceLength;
+ResourceTop = NextHob.ResourceDescriptor->PhysicalStart + 
ResourceLength;
+
+if (PcdGet64 (PcdFdBaseAddress) == 
NextHob.ResourceDescriptor->PhysicalStart) {
+  if (SystemMemoryTop != FdTop) {
+// Create the System Memory HOB for the firmware with the 
non-present attribute
+BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+PcdGet64 (PcdFdBaseAddress),
+PcdGet32 (PcdFdSize));
+
+// Top of the FD is system memory available for UEFI
+NextHob.ResourceDescriptor->PhysicalStart += PcdGet32(PcdFdSize);
+NextHob.ResourceDescriptor->ResourceLength -= PcdGet32(PcdFdSize);
+  }
+} else {
+  // Create the System Memory HOB for the firmware
+  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
+  ResourceAttributes,
+  PcdGet64 (PcdFdBaseAddress),
+  PcdGet32 (PcdFdSize));
+
+  // Update the HOB
+  NextHob.ResourceDescriptor->ResourceLength = PcdGet64 
(PcdFdBaseAddress) - NextHob.ResourceDescriptor->PhysicalStart;
+
+  // If there is some memory available on the top of the FD then 
create a HOB
+  if (FdTop < NextHob.ResourceDescriptor->PhysicalStart + 
ResourceLength) {
+// Create the System Memory HOB for the remaining region (top of 
the FD)
+BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY,
+ResourceAttributes,
+FdTop,
+

Re: [edk2] [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding standard

2018-01-02 Thread Ard Biesheuvel
On 2 January 2018 at 15:11, Evan Lloyd  wrote:
> Hi Ard.
> One aim of these changes is to get those files we have to play with into a 
> state where a beautifier like indent, astyle,  or clang-format can be used to 
> help tidy our changes.  (NOTE, we do not have that fully working yet, but 
> they do help.)  In a world where we have to play with several contradictory 
> formatting standards (not just EDK2) then anything that can help is welcome.
> Of the changes made:
> Fixing the include guards: is a small improvement.  (Ideally 
> patchcheck should reject these.)
> Reducing lines to 80 columns: makes Leif (at least) happy, and aligns 
> with formatter behaviour.
> Correcting Doxygen format comments: prevents Doxygen generating 
> gibberish.
> Spaces before '(': Maintains consistency, and aligns with desired 
> formatter behaviour.
>

To be honest, this is an aspect I hadn't considered at all. It would
be excellent if we could use tooling to fix our code wrt to coding
style, and if changes such as these bring us closer to that goal, I am
all for it.

Would it be feasible to run that on entire packages, i.e., ArmPkg and
ArmPlatformPkg?

> More below:
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: 23 December 2017 13:19
>> To: Evan Lloyd 
>> Cc: edk2-devel@lists.01.org; "ard.biesheu...@linaro.org"@arm.com;
>> "leif.lindh...@linaro.org"@arm.com;
>> "matteo.carl...@arm.com"@arm.com; "n...@arm.com"@arm.com
>> Subject: Re: [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding
>> standard
>>
>> On 22 December 2017 at 18:34,   wrote:
>> > From: Girish Pathak 
>> >
>> > There is no functional modification in this change As preparation for
>> > further work, the formatting is corrected to meet the EDKII coding
>> > standard.
>> > Of specific note, some invalid include guards were fixed.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Girish Pathak 
>> > Signed-off-by: Evan Lloyd 
>>
>> Hi Girish, Evan,
>>
>> I am sorry, but I really don't see the point of this patch. Given that the
>> coding standard is not in line with common practice in Tianocore, changing
>> comments to remove empty lines after // or changing one style to the
>> other is just pointless churn. Also, changes like
>>
>> >  VOID
>> > -LcdShutdown (
>> > -  VOID
>> > -  )
>> > +LcdShutdown (VOID)
>> >  {
>>
>> look backward to me, and so if the coding standard mandates that, we
>> should changes the coding standard, not the code.
>>
>> --
>> Ard.
>>
> [[Evan Lloyd]] Hi Ard.
> The coding standard doesn't mandate this format, but permits it (5.7.1.5).
> Our case is that whilst either format is acceptable, consistency is 
> desirable, so we aimed (however imperfectly) to use a consistent style.
> In this instance, though, this  would be reverted by the formatting tools, so 
> I agree that it is pointless.
>
>>
>>
>> > ---
>> >
>> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h |
>> 10 +-
>> >  ArmPlatformPkg/Include/Library/LcdPlatformLib.h|  14 
>> > +-
>> >  ArmPlatformPkg/Library/HdLcd/HdLcd.h   |  21 
>> > ++-
>> >
>> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c |
>> 187 +++-
>> >  ArmPlatformPkg/Library/HdLcd/HdLcd.c   |  96 
>> > +
>> -
>> >  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c |  72 
>> > --
>> --
>> >  6 files changed, 212 insertions(+), 188 deletions(-)
>> >
>> > diff --git
>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>> h
>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>> h
>> > index
>> b66efd34561f655b74a5ecfad8a97281cdd5929d..2b001b107927fc75317ce
>> 39d370049d7740953a8 100644
>> > ---
>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>> h
>> > +++
>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>> h
>> > @@ -1,6 +1,6 @@
>> >  /** @file
>> >
>> > -  Copyright (c) 2011, ARM Ltd. All rights reserved.
>> > +  Copyright (c) 2011-2017, ARM Ltd. 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
>> > @@ -11,9 +11,8 @@
>> >
>> >  **/
>> >
>> > -#ifndef __ARM_VE_GRAPHICS_DXE_H__
>> > -#define __ARM_VE_GRAPHICS_DXE_H__
>> > -
>> > +#ifndef LCD_GRAPHICS_OUTPUT_DXE_H_
>> > +#define LCD_GRAPHICS_OUTPUT_DXE_H_
>> >
>> >  #include 
>> >
>> > @@ -25,7 +24,6 @@
>> >
>> >  #include 
>> >
>> > -
>> >  //
>> >  // Device structures
>> >  //
>> > @@ -106,4 +104,4 @@ InitializeDisplay (
>> >IN LCD_INSTANCE* Instance
>> >  );
>> >
>> > -#endif /* 

Re: [edk2] [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding standard

2018-01-02 Thread Ard Biesheuvel
(fix Leif's email address)

On 2 January 2018 at 15:21, Ard Biesheuvel  wrote:
> On 2 January 2018 at 15:11, Evan Lloyd  wrote:
>> Hi Ard.
>> One aim of these changes is to get those files we have to play with into a 
>> state where a beautifier like indent, astyle,  or clang-format can be used 
>> to help tidy our changes.  (NOTE, we do not have that fully working yet, but 
>> they do help.)  In a world where we have to play with several contradictory 
>> formatting standards (not just EDK2) then anything that can help is welcome.
>> Of the changes made:
>> Fixing the include guards: is a small improvement.  (Ideally 
>> patchcheck should reject these.)
>> Reducing lines to 80 columns: makes Leif (at least) happy, and 
>> aligns with formatter behaviour.
>> Correcting Doxygen format comments: prevents Doxygen generating 
>> gibberish.
>> Spaces before '(': Maintains consistency, and aligns with desired 
>> formatter behaviour.
>>
>
> To be honest, this is an aspect I hadn't considered at all. It would
> be excellent if we could use tooling to fix our code wrt to coding
> style, and if changes such as these bring us closer to that goal, I am
> all for it.
>
> Would it be feasible to run that on entire packages, i.e., ArmPkg and
> ArmPlatformPkg?
>
>> More below:
>>
>>> -Original Message-
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: 23 December 2017 13:19
>>> To: Evan Lloyd 
>>> Cc: edk2-devel@lists.01.org; "ard.biesheu...@linaro.org"@arm.com;
>>> "leif.lindh...@linaro.org"@arm.com;
>>> "matteo.carl...@arm.com"@arm.com; "n...@arm.com"@arm.com
>>> Subject: Re: [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding
>>> standard
>>>
>>> On 22 December 2017 at 18:34,   wrote:
>>> > From: Girish Pathak 
>>> >
>>> > There is no functional modification in this change As preparation for
>>> > further work, the formatting is corrected to meet the EDKII coding
>>> > standard.
>>> > Of specific note, some invalid include guards were fixed.
>>> >
>>> > Contributed-under: TianoCore Contribution Agreement 1.1
>>> > Signed-off-by: Girish Pathak 
>>> > Signed-off-by: Evan Lloyd 
>>>
>>> Hi Girish, Evan,
>>>
>>> I am sorry, but I really don't see the point of this patch. Given that the
>>> coding standard is not in line with common practice in Tianocore, changing
>>> comments to remove empty lines after // or changing one style to the
>>> other is just pointless churn. Also, changes like
>>>
>>> >  VOID
>>> > -LcdShutdown (
>>> > -  VOID
>>> > -  )
>>> > +LcdShutdown (VOID)
>>> >  {
>>>
>>> look backward to me, and so if the coding standard mandates that, we
>>> should changes the coding standard, not the code.
>>>
>>> --
>>> Ard.
>>>
>> [[Evan Lloyd]] Hi Ard.
>> The coding standard doesn't mandate this format, but permits it (5.7.1.5).
>> Our case is that whilst either format is acceptable, consistency is 
>> desirable, so we aimed (however imperfectly) to use a consistent style.
>> In this instance, though, this  would be reverted by the formatting tools, 
>> so I agree that it is pointless.
>>
>>>
>>>
>>> > ---
>>> >
>>> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h |
>>> 10 +-
>>> >  ArmPlatformPkg/Include/Library/LcdPlatformLib.h|  14 
>>> > +-
>>> >  ArmPlatformPkg/Library/HdLcd/HdLcd.h   |  21 
>>> > ++-
>>> >
>>> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c |
>>> 187 +++-
>>> >  ArmPlatformPkg/Library/HdLcd/HdLcd.c   |  96 
>>> > +
>>> -
>>> >  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c |  72 
>>> > --
>>> --
>>> >  6 files changed, 212 insertions(+), 188 deletions(-)
>>> >
>>> > diff --git
>>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> > index
>>> b66efd34561f655b74a5ecfad8a97281cdd5929d..2b001b107927fc75317ce
>>> 39d370049d7740953a8 100644
>>> > ---
>>> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> > +++
>>> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
>>> h
>>> > @@ -1,6 +1,6 @@
>>> >  /** @file
>>> >
>>> > -  Copyright (c) 2011, ARM Ltd. All rights reserved.
>>> > +  Copyright (c) 2011-2017, ARM Ltd. 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
>>> > @@ -11,9 +11,8 @@
>>> >
>>> >  **/
>>> >
>>> > -#ifndef __ARM_VE_GRAPHICS_DXE_H__
>>> > -#define __ARM_VE_GRAPHICS_DXE_H__
>>> > -
>>> > +#ifndef LCD_GRAPHICS_OUTPUT_DXE_H_
>>> > +#define LCD_GRAPHICS_OUTPUT_DXE_H_
>>> 

Re: [edk2] [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding standard

2018-01-02 Thread Evan Lloyd
Hi Ard.
One aim of these changes is to get those files we have to play with into a 
state where a beautifier like indent, astyle,  or clang-format can be used to 
help tidy our changes.  (NOTE, we do not have that fully working yet, but they 
do help.)  In a world where we have to play with several contradictory 
formatting standards (not just EDK2) then anything that can help is welcome.
Of the changes made:
Fixing the include guards: is a small improvement.  (Ideally patchcheck 
should reject these.)
Reducing lines to 80 columns: makes Leif (at least) happy, and aligns 
with formatter behaviour.
Correcting Doxygen format comments: prevents Doxygen generating 
gibberish.
Spaces before '(': Maintains consistency, and aligns with desired 
formatter behaviour.

More below:

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: 23 December 2017 13:19
> To: Evan Lloyd 
> Cc: edk2-devel@lists.01.org; "ard.biesheu...@linaro.org"@arm.com;
> "leif.lindh...@linaro.org"@arm.com;
> "matteo.carl...@arm.com"@arm.com; "n...@arm.com"@arm.com
> Subject: Re: [PATCH v2 01/13] ArmPlatformPkg: Tidy Lcd code: Coding
> standard
> 
> On 22 December 2017 at 18:34,   wrote:
> > From: Girish Pathak 
> >
> > There is no functional modification in this change As preparation for
> > further work, the formatting is corrected to meet the EDKII coding
> > standard.
> > Of specific note, some invalid include guards were fixed.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Girish Pathak 
> > Signed-off-by: Evan Lloyd 
> 
> Hi Girish, Evan,
> 
> I am sorry, but I really don't see the point of this patch. Given that the
> coding standard is not in line with common practice in Tianocore, changing
> comments to remove empty lines after // or changing one style to the
> other is just pointless churn. Also, changes like
> 
> >  VOID
> > -LcdShutdown (
> > -  VOID
> > -  )
> > +LcdShutdown (VOID)
> >  {
> 
> look backward to me, and so if the coding standard mandates that, we
> should changes the coding standard, not the code.
> 
> --
> Ard.
> 
[[Evan Lloyd]] Hi Ard.
The coding standard doesn't mandate this format, but permits it (5.7.1.5).
Our case is that whilst either format is acceptable, consistency is desirable, 
so we aimed (however imperfectly) to use a consistent style.
In this instance, though, this  would be reverted by the formatting tools, so I 
agree that it is pointless.

> 
> 
> > ---
> >
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.h |
> 10 +-
> >  ArmPlatformPkg/Include/Library/LcdPlatformLib.h|  14 +-
> >  ArmPlatformPkg/Library/HdLcd/HdLcd.h   |  21 
> > ++-
> >
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.c |
> 187 +++-
> >  ArmPlatformPkg/Library/HdLcd/HdLcd.c   |  96 
> > +
> -
> >  ArmPlatformPkg/Library/PL111Lcd/PL111Lcd.c |  72 
> > --
> --
> >  6 files changed, 212 insertions(+), 188 deletions(-)
> >
> > diff --git
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> > index
> b66efd34561f655b74a5ecfad8a97281cdd5929d..2b001b107927fc75317ce
> 39d370049d7740953a8 100644
> > ---
> a/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> > +++
> b/ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.
> h
> > @@ -1,6 +1,6 @@
> >  /** @file
> >
> > -  Copyright (c) 2011, ARM Ltd. All rights reserved.
> > +  Copyright (c) 2011-2017, ARM Ltd. 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
> > @@ -11,9 +11,8 @@
> >
> >  **/
> >
> > -#ifndef __ARM_VE_GRAPHICS_DXE_H__
> > -#define __ARM_VE_GRAPHICS_DXE_H__
> > -
> > +#ifndef LCD_GRAPHICS_OUTPUT_DXE_H_
> > +#define LCD_GRAPHICS_OUTPUT_DXE_H_
> >
> >  #include 
> >
> > @@ -25,7 +24,6 @@
> >
> >  #include 
> >
> > -
> >  //
> >  // Device structures
> >  //
> > @@ -106,4 +104,4 @@ InitializeDisplay (
> >IN LCD_INSTANCE* Instance
> >  );
> >
> > -#endif /* __ARM_VE_GRAPHICS_DXE_H__ */
> > +#endif /* LCD_GRAPHICS_OUTPUT_DXE_H_ */
> > diff --git a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> > index
> b9bdf471e2d65dba7a0fcb0f7ecc352bd576b46b..b9316ec8de8425a83e2f6
> 27f5c24821ff9a2f750 100644
> > --- a/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> > +++ b/ArmPlatformPkg/Include/Library/LcdPlatformLib.h
> > @@ -1,6 +1,6 @@
> >  /** @file
> >
> > - Copyright (c) 2011, ARM Ltd. All rights reserved.
> > + Copyright (c) 2011-2017, ARM Ltd. 

Re: [edk2] [RFC] MdeModulePkg/PciHostBridge: Add address translation support

2018-01-02 Thread gary guo
On Tue, Jan 02, 2018 at 03:56:14PM +0800, Ni, Ruiyu wrote:
> On 12/26/2017 2:50 PM, Guo Heyi wrote:
> > Hi Ard, Ray,
> > 
> > Have we come to the final conclusion? Or are we still waiting for more 
> > comments on this?
> 
> Heyi,
> I think you can send out a draft version of changes for better
> understanding.

Sure, we can do that.
Thanks,

Gary

> 
> > 
> > Thanks,
> > 
> > Gary
> > 
> > On Thu, Dec 21, 2017 at 10:07:51AM +, Ard Biesheuvel wrote:
> > > On 21 December 2017 at 09:59, Ni, Ruiyu  wrote:
> > > > On 12/21/2017 5:52 PM, Ard Biesheuvel wrote:
> > > > > 
> > > > > On 21 December 2017 at 09:48, Ni, Ruiyu  wrote:
> > > > > > 
> > > > > > On 12/21/2017 5:14 PM, Guo Heyi wrote:
> > > > > > > 
> > > > > > > 
> > > > > > > On Thu, Dec 21, 2017 at 08:32:37AM +, Ard Biesheuvel wrote:
> > > > > > > > 
> > > > > > > > 
> > > > > > > > On 21 December 2017 at 08:27, Guo Heyi  
> > > > > > > > wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > On Wed, Dec 20, 2017 at 03:26:45PM +, Ard Biesheuvel 
> > > > > > > > > wrote:
> > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > On 20 December 2017 at 15:17, gary guo 
> > > > > > > > > >  wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > On Wed, Dec 20, 2017 at 09:13:58AM +, Ard Biesheuvel 
> > > > > > > > > > > wrote:
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > Hi Heyi,
> > > > > > > > > > > > 
> > > > > > > > > > > > On 20 December 2017 at 08:21, Heyi Guo 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 
> > > > > > > > > > > > > PCIe on some ARM platforms requires address 
> > > > > > > > > > > > > translation, not only
> > > > > > > > > > > > > for
> > > > > > > > > > > > > legacy IO access, but also for 32bit memory BAR 
> > > > > > > > > > > > > access as well.
> > > > > > > > > > > > > There
> > > > > > > > > > > > > will be "Address Translation Unit" or something 
> > > > > > > > > > > > > similar in PCI
> > > > > > > > > > > > > host
> > > > > > > > > > > > > bridges to translation CPU address to PCI address and 
> > > > > > > > > > > > > vice versa.
> > > > > > > > > > > > > So
> > > > > > > > > > > > > we think it may be useful to add address translation 
> > > > > > > > > > > > > support to
> > > > > > > > > > > > > the
> > > > > > > > > > > > > generic PCI host bridge driver.
> > > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > I agree. While unusual on a PC, it is quite common on 
> > > > > > > > > > > > other
> > > > > > > > > > > > architectures to have more complex non 1:1 topologies, 
> > > > > > > > > > > > which
> > > > > > > > > > > > currently
> > > > > > > > > > > > require a forked PciHostBridgeDxe driver with local 
> > > > > > > > > > > > changes
> > > > > > > > > > > > applied.
> > > > > > > > > > > > 
> > > > > > > > > > > > > This RFC only contains one minor change to the 
> > > > > > > > > > > > > definition of
> > > > > > > > > > > > > PciHostBridgeLib, and there certainly will be a lot 
> > > > > > > > > > > > > of other
> > > > > > > > > > > > > changes
> > > > > > > > > > > > > to make it work, including:
> > > > > > > > > > > > > 
> > > > > > > > > > > > > 1. Use CPU address for GCD space add and allocate 
> > > > > > > > > > > > > operations,
> > > > > > > > > > > > > instead
> > > > > > > > > > > > > of PCI address; also IO space will be changed to 
> > > > > > > > > > > > > memory space if
> > > > > > > > > > > > > translation exists.
> > > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > For I/O space, the translation should simply be applied 
> > > > > > > > > > > > to the I/O
> > > > > > > > > > > > range. I don't think it makes sense to use memory space 
> > > > > > > > > > > > here, given
> > > > > > > > > > > > that it is specific to architectures that lack native 
> > > > > > > > > > > > port I/O.
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > I made an assumption here that platforms supporting real 
> > > > > > > > > > > port IO
> > > > > > > > > > > space
> > > > > > > > > > > do not need address translation, like IA32 and X64, and 
> > > > > > > > > > > port IO
> > > > > > > > > > > translation implies the platform does not support real 
> > > > > > > > > > > port IO
> > > > > > > > > > > space.
> > > > > > > > > > > 
> > > > > > > > > > 
> > > > > > > > > > This may be a reasonable assumption. But I still think it 
> > > > > > > > > > is better
> > > > > > > > > > not to encode any assumptions in the first place.
> > > > > > > > > > 
> > > > > > > > > > > Indeed the assumption is not so "generic", so I'll agree 
> > > > > > > > > > > if you
> > > > > > > > > > > recommend to support IO to IO translation as well. But I 
> > > > > > > > > > > still hope
> > > > > > > > > > > to
> > > > > > > > > > > have IO to memory 

Re: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64 format string

2018-01-02 Thread Ard Biesheuvel
On 2 January 2018 at 12:18, Gao, Liming  wrote:
> Ard:
>   The issue is related to UINT64 definition in the different ARCHs. In 
> BaseTools X64 and IA32,  typedef uint64_t  UINT64. uint64_t doesn't work with 
> %llx. To support all archs and tool chain, I propose to use below style to 
> print X64 value.
>
> sprintf(PcdList[Index].Value, "0x%016llx", (unsigned long long) Value);
>

Yes, that looks like the best way forward. Some projects address this
by adding arch-dependent #defines of the format specifier string, but
I think that is overkill here.

-- 
Ard.


>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Tuesday, January 2, 2018 5:26 PM
>> To: Alex James 
>> Cc: Gao, Liming ; edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64 
>> format string
>>
>> On 2 January 2018 at 09:25, Ard Biesheuvel  wrote:
>> > On 29 December 2017 at 02:58, Alex James  wrote:
>> >> Hi Liming,
>> >>
>> >> I was able to reproduce this, will send out the v2 patch shortly.
>> >>
>> >
>> > This is still broken in today's tree. Could we get a fix please?
>> >
>>
>> Ehm, hold on. It is the other way around: on AARCH64, I now get
>>
>> PcdValueCommon.c: In function ‘__PcdSet’:
>> PcdValueCommon.c:270:43: error: format ‘%lx’ expects argument of type
>> ‘long unsigned int’, but argument 3 has type ‘UINT64 {aka long long
>> unsigned int}’ [-Werror=format=]
>>  sprintf(PcdList[Index].Value, "0x%016lx", Value);
>>
>>
>>
>>
>> >>
>> >> On Thu, Dec 28, 2017 at 7:46 PM Gao, Liming  wrote:
>> >>
>> >>> This fix will trig GCC build warning.
>> >>>
>> >>> PcdValueCommon.c: In function '__PcdSet':
>> >>> PcdValueCommon.c:269:35: error: format '%llx' expects argument of type
>> >>> 'long long unsigned int', but argument 3 has type 'UINT64 {aka long
>> >>> unsigned int}' [-Werror=format=]
>> >>>  sprintf(PcdList[Index].Value, "0x%016llx", Value);
>> >>>^
>> >>>
>> >>> Thanks
>> >>> Liming
>> >>> > -Original Message-
>> >>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> >>> Alex James
>> >>> > Sent: Friday, December 29, 2017 4:00 AM
>> >>> > To: edk2-devel@lists.01.org
>> >>> > Cc: Alex James 
>> >>> > Subject: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64
>> >>> format string
>> >>> >
>> >>> > Always specify unsigned long long for PcdDataTypeUint64. This is needed
>> >>> > to fix building with XCODE5.
>> >>> >
>> >>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> >>> > Signed-off-by: Alex James 
>> >>> > ---
>> >>> >  BaseTools/Source/C/Common/PcdValueCommon.c | 4 
>> >>> >  1 file changed, 4 deletions(-)
>> >>> >
>> >>> > diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c
>> >>> b/BaseTools/Source/C/Common/PcdValueCommon.c
>> >>> > index 6ca0994744..f5d68e79e0 100644
>> >>> > --- a/BaseTools/Source/C/Common/PcdValueCommon.c
>> >>> > +++ b/BaseTools/Source/C/Common/PcdValueCommon.c
>> >>> > @@ -266,11 +266,7 @@ Returns:
>> >>> >  sprintf(PcdList[Index].Value, "0x%08x", (UINT32)(Value &
>> >>> 0x));
>> >>> >  break;
>> >>> >case PcdDataTypeUint64:
>> >>> > -#ifdef __GNUC__
>> >>> > -sprintf(PcdList[Index].Value, "0x%016lx", Value);
>> >>> > -#else
>> >>> >  sprintf(PcdList[Index].Value, "0x%016llx", Value);
>> >>> > -#endif
>> >>> >  break;
>> >>> >case PcdDataTypePointer:
>> >>> >  fprintf (stderr, "PCD %s.%s.%s.%s is structure.  Use
>> >>> PcdSetPtr()\n", SkuName, DefaultValueName, TokenSpaceGuidName,
>> >>> > TokenName);
>> >>> > --
>> >>> > 2.15.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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] Fwd: FW: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in C Makefile (#122)

2018-01-02 Thread Chema Gonzalez
Sure.

Thanks,
-Chema

-- Forwarded message --
From: Gao, Liming 
Date: Wed, Dec 27, 2017 at 5:48 PM
Subject: FW: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in
C Makefile (#122)
To: "che...@gmail.com" 


Could you send patch to edk2-devel@lists.01.org?



From: chemag [mailto:notificati...@github.com]
Sent: Thursday, December 28, 2017 9:26 AM
To: tianocore/edk2 
Cc: Subscribed 
Subject: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in C
Makefile (#122)



I was getting HOST_ARCH set using the linux arch name ("x86_64"), which
is different from the MS one ("X64").

It is not clear anyway we can proceed without valid build variables
(ARCH_INCLUDE, BIN_PATH, LIB_PATH, SYS_BIN_PATH, and
SYS_LIB_PATH).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chema Gonzalez che...@gmail.com



You can view, comment on, or merge this pull request online at:

  https://github.com/tianocore/edk2/pull/122

Commit Summary

BaseTools: Barf on unknown HOST_ARCH in C Makefile

File Changes

M BaseTools/Source/C/Makefiles/ms.common (6)

Patch Links:

https://github.com/tianocore/edk2/pull/122.patch
https://github.com/tianocore/edk2/pull/122.diff

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
From b0657ee033daad00c996c561857d4b3aad1f47cf Mon Sep 17 00:00:00 2001
From: Chema Gonzalez 
Date: Wed, 27 Dec 2017 16:23:56 -0800
Subject: [PATCH] BaseTools: Barf on unknown HOST_ARCH in C Makefile

I was getting `HOST_ARCH` set using the linux arch name ("x86_64"), which
is different from the MS one ("X64").

It is not clear anyway we can proceed without valid build variables
(`ARCH_INCLUDE`, `BIN_PATH`, `LIB_PATH`, `SYS_BIN_PATH`, and
`SYS_LIB_PATH`).

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chema Gonzalez 
---
 BaseTools/Source/C/Makefiles/ms.common | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/ms.common b/BaseTools/Source/C/Makefiles/ms.common
index a6bfea5..06399df 100644
--- a/BaseTools/Source/C/Makefiles/ms.common
+++ b/BaseTools/Source/C/Makefiles/ms.common
@@ -42,14 +42,16 @@ BIN_PATH = $(BASE_TOOLS_PATH)\Bin\Win32
 LIB_PATH = $(BASE_TOOLS_PATH)\Lib\Win32
 SYS_BIN_PATH = $(EDK_TOOLS_PATH)\Bin\Win32
 SYS_LIB_PATH = $(EDK_TOOLS_PATH)\Lib\Win32
-!ENDIF
 
-!IF "$(HOST_ARCH)"=="X64"
+!ELSEIF "$(HOST_ARCH)"=="X64"
 ARCH_INCLUDE = $(SOURCE_PATH)\Include\X64
 BIN_PATH = $(BASE_TOOLS_PATH)\Bin\Win64
 LIB_PATH = $(BASE_TOOLS_PATH)\Lib\Win64
 SYS_BIN_PATH = $(EDK_TOOLS_PATH)\Bin\Win64
 SYS_LIB_PATH = $(EDK_TOOLS_PATH)\Lib\Win64
+
+!ELSE
+!ERROR "Unknown HOST_ARCH variable"
 !ENDIF
 
 CC = cl.exe
-- 
2.7.4

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


[edk2] [Patch] BaseTools: Fix the error about loop initial declarations

2018-01-02 Thread Yonghong Zhu
From: Yunhua Feng 

We met compile error about loop initial declarations are only allowed
in C99 or C11 mode on GCC48/GCC49.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/C/DevicePath/DevicePath.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePath.c 
b/BaseTools/Source/C/DevicePath/DevicePath.c
index 4c87163..4baa612 100644
--- a/BaseTools/Source/C/DevicePath/DevicePath.c
+++ b/BaseTools/Source/C/DevicePath/DevicePath.c
@@ -104,11 +104,12 @@ Returns:
 
 
 void print_mem(void const *vp, size_t n)
 {
 unsigned char const *p = vp;
-for (size_t i=0; i

Re: [edk2] [PATCH v2 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWICE' to SetJump

2018-01-02 Thread Gao, Liming
To align to longjump() declaration, could you update SetJump()?

UINTN
EFIAPI
RETURNS_TWICE
SetJump (
);

==>

RETURNS_TWICE
UINTN
EFIAPI
SetJump (
);

Thanks
Liming
> -Original Message-
> From: M1cha [mailto:sigmaepsilo...@gmail.com]
> Sent: Saturday, December 23, 2017 3:14 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Kinney, Michael D 
> ; Gao, Liming
> 
> Subject: [edk2] [PATCH v2 2/3] MdePkg/BaseLib: add attribute 'RETURNS_TWICE' 
> to SetJump
> 
> When compiling with any ARM toolchain and Os, registers can get
> trashed when returning for the second time from SetJump because GCC
> only handles this correctly when using standard names like 'setjmp' or
> 'getcontext'. When different names are used you have to use the
> attribute 'returns_twice' to tell gcc to be extra careful.
> 
> example:
> extern int  FN_NAME(void*);
> 
> void jmp_buf_set(void *jmpb, void (*f)(void))
> {
>   if (!FN_NAME(jmpb))
> f();
> }
> 
> this code produces this wrong code with Os:
>  :
>0: e92d4010 push {r4, lr}
>4: e1a04001 mov r4, r1
>8: ebfe bl 0 
>c: e350 cmp r0, #0
>   10: 01a03004 moveq r3, r4
>   14: 08bd4010 popeq {r4, lr}
>   18: 012fff13 bxeq r3
>   1c: e8bd4010 pop {r4, lr}
>   20: e12fff1e bx lr
> 
> The generated code pushes backups of r4 and lr to the stack and then
> saves all registers using nonstandard_setjmp.
> Then it pops the stack and jumps to the function in r3 which is the
> main problem because now the function can overwrite our register
> backups on the stack.
> When we return a second time from the call to nonstandard_setjmp, the
> stack pointer has it's original(pushed) position and when the code
> pops r4 and lr from the stack the values are not guaranteed to be the
> same.
> 
> When using a standard name like setjmp or getcontext or adding
> '__attribute__((returns_twice))' to nonstandard_setjmp's declaration
> the code looks different:
> 
>  :
>0: e92d4007 push {r0, r1, r2, lr}
>4: e58d1004 str r1, [sp, #4]
>8: ebfe bl 0 
>c: e350 cmp r0, #0
>   10: 059d3004 ldreq r3, [sp, #4]
>   14: 01a0e00f moveq lr, pc
>   18: 012fff13 bxeq r3
>   1c: e28dd00c add sp, sp, #12
>   20: e49de004 pop {lr} ; (ldr lr, [sp], #4)
>   24: e12fff1e bx lr
> 
> Here the problem is being solved by restoring r3 from the stack
> without popping it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Michael Zimmermann 
> ---
>  MdePkg/Include/Library/BaseLib.h | 1 +
>  MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c | 1 +
>  MdePkg/Library/BaseLib/Ia32/SetJump.c| 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/MdePkg/Include/Library/BaseLib.h 
> b/MdePkg/Include/Library/BaseLib.h
> index 2b98af4cd17e..10976032adaa 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -4905,6 +4905,7 @@ MemoryFence (
>  **/
>  UINTN
>  EFIAPI
> +RETURNS_TWICE
>  SetJump (
>OUT BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer
>);
> diff --git a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c 
> b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> index 4c0dba55d52f..e309e8b57d7a 100644
> --- a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> +++ b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> @@ -34,6 +34,7 @@
>  **/
>  UINTN
>  EFIAPI
> +RETURNS_TWICE
>  SetJump (
>OUT  BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer
>)
> diff --git a/MdePkg/Library/BaseLib/Ia32/SetJump.c 
> b/MdePkg/Library/BaseLib/Ia32/SetJump.c
> index 304f3839b108..40fd16bae8fd 100644
> --- a/MdePkg/Library/BaseLib/Ia32/SetJump.c
> +++ b/MdePkg/Library/BaseLib/Ia32/SetJump.c
> @@ -51,6 +51,7 @@ InternalAssertJumpBuffer (
>  _declspec (naked)
>  UINTN
>  EFIAPI
> +RETURNS_TWICE
>  SetJump (
>OUT BASE_LIBRARY_JUMP_BUFFER  *JumpBuffer
>)
> --
> 2.15.1

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


Re: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64 format string

2018-01-02 Thread Gao, Liming
Ard:
  The issue is related to UINT64 definition in the different ARCHs. In 
BaseTools X64 and IA32,  typedef uint64_t  UINT64. uint64_t doesn't work with 
%llx. To support all archs and tool chain, I propose to use below style to 
print X64 value.

sprintf(PcdList[Index].Value, "0x%016llx", (unsigned long long) Value);

Thanks
Liming
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, January 2, 2018 5:26 PM
> To: Alex James 
> Cc: Gao, Liming ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64 
> format string
> 
> On 2 January 2018 at 09:25, Ard Biesheuvel  wrote:
> > On 29 December 2017 at 02:58, Alex James  wrote:
> >> Hi Liming,
> >>
> >> I was able to reproduce this, will send out the v2 patch shortly.
> >>
> >
> > This is still broken in today's tree. Could we get a fix please?
> >
> 
> Ehm, hold on. It is the other way around: on AARCH64, I now get
> 
> PcdValueCommon.c: In function ‘__PcdSet’:
> PcdValueCommon.c:270:43: error: format ‘%lx’ expects argument of type
> ‘long unsigned int’, but argument 3 has type ‘UINT64 {aka long long
> unsigned int}’ [-Werror=format=]
>  sprintf(PcdList[Index].Value, "0x%016lx", Value);
> 
> 
> 
> 
> >>
> >> On Thu, Dec 28, 2017 at 7:46 PM Gao, Liming  wrote:
> >>
> >>> This fix will trig GCC build warning.
> >>>
> >>> PcdValueCommon.c: In function '__PcdSet':
> >>> PcdValueCommon.c:269:35: error: format '%llx' expects argument of type
> >>> 'long long unsigned int', but argument 3 has type 'UINT64 {aka long
> >>> unsigned int}' [-Werror=format=]
> >>>  sprintf(PcdList[Index].Value, "0x%016llx", Value);
> >>>^
> >>>
> >>> Thanks
> >>> Liming
> >>> > -Original Message-
> >>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >>> Alex James
> >>> > Sent: Friday, December 29, 2017 4:00 AM
> >>> > To: edk2-devel@lists.01.org
> >>> > Cc: Alex James 
> >>> > Subject: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64
> >>> format string
> >>> >
> >>> > Always specify unsigned long long for PcdDataTypeUint64. This is needed
> >>> > to fix building with XCODE5.
> >>> >
> >>> > Contributed-under: TianoCore Contribution Agreement 1.1
> >>> > Signed-off-by: Alex James 
> >>> > ---
> >>> >  BaseTools/Source/C/Common/PcdValueCommon.c | 4 
> >>> >  1 file changed, 4 deletions(-)
> >>> >
> >>> > diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c
> >>> b/BaseTools/Source/C/Common/PcdValueCommon.c
> >>> > index 6ca0994744..f5d68e79e0 100644
> >>> > --- a/BaseTools/Source/C/Common/PcdValueCommon.c
> >>> > +++ b/BaseTools/Source/C/Common/PcdValueCommon.c
> >>> > @@ -266,11 +266,7 @@ Returns:
> >>> >  sprintf(PcdList[Index].Value, "0x%08x", (UINT32)(Value &
> >>> 0x));
> >>> >  break;
> >>> >case PcdDataTypeUint64:
> >>> > -#ifdef __GNUC__
> >>> > -sprintf(PcdList[Index].Value, "0x%016lx", Value);
> >>> > -#else
> >>> >  sprintf(PcdList[Index].Value, "0x%016llx", Value);
> >>> > -#endif
> >>> >  break;
> >>> >case PcdDataTypePointer:
> >>> >  fprintf (stderr, "PCD %s.%s.%s.%s is structure.  Use
> >>> PcdSetPtr()\n", SkuName, DefaultValueName, TokenSpaceGuidName,
> >>> > TokenName);
> >>> > --
> >>> > 2.15.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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] BaseTools: Fix the regression bug of a74398 for SubFv Image

2018-01-02 Thread Gao, Liming
Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Tuesday, January 2, 2018 7:12 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch] BaseTools: Fix the regression bug of a74398 for SubFv 
> Image
> 
> in version a74398 we use guid value and Fv name as ffs dir for FILE
> statement, this patch apply this rule on subFv image.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/GenFds/Fv.py | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/Fv.py 
> b/BaseTools/Source/Python/GenFds/Fv.py
> index a69abb3..c0b869d 100644
> --- a/BaseTools/Source/Python/GenFds/Fv.py
> +++ b/BaseTools/Source/Python/GenFds/Fv.py
> @@ -1,9 +1,9 @@
>  ## @file
>  # process FV generation
>  #
> -#  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2007 - 2018, 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
> @@ -177,11 +177,11 @@ class FV (FvClassObject):
>  AddFileObj.close()
> 
>  if FvChildAddr != []:
>  # Update Ffs again
>  for FfsFile in self.FfsList :
> -FileName = FfsFile.GenFfs(MacroDict, FvChildAddr, 
> BaseAddress, IsMakefile=Flag)
> +FileName = FfsFile.GenFfs(MacroDict, FvChildAddr, 
> BaseAddress, IsMakefile=Flag,
> FvName=self.UiFvName)
> 
>  if GenFdsGlobalVariable.LargeFileInFvFlags[-1]:
>  FFSGuid = 
> GenFdsGlobalVariable.EFI_FIRMWARE_FILE_SYSTEM3_GUID;
>  #Update GenFv again
>  GenFdsGlobalVariable.GenerateFirmwareVolume(
> --
> 2.6.1.windows.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


[edk2] [Patch] BaseTools: Fix the regression bug of a74398 for SubFv Image

2018-01-02 Thread Yonghong Zhu
in version a74398 we use guid value and Fv name as ffs dir for FILE
statement, this patch apply this rule on subFv image.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/GenFds/Fv.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/Fv.py 
b/BaseTools/Source/Python/GenFds/Fv.py
index a69abb3..c0b869d 100644
--- a/BaseTools/Source/Python/GenFds/Fv.py
+++ b/BaseTools/Source/Python/GenFds/Fv.py
@@ -1,9 +1,9 @@
 ## @file
 # process FV generation
 #
-#  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2018, 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
@@ -177,11 +177,11 @@ class FV (FvClassObject):
 AddFileObj.close()
 
 if FvChildAddr != []:
 # Update Ffs again
 for FfsFile in self.FfsList :
-FileName = FfsFile.GenFfs(MacroDict, FvChildAddr, 
BaseAddress, IsMakefile=Flag)
+FileName = FfsFile.GenFfs(MacroDict, FvChildAddr, 
BaseAddress, IsMakefile=Flag, FvName=self.UiFvName)
 
 if GenFdsGlobalVariable.LargeFileInFvFlags[-1]:
 FFSGuid = 
GenFdsGlobalVariable.EFI_FIRMWARE_FILE_SYSTEM3_GUID;
 #Update GenFv again
 GenFdsGlobalVariable.GenerateFirmwareVolume(
-- 
2.6.1.windows.1

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


Re: [edk2] [PATCH edk2-platforms v2 00/18] ARM: Update GOP

2018-01-02 Thread Evan Lloyd
Hi Ard.
Happy New Year!
I have no idea what has caused that.  I haven't changed the script I use to 
generate patches, so I'm off to consult our IT guys to find out what's up.
I'll resume when I've done that.

Sorry,
Evan

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: 22 December 2017 19:30
> To: Evan Lloyd ; Leif Lindholm
> ; Ard Biesheuvel ;
> Matteo Carlini 
> Cc: edk2-devel@lists.01.org; Arvind Chauhan ;
> Daniil Egranov ; Thomas Abraham
> 
> Subject: Re: [PATCH edk2-platforms v2 00/18] ARM: Update GOP
>
> On 22 December 2017 at 19:08,   wrote:
> > From: EvanLloyd 
> >
>
> Hello Evan,
>
> Before reviewing in detail, could you please confirm that replying to the
> addresses below is going to work as expected for non @arm.com reviewers?
> They look a bit odd, but perhaps the arm.com SMTP server doesn't care??
>
> Cc: Arvind Chauhan , Daniil Egranov
> , Thomas Panakamattam Abraham
> , "ard.biesheu...@linaro.org"@arm.com,
> "leif.lindh...@linaro.org"@arm.com,
> "matteo.carl...@arm.com"@arm.com, "n...@arm.com"@arm.com
>
> --
> Ard.
>
>
> > This patch series addresses comments on the original
> > (https://lists.01.org/pipermail/edk2-devel/2017-September/015356.html)
> > reworking of the Graphics Output Protocol code in Platform/ARM.
> > It also contains updates for the new SCMI protocol (MTL Library).
> >
> > After a number of format and quality modifications, several errors
> > are corrected and new functionality added for Mali DP.
> >
> > The changes are tested on Juno, and FVP.
> >
> > Code is available for examination at:
> >   https://github.com/EvanLloyd/edk2-platforms/tree/166_gop_v2
> >
> > Ard Biesheuvel (1):
> >   ARM/VExpressPkg: Fix MODULE_TYPE of HDLCD/PL111 platform libraries
> >
> > EvanLloyd (1):
> >   ARM/VExpressPkg: HdLcdArmVExpressLib: Remove redundant Bpp
> >
> > Girish Pathak (16):
> >   ARM/VExpressPkg: Tidy HDLCD and PL11LCD platform Lib: Coding
> standard
> >   ARM/VExpressPkg: Tidy HdLcd/PL111Lcd code: Updated comments
> >   ARM/VExpressPkg: Remove unused PcdPL111LcdMaxMode from HDLCD
> inf
> >   ARM/VExpressPkg: PL111 and HDLCD: add const qualifier
> >   ARM/VExpressPkg: Add and update debug ASSERTS
> >   ARM/VExpressPkg: PL111LcdArmVExpressLib: Minor code cleanup
> >   ARM/VExpressPkg: PL111 and HDLCD: Use FixedPcdGet32
> >   ARM/VExpressPkg: PL11LcdArmVExpressLib: Improvement conditional
> >   ARM/VExpressPkg: HdLcdArmVExpressLib: Remove status check
> EFI_TIMEOUT
> >   ARM/VExpressPkg: Redefine LcdPlatformGetTimings function
> >   ARM/VExpressPkg: PL111 and HDLCD: Add PCD to select pixel format
> >   ARM/VExpressPkg: Reserving framebuffer at build
> >   ARM/VExpressPkg: New DP500/DP550/DP650 platform library.
> >   ARM/JunoPkg: Mapping Non-Trused SRAM as device memory
> >   ARM/JunoPkg: Adding SCMI MTL library
> >   ARM/JunoPkg: Add HDLCD platform library
> >
> >  Platform/ARM/JunoPkg/ArmJuno.dec   
> > |  17
> +-
> >  Platform/ARM/VExpressPkg/ArmVExpressPkg.dec
> |   3 +-
> >  Platform/ARM/JunoPkg/ArmJuno.dsc   
> > |  32
> ++
> >  Platform/ARM/JunoPkg/ArmJuno.fdf   
> > |  12
> +-
> >  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf
> |   5 +-
> >  Platform/ARM/JunoPkg/Library/ArmMtl/ArmMtl.inf
> |  39 ++
> >  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
> |  40 ++
> >  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.inf
> |  45 ++
> >
> Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/ArmVExpressLib
> .inf |   7 +-
> >
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
> ressLib.inf   |  13 +-
> >
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mVExpressLib.inf |   9 +-
> >  Platform/ARM/JunoPkg/Library/ArmMtl/ArmMtlPrivate.h
> |  94 
> >  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c
> |  24 +-
> >  Platform/ARM/JunoPkg/Library/ArmMtl/ArmMtl.c
> | 195 +++
> >  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c
> | 559 
> >  Platform/ARM/VExpressPkg/Library/ArmMaliDpLib/ArmMaliDpLib.c
> | 374 +
> >  Platform/ARM/VExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
> |  28 +-
> >
> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
> ress.c| 309 +++
> >
> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
> mVExpress.c  | 425 +--
> >  19 files changed, 1920 insertions(+), 310 deletions(-)
> >  create mode 100644
> Platform/ARM/JunoPkg/Library/ArmMtl/ArmMtl.inf
> >  create mode 100644
> 

Re: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64 format string

2018-01-02 Thread Ard Biesheuvel
On 2 January 2018 at 09:25, Ard Biesheuvel  wrote:
> On 29 December 2017 at 02:58, Alex James  wrote:
>> Hi Liming,
>>
>> I was able to reproduce this, will send out the v2 patch shortly.
>>
>
> This is still broken in today's tree. Could we get a fix please?
>

Ehm, hold on. It is the other way around: on AARCH64, I now get

PcdValueCommon.c: In function ‘__PcdSet’:
PcdValueCommon.c:270:43: error: format ‘%lx’ expects argument of type
‘long unsigned int’, but argument 3 has type ‘UINT64 {aka long long
unsigned int}’ [-Werror=format=]
 sprintf(PcdList[Index].Value, "0x%016lx", Value);




>>
>> On Thu, Dec 28, 2017 at 7:46 PM Gao, Liming  wrote:
>>
>>> This fix will trig GCC build warning.
>>>
>>> PcdValueCommon.c: In function '__PcdSet':
>>> PcdValueCommon.c:269:35: error: format '%llx' expects argument of type
>>> 'long long unsigned int', but argument 3 has type 'UINT64 {aka long
>>> unsigned int}' [-Werror=format=]
>>>  sprintf(PcdList[Index].Value, "0x%016llx", Value);
>>>^
>>>
>>> Thanks
>>> Liming
>>> > -Original Message-
>>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>> Alex James
>>> > Sent: Friday, December 29, 2017 4:00 AM
>>> > To: edk2-devel@lists.01.org
>>> > Cc: Alex James 
>>> > Subject: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64
>>> format string
>>> >
>>> > Always specify unsigned long long for PcdDataTypeUint64. This is needed
>>> > to fix building with XCODE5.
>>> >
>>> > Contributed-under: TianoCore Contribution Agreement 1.1
>>> > Signed-off-by: Alex James 
>>> > ---
>>> >  BaseTools/Source/C/Common/PcdValueCommon.c | 4 
>>> >  1 file changed, 4 deletions(-)
>>> >
>>> > diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c
>>> b/BaseTools/Source/C/Common/PcdValueCommon.c
>>> > index 6ca0994744..f5d68e79e0 100644
>>> > --- a/BaseTools/Source/C/Common/PcdValueCommon.c
>>> > +++ b/BaseTools/Source/C/Common/PcdValueCommon.c
>>> > @@ -266,11 +266,7 @@ Returns:
>>> >  sprintf(PcdList[Index].Value, "0x%08x", (UINT32)(Value &
>>> 0x));
>>> >  break;
>>> >case PcdDataTypeUint64:
>>> > -#ifdef __GNUC__
>>> > -sprintf(PcdList[Index].Value, "0x%016lx", Value);
>>> > -#else
>>> >  sprintf(PcdList[Index].Value, "0x%016llx", Value);
>>> > -#endif
>>> >  break;
>>> >case PcdDataTypePointer:
>>> >  fprintf (stderr, "PCD %s.%s.%s.%s is structure.  Use
>>> PcdSetPtr()\n", SkuName, DefaultValueName, TokenSpaceGuidName,
>>> > TokenName);
>>> > --
>>> > 2.15.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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64 format string

2018-01-02 Thread Ard Biesheuvel
On 29 December 2017 at 02:58, Alex James  wrote:
> Hi Liming,
>
> I was able to reproduce this, will send out the v2 patch shortly.
>

This is still broken in today's tree. Could we get a fix please?

>
> On Thu, Dec 28, 2017 at 7:46 PM Gao, Liming  wrote:
>
>> This fix will trig GCC build warning.
>>
>> PcdValueCommon.c: In function '__PcdSet':
>> PcdValueCommon.c:269:35: error: format '%llx' expects argument of type
>> 'long long unsigned int', but argument 3 has type 'UINT64 {aka long
>> unsigned int}' [-Werror=format=]
>>  sprintf(PcdList[Index].Value, "0x%016llx", Value);
>>^
>>
>> Thanks
>> Liming
>> > -Original Message-
>> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Alex James
>> > Sent: Friday, December 29, 2017 4:00 AM
>> > To: edk2-devel@lists.01.org
>> > Cc: Alex James 
>> > Subject: [edk2] [PATCH] BaseTools/PcdValueCommon: Fix PcdDataTypeUint64
>> format string
>> >
>> > Always specify unsigned long long for PcdDataTypeUint64. This is needed
>> > to fix building with XCODE5.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Alex James 
>> > ---
>> >  BaseTools/Source/C/Common/PcdValueCommon.c | 4 
>> >  1 file changed, 4 deletions(-)
>> >
>> > diff --git a/BaseTools/Source/C/Common/PcdValueCommon.c
>> b/BaseTools/Source/C/Common/PcdValueCommon.c
>> > index 6ca0994744..f5d68e79e0 100644
>> > --- a/BaseTools/Source/C/Common/PcdValueCommon.c
>> > +++ b/BaseTools/Source/C/Common/PcdValueCommon.c
>> > @@ -266,11 +266,7 @@ Returns:
>> >  sprintf(PcdList[Index].Value, "0x%08x", (UINT32)(Value &
>> 0x));
>> >  break;
>> >case PcdDataTypeUint64:
>> > -#ifdef __GNUC__
>> > -sprintf(PcdList[Index].Value, "0x%016lx", Value);
>> > -#else
>> >  sprintf(PcdList[Index].Value, "0x%016llx", Value);
>> > -#endif
>> >  break;
>> >case PcdDataTypePointer:
>> >  fprintf (stderr, "PCD %s.%s.%s.%s is structure.  Use
>> PcdSetPtr()\n", SkuName, DefaultValueName, TokenSpaceGuidName,
>> > TokenName);
>> > --
>> > 2.15.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
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdeModulePkg/MdeModulePkg.dec: clarify Heap Guard usage

2018-01-02 Thread Zeng, Star
Reviewed-by: Star Zeng  after you also update the 
MdeModulePkg.uni accordingly.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jian J 
Wang
Sent: Tuesday, January 2, 2018 4:27 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
Subject: [edk2] [PATCH] MdeModulePkg/MdeModulePkg.dec: clarify Heap Guard usage

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/MdeModulePkg.dec | 12 
 1 file changed, 12 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec 
index 037b16e2d0..491fb27663 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -893,6 +893,12 @@
   
gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack|0x5AA55AA5|UINT32|0x30001051
 
   ## Indicates which type allocation need guard page.
+  #
+  # If a bit is set, a head guard page and a tail guard page will be 
+ added just  # before and after corresponding type of pages allocated 
+ if there's enough  # free pages for all of them. The page allocation 
+ for the type related to  # cleared bits keeps the same as ususal.
+  #
   # Below is bit mask for this PCD: (Order is same as UEFI spec)
   #  EfiReservedMemoryType 0x0001
   #  EfiLoaderCode 0x0002
@@ -916,6 +922,12 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0x0|UINT64|0x30001052
 
   ## Indicates which type allocation need guard page.
+  #
+  # If a bit is set, a head guard page and a tail guard page will be 
+ added just  # before and after corresponding type of pages which the 
+ allocated pool occupies,  # if there's enough free memory for all of 
+ them. The pool allocation for the  # type related to cleared bits keeps the 
same as ususal.
+  #
   # Below is bit mask for this PCD: (Order is same as UEFI spec)
   #  EfiReservedMemoryType 0x0001
   #  EfiLoaderCode 0x0002
--
2.15.1.windows.2

___
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 0/2] Fix incomplete print output

2018-01-02 Thread Gao, Liming
Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jian
>J Wang
>Sent: Tuesday, January 02, 2018 4:20 PM
>To: edk2-devel@lists.01.org
>Subject: [edk2] [PATCH 0/2] Fix incomplete print output
>
>This is caused by previous patch tring to fix string over-read. The root
>cause is that the format string may be ascii or unicode but that patch
>assumes it just ascii string.
>
>Jian J Wang (2):
>  MdePkg/BasePrintLib: Fix incomplete print output
>  MdeModulePkg/DxePrintLibPrint2Protocol: Fix incomplete print output
>
> MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 8 ++--
> MdePkg/Library/BasePrintLib/PrintLibInternal.c| 8 ++--
> 2 files changed, 12 insertions(+), 4 deletions(-)
>
>--
>2.15.1.windows.2
>
>___
>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] MdeModulePkg/DxePrintLibPrint2Protocol: Fix incomplete print output

2018-01-02 Thread Zeng, Star
Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Wang, Jian J 
Sent: Tuesday, January 2, 2018 4:20 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Yao, Jiewen ; 
Zeng, Star 
Subject: [PATCH 2/2] MdeModulePkg/DxePrintLibPrint2Protocol: Fix incomplete 
print output

This is caused by a previous patch which tried to fix string over-read.
It's found that that patch for PrintLib in MdePkg will cause premature 
terminating of loop used to traversing format string and cause incomplete 
string output. Because this library uses similar code to do the same job, it 
has the same issue too. So the fix is also the same.

Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c 
b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
index 0e6178fc9c..e09520c81b 100644
--- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
+++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
@@ -2051,7 +2051,9 @@ InternalPrintLibSPrintMarker (
   // ArgumentString is either null-terminated, or it contains Precision 
characters
   //
   for (Count = 0;
-ArgumentString[Count * BytesPerArgumentCharacter] != '\0' &&
+(ArgumentString[Count * BytesPerArgumentCharacter] != '\0' ||
+ (BytesPerArgumentCharacter > 1 &&
+  ArgumentString[Count * BytesPerArgumentCharacter + 1]!= 
+ '\0')) &&
 (Count < Precision || ((Flags & PRECISION) == 0));
 Count++) {
 ArgumentCharacter = ((ArgumentString[Count * 
BytesPerArgumentCharacter] & 0xff) | ((ArgumentString[Count * 
BytesPerArgumentCharacter + 1]) << 8)) & ArgumentMask; @@ -2110,7 +2112,9 @@ 
InternalPrintLibSPrintMarker (
 //
 // Copy the string into the output buffer performing the required type 
conversions
 //
-while (Index < Count && (*ArgumentString) != '\0') {
+while (Index < Count &&
+   (ArgumentString[0] != '\0' ||
+(BytesPerArgumentCharacter > 1 && ArgumentString[1] != 
+ '\0'))) {
   ArgumentCharacter = ((*ArgumentString & 0xff) | 
(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
 
   LengthToReturn += (1 * BytesPerOutputCharacter);
--
2.15.1.windows.2

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


[edk2] [PATCH] MdeModulePkg/MdeModulePkg.dec: clarify Heap Guard usage

2018-01-02 Thread Jian J Wang
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/MdeModulePkg.dec | 12 
 1 file changed, 12 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 037b16e2d0..491fb27663 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -893,6 +893,12 @@
   
gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack|0x5AA55AA5|UINT32|0x30001051
 
   ## Indicates which type allocation need guard page.
+  #
+  # If a bit is set, a head guard page and a tail guard page will be added just
+  # before and after corresponding type of pages allocated if there's enough
+  # free pages for all of them. The page allocation for the type related to
+  # cleared bits keeps the same as ususal.
+  #
   # Below is bit mask for this PCD: (Order is same as UEFI spec)
   #  EfiReservedMemoryType 0x0001
   #  EfiLoaderCode 0x0002
@@ -916,6 +922,12 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0x0|UINT64|0x30001052
 
   ## Indicates which type allocation need guard page.
+  #
+  # If a bit is set, a head guard page and a tail guard page will be added just
+  # before and after corresponding type of pages which the allocated pool 
occupies,
+  # if there's enough free memory for all of them. The pool allocation for the
+  # type related to cleared bits keeps the same as ususal.
+  #
   # Below is bit mask for this PCD: (Order is same as UEFI spec)
   #  EfiReservedMemoryType 0x0001
   #  EfiLoaderCode 0x0002
-- 
2.15.1.windows.2

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


[edk2] [PATCH 2/2] MdeModulePkg/DxePrintLibPrint2Protocol: Fix incomplete print output

2018-01-02 Thread Jian J Wang
This is caused by a previous patch which tried to fix string over-read.
It's found that that patch for PrintLib in MdePkg will cause premature
terminating of loop used to traversing format string and cause incomplete
string output. Because this library uses similar code to do the same
job, it has the same issue too. So the fix is also the same.

Cc: Liming Gao 
Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c 
b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
index 0e6178fc9c..e09520c81b 100644
--- a/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
+++ b/MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c
@@ -2051,7 +2051,9 @@ InternalPrintLibSPrintMarker (
   // ArgumentString is either null-terminated, or it contains Precision 
characters
   //
   for (Count = 0;
-ArgumentString[Count * BytesPerArgumentCharacter] != '\0' &&
+(ArgumentString[Count * BytesPerArgumentCharacter] != '\0' ||
+ (BytesPerArgumentCharacter > 1 &&
+  ArgumentString[Count * BytesPerArgumentCharacter + 1]!= '\0')) &&
 (Count < Precision || ((Flags & PRECISION) == 0));
 Count++) {
 ArgumentCharacter = ((ArgumentString[Count * 
BytesPerArgumentCharacter] & 0xff) | ((ArgumentString[Count * 
BytesPerArgumentCharacter + 1]) << 8)) & ArgumentMask;
@@ -2110,7 +2112,9 @@ InternalPrintLibSPrintMarker (
 //
 // Copy the string into the output buffer performing the required type 
conversions
 //
-while (Index < Count && (*ArgumentString) != '\0') {
+while (Index < Count &&
+   (ArgumentString[0] != '\0' ||
+(BytesPerArgumentCharacter > 1 && ArgumentString[1] != '\0'))) {
   ArgumentCharacter = ((*ArgumentString & 0xff) | 
(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
 
   LengthToReturn += (1 * BytesPerOutputCharacter);
-- 
2.15.1.windows.2

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


[edk2] [PATCH 0/2] Fix incomplete print output

2018-01-02 Thread Jian J Wang
This is caused by previous patch tring to fix string over-read. The root
cause is that the format string may be ascii or unicode but that patch
assumes it just ascii string.

Jian J Wang (2):
  MdePkg/BasePrintLib: Fix incomplete print output
  MdeModulePkg/DxePrintLibPrint2Protocol: Fix incomplete print output

 MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 8 ++--
 MdePkg/Library/BasePrintLib/PrintLibInternal.c| 8 ++--
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.15.1.windows.2

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


[edk2] [PATCH 1/2] MdePkg/BasePrintLib: Fix incomplete print output

2018-01-02 Thread Jian J Wang
This is caused by previous patch which tried to fix string over-read,
which breaks UEFI menu rendering: the following

/--\
|   Device Manager |
\--/

is rendered as

/\
|   Device Manager |
\/.0 2.00 GHz

(the spurious digits are SMBIOS data from the home screen)

The problem appears to be that the CHAR16 value of BOXDRAW_HORIZONTAL
equals 0x2500, which means that testing ArgumentString[] != '\0'
(which tests the low byte only) will yield FALSE and terminate the
loop prematurely.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Library/BasePrintLib/PrintLibInternal.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c 
b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
index fc57255068..6f19b31449 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
@@ -1108,7 +1108,9 @@ BasePrintLibSPrintMarker (
   // ArgumentString is either null-terminated, or it contains Precision 
characters
   //
   for (Count = 0;
-ArgumentString[Count * BytesPerArgumentCharacter] != '\0' &&
+(ArgumentString[Count * BytesPerArgumentCharacter] != '\0' ||
+ (BytesPerArgumentCharacter > 1 &&
+  ArgumentString[Count * BytesPerArgumentCharacter + 1]!= '\0')) &&
 (Count < Precision || ((Flags & PRECISION) == 0));
   Count++) {
 ArgumentCharacter = ((ArgumentString[Count * 
BytesPerArgumentCharacter] & 0xff) | ((ArgumentString[Count * 
BytesPerArgumentCharacter + 1]) << 8)) & ArgumentMask;
@@ -1167,7 +1169,9 @@ BasePrintLibSPrintMarker (
 //
 // Copy the string into the output buffer performing the required type 
conversions
 //
-while (Index < Count && (*ArgumentString) != '\0') {
+while (Index < Count &&
+   (ArgumentString[0] != '\0' ||
+(BytesPerArgumentCharacter > 1 && ArgumentString[1] != '\0'))) {
   ArgumentCharacter = ((*ArgumentString & 0xff) | 
(((UINT8)*(ArgumentString + 1)) << 8)) & ArgumentMask;
 
   LengthToReturn += (1 * BytesPerOutputCharacter);
-- 
2.15.1.windows.2

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