[edk2] BaseTools: fix PLATFORM_DIR variable value for multiple workspace

[Yonghong Zhu]

when enable the multiple workspace, the PLATFORM_DIR still is
$(WORKSPACE)\AnyPkg, even though it is in a PACKAGES_PATH folder. this
patch fix this issue to use the real path.

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

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index a4844be..287046a 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -231,11 +231,11 @@ ${makefile_header}
 #
 PLATFORM_NAME = ${platform_name}
 PLATFORM_GUID = ${platform_guid}
 PLATFORM_VERSION = ${platform_version}
 PLATFORM_RELATIVE_DIR = ${platform_relative_directory}
-PLATFORM_DIR = $(WORKSPACE)${separator}${platform_relative_directory}
+PLATFORM_DIR = ${platform_dir}
 PLATFORM_OUTPUT_DIR = ${platform_output_directory}
 
 #
 # Module Macro Definition
 #
@@ -610,10 +610,11 @@ cleanlib:
 "platform_name" : self.PlatformInfo.Name,
 "platform_guid" : self.PlatformInfo.Guid,
 "platform_version"  : self.PlatformInfo.Version,
 "platform_relative_directory": self.PlatformInfo.SourceDir,
 "platform_output_directory" : self.PlatformInfo.OutputDir,
+"platform_dir"  : 
self._AutoGenObject.Macros["PLATFORM_DIR"],
 
 "module_name"   : self._AutoGenObject.Name,
 "module_guid"   : self._AutoGenObject.Guid,
 "module_name_guid"  : 
self._AutoGenObject._GetUniqueBaseName(),
 "module_version": self._AutoGenObject.Version,
@@ -987,11 +988,11 @@ ${makefile_header}
 #
 PLATFORM_NAME = ${platform_name}
 PLATFORM_GUID = ${platform_guid}
 PLATFORM_VERSION = ${platform_version}
 PLATFORM_RELATIVE_DIR = ${platform_relative_directory}
-PLATFORM_DIR = $(WORKSPACE)${separator}${platform_relative_directory}
+PLATFORM_DIR = $(platform_dir)
 PLATFORM_OUTPUT_DIR = ${platform_output_directory}
 
 #
 # Module Macro Definition
 #
@@ -1117,10 +1118,11 @@ ${BEGIN}\t-@${create_directory_command}\n${END}\
 "platform_name" : self.PlatformInfo.Name,
 "platform_guid" : self.PlatformInfo.Guid,
 "platform_version"  : self.PlatformInfo.Version,
 "platform_relative_directory": self.PlatformInfo.SourceDir,
 "platform_output_directory" : self.PlatformInfo.OutputDir,
+"platform_dir"  : 
self._AutoGenObject.Macros["PLATFORM_DIR"],
 
 "module_name"   : self._AutoGenObject.Name,
 "module_guid"   : self._AutoGenObject.Guid,
 "module_name_guid"  : 
self._AutoGenObject._GetUniqueBaseName(),
 "module_version": self._AutoGenObject.Version,
@@ -1169,11 +1171,11 @@ ${makefile_header}
 #
 PLATFORM_NAME = ${platform_name}
 PLATFORM_GUID = ${platform_guid}
 PLATFORM_VERSION = ${platform_version}
 PLATFORM_FILE = ${platform_file}
-PLATFORM_DIR = $(WORKSPACE)${separator}${platform_relative_directory}
+PLATFORM_DIR = $(platform_dir)
 PLATFORM_OUTPUT_DIR = ${platform_output_directory}
 
 #
 # Build Configuration Macro Definition
 #
@@ -1311,10 +1313,11 @@ cleanlib:
 "platform_version"  : PlatformInfo.Version,
 "platform_file" : self._AutoGenObject.MetaFile,
 "platform_relative_directory": PlatformInfo.SourceDir,
 "platform_output_directory" : PlatformInfo.OutputDir,
 "platform_build_directory"  : PlatformInfo.BuildDir,
+"platform_dir"  : 
self._AutoGenObject.Macros["PLATFORM_DIR"],
 
 "toolchain_tag" : PlatformInfo.ToolChain,
 "build_target"  : PlatformInfo.BuildTarget,
 "shell_command_code": 
self._SHELL_CMD_[self._FileType].keys(),
 "shell_command" : 
self._SHELL_CMD_[self._FileType].values(),
-- 
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: Fix issue in Ip6Dxe SetData

[Hegde, Nagaraj P]

Reviewed-by: Hegde Nagaraj P 
Tested-by: Hegde Nagaraj P 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Subramanian, Sriram (EG Servers Platform SW)
Sent: Wednesday, April 13, 2016 8:54 AM
To: Jiaxin Wu ; edk2-devel@lists.01.org
Cc: Ye Ting ; Fu Siyuan 
Subject: Re: [edk2] [Patch] NetworkPkg: Fix issue in Ip6Dxe SetData

Thanks Jiaxin for the quick fix.

Code looks fine. Reviewed-by: Sriram Subramanian 

We'll also test it and let you know.

Thanks,
Sriram.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Wednesday, April 13, 2016 8:39 AM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Fu Siyuan 
Subject: [edk2] [Patch] NetworkPkg: Fix issue in Ip6Dxe SetData

EFI_NOT_READY should not be treated as an error status returned from SetData 
for Ip6ConfigDataTypeManualAddress since there is an asynchronous operation for 
DAD process.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Hegde Nagaraj P 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/Ip6Dxe/Ip6Driver.c | 72 +--
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6Driver.c b/NetworkPkg/Ip6Dxe/Ip6Driver.c 
index ba70290..16617c1 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Driver.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Driver.c
@@ -576,11 +576,11 @@ Ip6DriverBindingStart (
Ip6Cfg,
Ip6ConfigDataTypeManualAddress,
DataItem->DataSize,
DataItem->Data.Ptr
);
-if (EFI_ERROR(Status)) {
+if (EFI_ERROR(Status) && Status != EFI_NOT_READY) {
   goto ON_ERROR;
 }
   }
 
   //
@@ -597,50 +597,48 @@ Ip6DriverBindingStart (
 if (EFI_ERROR(Status)) {
   goto ON_ERROR;
 }
   }
 
