Re: [edk2] [PATCH] ArmPlatformPkg/MemoryInitPeiLib: mark primary FV region as boot services data
On 3 January 2018 at 05:09, Udit Kumarwrote: > 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
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.
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.
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.
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
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
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
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"
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
Reviewed-by: Yonghong ZhuBest 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
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.
From: Wang FanThe 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.
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
From: Wang FanSee 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.
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
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
Thanks. Reviewed-by: Yonghong ZhuBest 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; i Type == 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
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
> -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
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; i Type == 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
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 Khanwrote: > 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
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
On 2 January 2018 at 15:11, Evan Lloydwrote: > 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
(fix Leif's email address) On 2 January 2018 at 15:21, Ard Biesheuvelwrote: > 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
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
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, Ruiyuwrote: > > > > 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
On 2 January 2018 at 12:18, Gao, Limingwrote: > 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)
Sure. Thanks, -Chema -- Forwarded message -- From: Gao, LimingDate: 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
From: Yunhua FengWe 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
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
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
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
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
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
On 2 January 2018 at 09:25, Ard Biesheuvelwrote: > 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
On 29 December 2017 at 02:58, Alex Jameswrote: > 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
Reviewed-by: Star Zengafter 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
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
Reviewed-by: Star ZengThanks, 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
Cc: Star ZengCc: 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
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 GaoCc: 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
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
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 KinneyCc: 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