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

[Zhu, Yonghong]

Hi Yunhua,

Please separate this patch into two. And remember to update copyright year.

Best Regards,
Zhu Yonghong


-Original Message-
From: Feng, YunhuaX 
Sent: Tuesday, January 09, 2018 10:14 AM
To: edk2-devel@lists.01.org
Cc: Feng, YunhuaX ; Zhu, Yonghong 
; Gao, Liming 
Subject: [PATCH] BaseTools: Enable MAX_CONCURRENT_THREAD_NUMBER = 0 feature

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

Re: [edk2] [patch] MdeModulePkg/VarCheckHii: Update data type for variable "ArrayIndex"

[Zeng, Star]

Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: Bi, Dandan 
Sent: Tuesday, January 9, 2018 3:25 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Gao, Liming 
Subject: [patch] MdeModulePkg/VarCheckHii: Update data type for variable 
"ArrayIndex"

In some case the ArrayIndex with UINT16 may be not large enough to hold the 
multiplication result of HiiQuestion->VarOffset * 8; So this patch update the 
data type to fix this potential issue.

Cc: Star Zeng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c 
b/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c
index 0db1383..debabb5 100644
--- a/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c
+++ b/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c
@@ -1,9 +1,9 @@
 /** @file
   Var Check Hii bin generation.
 
-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 accompanies this 
distribution.  The full text of the license may be found at  
http://opensource.org/licenses/bsd-license.php
 
@@ -513,11 +513,11 @@ MergeHiiQuestion (
   UINT64Maximum2;
   UINT64OneValue2;
   UINT8 *Ptr;
   UINT8 *Ptr1;
   UINT8 *Ptr2;
-  UINT16ArrayIndex;
+  UINTN ArrayIndex;
 
   //
   // Hii Question from Hii Database has high priority.
   // Do not to merge Hii Question from Fv to Hii Question from Hii Database.
   //
@@ -1062,11 +1062,11 @@ ParseHiiQuestion (
   IN BOOLEANFromFv,
   IN BOOLEANStoredInBitField
   )
 {
   VAR_CHECK_HII_QUESTION_HEADER *HiiQuestion;
-  UINT16ArrayIndex;
+  UINTN ArrayIndex;
 
   //
   // Currently only OneOf, CheckBox and Numeric can be stored in bit field.
   //
   switch (IfrOpCodeHeader->OpCode) {
--
1.9.5.msysgit.1

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

[edk2] [patch] MdeModulePkg/VarCheckHii: Update data type for variable "ArrayIndex"

[Dandan Bi]

In some case the ArrayIndex with UINT16 may be not large enough to
hold the multiplication result of HiiQuestion->VarOffset * 8;
So this patch update the data type to fix this potential issue.

Cc: Star Zeng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c 
b/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c
index 0db1383..debabb5 100644
--- a/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c
+++ b/MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGen.c
@@ -1,9 +1,9 @@
 /** @file
   Var Check Hii bin generation.
 
-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 accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -513,11 +513,11 @@ MergeHiiQuestion (
   UINT64Maximum2;
   UINT64OneValue2;
   UINT8 *Ptr;
   UINT8 *Ptr1;
   UINT8 *Ptr2;
-  UINT16ArrayIndex;
+  UINTN ArrayIndex;
 
   //
   // Hii Question from Hii Database has high priority.
   // Do not to merge Hii Question from Fv to Hii Question from Hii Database.
   //
@@ -1062,11 +1062,11 @@ ParseHiiQuestion (
   IN BOOLEANFromFv,
   IN BOOLEANStoredInBitField
   )
 {
   VAR_CHECK_HII_QUESTION_HEADER *HiiQuestion;
-  UINT16ArrayIndex;
+  UINTN ArrayIndex;
 
   //
   // Currently only OneOf, CheckBox and Numeric can be stored in bit field.
   //
   switch (IfrOpCodeHeader->OpCode) {
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch 1/2] MdeModulePkg: Fixed two issues when error occurs in Mtftp4Start.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Wang, Fan
> Sent: Tuesday, January 9, 2018 9:19 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting ; Fu,
> Siyuan 
> Subject: [Patch 1/2] MdeModulePkg: Fixed two issues when error occurs in
> Mtftp4Start.
> 
> * This function sets returned status as token status and signal token
>   when error occurs, and it results token status not compliance with
>   spec definition. This patch fixed this issue.
> * This function restore Tpl twice when Mtftp4WrqStart() returns an
>   error, this patch fixed this issue.
> 
> Cc: Jiaxin Wu 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 20 ---
> -
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> index 54384e1..f5f9e6d 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> @@ -1,10 +1,10 @@
>  /** @file
>Interface routine for Mtftp4.
> 
>  (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> -Copyright (c) 2006 - 2017, 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
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -407,31 +407,33 @@ Mtftp4Start (
>if (EFI_ERROR (Status)) {
>  gBS->RestoreTPL (OldTpl);
>  return Status;
>}
> 
> +  if ((Token->OverrideData != NULL) && !Mtftp4OverrideValid (Instance,
> Token->OverrideData)) {
> +Status = EFI_INVALID_PARAMETER;
> +gBS->RestoreTPL (OldTpl);
> +return Status;
> +  }
> +
>//
>// Set the Operation now to prevent the application start other
>// operations.
>//
>Instance->Operation = Operation;
>Override= Token->OverrideData;
> 
> -  if ((Override != NULL) && !Mtftp4OverrideValid (Instance, Override)) {
> -Status = EFI_INVALID_PARAMETER;
> -goto ON_ERROR;
> -  }
> -
>if (Token->OptionCount != 0) {
>  Status = Mtftp4ParseOption (
> Token->OptionList,
> Token->OptionCount,
> TRUE,
> >RequestOption
> );
> 
>  if (EFI_ERROR (Status)) {
> +  Status = EFI_DEVICE_ERROR;
>goto ON_ERROR;
>  }
>}
> 
>//
> @@ -482,10 +484,11 @@ Mtftp4Start (
>// Config the unicast UDP child to send initial request
>//
>Status = Mtftp4ConfigUnicastPort (Instance->UnicastPort, Instance);
> 
>if (EFI_ERROR (Status)) {
> +Status = EFI_DEVICE_ERROR;
>  goto ON_ERROR;
>}
> 
>//
>// Set initial status.
> @@ -499,17 +502,17 @@ Mtftp4Start (
>  Status = Mtftp4WrqStart (Instance, Operation);
>} else {
>  Status = Mtftp4RrqStart (Instance, Operation);
>}
> 
> -  gBS->RestoreTPL (OldTpl);
> -
>if (EFI_ERROR (Status)) {
> +Status = EFI_DEVICE_ERROR;
>  goto ON_ERROR;
>}
> 
>if (Token->Event != NULL) {
> +gBS->RestoreTPL (OldTpl);
>  return EFI_SUCCESS;
>}
> 
>//
>// Return immediately for asynchronous operation or poll the
> @@ -517,10 +520,11 @@ Mtftp4Start (
>//
>while (Token->Status == EFI_NOT_READY) {
>  This->Poll (This);
>}
> 
> +  gBS->RestoreTPL (OldTpl);
>return Token->Status;
> 
>  ON_ERROR:
>Mtftp4CleanOperation (Instance, Status);
>gBS->RestoreTPL (OldTpl);
> --
> 1.9.5.msysgit.1

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

Re: [edk2] Memory space entry is not removed after calling FreeMemorySpace and RemoveMemorySpace

[Wasim Khan]

Hi Star,

I agree with both the use cases.

I think only 'gDS->FreeMemorySpace' need to be updated, currently it directly 
sets the 'ImageHandle  = NULL' instead it should do below.
- If the memory is not in use by drivers, do the cleanup and set the 
'ImageHandle  = NULL'
- If the memory is in use by drivers, do not set the 'ImageHandle  = NULL' and 
return error like 'Memory is in use'.

I think no need to change anything for 'gDS->RemoveMemorySpace' call, as it 
returns error (EFI_ACCESS_DENIED) if the ImageHandle != NULL. So it will 
automatically pass for case1 and fails for case2.


Regards,
Wasim

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 7:16 PM
> To: Wasim Khan ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> Subject: RE: Memory space entry is not removed after calling FreeMemorySpace
> and RemoveMemorySpace
> 
> There are two cases here.
> 
> Case 1:
> 1. gDS->AddMemorySpace
>   The  memory range is automatically allocated for use by the UEFI memory
> services.
> 2. No any driver allocates memory at this memory range by gBS-
> >AllocatePages()/AllocatePool().
> 3. gDS->FreeMemorySpace
> 4. gDS->RemoveMemorySpace
> 
> Case 2:
> 1. gDS->AddMemorySpace
>   The  memory range is automatically allocated for use by the UEFI memory
> services.
> 2. There are driver(s) allocate memory at this memory range by gBS-
> >AllocatePages()/AllocatePool.
> 3. gDS->FreeMemorySpace
> 4. gDS->RemoveMemorySpace
> 
> For case 1, the memory range could be removed safely from UEFI memory map
> at step 3, and step 3 and 4 could return success.
> For case 2, the memory range could be not removed from UEFI memory map at
> step3 as the memory range is still been used by drivers, then step 3 and 4 
> should
> return failure.
> 
> 
> Thanks,
> Star
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wasim Khan
> Sent: Monday, January 8, 2018 8:48 PM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: Re: [edk2] Memory space entry is not removed after calling
> FreeMemorySpace and RemoveMemorySpace
> 
> Hi Star,
> 
> I agree with you that 'gDS->AddMemorySpace' should always pass, but if these
> functions are made to return the Status, we should ideally check the Status 
> and
> take necessary actions upon failure.
> 
> After going through the code, if a memory space for
> EfiGcdMemoryTypeSystemMemory (or EfiGcdMemoryTypeMoreReliable) is
> added then memory  range is automatically allocated for use by the UEFI
> memory services by adding a memory descriptor to gMemoryMap .
> But when we call 'gDS->FreeMemorySpace' and ''gDS->RemoveMemorySpace' ,
> memory space is only removed from GCD map and it is not removed from UEFI
> Services map.
> 
> So ideally while when a call to gDS->AddMemorySpace (for memory type
> EfiGcdMemoryTypeSystemMemory) automatically allocates it for use by UEFI
> memory services. It should also free the memory when 'gDS-
> >FreeMemorySpace' call is made.
> 
> I have opened a ticket for this issue
> (https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzill
> a.tianocore.org%2Fshow_bug.cgi%3Fid%3D841=02%7C01%7Cwasim.kha
> n%40nxp.com%7Cd45b6709b297491dba9b08d5569e20f7%7C686ea1d3bc2b4c6
> fa92cd99c5c301635%7C0%7C1%7C636510159548814134=B2yvF5RzoRJa
> IeNJejOR8hedKCJygbkCCkYieTNFXzw%3D=0) .
> 
> 
> Regards,
> Wasim
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, January 08, 2018 6:37 AM
> > To: Wasim Khan ; edk2-devel@lists.01.org
> > Cc: Gao, Liming ; Zeng, Star
> > 
> > Subject: RE: Memory space entry is not removed after calling
> > FreeMemorySpace and RemoveMemorySpace
> >
> > Hi Wasim,
> >
> > Got the point.
> >
> > Yes, gDS->AddMemorySpace should return success normally, you may can
> > assume it.
> > Or a little tricky method if you only want the memory space to be used
> > by OS kernel, but not post, I think you may can hook the
> > gBS->GetMemoryMap() with your own GetMemoryMap() function, it will
> > call original GetMemoryMap() function, and then append one entry for
> > the memory space you want to add for OS kernel.
> >
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Wasim Khan [mailto:wasim.k...@nxp.com]
> > Sent: Monday, January 8, 2018 3:03 AM
> > To: Zeng, Star ; edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: RE: Memory space entry is not removed after calling
> > FreeMemorySpace and RemoveMemorySpace
> >
> > Hi Star,
> >
> > Thank you for your reply.
> >
> > I cannot add memory space as EfiGcdMemoryTypeReserved because for my
> > use case, i am exposing memory space of an extended DDR memory to
> > kernel. If I add this memory space 

Re: [edk2] [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after close it.

[Fu, Siyuan]

Hi, Jiaxin,

The patch is good to me.

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, January 9, 2018 10:56 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after
> close it.
> 
> There are two place to close the ISCSI ExitBootServiceEvent:
> One is IScsiOnExitBootService callback function.
> Another is ISCSI driver stop() function.
> 
> When OS loader triggers ExitBootServiceEvent, firstly, the exit boot
> service
> callback function will close and free the ExitBootServiceEvent, then
> secondly
> the system will call ISCSI driver stop() function, the
> ExitBootServiceEvent
> will be closed and freed again, the use-after-free memory access happens.
> 
> This issue is recorded at
> https://bugzilla.tianocore.org/show_bug.cgi?id=742.
> This patch is to resolve the issue.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin 
> 
> *** BLURB HERE ***
> 
> Jiaxin Wu (2):
>   MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close
> it.
>   NetworkPkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.
> 
>  MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c | 12 
>  NetworkPkg/IScsiDxe/IScsiMisc.c | 12 
>  2 files changed, 16 insertions(+), 8 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] MdeModulePkg/PciBus: Change switch-case to if-else to fix EBC build

[Ruiyu Ni]

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;
   }
 }
   }
-- 
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 edk2-platforms v2 1/2] SATA : Added SATA controller driver.

[Meenakshi Aggarwal]

> -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
> >
> > diff --git a/Platform/NXP/Drivers/SataInitDxe/SataInit.c
> b/Platform/NXP/Drivers/SataInitDxe/SataInit.c
> > new file mode 100644
> > index 000..bac390b
> > --- /dev/null
> > +++ b/Platform/NXP/Drivers/SataInitDxe/SataInit.c
> > @@ -0,0 +1,285 @@
> > +/** @file
> > +  This driver module adds SATA controller support.
> > +
> > +  Copyright 2017 NXP
> > +
> > +  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
> > +
> https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fope
> nsource.org%2Flicenses%2Fbsd-
> license.php=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7C1bf888
> dbc6b34f8646fe08d556a93d5a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0
> %7C0%7C636510207254570536=hU2o5igZuy5SDt5emEUmAqhSn1gW9
> H40OgvmH8gMn9k%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 "SataInit.h"
> > +
> > +STATIC VOID*mDriverEventRegistration;
> > +
> > +/**
> > +  Read AHCI Operation register.
> > +
> > +  @param  PciIoThe PCI IO protocol instance.
> > +  @param  Offset   The operation register offset.
> > +
> > +  @return  The register content read.
> > +**/
> > +
> > +UINT32
> > +EFIAPI
> > +AhciReadReg (
> > +  IN  EFI_PCI_IO_PROTOCOL  *PciIo,
> > +  IN  UINT32   Offset
> > +  )
> > +{
> > +  UINT32   Data;
> > +
> > +  ASSERT (PciIo != NULL);
> > +
> > +  Data = 0;
> > +
> > +  PciIo->Mem.Read (
> > +  PciIo,
> > +  EfiPciIoWidthUint32,
> > +  AHCI_BAR_INDEX,
> > +  (UINT64) Offset,
> > +  1,
> > +  
> > +  );
> > +
> > +  return Data;
> > +}
> > +
> > +/**
> > +  Write AHCI Operation register.
> > +
> > +  @param PciIo The PCI IO protocol instance.
> > +  @param OffsetThe operation register offset.
> > +  @param Data  The data used to write down.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +AhciWriteReg (
> > +  IN EFI_PCI_IO_PROTOCOL   *PciIo,
> > +  IN UINT32Offset,
> > +  IN UINT32Data
> > +  )
> > +{
> > +  ASSERT (PciIo != NULL);
> > +
> > +  PciIo->Mem.Write (
> > +   PciIo,
> > +   EfiPciIoWidthUint32,
> > +   AHCI_BAR_INDEX,
> > +   (UINT64) Offset,
> > +   1,
> > +   
> > +   );
> > +
> > +  return;
> > +}
> > +
> > +STATIC
> > +VOID
> > +PciIoRegistrationEvent (
> > +  IN  EFI_EVENTEvent,
> > +  IN  VOID *Context
> > +  )
> > +{
> > +  EFI_STATUS   Status;
> > +  UINTNHandleCount;
> > +  UINTNAddress;
> > +  UINT32   Count;
> > +  UINT32   Data;
> > +  UINT8PciClass;
> > +  UINT8PciSubClass;
> > +  EFI_PCI_IO_PROTOCOL  *PciIo;
> > +  EFI_HANDLE   *HandleBuf;
> > +
> > +  PciIo = 

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

[Meenakshi Aggarwal]

> -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.

> > diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> > index 169cef0..23b46ad 100644
> > --- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> > +++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> > @@ -142,6 +142,18 @@ READ_LOCK_STATUS   = TRUE
> >
> >INF
> MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntime
> Dxe.inf
> >
> > +  #
> > +  # AHCI Support
> > +  #
> > +  INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > +  INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
> > +  INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
> > +  INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> > +  INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
> > +  INF
> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePci
> DeviceDxe.inf
> > +
> > +  INF edk2-platforms/Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf
> > +
> 
> Same here
> 
> ># FAT filesystem + GPT/MBR partitioning
> >#
> >INF
> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> > diff --git
> a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> > index 13a0ffb..002294e 100644
> > ---
> a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> > +++
> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> > @@ -68,3 +68,5 @@
> >gNxpQoriqLsTokenSpaceGuid.PcdDram3Size
> >gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr
> >gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize
> > +  gNxpQoriqLsTokenSpaceGuid.PcdDcsrBaseAddr
> > +  gNxpQoriqLsTokenSpaceGuid.PcdDcsrSize
> > diff --git
> a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > index 7022528..4b04ff5 100644
> > --- a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > +++
> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> > @@ -49,6 +49,8 @@
> >  

Re: [edk2] [Patch 1/2] MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.

[Ni, Ruiyu]

On 1/9/2018 10:56 AM, Jiaxin Wu wrote:

There are two place to close the ISCSI ExitBootServiceEvent:
One is IScsiOnExitBootService callback function.
Another is ISCSI driver stop() function.

When OS loader triggers ExitBootServiceEvent, firstly, the exit boot service
callback function will close and free the ExitBootServiceEvent, then secondly
the system will call ISCSI driver stop() function, the ExitBootServiceEvent
will be closed and freed again, the use-after-free memory access happens.

This issue is recorded at https://bugzilla.tianocore.org/show_bug.cgi?id=742.
This patch is to resolve the issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
  MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c | 12 
  1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c 
b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
index ae202c3..29dfe94 100644
--- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
+++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
@@ -1,9 +1,9 @@
  /** @file
Miscellaneous routines for iSCSI driver.
  
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.

+Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
  This program and the accompanying materials
  are licensed and made available under the terms and conditions of the BSD 
License
  which accompanies this distribution.  The full text of the license may be 
found at
  http://opensource.org/licenses/bsd-license.php
  
@@ -622,13 +622,14 @@ IScsiCleanDriverData (

  >IScsiExtScsiPassThru
  );
}
  
  EXIT:

-
-  gBS->CloseEvent (Private->ExitBootServiceEvent);
-
+  if (Private->ExitBootServiceEvent != NULL) {
+gBS->CloseEvent (Private->ExitBootServiceEvent);
+  }
+
FreePool (Private);
return Status;
  }
  
  /**

@@ -870,12 +871,15 @@ IScsiOnExitBootService (
)
  {
ISCSI_DRIVER_DATA *Private;
  
Private = (ISCSI_DRIVER_DATA *) Context;

+
gBS->CloseEvent (Private->ExitBootServiceEvent);
  
+  Private->ExitBootServiceEvent = NULL;

+
IScsiSessionAbort (>Session);
  }
  
  /**

Tests whether a controller handle is being managed by IScsi driver.



Jiaxin,
I agree your patch can fix the use-after-free issue in your network
driver.
But I think a more root fix should be in CoreCloseEvent().
You could refer to CoreValidateHandle(). It looks for the
Handle in handle database for a match before deferencing it.


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

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

[Ni, Ruiyu]

On 1/8/2018 7:16 PM, Meenakshi Aggarwal wrote:

Description:
Commands with 4 MB PRD length entries fail if PRD[DBC] is
set to the value according to AHCI standard spec.
Due to a logic error, 3F_h is misinterpreted by the
device as zero length.


Is the logic error mentioned here is the error in HW?
Then I do not prefer to add such PCD for a HW workaround.



Workaround:
Set PRD length to 0 when creating a PRD entry for
a maximum data transfer size of 4 MB to fix the erratum.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c   | 2 +-
  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 1 +
  MdeModulePkg/MdeModulePkg.dec  | 3 +++
  3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
index e6de5d6..fb6dc0b 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
@@ -591,7 +591,7 @@ AhciBuildCommand (
  if (RemainedData < EFI_AHCI_MAX_DATA_PER_PRDT) {
AhciRegisters->AhciCommandTable->PrdtTable[PrdtIndex].AhciPrdtDbc = 
(UINT32)RemainedData - 1;
  } else {
-  AhciRegisters->AhciCommandTable->PrdtTable[PrdtIndex].AhciPrdtDbc = 
EFI_AHCI_MAX_DATA_PER_PRDT - 1;
+  AhciRegisters->AhciCommandTable->PrdtTable[PrdtIndex].AhciPrdtDbc = 
PcdGet32 (PcdPrdtMaxDataLength);
  }
  
  Data64.Uint64 = (UINT64)MemAddr;

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
index 82d5f7a..8921dd5 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
@@ -70,6 +70,7 @@
  
  [Pcd]

gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable   ## SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength
  
  # [Event]

  # EVENT_TYPE_PERIODIC_TIMER ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 8efad57..b2f9f2b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1434,6 +1434,9 @@
# @Prompt Console Output Row of Text Setup
gEfiMdeModulePkgTokenSpaceGuid.PcdSetupConOutRow|25|UINT32|0x400e
  
+  ## This PCD specifies the Maximum data length for a PRD Entry

+  
gEfiMdeModulePkgTokenSpaceGuid.PcdPrdtMaxDataLength|0x3F|UINT32|0x400f
+
  [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
## UART clock frequency is for the baud rate configuration.
# @Prompt Serial Port Clock Rate.




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

[edk2] [PATCH 2/4] UefiCpuPkg/MtrrLib: Fix bug that may calculate wrong MTRR result

[Ruiyu Ni]

Code forgot to initialize the optional weight between adjacent
vertices. It caused wrong MTRR result was calculated for some
memory settings.

The logic was incorrectly removed when converting from POC
code. The patch adds back the initialization.

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

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index b83d768c5f..566a4cb67b 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -1583,20 +1583,33 @@ MtrrLibCalculateMtrrs (
   Vector[VectorCount - 1].Address = Base1;
 
   Weight = (UINT8 *) [VectorCount];
-  //
-  // Set mandatory weight between any vector to max
-  // Set optional weight and between any vector and self->self to 0
-  // E.g.:
-  //   00 FF FF FF
-  //   00 00 FF FF
-  //   00 00 00 FF
-  //   00 00 00 00
-  //
   for (VectorIndex = 0; VectorIndex < VectorCount; VectorIndex++) {
+//
+// Set optional weight between vertices and self->self to 0
+//
 SetMem ([M(VectorIndex, 0)], VectorIndex + 1, 0);
-if (VectorIndex != VectorCount - 1) {
-  Weight[M (VectorIndex, VectorIndex + 1)] = (DefaultType == 
Vector[VectorIndex].Type) ? 0 : 1;
-  SetMem ([M (VectorIndex, VectorIndex + 2)], VectorCount - 
VectorIndex - 2, MAX_WEIGHT);
+//
+// Set mandatory weight between vectors to MAX_WEIGHT
+//
+SetMem ([M (VectorIndex, VectorIndex + 1)], VectorCount - 
VectorIndex - 1, MAX_WEIGHT);
+
+// Final result looks like:
+//   00 FF FF FF
+//   00 00 FF FF
+//   00 00 00 FF
+//   00 00 00 00
+  }
+
+  //
+  // Set mandatory weight and optional weight for adjacent vertices
+  //
+  for (VectorIndex = 0; VectorIndex < VectorCount - 1; VectorIndex++) {
+if (Vector[VectorIndex].Type != DefaultType) {
+  Weight[M (VectorIndex, VectorIndex + 1)] = 1;
+  Weight[O (VectorIndex, VectorIndex + 1)] = 0;
+} else {
+  Weight[M (VectorIndex, VectorIndex + 1)] = 0;
+  Weight[O (VectorIndex, VectorIndex + 1)] = 1;
 }
   }
 
-- 
2.15.1.windows.2

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

[edk2] [PATCH 3/4] UefiCpuPkg/MtrrLib: Handle one setting request covering all memory

[Ruiyu Ni]

*SetMemoryAttribute*() API cannot handle the setting request that
looks like <0, MAX_ADDRESS, Type>. The buggy parameter checking
logic returns Unsupported for this case.
The patch fixes the checking logic to handle such case.

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

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 566a4cb67b..54f703606b 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -2270,8 +2270,13 @@ MtrrSetMemoryAttributesInMtrrSettings (
   goto Exit;
 }
 if (((Ranges[Index].BaseAddress & ~MtrrValidAddressMask) != 0) ||
-((Ranges[Index].Length & ~MtrrValidAddressMask) != 0)
+Ranges[Index].BaseAddress + Ranges[Index].Length) & 
~MtrrValidAddressMask) != 0) &&
+  (Ranges[Index].BaseAddress + Ranges[Index].Length) != 
MtrrValidBitsMask + 1)
 ) {
+  //
+  // Either the BaseAddress or the Limit doesn't follow the alignment 
requirement.
+  // Note: It's still valid if Limit doesn't follow the alignment 
requirement but equals to MAX Address.
+  //
   Status = RETURN_UNSUPPORTED;
   goto Exit;
 }
-- 
2.15.1.windows.2

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

[edk2] [PATCH 4/4] UefiCpuPkg/MtrrLib: Correct typo to change vector to vertex

[Ruiyu Ni]

The patch only change the comments and variable name so
doesn't impact the functionality.

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

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 54f703606b..eb46861163 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -1,7 +1,7 @@
 /** @file
   MTRR setting library
 
-  @par Note: 
+  @par Note:
 Most of services in this library instance are suggested to be invoked by 
BSP only,
 except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to 
APs.
 
@@ -32,8 +32,8 @@
 #define SCRATCH_BUFFER_SIZE   (4 * SIZE_4KB)
 #define MTRR_LIB_ASSERT_ALIGNED(B, L) ASSERT ((B & ~(L - 1)) == B);
 
-#define M(x,y) ((x) * VectorCount + (y))
-#define O(x,y) ((y) * VectorCount + (x))
+#define M(x,y) ((x) * VertexCount + (y))
+#define O(x,y) ((y) * VertexCount + (x))
 
 //
 // Context to save and restore when MTRRs are programmed
@@ -1142,21 +1142,21 @@ MtrrLibGetNumberOfTypes (
 }
 
 /**
-  Calculate the least MTRR number from vector Start to Stop and update
-  the Previous of all vectors from Start to Stop is updated to reflect
+  Calculate the least MTRR number from vertex Start to Stop and update
+  the Previous of all vertices from Start to Stop is updated to reflect
   how the memory range is covered by MTRR.
 
-  @param VectorCount The count of vectors in the graph.
-  @param Vector  Array holding all vectors.
-  @param Weight  2-dimention array holding weights between vectors.
-  @param Start   Start vector.
-  @param StopStop vector.
+  @param VertexCount The count of vertices in the graph.
+  @param VerticesArray holding all vertices.
+  @param Weight  2-dimention array holding weights between vertices.
+  @param Start   Start vertex.
+  @param StopStop vertex.
   @param IncludeOptional TRUE to count the optional weight.
 **/
 VOID
 MtrrLibCalculateLeastMtrrs (
-  IN UINT16  VectorCount,
-  IN MTRR_LIB_ADDRESS*Vector,
+  IN UINT16  VertexCount,
+  IN MTRR_LIB_ADDRESS*Vertices,
   IN OUT CONST UINT8 *Weight,
   IN UINT16  Start,
   IN UINT16  Stop,
@@ -1170,52 +1170,52 @@ MtrrLibCalculateLeastMtrrs (
   UINT8  Optional;
 
   for (Index = Start; Index <= Stop; Index++) {
-Vector[Index].Visited = FALSE;
-Vector[Index].Previous = VectorCount;
+Vertices[Index].Visited = FALSE;
+Vertices[Index].Previous = VertexCount;
 Mandatory = Weight[M(Start,Index)];
-Vector[Index].Weight = Mandatory;
+Vertices[Index].Weight = Mandatory;
 if (Mandatory != MAX_WEIGHT) {
   Optional = IncludeOptional ? Weight[O(Start, Index)] : 0;
-  Vector[Index].Weight += Optional;
-  ASSERT (Vector[Index].Weight >= Optional);
+  Vertices[Index].Weight += Optional;
+  ASSERT (Vertices[Index].Weight >= Optional);
 }
   }
 
   MinI = Start;
   MinWeight = 0;
-  while (!Vector[Stop].Visited) {
+  while (!Vertices[Stop].Visited) {
 //
-// Update the weight from the shortest vector to other unvisited vectors
+// Update the weight from the shortest vertex to other unvisited vertices
 //
 for (Index = Start + 1; Index <= Stop; Index++) {
-  if (!Vector[Index].Visited) {
+  if (!Vertices[Index].Visited) {
 Mandatory = Weight[M(MinI, Index)];
 if (Mandatory != MAX_WEIGHT) {
   Optional = IncludeOptional ? Weight[O(MinI, Index)] : 0;
-  if (MinWeight + Mandatory + Optional <= Vector[Index].Weight) {
-Vector[Index].Weight   = MinWeight + Mandatory + Optional;
-Vector[Index].Previous = MinI; // Previous is Start based.
+  if (MinWeight + Mandatory + Optional <= Vertices[Index].Weight) {
+Vertices[Index].Weight   = MinWeight + Mandatory + Optional;
+Vertices[Index].Previous = MinI; // Previous is Start based.
   }
 }
   }
 }
 
 //
-// Find the shortest vector from Start
+// Find the shortest vertex from Start
 //
-MinI  = VectorCount;
+MinI  = VertexCount;
 MinWeight = MAX_WEIGHT;
 for (Index = Start + 1; Index <= Stop; Index++) {
-  if (!Vector[Index].Visited && MinWeight > Vector[Index].Weight) {
+  if (!Vertices[Index].Visited && MinWeight > Vertices[Index].Weight) {
 MinI  = Index;
-MinWeight = Vector[Index].Weight;
+MinWeight = Vertices[Index].Weight;
   }
 }
 
 //
-// Mark the shortest vector from Start as visited
+// Mark the shortest 

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

[Ruiyu Ni]

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] [PATCH 1/4] UefiCpuPkg/MtrrLib: Refine the debug messages

[Ruiyu Ni]

MtrrSetMemoryAttributesInMtrrSettings() missed the debug messages
of memory attribute request and status. The patch moves all debug
messages from MtrrSetMemoryAttributeInMtrrSettings() to
MtrrSetMemoryAttributesInMtrrSettings() and refines the debug message
to carry more information.

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

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index f37b740fdf..b83d768c5f 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -5,7 +5,7 @@
 Most of services in this library instance are suggested to be invoked by 
BSP only,
 except for MtrrSetAllMtrrs() which is used to sync BSP's MTRR setting to 
APs.
 
-  Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2008 - 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
@@ -1570,7 +1570,7 @@ MtrrLibCalculateMtrrs (
   //
   VectorCount = VectorIndex + 1;
   DEBUG ((
-DEBUG_CACHE, "VectorCount (%016lx - %016lx) = %d\n", 
+DEBUG_CACHE, "  VectorCount (%016lx - %016lx) = %d\n",
 Ranges[0].BaseAddress, Ranges[RangeCount - 1].BaseAddress + 
Ranges[RangeCount - 1].Length, VectorCount
 ));
   ASSERT (VectorCount < MAX_UINT16);
@@ -2209,6 +2209,7 @@ MtrrSetMemoryAttributesInMtrrSettings (
   MTRR_CONTEXT  MtrrContext;
   BOOLEAN   MtrrContextValid;
 
+  Status = RETURN_SUCCESS;
   MtrrLibInitializeMtrrMask (, );
 
   //
@@ -2226,24 +2227,48 @@ MtrrSetMemoryAttributesInMtrrSettings (
   Above1MbExist = FALSE;
   OriginalVariableMtrrCount = 0;
 
+  //
+  // 0. Dump the requests.
+  //
+  DEBUG_CODE (
+DEBUG ((DEBUG_CACHE, "Mtrr: Set Mem Attribute to %a, ScratchSize = %x%a",
+(MtrrSetting == NULL) ? "Hardware" : "Buffer", ScratchSize,
+(RangeCount <= 1) ? "," : "\n"
+));
+for (Index = 0; Index < RangeCount; Index++) {
+  DEBUG ((DEBUG_CACHE, " %a: [%016lx, %016lx)\n",
+  mMtrrMemoryCacheTypeShortName[MIN (Ranges[Index].Type, 
CacheInvalid)],
+  Ranges[Index].BaseAddress, Ranges[Index].BaseAddress + 
Ranges[Index].Length
+  ));
+}
+  );
+
   //
   // 1. Validate the parameters.
   //
+  if (!IsMtrrSupported ()) {
+Status = RETURN_UNSUPPORTED;
+goto Exit;
+  }
+
   for (Index = 0; Index < RangeCount; Index++) {
 if (Ranges[Index].Length == 0) {
-  return RETURN_INVALID_PARAMETER;
+  Status = RETURN_INVALID_PARAMETER;
+  goto Exit;
 }
 if (((Ranges[Index].BaseAddress & ~MtrrValidAddressMask) != 0) ||
 ((Ranges[Index].Length & ~MtrrValidAddressMask) != 0)
 ) {
-  return RETURN_UNSUPPORTED;
+  Status = RETURN_UNSUPPORTED;
+  goto Exit;
 }
 if ((Ranges[Index].Type != CacheUncacheable) &&
 (Ranges[Index].Type != CacheWriteCombining) &&
 (Ranges[Index].Type != CacheWriteThrough) &&
 (Ranges[Index].Type != CacheWriteProtected) &&
 (Ranges[Index].Type != CacheWriteBack)) {
-  return RETURN_INVALID_PARAMETER;
+  Status = RETURN_INVALID_PARAMETER;
+  goto Exit;
 }
 if (Ranges[Index].BaseAddress + Ranges[Index].Length > BASE_1MB) {
   Above1MbExist = TRUE;
@@ -2309,7 +2334,7 @@ MtrrSetMemoryAttributesInMtrrSettings (
   if (Status == RETURN_ALREADY_STARTED) {
 Status = RETURN_SUCCESS;
   } else if (Status == RETURN_OUT_OF_RESOURCES) {
-return Status;
+goto Exit;
   } else {
 ASSERT_RETURN_ERROR (Status);
 Modified = TRUE;
@@ -2327,7 +2352,7 @@ MtrrSetMemoryAttributesInMtrrSettings (
  WorkingVariableMtrr, FirmwareVariableMtrrCount + 1, 

  );
   if (RETURN_ERROR (Status)) {
-return Status;
+goto Exit;
   }
 
   //
@@ -2346,7 +2371,8 @@ MtrrSetMemoryAttributesInMtrrSettings (
   }
 
   if (WorkingVariableMtrrCount > FirmwareVariableMtrrCount) {
-return RETURN_OUT_OF_RESOURCES;
+Status = RETURN_OUT_OF_RESOURCES;
+goto Exit;
   }
 
   //
@@ -2375,7 +2401,7 @@ MtrrSetMemoryAttributesInMtrrSettings (
Ranges[Index].BaseAddress, Ranges[Index].Length, 
Ranges[Index].Type
);
 if (RETURN_ERROR (Status)) {
-  return Status;
+  goto Exit;
 }
   }
 
@@ -2441,7 +2467,12 @@ MtrrSetMemoryAttributesInMtrrSettings (
 }
   }
 
-  return RETURN_SUCCESS;
+Exit:
+  DEBUG ((DEBUG_CACHE, "  Result = %r\n", Status));
+  if 

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

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Feng, YunhuaX 
Sent: Tuesday, January 09, 2018 9:56 AM
To: edk2-devel@lists.01.org
Cc: Feng, YunhuaX ; Zhu, Yonghong 
; Gao, Liming 
Subject: [PATCH] BaseTools: Correct Target Path in CodaTargetList replace Path

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

[edk2] [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after close it.

[Jiaxin Wu]

There are two place to close the ISCSI ExitBootServiceEvent:
One is IScsiOnExitBootService callback function.
Another is ISCSI driver stop() function.

When OS loader triggers ExitBootServiceEvent, firstly, the exit boot service
callback function will close and free the ExitBootServiceEvent, then secondly
the system will call ISCSI driver stop() function, the ExitBootServiceEvent
will be closed and freed again, the use-after-free memory access happens.

This issue is recorded at https://bugzilla.tianocore.org/show_bug.cgi?id=742.
This patch is to resolve the issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 

*** BLURB HERE ***

Jiaxin Wu (2):
  MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close
it.
  NetworkPkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.

 MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c | 12 
 NetworkPkg/IScsiDxe/IScsiMisc.c | 12 
 2 files changed, 16 insertions(+), 8 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/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.

[Jiaxin Wu]

There are two place to close the ISCSI ExitBootServiceEvent:
One is IScsiOnExitBootService callback function.
Another is ISCSI driver stop() function.

When OS loader triggers ExitBootServiceEvent, firstly, the exit boot service
callback function will close and free the ExitBootServiceEvent, then secondly
the system will call ISCSI driver stop() function, the ExitBootServiceEvent
will be closed and freed again, the use-after-free memory access happens.

This issue is recorded at https://bugzilla.tianocore.org/show_bug.cgi?id=742.
This patch is to resolve the issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c 
b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
index ae202c3..29dfe94 100644
--- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
+++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
@@ -1,9 +1,9 @@
 /** @file
   Miscellaneous routines for iSCSI driver.
 
-Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -622,13 +622,14 @@ IScsiCleanDriverData (
 >IScsiExtScsiPassThru
 );
   }
 
 EXIT:
-
-  gBS->CloseEvent (Private->ExitBootServiceEvent);
-
+  if (Private->ExitBootServiceEvent != NULL) {
+gBS->CloseEvent (Private->ExitBootServiceEvent);
+  }
+  
   FreePool (Private);
   return Status;
 }
 
 /**
@@ -870,12 +871,15 @@ IScsiOnExitBootService (
   )
 {
   ISCSI_DRIVER_DATA *Private;
 
   Private = (ISCSI_DRIVER_DATA *) Context;
+  
   gBS->CloseEvent (Private->ExitBootServiceEvent);
 
+  Private->ExitBootServiceEvent = NULL;
+
   IScsiSessionAbort (>Session);
 }
 
 /**
   Tests whether a controller handle is being managed by IScsi driver.
-- 
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] NetworkPkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.

[Jiaxin Wu]

There are two place to close the ISCSI ExitBootServiceEvent:
One is IScsiOnExitBootService callback function.
Another is ISCSI driver stop() function.

When OS loader triggers ExitBootServiceEvent, firstly, the exit boot service
callback function will close and free the ExitBootServiceEvent, then secondly
the system will call ISCSI driver stop() function, the ExitBootServiceEvent
will be closed and freed again, the use-after-free memory access happens.

This issue is recorded at https://bugzilla.tianocore.org/show_bug.cgi?id=742.
This patch is to resolve the issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/IScsiDxe/IScsiMisc.c | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index 9e4164c..9b26147 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -1,9 +1,9 @@
 /** @file
   Miscellaneous routines for iSCSI driver.
 
-Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -1796,12 +1796,13 @@ IScsiCleanDriverData (
   mPrivate->OneSessionEstablished = FALSE;
 }
   }
 
 EXIT:
-
-  gBS->CloseEvent (Private->ExitBootServiceEvent);
+  if (Private->ExitBootServiceEvent != NULL) {
+gBS->CloseEvent (Private->ExitBootServiceEvent); 
+  }
 
   mCallbackInfo->Current = NULL;
 
   FreePool (Private);
   return Status;
@@ -2483,12 +2484,15 @@ IScsiOnExitBootService (
   )
 {
   ISCSI_DRIVER_DATA *Private;
 
   Private = (ISCSI_DRIVER_DATA *) Context;
+  
   gBS->CloseEvent (Private->ExitBootServiceEvent);
-
+  
+  Private->ExitBootServiceEvent = NULL;
+  
   if (Private->Session != NULL) {
 IScsiSessionAbort (Private->Session);
   }
 }
 
-- 
1.9.5.msysgit.1

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

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

[Chema Gonzalez]

Done.

-Chema


On Mon, Jan 8, 2018 at 5:42 PM, Gao, Liming  wrote:
> Chema:
>   Sorry for late response. I think the change is good. For windows and gcc, 
> how about use the same error message for the unknown or unsupported arch?
>
> +else
> +$(error Bad HOST_ARCH)
> endif
>
> +!ELSE
> +!ERROR "Unknown HOST_ARCH variable"
>  !ENDIF
>
> Thanks
> Liming
>>-Original Message-
>>From: che...@gmail.com [mailto:che...@gmail.com] On Behalf Of Chema
>>Gonzalez
>>Sent: Friday, January 05, 2018 3:48 AM
>>To: Gao, Liming 
>>Cc: edk2-devel@lists.01.org
>>Subject: Re: FW: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in
>>C Makefile (#122)
>>
>>Added similar changes to `BaseTools/Source/C/Makefiles/header.makefile`.
>>
>>Thanks,
>>-Chema
>>
>>
>>On Fri, Dec 29, 2017 at 7:17 AM, Gao, Liming  wrote:
>>> I suggest GNUmakefile also adds this condition to report error message if
>>HOST_ARCH is not set correctly.
>>>
 -Original Message-
 From: che...@gmail.com [mailto:che...@gmail.com] On Behalf Of
>>Chema Gonzalez
 Sent: Friday, December 29, 2017 6:19 AM
 To: edk2-devel@lists.01.org
 Cc: Gao, Liming 
 Subject: Fwd: FW: [tianocore/edk2] BaseTools: Barf on unknown
>>HOST_ARCH in C Makefile (#122)

 Sure.

 Thanks,
 -Chema

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


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



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



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

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

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

 

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

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

 Commit Summary

 BaseTools: Barf on unknown HOST_ARCH in C Makefile

 File Changes

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

 Patch Links:

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

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

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

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

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chema Gonzalez 
---
 BaseTools/Source/C/Makefiles/header.makefile | 12 ++--
 BaseTools/Source/C/Makefiles/ms.common   |  6 --
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/BaseTools/Source/C/Makefiles/header.makefile b/BaseTools/Source/C/Makefiles/header.makefile
index 27aa28b..e034da2 100644
--- a/BaseTools/Source/C/Makefiles/header.makefile
+++ b/BaseTools/Source/C/Makefiles/header.makefile
@@ -29,18 +29,18 @@ BUILD_LD ?= ld
 LINKER ?= $(BUILD_CC)
 ifeq ($(HOST_ARCH), IA32)
 ARCH_INCLUDE = -I $(MAKEROOT)/Include/Ia32/
-endif
 
-ifeq ($(HOST_ARCH), X64)
+else ifeq ($(HOST_ARCH), X64)
 ARCH_INCLUDE = -I $(MAKEROOT)/Include/X64/
-endif
 
-ifeq ($(HOST_ARCH), ARM)
+else ifeq ($(HOST_ARCH), ARM)
 ARCH_INCLUDE = -I $(MAKEROOT)/Include/Arm/
-endif
 
-ifeq ($(HOST_ARCH), AARCH64)
+else ifeq ($(HOST_ARCH), AARCH64)
 ARCH_INCLUDE = -I $(MAKEROOT)/Include/AArch64/
+
+else
+$(error Bad HOST_ARCH)
 endif
 
 INCLUDE = $(TOOL_INCLUDE) -I $(MAKEROOT) -I $(MAKEROOT)/Include/Common -I $(MAKEROOT)/Include/ -I $(MAKEROOT)/Include/IndustryStandard -I $(MAKEROOT)/Common/ -I .. -I . $(ARCH_INCLUDE) 
diff --git a/BaseTools/Source/C/Makefiles/ms.common b/BaseTools/Source/C/Makefiles/ms.common
index a6bfea5..d23308c 100644
--- a/BaseTools/Source/C/Makefiles/ms.common
+++ b/BaseTools/Source/C/Makefiles/ms.common
@@ -42,14 +42,16 @@ 

Re: [edk2] [Patch 2/2] MdeModulePkg: Freed packet buffer when error occurs to avoid memory leak.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang Fan
> Sent: Tuesday, January 9, 2018 9:19 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [Patch 2/2] MdeModulePkg: Freed packet buffer when error
> occurs to avoid memory leak.
> 
> * In function Mtftp4WrqSendBlock(), when packet is not needed, function
>   returns EFI_ABORTED but not freed the packet buffer. It results some
>   memory leak and 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/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> index e825714..438659a 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> @@ -1,9 +1,9 @@
>  /** @file
>Routines to process Wrq (upload).
> 
> -Copyright (c) 2006 - 2014, 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
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -92,10 +92,14 @@ Mtftp4WrqSendBlock (
>  if (EFI_ERROR (Status) || (DataLen > Instance->BlkSize)) {
>if (DataBuf != NULL) {
>  FreePool (DataBuf);
>}
> 
> +  if (UdpPacket != NULL) {
> +NetbufFree (UdpPacket);
> +  }
> +
>Mtftp4SendError (
>  Instance,
>  EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
>  (UINT8 *) "User aborted the transfer"
>  );
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Gao, Liming]

Chema:
  Sorry for late response. I think the change is good. For windows and gcc, how 
about use the same error message for the unknown or unsupported arch?

+else
+$(error Bad HOST_ARCH)
endif

+!ELSE
+!ERROR "Unknown HOST_ARCH variable"
 !ENDIF

Thanks
Liming
>-Original Message-
>From: che...@gmail.com [mailto:che...@gmail.com] On Behalf Of Chema
>Gonzalez
>Sent: Friday, January 05, 2018 3:48 AM
>To: Gao, Liming 
>Cc: edk2-devel@lists.01.org
>Subject: Re: FW: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in
>C Makefile (#122)
>
>Added similar changes to `BaseTools/Source/C/Makefiles/header.makefile`.
>
>Thanks,
>-Chema
>
>
>On Fri, Dec 29, 2017 at 7:17 AM, Gao, Liming  wrote:
>> I suggest GNUmakefile also adds this condition to report error message if
>HOST_ARCH is not set correctly.
>>
>>> -Original Message-
>>> From: che...@gmail.com [mailto:che...@gmail.com] On Behalf Of
>Chema Gonzalez
>>> Sent: Friday, December 29, 2017 6:19 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Gao, Liming 
>>> Subject: Fwd: FW: [tianocore/edk2] BaseTools: Barf on unknown
>HOST_ARCH in C Makefile (#122)
>>>
>>> Sure.
>>>
>>> Thanks,
>>> -Chema
>>>
>>> -- Forwarded message --
>>> From: Gao, Liming 
>>> Date: Wed, Dec 27, 2017 at 5:48 PM
>>> Subject: FW: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in
>>> C Makefile (#122)
>>> To: "che...@gmail.com" 
>>>
>>>
>>> Could you send patch to edk2-devel@lists.01.org?
>>>
>>>
>>>
>>> From: chemag [mailto:notificati...@github.com]
>>> Sent: Thursday, December 28, 2017 9:26 AM
>>> To: tianocore/edk2 
>>> Cc: Subscribed 
>>> Subject: [tianocore/edk2] BaseTools: Barf on unknown HOST_ARCH in C
>>> Makefile (#122)
>>>
>>>
>>>
>>> I was getting HOST_ARCH set using the linux arch name ("x86_64"), which
>>> is different from the MS one ("X64").
>>>
>>> It is not clear anyway we can proceed without valid build variables
>>> (ARCH_INCLUDE, BIN_PATH, LIB_PATH, SYS_BIN_PATH, and
>>> SYS_LIB_PATH).
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Chema Gonzalez che...@gmail.com
>>>
>>> 
>>>
>>> You can view, comment on, or merge this pull request online at:
>>>
>>>   https://github.com/tianocore/edk2/pull/122
>>>
>>> Commit Summary
>>>
>>> BaseTools: Barf on unknown HOST_ARCH in C Makefile
>>>
>>> File Changes
>>>
>>> M BaseTools/Source/C/Makefiles/ms.common (6)
>>>
>>> Patch Links:
>>>
>>> https://github.com/tianocore/edk2/pull/122.patch
>>> https://github.com/tianocore/edk2/pull/122.diff
>>>
>>> —
>>> You are receiving this because you are subscribed to this thread.
>>> Reply to this email directly, view it on GitHub, or mute the thread.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 2/2] MdeModulePkg: Freed packet buffer when error occurs to avoid memory leak.

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang Fan
> Sent: Tuesday, January 9, 2018 9:19 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [Patch 2/2] MdeModulePkg: Freed packet buffer when error
> occurs to avoid memory leak.
> 
> * In function Mtftp4WrqSendBlock(), when packet is not needed, function
>   returns EFI_ABORTED but not freed the packet buffer. It results some
>   memory leak and 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/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> index e825714..438659a 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
> @@ -1,9 +1,9 @@
>  /** @file
>Routines to process Wrq (upload).
> 
> -Copyright (c) 2006 - 2014, 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
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -92,10 +92,14 @@ Mtftp4WrqSendBlock (
>  if (EFI_ERROR (Status) || (DataLen > Instance->BlkSize)) {
>if (DataBuf != NULL) {
>  FreePool (DataBuf);
>}
> 
> +  if (UdpPacket != NULL) {
> +NetbufFree (UdpPacket);
> +  }
> +
>Mtftp4SendError (
>  Instance,
>  EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
>  (UINT8 *) "User aborted the transfer"
>  );
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 1/2] MdeModulePkg: Fixed two issues when error occurs in Mtftp4Start.

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang Fan
> Sent: Tuesday, January 9, 2018 9:19 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [Patch 1/2] MdeModulePkg: Fixed two issues when error
> occurs in Mtftp4Start.
> 
> * This function sets returned status as token status and signal token
>   when error occurs, and it results token status not compliance with
>   spec definition. This patch fixed this issue.
> * This function restore Tpl twice when Mtftp4WrqStart() returns an
>   error, this patch fixed this issue.
> 
> Cc: Jiaxin Wu 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wang Fan 
> ---
>  .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 20 
> 
>  1 file changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> index 54384e1..f5f9e6d 100644
> --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> @@ -1,10 +1,10 @@
>  /** @file
>Interface routine for Mtftp4.
> 
>  (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> -Copyright (c) 2006 - 2017, 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
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -407,31 +407,33 @@ Mtftp4Start (
>if (EFI_ERROR (Status)) {
>  gBS->RestoreTPL (OldTpl);
>  return Status;
>}
> 
> +  if ((Token->OverrideData != NULL) && !Mtftp4OverrideValid (Instance,
> Token->OverrideData)) {
> +Status = EFI_INVALID_PARAMETER;
> +gBS->RestoreTPL (OldTpl);
> +return Status;
> +  }
> +
>//
>// Set the Operation now to prevent the application start other
>// operations.
>//
>Instance->Operation = Operation;
>Override= Token->OverrideData;
> 
> -  if ((Override != NULL) && !Mtftp4OverrideValid (Instance, Override)) {
> -Status = EFI_INVALID_PARAMETER;
> -goto ON_ERROR;
> -  }
> -
>if (Token->OptionCount != 0) {
>  Status = Mtftp4ParseOption (
> Token->OptionList,
> Token->OptionCount,
> TRUE,
> >RequestOption
> );
> 
>  if (EFI_ERROR (Status)) {
> +  Status = EFI_DEVICE_ERROR;
>goto ON_ERROR;
>  }
>}
> 
>//
> @@ -482,10 +484,11 @@ Mtftp4Start (
>// Config the unicast UDP child to send initial request
>//
>Status = Mtftp4ConfigUnicastPort (Instance->UnicastPort, Instance);
> 
>if (EFI_ERROR (Status)) {
> +Status = EFI_DEVICE_ERROR;
>  goto ON_ERROR;
>}
> 
>//
>// Set initial status.
> @@ -499,17 +502,17 @@ Mtftp4Start (
>  Status = Mtftp4WrqStart (Instance, Operation);
>} else {
>  Status = Mtftp4RrqStart (Instance, Operation);
>}
> 
> -  gBS->RestoreTPL (OldTpl);
> -
>if (EFI_ERROR (Status)) {
> +Status = EFI_DEVICE_ERROR;
>  goto ON_ERROR;
>}
> 
>if (Token->Event != NULL) {
> +gBS->RestoreTPL (OldTpl);
>  return EFI_SUCCESS;
>}
> 
>//
>// Return immediately for asynchronous operation or poll the
> @@ -517,10 +520,11 @@ Mtftp4Start (
>//
>while (Token->Status == EFI_NOT_READY) {
>  This->Poll (This);
>}
> 
> +  gBS->RestoreTPL (OldTpl);
>return Token->Status;
> 
>  ON_ERROR:
>Mtftp4CleanOperation (Instance, Status);
>gBS->RestoreTPL (OldTpl);
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch 2/2] MdeModulePkg: Freed packet buffer when error occurs to avoid memory leak.

[Wang Fan]

* In function Mtftp4WrqSendBlock(), when packet is not needed, function
  returns EFI_ABORTED but not freed the packet buffer. It results some
  memory leak and 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/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c 
b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
index e825714..438659a 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c
@@ -1,9 +1,9 @@
 /** @file
   Routines to process Wrq (upload).
   
-Copyright (c) 2006 - 2014, 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -92,10 +92,14 @@ Mtftp4WrqSendBlock (
 if (EFI_ERROR (Status) || (DataLen > Instance->BlkSize)) {
   if (DataBuf != NULL) {
 FreePool (DataBuf);
   }
 
+  if (UdpPacket != NULL) {
+NetbufFree (UdpPacket);
+  }
+
   Mtftp4SendError (
 Instance,
 EFI_MTFTP4_ERRORCODE_REQUEST_DENIED,
 (UINT8 *) "User aborted the transfer"
 );
-- 
1.9.5.msysgit.1

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

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

[Wang Fan]

See descriptions in each patch.

Wang Fan (2):
  MdeModulePkg: Fixed two issues when error occurs in Mtftp4Start.
  MdeModulePkg: Freed packet buffer when error occurs to avoid memory
leak.

 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 20 
 MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Wrq.c |  6 +-
 2 files changed, 17 insertions(+), 9 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: Fixed two issues when error occurs in Mtftp4Start.

[Wang Fan]

* This function sets returned status as token status and signal token
  when error occurs, and it results token status not compliance with
  spec definition. This patch fixed this issue.
* This function restore Tpl twice when Mtftp4WrqStart() returns an
  error, this patch fixed this issue.

Cc: Jiaxin Wu 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wang Fan 
---
 .../Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c 
b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
index 54384e1..f5f9e6d 100644
--- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
+++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
@@ -1,10 +1,10 @@
 /** @file
   Interface routine for Mtftp4.
   
 (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-Copyright (c) 2006 - 2017, 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
 http://opensource.org/licenses/bsd-license.php
 
@@ -407,31 +407,33 @@ Mtftp4Start (
   if (EFI_ERROR (Status)) {
 gBS->RestoreTPL (OldTpl);
 return Status;
   }
 
+  if ((Token->OverrideData != NULL) && !Mtftp4OverrideValid (Instance, 
Token->OverrideData)) {
+Status = EFI_INVALID_PARAMETER;
+gBS->RestoreTPL (OldTpl);
+return Status;
+  }
+
   //
   // Set the Operation now to prevent the application start other
   // operations.
   //
   Instance->Operation = Operation;
   Override= Token->OverrideData;
 
-  if ((Override != NULL) && !Mtftp4OverrideValid (Instance, Override)) {
-Status = EFI_INVALID_PARAMETER;
-goto ON_ERROR;
-  }
-
   if (Token->OptionCount != 0) {
 Status = Mtftp4ParseOption (
Token->OptionList,
Token->OptionCount,
TRUE,
>RequestOption
);
 
 if (EFI_ERROR (Status)) {
+  Status = EFI_DEVICE_ERROR;
   goto ON_ERROR;
 }
   }
 
   //
@@ -482,10 +484,11 @@ Mtftp4Start (
   // Config the unicast UDP child to send initial request
   //
   Status = Mtftp4ConfigUnicastPort (Instance->UnicastPort, Instance);
 
   if (EFI_ERROR (Status)) {
+Status = EFI_DEVICE_ERROR;
 goto ON_ERROR;
   }
 
   //
   // Set initial status.
@@ -499,17 +502,17 @@ Mtftp4Start (
 Status = Mtftp4WrqStart (Instance, Operation);
   } else {
 Status = Mtftp4RrqStart (Instance, Operation);
   }
 
-  gBS->RestoreTPL (OldTpl);
-
   if (EFI_ERROR (Status)) {
+Status = EFI_DEVICE_ERROR;
 goto ON_ERROR;
   }
 
   if (Token->Event != NULL) {
+gBS->RestoreTPL (OldTpl);
 return EFI_SUCCESS;
   }
 
   //
   // Return immediately for asynchronous operation or poll the
@@ -517,10 +520,11 @@ Mtftp4Start (
   //
   while (Token->Status == EFI_NOT_READY) {
 This->Poll (This);
   }
 
+  gBS->RestoreTPL (OldTpl);
   return Token->Status;
 
 ON_ERROR:
   Mtftp4CleanOperation (Instance, Status);
   gBS->RestoreTPL (OldTpl);
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch] NetworkPkg/HttpDxe: Fix build warning error if CHAR8 is unsigned.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, January 2, 2018 11:34 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Kinney,
> Michael D ; Wu, Jiaxin 
> Subject: [Patch] NetworkPkg/HttpDxe: Fix build warning error if CHAR8 is
> unsigned.
> 
> This patch is to fix the compiler warning error: C4245. The issue will
> happen
> if the below build option is enabled:
>   *_*_*_CC_FLAGS = -J.
> 
> That's because the value of ('A' - 'a') is a negative value, which will
> be converted to an unsigned type if CHAR8 is treated as unsigned:
>   Src -= ('A' - 'a');
> 
> The above issue is also recorded at:
> https://bugzilla.tianocore.org/show_bug.cgi?id=815.
> 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin 
> ---
>  NetworkPkg/HttpDxe/HttpsSupport.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpsSupport.c
> b/NetworkPkg/HttpDxe/HttpsSupport.c
> index e6f4d5a..6aed61a 100644
> --- a/NetworkPkg/HttpDxe/HttpsSupport.c
> +++ b/NetworkPkg/HttpDxe/HttpsSupport.c
> @@ -65,15 +65,15 @@ AsciiStrCaseStr (
>  && (*String != '\0')) {
>Src = *String;
>Dst = *SearchStringTmp;
> 
>if ((Src >= 'A') && (Src <= 'Z')) {
> -Src -= ('A' - 'a');
> +Src += ('a' - 'A');
>}
> 
>if ((Dst >= 'A') && (Dst <= 'Z')) {
> -Dst -= ('A' - 'a');
> +Dst += ('a' - 'A');
>}
> 
>if (Src != Dst) {
>  break;
>}
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing EFIAPI call conv spec

[Laszlo Ersek]

On 01/08/18 22:06, Carsey, Jaben wrote:
> Agree that the internal use is questionable.
> 
> Reviewed-by: Jaben Carsey 
> 
>> -Original Message-
>> From: Palmer, Thomas [mailto:thomas.pal...@hpe.com]
>> Sent: Monday, January 08, 2018 10:07 AM
>> To: Laszlo Ersek ; edk2-devel-01 > de...@lists.01.org>
>> Cc: Carsey, Jaben ; Gao, Liming
>> ; Rebecca Cran ; Ni, Ruiyu
>> 
>> Subject: RE: [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing
>> EFIAPI call conv spec
>> Importance: High
>>
>> Reviewed by thomas.pal...@hpe.com
>>
>>
>> Regards,
>>
>> Thomas Palmer

Commit 038720e89959.

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

Re: [edk2] [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing EFIAPI call conv spec

[Carsey, Jaben]

Agree that the internal use is questionable.

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Palmer, Thomas [mailto:thomas.pal...@hpe.com]
> Sent: Monday, January 08, 2018 10:07 AM
> To: Laszlo Ersek ; edk2-devel-01  de...@lists.01.org>
> Cc: Carsey, Jaben ; Gao, Liming
> ; Rebecca Cran ; Ni, Ruiyu
> 
> Subject: RE: [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing
> EFIAPI call conv spec
> Importance: High
> 
> Reviewed by thomas.pal...@hpe.com
> 
> 
> Regards,
> 
> Thomas Palmer
> 
> "I have only made this letter longer because I have not had the time to make
> it shorter" - Blaise Pascal
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Saturday, January 6, 2018 2:04 PM
> To: edk2-devel-01 
> Cc: Jaben Carsey ; Liming Gao
> ; Rebecca Cran ; Ruiyu Ni
> ; Palmer, Thomas 
> Subject: [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing EFIAPI
> call conv spec
> 
> UefiShellLevel2CommandsLib (somewhat questionably) calls the BaseLib-
> internal function InternalCharToUpper().
> 
> This function is declared in "MdePkg/Library/BaseLib/BaseLibInternals.h",
> which is not a public library class header. UefiShellLevel2CommandsLib
> therefore duplicates the function declaration, but a mistake was made: the
> EFIAPI calling convention is not spelled out on the duplicated declaration.
> Therefore calls made from UefiShellLevel2CommandsLib will not match the
> actual function definition in "MdePkg/Library/BaseLib/String.c",
> when GCC/X64 toolchains are used.
> 
> One consequence of this is that cross-filesystem copies don't work in the
> UEFI shell (see the StrniCmp() function in "UefiShellLevel2CommandsLib.c").
> From the original report:
> 
> > FS0:\efi\ubuntu\> cp grubx64.efi fs1:\
> >
> > cp: The source and destination are the same.
> 
> Copy the declaration from "BaseLibInternals.h" to
> "UefiShellLevel2CommandsLib.c" verbatim.
> 
> Reported-by: Rebecca Cran 
> Analyzed-by: Thomas Palmer 
> Analyzed-by: Liming Gao 
> Ref: http://mid.mail-archive.com/47cd17d8-f022-6ca5-2f52-
> 06a8250f8...@cran.org.uk
> Cc: Jaben Carsey 
> Cc: Liming Gao 
> Cc: Rebecca Cran 
> Cc: Ruiyu Ni 
> Cc: Thomas Palmer 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
> 
> ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib
> .c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> index 7948e53cfc46..e9ce63189224 100644
> ---
> a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Commands
> Lib.c
> +++
> b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command
> +++ sLib.c
> @@ -268,8 +268,9 @@ VerifyIntermediateDirectories (
>@return Char as an upper case character.
>  **/
>  CHAR16
> +EFIAPI
>  InternalCharToUpper (
> -  IN CONST CHAR16Char
> +  IN  CHAR16Char
>);
> 
>  /**
> --
> 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] OvmfPkg/BaseMemEncryptSevLib: Enable protection for newly added page table

[Brijesh Singh]

Hi Laszlo,

On 01/05/2018 05:38 AM, Laszlo Ersek wrote:

Hi Brijesh,

(Adding Ray, based on Jian's and Ray's feedback in another branch of
this thread.)

This time I prefer to write a shorter email:

(1) First of all, congrats on your family :)


Thank you :)



(2) Please file a new TianoCore BZ:
- for UefiCpuPkg (for the time being),
- with the title "create page table library to abstract all page
   table manipulations",
- please assign it to Ray,
- please CC Jian and myself on it,
- please add the edk2-devel archive URL of this email thread to the BZ
- I wonder if the earlier-filed TianoCore BZ
    should be
   associated with this new BZ somehow; e.g. in the "See Also" field.



I will create a new BZ and resubmit the patch soon. thanks
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 12 December 2017 at 10:56, Wu, Hao A  wrote:
> Hi Ard,
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
>> Biesheuvel
>> Sent: Tuesday, December 12, 2017 3:00 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu; Tian, Feng; Ard Biesheuvel; Wu, Hao A; Leif Lindholm; Kinney,
>> Michael D; Zeng, Star
>> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
>>
>> On 7 December 2017 at 22:43, Ard Biesheuvel 
>> wrote:
>> > Many SDHCI implementations exist that are almost spec complicant, and
>> > could be driven by the generic SD/MMC host controller driver except
>> > for some minimal necessary init time tweaks.
>> >
>> > Adding such tweaks to the generic driver is undesirable. On the other
>> > hand, forking the driver for every platform that has such a SDHCI
>> > controller is problematic when it comes to upstreaming and ongoing
>> > maintenance (which is arguably the point of upstreaming in the first
>> > place).
>> >
>> > So these patches propose a workaround that is minimally invasive on the
>> > EDK2 side, but gives platforms a lot of leeway when it comes to applying
>> > SDHCI quirks.
>> >
>> > Changes since v3:
>> > - remove PassThru argument from protocol members: it is unclear whether the
>> >   protocol is available when the override protocol is invoked, and my
>> >   example use case does not need it
>> > - replace incorrect HandleProtocol with LocateProtocol, given that the
>> override
>> >   protocol is now a singleton instance
>> > - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>> >   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
>> argument
>> >   and so the prototypes no longer belong in SdMmcPciHci.h and have been
>> moved
>> >   to SdMmcPciHcDxe.h
>> > - use VOID* type for capability not UINT64* since we don't know its
>> alignment
>> >
>> > Changes since v2:
>> > - use a singleton instance of the SD/MMC protocol rather than one per
>> >   controller; this is needed to support 'reconnect -r', as pointed out
>> >   by Ray
>> > - use EDKII prefixes for all types defined by the protocol
>> > - replace 'hook' with 'notify', and tweak some other identifiers
>> > - add missing function comment headers for factored out functions
>> >
>> > Changes since RFC/v1:
>> > - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
>> > - use UINT64* not VOID* to pass capability structure (which is always 64 
>> > bits
>> >   in size)
>> >
>> > Ard Biesheuvel (2):
>> >   MdeModulePkg: introduce SD/MMC override protocol
>> >   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>> >
>>
>> OK, so I did send my v4 but I couldn't find it in my mail folders :-)
>>
>> Comments anyone?
>
> I still need some time to evaluate whether the current proposed override
> protocol can be utilized in our using scenario.
>
> Will let you know the feedbacks as soon as I finish the evaluation.
>
> Really sorry for the delay.
>

Any news on this?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 15/18] ARM/VExpressPkg: New DP500/DP550/DP650 platform library.

[Evan Lloyd]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: 23 December 2017 16:07
> 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 15/18] ARM/VExpressPkg: New
> DP500/DP550/DP650 platform library.
>
> On 22 December 2017 at 19:08,   wrote:
> > From: Girish Pathak 
> >
...
> > +
> > +/* Internal helper function to allocate memory if memory is not
> > +already
> > +  reserved for framebuffer
> > +
> > +  @param[in]  VramSize  Requested framebuffer size in bytes.
> > +  @param[out] VramBaseAddress   Pointer to memory allocated for
> framebuffer.
> > +
> > +  @retval EFI_SUCCESS   Framebuffer memory allocated successfully.
> > +  @retval EFI_UNSUPPORTED   Allocated address wider than 40 bits.
> > +  @retval !EFI_SUCCESS  Other errors.
> > +**/
> > +STATIC
> > +EFI_STATUS
> > +GetVram (
> > +  IN  UINTN CONST  VramSize,
> > +  OUT EFI_PHYSICAL_ADDRESS *CONST  VramBaseAddress
> > +  )
> > +{
> > +  EFI_STATUS Status;
> > +
> > +  // Check if memory is already reserved for the framebuffer.
> > +#if (FixedPcdGet64 (PcdArmLcdDdrFrameBufferBase) != 0)
> > +
>
> Please don't use CPP conditionals for control flow
>
> > +#if (!DP_VALID_BASE_ADDR (FixedPcdGet64
> > +(PcdArmLcdDdrFrameBufferBase))) #error ARM Mali DP framebuffer
> base address cannot be wider than 40 bits.
> > +#else
> > +  *VramBaseAddress =
> > +(EFI_PHYSICAL_ADDRESS)FixedPcdGet64
> > +(PcdArmLcdDdrFrameBufferBase);
> > +
> > +  Status = EFI_SUCCESS;
> > +#endif
> > +
> > +#else
> > +  // If not already reserved, attempt to allocate the VRAM from the
> DRAM.
> > +  Status = gBS->AllocatePages (
> > +  AllocateAnyPages,
> > +  EfiBootServicesData,
> > +  EFI_SIZE_TO_PAGES (*VramSize),
> > +  VramBaseAddress
> > +  );
> > +  if (EFI_ERROR (Status)) {
> > +DEBUG ((DEBUG_ERROR, "ArmMaliDpLib: Failed to allocate
> framebuffer.\n"));
> > +ASSERT (FALSE);
> > +return Status;
> > +  }
> > +
> > +  // ARM Mali DP framebuffer base address can not be wider than 40 bits.
> > +  if (!DP_VALID_BASE_ADDR (*VramBaseAddress)) {
> > +gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES
> (*VramSize));
> > +ASSERT (DP_VALID_BASE_ADDR (*VramBaseAddress));
> > +return EFI_UNSUPPORTED;
> > +  }
> > +
> > +  // Mark the VRAM as write-combining. The VRAM is inside the DRAM,
> > + which is  // cacheable, for ARM/AArch64 EFI_MEMORY_WC memory is
> actually uncached.
> > +  Status = gDS->SetMemorySpaceAttributes (
> > +  *VramBaseAddress,
> > +  *VramSize,
> > +  EFI_MEMORY_WC
>
> Please add EFI_MEMORY_XP here
>

 [[Evan Lloyd]] We can do that, happily.  However, in looking at this we found 
that the UEFI spec has in "2.3.6 AArch64 Platforms", section "2.3.6.1 Memory 
types":
EFI_MEMORY_XP, ...  
   Not used or defined

Does that suggest we need a minor spec update?

> > +  );
> > +  if (EFI_ERROR (Status)) {
> > +ASSERT (FALSE);
> > +gBS->FreePages (*VramBaseAddress, EFI_SIZE_TO_PAGES
> (*VramSize));
> > +  }
> > +#endif
> > +
> > +  return Status;
> > +}
> > +
> > +/** Allocate VRAM memory in DRAM for the framebuffer
> > +  (unless it is reserved already).
> > +
> > +  The allocated address can be used to set the framebuffer as a base
> > + buffer  address for any layer of the ARM Mali DP.
> > +
> > +  @param[out] VramBaseAddress A pointer to the framebuffer
> address.
> > +  @param[out] VramSizeA pointer to the size of the frame
> > +  buffer in bytes
> > +
> > +  @retval EFI_SUCCESS Frame buffer memory allocation success.
> > +  @retval EFI_UNSUPPORTED Allocated address wider than 40 bits
> > +  @retval !EFI_SUCCESSOther errors.
> > +**/
> > +EFI_STATUS
> > +LcdPlatformGetVram (
> > +  OUT EFI_PHYSICAL_ADDRESS * CONST VramBaseAddress,
> > +  OUT UINTN* CONST VramSize
> > +  )
> > +{
> > +  ASSERT (VramBaseAddress != NULL);
> > +  ASSERT (VramSize != NULL);
> > +
> > +  // Set the VRAM size.
> > +  *VramSize = (UINTN)FixedPcdGet32 (PcdArmLcdDdrFrameBufferSize);
> > +
> > +  return GetVram (*VramSize, VramBaseAddress); }
> > +
> > +/** Return total number of modes supported.
> > +
> > +  Note: Valid mode numbers are 0 to MaxMode - 1  See Section 12.9 of
> > + the UEFI Specification 2.7
> > +
> > +  @retval UINT32 Mode Number.
> > +**/
> > +UINT32
> > +LcdPlatformGetMaxMode (VOID)
> > +{
> > +  return  

Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 4/5] Fix Chapter 4 Typo

[Evan Lloyd]

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: 03 January 2018 16:56
> To: Evan Lloyd ; edk2-devel@lists.01.org
> Cc: "ard.biesheu...@linaro.org"@arm.com;
> "leif.lindh...@linaro.org"@arm.com;
> "matteo.carl...@arm.com"@arm.com; "ler...@redhat.com"@arm.com;
> "liming@intel.com"@arm.com;
> "michael.d.kin...@intel.com"@arm.com;
> "jordan.l.jus...@intel.com"@arm.com; "n...@arm.com"@arm.com
> Subject: Re: [edk2-CCodingStandardsSpecification PATCH 4/5] Fix Chapter 4
> Typo
>
> On 01/03/18 12:22, evan.ll...@arm.com wrote:
> > From: Evan Lloyd 
> >
> > 4.1.3.2 - remove "See" from 'as specified in See "File Heading"'
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Evan Lloyd 
> > ---
> >  4_naming_conventions/README.md | 418 ++--
> >  1 file changed, 209 insertions(+), 209 deletions(-)
>
> Looks like this file underwent a line terminator conversion as well.
>
> If the file is currently not using CRLF, then I agree it should be converted;
> however, the conversion should be please separated from the typo fix.
>
> ... Hmm, the file already seems to use CRLF. So I think the conversion was to
> LF, and must have been unintended.

 [[Evan Lloyd]] It certainly was unintended, and seems to be an artefact of a 
core.autocrlf setting change.  Sorry.

>
> Can you please resubmit without the line terminator changes?
[[Evan Lloyd]] Will do.
>
> Thanks!
> Laszlo
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [edk2-CCodingStandardsSpecification PATCH 5/5] Fix Chapter 5 Typos

[Evan Lloyd]

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: 03 January 2018 17:08
> To: Evan Lloyd ; edk2-devel@lists.01.org
> Cc: "ard.biesheu...@linaro.org"@arm.com;
> "leif.lindh...@linaro.org"@arm.com;
> "matteo.carl...@arm.com"@arm.com; "ler...@redhat.com"@arm.com;
> "liming@intel.com"@arm.com;
> "michael.d.kin...@intel.com"@arm.com;
> "jordan.l.jus...@intel.com"@arm.com; "n...@arm.com"@arm.com
> Subject: Re: [edk2-CCodingStandardsSpecification PATCH 5/5] Fix Chapter 5
> Typos
>
> On 01/03/18 12:22, evan.ll...@arm.com wrote:
> > From: Evan Lloyd 
> >
> > 5.1.1 - Replace "less" with "fewer" (because columns is plural and
> > countable)
> > 5.1.5 - Correct tense.  (because the C specification still defines...)
> > Insert full stop.
> > Insert comma.
> > 5.1.8 - Correct "provided" to "proven".
> > 5.1.9 - remove hanging "This."
> > 5.2.3.1 - replace "is comprised of" with "comprises" (comprise means
> >   "consists of", so "comprised of" is a solecism.
> >   Remove use of tab, as they are forbidden.
> >   Remove -- before date in copyright header (None of the edk2
> >   files have it).
> > 5.4 - Add indent to comment text.
> > 5.6.1.2 - Fix copy/paste text (from UEFI spec).
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Evan Lloyd 
> > ---
> >  5_source_files/52_spacing.md|  9 +
> >  5_source_files/54_code_file_structure.md|  4 ++--
> >  5_source_files/56_declarations_and_types.md |  4 ++--
> >  5_source_files/README.md| 18 +-
> >  4 files changed, 18 insertions(+), 17 deletions(-)
> >
> > diff --git a/5_source_files/52_spacing.md
> > b/5_source_files/52_spacing.md index
> >
> ddeabf7753a8713bf04e143d6e2e9bccf881f691..3c79f4e4ee91bcd4035d6c
> f7d8d3
> > 2f1bb9c7756f 100644
> > --- a/5_source_files/52_spacing.md
> > +++ b/5_source_files/52_spacing.md
> > @@ -249,7 +249,7 @@ And the comment will end with:
> >  **/
> >  ```
> >
> > -The File Heading comment block is comprised of the following
> > sections: File
> > +The File Heading comment block comprises the following sections: File
> >  Description, Copyright, License, and the optional Specification
> > Reference and  Glossary sections.
> >
> > @@ -266,8 +266,9 @@ Glossary sections.
> >  **/
> >  ```
> >
> > -The following example begins each body line with a tab (two spaces).
> > This is -the preferred indentation, but two tabs (four spaces) is also
> acceptable.
> > +The following example begins each body line with an indent (two spaces).
> > +This is the preferred indentation, but a double indent (four spaces)
> > +is also acceptable.
> >
> >   Example
> >
> > @@ -278,7 +279,7 @@ the preferred indentation, but two tabs (four
> spaces) is also acceptable.
> >Detailed description of the file’s contents and other useful
> >information for a person viewing the file for the first time.
> >
> > -  Copyright (C) --20XX, Acme Corporation. All rights reserved.
> > +  Copyright (C) 20XX, Acme 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 diff
> > --git a/5_source_files/54_code_file_structure.md
> > b/5_source_files/54_code_file_structure.md
> > index
> >
> 8cc9f4f61412b07f765d80d7b680c6dd38b838c1..ac999aae99ae9cfd8b6f97
> dc483e
> > 51bfbd7c7a0b 100644
> > --- a/5_source_files/54_code_file_structure.md
> > +++ b/5_source_files/54_code_file_structure.md
> > @@ -68,8 +68,8 @@ these are C files with an extension of "`.c`".
> >
> >  /* Function Definitions */
> >
> > -/* If this is a protocol definition, the -protocol structure is
> > defined and initialized here.
> > +/* If this is a protocol definition, the protocol structure is
> > +defined and
> > +  initialized here.
> >  */
> >  ```
>
> So, I'm a bit hesitant about this. In edk2 we use /* comments */ (to my
> knowledge, that is) only when they have to be embedded in replacement
> texts of function-like macros, or in expressions that continue after the
> comment on the same line. IOW, multi-line /* comments */ are mostly
> unused, and I'm unsure if we should prepend a "*" to "initialized here".
>

 [[Evan Lloyd]] You are quite correct.  However, this patch only addresses 
typos in the existing document.
There are a number of inconsistencies in the comment formats used throughout, 
but they would need a patch with "actual" changes, not just typo corrections.

> On the other hand, the comments in this section look like they should be
> fully replaced by actual code. In that sense the comment style we use here
> does not matter.
>
>
> >
> > diff --git a/5_source_files/56_declarations_and_types.md
> > b/5_source_files/56_declarations_and_types.md
> > index
> >
> 

Re: [edk2] [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing EFIAPI call conv spec

[Palmer, Thomas]

Reviewed by thomas.pal...@hpe.com


Regards,

Thomas Palmer

"I have only made this letter longer because I have not had the time to make it 
shorter" - Blaise Pascal

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Saturday, January 6, 2018 2:04 PM
To: edk2-devel-01 
Cc: Jaben Carsey ; Liming Gao ; 
Rebecca Cran ; Ruiyu Ni ; Palmer, 
Thomas 
Subject: [PATCH] ShellPkg/UefiShellLevel2CommandsLib: add missing EFIAPI call 
conv spec

UefiShellLevel2CommandsLib (somewhat questionably) calls the BaseLib-internal 
function InternalCharToUpper().

This function is declared in "MdePkg/Library/BaseLib/BaseLibInternals.h",
which is not a public library class header. UefiShellLevel2CommandsLib 
therefore duplicates the function declaration, but a mistake was made: the 
EFIAPI calling convention is not spelled out on the duplicated declaration. 
Therefore calls made from UefiShellLevel2CommandsLib will not match the actual 
function definition in "MdePkg/Library/BaseLib/String.c",
when GCC/X64 toolchains are used.

One consequence of this is that cross-filesystem copies don't work in the UEFI 
shell (see the StrniCmp() function in "UefiShellLevel2CommandsLib.c"). From the 
original report:

> FS0:\efi\ubuntu\> cp grubx64.efi fs1:\
>
> cp: The source and destination are the same.

Copy the declaration from "BaseLibInternals.h" to 
"UefiShellLevel2CommandsLib.c" verbatim.

Reported-by: Rebecca Cran 
Analyzed-by: Thomas Palmer 
Analyzed-by: Liming Gao 
Ref: 
http://mid.mail-archive.com/47cd17d8-f022-6ca5-2f52-06a8250f8d14@cran.org.uk
Cc: Jaben Carsey 
Cc: Liming Gao 
Cc: Rebecca Cran 
Cc: Ruiyu Ni 
Cc: Thomas Palmer 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c | 3 
++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c 
b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
index 7948e53cfc46..e9ce63189224 100644
--- a/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2CommandsLib.c
+++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/UefiShellLevel2Command
+++ sLib.c
@@ -268,8 +268,9 @@ VerifyIntermediateDirectories (
   @return Char as an upper case character.
 **/
 CHAR16
+EFIAPI
 InternalCharToUpper (
-  IN CONST CHAR16Char
+  IN  CHAR16Char
   );
 
 /**
--
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 v2 2/2] LS1046 : Enable support of SATA controller

[Ard Biesheuvel]

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.

> diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf 
> b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> index 169cef0..23b46ad 100644
> --- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> +++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
> @@ -142,6 +142,18 @@ READ_LOCK_STATUS   = TRUE
>
>INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
>
> +  #
> +  # AHCI Support
> +  #
> +  INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> +  INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
> +  INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
> +  INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
> +  INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
> +  INF 
> MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
> +
> +  INF edk2-platforms/Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf
> +

Same here

># FAT filesystem + GPT/MBR partitioning
>#
>INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
> diff --git 
> a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf 
> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> index 13a0ffb..002294e 100644
> --- a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> +++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
> @@ -68,3 +68,5 @@
>gNxpQoriqLsTokenSpaceGuid.PcdDram3Size
>gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr
>gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize
> +  gNxpQoriqLsTokenSpaceGuid.PcdDcsrBaseAddr
> +  gNxpQoriqLsTokenSpaceGuid.PcdDcsrSize
> diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c 
> b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> index 7022528..4b04ff5 100644
> --- a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> +++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
> @@ -49,6 +49,8 @@
>  #define DRAM3_SIZEFixedPcdGet64 (PcdDram3Size)
>  #define QSPI_REGION_BASE_ADDR FixedPcdGet64 (PcdQspiRegionBaseAddr)
>  #define QSPI_REGION_SIZE  FixedPcdGet64 (PcdQspiRegionSize)
> +#define DCSR_BASE_ADDRFixedPcdGet64 (PcdDcsrBaseAddr)
> +#define DCSR_SIZE FixedPcdGet64 (PcdDcsrSize)
>
>
>  /**
> @@ -169,6 +171,12 @@ ArmPlatformGetVirtualMemoryMap (
>VirtualMemoryTable[Index].Length   = QSPI_REGION_SIZE;
>VirtualMemoryTable[Index].Attributes   = 
> ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
>
> +  // DCSR Space
> +  VirtualMemoryTable[++Index].PhysicalBase = DCSR_BASE_ADDR;
> +  VirtualMemoryTable[Index].VirtualBase  = DCSR_BASE_ADDR;
> +  VirtualMemoryTable[Index].Length   = DCSR_SIZE;
> +  VirtualMemoryTable[Index].Attributes   = 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +
>// End of Table
>VirtualMemoryTable[++Index].PhysicalBase = 0;
>VirtualMemoryTable[Index].VirtualBase  = 0;
> diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc b/Silicon/NXP/LS1046A/LS1046A.dsc
> index 4e7230a..33c57ad 100644
> --- a/Silicon/NXP/LS1046A/LS1046A.dsc
> +++ b/Silicon/NXP/LS1046A/LS1046A.dsc
> @@ -74,5 

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

[Ard Biesheuvel]

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
>
> diff --git a/Platform/NXP/Drivers/SataInitDxe/SataInit.c 
> b/Platform/NXP/Drivers/SataInitDxe/SataInit.c
> new file mode 100644
> index 000..bac390b
> --- /dev/null
> +++ b/Platform/NXP/Drivers/SataInitDxe/SataInit.c
> @@ -0,0 +1,285 @@
> +/** @file
> +  This driver module adds SATA controller support.
> +
> +  Copyright 2017 NXP
> +
> +  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
> +  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 "SataInit.h"
> +
> +STATIC VOID*mDriverEventRegistration;
> +
> +/**
> +  Read AHCI Operation register.
> +
> +  @param  PciIoThe PCI IO protocol instance.
> +  @param  Offset   The operation register offset.
> +
> +  @return  The register content read.
> +**/
> +
> +UINT32
> +EFIAPI
> +AhciReadReg (
> +  IN  EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN  UINT32   Offset
> +  )
> +{
> +  UINT32   Data;
> +
> +  ASSERT (PciIo != NULL);
> +
> +  Data = 0;
> +
> +  PciIo->Mem.Read (
> +  PciIo,
> +  EfiPciIoWidthUint32,
> +  AHCI_BAR_INDEX,
> +  (UINT64) Offset,
> +  1,
> +  
> +  );
> +
> +  return Data;
> +}
> +
> +/**
> +  Write AHCI Operation register.
> +
> +  @param PciIo The PCI IO protocol instance.
> +  @param OffsetThe operation register offset.
> +  @param Data  The data used to write down.
> +
> +**/
> +VOID
> +EFIAPI
> +AhciWriteReg (
> +  IN EFI_PCI_IO_PROTOCOL   *PciIo,
> +  IN UINT32Offset,
> +  IN UINT32Data
> +  )
> +{
> +  ASSERT (PciIo != NULL);
> +
> +  PciIo->Mem.Write (
> +   PciIo,
> +   EfiPciIoWidthUint32,
> +   AHCI_BAR_INDEX,
> +   (UINT64) Offset,
> +   1,
> +   
> +   );
> +
> +  return;
> +}
> +
> +STATIC
> +VOID
> +PciIoRegistrationEvent (
> +  IN  EFI_EVENTEvent,
> +  IN  VOID *Context
> +  )
> +{
> +  EFI_STATUS   Status;
> +  UINTNHandleCount;
> +  UINTNAddress;
> +  UINT32   Count;
> +  UINT32   Data;
> +  UINT8PciClass;
> +  UINT8PciSubClass;
> +  EFI_PCI_IO_PROTOCOL  *PciIo;
> +  EFI_HANDLE   *HandleBuf;
> +
> +  PciIo = NULL;
> +
> +  Status = gBS->LocateHandleBuffer (
> +  ByProtocol,
> +  ,
> +  NULL,
> +  ,
> +  );
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "Sata controller is not able to locate 
> gEfiPciIoProtocolGuid 0x%x\n",
> +Status));
> +return;
> +  }
> +
> +  for (Count = 0; Count < HandleCount; Count++) {
> +Status = gBS->OpenProtocol (
> +HandleBuf[Count],
> +,
> +(VOID **) ,
> +NULL,
> +NULL,
> +EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +if (EFI_ERROR (Status)) {
> +  continue;
> +}
> +
> +//
> +// Now further check the PCI header: Base class (offset 0x0B) and
> +// Sub Class (offset 0x0A). This controller should be an Ide controller
> +//
> +Status = PciIo->Pci.Read (
> +  PciIo,
> + 

Re: [edk2] Memory space entry is not removed after calling FreeMemorySpace and RemoveMemorySpace

[Zeng, Star]

There are two cases here.

Case 1:
1. gDS->AddMemorySpace
  The  memory range is automatically allocated for use by the UEFI memory 
services.
2. No any driver allocates memory at this memory range by 
gBS->AllocatePages()/AllocatePool().
3. gDS->FreeMemorySpace
4. gDS->RemoveMemorySpace

Case 2:
1. gDS->AddMemorySpace
  The  memory range is automatically allocated for use by the UEFI memory 
services.
2. There are driver(s) allocate memory at this memory range by 
gBS->AllocatePages()/AllocatePool.
3. gDS->FreeMemorySpace
4. gDS->RemoveMemorySpace

For case 1, the memory range could be removed safely from UEFI memory map at 
step 3, and step 3 and 4 could return success.
For case 2, the memory range could be not removed from UEFI memory map at step3 
as the memory range is still been used by drivers, then step 3 and 4 should 
return failure.


Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wasim 
Khan
Sent: Monday, January 8, 2018 8:48 PM
To: Zeng, Star ; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: Re: [edk2] Memory space entry is not removed after calling 
FreeMemorySpace and RemoveMemorySpace

Hi Star,

I agree with you that 'gDS->AddMemorySpace' should always pass, but if these 
functions are made to return the Status, we should ideally check the Status and 
take necessary actions upon failure.

After going through the code, if a memory space for 
EfiGcdMemoryTypeSystemMemory (or EfiGcdMemoryTypeMoreReliable) is added then 
memory  range is automatically allocated for use by the UEFI memory services by 
adding a memory descriptor to gMemoryMap .
But when we call 'gDS->FreeMemorySpace' and ''gDS->RemoveMemorySpace' , memory 
space is only removed from GCD map and it is not removed from UEFI Services map.

So ideally while when a call to gDS->AddMemorySpace (for memory type 
EfiGcdMemoryTypeSystemMemory) automatically allocates it for use by UEFI memory 
services. It should also free the memory when 'gDS->FreeMemorySpace' call is 
made.

I have opened a ticket for this issue 
(https://bugzilla.tianocore.org/show_bug.cgi?id=841) .


Regards,
Wasim

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 6:37 AM
> To: Wasim Khan ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> 
> Subject: RE: Memory space entry is not removed after calling 
> FreeMemorySpace and RemoveMemorySpace
> 
> Hi Wasim,
> 
> Got the point.
> 
> Yes, gDS->AddMemorySpace should return success normally, you may can 
> assume it.
> Or a little tricky method if you only want the memory space to be used 
> by OS kernel, but not post, I think you may can hook the 
> gBS->GetMemoryMap() with your own GetMemoryMap() function, it will 
> call original GetMemoryMap() function, and then append one entry for 
> the memory space you want to add for OS kernel.
> 
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Wasim Khan [mailto:wasim.k...@nxp.com]
> Sent: Monday, January 8, 2018 3:03 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: RE: Memory space entry is not removed after calling 
> FreeMemorySpace and RemoveMemorySpace
> 
> Hi Star,
> 
> Thank you for your reply.
> 
> I cannot add memory space as EfiGcdMemoryTypeReserved because for my 
> use case, i am exposing memory space of an extended DDR memory to 
> kernel. If I add this memory space as EfiGcdMemoryTypeReserved, then 
> linux is not able to allocate memory from this region to applications 
> (a simple application requesting memory from this region fails). So I 
> have to add the memory space as EfiGcdMemoryTypeSystemMemory only.
> 
> > What is the usage that the memory space needs to be added first and 
> > removed
> later?
> No specific use case, I am checking the status of 
> 'gDS->AddMemorySpace' and 'gDS->SetMemorySpaceAttributes', if any of 
> these calls fails, I want to remove the added memory space. Ideally it should 
> work.
> 
> 
> 
> Regards,
> Wasim
> 
> 
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Friday, January 05, 2018 4:35 PM
> > To: Wasim Khan ; edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Gao, Liming 
> > 
> > Subject: RE: Memory space entry is not removed after calling 
> > FreeMemorySpace and RemoveMemorySpace
> >
> > PI spec has clear description below in AddMemorySpace().
> >
> > "If the memory range specified by BaseAddress and Length is of type 
> > EfiGcdMemoryTypeSystemMemory or EfiGcdMemoryTypeMoreReliable, then
> the
> > memory range may be *automatically allocated for use by the UEFI 
> > memory services*."
> >
> > But PI spec has no clear description about removing the use from the 
> > UEFI memory services in 

[edk2] efi application to update the proc microcode

[Raphael G]

Hello,

We are currently attempting to find an 'OS-independent' way of applying 
some updated microcode early during the efi boot sequence (note we 
cannot flash the motherboard firmware directly since we have not yet 
been provided with the latest constructor firmwares)


Would it somehow be possible, for motherboards booting in uefi mode, to 
chain an efi application that would apply some microcode bin (in a uefi 
capsule or not) on flight before chaining to the next efi app (the os one) ?


Or do you have any other generic way to do this, regardless of the final 
operating system ?


We have found some interesting hints in edk2 framework, that seem 
promising (IntelSiliconPkg)


https://github.com/tianocore/edk2/tree/master/IntelSiliconPkg

but so far we have not managed to make it work, so any help/solution 
would be great :)


If possible we do not want to persist anything in any non volatile 
storage at all, just apply it at the right time (we commented the flags 
persist_across_reset+trigger_reset in the capsule fdf for this, not sure 
this was the right thing to do)


# #

workflow example: whatever.efi -> chain microcodeupdate.efi (apply 
ucode) -> chain final_os.efi


#!ipxe
echo Fetch images and capsule
imgfetch http://${server}/microcode_efi/MicrocodeUpdateDxe.efi
imgfetch http://${server}/microcode_efi/MICROCODECAPSULE.Cap
chain MicrocodeUpdateDxe.efi MICROCODECAPSULE.Cap || goto fail

[...]


Regards,


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

Re: [edk2] Memory space entry is not removed after calling FreeMemorySpace and RemoveMemorySpace

[Wasim Khan]

Hi Star,

I agree with you that 'gDS->AddMemorySpace' should always pass, but if these 
functions are made to 
return the Status, we should ideally check the Status and take necessary 
actions upon failure.

After going through the code, if a memory space for 
EfiGcdMemoryTypeSystemMemory (or EfiGcdMemoryTypeMoreReliable) is 
added then memory  range is automatically allocated for use by the UEFI memory 
services by adding a memory descriptor to gMemoryMap .
But when we call 'gDS->FreeMemorySpace' and ''gDS->RemoveMemorySpace' , memory 
space is only removed from GCD map and it is not 
removed from UEFI Services map.

So ideally while when a call to gDS->AddMemorySpace (for memory type 
EfiGcdMemoryTypeSystemMemory) 
automatically allocates it for use by UEFI memory services. It should also free 
the memory when 'gDS->FreeMemorySpace' call is made.

I have opened a ticket for this issue 
(https://bugzilla.tianocore.org/show_bug.cgi?id=841) .


Regards,
Wasim

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 6:37 AM
> To: Wasim Khan ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Zeng, Star 
> Subject: RE: Memory space entry is not removed after calling FreeMemorySpace
> and RemoveMemorySpace
> 
> Hi Wasim,
> 
> Got the point.
> 
> Yes, gDS->AddMemorySpace should return success normally, you may can
> assume it.
> Or a little tricky method if you only want the memory space to be used by OS
> kernel, but not post, I think you may can hook the gBS->GetMemoryMap() with
> your own GetMemoryMap() function, it will call original GetMemoryMap()
> function, and then append one entry for the memory space you want to add for
> OS kernel.
> 
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Wasim Khan [mailto:wasim.k...@nxp.com]
> Sent: Monday, January 8, 2018 3:03 AM
> To: Zeng, Star ; edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: RE: Memory space entry is not removed after calling FreeMemorySpace
> and RemoveMemorySpace
> 
> Hi Star,
> 
> Thank you for your reply.
> 
> I cannot add memory space as EfiGcdMemoryTypeReserved because for my use
> case, i am exposing memory space of an extended DDR memory to kernel. If I
> add this memory space as EfiGcdMemoryTypeReserved, then linux is not able to
> allocate memory from this region to applications (a simple application
> requesting memory from this region fails). So I have to add the memory space 
> as
> EfiGcdMemoryTypeSystemMemory only.
> 
> > What is the usage that the memory space needs to be added first and removed
> later?
> No specific use case, I am checking the status of 'gDS->AddMemorySpace' and
> 'gDS->SetMemorySpaceAttributes', if any of these calls fails, I want to remove
> the added memory space. Ideally it should work.
> 
> 
> 
> Regards,
> Wasim
> 
> 
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Friday, January 05, 2018 4:35 PM
> > To: Wasim Khan ; edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Gao, Liming
> > 
> > Subject: RE: Memory space entry is not removed after calling
> > FreeMemorySpace and RemoveMemorySpace
> >
> > PI spec has clear description below in AddMemorySpace().
> >
> > "If the memory range specified by BaseAddress and Length is of type
> > EfiGcdMemoryTypeSystemMemory or EfiGcdMemoryTypeMoreReliable, then
> the
> > memory range may be *automatically allocated for use by the UEFI
> > memory services*."
> >
> > But PI spec has no clear description about removing the use from the
> > UEFI memory services in FreeMemorySpace() and RemoveMemorySpace().
> > I think it is very hard (may be not possible) as the added memory
> > space may have been used by UEFI memory (page) services for drivers
> > after the memory space is added by AddMemorySpace().
> >
> > What is the usage that the memory space needs to be added first and
> > removed later?
> > Could the memory space be added as EfiGcdMemoryTypeReserved type
> > instead of EfiGcdMemoryTypeSystemMemory type?
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Wasim Khan
> > Sent: Friday, January 5, 2018 5:59 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] Memory space entry is not removed after calling
> > FreeMemorySpace and RemoveMemorySpace
> >
> > Hi All,
> >
> > I am facing a problem that if a add a memory space using 'gDS-
> > >AddMemorySpace'  and then remove it using 'gDS->FreeMemorySpace'
> > followed by 'gDS->RemoveMemorySpace'.
> > I can still see the entry of added memory space in the table shown by
> 'memmap'
> > command.
> >
> > I enabled DEBUG_GCD and as per logs , the entry is removed from
> > GcdMemorySpaceMap , but when I run 'memmap' command which gets the
> > memory map using 

Re: [edk2] [PATCH] MdePkg/PciExpressLib.h: Add missing include of PciExpress21.h

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Monday, January 8, 2018 6:03 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [PATCH] MdePkg/PciExpressLib.h: Add missing include of PciExpress21.h
> 
> PCI_ECAM_ADDRESS() macro is defined in PciExpress21.h so
> always include PciExpress21.h in the library header file.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Liming Gao 
> ---
>  MdePkg/Include/Library/PciExpressLib.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Library/PciExpressLib.h 
> b/MdePkg/Include/Library/PciExpressLib.h
> index f65b44384f..6ce3fbc139 100644
> --- a/MdePkg/Include/Library/PciExpressLib.h
> +++ b/MdePkg/Include/Library/PciExpressLib.h
> @@ -5,7 +5,7 @@
>configuration cycles must be through the 256 MB PCI Express MMIO window 
> whose base address
>is defined by PcdPciExpressBaseAddress.
> 
> -Copyright (c) 2006 - 2017, 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
> @@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
> EXPRESS OR IMPLIED.
>  #ifndef __PCI_EXPRESS_LIB_H__
>  #define __PCI_EXPRESS_LIB_H__
> 
> +#include 
> +
>  /**
>Macro that converts PCI Bus, PCI Device, PCI Function and PCI Register to 
> an
>address that can be passed to the PCI Library functions.
> --
> 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 v3 0/2] Fix wrong address set as Stack Guard for APs

[Laszlo Ersek]

On 01/08/18 06:39, Jian J Wang wrote:
>> 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(-)
> 

series
Reviewed-by: Laszlo Ersek 

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

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

[Meenakshi Aggarwal]

Hi,

We will set PCD value to 0 to support our board and its default value is set to 
maximum data length.

If someone change its value to any other value in their package, then he/she 
must be aware of the consequences.

I cannot say how controller will react if value is other than 0x3f_ and 0 
(in our case).

Thanks,
Meenakshi

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 3:54 PM
> To: Meenakshi Aggarwal ;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> de...@lists.01.org; Dong, Eric 
> Cc: Ni, Ruiyu ; Zeng, Star 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> How will the code work based on your patch if the this PCD is configured to
> other value, for example 0x20/0x30?
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> Sent: Monday, January 8, 2018 5:54 PM
> To: Zeng, Star ; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> 
> Cc: Ni, Ruiyu 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, January 08, 2018 3:18 PM
> > To: Meenakshi Aggarwal ;
> > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > de...@lists.01.org; Dong, Eric 
> > Cc: Ni, Ruiyu ; Zeng, Star 
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > So this PCD needs to be defined as 0x3F or may be 0x40, then
> > it needs to be configured to 0 for your case, right?
> > Could the PCD be configured to other values?
> No, in my case i need to set it to 0 only. No other value is needed.
> >
> > According to the statement you provided, is it possible to handle the
> > case in the device firmware?
> >
> > " Due to a logic error, 3F_h is misinterpreted by the device as
> > zero length."
> >
> No, it can't be handle in device firmware for existing SATA controller.
> There are chances that in future releases of SATA controller it will get 
> fixed in
> RTL, but this change will still be needed for LS2088 board.
> 
> 
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> > Sent: Monday, January 8, 2018 2:26 PM
> > To: Zeng, Star ; ard.biesheu...@linaro.org;
> > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > 
> > Cc: Ni, Ruiyu 
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi Star,
> >
> > Apologies and some correction in my last reply.
> >
> > As per the errata, PRDT Maximum value needs to be set to 0 only  when
> > creating a PRD entry for a maximum data transfer size.
> >
> > So there is no need to replace all occurrences of
> > EFI_AHCI_MAX_DATA_PER_PRDT in file.
> > Just need to change it where we are setting the Data length.
> >
> > I define it to 0x3F, as this is the actual value we are setting
> > and this PCD need to be used only once.
> >
> > I know, its NXP specific patch only, and i try to made changes which
> > will not impact any other package.
> >
> >
> > Thanks,
> > Meenakshi
> >
> > > -Original Message-
> > > From: Meenakshi Aggarwal
> > > Sent: Monday, January 08, 2018 11:25 AM
> > > To: 'Zeng, Star' ; ard.biesheu...@linaro.org;
> > > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > > 
> > > Cc: Ni, Ruiyu 
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Hi,
> > >
> > > I didn't prepare the full patch but will send in next few minutes,
> > >
> > > i  made the very basic changes required to test Errata implementation.
> > >
> > > I will redefine  it to 0x40.
> > > PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> > >
> > > Thanks,
> > > Meenakshi
> > >
> > > > -Original Message-
> > > > From: Zeng, Star [mailto:star.z...@intel.com]
> > > > Sent: Monday, January 08, 2018 11:19 AM
> > > > To: Meenakshi Aggarwal ;
> > > > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > > > de...@lists.01.org; Dong, Eric 
> > > > Cc: Ni, Ruiyu ; Zeng, Star
> > > > 
> > > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > > >
> > > > Do you have a full patch already?
> > > > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not
> > 0x40?
> > > >
> > > >
> > > > Thanks,
> > > > Star
> > > > -Original Message-
> > > > From: edk2-devel 

[edk2] [PATCH edk2-platforms 1/2] USB : Add DWC3 USB controller initialization driver.

[Meenakshi Aggarwal]

Add support of DWC3 controller driver which
Performs DWC3 controller initialization and
Register itself as NonDiscoverableMmioDevice

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c   | 213 ++
 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h   | 142 +
 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf |  48 ++
 Platform/NXP/NxpQoriqLs.dsc   |   9 ++
 4 files changed, 412 insertions(+)
 create mode 100644 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
 create mode 100644 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
 create mode 100644 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf

diff --git a/Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c 
b/Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
new file mode 100644
index 000..179e9b6
--- /dev/null
+++ b/Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
@@ -0,0 +1,213 @@
+/** @file
+
+  Copyright 2017 NXP
+
+  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 "UsbHcd.h"
+
+VOID
+XhciSetBeatBurstLength (
+  IN  UINTN  UsbReg
+  )
+{
+  Dwc3   *Dwc3Reg;
+
+  Dwc3Reg = (VOID *)(UsbReg + DWC3_REG_OFFSET);
+
+  MmioAndThenOr32 ((UINTN)>GSBusCfg0, ~USB3_ENABLE_BEAT_BURST_MASK,
+  USB3_ENABLE_BEAT_BURST);
+  MmioOr32 ((UINTN)>GSBusCfg1, USB3_SET_BEAT_BURST_LIMIT);
+
+  return;
+}
+
+VOID
+Dwc3SetFladj (
+  IN  Dwc3   *Dwc3Reg,
+  IN  UINT32 Val
+  )
+{
+  MmioOr32 ((UINTN)>GFLAdj, GFLADJ_30MHZ_REG_SEL |
+GFLADJ_30MHZ(Val));
+}
+
+VOID
+Dwc3SetMode (
+  IN  Dwc3   *Dwc3Reg,
+  IN  UINT32 Mode
+  )
+{
+  MmioAndThenOr32 ((UINTN)>GCtl,
+   ~(DWC3_GCTL_PRTCAPDIR(DWC3_GCTL_PRTCAP_OTG)),
+   DWC3_GCTL_PRTCAPDIR(Mode));
+}
+
+VOID
+Dwc3CoreSoftReset (
+  IN  Dwc3   *Dwc3Reg
+  )
+{
+  MmioOr32 ((UINTN)>GCtl, DWC3_GCTL_CORESOFTRESET);
+  MmioOr32 ((UINTN)>GUsb3PipeCtl[0], DWC3_GUSB3PIPECTL_PHYSOFTRST);
+  MmioOr32 ((UINTN)>GUsb2PhyCfg, DWC3_GUSB2PHYCFG_PHYSOFTRST);
+  MmioAnd32 ((UINTN)>GUsb3PipeCtl[0], ~DWC3_GUSB3PIPECTL_PHYSOFTRST);
+  MmioAnd32 ((UINTN)>GUsb2PhyCfg, ~DWC3_GUSB2PHYCFG_PHYSOFTRST);
+  MmioAnd32 ((UINTN)>GCtl, ~DWC3_GCTL_CORESOFTRESET);
+
+  return;
+}
+
+EFI_STATUS
+Dwc3CoreInit (
+  IN  Dwc3   *Dwc3Reg
+  )
+{
+  UINT32 Revision;
+  UINT32 Reg;
+  UINTN  Dwc3Hwparams1;
+
+  Revision = MmioRead32 ((UINTN)>GSnpsId);
+  //
+  // This should read as U3 followed by revision number
+  //
+  if ((Revision & DWC3_GSNPSID_MASK) != DWC3_SYNOPSIS_ID) {
+DEBUG ((DEBUG_ERROR,"This is not a DesignWare USB3 DRD Core.\n"));
+return EFI_NOT_FOUND;
+  }
+
+  Dwc3CoreSoftReset (Dwc3Reg);
+
+  Reg = MmioRead32 ((UINTN)>GCtl);
+  Reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
+  Reg &= ~DWC3_GCTL_DISSCRAMBLE;
+
+  Dwc3Hwparams1 = MmioRead32 ((UINTN)>GHwParams1);
+
+  if (DWC3_GHWPARAMS1_EN_PWROPT(Dwc3Hwparams1) == 
DWC3_GHWPARAMS1_EN_PWROPT_CLK) {
+Reg &= ~DWC3_GCTL_DSBLCLKGTNG;
+  } else {
+DEBUG ((DEBUG_ERROR,"No power optimization available.\n"));
+  }
+
+  if ((Revision & DWC3_RELEASE_MASK) < DWC3_RELEASE_190a) {
+Reg |= DWC3_GCTL_U2RSTECN;
+  }
+
+  MmioWrite32 ((UINTN)>GCtl, Reg);
+
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+XhciCoreInit (
+  IN  UINTN  UsbReg
+  )
+{
+  EFI_STATUS Status;
+  Dwc3   *Dwc3Reg;
+
+  Dwc3Reg = (VOID *)(UsbReg + DWC3_REG_OFFSET);
+
+  Status = Dwc3CoreInit (Dwc3Reg);
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Dwc3CoreInit Failed for controller 0x%x (0x%x) \n",
+  UsbReg, Status));
+return Status;
+  }
+
+  Dwc3SetMode (Dwc3Reg, DWC3_GCTL_PRTCAP_HOST);
+
+  Dwc3SetFladj (Dwc3Reg, GFLADJ_30MHZ_DEFAULT);
+
+  return Status;
+}
+
+EFI_STATUS
+EFIAPI
+InitializeUsbController (
+  IN  UINTN  UsbReg
+  )
+{
+  EFI_STATUS Status;
+
+  Status = XhciCoreInit (UsbReg);
+
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  //
+  // Change beat burst and outstanding pipelined transfers requests
+  //
+  XhciSetBeatBurstLength (UsbReg);
+
+  return Status;
+}
+
+/**
+  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
+InitializeUsbHcd (
+  IN EFI_HANDLEImageHandle,
+  IN 

[edk2] [PATCH edk2-platforms 2/2] LS2088 : Enable support of USB controller

[Meenakshi Aggarwal]

Enable support of USB drives on ls2088 board.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Meenakshi Aggarwal 
---
 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc |  1 +
 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf | 13 +
 2 files changed, 14 insertions(+)

diff --git a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc 
b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
index dbc4d33..1d840eb 100755
--- a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
+++ b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc
@@ -93,3 +93,4 @@
   # I2c
   #
   edk2-platforms/Platform/NXP/Drivers/I2cDxe/I2cDxe.inf
+  edk2-platforms/Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
diff --git a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf 
b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
index f5d2f0a..ae3b2e3 100644
--- a/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
+++ b/Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf
@@ -143,6 +143,19 @@ READ_LOCK_STATUS   = TRUE
   INF  MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
 !endif
 
+  INF 
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+
+  #
+  # USB Support
+  #
+  INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
+  INF MdeModulePkg/Bus/Pci/EhciDxe/EhciDxe.inf
+  INF MdeModulePkg/Bus/Pci/XhciDxe/XhciDxe.inf
+  INF MdeModulePkg/Bus/Usb/UsbBusDxe/UsbBusDxe.inf
+  INF MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
+
+  INF edk2-platforms/Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf
+
   #
   # FAT filesystem + GPT/MBR partitioning
   #
-- 
1.9.1

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

[edk2] [PATCH edk2-platforms 0/2] USB Controller Support

[Meenakshi Aggarwal]

Following set of patches add DWC3 (USB) controller driver
and enable USB support on LS2088RDB board.

DWC3 controller driver register USB as NonDiscoverableMmioDevice
of NonDiscoverableDeviceTypeXhci.

Meenakshi Aggarwal (2):
  USB : Add DWC3 USB controller initialization driver.
  LS2088 : Enable support of USB controller

 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c   | 213 ++
 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h   | 142 +
 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf |  48 ++
 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.dsc  |   1 +
 Platform/NXP/LS2088aRdbPkg/LS2088aRdbPkg.fdf  |  13 ++
 Platform/NXP/NxpQoriqLs.dsc   |   9 ++
 6 files changed, 426 insertions(+)
 create mode 100644 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.c
 create mode 100644 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.h
 create mode 100644 Platform/NXP/Drivers/UsbHcdInitDxe/UsbHcd.inf

-- 
1.9.1

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

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

[Zeng, Star]

How will the code work based on your patch if the this PCD is configured to 
other value, for example 0x20/0x30?


Thanks,
Star
-Original Message-
From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com] 
Sent: Monday, January 8, 2018 5:54 PM
To: Zeng, Star ; ard.biesheu...@linaro.org; 
leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 

Cc: Ni, Ruiyu 
Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402


> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal ; 
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> de...@lists.01.org; Dong, Eric 
> Cc: Ni, Ruiyu ; Zeng, Star 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> So this PCD needs to be defined as 0x3F or may be 0x40, then 
> it needs to be configured to 0 for your case, right?
> Could the PCD be configured to other values?
No, in my case i need to set it to 0 only. No other value is needed.
> 
> According to the statement you provided, is it possible to handle the 
> case in the device firmware?
> 
> " Due to a logic error, 3F_h is misinterpreted by the device as 
> zero length."
> 
No, it can't be handle in device firmware for existing SATA controller.
There are chances that in future releases of SATA controller it will get fixed 
in RTL, but this change will still be needed for LS2088 board. 


> 
> Thanks,
> Star
> -Original Message-
> From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star ; ard.biesheu...@linaro.org; 
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> 
> Cc: Ni, Ruiyu 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> Hi Star,
> 
> Apologies and some correction in my last reply.
> 
> As per the errata, PRDT Maximum value needs to be set to 0 only  when 
> creating a PRD entry for a maximum data transfer size.
> 
> So there is no need to replace all occurrences of 
> EFI_AHCI_MAX_DATA_PER_PRDT in file.
> Just need to change it where we are setting the Data length.
> 
> I define it to 0x3F, as this is the actual value we are setting 
> and this PCD need to be used only once.
> 
> I know, its NXP specific patch only, and i try to made changes which 
> will not impact any other package.
> 
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Meenakshi Aggarwal
> > Sent: Monday, January 08, 2018 11:25 AM
> > To: 'Zeng, Star' ; ard.biesheu...@linaro.org; 
> > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> > 
> > Cc: Ni, Ruiyu 
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi,
> >
> > I didn't prepare the full patch but will send in next few minutes,
> >
> > i  made the very basic changes required to test Errata implementation.
> >
> > I will redefine  it to 0x40.
> > PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -Original Message-
> > > From: Zeng, Star [mailto:star.z...@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal ; 
> > > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > > de...@lists.01.org; Dong, Eric 
> > > Cc: Ni, Ruiyu ; Zeng, Star 
> > > 
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not
> 0x40?
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On 
> > > Behalf Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > > de...@lists.01.org; Zeng, Star ; Dong, Eric 
> > > 
> > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Description:
> > > Commands with 4 MB PRD length entries fail if PRD[DBC] is set to 
> > > the value according to AHCI standard spec.
> > > Due to a logic error, 3F_h is misinterpreted by the device as 
> > > zero length.
> > >
> > > Workaround:
> > > Set PRD length to 0 when creating a PRD entry for a maximum data 
> > > transfer size of 4 MB to fix the erratum.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Meenakshi Aggarwal 
> > > ---
> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c   | 2 +-
> > >  

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

[Meenakshi Aggarwal]

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
diff --git a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf 
b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
index 169cef0..23b46ad 100644
--- a/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
+++ b/Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf
@@ -142,6 +142,18 @@ READ_LOCK_STATUS   = TRUE
 
   INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
 
+  #
+  # AHCI Support
+  #
+  INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
+  INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
+  INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
+  INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
+  INF MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
+  INF 
MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+
+  INF edk2-platforms/Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf
+
   # FAT filesystem + GPT/MBR partitioning
   #
   INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf 
b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
index 13a0ffb..002294e 100644
--- a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
+++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/ArmPlatformLib.inf
@@ -68,3 +68,5 @@
   gNxpQoriqLsTokenSpaceGuid.PcdDram3Size
   gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionBaseAddr
   gNxpQoriqLsTokenSpaceGuid.PcdQspiRegionSize
+  gNxpQoriqLsTokenSpaceGuid.PcdDcsrBaseAddr
+  gNxpQoriqLsTokenSpaceGuid.PcdDcsrSize
diff --git a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c 
b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
index 7022528..4b04ff5 100644
--- a/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
+++ b/Platform/NXP/LS1046aRdbPkg/Library/PlatformLib/NxpQoriqLsMem.c
@@ -49,6 +49,8 @@
 #define DRAM3_SIZEFixedPcdGet64 (PcdDram3Size)
 #define QSPI_REGION_BASE_ADDR FixedPcdGet64 (PcdQspiRegionBaseAddr)
 #define QSPI_REGION_SIZE  FixedPcdGet64 (PcdQspiRegionSize)
+#define DCSR_BASE_ADDRFixedPcdGet64 (PcdDcsrBaseAddr)
+#define DCSR_SIZE FixedPcdGet64 (PcdDcsrSize)
 
 
 /**
@@ -169,6 +171,12 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[Index].Length   = QSPI_REGION_SIZE;
   VirtualMemoryTable[Index].Attributes   = 
ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED;
 
+  // DCSR Space
+  VirtualMemoryTable[++Index].PhysicalBase = DCSR_BASE_ADDR;
+  VirtualMemoryTable[Index].VirtualBase  = DCSR_BASE_ADDR;
+  VirtualMemoryTable[Index].Length   = DCSR_SIZE;
+  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+
   // End of Table
   VirtualMemoryTable[++Index].PhysicalBase = 0;
   VirtualMemoryTable[Index].VirtualBase  = 0;
diff --git a/Silicon/NXP/LS1046A/LS1046A.dsc b/Silicon/NXP/LS1046A/LS1046A.dsc
index 4e7230a..33c57ad 100644
--- a/Silicon/NXP/LS1046A/LS1046A.dsc
+++ b/Silicon/NXP/LS1046A/LS1046A.dsc
@@ -74,5 +74,10 @@
   gNxpQoriqLsTokenSpaceGuid.PcdI2c2BaseAddr|0x021A
   gNxpQoriqLsTokenSpaceGuid.PcdI2c3BaseAddr|0x021B
   gNxpQoriqLsTokenSpaceGuid.PcdNumI2cController|4
+  gNxpQoriqLsTokenSpaceGuid.PcdDcsrBaseAddr|0x2000
+  gNxpQoriqLsTokenSpaceGuid.PcdDcsrSize|0x0400
+  gNxpQoriqLsTokenSpaceGuid.PcdSataBaseAddr|0x320
+  gNxpQoriqLsTokenSpaceGuid.PcdSataSize|0x1
+  gNxpQoriqLsTokenSpaceGuid.PcdNumSataController|0x1
 
 ##
-- 
1.9.1

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

[Meenakshi Aggarwal]

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

diff --git a/Platform/NXP/Drivers/SataInitDxe/SataInit.c 
b/Platform/NXP/Drivers/SataInitDxe/SataInit.c
new file mode 100644
index 000..bac390b
--- /dev/null
+++ b/Platform/NXP/Drivers/SataInitDxe/SataInit.c
@@ -0,0 +1,285 @@
+/** @file
+  This driver module adds SATA controller support.
+
+  Copyright 2017 NXP
+
+  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
+  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 "SataInit.h"
+
+STATIC VOID*mDriverEventRegistration;
+
+/**
+  Read AHCI Operation register.
+
+  @param  PciIoThe PCI IO protocol instance.
+  @param  Offset   The operation register offset.
+
+  @return  The register content read.
+**/
+
+UINT32
+EFIAPI
+AhciReadReg (
+  IN  EFI_PCI_IO_PROTOCOL  *PciIo,
+  IN  UINT32   Offset
+  )
+{
+  UINT32   Data;
+
+  ASSERT (PciIo != NULL);
+
+  Data = 0;
+
+  PciIo->Mem.Read (
+  PciIo,
+  EfiPciIoWidthUint32,
+  AHCI_BAR_INDEX,
+  (UINT64) Offset,
+  1,
+  
+  );
+
+  return Data;
+}
+
+/**
+  Write AHCI Operation register.
+
+  @param PciIo The PCI IO protocol instance.
+  @param OffsetThe operation register offset.
+  @param Data  The data used to write down.
+
+**/
+VOID
+EFIAPI
+AhciWriteReg (
+  IN EFI_PCI_IO_PROTOCOL   *PciIo,
+  IN UINT32Offset,
+  IN UINT32Data
+  )
+{
+  ASSERT (PciIo != NULL);
+
+  PciIo->Mem.Write (
+   PciIo,
+   EfiPciIoWidthUint32,
+   AHCI_BAR_INDEX,
+   (UINT64) Offset,
+   1,
+   
+   );
+
+  return;
+}
+
+STATIC
+VOID
+PciIoRegistrationEvent (
+  IN  EFI_EVENTEvent,
+  IN  VOID *Context
+  )
+{
+  EFI_STATUS   Status;
+  UINTNHandleCount;
+  UINTNAddress;
+  UINT32   Count;
+  UINT32   Data;
+  UINT8PciClass;
+  UINT8PciSubClass;
+  EFI_PCI_IO_PROTOCOL  *PciIo;
+  EFI_HANDLE   *HandleBuf;
+
+  PciIo = NULL;
+
+  Status = gBS->LocateHandleBuffer (
+  ByProtocol,
+  ,
+  NULL,
+  ,
+  );
+  if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Sata controller is not able to locate 
gEfiPciIoProtocolGuid 0x%x\n",
+Status));
+return;
+  }
+
+  for (Count = 0; Count < HandleCount; Count++) {
+Status = gBS->OpenProtocol (
+HandleBuf[Count],
+,
+(VOID **) ,
+NULL,
+NULL,
+EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+if (EFI_ERROR (Status)) {
+  continue;
+}
+
+//
+// Now further check the PCI header: Base class (offset 0x0B) and
+// Sub Class (offset 0x0A). This controller should be an Ide controller
+//
+Status = PciIo->Pci.Read (
+  PciIo,
+  EfiPciIoWidthUint8,
+  PCI_CLASSCODE_OFFSET + 2,
+  1,
+  
+  );
+if (EFI_ERROR (Status)) {
+  continue;
+}
+
+Status = PciIo->Pci.Read (
+  PciIo,
+  EfiPciIoWidthUint8,
+  PCI_CLASSCODE_OFFSET + 1,
+  1,
+  
+  );
+if (EFI_ERROR (Status)) {
+  continue;
+

[edk2] [PATCH edk2-platforms v2 0/2] Cover letter:SATA controller support

[Meenakshi Aggarwal]

V2:

1. Pci Emulation layer removed.
2. Made SATA driver as NonDiscoverablePciDevice.
3. Add support of SATA on LS1046RDB board.

Meenakshi Aggarwal (2):
  SATA : Added SATA controller driver.
  LS1046 : Enable support of SATA controller

 Platform/NXP/Drivers/SataInitDxe/SataInit.c| 285 +
 Platform/NXP/Drivers/SataInitDxe/SataInit.h|  36 +++
 Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf   |  52 
 Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.dsc   |   8 +
 Platform/NXP/LS1046aRdbPkg/LS1046aRdbPkg.fdf   |  12 +
 .../Library/PlatformLib/ArmPlatformLib.inf |   2 +
 .../Library/PlatformLib/NxpQoriqLsMem.c|   8 +
 Platform/NXP/NxpQoriqLs.dec|  14 +-
 Platform/NXP/NxpQoriqLs.dsc|  13 +
 Silicon/NXP/LS1046A/LS1046A.dsc|   5 +
 10 files changed, 433 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


V1 :

Following patches add support for pci emulation layer
and SATA on NXP boards.

Null Library for USB is also present to give completeness
to Pci Emulation layer.

Meenakshi Aggarwal (3):
  USB: Added Support of DWC3 USB controller.
  PciEmulation : Add support for Pci Emulation layer.
  SATA : Added SATA controller initialization driver.

 Platform/NXP/Drivers/PciEmulation/PciEmulation.c   | 624 +
 Platform/NXP/Drivers/PciEmulation/PciEmulation.h   | 306 ++
 Platform/NXP/Drivers/PciEmulation/PciEmulation.inf |  54 ++
 .../NXP/Drivers/PciEmulation/PciRootBridgeIo.c | 286 ++
 Platform/NXP/Drivers/SataInitDxe/SataInit.c| 122 
 Platform/NXP/Drivers/SataInitDxe/SataInit.h|  32 ++
 Platform/NXP/Drivers/SataInitDxe/SataInitDxe.inf   |  43 ++
 .../NXP/Library/UsbHcdNullLibrary/UsbHcdLibNull.c  |  25 +
 .../NXP/Library/UsbHcdNullLibrary/UsbHcdNull.inf   |  28 +
 Platform/NXP/NxpQoriqLs.dec|  14 +-
 Platform/NXP/NxpQoriqLs.dsc|  15 +
 11 files changed, 1547 insertions(+), 2 deletions(-)
 create mode 100644 Platform/NXP/Drivers/PciEmulation/PciEmulation.c
 create mode 100755 Platform/NXP/Drivers/PciEmulation/PciEmulation.h
 create mode 100644 Platform/NXP/Drivers/PciEmulation/PciEmulation.inf
 create mode 100644 Platform/NXP/Drivers/PciEmulation/PciRootBridgeIo.c
 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
 create mode 100644 Platform/NXP/Library/UsbHcdNullLibrary/UsbHcdLibNull.c
 create mode 100644 Platform/NXP/Library/UsbHcdNullLibrary

-- 
1.9.1

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

[edk2] [PATCH] MdePkg/PciExpressLib.h: Add missing include of PciExpress21.h

[Ruiyu Ni]

PCI_ECAM_ADDRESS() macro is defined in PciExpress21.h so
always include PciExpress21.h in the library header file.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Liming Gao 
---
 MdePkg/Include/Library/PciExpressLib.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/PciExpressLib.h 
b/MdePkg/Include/Library/PciExpressLib.h
index f65b44384f..6ce3fbc139 100644
--- a/MdePkg/Include/Library/PciExpressLib.h
+++ b/MdePkg/Include/Library/PciExpressLib.h
@@ -5,7 +5,7 @@
   configuration cycles must be through the 256 MB PCI Express MMIO window 
whose base address
   is defined by PcdPciExpressBaseAddress.
 
-Copyright (c) 2006 - 2017, 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
@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #ifndef __PCI_EXPRESS_LIB_H__
 #define __PCI_EXPRESS_LIB_H__
 
+#include 
+
 /**
   Macro that converts PCI Bus, PCI Device, PCI Function and PCI Register to an
   address that can be passed to the PCI Library functions.
-- 
2.15.1.windows.2

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

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

[Meenakshi Aggarwal]

> -Original Message-
> From: Zeng, Star [mailto:star.z...@intel.com]
> Sent: Monday, January 08, 2018 3:18 PM
> To: Meenakshi Aggarwal ;
> ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> de...@lists.01.org; Dong, Eric 
> Cc: Ni, Ruiyu ; Zeng, Star 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> So this PCD needs to be defined as 0x3F or may be 0x40, then it
> needs to be configured to 0 for your case, right?
> Could the PCD be configured to other values?
No, in my case i need to set it to 0 only. No other value is needed.
> 
> According to the statement you provided, is it possible to handle the case in
> the device firmware?
> 
> " Due to a logic error, 3F_h is misinterpreted by the device as zero
> length."
> 
No, it can't be handle in device firmware for existing SATA controller.
There are chances that in future releases of SATA controller it will get fixed 
in RTL,
but this change will still be needed for LS2088 board. 


> 
> Thanks,
> Star
> -Original Message-
> From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com]
> Sent: Monday, January 8, 2018 2:26 PM
> To: Zeng, Star ; ard.biesheu...@linaro.org;
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> 
> Cc: Ni, Ruiyu 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> Hi Star,
> 
> Apologies and some correction in my last reply.
> 
> As per the errata, PRDT Maximum value needs to be set to 0 only  when
> creating a PRD entry for a maximum data transfer size.
> 
> So there is no need to replace all occurrences of
> EFI_AHCI_MAX_DATA_PER_PRDT in file.
> Just need to change it where we are setting the Data length.
> 
> I define it to 0x3F, as this is the actual value we are setting and this 
> PCD
> need to be used only once.
> 
> I know, its NXP specific patch only, and i try to made changes which will not
> impact any other package.
> 
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Meenakshi Aggarwal
> > Sent: Monday, January 08, 2018 11:25 AM
> > To: 'Zeng, Star' ; ard.biesheu...@linaro.org;
> > leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric
> > 
> > Cc: Ni, Ruiyu 
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Hi,
> >
> > I didn't prepare the full patch but will send in next few minutes,
> >
> > i  made the very basic changes required to test Errata implementation.
> >
> > I will redefine  it to 0x40.
> > PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> >
> > Thanks,
> > Meenakshi
> >
> > > -Original Message-
> > > From: Zeng, Star [mailto:star.z...@intel.com]
> > > Sent: Monday, January 08, 2018 11:19 AM
> > > To: Meenakshi Aggarwal ;
> > > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > > de...@lists.01.org; Dong, Eric 
> > > Cc: Ni, Ruiyu ; Zeng, Star 
> > > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Do you have a full patch already?
> > > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not
> 0x40?
> > >
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> > > Of Meenakshi Aggarwal
> > > Sent: Monday, January 8, 2018 7:17 PM
> > > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2-
> > > de...@lists.01.org; Zeng, Star ; Dong, Eric
> > > 
> > > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> > >
> > > Description:
> > > Commands with 4 MB PRD length entries fail if PRD[DBC] is set to the
> > > value according to AHCI standard spec.
> > > Due to a logic error, 3F_h is misinterpreted by the device as
> > > zero length.
> > >
> > > Workaround:
> > > Set PRD length to 0 when creating a PRD entry for a maximum data
> > > transfer size of 4 MB to fix the erratum.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Meenakshi Aggarwal 
> > > ---
> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c   | 2 +-
> > >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 1 +
> > >  MdeModulePkg/MdeModulePkg.dec  | 3 +++
> > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > index e6de5d6..fb6dc0b 100644
> > > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > > @@ -591,7 +591,7 @@ AhciBuildCommand (
> > >  if (RemainedData 

Re: [edk2] [RFC] SATA : Implemented NXP errata A008402

[Zeng, Star]

So this PCD needs to be defined as 0x3F or may be 0x40, then it needs 
to be configured to 0 for your case, right?
Could the PCD be configured to other values?

According to the statement you provided, is it possible to handle the case in 
the device firmware?

" Due to a logic error, 3F_h is misinterpreted by the device as zero 
length."


Thanks,
Star
-Original Message-
From: Meenakshi Aggarwal [mailto:meenakshi.aggar...@nxp.com] 
Sent: Monday, January 8, 2018 2:26 PM
To: Zeng, Star ; ard.biesheu...@linaro.org; 
leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 

Cc: Ni, Ruiyu 
Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402

Hi Star,

Apologies and some correction in my last reply.

As per the errata, PRDT Maximum value needs to be set to 0 only  when creating 
a PRD entry for a maximum data transfer size.

So there is no need to replace all occurrences of EFI_AHCI_MAX_DATA_PER_PRDT in 
file.
Just need to change it where we are setting the Data length.

I define it to 0x3F, as this is the actual value we are setting and this 
PCD need to be used only once.

I know, its NXP specific patch only, and i try to made changes which will not 
impact any other package.


Thanks,
Meenakshi

> -Original Message-
> From: Meenakshi Aggarwal
> Sent: Monday, January 08, 2018 11:25 AM
> To: 'Zeng, Star' ; ard.biesheu...@linaro.org; 
> leif.lindh...@linaro.org; edk2-devel@lists.01.org; Dong, Eric 
> 
> Cc: Ni, Ruiyu 
> Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> 
> Hi,
> 
> I didn't prepare the full patch but will send in next few minutes,
> 
> i  made the very basic changes required to test Errata implementation.
> 
> I will redefine  it to 0x40.
> PcdPrdtMaxDataLength was defined to 0x3F just for testing purpose.
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Zeng, Star [mailto:star.z...@intel.com]
> > Sent: Monday, January 08, 2018 11:19 AM
> > To: Meenakshi Aggarwal ; 
> > ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > de...@lists.01.org; Dong, Eric 
> > Cc: Ni, Ruiyu ; Zeng, Star 
> > Subject: RE: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Do you have a full patch already?
> > Why the PcdPrdtMaxDataLength is defined to 0x3F, but not 0x40?
> >
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Meenakshi Aggarwal
> > Sent: Monday, January 8, 2018 7:17 PM
> > To: ard.biesheu...@linaro.org; leif.lindh...@linaro.org; edk2- 
> > de...@lists.01.org; Zeng, Star ; Dong, Eric 
> > 
> > Subject: [edk2] [RFC] SATA : Implemented NXP errata A008402
> >
> > Description:
> > Commands with 4 MB PRD length entries fail if PRD[DBC] is set to the 
> > value according to AHCI standard spec.
> > Due to a logic error, 3F_h is misinterpreted by the device as 
> > zero length.
> >
> > Workaround:
> > Set PRD length to 0 when creating a PRD entry for a maximum data 
> > transfer size of 4 MB to fix the erratum.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Meenakshi Aggarwal 
> > ---
> >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c   | 2 +-
> >  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf | 1 +
> >  MdeModulePkg/MdeModulePkg.dec  | 3 +++
> >  3 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > index e6de5d6..fb6dc0b 100644
> > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AhciMode.c
> > @@ -591,7 +591,7 @@ AhciBuildCommand (
> >  if (RemainedData < EFI_AHCI_MAX_DATA_PER_PRDT) {
> >AhciRegisters->AhciCommandTable- 
> >PrdtTable[PrdtIndex].AhciPrdtDbc  = (UINT32)RemainedData - 1;
> >  } else {
> > -  AhciRegisters->AhciCommandTable-
> >PrdtTable[PrdtIndex].AhciPrdtDbc
> > = EFI_AHCI_MAX_DATA_PER_PRDT - 1;
> > +  AhciRegisters->AhciCommandTable-
> >PrdtTable[PrdtIndex].AhciPrdtDbc
> > = PcdGet32 (PcdPrdtMaxDataLength);
> >  }
> >
> >  Data64.Uint64 = (UINT64)MemAddr; diff --git
> a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > index 82d5f7a..8921dd5 100644
> > --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
> > @@ -70,6 +70,7 @@
> >
> >  [Pcd]
> >gEfiMdeModulePkgTokenSpaceGuid.PcdAtaSmartEnable   ##
> > SOMETIMES_CONSUMES
> > +