date:20151030

Re: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

2015-10-30 Thread Hegde, Nagaraj P

Can someone help me with commit of this patch?

-Original Message-
From: Ye, Ting [mailto:ting...@intel.com] 
Sent: Thursday, October 29, 2015 6:57 AM
To: Hegde, Nagaraj P; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

Looks good to me. 

Reviewed-by: Ye Ting  

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Nagaraj 
Hegde
Sent: Wednesday, October 28, 2015 4:54 PM
To: edk2-devel@lists.01.org
Cc: Nagaraj Hegde
Subject: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

In EfiHttpRequest, HostName was getting freed twice whenever
HttpTransmitTcp4 failed. Moved FreePool (HostName) after
HttpTransmitTcp4 call to avoid a double free.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Nagaraj Hegde 
---
 NetworkPkg/HttpDxe/HttpImpl.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c 
index 588e1bb..3094400 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -485,10 +485,6 @@ EfiHttpRequest (
 goto Error4;
   }
 
-  if (HostName != NULL) {
-FreePool (HostName);
-  }
-
   //
   // Transmit the request message.
   //
@@ -504,6 +500,10 @@ EfiHttpRequest (
 
   DispatchDpc ();
 
+  if (HostName != NULL) {
+FreePool (HostName);
+  }
+
   return EFI_SUCCESS;
 
 Error5:
--
2.6.2.windows.1

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

Re: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

2015-10-30 Thread Ye, Ting

I will help to commit the patch. Thanks for the reminder.

Thanks,
Ting

-Original Message-
From: Hegde, Nagaraj P [mailto:nagaraj-p.he...@hpe.com] 
Sent: Friday, October 30, 2015 2:09 PM
To: Ye, Ting; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

Can someone help me with commit of this patch?

-Original Message-
From: Ye, Ting [mailto:ting...@intel.com] 
Sent: Thursday, October 29, 2015 6:57 AM
To: Hegde, Nagaraj P; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

Looks good to me. 

Reviewed-by: Ye Ting  

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Nagaraj 
Hegde
Sent: Wednesday, October 28, 2015 4:54 PM
To: edk2-devel@lists.01.org
Cc: Nagaraj Hegde
Subject: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

In EfiHttpRequest, HostName was getting freed twice whenever
HttpTransmitTcp4 failed. Moved FreePool (HostName) after
HttpTransmitTcp4 call to avoid a double free.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Nagaraj Hegde 
---
 NetworkPkg/HttpDxe/HttpImpl.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c 
index 588e1bb..3094400 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -485,10 +485,6 @@ EfiHttpRequest (
 goto Error4;
   }
 
-  if (HostName != NULL) {
-FreePool (HostName);
-  }
-
   //
   // Transmit the request message.
   //
@@ -504,6 +500,10 @@ EfiHttpRequest (
 
   DispatchDpc ();
 
+  if (HostName != NULL) {
+FreePool (HostName);
+  }
+
   return EFI_SUCCESS;
 
 Error5:
--
2.6.2.windows.1

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

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

Re: [edk2] RE enquote PATH: edk2-devel Digest, Vol 4, Issue 214

2015-10-30 Thread Zhu, Yonghong

Hi  Carl,

Could you let me know your environment info, os info, etc.  actually I can't 
reproduce the issue. So please help me to reproduce it, thanks.

Best Regards,

Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Miller, 
Carl H
Sent: Thursday, October 29, 2015 9:58 PM
To: edk2-devel@lists.01.org
Subject: [edk2] RE enquote PATH: edk2-devel Digest, Vol 4, Issue 214

Zhu Yonghong,
the error you might see would depend on the particular setting of your PATH 
variable at time of running toolsetup, for instance, my PATH contained the 
string "C:\Program Files (x86)\Cmake\bin" without the quotes. when i ran 
toolsetup, i got an error to the effect of "unknown command '\Cmake\bin' "

--

Message: 6
Date: Thu, 29 Oct 2015 08:58:55 +
From: "Zhu, Yonghong" 
To: "Miller, Carl H" , "edk2-devel@lists.01.org"

Cc: "Gao, Liming" 
Subject: Re: [edk2] PATCH BaseTools/toolsetup.bat : enquote PATH
reassignment
Message-ID:


Content-Type: text/plain; charset="us-ascii"

Hi Carl,

May I know the detail error we would meet if we don't add the quotes ?

Best Regards,

Zhu Yonghong

From: Miller, Carl H [mailto:carl.mil...@pnnl.gov]
Sent: Wednesday, October 28, 2015 11:03 PM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong; Gao, Liming
Subject: PATCH BaseTools/toolsetup.bat : enquote PATH reassignment

add quotes around updating of the PATH env. var. to avoid errors when existing 
path contains spaces.

Cc: M: Yonghong Zhu >
   M: Liming Gao >
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thompson, James J 
>
diff --git a/BaseTools/toolsetup.bat b/BaseTools/toolsetup.bat index 
59874c5..e90e28e 100755
--- a/BaseTools/toolsetup.bat
+++ b/BaseTools/toolsetup.bat
@@ -322,10 +322,10 @@ goto end
   echo !!! WARNING !!! Will not be able to compile Python programs to .exe
   echo Will setup environment to run Python scripts directly.
   echo.
-  set PATH=%BASETOOLS_PYTHON_SOURCE%\Trim;%PATH%
-  set PATH=%BASETOOLS_PYTHON_SOURCE%\GenFds;%PATH%
-  set PATH=%BASETOOLS_PYTHON_SOURCE%\build;%PATH%
-  set PATHEXT=%PATHEXT%;.py
+ set PATH="%BASETOOLS_PYTHON_SOURCE%\Trim;%PATH%"
+ set PATH="%BASETOOLS_PYTHON_SOURCE%\GenFds;%PATH%"
+ set PATH="%BASETOOLS_PYTHON_SOURCE%\build;%PATH%"
+ set PATHEXT=%PATHEXT%;.py
 )
   )


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

Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

2015-10-30 Thread Ni, Ruiyu

Sunny,
You could move the two FreePool (FullInstance) to one place
which is under the LocateDevicePath() call to make the change smaller a bit.
What do you think?

Thanks,
Ray

-Original Message-
From: Wang, Sunny (HPS SW) [mailto:sunnyw...@hpe.com] 
Sent: Friday, October 30, 2015 6:21 PM
To: Ni, Ruiyu 
Cc: Tian, Feng ; El-Haj-Mahmoud, Samer 
; Wang, Sunny (HPS SW) ; 
edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Hi Ray,
Are you the owner of this module (UefiBootManagerLib)? If so, could you 
help to review and commit it?  If not, could you tell me who is the owner? 
In addition, the attached patch would also need your help to get 
committed. Thanks!

Regards,
Sunny Wang

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: Thursday, October 29, 2015 8:21 AM
To: El-Haj-Mahmoud, Samer; Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Cc: Tian, Feng
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues
Importance: High

I will let module owner review and commit it.

Thanks
Feng

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com] 
Sent: Wednesday, October 28, 2015 22:58
To: Wang, Sunny (HPS SW); edk2-devel@lists.01.org; Tian, Feng
Subject: RE: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

+ MdeModuklePkg maintainers.

Can someone help by committing this please?



-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
El-Haj-Mahmoud, Samer
Sent: Tuesday, October 27, 2015 10:14 AM
To: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Reviewed-by: Samer El-Haj-Mahmoud 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Tuesday, October 27, 2015 3:47 AM
To: edk2-devel@lists.01.org
Cc: El-Haj-Mahmoud, Samer 
Subject: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Fix memory leak issues

Contributed-under: TianoCore Contribution Agreement 1.0

Signed-off-by: Sunny Wang 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
index 86b4fac..0830166 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmConsole.c
@@ -2,6 +2,7 @@
   Library functions which contain all the code to connect console device.
 
 Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -301,6 +302,7 @@ 
BmUpdateSystemTableConsole (
   EFI_DEVICE_PATH_PROTOCOL*FullDevicePath;
   EFI_DEVICE_PATH_PROTOCOL*VarConsole;
   EFI_DEVICE_PATH_PROTOCOL*Instance;
+  EFI_DEVICE_PATH_PROTOCOL*FullInstance;
   VOID*Interface;
   EFI_HANDLE  NewHandle;
   EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL *TextOut; @@ -354,6 +356,7 @@ 
BmUpdateSystemTableConsole (
 //
 // Find console device handle by device path instance
 //
+FullInstance = Instance;
 Status = gBS->LocateDevicePath (
 ConsoleGuid,
 ,
@@ -383,15 +386,18 @@ BmUpdateSystemTableConsole (
 TextOut->SetMode (TextOut, 0);
   }
 }
+FreePool (FullDevicePath);
+FreePool (FullInstance);
 return TRUE;
   }
 }
