[edk2] Communicate with soft-smi-handler with uefi-application question.

[krishnaLee]

Hi,
I'm learning  to write and register some soft-Smi-Handler in smm-mode;
then using QEMU to boot my ovmf.fd,run into uefi shell;
then write uefi-application using EFI_SMM_COMMUNICATION_PROTOCOL to Communicate 
to my Smi-Handler,but failed when run my uefi-application,the log show error.
I don't know why,maybe I do not full understand uefi-smm,but how to communicate 
to my smi handler?


//error-message
[Security] 3rd party image[0] can be loaded after EndOfDxe: 
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x0,0x,0x0)/HD(1,MBR,0xBE1AFDFA,0x3F,0xFBFC1)/\mytestsmm.efi.
InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 65FC4A8
Loading driver at 0x62B EntryPoint=0x62B10F5 mytestsmm.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 65FCB10
ProtectUefiImageCommon - 0x65FC4A8
  - 0x062B - 0x7000
InstallProtocolInterface: 752F3136-4E16-4FDC-A22A-E5F46812F4CA 7700CFC
InstallProtocolInterface: 4C8A2451-C207-405B-9694-99EA13251341 62B40B0
Locate EfiSmmCommunicationProtocol success
SmmIsBufferOutsideSmmValid: Not in ValidCommunicationRegion: Buffer (0x7700C94) 
- Length (0x2A), ASSERT [PiSmmCore] 
d:\edk2-vudk2017\MdePkg\Library\SmmMemLib\SmmMemLib.c(178): ((BOOLEAN)(0==1))
//error-message-end


//my register-smi-handler code:
//edk2-vUDK2017\MdeModulePkg\Universal\LockBox\SmmLockBox\SmmLockBox.c

EFI_STATUS

EFIAPI

MyTestSmmHandler (

IN EFI_HANDLE DispatchHandle,

IN CONST VOID *Context OPTIONAL,

IN OUT VOID *CommBuffer OPTIONAL,

IN OUT UINTN *CommBufferSize OPTIONAL

)

{

DEBUG ((DEBUG_INFO, "My Test Smm Handler Enter\n"));

DEBUG ((DEBUG_INFO, "My Test Smm Handler exit\n"));

return EFI_SUCCESS;

}




EFI_STATUS

EFIAPI

SmmLockBoxEntryPoint (

IN EFI_HANDLE ImageHandle,

IN EFI_SYSTEM_TABLE *SystemTable

)

{…

//Register My Test Smm handler

Status = gSmst->SmiHandlerRegister (

MyTestSmmHandler,

,



);

ASSERT_EFI_ERROR (Status);

…

}



//the uefi-application code
EFI_STATUS
EFIAPI
UefiMain (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
EFI_STATUS Status;
EFI_SMM_COMMUNICATION_PROTOCOL *mSmmCommunication;
EFI_SMM_COMMUNICATE_HEADER *SmmCommunicateHeader;
UINT8 *buffer;
UINTN bufferSize;


bufferSize=sizeof(EFI_SMM_COMMUNICATE_HEADER)*2;
gBS->AllocatePool (EfiRuntimeServicesData,bufferSize,);
if(buffer==NULL)
{
Print(L"EFI_OUT_OF_RESOURCES, return\n");
return EFI_OUT_OF_RESOURCES;
}


SmmCommunicateHeader=(EFI_SMM_COMMUNICATE_HEADER*)buffer;
CopyGuid(>HeaderGuid,);
SmmCommunicateHeader->MessageLength=sizeof(EFI_SMM_COMMUNICATE_HEADER);


Status = gBS->LocateProtocol (, NULL, (VOID 
**) );
if(Status==EFI_SUCCESS)
{
Print(L"Locate EfiSmmCommunicationProtocol success\n");
}else
{
Print(L"Locate EfiSmmCommunicationProtocol failed return\n");
return EFI_SUCCESS;
}


Status=mSmmCommunication->Communicate(mSmmCommunication,,);
if(Status==EFI_SUCCESS)
{
Print(L"Communication success\n");
}else
{
Print(L"Communication failed\n");
return EFI_SUCCESS;
}


gBS->FreePool(buffer);


return EFI_SUCCESS;
}


any help will be appreciated!
by krishna

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

[edk2] [PATCH] EmbeddedPkg/FdtLib: build fdt_empty_tree.c

[Michael Zimmermann]

We're currently building everything except for this file.
Since 'fdt_create_empty_tree' can be useful for some
platforms, compile this one too.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann 
---
 EmbeddedPkg/Library/FdtLib/FdtLib.inf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/EmbeddedPkg/Library/FdtLib/FdtLib.inf 
b/EmbeddedPkg/Library/FdtLib/FdtLib.inf
index b26d37422da2..7aa44a302d89 100644
--- a/EmbeddedPkg/Library/FdtLib/FdtLib.inf
+++ b/EmbeddedPkg/Library/FdtLib/FdtLib.inf
@@ -26,6 +26,7 @@
 #
 
 [Sources]
+  fdt_empty_tree.c
   fdt_ro.c
   fdt_rw.c
   fdt_strerror.c
-- 
2.15.1

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

[edk2] [PATCH] ArmPkg/Library/ArmLib: add ArmWriteSctlr

[Michael Zimmermann]

This currently isn't needed by anything in the edk2 tree but
it's useful for externally maintained platforms which have
to set this register e.g. to disable alignment aborts.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael Zimmermann 
---
 ArmPkg/Include/Library/ArmLib.h | 6 ++
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S   | 4 
 ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 24e84c7a1965..ffda50e9d767 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -558,6 +558,12 @@ ArmReadSctlr (
   VOID
   );
 
+VOID
+EFIAPI
+ArmWriteSctlr (
+  IN  UINT32   Value
+  );
+
 UINTN
 EFIAPI
 ArmReadHVBar (
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S 
b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
index a0b5ed500298..149b57e059ee 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
+++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
@@ -153,6 +153,10 @@ ASM_FUNC(ArmReadSctlr)
   mrc p15, 0, r0, c1, c0, 0  @ Read SCTLR into R0 (Read control 
register configuration data)
   bx  lr
 
+ASM_FUNC(ArmWriteSctlr)
+  mcr p15, 0, r0, c1, c0, 0
+  bx  lr
+
 ASM_FUNC(ArmReadCpuActlr)
   mrc p15, 0, r0, c1, c0, 1
   bx  lr
diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm 
b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
index 85b0feee20d4..219140c22b13 100644
--- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
+++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
@@ -155,6 +155,9 @@
   mrc p15, 0, r0, c1, c0, 0  // Read SCTLR into R0 (Read control 
register configuration data)
   bx  lr
 
+ RVCT_ASM_EXPORT ArmWriteSctlr
+  mcr p15, 0, r0, c1, c0, 0
+  bx  lr
 
  RVCT_ASM_EXPORT ArmReadCpuActlr
   mrc p15, 0, r0, c1, c0, 1
-- 
2.15.1

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

Re: [edk2] [Patch 2/2] MdeModulePkg SectionExtractionPei: remove the hard code alignment adjustment

[Zeng, Star]

Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, January 10, 2018 1:34 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: [edk2] [Patch 2/2] MdeModulePkg SectionExtractionPei: remove the hard 
code alignment adjustment

