[edk2] [Patch] BaseTools/GenFds: remove the old logic since ActivePlatform is abs. path

[Yonghong Zhu]

We can support the DSC file out of workspace. this old logic first make
the absolute path to relative path and strips the leading slash off,
then append it to workspace. it cause GenFds failure on Linux when the
DSC file is out of workspace. Since we make sure the ActivePlatform is
abs. path, so we don't need this old logic to change the abs. path to
relative.

Cc: Liming Gao 
Cc: Marvin Haeuser 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/GenFds/GenFds.py | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
index d97fc28..c2a2bd3 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -136,22 +136,14 @@ def main():
 if not os.path.isabs (ActivePlatform):
 ActivePlatform = mws.join(GenFdsGlobalVariable.WorkSpaceDir, 
ActivePlatform)
 
 if not os.path.exists(ActivePlatform)  :
 EdkLogger.error("GenFds", FILE_NOT_FOUND, "ActivePlatform 
doesn't exist!")
-
-if os.path.normcase (ActivePlatform).find(Workspace) == 0:
-ActivePlatform = mws.relpath(ActivePlatform, Workspace)
-if len(ActivePlatform) > 0 :
-if ActivePlatform[0] == '\\' or ActivePlatform[0] == '/':
-ActivePlatform = ActivePlatform[1:]
-else:
-EdkLogger.error("GenFds", FILE_NOT_FOUND, "ActivePlatform 
doesn't exist!")
 else:
 EdkLogger.error("GenFds", OPTION_MISSING, "Missing active 
platform")
 
-GenFdsGlobalVariable.ActivePlatform = 
PathClass(NormPath(ActivePlatform), Workspace)
+GenFdsGlobalVariable.ActivePlatform = 
PathClass(NormPath(ActivePlatform))
 
 if (Options.ConfDirectory):
 # Get alternate Conf location, if it is absolute, then just use 
the absolute directory name
 ConfDirectoryPath = os.path.normpath(Options.ConfDirectory)
 if ConfDirectoryPath.startswith('"'):
-- 
2.6.1.windows.1

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

Re: [edk2] GenFds fails with MWS.

[Zhu, Yonghong]

Hi Marvin,

Thanks for the info. I can reproduce it.
You are correct. The below block is long ago 's logic, we can remove it now 
since for the ActivePlatform we already make sure it is absolute path.
I have a patch to remove this block. thanks.
If you have time, please help to review/test it. thanks.
Patch: [edk2] [Patch] BaseTools/GenFds: remove the old logic since   
ActivePlatform is abs. path

Best Regards,
Zhu Yonghong

From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
Sent: Saturday, April 16, 2016 5:08 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong ; Gao, Liming 
Subject: RE: [edk2] GenFds fails with MWS.


PS: I think it might work on Windows because GenFds checks the first character 
of \ or /. Though Windows paths start with the drive letter. :)



Von: Marvin Häuser
Gesendet: Freitag, 15. April 2016 22:51
An: edk2-devel@lists.01.org
Cc: liming@intel.com
Betreff: Re: [edk2] GenFds fails with MWS.


Hey Yonghong,


I have investigated the issue a little more. For some reason, it works fine on 
Windows, though is broken in WSL. After outputing some of the steps done to the 
ActivePlatform path, I figured out it was this block that causes the issue:


if os.path.normcase (ActivePlatform).find(Workspace) == 0:

 ActivePlatform = mws.relpath(ActivePlatform, Workspace)

if len(ActivePlatform) > 0 :

 if ActivePlatform[0] == '\\' or ActivePlatform[0] == '/':

 ActivePlatform = ActivePlatform[1:]



This block strips the leading slash (making it an absolute path) off and then 
appends it to the Workspace path later on. I assume the intention is to 1) make 
it relative to WORKSPACE and then 2) append it to it (why?). Could this be 
solved by only running that code when the path is not already absolute? Why 
would one need to make the abs. path relatve?

I wish I could tell you why it works fine on Windows and not in the Subsystem, 
but I have no idea.


The steps to reproduce are quite easy - just build a package with a fdf, while 
that package is not in WORKSPACE. I cannot test it in a Virtual Machine right 
now, but will try as soon as possible.


Kind regards,

Marvin.



Von: Zhu, Yonghong mailto:yonghong@intel.com>>
Gesendet: Freitag, 15. April 2016 08:34
An: marvin.haeu...@outlook.com
Cc: Gao, Liming; Zhu, Yonghong
Betreff: RE: GenFds fails with MWS.


Hi Marvin,



I failed to reproduce this bug,  Do you have any reproduce steps ? thanks in 
advance.



Best Regards,

Zhu Yonghong



From: Zhu, Yonghong
Sent: Friday, April 15, 2016 8:38 AM
To: 'Marvin Häuser' 
mailto:marvin.haeu...@outlook.com>>; 
edk2-devel@lists.01.org
Cc: Gao, Liming mailto:liming@intel.com>>; Zhu, 
Yonghong mailto:yonghong@intel.com>>
Subject: RE: GenFds fails with MWS.



Hi Marvin,



Thanks for report it. I will first to reproduce and investigate.



Best Regards,

Zhu Yonghong



From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
Sent: Friday, April 15, 2016 2:05 AM
To: 
edk2-devel@lists.01.org>
Cc: Gao, Liming 
mailto:liming@intel.com>>;
 Zhu, Yonghong 
mailto:yonghong@intel.com>>
Subject: GenFds fails with MWS.



Hello dear TianoCore developers,

Liming,

Yonghong,



Today I wanted to try out the changes intoduced with the 'BaseTools: fix 
PLATFORM_DIR variable value for multiple workspace' commit. After having fixed 
the 'brace-mismatched' I sent as a patch earlier today, the build process works 
fine until GenFds is invoked.



For some reason, the correct file path is appended to the WORKSPACE path, which 
of course leads to an error. You can find the macro definitions and error log 
below.



I am sorry to bug you again about BaseTools, though, after having looked at the 
call stack for at least half an hour, I still don't understand the control flow 
as Python is giving me headaches.



Thank you in advance for your time!



Regards,

Marvin.







root@localhost:/home/EdkWorkspace/edk2# build -a X64 -b RELEASE -t GCC49 -p 
AnyPkg/AnyPkg.dsc

Build environment: Linux-3.4.0+-x86_64-with-Ubuntu-14.04-trusty

Build start time: 17:56:48, Apr.14 2016



WORKSPACE= /home/EdkWorkspace/edk2

PACKAGES_PATH= /home/EdkWorkspace/PackagesPath

ECP_SOURCE   = /home/EdkWorkspace/edk2/EdkCompatibilityPkg

EDK_SOURCE   = /home/EdkWorkspace/edk2/EdkCompatibilityPkg

EFI_SOURCE   = /home/EdkWorkspace/edk2/EdkCompatibilityPkg

EDK_TOOLS_PATH   = /home/EdkWorkspace/edk2/BaseTools





Architecture(s)  = X64

Build target = RELEASE

Toolchain= GCC49



Active Platform  = /home/EdkWorkspace/PackagesPath/AnyPkg/AnyPkg.dsc

Flash Ima

[edk2] [PATCH v2 0/2] Get default value from CallBack function for OrderedList

[Dandan Bi]

For a question, it can retrive default value from Callback() function.
For OrderedList question, the value is stored in a buffer, not in
HiiValue. So when calling Callback() function to get default value for
OrderedList, it need to pass the buffer, not the HiiValue.
Patch 1 is to enahnce the SetupBrowser to fix the issue described above.
Patch 2 is to add a sample case for orderedlist to get default value
through Callback() function.

Notes:
  V2:
  - Refine the sample case in patch 2.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 

Dandan Bi (2):
  MdeModulePkg/SetupBrowserDxe: Get default from callback for
orderedList
  MdeModulePkg/DriverSampleDxe: Add a sample case

 .../Universal/DriverSampleDxe/DriverSample.c   | 53 +-
 .../Universal/DriverSampleDxe/NVDataStruc.h|  1 +
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 13 +-
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 10 +++-
 4 files changed, 74 insertions(+), 3 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 v2 1/2] MdeModulePkg/SetupBrowserDxe: Get default from callback for orderedList

[Dandan Bi]

For orderedlist question, the value is stored in a buffer,
not in HiiValue. So when need to get default value from callback
function for orderedlist, need to pass the buffer.
This patch is to enhance this logic.


Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Universal/SetupBrowserDxe/Setup.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c 
b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
index b357e29..c36588e 100644
--- a/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
+++ b/MdeModulePkg/Universal/SetupBrowserDxe/Setup.c
@@ -3895,10 +3895,11 @@ GetQuestionDefault (
   EFI_STRING  StrValue;
   EFI_HII_CONFIG_ACCESS_PROTOCOL  *ConfigAccess;
   EFI_BROWSER_ACTION_REQUEST  ActionRequest;
   INTNAction;
   CHAR16  *NewString;
+  EFI_IFR_TYPE_VALUE  *TypeValue;
 
   Status   = EFI_NOT_FOUND;
   StrValue = NULL;
 
   //
@@ -3915,10 +3916,17 @@ GetQuestionDefault (
   //  3, use nested EFI_IFR_DEFAULT 
   //  4, set flags of EFI_ONE_OF_OPTION (provide Standard and Manufacturing 
default)
   //  5, set flags of EFI_IFR_CHECKBOX (provide Standard and Manufacturing 
default) (lowest priority)
   //
   HiiValue = &Question->HiiValue;
+  TypeValue = &HiiValue->Value;
+  if (HiiValue->Type == EFI_IFR_TYPE_BUFFER && Question->BufferValue != NULL) {
+//
+// For orderedlist, need to pass the BufferValue to Callback function.
+//
+TypeValue = (EFI_IFR_TYPE_VALUE *) Question->BufferValue;
+  }
 
   //
   // Get Question defaut value from call back function.
   //
   ConfigAccess = FormSet->ConfigAccess;