-
+FreePool (FullInstance);
   } while (Instance != NULL);
 
   //
   // No any available console devcie found.
   //
+  FreePool (FullDevicePath);
   return FALSE;
 }
 
--
2.5.0.windows.1

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


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

Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

2015-10-30 Thread Ni, Ruiyu

Sunny,
I don't like to add a library interface as a short term fix. I will provide a 
patch next week to enable platform recovery (We are still working on OS 
recovery) so that you don't need PlatformBootManagerDefaultBootFail() interface.

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 6:14 PM
To: Ni, Ruiyu 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Yes, I think OS/Platform recovery feature can meet our requirement.
However, OS/Platform recovery feature has not yet been completely implemented, 
so we would like to have a short-term solution for this.
Just like Samer said, if you are OK with introducing a new StatusCode in either 
BootAllBootOptions() or DefaultBootBehavior(), I can update patch for this. 
Of cause, If you think my current patch (adding hook function) could be used 
for short-term, it would be great.
By the way, I have the other similar need as the attached email.  If you have 
time, please help to review it. Thanks! 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Friday, October 30, 2015 2:41 PM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
Can the OS/Platform recovery feature in UEFI spec can meet your requirement?

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 11:46 AM
To: Ni, Ruiyu 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Thanks for the information.
However, the Status codes you mentioned in EfiBootManagerBoot() is for 
indicating booting a specific boot option failure rather than booting ALL boot 
options failure, so I can't use these Status codes for my purpose. The 
following two are detailed reasons:
-  I can't figure out how to know that all the boot options are booted in the 
Status code handler which is triggered by the REPORT_STATUS_CODE in 
EfiBootManagerBoot()
-  If there is no boot option in the boot order, this status code will not work 
either because BDS code doesn't call EfiBootManagerBoot().

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Thursday, October 29, 2015 1:07 PM
To: El-Haj-Mahmoud, Samer
Cc: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Samer,
EfiBootManagerBoot() function reports the failure to load boot option and the 
failure to start boot option through status code.

//
// Report Status Code to indicate that the failure to load boot option // 
REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
  );
...

//
// Report Status Code to indicate that boot failure // REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
  );

Platform can hook the above two status code so that the 
PlatformBootManagerDefaultBootFail() can be avoided.

We are working on the Platform Recovery feature right now. But not sure whether 
it can be done by the end of this year.
Some spec clarification/discussion are on-going.

Thanks,
Ray

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 10:49 PM
To: Ni, Ruiyu 
Cc: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org; 
El-Haj-Mahmoud, Samer 
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Ray,

The use case is in taking some platform specific action when all boot options 
have failed. I guess this is more in line with UEFI 2.5 Platform Recovery 
(section 3.4.3). Since EDK2 has not implemented that yet, and there is no 
processing of PlatformRecovery variables, a hook function seems to solve 
the issue for now.

I guess a Status Code could solve this as well, if you are OK with introducting 
a new StatusCode in BootAllBootOptions() or DefaultBootBehavior(). If so, Sunny 
can update the patch with this proposal.

On a separate note, what is the plan for implementing UEFI 2.5 Platform 
Recovery in EDK2?

Thanks,
--Samer






-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Wednesday, October 28,

[edk2] [PATCH] Add common HTTP definitions for use in HTTP clients/applications. List includes: HTTP methods, request/response headers, and encodings.

2015-10-30 Thread Samer El-Haj-Mahmoud

Signed-off-by: Samer El-Haj-Mahmoud 
---
 MdeModulePkg/Include/Library/HttpLib.h | 43 ++
 1 file changed, 43 insertions(+)

diff --git a/MdeModulePkg/Include/Library/HttpLib.h 
b/MdeModulePkg/Include/Library/HttpLib.h
index ce5a839..8d5412e 100644
--- a/MdeModulePkg/Include/Library/HttpLib.h
+++ b/MdeModulePkg/Include/Library/HttpLib.h
@@ -3,6 +3,7 @@
   It provides the helper routines to parse the HTTP message byte stream.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -18,6 +19,48 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include 
 