Section data alignment should be made in the build generation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 .../Universal/SectionExtractionPei/SectionExtractionPei.c| 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c 
b/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c
index c4a3508..63fc94c 100644
--- a/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c
+++ b/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c
@@ -1,7 +1,7 @@
 /** @file
  Section Extraction PEIM
 
-Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  
 are licensed and made available under the terms and conditions of the BSD 
License 
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -194,16 +194,11 @@ CustomGuidedSectionExtract (
 //
 // Allocate output buffer
 //
-*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize) + 1);
+*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize));
 if (*OutputBuffer == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
 DEBUG ((DEBUG_INFO, "Customized Guided section Memory Size required is 
0x%x and address is 0x%p\n", OutputBufferSize, *OutputBuffer));
-//
-// *OutputBuffer still is one section. Adjust *OutputBuffer offset, 
-// skip EFI section header to make section data at page alignment.
-//
-*OutputBuffer = (VOID *)((UINT8 *) *OutputBuffer + EFI_PAGE_SIZE - sizeof 
(EFI_COMMON_SECTION_HEADER));
   }
   
   Status = ExtractGuidedSectionDecode (
-- 
2.8.0.windows.1

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

Re: [edk2] [Patch] MdeModulePkg DxeIpl: remove the hard code alignment adjustment in Decompress()

[Zeng, Star]

Reviewed-by: Star Zeng 


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, January 10, 2018 1:46 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: [edk2] [Patch] MdeModulePkg DxeIpl: remove the hard code alignment 
adjustment in Decompress()

Section data alignment should be made in the build generation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c 
b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index f4d7528..178bac4 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -708,18 +708,13 @@ Decompress (
 return EFI_OUT_OF_RESOURCES;
   }
   //
-  // Allocate destination buffer, extra one page for adjustment 
+  // Allocate destination buffer
   //
-  DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize) + 1);
+  DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
   if (DstBuffer == NULL) {
 return EFI_OUT_OF_RESOURCES;
   }
   //
-  // DstBuffer still is one section. Adjust DstBuffer offset, skip EFI 
section header
-  // to make section data at page alignment.
-  //
-  DstBuffer = DstBuffer + EFI_PAGE_SIZE - sizeof 
(EFI_COMMON_SECTION_HEADER);
-  //
   // Call decompress function
   //
   Status = UefiDecompress (
@@ -749,16 +744,11 @@ Decompress (
 // Allocate destination buffer
 //
 DstBufferSize = UncompressedLength;
-DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize) + 1);
+DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
 if (DstBuffer == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
 //
-// Adjust DstBuffer offset, skip EFI section header
-// to make section data at page alignment.
-//
-DstBuffer = DstBuffer + EFI_PAGE_SIZE - sizeof (EFI_COMMON_SECTION_HEADER);
-//
 // stream is not actually compressed, just encapsulated.  So just copy it.
 //
 CopyMem (DstBuffer, CompressionSource, DstBufferSize);
-- 
2.8.0.windows.1

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

Re: [edk2] [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment adjustment.

[Zeng, Star]

Ok, Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Gao, Liming 
Sent: Wednesday, January 10, 2018 1:40 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Subject: RE: [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment 
adjustment.

Star:
  Right. I will send the separate patch for it. 

>-Original Message-
>From: Zeng, Star
>Sent: Wednesday, January 10, 2018 1:38 PM
>To: Gao, Liming ; edk2-devel@lists.01.org
>Cc: Zeng, Star 
>Subject: RE: [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code 
>alignment adjustment.
>
>Liming,
>
>Similar change should be also done in Decompress() of DxeLoad.c, right?
>
>
>Thanks,
>Star
>-Original Message-
>From: Gao, Liming
>Sent: Wednesday, January 10, 2018 1:33 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star 
>Subject: [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code 
>alignment adjustment.
>
>Section data alignment should be made in the build generation.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Liming Gao 
>Cc: Star Zeng 
>---
> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
>diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>index 1f626a9..f4d7528 100644
>--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>@@ -3,7 +3,7 @@
>   Responsibility of this module is to load the DXE Core from a 
>Firmware Volume.
>
> Copyright (c) 2016 HP Development Company, L.P.
>-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials  are licensed and made 
>available under the terms and conditions of the BSD License  which 
>accompanies this distribution.  The full text of the license may be 
>found at @@ -593,16 +593,11 @@ CustomGuidedSectionExtract (
> //
> // Allocate output buffer
> //
>-*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize) +
>1);
>+*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES 
>+ (OutputBufferSize));
> if (*OutputBuffer == NULL) {
>   return EFI_OUT_OF_RESOURCES;
> }
> DEBUG ((DEBUG_INFO, "Customized Guided section Memory Size 
>required is 0x%x and address is 0x%p\n", OutputBufferSize, *OutputBuffer));
>-//
>-// *OutputBuffer still is one section. Adjust *OutputBuffer offset,
>-// skip EFI section header to make section data at page alignment.
>-//
>-*OutputBuffer = (VOID *)((UINT8 *) *OutputBuffer + EFI_PAGE_SIZE -
>sizeof (EFI_COMMON_SECTION_HEADER));
>   }
>
>   Status = ExtractGuidedSectionDecode (
>--
>2.8.0.windows.1

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

[edk2] [Patch] MdeModulePkg DxeIpl: remove the hard code alignment adjustment in Decompress()

[Liming Gao]

Section data alignment should be made in the build generation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 16 +++-
 1 file changed, 3 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c 
b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index f4d7528..178bac4 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -708,18 +708,13 @@ Decompress (
 return EFI_OUT_OF_RESOURCES;
   }
   //
-  // Allocate destination buffer, extra one page for adjustment 
+  // Allocate destination buffer
   //
-  DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize) + 1);
+  DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
   if (DstBuffer == NULL) {
 return EFI_OUT_OF_RESOURCES;
   }
   //
-  // DstBuffer still is one section. Adjust DstBuffer offset, skip EFI 
section header
-  // to make section data at page alignment.
-  //
-  DstBuffer = DstBuffer + EFI_PAGE_SIZE - sizeof 
(EFI_COMMON_SECTION_HEADER);
-  //
   // Call decompress function
   //
   Status = UefiDecompress (
@@ -749,16 +744,11 @@ Decompress (
 // Allocate destination buffer
 //
 DstBufferSize = UncompressedLength;
-DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize) + 1);
+DstBuffer = AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
 if (DstBuffer == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
 //
-// Adjust DstBuffer offset, skip EFI section header
-// to make section data at page alignment.
-//
-DstBuffer = DstBuffer + EFI_PAGE_SIZE - sizeof (EFI_COMMON_SECTION_HEADER);
-//
 // stream is not actually compressed, just encapsulated.  So just copy it.
 //
 CopyMem (DstBuffer, CompressionSource, DstBufferSize);
-- 
2.8.0.windows.1

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

[edk2] [PATCH 2/2] UefiCpuPkg/MtrrLib: Fix an assertion bug

[Ruiyu Ni]

0 40  f0 100
+---WT--+--UC--+--WT--+-WB+UC+

When calculating the shortest path from 0 to 100, the
MtrrLibCalculateLeastMtrrs() is called to update the
Vertices.Previous.
When calculating the shortest path from 0 to 40,
MtrrLibCalculateLeastMtrrs() is called recursively to update the
Vertices.Previous.
The second call corrupt the Previous value that will be used
later.
The patch removes the code that corrupts Previous.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Star Zeng 
Cc: Eric Dong 
---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 768d4d5cff..30b0df030b 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -1171,7 +1171,6 @@ MtrrLibCalculateLeastMtrrs (
 
   for (Index = Start; Index <= Stop; Index++) {
 Vertices[Index].Visited = FALSE;
-Vertices[Index].Previous = VertexCount;
 Mandatory = Weight[M(Start,Index)];
 Vertices[Index].Weight = Mandatory;
 if (Mandatory != MAX_WEIGHT) {
-- 
2.15.1.windows.2

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

[edk2] [PATCH 1/2] UefiCpuPkg/MtrrLib: Fix a MTRR calculation bug

[Ruiyu Ni]

80   A8   B0   B8   C0
+--WB+-UC-+-WT-+-WB-+

For above memory settings, current code caused the final MTRR
settings miss [A8, B0, UC] when default memory type is UC.

The root cause is the code only checks the mandatory weight
between A8 to B0, but skips to check the optional weight.
The patch fixes this issue.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Star Zeng 
---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index fafa15fd63..768d4d5cff 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -47,7 +47,7 @@ typedef struct {
   UINT64 Address;
   UINT64 Alignment;
   UINT64 Length;
-  UINT8  Type : 7;
+  MTRR_MEMORY_CACHE_TYPE Type : 7;
 
   //
   // Temprary use for calculating the best MTRR settings.
@@ -1429,7 +1429,7 @@ MtrrLibCalculateSubtractivePath (
   while (SubStart != SubStop) {
 Status = MtrrLibAppendVariableMtrr (
   Mtrrs, MtrrCapacity, MtrrCount,
-  Vertices[SubStart].Address, Vertices[SubStart].Length, 
(MTRR_MEMORY_CACHE_TYPE) Vertices[SubStart].Type
+  Vertices[SubStart].Address, Vertices[SubStart].Length, 
Vertices[SubStart].Type
 );
 if (RETURN_ERROR (Status)) {
   return Status;
@@ -1450,10 +1450,11 @@ MtrrLibCalculateSubtractivePath (
 Pre = Vertices[Cur].Previous;
 SubStop = Pre;
 
-if (Weight[M (Pre, Cur)] != 0) {
+if (Weight[M (Pre, Cur)] + Weight[O (Pre, Cur)] != 0) {
   Status = MtrrLibAppendVariableMtrr (
 Mtrrs, MtrrCapacity, MtrrCount,
-Vertices[Pre].Address, Vertices[Cur].Address - 
Vertices[Pre].Address, LowestPrecedentType
+Vertices[Pre].Address, Vertices[Cur].Address - 
Vertices[Pre].Address,
+(Pre != Cur - 1) ? LowestPrecedentType : Vertices[Pre].Type
   );
   if (RETURN_ERROR (Status)) {
 return Status;
-- 
2.15.1.windows.2

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

[edk2] [PATCH 0/2] Quality improvement of MtrrLib

[Ruiyu Ni]

The patch set fixes two bugs in MtrrLib.
With the two fixes, randomly generated 1000 memory settings
can be set correctly.

Ruiyu Ni (2):
  UefiCpuPkg/MtrrLib: Fix a MTRR calculation bug
  UefiCpuPkg/MtrrLib: Fix an assertion bug

 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
2.15.1.windows.2

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

Re: [edk2] [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment adjustment.

[Gao, Liming]

Star:
  Right. I will send the separate patch for it. 

>-Original Message-
>From: Zeng, Star
>Sent: Wednesday, January 10, 2018 1:38 PM
>To: Gao, Liming ; edk2-devel@lists.01.org
>Cc: Zeng, Star 
>Subject: RE: [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code
>alignment adjustment.
>
>Liming,
>
>Similar change should be also done in Decompress() of DxeLoad.c, right?
>
>
>Thanks,
>Star
>-Original Message-
>From: Gao, Liming
>Sent: Wednesday, January 10, 2018 1:33 PM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star 
>Subject: [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment
>adjustment.
>
>Section data alignment should be made in the build generation.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Liming Gao 
>Cc: Star Zeng 
>---
> MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 9 ++---
> 1 file changed, 2 insertions(+), 7 deletions(-)
>
>diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>index 1f626a9..f4d7528 100644
>--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
>@@ -3,7 +3,7 @@
>   Responsibility of this module is to load the DXE Core from a Firmware
>Volume.
>
> Copyright (c) 2016 HP Development Company, L.P.
>-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
>+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials  are licensed and made
>available under the terms and conditions of the BSD License  which
>accompanies this distribution.  The full text of the license may be found at
>@@ -593,16 +593,11 @@ CustomGuidedSectionExtract (
> //
> // Allocate output buffer
> //
>-*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize) +
>1);
>+*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES
>+ (OutputBufferSize));
> if (*OutputBuffer == NULL) {
>   return EFI_OUT_OF_RESOURCES;
> }
> DEBUG ((DEBUG_INFO, "Customized Guided section Memory Size required
>is 0x%x and address is 0x%p\n", OutputBufferSize, *OutputBuffer));
>-//
>-// *OutputBuffer still is one section. Adjust *OutputBuffer offset,
>-// skip EFI section header to make section data at page alignment.
>-//
>-*OutputBuffer = (VOID *)((UINT8 *) *OutputBuffer + EFI_PAGE_SIZE -
>sizeof (EFI_COMMON_SECTION_HEADER));
>   }
>
>   Status = ExtractGuidedSectionDecode (
>--
>2.8.0.windows.1

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

Re: [edk2] [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment adjustment.

[Zeng, Star]

Liming,

Similar change should be also done in Decompress() of DxeLoad.c, right?


Thanks,
Star
-Original Message-
From: Gao, Liming 
Sent: Wednesday, January 10, 2018 1:33 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star 
Subject: [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment 
adjustment.

Section data alignment should be made in the build generation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c 
b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index 1f626a9..f4d7528 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -3,7 +3,7 @@
   Responsibility of this module is to load the DXE Core from a Firmware Volume.
 
 Copyright (c) 2016 HP Development Company, L.P.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -593,16 +593,11 
@@ CustomGuidedSectionExtract (
 //
 // Allocate output buffer
 //
-*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize) + 1);
+*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES 
+ (OutputBufferSize));
 if (*OutputBuffer == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
 DEBUG ((DEBUG_INFO, "Customized Guided section Memory Size required is 
0x%x and address is 0x%p\n", OutputBufferSize, *OutputBuffer));
-//
-// *OutputBuffer still is one section. Adjust *OutputBuffer offset, 
-// skip EFI section header to make section data at page alignment.
-//
-*OutputBuffer = (VOID *)((UINT8 *) *OutputBuffer + EFI_PAGE_SIZE - sizeof 
(EFI_COMMON_SECTION_HEADER));
   }
   
   Status = ExtractGuidedSectionDecode (
--
2.8.0.windows.1

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

[edk2] [Patch 0/2] MdeModulePkg: Remove the hard code alignment adjustment.

[Liming Gao]

Liming Gao (2):
  MdeModulePkg DxeIpl: remove the hard code alignment adjustment.
  MdeModulePkg SectionExtractionPei: remove the hard code alignment
adjustment

 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c   | 9 ++---
 .../Universal/SectionExtractionPei/SectionExtractionPei.c| 9 ++---
 2 files changed, 4 insertions(+), 14 deletions(-)

-- 
2.8.0.windows.1

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

[edk2] [Patch 2/2] MdeModulePkg SectionExtractionPei: remove the hard code alignment adjustment

[Liming Gao]

Section data alignment should be made in the build generation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 .../Universal/SectionExtractionPei/SectionExtractionPei.c| 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c 
b/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c
index c4a3508..63fc94c 100644
--- a/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c
+++ b/MdeModulePkg/Universal/SectionExtractionPei/SectionExtractionPei.c
@@ -1,7 +1,7 @@
 /** @file
  Section Extraction PEIM
 
-Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2013 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials  
 are licensed and made available under the terms and conditions of the BSD 
License 
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -194,16 +194,11 @@ CustomGuidedSectionExtract (
 //
 // Allocate output buffer
 //
-*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize) + 1);
+*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize));
 if (*OutputBuffer == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
 DEBUG ((DEBUG_INFO, "Customized Guided section Memory Size required is 
0x%x and address is 0x%p\n", OutputBufferSize, *OutputBuffer));
-//
-// *OutputBuffer still is one section. Adjust *OutputBuffer offset, 
-// skip EFI section header to make section data at page alignment.
-//
-*OutputBuffer = (VOID *)((UINT8 *) *OutputBuffer + EFI_PAGE_SIZE - sizeof 
(EFI_COMMON_SECTION_HEADER));
   }
   
   Status = ExtractGuidedSectionDecode (
-- 
2.8.0.windows.1

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

[edk2] [Patch 1/2] MdeModulePkg DxeIpl: remove the hard code alignment adjustment.

[Liming Gao]

Section data alignment should be made in the build generation.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Star Zeng 
---
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c 
b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
index 1f626a9..f4d7528 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c
@@ -3,7 +3,7 @@
   Responsibility of this module is to load the DXE Core from a Firmware Volume.
 
 Copyright (c) 2016 HP Development Company, L.P.
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -593,16 +593,11 @@ CustomGuidedSectionExtract (
 //
 // Allocate output buffer
 //
-*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize) + 1);
+*OutputBuffer = AllocatePages (EFI_SIZE_TO_PAGES (OutputBufferSize));
 if (*OutputBuffer == NULL) {
   return EFI_OUT_OF_RESOURCES;
 }
 DEBUG ((DEBUG_INFO, "Customized Guided section Memory Size required is 
0x%x and address is 0x%p\n", OutputBufferSize, *OutputBuffer));
-//
-// *OutputBuffer still is one section. Adjust *OutputBuffer offset, 
-// skip EFI section header to make section data at page alignment.
-//
-*OutputBuffer = (VOID *)((UINT8 *) *OutputBuffer + EFI_PAGE_SIZE - sizeof 
(EFI_COMMON_SECTION_HEADER));
   }
   
   Status = ExtractGuidedSectionDecode (
-- 
2.8.0.windows.1

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

Re: [edk2] [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.

[Wang, Fan]

Thanks, Ray, will revise the title.

Best Regards
Fan

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ni, Ruiyu
Sent: Wednesday, January 10, 2018 12:51 PM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] [Patch 2/2] MdeModulePkg: Did some code enhancement for 
DxeIpIpLib.

Patch title: DxeIpIoLib

On 1/10/2018 11:16 AM, Wang Fan wrote:
> * In DxeIpIo, there are several places use ASSERT() to check input
>parameters without and descriptions or error handling. This patch
>fixed this issue.
> * Fixed some incorrect descriptions in code commence.
> * Remove unneeded Exit tag in function IpIoOpen and IpIoConfigIp.
> * Add EFIAPI tag for function IpIoRefreshNeighbor.
> 
> Cc: Jiaxin Wu 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>   MdeModulePkg/Include/Library/IpIoLib.h   | 21 +--
>   MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 52 
> ++--
>   2 files changed, 52 insertions(+), 21 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/IpIoLib.h 
> b/MdeModulePkg/Include/Library/IpIoLib.h
> index 463bf95..61653b0 100644
> --- a/MdeModulePkg/Include/Library/IpIoLib.h
> +++ b/MdeModulePkg/Include/Library/IpIoLib.h
> @@ -308,10 +308,12 @@ typedef struct _IP_IO_IP_INFO {
> UINT8 IpVersion;
>   } IP_IO_IP_INFO;
>   
>   /**
> Create a new IP_IO instance.
> +
> +  If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
> 
> This function uses IP4/IP6 service binding protocol in Controller to 
> create
> an IP4/IP6 child (aka IP4/IP6 instance).
>   
> @param[in]  Image The image handle of the driver or 
> application that
> @@ -351,10 +353,12 @@ IpIoDestroy (
> IN OUT IP_IO *IpIo
> );
>   
>   /**
> Stop an IP_IO instance.
> +
> +  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
> 
> This function is paired with IpIoOpen(). The IP_IO will be unconfigured, 
> and all
> pending send/receive tokens will be canceled.
>   
> @param[in, out]  IpIoThe pointer to the IP_IO instance that 
> needs to stop.
> @@ -370,11 +374,13 @@ IpIoStop (
> IN OUT IP_IO *IpIo
> );
>   
>   /**
> Open an IP_IO instance for use.
> -
> +
> +  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
> +
> This function is called after IpIoCreate(). It is used for configuring 
> the IP
> instance and register the callbacks and their context data for sending and
> receiving IP packets.
>   
> @param[in, out]  IpIo   The pointer to an IP_IO instance that 
> needs
> @@ -399,11 +405,11 @@ IpIoOpen (
> );
>   
>   /**
> Send out an IP packet.
> 
> -  This function is called after IpIoOpen(). The data to be sent are 
> wrapped in
> +  This function is called after IpIoOpen(). The data to be sent is 
> + wrapped in
> Pkt. The IP instance wrapped in IpIo is used for sending by default but 
> can be
> overriden by Sender. Other sending configs, like source address and 
> gateway
> address etc., are specified in OverrideData.
>   
> @param[in, out]  IpIo  Pointer to an IP_IO instance used 
> for sending IP
> @@ -437,10 +443,13 @@ IpIoSend (
> );
>   
>   /**
> Cancel the IP transmit token that wraps this Packet.
>   
> +  If IpIo is NULL, then ASSERT().
> +  If Packet is NULL, then ASSERT().
> +
> @param[in]  IpIo  The pointer to the IP_IO instance.
> @param[in]  PacketThe pointer to the packet of NET_BUF to 
> cancel.
>   
>   **/
>   VOID
> @@ -450,10 +459,13 @@ IpIoCancelTxToken (
> IN VOID   *Packet
> );
>   
>   /**
> Add a new IP instance for sending data.
> +
> +  If IpIo is NULL, then ASSERT().
> +  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
> 
> The function is used to add the IP_IO to the IP_IO sending list. The 
> caller
> can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to 
> send
> data.
>   
> @@ -471,10 +483,12 @@ IpIoAddIp (
>   
>   /**
> Configure the IP instance of this IpInfo and start the receiving if 
> IpConfigData
> is not NULL.
>   
> +  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
> +
> @param[in, out]  IpInfo  The pointer to the IP_IO_IP_INFO 
> instance.
> @param[in, out]  IpConfigDataThe IP4 or IP6 configure data used to 
> configure
>  the IP instance. If NULL, the IP 
> instance is reset.
>  If UseDefaultAddress is set to TRUE, and 
> the configure
>  operation succeeds, the default 
> address information @@ -493,10 +507,12 @@ IpIoConfigIp (
> );
>   
>   /**
> Destroy an IP 

Re: [edk2] [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.

[Ni, Ruiyu]

Patch title: DxeIpIoLib

On 1/10/2018 11:16 AM, Wang Fan wrote:

* In DxeIpIo, there are several places use ASSERT() to check input
   parameters without and descriptions or error handling. This patch
   fixed this issue.
* Fixed some incorrect descriptions in code commence.
* Remove unneeded Exit tag in function IpIoOpen and IpIoConfigIp.
* Add EFIAPI tag for function IpIoRefreshNeighbor.

Cc: Jiaxin Wu 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
  MdeModulePkg/Include/Library/IpIoLib.h   | 21 +--
  MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 52 ++--
  2 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Include/Library/IpIoLib.h 
b/MdeModulePkg/Include/Library/IpIoLib.h
index 463bf95..61653b0 100644
--- a/MdeModulePkg/Include/Library/IpIoLib.h
+++ b/MdeModulePkg/Include/Library/IpIoLib.h
@@ -308,10 +308,12 @@ typedef struct _IP_IO_IP_INFO {
UINT8 IpVersion;
  } IP_IO_IP_INFO;
  
  /**

Create a new IP_IO instance.
+
+  If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().

This function uses IP4/IP6 service binding protocol in Controller to create

an IP4/IP6 child (aka IP4/IP6 instance).
  
@param[in]  Image The image handle of the driver or application that

@@ -351,10 +353,12 @@ IpIoDestroy (
IN OUT IP_IO *IpIo
);
  
  /**

Stop an IP_IO instance.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().

This function is paired with IpIoOpen(). The IP_IO will be unconfigured, and all

pending send/receive tokens will be canceled.
  
@param[in, out]  IpIoThe pointer to the IP_IO instance that needs to stop.

@@ -370,11 +374,13 @@ IpIoStop (
IN OUT IP_IO *IpIo
);
  
  /**

Open an IP_IO instance for use.
-
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
This function is called after IpIoCreate(). It is used for configuring the 
IP
instance and register the callbacks and their context data for sending and
receiving IP packets.
  
@param[in, out]  IpIo   The pointer to an IP_IO instance that needs

@@ -399,11 +405,11 @@ IpIoOpen (
);
  
  /**

Send out an IP packet.

-  This function is called after IpIoOpen(). The data to be sent are wrapped in

+  This function is called after IpIoOpen(). The data to be sent is wrapped in
Pkt. The IP instance wrapped in IpIo is used for sending by default but can 
be
overriden by Sender. Other sending configs, like source address and gateway
address etc., are specified in OverrideData.
  
@param[in, out]  IpIo  Pointer to an IP_IO instance used for sending IP

@@ -437,10 +443,13 @@ IpIoSend (
);
  
  /**

Cancel the IP transmit token that wraps this Packet.
  
+  If IpIo is NULL, then ASSERT().

+  If Packet is NULL, then ASSERT().
+
@param[in]  IpIo  The pointer to the IP_IO instance.
@param[in]  PacketThe pointer to the packet of NET_BUF to 
cancel.
  
  **/

  VOID
