Re: [edk2] [PATCH] MdePkg/BaseLib: Add an additional check within AsciiStriCmp

[Wu, Hao A]

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu
> Ni
> Sent: Friday, August 03, 2018 11:41 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [edk2] [PATCH] MdePkg/BaseLib: Add an additional check within
> AsciiStriCmp
> 
> This commit adds an addtional check in AsciiStriCmp. It
> explicitly checks the end of the sting pointed by 'SecondString' to make
> the code logic easier for reading and to prevent possible mis-reports by
> static code checkers.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Hao A Wu 
> ---
>  MdePkg/Library/BaseLib/String.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Library/BaseLib/String.c
> b/MdePkg/Library/BaseLib/String.c
> index e7fe513aec..cb90774c86 100644
> --- a/MdePkg/Library/BaseLib/String.c
> +++ b/MdePkg/Library/BaseLib/String.c
> @@ -1262,7 +1262,7 @@ AsciiStriCmp (
> 
>UpperFirstString  = InternalBaseLibAsciiToUpper (*FirstString);
>UpperSecondString = InternalBaseLibAsciiToUpper (*SecondString);
> -  while ((*FirstString != '\0') && (UpperFirstString == UpperSecondString)) {
> +  while ((*FirstString != '\0') && (*SecondString != '\0') && 
> (UpperFirstString
> == UpperSecondString)) {
>  FirstString++;
>  SecondString++;
>  UpperFirstString  = InternalBaseLibAsciiToUpper (*FirstString);
> --
> 2.16.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

Re: [edk2] [PATCH] ShellPkg/acpi: Code cleanup to pass static code checker

[Wu, Hao A]

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, August 03, 2018 11:42 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [PATCH] ShellPkg/acpi: Code cleanup to pass static code checker
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Hao A Wu 
> ---
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c   | 2 
> +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c | 1 +
>  .../Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c   |
> 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 18c4983e95..5cab20023d 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -297,7 +297,7 @@ DumpUint64 (
>Val = *(UINT32*)(Ptr + sizeof (UINT32));
> 
>Val <<= 32;
> -  Val |= *(UINT32*)Ptr;
> +  Val |= (UINT64)*(UINT32*)Ptr;
> 
>Print (Format, Val);
>  }
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> index 47ce93f104..4199e82ab1 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> @@ -284,6 +284,7 @@ ConvertStrToAcpiSignature (
>UINT8 Index;
>CHAR8 Ptr[4];
> 
> +  ZeroMem (Ptr, sizeof (Ptr));
>Index = 0;
> 
>// Convert to Upper case and convert to ASCII
> diff --git
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.c
> index 245700a253..c6eb7087cf 100644
> ---
> a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.c
> +++
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.c
> @@ -118,7 +118,7 @@ UefiShellAcpiViewCommandLibConstructor (
>gShellAcpiViewHiiHandle = NULL;
> 
>// Check Shell Profile Debug1 bit of the profiles mask
> -  if ((FixedPcdGet8 (PcdShellProfileMask) & BIT1) == 0) {
> +  if ((PcdGet8 (PcdShellProfileMask) & BIT1) == 0) {
>  return EFI_SUCCESS;
>}
> 
> --
> 2.16.1.windows.1

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

Re: [edk2] Tool to check memory leaks

[Udit Kumar]

Thanks Jiewen

> -Original Message-
> From: Yao, Jiewen [mailto:jiewen@intel.com]
> Sent: Thursday, August 2, 2018 6:24 PM
> To: Udit Kumar ; edk2-devel@lists.01.org
> Cc: Sumit Batra 
> Subject: RE: Tool to check memory leaks
> 
> Please refer to
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> ub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FMemory-leak-
> detection-with-memory-profile-
> featuredata=02%7C01%7Cudit.kumar%40nxp.com%7Ca2004032ed144
> 4b8729408d5f8770101%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0
> %7C636688112374410700sdata=PF%2BzraEcqtfzy6K5N6T0RGFHNQTK
> Aap3QGQCwUnzkxg%3Dreserved=0
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Udit Kumar
> > Sent: Thursday, August 2, 2018 5:00 PM
> > To: edk2-devel@lists.01.org
> > Cc: Sumit Batra 
> > Subject: [edk2] Tool to check memory leaks
> >
> > Hi All
> > Is there some tool/debug option in edk2 to detect memory leak.
> > Thanks
> > Udit
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> develdata=02%7C01%7Cudit.ku
> >
> mar%40nxp.com%7Ca2004032ed1444b8729408d5f8770101%7C686ea1d3bc2
> b4c6fa92
> >
> cd99c5c301635%7C0%7C0%7C636688112374410700sdata=JpJI1U9eYC
> AHH0DAg
> > 7IEp1mesytfHVjvVoZa%2Bmmz0NY%3Dreserved=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ShellPkg/acpi: Code cleanup to pass static code checker

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Hao A Wu 
---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c   | 2 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c | 1 +
 .../Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c   | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 18c4983e95..5cab20023d 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -297,7 +297,7 @@ DumpUint64 (
   Val = *(UINT32*)(Ptr + sizeof (UINT32));
 
   Val <<= 32;
-  Val |= *(UINT32*)Ptr;
+  Val |= (UINT64)*(UINT32*)Ptr;
 
   Print (Format, Val);
 }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index 47ce93f104..4199e82ab1 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -284,6 +284,7 @@ ConvertStrToAcpiSignature (
   UINT8 Index;
   CHAR8 Ptr[4];
 
+  ZeroMem (Ptr, sizeof (Ptr));
   Index = 0;
 
   // Convert to Upper case and convert to ASCII
diff --git 
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
index 245700a253..c6eb7087cf 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c
@@ -118,7 +118,7 @@ UefiShellAcpiViewCommandLibConstructor (
   gShellAcpiViewHiiHandle = NULL;
 
   // Check Shell Profile Debug1 bit of the profiles mask
-  if ((FixedPcdGet8 (PcdShellProfileMask) & BIT1) == 0) {
+  if ((PcdGet8 (PcdShellProfileMask) & BIT1) == 0) {
 return EFI_SUCCESS;
   }
 
-- 
2.16.1.windows.1

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

[edk2] [PATCH] MdePkg/BaseLib: Add an additional check within AsciiStriCmp

[Ruiyu Ni]

This commit adds an addtional check in AsciiStriCmp. It
explicitly checks the end of the sting pointed by 'SecondString' to make
the code logic easier for reading and to prevent possible mis-reports by
static code checkers.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Hao A Wu 
---
 MdePkg/Library/BaseLib/String.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseLib/String.c b/MdePkg/Library/BaseLib/String.c
index e7fe513aec..cb90774c86 100644
--- a/MdePkg/Library/BaseLib/String.c
+++ b/MdePkg/Library/BaseLib/String.c
@@ -1262,7 +1262,7 @@ AsciiStriCmp (
 
   UpperFirstString  = InternalBaseLibAsciiToUpper (*FirstString);
   UpperSecondString = InternalBaseLibAsciiToUpper (*SecondString);
-  while ((*FirstString != '\0') && (UpperFirstString == UpperSecondString)) {
+  while ((*FirstString != '\0') && (*SecondString != '\0') && 
(UpperFirstString == UpperSecondString)) {
 FirstString++;
 SecondString++;
 UpperFirstString  = InternalBaseLibAsciiToUpper (*FirstString);
-- 
2.16.1.windows.1

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

Re: [edk2] [Patch] BaseTools/Pkcs7Sign: Add PKCS7 test key include files

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Kinney, Michael D 
Sent: Friday, August 3, 2018 9:39 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong ; Gao, Liming ; 
Kinney, Michael D 
Subject: [Patch] BaseTools/Pkcs7Sign: Add PKCS7 test key include files

https://bugzilla.tianocore.org/show_bug.cgi?id=1073

Add PCD statement include files for the PKCS7 test key.
* gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer
* gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr

These include files can be used in !include statements in PCD sections of a 
platform DSC file to assign these PCDs to the test key certificate values.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 BaseTools/Source/Python/Pkcs7Sign/Readme.md| 40 ++
 ...ecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc |  1 +  
...kenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc |  1 +
 3 files changed, 42 insertions(+)
 create mode 100644 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
 create mode 100644 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc

diff --git a/BaseTools/Source/Python/Pkcs7Sign/Readme.md 
b/BaseTools/Source/Python/Pkcs7Sign/Readme.md
index fee0327876..5315b7fca4 100644
--- a/BaseTools/Source/Python/Pkcs7Sign/Readme.md
+++ b/BaseTools/Source/Python/Pkcs7Sign/Readme.md
@@ -116,3 +116,43 @@ Convert Key and Certificate for signing. Password is 
removed with -nodes flag fo
 
 openssl smime -verify -inform DER -in test.bin.p7 -content test.bin 
-CAfile TestRoot.pub.pem -out test.org.bin
 
+## Generate DSC PCD include files for Certificate
+
+The `BinToPcd` utility can be used to convert the binary Certificate 
+file to a text file can be included from a DSC file to set a PCD to the 
+contents of the Certificate file.
+
+The following 2 PCDs can be set to the PKCS7 Certificate value.  The 
+first one supports a single certificate.  The second one supports 
+multiple certificate values using the XDR format.
+* `gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer`
+* `gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr`
+
+Generate DSC PCD include files:
+```
+BinToPcd.py -i TestRoot.cer -p 
+gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer -o 
+TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
+BinToPcd.py -i TestRoot.cer -p 
+gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr -x -o 
+TestRoot.cer.gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr
+.inc
+```
+
+These files can be used in `!include` statements in DSC file PCD sections.  
For example:
+
+* Platform scoped fixed at build PCD section ``` [PcdsFixedAtBuild]
+  !include 
+BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpac
+eGuid.PcdPkcs7CertBuffer.inc
+```
+
+* Platform scoped patchable in module PCD section ``` 
+[PcdsPatchableInModule]
+  !include 
+BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gFmpDevicePkgTokenSpaceG
+uid.PcdFmpDevicePkcs7CertBufferXdr.inc
+```
+
+* Module scoped fixed at build PCD section ``` [Components]
+  FmpDevicePkg/FmpDxe/FmpDxe.inf {
+
+  !include 
+BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gFmpDevicePkgTokenSpaceG
+uid.PcdFmpDevicePkcs7CertBufferXdr.inc
+  }
+```
diff --git 
a/BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
 
b/BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
new file mode 100644
index 00..907c70dd92
--- /dev/null
+++ b/BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgToke
+++ nSpaceGuid.PcdPkcs7CertBuffer.inc
@@ -0,0 +1 @@
+  gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer|{0x30, 0x82, 0x03, 
+ 0xEC, 0x30, 0x82, 0x02, 0xD4, 0xA0, 0x03, 0x02, 0x01, 0x02, 0x02, 
+ 0x09, 0x00, 0xC0, 0x91, 0xC5, 0xE2, 0xB7, 0x66, 0xC0, 0xF8, 0x30, 
+ 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 
+ 0x0B, 0x05, 0x00, 0x30, 0x81, 0x82, 0x31, 0x0B, 0x30, 0x09, 0x06, 
+ 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x43, 0x4E, 0x31, 0x0B, 0x30, 
+ 0x09, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0C, 0x02, 0x53, 0x48, 0x31, 
+ 0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x07, 0x0C, 0x02, 0x53, 
+ 0x48, 0x31, 0x12, 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0A, 0x0C, 
+ 0x09, 0x54, 0x69, 0x61, 0x6E, 0x6F, 0x43, 0x6F, 0x72, 0x65, 0x31, 
+ 0x0E, 0x30, 0x0C, 0x06, 0x03, 0x55, 0x04, 0x0B, 0x0C, 0x05, 0x45, 
+ 0x44, 0x4B, 0x49, 0x49, 0x31, 0x11, 0x30, 0x0F, 0x06, 0x03, 0x55, 
+ 0x04, 0x03, 0x0C, 0x08, 0x54, 0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F, 
+ 0x74, 0x31, 0x22, 0x30, 0x20, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 
+ 0xF7, 0x0D, 0x01, 0x09, 0x01, 0x16, 0x13, 0x65, 0x64, 0x6B, 0x69, 
+ 0x69, 0x40, 0x74, 0x69, 0x61, 0x6E, 0x6F, 0x63, 0x6F, 0x72, 0x65, 
+ 0x2E, 0x6F, 0x72, 0x67, 0x30, 0x1E, 0x17, 0x0D, 0x31, 0x37, 

[edk2] [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Implement AllocateAcpiCpuData function.

[Eric Dong]

Current code logic can't confirm CpuS3DataDxe driver start before
CpuFeaturesDxe driver. So the assumption in CpuFeaturesDxe not
valid. Add implementation for AllocateAcpiCpuData function to remove
this assumption.

Pass OS boot and resume from S3 test.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 .../DxeRegisterCpuFeaturesLib.c| 57 --
 1 file changed, 54 insertions(+), 3 deletions(-)

diff --git 
a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c 
b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
index 902a339529..0722db6c64 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.c
@@ -207,11 +207,62 @@ AllocateAcpiCpuData (
   VOID
   )
 {
+  EFI_STATUS   Status;
+  UINTNNumberOfCpus;
+  UINTNNumberOfEnabledProcessors;
+  ACPI_CPU_DATA*AcpiCpuData;
+  EFI_PHYSICAL_ADDRESS Address;
+  UINTNTableSize;
+  CPU_REGISTER_TABLE   *RegisterTable;
+  UINTNIndex;
+  EFI_PROCESSOR_INFORMATIONProcessorInfoBuffer;
+
+  Address = BASE_4GB - 1;
+  Status  = gBS->AllocatePages (
+   AllocateMaxAddress,
+   EfiACPIMemoryNVS,
+   EFI_SIZE_TO_PAGES (sizeof (ACPI_CPU_DATA)),
+   
+   );
+  ASSERT_EFI_ERROR (Status);
+  AcpiCpuData = (ACPI_CPU_DATA *) (UINTN) Address;
+  ASSERT (AcpiCpuData != NULL);
+
+  GetNumberOfProcessor (, );
+  AcpiCpuData->NumberOfCpus = (UINT32) NumberOfCpus;
+
   //
-  // CpuS3DataDxe will do it.
+  // Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable for 
all CPUs
   //
-  ASSERT (FALSE);
-  return NULL;
+  TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
+  Address = BASE_4GB - 1;
+  Status  = gBS->AllocatePages (
+   AllocateMaxAddress,
+   EfiACPIMemoryNVS,
+   EFI_SIZE_TO_PAGES (TableSize),
+   
+   );
+  ASSERT_EFI_ERROR (Status);
+  RegisterTable = (CPU_REGISTER_TABLE *) (UINTN) Address;
+
+  for (Index = 0; Index < NumberOfCpus; Index++) {
+Status = GetProcessorInformation (Index, );
+ASSERT_EFI_ERROR (Status);
+
+RegisterTable[Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
+RegisterTable[Index].TableLength= 0;
+RegisterTable[Index].AllocatedSize  = 0;
+RegisterTable[Index].RegisterTableEntry = 0;
+
+RegisterTable[NumberOfCpus + Index].InitialApicId  = 
(UINT32)ProcessorInfoBuffer.ProcessorId;
+RegisterTable[NumberOfCpus + Index].TableLength= 0;
+RegisterTable[NumberOfCpus + Index].AllocatedSize  = 0;
+RegisterTable[NumberOfCpus + Index].RegisterTableEntry = 0;
+  }
+  AcpiCpuData->RegisterTable   = 
(EFI_PHYSICAL_ADDRESS)(UINTN)RegisterTable;
+  AcpiCpuData->PreSmmInitRegisterTable = 
(EFI_PHYSICAL_ADDRESS)(UINTN)(RegisterTable + NumberOfCpus);
+
+  return AcpiCpuData;
 }
 
 /**
-- 
2.15.0.windows.1

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

[edk2] [Patch] BaseTools/Pkcs7Sign: Add PKCS7 test key include files

[Kinney, Michael D]

https://bugzilla.tianocore.org/show_bug.cgi?id=1073

Add PCD statement include files for the PKCS7 test key.
* gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer
* gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr

These include files can be used in !include statements in PCD
sections of a platform DSC file to assign these PCDs to the
test key certificate values.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 BaseTools/Source/Python/Pkcs7Sign/Readme.md| 40 ++
 ...ecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc |  1 +
 ...kenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc |  1 +
 3 files changed, 42 insertions(+)
 create mode 100644 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
 create mode 100644 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc

diff --git a/BaseTools/Source/Python/Pkcs7Sign/Readme.md 
b/BaseTools/Source/Python/Pkcs7Sign/Readme.md
index fee0327876..5315b7fca4 100644
--- a/BaseTools/Source/Python/Pkcs7Sign/Readme.md
+++ b/BaseTools/Source/Python/Pkcs7Sign/Readme.md
@@ -116,3 +116,43 @@ Convert Key and Certificate for signing. Password is 
removed with -nodes flag fo
 
 openssl smime -verify -inform DER -in test.bin.p7 -content test.bin 
-CAfile TestRoot.pub.pem -out test.org.bin
 
+## Generate DSC PCD include files for Certificate
+
+The `BinToPcd` utility can be used to convert the binary Certificate file to a
+text file can be included from a DSC file to set a PCD to the contents of the
+Certificate file.
+
+The following 2 PCDs can be set to the PKCS7 Certificate value.  The first one
+supports a single certificate.  The second one supports multiple certificate
+values using the XDR format.
+* `gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer`
+* `gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr`
+
+Generate DSC PCD include files:
+```
+BinToPcd.py -i TestRoot.cer -p 
gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer -o 
TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
+BinToPcd.py -i TestRoot.cer -p 
gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr -x -o 
TestRoot.cer.gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc
+```
+
+These files can be used in `!include` statements in DSC file PCD sections.  
For example:
+
+* Platform scoped fixed at build PCD section
+```
+[PcdsFixedAtBuild]
+  !include 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
+```
+
+* Platform scoped patchable in module PCD section
+```
+[PcdsPatchableInModule]
+  !include 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc
+```
+
+* Module scoped fixed at build PCD section
+```
+[Components]
+  FmpDevicePkg/FmpDxe/FmpDxe.inf {
+
+  !include 
BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gFmpDevicePkgTokenSpaceGuid.PcdFmpDevicePkcs7CertBufferXdr.inc
+  }
+```
diff --git 
a/BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
 
b/BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
new file mode 100644
index 00..907c70dd92
--- /dev/null
+++ 
b/BaseTools/Source/Python/Pkcs7Sign/TestRoot.cer.gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer.inc
@@ -0,0 +1 @@
+  gEfiSecurityPkgTokenSpaceGuid.PcdPkcs7CertBuffer|{0x30, 0x82, 0x03, 0xEC, 
0x30, 0x82, 0x02, 0xD4, 0xA0, 0x03, 0x02, 0x01, 0x02, 0x02, 0x09, 0x00, 0xC0, 
0x91, 0xC5, 0xE2, 0xB7, 0x66, 0xC0, 0xF8, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 
0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x0B, 0x05, 0x00, 0x30, 0x81, 0x82, 0x31, 
0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x43, 0x4E, 0x31, 
0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0C, 0x02, 0x53, 0x48, 0x31, 
0x0B, 0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x07, 0x0C, 0x02, 0x53, 0x48, 0x31, 
0x12, 0x30, 0x10, 0x06, 0x03, 0x55, 0x04, 0x0A, 0x0C, 0x09, 0x54, 0x69, 0x61, 
0x6E, 0x6F, 0x43, 0x6F, 0x72, 0x65, 0x31, 0x0E, 0x30, 0x0C, 0x06, 0x03, 0x55, 
0x04, 0x0B, 0x0C, 0x05, 0x45, 0x44, 0x4B, 0x49, 0x49, 0x31, 0x11, 0x30, 0x0F, 
0x06, 0x03, 0x55, 0x04, 0x03, 0x0C, 0x08, 0x54, 0x65, 0x73, 0x74, 0x52, 0x6F, 
0x6F, 0x74, 0x31, 0x22, 0x30, 0x20, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 
0x0D, 0x01, 0x09, 0x01, 0x16, 0x13, 0x65, 0x64, 0x6B, 0x69, 0x6
 9, 0x40, 0x74, 0x69, 0x61, 0x6E, 0x6F, 0x63, 0x6F, 0x72, 0x65, 0x2E, 0x6F, 
0x72, 0x67, 0x30, 0x1E, 0x17, 0x0D, 0x31, 0x37, 0x30, 0x34, 0x31, 0x30, 0x30, 
0x38, 0x32, 0x37, 0x34, 0x30, 0x5A, 0x17, 0x0D, 0x31, 0x37, 0x30, 0x35, 0x31, 
0x30, 0x30, 0x38, 0x32, 0x37, 0x34, 0x30, 0x5A, 0x30, 0x81, 0x82, 0x31, 0x0B, 
0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x06, 0x13, 0x02, 0x43, 0x4E, 0x31, 0x0B, 
0x30, 0x09, 0x06, 0x03, 0x55, 0x04, 0x08, 0x0C, 0x02, 0x53, 0x48, 0x31, 0x0B, 
0x30, 0x09, 

[edk2] [PATCH] OvmfPkg/PlatformDebugLibIoPort: fix port detection for use in the DXE Core

[Laszlo Ersek]

The DXE Core is one of those modules that call
ProcessLibraryConstructorList() manually.

Before DxeMain() [MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c] calls
ProcessLibraryConstructorList(), and through it, our
PlatformDebugLibIoPortConstructor() function, DxeMain() invokes the
DEBUG() macro multiple times. That macro lands in our
PlatformDebugLibIoPortFound() function -- which currently relies on the
"mDebugIoPortFound" global variable that has (not yet) been set by the
constructor. As a result, early debug messages from the DXE Core are lost.

Move the device detection into PlatformDebugLibIoPortFound(), also caching
the fact (not just the result) of the device detection.

(We could introduce a separate DebugLib instance just for the DXE Core,
but the above approach works for all modules that currently consume the
PlatformDebugLibIoPort instance (which means "everything but SEC").)

This restores messages such as:

> CoreInitializeMemoryServices:
>   BaseAddress - 0x7AF21000 Length - 0x3CDE000 MinimalMemorySizeNeeded - 
> 0x10F4000

Keep the empty constructor function -- OVMF's DebugLib instances have
always had constructors; we had better not upset constructor dependency
ordering by making our instance(s) constructor-less.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jordan Justen 
Fixes: c09d9571300a089c35f5df2773b70edc25050d0d
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
Brijesh, can you please test this patch on SEV, with and without
capturing the debug port? (In the first case, the debug log should just
work; in the second case, the boot should remain fast.) Thanks!

Repo:   https://github.com/lersek/edk2.git
Branch: debuglib_dxecore

 OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c | 24 

 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c 
b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
index 81c44eece95f..74aef2e37b42 100644
--- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
+++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLibDetect.c
@@ -16,14 +16,26 @@
 #include 
 #include "DebugLibDetect.h"
 
+
+//
+// Set to TRUE if the debug I/O port has been checked
+//
+STATIC BOOLEAN mDebugIoPortChecked = FALSE;
 //
 // Set to TRUE if the debug I/O port is enabled
 //
 STATIC BOOLEAN mDebugIoPortFound = FALSE;
 
 /**
-  This constructor function checks if the debug I/O port device is present,
-  caching the result for later use.
+  This constructor function must not do anything.
+
+  Some modules consuming this library instance, such as the DXE Core, invoke
+  the DEBUG() macro before they explicitly call
+  ProcessLibraryConstructorList(). Therefore the auto-generated call from
+  ProcessLibraryConstructorList() to this constructor function may be preceded
+  by some calls to PlatformDebugLibIoPortFound() below. Hence
+  PlatformDebugLibIoPortFound() must not rely on anything this constructor
+  could set up.
 
   @retval RETURN_SUCCESS   The constructor always returns RETURN_SUCCESS.
 
@@ -34,12 +46,12 @@ PlatformDebugLibIoPortConstructor (
   VOID
   )
 {
-  mDebugIoPortFound = PlatformDebugLibIoPortDetect();
   return RETURN_SUCCESS;
 }
 
 /**
-  Return the cached result of detecting the debug I/O port device.
+  At the first call, check if the debug I/O port device is present, and cache
+  the result for later use. At subsequent calls, return the cached result.
 
   @retval TRUE   if the debug I/O port device was detected.
   @retval FALSE  otherwise
@@ -51,5 +63,9 @@ PlatformDebugLibIoPortFound (
   VOID
   )
 {
+  if (!mDebugIoPortChecked) {
+mDebugIoPortFound = PlatformDebugLibIoPortDetect ();
+mDebugIoPortChecked = TRUE;
+  }
   return mDebugIoPortFound;
 }
-- 
2.14.1.3.gb7cf6e02401b

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

Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with OVMF

[Zhang, Chao B]

Tks Lazslo.  And please make sure PcdLib is correctly lined in OVMF

From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Thursday, August 2, 2018 9:14 PM
To: Zhang, Chao B ; Ricardo Araújo 
; Marc-André Lureau 
Cc: edk2-devel@lists.01.org; Gao, Liming ; Zeng, Star 

Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with 
OVMF

On 08/02/18 04:04, Zhang, Chao B wrote:
> Hi Laszlo & Ricardo
> The patch was intended to reduce the time to read TPM interface ID register. 
> The interface type should not change within boot cycle according to PTP spec.
> I agree to add some ASSERT after PCDSetxxsS.
> But It is a core solution without platform change as PCD has been configured 
> as DYN, DYNEx in DEC.  I don’t know why you meet Set Failure
> In OVMF. Here, I include PCD expert to explain this.

As far as I recall, dynamic PCDs have never behaved as actually settable
for me unless I added dynamic defaults for them in the OVMF DSC files.

I never really researched why this was the case, I just accepted that
the dynamic defaults were apparently necessary.

Let's wait for Ricardo's response. Perhaps my analysis / suspicion were
incorrect and it's not actually the "dynamism" of the PCD that's missing
for OVMF. Ricardo's answer will tell us if there's another issue.

Thanks
Laszlo

> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, August 2, 2018 5:49 AM
> To: Ricardo Araújo mailto:rica...@lsd.ufcg.edu.br>>; 
> Zhang, Chao B mailto:chao.b.zh...@intel.com>>; 
> Marc-André Lureau 
> mailto:marcandre.lur...@redhat.com>>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
> with OVMF
>
> On 08/01/18 19:50, Ricardo Araújo wrote:
>> The commit I was referring to is:
>> https://github.com/tianocore/edk2/commit/f15cb995bb3880b77e15afe6facd3da05e599a17
>>
>> Regards,
>>
>> Ricardo Araujo -
>> www.lsd.ufcg.edu.br/~ricardo>
>>
>> - Mensagem original -
>> De: "Ricardo Araújo" 
>> mailto:rica...@lsd.ufcg.edu.br>>
>> Para: 
>> edk2-devel@lists.01.org>
>> Enviadas: Quarta-feira, 1 de agosto de 2018 14:33:45
>> Assunto: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7
>> with OVMF
>>
>> Hi everyone,
>>
>> I'm using OVMF with a simulated TPM 2.0 (from
>> https://github.com/stefanberger/swtpm) and I noticed lately that PCRs
>> 0-7 are zeroed after booting the vm (ubuntu 18.04) and the only
>> message related to this in dmesg is:
>>
>> [ 2.286690] tpm_tis 00:06: 2.0 TPM (device-id 0x1, rev-id 1)
>> [ 2.303753] tpm tpm0: A TPM error (256) occurred continue selftest
>> [ 2.314199] tpm tpm0: starting up the TPM manually
>>
>> I found this started to happen after this commit , previous commits to
>> that are showing boot time measurements on PCR 0-7 normally and the
>> error message is gone. Has anyone experienced the same behavior? I
>> followed the instructions here for building OVMF but I added the
>> parameters -D TPM2_ENABLE=TRUE -D SECURE_BOOT_ENABLE=TRUE -D
>> HTTP_BOOT_ENABLE=TRUE. Is there anything else I need to add to enable
>> these measurements?
>>
>> Regards,
>>
>> Ricardo Araujo
>> www.lsd.ufcg.edu.br/~ricardo>
>
> Thank you for the bug report. It looks like a regression to me, but the
> details aren't immediately clear.
>
> Adding Marc-André who contributed the TPM enablement for OVMF, and Chao
> Zhang who authored the commit in question.
>
> If I recall correctly, in OVMF we decided to never cache the TPM type
> but always detect it. I could be remembering wrong though. See commit
> 6cf1880fb5b6 ("OvmfPkg: add customized Tcg2ConfigPei clone",
> 2018-03-09).
>
> Chao Zhang: can you please explain what additional requirements are
> presented for a platform by commit f15cb995bb38? In OVMF we use a
> customized Tcg2ConfigPei module (see the commit above).
>
>
> Oh wait, I suspect what's wrong. I believe there are two bugs in commit
> f15cb995bb38 ("SecurityPkg: Cache TPM interface type info", 2018-06-25):
>
> * Bug#1:
>
> Commit f15cb995bb38  introduces a new PCD, called
> "PcdActiveTpmInterfaceType", in section [PcdsDynamic, PcdsDynamicEx] of
> "SecurityPkg.dec", and makes core modules from SecurityPkg dependent on
> it.
>
> Obviously this means that platforms are required to provide a Dynamic
> Default for the new PCD in their DSC files, if they include those core
> modules from SecurityPkg, otherwise the PCD won't actually behave
> dynamically -- "set" operations will fail, and "get" operations will
> just return the central default from the SecurityPkg.dec file. As a
> result, the cached TPM type will always 

Re: [edk2] [Patch] BaseTools/BinToPcd: Encode string returned from ByteArray()

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: Kinney, Michael D 
Sent: Friday, August 3, 2018 5:26 AM
To: edk2-devel@lists.01.org
Cc: Sun, Yanyan ; Zhu, Yonghong ; 
Gao, Liming ; Kinney, Michael D 

Subject: [Patch] BaseTools/BinToPcd: Encode string returned from ByteArray()

https://bugzilla.tianocore.org/show_bug.cgi?id=1069

The ByteArray() method returns a string with the hex bytes of a PCD value.  
Make sure the string is always encoded as a string, so it can be used to build 
a complete PCD statement string and be written out to a file.  This change is 
required for Python 3.x compatibility.

Cc: YanYan Sun 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 BaseTools/Scripts/BinToPcd.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/BinToPcd.py b/BaseTools/Scripts/BinToPcd.py 
index 25b74f6004..1495a36933 100644
--- a/BaseTools/Scripts/BinToPcd.py
+++ b/BaseTools/Scripts/BinToPcd.py
@@ -70,7 +70,8 @@ if __name__ == '__main__':
 #
 # Return a PCD value of the form '{0x01, 0x02, ...}' along with the 
PCD length in bytes
 #
-return '{' + (', '.join (['0x{Byte:02X}'.format (Byte = Item) for Item 
in Buffer])) + '}', len (Buffer)
+PcdValue = '{' + ', '.join (['0x{Byte:02X}'.format (Byte = Item) for 
Item in Buffer]) + '}'
+return PcdValue.encode (), len (Buffer)
 
 #
 # Create command line argument parser object
--
2.14.2.windows.3

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

[edk2] [PATCH v1 1/1] PatchCheck - add error message for invalid parameter

[Jaben Carsey]

Currently if an invalid parameter is passed, it gives a stack trace.
This changes it to an error message.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Scripts/PatchCheck.py | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/BaseTools/Scripts/PatchCheck.py b/BaseTools/Scripts/PatchCheck.py
index 7b7fba8b7044..96b3cdf1fd8a 100755
--- a/BaseTools/Scripts/PatchCheck.py
+++ b/BaseTools/Scripts/PatchCheck.py
@@ -1,7 +1,7 @@
 ## @file
 #  Check a patch for various format issues
 #
-#  Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2015 - 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
@@ -528,6 +528,8 @@ class CheckGitCommits:
 print('Checking git commit:', commit)
 patch = self.read_patch_from_git(commit)
 self.ok &= CheckOnePatch(commit, patch).ok
+if not commits:
+print("Couldn't find commit matching: '{}'".format(rev_spec))
 
 def read_commit_list_from_git(self, rev_spec, max_count):
 # Run git to get the commit patch
@@ -536,7 +538,7 @@ class CheckGitCommits:
 cmd.append('--max-count=' + str(max_count))
 cmd.append(rev_spec)
 out = self.run_git(*cmd)
-return out.split()
+return out.split() if out else []
 
 def read_patch_from_git(self, commit):
 # Run git to get the commit patch
@@ -548,7 +550,8 @@ class CheckGitCommits:
 p = subprocess.Popen(cmd,
  stdout=subprocess.PIPE,
  stderr=subprocess.STDOUT)
-return p.communicate()[0].decode('utf-8', 'ignore')
+Result = p.communicate()
+return Result[0].decode('utf-8', 'ignore') if Result[0] and 
Result[0].find("fatal")!=0 else None
 
 class CheckOnePatchFile:
 """Performs a patch check for a single file.
-- 
2.16.2.windows.1

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

[edk2] [Patch] BaseTools/BinToPcd: Encode string returned from ByteArray()

[Kinney, Michael D]

https://bugzilla.tianocore.org/show_bug.cgi?id=1069

The ByteArray() method returns a string with the hex bytes of
a PCD value.  Make sure the string is always encoded as a string,
so it can be used to build a complete PCD statement string and be
written out to a file.  This change is required for Python 3.x
compatibility.

Cc: YanYan Sun 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney 
---
 BaseTools/Scripts/BinToPcd.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Scripts/BinToPcd.py b/BaseTools/Scripts/BinToPcd.py
index 25b74f6004..1495a36933 100644
--- a/BaseTools/Scripts/BinToPcd.py
+++ b/BaseTools/Scripts/BinToPcd.py
@@ -70,7 +70,8 @@ if __name__ == '__main__':
 #
 # Return a PCD value of the form '{0x01, 0x02, ...}' along with the 
PCD length in bytes
 #
-return '{' + (', '.join (['0x{Byte:02X}'.format (Byte = Item) for Item 
in Buffer])) + '}', len (Buffer)
+PcdValue = '{' + ', '.join (['0x{Byte:02X}'.format (Byte = Item) for 
Item in Buffer]) + '}'
+return PcdValue.encode (), len (Buffer)
 
 #
 # Create command line argument parser object
-- 
2.14.2.windows.3

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

[edk2] [PATCH v2 1/1] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Marcin Wojtas]

According to the SBSA specification the Watchdog Compare
Register is split into two separate 32bit registers.
EDK2 code uses a single 64bit transaction to update
them, which can be problematic, depending on the SoC
implementation and could result in an unpredicted behavior.

Fix this by modifying WatchdogWriteCompareRegister routine to
use two consecutive 32bit writes to the Watchdog Compare Register
Low and High, using new dedicated macros.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas 
Reviewed-by: Leif Lindholm 
---
Changelog v1 -> v2:
- use separate macros for WCV register low and high
- improve commit message
- add Leif's RB

 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h| 3 ++-
 ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
index 9e2aebc..4f42c56 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdog.h
@@ -20,7 +20,8 @@
 // Control Frame:
 #define GENERIC_WDOG_CONTROL_STATUS_REG   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x000)
 #define GENERIC_WDOG_OFFSET_REG   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x008)
-#define GENERIC_WDOG_COMPARE_VALUE_REG((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x010)
+#define GENERIC_WDOG_COMPARE_VALUE_REG_LOW((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x010)
+#define GENERIC_WDOG_COMPARE_VALUE_REG_HIGH   ((UINTN)FixedPcdGet64 
(PcdGenericWatchdogControlBase) + 0x014)
 
 // Values of bit 0 of the Control/Status Register
 #define GENERIC_WDOG_ENABLED  1
diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
index 3180f01..8ccf153 100644
--- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
+++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
@@ -56,7 +56,8 @@ WatchdogWriteCompareRegister (
   UINT64  Value
   )
 {
-  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
+  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_LOW, Value & MAX_UINT32);
+  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG_HIGH, (Value >> 32) & 
MAX_UINT32);
 }
 
 VOID
-- 
2.7.4

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

Re: [edk2] Question about memory map entries

[Laszlo Ersek]

On 08/02/18 21:18, Rafael Machado wrote:
> Just found something interesting.
> Based on the logs from the serial port.
> 
> This system works fine:
> 
> "PeiInstallPeiMemory MemoryBegin 0x93D5, MemoryLength 0xA2B
> Temp Stack : BaseAddress=0x40 Length=0x8
> Temp Heap  : BaseAddress=0x48 Length=0x8
> Total temporary memory:1048576 bytes.
>   temporary memory stack ever used: 524288 bytes.
>   temporary memory heap used:   63304 bytes.
> Old Stack size 524288, New stack size 524288
> Stack Hob: BaseAddress=0x93D5 Length=0x8
> Heap Offset = 0x9395 Stack Offset = 0x9395
> Loading PEIM at 0x0009DFF4000 EntryPoint=0x0009DFF4260 PeiCore.efi"
> ...
> "CoreInitializeMemoryServices:
>   BaseAddress - 0x93DE1000 Length - 0x8135000 MinimalMemorySizeNeeded -
> 0x5AC"
> 
> This one is bricked:
> 
> "PeiInstallPeiMemory MemoryBegin 0x9C9000, MemoryLength 0x9D637000
> Temp Stack : BaseAddress=0x40 Length=0x8
> Temp Heap  : BaseAddress=0x48 Length=0x8
> Total temporary memory:1048576 bytes.
>   temporary memory stack ever used: 524288 bytes.
>   temporary memory heap used:   63304 bytes.
> Old Stack size 524288, New stack size 524288
> Stack Hob: BaseAddress=0x9C9000 Length=0x8
> Heap Offset = 0x5C9000 Stack Offset = 0x5C9000
> Loading PEIM at 0x0009DFF4000 EntryPoint=0x0009DFF4260 PeiCore.efi"
> ...
> "CoreInitializeMemoryServices:
>   BaseAddress - 0x0 Length - 0x0 MinimalMemorySizeNeeded - 0x98E47000
> "
> ...
> "ASSERT_EFI_ERROR (Status = Out of Resources)
> ASSERT [DxeCore] ...\MdeModulePkg\Core\Dxe\DxeMain\DxeMain.c(299):
> !EFI_ERROR (Status)
> AllocatePoolPages: failed to allocate 1 pages
> AllocatePool: failed to allocate 120 bytes"

The location and the size of the permanent PEI RAM are extremely
different between the two cases.

- Functional system:

  PeiInstallPeiMemory MemoryBegin 0x93D5, MemoryLength 0xA2B

  Base address is ~2365 MB, size is ~162 MB

- Unbootable system:

  PeiInstallPeiMemory MemoryBegin 0x9C9000, MemoryLength 0x9D637000

  Base address is ~9 MB, size is ~2518 MB

The numbers in the second (unbootable) case look very unusual to me. The
permanent PEI RAM is usually tens or (maybe) hundreds of megabytes in
size, and tends to start much higher (usually as high as possible under
4GB, on x86 anyway).


Consult the following sections in the PI spec (v1.6), volume 3:

- 4.3 Example HOB Producer Phase Memory Map and Usage
- 5.3 PHIT HOB

The CoreInitializeMemoryServices() function searches the HOB list for a
tested system memory "Resource Descriptor HOB that contains PHIT range
EfiFreeMemoryBottom..EfiFreeMemoryTop". (Quoted a comment from the code.)

Basically, the function locates the system RAM HOB that contains the
free permanent PEI RAM.

If the search fails, then we trip an ASSERT(). This does not happen in
your case, the search succeeds.

If the search succeeds, then the DXE core will try to initialize itself
in one of three locations in the RAM area defined by that HOB. In
descending preference order: above the permanent PEI RAM, within the
free permanent PEI RAM, and below the permanent PEI RAM.

There is also a fallback (a fourth option) when even the third one from
before proves too small -- the function will then search again for a
memory descriptor HOB, top-down, avoiding the one HOB that it found
earlier to contain the permanent PEI RAM (because, all three options for
that have already failed, see above).

As the result of this search, your broken system finishes with:

BaseAddress - 0x0 Length - 0x0 MinimalMemorySizeNeeded - 0x98E47000

"MinimalMemorySizeNeeded" includes the previous measurements from
MemoryTypeInformation, and the concrete value is very strange -- it
seems to imply that you need ~2446 MB for initializing the DXE core.
It's not surprising that the function cannot fit that anywhere, in
either of the four options described above.

If your system has more (high) RAM to spare, try to install a resource
descriptor HOB for it. Then the fourth option might succeed.

Honestly though, those permanent PEI RAM values (base address at ~9 MB,
size ~2518 MB) look super fishy to me in the first place. Something must
be wrong in the PEI phase with the calculation of those parameters.
Possibly, the memory resource descriptor HOBs could be wrong too.


... Considering commit 3a05b13106d1 ("MdeModulePkg DxeCore: Take the
range in resource HOB for PHIT as higher priority", 2015-09-18), it writes,

"Also let the minimal memory size needed include the total memory bin
size needed to make sure memory bin could be allocated successfully."

This is the reason "MinimalMemorySizeNeeded" includes
MemoryTypeInformation. Normally, MemoryTypeInformation tracks long-term
maximums that are necessary for booting an OS without memory map
fragmentation (including S4 resume). However, if you have a UEFI
application that allocates huge amounts of memory, and then *doesn't*
boot an OS, then it could invalidate 

Re: [edk2] [PATCH] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Leif Lindholm]

On Thu, Aug 02, 2018 at 08:32:18PM +0200, Ard Biesheuvel wrote:
> On 2 August 2018 at 17:12, Marcin Wojtas  wrote:
> > czw., 2 sie 2018 o 16:59 Ard Biesheuvel  
> > napisał(a):
> >>
> >> On 2 August 2018 at 16:49, Marcin Wojtas  wrote:
> >> > EDK2 code uses a single 64bit write to update SBSA watchdog
> >> > compare registers, however an access to mmio registers should
> >> > be 32bit for some SoCs. Current usage of MmioWrite64 resulted
> >> > in an unpredicted behavior. Fix this by modifying
> >> > WatchdogWriteCompareRegister routine to use two consecutive
> >> > 32bit writes to Watchdog Compare Register Low and High.
> >> >
> >>
> >> You describe it as if this is generally the case, but this is just a
> >> silicon bug, right?
> >
> > Not sure if it's a bug, or simply SoC characterisctics to place SoC
> > registers to allow only mmio32 access to 32-bit registers. In any way,
> > even updated routine should be fine also for the ones capable of
> > mmio64 registers access. Do you have strong objections to the change?
> >
> 
> To be fair, the SBSA v5.0 does describe this register as
> 
> 0x010-0x013  WCV[31:0]   Watchdog compare value. Read/write registers
> 0x014-0x017  WCV[63:32]  containing the current value in the watchdog
>  compare register.
> 
> so I guess it is implied that any implementation should tolerate this
> value being written as 2 32-bit quantities.
> 
> Leif?

Yep, the way the SBSA describes it, these are two separate 32-bit
registers, making accessing them with a single transaction ends up
being implementation defined behaviour.

We should maybe add separate _HIGH and _LOW #defines for the two rather
than calculating register addresses inline?

But apart from that:
Reviewed-by: Leif Lindholm 

> >
> >>
> >> > Contributed-under: TianoCore Contribution Agreement 1.1
> >> > Signed-off-by: Marcin Wojtas 
> >> > ---
> >> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 6 +-
> >> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >> >
> >> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> >> > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> >> > index 3180f01..b25d210 100644
> >> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> >> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> >> > @@ -56,7 +56,11 @@ WatchdogWriteCompareRegister (
> >> >UINT64  Value
> >> >)
> >> >  {
> >> > -  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
> >> > +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG, Value & MAX_UINT32);
> >> > +  MmioWrite32 (
> >> > +GENERIC_WDOG_COMPARE_VALUE_REG + sizeof (UINT32),
> >> > +(Value >> 32) & MAX_UINT32
> >> > +);
> >> >  }
> >> >
> >> >  VOID
> >> > --
> >> > 2.7.4
> >> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Question about memory map entries

[Rafael Machado]

Just found something interesting.
Based on the logs from the serial port.

This system works fine:

"PeiInstallPeiMemory MemoryBegin 0x93D5, MemoryLength 0xA2B
Temp Stack : BaseAddress=0x40 Length=0x8
Temp Heap  : BaseAddress=0x48 Length=0x8
Total temporary memory:1048576 bytes.
  temporary memory stack ever used: 524288 bytes.
  temporary memory heap used:   63304 bytes.
Old Stack size 524288, New stack size 524288
Stack Hob: BaseAddress=0x93D5 Length=0x8
Heap Offset = 0x9395 Stack Offset = 0x9395
Loading PEIM at 0x0009DFF4000 EntryPoint=0x0009DFF4260 PeiCore.efi"
...
"CoreInitializeMemoryServices:
  BaseAddress - 0x93DE1000 Length - 0x8135000 MinimalMemorySizeNeeded -
0x5AC"

This one is bricked:

"PeiInstallPeiMemory MemoryBegin 0x9C9000, MemoryLength 0x9D637000
Temp Stack : BaseAddress=0x40 Length=0x8
Temp Heap  : BaseAddress=0x48 Length=0x8
Total temporary memory:1048576 bytes.
  temporary memory stack ever used: 524288 bytes.
  temporary memory heap used:   63304 bytes.
Old Stack size 524288, New stack size 524288
Stack Hob: BaseAddress=0x9C9000 Length=0x8
Heap Offset = 0x5C9000 Stack Offset = 0x5C9000
Loading PEIM at 0x0009DFF4000 EntryPoint=0x0009DFF4260 PeiCore.efi"
...
"CoreInitializeMemoryServices:
  BaseAddress - 0x0 Length - 0x0 MinimalMemorySizeNeeded - 0x98E47000
"
...
"ASSERT_EFI_ERROR (Status = Out of Resources)
ASSERT [DxeCore] ...\MdeModulePkg\Core\Dxe\DxeMain\DxeMain.c(299):
!EFI_ERROR (Status)
AllocatePoolPages: failed to allocate 1 pages
AllocatePool: failed to allocate 120 bytes"


It's really strange that the "Stack Hob base address" is so different, and
the "MemoryBegin" also.
This is making the memory to be detected incorrectly as far as I could
understand. So CoreInitializeMemoryServices does not have enougth memory to
work on.
Any idea about what could cause this difference?

Unfortunately I don't have the system in hands. And also cannot share the
entire log due to legal. But these are the differences between the bricked
system and the normal one.
Any idea?

Thanks and Regards
Rafael R. Machado


Em qui, 2 de ago de 2018 às 16:02, Rafael Machado <
rafaelrodrigues.mach...@gmail.com> escreveu:

> Hi Laszlo
>
> Got your point. Thanks for the comment.
>
> I'll keep working on it.
> In case someone has some other information or idea feel free to share.
>
> Thanks
> Rafael
>
> Em qui, 2 de ago de 2018 às 14:48, Laszlo Ersek 
> escreveu:
>
>> On 08/02/18 18:42, Rafael Machado wrote:
>> > Thanks Andrew and Laszlo for the clarification and guidance.
>> >
>> > About Laszlo questions
>> >
>> >> Is the reboot automatic (from the platform firmware), or application /
>> >> user initiated?
>> > Yes. We just do some clean up, finish the events and "return
>> EFI_SUCCESS;"
>>
>> That's really strange. I don't think that's valid or expected behavior.
>> If a boot option exits with success, then, as I understand it, the boot
>> manager is expected to return to the setup UI at once. (I don't have a
>> reference ready for this, but I remember someone mentioning it.) Boot
>> option processing continues only if the current boot option exits with
>> failure.
>>
>> I think the reboot you see is actually a crash. (Not saying that the
>> issue is in your application, just that the reboot should not be
>> triggered by either the application or the platform firmware.)
>>
>> Thanks,
>> Laszlo
>>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH v2 0/6] Armada7k8k ComPhy rework

[Ard Biesheuvel]

On 26 July 2018 at 09:21, Marcin Wojtas  wrote:
> Hi,
>
> The second version of the patchset brings minor corrections,
> that were pointed out during review. Details can be found
> int the commit logs.
>
> The patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/comphy-upstream-r20180726
>
> I'm looking forward to review and any comments/remarks.
>
> Best regards,
> Marcin
>
> Changelog:
> v1 -> v2
>  * 1/6
>- improve commit message
>- create dedicated header for externally defined SiP services
>  ComPhy parameters
>- add comments about external definitions
>
>  * 2,3,4/6
>- s/firmware/ARM-TF/ in the commit messages
>
>  * 5,6/6
>- swap order without any changes in the patches - in the first
>  series compilation was broken on 5th and immediately fixed in
>  the last patch. Now every patch compiles.
>
> Grzegorz Jaszczyk (5):
>   Marvell/Library: ComPhyLib: Configure SATA, SGMII and SFI in ARM-TF
>   Marvell/Library: ComPhyLib: Configure PCIE in ARM-TF
>   Marvell/Library: ComPhyLib: Configure RXAUI in ARM-TF
>   Marvell/Library: ComPhyLib: Configure USB in ARM-TF
>   Marvell/Library: ComPhyLib: Remove both PHY and PIPE selector config
>   Marvell/Library: ComPhyLib: Clean up the library after rework
>

Reviewed-by: Ard Biesheuvel 

Pushed as 016d55843a01..9dae9a0c7996

Thanks!

>  Silicon/Marvell/Armada7k8k/Armada7k8k.dsc.inc
>   |   23 +-
>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.inf  
>   |2 +-
>  
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.h
>  |   22 -
>  Silicon/Marvell/Include/Library/SampleAtResetLib.h   
>   |7 -
>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.h
>   |  512 +-
>  Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h 
>   |   86 +
>  
> Silicon/Marvell/Armada7k8k/Library/Armada7k8kSampleAtResetLib/Armada7k8kSampleAtResetLib.c
>  |   19 -
>  Silicon/Marvell/Library/ComPhyLib/ComPhyCp110.c  
>   | 1777 +---
>  Silicon/Marvell/Library/ComPhyLib/ComPhyLib.c
>   |   41 +-
>  Silicon/Marvell/Library/ComPhyLib/ComPhyMux.c
>   |  132 --
>  10 files changed, 170 insertions(+), 2451 deletions(-)

(/me does a little dance after reading this line)

>  create mode 100644 Silicon/Marvell/Library/ComPhyLib/ComPhySipSvc.h
>  delete mode 100644 Silicon/Marvell/Library/ComPhyLib/ComPhyMux.c
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Question about memory map entries

[Rafael Machado]

Hi Laszlo

Got your point. Thanks for the comment.

I'll keep working on it.
In case someone has some other information or idea feel free to share.

Thanks
Rafael

Em qui, 2 de ago de 2018 às 14:48, Laszlo Ersek 
escreveu:

> On 08/02/18 18:42, Rafael Machado wrote:
> > Thanks Andrew and Laszlo for the clarification and guidance.
> >
> > About Laszlo questions
> >
> >> Is the reboot automatic (from the platform firmware), or application /
> >> user initiated?
> > Yes. We just do some clean up, finish the events and "return
> EFI_SUCCESS;"
>
> That's really strange. I don't think that's valid or expected behavior.
> If a boot option exits with success, then, as I understand it, the boot
> manager is expected to return to the setup UI at once. (I don't have a
> reference ready for this, but I remember someone mentioning it.) Boot
> option processing continues only if the current boot option exits with
> failure.
>
> I think the reboot you see is actually a crash. (Not saying that the
> issue is in your application, just that the reboot should not be
> triggered by either the application or the platform firmware.)
>
> Thanks,
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Ard Biesheuvel]

On 2 August 2018 at 17:12, Marcin Wojtas  wrote:
> czw., 2 sie 2018 o 16:59 Ard Biesheuvel  
> napisał(a):
>>
>> On 2 August 2018 at 16:49, Marcin Wojtas  wrote:
>> > EDK2 code uses a single 64bit write to update SBSA watchdog
>> > compare registers, however an access to mmio registers should
>> > be 32bit for some SoCs. Current usage of MmioWrite64 resulted
>> > in an unpredicted behavior. Fix this by modifying
>> > WatchdogWriteCompareRegister routine to use two consecutive
>> > 32bit writes to Watchdog Compare Register Low and High.
>> >
>>
>> You describe it as if this is generally the case, but this is just a
>> silicon bug, right?
>
> Not sure if it's a bug, or simply SoC characterisctics to place SoC
> registers to allow only mmio32 access to 32-bit registers. In any way,
> even updated routine should be fine also for the ones capable of
> mmio64 registers access. Do you have strong objections to the change?
>

To be fair, the SBSA v5.0 does describe this register as

0x010-0x013  WCV[31:0]   Watchdog compare value. Read/write registers
0x014-0x017  WCV[63:32]  containing the current value in the watchdog
 compare register.

so I guess it is implied that any implementation should tolerate this
value being written as 2 32-bit quantities.

Leif?


>
>>
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Marcin Wojtas 
>> > ---
>> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 6 +-
>> >  1 file changed, 5 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
>> > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
>> > index 3180f01..b25d210 100644
>> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
>> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
>> > @@ -56,7 +56,11 @@ WatchdogWriteCompareRegister (
>> >UINT64  Value
>> >)
>> >  {
>> > -  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
>> > +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG, Value & MAX_UINT32);
>> > +  MmioWrite32 (
>> > +GENERIC_WDOG_COMPARE_VALUE_REG + sizeof (UINT32),
>> > +(Value >> 32) & MAX_UINT32
>> > +);
>> >  }
>> >
>> >  VOID
>> > --
>> > 2.7.4
>> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Undefined types referenced in MdePkg

[Laszlo Ersek]

On 08/02/18 15:05, Gao, Liming wrote:
> MdePkg provides the basic module type header file, such as PiPei.h,
> PiDxe.h, Uefi.h. PiPei.h and PiDxe.h will include the basic PI
> definition. They have included PiHob.h and PiFirmwareVolume.h. 
> Because the module type header file is included into AutoGen.h based
> on the module type. And, AutoGen.h is always included by compiler
> option. So, in edk2 build system, there is no issue to include
> HobLib.h and Protocol/FirmwareVolume2.h in source files.

Thanks, this is helpful!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 10/38] Hisilicon/D06: Add ACPI Tables for D06

[Ard Biesheuvel]

On 2 August 2018 at 20:13, Leif Lindholm  wrote:
> Graeme, Ard, do either of you have the stamina to go through all this,
> or will wi settle for testing it?
>

Well, without a SoC manual, it is rather difficult to review this in
great detail.

I did give it a quick skim, and the only thing that looked peculiar to
me is that all PCI root complexes use IRQs 640-643 for the legacy INTx
interrupts for all slots. Also, there's a cacheline size value of 32
bytes somewhere. In summary, the code does not look wrong per se, but
it does not necessarily look like it was put together with great
diligence.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 10/38] Hisilicon/D06: Add ACPI Tables for D06

[Leif Lindholm]

Graeme, Ard, do either of you have the stamina to go through all this,
or will wi settle for testing it?

On Tue, Jul 24, 2018 at 03:08:54PM +0800, Ming Huang wrote:
> From: Yan Zhang 
> 
> ACPI tables for D06 2P, especially,Hi1620Iort.asl is include smmu
> and Hi1620IortNoSmmu.asl is without smmu.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yan Zhang 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc  |
> 1 +
>  Platform/Hisilicon/D06/D06.fdf  |
> 1 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf  |   
> 59 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/CPU.asl  |  
> 409 
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Com.asl  |   
> 30 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/DsdtHi1620.asl   |   
> 35 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Apei.asl   |   
> 93 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Ged.asl|   
> 58 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Mbig.asl   | 
> 1459 ++
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Mctp.asl   |   
> 41 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Pci.asl| 
> 1216 
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Power.asl  |   
> 28 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Rde.asl|   
> 47 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Sec.asl|   
> 57 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Socip4_i2c100k.asl |  
> 249 +++
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Hi1620Socip4_i2c400k.asl |  
> 249 +++
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/LpcUart_clk.asl  |   
> 49 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/Pv680UncorePmu.asl   | 
> 1658 
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Dsdt/ipmi.asl |   
> 49 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Facs.aslc |   
> 67 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Fadt.aslc |   
> 91 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Gtdt.aslc |   
> 86 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Dbg2.aslc   |   
> 86 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Iort.asl| 
> 1989 
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620IortNoSmmu.asl  | 
> 1736 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Mcfg.aslc   |   
> 64 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Platform.h  |   
> 48 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Slit.aslc   |   
> 64 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Spcr.aslc   |   
> 81 +
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/Hi1620Srat.aslc   |  
> 166 ++
>  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/MadtHi1620.aslc   |  
> 375 
>  31 files changed, 10641 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 3f6f1ff20d..392225250f 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -336,6 +336,7 @@
>MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
>Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/AcpiPlatformDxe.inf
>  
> +  Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
>Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  
>#
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index bc016a32ae..586e9ed77e 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -249,6 +249,7 @@ READ_LOCK_STATUS   = TRUE
>INF MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
>INF Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/AcpiPlatformDxe.inf
>  
> +  INF RuleOverride=ACPITABLE 
> Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
>INF Silicon/Hisilicon/Drivers/AcpiPlatformDxe/AcpiPlatformDxe.inf
>  
>#
> diff --git a/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf 
> b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
> new file mode 100644
> index 00..6229398a17
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi1620/Hi1620AcpiTables/AcpiTablesHi1620.inf
> @@ -0,0 +1,59 @@
> +## @file
> +#
> +#  ACPI table data and ASL sources required to boot the platform.
> +#
> +#  Copyright (c) 2014, ARM Ltd. All rights reserved.
> +#  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +#  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are 

Re: [edk2] [PATCH edk2-platforms v1 09/38] Hisilicon/D06: Add Debug Serial Port Init Driver

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:53PM +0800, Ming Huang wrote:
> From: Yan Zhang 
> 
> Debug serial port init driver is added to initilize debug
> serial port.

What is special about this platform that makes it require a separate
initialization function for the PL011?

/
Leif

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Yan Zhang 
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc   
> |  1 +
>  Platform/Hisilicon/D06/D06.fdf   
> |  1 +
>  
> Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.c
>| 64 
>  
> Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
>  | 48 +++
>  4 files changed, 114 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index f4dfef1087..3f6f1ff20d 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -426,6 +426,7 @@
># Memory test
>#
>MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
> +  
> Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
>MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index 2730eb42a9..bc016a32ae 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -303,6 +303,7 @@ READ_LOCK_STATUS   = TRUE
>INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
>INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
>  
> +  INF 
> Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>#
># Build Shell from latest source code instead of prebuilt binary
> diff --git 
> a/Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.c
>  
> b/Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.c
> new file mode 100644
> index 00..b7233eed21
> --- /dev/null
> +++ 
> b/Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.c
> @@ -0,0 +1,64 @@
> +/** @file
> +
> +Copyright (c) 2016 - 2018, Hisilicon Limited. All rights reserved.
> +Copyright (c) 2016 - 2018, Linaro Limited. All rights reserved.
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the 
> BSD License
> +which accompanies this distribution. The full text of the license may be 
> found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +RETURN_STATUS
> +EFIAPI
> +DebugSerialPortInitialize (
> +  VOID
> +  )
> +{
> +  UINT64  BaudRate;
> +  UINT32  ReceiveFifoDepth;
> +  EFI_PARITY_TYPE Parity;
> +  UINT8   DataBits;
> +  EFI_STOP_BITS_TYPE  StopBits;
> +
> +  BaudRate = FixedPcdGet64 (PcdUartDefaultBaudRate);
> +  ReceiveFifoDepth = 0; // Use default FIFO depth
> +  Parity = (EFI_PARITY_TYPE)FixedPcdGet8 (PcdUartDefaultParity);
> +  DataBits = FixedPcdGet8 (PcdUartDefaultDataBits);
> +  StopBits = (EFI_STOP_BITS_TYPE) FixedPcdGet8 (PcdUartDefaultStopBits);
> +  return PL011UartInitializePort (
> +   (UINTN)FixedPcdGet64 (PcdSerialDbgRegisterBase),
> +   FixedPcdGet32 (PL011UartClkInHz),
> +   ,
> +   ,
> +   ,
> +   ,
> +   
> +   );
> +}
> +
> +EFI_STATUS
> +SerialPortEntry (
> +  IN EFI_HANDLE ImageHandle,
> +  IN EFI_SYSTEM_TABLE   *SystemTable
> +  )
> +{
> +  EFI_STATUS Status;
> +  Status = DebugSerialPortInitialize ();
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "CPU1 TB serial port init ERROR: %r\n", Status));
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> diff --git 
> a/Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
>  
> b/Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
> new file mode 100644
> index 00..8c91bdf0f4
> --- /dev/null
> +++ 
> b/Silicon/Hisilicon/Hi1620/Drivers/Pl011DebugSerialPortInitDxe/Pl011DebugSerialPortInitDxe.inf
> @@ -0,0 +1,48 @@
> +#/** @file
> +#
> +#Copyright (c) 2016 - 2018, Hisilicon Limited. All rights reserved.
> +#Copyright (c) 2016 - 2018, Linaro Limited. All 

Re: [edk2] Question about memory map entries

[Laszlo Ersek]

On 08/02/18 18:42, Rafael Machado wrote:
> Thanks Andrew and Laszlo for the clarification and guidance.
> 
> About Laszlo questions
> 
>> Is the reboot automatic (from the platform firmware), or application /
>> user initiated?
> Yes. We just do some clean up, finish the events and "return EFI_SUCCESS;"

That's really strange. I don't think that's valid or expected behavior.
If a boot option exits with success, then, as I understand it, the boot
manager is expected to return to the setup UI at once. (I don't have a
reference ready for this, but I remember someone mentioning it.) Boot
option processing continues only if the current boot option exits with
failure.

I think the reboot you see is actually a crash. (Not saying that the
issue is in your application, just that the reboot should not be
triggered by either the application or the platform firmware.)

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

Re: [edk2] [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Laszlo Ersek]

On 08/02/18 17:40, Gao, Liming wrote:
> Laszlo:
>   I understand this patch set is to provide the way to append compile and 
> link option for BaseTools source build.

Yes.

> If so, the extend flag name may be EXTRA_CCFLAGS

I can rename EXTRA_OPTFLAGS to EXTRA_CCFLAGS, but in that case,
internally we will have:

  BUILD_OPTFLAGS = -O2 $(EXTRA_CCFLAGS)

in "header.makefile". In that case, I expect to receive a comment that
we shouldn't append a generic "CCFLAGS" variable to a more specialized
"OPTFLAGS" variable.

Obviously, I can rename "BUILD_OPTFLAGS" to "BUILD_CCFLAGS" as well --
but, in that case, I expect to receive a comment that we already have
"BUILD_CFLAGS".

The variable (more precisely, "RPM macro") that the Fedora distribution
will put into EXTRA_OPTFLAGS is also called %{optflags}. So I think
EXTRA_OPTFLAGS is an appropriate name.


If you still disagree, then can you please suggest a new name not just
for EXTRA_OPTFLAGS (-->EXTRA_CCFLAGS), but also for BUILD_OPTFLAGS?
Patch #3 explains why we need a separate BUILD_OPTFLAGS Makefile macro.


> and EXTRA_LDFLAGS.

Right, that's the currently proposed name.

> And, the extend flags are appended in the tail. 

Correct.

>   Besides, Pccts is the internal tool to generate VfrCompiler syntax source 
> file. It is not used in build process. I am not sure why they also require 
> the additional CC and LD flags.

It's a general policy thing; all native binaries should be built with
the system-wide flags. Some of those flags will let the binaries detect
some buffer overflows automatically, for example, which is helpful even
if the utility is never installed / packaged, just used as a one-off
build tool.

Thanks!
Laszlo

> 
> Thanks
> Liming
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, July 26, 2018 8:44 AM
>> To: edk2-devel-01 
>> Cc: Gao, Liming ; Zhu, Yonghong 
>> 
>> Subject: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and 
>> EXTRA_LDFLAGS from the caller
>>
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: extra_flags_rhbz1540244
>>
>> In the Fedora distribution, we'd like to pass system-wide flags related
>> to optimization and linking when the C and C++ language base tools are
>> built. This series lets the outermost "make" command push the
>> EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.
>>
>> Cc: Liming Gao 
>> Cc: Yonghong Zhu 
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (6):
>>   BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
>>   BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
>>   BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
>>   BaseTools/Pccts: clean up antlr and dlg makefiles
>>   BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
>>   BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
>>
>>  BaseTools/Source/C/Makefiles/footer.makefile   |  2 +-
>>  BaseTools/Source/C/Makefiles/header.makefile   | 16 ---
>>  BaseTools/Source/C/VfrCompile/GNUmakefile  | 11 +---
>>  BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++-
>>  BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +---
>>  5 files changed, 56 insertions(+), 23 deletions(-)
>>
>> --
>> 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 v1 08/38] Silicon/Hisilicon/Acpi: Unify HisiAcipPlatformDxe

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:52PM +0800, Ming Huang wrote:
> The EFI_ACPI_STATIC_RESOURCE_AFFINITY_TABLE struct is used by
> UpdateAcpiTable.c and Srat aslc. The struct may be different
> according to chips, so move some macro to PlatformArch.h.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 

Drop Heyi's Signed-off-by and this one can have:
Reviewed-by: Leif Lindholm 

/
Leif

> ---
>  Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c |  2 --
>  Silicon/Hisilicon/Hi1610/Include/PlatformArch.h |  6 
>  Silicon/Hisilicon/Hi1616/Include/PlatformArch.h |  6 
>  Silicon/Hisilicon/Hi1620/Include/PlatformArch.h |  6 
>  Silicon/Hisilicon/Include/Library/AcpiNextLib.h | 31 
> ++--
>  5 files changed, 40 insertions(+), 11 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c 
> b/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> index f5869841dc..54f49977c3 100644
> --- a/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> +++ b/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> @@ -20,8 +20,6 @@
>  #include 
>  #include 
>  
> -#define CORE_NUM_PER_SOCKET  32
> -#define NODE_IN_SOCKET   2
>  #define CORECOUNT(X) ((X) * CORE_NUM_PER_SOCKET)
>  
>  STATIC
> diff --git a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> index 45995c5893..f2e931f30b 100644
> --- a/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> @@ -29,6 +29,12 @@
>  // Max NUMA node number for each node type
>  #define MAX_NUM_PER_TYPE 8
>  
> +// for acpi
> +#define NODE_IN_SOCKET  2
> +#define CORE_NUM_PER_SOCKET 32
> +#define EFI_ACPI_MEMORY_AFFINITY_STRUCTURE_COUNT10
> +#define EFI_ACPI_6_2_ITS_AFFINITY_STRUCTURE_COUNT   8
> +
>  #define S1_BASE   0x400
>  
>  #endif
> diff --git a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> index 45995c5893..f2e931f30b 100644
> --- a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> @@ -29,6 +29,12 @@
>  // Max NUMA node number for each node type
>  #define MAX_NUM_PER_TYPE 8
>  
> +// for acpi
> +#define NODE_IN_SOCKET  2
> +#define CORE_NUM_PER_SOCKET 32
> +#define EFI_ACPI_MEMORY_AFFINITY_STRUCTURE_COUNT10
> +#define EFI_ACPI_6_2_ITS_AFFINITY_STRUCTURE_COUNT   8
> +
>  #define S1_BASE   0x400
>  
>  #endif
> diff --git a/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> index 7243a9ec35..2fc1b9219d 100644
> --- a/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> +++ b/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> @@ -30,5 +30,11 @@
>  #define MAX_NUM_PER_TYPE 8
>  
>  
> +// for acpi
> +#define NODE_IN_SOCKET  2
> +#define CORE_NUM_PER_SOCKET 48
> +#define EFI_ACPI_MEMORY_AFFINITY_STRUCTURE_COUNT16
> +#define EFI_ACPI_6_2_ITS_AFFINITY_STRUCTURE_COUNT   1
> +
>  #endif
>  
> diff --git a/Silicon/Hisilicon/Include/Library/AcpiNextLib.h 
> b/Silicon/Hisilicon/Include/Library/AcpiNextLib.h
> index fd05a3b960..2abffb65fc 100644
> --- a/Silicon/Hisilicon/Include/Library/AcpiNextLib.h
> +++ b/Silicon/Hisilicon/Include/Library/AcpiNextLib.h
> @@ -19,6 +19,21 @@
>  #ifndef __ACPI_NEXT_LIB_H__
>  #define __ACPI_NEXT_LIB_H__
>  
> +#include 
> +
> +///
> +/// ITS Affinity Structure Definition
> +///
> +#pragma pack(1)
> +typedef struct {
> +  UINT8   Type;
> +  UINT8   Length;
> +  UINT32  ProximityDomain;
> +  UINT16  Reserved;
> +  UINT32  ItsHwId;
> +} EFI_ACPI_6_2_ITS_AFFINITY_STRUCTURE;
> +#pragma pack()
> +
>  #define EFI_ACPI_6_1_GIC_ITS_INIT(GicITSHwId, GicITSBase) \
>{ \
>  EFI_ACPI_6_1_GIC_ITS, sizeof (EFI_ACPI_6_1_GIC_ITS_STRUCTURE), 
> EFI_ACPI_RESERVED_WORD, \
> @@ -42,8 +57,8 @@
>  #define EFI_ACPI_6_2_ITS_AFFINITY_STRUCTURE_INIT(
>\
>  ProximityDomain, ItsId)  
>\
>{  
>\
> -4, sizeof (EFI_ACPI_6_2_GIC_ITS_AFFINITY_STRUCTURE), ProximityDomain,
>\
> -{EFI_ACPI_RESERVED_BYTE, EFI_ACPI_RESERVED_BYTE}, ItsId  
>  \
> +4, sizeof (EFI_ACPI_6_2_ITS_AFFINITY_STRUCTURE), ProximityDomain,
>\
> +EFI_ACPI_RESERVED_WORD, ItsId
>

Re: [edk2] [PATCH edk2-platforms v1 07/38] Silicon/Hisilicon/D06: Wait for all disk ready

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:51PM +0800, Ming Huang wrote:
> This patch is relative to D06 SasDxe driver. The SasDxe set a
> variable to notice this libray. Here Wait for all disk ready
> for 30S at most.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Silicon/Hisilicon/HisiPkg.dec   
> |  1 +
>  Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c   
> | 43 
>  Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> |  2 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/Silicon/Hisilicon/HisiPkg.dec b/Silicon/Hisilicon/HisiPkg.dec
> index 35bea970ec..b56a6a6af7 100644
> --- a/Silicon/Hisilicon/HisiPkg.dec
> +++ b/Silicon/Hisilicon/HisiPkg.dec
> @@ -45,6 +45,7 @@
>  
>gHisiEfiMemoryMapGuid  = {0xf8870015, 0x6994, 0x4b98, {0x95, 0xa2, 0xbd, 
> 0x56, 0xda, 0x91, 0xc0, 0x7f}}
>gVersionInfoHobGuid = {0xe13a14c, 0x859c, 0x4f22, {0x82, 0xbd, 0x18, 0xe, 
> 0xe1, 0x42, 0x12, 0xbf}}
> +  gHisiOemVariableGuid = {0xac62b9a5, 0x9939, 0x41d3, {0xff, 0x5c, 0xc5, 
> 0x80, 0x32, 0x7d, 0x9b, 0x29}}
>gOemBootVariableGuid = {0xb7784577, 0x5aaf, 0x4557, {0xa1, 0x99, 0xd4, 
> 0xa4, 0x2f, 0x45, 0x06, 0xf8}}

What is the difference between gHisiOemVariableGuid and gOemBootVariableGuid?

>gEfiHisiSocControllerGuid = {0xee369cc3, 0xa743, 0x5382, {0x75, 0x64, 
> 0x53, 0xe4, 0x31, 0x19, 0x38, 0x35}}
>  
> diff --git a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> index 7dd5ba615c..f7536bfea3 100644
> --- a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -20,6 +20,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -554,6 +555,47 @@ PlatformBootManagerBeforeConsole (
>PlatformRegisterOptionsAndKeys ();
>  }
>  
> +STATIC
> +VOID
> +WaitForDiskReady (
> +  )
> +{
> +  EFI_STATUSStatus;
> +  UINT32Index;
> +  UINTN DataSize;
> +  UINT32DiskInfo;
> +  UINT8 IsFinished;
> +
> +  Status = EFI_NOT_FOUND;
> +  DataSize = sizeof (UINT32);
> +  // Wait for 30 seconds at most.
> +  for (Index=0; Index<30; Index++) {

Spaces around '=' and '<'.

> +Status = gRT->GetVariable (
> +L"SASDiskInfo",
> +,
> +NULL,
> +,
> +
> +);

Wait...
Are we synchronizing against the storage controller driver using an
environment variable and looping over it for 30 seconds?

That can't go in.
Please look into implementing an event in the SAS driver which you can
wait for here.

> +if (EFI_ERROR(Status)) {
> +  DEBUG ((DEBUG_ERROR, "Get DiskInfo:%r\n", Status));
> +  break;
> +}
> +
> +IsFinished = (UINT8)(DiskInfo >> 24);
> +if (IsFinished) {
> +  break;
> +}
> +DEBUG ((DEBUG_ERROR, "%a", Index == 0 ? "Wait for disk." : "."));
> +MicroSecondDelay(1000*1000); // 1S

Spaces around '*'.
The code already says to sleep a million microseconds, comment superfluous.

> +  }
> +
> +  if (!EFI_ERROR(Status)) {
> +DEBUG ((DEBUG_ERROR, "DiskInfo:%x\n", DiskInfo));
> +EfiBootManagerConnectAll ();

Why not call WaitForDiskReady() before EfiBootManagerConnectAll () in
PlatformBootManagerAfterConsole ()?

/
Leif

> +  }
> +}
> +
>  /**
>Do the platform specific action after the console is ready
>Possible things that can be done in PlatformBootManagerAfterConsole:
> @@ -583,6 +625,7 @@ PlatformBootManagerAfterConsole (
>// Connect the rest of the devices.
>//
>EfiBootManagerConnectAll ();
> +  WaitForDiskReady ();
>  
>//
>// Enumerate all possible boot options.
> diff --git 
> a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 7a53befc44..a093f13fb0 100644
> --- 
> a/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ 
> b/Silicon/Hisilicon/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -49,6 +49,7 @@
>MemoryAllocationLib
>PcdLib
>PrintLib
> +  TimerLib
>UefiBootManagerLib
>UefiBootServicesTableLib
>UefiLib
> @@ -67,6 +68,7 @@
>  [Guids]
>gEfiEndOfDxeEventGroupGuid
>gEfiTtyTermGuid
> +  gHisiOemVariableGuid
>  
>  [Protocols]
>gEfiGenericMemTestProtocolGuid
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 06/38] Hisilicon/D06: Add OemMiscLibD06

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:50PM +0800, Ming Huang wrote:
> This library include BoardFeatureD06.c and OemMiscLibD06.c c file,
> use for several modules like PciHostBridgeLib and Smbios.
> Enlarge macro PCIEDEVICE_REPORT_MAX for D06.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc  |   
> 1 +
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c  | 
> 432 
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06Strings.uni | 
> Bin 0 -> 5204 bytes

We appear to be permitting UTF-8 these days, which would be easier to
review. Would you be able to convert this for the next revision?

>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.c| 
> 157 +++
>  Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf  |  
> 47 +++
>  Silicon/Hisilicon/Include/Library/OemMiscLib.h  |   
> 2 +-
>  6 files changed, 638 insertions(+), 1 deletion(-)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 88869ba26e..f4dfef1087 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -71,6 +71,7 @@
>  
>TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>
> RealTimeClockLib|Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
> +  OemMiscLib|Platform/Hisilicon/D06/Library/OemMiscLibD06/OemMiscLibD06.inf
>
> OemAddressMapLib|Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.inf
>
> PlatformSysCtrlLib|Silicon/Hisilicon/Hi1620/Library/PlatformSysCtrlLibHi1620/PlatformSysCtrlLibHi1620.inf
>  
> diff --git a/Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c 
> b/Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c
> new file mode 100644
> index 00..c8f6cd0e29
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/Library/OemMiscLibD06/BoardFeatureD06.c
> @@ -0,0 +1,432 @@
> +/** @file
> +*
> +*  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +I2C_DEVICE gDS3231RtcDevice = {

*cough* *cough*

> +  .Socket = 0,
> +  .Port = 5,
> +  .DeviceType = DEVICE_TYPE_SPD,
> +  .SlaveDeviceAddress = 0x68
> +};
> +
> +SERDES_POLARITY_INVERT gSerdesPolarityTxDesc[] =
> +{
> +  {SERDES_INVALID_MACRO_ID, SERDES_INVALID_LANE_NUM}
> +};
> +
> +SERDES_POLARITY_INVERT gSerdesPolarityRxDesc[] =
> +{
> +  {SERDES_INVALID_MACRO_ID, SERDES_INVALID_LANE_NUM}
> +};
> +
> +SERDES_PARAM gSerdesParamNA = {
> +  .Hilink0Mode = EmHilink0Hccs1X8Width16,
> +  .Hilink1Mode = EmHilink1Hccs0X8Width16,
> +  .Hilink2Mode = EmHilink2Pcie2X8,
> +  .Hilink3Mode = 0x0,
> +  .Hilink4Mode = 0xF,
> +  .Hilink5Mode = EmHilink5Sas1X4,
> +  .Hilink6Mode = 0x0,
> +  .UseSsc  = 0,
> +};
> +
> +SERDES_PARAM gSerdesParamNB = {
> +  .Hilink0Mode = EmHilink0Pcie1X8,
> +  .Hilink1Mode = EmHilink1Pcie0X8,
> +  .Hilink2Mode = EmHilink2Sas0X8,
> +  .Hilink3Mode = 0x0,
> +  .Hilink4Mode = 0xF,
> +  .Hilink5Mode = EmHilink5Pcie2X2Pcie3X2,
> +  .Hilink6Mode = 0xF,
> +  .UseSsc  = 0,
> +};
> +
> +SERDES_PARAM gSerdesParamS1NA = {
> +  .Hilink0Mode = EmHilink0Hccs1X8Width16,
> +  .Hilink1Mode = EmHilink1Hccs0X8Width16,
> +  .Hilink2Mode = EmHilink2Pcie2X8,
> +  .Hilink3Mode = 0x0,
> +  .Hilink4Mode = 0xF,
> +  .Hilink5Mode = EmHilink5Sas1X4,
> +  .Hilink6Mode = 0x0,
> +  .UseSsc  = 0,
> +};
> +
> +SERDES_PARAM gSerdesParamS1NB = {
> +  .Hilink0Mode = EmHilink0Pcie1X8,
> +  .Hilink1Mode = EmHilink1Pcie0X8,
> +  .Hilink2Mode = EmHilink2Sas0X8,
> +  .Hilink3Mode = 0x0,
> +  .Hilink4Mode = 0xF,
> +  .Hilink5Mode = EmHilink5Pcie2X2Pcie3X2,
> +  .Hilink6Mode = 0xF,
> +  .UseSsc  = 0,
> +};
> +
> +
> +EFI_STATUS
> +OemGetSerdesParam (
> +  OUT SERDES_PARAM *ParamA,
> +  OUT SERDES_PARAM *ParamB,
> +  IN  UINT32   SocketId
> + )
> +{
> +  if (NULL == ParamA) {
> +DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Param == NULL!\n", __FUNCTION__, 
> __LINE__));
> +return EFI_INVALID_PARAMETER;
> +  } if (NULL == ParamB) {
> +DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Param == NULL!\n", __FUNCTION__, 
> __LINE__));
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (0 == SocketId) {
> +

Re: [edk2] [PATCH edk2-platforms v1 05/38] Platform/Hisilicon/D06: Add binary file for D06

[Leif Lindholm]

Could the subject be changed to "add edk2-non-osi components for D06"?

I should point out that I really like how this is done as a separate
patch.

On Tue, Jul 24, 2018 at 03:08:49PM +0800, Ming Huang wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc |  7 +++
>  Platform/Hisilicon/D06/D06.fdf | 17 +
>  2 files changed, 24 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index e05c97e1c6..88869ba26e 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -42,6 +42,8 @@
>  
>I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLib.inf
>TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +  IpmiCmdLib|Silicon/Hisilicon/Hi1610/Library/IpmiCmdLib/IpmiCmdLib.inf
> +

Hmm, does this suggest that the IpmiCmdLib should move out of Hi1610
to Silicon/Hisilicon/Library?
This is now the fourth platform making use of the Hi1610 variant.

/
Leif

>NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
>DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
>HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
> @@ -65,8 +67,12 @@
>  
>CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
>  
> +  SerdesLib|Silicon/Hisilicon/Hi1620/Library/Hi1620Serdes/Hi1620SerdesLib.inf
> +
>TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
>
> RealTimeClockLib|Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
> +  
> OemAddressMapLib|Platform/Hisilicon/D06/Library/OemAddressMapD06/OemAddressMapD06.inf
> +  
> PlatformSysCtrlLib|Silicon/Hisilicon/Hi1620/Library/PlatformSysCtrlLibHi1620/PlatformSysCtrlLibHi1620.inf
>  
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>
> GenericBdsLib|IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> @@ -82,6 +88,7 @@
># USB Requirements
>UefiUsbLib|MdePkg/Library/UefiUsbLib/UefiUsbLib.inf
>  
> +  LpcLib|Silicon/Hisilicon/Hi1620/Library/LpcLibHi1620/LpcLib.inf
>
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>  !if $(SECURE_BOOT_ENABLE) == TRUE
>FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> diff --git a/Platform/Hisilicon/D06/D06.fdf b/Platform/Hisilicon/D06/D06.fdf
> index 93c464c9f7..2730eb42a9 100644
> --- a/Platform/Hisilicon/D06/D06.fdf
> +++ b/Platform/Hisilicon/D06/D06.fdf
> @@ -56,6 +56,7 @@ NumBlocks = 0x40
>  
>  0x|0x0010
>  gArmTokenSpaceGuid.PcdSecureFvBaseAddress|gArmTokenSpaceGuid.PcdSecureFvSize
> +FILE = Platform/Hisilicon/D06/Sec/FVMAIN_SEC.Fv
>  
>  0x0010|0x0028
>  gArmTokenSpaceGuid.PcdFvBaseAddress|gArmTokenSpaceGuid.PcdFvSize
> @@ -163,6 +164,7 @@ READ_LOCK_STATUS   = TRUE
>INF MdeModulePkg/Core/Dxe/DxeMain.inf
>INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>  
> +  INF Platform/Hisilicon/D06/Drivers/IoInitDxe/IoInitDxe.inf
>#
># PI DXE Drivers producing Architectural Protocols (EFI Services)
>#
> @@ -170,6 +172,7 @@ READ_LOCK_STATUS   = TRUE
>INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>  
>INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +  INF Platform/Hisilicon/D06/Drivers/SFC/SfcDxeDriver.inf
>  
>  
>INF Silicon/Hisilicon/Drivers/FlashFvbDxe/FlashFvbDxe.inf
> @@ -225,10 +228,15 @@ READ_LOCK_STATUS   = TRUE
>INF MdeModulePkg/Bus/Usb/UsbMouseDxe/UsbMouseDxe.inf
>INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> +  INF 
> Platform/Hisilicon/D06/Drivers/Ipmi/IpmiInterfaceDxe/IpmiInterfaceDxe.inf
> +  INF Platform/Hisilicon/D06/Drivers/GetInfoFromBmc/GetInfoFromBmc.inf
>INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
>INF Silicon/Hisilicon/Drivers/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
>INF Silicon/Hisilicon/Drivers/Smbios/AddSmbiosType9/AddSmbiosType9.inf
> +  INF Platform/Hisilicon/D06/Drivers/TransferSmbiosInfo/TransSmbiosInfo.inf
> +  INF Platform/Hisilicon/D06/Drivers/IpmiMiscOpDxe/IpmiMiscOpDxe.inf
>  
> +  INF Platform/Hisilicon/D06/Drivers/IpmiWatchdogDxe/IpmiWatchdogDxe.inf
>  
>  
>INF 
> Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClassDxe.inf
> @@ -246,6 +254,7 @@ READ_LOCK_STATUS   = TRUE
>#
>#Network
>#
> +  INF Platform/Hisilicon/D06/Drivers/Net/SnpHi1620NewDxe/SnpDxe.inf
>  
>INF MdeModulePkg/Universal/Network/SnpDxe/SnpDxe.inf
>INF MdeModulePkg/Universal/Network/ArpDxe/ArpDxe.inf
> @@ -282,8 +291,14 @@ READ_LOCK_STATUS   = TRUE
>INF ArmPkg/Drivers/ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf
>INF MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
>INF MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
> +  INF Platform/Hisilicon/D06/Drivers/PcieRasInitDxe/PcieRasInitDxe.inf
> +  INF Platform/Hisilicon/D06/Drivers/RasInitDxe/RasInitDxe.inf
>  
> +  # VGA Driver
> +  #
> +  INF 

Re: [edk2] [PATCH edk2-platforms v1 04/38] Platform/Hisilicon/D06: Add M41T83RealTimeClockLib

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:48PM +0800, Ming Huang wrote:
> Add M41T83RealTimeClockLib for RTC.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 
> ---
>  Platform/Hisilicon/D06/D06.dsc   
> |   1 +
>  Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h  
> | 168 ++
>  
> Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.c
>| 603 
>  
> Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>  |  45 ++

Move this to Silicon/Hisilicon/Library?

>  4 files changed, 817 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> index 91470118b2..e05c97e1c6 100644
> --- a/Platform/Hisilicon/D06/D06.dsc
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -66,6 +66,7 @@
>CpldIoLib|Silicon/Hisilicon/Library/CpldIoLib/CpldIoLib.inf
>  
>TimeBaseLib|EmbeddedPkg/Library/TimeBaseLib/TimeBaseLib.inf
> +  
> RealTimeClockLib|Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClockLib.inf
>  
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>
> GenericBdsLib|IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> diff --git 
> a/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h 
> b/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
> new file mode 100644
> index 00..12a67948c3
> --- /dev/null
> +++ 
> b/Platform/Hisilicon/D06/Library/M41T83RealTimeClockLib/M41T83RealTimeClock.h
> @@ -0,0 +1,168 @@
> +/** @file
> +
> +  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#ifndef __M41T83_REAL_TIME_CLOCK_H__
> +#define __M41T83_REAL_TIME_CLOCK_H__
> +
> +#define M41T83_REGADDR_DOTSECONDS   0x00
> +#define M41T83_REGADDR_SECONDS  0x01
> +#define M41T83_REGADDR_MINUTES  0x02
> +#define M41T83_REGADDR_HOURS0x03
> +#define M41T83_REGADDR_WEEK_DAY 0x04
> +#define M41T83_REGADDR_DAY  0x05
> +#define M41T83_REGADDR_MONTH0x06
> +#define M41T83_REGADDR_YEAR 0x07
> +#define M41T83_REGADDR_ALARM1SEC0x0E
> +#define M41T83_REGADDR_ALARM1MIN0x0D
> +#define M41T83_REGADDR_ALARM1HOUR   0x0C
> +#define M41T83_REGADDR_ALARM1DATE   0x0B
> +#define M41T83_REGADDR_ALARM1MONTH  0x0A
> +
> +#define M41T83_REGADDR_TIMERCONTROL 0x11
> +
> +#define M41T83_REGADDR_ALARM2SEC0x18
> +#define M41T83_REGADDR_ALARM2MIN0x17
> +#define M41T83_REGADDR_ALARM2HOUR   0x16
> +#define M41T83_REGADDR_ALARM2DATE   0x15
> +#define M41T83_REGADDR_ALARM2MONTH  0x14
> +
> +#pragma pack(1)

It is not obivous to me why this pragma is needed here.
I don't mind the use of pack where it's helpful, but I don't want it
added "just in case".

> +
> +typedef union {
> +  struct {
> +UINT8 TD0:1;
> +UINT8 TD1:1;
> +UINT8 RSV:3;
> +UINT8 TIE:1;
> +UINT8 TITP:1;
> +UINT8 TE:1;
> +  } bits;

Bits.

> +  UINT8 u8;

Uint8.

(Please follow this pattern throughout.)

> +} RTC_M41T83_TIMERCONTROL;
> +
> +typedef union {
> +  struct {
> +UINT8 MicroSeconds;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_DOTSECOND;
> +
> +typedef union {
> +  struct{
> +UINT8 Seconds:7;
> +UINT8 ST:1;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_SECOND;
> +
> +typedef union {
> +  struct {
> +UINT8 Minutes:7;
> +UINT8 Rsv:1;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_MINUTE;
> +
> +typedef union {
> +  struct {
> +UINT8 Hours:6;
> +UINT8 CB:2;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_HOUR;
> +
> +typedef union {
> +  struct{
> +UINT8 Days:3;
> +UINT8 Rsv:5;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_WEEK_DAY;
> +
> +typedef union {
> +  struct{
> +UINT8 Days:6;
> +UINT8 Rsv:2;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_MONTH_DAY;
> +
> +typedef union {
> +  struct {
> +UINT8 Months:5;
> +UINT8 Rsv:3;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_MONTH;
> +
> +typedef union {
> +  struct {
> +UINT8 Years:8;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_YEAR;
> +
> +typedef union {
> +  struct {
> +UINT8 Second:7;
> +UINT8 RPT11:1;
> +  } bits;
> +  UINT8 u8;
> +} RTC_M41T83_ALARM1SEC;
> +
> +typedef union {
> +  struct {
> +UINT8 Minute:7;
> +UINT8 RPT12:1;
> +  } bits;
> + 

Re: [edk2] Question about memory map entries

[Rafael Machado]

Thanks Andrew and Laszlo for the clarification and guidance.

About Laszlo questions

>Is the reboot automatic (from the platform firmware), or application /
>user initiated?
Yes. We just do some clean up, finish the events and "return EFI_SUCCESS;"

>Do you exit the application before the system is powered off?
No. At this test we just let the application finishes it's work and power
off the system.  No "return EFI_SUCCESS;"

Thanks and Regards
Rafael

Em qui, 2 de ago de 2018 às 11:44, Laszlo Ersek 
escreveu:

> On 08/02/18 14:39, Rafael Machado wrote:
> > Hi everyone
> >
> > After some other tasks I am back to this case :)
> >
> > After some debug, we detected the moment where things start to go wrong,
> > but I am not sure what may cause this.
> >
> > What we noticed is that the following assert is reached:
> >
> https://github.com/tianocore/edk2/blob/87acb6e298e718250dd8b741b6888a3a54c7cb5a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2199
> >
> > Just to remember, this assert is reached with the following steps:
> > 1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
> > 2 - Execute the application tasks
> > 3 - exit the application (free everything, all events closed and  no
> memory
> > leaks detected as suggested to check by Andrew on the previous e-mail,
> then
> > return efi_success)
> > 4 - the system will reboot and reach the assert
>
> Is the reboot automatic (from the platform firmware), or application /
> user initiated?
>
> > But it does not happen with the following scenario:
> > 1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
> > 2 - Execute the application tasks
> > 3 - Power off the system
>
> Do you exit the application before the system is powered off?
>
> >
> > As far as I could understand (please correct my understanding that may be
> > wrong since is the first time I look at this part of the code), at this
> > point the HOBs passed from sec phase are processed by PEI so the memory
> > could be "detected/mapped/initialized" correctly. But for some reason the
> > required HOB is no present at the list.
> >
> > Could someone with more experience at this part of the code please
> confirm
> > my understanding, and if possible give some guesses about what could
> cause
> > this scenario?
>
> PEI may act differently (produce different HOBs) dependent on boot mode.
> The PI spec defines several boot modes; it's platform-dependent what
> hardware states / transitions are mapped to what PI boot modes by the
> firmware.
>
> > My guess is that some memory cleanup that should be done by the bios
> after
> > the application exits is not being done correctly. So I believe the
> problem
> > is not at the application, but at the BIOS. A friend here mentioned about
> > the MemoryTypeInformation efi var, that may be corrupted, and considering
> > it's used to guide the boot process it may impact the boot, but I am not
> > sure if this is the case and also I didn't find to much information about
> > this var and it's usage, so any help about this would be well received
> also.
>
> MemoryTypeInformation measures peak usage (of various UEFI memory types)
> during boot, so that at next boot, the internal allocation "bins" can be
> primed with large enough sizes. The goal is to reduce fragmentation due
> to "unforeseen" allocations.
>
> If you exit the application gracefully in both scenarios (and the only
> difference is whether you reboot the system, or power it down,
> afterwards, e.g. by passing different options to the RESET command of
> the UEFI shell), then I don't see how MemoryTypeInformation could be
> relevant.
>
> Thanks
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 03/38] Hisilicon/D06: Add several basal file for D06

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:47PM +0800, Ming Huang wrote:
> Add several basal head file and add several build configuration

basal -> base

> for D06.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang 
> Signed-off-by: Heyi Guo 

Ah, yes - please drop the multiple Signed-off-bys as well.
If Heyi wrote the patch, keep him as Author - but only your
Signed-off-by on patches you send out.

> ---
>  Platform/Hisilicon/D06/D06.dec   |  29 ++
>  Platform/Hisilicon/D06/D06.dsc   | 459 
> 
>  Platform/Hisilicon/D06/D06.fdf   | 351 +++
>  Platform/Hisilicon/D06/Include/Library/CpldD06.h |  37 ++
>  Silicon/Hisilicon/Hi1620/Include/Library/SerdesLib.h |  85 
>  Silicon/Hisilicon/Include/Library/OemAddressMapLib.h |   6 +
>  Silicon/Hisilicon/Include/Library/OemNicLib.h|  58 +++
>  7 files changed, 1025 insertions(+)
> 
> diff --git a/Platform/Hisilicon/D06/D06.dec b/Platform/Hisilicon/D06/D06.dec
> new file mode 100644
> index 00..555f816e69
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/D06.dec
> @@ -0,0 +1,29 @@
> +#/** @file
> +#
> +#Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +#Copyright (c) 2018, Linaro Limited. All rights reserved.
> +#
> +#This program and the accompanying materials
> +#are licensed and made available under the terms and conditions of the 
> BSD License
> +#which accompanies this distribution. The full text of the license may 
> be found at
> +#http://opensource.org/licenses/bsd-license.php
> +#
> +#THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#**/
> +
> +#
> +# D06 Package
> +#
> +#
> +#
> +
> +[Defines]
> +  DEC_SPECIFICATION  = 0x00010005

0x0001001a

> +  PACKAGE_NAME   = D06Pkg
> +  PACKAGE_GUID   = B46F75D7-3864-450D-86D9-A0346A882232
> +  PACKAGE_VERSION= 0.1
> +
> +[Includes]
> +  Include
> diff --git a/Platform/Hisilicon/D06/D06.dsc b/Platform/Hisilicon/D06/D06.dsc
> new file mode 100644
> index 00..91470118b2
> --- /dev/null
> +++ b/Platform/Hisilicon/D06/D06.dsc
> @@ -0,0 +1,459 @@
> +#
> +#  Copyright (c) 2011-2012, ARM Limited. All rights reserved.
> +#  Copyright (c) 2018, Hisilicon Limited. All rights reserved.
> +#  Copyright (c) 2018, Linaro Limited. All rights reserved.
> +#
> +#  This program and the accompanying materials
> +#  are licensed and made available under the terms and conditions of the BSD 
> License
> +#  which accompanies this distribution.  The full text of the license may be 
> found at
> +#  http://opensource.org/licenses/bsd-license.php
> +#
> +#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +#
> +#
> +
> +
> +#
> +# Defines Section - statements that will be processed to create a Makefile.
> +#
> +
> +[Defines]
> +  PLATFORM_NAME  = D06
> +  PLATFORM_GUID  = D0D445F1-B2CA-4101-9986-1B23525CBEA6
> +  PLATFORM_VERSION   = 0.1
> +  DSC_SPECIFICATION  = 0x00010005

0x0001001a

> +  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
> +  SUPPORTED_ARCHITECTURES= AARCH64
> +  BUILD_TARGETS  = DEBUG|RELEASE

Can you also add NOOPT please?
Between DEBUG and RELEASE.

> +  SKUID_IDENTIFIER   = DEFAULT
> +  FLASH_DEFINITION   = 
> Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
> +  DEFINE INCLUDE_TFTP_COMMAND=1

TFTP needs to be disabled by default.
Also, if you set a default value here to edit, the test belows needs to
be !if ... == 1 (but preferable to use TRUE/FALSE) rather than !ifdef ...
(Currently it would be included regardless of what you set this
variable to.)

> +  DEFINE NETWORK_IP6_ENABLE  = FALSE
> +  DEFINE HTTP_BOOT_ENABLE= FALSE
> +  DEFINE SECURE_BOOT_ENABLE  = FALSE
> +
> +!include Silicon/Hisilicon/Hisilicon.dsc.inc
> +
> +[LibraryClasses.common]
> +  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
> +  
> ArmPlatformLib|Silicon/Hisilicon/Library/ArmPlatformLibHisilicon/ArmPlatformLib.inf
> +
> +
> +  I2CLib|Silicon/Hisilicon/Library/I2CLib/I2CLib.inf
> +  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
> +  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
> +  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> +  HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
> +  
> UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
> +  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
> +  

Re: [edk2] [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and EXTRA_LDFLAGS from the caller

[Gao, Liming]

Laszlo:
  I understand this patch set is to provide the way to append compile and link 
option for BaseTools source build. If so, the extend flag name may be 
EXTRA_CCFLAGS and EXTRA_LDFLAGS. And, the extend flags are appended in the 
tail. 

  Besides, Pccts is the internal tool to generate VfrCompiler syntax source 
file. It is not used in build process. I am not sure why they also require the 
additional CC and LD flags. 

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, July 26, 2018 8:44 AM
> To: edk2-devel-01 
> Cc: Gao, Liming ; Zhu, Yonghong 
> Subject: [PATCH 0/6] BaseTools/Source/C: take EXTRA_OPTFLAGS and 
> EXTRA_LDFLAGS from the caller
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: extra_flags_rhbz1540244
> 
> In the Fedora distribution, we'd like to pass system-wide flags related
> to optimization and linking when the C and C++ language base tools are
> built. This series lets the outermost "make" command push the
> EXTRA_OPTFLAGS and EXTRA_LDFLAGS macros into the BaseTools build.
> 
> Cc: Liming Gao 
> Cc: Yonghong Zhu 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (6):
>   BaseTools/footer.makefile: expand BUILD_CFLAGS last for C files too
>   BaseTools/header.makefile: remove "-c" from BUILD_CFLAGS
>   BaseTools/Source/C: split "-O2" to BUILD_OPTFLAGS
>   BaseTools/Pccts: clean up antlr and dlg makefiles
>   BaseTools/Source/C: take EXTRA_OPTFLAGS from the caller
>   BaseTools/Source/C: take EXTRA_LDFLAGS from the caller
> 
>  BaseTools/Source/C/Makefiles/footer.makefile   |  2 +-
>  BaseTools/Source/C/Makefiles/header.makefile   | 16 ---
>  BaseTools/Source/C/VfrCompile/GNUmakefile  | 11 +---
>  BaseTools/Source/C/VfrCompile/Pccts/antlr/makefile | 22 ++-
>  BaseTools/Source/C/VfrCompile/Pccts/dlg/makefile   | 28 +---
>  5 files changed, 56 insertions(+), 23 deletions(-)
> 
> --
> 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 v1 00/38] Upload for D06 platform

[Graeme Gregory]

On Thu, Aug 02, 2018 at 11:12:56AM +0100, Leif Lindholm wrote:
> On Thu, Aug 02, 2018 at 09:46:13AM +0800, Ming wrote:
> > I am sorry for the first issue, the modify FIRMWARE_VER patch is add
> > alone just befor send out the patchset.
> > 
> > For generating acpi table, I use acpica-tools 20180508 version and it works.
> > I think acpica-tools are not backward compatibility and confused about 
> > acpica-tools.
> 
> Yes, it's a real pain. We used to have lots of issues with this, but
> the last couple of years have been less bad.
> Some platforms moved from .asl to .aslc to get around this.
> 
> I can confirm using 20180508 version resolves this issue.
> And that 20180629 does not work :(
> Unfortunately that won't currently work with our AMD overdrive
> platforms. But we'll have to live with that for now.
> 
This sounds like a bug in IORT support in acpica tools as it works for
other tables.

Graeme

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

Re: [edk2] [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM.

[Gao, Liming]

Marvin:
  I suggest to verify this library functionality on Standalone MM driver first, 
then propose how to change it to support more module type. 

Thanks
Liming
> -Original Message-
> From: Marvin H?user [mailto:marvin.haeu...@outlook.com]
> Sent: Monday, July 23, 2018 9:02 PM
> To: edk2-devel@lists.01.org; Gao, Liming 
> Cc: Ni, Ruiyu 
> Subject: RE: [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM.
> 
> Best regards,
> Marvin.
> 
> > -Original Message-
> > From: Gao, Liming 
> > Sent: Monday, July 23, 2018 8:46 AM
> > To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu 
> > Subject: RE: [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone MM.
> >
> > Marvin:
> >   If this library supports standalone MM module only, I agree its source 
> > file
> > includes Standalone MM
> >   But, this library instance also supports DXE, SMM. I don't think
> > StandaloneMm name is good for them.
> 
> I called all files "DxeStandaloneMm", the same way as other libraries append 
> environment types ("PeiDxePostCodeLib", etc).
> Did I overlook something?
> 
> >
> >   Besides, I want to know why you changes this library instance. Are there 
> > the
> > standalone MM module to depend on this AcpiTimerLib?
> 
> I wanted to try out porting a private module which happened to depend on 
> AcpiTimerLib and thus HobLib.
> HobLib will follow once the MmServicesTableLib patch has been decided on and 
> I will adapt others too once I run into needing them.
> 
> >
> > Thanks
> > Liming
> > >-Original Message-
> > >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > >Marvin H?user
> > >Sent: Monday, July 23, 2018 7:05 AM
> > >To: edk2-devel@lists.01.org
> > >Cc: Ni, Ruiyu 
> > >Subject: [edk2] [PATCH] PcAtChipsetPkg/AcpiTimerLib: Support Standalone
> > >MM.
> > >
> > >To support Standalone MM, the current DXE implementation, which is also
> > >used to support DXE SMM Drivers, has been modified. Its type was
> > >changed to BASE to make the constructor function generic,
> > MM_STANDALONE
> > >modules types have been added to the support list and the internal
> > >files were adapted to show support.
> > >
> > >"DxeAcpiTimerLib.inf" has not been renamed to not break packages.
> > >This might be addressed with a separate patch.
> > >
> > >Contributed-under: TianoCore Contribution Agreement 1.1
> > >Signed-off-by: Marvin Haeuser 
> > >---
> > > PcAtChipsetPkg/Library/AcpiTimerLib/{DxeAcpiTimerLib.c =>
> > >DxeStandaloneMmAcpiTimerLib.c} |  9 +++--
> > > PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >| 14 +++---
> > > PcAtChipsetPkg/Library/AcpiTimerLib/{DxeAcpiTimerLib.uni =>
> > >StandaloneMmDxeAcpiTimerLib.uni} |  2 +-
> > > 3 files changed, 11 insertions(+), 14 deletions(-)
> > >
> > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > >similarity index 88%
> > >rename from PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > >rename to
> > >PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > >index 9ed10ef2e297..784f33871d75 100644
> > >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.c
> > >+++
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeStandaloneMmAcpiTimerLib.c
> > >@@ -13,6 +13,7 @@
> > > **/
> > >
> > > #include 
> > >+#include 
> > > #include 
> > > #include 
> > > #include 
> > >@@ -78,17 +79,13 @@ InternalGetPerformanceCounterFrequency (
> > > /**
> > >   The constructor function enables ACPI IO space, and caches
> > >PerformanceCounterFrequency.
> > >
> > >-  @param  ImageHandle   The firmware allocated handle for the EFI image.
> > >-  @param  SystemTable   A pointer to the EFI System Table.
> > >-
> > >   @retval EFI_SUCCESS   The constructor always returns RETURN_SUCCESS.
> > >
> > > **/
> > > EFI_STATUS
> > > EFIAPI
> > >-DxeAcpiTimerLibConstructor (
> > >-  IN EFI_HANDLEImageHandle,
> > >-  IN EFI_SYSTEM_TABLE  *SystemTable
> > >+DxeStandaloneMmAcpiTimerLibConstructor (
> > >+  VOID
> > >   )
> > > {
> > >   EFI_HOB_GUID_TYPE   *GuidHob;
> > >diff --git a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >index 601041c80137..f1f62247649e 100644
> > >--- a/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >+++ b/PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> > >@@ -1,5 +1,5 @@
> > > ## @file
> > >-#  DXE ACPI Timer Library
> > >+#  DXE and Standalone MM ACPI Timer Library
> > > #
> > > #  Provides basic timer support using the ACPI timer hardware.  The
> > >performance  #  counter features are provided by the processors time
> > >stamp counter.
> > >@@ -20,17 +20,17 @@
> > >
> > > [Defines]
> > >   INF_VERSION= 0x00010005
> > >-  BASE_NAME  = DxeAcpiTimerLib
> > >+  BASE_NAME  = DxeStandaloneMmAcpiTimerLib
> > >   FILE_GUID 

Re: [edk2] [PATCH] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Marcin Wojtas]

czw., 2 sie 2018 o 16:59 Ard Biesheuvel  napisał(a):
>
> On 2 August 2018 at 16:49, Marcin Wojtas  wrote:
> > EDK2 code uses a single 64bit write to update SBSA watchdog
> > compare registers, however an access to mmio registers should
> > be 32bit for some SoCs. Current usage of MmioWrite64 resulted
> > in an unpredicted behavior. Fix this by modifying
> > WatchdogWriteCompareRegister routine to use two consecutive
> > 32bit writes to Watchdog Compare Register Low and High.
> >
>
> You describe it as if this is generally the case, but this is just a
> silicon bug, right?

Not sure if it's a bug, or simply SoC characterisctics to place SoC
registers to allow only mmio32 access to 32-bit registers. In any way,
even updated routine should be fine also for the ones capable of
mmio64 registers access. Do you have strong objections to the change?

Marcin

>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> > b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > index 3180f01..b25d210 100644
> > --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> > @@ -56,7 +56,11 @@ WatchdogWriteCompareRegister (
> >UINT64  Value
> >)
> >  {
> > -  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
> > +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG, Value & MAX_UINT32);
> > +  MmioWrite32 (
> > +GENERIC_WDOG_COMPARE_VALUE_REG + sizeof (UINT32),
> > +(Value >> 32) & MAX_UINT32
> > +);
> >  }
> >
> >  VOID
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/2] Introduce UEFI PI 1.5 MM PPI.

[Gao, Liming]

This version change is good to me. 

Reviewed-by: Liming Gao 

> -Original Message-
> From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
> Sent: Tuesday, July 24, 2018 8:10 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming 
> ; Zeng, Star ; Dong,
> Eric ; Ni, Ruiyu 
> Subject: [PATCH v2 0/2] Introduce UEFI PI 1.5 MM PPI.
> 
> This patch series adds the MM PPIs introduced with the UEFI PI 1.5
> specification and in consequence changes the previous MdeModulePkg
> SMM PPIs to shim these.
> 
> Marvin Haeuser (2):
>   MdePkg: Add PI 1.5 MM PPI declarations.
>   MdeModulePkg: Change SMM PPIs to shim MM PPIs.
> 
>  MdeModulePkg/Include/Ppi/SmmAccess.h  | 114 +-
>  MdeModulePkg/Include/Ppi/SmmCommunication.h   |  36 +-
>  MdeModulePkg/Include/Ppi/SmmControl.h |  65 +-
>  .../Include/Ppi/MmAccess.h|  92 +++---
>  .../Include/Ppi/MmCommunication.h |  36 +++---
>  MdePkg/Include/Ppi/MmConfiguration.h  |  86 +
>  .../Include/Ppi/MmControl.h   |  50 
>  MdePkg/MdePkg.dec |  12 ++
>  8 files changed, 196 insertions(+), 295 deletions(-)
>  copy MdeModulePkg/Include/Ppi/SmmAccess.h => MdePkg/Include/Ppi/MmAccess.h 
> (58%)
>  copy MdeModulePkg/Include/Ppi/SmmCommunication.h => 
> MdePkg/Include/Ppi/MmCommunication.h (58%)
>  create mode 100644 MdePkg/Include/Ppi/MmConfiguration.h
>  copy MdeModulePkg/Include/Ppi/SmmControl.h => MdePkg/Include/Ppi/MmControl.h 
> (65%)
> 
> --
> 2.18.0.windows.1

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

Re: [edk2] [PATCH] ArmPkg/GenericWatchdogDxe: Split 64bit register write to 2x32bit

[Ard Biesheuvel]

On 2 August 2018 at 16:49, Marcin Wojtas  wrote:
> EDK2 code uses a single 64bit write to update SBSA watchdog
> compare registers, however an access to mmio registers should
> be 32bit for some SoCs. Current usage of MmioWrite64 resulted
> in an unpredicted behavior. Fix this by modifying
> WatchdogWriteCompareRegister routine to use two consecutive
> 32bit writes to Watchdog Compare Register Low and High.
>

You describe it as if this is generally the case, but this is just a
silicon bug, right?

> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c 
> b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> index 3180f01..b25d210 100644
> --- a/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> +++ b/ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.c
> @@ -56,7 +56,11 @@ WatchdogWriteCompareRegister (
>UINT64  Value
>)
>  {
> -  MmioWrite64 (GENERIC_WDOG_COMPARE_VALUE_REG, Value);
> +  MmioWrite32 (GENERIC_WDOG_COMPARE_VALUE_REG, Value & MAX_UINT32);
> +  MmioWrite32 (
> +GENERIC_WDOG_COMPARE_VALUE_REG + sizeof (UINT32),
> +(Value >> 32) & MAX_UINT32
> +);
>  }
>
>  VOID
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v1 02/38] Silicon/Hisilicon: Separate PlatformArch.h

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:46PM +0800, Ming Huang wrote:
> As the mocro of PlatformArch.h is platform special, so Separate

mocro -> macro (typo)
special -> specific (subtleties of English language :)
Separate -> separate (case)

> PlatformArch.h to Hi1610,Hi1616,Hi1620 for unifying D0x.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhou You 
> Signed-off-by: Ming Huang 
> ---
>  Silicon/Hisilicon/{ => Hi1610}/Include/PlatformArch.h |  0
>  Silicon/Hisilicon/Hi1616/Include/PlatformArch.h   | 35 
> 
>  Silicon/Hisilicon/Hi1620/Include/PlatformArch.h   | 34 
> +++

Could you move the Hi1620 file to the "add d06 base files" patch?
There is nothing else for D06 in the tree at this point.
(Don't forget to update comment to reflect.)

>  3 files changed, 69 insertions(+)
> 
> diff --git a/Silicon/Hisilicon/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> similarity index 100%
> rename from Silicon/Hisilicon/Include/PlatformArch.h
> rename to Silicon/Hisilicon/Hi1610/Include/PlatformArch.h
> diff --git a/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> new file mode 100644
> index 00..45995c5893
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi1616/Include/PlatformArch.h
> @@ -0,0 +1,35 @@
> +/** @file
> +*
> +*  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2015, Linaro Limited. All rights reserved.

Update copyright year?

> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +
> +
> +#ifndef _PLATFORM_ARCH_H_
> +#define _PLATFORM_ARCH_H_
> +
> +#define MAX_SOCKET  2
> +#define MAX_DIE 4
> +#define MAX_DDRC2
> +#define MAX_NODE(MAX_SOCKET * MAX_DIE)
> +#define MAX_CHANNEL 4
> +#define MAX_DIMM3
> +#define MAX_RANK_CH 12
> +#define MAX_RANK_DIMM   4
> +// Max NUMA node number for each node type
> +#define MAX_NUM_PER_TYPE 8
> +
> +#define S1_BASE   0x400
> +
> +#endif
> +
> diff --git a/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h 
> b/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> new file mode 100644
> index 00..7243a9ec35
> --- /dev/null
> +++ b/Silicon/Hisilicon/Hi1620/Include/PlatformArch.h
> @@ -0,0 +1,34 @@
> +/** @file
> +*
> +*  Copyright (c) 2015, Hisilicon Limited. All rights reserved.
> +*  Copyright (c) 2015, Linaro Limited. All rights reserved.

Update copyright year? (in other patch)

/
Leif

> +*
> +*  This program and the accompanying materials
> +*  are licensed and made available under the terms and conditions of the BSD 
> License
> +*  which accompanies this distribution.  The full text of the license may be 
> found at
> +*  http://opensource.org/licenses/bsd-license.php
> +*
> +*  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +*  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +*
> +**/
> +
> +
> +
> +#ifndef _PLATFORM_ARCH_H_
> +#define _PLATFORM_ARCH_H_
> +
> +#define MAX_SOCKET  2
> +#define MAX_DIE 4
> +#define MAX_DDRC4
> +#define MAX_NODE(MAX_SOCKET * MAX_DIE)
> +#define MAX_CHANNEL 8
> +#define MAX_DIMM2
> +#define MAX_RANK_CH 8
> +#define MAX_RANK_DIMM   4
> +// Max NUMA node number for each node type
> +#define MAX_NUM_PER_TYPE 8
> +
> +
> +#endif
> +
> -- 
> 2.17.0
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()

[Laszlo Ersek]

On 08/02/18 06:06, Gao, Liming wrote:
> Laszlo:
>   I have no other comments. IntelFrameworkPkg has another UefiLib library 
> instance FrameworkUefiLib. Could you help update it also?
> 
>   For MdePkg, you can add Reviewed-by: Liming Gao 

Thanks! I have now enough feedback for posting v2.

Cheers
Laszlo

> 
> Thanks
> Liming
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Monday, July 30, 2018 10:14 PM
>> To: Ni, Ruiyu ; edk2-devel-01 
>> Cc: Zhang, Chao B ; Dong, Eric
>> ; Carsey, Jaben ; Wu, Jiaxin
>> ; Yao, Jiewen ; Gao, Liming
>> ; Kinney, Michael D ;
>> Roman Bacik ; Fu, Siyuan
>> ; Zeng, Star 
>> Subject: Re: [PATCH 1/6] MdePkg/UefiLib: introduce
>> EfiOpenFileByDevicePath()
>>
>> On 07/30/18 03:54, Ni, Ruiyu wrote:
>>> On 7/27/2018 8:06 PM, Laszlo Ersek wrote:
 On 07/27/18 11:28, Ni, Ruiyu wrote:
> On 7/19/2018 4:50 AM, Laszlo Ersek wrote:
>
>> +  //
>> +  // Traverse the device path nodes relative to the filesystem.
>> +  //
>> +  while (!IsDevicePathEnd (*FilePath)) {
>> +    //
>> +    // Keep local variables that relate to the current device path
>> node tightly
>> +    // scoped.
>> +    //
>> +    FILEPATH_DEVICE_PATH *FilePathNode;
>> +    CHAR16   *AlignedPathName;
>> +    CHAR16   *PathName;
>> +    EFI_FILE_PROTOCOL    *NextFile;
> 1. Not sure if it follows the coding style. I would prefer to move the
> definition to the beginning of the function.

 OK, will do.
>>>
>>> Thanks!
>>>

>
>> +
>> +    if (DevicePathType (*FilePath) != MEDIA_DEVICE_PATH ||
>> +    DevicePathSubType (*FilePath) != MEDIA_FILEPATH_DP) {
>> +  Status = EFI_INVALID_PARAMETER;
>> +  goto CloseLastFile;
>> +    }
>> +    FilePathNode = (FILEPATH_DEVICE_PATH *)*FilePath;
>> +
>> +    //
>> +    // FilePathNode->PathName may be unaligned, and the UEFI
>> specification
>> +    // requires pointers that are passed to protocol member functions
>> to be
>> +    // aligned. Create an aligned copy of the pathname if necessary.
>> +    //
>> +    if ((UINTN)FilePathNode->PathName % sizeof
>> *FilePathNode->PathName == 0) {
>> +  AlignedPathName = NULL;
>> +  PathName = FilePathNode->PathName;
>> +    } else {
>> +  AlignedPathName = AllocateCopyPool (
>> +  (DevicePathNodeLength (FilePathNode) -
>> +   SIZE_OF_FILEPATH_DEVICE_PATH),
>> +  FilePathNode->PathName
>> +  );
>> +  if (AlignedPathName == NULL) {
>> +    Status = EFI_OUT_OF_RESOURCES;
>> +    goto CloseLastFile;
>> +  }
>> +  PathName = AlignedPathName;
>> +    }
>> +
>> +    //
>> +    // Open the next pathname fragment with EFI_FILE_MODE_CREATE
>> masked out and
>> +    // with Attributes set to 0.
>> +    //
>> +    Status = LastFile->Open (
>> + LastFile,
>> + ,
>> + PathName,
>> + OpenMode & ~(UINT64)EFI_FILE_MODE_CREATE,
>> + 0
>> + );
> 2. As I said in previous mail, is it really needed?
> Per spec it's not required. Per FAT driver implementation, it's also not
> required.

 I can do that, but it's out of scope for this series. The behavior that
 you point out is not a functionality bug (it is not observably erroneous
 behavior), just sub-optimal implementation. This series is about
 unifying the implementation and fixing those issues that are actual
 bugs. I suggest that we report a separate BZ about this question,
 discuss it separately, and then I can send a separate patch (which will
 benefit all client code at once).

 Does that sound acceptable?
>>>
>>> I agree.
>>>

>
>> +
>> +    //
>> +    // Retry with EFI_FILE_MODE_CREATE and the original Attributes if
>> the first
>> +    // attempt failed, and the caller specified EFI_FILE_MODE_CREATE.
>> +    //
>> +    if (EFI_ERROR (Status) && (OpenMode &
>> EFI_FILE_MODE_CREATE) !=
>> 0) {
>> +  Status = LastFile->Open (
>> +   LastFile,
>> +   ,
>> +   PathName,
>> +   OpenMode,
>> +   Attributes
>> +   );
>> +    }
>> +
>> +    //
>> +    // Release any AlignedPathName on both error and success paths;
>> PathName is
>> +    // no longer needed.
>> +    //
>> +    if (AlignedPathName != NULL) {
>> +  FreePool (AlignedPathName);
>> +    }
>> +    if (EFI_ERROR (Status)) {
>> +  goto 

Re: [edk2] Question about memory map entries

[Laszlo Ersek]

On 08/02/18 14:39, Rafael Machado wrote:
> Hi everyone
> 
> After some other tasks I am back to this case :)
> 
> After some debug, we detected the moment where things start to go wrong,
> but I am not sure what may cause this.
> 
> What we noticed is that the following assert is reached:
> https://github.com/tianocore/edk2/blob/87acb6e298e718250dd8b741b6888a3a54c7cb5a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2199
> 
> Just to remember, this assert is reached with the following steps:
> 1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
> 2 - Execute the application tasks
> 3 - exit the application (free everything, all events closed and  no memory
> leaks detected as suggested to check by Andrew on the previous e-mail, then
> return efi_success)
> 4 - the system will reboot and reach the assert

Is the reboot automatic (from the platform firmware), or application /
user initiated?

> But it does not happen with the following scenario:
> 1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
> 2 - Execute the application tasks
> 3 - Power off the system

Do you exit the application before the system is powered off?

> 
> As far as I could understand (please correct my understanding that may be
> wrong since is the first time I look at this part of the code), at this
> point the HOBs passed from sec phase are processed by PEI so the memory
> could be "detected/mapped/initialized" correctly. But for some reason the
> required HOB is no present at the list.
> 
> Could someone with more experience at this part of the code please confirm
> my understanding, and if possible give some guesses about what could cause
> this scenario?

PEI may act differently (produce different HOBs) dependent on boot mode.
The PI spec defines several boot modes; it's platform-dependent what
hardware states / transitions are mapped to what PI boot modes by the
firmware.

> My guess is that some memory cleanup that should be done by the bios after
> the application exits is not being done correctly. So I believe the problem
> is not at the application, but at the BIOS. A friend here mentioned about
> the MemoryTypeInformation efi var, that may be corrupted, and considering
> it's used to guide the boot process it may impact the boot, but I am not
> sure if this is the case and also I didn't find to much information about
> this var and it's usage, so any help about this would be well received also.

MemoryTypeInformation measures peak usage (of various UEFI memory types)
during boot, so that at next boot, the internal allocation "bins" can be
primed with large enough sizes. The goal is to reduce fragmentation due
to "unforeseen" allocations.

If you exit the application gracefully in both scenarios (and the only
difference is whether you reboot the system, or power it down,
afterwards, e.g. by passing different options to the RESET command of
the UEFI shell), then I don't see how MemoryTypeInformation could be
relevant.

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

Re: [edk2] [PATCH edk2-platforms v1 01/38] Silicon/Hisilicon: Modify the MRC interface for other module

[Leif Lindholm]

On Tue, Jul 24, 2018 at 03:08:45PM +0800, Ming Huang wrote:
> This patch is to unify D0x. Add pGBL_INTERFACE struct define
> and remove useless interfece. Replace DMRC pGblData with pGblInterface;
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Zhou You 
> Signed-off-by: Ming Huang 
> ---
>  Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c |   4 +-
>  Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c |  22 +-
>  Silicon/Hisilicon/Include/Library/HwMemInitLib.h| 351 
> 

Please add an orderfile as described in
https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-10
in order to make api/stuct changes are visible before changes to their
use.

You can set one permanently for your repository using
$ git config diff.orderFile 

>  3 files changed, 74 insertions(+), 303 deletions(-)
> 
> diff --git a/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c 
> b/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> index 7d06fccc2b..f5869841dc 100644
> --- a/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> +++ b/Silicon/Hisilicon/Drivers/HisiAcpiPlatformDxe/UpdateAcpiTable.c
> @@ -56,7 +56,7 @@ UpdateSrat (
>UINT8   Skt = 0;
>UINTN   Index = 0;
>VOID*HobList;
> -  GBL_DATA*Gbl_Data;
> +  GBL_INTERFACE   *Gbl_Data;
>UINTN   Base;
>UINTN   Size;
>UINT8   NodeId;
> @@ -69,7 +69,7 @@ UpdateSrat (
>if (HobList == NULL) {
>  return EFI_UNSUPPORTED;
>}
> -  Gbl_Data = (GBL_DATA*)GetNextGuidHob(, HobList);
> +  Gbl_Data = (GBL_INTERFACE*)GetNextGuidHob(, HobList);
>if (Gbl_Data == NULL) {
>  DEBUG((DEBUG_ERROR, "Get next Guid HOb fail.\n"));
>  return EFI_NOT_FOUND;
> diff --git 
> a/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c 
> b/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c
> index da714c9e22..262b129419 100644
> --- a/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c
> +++ b/Silicon/Hisilicon/Drivers/Smbios/MemorySubClassDxe/MemorySubClass.c
> @@ -45,7 +45,7 @@ SmbiosGetManufacturer (
>  
>  VOID
>  SmbiosGetPartNumber (
> -  IN pGBL_DATA  pGblData,
> +  IN pGBL_INTERFACE pGblData,
>IN UINT8  Skt,
>IN UINT8  Ch,
>IN UINT8  Dimm,
> @@ -78,7 +78,7 @@ SmbiosGetPartNumber (
>  
>  VOID
>  SmbiosGetSerialNumber (
> -  IN pGBL_DATA  pGblData,
> +  IN pGBL_INTERFACE pGblData,
>IN UINT8  Skt,
>IN UINT8  Ch,
>IN UINT8  Dimm,
> @@ -96,7 +96,7 @@ SmbiosGetSerialNumber (
>  
>  BOOLEAN
>  IsDimmPresent (
> -  IN  pGBL_DATA  pGblData,
> +  IN  pGBL_INTERFACE pGblData,
>IN  UINT8  Skt,
>IN  UINT8  Ch,
>IN  UINT8  Dimm
> @@ -115,7 +115,7 @@ IsDimmPresent (
>  
>  UINT8
>  SmbiosGetMemoryType (
> -  IN  pGBL_DATA  pGblData,
> +  IN  pGBL_INTERFACE pGblData,
>IN  UINT8  Skt,
>IN  UINT8  Ch,
>IN  UINT8  Dimm
> @@ -146,7 +146,7 @@ SmbiosGetMemoryType (
>  
>  VOID
>  SmbiosGetTypeDetail (
> -  IN  pGBL_DATA pGblData,
> +  IN  pGBL_INTERFACEpGblData,
>IN  UINT8 Skt,
>IN  UINT8 Ch,
>IN  UINT8 Dimm,
> @@ -186,7 +186,7 @@ SmbiosGetTypeDetail (
>  
>  VOID
>  SmbiosGetDimmVoltageInfo (
> -  IN pGBL_DATA pGblData,
> +  IN pGBL_INTERFACEpGblData,
>IN UINT8 Skt,
>IN UINT8 Ch,
>IN UINT8 Dimm,
> @@ -281,7 +281,7 @@ SmbiosGetPartitionWidth (
>  
>  EFI_STATUS
>  SmbiosAddType16Table (
> -  IN  pGBL_DATA  pGblData,
> +  IN  pGBL_INTERFACE pGblData,
>OUT EFI_SMBIOS_HANDLE  *MemArraySmbiosHandle
>)
>  {
> @@ -345,7 +345,7 @@ SmbiosAddType16Table (
>  
>  EFI_STATUS
>  SmbiosAddType19Table (
> -  IN pGBL_DATA  pGblData,
> +  IN pGBL_INTERFACE pGblData,
>IN EFI_SMBIOS_HANDLE  MemArraySmbiosHandle
>)
>  {
> @@ -397,7 +397,7 @@ SmbiosAddType19Table (
>  
>  EFI_STATUS
>  SmbiosAddType17Table (
> -  IN pGBL_DATA  pGblData,
> +  IN pGBL_INTERFACE pGblData,
>IN UINT8  Skt,
>IN UINT8  Ch,
>IN UINT8  Dimm,
> @@ -692,7 +692,7 @@ MemorySubClassEntryPoint(
>  EFI_STATUS  Status;
>  EFI_SMBIOS_PROTOCOL *Smbios;
>  EFI_HOB_GUID_TYPE   *GuidHob;
> -pGBL_DATA   pGblData;
> +pGBL_INTERFACE  pGblData;
>  EFI_SMBIOS_HANDLE   MemArraySmbiosHandle;
>  UINT8   Skt, Ch, Dimm;
>  
> @@ -702,7 +702,7 @@ 

Re: [edk2] Question about memory map entries

[Andrew Fish]

> On Aug 2, 2018, at 5:39 AM, Rafael Machado 
>  wrote:
> 
> Hi everyone
> 
> After some other tasks I am back to this case :)
> 
> After some debug, we detected the moment where things start to go wrong, but 
> I am not sure what may cause this.
> 
> What we noticed is that the following assert is reached:
> https://github.com/tianocore/edk2/blob/87acb6e298e718250dd8b741b6888a3a54c7cb5a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L21
>  
> h
>  
> 99
>  
> 
> Just to remember, this assert is reached with the following steps:
> 1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
> 2 - Execute the application tasks
> 3 - exit the application (free everything, all events closed and  no memory 
> leaks detected as suggested to check by Andrew on the previous e-mail, then 
> return efi_success)
> 4 - the system will reboot and reach the assert
> 
> But it does not happen with the following scenario:
> 1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
> 2 - Execute the application tasks
> 3 - Power off the system
> 
> As far as I could understand (please correct my understanding that may be 
> wrong since is the first time I look at this part of the code), at this point 
> the HOBs passed from sec phase are processed by PEI so the memory could be 
> "detected/mapped/initialized" correctly. But for some reason the required HOB 
> is no present at the list.
> 
> Could someone with more experience at this part of the code please confirm my 
> understanding, and if possible give some guesses about what could cause this 
> scenario?
> 
> My guess is that some memory cleanup that should be done by the bios after 
> the application exits is not being done correctly. So I believe the problem 
> is not at the application, but at the BIOS. A friend here mentioned about the 
> MemoryTypeInformation efi var, that may be corrupted, and considering it's 
> used to guide the boot process it may impact the boot, but I am not sure if 
> this is the case and also I didn't find to much information about this var 
> and it's usage, so any help about this would be well received also.
> 
> Any ideas?
> 

Rafael,

The gEfiMemoryTypeInformationGuid HOB is optional and used to defragment the 
EFI Memory Map. While it is copied it is not really in use at the point of your 
ASSERT. 

The PHIT HOB[1] must be the 1st HOB entry and it is the  layout of the memory 
that was in use by the PEI phase. At this point int he boot it is likely the 
memory registered in PEI via the InstallPeiMemory() PEI Service. The error is 
the memory in the PHIT hob does not have a corresponding 
EFI_HOB_TYPE_RESOURCE_DESCRIPTOR of type EFI_RESOURCE_SYSTEM_MEMORY.

Here is an example of code doing the registration. As you can see it calls 
PeiServicesInstallPeiMemory() and also generates the Resource HOB. 
https://github.com/tianocore/edk2/blob/master/QuarkPlatformPkg/Platform/Pei/PlatformInit/MrcWrapper.c#L738

You could try to track down the code in your code base doing the above 
operation, and if that looks OK maybe add DEBUG prints and dump out the HOBs to 
see if the got corrupted some how?

[1] PHIT HOB
///
/// Contains general state information used by the HOB producer phase.
/// This HOB must be the first one in the HOB list.
///
typedef struct {
  ///
  /// The HOB generic header. Header.HobType = EFI_HOB_TYPE_HANDOFF.
  ///
  EFI_HOB_GENERIC_HEADER  Header;
  ///
  /// The version number pertaining to the PHIT HOB definition.
  /// This value is four bytes in length to provide an 8-byte aligned entry
  /// when it is combined with the 4-byte BootMode.
  ///
  UINT32  Version;
  ///
  /// The system boot mode as determined during the HOB producer phase.
  ///
  EFI_BOOT_MODE   BootMode;
  ///
  /// The highest address location of memory that is allocated for use by the 
HOB producer
  /// phase. This address must be 4-KB aligned to meet page restrictions of 
UEFI.
  ///
  EFI_PHYSICAL_ADDRESSEfiMemoryTop;
  ///
  /// The lowest address location of memory that is allocated for use by the 
HOB producer phase.
  ///
  EFI_PHYSICAL_ADDRESSEfiMemoryBottom;
  ///
  /// The highest address location of free memory that is currently available
  /// for use by the HOB producer phase.
  ///
  EFI_PHYSICAL_ADDRESSEfiFreeMemoryTop;
  ///
  /// The lowest address location of free memory that is available for use by 
the HOB producer phase.
  ///
  EFI_PHYSICAL_ADDRESSEfiFreeMemoryBottom;
  ///
  /// The end of the HOB list.
  ///
  EFI_PHYSICAL_ADDRESSEfiEndOfHobList;
} EFI_HOB_HANDOFF_INFO_TABLE;


Thanks,

Andrew Fish

> Thanks and Regards
> Rafael 

Re: [edk2] [PATCH v2] NetworkPkg/HttpDxe: Strip square brackets in IPv6 expressed HostName.

[Laszlo Ersek]

On 08/02/18 03:20, Jiaxin Wu wrote:
> *v2: Optimize the patch by calculating AsciiStrSize() only once.
> 
> In URI, the colon (:) is used to terminate the HostName path before
> a port number. However, if HostName is expressed as IPv6 format, colon
> characters in IPv6 addresses will conflict with the colon before port
> number. To alleviate this conflict in URI, the IPv6 expressed HostName
> are enclosed in square brackets ([]). To record the real IPv6 HostName,
> square brackets should be stripped.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin 
> Reviewed-by: Fu Siyuan 
> Reviewed-by: Laszlo Ersek 
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
> index 17deceb395..de48243982 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -403,14 +403,26 @@ EfiHttpRequest (
>  Status = HttpParseUrl (Url, (UINT32) AsciiStrLen (Url), FALSE, 
> );
>  if (EFI_ERROR (Status)) {
>goto Error1;
>  }
>  
> -HostName   = NULL;
> -Status = HttpUrlGetHostName (Url, UrlParser, );
> +Status = HttpUrlGetHostName (Url, UrlParser, );
>  if (EFI_ERROR (Status)) {
> - goto Error1;
> +  goto Error1;
> +}
> +
> +if (HttpInstance->LocalAddressIsIPv6) {
> +  HostNameSize = AsciiStrSize (HostName);
> +
> +  if (HostNameSize > 2 && HostName[0] == '[' && HostName[HostNameSize - 
> 2] == ']') {
> +//
> +// HostName format is expressed as IPv6, so, remove '[' and ']'.
> +//
> +HostNameSize -= 2;
> +CopyMem (HostName, HostName + 1, HostNameSize - 1);
> +HostName[HostNameSize - 1] = '\0';
> +  }
>  }
>  
>  Status = HttpUrlGetPort (Url, UrlParser, );
>  if (EFI_ERROR (Status)) {
>if (HttpInstance->UseHttps) {
> 

Looks good to me.

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

Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 with OVMF

[Laszlo Ersek]

On 08/02/18 04:04, Zhang, Chao B wrote:
> Hi Laszlo & Ricardo
> The patch was intended to reduce the time to read TPM interface ID register. 
> The interface type should not change within boot cycle according to PTP spec.
> I agree to add some ASSERT after PCDSetxxsS.
> But It is a core solution without platform change as PCD has been configured 
> as DYN, DYNEx in DEC.  I don’t know why you meet Set Failure
> In OVMF. Here, I include PCD expert to explain this.

As far as I recall, dynamic PCDs have never behaved as actually settable
for me unless I added dynamic defaults for them in the OVMF DSC files.

I never really researched why this was the case, I just accepted that
the dynamic defaults were apparently necessary.

Let's wait for Ricardo's response. Perhaps my analysis / suspicion were
incorrect and it's not actually the "dynamism" of the PCD that's missing
for OVMF. Ricardo's answer will tell us if there's another issue.

Thanks
Laszlo

> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, August 2, 2018 5:49 AM
> To: Ricardo Araújo ; Zhang, Chao B 
> ; Marc-André Lureau 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7 
> with OVMF
> 
> On 08/01/18 19:50, Ricardo Araújo wrote:
>> The commit I was referring to is:
>> https://github.com/tianocore/edk2/commit/f15cb995bb3880b77e15afe6facd3da05e599a17
>>
>> Regards,
>>
>> Ricardo Araujo -
>> www.lsd.ufcg.edu.br/~ricardo
>>
>> - Mensagem original -
>> De: "Ricardo Araújo" 
>> mailto:rica...@lsd.ufcg.edu.br>>
>> Para: edk2-devel@lists.01.org
>> Enviadas: Quarta-feira, 1 de agosto de 2018 14:33:45
>> Assunto: [edk2] Missing boot related measurements at TPM 2.0 PCRs 0-7
>> with OVMF
>>
>> Hi everyone,
>>
>> I'm using OVMF with a simulated TPM 2.0 (from
>> https://github.com/stefanberger/swtpm) and I noticed lately that PCRs
>> 0-7 are zeroed after booting the vm (ubuntu 18.04) and the only
>> message related to this in dmesg is:
>>
>> [ 2.286690] tpm_tis 00:06: 2.0 TPM (device-id 0x1, rev-id 1)
>> [ 2.303753] tpm tpm0: A TPM error (256) occurred continue selftest
>> [ 2.314199] tpm tpm0: starting up the TPM manually
>>
>> I found this started to happen after this commit , previous commits to
>> that are showing boot time measurements on PCR 0-7 normally and the
>> error message is gone. Has anyone experienced the same behavior? I
>> followed the instructions here for building OVMF but I added the
>> parameters -D TPM2_ENABLE=TRUE -D SECURE_BOOT_ENABLE=TRUE -D
>> HTTP_BOOT_ENABLE=TRUE. Is there anything else I need to add to enable
>> these measurements?
>>
>> Regards,
>>
>> Ricardo Araujo
>> www.lsd.ufcg.edu.br/~ricardo
> 
> Thank you for the bug report. It looks like a regression to me, but the
> details aren't immediately clear.
> 
> Adding Marc-André who contributed the TPM enablement for OVMF, and Chao
> Zhang who authored the commit in question.
> 
> If I recall correctly, in OVMF we decided to never cache the TPM type
> but always detect it. I could be remembering wrong though. See commit
> 6cf1880fb5b6 ("OvmfPkg: add customized Tcg2ConfigPei clone",
> 2018-03-09).
> 
> Chao Zhang: can you please explain what additional requirements are
> presented for a platform by commit f15cb995bb38? In OVMF we use a
> customized Tcg2ConfigPei module (see the commit above).
> 
> 
> Oh wait, I suspect what's wrong. I believe there are two bugs in commit
> f15cb995bb38 ("SecurityPkg: Cache TPM interface type info", 2018-06-25):
> 
> * Bug#1:
> 
> Commit f15cb995bb38  introduces a new PCD, called
> "PcdActiveTpmInterfaceType", in section [PcdsDynamic, PcdsDynamicEx] of
> "SecurityPkg.dec", and makes core modules from SecurityPkg dependent on
> it.
> 
> Obviously this means that platforms are required to provide a Dynamic
> Default for the new PCD in their DSC files, if they include those core
> modules from SecurityPkg, otherwise the PCD won't actually behave
> dynamically -- "set" operations will fail, and "get" operations will
> just return the central default from the SecurityPkg.dec file. As a
> result, the cached TPM type will always be wrong (it will look like
> "undetected", 0xFF).
> 
> This could have been avoided by grepping all "*dsc*" files in the edk2
> tree for references to the SecurityPkg module INF files that were about
> to receive a dependency on the PCD. Such as:
> 
>   git grep -l -F \
> -e SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf \
> --or -e SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf \
> --or -e SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf \
> --or -e SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.inf \
> '*dsc*'
> 
> This would have listed all platforms in-tree that were going to depend
> on the new dynamic PCD via inclusion of the affected SecurityPkg
> modules.
> 
> Running this command now, I get the following 

Re: [edk2] Undefined types referenced in MdePkg

[Gao, Liming]

MdePkg provides the basic module type header file, such as PiPei.h, PiDxe.h, 
Uefi.h. PiPei.h and PiDxe.h will include the basic PI definition. They have 
included PiHob.h and PiFirmwareVolume.h. 
Because the module type header file is included into AutoGen.h based on the 
module type. And, AutoGen.h is always included by compiler option. So, in edk2 
build system, there is no issue to include HobLib.h and 
Protocol/FirmwareVolume2.h in source files. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> edk2-li...@mc2research.org
> Sent: Thursday, August 2, 2018 7:51 AM
> To: 'Laszlo Ersek' ; edk2-devel@lists.01.org
> Subject: Re: [edk2] Undefined types referenced in MdePkg
> 
> Laszlo,
> 
> Thank you for the pointers to the dependent header files.
> 
> I agree that main API header files should include any headers that they 
> depend upon and not require the developer to search out the
> dependencies.  Most modern pre-processors and compilers have optimizations to 
> mitigate reading the same header file multiple
> times.  This invalidates the old rationale for policies that don't allow 
> header files to include other header files.
> 
> Now, I need to find out why my search tool didn't find those headers.  ☹
> 
> In gratitude,
> Daryl McDaniel
> The Year of the Justifiably Defensive Lobster.
> The Season of the fraternizing CEOs.
> 
> -Original Message-
> From: edk2-devel  On Behalf Of Laszlo Ersek
> Sent: Wednesday, August 1, 2018 2:56 PM
> To: edk2-li...@mc2research.org; edk2-devel@lists.01.org
> Subject: Re: [edk2] Undefined types referenced in MdePkg
> 
> On 08/01/18 19:49, edk2-li...@mc2research.org wrote:
> > When including MdePkg/Include/Library/HobLib.h in my project I get
> > errors indicating that the following types are undefined:
> >
> > *   EFI_RESOURCE_TYPE, lines 206 and 231
> > *   EFI_RESOURCE_ATTRIBUTE_TYPE, lines 207 and 232
> >
> >
> >
> > A similar thing is happening with MdePkg/Include/Protocol/FirmwareVolume2.h.
> >
> > *   EFI_FV_FILE_ATTRIBUTES, lines 309, 443, 612
> >
> >
> >
> > I updated my EDK II tree then searched the entire tree for these
> > types.  They only show up in the locations mentioned above; never defined.
> >
> >
> >
> > Am I missing something or are these really undefined?
> >
> >
> >
> > I don't want to just disable these errors since that could hide a
> > bigger problem.
> 
> IMO, the header files you mention should include the following files,
> respectively:
> 
> - MdePkg/Include/Library/HobLib.h
>   --> MdePkg/Include/Pi/PiHob.h
> 
> - MdePkg/Include/Protocol/FirmwareVolume2.h
>   --> MdePkg/Include/Pi/PiFirmwareVolume.h
> 
> I'm firmly in the camp that believes that developers should not have to hunt 
> down themselves the dependencies of the main API header
> file that they are actually interested in.
> 
> Thanks
> Laszlo
> ___
> 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] Tool to check memory leaks

[Yao, Jiewen]

Please refer to 
https://github.com/tianocore/tianocore.github.io/wiki/Memory-leak-detection-with-memory-profile-feature

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Udit
> Kumar
> Sent: Thursday, August 2, 2018 5:00 PM
> To: edk2-devel@lists.01.org
> Cc: Sumit Batra 
> Subject: [edk2] Tool to check memory leaks
> 
> Hi All
> Is there some tool/debug option in edk2 to detect memory leak.
> Thanks
> Udit
> ___
> 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] Question about memory map entries

[Rafael Machado]

Hi everyone

After some other tasks I am back to this case :)

After some debug, we detected the moment where things start to go wrong,
but I am not sure what may cause this.

What we noticed is that the following assert is reached:
https://github.com/tianocore/edk2/blob/87acb6e298e718250dd8b741b6888a3a54c7cb5a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c#L2199

Just to remember, this assert is reached with the following steps:
1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
2 - Execute the application tasks
3 - exit the application (free everything, all events closed and  no memory
leaks detected as suggested to check by Andrew on the previous e-mail, then
return efi_success)
4 - the system will reboot and reach the assert

But it does not happen with the following scenario:
1 - Boot the application (renamed to BOOTX64.efi) from a usb stick
2 - Execute the application tasks
3 - Power off the system

As far as I could understand (please correct my understanding that may be
wrong since is the first time I look at this part of the code), at this
point the HOBs passed from sec phase are processed by PEI so the memory
could be "detected/mapped/initialized" correctly. But for some reason the
required HOB is no present at the list.

Could someone with more experience at this part of the code please confirm
my understanding, and if possible give some guesses about what could cause
this scenario?

My guess is that some memory cleanup that should be done by the bios after
the application exits is not being done correctly. So I believe the problem
is not at the application, but at the BIOS. A friend here mentioned about
the MemoryTypeInformation efi var, that may be corrupted, and considering
it's used to guide the boot process it may impact the boot, but I am not
sure if this is the case and also I didn't find to much information about
this var and it's usage, so any help about this would be well received also.

Any ideas?

Thanks and Regards
Rafael R. Machado

Em sáb, 30 de jun de 2018 às 16:23, Andrew Fish  escreveu:

>
>
> > On Jun 30, 2018, at 5:02 AM, Rafael Machado <
> rafaelrodrigues.mach...@gmail.com> wrote:
> >
> > Hi everyone. Thanks for the answers!
> > In this case, I just executed 3 shell command:
> >
> > memmap >> before.txt
> > app.efi
> > memmap >> after.txt
> >
> > Does anyone could clarify what could cause a new entry to be created at
> the
> > memmap output command?
>
> There is fragmentation caused the Apps high usage of memory and this can
> cause a lot more entries. I guess the DXE Core might also need to allocate
> some extra pages to track the fragmentation of the memory pool caused by
> the App.
>
> > My understanding was that the entries at the memmap command reflect the
> GCD
> > (global coherence domain), that is something that should not change too
> > much after the system is already at BDS phase.
>
> It is not really showing you GCD, it is showing the UEFI memory map. GCD
> implies the memory is being used as DRAM by the CPU , the UEFI memory map
> tracks the type of allocation and what areas of memory are free. That usage
> patter is changed by your App running.
>
> > As mentioned, the
> > application does a lot of AllocatePool() FreePool() calls. And these
> calls
> > are, as far as I could understand, creating a lot of entries of type
> "BS_Data"
> > and "Available".
> > Shouldn't the bios allocation routines try to reuse the pools already
> used
> > and freed to avoid massing and fragmenting the memory?
> >
> > Besides that, we just found a system that hangs on the subsequent boot
> > after executing the application. The strange is that the system just
> hangs
> > if you do the following steps:
> >
> > 1 - execute the application: app.efi
> > 2 - exit the shell with the command: exit
> > 3 - boot hangs not presenting the shell prompt
> >
> >
> > In case you do the following steps the hang doesn't happen:
> >
> > 1 - execute the application: app.efi
> > 2 - shut down the system by pressing the power button
> > 3 - boots normally entering at the shell prompt
> >
> > Any idea about what could cause this strange behavior, and if this may
> have
> > some relation with the increase of the memmap output entries?
>
> A common bug is for an Application to not clean up something and have the
> resource get freed. For example the App starts a timer event, forgets to
> stop it and when the App exits the memory gets freed back and if some one
> else allocates that memory they overwrite the code that executes in the
> timer event and kaboom. Same goes for publishing a protocol, etc.
>
> Given the issue is only with your App I'd focus on the App and not the
> delta in the memory map.
>
> On thing that may be helpful is to turn on this property it will cause the
> freed pool to get filled with 0xAF. 0xAFAFAFAFAFAFAFAF will GP fault if it
> is used as a memory address so this helps catch using freed resources
> closer to the source.
> PcdDebugPropertyMask set 

Re: [edk2] [PATCH edk2-platforms 1/3] Platform/Microsoft: Add SdMmc Dxe Driver

[Leif Lindholm]

On Thu, Aug 02, 2018 at 12:05:38AM +, Chris Co wrote:
> > Hmm, I'm already slightly unhappy that we have duplication between
> > EmbeddedPkg and MdeModulePkg (not to mention the extra fork for
> > Marvell's XenonDxe).
> > 
> > I'm obviously keen for the added functionality to be available to more
> > platforms. Is there any way for this support to be added into either the
> > EmbeddedPkg or the MdeModulePkg driver?
> 
> Understood - from talking with the other developers, we duplicated
> since we didn't know if these changes would be desired in edk2 core.
> As part of the review, do let me know if changes would be better
> placed in edk2 core instead of platform.

EDK2 has a history of copy-and-modify rather than refactoring code to
fit multiple cases. I am very strongly pushing for a shift towards the
latter.

Also, since we know there are more consumers than can be found in the
public trees, my threshold for at what point it's useful to add new
functionality in the core is very low.

Particularly for Sd/(e)Mmc, my goal is to integrate the EmbeddedPkg
and MdeModulePkg drivers back together at some point.

> I will be happy to refactor and submit patches to edk2 core.

Thanks!

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

Re: [edk2] [PATCH edk2-platforms 3/3] Platform/Microsoft: Add Lauterbach debug library

[Leif Lindholm]

On Thu, Aug 02, 2018 at 01:27:16AM +, Chris Co wrote:
> > > The 'data.load.elf' statement in
> > > PeCoffLoaderRelocateImageExtraAction() is particular to Lauterbach
> > 
> > Oh, right.
> > 
> > That (original) code badly needs reformatting.
> > 
> > Still, if that's the only difference - and in a debug printout, why fork the
> > module?
> > 
> > If there is a way to identify which debugger is being used, use that.
> > If not, dump all the possible strings.
> 
> Currently I didn't find a way to identify the debugger being used.

Well, it was worth a shot.

> Default behavior of the original code assumes ARM platforms use DS5
> debugger.  I could introduce a PCD or compile flag so the developer
> can indicate DS5 vs Lauterbach debugger and key the debug print off
> of that.

That would certainly be preferable over keeping a separate copy.
If we find a way to determine dynamically, that would be better.

Maybe it could even be worth to have a dynamic pcd and a menu setting,
defaulting to just printing module names and addresses/sizes if no
specific debugger has been selected? (Where the default could be
platform-specific.)

I have some theory about adding information to qemu fw_cfg to let EDK2
know it's being debugged, but clearly that won't help real platforms.

> Regarding dumping all possible strings, I am not sure exactly how
> the DS5 output string is being used.  i.e. does the DS5 software
> receive the serial info directly?  If so, does it expect to see a
> specific format?  Is the software resilient enough that it can
> handle the Lauterbach spew on the same channel?

So, this predates my involvement with edk2 - but I think it was just
added so you could copy/paste the command line needed to get symbols
in your debugger for a specific file.
(I will note here that the DS5 syntax is also suspiciously like GDB
syntax.)

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

Re: [edk2] [PATCH edk2-platforms v1 00/38] Upload for D06 platform

[Leif Lindholm]

On Thu, Aug 02, 2018 at 09:46:13AM +0800, Ming wrote:
> I am sorry for the first issue, the modify FIRMWARE_VER patch is add
> alone just befor send out the patchset.
> 
> For generating acpi table, I use acpica-tools 20180508 version and it works.
> I think acpica-tools are not backward compatibility and confused about 
> acpica-tools.

Yes, it's a real pain. We used to have lots of issues with this, but
the last couple of years have been less bad.
Some platforms moved from .asl to .aslc to get around this.

I can confirm using 20180508 version resolves this issue.
And that 20180629 does not work :(
Unfortunately that won't currently work with our AMD overdrive
platforms. But we'll have to live with that for now.

However, I notice the D02 build is now broken, due to the removal of
Silicon/Hisilicon/Drivers/PciHostBridgeDxe. I guess this is not a huge
issue, and maybe it is time to retire that platform anyway.
If so, could you create a set removing D02 and the bits only used by
it?

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

Re: [edk2] [PATCH] MdeModulePkg: Remove redundant library classes and GUIDs

[Zeng, Star]

Shenglei,

I suggest merging this patch with 
https://lists.01.org/pipermail/edk2-devel/2018-August/027883.html to one patch.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of shenglei
Sent: Thursday, August 2, 2018 4:45 PM
To: edk2-devel@lists.01.org
Cc: Dong, Eric ; Zeng, Star 
Subject: [edk2] [PATCH] MdeModulePkg: Remove redundant library classes and GUIDs

Some redundant library classes and GUIDs have been removed in inf, .c and .h 
files.
https://bugzilla.tianocore.org/show_bug.cgi?id=1044
https://bugzilla.tianocore.org/show_bug.cgi?id=1045
https://bugzilla.tianocore.org/show_bug.cgi?id=1047
https://bugzilla.tianocore.org/show_bug.cgi?id=1049
https://bugzilla.tianocore.org/show_bug.cgi?id=1051
https://bugzilla.tianocore.org/show_bug.cgi?id=1052
https://bugzilla.tianocore.org/show_bug.cgi?id=1053
https://bugzilla.tianocore.org/show_bug.cgi?id=1054
https://bugzilla.tianocore.org/show_bug.cgi?id=1055
https://bugzilla.tianocore.org/show_bug.cgi?id=1056

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 1 -
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 1 -
 .../Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c  | 1 -
 .../SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf| 1 -
 MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf | 1 -
 MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h | 1 -
 MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf   | 1 -
 .../Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf | 3 +--
 .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c   | 2 --
 .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf | 2 --
 MdeModulePkg/Universal/EbcDxe/EbcDebugger.inf  | 2 --
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCommon.h  | 2 --
 MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf | 1 -
 MdeModulePkg/Universal/EsrtDxe/EsrtImpl.h  | 1 -
 MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h | 1 -
 .../Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf  | 1 -
 .../SmmCommunicationBufferDxe/SmmCommunicationBufferDxe.c  | 1 -
 .../SmmCommunicationBufferDxe/SmmCommunicationBufferDxe.inf| 1 -
 18 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index 894da2f2d9..4d907242f3 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
index 3a67c6b909..d1b11318bb 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
@@ -40,7 +40,6 @@
   MdeModulePkg/MdeModulePkg.dec
 
 [Guids]
-  gEfiGlobalVariableGuid ## CONSUMES   ## GUID
   gEfiCapsuleReportGuid  ## CONSUMES   ## GUID
   gEfiFmpCapsuleGuid ## CONSUMES   ## GUID
   gWindowsUxCapsuleGuid  ## CONSUMES   ## GUID
diff --git 
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c 
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
index 96e9977aad..a77164b436 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileIn
+++ fo.c
@@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git 
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf 
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
index 73cc052cc3..cc189fd480 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileIn
+++ fo.inf
@@ -42,7 +42,6 @@
   UefiLib
   PrintLib
   DevicePathLib
-  PeCoffGetEntryPointLib
   DxeServicesLib
 
 [Protocols]
diff --git a/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf 
b/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf
index 2fb1085c6d..c9b87eb50c 100644
--- a/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf
+++ b/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf
@@ -36,7 +36,6 @@
 [LibraryClasses]
   BaseMemoryLib
   DebugLib
-  DevicePathLib
   MemoryAllocationLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
diff --git a/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h
index a2e61d6128..5a83be968f 100644
--- 

[edk2] Tool to check memory leaks

[Udit Kumar]

Hi All
Is there some tool/debug option in edk2 to detect memory leak.
Thanks
Udit
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] MdeModulePkg: Remove redundant library classes and GUIDs

[shenglei]

Some redundant library classes and GUIDs
have been removed in inf, .c and .h files.
https://bugzilla.tianocore.org/show_bug.cgi?id=1044
https://bugzilla.tianocore.org/show_bug.cgi?id=1045
https://bugzilla.tianocore.org/show_bug.cgi?id=1047
https://bugzilla.tianocore.org/show_bug.cgi?id=1049
https://bugzilla.tianocore.org/show_bug.cgi?id=1051
https://bugzilla.tianocore.org/show_bug.cgi?id=1052
https://bugzilla.tianocore.org/show_bug.cgi?id=1053
https://bugzilla.tianocore.org/show_bug.cgi?id=1054
https://bugzilla.tianocore.org/show_bug.cgi?id=1055
https://bugzilla.tianocore.org/show_bug.cgi?id=1056

Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c   | 1 -
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 1 -
 .../Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c  | 1 -
 .../SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf| 1 -
 MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf | 1 -
 MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h | 1 -
 MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf   | 1 -
 .../Universal/Acpi/AcpiPlatformDxe/AcpiPlatformDxe.inf | 3 +--
 .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.c   | 2 --
 .../FirmwarePerformanceDataTableDxe/FirmwarePerformanceDxe.inf | 2 --
 MdeModulePkg/Universal/EbcDxe/EbcDebugger.inf  | 2 --
 MdeModulePkg/Universal/EbcDxe/EbcDebugger/EdbCommon.h  | 2 --
 MdeModulePkg/Universal/EsrtDxe/EsrtDxe.inf | 1 -
 MdeModulePkg/Universal/EsrtDxe/EsrtImpl.h  | 1 -
 MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h | 1 -
 .../Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf  | 1 -
 .../SmmCommunicationBufferDxe/SmmCommunicationBufferDxe.c  | 1 -
 .../SmmCommunicationBufferDxe/SmmCommunicationBufferDxe.inf| 1 -
 18 files changed, 1 insertion(+), 23 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index 894da2f2d9..4d907242f3 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -23,7 +23,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
index 3a67c6b909..d1b11318bb 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
@@ -40,7 +40,6 @@
   MdeModulePkg/MdeModulePkg.dec
 
 [Guids]
-  gEfiGlobalVariableGuid ## CONSUMES   ## GUID
   gEfiCapsuleReportGuid  ## CONSUMES   ## GUID
   gEfiFmpCapsuleGuid ## CONSUMES   ## GUID
   gWindowsUxCapsuleGuid  ## CONSUMES   ## GUID
diff --git 
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c 
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
index 96e9977aad..a77164b436 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c
@@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git 
a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf 
b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
index 73cc052cc3..cc189fd480 100644
--- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
+++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.inf
@@ -42,7 +42,6 @@
   UefiLib
   PrintLib
   DevicePathLib
-  PeCoffGetEntryPointLib
   DxeServicesLib
 
 [Protocols]
diff --git a/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf 
b/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf
index 2fb1085c6d..c9b87eb50c 100644
--- a/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf
+++ b/MdeModulePkg/Bus/I2c/I2cDxe/I2cHostDxe.inf
@@ -36,7 +36,6 @@
 [LibraryClasses]
   BaseMemoryLib
   DebugLib
-  DevicePathLib
   MemoryAllocationLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
diff --git a/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h
index a2e61d6128..5a83be968f 100644
--- a/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.h
@@ -32,7 +32,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf 
b/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf
index 525235635a..900fa01698 100644
--- a/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf
+++ b/MdeModulePkg/Bus/Pci/UfsPciHcDxe/UfsPciHcDxe.inf
@@ -42,7 +42,6 @@
 

Re: [edk2] [PATCH] BaseTools: Guid.xref doesn't specify the correct GUID value for Driver

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Tuesday, July 31, 2018 8:19 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [edk2] [PATCH] BaseTools: Guid.xref doesn't specify the correct GUID 
value for Driver

From: Yunhua Feng 

In DSC, we can define the driver with the different FILE GUID. So, this driver 
name and its FILE GUID should also be listed in Build output Guid.xref. But 
now, Guid.xref still lists the driver MODULE_GUID.

The case in Platform.dsc:
  MdeModulePkg/Universal/DriverSampleDxe/DriverSampleDxe.inf {

  FILE_GUID = 3A4A354F-6935-40fa-B19C-500EEEBF0BC2

  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
  }

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/GenFds/GenFds.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
index a7c1e6c853..156aae1d0e 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -603,19 +603,28 @@ class GenFds :
 GuidXRefFileName = os.path.join(GenFdsGlobalVariable.FvDir, 
"Guid.xref")
 GuidXRefFile = BytesIO('')
 GuidDict = {}
 ModuleList = []
 FileGuidList = []
+GuidPattern = re.compile("\s*([0-9a-fA-F]){8}-"
+   "([0-9a-fA-F]){4}-"
+   "([0-9a-fA-F]){4}-"
+   "([0-9a-fA-F]){4}-"
+   "([0-9a-fA-F]){12}\s*")
 for Arch in ArchList:
 PlatformDataBase = 
BuildDb.BuildObject[GenFdsGlobalVariable.ActivePlatform, Arch, 
GenFdsGlobalVariable.TargetName, GenFdsGlobalVariable.ToolChainTag]
 for ModuleFile in PlatformDataBase.Modules:
 Module = BuildDb.BuildObject[ModuleFile, Arch, 
GenFdsGlobalVariable.TargetName, GenFdsGlobalVariable.ToolChainTag]
 if Module in ModuleList:
 continue
 else:
 ModuleList.append(Module)
-GuidXRefFile.write("%s %s\n" % (Module.Guid, Module.BaseName))
+GuidMatch = GuidPattern.match(ModuleFile.BaseName)
+if GuidMatch is not None:
+GuidXRefFile.write("%s %s\n" % (ModuleFile.BaseName, 
Module.BaseName))
+else:
+GuidXRefFile.write("%s %s\n" % (Module.Guid, 
+ Module.BaseName))
 for key, item in Module.Protocols.items():
 GuidDict[key] = item
 for key, item in Module.Guids.items():
 GuidDict[key] = item
 for key, item in Module.Ppis.items():
--
2.12.2.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