+
+///
+/// HTTP Request Method definitions
+///
+#define HTTP_METHOD_GET "GET"
+#define HTTP_METHOD_POST"POST"
+#define HTTP_METHOD_PATCH   "PATCH"
+#define HTTP_METHOD_OPTIONS "OPTIONS"
+#define HTTP_METHOD_CONNECT "CONNECT"
+#define HTTP_METHOD_HEAD"HEAD"
+#define HTTP_METHOD_PUT "PUT"
+#define HTTP_METHOD_DELETE  "DELETE"
+#define HTTP_METHOD_TRACE   "TRACE"
+
+///
+/// HTTP Request Headers definitions
+///
+#define HTTP_REQUEST_HEADER_ACCEPT  "Accept"
+#define HTTP_REQUEST_HEADER_ACCEPT_LANGUAGE "Accept-Language"
+#define HTTP_REQUEST_HEADER_CONTENT_LENGTH  "Content-Length"
+#define HTTP_REQUEST_HEADER_CONTENT_TYPE"Content-Type"
+#define HTTP_REQUEST_HEADER_CONTENT_ENCODING"Content-Encoding"
+#define HTTP_REQUEST_HEADER_HOST"Host"
+#define HTTP_REQUEST_HEADER_IF_MATCH"If-Match"
+#define HTTP_REQUEST_HEADER_IF_NONE_MATCH   "If-None-Match"
+#define HTTP_REQUEST_HEADER_AUTHORIZATION   "Authorization"
+
+///
+/// HTTP Response Header definitions
+///
+#define HTTP_RESPONSE_HEADER_CONTENT_LENGTH "Content-Length"
+#define HTTP_RESPONSE_HEADER_CONTENT_TYPE   "Content-Type"
+#define HTTP_RESPONSE_HEADER_LOCATION   "Location"
+#define HTTP_RESPONSE_HEADER_CACHE_CONTROL  "Cache-Control"
+#define HTTP_RESPONSE_HEADER_ETAG   "ETag"
+
+///
+/// HTTP Content-Type encodings
+///
+#define HTTP_CONTENT_TYPE_APP_JSON  "application/json"
+#define HTTP_CONTENT_TYPE_APP_JSON_UTF8 "application/json; 
charset=utf-8"
+
 /**
   Decode a percent-encoded URI component to the ASCII character.
   
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching max perf log entries

2015-10-30 Thread Samer El-Haj-Mahmoud

Add a DEBUG statement when the number of PEI perf log entries
exceeds PcdMaxPeiPerformanceLogEntries

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c 
b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
index 0b5a717..fdc0ae0 100644
--- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
+++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
@@ -7,6 +7,7 @@
   number of performance logging entry is specified by 
PcdMaxPeiPerformanceLogEntries.  
 
 Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -183,6 +184,7 @@ StartPerformanceMeasurementEx (
   InternalGetPerformanceHobLog (, );
 
   if (PeiPerformanceLog->NumberOfEntries >= PcdGet8 
(PcdMaxPeiPerformanceLogEntries)) {
+DEBUG ((DEBUG_ERROR, "PEI performance log arrray out of resources\n"));
 return RETURN_OUT_OF_RESOURCES;
   }
   Index   = PeiPerformanceLog->NumberOfEntries++;
-- 
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: Add DEBUG statement on reaching PEI perf log max entries

2015-10-30 Thread Samer El-Haj-Mahmoud

Signed-off-by: Samer El-Haj-Mahmoud 
---
 MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c 
b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
index 0b5a717..fdc0ae0 100644
--- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
+++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
@@ -7,6 +7,7 @@
   number of performance logging entry is specified by 
PcdMaxPeiPerformanceLogEntries.  
 
 Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -183,6 +184,7 @@ StartPerformanceMeasurementEx (
   InternalGetPerformanceHobLog (, );
 
   if (PeiPerformanceLog->NumberOfEntries >= PcdGet8 
(PcdMaxPeiPerformanceLogEntries)) {
+DEBUG ((DEBUG_ERROR, "PEI performance log arrray out of resources\n"));
 return RETURN_OUT_OF_RESOURCES;
   }
   Index   = PeiPerformanceLog->NumberOfEntries++;
-- 
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 v2] MdeModulePkg: Add some common HTTP definitions

2015-10-30 Thread Carsey, Jaben

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Samer El-Haj-Mahmoud
> Sent: Friday, October 30, 2015 4:08 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng ; Samer El-Haj-Mahmoud  mahm...@hp.com>
> Subject: [edk2] [PATCH v2] MdeModulePkg: Add some common HTTP
> definitions
> 
> Add common HTTP definitions for use in HTTP clients/applications.
> List includes: HTTP methods, request/response headers, and encodings.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Samer El-Haj-Mahmoud 
> ---
>  MdeModulePkg/Include/Library/HttpLib.h | 43
> ++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Library/HttpLib.h
> b/MdeModulePkg/Include/Library/HttpLib.h
> index ce5a839..8d5412e 100644
> --- a/MdeModulePkg/Include/Library/HttpLib.h
> +++ b/MdeModulePkg/Include/Library/HttpLib.h
> @@ -3,6 +3,7 @@
>It provides the helper routines to parse the HTTP message byte stream.
> 
>  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be 
> found
> at
> @@ -18,6 +19,48 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #include 
> 
> +
> +///
> +/// HTTP Request Method definitions
> +///
> +#define HTTP_METHOD_GET "GET"
> +#define HTTP_METHOD_POST"POST"
> +#define HTTP_METHOD_PATCH   "PATCH"
> +#define HTTP_METHOD_OPTIONS "OPTIONS"
> +#define HTTP_METHOD_CONNECT "CONNECT"
> +#define HTTP_METHOD_HEAD"HEAD"
> +#define HTTP_METHOD_PUT "PUT"
> +#define HTTP_METHOD_DELETE  "DELETE"
> +#define HTTP_METHOD_TRACE   "TRACE"
> +
> +///
> +/// HTTP Request Headers definitions
> +///
> +#define HTTP_REQUEST_HEADER_ACCEPT  "Accept"
> +#define HTTP_REQUEST_HEADER_ACCEPT_LANGUAGE "Accept-Language"
> +#define HTTP_REQUEST_HEADER_CONTENT_LENGTH  "Content-Length"
> +#define HTTP_REQUEST_HEADER_CONTENT_TYPE"Content-Type"
> +#define HTTP_REQUEST_HEADER_CONTENT_ENCODING"Content-
> Encoding"
> +#define HTTP_REQUEST_HEADER_HOST"Host"
> +#define HTTP_REQUEST_HEADER_IF_MATCH"If-Match"
> +#define HTTP_REQUEST_HEADER_IF_NONE_MATCH   "If-None-Match"
> +#define HTTP_REQUEST_HEADER_AUTHORIZATION   "Authorization"
> +
> +///
> +/// HTTP Response Header definitions
> +///
> +#define HTTP_RESPONSE_HEADER_CONTENT_LENGTH "Content-Length"
> +#define HTTP_RESPONSE_HEADER_CONTENT_TYPE   "Content-Type"
> +#define HTTP_RESPONSE_HEADER_LOCATION   "Location"
> +#define HTTP_RESPONSE_HEADER_CACHE_CONTROL  "Cache-Control"
> +#define HTTP_RESPONSE_HEADER_ETAG   "ETag"
> +
> +///
> +/// HTTP Content-Type encodings
> +///
> +#define HTTP_CONTENT_TYPE_APP_JSON  "application/json"
> +#define HTTP_CONTENT_TYPE_APP_JSON_UTF8 "application/json;
> charset=utf-8"
> +
>  /**
>Decode a percent-encoded URI component to the ASCII character.
> 
> --
> 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 v2] MdeModulePkg: Add some common HTTP definitions

2015-10-30 Thread Samer El-Haj-Mahmoud

Add common HTTP definitions for use in HTTP clients/applications.
List includes: HTTP methods, request/response headers, and encodings.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 MdeModulePkg/Include/Library/HttpLib.h | 43 ++
 1 file changed, 43 insertions(+)

diff --git a/MdeModulePkg/Include/Library/HttpLib.h 
b/MdeModulePkg/Include/Library/HttpLib.h
index ce5a839..8d5412e 100644
--- a/MdeModulePkg/Include/Library/HttpLib.h
+++ b/MdeModulePkg/Include/Library/HttpLib.h
@@ -3,6 +3,7 @@
   It provides the helper routines to parse the HTTP message byte stream.
 
 Copyright (c) 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -18,6 +19,48 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include 
 
+
+///
+/// HTTP Request Method definitions
+///
+#define HTTP_METHOD_GET "GET"
+#define HTTP_METHOD_POST"POST"
+#define HTTP_METHOD_PATCH   "PATCH"
+#define HTTP_METHOD_OPTIONS "OPTIONS"
+#define HTTP_METHOD_CONNECT "CONNECT"
+#define HTTP_METHOD_HEAD"HEAD"
+#define HTTP_METHOD_PUT "PUT"
+#define HTTP_METHOD_DELETE  "DELETE"
+#define HTTP_METHOD_TRACE   "TRACE"
+
+///
+/// HTTP Request Headers definitions
+///
+#define HTTP_REQUEST_HEADER_ACCEPT  "Accept"
+#define HTTP_REQUEST_HEADER_ACCEPT_LANGUAGE "Accept-Language"
+#define HTTP_REQUEST_HEADER_CONTENT_LENGTH  "Content-Length"
+#define HTTP_REQUEST_HEADER_CONTENT_TYPE"Content-Type"
+#define HTTP_REQUEST_HEADER_CONTENT_ENCODING"Content-Encoding"
+#define HTTP_REQUEST_HEADER_HOST"Host"
+#define HTTP_REQUEST_HEADER_IF_MATCH"If-Match"
+#define HTTP_REQUEST_HEADER_IF_NONE_MATCH   "If-None-Match"
+#define HTTP_REQUEST_HEADER_AUTHORIZATION   "Authorization"
+
+///
+/// HTTP Response Header definitions
+///
+#define HTTP_RESPONSE_HEADER_CONTENT_LENGTH "Content-Length"
+#define HTTP_RESPONSE_HEADER_CONTENT_TYPE   "Content-Type"
+#define HTTP_RESPONSE_HEADER_LOCATION   "Location"
+#define HTTP_RESPONSE_HEADER_CACHE_CONTROL  "Cache-Control"
+#define HTTP_RESPONSE_HEADER_ETAG   "ETag"
+
+///
+/// HTTP Content-Type encodings
+///
+#define HTTP_CONTENT_TYPE_APP_JSON  "application/json"
+#define HTTP_CONTENT_TYPE_APP_JSON_UTF8 "application/json; 
charset=utf-8"
+
 /**
   Decode a percent-encoded URI component to the ASCII character.
   
-- 
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] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

2015-10-30 Thread Wang, Sunny (HPS SW)

Hi Ray,
Yes, I think OS/Platform recovery feature can meet our requirement.
However, OS/Platform recovery feature has not yet been completely implemented, 
so we would like to have a short-term solution for this.
Just like Samer said, if you are OK with introducing a new StatusCode in either 
BootAllBootOptions() or DefaultBootBehavior(), I can update patch for this. 
Of cause, If you think my current patch (adding hook function) could be used 
for short-term, it would be great.
By the way, I have the other similar need as the attached email.  If you have 
time, please help to review it. Thanks! 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Friday, October 30, 2015 2:41 PM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
Can the OS/Platform recovery feature in UEFI spec can meet your requirement?

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 11:46 AM
To: Ni, Ruiyu 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Thanks for the information.
However, the Status codes you mentioned in EfiBootManagerBoot() is for 
indicating booting a specific boot option failure rather than booting ALL boot 
options failure, so I can't use these Status codes for my purpose. The 
following two are detailed reasons:
-  I can't figure out how to know that all the boot options are booted in the 
Status code handler which is triggered by the REPORT_STATUS_CODE in 
EfiBootManagerBoot()
-  If there is no boot option in the boot order, this status code will not work 
either because BDS code doesn't call EfiBootManagerBoot().

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Thursday, October 29, 2015 1:07 PM
To: El-Haj-Mahmoud, Samer
Cc: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Samer,
EfiBootManagerBoot() function reports the failure to load boot option and the 
failure to start boot option through status code.

//
// Report Status Code to indicate that the failure to load boot option // 
REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
  );
...

//
// Report Status Code to indicate that boot failure // REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
  );

Platform can hook the above two status code so that the 
PlatformBootManagerDefaultBootFail() can be avoided.

We are working on the Platform Recovery feature right now. But not sure whether 
it can be done by the end of this year.
Some spec clarification/discussion are on-going.

Thanks,
Ray

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 10:49 PM
To: Ni, Ruiyu 
Cc: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org; 
El-Haj-Mahmoud, Samer 
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Ray,

The use case is in taking some platform specific action when all boot options 
have failed. I guess this is more in line with UEFI 2.5 Platform Recovery 
(section 3.4.3). Since EDK2 has not implemented that yet, and there is no 
processing of PlatformRecovery variables, a hook function seems to solve 
the issue for now.

I guess a Status Code could solve this as well, if you are OK with introducting 
a new StatusCode in BootAllBootOptions() or DefaultBootBehavior(). If so, Sunny 
can update the patch with this proposal.

On a separate note, what is the plan for implementing UEFI 2.5 Platform 
Recovery in EDK2?

Thanks,
--Samer






-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Wednesday, October 28, 2015 8:18 AM
To: El-Haj-Mahmoud, Samer 
Cc: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Do you have real requirement about the boot failure notify?
Have you considered to use report status handler?

Thanks,
Ray

> 在 2015年10月27日，23:18，El-Haj-Mahmoud, Samer 
>  写道：
> 
> Reviewed-by: Samer El-Haj-Mahmoud 
> 
> -Original Message-
> From: Wang, Sunny (HPS SW)
> Sent: Tuesday,

[edk2] SecurityPkg: PeiRsa2048Sha256GuidedSectionExtractLib error handling

2015-10-30 Thread Cohen, Eugene

Dear SecurityPkg maintainer,

I'm trying to track down the best way for platform policy to handle an 
authentication failure in the PEI Rsa2048Sha256 guided section extraction 
library and ran across this curious state near the end of 
Rsa2048Sha256GuidedSectionHandler.

  //
  // Temp solution until PeiCore checks AUTH Status.
  //
  if ((*AuthenticationStatus & (EFI_AUTH_STATUS_TEST_FAILED | 
EFI_AUTH_STATUS_NOT_TESTED)) != 0) {
Status = EFI_ACCESS_DENIED;
  }

>From what I can tell the caller is checking AuthenticationStatus so this may 
>be some development code that was not removed.

Setting the return status to EFI_ACCESS_DENIED prevents us from getting to 
platform policy code (via the EFI_PEI_SECURITY2_PPI)

   Status = ParentFvPpi->FindSectionByType (
ParentFvPpi,
EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
ParentFvFileHandle,
(VOID **)
);
  }
  if (EFI_ERROR (Status)) {
return Status;  <--- we bail out here
  }

  Status = VerifyPeim (PrivateData, ParentFvHandle, ParentFvFileHandle, 
AuthenticationStatus); <-- this would have called the EFI_PEI_SECURITY2_PPI
  if (Status == EFI_SECURITY_VIOLATION) {
return Status;
  }

Because of this issue we cannot run a policy handler that we'd like to because 
we remain in the PEI core going down error paths.

I think this fix may be as simple as removing the 'Temp solution' block but 
since I'm unfamiliar with the intent I wanted to check with you what the 
intended flow is for platform code discovering and handling authentication 
errors.

Thanks,

Eugene

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

[edk2] [PATCH] MdeModulePkg: PiSmmCore: Remove confusing CopyMem() of SMM_ENTRY_CONTEXT

2015-10-30 Thread Michael Kinney

A subset of fields in the EFI_SMM_SYSTEM_TABLE2 structure are identical
to the fields in the SMM_ENTRY_CONTEXT structure.  CopyMem() is used to
transfer the contents of the SMM_ENTRY_CONTEXT structure into the
EFI_SMM_SYSTEM_TABLE2.  This is confusing because SMM_ENTRY_CONTEXT is
not used in the declaration of EFI_SMM_SYSTEM_TABLE2 and field contents
are transferred without any reference to individual field names (i.e.
CurrentlyExecutingCpu).  In order to make the code easier to maintain
and understand, the CopyMem() is replaced with statements that transfer
each field of SMM_ENTRY_CONTEXT into EFI_SMM_SYSTEM_TABLE2.

Reported-by: Laszlo Ersek 
Link: http://article.gmane.org/gmane.comp.bios.edk2.devel/3567

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c 
b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 496638a..cc7ccec 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -430,13 +430,17 @@ SmmEntryPoint (
   BOOLEAN IsOverlapped;
 
   PERF_START (NULL, "SMM", NULL, 0) ;
 
   //
-  // Update SMST using the context
+  // Update SMST with contents of the SmmEntryContext structure
   //
-  CopyMem (, SmmEntryContext, sizeof 
(EFI_SMM_ENTRY_CONTEXT));
+  gSmmCoreSmst.SmmStartupThisAp  = SmmEntryContext->SmmStartupThisAp;
+  gSmmCoreSmst.CurrentlyExecutingCpu = SmmEntryContext->CurrentlyExecutingCpu;
+  gSmmCoreSmst.NumberOfCpus  = SmmEntryContext->NumberOfCpus;
+  gSmmCoreSmst.CpuSaveStateSize  = SmmEntryContext->CpuSaveStateSize;
+  gSmmCoreSmst.CpuSaveState  = SmmEntryContext->CpuSaveState;
 
   //
   // Call platform hook before Smm Dispatch
   //
   PlatformHookBeforeSmmDispatch ();
-- 
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 6/6] OvmfPkg/PlatformPei: Set PcdCpuMaxLogicalProcessorNumber using QEMU fw_cfg

2015-10-30 Thread Laszlo Ersek

On 10/30/15 14:04, Janusz Mocek wrote:
> W dniu 30.10.2015 o 13:26, Laszlo Ersek pisze:
>> CC'ing Xiao and Alex again.
>>
>> On 10/29/15 19:39, Jordan Justen wrote:
>>> On 2015-10-29 04:45:37, Laszlo Ersek wrote:
 On 10/29/15 02:32, Jordan Justen wrote:
> +ASSERT (MaxProcessors > 0);
> +PcdSet32 (PcdCpuMaxLogicalProcessorNumber, MaxProcessors);
 I think that when this branch is active, then
 PcdCpuApInitTimeOutInMicroSeconds should *also* be set, namely to
 MAX_UINT32 (~71 minutes, the closest we can get to "infinity"). When
 this hint is available from QEMU, then we should practically disable
 the timeout option in CpuDxe's AP counting.
>>> I think this is a good idea, but I don't think 71 minutes is useful.
>>> Perhaps 30 seconds? This seems more than adequate for hundreds of
>>> processors to startup. Or perhaps some timeout based on the number of
>>> processors?
>>>
>>> Janusz and I were discussing
>>> https://github.com/tianocore/edk2/issues/21 on irc. We increased the
>>> timeout to 10 seconds, and with only 8 processors it was still timing
>>> out.
>>>
>>> Obviously we are somehow failing to start the processors correctly, or
>>> QEMU/KVM is doing something wrong.
>>>
>>> Have you been able to reproduce this issue? It seems like we need to
>>> set the timeout to 71 minutes, and then debug QEMU/KVM to see what
>>> state the APs are in...
>>>
>>> Unfortunately I haven't yet been able to reproduce the bug on my
>>> system. :(
>> I've been staring at the following things for a few tens of minutes now:
>>
>> (1) Kernel commit b18d5431acc7. Note that the commit changes the return
>> value of the vmx_get_mt_mask() function *exactly* in the following
>> case:
>>
>>   kvm_arch_has_noncoherent_dma(vcpu->kvm) &&
>>   (kvm_read_cr0(vcpu) & X86_CR0_CD)
>>
>> The first sub-condition is satisfied by GPU passthrough / device
>> assignment, I think; the second part depends on the VCPU having
>> turned on (or having *left* on) CR0.CD.
>>
>> (2) Consult the vmx_vcpu_reset() function in "arch/x86/kvm/vmx.c"
>> (current upstream). You will find:
>>
>>  cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET;
>>  vmx_set_cr0(vcpu, cr0); /* enter rmode */
>>
>> Meaning a VCPU will start with CD and NW set, in real mode, after
>> re-set.
>>
>> This setting dates back to the birth of KVM:
>>
>>   commit 6aa8b732ca01c3d7a54e93f4d701b8aabbe60fb7
>>   Author: Avi Kivity 
>>   Date: Sun Dec 10 02:21:36 2006 -0800
>>
>>   [PATCH] kvm: userspace interface
>>
>> Search that commit for "0x6010" (the second hit, although the
>> comment that contains the first hit is quite telling as well).
>>
>> (3) Consult the Intel SDM, Table 11-5. "Cache Operating Modes".
>>
>> The (CD, NW) == (1, 1) setting in CR0 is documented as:
>> - "Memory coherency is not maintained."
>> - "(P6 family and Pentium processors.) State of the processor after
>>   a power up or reset. "
>> - [in footnote 2] "The Pentium 4 and more recent processor families
>>   do not support this mode; setting the CD and NW bits to 1 selects
>>   the no-fill cache mode."
>>
>> In other words, the settings implemented by vmx_vcpu_reset()
>> actually invoke the behavior of the "no-fill cache mode" (which is
>> (CD, NW) == (1, 0)) for all practical purposes.
>>
>> (4) Same reference.
>>
>> The (CD, NW) == (1, 0) setting in CR0 is documented as:
>> - "No-fill Cache Mode. Memory coherency is maintained."
>> - "(Pentium 4 and later processor families.) State of processor
>>   after a power up or reset. "
>>
>> (5) The AsmEnableCache() function in
>> "MdePkg/Library/BaseLib/Ia32/EnableCache.c". It clears both CD and
>> NW in CR0.
>>
>> (6) This setting ((CD, NW) == (0, 0))is documented in the Intel SDM as:
>> - "Normal Cache Mode. Highest performance cache operation."
>>
>> (7) The AsmEnableCache() function is invoked by MtrrLib
>> [UefiCpuPkg/Library/MtrrLib/MtrrLib.c] after any and all MTRR
>> changes. Consider:
>>
>> PostMtrrChange() | MtrrSetAllMtrrs()
>>   PostMtrrChangeEnableCache()
>> AsmEnableCache()
>>
>> Where MtrrSetAllMtrrs() is a public function of the library; plus
>> PostMtrrChange() is invoked by all of the following public
>> functions:
>>
>> - MtrrSetMemoryAttribute()
>> - MtrrSetVariableMtrr()
>> - MtrrSetFixedMtrr()
>>
>> (8) Because we call MtrrLib in PlatformPei first, there are two
>> consequences:
>>
>> (a) The boot VCPU has CR0.CD *set* in all parts of OVMF that run
>> earlier than that.
>>
>> This caused a widely reported boot perf regression in SEC (the
>> LZMA decompression). Ultimately another MTRR change in KVM was
>> reverted, so (as far as I know) this symptom has not been seen
>> recently. (In any case, we should probably fix this sometime...)
>>
>> (b) The