@@ -450,10 +459,13 @@ IpIoCancelTxToken (
IN VOID   *Packet
);
  
  /**

Add a new IP instance for sending data.
+
+  If IpIo is NULL, then ASSERT().
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().

The function is used to add the IP_IO to the IP_IO sending list. The caller

can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to send
data.
  
@@ -471,10 +483,12 @@ IpIoAddIp (
  
  /**

Configure the IP instance of this IpInfo and start the receiving if 
IpConfigData
is not NULL.
  
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().

+
@param[in, out]  IpInfo  The pointer to the IP_IO_IP_INFO instance.
@param[in, out]  IpConfigDataThe IP4 or IP6 configure data used to 
configure
 the IP instance. If NULL, the IP instance 
is reset.
 If UseDefaultAddress is set to TRUE, and 
the configure
 operation succeeds, the default address 
information
@@ -493,10 +507,12 @@ IpIoConfigIp (
);
  
  /**

Destroy an IP instance maintained in IpIo->IpList for
sending purpose.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().

This function pairs with IpIoAddIp(). The IpInfo is previously created by

IpIoAddIp(). The IP_IO_IP_INFO::RefCnt is decremented and the IP instance
will be dstroyed if the RefCnt is zero.
  
@@ -584,10 +600,11 @@ IpIoGetIcmpErrStatus (

@retval  EFI_UNSUPPORTED   IP version is IPv4, which doesn't 
support neighbor cache refresh.
@retval  EFI_OUT_OF_RESOURCES  Failed due to resource limitations.
  
  **/

  EFI_STATUS