-  if (!EFI_ERROR (Status)) {
-//
-// ready to go: start the receiving and timer
-//
-Status = Ip6ReceiveFrame (Ip6AcceptFrame, IpSb);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // ready to go: start the receiving and timer  //  Status = 
+ Ip6ReceiveFrame (Ip6AcceptFrame, IpSb);  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// The timer expires every 100 (IP6_TIMER_INTERVAL_IN_MS) milliseconds.
-//
-Status = gBS->SetTimer (
-IpSb->FasterTimer,
-TimerPeriodic,
-TICKS_PER_MS * IP6_TIMER_INTERVAL_IN_MS
-);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // The timer expires every 100 (IP6_TIMER_INTERVAL_IN_MS) milliseconds.
+  //
+  Status = gBS->SetTimer (
+  IpSb->FasterTimer,
+  TimerPeriodic,
+  TICKS_PER_MS * IP6_TIMER_INTERVAL_IN_MS
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// The timer expires every 1000 (IP6_ONE_SECOND_IN_MS) milliseconds.
-//
-Status = gBS->SetTimer (
-IpSb->Timer,
-TimerPeriodic,
-TICKS_PER_MS * IP6_ONE_SECOND_IN_MS
-);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // The timer expires every 1000 (IP6_ONE_SECOND_IN_MS) milliseconds.
+  //
+  Status = gBS->SetTimer (
+  IpSb->Timer,
+  TimerPeriodic,
+  TICKS_PER_MS * IP6_ONE_SECOND_IN_MS
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// Initialize the IP6 ID
-//
-mIp6Id = NET_RANDOM (NetRandomInitSeed ());
+  //
+  // Initialize the IP6 ID
+  //
+  mIp6Id = NET_RANDOM (NetRandomInitSeed ());
 
-return EFI_SUCCESS;
-  }
+  return EFI_SUCCESS;
 
 ON_ERROR:
   Ip6CleanService (IpSb);
   FreePool (IpSb);
   return Status;
--
1.9.5.msysgit.1

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

Re: [edk2] [Patch] NetworkPkg: Fix issue in Ip6Dxe SetData

[Subramanian, Sriram (EG Servers Platform SW)]

Thanks Jiaxin for the quick fix.

Code looks fine. Reviewed-by: Sriram Subramanian 

We'll also test it and let you know.

Thanks,
Sriram.

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiaxin Wu
Sent: Wednesday, April 13, 2016 8:39 AM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Fu Siyuan 
Subject: [edk2] [Patch] NetworkPkg: Fix issue in Ip6Dxe SetData

EFI_NOT_READY should not be treated as an error status
returned from SetData for Ip6ConfigDataTypeManualAddress
since there is an asynchronous operation for DAD process.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Hegde Nagaraj P 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/Ip6Dxe/Ip6Driver.c | 72 +--
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6Driver.c b/NetworkPkg/Ip6Dxe/Ip6Driver.c
index ba70290..16617c1 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Driver.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Driver.c
@@ -576,11 +576,11 @@ Ip6DriverBindingStart (
Ip6Cfg,
Ip6ConfigDataTypeManualAddress,
DataItem->DataSize,
DataItem->Data.Ptr
);
-if (EFI_ERROR(Status)) {
+if (EFI_ERROR(Status) && Status != EFI_NOT_READY) {
   goto ON_ERROR;
 }
   }
 
   //
@@ -597,50 +597,48 @@ Ip6DriverBindingStart (
 if (EFI_ERROR(Status)) {
   goto ON_ERROR;
 }
   }
 
-  if (!EFI_ERROR (Status)) {
-//
-// ready to go: start the receiving and timer
-//
-Status = Ip6ReceiveFrame (Ip6AcceptFrame, IpSb);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // ready to go: start the receiving and timer
+  //
+  Status = Ip6ReceiveFrame (Ip6AcceptFrame, IpSb);
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// The timer expires every 100 (IP6_TIMER_INTERVAL_IN_MS) milliseconds.
-//
-Status = gBS->SetTimer (
-IpSb->FasterTimer,
-TimerPeriodic,
-TICKS_PER_MS * IP6_TIMER_INTERVAL_IN_MS
-);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // The timer expires every 100 (IP6_TIMER_INTERVAL_IN_MS) milliseconds.
+  //
+  Status = gBS->SetTimer (
+  IpSb->FasterTimer,
+  TimerPeriodic,
+  TICKS_PER_MS * IP6_TIMER_INTERVAL_IN_MS
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// The timer expires every 1000 (IP6_ONE_SECOND_IN_MS) milliseconds.
-//
-Status = gBS->SetTimer (
-IpSb->Timer,
-TimerPeriodic,
-TICKS_PER_MS * IP6_ONE_SECOND_IN_MS
-);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // The timer expires every 1000 (IP6_ONE_SECOND_IN_MS) milliseconds.
+  //
+  Status = gBS->SetTimer (
+  IpSb->Timer,
+  TimerPeriodic,
+  TICKS_PER_MS * IP6_ONE_SECOND_IN_MS
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// Initialize the IP6 ID
-//
-mIp6Id = NET_RANDOM (NetRandomInitSeed ());
+  //
+  // Initialize the IP6 ID
+  //
+  mIp6Id = NET_RANDOM (NetRandomInitSeed ());
 
-return EFI_SUCCESS;
-  }
+  return EFI_SUCCESS;
 
 ON_ERROR:
   Ip6CleanService (IpSb);
   FreePool (IpSb);
   return Status;
-- 
1.9.5.msysgit.1

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

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

[Jiaxin Wu]

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_ITEMPingParamList[] = {
   {
 L"-n",
 TypeValue
   },
   {
+L"-s",
+TypeValue
+  },
+  {
 L"-_s",
 TypeValue
   },
   {
 L"-_ip6",
@@ -1510,11 +1514,15 @@ ShellCommandRunPing (
   ZeroMem (, sizeof (EFI_IPv6_ADDRESS));
 
   //
   // Parse the paramter of source ip address.
   //
-  ValueStr = ShellCommandLineGetValue (ParamPackage, L"-_s");
+  ValueStr = ShellCommandLineGetValue (ParamPackage, L"-s");
+  if (ValueStr == NULL) {
+ValueStr = ShellCommandLineGetValue (ParamPackage, L"-_s");
+  }
+  
   if (ValueStr != NULL) {
 mSrcString = ValueStr;
 if (IpChoice == PING_IP_CHOICE_IP6) {
   Status = NetLibStrToIp6 (ValueStr, );
 } else {
diff --git 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
index bc6acac..7d6f2da 100644
--- 
a/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
+++ 
b/ShellPkg/Library/UefiShellNetwork1CommandsLib/UefiShellNetwork1CommandsLib.uni
@@ -1,9 +1,9 @@
 // /**
 //
 // (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-// Copyright (c) 2010 - 2015, 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
 // http://opensource.org/licenses/bsd-license.php
 //
@@ -86,28 +86,27 @@
 #string STR_IFCONFIG_INFO_GATEWAY_HEAD#language en-US"\n%Hdefault 
gateway: %N"
 #string STR_IFCONFIG_INFO_DNS_ADDR_HEAD   #language en-US"\n%HDNS 
server   : %N\n"
 #string STR_IFCONFIG_INFO_IP_ADDR_BODY#language en-US
"%d.%d.%d.%d\n"
 
 #string STR_GET_HELP_PING #language en-US ""
-".TH ping 0 "Pings the target host with an IPv4 or IPv6 stack."\r\n"
+".TH ping 0 "Ping the target host with an IPv4 stack."\r\n"
 ".SH NAME\r\n"
-"Pings the target host with an IPv4 or IPv6 stack.\r\n"
+"Ping the target host with an IPv4 stack.\r\n"
 ".SH SYNOPSIS\r\n"
 " \r\n"
-"PING [-_ip6] [-_s SourceIp] [-n count] [-l size] TargetIp\r\n"
+"PING [-n count] [-l size] [-s SourceIp] TargetIp\r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
 "  -n   - Specifies the number of echo request datagrams to be sent.\r\n"
 "  -l   - Specifies the size of the data buffer in the echo request 
datagram.\r\n"
-"  -_ip6- Specifies the IPv6 stack usage mode (Default is IPv4 stack).\r\n"
-"  -_s  - Specifies the source adapter as IPv4 or IPv6 address.\r\n"
-"  SourceIp - Specifies the IPv4 or IPv6 address of the source machine.\r\n"
-"  TargetIp - Specifies the IPv4 or IPv6 address of the target machine.\r\n"
+"  -s   - Specifies the source adapter as IPv4 address.\r\n"
+"  SourceIp - Specifies the IPv4 address of the source machine.\r\n"
+"  TargetIp - Specifies the IPv4 address of the target machine.\r\n"
 ".SH DESCRIPTION\r\n"
 " \r\n"
 "NOTES:\r\n"
-"  1. This command uses the ICMPv4 or ICMPv6 ECHO_REQUEST datagram to elicit 
an\r\n"
+"  1. This command uses the ICMPv4 ECHO_REQUEST datagram to elicit an\r\n"
 " ECHO_REPLY from a host.\r\n"
 ".SH EXAMPLES\r\n"
 " \r\n"
 "EXAMPLES:\r\n"
 "  * To ping the target host with 64 bytes data:\r\n"
@@ -115,14 +114,11 @@
 " \r\n"
 "  * To ping the 

[edk2] [Patch] NetworkPkg: Fix issue in Ip6Dxe SetData

[Jiaxin Wu]

EFI_NOT_READY should not be treated as an error status
returned from SetData for Ip6ConfigDataTypeManualAddress
since there is an asynchronous operation for DAD process.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Hegde Nagaraj P 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/Ip6Dxe/Ip6Driver.c | 72 +--
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/NetworkPkg/Ip6Dxe/Ip6Driver.c b/NetworkPkg/Ip6Dxe/Ip6Driver.c
index ba70290..16617c1 100644
--- a/NetworkPkg/Ip6Dxe/Ip6Driver.c
+++ b/NetworkPkg/Ip6Dxe/Ip6Driver.c
@@ -576,11 +576,11 @@ Ip6DriverBindingStart (
Ip6Cfg,
Ip6ConfigDataTypeManualAddress,
DataItem->DataSize,
DataItem->Data.Ptr
);
-if (EFI_ERROR(Status)) {
+if (EFI_ERROR(Status) && Status != EFI_NOT_READY) {
   goto ON_ERROR;
 }
   }
 
   //
@@ -597,50 +597,48 @@ Ip6DriverBindingStart (
 if (EFI_ERROR(Status)) {
   goto ON_ERROR;
 }
   }
 
-  if (!EFI_ERROR (Status)) {
-//
-// ready to go: start the receiving and timer
-//
-Status = Ip6ReceiveFrame (Ip6AcceptFrame, IpSb);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // ready to go: start the receiving and timer
+  //
+  Status = Ip6ReceiveFrame (Ip6AcceptFrame, IpSb);
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// The timer expires every 100 (IP6_TIMER_INTERVAL_IN_MS) milliseconds.
-//
-Status = gBS->SetTimer (
-IpSb->FasterTimer,
-TimerPeriodic,
-TICKS_PER_MS * IP6_TIMER_INTERVAL_IN_MS
-);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // The timer expires every 100 (IP6_TIMER_INTERVAL_IN_MS) milliseconds.
+  //
+  Status = gBS->SetTimer (
+  IpSb->FasterTimer,
+  TimerPeriodic,
+  TICKS_PER_MS * IP6_TIMER_INTERVAL_IN_MS
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// The timer expires every 1000 (IP6_ONE_SECOND_IN_MS) milliseconds.
-//
-Status = gBS->SetTimer (
-IpSb->Timer,
-TimerPeriodic,
-TICKS_PER_MS * IP6_ONE_SECOND_IN_MS
-);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
-}
+  //
+  // The timer expires every 1000 (IP6_ONE_SECOND_IN_MS) milliseconds.
+  //
+  Status = gBS->SetTimer (
+  IpSb->Timer,
+  TimerPeriodic,
+  TICKS_PER_MS * IP6_ONE_SECOND_IN_MS
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
 
-//
-// Initialize the IP6 ID
-//
-mIp6Id = NET_RANDOM (NetRandomInitSeed ());
+  //
+  // Initialize the IP6 ID
+  //
+  mIp6Id = NET_RANDOM (NetRandomInitSeed ());
 
-return EFI_SUCCESS;
-  }
+  return EFI_SUCCESS;
 
 ON_ERROR:
   Ip6CleanService (IpSb);
   FreePool (IpSb);
   return Status;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch 3/3] MdeModulePkg: Update Guid/Protocol usages in INF files.

[Zeng, Star]

Liming,

Some comments below.

On 2016/4/7 14:54, Liming Gao wrote:

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
  .../Library/VarCheckUefiLib/VarCheckUefiLib.inf| 78 +++---
  .../Variable/RuntimeDxe/VariableRuntimeDxe.inf | 10 +--
  2 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf 
b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
index 77ef210..128c44d 100644
--- a/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+++ b/MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
@@ -45,44 +45,44 @@
VarCheckLib

  [Guids]
-  ## CONSUMES   ## Variable:L"LangCodes"
-  ## CONSUMES   ## Variable:L"Lang"
-  ## CONSUMES   ## Variable:L"Timeout"
-  ## CONSUMES   ## Variable:L"PlatformLangCodes"
-  ## CONSUMES   ## Variable:L"PlatformLang"
-  ## CONSUMES   ## Variable:L"ConIn"
-  ## CONSUMES   ## Variable:L"ConOut"
-  ## CONSUMES   ## Variable:L"ErrOut"
-  ## CONSUMES   ## Variable:L"ConInDev"
-  ## CONSUMES   ## Variable:L"ConOutDev"
-  ## CONSUMES   ## Variable:L"ErrOutDev"
-  ## CONSUMES   ## Variable:L"BootOrder"
-  ## CONSUMES   ## Variable:L"BootNext"
-  ## CONSUMES   ## Variable:L"BootCurrent"
-  ## CONSUMES   ## Variable:L"BootOptionSupport"
-  ## CONSUMES   ## Variable:L"DriverOrder"
-  ## CONSUMES   ## Variable:L"SysPrepOrder"
-  ## CONSUMES   ## Variable:L"HwErrRecSupport"
-  ## CONSUMES   ## Variable:L"SetupMode"
-  ## CONSUMES   ## Variable:L"PK"
-  ## CONSUMES   ## Variable:L"KEK"
-  ## CONSUMES   ## Variable:L"SignatureSupport"
-  ## CONSUMES   ## Variable:L"SecureBoot"
-  ## CONSUMES   ## Variable:L"KEKDefault"
-  ## CONSUMES   ## Variable:L"PKDefault"
-  ## CONSUMES   ## Variable:L"dbDefault"
-  ## CONSUMES   ## Variable:L"dbxDefault"
-  ## CONSUMES   ## Variable:L"dbtDefault"
-  ## CONSUMES   ## Variable:L"OsIndicationsSupported"
-  ## CONSUMES   ## Variable:L"OsIndications"
-  ## CONSUMES   ## Variable:L"VendorKeys"
-  ## CONSUMES   ## Variable:L"Boot"
-  ## CONSUMES   ## Variable:L"Driver"
-  ## CONSUMES   ## Variable:L"SysPrep"
-  ## CONSUMES   ## Variable:L"Key"
+  ## SOMETIMES_CONSUMES   ## Variable:L"LangCodes"
+  ## SOMETIMES_CONSUMES   ## Variable:L"Lang"
+  ## SOMETIMES_CONSUMES   ## Variable:L"Timeout"
+  ## SOMETIMES_CONSUMES   ## Variable:L"PlatformLangCodes"
+  ## SOMETIMES_CONSUMES   ## Variable:L"PlatformLang"
+  ## SOMETIMES_CONSUMES   ## Variable:L"ConIn"
+  ## SOMETIMES_CONSUMES   ## Variable:L"ConOut"
+  ## SOMETIMES_CONSUMES   ## Variable:L"ErrOut"
+  ## SOMETIMES_CONSUMES   ## Variable:L"ConInDev"
+  ## SOMETIMES_CONSUMES   ## Variable:L"ConOutDev"
+  ## SOMETIMES_CONSUMES   ## Variable:L"ErrOutDev"
+  ## SOMETIMES_CONSUMES   ## Variable:L"BootOrder"
+  ## SOMETIMES_CONSUMES   ## Variable:L"BootNext"
+  ## SOMETIMES_CONSUMES   ## Variable:L"BootCurrent"
+  ## SOMETIMES_CONSUMES   ## Variable:L"BootOptionSupport"
+  ## SOMETIMES_CONSUMES   ## Variable:L"DriverOrder"
+  ## SOMETIMES_CONSUMES   ## Variable:L"SysPrepOrder"
+  ## SOMETIMES_CONSUMES   ## Variable:L"HwErrRecSupport"
+  ## SOMETIMES_CONSUMES   ## Variable:L"SetupMode"
+  ## SOMETIMES_CONSUMES   ## Variable:L"PK"
+  ## SOMETIMES_CONSUMES   ## Variable:L"KEK"
+  ## SOMETIMES_CONSUMES   ## Variable:L"SignatureSupport"
+  ## SOMETIMES_CONSUMES   ## Variable:L"SecureBoot"
+  ## SOMETIMES_CONSUMES   ## Variable:L"KEKDefault"
+  ## SOMETIMES_CONSUMES   ## Variable:L"PKDefault"
+  ## SOMETIMES_CONSUMES   ## Variable:L"dbDefault"
+  ## SOMETIMES_CONSUMES   ## Variable:L"dbxDefault"
+  ## SOMETIMES_CONSUMES   ## Variable:L"dbtDefault"
+  ## SOMETIMES_CONSUMES   ## Variable:L"OsIndicationsSupported"
+  ## SOMETIMES_CONSUMES   ## Variable:L"OsIndications"
+  ## SOMETIMES_CONSUMES   ## Variable:L"VendorKeys"
+  ## SOMETIMES_CONSUMES   ## Variable:L"Boot"
+  ## SOMETIMES_CONSUMES   ## Variable:L"Driver"
+  ## SOMETIMES_CONSUMES   ## Variable:L"SysPrep"
+  ## SOMETIMES_CONSUMES   ## Variable:L"Key"
gEfiGlobalVariableGuid
-  ## CONSUMES   ## Variable:L"DB"
-  ## CONSUMES   ## Variable:L"DBX"
-  ## CONSUMES   ## Variable:L"DBT"
+  ## SOMETIMES_CONSUMES   ## Variable:L"DB"
+  ## SOMETIMES_CONSUMES   ## Variable:L"DBX"
+  ## SOMETIMES_CONSUMES   ## Variable:L"DBT"
gEfiImageSecurityDatabaseGuid
-  gEfiHardwareErrorVariableGuid  ## CONSUMES## Variable:L"HwErrRec"
+  gEfiHardwareErrorVariableGuid  ## SOMETIMES_CONSUMES## 
Variable:L"HwErrRec"
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
index 5e7b5c5..1704b5d 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
@@ -78,8 +78,8 @@
gEdkiiVarCheckProtocolGuid## PRODUCES

  [Guids]
-  ## PRODUCES 

[edk2] [patch 1/3] MdeModulePkg/Sd: Fix wrong response type of SD Deselect cmd

[Feng Tian]

The SD CMD7 deselect cmd have no response according to SD
physical layer simplified spec.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 6 --
 MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c | 6 --
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 07bd07a..d05eb9e 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1,7 +1,7 @@
 /** @file
   This file provides some helper functions which are specific for SD card 
device.
 
-  Copyright (c) 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 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
@@ -464,7 +464,9 @@ SdCardSelect (
 
   SdMmcCmdBlk.CommandIndex = SD_SELECT_DESELECT_CARD;
   SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
-  SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
+  if (Rca != 0) {
+SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
+  }
   SdMmcCmdBlk.CommandArgument = (UINT32)Rca << 16;
 
   Status = SdMmcPassThruPassThru (PassThru, Slot, , NULL);
diff --git a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c 
b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
index 341d788..b7a5fe4 100644
--- a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
@@ -1,7 +1,7 @@
 /** @file
   The helper functions for BlockIo and BlockIo2 protocol.
 
-  Copyright (c) 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 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
@@ -133,7 +133,9 @@ SdSelect (
 
   SdMmcCmdBlk.CommandIndex = SD_SELECT_DESELECT_CARD;
   SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
-  SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
+  if (Rca != 0) {
+SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1b;
+  }
   SdMmcCmdBlk.CommandArgument = (UINT32)Rca << 16;
 
   Status = PassThru->PassThru (PassThru, Device->Slot, , NULL);
-- 
2.7.1.windows.2

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

[edk2] [patch 2/3] MdeModulePkg/SdMmcPciHc: Reset the slot when sd device is connected

[Feng Tian]

The original code doesn't reset the slot when there is device change.
It may bring issue on device identification procedure of some SD cards.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index a841f47..053a80f 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -275,6 +275,13 @@ SdMmcPciHcEnumerateDevice (
   if ((Status == EFI_MEDIA_CHANGED) && (MediaPresent == TRUE)) {
 DEBUG ((EFI_D_INFO, "SdMmcPciHcEnumerateDevice: device connected at 
slot %d of pci %p\n", Slot, Private->PciIo));
 //
+// Reset the specified slot of the SD/MMC Pci Host Controller
+//
+Status = SdMmcHcReset (Private->PciIo, Slot);
+if (EFI_ERROR (Status)) {
+  continue;
+}
+//
 // Reinitialize slot and restart identification process for the new 
attached device
 //
 Status = SdMmcHcInitHost (Private->PciIo, Slot, 
Private->Capability[Slot]);
-- 
2.7.1.windows.2

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

[edk2] [patch 3/3] MdeModulePkg/Sd: wait 1ms before check DATA line in voltage switch proc

[Feng Tian]

According to SD Host Controller 3.0 spec figure 3-10, we have to wait
1ms before checking DAT[3:0] in voltage switch proc

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 2 +-
 MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index d05eb9e..b2a0857 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1123,7 +1123,7 @@ SdCardIdentification (
 
   SdMmcHcInitClockFreq (PciIo, Slot, Private->Capability[Slot]);
 
-  gBS->Stall (1);
+  gBS->Stall (1000);
 
   SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_PRESENT_STATE, TRUE, sizeof 
(PresentState), );
   if (((PresentState >> 20) & 0xF) != 0xF) {
diff --git a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c 
b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
index 8f7ecf9..cbee947 100644
--- a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
+++ b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
@@ -2832,7 +2832,7 @@ SdPeimIdentification (
 
   SdPeimHcInitClockFreq (Slot->SdHcBase);
 
-  MicroSecondDelay (1);
+  MicroSecondDelay (1000);
 
   SdPeimHcRwMmio (Slot->SdHcBase + SD_HC_PRESENT_STATE, TRUE, sizeof 
(PresentState), );
   if (((PresentState >> 20) & 0xF) != 0xF) {
-- 
2.7.1.windows.2

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

[edk2] [patch 0/3] Fix some bugs found in EDKII SD/MMC stack

[Feng Tian]

this series is used to fix some bugs found with some sd cards

Feng Tian (3):
  MdeModulePkg/Sd: Fix wrong response type of SD Deselect cmd
  MdeModulePkg/SdMmcPciHc: Reset the slot when sd device is connected
  MdeModulePkg/Sd: wait 1ms before check DATA line in voltage switch
proc

 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  | 8 +---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 7 +++
 MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c   | 2 +-
 MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c  | 6 --
 4 files changed, 17 insertions(+), 6 deletions(-)

-- 
2.7.1.windows.2

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

Re: [edk2] HII incompatibility between edk2 and iPXE?

[Dong, Eric]

> On 05/04/16 03:10, Dong, Eric wrote:
> >> On a separate but related note: The ConfigHdr portion of Request and
> >> Response seems to contain absolutely zero information by the time it
> >> reaches EFI_HII_CONFIG_ACCESS_PROTOCOL.  As far as I can tell, it is
> >> meaningful only to EFI_HII_CONFIG_ROUTING_PROTOCOL and a sensible API
> >> design would never have exposed it to EFI_HII_CONFIG_ACCESS_PROTOCOL
> >> instances.
> >>
> >> Does the ConfigHdr have any meaning for EFI_HII_CONFIG_ACCESS_PROTOCOL,
> >> or is it just a pointless blob of noise?
> >
> > The GUID/NAME value in the ConfigHdr is get from the storage used in this 
> > driver. If one driver has more than one storage, the Guid +
> Name specify which storage info will be return.
> 
> Surely the EFI_HII_CONFIG_ACCESS_PROTOCOL's *This pointer already
> uniquely identifies the storage.
> 
> (If not, then why not?)

The Config Access Protocol supports more than one storages in this protocol at 
one time.  Just like examples in DriverSampleDxe, it has three storages in this 
driver:
  //
  // Define a Buffer Storage (EFI_IFR_VARSTORE)
  //
  varstore DRIVER_SAMPLE_CONFIGURATION, // This is the data structure type
varid = CONFIGURATION_VARSTORE_ID,  // Optional VarStore ID
name  = MyIfrNVData,// Define referenced name in vfr
guid  = DRIVER_SAMPLE_FORMSET_GUID; // GUID of this buffer storage

  //
  // Define a EFI variable Storage (EFI_IFR_VARSTORE_EFI)
  //
  efivarstore MY_EFI_VARSTORE_DATA,
attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_NON_VOLATILE,  
// EFI variable attribures  
name  = MyEfiVar,
guid  = DRIVER_SAMPLE_FORMSET_GUID;

  //
  // Define a Name/Value Storage (EFI_IFR_VARSTORE_NAME_VALUE)
  //
  namevaluevarstore MyNameValueVar,// Define storage 
reference name in vfr
name = STRING_TOKEN(STR_NAME_VALUE_VAR_NAME0), // Define Name list of this 
storage, refer it by MyNameValueVar[0]
name = STRING_TOKEN(STR_NAME_VALUE_VAR_NAME1), // Define Name list of this 
storage, refer it by MyNameValueVar[1]
name = STRING_TOKEN(STR_NAME_VALUE_VAR_NAME2), // Define Name list of this 
storage, refer it by MyNameValueVar[2]
guid = DRIVER_SAMPLE_FORMSET_GUID; // GUID of this Name/Value 
storage


The ConfigHdr string includes the Name + Guid + DevicePath info.  The name +  
guid is got from the storage definition. So in EFI_HII_CONFIG_ACCESS_PROTOCOL's 
ExtractCofig function, it base on the input Name + guid info to know which 
storage data need to be return. It's not meaningless. 

Because we almost add all examples related to HII to DriverSampleDxe, this 
driver may seems a little big. But I think these codes are valuable for the 
newbie of Hii if he can spend some time to learn it.

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

Re: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

[Wu, Jiaxin]

That's fine, but whether commit to the newly proposed branches in github or 
not, I'm not sure since there is no final conclusion for that currently.

Thanks your feedback again.

From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, April 13, 2016 10:16 AM
To: edk2-devel@lists.01.org; Wu, Jiaxin 
Cc: Ye, Ting ; Fu, Siyuan ; Long, Qin 

Subject: RE: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

HPE verified both https boot (with dhcp handshake) and https download from a 
url to work, but with a hard coded certificate (not using the variable yet).

I think PEER cert validation is working, but not the timestamp check. We are. 
looking at that tomorrow.

There might be a few changes needed on top of your patches. I will take a look 
and try to generate a patch and send it for review.

This would have been a great feature to co-develop in the newly proposed 
branches in github...



-Original Message-
From: Wu, Jiaxin [jiaxin...@intel.com]
Received: Tuesday, 12 Apr 2016, 9:06PM
To: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]; 
edk2-devel@lists.01.org 
[edk2-devel@lists.01.org]
CC: Ye, Ting [ting...@intel.com]; Fu, Siyuan [siyuan...@intel.com]; Long, Qin 
[qin.l...@intel.com]
Subject: RE: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6
Thanks Samer. I'm collecting the test result for this new feature. So, I want 
to know whether the patch pass your testing environment?

For commit process, it's depending on the test result before commit to EDKII 
trunk directly since it is the new feature. Once we draw the finally 
conclusion, I will inform you first time.

Thanks.
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of El-
> Haj-Mahmoud, Samer
> Sent: Wednesday, April 13, 2016 1:37 AM
> To: Wu, Jiaxin >; 
> edk2-devel@lists.01.org
> Cc: Ye, Ting >; Fu, Siyuan 
> >; Long,
> Qin >
> Subject: Re: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4
> and IPv6
>
>
> Looks good. Are you planning on committing this code? I have a few updates
> I would like to send for review, but they need the initial patches to be
> committed first.
>
> Series reviewed-by: Samer El-Haj-Mahmoud >
>
>
> -Original Message-
> From: Jiaxin Wu [mailto:jiaxin...@intel.com]
> Sent: Monday, April 11, 2016 2:50 AM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting >; Fu Siyuan 
> >; Long Qin
> >; El-Haj-Mahmoud, Samer 
>  mahm...@hpe.com>
> Subject: [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6
>
> v2:
> To support the multiple certificate configuration,
> EFI_SIGNATURE_LIST format is used for the variable
> 'TlsCaCertificate'.
>
> This patch is used to enable HTTPS feature. HttpDxe driver
> will consume TlsDxe driver. It can both support
> http and https feature, it's depended on the information in URL,
> the HTTP instance can be able to determine whether to use http
> or https.
>
> Cc: Ye Ting >
> Cc: Fu Siyuan >
> Cc: Long Qin >
> Cc: El-Haj-Mahmoud Samer 
> >
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu >
> ---
>  NetworkPkg/HttpDxe/HttpDriver.h   |8 +-
>  NetworkPkg/HttpDxe/HttpDxe.inf|8 +-
>  NetworkPkg/HttpDxe/HttpImpl.c |  188 +++-
>  NetworkPkg/HttpDxe/HttpProto.c|  395 ++---
>  NetworkPkg/HttpDxe/HttpProto.h|   65 +-
>  NetworkPkg/HttpDxe/HttpsSupport.c | 1701
> +
>  NetworkPkg/HttpDxe/HttpsSupport.h |  314 +++
>  7 files changed, 2542 insertions(+), 137 deletions(-)
>  create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.c
>  create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.h
>
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.h
> b/NetworkPkg/HttpDxe/HttpDriver.h
> index 9c0002a..3c30c12 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.h
> +++ b/NetworkPkg/HttpDxe/HttpDriver.h
> @@ -1,9 +1,9 @@
>  /** @file
>The header files of the driver binding and service binding protocol for
> HttpDxe driver.
>
> -  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>(C) Copyright 2016 Hewlett Packard 

Re: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

[El-Haj-Mahmoud, Samer]

HPE verified both https boot (with dhcp handshake) and https download from a 
url to work, but with a hard coded certificate (not using the variable yet).

I think PEER cert validation is working, but not the timestamp check. We are. 
looking at that tomorrow.

There might be a few changes needed on top of your patches. I will take a look 
and try to generate a patch and send it for review.

This would have been a great feature to co-develop in the newly proposed 
branches in github...



-Original Message-
From: Wu, Jiaxin [jiaxin...@intel.com]
Received: Tuesday, 12 Apr 2016, 9:06PM
To: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]; 
edk2-devel@lists.01.org [edk2-devel@lists.01.org]
CC: Ye, Ting [ting...@intel.com]; Fu, Siyuan [siyuan...@intel.com]; Long, Qin 
[qin.l...@intel.com]
Subject: RE: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

Thanks Samer. I'm collecting the test result for this new feature. So, I want 
to know whether the patch pass your testing environment?

For commit process, it's depending on the test result before commit to EDKII 
trunk directly since it is the new feature. Once we draw the finally 
conclusion, I will inform you first time.

Thanks.
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of El-
> Haj-Mahmoud, Samer
> Sent: Wednesday, April 13, 2016 1:37 AM
> To: Wu, Jiaxin ; edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Long,
> Qin 
> Subject: Re: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4
> and IPv6
>
>
> Looks good. Are you planning on committing this code? I have a few updates
> I would like to send for review, but they need the initial patches to be
> committed first.
>
> Series reviewed-by: Samer El-Haj-Mahmoud 
>
>
> -Original Message-
> From: Jiaxin Wu [mailto:jiaxin...@intel.com]
> Sent: Monday, April 11, 2016 2:50 AM
> To: edk2-devel@lists.01.org
> Cc: Ye Ting ; Fu Siyuan ; Long Qin
> ; El-Haj-Mahmoud, Samer  mahm...@hpe.com>
> Subject: [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6
>
> v2:
> To support the multiple certificate configuration,
> EFI_SIGNATURE_LIST format is used for the variable
> 'TlsCaCertificate'.
>
> This patch is used to enable HTTPS feature. HttpDxe driver
> will consume TlsDxe driver. It can both support
> http and https feature, it’s depended on the information in URL,
> the HTTP instance can be able to determine whether to use http
> or https.
>
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Long Qin 
> Cc: El-Haj-Mahmoud Samer 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/HttpDxe/HttpDriver.h   |8 +-
>  NetworkPkg/HttpDxe/HttpDxe.inf|8 +-
>  NetworkPkg/HttpDxe/HttpImpl.c |  188 +++-
>  NetworkPkg/HttpDxe/HttpProto.c|  395 ++---
>  NetworkPkg/HttpDxe/HttpProto.h|   65 +-
>  NetworkPkg/HttpDxe/HttpsSupport.c | 1701
> +
>  NetworkPkg/HttpDxe/HttpsSupport.h |  314 +++
>  7 files changed, 2542 insertions(+), 137 deletions(-)
>  create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.c
>  create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.h
>
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.h
> b/NetworkPkg/HttpDxe/HttpDriver.h
> index 9c0002a..3c30c12 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.h
> +++ b/NetworkPkg/HttpDxe/HttpDriver.h
> @@ -1,9 +1,9 @@
>  /** @file
>The header files of the driver binding and service binding protocol for
> HttpDxe driver.
>
> -  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
>(C) Copyright 2016 Hewlett Packard Enterprise Development LP
>
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -22,10 +22,11 @@
>
>  //
>  // Libraries
>  //
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> @@ -48,12 +49,14 @@
>  #include 
>  #include 
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
>
> -
> +#include 
>  //
>  // Produced Protocols
>  //
>  #include 
>
> @@ -77,10 +80,11 @@ extern EFI_HTTP_UTILITIES_PROTOCOL
> *mHttpUtilities;
>  // Include files with function prototypes
>  //
>  #include "ComponentName.h"
>  #include "HttpImpl.h"
>  #include "HttpProto.h"
> +#include "HttpsSupport.h"
>  #include "HttpDns.h"
>
>  typedef struct {
>EFI_SERVICE_BINDING_PROTOCOL  *ServiceBinding;
>UINTN NumberOfChildren;
> diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf
> 

Re: [edk2] [Patch 5/6] Vlv2TbltDevicePkg: Reference the PCD defined in MdeModulePkg

[Wei, David]

It is good for me.

Reviewed-by: zwei4  

Thanks,
David  

Intel SSG BIOS

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, April 11, 2016 3:47 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Wei, David 
Subject: [Patch 5/6] Vlv2TbltDevicePkg: Reference the PCD defined in 
MdeModulePkg

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: David Wei 
---
 Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc | 4 ++--
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc   | 4 ++--
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc| 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
index c78ff87..d8e48c0 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgGccX64.dsc
@@ -589,10 +589,10 @@
 
 
   ## This PCD specifies whether PS2 keyboard does a extended verification 
during start.
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
 
   ## This PCD specifies whether PS2 mouse does a extended verification during 
start.
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
 
 !if $(VARIABLE_INFO_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index dd5f383..0eae7b5 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -589,10 +589,10 @@
 
 
   ## This PCD specifies whether PS2 keyboard does a extended verification 
during start.
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
 
   ## This PCD specifies whether PS2 mouse does a extended verification during 
start.
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
 
 !if $(VARIABLE_INFO_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 7b04efa..4205201 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -589,10 +589,10 @@
 
 
   ## This PCD specifies whether PS2 keyboard does a extended verification 
during start.
-  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2KbdExtendedVerification|FALSE
 
   ## This PCD specifies whether PS2 mouse does a extended verification during 
start.
-  
gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE
 
 !if $(VARIABLE_INFO_ENABLE) == TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE
-- 
2.7.0.windows.1

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

[edk2] [PATCH v2] ShellPkg : Cache the environment variable into memory to enhance the performance.

[Qiu Shumin]

Currently UEFI Shell reads variable storage to get the environment
variables every time running a new command. And reading(writing)
UEFI variables is a high cost operation on most platforms. In order
to enhance the performance this patch read the variable storage once
and cache the environment variables in memory. Every further 'set'
command will save the variable not only to Shell cache, but also the
flash variable storage.

Difference with previous patch:
1. Refine the comments.
2. In function 'ShellAddEnvVarToList' assign the value for 'Node->Atts'
   when adding a new node in gShellEnvVarList.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
Reviewed-by: Jaben Carsey 
---
 ShellPkg/Application/Shell/Shell.c |   4 +
 ShellPkg/Application/Shell/ShellEnvVar.c   | 180 -
 ShellPkg/Application/Shell/ShellEnvVar.h   |  82 -
 ShellPkg/Application/Shell/ShellProtocol.c | 101 +---
 4 files changed, 322 insertions(+), 45 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index bd695a4..12daff9 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -445,6 +445,8 @@ UefiMain (
 Status = CommandInit();
 ASSERT_EFI_ERROR(Status);
 
+Status = ShellInitEnvVarList ();
+
 //
 // Check the command line
 //
@@ -702,6 +704,8 @@ FreeResources:
 DEBUG_CODE(ShellInfoObject.ConsoleInfo = NULL;);
   }
 
+  ShellFreeEnvVarList ();
+
   if (ShellCommandGetExit()) {
 return ((EFI_STATUS)ShellCommandGetExitCode());
   }
diff --git a/ShellPkg/Application/Shell/ShellEnvVar.c 
b/ShellPkg/Application/Shell/ShellEnvVar.c
index a8f177e..5eb382a 100644
--- a/ShellPkg/Application/Shell/ShellEnvVar.c
+++ b/ShellPkg/Application/Shell/ShellEnvVar.c
@@ -1,7 +1,7 @@
 /** @file
   function declarations for shell environment functions.
 
-  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
@@ -17,6 +17,11 @@
 #define INIT_NAME_BUFFER_SIZE  128
 #define INIT_DATA_BUFFER_SIZE  1024
 
+//
+// The list is used to cache the environment variables.
+//
+ENV_VAR_LIST   gShellEnvVarList;
+
 /**
   Reports whether an environment variable is Volatile or Non-Volatile.
 
@@ -379,3 +384,176 @@ SetEnvironmentVariables(
   //
   return (SetEnvironmentVariableList(>Link));
 }
+
+/**
+  Find an environment variable in the gShellEnvVarList.
+
+  @param KeyThe name of the environment variable.
+  @param Value  The value of the environment variable, the buffer
+shoule be freed by the caller.
+  @param ValueSize  The size in bytes of the environment variable
+including the tailing CHAR_NELL.
+  @param Atts   The attributes of the variable.
+
+  @retval EFI_SUCCESS   The command executed successfully.
+  @retval EFI_NOT_FOUND The environment variable is not found in
+gShellEnvVarList.
+
+**/
+EFI_STATUS
+ShellFindEnvVarInList (
+  IN  CONST CHAR16*Key,
+  OUT CHAR16  **Value,
+  OUT UINTN   *ValueSize,
+  OUT UINT32  *Atts OPTIONAL
+  )
+{
+  ENV_VAR_LIST  *Node;
+  
+  if (Key == NULL || Value == NULL || ValueSize == NULL) {
+return SHELL_INVALID_PARAMETER;
+  }
+
+  for ( Node = (ENV_VAR_LIST*)GetFirstNode()
+  ; !IsNull(, >Link)
+  ; Node = (ENV_VAR_LIST*)GetNextNode(, >Link)
+ ){
+if (Node->Key != NULL && StrCmp(Key, Node->Key) == 0) {
+  *Value  = AllocateCopyPool(StrSize(Node->Val), Node->Val);
+  *ValueSize  = StrSize(Node->Val);
+  if (Atts != NULL) {
+*Atts = Node->Atts;
+  }
+  return EFI_SUCCESS;
+}
+  }
+
+  return EFI_NOT_FOUND;
+}
+
+/**
+  Add an environment variable into gShellEnvVarList.
+
+  @param KeyThe name of the environment variable.
+  @param Value  The value of environment variable.
+  @param ValueSize  The size in bytes of the environment variable
+including the tailing CHAR_NULL
+  @param Atts   The attributes of the variable.
+
+**/
+VOID
+ShellAddEnvVarToList (
+  IN CONST CHAR16 *Key,
+  IN CONST CHAR16 *Value,
+  IN UINTNValueSize,
+  IN UINT32   Atts
+  )
+{
+  ENV_VAR_LIST  *Node;
+  
+  if (Key == NULL || Value == NULL || ValueSize == 0) {
+return;
+  }
+
+  //
+  // Update the variable value if it exists in gShellEnvVarList.
+  //
+  for ( Node = (ENV_VAR_LIST*)GetFirstNode()
+  ; !IsNull(, >Link)
+  ; Node = (ENV_VAR_LIST*)GetNextNode(, >Link)
+ ){
+if (Node->Key != 

Re: [edk2] [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

[El-Haj-Mahmoud, Samer]

Looks good. Are you planning on committing this code? I have a few updates I 
would like to send for review, but they need the initial patches to be 
committed first.
 
Series reviewed-by: Samer El-Haj-Mahmoud 


-Original Message-
From: Jiaxin Wu [mailto:jiaxin...@intel.com] 
Sent: Monday, April 11, 2016 2:50 AM
To: edk2-devel@lists.01.org
Cc: Ye Ting ; Fu Siyuan ; Long Qin 
; El-Haj-Mahmoud, Samer 
Subject: [PATCH v2 5/6] NetworkPkg: HTTPS support over IPv4 and IPv6

v2:
To support the multiple certificate configuration,
EFI_SIGNATURE_LIST format is used for the variable
'TlsCaCertificate'.

This patch is used to enable HTTPS feature. HttpDxe driver
will consume TlsDxe driver. It can both support
http and https feature, it’s depended on the information in URL,
the HTTP instance can be able to determine whether to use http
or https.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Long Qin 
Cc: El-Haj-Mahmoud Samer 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpDriver.h   |8 +-
 NetworkPkg/HttpDxe/HttpDxe.inf|8 +-
 NetworkPkg/HttpDxe/HttpImpl.c |  188 +++-
 NetworkPkg/HttpDxe/HttpProto.c|  395 ++---
 NetworkPkg/HttpDxe/HttpProto.h|   65 +-
 NetworkPkg/HttpDxe/HttpsSupport.c | 1701 +
 NetworkPkg/HttpDxe/HttpsSupport.h |  314 +++
 7 files changed, 2542 insertions(+), 137 deletions(-)
 create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.c
 create mode 100644 NetworkPkg/HttpDxe/HttpsSupport.h

diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 9c0002a..3c30c12 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -1,9 +1,9 @@
 /** @file
   The header files of the driver binding and service binding protocol for 
HttpDxe driver.
 
-  Copyright (c) 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
   (C) Copyright 2016 Hewlett Packard Enterprise Development LP
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -22,10 +22,11 @@
 
 //
 // Libraries
 //
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
@@ -48,12 +49,14 @@
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
+#include 
 
-
+#include 
 //
 // Produced Protocols
 //
 #include 
 
@@ -77,10 +80,11 @@ extern EFI_HTTP_UTILITIES_PROTOCOL  *mHttpUtilities;
 // Include files with function prototypes
 //
 #include "ComponentName.h"
 #include "HttpImpl.h"
 #include "HttpProto.h"
+#include "HttpsSupport.h"
 #include "HttpDns.h"
 
 typedef struct {
   EFI_SERVICE_BINDING_PROTOCOL  *ServiceBinding;
   UINTN NumberOfChildren;
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index bf2cbee..a228c3d 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,9 +1,9 @@
 ## @file
 #  Implementation of EFI HTTP protocol interfaces.
 #
-#  Copyright (c) 2015, Intel Corporation. All rights reserved.
+#  Copyright (c) 2015 - 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.
@@ -36,14 +36,17 @@
   HttpDriver.c
   HttpImpl.h
   HttpImpl.c
   HttpProto.h
   HttpProto.c
+  HttpsSupport.h
+  HttpsSupport.c
 
 [LibraryClasses]
   UefiDriverEntryPoint
   UefiBootServicesTableLib
+  UefiRuntimeServicesTableLib
   MemoryAllocationLib
   BaseLib
   UefiLib
   DebugLib
   NetLib
@@ -62,8 +65,11 @@
   gEfiDns4ProtocolGuid ## SOMETIMES_CONSUMES
   gEfiDns6ServiceBindingProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiDns6ProtocolGuid ## SOMETIMES_CONSUMES
   gEfiIp4Config2ProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiIp6ConfigProtocolGuid## SOMETIMES_CONSUMES
+  gEfiTlsServiceBindingProtocolGuid## SOMETIMES_CONSUMES
+  gEfiTlsProtocolGuid  ## SOMETIMES_CONSUMES
+  gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
 
 [UserExtensions.TianoCore."ExtraFiles"]
   HttpDxeExtra.uni
\ No newline at end of file
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 63b683e..8d81a90 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -238,10 +238,11 @@ 

Re: [edk2] [PATCH 4/4] ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling

[Ard Biesheuvel]

On 12 April 2016 at 18:10, Laszlo Ersek  wrote:
> On 04/12/16 15:47, Ard Biesheuvel wrote:
>> Now that the PCI host bridge driver parses the DT node that describes
>> the PCI host bridge directly via the FDT client protocol, we can drop the
>> handling from VirtFdtDxe completely.
>>
>> This means some PCI related PCDs are no longer set, such as PcdPciBusMin,
>> PcdPciBusMax, PcdPciIoBase, PcdPciIoSize, PcdPciIoTranslation,
>> PcdPciMmio32Base and PcdPciMmio32Size. Since these PCDs are specific to
>> ARM (and declared in ArmPlatformPkg), and not used anywhere else by the
>> ArmVirtPkg platforms, we can simply stop populating them, and drop all
>> references to them.
>>
>> It also means that we can no longer rely on PcdPciExpressBaseAddress and
>> PcdPciDisableBusEnumeration to be set before they may be needed by
>> either PciBusDxe or QemuFwCfgAcpiPlatformDxe, so make those depend on
>> FdtPciPcdProducerLib explicitly via NULL library class resolution.
>
> Please reword the above paragraph like this (basically, drop
> PcdPciExpressBaseAddress, and remove "may"):
>
>   It also means that we can no longer rely on
>   PcdPciDisableBusEnumeration to be set before it is consumed by
>   PciBusDxe and QemuFwCfgAcpiPlatformDxe, so make those depend on
>   FdtPciPcdProducerLib explicitly via NULL library class resolution.
>

OK

>> diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf 
>> b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
>> index 6c8ba68599ae..8ebce337747f 100644
>> --- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
>> +++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
>> @@ -51,15 +51,6 @@ [Guids]
>>
>>  [Pcd]
>>gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
>> -  gArmPlatformTokenSpaceGuid.PcdPciBusMin
>> -  gArmPlatformTokenSpaceGuid.PcdPciBusMax
>> -  gArmPlatformTokenSpaceGuid.PcdPciIoBase
>> -  gArmPlatformTokenSpaceGuid.PcdPciIoSize
>> -  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation
>> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base
>> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size
>> -  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>> -  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
>
> Can you perhaps drop "ArmPlatformPkg/ArmPlatformPkg.dec" too? If not,
> that's not a problem, but if it works, we should.
>

No, see the top Pcd. But I will keep it in mind for the remaining patches.

> With those:
>
> Reviewed-by: Laszlo Ersek 
>
> Since this sub-series is now fully reviewed (dependent on a few tweaks),
> feel free to push it too.
>

Thanks, and pushed ...

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

Re: [edk2] TCPIP Client for UEFI

[Mahan, Patrick]

Jim,

I think your only recourse is to contact the who provided the SOC board and ask 
about how to rebuild the UEFI bootloader image for that board.  Then look at 
those instructions to see how to enable the network stack.

Good luck,

Patrick


From: edk2-devel  on behalf of Jim Slaughter 

Sent: Monday, April 11, 2016 4:29 PM
To: Mahan, Patrick
Cc: Edk2-devel
Subject: Re: [edk2] TCPIP Client for UEFI

Hello Patrick,Thank you.I do not have a setup screen with this BIOS/uEFI. Its n 
SOC on a board but not really a consumer PC type.I need to figure out a way to 
setup the NIC, find something already written or write it myself. I am not sure 
if a setup can be done after the actual uefi boot. think Setup on a computer is 
really before the boot process.All the ifconfig commands give the same protocol 
error.Any help?Jim S.


On Saturday, April 9, 2016 11:25 PM, "Mahan, Patrick" 
 wrote:


 Jim,

Okay, that’s better.

So now I would suggest looking at the BIOS/UEFI setup screens for any hints 
about enabling the network or
enabling network booting.

For example, I have a DELL 5810 Precision Tower that I am currently using for 
UEFI testing.  To enable the
network stack and PXE booting, I have to go to the config screen that shows the 
onboard NIC config and
enable the Network stack and PXE booting.  Not sure what the Insyde screens 
show, but a quick google-foo
seems to show that PXE is enabled on the ‘Boot’ config.  I would look for any 
manual you might have for
configuring the BIOS/UEFI.

Now here is the problem.  You can load the UNDI driver by copying it to your 
EFI directory on boot device.
For most Linux systems, this is /boot/efi/EFI/ (e.g. 
/boot/efi/EFI/ubuntu or /boot/efi/EFI/centos).

Then start the EFI shell, and in that shell use the ‘load’ command to load the 
UNDI driver, probably from
fs0:\EFI\\.efi, then exit the shell and back in the boot menu 
you should be able to add
the new NIC.  The assumption here is that when you exit the EFI shell you fall 
back into the boot menu,
which is not the case on the DELL I am using.

If this doesn’t work, then I suggest first contacting the AMD SOC support or 
the Insyde folks.

Good luck,

Patrick

> On Apr 9, 2016, at 8:00 PM, Jim Slaughter  wrote:
>
> I downloaded there binary as they do not have the source listed,
> UEFI UNDI Driver
> from  
> http://www.realtek.com.tw/downloads/downloadsView.aspx?Langid=1=13=5=5=4=3=false
> We are using there chip on out board.
> If I do an "ifconfig" command (no arguments) I get the following:ifconfig: 
> "Locate protocol error - ip4Cofi Protocol"
> Something is missing I think? No ifconfig's work.
> Jim Slaughter
>
>
>    On Saturday, April 9, 2016 9:29 AM, "Mahan, Patrick" 
> wrote:
>
>
> Jim,
>
> What exactly did you download?  If I google ‘realtek refi’ I get the realtek 
> refi undo driver as a UEFI binary (*.efi) but
> no source code.
>
> So I am not sure what you have exactly.  If it is source code then it should 
> be a UEFI package which should contain
> all the files to build the NIC.
>
> If you haven’t already, you can take a look at how to get started developing 
> in UEFI at the following URL -
>
> https://github.com/tianocore/tianocore.github.io/wiki/Getting%20Started%20with%20EDK%20II
>
> (UEFI is based on EDK II)
>
> I personally have never gotten ‘ipconfig’ to work, I instead use ‘ifconfig’.  
> Do a ‘ifconfig -?’ for help.
>
> Patrick
>
>> On Apr 8, 2016, at 3:57 PM, Jim Slaughter  wrote:
>>
>> Hello,
>> The platform is an AMD based SOC. The uEFI is from Insyde. Yes I do have a 
>> uEFI shell.
>> When I downloaded the C driver there were no instructions. I have not yet 
>> configured the card. I have not seen any instructions on how to configure. 
>> ipconfig does not work.
>> Jim S.
>>
>>
>>    On Friday, April 8, 2016 3:40 PM, "Mahan, Patrick" 
>> wrote:
>>
>>
>>
>> Jim,
>>
>> This depends on a number of factors:
>>
>> 1. What platform?  Who wrote the UEFI code for it?  Do you have access to a 
>> EFI or UEFI shell?
>>
>> 2. I'm not sure about the Realtek NIC, but did they have any instructions 
>> for using the driver?  UEFI supports
>>    having device specific directories that a Vendor may populate with their 
>>driver.  That might be the case for
>>    Realtek.
>>
>> 3. Go into your UEFI config (again this depends upon whose UEFI is running) 
>> and make sure you have enabled
>>    the network stack.
>>
>> Always be specific in details, it makes it easier to provide you help,
>>
>> Thanks,
>>
>> Patrick
>>
>> 
>> From: edk2-devel  on behalf of Jim 
>> Slaughter 
>> Sent: Friday, April 8, 2016 3:27 PM
>> To: Edk2-devel
>> Subject: [edk2] TCPIP Client for UEFI

Re: [edk2] [PATCH 4/4] ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling

[Laszlo Ersek]

On 04/12/16 15:47, Ard Biesheuvel wrote:
> Now that the PCI host bridge driver parses the DT node that describes
> the PCI host bridge directly via the FDT client protocol, we can drop the
> handling from VirtFdtDxe completely.
> 
> This means some PCI related PCDs are no longer set, such as PcdPciBusMin,
> PcdPciBusMax, PcdPciIoBase, PcdPciIoSize, PcdPciIoTranslation,
> PcdPciMmio32Base and PcdPciMmio32Size. Since these PCDs are specific to
> ARM (and declared in ArmPlatformPkg), and not used anywhere else by the
> ArmVirtPkg platforms, we can simply stop populating them, and drop all
> references to them.
> 
> It also means that we can no longer rely on PcdPciExpressBaseAddress and
> PcdPciDisableBusEnumeration to be set before they may be needed by
> either PciBusDxe or QemuFwCfgAcpiPlatformDxe, so make those depend on
> FdtPciPcdProducerLib explicitly via NULL library class resolution.

Please reword the above paragraph like this (basically, drop
PcdPciExpressBaseAddress, and remove "may"):

  It also means that we can no longer rely on
  PcdPciDisableBusEnumeration to be set before it is consumed by
  PciBusDxe and QemuFwCfgAcpiPlatformDxe, so make those depend on
  FdtPciPcdProducerLib explicitly via NULL library class resolution.

> diff --git a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf 
> b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
> index 6c8ba68599ae..8ebce337747f 100644
> --- a/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
> +++ b/ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf
> @@ -51,15 +51,6 @@ [Guids]
>  
>  [Pcd]
>gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
> -  gArmPlatformTokenSpaceGuid.PcdPciBusMin
> -  gArmPlatformTokenSpaceGuid.PcdPciBusMax
> -  gArmPlatformTokenSpaceGuid.PcdPciIoBase
> -  gArmPlatformTokenSpaceGuid.PcdPciIoSize
> -  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation
> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base
> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size
> -  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration

Can you perhaps drop "ArmPlatformPkg/ArmPlatformPkg.dec" too? If not,
that's not a problem, but if it works, we should.

With those:

Reviewed-by: Laszlo Ersek 

Since this sub-series is now fully reviewed (dependent on a few tweaks),
feel free to push it too.

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

Re: [edk2] [PATCH 3/4] ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol

[Laszlo Ersek]

On 04/12/16 17:59, Ard Biesheuvel wrote:
> On 12 April 2016 at 17:41, Laszlo Ersek  wrote:
>> On 04/12/16 15:47, Ard Biesheuvel wrote:
>>> Instead of relying on VirtFdtDxe to populate various dynamic PCDs with
>>> information retrieved from the host-provided device tree, perform the
>>> PCI ECAM related DT node parsing directly in PciHostBridgeDxe.
>>>
>>> Since we expect PcdPciExpressBaseAddress to have already been assigned
>>> by the time this module's entry point is called, add a dependency on
>>> PciPcdProducerLib as well.
>>
>> Hm, I disagree with the last paragraph (and the corresponding INF file
>> change). The point of adding the PciPcdProducerLib dependency to our
>> BaseCachingPciExpressLib instance, in patch #2, was exactly that
>> DXE_DRIVER modules that depend (transitively) on
>> BaseCachingPciExpressLib inherit the FdtPciPcdProducerLib constructor
>> automatically.
>>
>> In other words, patch #2 ensures that any DXE_DRIVER in ArmVirtPkg that
>> performs PCI config space access (via our BaseCachingPciExpressLib, of
>> course) will automatically receive a discovered (possibly zero) ECAM
>> base address.
>>
>> We have the following resolutions:
>>
>> PciLib -> MdePkg/Library/BasePciLibPciExpress [ArmVirt.dsc.inc]
>>   depends on PciExpressLib
>>
>> PciExpressLib -> BaseCachingPciExpressLib [ArmVirt.dsc.inc]
>>  depends on PciPcdProducerLib (patch 2)
>>
>> PciPcdProducerLib -> FdtPciPcdProducerLib [ArmVirtQemu.dsc, 
>> ArmVirtQemuKernel.dsc]
>>  (also patch 2)
>>
>> The automation in this resolution chain is intentional. The direct
>> PciPcdProducerLib dependency was only necessary for the
>> BaseCachingPciExpressLib instance.
>>
>> (And the explicit plugging is only necessary for DXE_DRIVER and
>> UEFI_DRIVER modules that don't use PciLib and friends for PCI config
>> space access (hence they don't inherit our FdtPciPcdProducerLib
>> constructor), but depend on PcdPciDisableBusEnumeration.)
>>
> 
> For all cases except this one, it is blatantly obvious. However, the
> fact that this module does not go via PciLib but refers to
> PcdPciExpressBaseAddress directly makes the line a little fuzzy.

I agree that the line is a bit more fuzzy than in the previous
iteration, when our PciExpressLib instance was setting
PcdPciExpressBaseAddress directly (and here it is delegated even more
deeply).

However, it remains true (in our little ArmVirtPkg world) that
"somewhere down the construction chain" of PciLib,
PcdPciExpressBaseAddress must become assigned. In other words, our PCI
config space accessing DXE_DRIVER modules should be able to say, "I'm
consuming PciLib for config space access, so I trust the ECAM base
address will be initialized by the time I gain control".

> But ultimately, I care most about whether it is guaranteed to work as
> expected, which is clearly the case, so I am happy to incorporate this
> change.

Thank you.

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

Re: [edk2] [PATCH 3/4] ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol

[Ard Biesheuvel]

On 12 April 2016 at 17:41, Laszlo Ersek  wrote:
> On 04/12/16 15:47, Ard Biesheuvel wrote:
>> Instead of relying on VirtFdtDxe to populate various dynamic PCDs with
>> information retrieved from the host-provided device tree, perform the
>> PCI ECAM related DT node parsing directly in PciHostBridgeDxe.
>>
>> Since we expect PcdPciExpressBaseAddress to have already been assigned
>> by the time this module's entry point is called, add a dependency on
>> PciPcdProducerLib as well.
>
> Hm, I disagree with the last paragraph (and the corresponding INF file
> change). The point of adding the PciPcdProducerLib dependency to our
> BaseCachingPciExpressLib instance, in patch #2, was exactly that
> DXE_DRIVER modules that depend (transitively) on
> BaseCachingPciExpressLib inherit the FdtPciPcdProducerLib constructor
> automatically.
>
> In other words, patch #2 ensures that any DXE_DRIVER in ArmVirtPkg that
> performs PCI config space access (via our BaseCachingPciExpressLib, of
> course) will automatically receive a discovered (possibly zero) ECAM
> base address.
>
> We have the following resolutions:
>
> PciLib -> MdePkg/Library/BasePciLibPciExpress [ArmVirt.dsc.inc]
>   depends on PciExpressLib
>
> PciExpressLib -> BaseCachingPciExpressLib [ArmVirt.dsc.inc]
>  depends on PciPcdProducerLib (patch 2)
>
> PciPcdProducerLib -> FdtPciPcdProducerLib [ArmVirtQemu.dsc, 
> ArmVirtQemuKernel.dsc]
>  (also patch 2)
>
> The automation in this resolution chain is intentional. The direct
> PciPcdProducerLib dependency was only necessary for the
> BaseCachingPciExpressLib instance.
>
> (And the explicit plugging is only necessary for DXE_DRIVER and
> UEFI_DRIVER modules that don't use PciLib and friends for PCI config
> space access (hence they don't inherit our FdtPciPcdProducerLib
> constructor), but depend on PcdPciDisableBusEnumeration.)
>

For all cases except this one, it is blatantly obvious. However, the
fact that this module does not go via PciLib but refers to
PcdPciExpressBaseAddress directly makes the line a little fuzzy.

But ultimately, I care most about whether it is guaranteed to work as
expected, which is clearly the case, so I am happy to incorporate this
change.

>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c  | 261 ++--
>>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h  |   1 +
>>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |  14 +-
>>  3 files changed, 253 insertions(+), 23 deletions(-)
>
> INF file first:
>
>> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf 
>> b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
>> index 6d782582e02d..e58a45ff0e2f 100644
>> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
>> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
>> @@ -22,6 +22,7 @@ [Defines]
>>ENTRY_POINT= InitializePciHostBridge
>>
>>  [Packages]
>> +  MdeModulePkg/MdeModulePkg.dec
>>MdePkg/MdePkg.dec
>>ArmPlatformPkg/ArmPlatformPkg.dec
>
> If you drop "ArmPlatformPkg/ArmPlatformPkg.dec" (in addition to the
> other changes in this patch), does the driver continue to build? If so,
> please include that removal.
>

Yes, that works

>>ArmVirtPkg/ArmVirtPkg.dec
>> @@ -39,6 +40,7 @@ [LibraryClasses]
>>IoLib
>>PciLib
>>PcdLib
>> +  PciPcdProducerLib
>
> So, my main point is, please drop this hunk. PciHostBridgeDxe uses
> PciLib directly (and see the resolution chain near the top); the PciLib
> entry in [LibraryClasses] here is not accidental.
>
> You can find, for example, PciRead8() / PciRead16() / PciRead32() calls
> in the source. PciHostBridgeDxe uses PciLib to implement the
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.(Read|Write) accessors.
>
>>  [Sources]
>>PciHostBridge.c
>> @@ -50,20 +52,16 @@ [Protocols]
>>gEfiPciRootBridgeIoProtocolGuid   ## PRODUCES
>>gEfiMetronomeArchProtocolGuid ## CONSUMES
>>gEfiDevicePathProtocolGuid## PRODUCES
>> +  gFdtClientProtocolGuid## CONSUMES
>>
>>  [Pcd]
>> -  gArmPlatformTokenSpaceGuid.PcdPciBusMin
>> -  gArmPlatformTokenSpaceGuid.PcdPciBusMax
>> -  gArmPlatformTokenSpaceGuid.PcdPciIoBase
>> -  gArmPlatformTokenSpaceGuid.PcdPciIoSize
>> -  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation
>> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base
>> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size
>>gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>>
>>  [FeaturePcd]
>>gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached
>> +  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
>>
>>  [depex]
>>gEfiMetronomeArchProtocolGuid AND
>> -  gEfiCpuArchProtocolGuid
>> +  gEfiCpuArchProtocolGuid AND
>> +  gFdtClientProtocolGuid
>>
>
> Looks good otherwise; in particular I do agree with the explicit 

Re: [edk2] [Patch] QuarkSocPkg: Add /Oi option to let MemoryInit pass build.

[Kinney, Michael D]

Reviewed-by: Michael Kinney 



> -Original Message-
> From: Gao, Liming
> Sent: Monday, April 11, 2016 8:04 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D 
> Subject: [Patch] QuarkSocPkg: Add /Oi option to let MemoryInit pass build.
> 
> MemoryInit uses the intrinsic memset function. To keep it pass build in VS
> tool chain without source code change, /Oi option will be added.
> 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
> b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
> index e327684..78821f5 100644
> --- a/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
> +++ b/QuarkSocPkg/QuarkNorthCluster/MemoryInit/Pei/MemoryInitPei.inf
> @@ -74,3 +74,7 @@
> 
>  [Depex]
>TRUE
> +
> +[BuildOptions]
> +  # /Oi option to use the intrinsic memset function in source code.
> +  MSFT:*_*_*_CC_FLAGS = /Oi
> --
> 2.8.0.windows.1

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

Re: [edk2] [PATCH 3/4] ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol

[Laszlo Ersek]

On 04/12/16 15:47, Ard Biesheuvel wrote:
> Instead of relying on VirtFdtDxe to populate various dynamic PCDs with
> information retrieved from the host-provided device tree, perform the
> PCI ECAM related DT node parsing directly in PciHostBridgeDxe.
>
> Since we expect PcdPciExpressBaseAddress to have already been assigned
> by the time this module's entry point is called, add a dependency on
> PciPcdProducerLib as well.

Hm, I disagree with the last paragraph (and the corresponding INF file
change). The point of adding the PciPcdProducerLib dependency to our
BaseCachingPciExpressLib instance, in patch #2, was exactly that
DXE_DRIVER modules that depend (transitively) on
BaseCachingPciExpressLib inherit the FdtPciPcdProducerLib constructor
automatically.

In other words, patch #2 ensures that any DXE_DRIVER in ArmVirtPkg that
performs PCI config space access (via our BaseCachingPciExpressLib, of
course) will automatically receive a discovered (possibly zero) ECAM
base address.

We have the following resolutions:

PciLib -> MdePkg/Library/BasePciLibPciExpress [ArmVirt.dsc.inc]
  depends on PciExpressLib

PciExpressLib -> BaseCachingPciExpressLib [ArmVirt.dsc.inc]
 depends on PciPcdProducerLib (patch 2)

PciPcdProducerLib -> FdtPciPcdProducerLib [ArmVirtQemu.dsc, 
ArmVirtQemuKernel.dsc]
 (also patch 2)

The automation in this resolution chain is intentional. The direct
PciPcdProducerLib dependency was only necessary for the
BaseCachingPciExpressLib instance.

(And the explicit plugging is only necessary for DXE_DRIVER and
UEFI_DRIVER modules that don't use PciLib and friends for PCI config
space access (hence they don't inherit our FdtPciPcdProducerLib
constructor), but depend on PcdPciDisableBusEnumeration.)

> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c  | 261 ++--
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h  |   1 +
>  ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |  14 +-
>  3 files changed, 253 insertions(+), 23 deletions(-)

INF file first:

> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf 
> b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> index 6d782582e02d..e58a45ff0e2f 100644
> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
> @@ -22,6 +22,7 @@ [Defines]
>ENTRY_POINT= InitializePciHostBridge
>
>  [Packages]
> +  MdeModulePkg/MdeModulePkg.dec
>MdePkg/MdePkg.dec
>ArmPlatformPkg/ArmPlatformPkg.dec

If you drop "ArmPlatformPkg/ArmPlatformPkg.dec" (in addition to the
other changes in this patch), does the driver continue to build? If so,
please include that removal.

>ArmVirtPkg/ArmVirtPkg.dec
> @@ -39,6 +40,7 @@ [LibraryClasses]
>IoLib
>PciLib
>PcdLib
> +  PciPcdProducerLib

So, my main point is, please drop this hunk. PciHostBridgeDxe uses
PciLib directly (and see the resolution chain near the top); the PciLib
entry in [LibraryClasses] here is not accidental.

You can find, for example, PciRead8() / PciRead16() / PciRead32() calls
in the source. PciHostBridgeDxe uses PciLib to implement the
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL.Pci.(Read|Write) accessors.

>  [Sources]
>PciHostBridge.c
> @@ -50,20 +52,16 @@ [Protocols]
>gEfiPciRootBridgeIoProtocolGuid   ## PRODUCES
>gEfiMetronomeArchProtocolGuid ## CONSUMES
>gEfiDevicePathProtocolGuid## PRODUCES
> +  gFdtClientProtocolGuid## CONSUMES
>
>  [Pcd]
> -  gArmPlatformTokenSpaceGuid.PcdPciBusMin
> -  gArmPlatformTokenSpaceGuid.PcdPciBusMax
> -  gArmPlatformTokenSpaceGuid.PcdPciIoBase
> -  gArmPlatformTokenSpaceGuid.PcdPciIoSize
> -  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation
> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base
> -  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size
>gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
>
>  [FeaturePcd]
>gArmVirtTokenSpaceGuid.PcdKludgeMapPciMmioAsCached
> +  gArmVirtTokenSpaceGuid.PcdPureAcpiBoot
>
>  [depex]
>gEfiMetronomeArchProtocolGuid AND
> -  gEfiCpuArchProtocolGuid
> +  gEfiCpuArchProtocolGuid AND
> +  gFdtClientProtocolGuid
>

Looks good otherwise; in particular I do agree with the explicit depex,
since we are going to use the protocol explicitly.

Let's see the header file:

> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h 
> b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h
> index 8161b546ff87..647fe1a52a7d 100644
> --- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h
> +++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h
> @@ -24,6 +24,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>
>  #include 

Makes sense.

C source:

> diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c 
> b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
> index 

Re: [edk2] [PATCH 2/4] ArmVirtPkg/BaseCachingPciExpressLib: depend on PciPcdProducerLib

[Laszlo Ersek]

On 04/12/16 15:47, Ard Biesheuvel wrote:
> Make BaseCachingPciExpressLib depend on PciPcdProducerLib, so that we
> have a chance to populate PcdPciExpressBaseAddress based on the contents
> of the device tree. 
> 
> Also update the platforms under ArmVirtPkg that support PCI to use this
> special MAX_UINT64 as the build time default for PcdPciExpressBaseAddress,

Just a suggestion: s/MAX_UINT64/MAX_UINT64 value/?

> and to specify a suitable resolution for PciPcdProducerLib.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtQemu.dsc   | 6 
> +-
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 6 
> +-
>  ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf | 2 
> ++
>  3 files changed, 12 insertions(+), 2 deletions(-)

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

Re: [edk2] [PATCH 1/4] ArmVirtPkg: implement FdtPciPcdProducerLib

[Laszlo Ersek]

On 04/12/16 15:47, Ard Biesheuvel wrote:
> This implements a library FdtPciPcdProducerLib which is intended to
> be incorporated into modules that consume the PCI related dynamic PCDs
> PcdPciExpressBaseAddress and PcdPciDisableBusEnumeration, either via NULL
> library class resolution or via a direct dependency (for other libraries
> or modules in ArmVirtPkg). This allows us to make them depend on the FDT
> client protocol, and populate these PCDs based on the presence and the
> contents of a 'pci-ecam-generic' DT node.

Typo: should be 'pci-host-ecam-generic'.

> This also overloads the meaning of PcdPciExpressBaseAddress, which we will
> set to MAX_UINT64 to signify that the actual values of these two PCDs have
> not been assigned yet.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   | 61 
> 
>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf | 47 
> +++
>  2 files changed, 108 insertions(+)

INF file first:

> diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf 
> b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> new file mode 100644
> index ..546125997ba6
> --- /dev/null
> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> @@ -0,0 +1,47 @@
> +#/** @file
> +#  FDT client library for consumers of PCI related dynamic PCDs
> +#
> +#  Copyright (c) 2016, Linaro Ltd. 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.
> +#
> +#
> +#**/

I believe one of these trailing empty lines should be between the URL of
the 2-BSDL and the no-warranty disclaimer.

> +
> +[Defines]
> +  INF_VERSION= 0x00010005
> +  BASE_NAME  = FdtPciPcdProducerLib
> +  FILE_GUID  = D584275B-BF1E-4DF8-A53D-980F5645C5E7
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = PciPcdProducerLib|DXE_DRIVER UEFI_DRIVER

I think it would make sense to match the library class name to the
(single) library instance we have. I don't insist of course, but I
believe this may have been an oversight. If it was intentional, then I'm
fine with it. (The subject also says FdtPciPcdProducerLib.)

> +  CONSTRUCTOR= FdtPciPcdProducerLibConstructor
> +
> +[Sources]
> +  FdtPciPcdProducerLib.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +
> +[Protocols]
> +  gFdtClientProtocolGuid  ## CONSUMES
> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress   ## PRODUCES

For this, I think I proposed (based on existing usage in edk2):

  ## CONSUMES ## SOMETIMES_PRODUCES

I agree we do not "consume" the PCD any longer in the strict sense, in
this module; we just look if it was initialized. Hm... I guess it is
fine as-is.

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration  ## PRODUCES
> +
> +[Depex]
> +  gFdtClientProtocolGuid

source code:

> diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c 
> b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
> new file mode 100644
> index ..64a3aa969680
> --- /dev/null
> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
> @@ -0,0 +1,61 @@
> +/** @file
> +  FDT client library for consumers of PCI related dynamic PCDs
> +
> +  Copyright (c) 2016, Linaro Ltd. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD 
> License
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +RETURN_STATUS
> +EFIAPI
> +FdtPciPcdProducerLibConstructor (
> +  VOID
> +  )
> +{
> +  UINT64  PciExpressBaseAddress;
> +  FDT_CLIENT_PROTOCOL *FdtClient;
> +  CONST UINT64*Reg;
> +  UINT32  RegElemSize, RegSize;
> +  EFI_STATUS  Status;
> +
> +  

Re: [edk2] [PATCH 1/4] ArmVirtPkg: implement FdtPciPcdProducerLib

[Laszlo Ersek]

On 04/12/16 15:47, Ard Biesheuvel wrote:
> This implements a library FdtPciPcdProducerLib which is intended to
> be incorporated into modules that consume the PCI related dynamic PCDs
> PcdPciExpressBaseAddress and PcdPciDisableBusEnumeration, either via NULL
> library class resolution or via a direct dependency (for other libraries
> or modules in ArmVirtPkg). This allows us to make them depend on the FDT
> client protocol, and populate these PCDs based on the presence and the
> contents of a 'pci-ecam-generic' DT node.

Typo: should be 'pci-host-ecam-generic'.

> This also overloads the meaning of PcdPciExpressBaseAddress, which we will
> set to MAX_UINT64 to signify that the actual values of these two PCDs have
> not been assigned yet.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   | 61

>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf | 47
+++
>  2 files changed, 108 insertions(+)

INF file first:

> diff --git
a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> new file mode 100644
> index ..546125997ba6
> --- /dev/null
> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
> @@ -0,0 +1,47 @@
> +#/** @file
> +#  FDT client library for consumers of PCI related dynamic PCDs
> +#
> +#  Copyright (c) 2016, Linaro Ltd. 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.
> +#
> +#
> +#**/

I believe one of these trailing empty lines should be between the URL of
the 2-BSDL and the no-warranty disclaimer.

> +
> +[Defines]
> +  INF_VERSION= 0x00010005
> +  BASE_NAME  = FdtPciPcdProducerLib
> +  FILE_GUID  = D584275B-BF1E-4DF8-A53D-980F5645C5E7
> +  MODULE_TYPE= BASE
> +  VERSION_STRING = 1.0
> +  LIBRARY_CLASS  = PciPcdProducerLib|DXE_DRIVER
UEFI_DRIVER

I think it would make sense to match the library class name to the
(single) library instance we have. I don't insist of course, but I
believe this may have been an oversight. If it was intentional, then I'm
fine with it. (The subject also says FdtPciPcdProducerLib.)

> +  CONSTRUCTOR= FdtPciPcdProducerLibConstructor
> +
> +[Sources]
> +  FdtPciPcdProducerLib.c
> +
> +[Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  PcdLib
> +  UefiBootServicesTableLib
> +
> +[Protocols]
> +  gFdtClientProtocolGuid  ## CONSUMES
> +
> +[Pcd]
> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress   ## PRODUCES

For this, I think I proposed (based on existing usage in edk2):

  ## CONSUMES ## SOMETIMES_PRODUCES

I agree we do not "consume" the PCD any longer in the strict sense, in
this module; we just look if it was initialized. Hm... I guess it is
fine as-is.

> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration  ## PRODUCES
> +
> +[Depex]
> +  gFdtClientProtocolGuid

source code:

> diff --git
a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
> new file mode 100644
> index ..64a3aa969680
> --- /dev/null
> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
> @@ -0,0 +1,61 @@
> +/** @file
> +  FDT client library for consumers of PCI related dynamic PCDs
> +
> +  Copyright (c) 2016, Linaro Ltd. All rights reserved.
> +
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of
the BSD License
> +  which accompanies this distribution.  The full text of the license
may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
OR IMPLIED.
> +
> +**/
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +
> +RETURN_STATUS
> +EFIAPI
> +FdtPciPcdProducerLibConstructor (
> +  VOID
> +  )
> +{
> +  UINT64  PciExpressBaseAddress;
> +  FDT_CLIENT_PROTOCOL *FdtClient;
> +  CONST UINT64*Reg;
> +  UINT32  RegElemSize, RegSize;
> +  EFI_STATUS  Status;
> +
> +  PciExpressBaseAddress = PcdGet64 

Re: [edk2] [PATCH 1/4] ArmVirtPkg: implement FdtPciPcdProducerLib

[Laszlo Ersek]

On 04/12/16 16:16, Laszlo Ersek wrote:
> [...]

Aargh, ignore this. I forgot to disable word wrapping in tbird, before
pasting my reply. I'll resend my reply sanely wrapped.

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

Re: [edk2] [PATCH 1/4] ArmVirtPkg: implement FdtPciPcdProducerLib

[Ard Biesheuvel]

On 12 April 2016 at 16:18, Laszlo Ersek  wrote:
> On 04/12/16 15:47, Ard Biesheuvel wrote:
>> This implements a library FdtPciPcdProducerLib which is intended to
>> be incorporated into modules that consume the PCI related dynamic PCDs
>> PcdPciExpressBaseAddress and PcdPciDisableBusEnumeration, either via NULL
>> library class resolution or via a direct dependency (for other libraries
>> or modules in ArmVirtPkg). This allows us to make them depend on the FDT
>> client protocol, and populate these PCDs based on the presence and the
>> contents of a 'pci-ecam-generic' DT node.
>
> Typo: should be 'pci-host-ecam-generic'.
>

OK

>> This also overloads the meaning of PcdPciExpressBaseAddress, which we will
>> set to MAX_UINT64 to signify that the actual values of these two PCDs have
>> not been assigned yet.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   | 61 
>> 
>>  ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf | 47 
>> +++
>>  2 files changed, 108 insertions(+)
>
> INF file first:
>
>> diff --git 
>> a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf 
>> b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>> new file mode 100644
>> index ..546125997ba6
>> --- /dev/null
>> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
>> @@ -0,0 +1,47 @@
>> +#/** @file
>> +#  FDT client library for consumers of PCI related dynamic PCDs
>> +#
>> +#  Copyright (c) 2016, Linaro Ltd. 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.
>> +#
>> +#
>> +#**/
>
> I believe one of these trailing empty lines should be between the URL of
> the 2-BSDL and the no-warranty disclaimer.
>

OK

>> +
>> +[Defines]
>> +  INF_VERSION= 0x00010005
>> +  BASE_NAME  = FdtPciPcdProducerLib
>> +  FILE_GUID  = D584275B-BF1E-4DF8-A53D-980F5645C5E7
>> +  MODULE_TYPE= BASE
>> +  VERSION_STRING = 1.0
>> +  LIBRARY_CLASS  = PciPcdProducerLib|DXE_DRIVER UEFI_DRIVER
>
> I think it would make sense to match the library class name to the
> (single) library instance we have. I don't insist of course, but I
> believe this may have been an oversight. If it was intentional, then I'm
> fine with it. (The subject also says FdtPciPcdProducerLib.)
>

Actually, it was intentional, since this way, at least you can reuse
the dependent modules by supplying a Null implementation of the
library class if you are not using DT.

>> +  CONSTRUCTOR= FdtPciPcdProducerLibConstructor
>> +
>> +[Sources]
>> +  FdtPciPcdProducerLib.c
>> +
>> +[Packages]
>> +  ArmVirtPkg/ArmVirtPkg.dec
>> +  MdeModulePkg/MdeModulePkg.dec
>> +  MdePkg/MdePkg.dec
>> +
>> +[LibraryClasses]
>> +  BaseLib
>> +  DebugLib
>> +  PcdLib
>> +  UefiBootServicesTableLib
>> +
>> +[Protocols]
>> +  gFdtClientProtocolGuid  ## CONSUMES
>> +
>> +[Pcd]
>> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress   ## PRODUCES
>
> For this, I think I proposed (based on existing usage in edk2):
>
>   ## CONSUMES ## SOMETIMES_PRODUCES
>
> I agree we do not "consume" the PCD any longer in the strict sense, in
> this module; we just look if it was initialized. Hm... I guess it is
> fine as-is.
>
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration  ## PRODUCES
>> +
>> +[Depex]
>> +  gFdtClientProtocolGuid
>
> source code:
>
>> diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c 
>> b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
>> new file mode 100644
>> index ..64a3aa969680
>> --- /dev/null
>> +++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
>> @@ -0,0 +1,61 @@
>> +/** @file
>> +  FDT client library for consumers of PCI related dynamic PCDs
>> +
>> +  Copyright (c) 2016, Linaro Ltd. 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.
>> +

[edk2] [PATCH 1/4] ArmVirtPkg: implement FdtPciPcdProducerLib

[Ard Biesheuvel]

This implements a library FdtPciPcdProducerLib which is intended to
be incorporated into modules that consume the PCI related dynamic PCDs
PcdPciExpressBaseAddress and PcdPciDisableBusEnumeration, either via NULL
library class resolution or via a direct dependency (for other libraries
or modules in ArmVirtPkg). This allows us to make them depend on the FDT
client protocol, and populate these PCDs based on the presence and the
contents of a 'pci-ecam-generic' DT node.

This also overloads the meaning of PcdPciExpressBaseAddress, which we will
set to MAX_UINT64 to signify that the actual values of these two PCDs have
not been assigned yet.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   | 61 

 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf | 47 
+++
 2 files changed, 108 insertions(+)

diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c 
b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
new file mode 100644
index ..64a3aa969680
--- /dev/null
+++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
@@ -0,0 +1,61 @@
+/** @file
+  FDT client library for consumers of PCI related dynamic PCDs
+
+  Copyright (c) 2016, Linaro Ltd. All rights reserved.
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+RETURN_STATUS
+EFIAPI
+FdtPciPcdProducerLibConstructor (
+  VOID
+  )
+{
+  UINT64  PciExpressBaseAddress;
+  FDT_CLIENT_PROTOCOL *FdtClient;
+  CONST UINT64*Reg;
+  UINT32  RegElemSize, RegSize;
+  EFI_STATUS  Status;
+
+  PciExpressBaseAddress = PcdGet64 (PcdPciExpressBaseAddress);
+  if (PciExpressBaseAddress != MAX_UINT64) {
+return EFI_SUCCESS;
+  }
+
+  Status = gBS->LocateProtocol (, NULL,
+  (VOID **));
+  ASSERT_EFI_ERROR (Status);
+
+  Status = FdtClient->FindCompatibleNodeReg (FdtClient,
+"pci-host-ecam-generic", (CONST VOID **),
+, );
+
+  if (EFI_ERROR (Status)) {
+PciExpressBaseAddress = 0;
+  } else {
+ASSERT (RegElemSize == sizeof (UINT64));
+PciExpressBaseAddress = SwapBytes64 (*Reg);
+  }
+
+  PcdSet64 (PcdPciExpressBaseAddress, PciExpressBaseAddress);
+  PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
+
+  return RETURN_SUCCESS;
+}
diff --git a/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf 
b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
new file mode 100644
index ..546125997ba6
--- /dev/null
+++ b/ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
@@ -0,0 +1,47 @@
+#/** @file
+#  FDT client library for consumers of PCI related dynamic PCDs
+#
+#  Copyright (c) 2016, Linaro Ltd. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution. The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#
+#**/
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = FdtPciPcdProducerLib
+  FILE_GUID  = D584275B-BF1E-4DF8-A53D-980F5645C5E7
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = PciPcdProducerLib|DXE_DRIVER UEFI_DRIVER
+  CONSTRUCTOR= FdtPciPcdProducerLibConstructor
+
+[Sources]
+  FdtPciPcdProducerLib.c
+
+[Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  PcdLib
+  UefiBootServicesTableLib
+
+[Protocols]
+  gFdtClientProtocolGuid  ## CONSUMES
+
+[Pcd]
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress   ## PRODUCES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration  ## PRODUCES
+
+[Depex]
+  gFdtClientProtocolGuid
-- 
2.5.0

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

[edk2] [PATCH 4/4] ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling

[Ard Biesheuvel]

Now that the PCI host bridge driver parses the DT node that describes
the PCI host bridge directly via the FDT client protocol, we can drop the
handling from VirtFdtDxe completely.

This means some PCI related PCDs are no longer set, such as PcdPciBusMin,
PcdPciBusMax, PcdPciIoBase, PcdPciIoSize, PcdPciIoTranslation,
PcdPciMmio32Base and PcdPciMmio32Size. Since these PCDs are specific to
ARM (and declared in ArmPlatformPkg), and not used anywhere else by the
ArmVirtPkg platforms, we can simply stop populating them, and drop all
references to them.

It also means that we can no longer rely on PcdPciExpressBaseAddress and
PcdPciDisableBusEnumeration to be set before they may be needed by
either PciBusDxe or QemuFwCfgAcpiPlatformDxe, so make those depend on
FdtPciPcdProducerLib explicitly via NULL library class resolution.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc   |  18 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc |  18 +-
 ArmVirtPkg/ArmVirtXen.dsc|  12 --
 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c   | 203 
 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf |   9 -
 5 files changed, 16 insertions(+), 244 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index f2eccf8cca63..c42092f54163 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -213,14 +213,6 @@ [PcdsDynamicDefault.common]
   ## PL031 RealTimeClock
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0
 
-  gArmPlatformTokenSpaceGuid.PcdPciBusMin|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciBusMax|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciIoBase|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciIoSize|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0
-
   # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
   # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x
@@ -370,7 +362,10 @@ [Components.common]
   # PCI support
   #
   ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
-  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
+
+  NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
   OvmfPkg/Virtio10Dxe/Virtio10.inf
 
@@ -397,4 +392,7 @@ [Components.AARCH64]
   # ACPI Support
   #
   MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
-  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf {
+
+  NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 107627e6a6f4..2cbeced695f9 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -198,14 +198,6 @@ [PcdsDynamicDefault.common]
   ## PL031 RealTimeClock
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase|0x0
 
-  gArmPlatformTokenSpaceGuid.PcdPciBusMin|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciBusMax|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciIoBase|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciIoSize|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0
-  gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0
-
   # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
   # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x
@@ -346,13 +338,19 @@ [Components.common]
   # ACPI Support
   #
   MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableDxe.inf
-  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf
+  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpiPlatformDxe.inf {
+
+  NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
 
   #
   # PCI support
   #
   ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf
-  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf {
+
+  NULL|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
+  }
   OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
   OvmfPkg/Virtio10Dxe/Virtio10.inf
 
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index d5057bb46326..112dc8ed33df 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -114,9 +114,6 @@ [PcdsPatchableInModule.common]
   gArmTokenSpaceGuid.PcdFvBaseAddress|0x0
 
 [PcdsDynamicDefault.common]
-  ## If TRUE, OvmfPkg/AcpiPlatformDxe will not wait for PCI
-  #  enumeration to complete before installing ACPI tables.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
 
   gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
   

[edk2] [PATCH 2/4] ArmVirtPkg/BaseCachingPciExpressLib: depend on PciPcdProducerLib

[Ard Biesheuvel]

Make BaseCachingPciExpressLib depend on PciPcdProducerLib, so that we
have a chance to populate PcdPciExpressBaseAddress based on the contents
of the device tree. 

Also update the platforms under ArmVirtPkg that support PCI to use this
special MAX_UINT64 as the build time default for PcdPciExpressBaseAddress,
and to specify a suitable resolution for PciPcdProducerLib.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 6 
+-
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 6 
+-
 ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf | 2 ++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index bb994262268f..f2eccf8cca63 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -66,6 +66,7 @@ [LibraryClasses.common]
 !if $(SECURE_BOOT_ENABLE) == TRUE
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
 !endif
+  
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -219,7 +220,10 @@ [PcdsDynamicDefault.common]
   gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
   gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0
   gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0
-  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x0
+
+  # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
+  # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x
 
   #
   # Set video resolution for boot options and for text setup.
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index d6c73d960805..107627e6a6f4 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -65,6 +65,7 @@ [LibraryClasses.common]
 !if $(SECURE_BOOT_ENABLE) == TRUE
   FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
 !endif
+  
PciPcdProducerLib|ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
 
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
@@ -204,7 +205,10 @@ [PcdsDynamicDefault.common]
   gArmPlatformTokenSpaceGuid.PcdPciIoTranslation|0x0
   gArmPlatformTokenSpaceGuid.PcdPciMmio32Base|0x0
   gArmPlatformTokenSpaceGuid.PcdPciMmio32Size|0x0
-  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x0
+
+  # set PcdPciExpressBaseAddress to MAX_UINT64, which signifies that this
+  # PCD and PcdPciDisableBusEnumeration above have not been assigned yet
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0x
 
   #
   # Set video resolution for boot options and for text setup.
diff --git 
a/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf 
b/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
index f6a346d49f22..b32554a111e6 100644
--- a/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
+++ b/ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
@@ -33,6 +33,7 @@ [Sources]
   PciExpressLib.c
 
 [Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
   MdePkg/MdePkg.dec
 
 [LibraryClasses]
@@ -40,6 +41,7 @@ [LibraryClasses]
   PcdLib
   DebugLib
   IoLib
+  PciPcdProducerLib
 
 [Pcd]
   gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress  ## CONSUMES
-- 
2.5.0

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

[edk2] [PATCH 3/4] ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol

[Ard Biesheuvel]

Instead of relying on VirtFdtDxe to populate various dynamic PCDs with
information retrieved from the host-provided device tree, perform the
PCI ECAM related DT node parsing directly in PciHostBridgeDxe.

Since we expect PcdPciExpressBaseAddress to have already been assigned
by the time this module's entry point is called, add a dependency on
PciPcdProducerLib as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c  | 261 ++--
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h  |   1 +
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |  14 +-
 3 files changed, 253 insertions(+), 23 deletions(-)

diff --git a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c 
b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
index 735c7f216318..eba6a7c47882 100644
--- a/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
+++ b/ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c
@@ -79,6 +79,230 @@ PCI_HOST_BRIDGE_INSTANCE mPciHostBridgeInstanceTemplate = {
 // Implementation
 //
 
+STATIC
+VOID
+SetLinuxPciProbeOnlyProperty (
+  IN FDT_CLIENT_PROTOCOL *FdtClient
+  )
+{
+  INT32 Node, Tmp;
+  EFI_STATUSStatus;
+
+  if (!FeaturePcdGet (PcdPureAcpiBoot)) {
+//
+// Set the /chosen/linux,pci-probe-only property to 1, so that the PCI
+// setup we will perform in the firmware is honored by the Linux OS,
+// rather than torn down and done from scratch. This is generally a more
+// sensible approach, and aligns with what ACPI based OSes do typically.
+//
+// In case we are exposing an emulated VGA PCI device to the guest, which
+// may subsequently get exposed via the Graphics Output protocol and
+// driven as an efifb by Linux, we need this setting to prevent the
+// framebuffer from becoming unresponsive.
+//
+Status = FdtClient->GetOrInsertChosenNode (FdtClient, );
+
+if (Status == EFI_SUCCESS) {
+  Tmp = SwapBytes32 (1);
+  Status = FdtClient->SetNodeProperty (FdtClient, Node,
+"linux,pci-probe-only", , sizeof (Tmp));
+}
+if (EFI_ERROR (Status)) {
+  DEBUG ((EFI_D_WARN,
+"Failed to set /chosen/linux,pci-probe-only property\n"));
+}
+  }
+}
+
+//
+// We expect the "ranges" property of "pci-host-ecam-generic" to consist of
+// records like this.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32 Type;
+  UINT64 ChildBase;
+  UINT64 CpuBase;
+  UINT64 Size;
+} DTB_PCI_HOST_RANGE_RECORD;
+#pragma pack ()
+
+#define DTB_PCI_HOST_RANGE_RELOCATABLE  BIT31
+#define DTB_PCI_HOST_RANGE_PREFETCHABLE BIT30
+#define DTB_PCI_HOST_RANGE_ALIASED  BIT29
+#define DTB_PCI_HOST_RANGE_MMIO32   BIT25
+#define DTB_PCI_HOST_RANGE_MMIO64   (BIT25 | BIT24)
+#define DTB_PCI_HOST_RANGE_IO   BIT24
+#define DTB_PCI_HOST_RANGE_TYPEMASK (BIT31 | BIT30 | BIT29 | BIT25 | BIT24)
+
+STATIC
+EFI_STATUS
+EFIAPI
+ProcessPciHost (
+  OUT  UINT64*IoBase,
+  OUT  UINT64*IoSize,
+  OUT  UINT64*IoTranslation,
+  OUT  UINT64*MmioBase,
+  OUT  UINT64*MmioSize,
+  OUT  UINT64*MmioTranslation,
+  OUT  UINT32*BusMin,
+  OUT  UINT32*BusMax
+  )
+{
+  FDT_CLIENT_PROTOCOL *FdtClient;
+  INT32   Node;
+  UINT64  ConfigBase, ConfigSize;
+  CONST VOID  *Prop;
+  UINT32  Len;
+  UINT32  RecordIdx;
+  EFI_STATUS  Status;
+
+  //
+  // The following output arguments are initialized only in
+  // order to suppress '-Werror=maybe-uninitialized' warnings
+  // *incorrectly* emitted by some gcc versions.
+  //
+  *IoBase = 0;
+  *IoTranslation = 0;
+  *MmioBase = 0;
+  *MmioTranslation = 0;
+  *BusMin = 0;
+  *BusMax = 0;
+
+  //
+  // *IoSize and *MmioSize are initialized to zero because the logic below
+  // requires it. However, since they are also affected by the issue reported
+  // above, they are initialized early.
+  //
+  *IoSize = 0;
+  *MmioSize = 0;
+
+  Status = gBS->LocateProtocol (, NULL,
+  (VOID **));
+  ASSERT_EFI_ERROR (Status);
+
+  Status = FdtClient->FindCompatibleNode (FdtClient, "pci-host-ecam-generic",
+);
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_INFO,
+  "%a: No 'pci-host-ecam-generic' compatible DT node found\n",
+  __FUNCTION__));
+return EFI_NOT_FOUND;
+  }
+
+  DEBUG_CODE (
+INT32 Tmp;
+
+//
+// A DT can legally describe multiple PCI host bridges, but we are not
+// equipped to deal with that. So assert that there is only one.
+//
+Status = FdtClient->FindNextCompatibleNode (FdtClient,
+  "pci-host-ecam-generic", Node, );
+ASSERT (Status == EFI_NOT_FOUND);
+  );
+
+  Status = FdtClient->GetNodeProperty (FdtClient, Node, "reg", , );
+  if (EFI_ERROR (Status) || Len != 2 * sizeof(UINT64)) {
+DEBUG ((EFI_D_ERROR, "%a: 'reg' property not 

[edk2] [PATCH 0/4] VirtFdtDxe refactoring and removal: the PCI edition

[Ard Biesheuvel]

These four patches have been split off from the series 'VirtFdtDxe refactoring
and removal' for which I sent out a v2 on Friday. This series deals with the
removal of PCI host bridge discovery from VirtFdtDxe in a way that ensures
that all dependencies on the various PCDs that VirtFdtDxe sets based on the
'pci-ecam-generic' DT node are still satisfied.

Ard Biesheuvel (4):
  ArmVirtPkg: implement FdtPciPcdProducerLib
  ArmVirtPkg/BaseCachingPciExpressLib: depend on PciPcdProducerLib
  ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol
  ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling

 ArmVirtPkg/ArmVirtQemu.dsc   |  22 
+-
 ArmVirtPkg/ArmVirtQemuKernel.dsc |  22 
+-
 ArmVirtPkg/ArmVirtXen.dsc|  12 
-
 ArmVirtPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf |   2 
+
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c   |  61 
+
 ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf |  47 

 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.c  | 261 
++--
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridge.h  |   1 
+
 ArmVirtPkg/PciHostBridgeDxe/PciHostBridgeDxe.inf |  14 
+-
 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.c   | 203 
---
 ArmVirtPkg/VirtFdtDxe/VirtFdtDxe.inf |   9 
-
 11 files changed, 387 insertions(+), 267 deletions(-)
 create mode 100644 
ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c
 create mode 100644 
ArmVirtPkg/Library/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf

-- 
2.5.0

___
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 06/04/16 14:39, Liuxilong (A) wrote:

  Multiple boards connect to the DHCP server through a switch .
The MAC of each board is the same and building VLAN through the switch
to isolate each board.


Don't do that.  Use a unique MAC address for each board.

If I understand you correctly, and all boards have the same MAC address, 
then I am not surprised that your DHCP server is behaving unexpectedly.


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

Re: [edk2] HII incompatibility between edk2 and iPXE?

[Michael Brown]

On 05/04/16 03:10, Dong, Eric wrote:

On a separate but related note: The ConfigHdr portion of Request and
Response seems to contain absolutely zero information by the time it
reaches EFI_HII_CONFIG_ACCESS_PROTOCOL.  As far as I can tell, it is
meaningful only to EFI_HII_CONFIG_ROUTING_PROTOCOL and a sensible API
design would never have exposed it to EFI_HII_CONFIG_ACCESS_PROTOCOL
instances.

Does the ConfigHdr have any meaning for EFI_HII_CONFIG_ACCESS_PROTOCOL,
or is it just a pointless blob of noise?


The GUID/NAME value in the ConfigHdr is get from the storage used in this 
driver. If one driver has more than one storage, the Guid + Name specify which 
storage info will be return.


Surely the EFI_HII_CONFIG_ACCESS_PROTOCOL's *This pointer already 
uniquely identifies the storage.


(If not, then why not?)

Thanks,

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

Re: [edk2] HII incompatibility between edk2 and iPXE?

[Michael Brown]

On 01/04/16 21:41, Laszlo Ersek wrote:

I am (and was) aware of "MdeModulePkg/Universal/DriverSampleDxe", the same 
driver that Eric recommended as an example in this thread earlier. In my opinion, that 
driver is supremely over-sized and -cluttered to be usable as a learning tool.


I'm glad it's not just me.  :)


I'm sorry that I couldn't help more. If we want to go with practicality (= "working 
code trumps precise documentation"), then you might want to consider applying the 
patch you proposed above. (Just don't add more angry comments pls.)


Thanks for all your detailed help in tracking this down.  I appreciate it.

I've pushed this workaround patch to iPXE:

  http://git.ipxe.org/ipxe.git/commitdiff/5238c85

along with a detailed (and non-angry) commit message.

Thanks again,

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/12/16 12:59, Laszlo Ersek wrote:

> I wonder what a good name could be, for this new plugin lib.
> PciPcdProducerLib?

Maybe stick in a reference to "Fdt" as well...

Laszlo

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

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

[Laszlo Ersek]

On 04/12/16 12:51, Dandan Bi wrote:

> diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
> b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c

> +  // ::=�OFFSET=��=�=�.

> +//  ::= �=� | �=�.

> +// ::=�OFFSET=��=�=� 
> style.

> +//  ::= �=� | �=� style.

Please do not cut n' paste non-ASCII unicode from other documents into
the source code.

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/12/16 12:15, Ard Biesheuvel wrote:
> On 11 April 2016 at 21:14, Laszlo Ersek  wrote:

>> * Patch
>>
>>   8528ac9292e4 ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol
>>
>>   also looks mostly good. Some comments:
>>
>>   - Please fix a typo in the commit message:
>>
>> s/host provided/host-provided/
>>
>>   - Also, the paragraph "This means some PCI related PCDs...", in the
>> commit message, belongs to the next patch
>> ("ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling"). Please
>> move it over.
>>
>>   - The ProcessPciHost() function's leading comment has become stale.
>> Please drop it.
>>
>>   - Please do not introduce those global variables just for
>> returning data from ProcessPciHost() to InitializePciHostBridge().
>> Use locals and an appropriate signature for ProcessPciHost(). (You
>> can collect those values in a new structure type too.)
>>
>> This might require you to bring back those explicit
>> zero-assignments, before the "ranges" loop.
>>
>>   - Since the MmioBase and MmioSize values are no longer kept in UINT32
>> PCDs (but UINT64 variables), the explicit (UINT64) cast is
>> unnecessary, in InitializePciHostBridge().
>>
>>   - The nullity checks in (EFI_ERROR (Status) || Prop == NULL) are
>> redundant, I think, after all of the GetNodeProperty() calls, in
>> ProcessPciHost().
>>
>>   - Can you factor out the setting of the /chosen node to a separate
>> function? (Together with the Tmp variable.)
>>
>>   - In this (v3-wip) iteration, you also removed the
>>
>> PcdSetBool (PcdPciDisableBusEnumeration, FALSE);
>>
>> call. I almost suggested to reinstate it, but doing so wouldn't be
>> safe actually.
>>
>> Namely, according to the build report, two drivers consume
>> PcdPciDisableBusEnumeration: PciBusDxe and
>> QemuFwCfgAcpiPlatformDxe.
>>
>> The PciBusDxe case is safe, because it is a UEFI driver model
>> driver that depends on protocols produced by PciHostBridgeDxe.
>> However, QemuFwCfgAcpiPlatformDxe is just a DXE_DRIVER, and it
>> consumes PcdPciDisableBusEnumeration first thing in its entry point.
>>
>> This means that you will have to introduce another plugin library
>> (in a separate patch) just for setting PcdPciDisableBusEnumeration,
>> and link it into both PciBusDxe and QemuFwCfgAcpiPlatformDxe.
>>
>> Also, in this plugin library, since the PCD is just a boolean, we
>> cannot use the caching trick (through a special value of the PCD),
>> hence minimally the presence of "pci-host-ecam-generic" will have to
>> be verified every time.
>>
> 
> Actually, looking at this again, wouldn't it make sense to have a
> single plugin library that sets both PcdPciDisableBusEnumeration and
> PcdPciExpressBaseAddress, and use it for these two modules and for the
> BaseCachingPciExpressLib?
> 

This could be a valid idea, but it requires some care.

- BaseCachingPciExpressLib will have to spell out the dependency on
  this additional library class explicitly, otherwise the order of
  constructors would not be ensured. In other words, for clients of
  BaseCachingPciExpressLib, the new library instance would not be
  "plugged", but regularly inherited.

  It should also mean that the source code of the
  BaseCachingPciExpressLib instance doesn't change (relative to
  master), only the [LibraryClasses] section does, in its INF file.

- For modules that need PcdPciDisableBusEnumeration, the plugin lib
  instance would have to remain a plugin. I'm unsure about the case
  when a module gets the plugin both implicitly and explicitly.

- Should the condition (PcdPciExpressBaseAddress != MAX_UINT64) imply
  that PcdPciDisableBusEnumeration has been finalized as well? Probably
  so, but this should be documented in the DSC files, near the
  MAX_UINT64 default value.

- The above co-caching saves some cycles, and we avoid yet another
  library class+instance. That's good. However, we should check how
  much potential is there for wasted work.

  For a client of PciExpressLib, the waste is only a single
  PcdSetBool(), which is negligible.

  For a client that needs only PcdPciDisableBusEnumeration, the waste
  is: a PcdSet64(), which is negligible, and the fact that we don't
  just look up the "pci-host-ecam-generic" node, but also grab its
  "reg" property (plus a SwapBytes64() call). I don't think it's bad.

  Clearly, the "waste" in the above cases can only be called "waste" if
  nothing in the firmware needs that "other" thing later. And even
  then, the "waste" is spent only once, due to the caching through
  (PcdPciExpressBaseAddress != MAX_UINT64).

So, I think this is worth a shot!

I wonder what a good name could be, for this new plugin lib.
PciPcdProducerLib?

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

[edk2] [patch] 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.


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

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 4ed4ecc..03098fd 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -508,10 +508,462 @@ 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  Value  Pointer to the value correspond to the 
BlockName.
+  @param  Found  The Block whether has been found.
+
+  @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*Value,
+  OUT BOOLEAN  *Found
+  )
+{
+  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, , );
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+ASSERT (TempBuffer != NULL);
+if (*TempBuffer == *Value) {
+  *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 ValueLength;
+  UINTN AppendSize;
+  UINTN TotalSize;
+  BOOLEAN   FoundOffset;
+
+  AppendString = NULL;
+  AltConfigHdrPtr = StrStr (DefaultAltCfgResp, AltConfigHdr);
+  ASSERT (AltConfigHdrPtr != NULL);
+  BlockPtr = StrStr (AltConfigHdrPtr, L"=");
+
+  while (BlockPtr != NULL) {
+//
+//Find the "=" block and get the value of the Number with 
AltConfigHdr in DefaultAltCfgResp.
+//
+BlockPtrStart = BlockPtr;
+BlockPtr += StrLen (L"=");
+Status = GetValueOfNumber (BlockPtr, , );
+if (EFI_ERROR (Status)) {
+  Status = EFI_OUT_OF_RESOURCES;
+  goto Exit;
+}
+//
+// To find the same "=" block in ConfigAltResp.
+//
+StringPtr = StrStr (*ConfigAltResp, AltConfigHdr);
+ASSERT (StringPtr != NULL);
+Status = FindSameBlockElement (StringPtr, L"=", TempBuffer, 
);
+if (EFI_ERROR (Status)) {
+  Status = EFI_OUT_OF_RESOURCES;
+  goto Exit;
+}
+if (!FoundOffset) {
+  //
+  // Don't find the same "=" block in ConfigAltResp.
+  // Calculate the size of .
+  // ::=�OFFSET=��=�=�.
+  //
+   

Re: [edk2] [PATCH v2] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Mark Rutland]

On Mon, Apr 11, 2016 at 06:50:06PM +0200, Ard Biesheuvel wrote:
> On 11 April 2016 at 18:47, Mark Rutland  wrote:
> > On Mon, Apr 11, 2016 at 06:08:29PM +0200, Ard Biesheuvel wrote:
> >> +//VOID
> >> +//ArmReplaceLiveTranslationEntry (
> >> +//  IN  UINT64  *Entry,
> >> +//  IN  UINT64  Value
> >> +//  )
> >> +ASM_PFX(ArmReplaceLiveTranslationEntry):
> >> +  .macro __replace_entry, el
> >> +  mrs   x8, sctlr_el\el
> >> +  and   x9, x8, #~CTRL_M_BIT  // Clear MMU enable bit
> >> +  msr   sctlr_el\el, x9
> >> +  isb
> >> +
> >> +  // write an invalid entry and invalidate it in the caches
> >> +  str   xzr, [x0]
> >> +  dmb   sy
> >> +  dcivac, x0
> >
> > Is it definitely the case that the line is not potentially dirty?
> >
> > If it is a possiblity, you need to invalidate first.
> >
> 
> Yes, that happens before this macro is invoked
> 
> >> +  .if   \el == 1
> >> +  tlbi  vmalle1
> >> +  .else
> >> +  tlbi  alle\el
> >> +  .endif
> >> +  dsb   sy
> >> +  msr   sctlr_el\el, x8
> >> +  isb
> >
> > We never did str x1, [x0], so we re-enable the MMU with the entry
> > invalid. If that's safe, then there's no point turning the MMU off in
> > the first place; this is just a convoluted BBM sequence
> >
> > I thought the problem was that the entry might be in active use by this
> > code (either mapping code or data). For that, you also need to create
> > the new entry before re-enabling the MMU.
> >
> 
> Indeed. I had spotted that myself, and mentioned it in my reply to self
> 
> > So long as speculation isn't a problem, you only need the prior
> > invalidate. Though for KVM you're at the mercy of the host's cacheable
> > alias regardless...
> >
> 
> Could you elaborate?

The KVM host has a linear alias of RAM, with the usual attributes we
expect (Normal, Inner Write-Back, Outer Write-Back, Inner Shareable).
It's possible for cache allocations to occur as a result of this
mapping, regardless of the state of the guest (e.g. even if the MMU is
off from its PoV).

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

[edk2] [patch] MdeModulePkg/HiiDatabaseDxe: Correct the ReallocatePool size

[Dandan Bi]

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

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
index 3a871cf..4ed4ecc 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c
@@ -598,15 +598,15 @@ MergeDefaultString (
 // Append the found default value string to the input AltCfgResp
 // 
 if (StringPtr == NULL) {
   StringPtrEnd   = StrStr (StringPtrDefault + 1, L"");
   SizeAltCfgResp = StrSize (*AltCfgResp);
-  TotalSize = SizeAltCfgResp + StrSize (StringPtrDefault);
   if (StringPtrEnd == NULL) {
 //
 // No more default string is found.
 //
+TotalSize = SizeAltCfgResp + StrSize (StringPtrDefault);
 *AltCfgResp= (EFI_STRING) ReallocatePool (
  SizeAltCfgResp,
  TotalSize,
  (VOID *) (*AltCfgResp)
  );
@@ -617,10 +617,11 @@ MergeDefaultString (
 StrCatS (*AltCfgResp, TotalSize / sizeof (CHAR16), StringPtrDefault);
 break;
   } else {
 TempChar = *StringPtrEnd;
 *StringPtrEnd = L'\0';
+TotalSize = SizeAltCfgResp + StrSize (StringPtrDefault);
 *AltCfgResp = (EFI_STRING) ReallocatePool (
  SizeAltCfgResp,
  TotalSize,
  (VOID *) (*AltCfgResp)
  );
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Laszlo Ersek]

On 04/12/16 12:10, Ard Biesheuvel wrote:
> On 11 April 2016 at 21:14, Laszlo Ersek  wrote:

>> * Can you please submit a separate patch that removes
>>   PcdPciExpressBaseAddress from "ArmVirtPkg/ArmVirtXen.dsc"?
>>
> 
> Well, this patch needs to wait after the PCI references are removed
> from VirtFdtDxe, and it is already removed by that patch. In fact, I
> should probably drop PcdPciDisableBusEnumeration from ArmVirtXen.dsc
> in that patch as well.

Okay.

>> * Patch
>>
>>   8528ac9292e4 ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol
>>
>>   also looks mostly good. Some comments:

> 
> OK, that all looks reasonable. I will send it out as a separate
> series, and once we are happy with those, I will repost the remaining
> patches.

That's what I thought as well.

Thanks!
Laszlo

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

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Ard Biesheuvel]

On 11 April 2016 at 21:14, Laszlo Ersek  wrote:
> On 04/11/16 18:22, Ard Biesheuvel wrote:
>> On 11 April 2016 at 16:50, Laszlo Ersek  wrote:
>>> On 04/11/16 16:34, Ard Biesheuvel wrote:
 On 11 April 2016 at 16:12, Laszlo Ersek  wrote:
> On 04/11/16 15:43, Ard Biesheuvel wrote:
>>>
>>> [snip]
>>>
 I simply want to check that the 'pci-ecam-generic' DT node we end up
 consuming is the only one that exists in the device tree. In fact, I
 think it makes sense to assert that PcdPciExpressBaseAddress equals
 the DT node reg property when we encounter it.
>>>
>>> Yes, that definitely makes sense (at least if I understand it
>>> correctly). That is, the library constructor would do the "reg" parsing
>>> / PCD setting / caching etc (for any DXE_DRIVER that uses this instance,
>>> really), and *specifically* PciHostBridgeDxe, when it parses the same
>>> node (for those other two properties), could also look at "reg", and
>>> assert that PcdPciExpressBaseAddress is *already set* the way it expects
>>> it to be. I think that's a good idea.
>>>
> Do you have a fresh v3-wip branch somewhere? I'd like to try expressing
> my proposal for patches #13 and #14 in code. (If you are okay with that.)
>

 Sure, give me 30 mins.
>>>
>>> Let's do the following instead (as we discussed off-list):
>>>
>>> Please commit patches v2 1-10 (with the fixes I requested) to the master
>>> branch, and then we can collaborate, on the list, with patches for
>>> fw_cfg and PciExpressLib / PciHostBridgeDxe, directly against the master
>>> branch.
>>>
>>
>> OK, wip branch is here
>> git://git.linaro.org/people/ard.biesheuvel/uefi-next.git virt-fdt-refactor-v3
>
> Great, thank you.
>
> * Patch
>
>   f3089d080cd0 ArmVirtPkg/QemuFwCfgLib: move to FDT client protocol
>
>   is exactly what I had in mind.
>
>   Please modify the commit message (as I pointed out previously, in the
>   v2 11/24 sub-thread here) to include "SmbiosPlatformDxe" in the list
>   of client drivers, plus update the count "three" to "four". With that,
>   you can add my
>
>   Reviewed-by: Laszlo Ersek 
>
>   to the patch.
>
>   I think it's also fine to push the first two patches to master
>   afterwards ("ArmVirtPkg/QemuFwCfgLib: move to FDT client protocol"
>   and "ArmVirtPkg/VirtFdtDxe: remove handling of fw_cfg DT node"
>   (currently a889387fee53)).
>
> After pushing these two patches above (with my requested updates in the
> first one), can you please submit a smaller, separate series that
> focuses on the conversion of the PCI host node exclusively? Please see
> my comments below:
>
> * Patch
>
>   60471256eb10 ArmVirtPkg/BaseCachingPciExpressLib: convert to FDT
>client protocol
>
>   is also precisely what I had in mind.
>
>   - There is one typo in the commit message (with two instances):
>
> s/PciExpressBaseAddress/PcdPciExpressBaseAddress/
>
>   - Also, please add a minimal comment above the 0x
> default values in the two DSC files, saying that the MAX_UINT64
> value means "undiscovered".
>
>   - Furthermore, please modify the comment on PcdPciExpressBaseAddress
> in the INF file like this:
>
> ## CONSUMES ## SOMETIMES_PRODUCES
>
>   With those tweaked:
>
>   Reviewed-by: Laszlo Ersek 
>
>   (However, this shouldn't be pushed without the rest below.)
>
> * Can you please submit a separate patch that removes
>   PcdPciExpressBaseAddress from "ArmVirtPkg/ArmVirtXen.dsc"?
>
> * Patch
>
>   8528ac9292e4 ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol
>
>   also looks mostly good. Some comments:
>
>   - Please fix a typo in the commit message:
>
> s/host provided/host-provided/
>
>   - Also, the paragraph "This means some PCI related PCDs...", in the
> commit message, belongs to the next patch
> ("ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling"). Please
> move it over.
>
>   - The ProcessPciHost() function's leading comment has become stale.
> Please drop it.
>
>   - Please do not introduce those global variables just for
> returning data from ProcessPciHost() to InitializePciHostBridge().
> Use locals and an appropriate signature for ProcessPciHost(). (You
> can collect those values in a new structure type too.)
>
> This might require you to bring back those explicit
> zero-assignments, before the "ranges" loop.
>
>   - Since the MmioBase and MmioSize values are no longer kept in UINT32
> PCDs (but UINT64 variables), the explicit (UINT64) cast is
> unnecessary, in InitializePciHostBridge().
>
>   - The nullity checks in (EFI_ERROR (Status) || Prop == NULL) are
> redundant, I think, after all of the GetNodeProperty() calls, in
> ProcessPciHost().
>
>   - Can you factor out the setting of the /chosen node to a separate
> function? (Together with the Tmp variable.)
>
>   - In this (v3-wip) iteration, you also 

Re: [edk2] [PATCH v2 13/24] ArmVirtPkg/BaseCachingPciExpressLib: construct at first invocation

[Ard Biesheuvel]

On 11 April 2016 at 21:14, Laszlo Ersek  wrote:
> On 04/11/16 18:22, Ard Biesheuvel wrote:
>> On 11 April 2016 at 16:50, Laszlo Ersek  wrote:
>>> On 04/11/16 16:34, Ard Biesheuvel wrote:
 On 11 April 2016 at 16:12, Laszlo Ersek  wrote:
> On 04/11/16 15:43, Ard Biesheuvel wrote:
>>>
>>> [snip]
>>>
 I simply want to check that the 'pci-ecam-generic' DT node we end up
 consuming is the only one that exists in the device tree. In fact, I
 think it makes sense to assert that PcdPciExpressBaseAddress equals
 the DT node reg property when we encounter it.
>>>
>>> Yes, that definitely makes sense (at least if I understand it
>>> correctly). That is, the library constructor would do the "reg" parsing
>>> / PCD setting / caching etc (for any DXE_DRIVER that uses this instance,
>>> really), and *specifically* PciHostBridgeDxe, when it parses the same
>>> node (for those other two properties), could also look at "reg", and
>>> assert that PcdPciExpressBaseAddress is *already set* the way it expects
>>> it to be. I think that's a good idea.
>>>
> Do you have a fresh v3-wip branch somewhere? I'd like to try expressing
> my proposal for patches #13 and #14 in code. (If you are okay with that.)
>

 Sure, give me 30 mins.
>>>
>>> Let's do the following instead (as we discussed off-list):
>>>
>>> Please commit patches v2 1-10 (with the fixes I requested) to the master
>>> branch, and then we can collaborate, on the list, with patches for
>>> fw_cfg and PciExpressLib / PciHostBridgeDxe, directly against the master
>>> branch.
>>>
>>
>> OK, wip branch is here
>> git://git.linaro.org/people/ard.biesheuvel/uefi-next.git virt-fdt-refactor-v3
>
> Great, thank you.
>
> * Patch
>
>   f3089d080cd0 ArmVirtPkg/QemuFwCfgLib: move to FDT client protocol
>
>   is exactly what I had in mind.
>
>   Please modify the commit message (as I pointed out previously, in the
>   v2 11/24 sub-thread here) to include "SmbiosPlatformDxe" in the list
>   of client drivers, plus update the count "three" to "four". With that,
>   you can add my
>
>   Reviewed-by: Laszlo Ersek 
>
>   to the patch.
>
>   I think it's also fine to push the first two patches to master
>   afterwards ("ArmVirtPkg/QemuFwCfgLib: move to FDT client protocol"
>   and "ArmVirtPkg/VirtFdtDxe: remove handling of fw_cfg DT node"
>   (currently a889387fee53)).
>
> After pushing these two patches above (with my requested updates in the
> first one),

Done

> can you please submit a smaller, separate series that
> focuses on the conversion of the PCI host node exclusively? Please see
> my comments below:
>
> * Patch
>
>   60471256eb10 ArmVirtPkg/BaseCachingPciExpressLib: convert to FDT
>client protocol
>
>   is also precisely what I had in mind.
>
>   - There is one typo in the commit message (with two instances):
>
> s/PciExpressBaseAddress/PcdPciExpressBaseAddress/
>
>   - Also, please add a minimal comment above the 0x
> default values in the two DSC files, saying that the MAX_UINT64
> value means "undiscovered".
>
>   - Furthermore, please modify the comment on PcdPciExpressBaseAddress
> in the INF file like this:
>
> ## CONSUMES ## SOMETIMES_PRODUCES
>
>   With those tweaked:
>
>   Reviewed-by: Laszlo Ersek 
>
>   (However, this shouldn't be pushed without the rest below.)
>

OK

> * Can you please submit a separate patch that removes
>   PcdPciExpressBaseAddress from "ArmVirtPkg/ArmVirtXen.dsc"?
>

Well, this patch needs to wait after the PCI references are removed
from VirtFdtDxe, and it is already removed by that patch. In fact, I
should probably drop PcdPciDisableBusEnumeration from ArmVirtXen.dsc
in that patch as well.

> * Patch
>
>   8528ac9292e4 ArmVirtPkg/PciHostBridgeDxe: move to FDT client protocol
>
>   also looks mostly good. Some comments:
>
>   - Please fix a typo in the commit message:
>
> s/host provided/host-provided/
>
>   - Also, the paragraph "This means some PCI related PCDs...", in the
> commit message, belongs to the next patch
> ("ArmVirtPkg/VirtFdtDxe: drop PCI host bridge handling"). Please
> move it over.
>
>   - The ProcessPciHost() function's leading comment has become stale.
> Please drop it.
>
>   - Please do not introduce those global variables just for
> returning data from ProcessPciHost() to InitializePciHostBridge().
> Use locals and an appropriate signature for ProcessPciHost(). (You
> can collect those values in a new structure type too.)
>
> This might require you to bring back those explicit
> zero-assignments, before the "ranges" loop.
>
>   - Since the MmioBase and MmioSize values are no longer kept in UINT32
> PCDs (but UINT64 variables), the explicit (UINT64) cast is
> unnecessary, in InitializePciHostBridge().
>
>   - The nullity checks in (EFI_ERROR (Status) || Prop == NULL) are
> redundant, I 

[edk2] [PATCH] PerformancePkg: Make Dp print help information with -? flag in Shell.

[Qiu Shumin]

Since Shell supports finding help information from resource section
of application image. We enhance the Dp to add help information
string. After the Dp are loaded in system the help string will
be stored in resource section of the application image.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin 
---
 PerformancePkg/Dp_App/Dp.c  | 369 +++-
 PerformancePkg/Dp_App/Dp.inf|   9 +-
 PerformancePkg/Dp_App/DpStrings.uni |  74 ++--
 3 files changed, 262 insertions(+), 190 deletions(-)

diff --git a/PerformancePkg/Dp_App/Dp.c b/PerformancePkg/Dp_App/Dp.c
index b24a0de..e5c035e 100644
--- a/PerformancePkg/Dp_App/Dp.c
+++ b/PerformancePkg/Dp_App/Dp.c
@@ -13,7 +13,7 @@
   Dp uses this information to group records in different ways.  It also uses
   timer information to calculate elapsed time for each measurement.
  
-  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
   (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -25,12 +25,14 @@
 **/
 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -42,6 +44,16 @@
 #include "DpInternal.h"
 
 //
+// String token ID of help message text.
+// Shell supports to find help message in the resource section of an 
application image if
+// .MAN file is not found. This global variable is added to make build tool 
recognizes
+// that the help string is consumed by user and then build tool will add the 
string into
+// the resource section. Thus the application can use '-?' option to show help 
message in
+// Shell.
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_STRING_ID mDpStrEngHelpTokenId = 
STRING_TOKEN (STR_DP_HELP_INFORMATION);
+
+//
 /// Module-Global Variables
 ///@{
 EFI_HII_HANDLE   gHiiHandle;
@@ -69,9 +81,6 @@ PERF_CUM_DATA CumData[] = {
 UINT32 const  NumCum = sizeof(CumData) / sizeof(PERF_CUM_DATA);
 
 PARAM_ITEM_LIST  ParamList[] = {
-  {STRING_TOKEN (STR_DP_OPTION_QH), TypeFlag},   // -?   Help
-  {STRING_TOKEN (STR_DP_OPTION_LH), TypeFlag},   // -h   Help
-  {STRING_TOKEN (STR_DP_OPTION_UH), TypeFlag},   // -H   Help
   {STRING_TOKEN (STR_DP_OPTION_LV), TypeFlag},   // -v   Verbose Mode
   {STRING_TOKEN (STR_DP_OPTION_UA), TypeFlag},   // -A   All, Cooked
   {STRING_TOKEN (STR_DP_OPTION_UR), TypeFlag},   // -R   RAW All
@@ -217,21 +226,19 @@ InitializeDp (
   EFI_STRINGStringPtr;
   UINTN Number2Display;
 
-  EFI_STATUSStatus;
-  BOOLEAN   SummaryMode;
-  BOOLEAN   VerboseMode;
-  BOOLEAN   AllMode;
-  BOOLEAN   RawMode;
-  BOOLEAN   TraceMode;
-  BOOLEAN   ProfileMode;
-  BOOLEAN   ExcludeMode;
-  BOOLEAN   CumulativeMode;
-  CONST CHAR16  *CustomCumulativeToken;
-  PERF_CUM_DATA *CustomCumulativeData;
-
-  EFI_STRINGStringDpOptionQh;
-  EFI_STRINGStringDpOptionLh;
-  EFI_STRINGStringDpOptionUh;
+  EFI_STATUSStatus;
+  BOOLEAN   SummaryMode;
+  BOOLEAN   VerboseMode;
+  BOOLEAN   AllMode;
+  BOOLEAN   RawMode;
+  BOOLEAN   TraceMode;
+  BOOLEAN   ProfileMode;
+  BOOLEAN   ExcludeMode;
+  BOOLEAN   CumulativeMode;
+  CONST CHAR16  *CustomCumulativeToken;
+  PERF_CUM_DATA *CustomCumulativeData;
+  EFI_HII_PACKAGE_LIST_HEADER   *PackageList;
+
   EFI_STRINGStringDpOptionLv;
   EFI_STRINGStringDpOptionUs;
   EFI_STRINGStringDpOptionLs;
@@ -255,9 +262,6 @@ InitializeDp (
   CumulativeMode = FALSE;
   CustomCumulativeData = NULL;
 
-  StringDpOptionQh = NULL;
-  StringDpOptionLh = NULL;
-  StringDpOptionUh = NULL;
   StringDpOptionLv = NULL;
   StringDpOptionUs = NULL;
   StringDpOptionLs = NULL;
@@ -277,10 +281,35 @@ InitializeDp (
   //
   Ticker  = GetPerformanceCounter ();
 
-  // Register our string package with HII and return the handle to it.
   //
-  gHiiHandle = HiiAddPackages (, ImageHandle, DPStrings, 
NULL);
+  // Retrieve HII package list from ImageHandle
+  //
+  Status = gBS->OpenProtocol (
+  ImageHandle,
+  ,
+  (VOID **) ,
+  ImageHandle,
+  NULL,
+  EFI_OPEN_PROTOCOL_GET_PROTOCOL
+  );
+  if (EFI_ERROR (Status)) {
+

[edk2] [PATCH] MdePkg: Add EFI Erase Block Protocol definitions

[Hao Wu]

This protocol is newly introduced in UEFI 2.6 spec.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdePkg/Include/Protocol/EraseBlock.h | 105 +++
 MdePkg/MdePkg.dec|   3 +
 2 files changed, 108 insertions(+)
 create mode 100644 MdePkg/Include/Protocol/EraseBlock.h

diff --git a/MdePkg/Include/Protocol/EraseBlock.h 
b/MdePkg/Include/Protocol/EraseBlock.h
new file mode 100644
index 000..79e84bb
--- /dev/null
+++ b/MdePkg/Include/Protocol/EraseBlock.h
@@ -0,0 +1,105 @@
+/** @file
+  This file defines the EFI Erase Block Protocol.
+
+  Copyright (c) 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.
+
+  @par Revision Reference:
+  This Protocol is introduced in UEFI Specification 2.6
+
+**/
+
+#ifndef __EFI_ERASE_BLOCK_PROTOCOL_H__
+#define __EFI_ERASE_BLOCK_PROTOCOL_H__
+
+#define EFI_ERASE_BLOCK_PROTOCOL_GUID \
+  { \
+0x95a9a93e, 0xa86e, 0x4926, { 0xaa, 0xef, 0x99, 0x18, 0xe7, 0x72, 0xd9, 
0x87 } \
+  }
+
+typedef struct _EFI_ERASE_BLOCK_PROTOCOL EFI_ERASE_BLOCK_PROTOCOL;
+
+#define EFI_ERASE_BLOCK_PROTOCOL_REVISION ((2<<16) | (60))
+
+///
+/// EFI_ERASE_BLOCK_TOKEN
+///
+typedef struct {
+  //
+  // If Event is NULL, then blocking I/O is performed. If Event is not NULL and
+  // non-blocking I/O is supported, then non-blocking I/O is performed, and
+  // Event will be signaled when the erase request is completed.
+  //
+  EFI_EVENT Event;
+  //
+  // Defines whether the signaled event encountered an error.
+  //
+  EFI_STATUSTransactionStatus;
+} EFI_ERASE_BLOCK_TOKEN;
+
+/**
+  Erase a specified number of device blocks.
+
+  @param[in]   This   Indicates a pointer to the calling context.
+  @param[in]   MediaIdThe media ID that the erase request is for.
+  @param[in]   LBAThe starting logical block address to be
+  erased. The caller is responsible for erasing
+  only legitimate locations.
+  @param[in, out]  Token  A pointer to the token associated with the
+  transaction.
+  @param[in]   Size   The size in bytes to be erased. This must be
+  a multiple of the physical block size of the
+  device.
+
+  @retval EFI_SUCCESS The erase request was queued if Event is not
+  NULL. The data was erased correctly to the
+  device if the Event is NULL.to the device.
+  @retval EFI_WRITE_PROTECTED The device cannot be erased due to write
+  protection.
+  @retval EFI_DEVICE_ERRORThe device reported an error while attempting
+  to perform the erase operation.
+  @retval EFI_INVALID_PARAMETER   The erase request contains LBAs that are not
+  valid.
+  @retval EFI_NO_MEDIAThere is no media in the device.
+  @retval EFI_MEDIA_CHANGED   The MediaId is not for the current media.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_BLOCK_ERASE) (
+  IN EFI_ERASE_BLOCK_PROTOCOL  *This,
+  IN UINT32MediaId,
+  IN EFI_LBA   LBA,
+  IN OUT EFI_ERASE_BLOCK_TOKEN *Token,
+  IN UINTN Size
+  );
+
+///
+/// The EFI Erase Block Protocol provides the ability for a device to expose
+/// erase functionality. This optional protocol is installed on the same handle
+/// as the EFI_BLOCK_IO_PROTOCOL or EFI_BLOCK_IO2_PROTOCOL.
+///
+struct _EFI_ERASE_BLOCK_PROTOCOL {
+  //
+  // The revision to which the EFI_ERASE_BLOCK_PROTOCOL adheres. All future
+  // revisions must be backwards compatible. If a future version is not
+  // backwards compatible, it is not the same GUID.
+  //
+  UINT64 Revision;
+  //
+  // Returns the erase length granularity as a number of logical blocks. A
+  // value of 1 means the erase granularity is one logical block.
+  //
+  UINT32 EraseLengthGranularity;
+  EFI_BLOCK_ERASEEraseBlocks;
+};
+
+extern EFI_GUID gEfiEraseBlockProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 10de299..458d568 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1628,6 +1628,9 @@
   ## Include/Protocol/SdMmcPassThru.h
   gEfiSdMmcPassThruProtocolGuid= { 0x716ef0d9, 0xff83, 0x4f69, {0x81, 

Re: [edk2] [Patch 1/3] SecurityPkg: Update protocol usage in module INF files.

[Zhang, Chao B]

Reviewed-by: Chao Zhang 





Thanks & Best regards
Chao Zhang

-Original Message-
From: Gao, Liming 
Sent: Thursday, April 07, 2016 2:54 PM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B
Subject: [Patch 1/3] SecurityPkg: Update protocol usage in module INF files.

Update TCG and Library module uses gEdkiiVariableLockProtocolGuid as 
SOMETIMES_CONSUMES instead of CONSUMES to follow the code logic.

Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf   | 6 +++---
 .../Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.inf | 6 +++---
 .../DxeTrEEPhysicalPresenceLib/DxeTrEEPhysicalPresenceLib.inf   | 6 +++---
 SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf| 2 +-
 SecurityPkg/Tcg/TrEEConfig/TrEEConfigDxe.inf| 2 +-
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf 
b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
index bad4fe4..f4aa0da 100644
--- 
a/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
+++ b/SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPres
+++ enceLib.inf
@@ -57,11 +57,11 @@
   Tcg2PpVendorLib
 
 [Protocols]
-  gEfiTcg2ProtocolGuid ## CONSUMES
-  gEdkiiVariableLockProtocolGuid   ## CONSUMES
+  gEfiTcg2ProtocolGuid ## SOMETIMES_CONSUMES
+  gEdkiiVariableLockProtocolGuid   ## SOMETIMES_CONSUMES
 
 [Guids]
-  ## CONSUMES   ## HII
+  ## SOMETIMES_CONSUMES ## HII
   ## SOMETIMES_PRODUCES ## Variable:L"Tcg2PhysicalPresence"
   ## SOMETIMES_CONSUMES ## Variable:L"Tcg2PhysicalPresence"
   ## SOMETIMES_PRODUCES ## Variable:L"Tcg2PhysicalPresenceFlags"
diff --git 
a/SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.inf 
b/SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.inf
index b48b887..3aacba5 100644
--- 
a/SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.inf
+++ b/SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresen
+++ ceLib.inf
@@ -57,11 +57,11 @@
   TcgPpVendorLib
 
 [Protocols]
-  gEfiTcgProtocolGuid   ## CONSUMES
-  gEdkiiVariableLockProtocolGuid## CONSUMES
+  gEfiTcgProtocolGuid   ## SOMETIMES_CONSUMES
+  gEdkiiVariableLockProtocolGuid## SOMETIMES_CONSUMES
 
 [Guids]
-  ## CONSUMES   ## HII
+  ## SOMETIMES_CONSUMES ## HII
   ## SOMETIMES_PRODUCES ## Variable:L"PhysicalPresence"
   ## SOMETIMES_CONSUMES ## Variable:L"PhysicalPresence"
   ## SOMETIMES_PRODUCES ## Variable:L"PhysicalPresenceFlags"
diff --git 
a/SecurityPkg/Library/DxeTrEEPhysicalPresenceLib/DxeTrEEPhysicalPresenceLib.inf 
b/SecurityPkg/Library/DxeTrEEPhysicalPresenceLib/DxeTrEEPhysicalPresenceLib.inf
index 0612226..1c123ef 100644
--- 
a/SecurityPkg/Library/DxeTrEEPhysicalPresenceLib/DxeTrEEPhysicalPresenceLib.inf
+++ b/SecurityPkg/Library/DxeTrEEPhysicalPresenceLib/DxeTrEEPhysicalPres
+++ enceLib.inf
@@ -57,11 +57,11 @@
   TrEEPpVendorLib
 
 [Protocols]
-  gEfiTrEEProtocolGuid ## CONSUMES
-  gEdkiiVariableLockProtocolGuid   ## CONSUMES
+  gEfiTrEEProtocolGuid ## SOMETIMES_CONSUMES
+  gEdkiiVariableLockProtocolGuid   ## SOMETIMES_CONSUMES
 
 [Guids]
-  ## CONSUMES   ## HII
+  ## SOMETIMES_CONSUMES ## HII
   ## SOMETIMES_PRODUCES ## Variable:L"PhysicalPresence"
   ## SOMETIMES_CONSUMES ## Variable:L"PhysicalPresence"
   ## SOMETIMES_PRODUCES ## Variable:L"PhysicalPresenceFlags"
diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf 
b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
index dd2247b..d9340d6 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
@@ -70,7 +70,7 @@
 [Protocols]
   gEfiHiiConfigAccessProtocolGuid   ## PRODUCES
   gEfiDevicePathProtocolGuid## PRODUCES
-  gEdkiiVariableLockProtocolGuid## CONSUMES
+  gEdkiiVariableLockProtocolGuid## SOMETIMES_CONSUMES
   gEfiTcg2ProtocolGuid  ## CONSUMES
 
 [Pcd]
diff --git a/SecurityPkg/Tcg/TrEEConfig/TrEEConfigDxe.inf 
b/SecurityPkg/Tcg/TrEEConfig/TrEEConfigDxe.inf
index 9935e40..368570a 100644
--- a/SecurityPkg/Tcg/TrEEConfig/TrEEConfigDxe.inf
+++ b/SecurityPkg/Tcg/TrEEConfig/TrEEConfigDxe.inf
@@ -72,7 +72,7 @@
 [Protocols]
   gEfiHiiConfigAccessProtocolGuid   ## PRODUCES
   gEfiDevicePathProtocolGuid## PRODUCES
-  gEdkiiVariableLockProtocolGuid## CONSUMES
+  gEdkiiVariableLockProtocolGuid## SOMETIMES_CONSUMES
 
 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid## CONSUMES
--
2.8.0.windows.1

[edk2] [PATCH v3] ArmPkg/AArch64Mmu: disable MMU during page table manipulations

[Ard Biesheuvel]

On ARM, manipulating live page tables is cumbersome since the architecture
mandates the use of break-before-make, i.e., replacing a block entry with
a table entry requires an intermediate step via an invalid entry, or TLB
conflicts may occur.

Since it is not generally feasible to decide in the page table manipulation
routines whether such an invalid entry will result in those routines
themselves to become unavailable, use a function that is callable with
the MMU off (i.e., a leaf function that does not access the stack) to
perform the change of a block entry into a table entry.

Note that the opposite should never occur, i.e., table entries are never
coalesced into block entries.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Include/Library/ArmLib.h   |  6 +++
 ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf  |  5 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c | 36 +
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c| 17 +-
 ArmPkg/Library/ArmLib/AArch64/AArch64Support.S| 57 
 5 files changed, 119 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
index 15f610d82e1d..1689f0072db6 100644
--- a/ArmPkg/Include/Library/ArmLib.h
+++ b/ArmPkg/Include/Library/ArmLib.h
@@ -613,4 +613,10 @@ ArmClearMemoryRegionReadOnly (
   IN  UINT64Length
   );
 
+VOID
+ArmReplaceLiveTranslationEntry (
+  IN  UINT64  *Entry,
+  IN  UINT64  Value
+  );
+
 #endif // __ARM_LIB__
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
index dd585dea91fb..58684e8492f2 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Lib.inf
@@ -17,9 +17,10 @@ [Defines]
   INF_VERSION= 0x00010005
   BASE_NAME  = AArch64Lib
   FILE_GUID  = ef20ddf5-b334-47b3-94cf-52ff44c29138
-  MODULE_TYPE= DXE_DRIVER
+  MODULE_TYPE= BASE
   VERSION_STRING = 1.0
   LIBRARY_CLASS  = ArmLib
+  CONSTRUCTOR= AArch64LibConstructor
 
 [Sources.AARCH64]
   AArch64Lib.c
@@ -31,6 +32,7 @@ [Sources.AARCH64]
 
   ../Common/AArch64/ArmLibSupport.S
   ../Common/ArmLib.c
+  AArch64LibConstructor.c
 
 [Packages]
   ArmPkg/ArmPkg.dec
@@ -38,6 +40,7 @@ [Packages]
 
 [LibraryClasses]
   MemoryAllocationLib
+  CacheMaintenanceLib
 
 [Protocols]
   gEfiCpuArchProtocolGuid
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
new file mode 100644
index ..d2d0d3c15ee3
--- /dev/null
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64LibConstructor.c
@@ -0,0 +1,36 @@
+#/* @file
+#
+#  Copyright (c) 2016, Linaro Limited. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution.  The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php
+#
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+#*/
+
+#include 
+
+#include 
+#include 
+
+RETURN_STATUS
+EFIAPI
+AArch64LibConstructor (
+  VOID
+  )
+{
+  extern UINT32 ArmReplaceLiveTranslationEntrySize;
+
+  //
+  // The ArmReplaceLiveTranslationEntry () helper function may be invoked
+  // with the MMU off so we have to ensure that it gets cleaned to the PoC
+  //
+  WriteBackDataCacheRange (ArmReplaceLiveTranslationEntry,
+ArmReplaceLiveTranslationEntrySize);
+
+  return RETURN_SUCCESS;
+}
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index b7d23c6f3286..2cc6fc45aecf 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -169,6 +169,20 @@ GetRootTranslationTableInfo (
 
 STATIC
 VOID
+ReplaceLiveEntry (
+  IN  UINT64  *Entry,
+  IN  UINT64  Value
+  )
+{
+  if (!ArmMmuEnabled ()) {
+*Entry = Value;
+  } else {
+ArmReplaceLiveTranslationEntry (Entry, Value);
+  }
+}
+
+STATIC
+VOID
 LookupAddresstoRootTable (
   IN  UINT64  MaxAddress,
   OUT UINTN  *T0SZ,
@@ -330,7 +344,8 @@ GetBlockEntryListFromAddress (
 }
 
 // Fill the BlockEntry with the new TranslationTable
-*BlockEntry = ((UINTN)TranslationTable & 
TT_ADDRESS_MASK_DESCRIPTION_TABLE) | TableAttributes | TT_TYPE_TABLE_ENTRY;
+ReplaceLiveEntry (BlockEntry,
+  ((UINTN)TranslationTable & TT_ADDRESS_MASK_DESCRIPTION_TABLE) | 
TableAttributes | TT_TYPE_TABLE_ENTRY);
   }
 } else {
   if (IndexLevel != PageLevel) {
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Support.S