Re: [edk2] [PATCH 6/6] OvmfPkg/PlatformPei: Set PcdCpuMaxLogicalProcessorNumber using QEMU fw_cfg

2015-10-30 Thread Laszlo Ersek

On 10/30/15 13:26, Laszlo Ersek wrote:

>> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c
>> index 3f56faa..e7f5b41 100644
>> --- a/UefiCpuPkg/CpuDxe/CpuMp.c
>> +++ b/UefiCpuPkg/CpuDxe/CpuMp.c
>> @@ -1451,6 +1451,8 @@ ApEntryPointInC (
>>VOID*   TopOfApStack;
>>UINTN   ProcessorNumber;
>>
>> +  AsmEnableCache ();
>> +
>>if (!mAPsAlreadyInitFinished) {
>>  FillInProcessorInformation (FALSE, mMpSystemData.NumberOfProcessors);
>>  TopOfApStack  = (UINT8*)mApStackStart + gApStackSize;
> 
> This should clear CR0.CD, and "undo" kernel commit b18d5431acc7 for
> the AP (by falsifying the second subcondition seen in (1)).
> 
> Janusz, can you please test this one-liner (with no other out-of-tree
> patch applied)?

If it doesn't help (much), we can also try to do this in assembly, in
earlier parts of the startup code. Because, AsmApEntryPoint() in

UefiCpuPkg/CpuDxe/Ia32/MpAsm.asm
UefiCpuPkg/CpuDxe/Ia32/MpAsm.nasm
UefiCpuPkg/CpuDxe/X64/MpAsm.asm
UefiCpuPkg/CpuDxe/X64/MpAsm.nasm

already runs spinlock-like code.

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

Re: [edk2] [PATCH 6/6] OvmfPkg/PlatformPei: Set PcdCpuMaxLogicalProcessorNumber using QEMU fw_cfg

2015-10-30 Thread Laszlo Ersek

CC'ing Xiao and Alex again.

On 10/29/15 19:39, Jordan Justen wrote:
> On 2015-10-29 04:45:37, Laszlo Ersek wrote:
>> On 10/29/15 02:32, Jordan Justen wrote:
>>> +ASSERT (MaxProcessors > 0);
>>> +PcdSet32 (PcdCpuMaxLogicalProcessorNumber, MaxProcessors);
>>
>> I think that when this branch is active, then
>> PcdCpuApInitTimeOutInMicroSeconds should *also* be set, namely to
>> MAX_UINT32 (~71 minutes, the closest we can get to "infinity"). When
>> this hint is available from QEMU, then we should practically disable
>> the timeout option in CpuDxe's AP counting.
>
> I think this is a good idea, but I don't think 71 minutes is useful.
> Perhaps 30 seconds? This seems more than adequate for hundreds of
> processors to startup. Or perhaps some timeout based on the number of
> processors?
>
> Janusz and I were discussing
> https://github.com/tianocore/edk2/issues/21 on irc. We increased the
> timeout to 10 seconds, and with only 8 processors it was still timing
> out.
>
> Obviously we are somehow failing to start the processors correctly, or
> QEMU/KVM is doing something wrong.
>
> Have you been able to reproduce this issue? It seems like we need to
> set the timeout to 71 minutes, and then debug QEMU/KVM to see what
> state the APs are in...
>
> Unfortunately I haven't yet been able to reproduce the bug on my
> system. :(

I've been staring at the following things for a few tens of minutes now:

(1) Kernel commit b18d5431acc7. Note that the commit changes the return
value of the vmx_get_mt_mask() function *exactly* in the following
case:

  kvm_arch_has_noncoherent_dma(vcpu->kvm) &&
  (kvm_read_cr0(vcpu) & X86_CR0_CD)

The first sub-condition is satisfied by GPU passthrough / device
assignment, I think; the second part depends on the VCPU having
turned on (or having *left* on) CR0.CD.

(2) Consult the vmx_vcpu_reset() function in "arch/x86/kvm/vmx.c"
(current upstream). You will find:

cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET;
vmx_set_cr0(vcpu, cr0); /* enter rmode */

Meaning a VCPU will start with CD and NW set, in real mode, after
re-set.

This setting dates back to the birth of KVM:

  commit 6aa8b732ca01c3d7a54e93f4d701b8aabbe60fb7
  Author: Avi Kivity 
  Date: Sun Dec 10 02:21:36 2006 -0800

  [PATCH] kvm: userspace interface

Search that commit for "0x6010" (the second hit, although the
comment that contains the first hit is quite telling as well).

(3) Consult the Intel SDM, Table 11-5. "Cache Operating Modes".

The (CD, NW) == (1, 1) setting in CR0 is documented as:
- "Memory coherency is not maintained."
- "(P6 family and Pentium processors.) State of the processor after
  a power up or reset. "
- [in footnote 2] "The Pentium 4 and more recent processor families
  do not support this mode; setting the CD and NW bits to 1 selects
  the no-fill cache mode."

In other words, the settings implemented by vmx_vcpu_reset()
actually invoke the behavior of the "no-fill cache mode" (which is
(CD, NW) == (1, 0)) for all practical purposes.

(4) Same reference.

The (CD, NW) == (1, 0) setting in CR0 is documented as:
- "No-fill Cache Mode. Memory coherency is maintained."
- "(Pentium 4 and later processor families.) State of processor
  after a power up or reset. "

(5) The AsmEnableCache() function in
"MdePkg/Library/BaseLib/Ia32/EnableCache.c". It clears both CD and
NW in CR0.

(6) This setting ((CD, NW) == (0, 0))is documented in the Intel SDM as:
- "Normal Cache Mode. Highest performance cache operation."

(7) The AsmEnableCache() function is invoked by MtrrLib
[UefiCpuPkg/Library/MtrrLib/MtrrLib.c] after any and all MTRR
changes. Consider:

PostMtrrChange() | MtrrSetAllMtrrs()
  PostMtrrChangeEnableCache()
AsmEnableCache()

Where MtrrSetAllMtrrs() is a public function of the library; plus
PostMtrrChange() is invoked by all of the following public
functions:

- MtrrSetMemoryAttribute()
- MtrrSetVariableMtrr()
- MtrrSetFixedMtrr()

(8) Because we call MtrrLib in PlatformPei first, there are two
consequences:

(a) The boot VCPU has CR0.CD *set* in all parts of OVMF that run
earlier than that.

This caused a widely reported boot perf regression in SEC (the
LZMA decompression). Ultimately another MTRR change in KVM was
reverted, so (as far as I know) this symptom has not been seen
recently. (In any case, we should probably fix this sometime...)

(b) The other consequence is that the boot VCPU's CR0.CD is clear in
the rest of OVMF. Which is what makes its speed acceptable, I
guess (as long as no APs are started up).

(9) Our AP startup code massages CR0, but only for mode switches. CR0.CD
and CR0.NW are never touched.

Now, I guess this could be easily added to the assembly encoded as a
C array

Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in S3Resume2Pei

2015-10-30 Thread Yao, Jiewen

Yes, first, thanks the great analysis from Laszlo.

Is that possible to eliminate the assumption by parsing current GDT entry?

Instead of hardcode 0x38 or 0x28, if PiSmmCpu inherits GDT from other module, 
should it parse GDT to get correct long mode segment?

I do not object the idea to change DxeCpu driver. I'm just thinking if we have 
robust way to prevent error happening again... if DxeCpu driver is not written 
by us.

Thank you
Yao Jiewen

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Friday, October 30, 2015 4:56 AM
To: Kinney, Michael D; Yao, Jiewen; Fan, Jeff
Cc: Justen, Jordan L; edk2-devel-01
Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in S3Resume2Pei

On 10/29/15 18:30, Kinney, Michael D wrote:
> Laszlo,
> 
> Thanks for the very detailed and complete root cause on this issue.
> I agree that the change you suggest is functional, but I am going to 
> recommend a different solution.
> 
> Unfortunately, there are several modules that set up a GDT and it is 
> easier from an implementation perspective and for debugging if all the 
> modules agree to use the same GDT layout.  It would be even better if 
> no modules make any assumptions about GDT layouts, but that is a 
> bigger change that we can tackle later.
> 
> The root cause of this specific issue is that the GDT that is set up 
> by MdeModulePkg/Core/DxeIplPeim is not the same GDT that is set up by 
> UefiCpuPkg /CpuDxe.  And the UefiCpuPkg/PiSmmCpuDxeSmm module has some 
> assumptions that the GDT is the one setup by 
> MdeModulePkg/Core/DxeIplPeim.  I recommend we update UefiCpuPkg/CpuDxe 
> to use the same GDT layout as MdeModulePkg/Core/DxeIplPeim and then 
> the changes you found worked for PiSmmCpuDxeSmm are not required.
> 
> I will work on a patch for you to test.

I agree this is the best solution. (I didn't know which one of CpuDxe and 
DxeIplPeim should prevail in dictating the GDT convention, so I didn't suggest 
any unification like this.)

Thanks!
Laszlo

> 
> Mike
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> Of Laszlo Ersek
>> Sent: Thursday, October 29, 2015 9:19 AM
>> To: Yao, Jiewen; Kinney, Michael D; Fan, Jeff
>> Cc: Justen, Jordan L; edk2-devel-01
>> Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in 
>> S3Resume2Pei
>>
>> Hi Jiewen,
>>
>> On 10/29/15 03:28, Yao, Jiewen wrote:
>>> Thanks for the info. I think we might have to dig out why LONG_JUMP 
>>> fail
>> here.
>>> Most our IA BIOS support Ia32X64 mode, so I am sure it should work.
>>
>> I think I have found the bug.
>>
>> (1)
>> I added a debug patch to OvmfPkg/QuarkPort/CpuS3DataDxe. I'm 
>> attaching it for illustration. The debug patch dumps the following 
>> information:
>> - the location of the GDT descriptor (that gets loaded into the GDT 
>> register)
>> - the contents of the GDT descriptor (i.e., the base and limit of the 
>> global descriptor table)
>> - the contents of the GDT entries (the individual segment 
>> descriptors)
>>
>> CpuS3DataDxe saves these in AcpiNVS. Later they are saved by  
>>PiSmmCpuDxeSmm from AcpiNVS to SMRAM (during boot), and then restored 
>>from SMRAM to AcpiNVS (during S3 Resume).
>>
>> The patch dumps these details when the preparation for 
>> PiSmmCpuDxeSmm, in AcpiNVS, is ready.
>>
>> Here's the debug output:
>>
>> SaveCpuS3Data: 350: GDT descriptor at 0x7E3E2050
>> SaveCpuS3Data: 353: GDT Base=0x7F706000 Limit=0x3F
>> SaveCpuS3Data: 363: Idx=0x00 Base=0x Limit=0x 
>> Type=0x0
>> System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x0
>> SaveCpuS3Data: 363: Idx=0x08 Base=0x Limit=0x 
>> Type=0x3
>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x1
>> SaveCpuS3Data: 363: Idx=0x10 Base=0x Limit=0x 
>> Type=0xB
>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x1
>> SaveCpuS3Data: 363: Idx=0x18 Base=0x Limit=0x 
>> Type=0x2
>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x1
>> SaveCpuS3Data: 363: Idx=0x20 Base=0x Limit=0x 
>> Type=0xA
>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x1
>> SaveCpuS3Data: 363: Idx=0x28 Base=0x Limit=0x 
>> Type=0xB
>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x1 
>> DefaultSize=0x0
>> SaveCpuS3Data: 363: Idx=0x30 Base=0x Limit=0x 
>> Type=0x0
>> System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x0
>> SaveCpuS3Data: 363: Idx=0x38 Base=0x Limit=0x 
>> Type=0x0
>> System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0 
>> DefaultSize=0x0
>>
>> (Importantly, the debug output is identical between the pure Ia32 
>> build and the Ia32X64 build, except the addresses in the first two 
>> lines -- the location of the GDT descriptor, and the location of the

Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in S3Resume2Pei

2015-10-30 Thread Kinney, Michael D

Jiewen,

Yes.  That is a longer term goal for the PiSmmCpuDxeSmm.  Making the GDTs 
consistent helps debug and is a fix that works until we can add logic to 
PiSmmCpuiDxeSmm to inherit current GDT information and use it throughout this 
module.

Thanks,

Mike

>-Original Message-
>From: Yao, Jiewen
>Sent: Friday, October 30, 2015 7:18 AM
>To: Laszlo Ersek; Kinney, Michael D; Fan, Jeff
>Cc: Justen, Jordan L; edk2-devel-01
>Subject: RE: [edk2] about the SMM_S3_RESUME_SMM_64 branch in
>S3Resume2Pei
>
>Yes, first, thanks the great analysis from Laszlo.
>
>Is that possible to eliminate the assumption by parsing current GDT entry?
>
>Instead of hardcode 0x38 or 0x28, if PiSmmCpu inherits GDT from other
>module, should it parse GDT to get correct long mode segment?
>
>I do not object the idea to change DxeCpu driver. I'm just thinking if we have
>robust way to prevent error happening again... if DxeCpu driver is not written
>by us.
>
>Thank you
>Yao Jiewen
>
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Laszlo Ersek
>Sent: Friday, October 30, 2015 4:56 AM
>To: Kinney, Michael D; Yao, Jiewen; Fan, Jeff
>Cc: Justen, Jordan L; edk2-devel-01
>Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in
>S3Resume2Pei
>
>On 10/29/15 18:30, Kinney, Michael D wrote:
>> Laszlo,
>>
>> Thanks for the very detailed and complete root cause on this issue.
>> I agree that the change you suggest is functional, but I am going to
>> recommend a different solution.
>>
>> Unfortunately, there are several modules that set up a GDT and it is
>> easier from an implementation perspective and for debugging if all the
>> modules agree to use the same GDT layout.  It would be even better if
>> no modules make any assumptions about GDT layouts, but that is a
>> bigger change that we can tackle later.
>>
>> The root cause of this specific issue is that the GDT that is set up
>> by MdeModulePkg/Core/DxeIplPeim is not the same GDT that is set up by
>> UefiCpuPkg /CpuDxe.  And the UefiCpuPkg/PiSmmCpuDxeSmm module has
>some
>> assumptions that the GDT is the one setup by
>> MdeModulePkg/Core/DxeIplPeim.  I recommend we update
>UefiCpuPkg/CpuDxe
>> to use the same GDT layout as MdeModulePkg/Core/DxeIplPeim and then
>> the changes you found worked for PiSmmCpuDxeSmm are not required.
>>
>> I will work on a patch for you to test.
>
>I agree this is the best solution. (I didn't know which one of CpuDxe and
>DxeIplPeim should prevail in dictating the GDT convention, so I didn't suggest
>any unification like this.)
>
>Thanks!
>Laszlo
>
>>
>> Mike
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>>> Of Laszlo Ersek
>>> Sent: Thursday, October 29, 2015 9:19 AM
>>> To: Yao, Jiewen; Kinney, Michael D; Fan, Jeff
>>> Cc: Justen, Jordan L; edk2-devel-01
>>> Subject: Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in
>>> S3Resume2Pei
>>>
>>> Hi Jiewen,
>>>
>>> On 10/29/15 03:28, Yao, Jiewen wrote:
 Thanks for the info. I think we might have to dig out why LONG_JUMP
 fail
>>> here.
 Most our IA BIOS support Ia32X64 mode, so I am sure it should work.
>>>
>>> I think I have found the bug.
>>>
>>> (1)
>>> I added a debug patch to OvmfPkg/QuarkPort/CpuS3DataDxe. I'm
>>> attaching it for illustration. The debug patch dumps the following
>information:
>>> - the location of the GDT descriptor (that gets loaded into the GDT
>>> register)
>>> - the contents of the GDT descriptor (i.e., the base and limit of the
>>> global descriptor table)
>>> - the contents of the GDT entries (the individual segment
>>> descriptors)
>>>
>>> CpuS3DataDxe saves these in AcpiNVS. Later they are saved by
>>>PiSmmCpuDxeSmm from AcpiNVS to SMRAM (during boot), and then
>restored
>>>from SMRAM to AcpiNVS (during S3 Resume).
>>>
>>> The patch dumps these details when the preparation for
>>> PiSmmCpuDxeSmm, in AcpiNVS, is ready.
>>>
>>> Here's the debug output:
>>>
>>> SaveCpuS3Data: 350: GDT descriptor at 0x7E3E2050
>>> SaveCpuS3Data: 353: GDT Base=0x7F706000 Limit=0x3F
>>> SaveCpuS3Data: 363: Idx=0x00 Base=0x Limit=0x
>>> Type=0x0
>>> System=0x0 Dpl=0x0 Present=0x0 Software=0x0 LCodeSeg=0x0
>>> DefaultSize=0x0
>>> SaveCpuS3Data: 363: Idx=0x08 Base=0x Limit=0x
>>> Type=0x3
>>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0
>>> DefaultSize=0x1
>>> SaveCpuS3Data: 363: Idx=0x10 Base=0x Limit=0x
>>> Type=0xB
>>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0
>>> DefaultSize=0x1
>>> SaveCpuS3Data: 363: Idx=0x18 Base=0x Limit=0x
>>> Type=0x2
>>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0
>>> DefaultSize=0x1
>>> SaveCpuS3Data: 363: Idx=0x20 Base=0x Limit=0x
>>> Type=0xA
>>> System=0x1 Dpl=0x0 Present=0x1 Software=0x0 LCodeSeg=0x0
>>> DefaultSize=0x1
>>> SaveCpuS3Data: 363: Idx=0x28 Base=0x Limit=0x
>>> Type=0xB
>>> System=0x1 Dpl=0x0

Re: [edk2] [Patch] MdeModulePkg: Fix a PciBusDxe hot plug bug

2015-10-30 Thread Leif Lindholm

Hi Ray,

Many thanks for this.

On Fri, Oct 30, 2015 at 04:53:54AM +, Ni, Ruiyu wrote:
> Ok I will revert the two patches I committed (big patch + small
> patch). But I want to clarify one thing that not every big patch can
> be easily understood by just splitting to small patches.

Certainly, but it is always helpful to split as much as is possible.
The shorter a patch is to review, the more likely reviewers are to
spot mistakes - especially when reviewing code they are not already
very familiar with.

As an example, I have split your set up a little bit further by
breaking 2/3 into two separate patches.
https://git.linaro.org/people/leif.lindholm/edk2.git/shortlog/refs/heads/ray-pci

I have also tried to remove modifications completely unrelated to this
fix - resulting in a difference between the tree with your current
patch set and my variant as included below. The changes in your set
are all valid changes, but they are not related to the fix.

The method behind all of this madness is to make the output of "git
blame" as relevant as possible, as well as making automated "git
bisect" sessions for tracking down which specific change caused an
issue more useful.

Best Regards,

Leif


Diff between Ray's v2 and my version of it:

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
index 12647be..523c698 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c
@@ -281,13 +281,13 @@ DumpResourceMap (
   IN UINTN ResourceCount
   )
 {
-  EFI_STATUS   Status;
-  LIST_ENTRY   *Link;
-  PCI_IO_DEVICE*Device;
-  UINTNIndex;
-  CHAR16   *Str;
-  PCI_RESOURCE_NODE**ChildResources;
-  UINTNChildResourceCount;
+  EFI_STATUS   Status;^M
+  LIST_ENTRY   *Link;^M
+  PCI_IO_DEVICE*Device;^M
+  UINTNIndex;^M
+  CHAR16   *Str;^M
+  PCI_RESOURCE_NODE**ChildResources;^M
+  UINTNChildResourceCount;^M
 
   DEBUG ((EFI_D_INFO, "PciBus: Resource Map for "));
 
@@ -976,7 +976,7 @@ PciScanBus (
   UINT8 Device;
   UINT8 Func;
   UINT64Address;
-  UINT8 SecondBus;
+  UINTN SecondBus;^M
   UINT8 PaddedSubBus;
   UINT16Register;
   UINTN HpIndex;
@@ -1211,7 +1211,7 @@ PciScanBus (
 
   Status = PciScanBus (
 PciDevice,
-SecondBus,
+(UINT8) (SecondBus),^M
 SubBusNumber,
 PaddedBusRange
 );
@@ -1227,7 +1227,7 @@ PciScanBus (
   if ((Attributes == EfiPaddingPciRootBridge) &&
   (State & EFI_HPC_STATE_ENABLED) != 0&&
   (State & EFI_HPC_STATE_INITIALIZED) != 0) {
-*PaddedBusRange = (UINT8) ((UINT8) (BusRange) + *PaddedBusRange);
+*PaddedBusRange = (UINT8) ((UINT8) (BusRange) +*PaddedBusRange);^M
   } else {
 //
 // Reserve the larger one between the actual occupied bus
 // number and padded bus number
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index e12d59f..fb6270b 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -245,6 +245,7 @@ CalculateApertureIo16 (
   ) {
 
 Node = RESOURCE_NODE_FROM_LINK (CurrentLink);
+^M
 if (Node->ResourceUsage == PciResUsagePadding) {
   ASSERT (PaddingAperture == 0);
   PaddingAperture = Node->Length;
@@ -303,7 +304,8 @@ CalculateApertureIo16 (
   }
 
   //
-  // Adjust the aperture with the bridge's alignment
+  // At last, adjust the aperture with the bridge's^M
+  // alignment^M
   //
   Offset = Aperture & (Bridge->Alignment);
 
@@ -346,6 +348,7 @@ CalculateResourceAperture (
   UINT64Aperture;
   LIST_ENTRY*CurrentLink;
   PCI_RESOURCE_NODE *Node;
+^M
   UINT64PaddingAperture;
   UINT64Offset;
 
@@ -419,7 +422,7 @@ CalculateResourceAperture (
   }
 
   //
-  // Adjust the bridge's alignment to the first child's alignment
+  // At last, adjust the bridge's alignment to the first child's
   // alignment^M
   // if the bridge has at least one child
   //
   CurrentLink = Bridge->ChildList.ForwardLink;


> 
> Thanks,
> Ray
> 
> -Original Message-
> From: Tian, Feng 
> Sent: Friday, October 30, 2015 9:57 AM
> To: Leif Lindholm 
> Cc: Ni, Ruiyu ; Kinney, Michael D 
> ; edk2-devel@lists.01.org; Fan, Jeff 
> ; Tian, Feng

[edk2] [PATCH] UefiCpuPkg: PiSmmCpuDxeSmm: Remove Framework compatibility

2015-10-30 Thread Michael Kinney

The PiSmmCpuDxeSmm module is using PcdFrameworkCompatibilitySupport to
provide compatibility with the SMM support in the IntelFrameworkPkg.
This change removes the Framework compatibility and requires all SMM
modules that provide SMI handlers to follow the PI Specification.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Michael Kinney 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c   | 60 
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h   |  1 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf |  3 --
 3 files changed, 8 insertions(+), 56 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index e210c8d..7e4d0d9 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -74,17 +74,10 @@ EFI_SMM_CPU_PROTOCOL  mSmmCpu  = {
   SmmWriteSaveState
 };
 
 EFI_CPU_INTERRUPT_HANDLER   mExternalVectorTable[EXCEPTION_VECTOR_NUMBER];
 
-///
-/// SMM CPU Save State Protocol instance
-///
-EFI_SMM_CPU_SAVE_STATE_PROTOCOL  mSmmCpuSaveState = {
-  NULL
-};
-
 //
 // SMM stack information
 //
 UINTN mSmmStackArrayBase;
 UINTN mSmmStackArrayEnd;
@@ -528,42 +521,32 @@ SmmRestoreCpu (
 //
 InitializeDebugAgent (DEBUG_AGENT_INIT_THUNK_PEI_IA32TOX64, (VOID 
*), NULL);
   }
 
   //
-  // Do below CPU things for native platform only
+  // Skip initialization if mAcpiCpuData is not valid
   //
-  if (!FeaturePcdGet(PcdFrameworkCompatibilitySupport)) {
+  if (mAcpiCpuData.NumberOfCpus > 0) {
 //
-// Skip initialization if mAcpiCpuData is not valid
+// First time microcode load and restore MTRRs
 //
-if (mAcpiCpuData.NumberOfCpus > 0) {
-  //
-  // First time microcode load and restore MTRRs
-  //
-  EarlyInitializeCpu ();
-}
+EarlyInitializeCpu ();
   }
 
   //
   // Restore SMBASE for BSP and all APs
   //
   SmmRelocateBases ();
 
   //
-  // Do below CPU things for native platform only
+  // Skip initialization if mAcpiCpuData is not valid
   //
-  if (!FeaturePcdGet(PcdFrameworkCompatibilitySupport)) {
+  if (mAcpiCpuData.NumberOfCpus > 0) {
 //
-// Skip initialization if mAcpiCpuData is not valid
+// Restore MSRs for BSP and all APs
 //
-if (mAcpiCpuData.NumberOfCpus > 0) {
-  //
-  // Restore MSRs for BSP and all APs
-  //
-  InitializeCpu ();
-}
+InitializeCpu ();
   }
 
   //
   // Set a flag to restore SMM configuration in S3 path.
   //
@@ -685,17 +668,10 @@ SmmReadyToLockEventNotify (
   // Prevent use of mAcpiCpuData by initialize NumberOfCpus to 0
   //
   mAcpiCpuData.NumberOfCpus = 0;
 
   //
-  // If FrameworkCompatibilitySspport is enabled, then do not copy CPU S3 Data 
into SMRAM
-  //
-  if (FeaturePcdGet (PcdFrameworkCompatibilitySupport)) {
-goto Done;
-  }
-
-  //
   // If PcdCpuS3DataAddress was never set, then do not copy CPU S3 Data into 
SMRAM
   //
   AcpiCpuData = (ACPI_CPU_DATA *)(UINTN)PcdGet64 (PcdCpuS3DataAddress);
   if (AcpiCpuData == 0) {
 goto Done;
@@ -1007,11 +983,10 @@ PiCpuSmmEntry (
   gSmmCpuPrivate->CpuSaveState = (VOID **)AllocatePool (sizeof (VOID *) * 
mMaxNumberOfCpus);
   ASSERT (gSmmCpuPrivate->CpuSaveState != NULL);
 
   mSmmCpuPrivateData.SmmCoreEntryContext.CpuSaveStateSize = 
gSmmCpuPrivate->CpuSaveStateSize;
   mSmmCpuPrivateData.SmmCoreEntryContext.CpuSaveState = 
gSmmCpuPrivate->CpuSaveState;
-  mSmmCpuSaveState.CpuSaveState = (EFI_SMM_CPU_STATE 
**)gSmmCpuPrivate->CpuSaveState;
 
   //
   // Allocate buffer for pointers to array in CPU_HOT_PLUG_DATA.
   //
   mCpuHotPlugData.ApicId = (UINT64 *)AllocatePool (sizeof (UINT64) * 
mMaxNumberOfCpus);
@@ -1148,29 +1123,10 @@ PiCpuSmmEntry (
   // Initialize SMM CPU Services Support
   //
   Status = InitializeSmmCpuServices (mSmmCpuHandle);
   ASSERT_EFI_ERROR (Status);
 
-  if (FeaturePcdGet (PcdFrameworkCompatibilitySupport)) {
-//
-// Install Framework SMM Save State Protocol into UEFI protocol database 
for backward compatibility
-//
-Status = SystemTable->BootServices->InstallMultipleProtocolInterfaces (
-  >SmmCpuHandle,
-  ,
-  ,
-  NULL
-  );
-ASSERT_EFI_ERROR (Status);
-//
-// The SmmStartupThisAp service in Framework SMST should always be 
non-null.
-// Update SmmStartupThisAp pointer in PI SMST here so that PI/Framework 
SMM thunk
-// can have it ready when constructing Framework SMST.
-//
-gSmst->SmmStartupThisAp = SmmStartupThisAp;
-  }
-
   //
   // register SMM Ready To Lock Protocol notification
   //
   Status = gSmst->SmmRegisterProtocolNotify (
 ,
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index

Re: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

Re: [edk2] [PATCH] NetworkPkg: HttpDxe: Address double FreePool issue

Re: [edk2] RE enquote PATH: edk2-devel Digest, Vol 4, Issue 214

Re: [edk2] [PATCH] MdeModulePkg: Fix memory leak issues

Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

[edk2] [PATCH] Add common HTTP definitions for use in HTTP clients/applications. List includes: HTTP methods, request/response headers, and encodings.

[edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching max perf log entries

[edk2] [PATCH] MdeModulePkg: Add DEBUG statement on reaching PEI perf log max entries

Re: [edk2] [PATCH v2] MdeModulePkg: Add some common HTTP definitions

[edk2] [PATCH v2] MdeModulePkg: Add some common HTTP definitions

Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

[edk2] SecurityPkg: PeiRsa2048Sha256GuidedSectionExtractLib error handling

[edk2] [PATCH] MdeModulePkg: PiSmmCore: Remove confusing CopyMem() of SMM_ENTRY_CONTEXT

Re: [edk2] [PATCH 6/6] OvmfPkg/PlatformPei: Set PcdCpuMaxLogicalProcessorNumber using QEMU fw_cfg

Re: [edk2] [PATCH 6/6] OvmfPkg/PlatformPei: Set PcdCpuMaxLogicalProcessorNumber using QEMU fw_cfg

Re: [edk2] [PATCH 6/6] OvmfPkg/PlatformPei: Set PcdCpuMaxLogicalProcessorNumber using QEMU fw_cfg

Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in S3Resume2Pei

Re: [edk2] about the SMM_S3_RESUME_SMM_64 branch in S3Resume2Pei

Re: [edk2] [Patch] MdeModulePkg: Fix a PciBusDxe hot plug bug

[edk2] [PATCH] UefiCpuPkg: PiSmmCpuDxeSmm: Remove Framework compatibility

20 matches

Site Navigation

Mail list logo

Footer information