+EFIAPI
  

Re: [edk2] [PATCH v3 3/3] MdePkg: add NORETURN attribute to LongJump and InternalLongJump

[Gao, Liming]

Michael:
  Just push the change on RETURNS_TWICE

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Michael Zimmermann
> Sent: Monday, January 8, 2018 2:22 PM
> To: Gao, Liming 
> Cc: Kinney, Michael D ; edk2-devel@lists.01.org; 
> Ard Biesheuvel 
> Subject: Re: [edk2] [PATCH v3 3/3] MdePkg: add NORETURN attribute to LongJump 
> and InternalLongJump
> 
> Liming:
> It's not required to fix the bug which generates bad code. The NORETURN
> attribute just fixes the compiler warning mentioned in the commit message
> so I'm fine with separating this patch from the series.
> 
> And actually this "unreachable code" warning sounds like a more complicated
> problem because while the compiler is right and the code after the call to
> LongJump should never run there is a slight possibility for that to happen
> anyway - just imagine that because of memory corruption the PC/LR inside
> the jump buffer point to after the call of longjump.
> 
> On Mon, Jan 8, 2018 at 6:34 AM, Gao, Liming  wrote:
> 
> > Michael:
> >   After more test, this patch will cause MdeModulePkg DxeCore link failure
> > on VS2015x86 tool chain. The warning message is caused by NORETURN
> > attribute in LongJump() API. Is this patch really required? Could we
> > separate it from the change RETURNS_TWICE? If yes, I suggest to commit
> > RETURNS_TWICE patch first, then work out the solution on NORETURN attribute
> > enabling.
> >
> > c:\edk2\mdemodulepkg\core\dxe\image\image.c(1863) : error C2220: warning
> > treated as error - no 'executable' file generated
> > c:\edk2\mdemodulepkg\core\dxe\image\image.c(1863) : warning C4702:
> > unreachable code
> > c:\edk2\mdemodulepkg\core\dxe\image\image.c(1864) : warning C4702:
> > unreachable code
> >
> > >-Original Message-
> > >From: M1cha [mailto:sigmaepsilo...@gmail.com]
> > >Sent: Thursday, December 28, 2017 3:29 AM
> > >To: edk2-devel@lists.01.org
> > >Cc: Ard Biesheuvel ; Kinney, Michael D
> > >; Gao, Liming 
> > >Subject: [edk2] [PATCH v3 3/3] MdePkg: add NORETURN attribute to
> > >LongJump and InternalLongJump
> > >
> > >This fixes compiler warnings when using them in functions which
> > >should return a value but rely on LongJump to never return instead.
> > >
> > >Contributed-under: TianoCore Contribution Agreement 1.1
> > >Signed-off-by: Michael Zimmermann 
> > >---
> > >V3: fix VS compilation errors
> > >
> > > MdePkg/Include/Library/BaseLib.h | 1 +
> > > MdePkg/Library/BaseLib/BaseLibInternals.h| 1 +
> > > MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c | 1 +
> > > MdePkg/Library/BaseLib/Ia32/LongJump.c   | 1 +
> > > MdePkg/Library/BaseLib/LongJump.c| 1 +
> > > 5 files changed, 5 insertions(+)
> > >
> > >diff --git a/MdePkg/Include/Library/BaseLib.h
> > >b/MdePkg/Include/Library/BaseLib.h
> > >index 10976032adaa..c6783f4624e2 100644
> > >--- a/MdePkg/Include/Library/BaseLib.h
> > >+++ b/MdePkg/Include/Library/BaseLib.h
> > >@@ -4927,6 +4927,7 @@ SetJump (
> > >   restored and must be non-zero.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > LongJump (
> > >diff --git a/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >b/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >index 9dca97a0dcc9..5cc83d2956e5 100644
> > >--- a/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >+++ b/MdePkg/Library/BaseLib/BaseLibInternals.h
> > >@@ -441,6 +441,7 @@ InternalAssertJumpBuffer (
> > >   @param  Value The value to return when the SetJump() context is
> > >restored.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > InternalLongJump (
> > >diff --git a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >index e309e8b57d7a..f48d7d17c4e4 100644
> > >--- a/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >+++ b/MdePkg/Library/BaseLib/Ebc/SetJumpLongJump.c
> > >@@ -54,6 +54,7 @@ SetJump (
> > >   @param  Value The value to return when the SetJump() context is
> > >restored.
> > >
> > > **/
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > InternalLongJump (
> > >diff --git a/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >b/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >index 73973a9cceae..8fc86f9efb69 100644
> > >--- a/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >+++ b/MdePkg/Library/BaseLib/Ia32/LongJump.c
> > >@@ -28,6 +28,7 @@
> > >
> > > **/
> > > __declspec (naked)
> > >+NORETURN
> > > VOID
> > > EFIAPI
> > > InternalLongJump (
> > >diff --git a/MdePkg/Library/BaseLib/LongJump.c
> > >b/MdePkg/Library/BaseLib/LongJump.c
> > >index 062be8f2daa1..fef764d6787e 100644
> > >--- a/MdePkg/Library/BaseLib/LongJump.c
> > >+++ b/MdePkg/Library/BaseLib/LongJump.c
> > >@@ -33,6 +33,7 @@
> > >   restored and must be non-zero.
> > 

[edk2] [Patch 0/2] Fixed some issues in DxeIpIoLib

[Wang Fan]

See descriptions in each patch.

Wang Fan (2):
  MdeModulePkg: Freed the received packet buffer if it is not expected.
  MdeModulePkg: Did some code enhancement for DxeIpIpLib.

 MdeModulePkg/Include/Library/IpIoLib.h   | 21 -
 MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 70 +++-
 2 files changed, 66 insertions(+), 25 deletions(-)

-- 
1.9.5.msysgit.1

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

[edk2] [Patch 1/2] MdeModulePkg: Freed the received packet buffer if it is not expected.

[Wang Fan]

* When the packet is not normal packet or icmp error packet, the code
  does not recycle it by signal RecycleSignal event, and this will
  result some memory leak. This patch is to fix this issue.

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

diff --git a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c 
b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
index a06c0b6..c7bc1aa 100644
--- a/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
+++ b/MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c
@@ -1037,16 +1037,26 @@ IpIoListenHandlerDpc (
 // The reception is actively aborted by the consumer, directly return.
 //
 return;
   }
 
-  if (((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) || (NULL == 
RxData)) {
+  if ((EFI_SUCCESS != Status) && (EFI_ICMP_ERROR != Status)) {
 //
-// @bug Only process the normal packets and the icmp error packets, if 
RxData is NULL
-// @bug with Status == EFI_SUCCESS or EFI_ICMP_ERROR, just resume the 
receive although
-// @bug this should be a bug of the low layer (IP).
+// Only process the normal packets and the icmp error packets.
 //
+if (RxData != NULL) {
+  goto CleanUp;
+} else {
+  goto Resume;
+}
+  }
+
+  //
+  // if RxData is NULL with Status == EFI_SUCCESS or EFI_ICMP_ERROR, this 
should be a code issue in the low layer (IP).
+  //
+  ASSERT (RxData != NULL);
+  if (RxData == NULL) {
 goto Resume;
   }
 
   if (NULL == IpIo->PktRcvdNotify) {
 goto CleanUp;
-- 
1.9.5.msysgit.1

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

[edk2] [Patch 2/2] MdeModulePkg: Did some code enhancement for DxeIpIpLib.

[Wang Fan]

* In DxeIpIo, there are several places use ASSERT() to check input
  parameters without and descriptions or error handling. This patch
  fixed this issue.
* Fixed some incorrect descriptions in code commence.
* Remove unneeded Exit tag in function IpIoOpen and IpIoConfigIp.
* Add EFIAPI tag for function IpIoRefreshNeighbor.

Cc: Jiaxin Wu 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 MdeModulePkg/Include/Library/IpIoLib.h   | 21 +--
 MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.c | 52 ++--
 2 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Include/Library/IpIoLib.h 
b/MdeModulePkg/Include/Library/IpIoLib.h
index 463bf95..61653b0 100644
--- a/MdeModulePkg/Include/Library/IpIoLib.h
+++ b/MdeModulePkg/Include/Library/IpIoLib.h
@@ -308,10 +308,12 @@ typedef struct _IP_IO_IP_INFO {
   UINT8 IpVersion;
 } IP_IO_IP_INFO;
 
 /**
   Create a new IP_IO instance.
+
+  If IpVersion is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function uses IP4/IP6 service binding protocol in Controller to create
   an IP4/IP6 child (aka IP4/IP6 instance).
 
   @param[in]  Image The image handle of the driver or application 
that
@@ -351,10 +353,12 @@ IpIoDestroy (
   IN OUT IP_IO *IpIo
   );
 
 /**
   Stop an IP_IO instance.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   This function is paired with IpIoOpen(). The IP_IO will be unconfigured, and 
all
   pending send/receive tokens will be canceled.
 
   @param[in, out]  IpIoThe pointer to the IP_IO instance that 
needs to stop.
@@ -370,11 +374,13 @@ IpIoStop (
   IN OUT IP_IO *IpIo
   );
 
 /**
   Open an IP_IO instance for use.
-  
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
   This function is called after IpIoCreate(). It is used for configuring the IP
   instance and register the callbacks and their context data for sending and
   receiving IP packets.
 
   @param[in, out]  IpIo   The pointer to an IP_IO instance that 
needs
@@ -399,11 +405,11 @@ IpIoOpen (
   );
 
 /**
   Send out an IP packet.
   
-  This function is called after IpIoOpen(). The data to be sent are wrapped in
+  This function is called after IpIoOpen(). The data to be sent is wrapped in
   Pkt. The IP instance wrapped in IpIo is used for sending by default but can 
be
   overriden by Sender. Other sending configs, like source address and gateway
   address etc., are specified in OverrideData.
 
   @param[in, out]  IpIo  Pointer to an IP_IO instance used for 
sending IP
@@ -437,10 +443,13 @@ IpIoSend (
   );
 
 /**
   Cancel the IP transmit token that wraps this Packet.
 
+  If IpIo is NULL, then ASSERT().
+  If Packet is NULL, then ASSERT().
+
   @param[in]  IpIo  The pointer to the IP_IO instance.
   @param[in]  PacketThe pointer to the packet of NET_BUF to 
cancel.
 
 **/
 VOID
@@ -450,10 +459,13 @@ IpIoCancelTxToken (
   IN VOID   *Packet
   );
 
 /**
   Add a new IP instance for sending data.
+
+  If IpIo is NULL, then ASSERT().
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
   
   The function is used to add the IP_IO to the IP_IO sending list. The caller
   can later use IpIoFindSender() to get the IP_IO and call IpIoSend() to send
   data.
 
@@ -471,10 +483,12 @@ IpIoAddIp (
 
 /**
   Configure the IP instance of this IpInfo and start the receiving if 
IpConfigData
   is not NULL.
 
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().
+
   @param[in, out]  IpInfo  The pointer to the IP_IO_IP_INFO instance.
   @param[in, out]  IpConfigDataThe IP4 or IP6 configure data used to 
configure 
the IP instance. If NULL, the IP instance 
is reset.
If UseDefaultAddress is set to TRUE, and 
the configure
operation succeeds, the default address 
information
@@ -493,10 +507,12 @@ IpIoConfigIp (
   );
 
 /**
   Destroy an IP instance maintained in IpIo->IpList for
   sending purpose.
+
+  If Ip version is not IP_VERSION_4 or IP_VERSION_6, then ASSERT().  
   
   This function pairs with IpIoAddIp(). The IpInfo is previously created by
   IpIoAddIp(). The IP_IO_IP_INFO::RefCnt is decremented and the IP instance
   will be dstroyed if the RefCnt is zero.
 
@@ -584,10 +600,11 @@ IpIoGetIcmpErrStatus (
   @retval  EFI_UNSUPPORTED   IP version is IPv4, which doesn't support 
neighbor cache refresh.
   @retval  EFI_OUT_OF_RESOURCES  Failed due to resource limitations.
 
 **/
 EFI_STATUS
+EFIAPI
 IpIoRefreshNeighbor (
   IN IP_IO   *IpIo,
   IN EFI_IP_ADDRESS  *Neighbor,
   IN UINT32  Timeout  
   );
diff --git 

Re: [edk2] [PATCH 0/4] Quality improvement of MtrrLib

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Tuesday, January 9, 2018 11:36 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 0/4] Quality improvement of MtrrLib

The patch sets fix four issues.
For details, please refer to the invidual patch.
Patch 2/4 fixes the most critical bug.

Ruiyu Ni (4):
  UefiCpuPkg/MtrrLib: Refine the debug messages
  UefiCpuPkg/MtrrLib: Fix bug that may calculate wrong MTRR result
  UefiCpuPkg/MtrrLib: Handle one setting request covering all memory
  UefiCpuPkg/MtrrLib: Correct typo to change vector to vertex

 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 295 ---
 1 file changed, 168 insertions(+), 127 deletions(-)

-- 
2.15.1.windows.2

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

Re: [edk2] [PATCH v3 0/2] Fix wrong address set as Stack Guard for APs

[Dong, Eric]

Reviewed-by: Eric Dong 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jian J 
Wang
Sent: Monday, January 8, 2018 1:40 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v3 0/2] Fix wrong address set as Stack Guard for APs

> v3 changes:
> a. Split the patch into two patch files.
> b. Pass MpServiceProtocol test cases in PI SCT.

> v2 changes:
> a. Use each AP's ApTopOfStack to get the stack base address instead of
>cpu0's ApTopOfStack which is actually set incorrectly before.
> b. Fix cpu0's ApTopOfStack initialization.
> c. Fix wrong debug print format.

The reason is that DXE part initialization will reuse the stack allocated at 
PEI phase, if MP was initialized before. Some code added to check this 
situation and use stack base address saved in HOB passed from PEI.

Jian J Wang (2):
  UefiCpuPkg/MpInitLib: fix incorrect stack base init for cpu0
  UefiCpuPkg/MpInitLib: fix wrong address set as Stack Guard for APs

 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 19 ++-
 UefiCpuPkg/Library/MpInitLib/MpLib.c|  2 +-
 2 files changed, 19 insertions(+), 2 deletions(-)

--
2.15.1.windows.2

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

Re: [edk2] [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add EFI_MM_COMMUNICATION_PROTOCOL DXE driver.

[Supreeth Venkatesh]

On Wed, 2017-12-13 at 11:26 +, Meenakshi Aggarwal wrote:
> Hi Supreeth,
> 
> Few comments inline.
Thank you for your comments.
> 
> 
> > 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > Of
> > Supreeth Venkatesh
> > Sent: Friday, November 17, 2017 10:18 PM
> > To: edk2-devel@lists.01.org
> > Cc: leif.lindh...@linaro.org; ard.biesheu...@linaro.org
> > Subject: [edk2] [edk2 PATCH v3 2/2] ArmPkg/Drivers: Add
> > EFI_MM_COMMUNICATION_PROTOCOL DXE driver.
> > 
> > PI v1.5 Specification Volume 4 defines Management Mode Core
> > Interface
> > and defines EFI_MM_COMMUNICATION_PROTOCOL. This protocol provides
> > a
> > means of communicating between drivers outside of MM and MMI
> > handlers inside of MM.
> > 
> > This patch implements the EFI_MM_COMMUNICATION_PROTOCOL DXE
> > runtime
> > driver for AARCH64 platforms. It uses SMCs allocated from the
> > standard
> > SMC range defined in
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fi
> > nfo
> > center.arm.com%2Fhelp%2Ftopic%2Fcom.arm.doc.den0060a%2FDEN0060A_
> > ARM_MM_Interface_Specification.pdf=02%7C01%7Cmeenakshi.aggar
> > wal%40nxp.com%7C27ae12fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4
> > c6fa92cd99c5c301635%7C0%7C1%7C636465341155830632=pYKl2uUUF
> > VCS4M87y%2BRK8TE852QqcusN0Fm208IkjtU%3D=0
> > to communicate with the standalone MM environment in the secure
> > world.
> > 
> > This patch also adds the MM Communication driver (.inf) file to
> > define entry
> > point for this driver and other compile related information the
> > driver
> > needs.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Achin Gupta 
> > Signed-off-by: Supreeth Venkatesh 
> > ---
> >  .../Drivers/MmCommunicationDxe/MmCommunication.c   | 339
> > +
> >  .../Drivers/MmCommunicationDxe/MmCommunication.inf |  50 +++
> >  2 files changed, 389 insertions(+)
> >  create mode 100644
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> >  create mode 100644
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > 
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > new file mode 100644
> > index 00..e801c1c601
> > --- /dev/null
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > @@ -0,0 +1,339 @@
> > +/** @file
> > +
> > +  Copyright (c) 2016-2017, ARM 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
> > +
> > https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fo
> > pe
> > nsource.org%2Flicenses%2Fbsd-
> > license.php=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C27ae12
> > fc41414e501f8208d52ddb09f3%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> > %7C0%7C636465341155830632=GPd9o7ovyTU10etIxP%2BBYNsYUKq
> > m37tPcc%2BQDKtext4%3D=0
> > +
> > +  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 
> > +
> > +//
> > +// Address, Length of the pre-allocated buffer for communication
> > with the
> > secure
> > +// world.
> > +//
> > +STATIC ARM_MEMORY_REGION_DESCRIPTOR  mNsCommBuffMemRegion;
> > +
> > +// Notification event when virtual address map is set.
> > +STATIC EFI_EVENT  mSetVirtualAddressMapEvent;
> > +
> > +//
> > +// Handle to install the MM Communication Protocol
> > +//
> > +STATIC EFI_HANDLE  mMmCommunicateHandle;
> > +
> > +/**
> > +  Communicates with a registered handler.
> > +
> > +  This function provides an interface to send and receive messages
> > to the
> > +  Standalone MM environment on behalf of UEFI services.  This
> > function is
> > part
> > +  of the MM Communication Protocol that may be called in physical
> > mode
> > prior to
> > +  SetVirtualAddressMap() and in virtual mode after
> > SetVirtualAddressMap().
> > +
> > +  @param[in]  ThisThe
> > EFI_MM_COMMUNICATION_PROTOCOL
> > +  instance.
> > +  @param[in, out] CommBuffer  A pointer to the buffer to
> > convey
> > +  into MMRAM.
> > +  @param[in, out] CommSizeThe size of the data buffer
> > being
> > +  passed in. This is optional.
> > +
> > +  @retval EFI_SUCCESS The message was successfully
> > posted.
> > +  @retval EFI_INVALID_PARAMETER   The CommBuffer was NULL.
> > +  @retval EFI_BAD_BUFFER_SIZE The buffer 

Re: [edk2] [PATCH edk2-platforms v2 18/18] ARM/JunoPkg: Add HDLCD platform library

[Ard Biesheuvel]

On 9 January 2018 at 18:21, Evan Lloyd  wrote:
>
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: 23 December 2017 16:23
>> To: Evan Lloyd 
>> Cc: edk2-devel@lists.01.org; Arvind Chauhan ;
>> Daniil Egranov ; Thomas Abraham
>> ; "ard.biesheu...@linaro.org"@arm.com;
>> "leif.lindh...@linaro.org"@arm.com;
>> "matteo.carl...@arm.com"@arm.com; "n...@arm.com"@arm.com
>> Subject: Re: [PATCH edk2-platforms v2 18/18] ARM/JunoPkg: Add HDLCD
>> platform library
>>
>> On 22 December 2017 at 19:08,   wrote:
>> > From: Girish Pathak 
>> >
>> > This change adds the HDLCD platform lib for the Juno plaform. This
>> > library will be instantiated as a LcdPlatformLib to link with
>> > LcdGraphicsOutputDxe for the Juno platform.
>> >
>> > HDLCD platform library depends on the Arm SCMI DXE driver for
>> > communication with the SCP for clock setting. Therefore this change
>> > also enables building of Arm SCMI DXE driver for the Juno platform.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Girish Pathak 
>>
>> Missing signoff?
> [[Evan Lloyd]] Yes.  Oops.
>
>>
>> > ---
>> >  Platform/ARM/JunoPkg/ArmJuno.dec |   8 +
>> >  Platform/ARM/JunoPkg/ArmJuno.dsc |  29 +
>> >  Platform/ARM/JunoPkg/ArmJuno.fdf |  12 +-
>> >  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf   |   5 +-
>> >  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
>> |  40 ++
>> >  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c |
>> 18 +-
>> >  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c  |
>> 559 
>> >  7 files changed, 668 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec
>> > b/Platform/ARM/JunoPkg/ArmJuno.dec
>> > index
>> >
>> b733480c3198d135df16ca024b5e85ff350e11c7..cd6710feb2faf0bd17b5ea
>> 39a21d
>> > be5406cd4ffd 100644
>> > --- a/Platform/ARM/JunoPkg/ArmJuno.dec
>> > +++ b/Platform/ARM/JunoPkg/ArmJuno.dec
>> > @@ -53,3 +53,11 @@ [PcdsFixedAtBuild.common]
>> >
>> gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxBase|0x2E00|UINT64|0
>> x0025
>> >
>> gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxSize|0x80|UINT32|0x0
>> 026
>> >
>> > +  # MaxMode must be one number higher than the actual max mode,  #
>> > + i.e. for actual maximum mode 2, set the value to 3.
>> > +  #
>> > +  # Default value zero allows platform to enumerate maximum
>> supported mode.
>> > +  #
>> > +  # For a list of mode numbers look in HdLcdArmJuno.c
>> > +
>> gArmJunoTokenSpaceGuid.PcdArmHdLcdMaxMode|0|UINT32|0x001
>> 7
>> > +
>> > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc
>> > b/Platform/ARM/JunoPkg/ArmJuno.dsc
>> > index
>> >
>> fe860956a4dc497cac52be70bab3657246a08bd0..9027c5b0728a6941f850
>> 636b3bc3
>> > 15fd33b867fb 100644
>> > --- a/Platform/ARM/JunoPkg/ArmJuno.dsc
>> > +++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
>> > @@ -50,6 +50,11 @@ [LibraryClasses.common]
>> ># SCMI Mailbox Transport Layer
>> >ArmMtl|Platform/ARM/JunoPkg/Library/ArmMtl/ArmMtl.inf
>> >
>> > +!ifndef HEADLESS_PLATFORM
>>
>> Wouldn't it make more sense to add a macro ENABLE_HDLCD, rather than
>> inverting the logic?
>
>  [[Evan Lloyd]] We used this to correspond with the ACPI (FADT) terminology, 
> where HEADLESS implies more than just no display (e.g. no keyboard or mouse).
> It would be possible to insert another level to define ENABLE_HDLCD, 
> ENABLE_KEYBOARD, and ENABLE_MOUSE when  HEADLESS_PLATFORM is not defined, but 
> I'm not convinced that would improve clarity.
> For mobile platforms (Juno, say) the default seems obvious, as a mobile 
> without MMI is pretty pointless (unless you contemplate UEFI on IOT?).  For 
> servers, less so, but the option is still available.  It also seems more 
> natural to explicitly exclude support for hardware that is present, rather 
> than defaulting to a crippled state.
>
>

Fair enough.

>>
>> > +
>> >
> ...
>> > +  # SCMI Driver
>> > +  ArmPlatformPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf {
>> > +
>> > +
>> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>>
>> I take it your trusted SRAM does not tolerate unaligned memcpy() because
>> it is mapped as device memory. Couldn't you map it as non-cacheable
>> memory instead? (I meant to ask in response to the other patch but I forgot)
>>
>
>  [[Evan Lloyd]]  As this is generic code we can't know what the platform SCP 
> interface memory constraints might be, so we've gone for the safest option. 
> (It might be fine for Juno, but future stuff is unknown.)  As far as I can 
> tell, the only advantage of non-cacheable would be to permit unaligned 
> access, which might give a small performance improvement.  My current guess 

Re: [edk2] [PATCH edk2-platforms v2 18/18] ARM/JunoPkg: Add HDLCD platform library

[Evan Lloyd]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: 23 December 2017 16:23
> To: Evan Lloyd 
> Cc: edk2-devel@lists.01.org; Arvind Chauhan ;
> Daniil Egranov ; Thomas Abraham
> ; "ard.biesheu...@linaro.org"@arm.com;
> "leif.lindh...@linaro.org"@arm.com;
> "matteo.carl...@arm.com"@arm.com; "n...@arm.com"@arm.com
> Subject: Re: [PATCH edk2-platforms v2 18/18] ARM/JunoPkg: Add HDLCD
> platform library
>
> On 22 December 2017 at 19:08,   wrote:
> > From: Girish Pathak 
> >
> > This change adds the HDLCD platform lib for the Juno plaform. This
> > library will be instantiated as a LcdPlatformLib to link with
> > LcdGraphicsOutputDxe for the Juno platform.
> >
> > HDLCD platform library depends on the Arm SCMI DXE driver for
> > communication with the SCP for clock setting. Therefore this change
> > also enables building of Arm SCMI DXE driver for the Juno platform.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Girish Pathak 
>
> Missing signoff?
[[Evan Lloyd]] Yes.  Oops.

>
> > ---
> >  Platform/ARM/JunoPkg/ArmJuno.dec |   8 +
> >  Platform/ARM/JunoPkg/ArmJuno.dsc |  29 +
> >  Platform/ARM/JunoPkg/ArmJuno.fdf |  12 +-
> >  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoLib.inf   |   5 +-
> >  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJunoLib.inf
> |  40 ++
> >  Platform/ARM/JunoPkg/Library/ArmJunoLib/ArmJunoMem.c |
> 18 +-
> >  Platform/ARM/JunoPkg/Library/HdLcdArmJunoLib/HdLcdArmJuno.c  |
> 559 
> >  7 files changed, 668 insertions(+), 3 deletions(-)
> >
> > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dec
> > b/Platform/ARM/JunoPkg/ArmJuno.dec
> > index
> >
> b733480c3198d135df16ca024b5e85ff350e11c7..cd6710feb2faf0bd17b5ea
> 39a21d
> > be5406cd4ffd 100644
> > --- a/Platform/ARM/JunoPkg/ArmJuno.dec
> > +++ b/Platform/ARM/JunoPkg/ArmJuno.dec
> > @@ -53,3 +53,11 @@ [PcdsFixedAtBuild.common]
> >
> gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxBase|0x2E00|UINT64|0
> x0025
> >
> gArmJunoTokenSpaceGuid.PcdArmMtlMailBoxSize|0x80|UINT32|0x0
> 026
> >
> > +  # MaxMode must be one number higher than the actual max mode,  #
> > + i.e. for actual maximum mode 2, set the value to 3.
> > +  #
> > +  # Default value zero allows platform to enumerate maximum
> supported mode.
> > +  #
> > +  # For a list of mode numbers look in HdLcdArmJuno.c
> > +
> gArmJunoTokenSpaceGuid.PcdArmHdLcdMaxMode|0|UINT32|0x001
> 7
> > +
> > diff --git a/Platform/ARM/JunoPkg/ArmJuno.dsc
> > b/Platform/ARM/JunoPkg/ArmJuno.dsc
> > index
> >
> fe860956a4dc497cac52be70bab3657246a08bd0..9027c5b0728a6941f850
> 636b3bc3
> > 15fd33b867fb 100644
> > --- a/Platform/ARM/JunoPkg/ArmJuno.dsc
> > +++ b/Platform/ARM/JunoPkg/ArmJuno.dsc
> > @@ -50,6 +50,11 @@ [LibraryClasses.common]
> ># SCMI Mailbox Transport Layer
> >ArmMtl|Platform/ARM/JunoPkg/Library/ArmMtl/ArmMtl.inf
> >
> > +!ifndef HEADLESS_PLATFORM
>
> Wouldn't it make more sense to add a macro ENABLE_HDLCD, rather than
> inverting the logic?

 [[Evan Lloyd]] We used this to correspond with the ACPI (FADT) terminology, 
where HEADLESS implies more than just no display (e.g. no keyboard or mouse).
It would be possible to insert another level to define ENABLE_HDLCD, 
ENABLE_KEYBOARD, and ENABLE_MOUSE when  HEADLESS_PLATFORM is not defined, but 
I'm not convinced that would improve clarity.
For mobile platforms (Juno, say) the default seems obvious, as a mobile without 
MMI is pretty pointless (unless you contemplate UEFI on IOT?).  For servers, 
less so, but the option is still available.  It also seems more natural to 
explicitly exclude support for hardware that is present, rather than defaulting 
to a crippled state.


>
> > +
> >
...
> > +  # SCMI Driver
> > +  ArmPlatformPkg/Drivers/ArmScmiDxe/ArmScmiDxe.inf {
> > +
> > +
> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>
> I take it your trusted SRAM does not tolerate unaligned memcpy() because
> it is mapped as device memory. Couldn't you map it as non-cacheable
> memory instead? (I meant to ask in response to the other patch but I forgot)
>

 [[Evan Lloyd]]  As this is generic code we can't know what the platform SCP 
interface memory constraints might be, so we've gone for the safest option. (It 
might be fine for Juno, but future stuff is unknown.)  As far as I can tell, 
the only advantage of non-cacheable would be to permit unaligned access, which 
might give a small performance improvement.  My current guess is that the 
caller's protocol message structures are likely to be properly aligned anyway, 
so the benefit might be negligible.  The drawback would be that you need to add 
barriers.
The bottom line is, we'll change it if 

[edk2] [PATCH 1/2] BaseTools: Enable MAX_CONCURRENT_THREAD_NUMBER = 0 feature

[Feng, YunhuaX]

when set 'MAX_CONCURRENT_THREAD_NUMBER=0',  will auto-detect number of 
processor threads for MAX_CONCURRENT_THREAD_NUMBER

Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=775
Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/build/build.py | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git BaseTools/Source/Python/build/build.py 
BaseTools/Source/Python/build/build.py
index 38498046d7..de19756d99 100644
--- BaseTools/Source/Python/build/build.py
+++ BaseTools/Source/Python/build/build.py
@@ -2,7 +2,7 @@
 # build a platform or a module
 #
 #  Copyright (c) 2014, Hewlett-Packard Development Company, L.P.
-#  Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -26,6 +26,7 @@ import platform
 import traceback
 import encodings.ascii
 import itertools
+import multiprocessing

 from struct import *
 from threading import *
@@ -936,7 +937,10 @@ class Build():
 self.ThreadNumber = int(self.ThreadNumber, 0)

 if self.ThreadNumber == 0:
-self.ThreadNumber = 1
+try:
+self.ThreadNumber = multiprocessing.cpu_count()
+except (ImportError, NotImplementedError):
+self.ThreadNumber = 1

 if not self.PlatformFile:
 PlatformFile = 
self.TargetTxt.TargetTxtDictionary[DataType.TAB_TAT_DEFINES_ACTIVE_PLATFORM]
--
2.12.2.windows.2

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

[edk2] [PATCH] BaseTools: Enable MAX_CONCURRENT_THREAD_NUMBER = 0 feature

[Feng, YunhuaX]

Adding 'MAX_CONCURRENT_THREAD_NUMBER=0' option for user to
enable 'auto-detect thread number' feature, and changing default value 
to '0' when initial build environment is configured.

Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=775
Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Conf/target.template | 5 +++--
 BaseTools/Source/Python/build/build.py | 6 +-
 2 files changed, 8 insertions(+), 3 deletions(-)

diff --git BaseTools/Conf/target.template BaseTools/Conf/target.template
index 787fc64fb1..ee8745610f 100644
--- BaseTools/Conf/target.template
+++ BaseTools/Conf/target.template
@@ -61,8 +61,9 @@ TOOL_CHAIN_TAG= MYTOOLS
 
 # MAX_CONCURRENT_THREAD_NUMBER  NUMBER  Optional  The number of concurrent 
threads. Recommend to set this
 # value to one more than the 
number of your compurter
-# cores or CPUs. Less than 2 
means disable multithread build.
-MAX_CONCURRENT_THREAD_NUMBER = 1
+# cores or CPUs. automatically 
detect number of processor threads
+# when 
'MAX_CONCURRENT_THREAD_NUMBER=0'
+MAX_CONCURRENT_THREAD_NUMBER = 0
 
 
 # BUILD_RULE_CONF  Filename Optional  Specify the file name to use for the 
build rules that are followed
diff --git BaseTools/Source/Python/build/build.py 
BaseTools/Source/Python/build/build.py
index 38498046d7..fd2681e05d 100644
--- BaseTools/Source/Python/build/build.py
+++ BaseTools/Source/Python/build/build.py
@@ -26,6 +26,7 @@ import platform
 import traceback
 import encodings.ascii
 import itertools
+import multiprocessing
 
 from struct import *
 from threading import *
@@ -936,7 +937,10 @@ class Build():
 self.ThreadNumber = int(self.ThreadNumber, 0)
 
 if self.ThreadNumber == 0:
-self.ThreadNumber = 1
+try:
+self.ThreadNumber = multiprocessing.cpu_count()
+except (ImportError, NotImplementedError):
+self.ThreadNumber = 1
 
 if not self.PlatformFile:
 PlatformFile = 
self.TargetTxt.TargetTxtDictionary[DataType.TAB_TAT_DEFINES_ACTIVE_PLATFORM]
-- 
2.12.2.windows.2

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

[edk2] [PATCH 2/2] BaseTools: Modify MAX_CONCURRENT_THREAD_NUMBER default values

[Feng, YunhuaX]

Modify MAX_CONCURRENT_THREAD_NUMBER default values

Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=775
Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Yunhua Feng 
---
 BaseTools/Conf/target.template | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git BaseTools/Conf/target.template BaseTools/Conf/target.template
index 787fc64fb1..c50403bd06 100644
--- BaseTools/Conf/target.template
+++ BaseTools/Conf/target.template
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 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
@@ -60,9 +60,10 @@ TOOL_CHAIN_CONF   = Conf/tools_def.txt
 TOOL_CHAIN_TAG= MYTOOLS
 
 # MAX_CONCURRENT_THREAD_NUMBER  NUMBER  Optional  The number of concurrent 
threads. Recommend to set this
-# value to one more than the 
number of your compurter
-# cores or CPUs. Less than 2 
means disable multithread build.
-MAX_CONCURRENT_THREAD_NUMBER = 1
+# value to one more than the 
number of your computer
+# cores or CPUs. automatically 
detect number of processor threads
+# when 
'MAX_CONCURRENT_THREAD_NUMBER=0'
+MAX_CONCURRENT_THREAD_NUMBER = 0
 
 
 # BUILD_RULE_CONF  Filename Optional  Specify the file name to use for the 
build rules that are followed
-- 
2.12.2.windows.2

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

[edk2] [PATCH] BaseTools: Correct Target Path in CodaTargetList replace Path

[Feng, YunhuaX]

Target Path in CodaTargetList is DebugDir path, correct replace
path with DebugDir

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

diff --git BaseTools/Source/Python/AutoGen/AutoGen.py 
BaseTools/Source/Python/AutoGen/AutoGen.py
index 8be5bfca83..2cd15dfd50 100644
--- BaseTools/Source/Python/AutoGen/AutoGen.py
+++ BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -4149,8 +4149,9 @@ class ModuleAutoGen(AutoGen):
 AsBuiltInfDict['module_pi_specification_version'] += 
[self.Specification['PI_SPECIFICATION_VERSION']]

 OutputDir = self.OutputDir.replace('\\', '/').strip('/')
+DebugDir = self.DebugDir.replace('\\', '/').strip('/')
 for Item in self.CodaTargetList:
-File = Item.Target.Path.replace('\\', 
'/').strip('/').replace(OutputDir, '').strip('/')
+File = Item.Target.Path.replace('\\', 
'/').strip('/').replace(DebugDir, '').strip('/')
 if File not in self.OutputFile:
 self.OutputFile.append(File)
 if Item.Target.Ext.lower() == '.aml':
--
2.12.2.windows.2

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

Re: [edk2] [PATCH] MdeModulePkg/PciBus: Change switch-case to if-else to fix EBC build

[Laszlo Ersek]

On 01/09/18 06:53, Ruiyu Ni wrote:
> EBC compiler doesn't treat EFI_xxx as constant due to these macros
> are UINT64 type in 64bit env and UINT32 type in 32bit env.
> So it reports error when "case EFI_xxx" is used.
> The patch changes to use if-else to fix EBC build failure.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Laszlo Ersek 
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c | 20 +++-
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c 
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> index dc1086606f..976496379a 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
> @@ -1154,19 +1154,13 @@ PciScanBus (
>  
>FreePool (Descriptors);
>  
> -  switch (Status) {
> -case EFI_SUCCESS:
> -  BusPadding = TRUE;
> -  break;
> -
> -case EFI_NOT_FOUND:
> -  //
> -  // no bus number padding requested
> -  //
> -  break;
> -
> -default:
> -  return Status;
> +  if (!EFI_ERROR (Status)) {
> +BusPadding = TRUE;
> +  } else if (Status != EFI_NOT_FOUND) {
> +//
> +// EFI_NOT_FOUND is not a real error. It indicates no bus 
> number padding requested.
> +//
> +return Status;
>}
>  }
>}
> 

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

Re: [edk2] [PATCH edk2-platforms v2 2/2] LS1046 : Enable support of SATA controller

[Ard Biesheuvel]

On 9 January 2018 at 04:37, Meenakshi Aggarwal
 wrote:
>
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Monday, January 08, 2018 8:42 PM
>> To: Meenakshi Aggarwal 
>> Cc: Leif Lindholm ; Kinney, Michael D
>> ; edk2-devel@lists.01.org; Udit Kumar
>> ; Varun Sethi 
>> Subject: Re: [PATCH edk2-platforms v2 2/2] LS1046 : Enable support of SATA
>> controller
>>
>> On 8 January 2018 at 15:55, Meenakshi Aggarwal
>>  wrote:
>> > Enable support of SATA drives on ls1046 board.
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Meenakshi Aggarwal 
>> > ---
>> >  Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc |  8 
>> >  Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf | 12
>> 
>> >  .../NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf |  2 ++
>> >  .../NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c|  8
>> 
>> >  Silicon/NXP/LS1046A/LS1046A.dsc  |  5 +
>> >  5 files changed, 35 insertions(+)
>> >
>> > diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> > index 9d2482b..93fc848 100644
>> > --- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> > +++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc
>> > @@ -63,6 +63,13 @@
>> >#
>> >gNxpQoriqLsTokenSpaceGuid.PcdI2cSlaveAddress|0x51
>> >
>> > +  #
>> > +  # Errata Pcds
>> > +  #
>> > +  gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA009185|TRUE
>> > +  gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA010554|TRUE
>> > +  gNxpQoriqLsTokenSpaceGuid.PcdSataErratumA010635|TRUE
>> > +
>> >
>> ##
>> ##
>> >  #
>> >  # Components Section - list of all EDK II Modules needed by this Platform
>> > @@ -71,3 +78,4 @@
>> >  [Components.common]
>> >edk2-platforms/Platform/NXP/Drivers/WatchDog/WatchDogDxe.inf
>> >edk2-platforms/Platform/NXP/Drivers/I2cDxe/I2cDxe.inf
>> > +  edk2-platforms/Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf
>>
>> This looks wrong to me. Your .dsc/.fdf files should not contain these
>> edk2-platforms prefixes. Instead, you should set your PACKAGES_PATH
>> correctly to include your edk2-platforms directory.
>>
> OK, We will remove this from .dsc/.fdf files.
> My concern is as there are already a lot of patches are under review so it 
> will be
> Better if review gets completed once, then we will share the updated in next 
> revision of patch
> As this needs to be change in multiple patches.
>
> There is one more comment from you on keeping shred Drivers and Library in 
> Silicon/NXP directory.
> In this case also, this will need a rework in all patches sent till date.
>
> So once review comments been recieved we will made the changes in next 
> revision of patch.
>

I have a better idea. Let's disregard all current submissions in
flight, and repost the next one as a single series so that I don't
have to keep track of all the different ones.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 1/2] SATA : Added SATA controller driver.

[Ard Biesheuvel]

On 9 January 2018 at 04:50, Meenakshi Aggarwal
 wrote:
>
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Monday, January 08, 2018 8:35 PM
>> To: Meenakshi Aggarwal 
>> Cc: Leif Lindholm ; Kinney, Michael D
>> ; edk2-devel@lists.01.org; Udit Kumar
>> ; Varun Sethi 
>> Subject: Re: [PATCH edk2-platforms v2 1/2] SATA : Added SATA controller
>> driver.
>>
>> Hi Meenakshi,
>>
>> This is looking much better - thanks for rewriting it. I do have some
>> comments below
>>
>> On 8 January 2018 at 15:55, Meenakshi Aggarwal
>>  wrote:
>> > This patch adds support of SATA controller, which
>> > Initialize SATA controller,
>> > apply platform specific errata and
>> > Register itself as NonDiscoverableMmioDevice
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Meenakshi Aggarwal 
>> > ---
>> >  Platform/NXP/Drivers/SataInitDxe/SataInit.c  | 285
>> +++
>> >  Platform/NXP/Drivers/SataInitDxe/SataInit.h  |  36 +++
>> >  Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf |  52 +
>> >  Platform/NXP/NxpQoriqLs.dec  |  14 +-
>> >  Platform/NXP/NxpQoriqLs.dsc  |  13 ++
>> >  5 files changed, 398 insertions(+), 2 deletions(-)
>> >  create mode 100644 Platform/NXP/Drivers/SataInitDxe/SataInit.c
>> >  create mode 100644 Platform/NXP/Drivers/SataInitDxe/SataInit.h
>> >  create mode 100644 Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf
>> >

>> > +/**
>> > +  The Entry Point of module. It follows the standard UEFI driver model.
>> > +
>> > +  @param[in] ImageHandle   The firmware allocated handle for the EFI
>> image.
>> > +  @param[in] SystemTable   A pointer to the EFI System Table.
>> > +
>> > +  @retval EFI_SUCCESS  The entry point is executed successfully.
>> > +  @retval otherSome error occurs when executing this entry 
>> > point.
>> > +
>> > +**/
>> > +EFI_STATUS
>> > +EFIAPI
>> > +InitializeSataController (
>> > +  IN EFI_HANDLEImageHandle,
>> > +  IN EFI_SYSTEM_TABLE  *SystemTable
>> > +  )
>> > +{
>> > +  EFI_STATUS   Status;
>> > +  UINT32   NumSataController;
>> > +  UINTNControllerAddr;
>> > +
>> > +  Status = EFI_SUCCESS;
>> > +  NumSataController = PcdGet32 (PcdNumSataController);
>> > +
>> > +  //
>> > +  // Impact : The SATA controller does not detect some hard drives 
>> > reliably
>> with
>> > +  // the default SerDes register setting.
>> > +  // Workaround : write value 0x80104e20 to 0x1eb1300 (serdes 2)
>> > +  //
>> > +  if (PcdGetBool (PcdSataErratumA010554)) {
>> > +BeMmioWrite32 ((UINTN)SERDES2_SATA_ERRATA, 0x80104e20);
>> > +  }
>> > +
>> > +  //
>> > +  // Impact : Device may see false CRC errors causing unreliable SATA
>> operation.
>> > +  // Workaround : write 0x8000 to the address 0x20140520 (dcsr).
>> > +  //
>> > +  if (PcdGetBool (PcdSataErratumA010635)) {
>> > +BeMmioWrite32 ((UINTN)DCSR_SATA_ERRATA, 0x8000);
>> > +  }
>> > +
>> > +  while (NumSataController) {
>> > +NumSataController--;
>> > +ControllerAddr = PcdGet32 (PcdSataBaseAddr) +
>> > + (NumSataController * PcdGet32 (PcdSataSize));
>> > +
>> > +Status = RegisterNonDiscoverableMmioDevice (
>> > +   NonDiscoverableDeviceTypeAhci,
>> > +   NonDiscoverableDeviceDmaTypeNonCoherent,
>> > +   NULL,
>> > +   NULL,
>> > +   1,
>> > +   ControllerAddr, PcdGet32 (PcdSataSize)
>> > + );
>> > +
>> > +if (EFI_ERROR (Status)) {
>> > +  DEBUG ((DEBUG_ERROR, "Failed to register SATA device (0x%x) with
>> error 0x%x \n",
>> > +   ControllerAddr, Status));
>>
>> Please don't use if/else for the expected path: instead, return here
>> or goto the error/unwind code at the end of the function
>>
> In case of more than one controller, we cannot return/goto from here because 
> there are chances that other controller might get register successfully.
> So used if-else, please suggest the correct way.
>

Then use 'continue'

In any case, all RegisterNonDiscoverableMmioDevice() does is install a
protocol on a new handle, so it is unlikely to fail. You can just
replace the error handling with ASSERT_EFI_ERROR().


>> > +} else {
>> > +  //
>> > +  // Register a protocol registration notification callback on the 
>> > driver
>> > +  // binding protocol so we can attempt to connect to it as soon as it
>> appears.
>> > +  //
>> > +  EfiCreateProtocolNotifyEvent (
>> > +,
>> > +TPL_CALLBACK,
>> > +PciIoRegistrationEvent,
>> > +(VOID *)ControllerAddr,
>> > +);
>>
>> What is the point of this?