@@ -3928,11 +3936,11 @@ GetQuestionDefault (
 Status = ConfigAccess->Callback (
  ConfigAccess,
  Action,
  Question->QuestionId,
  HiiValue->Type,
- &HiiValue->Value,
+ TypeValue,
  &ActionRequest
  );
 if (!EFI_ERROR (Status)) {
   if (HiiValue->Type == EFI_IFR_TYPE_STRING) {
 NewString = GetToken (Question->HiiValue.Value.string, 
FormSet->HiiHandle);
-- 
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 2/2] MdeModulePkg/DriverSampleDxe: Add a sample case

[Dandan Bi]

Add the sample case for orderedlist to get standard
default value from Callback function.

Notes:
  V2:
  - Refine the sample case.

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/DriverSampleDxe/DriverSample.c   | 53 +-
 .../Universal/DriverSampleDxe/NVDataStruc.h|  1 +
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr | 13 +-
 3 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c 
b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
index cdb8889..f682560 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
+++ b/MdeModulePkg/Universal/DriverSampleDxe/DriverSample.c
@@ -67,10 +67,52 @@ HII_VENDOR_DEVICE_PATH  mHiiVendorDevicePath1 = {
 }
   }
 };
 
 /**
+  Set value of a data element in an Array by its Index.
+
+  @param  Array  The data array.
+  @param  Type   Type of the data in this array.
+  @param  Index  Zero based index for data in this array.
+  @param  Value  The value to be set.
+
+**/
+VOID
+SetArrayData (
+  IN VOID *Array,
+  IN UINT8Type,
+  IN UINTNIndex,
+  IN UINT64   Value
+  )
+{
+
+  ASSERT (Array != NULL);
+
+  switch (Type) {
+  case EFI_IFR_TYPE_NUM_SIZE_8:
+*(((UINT8 *) Array) + Index) = (UINT8) Value;
+break;
+
+  case EFI_IFR_TYPE_NUM_SIZE_16:
+*(((UINT16 *) Array) + Index) = (UINT16) Value;
+break;
+
+  case EFI_IFR_TYPE_NUM_SIZE_32:
+*(((UINT32 *) Array) + Index) = (UINT32) Value;
+break;
+
+  case EFI_IFR_TYPE_NUM_SIZE_64:
+*(((UINT64 *) Array) + Index) = (UINT64) Value;
+break;
+
+  default:
+break;
+  }
+}
+
+/**
   Add empty function for event process function.
 
   @param EventThe Event need to be process
   @param Context  The context of the event.
 
@@ -1281,20 +1323,23 @@ DriverCallback (
   EFI_FORM_ID FormId;
   EFI_STRING  Progress;
   EFI_STRING  Results;
   UINT32  ProgressErr;
   CHAR16  *TmpStr;
-  
+  UINTN   Index;
+  UINT64  BufferValue;
+
   if (((Value == NULL) && (Action != EFI_BROWSER_ACTION_FORM_OPEN) && (Action 
!= EFI_BROWSER_ACTION_FORM_CLOSE))||
 (ActionRequest == NULL)) {
 return EFI_INVALID_PARAMETER;
   }
 
 
   FormId = 0;
   ProgressErr = 0;
   Status = EFI_SUCCESS;
+  BufferValue = 3;
   PrivateData = DRIVER_SAMPLE_PRIVATE_FROM_THIS (This);
 
   switch (Action) {
   case EFI_BROWSER_ACTION_FORM_OPEN:
 {
@@ -1468,10 +1513,16 @@ DriverCallback (
   switch (QuestionId) {
   case 0x1240:
 Value->u8 = DEFAULT_CLASS_STANDARD_VALUE;
   break;
 
+  case 0x1252:
+for (Index = 0; Index < 3; Index ++) {
+  SetArrayData (Value, EFI_IFR_TYPE_NUM_SIZE_8, Index, BufferValue--);
+}
+break;
+
   default:
 Status = EFI_UNSUPPORTED;
   break;
   }
 }
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h 
b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
index 0b9e15f..17b4d99 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
+++ b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
@@ -67,10 +67,11 @@ typedef struct {
   UINT8   GetDefaultValueFromCallBack;
   UINT8   GetDefaultValueFromAccess;
   EFI_HII_TIME  Time;
   UINT8   RefreshGuidCount;
   UINT8   Match2;
+  UINT8   GetDefaultValueFromCallBackForOrderedList[3];
 } DRIVER_SAMPLE_CONFIGURATION;
 
 //
 // 2nd NV data structure definition
 //
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr 
b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
index d1ce68d..6e7b96b 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
+++ b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
@@ -1,10 +1,10 @@
 ///** @file
 //
 //Sample Setup formset.
 //
-//  Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+//  Copyright (c) 2004 - 2016, 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
 //
@@ -432,10 +432,21 @@ formset
 minimum = 0,
 maximum = 255,
 step= 1,
 default = 18,
 endnumeric;
+
+orderedlist
+varid   = MyIfrNVData.GetDefaultValueFromCallBackForOrderedList,
+questionid  = 0x1252,
+prompt  = STRING_TOKEN(STR_DEFAULT_VALUE_FROM_CALLBACK_PROMPT),
+help= STRING_TOKEN(STR_DEFAULT_VALUE_FROM_CALLBACK_HELP),
+flags   = INTERACTIVE,
+option text 

[edk2] [patch] MdeModulePkg: refine codes of iSCSI driver.

[Zhang Lubo]

Add error handling logic in DriverBingingStop function,
it may return error status when invoking the
UninstallProtocolInterface.

Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 .../Universal/Network/IScsiDxe/IScsiDriver.c   | 11 +--
 .../Universal/Network/IScsiDxe/IScsiMisc.c | 38 ++
 .../Universal/Network/IScsiDxe/IScsiMisc.h |  9 +++--
 3 files changed, 40 insertions(+), 18 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c 
b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
index e55bee8..74379e1 100644
--- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
+++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
@@ -1,9 +1,9 @@
 /** @file
   The entry point of IScsi driver.
 
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2016, 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
 
@@ -337,10 +337,14 @@ ON_ERROR:
   @param[in]  ChildHandleBuffer An array of child handles to be freed. May be 
NULL 
 if NumberOfChildren is 0.Not used.
 
   @retval EFI_SUCCESS   The device was stopped.
   @retval EFI_DEVICE_ERROR  The device could not be stopped due to a 
device error.
+  @retval EFI_INVALID_PARAMETER Child handle is NULL.
+  @retval EFI_ACCESS_DENIED The interface was not removed because the 
interface is
+still being used by a driver.
+
 **/
 EFI_STATUS
 EFIAPI
 IScsiDriverBindingStop (
   IN EFI_DRIVER_BINDING_PROTOCOL  *This,
@@ -449,11 +453,14 @@ IScsiDriverBindingStop (
   // Update the iSCSI Boot Firware Table.
   //
   IScsiPublishIbft ();
 
   IScsiSessionAbort (&Private->Session);
-  IScsiCleanDriverData (Private);
+  Status = IScsiCleanDriverData (Private);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
 
   return EFI_SUCCESS;
 }
 
 /**
diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c 
b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
index ebd9f5b..bb48d8c 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 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2016, 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
 
@@ -585,38 +585,50 @@ IScsiCreateDriverData (
 }
 
 /**
   Clean the iSCSI driver data.
 
-  @param[in]  Private The iSCSI driver data.
+  @param[in]  Private The iSCSI driver data.
+
+  @retval EFI_SUCCES  The clean operation is successful.
+  @retval Others  Other errors as indicated.
+
 **/
-VOID
+EFI_STATUS
 IScsiCleanDriverData (
   IN ISCSI_DRIVER_DATA  *Private
   )
 {
+  EFI_STATUS Status;
+
   if (Private->DevicePath != NULL) {
-gBS->UninstallProtocolInterface (
-  Private->ExtScsiPassThruHandle,
-  &gEfiDevicePathProtocolGuid,
-  Private->DevicePath
-  );
+Status = gBS->UninstallProtocolInterface (
+Private->ExtScsiPassThruHandle,
+&gEfiDevicePathProtocolGuid,
+Private->DevicePath
+);
+if (EFI_ERROR (Status)) {
+  goto EXIT;
+}
 
 FreePool (Private->DevicePath);
   }
 
   if (Private->ExtScsiPassThruHandle != NULL) {
-gBS->UninstallProtocolInterface (
-  Private->ExtScsiPassThruHandle,
-  &gEfiExtScsiPassThruProtocolGuid,
-  &Private->IScsiExtScsiPassThru
-  );
+Status = gBS->UninstallProtocolInterface (
+Private->ExtScsiPassThruHandle,
+&gEfiExtScsiPassThruProtocolGuid,
+&Private->IScsiExtScsiPassThru
+);
   }
 
+EXIT:
+
   gBS->CloseEvent (Private->ExitBootServiceEvent);
 
   FreePool (Private);
+  return Status;
 }
 
 /**
   Check wheather the Controller is configured to use DHCP protocol.
 
diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.h 
b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.h
index 8a95493..0ab44d0 100644
--- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.h
+++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.h
@@ -1,9 +1,9 @@
 /** @file
   Miscellaneous definitions for iSCSI driver.
 
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+Cop

[edk2] [patch] NetworkPkg: refine codes of iSCSI driver.

[Zhang Lubo]

Add error handling logic in DriverBingingStop function,
it may return error status when invoking the
UninstallProtocolInterface.

Cc: Fu Siyuan 
Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 NetworkPkg/IScsiDxe/IScsiDriver.c | 20 
 NetworkPkg/IScsiDxe/IScsiMisc.c   | 25 +
 NetworkPkg/IScsiDxe/IScsiMisc.h   |  9 ++---
 3 files changed, 39 insertions(+), 15 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c 
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 51ce169..cdc818f 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -1,9 +1,9 @@
 /** @file
   The entry point of IScsi driver.
 
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2016, 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
 
@@ -321,11 +321,13 @@ IScsiSupported (
   @retval EFI_SUCCESThis driver was started.
   @retval EFI_ALREADY_STARTED   This driver is already running on this device.
   @retval EFI_INVALID_PARAMETER Any input parameter is invalid.
   @retval EFI_NOT_FOUND There is no sufficient information to establish
 the iScsi session.
-  @retval EFI_DEVICE_ERROR  Failed to get TCP connection device path.  

+  @retval EFI_DEVICE_ERROR  Failed to get TCP connection device path.
+  @retval EFI_ACCESS_DENIED The interface was not removed because the 
interface is
+still being used by a driver.
 
 **/
 EFI_STATUS
 IScsiStart (
   IN EFI_HANDLE   Image,
@@ -861,11 +863,14 @@ IScsiStart (
 IScsiRemoveNic (ExistPrivate->Controller);
 if (ExistPrivate->Session != NULL) {
   IScsiSessionAbort (ExistPrivate->Session);
 }
 
-IScsiCleanDriverData (ExistPrivate);
+Status = IScsiCleanDriverData (ExistPrivate);
+if (EFI_ERROR (Status)) {
+  goto ON_ERROR;
+}
   }
 } else {
   //
   // Use the attempt in earlier order as boot selected in single path mode.
   //
@@ -961,10 +966,13 @@ ON_ERROR:
 if NumberOfChildren is 0.
   @param[in]  IpVersion IP_VERSION_4 or IP_VERSION_6.
   
   @retval EFI_SUCCESS   The device was stopped.
   @retval EFI_DEVICE_ERROR  The device could not be stopped due to a 
device error.
+  @retval EFI_INVALID_PARAMETER Child handle is NULL.
+  @retval EFI_ACCESS_DENIED The interface was not removed because the 
interface is
+still being used by a driver.
 
 **/
 EFI_STATUS
 EFIAPI
 IScsiStop (
@@ -1103,11 +,15 @@ IScsiStop (
 
   if (Private->Session != NULL) {
 IScsiSessionAbort (Private->Session);
   }
 
-  IScsiCleanDriverData (Private);
+  Status = IScsiCleanDriverData (Private);
+
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
 
   return EFI_SUCCESS;
 }
 
 /**
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index 5fc25a0..2406717 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -1,9 +1,9 @@
 /** @file
   Miscellaneous routines for iSCSI driver.
 
-Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2004 - 2016, 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
 
@@ -855,26 +855,32 @@ IScsiCreateDriverData (
 
 
 /**
   Clean the iSCSI driver data.
 
-  @param[in]  Private The iSCSI driver data.
+  @param[in]  Private The iSCSI driver data.
+
+  @retval EFI_SUCCES  The clean operation is successful.
+  @retval Others  Other errors as indicated.
 
 **/
-VOID
+EFI_STATUS
 IScsiCleanDriverData (
   IN ISCSI_DRIVER_DATA  *Private
   )
 {
   EFI_STATUSStatus;
 
   if (Private->DevicePath != NULL) {
-gBS->UninstallProtocolInterface (
-   Private->ExtScsiPassThruHandle,
-   &gEfiDevicePathProtocolGuid,
-   Private->DevicePath
-   );
+Status = gBS->UninstallProtocolInterface (
+Private->ExtScsiPassThruHandle,
+&gEfiDevicePathProtocolGuid,
+Private->DevicePath
+);
+if (EFI_ERROR (Status)) {
+  goto EXIT;
+}
 
 FreePool (Private->DevicePath);
   }
 
   if (Private->ExtScsiPassThruHandle != NULL) {
@@ -886,13 +892,16 @@ IScsiCleanDriverData (
 if (!

[edk2] [PATCH] ShellPkg: Support connect a device handle recursively with '-r'.

[Qiu Shumin]

This patch make Shell 'connect' a device handle recursively with
'-r' option.

Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c 
b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
index 5d61c2f..dd3d4a6 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Connect.c
@@ -2,7 +2,7 @@
   Main file for connect shell Driver1 function.
 
   (C) Copyright 2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2016, 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
@@ -452,9 +452,10 @@ ShellCommandRunConnect (
 //
 Count = (gInReconnect?0x4:0x3);
 if ((ShellCommandLineGetCount(Package) > Count)
-  ||((ShellCommandLineGetFlag(Package, L"-r") || 
ShellCommandLineGetFlag(Package, L"-c")) && ShellCommandLineGetCount(Package)>1)
+  ||(ShellCommandLineGetFlag(Package, L"-c") && 
ShellCommandLineGetCount(Package)>1)
+  ||(ShellCommandLineGetFlag(Package, L"-r") && 
ShellCommandLineGetCount(Package)>2)
   ||(ShellCommandLineGetFlag(Package, L"-r") && 
ShellCommandLineGetFlag(Package, L"-c") )
- ){
+){
   //
   // error for too many parameters
   //
-- 
2.7.1.windows.2

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

Re: [edk2] [PATCH] ArmPlatformPkg: fixups for 64-bit mailbox pointers

[Ard Biesheuvel]

On 8 April 2016 at 13:26, Ard Biesheuvel  wrote:
> On 24 March 2016 at 23:33, Ard Biesheuvel  wrote:
>> On 24 March 2016 at 21:30, Leo Duran  wrote:
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Leo Duran 
>>> ---
>>>  ArmPlatformPkg/PrePeiCore/MainMPCore.c | 10 --
>>>  ArmPlatformPkg/PrePi/MainMPCore.c  | 10 --
>>>  2 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>
>>
>> ArmPlatformPkg/ArmVExpressPkg/Include/VExpressMotherBoard.h has the
>> following defines
>>
>> #define ARM_VE_SYS_FLAGS_SET_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00030)
>> #define ARM_VE_SYS_FLAGS_CLR_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00034)
>> #define ARM_VE_SYS_FLAGS_NV_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00038)
>>
>> and is used on Juno as well as the older 32-bit archs. I don't know if
>> this is dead code now that PSCI is implemented by ARM trusted
>> firmware, and so we never bring up the secondaries in UEFI. But I
>> would like Leif's take on this when he gets back, since writing 64-bit
>> values is clearly not correct either in this particular case.
>>
>
> Hi Leif,
>
> It would be good if you could shed some light on this one. It is not
> entirely clear to me how these mailboxes are backed on Vexpress, and
> whether we could support 64-bit addresses here.

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

Re: [edk2] 答复: 答复: 答复: [EDK2]an issue that PXE boot failed when received a NACK from the DHCP server

[Michael Brown]

On 18/04/16 02:57, Liuxilong (A) wrote:

We also do not want to make all of the boards the same MAC 
address. But our client requires us to do it. The client thinks that VLAN can 
solve this conflict of the same MAC through VLAN's isolation. What do you think 
about it?


It's a bad idea.  You're likely to hit multiple problems.

You may be able to work around it by running a separate IPv4 subnet on 
each VLAN and using a separate DHCP server process on each of these 
subnets.  It's very messy.


A better solution would probably be to have the NIC driver generate a 
transient random MAC address if the NIC does not have a valid permanent 
MAC address.  iPXE does this for some cards (e.g. Intel virtual 
functions, which may not have their own addresses).  You can use code 
such as:


  void eth_random_addr ( void *hw_addr ) {
uint8_t *addr = hw_addr;
unsigned int i;

for ( i = 0 ; i < ETH_ALEN ; i++ )
addr[i] = random();
addr[0] &= ~0x01; /* Clear multicast bit */
addr[0] |= 0x02; /* Set locally-assigned bit */
  }

i.e. generate six random bytes, then convert it to a valid MAC address 
by ensuring that the "multicast" bit is clear and the "locally-assigned" 
bit is set.


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

[edk2] [PATCH v3] MdeModulePkg/HiiDatabaseDxe: Support EfiVarStore to get AltCfg from Driver

[Dandan Bi]

Allow EfiVarStore to get  from Hii Driver, and enhance code logic
in MergeDefaultString function to get a full AltCfgResp.
The logic in function MergeDefaultString after enhancement:
(1) If there are no  in AltCfgResp, merge the  in
DefaultAltCfgResp to AltCfgResp.
(2) If there are  in AltCfgResp, for the same , if
the  already in AltCfgResp, don't need to merge from
DefaultAltCfgResp, else merge the  in the DefaultAltCfgResp
to the related  in AltCfgResp.
AltCfgResp: Generated by Driver.
DefaultAltCfgResp: Generated by HiiDatabase.

Notes:
  V3:
  - Enhance the logic in function FindSameBlockElement ().

Cc: Liming Gao 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 .../Universal/HiiDatabaseDxe/ConfigRouting.c   | 541 +
 1 file changed, 541 insertions(+)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 3a871cf..2e3608c 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -508,10 +508,481 @@ Exit:
 
   return Status;
 }
 
 /**
+ To find the BlockName in the string with same value.
+
+  @param  String Pointer to a Null-terminated Unicode string.
+  @param  BlockName  Pointer to a Null-terminated Unicode string 
to search for.
+  @param  Buffer Pointer to the value correspond to the 
BlockName.
+  @param  Found  The Block whether has been found.
+  @param  BufferLen  The length of the buffer.
+
+  @retval EFI_OUT_OF_RESOURCES   Insufficient resources to store neccessary 
structures.
+  @retval EFI_SUCCESSThe function finishes successfully.
+
+**/
+EFI_STATUS
+FindSameBlockElement(
+  IN  EFI_STRING   String,
+  IN  EFI_STRING   BlockName,
+  IN  UINT8*Buffer,
+  OUT BOOLEAN  *Found,
+  IN  UINTNBufferLen
+  )
+{
+  EFI_STRING   BlockPtr;
+  UINTNLength;
+  UINT8*TempBuffer;
+  EFI_STATUS   Status;
+
+  TempBuffer = NULL;
+  *Found = FALSE;
+  BlockPtr = StrStr (String, BlockName);
+
+  while (BlockPtr != NULL) {
+BlockPtr += StrLen (BlockName);
+Status = GetValueOfNumber (BlockPtr, &TempBuffer, &Length);
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+ASSERT (TempBuffer != NULL);
+if ((BufferLen == Length) && (0 == CompareMem (Buffer, TempBuffer, 
Length))) {
+  *Found = TRUE;
+  return EFI_SUCCESS;
+} else {
+  FreePool (TempBuffer);
+  TempBuffer = NULL;
+  BlockPtr = StrStr (BlockPtr + 1, BlockName);
+}
+  }
+  return EFI_SUCCESS;
+}
+
+/**
+  Compare the  in ConfigAltResp and DefaultAltCfgResp, if the 

+  in DefaultAltCfgResp but not in ConfigAltResp,add it to the ConfigAltResp.
+
+  @param  DefaultAltCfgResp  Pointer to a null-terminated Unicode string in
+  format. The default value
+ string may contain more than one ConfigAltResp
+ string for the different varstore buffer.
+  @param  ConfigAltResp  Pointer to a null-terminated Unicode string in
+  format.
+  @param  AltConfigHdr   Pointer to a Unicode string in  
format.
+  @param  ConfigAltRespChanged   Whether the ConfigAltResp has been changed.
+
+  @retval EFI_OUT_OF_RESOURCES   Insufficient resources to store neccessary 
structures.
+  @retval EFI_SUCCESSThe function finishes  successfully.
+
+**/
+EFI_STATUS
+CompareBlockElementDefault (
+  IN  EFI_STRING  DefaultAltCfgResp,
+  IN OUT  EFI_STRING  *ConfigAltResp,
+  IN  EFI_STRING  AltConfigHdr,
+  IN OUT  BOOLEAN *ConfigAltRespChanged
+)
+{
+  EFI_STATUSStatus;
+  EFI_STRINGBlockPtr;
+  EFI_STRINGBlockPtrStart;
+  EFI_STRINGStringPtr;
+  EFI_STRINGAppendString;
+  EFI_STRINGAltConfigHdrPtr;
+  UINT8 *TempBuffer;
+  UINTN OffsetLength;
+  UINTN AppendSize;
+  UINTN TotalSize;
+  BOOLEAN   FoundOffset;
+
+  AppendString = NULL;
+  TempBuffer   = NULL;
+  //
+  // Make BlockPtr point to the first  with AltConfigHdr in 
DefaultAltCfgResp.
+  //
+  AltConfigHdrPtr = StrStr (DefaultAltCfgResp, AltConfigHdr);
+  ASSERT (AltConfigHdrPtr != NULL);
+  BlockPtr = StrStr (AltConfigHdrPtr, L"&OFFSET=");
+  //
+  // Make StringPtr point to the AltConfigHdr in ConfigAltResp.
+  //
+  StringPtr = StrStr (*ConfigAltResp, AltConfigHdr);
+  ASSERT (StringPtr != NULL);
+
+  while (BlockPtr != NULL) {
+//
+// Find the "&OFFSET=" block and get the value of the Number with 
AltConfigHdr in DefaultAltCfgResp.
+//
+BlockPtrStart = BlockPtr;
+BlockPtr += StrLen (L"&OFFSET=");
+Status = GetValueOfNumber (BlockPtr, &TempBuffer, &OffsetLength);
+if (EFI_ERROR (Status)) {
+  Status = EFI_OUT_OF_RESOURCES;
+  goto Exit;
+}
+//
+// To find the

Re: [edk2] [Patch 2/3] OvmfPkg: Create PlatformBootManagerLib to work with UefiBootManagerLib

[Gary Lin]

On Mon, Apr 18, 2016 at 02:43:02PM +0800, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 

Hi Ruiyu,

I got the error when building OVMF with OvmfPkg/build.sh:

Building ... 
/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
 [X64]
"gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror -Wno-array-bounds 
-ffunction-sections -fdata-sections -c -include AutoGen.h -fno-common 
-DSTRING_ARRAY_NAME=PlatformBootManagerLibStrings -m64 -fno-stack-protector 
"-DEFIAPI=__attribute__((ms_abi))" -DNO_BUILTIN_VA_FUNCS -mno-red-zone 
-Wno-address -mcmodel=large -fno-asynchronous-unwind-tables -Wno-address 
-Wno-unused-but-set-variable -mno-mmx -mno-sse -o 
/home/gary/git/edk2/Build/OvmfX64/DEBUG_GCC49/X64/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib/OUTPUT/./BdsPlatform.obj
 -I/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib 
-I/home/gary/git/edk2/Build/OvmfX64/DEBUG_GCC49/X64/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib/DEBUG
 -I/home/gary/git/edk2/MdePkg -I/home/gary/git/edk2/MdePkg/Include 
-I/home/gary/git/edk2/MdePkg/Include/X64 -I/home/gary/git/edk2/MdeModulePkg 
-I/home/gary/git/edk2/MdeModulePkg/Include 
-I/home/gary/git/edk2/IntelFrameworkModulePkg 
-I/home/gary/git/edk2/IntelFrameworkModulePkg/Include 
-I/home/gary/git/edk2/OvmfPkg -I/home/gary/git/edk2/OvmfPkg/Include 
/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
In file included from 
/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:15:0:
/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h:219:1: 
error: conflicting types for ‘EnableQuietBoot’
 EnableQuietBoot (
 ^
In file included from 
/home/gary/git/edk2/IntelFrameworkModulePkg/Include/Library/PlatformBdsLib.h:20:0,
 from 
/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h:43,
 from 
/home/gary/git/edk2/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:15:
/home/gary/git/edk2/IntelFrameworkModulePkg/Include/Library/GenericBdsLib.h:1095:1:
 note: previous declaration of ‘EnableQuietBoot’ was here
 EnableQuietBoot (
 ^
make: *** 
[/home/gary/git/edk2/Build/OvmfX64/DEBUG_GCC49/X64/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib/OUTPUT/BdsPlatform.obj]
 Error 1
GNUmakefile:443: recipe for target 
'/home/gary/git/edk2/Build/OvmfX64/DEBUG_GCC49/X64/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib/OUTPUT/BdsPlatform.obj'
 failed

Would you mind to check it?

Thanks,

Gary Lin

> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1417 
> 
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 
>  .../Library/PlatformBootManagerLib/MemoryTest.c|  450 +++
>  .../PlatformBootManagerLib.inf |   81 ++
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   35 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c|  170 +++
>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  673 ++
>  OvmfPkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3658 bytes
>  8 files changed, 3072 insertions(+)
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/MemoryTest.c
>  create mode 100644 
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/Strings.uni
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> new file mode 100644
> index 000..c93b900
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -0,0 +1,1417 @@
> +/** @file
> +  Platform BDS customizations.
> +
> +  Copyright (c) 2004 - 2016, 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
> +
> +  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 "BdsPlatform.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +
> +//
> +// Global data
> +//
> +
> +VOID  *mEfiDevPathNotifyReg;
> +EFI_EVENT mEfiDevPathEvent;
> +VOID  *mEmuVariableEventReg;
> +EFI_EVENT mEmuVariableEvent;
> +

Re: [edk2] [Patch 2/3] OvmfPkg: Create PlatformBootManagerLib to work with UefiBootManagerLib

[Gary Lin]

On Mon, Apr 18, 2016 at 02:43:02PM +0800, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
I found where the errors came from and below are my comments.

> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1417 
> 
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 
>  .../Library/PlatformBootManagerLib/MemoryTest.c|  450 +++
>  .../PlatformBootManagerLib.inf |   81 ++
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   35 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c|  170 +++
>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  673 ++
>  OvmfPkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3658 bytes
>  8 files changed, 3072 insertions(+)
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/MemoryTest.c
>  create mode 100644 
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/Strings.uni
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> new file mode 100644
> index 000..c93b900
> --- /dev/null
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -0,0 +1,1417 @@
> +/** @file
> +  Platform BDS customizations.
> +
> +  Copyright (c) 2004 - 2016, 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
> +
> +  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 "BdsPlatform.h"
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +
> +//
> +// Global data
> +//
> +
> +VOID  *mEfiDevPathNotifyReg;
> +EFI_EVENT mEfiDevPathEvent;
> +VOID  *mEmuVariableEventReg;
> +EFI_EVENT mEmuVariableEvent;
> +BOOLEAN   mDetectVgaOnly;
> +UINT16mHostBridgeDevId;
> +
> +//
> +// Table of host IRQs matching PCI IRQs A-D
> +// (for configuring PCI Interrupt Line register)
> +//
> +CONST UINT8 PciHostIrqs[] = {
> +  0x0a, 0x0a, 0x0b, 0x0b
> +};
> +
> +//
> +// Array Size macro
> +//
> +#define ARRAY_SIZE(array) (sizeof (array) / sizeof (array[0]))
> +
> +//
> +// Type definitions
> +//
> +
> +typedef
> +EFI_STATUS
> +(EFIAPI *PROTOCOL_INSTANCE_CALLBACK)(
> +  IN EFI_HANDLE   Handle,
> +  IN VOID *Instance,
> +  IN VOID *Context
> +  );
> +
> +/**
> +  @param[in]  Handle - Handle of PCI device instance
> +  @param[in]  PciIo - PCI IO protocol instance
> +  @param[in]  Pci - PCI Header register block
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *VISIT_PCI_INSTANCE_CALLBACK)(
> +  IN EFI_HANDLE   Handle,
> +  IN EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN PCI_TYPE00   *Pci
> +  );
> +
> +
> +//
> +// Function prototypes
> +//
> +
> +EFI_STATUS
> +VisitAllInstancesOfProtocol (
> +  IN EFI_GUID*Id,
> +  IN PROTOCOL_INSTANCE_CALLBACK  CallBackFunction,
> +  IN VOID*Context
> +  );
> +
> +EFI_STATUS
> +VisitAllPciInstancesOfProtocol (
> +  IN VISIT_PCI_INSTANCE_CALLBACK CallBackFunction
> +  );
> +
> +VOID
> +InstallDevicePathCallback (
> +  VOID
> +  );
> +
> +/**
> +  Return the index of the load option in the load option array.
> +
> +  The function consider two load options are equal when the 
> +  OptionType, Attributes, Description, FilePath and OptionalData are equal.
> +
> +  @param KeyPointer to the load option to be found.
> +  @param Array  Pointer to the array of load options to be found.
> +  @param Count  Number of entries in the Array.
> +
> +  @retval -1  Key wasn't found in the Array.
> +  @retval 0 ~ Count-1 The index of the Key in the Array.
> +**/
> +INTN
> +PlatformFindLoadOption (
> +  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Key,
> +  IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Array,
> +  IN UINTN  Count
> +  )
> +{
> +  UINTN Index;
> +
> +  for (Index = 0; Index < Count; Index++) {
> +if ((Key->OptionType == Array[Index].OptionType) &&
> +(Key->Attributes == Array[Index].Attributes) &&
> +(StrCmp (Key->Description, Array[Index].Description) == 0) &&
> +(Compar

Re: [edk2] [PATCH] ArmPlatformPkg: fixups for 64-bit mailbox pointers

[Leif Lindholm]

On Thu, Mar 24, 2016 at 11:33:22PM +0100, Ard Biesheuvel wrote:
> On 24 March 2016 at 21:30, Leo Duran  wrote:
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Leo Duran 
> > ---
> >  ArmPlatformPkg/PrePeiCore/MainMPCore.c | 10 --
> >  ArmPlatformPkg/PrePi/MainMPCore.c  | 10 --
> >  2 files changed, 16 insertions(+), 4 deletions(-)
> >
> 
> 
> ArmPlatformPkg/ArmVExpressPkg/Include/VExpressMotherBoard.h has the
> following defines
> 
> #define ARM_VE_SYS_FLAGS_SET_REG
> (ARM_VE_BOARD_PERIPH_BASE + 0x00030)
> #define ARM_VE_SYS_FLAGS_CLR_REG
> (ARM_VE_BOARD_PERIPH_BASE + 0x00034)
> #define ARM_VE_SYS_FLAGS_NV_REG
> (ARM_VE_BOARD_PERIPH_BASE + 0x00038)
> 
> and is used on Juno as well as the older 32-bit archs. I don't know if
> this is dead code now that PSCI is implemented by ARM trusted
> firmware, and so we never bring up the secondaries in UEFI. But I
> would like Leif's take on this when he gets back, since writing 64-bit
> values is clearly not correct either in this particular case.

Yeah, these remain 32-bit also on Juno.

Moreover, only user I see of this module in public code is FVP
(OpenPlatformPkg), which should probably be investigated.

Leo: do you actually need PrePeiCoreMPCore in the current version of
your platform code?

/
Leif

> > diff --git a/ArmPlatformPkg/PrePeiCore/MainMPCore.c 
> > b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> > index fa544c7..8309f62 100644
> > --- a/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> > +++ b/ArmPlatformPkg/PrePeiCore/MainMPCore.c
> > @@ -80,13 +80,19 @@ SecondaryMain (
> >ASSERT (Index != ArmCoreCount);
> >
> >// Clear Secondary cores MailBox
> > -  MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, 
> > ArmCoreInfoTable[Index].MailboxClearValue);
> > +  if (sizeof(UINTN) == sizeof(UINT64))
> > +MmioWrite64 (ArmCoreInfoTable[Index].MailboxClearAddress, 
> > ArmCoreInfoTable[Index].MailboxClearValue);
> > +  else
> > +MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, 
> > ArmCoreInfoTable[Index].MailboxClearValue);
> >
> >do {
> >  ArmCallWFI ();
> >
> >  // Read the Mailbox
> > -SecondaryEntryAddr = MmioRead32 
> > (ArmCoreInfoTable[Index].MailboxGetAddress);
> > +if (sizeof(UINTN) == sizeof(UINT64))
> > +  SecondaryEntryAddr = MmioRead64 
> > (ArmCoreInfoTable[Index].MailboxGetAddress);
> > +else
> > +  SecondaryEntryAddr = MmioRead32 
> > (ArmCoreInfoTable[Index].MailboxGetAddress);
> >
> >  // Acknowledge the interrupt and send End of Interrupt signal.
> >  AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet32 
> > (PcdGicInterruptInterfaceBase), &InterruptId);
> > diff --git a/ArmPlatformPkg/PrePi/MainMPCore.c 
> > b/ArmPlatformPkg/PrePi/MainMPCore.c
> > index 603f4bb..d7e2352 100644
> > --- a/ArmPlatformPkg/PrePi/MainMPCore.c
> > +++ b/ArmPlatformPkg/PrePi/MainMPCore.c
> > @@ -79,13 +79,19 @@ SecondaryMain (
> >ASSERT (Index != ArmCoreCount);
> >
> >// Clear Secondary cores MailBox
> > -  MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, 
> > ArmCoreInfoTable[Index].MailboxClearValue);
> > +  if (sizeof(UINTN) == sizeof(UINT64))
> > +MmioWrite64 (ArmCoreInfoTable[Index].MailboxClearAddress, 
> > ArmCoreInfoTable[Index].MailboxClearValue);
> > +  else
> > +MmioWrite32 (ArmCoreInfoTable[Index].MailboxClearAddress, 
> > ArmCoreInfoTable[Index].MailboxClearValue);
> >
> >do {
> >  ArmCallWFI ();
> >
> >  // Read the Mailbox
> > -SecondaryEntryAddr = MmioRead32 
> > (ArmCoreInfoTable[Index].MailboxGetAddress);
> > +if (sizeof(UINTN) == sizeof(UINT64))
> > +  SecondaryEntryAddr = MmioRead64 
> > (ArmCoreInfoTable[Index].MailboxGetAddress);
> > +else
> > +  SecondaryEntryAddr = MmioRead32 
> > (ArmCoreInfoTable[Index].MailboxGetAddress);
> >
> >  // Acknowledge the interrupt and send End of Interrupt signal.
> >  AcknowledgeInterrupt = ArmGicAcknowledgeInterrupt (PcdGet32 
> > (PcdGicInterruptInterfaceBase), &InterruptId);
> > --
> > 1.9.1
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPlatformPkg: fixups for 64-bit mailbox pointers

[Ard Biesheuvel]

On 18 April 2016 at 12:39, Leif Lindholm  wrote:
> On Thu, Mar 24, 2016 at 11:33:22PM +0100, Ard Biesheuvel wrote:
>> On 24 March 2016 at 21:30, Leo Duran  wrote:
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Leo Duran 
>> > ---
>> >  ArmPlatformPkg/PrePeiCore/MainMPCore.c | 10 --
>> >  ArmPlatformPkg/PrePi/MainMPCore.c  | 10 --
>> >  2 files changed, 16 insertions(+), 4 deletions(-)
>> >
>>
>>
>> ArmPlatformPkg/ArmVExpressPkg/Include/VExpressMotherBoard.h has the
>> following defines
>>
>> #define ARM_VE_SYS_FLAGS_SET_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00030)
>> #define ARM_VE_SYS_FLAGS_CLR_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00034)
>> #define ARM_VE_SYS_FLAGS_NV_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00038)
>>
>> and is used on Juno as well as the older 32-bit archs. I don't know if
>> this is dead code now that PSCI is implemented by ARM trusted
>> firmware, and so we never bring up the secondaries in UEFI. But I
>> would like Leif's take on this when he gets back, since writing 64-bit
>> values is clearly not correct either in this particular case.
>
> Yeah, these remain 32-bit also on Juno.
>
> Moreover, only user I see of this module in public code is FVP
> (OpenPlatformPkg), which should probably be investigated.
>
> Leo: do you actually need PrePeiCoreMPCore in the current version of
> your platform code?
>

I suppose the question here is whether all cores enter UEFI or only
the boot CPU. If the EL3 firmware is doing PSCI, then only the boot
core enters UEFI, and this code could be dropped or simplified. Note
that there are some checks in the code still that prevent you from
running the unicore version on a SMP system, so that should still be
addressed afair
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPlatformPkg: fixups for 64-bit mailbox pointers

[Bhupesh Sharma]

We also should consider ARM SOCs which have Internal ROM code running even 
before the ATF starts running in EL3. Such a internal ROM code might be 
configured to make sure that only the primary core runs the ATF and UEFI code 
and the secondaries enter the ATF code first time only when Linux does a 
secondary CPU_ON call.

Regards,
Bhupesh


From: edk2-devel  on behalf of Ard Biesheuvel 

Sent: Monday, April 18, 2016 4:12:15 PM
To: Leif Lindholm
Cc: edk2-devel@lists.01.org; Leo Duran
Subject: Re: [edk2] [PATCH] ArmPlatformPkg: fixups for 64-bit mailbox   pointers

On 18 April 2016 at 12:39, Leif Lindholm  wrote:
> On Thu, Mar 24, 2016 at 11:33:22PM +0100, Ard Biesheuvel wrote:
>> On 24 March 2016 at 21:30, Leo Duran  wrote:
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Leo Duran 
>> > ---
>> >  ArmPlatformPkg/PrePeiCore/MainMPCore.c | 10 --
>> >  ArmPlatformPkg/PrePi/MainMPCore.c  | 10 --
>> >  2 files changed, 16 insertions(+), 4 deletions(-)
>> >
>>
>>
>> ArmPlatformPkg/ArmVExpressPkg/Include/VExpressMotherBoard.h has the
>> following defines
>>
>> #define ARM_VE_SYS_FLAGS_SET_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00030)
>> #define ARM_VE_SYS_FLAGS_CLR_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00034)
>> #define ARM_VE_SYS_FLAGS_NV_REG
>> (ARM_VE_BOARD_PERIPH_BASE + 0x00038)
>>
>> and is used on Juno as well as the older 32-bit archs. I don't know if
>> this is dead code now that PSCI is implemented by ARM trusted
>> firmware, and so we never bring up the secondaries in UEFI. But I
>> would like Leif's take on this when he gets back, since writing 64-bit
>> values is clearly not correct either in this particular case.
>
> Yeah, these remain 32-bit also on Juno.
>
> Moreover, only user I see of this module in public code is FVP
> (OpenPlatformPkg), which should probably be investigated.
>
> Leo: do you actually need PrePeiCoreMPCore in the current version of
> your platform code?
>

I suppose the question here is whether all cores enter UEFI or only
the boot CPU. If the EL3 firmware is doing PSCI, then only the boot
core enters UEFI, and this code could be dropped or simplified. Note
that there are some checks in the code still that prevent you from
running the unicore version on a SMP system, so that should still be
addressed afair
___
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] ArmPlatformPkg: fixups for 64-bit mailbox pointers

[Leif Lindholm]

On Mon, Apr 18, 2016 at 10:58:26AM +, Bhupesh Sharma wrote:
> We also should consider ARM SOCs which have Internal ROM code
> running even before the ATF starts running in EL3. Such a internal
> ROM code might be configured to make sure that only the primary core
> runs the ATF and UEFI code and the secondaries enter the ATF code
> first time only when Linux does a secondary CPU_ON call.

Sure.

But this code lives in ArmPlatformPkg, making it (theoretically) only
relevant to ARM ltd. platforms. I know this line has historically been
pretty blurred, but really, any code from here being used in other
platforms should be looking to migrate to a different package.

Regards,

Leif

> 
> Regards,
> Bhupesh
> 
> 
> From: edk2-devel  on behalf of Ard 
> Biesheuvel 
> Sent: Monday, April 18, 2016 4:12:15 PM
> To: Leif Lindholm
> Cc: edk2-devel@lists.01.org; Leo Duran
> Subject: Re: [edk2] [PATCH] ArmPlatformPkg: fixups for 64-bit mailbox   
> pointers
> 
> On 18 April 2016 at 12:39, Leif Lindholm  wrote:
> > On Thu, Mar 24, 2016 at 11:33:22PM +0100, Ard Biesheuvel wrote:
> >> On 24 March 2016 at 21:30, Leo Duran  wrote:
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Leo Duran 
> >> > ---
> >> >  ArmPlatformPkg/PrePeiCore/MainMPCore.c | 10 --
> >> >  ArmPlatformPkg/PrePi/MainMPCore.c  | 10 --
> >> >  2 files changed, 16 insertions(+), 4 deletions(-)
> >> >
> >>
> >>
> >> ArmPlatformPkg/ArmVExpressPkg/Include/VExpressMotherBoard.h has the
> >> following defines
> >>
> >> #define ARM_VE_SYS_FLAGS_SET_REG
> >> (ARM_VE_BOARD_PERIPH_BASE + 0x00030)
> >> #define ARM_VE_SYS_FLAGS_CLR_REG
> >> (ARM_VE_BOARD_PERIPH_BASE + 0x00034)
> >> #define ARM_VE_SYS_FLAGS_NV_REG
> >> (ARM_VE_BOARD_PERIPH_BASE + 0x00038)
> >>
> >> and is used on Juno as well as the older 32-bit archs. I don't know if
> >> this is dead code now that PSCI is implemented by ARM trusted
> >> firmware, and so we never bring up the secondaries in UEFI. But I
> >> would like Leif's take on this when he gets back, since writing 64-bit
> >> values is clearly not correct either in this particular case.
> >
> > Yeah, these remain 32-bit also on Juno.
> >
> > Moreover, only user I see of this module in public code is FVP
> > (OpenPlatformPkg), which should probably be investigated.
> >
> > Leo: do you actually need PrePeiCoreMPCore in the current version of
> > your platform code?
> >
> 
> I suppose the question here is whether all cores enter UEFI or only
> the boot CPU. If the EL3 firmware is doing PSCI, then only the boot
> core enters UEFI, and this code could be dropped or simplified. Note
> that there are some checks in the code still that prevent you from
> running the unicore version on a SMP system, so that should still be
> addressed afair
> ___
> 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] OVMF: hang when booting Linux via GRUB after driver allocates 2GB above 4GB

[Laszlo Ersek]

On 04/18/16 16:25, Laszlo Ersek wrote:

> I think at this point I'll copy Matt :) , and ask you to reproduce
> the issue with a fresh upstream kernel (most recent Linux release, or
> even fresh git). If it reproduces, then it's an upstream kernel bug I
> think; if it doesn't reproduce, then please report an RHBZ about it.

BTW, what happens if you boot UEFI Windows (i.e., 8+) in your VM? Does
it boot?

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

Re: [edk2] OVMF: hang when booting Linux via GRUB after driver allocates 2GB above 4GB

[Laszlo Ersek]

On 04/17/16 03:08, Bruce Cran wrote:
> On 4/16/16 1:28 PM, Bruce Cran wrote:
>> On 4/16/16 12:28 PM, Laszlo Ersek wrote:
>>
>>> Then boot the guest again, as you would normally do (including your
>>> driver in OVMF), just pass the above triplet with the -kernel / -initrd
>>> / -append options.
> 
> With a debug OVMF build, I get some possibly useful information on the
> debug console. At one point, the following gets printed:
> 
> ConvertPages: failed to find range 18000 - 1F000

I enabled DEBUG_GCD (0x0010) in PcdDebugPrintErrorLevel. I also modified 
one of the DXE drivers in OvmfPkg (PlatformDxe to be exact, but it's 
irrelevant) to allocate 2GB memory at 6GB, in its entry point function:

+  {
+EFI_PHYSICAL_ADDRESS Address;
+EFI_STATUS   Status2;
+
+Address = 0x18000;
+Status2 = gBS->AllocatePages(AllocateAddress, EfiBootServicesData, 0x8,
+ &Address);
+ASSERT_EFI_ERROR (Status2);
+  }
+

Finally I booted this in a 16GB virtual machine. Here's what I see:

(1) Since "OvmfPkg/PlatformPei/MemDetect.c" adds the memory above 4GB as 
untested memory (*), with

if (UpperMemorySize != 0) {
  AddUntestedMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
}

the DXE core initializes this range in the GCD memory space map as Reserved. 
(See the CoreInitializeGcdServices() function -- search it for the comment 
"Walk the HOB list and add all resource descriptors to the GCD" and the macro 
TESTED_MEMORY_ATTRIBUTES.)

Because this range is Reserved when the DXE driver in question tries to 
allocate 2GB at 6GB, the AllocatePages() boot service fails. That's when you 
see an error message like

  ConvertPages: failed to find range ...

In other words, if the AllocatePages() call in your driver causes this message 
to appear, then the allocation fails, and the driver should not try to use the 
address. Are you checking the return value from AllocatePages()? For me, it is 
EFI_NOT_FOUND.

(*) This is a side point. I guess you might want to know why PlatformPei adds 
the RAM above 4GB as untested. It is because we want the DXE IPL to load the 
DXE Core into the permanent PEI RAM that our PEI phase installs. There was a 
thread in the past where we discussed this at lenght; I've forgotten most of 
the details by now, but the point is, such allocations won't succeed in the 
entry points of DXE_DRIVER modules.

However, this per se should never cause the problem you are seeing (as long as 
you are obeying the return status of AllocatePages(), of course).

(2) Now, memory testing in BDS promotes this memory range to usable system 
memory. Then it becomes available to UEFI_DRIVER modules, and code in 
DXE_DRIVER modules that allocates memory this late (for example, protocol 
installation callbacks). See PlatformBdsDiagnostics() in 
"OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c".

Moving the AllocatePages() hunk above to just after PlatformBdsDiagnostics(), 
the call succeeds. The kernel is booted (I tested with grub, using a RHEL-7.2 
guest), but it does fall into an infinite loop, exactly where you described. 
IOW, I can reproduce the issue.

(3) At this point I paused the guest, and dumped its memory contents to disk, 
with the following command:

# virsh dump ovmf.rhel7 ovmf.rhel7.dump --memory-only --format kdump-lzo

(Note that you can do the same using just QEMU: see the "dump-guest-memory" 
monitor command. You will want to *disable* paging, and pick either the 
kdump-lzo or kdump-snappy formats.)

After this, the guest can be forced off, we'll work with the memory dump only.

(4) I installed the debug symbols for the guest kernel onto the host 
(kernel-debuginfo + kernel-debuginfo-common packages), and also the "crash" 
utility. (The same should be doable on CentOS 7 too.) The "crash" utility had 
been extended to handle dumps of guest kernels running on top of OVMF (see 
.)

(5) The dump can be opened like this:

  crash \
/usr/lib/debug/usr/lib/modules/3.10.0-327.el7.x86_64/vmlinux \
ovmf.rhel7.dump

The stack dump I get with "bt -l" is:

PID: 0  TASK: 81951440  CPU: 0   COMMAND: "swapper/0"
[exception RIP: native_set_pmd+1]
RIP: 810592b1  RSP: 8193fc68  RFLAGS: 0282
RAX: 007f05b2156000e3  RBX: 007f05af10e0  RCX: 8800
RDX: 88042f065438  RSI: 007f05b2156000e3  RDI: 88042f065438
RBP: 8193fcb8   R8: 0063   R9: 0063
R10: 88043ffc7000  R11: 0001  R12: ff80fa4eeaa0
R13: 8193fe28  R14: 0063  R15: 88000294dfc8
CS: 0010  SS: 
 #0 [8193fc70] populate_pmd at 81060f16

/usr/src/debug/kernel-3.10.0-327.el7/linux-3.10.0-327.el7.x86_64/arch/x86/include/asm/paravirt.h:
 545
 #1 [8193fcc0] __cpa_process_fault at 810613ab

/usr/src/debug/kernel-3.10.0-327.el7/linux-3.10.0-327.el7.x86_64/arch/x86

Re: [edk2] OVMF: hang when booting Linux via GRUB after driver allocates 2GB above 4GB

[Laszlo Ersek]

huh, Matt's email @Intel stopped working. I'm resending the message to his 
email seen in the kernel's MAINTAINERS file.

Bruce, if you follow up, please do so in this sub-thread.

Thanks
Laszlo

On 04/17/16 03:08, Bruce Cran wrote:
> On 4/16/16 1:28 PM, Bruce Cran wrote:
>> On 4/16/16 12:28 PM, Laszlo Ersek wrote:
>>
>>> Then boot the guest again, as you would normally do (including your
>>> driver in OVMF), just pass the above triplet with the -kernel / -initrd
>>> / -append options.
> 
> With a debug OVMF build, I get some possibly useful information on the
> debug console. At one point, the following gets printed:
> 
> ConvertPages: failed to find range 18000 - 1F000

I enabled DEBUG_GCD (0x0010) in PcdDebugPrintErrorLevel. I also modified 
one of the DXE drivers in OvmfPkg (PlatformDxe to be exact, but it's 
irrelevant) to allocate 2GB memory at 6GB, in its entry point function:

+  {
+EFI_PHYSICAL_ADDRESS Address;
+EFI_STATUS   Status2;
+
+Address = 0x18000;
+Status2 = gBS->AllocatePages(AllocateAddress, EfiBootServicesData, 0x8,
+ &Address);
+ASSERT_EFI_ERROR (Status2);
+  }
+

Finally I booted this in a 16GB virtual machine. Here's what I see:

(1) Since "OvmfPkg/PlatformPei/MemDetect.c" adds the memory above 4GB as 
untested memory (*), with

if (UpperMemorySize != 0) {
  AddUntestedMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
}

the DXE core initializes this range in the GCD memory space map as Reserved. 
(See the CoreInitializeGcdServices() function -- search it for the comment 
"Walk the HOB list and add all resource descriptors to the GCD" and the macro 
TESTED_MEMORY_ATTRIBUTES.)

Because this range is Reserved when the DXE driver in question tries to 
allocate 2GB at 6GB, the AllocatePages() boot service fails. That's when you 
see an error message like

  ConvertPages: failed to find range ...

In other words, if the AllocatePages() call in your driver causes this message 
to appear, then the allocation fails, and the driver should not try to use the 
address. Are you checking the return value from AllocatePages()? For me, it is 
EFI_NOT_FOUND.

(*) This is a side point. I guess you might want to know why PlatformPei adds 
the RAM above 4GB as untested. It is because we want the DXE IPL to load the 
DXE Core into the permanent PEI RAM that our PEI phase installs. There was a 
thread in the past where we discussed this at lenght; I've forgotten most of 
the details by now, but the point is, such allocations won't succeed in the 
entry points of DXE_DRIVER modules.

However, this per se should never cause the problem you are seeing (as long as 
you are obeying the return status of AllocatePages(), of course).

(2) Now, memory testing in BDS promotes this memory range to usable system 
memory. Then it becomes available to UEFI_DRIVER modules, and code in 
DXE_DRIVER modules that allocates memory this late (for example, protocol 
installation callbacks). See PlatformBdsDiagnostics() in 
"OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c".

Moving the AllocatePages() hunk above to just after PlatformBdsDiagnostics(), 
the call succeeds. The kernel is booted (I tested with grub, using a RHEL-7.2 
guest), but it does fall into an infinite loop, exactly where you described. 
IOW, I can reproduce the issue.

(3) At this point I paused the guest, and dumped its memory contents to disk, 
with the following command:

# virsh dump ovmf.rhel7 ovmf.rhel7.dump --memory-only --format kdump-lzo

(Note that you can do the same using just QEMU: see the "dump-guest-memory" 
monitor command. You will want to *disable* paging, and pick either the 
kdump-lzo or kdump-snappy formats.)

After this, the guest can be forced off, we'll work with the memory dump only.

(4) I installed the debug symbols for the guest kernel onto the host 
(kernel-debuginfo + kernel-debuginfo-common packages), and also the "crash" 
utility. (The same should be doable on CentOS 7 too.) The "crash" utility had 
been extended to handle dumps of guest kernels running on top of OVMF (see 
.)

(5) The dump can be opened like this:

  crash \
/usr/lib/debug/usr/lib/modules/3.10.0-327.el7.x86_64/vmlinux \
ovmf.rhel7.dump

The stack dump I get with "bt -l" is:

PID: 0  TASK: 81951440  CPU: 0   COMMAND: "swapper/0"
[exception RIP: native_set_pmd+1]
RIP: 810592b1  RSP: 8193fc68  RFLAGS: 0282
RAX: 007f05b2156000e3  RBX: 007f05af10e0  RCX: 8800
RDX: 88042f065438  RSI: 007f05b2156000e3  RDI: 88042f065438
RBP: 8193fcb8   R8: 0063   R9: 0063
R10: 88043ffc7000  R11: 0001  R12: ff80fa4eeaa0
R13: 8193fe28  R14: 0063  R15: 88000294dfc8
CS: 0010  SS: 
 #0 [8193fc70] populate_pmd at 81060f16

/usr/src/debug/kernel-3.10.0-327.el7/linux-3.10.

Re: [edk2] OVMF: hang when booting Linux via GRUB after driver allocates 2GB above 4GB

[Laszlo Ersek]

resending this one too

On 04/18/16 16:25, Laszlo Ersek wrote:

> I think at this point I'll copy Matt :) , and ask you to reproduce
> the issue with a fresh upstream kernel (most recent Linux release, or
> even fresh git). If it reproduces, then it's an upstream kernel bug I
> think; if it doesn't reproduce, then please report an RHBZ about it.

BTW, what happens if you boot UEFI Windows (i.e., 8+) in your VM? Does
it boot?

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

Re: [edk2] OVMF: hang when booting Linux via GRUB after driver allocates 2GB above 4GB

[Laszlo Ersek]

I love Thunderbird, except when I hate it. Let me try to use the correct
address for Matt again...

On 04/18/16 16:25, Laszlo Ersek wrote:

> I think at this point I'll copy Matt :) , and ask you to reproduce
> the issue with a fresh upstream kernel (most recent Linux release, or
> even fresh git). If it reproduces, then it's an upstream kernel bug I
> think; if it doesn't reproduce, then please report an RHBZ about it.

BTW, what happens if you boot UEFI Windows (i.e., 8+) in your VM? Does
it boot?

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

Re: [edk2] [PATCH] OvmfPkg: Don't enable unsupported attributes

[Laszlo Ersek]

Nice catch!

Please send a v2 with the following cosmetic changes:

(1) The subject should be:

  OvmfPkg: AcpiPlatformDxe: Don't enable unsupported PCI attributes

On 04/17/16 13:47, Volker Rümelin wrote:
> Current code in PciEnableDecoding tries to unconditionally enable
> EFI_PCI_IO_ATTRIBUTE_IO and EFI_PCI_IO_ATTRIBUTE_MEMORY even if they
> are unsupported attributes. This fails on devices which don't
> support both attributes.
> 
> This patch masks out unsupported attributes.
> 
> Information to reproduce the bug.
> 
> Host lspci -s :04:00.0 -vnn:
> 04:00.0 USB controller [0c03]: Renesas Technology Corp. uPD720201 USB 3.0 
> Host Controller [1912:0014] (rev 03) (prog-if 30 [XHCI])

(2) Please wrap this line to 76 characters (I do realize this is lspci
output). ... Actually, not just this line, but the complete of the
commit message, if possible. (You can also wrap XML tags, at whitespace
between attributes.)

>   Flags: fast devsel, IRQ 19
>   Memory at ef90 (64-bit, non-prefetchable) [size=8K]
>   Capabilities: [50] Power Management version 3
>   Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+
>   Capabilities: [90] MSI-X: Enable- Count=8 Masked-
>   Capabilities: [a0] Express Endpoint, MSI 00
>   Capabilities: [100] Advanced Error Reporting
>   Capabilities: [150] Latency Tolerance Reporting
>   Kernel driver in use: pci-stub
>   Kernel modules: xhci_pci
> 
> libvirt xml:
> 
>   
>   
> 
>   
>function='0'/>
> 
> 
> OVMF debug log with additional DEBUG statement:
> OnRootBridgesConnected: root bridges have been connected, installing ACPI 
> tables
> Select Item: 0x19
> EnablePciDecoding: GetLocation: D=:00:00.0
> OrigAttr=4000 SuppAttr=E700
> EnablePciDecoding: GetLocation: D=:00:10.0
> OrigAttr=4000 SuppAttr=E700
> EnablePciDecoding: GetLocation: D=:00:11.0
> OrigAttr=4000 SuppAttr=E600
> EnablePciDecoding: EfiPciIoAttributeOperationEnable: Unsupported
> Select Item: 0x28
> Select Item: 0x19
> Select Item: 0x2A
> Select Item: 0x19
> Select Item: 0x27
> InstallQemuFwCfgTables: installed 6 tables
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Volker Rümelin 
> ---
>  OvmfPkg/AcpiPlatformDxe/PciDecoding.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/PciDecoding.c 
> b/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
> index 3b9b12c..0d9b7ad 100644
> --- a/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
> +++ b/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
> @@ -53,6 +53,7 @@ EnablePciDecoding (
>EFI_HANDLE  *Handles;
>ORIGINAL_ATTRIBUTES *OrigAttrs;
>UINTN   Idx;
> +  UINT64  Attributes;

(3) Please move this local variable into the tighter block scope (the
loop) where the "PciIo" variable is also declared.

>  
>*OriginalAttributes = NULL;
>*Count  = 0;
> @@ -112,9 +113,16 @@ EnablePciDecoding (
>  //
>  // Enable IO and MMIO decoding
>  //

(4) Please add a comment saying "Retrieve supported attributes".

> +Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationSupported, 
> 0,
> +  &Attributes);
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((EFI_D_WARN, "%a: EfiPciIoAttributeOperationSupported: %r\n",
> +__FUNCTION__, Status));
> +  goto RestoreAttributes;
> +}

(5) Please insert an empty line here, then move the "Enable IO and MMIO
decoding" comment below it, from above.

> +Attributes &= EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY;
>  Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationEnable,
> -  EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY,
> -  NULL);
> +  Attributes, NULL);
>  if (EFI_ERROR (Status)) {
>DEBUG ((EFI_D_WARN, "%a: EfiPciIoAttributeOperationEnable: %r\n",
>  __FUNCTION__, Status));
> 

If you prefer, I can do these changes for you too -- I would keep your
authorship and S-o-b in the first position. I'd mark my changes below
your S-o-b.

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

Re: [edk2] [Patch 0/3] Use MdeModulePkg/BDS in OVMF platform

[Laszlo Ersek]

Ray,

On 04/18/16 08:43, Ruiyu Ni wrote:
> The patch serials creates a flag USE_OLD_BDS and by default the value
> of the flag is FALSE so that the new MdeModulePkg/BDS is used.
> User can define USE_OLD_BDS as TRUE to force to use IntelFrameworkModulePkg
> /BDS.

Thanks for looking into this again.

How much time do you have for working on this?

Namely, the series contains barely any documentation, while patch #1
adds 2410 lines, and patch #2 adds 3072 lines. That this is practically
unreviewable.

It is also not explained how this version differs from v1
, and/or how my
v1 comments have been addressed.

For example, while reviewing that version, I asked (as point (10)) for
the BmGetLoadOptionBuffer() function to be made available to platform
BDS code. I don't see it happening here.

On one hand, we (Jordan and I) shouldn't have to reconstruct your
thinking from the code in these huge patches; your thinking and work
process should be laid out in small patches and detailed commit messages.

On the other hand, you are doing us a favor by writing the conversion
for us.

So, I'm not sure what to say. I don't want to review this series as-is
(let alone accept it unreviewed). However, I don't know how much time
you have to iterate on it. (I expect quite a few iterations, for a
series this size.)

Perhaps, we could do the following: I could start reading through your
work slowly, and factor out as many small patches as possible. Then we'd
end up with a series where you and I share authorship on a bunch of
patches, and Jordan could review it.

What do you think?

Thanks
Laszlo

> 
> Ruiyu Ni (3):
>   OvmfPkg: Create QemuNewBootOrderLib to work with UefiBootManagerLib
>   OvmfPkg: Create PlatformBootManagerLib to work with UefiBootManagerLib
>   OvmfPkg: Use MdeModulePkg/BDS
> 
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1417 ++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 +++
>  .../Library/PlatformBootManagerLib/MemoryTest.c|  450 +
>  .../PlatformBootManagerLib.inf |   81 +
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   35 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c|  170 ++
>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  673 +++
>  OvmfPkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3658 bytes
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.c  |  313 +++
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.h  |   40 +
>  .../Library/QemuNewBootOrderLib/QemuBootOrderLib.c | 1989 
> 
>  .../QemuNewBootOrderLib/QemuBootOrderLib.inf   |   68 +
>  OvmfPkg/OvmfPkgIa32.dsc|   40 +-
>  OvmfPkg/OvmfPkgIa32.fdf|5 +
>  OvmfPkg/OvmfPkgIa32X64.dsc |   42 +-
>  OvmfPkg/OvmfPkgIa32X64.fdf |5 +
>  OvmfPkg/OvmfPkgX64.dsc |   40 +-
>  OvmfPkg/OvmfPkgX64.fdf |5 +
>  18 files changed, 5603 insertions(+), 16 deletions(-)
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/MemoryTest.c
>  create mode 100644 
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/Strings.uni
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.h
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>  create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.inf



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

Re: [edk2] [Patch 0/3] Use MdeModulePkg/BDS in OVMF platform

[Ni, Ruiyu]

No, please don't waste your time to read the patch. I will send new version. 
The iterations of the change actually are simple.

Thanks,
Ray

> 在 2016年4月19日，上午12:05，Laszlo Ersek  写道：
> 
> Ray,
> 
>> On 04/18/16 08:43, Ruiyu Ni wrote:
>> The patch serials creates a flag USE_OLD_BDS and by default the value
>> of the flag is FALSE so that the new MdeModulePkg/BDS is used.
>> User can define USE_OLD_BDS as TRUE to force to use IntelFrameworkModulePkg
>> /BDS.
> 
> Thanks for looking into this again.
> 
> How much time do you have for working on this?
> 
> Namely, the series contains barely any documentation, while patch #1
> adds 2410 lines, and patch #2 adds 3072 lines. That this is practically
> unreviewable.
> 
> It is also not explained how this version differs from v1
> , and/or how my
> v1 comments have been addressed.
> 
> For example, while reviewing that version, I asked (as point (10)) for
> the BmGetLoadOptionBuffer() function to be made available to platform
> BDS code. I don't see it happening here.
> 
> On one hand, we (Jordan and I) shouldn't have to reconstruct your
> thinking from the code in these huge patches; your thinking and work
> process should be laid out in small patches and detailed commit messages.
> 
> On the other hand, you are doing us a favor by writing the conversion
> for us.
> 
> So, I'm not sure what to say. I don't want to review this series as-is
> (let alone accept it unreviewed). However, I don't know how much time
> you have to iterate on it. (I expect quite a few iterations, for a
> series this size.)
> 
> Perhaps, we could do the following: I could start reading through your
> work slowly, and factor out as many small patches as possible. Then we'd
> end up with a series where you and I share authorship on a bunch of
> patches, and Jordan could review it.
> 
> What do you think?
> 
> Thanks
> Laszlo
> 
>> 
>> Ruiyu Ni (3):
>>  OvmfPkg: Create QemuNewBootOrderLib to work with UefiBootManagerLib
>>  OvmfPkg: Create PlatformBootManagerLib to work with UefiBootManagerLib
>>  OvmfPkg: Use MdeModulePkg/BDS
>> 
>> .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1417 ++
>> .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 +++
>> .../Library/PlatformBootManagerLib/MemoryTest.c|  450 +
>> .../PlatformBootManagerLib.inf |   81 +
>> .../Library/PlatformBootManagerLib/PlatformData.c  |   35 +
>> .../Library/PlatformBootManagerLib/QemuKernel.c|  170 ++
>> OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  673 +++
>> OvmfPkg/Library/PlatformBootManagerLib/Strings.uni |  Bin 0 -> 3658 bytes
>> .../Library/QemuNewBootOrderLib/ExtraRootBusMap.c  |  313 +++
>> .../Library/QemuNewBootOrderLib/ExtraRootBusMap.h  |   40 +
>> .../Library/QemuNewBootOrderLib/QemuBootOrderLib.c | 1989 
>> 
>> .../QemuNewBootOrderLib/QemuBootOrderLib.inf   |   68 +
>> OvmfPkg/OvmfPkgIa32.dsc|   40 +-
>> OvmfPkg/OvmfPkgIa32.fdf|5 +
>> OvmfPkg/OvmfPkgIa32X64.dsc |   42 +-
>> OvmfPkg/OvmfPkgIa32X64.fdf |5 +
>> OvmfPkg/OvmfPkgX64.dsc |   40 +-
>> OvmfPkg/OvmfPkgX64.fdf |5 +
>> 18 files changed, 5603 insertions(+), 16 deletions(-)
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/MemoryTest.c
>> create mode 100644 
>> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QemuKernel.c
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
>> create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/Strings.uni
>> create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.c
>> create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/ExtraRootBusMap.h
>> create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>> create mode 100644 OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.inf
> 
> 
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] ShellPkg: Update ping command options to sync with Spec

[Carsey, Jaben]

That sounds like a good enhancement to me.

-Jaben


> -Original Message-
> From: Wu, Jiaxin
> Sent: Thursday, April 14, 2016 8:52 PM
> To: Carsey, Jaben ; David Van Arnem
> ; Bhupesh Sharma ;
> Laszlo Ersek ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan 
> Subject: RE: [edk2] [Patch] ShellPkg: Update ping command options to sync
> with Spec
> Importance: High
> 
> Jaben ,
> 
> Do you agree to support no source IP specified case? After update, Ping will
> select the first both connected and configured interface automatically instead
> of always the first NIC.
> If no objection, patch will be sent out for review.
> 
> Thanks.
> Jiaxin
> 
> 
> > -Original Message-
> > From: David Van Arnem [mailto:dvanar...@cmlab.biz]
> > Sent: Friday, April 15, 2016 1:32 AM
> > To: Wu, Jiaxin ; Bhupesh Sharma
> > ; Laszlo Ersek ; edk2-
> > de...@lists.01.org
> > Cc: Ye, Ting ; Carsey, Jaben ;
> > Fu, Siyuan 
> > Subject: Re: [edk2] [Patch] ShellPkg: Update ping command options to sync
> > with Spec
> >
> > On 04/13/2016 09:13 PM, Wu, Jiaxin wrote:
> > > Hi David and Laszlo,
> > >
> > > This patch is not focused on the discussion about the multiple NICs 
> > > existed
> > case, just use to update the ping command option. So, please don't
> > misunderstand.
> > >
> >
> > Ah, ok.  Sorry to clutter up this patch discussion then, I wanted to make 
> > sure
> > the multiple NIC case wasn't lost in the discussion about "-s"/"-_s".  
> > Thanks!
> >
> > > For the discussion in
> > , If there
> > are multiple NICs existed in the platform and no source IP is specified (no 
> > '-s'
> > option), the first NIC will be selected as default interface to ping the 
> > target. '-
> > s SourceIp' can be used to specify any interface you want. That's the 
> > current
> > implementation choice.
> > >
> > > I agree your and Bhupesh 's option to support no source IP specified case,
> > that's meaningful to improve the command's friendliness(Windows and
> Linus
> > do this resolution). How about selecting the first both connected and
> > configured interface automatically?   If so, I can create another patch to
> > support that solution.
> > >
> >
> > That sounds great to me, as long as it's not too much trouble :-) 
> > Alternately,
> a
> > more useful error message in that case might suffice (e.g. "Multiple
> > configured interfaces found, please specify one with '-s'").  But, since
> > Windows and Linux both do the automatic resolution, I think consistency
> > would be beneficial.
> >
> > Thanks again!
> >
> > David
> >
> > > Thanks.
> > > Jiaxin
> > >
> > >> -Original Message-
> > >> From: David Van Arnem [mailto:dvanar...@cmlab.biz]
> > >> Sent: Thursday, April 14, 2016 4:02 AM
> > >> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> > >> Cc: Ye, Ting ; Carsey, Jaben
> > >> ; Fu, Siyuan 
> > >> Subject: Re: [edk2] [Patch] ShellPkg: Update ping command options to
> > >> sync with Spec
> > >>
> > >> On 04/12/2016 09:16 PM, Jiaxin Wu wrote:
> > >>> This patch is used to update ping command options to sync with
> > >>> shell2.2 Spec.
> > >>> Considering the backward compatible issue, the patch keeps ‘-_s’
> > >>> command option unchanged, only add the new option '-s'
> > >>> and make the old option '-_s' function same as new one.
> > >>>
> > >>> Cc: Ye Ting 
> > >>> Cc: Fu Siyuan 
> > >>> Cc: Jaben Carsey 
> > >>> Contributed-under: TianoCore Contribution Agreement 1.0
> > >>> Signed-off-by: Jiaxin Wu 
> > >>> ---
> > >>>.../Library/UefiShellNetwork1CommandsLib/Ping.c| 12
> ++-
> > -
> > >>>.../UefiShellNetwork1CommandsLib.uni   | 22 
> > >>> +
> -
> > 
> > >>>2 files changed, 19 insertions(+), 15 deletions(-)
> > >>>
> > >>> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > >>> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > >>> index dbee764..13bcdde 100644
> > >>> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > >>> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > >>> @@ -1,10 +1,10 @@
> > >>>/** @file
> > >>>  The implementation for Ping shell command.
> > >>>
> > >>>  (C) Copyright 2015 Hewlett-Packard Development Company,
> > >>> L.P.
> > >>> -  Copyright (c) 2009 - 2015, Intel Corporation. All rights
> > >>> reserved.
> > >>> +  Copyright (c) 2009 - 2016, 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.
> > >>> @@ -196,10 +196,14 @@ STATIC CONST SHELL_PARAM_ITEM
> > >> PingParamList[] = {
> > >>>  {
> > >>>L"-n",
> > >>>TypeValue
> > >>>  },
> > >>>  {
> > >>> +   

Re: [edk2] [PATCH] OvmfPkg: Don't enable unsupported attributes

[Volker Rümelin]

Thank you for the review.

> Nice catch!
>
> Please send a v2 with the following cosmetic changes:

> If you prefer, I can do these changes for you too -- I would keep your
> authorship and S-o-b in the first position. I'd mark my changes below
> your S-o-b.
>
> Thank you,
> Laszlo

I will send a v2 with the suggested changes.

With best regards,
Volker

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

[edk2] [PATCH v2] OvmfPkg: AcpiPlatformDxe: Don't enable unsupported PCI attributes

[Volker Rümelin]

Current code in PciEnableDecoding tries to unconditionally enable
EFI_PCI_IO_ATTRIBUTE_IO and EFI_PCI_IO_ATTRIBUTE_MEMORY even if they
are unsupported attributes. This fails on devices which don't
support both attributes.

This patch masks out unsupported attributes.

Information to reproduce the bug.

Host lspci -s :04:00.0 -vnn:
04:00.0 USB controller [0c03]: Renesas Technology Corp. uPD720201 USB
3.0 Host Controller [1912:0014] (rev 03) (prog-if 30 [XHCI])
Flags: fast devsel, IRQ 19
Memory at ef90 (64-bit, non-prefetchable) [size=8K]
Capabilities: [50] Power Management version 3
Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+
Capabilities: [90] MSI-X: Enable- Count=8 Masked-
Capabilities: [a0] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [150] Latency Tolerance Reporting
Kernel driver in use: pci-stub
Kernel modules: xhci_pci

libvirt xml:

  
  

  
  


OVMF debug log with additional DEBUG statement:
OnRootBridgesConnected: root bridges have been connected, installing
ACPI tables
Select Item: 0x19
EnablePciDecoding: GetLocation: D=:00:00.0
OrigAttr=4000 SuppAttr=E700
EnablePciDecoding: GetLocation: D=:00:10.0
OrigAttr=4000 SuppAttr=E700
EnablePciDecoding: GetLocation: D=:00:11.0
OrigAttr=4000 SuppAttr=E600
EnablePciDecoding: EfiPciIoAttributeOperationEnable: Unsupported
Select Item: 0x28
Select Item: 0x19
Select Item: 0x2A
Select Item: 0x19
Select Item: 0x27
InstallQemuFwCfgTables: installed 6 tables

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Volker Rümelin 
---

Changes since v1:
- Cosmetic changes from Laszlo's feedback

 OvmfPkg/AcpiPlatformDxe/PciDecoding.c | 16 ++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/PciDecoding.c 
b/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
index 3b9b12c..d63b57e 100644
--- a/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
+++ b/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
@@ -89,6 +89,7 @@ EnablePciDecoding (
 
   for (Idx = 0; Idx < NoHandles; ++Idx) {
 EFI_PCI_IO_PROTOCOL *PciIo;
+UINT64  Attributes;
 
 //
 // Look up PciIo on the handle and stash it
@@ -110,11 +111,22 @@ EnablePciDecoding (
 }
 
 //
+// Retrieve supported attributes
+//
+Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationSupported, 0,
+  &Attributes);
+if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_WARN, "%a: EfiPciIoAttributeOperationSupported: %r\n",
+__FUNCTION__, Status));
+  goto RestoreAttributes;
+}
+
+//
 // Enable IO and MMIO decoding
 //
+Attributes &= EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY;
 Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationEnable,
-  EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY,
-  NULL);
+  Attributes, NULL);
 if (EFI_ERROR (Status)) {
   DEBUG ((EFI_D_WARN, "%a: EfiPciIoAttributeOperationEnable: %r\n",
 __FUNCTION__, Status));
-- 
1.8.4.5

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

Re: [edk2] [PATCH v2] OvmfPkg: AcpiPlatformDxe: Don't enable unsupported PCI attributes

[Jordan Justen]

Reviewed-by: Jordan Justen 

On 2016-04-18 12:51:30, Volker Rümelin wrote:
> Current code in PciEnableDecoding tries to unconditionally enable
> EFI_PCI_IO_ATTRIBUTE_IO and EFI_PCI_IO_ATTRIBUTE_MEMORY even if they
> are unsupported attributes. This fails on devices which don't
> support both attributes.
> 
> This patch masks out unsupported attributes.
> 
> Information to reproduce the bug.
> 
> Host lspci -s :04:00.0 -vnn:
> 04:00.0 USB controller [0c03]: Renesas Technology Corp. uPD720201 USB
> 3.0 Host Controller [1912:0014] (rev 03) (prog-if 30 [XHCI])
> Flags: fast devsel, IRQ 19
> Memory at ef90 (64-bit, non-prefetchable) [size=8K]
> Capabilities: [50] Power Management version 3
> Capabilities: [70] MSI: Enable- Count=1/8 Maskable- 64bit+
> Capabilities: [90] MSI-X: Enable- Count=8 Masked-
> Capabilities: [a0] Express Endpoint, MSI 00
> Capabilities: [100] Advanced Error Reporting
> Capabilities: [150] Latency Tolerance Reporting
> Kernel driver in use: pci-stub
> Kernel modules: xhci_pci
> 
> libvirt xml:
> 
>   
>   
> 
>   
>   function='0'/>
> 
> 
> OVMF debug log with additional DEBUG statement:
> OnRootBridgesConnected: root bridges have been connected, installing
> ACPI tables
> Select Item: 0x19
> EnablePciDecoding: GetLocation: D=:00:00.0
> OrigAttr=4000 SuppAttr=E700
> EnablePciDecoding: GetLocation: D=:00:10.0
> OrigAttr=4000 SuppAttr=E700
> EnablePciDecoding: GetLocation: D=:00:11.0
> OrigAttr=4000 SuppAttr=E600
> EnablePciDecoding: EfiPciIoAttributeOperationEnable: Unsupported
> Select Item: 0x28
> Select Item: 0x19
> Select Item: 0x2A
> Select Item: 0x19
> Select Item: 0x27
> InstallQemuFwCfgTables: installed 6 tables
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Volker Rümelin 
> ---
> 
> Changes since v1:
> - Cosmetic changes from Laszlo's feedback
> 
>  OvmfPkg/AcpiPlatformDxe/PciDecoding.c | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/PciDecoding.c 
> b/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
> index 3b9b12c..d63b57e 100644
> --- a/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
> +++ b/OvmfPkg/AcpiPlatformDxe/PciDecoding.c
> @@ -89,6 +89,7 @@ EnablePciDecoding (
>  
>for (Idx = 0; Idx < NoHandles; ++Idx) {
>  EFI_PCI_IO_PROTOCOL *PciIo;
> +UINT64  Attributes;
>  
>  //
>  // Look up PciIo on the handle and stash it
> @@ -110,11 +111,22 @@ EnablePciDecoding (
>  }
>  
>  //
> +// Retrieve supported attributes
> +//
> +Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationSupported, 
> 0,
> +  &Attributes);
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((EFI_D_WARN, "%a: EfiPciIoAttributeOperationSupported: %r\n",
> +__FUNCTION__, Status));
> +  goto RestoreAttributes;
> +}
> +
> +//
>  // Enable IO and MMIO decoding
>  //
> +Attributes &= EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY;
>  Status = PciIo->Attributes (PciIo, EfiPciIoAttributeOperationEnable,
> -  EFI_PCI_IO_ATTRIBUTE_IO | EFI_PCI_IO_ATTRIBUTE_MEMORY,
> -  NULL);
> +  Attributes, NULL);
>  if (EFI_ERROR (Status)) {
>DEBUG ((EFI_D_WARN, "%a: EfiPciIoAttributeOperationEnable: %r\n",
>  __FUNCTION__, Status));
> -- 
> 1.8.4.5
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] OVMF: hang when booting Linux via GRUB after driver allocates 2GB above 4GB

[Bruce Cran]

On 4/18/16 8:36 AM, Laszlo Ersek wrote:


BTW, what happens if you boot UEFI Windows (i.e., 8+) in your VM? Does
it boot?


Yes, Windows 10 (build 1511) boots fine, as does OpenSUSE with kernel 
4.5.0.  That being the case, I'll file a bug with the CentOS tracker.


By the way, when I fetch the memory map (gBS->GetMemoryMap) in the 
optrom driver's entrypoint, it only reports 4GB RAM (therefore it makes 
sense that your AllocatePages call would fail); by the time its 
BindingStart function is called, however, it reports the full 16GB.


That's where I was allocating the 2GB at 6GB (in BindingStart), but I've 
now adapted the Hello application from AppPkg to do the allocation 
instead, to get vfio out of the way and let me just run an application 
to allocate the memory.


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

Re: [edk2] [Patch] ShellPkg: Update ping command options to sync with Spec

[Wu, Jiaxin]

Got it, the patch will be sent out later. 

Thanks.
Jiaxin

> -Original Message-
> From: Carsey, Jaben
> Sent: Tuesday, April 19, 2016 1:41 AM
> To: Wu, Jiaxin ; David Van Arnem
> ; Bhupesh Sharma ;
> Laszlo Ersek ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Carsey,
> Jaben 
> Subject: RE: [edk2] [Patch] ShellPkg: Update ping command options to sync
> with Spec
> 
> That sounds like a good enhancement to me.
> 
> -Jaben
> 
> 
> > -Original Message-
> > From: Wu, Jiaxin
> > Sent: Thursday, April 14, 2016 8:52 PM
> > To: Carsey, Jaben ; David Van Arnem
> > ; Bhupesh Sharma ;
> Laszlo
> > Ersek ; edk2-devel@lists.01.org
> > Cc: Ye, Ting ; Fu, Siyuan 
> > Subject: RE: [edk2] [Patch] ShellPkg: Update ping command options to
> > sync with Spec
> > Importance: High
> >
> > Jaben ,
> >
> > Do you agree to support no source IP specified case? After update,
> > Ping will select the first both connected and configured interface
> > automatically instead of always the first NIC.
> > If no objection, patch will be sent out for review.
> >
> > Thanks.
> > Jiaxin
> >
> >
> > > -Original Message-
> > > From: David Van Arnem [mailto:dvanar...@cmlab.biz]
> > > Sent: Friday, April 15, 2016 1:32 AM
> > > To: Wu, Jiaxin ; Bhupesh Sharma
> > > ; Laszlo Ersek ; edk2-
> > > de...@lists.01.org
> > > Cc: Ye, Ting ; Carsey, Jaben
> > > ; Fu, Siyuan 
> > > Subject: Re: [edk2] [Patch] ShellPkg: Update ping command options to
> > > sync with Spec
> > >
> > > On 04/13/2016 09:13 PM, Wu, Jiaxin wrote:
> > > > Hi David and Laszlo,
> > > >
> > > > This patch is not focused on the discussion about the multiple
> > > > NICs existed
> > > case, just use to update the ping command option. So, please don't
> > > misunderstand.
> > > >
> > >
> > > Ah, ok.  Sorry to clutter up this patch discussion then, I wanted to
> > > make sure the multiple NIC case wasn't lost in the discussion about "-
> s"/"-_s".  Thanks!
> > >
> > > > For the discussion in
> > > , If there
> > > are multiple NICs existed in the platform and no source IP is specified
> (no '-s'
> > > option), the first NIC will be selected as default interface to ping
> > > the target. '- s SourceIp' can be used to specify any interface you
> > > want. That's the current implementation choice.
> > > >
> > > > I agree your and Bhupesh 's option to support no source IP
> > > > specified case,
> > > that's meaningful to improve the command's friendliness(Windows and
> > Linus
> > > do this resolution). How about selecting the first both connected and
> > > configured interface automatically?   If so, I can create another patch to
> > > support that solution.
> > > >
> > >
> > > That sounds great to me, as long as it's not too much trouble :-)
> > > Alternately,
> > a
> > > more useful error message in that case might suffice (e.g. "Multiple
> > > configured interfaces found, please specify one with '-s'").  But,
> > > since Windows and Linux both do the automatic resolution, I think
> > > consistency would be beneficial.
> > >
> > > Thanks again!
> > >
> > > David
> > >
> > > > Thanks.
> > > > Jiaxin
> > > >
> > > >> -Original Message-
> > > >> From: David Van Arnem [mailto:dvanar...@cmlab.biz]
> > > >> Sent: Thursday, April 14, 2016 4:02 AM
> > > >> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> > > >> Cc: Ye, Ting ; Carsey, Jaben
> > > >> ; Fu, Siyuan 
> > > >> Subject: Re: [edk2] [Patch] ShellPkg: Update ping command options
> > > >> to sync with Spec
> > > >>
> > > >> On 04/12/2016 09:16 PM, Jiaxin Wu wrote:
> > > >>> This patch is used to update ping command options to sync with
> > > >>> shell2.2 Spec.
> > > >>> Considering the backward compatible issue, the patch keeps ‘-_s’
> > > >>> command option unchanged, only add the new option '-s'
> > > >>> and make the old option '-_s' function same as new one.
> > > >>>
> > > >>> Cc: Ye Ting 
> > > >>> Cc: Fu Siyuan 
> > > >>> Cc: Jaben Carsey 
> > > >>> Contributed-under: TianoCore Contribution Agreement 1.0
> > > >>> Signed-off-by: Jiaxin Wu 
> > > >>> ---
> > > >>>.../Library/UefiShellNetwork1CommandsLib/Ping.c| 12
> > ++-
> > > -
> > > >>>.../UefiShellNetwork1CommandsLib.uni   | 22 
> > > >>> +---
> -
> > -
> > > 
> > > >>>2 files changed, 19 insertions(+), 15 deletions(-)
> > > >>>
> > > >>> diff --git
> > > >>> a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > >>> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > >>> index dbee764..13bcdde 100644
> > > >>> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > >>> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> > > >>> @@ -1,10 +1,10 @@
> > > >>>/** @file
> > > >>>  The implementation for Ping shell command.
> > > >>>
> > > >>>  (C) Copyright 2015 Hewlett-Packard Development Company,
> > > >>> L.P.
> > > >>> -  Copyright (c) 2009 - 2015, Intel Corporation. All rights
> > > >>> reserve

[edk2] [Patch] ShellPkg: Enhance ping to select the interface automatically

[Jiaxin Wu]

This patch is used to support no source IP specified case
while multiple NICs existed in the platform. The command
will select the first both connected and configured interface
automatically.

Cc: David Van Arnem 
Cc: Bhupesh Sharma 
Cc: Jaben Carsey 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 .../Library/UefiShellNetwork1CommandsLib/Ping.c| 224 -
 1 file changed, 127 insertions(+), 97 deletions(-)

diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
index 13bcdde..6b05884 100644
--- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
+++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
@@ -874,20 +874,24 @@ PingCreateIpInstance (
 {
   EFI_STATUS   Status;
   UINTNHandleIndex;
   UINTNHandleNum;
   EFI_HANDLE   *HandleBuffer;
+  BOOLEAN  UnspecifiedSrc;
+  BOOLEAN  MediaPresent;
   EFI_SERVICE_BINDING_PROTOCOL *EfiSb;
   VOID *IpXCfg;
   EFI_IP6_CONFIG_DATA  Ip6Config;
   EFI_IP4_CONFIG_DATA  Ip4Config;
   VOID *IpXInterfaceInfo;
   UINTNIfInfoSize;
   EFI_IPv6_ADDRESS *Addr;
   UINTNAddrIndex;
 
   HandleBuffer  = NULL;
+  UnspecifiedSrc= FALSE;
+  MediaPresent  = TRUE;
   EfiSb = NULL;
   IpXInterfaceInfo  = NULL;
   IfInfoSize= 0;
 
   //
@@ -923,139 +927,165 @@ PingCreateIpInstance (
   ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV), 
gShellNetwork1HiiHandle, L"ping", mSrcString);  
   Status = EFI_INVALID_PARAMETER;
   goto ON_ERROR;
 }
   }
+
+  if (Private->IpChoice == PING_IP_CHOICE_IP6 ? NetIp6IsUnspecifiedAddr 
((EFI_IPv6_ADDRESS*)&Private->SrcAddress) : \
+  PingNetIp4IsUnspecifiedAddr ((EFI_IPv4_ADDRESS*)&Private->SrcAddress)) {
+//
+// SrcAddress is unspecified. So, both connected and configured interface 
will be automatic selected. 
+//
+UnspecifiedSrc = TRUE;
+  }
+  
   //
   // For each ip6 protocol, check interface addresses list.
   //
   for (HandleIndex = 0; HandleIndex < HandleNum; HandleIndex++) {
-
 EfiSb = NULL;
 IpXInterfaceInfo  = NULL;
 IfInfoSize= 0;
 
+if (UnspecifiedSrc) {
+  //
+  // Check media.
+  //
+  NetLibDetectMedia (HandleBuffer[HandleIndex], &MediaPresent);
+  if (!MediaPresent) {
+//
+// Skip this one.
+//
+continue;
+  }
+}
+
 Status = gBS->HandleProtocol (
 HandleBuffer[HandleIndex],
 Private->IpChoice == 
PING_IP_CHOICE_IP6?&gEfiIp6ServiceBindingProtocolGuid:&gEfiIp4ServiceBindingProtocolGuid,
 (VOID **) &EfiSb
 );
 if (EFI_ERROR (Status)) {
   goto ON_ERROR;
 }
 
-if (Private->IpChoice == PING_IP_CHOICE_IP6?NetIp6IsUnspecifiedAddr 
((EFI_IPv6_ADDRESS*)&Private->SrcAddress):PingNetIp4IsUnspecifiedAddr 
((EFI_IPv4_ADDRESS*)&Private->SrcAddress)) {
-  //
-  // No need to match interface address.
-  //
-  break;
+//
+// Ip6config protocol and ip6 service binding protocol are installed
+// on the same handle.
+//
+Status = gBS->HandleProtocol (
+HandleBuffer[HandleIndex],
+Private->IpChoice == 
PING_IP_CHOICE_IP6?&gEfiIp6ConfigProtocolGuid:&gEfiIp4Config2ProtocolGuid,
+(VOID **) &IpXCfg
+);
+
+if (EFI_ERROR (Status)) {
+  goto ON_ERROR;
+}
+//
+// Get the interface information size.
+//
+if (Private->IpChoice == PING_IP_CHOICE_IP6) {
+  Status = ((EFI_IP6_CONFIG_PROTOCOL*)IpXCfg)->GetData (
+ IpXCfg,
+ Ip6ConfigDataTypeInterfaceInfo,
+ &IfInfoSize,
+ NULL
+ );
 } else {
-  //
-  // Ip6config protocol and ip6 service binding protocol are installed
-  // on the same handle.
-  //
-  Status = gBS->HandleProtocol (
-  HandleBuffer[HandleIndex],
-  Private->IpChoice == 
PING_IP_CHOICE_IP6?&gEfiIp6ConfigProtocolGuid:&gEfiIp4Config2ProtocolGuid,
-  (VOID **) &IpXCfg
-  );
+  Status = ((EFI_IP4_CONFIG2_PROTOCOL*)IpXCfg)->GetData (
+ IpXCfg,
+ Ip4Config2DataTypeInterfaceInfo,
+ &IfInfoSize,
+ NULL
+ );
+}
+
+//
+// Skip the ones not in current use.
+//
+if (Status == EFI_NOT_STARTED) {
+  continue;
+}
 
-  if 

Re: [edk2] [PATCH] SecurityPkg: AuthVariableLib & SecureBootConfigDxe: Fix SecureBootEnable & PK inconsistency issue

[Fu, Siyuan]

Hi, Chao

The patch is good to me. In the commit message you'd better to clearly list 
which fix is revert by this patch.

Reviewed-by: Fu Siyuan 


> -Original Message-
> From: Zhang, Chao B
> Sent: Wednesday, April 13, 2016 3:34 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Zhang, Chao B
> 
> Subject: [PATCH] SecurityPkg: AuthVariableLib & SecureBootConfigDxe: Fix
> SecureBootEnable & PK inconsistency issue
> 
> Revert previous fix in AuthVariable driver init which breaks SecureBootEnable
> original behavior. Add more error handling logic in SecureBootConfigDxe to
> prevent wrong display info when SecureBootEnable & PK inconsistency
> happens.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  SecurityPkg/Library/AuthVariableLib/AuthService.c  | 14 +--
>  .../SecureBootConfigDxe/SecureBootConfigImpl.c | 47 
> --
>  2 files changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> index f11b868..4649e50 100644
> --- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
> +++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
> @@ -441,19 +441,7 @@ InitSecureBootVariables (
>SecureBootEnable = SECURE_BOOT_DISABLE;
>Status = AuthServiceInternalFindVariable
> (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid,
> (VOID **)&Data, &DataSize);
>if (!EFI_ERROR(Status)) {
> -if (!IsPkPresent) {
> -  //
> -  // PK is cleared in runtime. "SecureBootMode" is not updated before
> reboot
> -  // Delete "SecureBootMode"
> -  //
> -  Status = AuthServiceInternalUpdateVariable (
> - EFI_SECURE_BOOT_ENABLE_NAME,
> - &gEfiSecureBootEnableDisableGuid,
> - &SecureBootEnable,
> - 0,
> - EFI_VARIABLE_NON_VOLATILE |
> EFI_VARIABLE_BOOTSERVICE_ACCESS
> - );
> -} else {
> +if (IsPkPresent) {
>SecureBootEnable = *Data;
>  }
>} else if ((SecureBootMode == SecureBootModeTypeUserMode) ||
> (SecureBootMode == SecureBootModeTypeDeployedMode)) {
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
> gImpl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> igImpl.c
> index e840316..c8f4d97 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
> gImpl.c
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> igImpl.c
> @@ -3167,20 +3167,6 @@ SecureBootExtractConfigFromVariable (
>ConfigData->RevocationTime.Minute = CurrTime.Minute;
>ConfigData->RevocationTime.Second = 0;
> 
> -  //
> -  // If the SecureBootEnable Variable doesn't exist, hide the SecureBoot
> Enable/Disable
> -  // Checkbox.
> -  //
> -  ConfigData->AttemptSecureBoot = FALSE;
> -  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME,
> &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);
> -  if (SecureBootEnable == NULL) {
> -ConfigData->HideSecureBoot = TRUE;
> -  } else {
> -ConfigData->HideSecureBoot = FALSE;
> -if ((*SecureBootEnable) == SECURE_BOOT_ENABLE) {
> -  ConfigData->AttemptSecureBoot = TRUE;
> -}
> -  }
> 
>//
>// If it is Physical Presence User, set the PhysicalPresent to true.
> @@ -3215,6 +3201,26 @@ SecureBootExtractConfigFromVariable (
>  ConfigData->HasPk = TRUE;
>}
> 
> +  //
> +  // Check SecureBootEnable & Pk status, fix the inconsistence.
> +  // If the SecureBootEnable Variable doesn't exist, hide the SecureBoot
> Enable/Disable
> +  // Checkbox.
> +  //
> +  ConfigData->AttemptSecureBoot = FALSE;
> +  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME,
> &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);
> +
> +  //
> +  // Fix Pk, SecureBootEnable inconsistence
> +  //
> +  if (ConfigData->CurSecureBootMode ==
> SECURE_BOOT_MODE_USER_MODE || ConfigData->CurSecureBootMode ==
> SECURE_BOOT_MODE_DEPLOYED_MODE) {
> +ConfigData->HideSecureBoot = FALSE;
> +if ((SecureBootEnable != NULL) && (*SecureBootEnable ==
> SECURE_BOOT_ENABLE)) {
> +  ConfigData->AttemptSecureBoot = TRUE;
> +}
> +  } else {
> +ConfigData->HideSecureBoot = TRUE;
> +  }
> +
>if (SecureBootEnable != NULL) {
>  FreePool (SecureBootEnable);
>}
> @@ -3363,7 +3369,6 @@ SecureBootRouteConfig (
> OUT EFI_STRING  *Progress
>)
>  {
> -  UINT8  *SecureBootEnable;
>SECUREBOOT_CONFIGURATION   IfrNvData;
>UINTN  BufferSize;
>EFI_STATUS Status;
> @@ -3400,10 +3405,7 @@ SecureBootRouteConfig (
>//
>// Store Buffer Storage back to EFI variable if needed
>//
> -  SecureBootEnable = NULL;
> -  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME,
> &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);
> -  if (NULL != Sec

[edk2] [Patch v2] BaseTools: fix a bug for PEI VPD Pcd collection

[Yonghong Zhu]

When a PEI phase VPD PCD only list in the DSC IA32 arch, then build X64
arch image, it missed to collect this PEI VPD pcd into VPD Pcd map file.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index f29d368..50d585f 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -859,10 +859,11 @@ class PlatformAutoGen(AutoGen):
 # Used to store all PCDs for both PEI and DXE phase, in order to generate 
 # correct PCD database
 # 
 _DynaPcdList_ = []
 _NonDynaPcdList_ = []
+_PlatformPcds = {}
 
 #
 # The priority list while override build option 
 #
 PrioList = {"0x1"  : 16, #  
TARGET_TOOLCHAIN_ARCH_COMMANDTYPE_ATTRIBUTE (Highest)
@@ -1211,12 +1212,16 @@ class PlatformAutoGen(AutoGen):
 UnicodePcdArray = []
 HiiPcdArray = []
 OtherPcdArray   = []
 VpdPcdDict  = {}
 VpdFile   = VpdInfoFile.VpdInfoFile()
-NeedProcessVpdMapFile = False
-
+NeedProcessVpdMapFile = False
+
+for pcd in self.Platform.Pcds.keys():
+if pcd not in self._PlatformPcds.keys():
+self._PlatformPcds[pcd] = self.Platform.Pcds[pcd]
+
 if (self.Workspace.ArchList[-1] == self.Arch): 
 for Pcd in self._DynamicPcdList:
 # just pick the a value to determine whether is unicode string 
type
 Sku = Pcd.SkuInfoList[Pcd.SkuInfoList.keys()[0]]
 Sku.VpdOffset = Sku.VpdOffset.strip()
@@ -1231,17 +1236,17 @@ class PlatformAutoGen(AutoGen):
 else:
 OtherPcdArray.append(Pcd)
 if Pcd.Type in [TAB_PCDS_DYNAMIC_VPD, TAB_PCDS_DYNAMIC_EX_VPD]:
 VpdPcdDict[(Pcd.TokenCName, Pcd.TokenSpaceGuidCName)] = Pcd
 
-PlatformPcds = self.Platform.Pcds.keys()
+PlatformPcds = self._PlatformPcds.keys()
 PlatformPcds.sort()
 #
 # Add VPD type PCD into VpdFile and determine whether the VPD PCD 
need to be fixed up.
 #
 for PcdKey in PlatformPcds:
-Pcd = self.Platform.Pcds[PcdKey]
+Pcd = self._PlatformPcds[PcdKey]
 if Pcd.Type in [TAB_PCDS_DYNAMIC_VPD, TAB_PCDS_DYNAMIC_EX_VPD] 
and \
PcdKey in VpdPcdDict:
 Pcd = VpdPcdDict[PcdKey]
 for (SkuName,Sku) in Pcd.SkuInfoList.items():
 Sku.VpdOffset = Sku.VpdOffset.strip()
@@ -1279,11 +1284,11 @@ class PlatformAutoGen(AutoGen):
 #
 # Fix the PCDs define in VPD PCD section that never referenced by 
module.
 # An example is PCD for signature usage.
 #
 for DscPcd in PlatformPcds:
-DscPcdEntry = self.Platform.Pcds[DscPcd]
+DscPcdEntry = self._PlatformPcds[DscPcd]
 if DscPcdEntry.Type in [TAB_PCDS_DYNAMIC_VPD, 
TAB_PCDS_DYNAMIC_EX_VPD]:
 if not (self.Platform.VpdToolGuid == None or 
self.Platform.VpdToolGuid == ''):
 FoundFlag = False
 for VpdPcd in VpdFile._VpdArray.keys():
 # This PCD has been referenced by module
-- 
2.6.1.windows.1

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

Re: [edk2] [patch] NetworkPkg: refine codes of iSCSI driver.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zhang Lubo
> Sent: Monday, April 18, 2016 4:13 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [patch] NetworkPkg: refine codes of iSCSI driver.
> 
> Add error handling logic in DriverBingingStop function,
> it may return error status when invoking the
> UninstallProtocolInterface.
> 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Cc: Wu Jiaxin 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> ---
>  NetworkPkg/IScsiDxe/IScsiDriver.c | 20 
>  NetworkPkg/IScsiDxe/IScsiMisc.c   | 25 +
>  NetworkPkg/IScsiDxe/IScsiMisc.h   |  9 ++---
>  3 files changed, 39 insertions(+), 15 deletions(-)
> 
> diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c
> b/NetworkPkg/IScsiDxe/IScsiDriver.c
> index 51ce169..cdc818f 100644
> --- a/NetworkPkg/IScsiDxe/IScsiDriver.c
> +++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
> @@ -1,9 +1,9 @@
>  /** @file
>The entry point of IScsi driver.
> 
> -Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2016, 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
> 
> @@ -321,11 +321,13 @@ IScsiSupported (
>@retval EFI_SUCCESThis driver was started.
>@retval EFI_ALREADY_STARTED   This driver is already running on this
> device.
>@retval EFI_INVALID_PARAMETER Any input parameter is invalid.
>@retval EFI_NOT_FOUND There is no sufficient information to 
> establish
>  the iScsi session.
> -  @retval EFI_DEVICE_ERROR  Failed to get TCP connection device path.
> +  @retval EFI_DEVICE_ERROR  Failed to get TCP connection device path.
> +  @retval EFI_ACCESS_DENIED The interface was not removed because the
> interface is
> +still being used by a driver.
> 
>  **/
>  EFI_STATUS
>  IScsiStart (
>IN EFI_HANDLE   Image,
> @@ -861,11 +863,14 @@ IScsiStart (
>  IScsiRemoveNic (ExistPrivate->Controller);
>  if (ExistPrivate->Session != NULL) {
>IScsiSessionAbort (ExistPrivate->Session);
>  }
> 
> -IScsiCleanDriverData (ExistPrivate);
> +Status = IScsiCleanDriverData (ExistPrivate);
> +if (EFI_ERROR (Status)) {
> +  goto ON_ERROR;
> +}
>}
>  } else {
>//
>// Use the attempt in earlier order as boot selected in single path 
> mode.
>//
> @@ -961,10 +966,13 @@ ON_ERROR:
>  if NumberOfChildren is 0.
>@param[in]  IpVersion IP_VERSION_4 or IP_VERSION_6.
> 
>@retval EFI_SUCCESS   The device was stopped.
>@retval EFI_DEVICE_ERROR  The device could not be stopped due to a
> device error.
> +  @retval EFI_INVALID_PARAMETER Child handle is NULL.
> +  @retval EFI_ACCESS_DENIED The interface was not removed because the
> interface is
> +still being used by a driver.
> 
>  **/
>  EFI_STATUS
>  EFIAPI
>  IScsiStop (
> @@ -1103,11 +,15 @@ IScsiStop (
> 
>if (Private->Session != NULL) {
>  IScsiSessionAbort (Private->Session);
>}
> 
> -  IScsiCleanDriverData (Private);
> +  Status = IScsiCleanDriverData (Private);
> +
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> 
>return EFI_SUCCESS;
>  }
> 
>  /**
> diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c
> b/NetworkPkg/IScsiDxe/IScsiMisc.c
> index 5fc25a0..2406717 100644
> --- a/NetworkPkg/IScsiDxe/IScsiMisc.c
> +++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
> @@ -1,9 +1,9 @@
>  /** @file
>Miscellaneous routines for iSCSI driver.
> 
> -Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2016, 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
> 
> @@ -855,26 +855,32 @@ IScsiCreateDriverData (
> 
> 
>  /**
>Clean the iSCSI driver data.
> 
> -  @param[in]  Private The iSCSI driver data.
> +  @param[in]  Private The iSCSI driver data.
> +
> +  @retval EFI_SUCCES  The clean operation is successful.
> +  @retval Others  Other errors as indicated.
> 
>  **/
> -VOID
> +EFI_STATUS
>  IScsiCleanDriverData (
>IN ISCSI_DRIVER_DATA  *Private
>)
>  {
>EFI_STATUSStatus;
> 
>if (Private->DevicePath != NULL) {
> -gBS->UninstallProtocolInterface (
>

Re: [edk2] [patch] MdeModulePkg: refine codes of iSCSI driver.

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zhang Lubo
> Sent: Monday, April 18, 2016 4:12 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu,
> Jiaxin 
> Subject: [edk2] [patch] MdeModulePkg: refine codes of iSCSI driver.
> 
> Add error handling logic in DriverBingingStop function,
> it may return error status when invoking the
> UninstallProtocolInterface.
> 
> Cc: Fu Siyuan 
> Cc: Ye Ting 
> Cc: Wu Jiaxin 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> ---
>  .../Universal/Network/IScsiDxe/IScsiDriver.c   | 11 +--
>  .../Universal/Network/IScsiDxe/IScsiMisc.c | 38 
> ++
>  .../Universal/Network/IScsiDxe/IScsiMisc.h |  9 +++--
>  3 files changed, 40 insertions(+), 18 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
> b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
> index e55bee8..74379e1 100644
> --- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
> +++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiDriver.c
> @@ -1,9 +1,9 @@
>  /** @file
>The entry point of IScsi driver.
> 
> -Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2016, 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
> 
> @@ -337,10 +337,14 @@ ON_ERROR:
>@param[in]  ChildHandleBuffer An array of child handles to be freed. May
> be NULL
>  if NumberOfChildren is 0.Not used.
> 
>@retval EFI_SUCCESS   The device was stopped.
>@retval EFI_DEVICE_ERROR  The device could not be stopped due to a
> device error.
> +  @retval EFI_INVALID_PARAMETER Child handle is NULL.
> +  @retval EFI_ACCESS_DENIED The interface was not removed because the
> interface is
> +still being used by a driver.
> +
>  **/
>  EFI_STATUS
>  EFIAPI
>  IScsiDriverBindingStop (
>IN EFI_DRIVER_BINDING_PROTOCOL  *This,
> @@ -449,11 +453,14 @@ IScsiDriverBindingStop (
>// Update the iSCSI Boot Firware Table.
>//
>IScsiPublishIbft ();
> 
>IScsiSessionAbort (&Private->Session);
> -  IScsiCleanDriverData (Private);
> +  Status = IScsiCleanDriverData (Private);
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> 
>return EFI_SUCCESS;
>  }
> 
>  /**
> diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
> b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c
> index ebd9f5b..bb48d8c 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 - 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2016, 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
> 
> @@ -585,38 +585,50 @@ IScsiCreateDriverData (
>  }
> 
>  /**
>Clean the iSCSI driver data.
> 
> -  @param[in]  Private The iSCSI driver data.
> +  @param[in]  Private The iSCSI driver data.
> +
> +  @retval EFI_SUCCES  The clean operation is successful.
> +  @retval Others  Other errors as indicated.
> +
>  **/
> -VOID
> +EFI_STATUS
>  IScsiCleanDriverData (
>IN ISCSI_DRIVER_DATA  *Private
>)
>  {
> +  EFI_STATUS Status;
> +
>if (Private->DevicePath != NULL) {
> -gBS->UninstallProtocolInterface (
> -  Private->ExtScsiPassThruHandle,
> -  &gEfiDevicePathProtocolGuid,
> -  Private->DevicePath
> -  );
> +Status = gBS->UninstallProtocolInterface (
> +Private->ExtScsiPassThruHandle,
> +&gEfiDevicePathProtocolGuid,
> +Private->DevicePath
> +);
> +if (EFI_ERROR (Status)) {
> +  goto EXIT;
> +}
> 
>  FreePool (Private->DevicePath);
>}
> 
>if (Private->ExtScsiPassThruHandle != NULL) {
> -gBS->UninstallProtocolInterface (
> -  Private->ExtScsiPassThruHandle,
> -  &gEfiExtScsiPassThruProtocolGuid,
> -  &Private->IScsiExtScsiPassThru
> -  );
> +Status = gBS->UninstallProtocolInterface (
> +Private->ExtScsiPassThruHandle,
> +&gEfiExtScsiPassThruProtocolGuid,
> +&Private->IScsiExtScsiPassThru
> +);
>}
> 
> +EXIT:
> +
>

Re: [edk2] [Patch v2] BaseTools: fix a bug for PEI VPD Pcd collection

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Zhu, Yonghong
> Sent: Tuesday, April 19, 2016 10:47 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming
> Subject: [Patch v2] BaseTools: fix a bug for PEI VPD Pcd collection
> 
> When a PEI phase VPD PCD only list in the DSC IA32 arch, then build X64
> arch image, it missed to collect this PEI VPD pcd into VPD Pcd map file.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/AutoGen/AutoGen.py | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py
> b/BaseTools/Source/Python/AutoGen/AutoGen.py
> index f29d368..50d585f 100644
> --- a/BaseTools/Source/Python/AutoGen/AutoGen.py
> +++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
> @@ -859,10 +859,11 @@ class PlatformAutoGen(AutoGen):
>  # Used to store all PCDs for both PEI and DXE phase, in order to generate
>  # correct PCD database
>  #
>  _DynaPcdList_ = []
>  _NonDynaPcdList_ = []
> +_PlatformPcds = {}
> 
>  #
>  # The priority list while override build option
>  #
>  PrioList = {"0x1"  : 16, #
> TARGET_TOOLCHAIN_ARCH_COMMANDTYPE_ATTRIBUTE (Highest)
> @@ -1211,12 +1212,16 @@ class PlatformAutoGen(AutoGen):
>  UnicodePcdArray = []
>  HiiPcdArray = []
>  OtherPcdArray   = []
>  VpdPcdDict  = {}
>  VpdFile   = VpdInfoFile.VpdInfoFile()
> -NeedProcessVpdMapFile = False
> -
> +NeedProcessVpdMapFile = False
> +
> +for pcd in self.Platform.Pcds.keys():
> +if pcd not in self._PlatformPcds.keys():
> +self._PlatformPcds[pcd] = self.Platform.Pcds[pcd]
> +
>  if (self.Workspace.ArchList[-1] == self.Arch):
>  for Pcd in self._DynamicPcdList:
>  # just pick the a value to determine whether is unicode 
> string type
>  Sku = Pcd.SkuInfoList[Pcd.SkuInfoList.keys()[0]]
>  Sku.VpdOffset = Sku.VpdOffset.strip()
> @@ -1231,17 +1236,17 @@ class PlatformAutoGen(AutoGen):
>  else:
>  OtherPcdArray.append(Pcd)
>  if Pcd.Type in [TAB_PCDS_DYNAMIC_VPD,
> TAB_PCDS_DYNAMIC_EX_VPD]:
>  VpdPcdDict[(Pcd.TokenCName, Pcd.TokenSpaceGuidCName)] =
> Pcd
> 
> -PlatformPcds = self.Platform.Pcds.keys()
> +PlatformPcds = self._PlatformPcds.keys()
>  PlatformPcds.sort()
>  #
>  # Add VPD type PCD into VpdFile and determine whether the VPD PCD
> need to be fixed up.
>  #
>  for PcdKey in PlatformPcds:
> -Pcd = self.Platform.Pcds[PcdKey]
> +Pcd = self._PlatformPcds[PcdKey]
>  if Pcd.Type in [TAB_PCDS_DYNAMIC_VPD,
> TAB_PCDS_DYNAMIC_EX_VPD] and \
> PcdKey in VpdPcdDict:
>  Pcd = VpdPcdDict[PcdKey]
>  for (SkuName,Sku) in Pcd.SkuInfoList.items():
>  Sku.VpdOffset = Sku.VpdOffset.strip()
> @@ -1279,11 +1284,11 @@ class PlatformAutoGen(AutoGen):
>  #
>  # Fix the PCDs define in VPD PCD section that never referenced by
> module.
>  # An example is PCD for signature usage.
>  #
>  for DscPcd in PlatformPcds:
> -DscPcdEntry = self.Platform.Pcds[DscPcd]
> +DscPcdEntry = self._PlatformPcds[DscPcd]
>  if DscPcdEntry.Type in [TAB_PCDS_DYNAMIC_VPD,
> TAB_PCDS_DYNAMIC_EX_VPD]:
>  if not (self.Platform.VpdToolGuid == None or
> self.Platform.VpdToolGuid == ''):
>  FoundFlag = False
>  for VpdPcd in VpdFile._VpdArray.keys():
>  # This PCD has been referenced by module
> --
> 2.6.1.windows.1

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

Re: [edk2] [PATCH v3] MdeModulePkg/HiiDatabaseDxe: Support EfiVarStore to get AltCfg from Driver

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Dandan Bi
> Sent: Monday, April 18, 2016 5:04 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric; Gao, Liming
> Subject: [edk2] [PATCH v3] MdeModulePkg/HiiDatabaseDxe: Support
> EfiVarStore to get AltCfg from Driver
> 
> Allow EfiVarStore to get  from Hii Driver, and enhance code logic
> in MergeDefaultString function to get a full AltCfgResp.
> The logic in function MergeDefaultString after enhancement:
> (1) If there are no  in AltCfgResp, merge the  in
> DefaultAltCfgResp to AltCfgResp.
> (2) If there are  in AltCfgResp, for the same , if
> the  already in AltCfgResp, don't need to merge from
> DefaultAltCfgResp, else merge the  in the
> DefaultAltCfgResp
> to the related  in AltCfgResp.
> AltCfgResp: Generated by Driver.
> DefaultAltCfgResp: Generated by HiiDatabase.
> 
> Notes:
>   V3:
>   - Enhance the logic in function FindSameBlockElement ().
> 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  .../Universal/HiiDatabaseDxe/ConfigRouting.c   | 541
> +
>  1 file changed, 541 insertions(+)
> 
> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> index 3a871cf..2e3608c 100644
> --- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> +++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
> @@ -508,10 +508,481 @@ Exit:
> 
>return Status;
>  }
> 
>  /**
> + To find the BlockName in the string with same value.
> +
> +  @param  String Pointer to a Null-terminated Unicode string.
> +  @param  BlockName  Pointer to a Null-terminated Unicode string 
> to
> search for.
> +  @param  Buffer Pointer to the value correspond to the 
> BlockName.
> +  @param  Found  The Block whether has been found.
> +  @param  BufferLen  The length of the buffer.
> +
> +  @retval EFI_OUT_OF_RESOURCES   Insufficient resources to store
> neccessary structures.
> +  @retval EFI_SUCCESSThe function finishes successfully.
> +
> +**/
> +EFI_STATUS
> +FindSameBlockElement(
> +  IN  EFI_STRING   String,
> +  IN  EFI_STRING   BlockName,
> +  IN  UINT8*Buffer,
> +  OUT BOOLEAN  *Found,
> +  IN  UINTNBufferLen
> +  )
> +{
> +  EFI_STRING   BlockPtr;
> +  UINTNLength;
> +  UINT8*TempBuffer;
> +  EFI_STATUS   Status;
> +
> +  TempBuffer = NULL;
> +  *Found = FALSE;
> +  BlockPtr = StrStr (String, BlockName);
> +
> +  while (BlockPtr != NULL) {
> +BlockPtr += StrLen (BlockName);
> +Status = GetValueOfNumber (BlockPtr, &TempBuffer, &Length);
> +if (EFI_ERROR (Status)) {
> +  return Status;
> +}
> +ASSERT (TempBuffer != NULL);
> +if ((BufferLen == Length) && (0 == CompareMem (Buffer, TempBuffer,
> Length))) {
> +  *Found = TRUE;
> +  return EFI_SUCCESS;
> +} else {
> +  FreePool (TempBuffer);
> +  TempBuffer = NULL;
> +  BlockPtr = StrStr (BlockPtr + 1, BlockName);
> +}
> +  }
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  Compare the  in ConfigAltResp and DefaultAltCfgResp, if the
> 
> +  in DefaultAltCfgResp but not in ConfigAltResp,add it to the ConfigAltResp.
> +
> +  @param  DefaultAltCfgResp  Pointer to a null-terminated Unicode string
> in
> +  format. The default 
> value
> + string may contain more than one 
> ConfigAltResp
> + string for the different varstore buffer.
> +  @param  ConfigAltResp  Pointer to a null-terminated Unicode string 
> in
> +  format.
> +  @param  AltConfigHdr   Pointer to a Unicode string in 
> 
> format.
> +  @param  ConfigAltRespChanged   Whether the ConfigAltResp has been
> changed.
> +
> +  @retval EFI_OUT_OF_RESOURCES   Insufficient resources to store
> neccessary structures.
> +  @retval EFI_SUCCESSThe function finishes  successfully.
> +
> +**/
> +EFI_STATUS
> +CompareBlockElementDefault (
> +  IN  EFI_STRING  DefaultAltCfgResp,
> +  IN OUT  EFI_STRING  *ConfigAltResp,
> +  IN  EFI_STRING  AltConfigHdr,
> +  IN OUT  BOOLEAN *ConfigAltRespChanged
> +)
> +{
> +  EFI_STATUSStatus;
> +  EFI_STRINGBlockPtr;
> +  EFI_STRINGBlockPtrStart;
> +  EFI_STRINGStringPtr;
> +  EFI_STRINGAppendString;
> +  EFI_STRINGAltConfigHdrPtr;
> +  UINT8 *TempBuffer;
> +  UINTN OffsetLength;
> +  UINTN AppendSize;
> +  UINTN TotalSize;
> +  BOOLEAN   FoundOffset;
> +
> +  AppendString = NULL;
> +  TempBuffer   = NULL;
> +  //
> +  // Make BlockPtr point to the first  with AltConfigHdr in
> DefaultAltCfgResp.
> +  //
> +  AltConfigHdrPtr = StrStr (DefaultAltCfgResp, AltConfigHdr);
> +  ASSERT (AltConfigHdrPtr != NULL);

Re: [edk2] [Patch] BaseTools/GenFds: remove the old logic since ActivePlatform is abs. path

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Zhu, Yonghong
> Sent: Monday, April 18, 2016 3:40 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming; Marvin Haeuser
> Subject: [Patch] BaseTools/GenFds: remove the old logic since
> ActivePlatform is abs. path
> 
> We can support the DSC file out of workspace. this old logic first make
> the absolute path to relative path and strips the leading slash off,
> then append it to workspace. it cause GenFds failure on Linux when the
> DSC file is out of workspace. Since we make sure the ActivePlatform is
> abs. path, so we don't need this old logic to change the abs. path to
> relative.
> 
> Cc: Liming Gao 
> Cc: Marvin Haeuser 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/GenFds/GenFds.py | 10 +-
>  1 file changed, 1 insertion(+), 9 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/GenFds.py
> b/BaseTools/Source/Python/GenFds/GenFds.py
> index d97fc28..c2a2bd3 100644
> --- a/BaseTools/Source/Python/GenFds/GenFds.py
> +++ b/BaseTools/Source/Python/GenFds/GenFds.py
> @@ -136,22 +136,14 @@ def main():
>  if not os.path.isabs (ActivePlatform):
>  ActivePlatform = mws.join(GenFdsGlobalVariable.WorkSpaceDir,
> ActivePlatform)
> 
>  if not os.path.exists(ActivePlatform)  :
>  EdkLogger.error("GenFds", FILE_NOT_FOUND, "ActivePlatform
> doesn't exist!")
> -
> -if os.path.normcase (ActivePlatform).find(Workspace) == 0:
> -ActivePlatform = mws.relpath(ActivePlatform, Workspace)
> -if len(ActivePlatform) > 0 :
> -if ActivePlatform[0] == '\\' or ActivePlatform[0] == '/':
> -ActivePlatform = ActivePlatform[1:]
> -else:
> -EdkLogger.error("GenFds", FILE_NOT_FOUND, "ActivePlatform
> doesn't exist!")
>  else:
>  EdkLogger.error("GenFds", OPTION_MISSING, "Missing active
> platform")
> 
> -GenFdsGlobalVariable.ActivePlatform =
> PathClass(NormPath(ActivePlatform), Workspace)
> +GenFdsGlobalVariable.ActivePlatform =
> PathClass(NormPath(ActivePlatform))
> 
>  if (Options.ConfDirectory):
>  # Get alternate Conf location, if it is absolute, then just use 
> the
> absolute directory name
>  ConfDirectoryPath = os.path.normpath(Options.ConfDirectory)
>  if ConfDirectoryPath.startswith('"'):
> --
> 2.6.1.windows.1

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