Re: [edk2] [PATCH 1/9] edksetup.sh: Create the Conf directory if necessary

[Gao, Liming]

Andrew:
  Now, Build tool will report error message if Conf directory or build 
configuration file is not found. It can detect the wrong setting when user 
specifies Conf by mistake. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Saturday, March 05, 2016 2:51 AM
> To: Leahy, Leroy P
> Cc: Justen, Jordan L; edk2-devel@lists.01.org; Bjorge, Erik C
> Subject: Re: [edk2] [PATCH 1/9] edksetup.sh: Create the Conf directory if
> necessary
> 
> 
> > On Mar 4, 2016, at 8:54 AM, Lee Leahy  wrote:
> >
> > Edit the shell script to determine if the Conf directory is present.  If
> > not then create the Conf directory.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Lee Leahy 
> > ---
> > edksetup.sh | 4 
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/edksetup.sh b/edksetup.sh
> > index 57368b5..d89ef9d 100755
> > --- a/edksetup.sh
> > +++ b/edksetup.sh
> > @@ -72,6 +72,10 @@ function SetWorkspace()
> >
> > function SetupEnv()
> > {
> > +  if [ ! -d "$WORKSPACE/Conf" ]
> > +  then
> > +mkdir $WORKSPACE/Conf
> > +  fi
> 
> This comment is not related to this patch.
> 
> What is the process to propose a change to the edk2 build specification?
> 
> I think the creation of Conf/ if it does not exist should be moved to build.py
> (build.exe). The copy of the *.template files to Conf/*.txt could also be
> moved into build.py.
> 
> The reason I advocate this is we have started using the build --
> conf=CONFDIRECTORY flag, and we point CONFDIRECTORY to the build
> output directory. This enables building different platforms in parallel. It 
> also
> makes it possible to write a top level GNUmakefile and not have to call any
> shell scripts to build. Basically the top level makefile can do a `export
> WORKSPACE, export EDK_TOOLS_PATH, export PATH` and it is possible to
> avoid calling any scripts to setup the environment.
> 
> Thus moving the Conf/ processing to build.py (build.exe) makes it more
> convenient to use the --conf flag, and helps make the setup scripts less
> complex.
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> >   if [ -n "$EDK_TOOLS_PATH" ]
> >   then
> > . $EDK_TOOLS_PATH/BuildEnv $*
> > --
> > 1.9.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

[edk2] [PATCH v2] MdeModulePkg: Change the type of PcdMaxPeiPerformanceLogEntries to UINT16

[Cinnamon Shia]

Change the type of PcdMaxPeiPerformanceLogEntries from UINT8 to UINT16 to
log more than 255 performance entries in PEI.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
 .../DxeCorePerformanceLib/DxeCorePerformanceLib.c|  5 -
 .../DxeCorePerformanceLib/DxeCorePerformanceLib.inf  |  2 ++
 .../Library/PeiPerformanceLib/PeiPerformanceLib.c| 20 +++-
 .../Library/PeiPerformanceLib/PeiPerformanceLib.inf  |  2 ++
 MdeModulePkg/MdeModulePkg.dec|  9 +
 5 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c 
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
index 0eb8e57..bbad2e9 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
@@ -11,6 +11,7 @@
   Performance Protocol is installed at the very beginning of DXE phase.
 
 Copyright (c) 2006 - 2015, 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
@@ -522,7 +523,9 @@ DxeCorePerformanceLibConstructor (
   );
   ASSERT_EFI_ERROR (Status);
 
-  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet8 
(PcdMaxPeiPerformanceLogEntries);
+  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + (UINT16) (PcdGet8 
(PcdMaxPeiPerformanceLogEntries) != 0 ?
+ PcdGet8 
(PcdMaxPeiPerformanceLogEntries) != 0 :
+ PcdGet16 
(PcdMaxPeiPerformanceLogEntriesMoreThan255));
 
   mGaugeData = AllocateZeroPool (sizeof (GAUGE_DATA_HEADER) + (sizeof 
(GAUGE_DATA_ENTRY_EX) * mMaxGaugeRecords));
   ASSERT (mGaugeData != NULL);
diff --git 
a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf 
b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
index 5f29063..8cecdd2 100644
--- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
+++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.inf
@@ -10,6 +10,7 @@
 #  Performance and PerformanceEx Protocol are installed at the very beginning 
of DXE phase.
 #  
 #  Copyright (c) 2006 - 2014, 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
@@ -67,4 +68,5 @@
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPerformanceLogEntries ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxPeiPerformanceLogEntriesMoreThan255 ## 
CONSUMES
   gEfiMdePkgTokenSpaceGuid.PcdPerformanceLibraryPropertyMask## CONSUMES
diff --git a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c 
b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
index 9674bbc..0f90ca8 100644
--- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
+++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
@@ -4,10 +4,11 @@
   This file implements all APIs in Performance Library class in MdePkg. It 
creates
   performance logging GUIDed HOB on the first performance logging and then 
logs the
   performance data to the GUIDed HOB. Due to the limitation of temporary RAM, 
the maximum
-  number of performance logging entry is specified by 
PcdMaxPeiPerformanceLogEntries.  
+  number of performance logging entry is specified by 
PcdMaxPeiPerformanceLogEntries or 
+  PcdMaxPeiPerformanceLogEntriesMoreThan255
 
 Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
-(C) Copyright 2015 Hewlett Packard Enterprise Development LP
+(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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -51,10 +52,14 @@ InternalGetPerformanceHobLog (
 {
   EFI_HOB_GUID_TYPE   *GuidHob;
   UINTN   PeiPerformanceSize;
+  UINT16  PeiPerformanceLogEntries;
 
   ASSERT (PeiPerformanceLog != NULL);
   ASSERT (PeiPerformanceIdArray != NULL);
 
+  PeiPerformanceLogEntries = (UINT16) (PcdGet8 
(PcdMaxPeiPerformanceLogEntries) != 0 ?
+   PcdGet8 
(PcdMaxPeiPerformanceLogEntries) :
+   PcdGet16 
(PcdMaxPeiPerformanceLogEntriesMoreThan255));
   GuidHob = GetFirstGuidHob ();
 
   if (GuidHob != NULL) {
@@ 

[edk2] [PATCH] MdeModulePkg AcpiTableDxe: Use Rsdt to check against NULL

[Star Zeng]

Some static scan tool may regard CurrentRsdtEntry to be potentially
referenced to NULL pointer if CurrentRsdtEntry == NULL is used in
the right above if condition judgment.

CopyMem (CurrentRsdtEntry, CurrentRsdtEntry + 1, (*NumberOfTableEntries 
- Index) * sizeof (UINT32));

It is introduced by commit f9bbb8d9c3f065faba9f266cf4e731fe2ca70c4d.
To avoid it and have same style with
"((Xsdt == NULL) || CurrentTablePointer64 == (UINT64) (UINTN) Table->Table)",
use Rsdt instead of CurrentRsdtEntry to check against NULL.

Cc: Jiewen Yao 
Cc: Ard Biesheuvel 
Cc: Shumin Qiu 
Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c 
b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
index f6229ca..7f95b9d 100644
--- a/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
+++ b/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI Table Protocol Implementation
 
-  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
   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
@@ -1132,7 +1132,7 @@ RemoveTableFromRsdt (
 //
 // Check if we have found the corresponding entry in both RSDT and XSDT
 //
-if ((CurrentRsdtEntry == NULL || *CurrentRsdtEntry == (UINT32) (UINTN) 
Table->Table) &&
+if (((Rsdt == NULL) || *CurrentRsdtEntry == (UINT32) (UINTN) Table->Table) 
&&
 ((Xsdt == NULL) || CurrentTablePointer64 == (UINT64) (UINTN) 
Table->Table)
 ) {
   //
-- 
2.7.0.windows.1

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

Re: [edk2] [PATCH v2 1/2] PerformancePkg/Dp_App: Support execution break

[Zeng, Star]

Series: Reviewed-by: Star Zeng 

On 2016/3/7 11:23, Cinnamon Shia wrote:

Support UEFI shell execution break.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
  PerformancePkg/Dp_App/Dp.c | 36 --
  PerformancePkg/Dp_App/DpInternal.h | 27 -
  PerformancePkg/Dp_App/DpTrace.c| 62 --
  3 files changed, 99 insertions(+), 26 deletions(-)

diff --git a/PerformancePkg/Dp_App/Dp.c b/PerformancePkg/Dp_App/Dp.c
index e052216..e36a032 100644
--- a/PerformancePkg/Dp_App/Dp.c
+++ b/PerformancePkg/Dp_App/Dp.c
@@ -14,7 +14,7 @@
timer information to calculate elapsed time for each measurement.

Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
which accompanies this distribution.  The full text of the license may be 
found at
@@ -195,11 +195,11 @@ InitCumulativeData (

@param[in]  ImageHandle The image handle.
@param[in]  SystemTable The system table.
-
+
@retval EFI_SUCCESSCommand completed successfully.
@retval EFI_INVALID_PARAMETER  Command usage error.
+  @retval EFI_ABORTEDThe user aborts the operation.
@retval value  Unknown error.
-
  **/
  EFI_STATUS
  EFIAPI
@@ -443,7 +443,10 @@ InitializeDp (
  ProcessCumulative (CustomCumulativeData);
} else if (AllMode) {
  if (TraceMode) {
-  DumpAllTrace( Number2Display, ExcludeMode);
+  Status = DumpAllTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+goto Done;
+  }
  }
  if (ProfileMode) {
DumpAllProfile( Number2Display, ExcludeMode);
@@ -451,7 +454,10 @@ InitializeDp (
}
else if (RawMode) {
  if (TraceMode) {
-  DumpRawTrace( Number2Display, ExcludeMode);
+  Status = DumpRawTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+goto Done;
+  }
  }
  if (ProfileMode) {
DumpRawProfile( Number2Display, ExcludeMode);
@@ -463,11 +469,21 @@ InitializeDp (
ProcessPhases ( Ticker );
if ( ! SummaryMode) {
  Status = ProcessHandles ( ExcludeMode);
-if ( ! EFI_ERROR( Status)) {
-  ProcessPeims ( );
-  ProcessGlobal ();
-  ProcessCumulative (NULL);
+if (Status == EFI_ABORTED) {
+  goto Done;
  }
+
+Status = ProcessPeims ();
+if (Status == EFI_ABORTED) {
+  goto Done;
+}
+
+Status = ProcessGlobal ();
+if (Status == EFI_ABORTED) {
+  goto Done;
+}
+
+ProcessCumulative (NULL);
}
  }
  if (ProfileMode) {
@@ -480,6 +496,8 @@ InitializeDp (
  }
}

+Done:
+
//
// Free the memory allocate from HiiGetString
//
diff --git a/PerformancePkg/Dp_App/DpInternal.h 
b/PerformancePkg/Dp_App/DpInternal.h
index 0e97e1e..53c5fb2 100644
--- a/PerformancePkg/Dp_App/DpInternal.h
+++ b/PerformancePkg/Dp_App/DpInternal.h
@@ -7,7 +7,7 @@
DpUtilities.c, DpTrace.c, and DpProfile.c are included here.

Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
which accompanies this distribution.  The full text of the license may be 
found at
@@ -215,8 +215,11 @@ GatherStatistics(
@param[in]Limit   The number of records to print.  Zero is ALL.
@param[in]ExcludeFlag TRUE to exclude individual Cumulative items from 
display.

+  @retval EFI_SUCCESS   The operation was successful.
+  @retval EFI_ABORTED   The user aborts the operation.
+  @return Othersfrom a call to gBS->LocateHandleBuffer().
  **/
-VOID
+EFI_STATUS
  DumpAllTrace(
IN UINTN Limit,
IN BOOLEAN   ExcludeFlag
@@ -238,9 +241,11 @@ DumpAllTrace(

@param[in]Limit   The number of records to print.  Zero is ALL.
@param[in]ExcludeFlag TRUE to exclude individual Cumulative items from 
display.
-
+
+  @retval EFI_SUCCESS   The operation was successful.
+  @retval EFI_ABORTED   The user aborts the operation.
  **/
-VOID
+EFI_STATUS
  DumpRawTrace(
IN UINTN  Limit,
IN BOOLEANExcludeFlag
@@ -262,8 +267,10 @@ 

Re: [edk2] [PATCH 1/9] edksetup.sh: Create the Conf directory if necessary

[Gao, Liming]

Jordan:
  We have updated edk2\BaseTools\BuildEnv to handle PACKAGES_PATH. I think this 
change should be in edk2\BaseTools\BuildEnv instead of edksetup.sh. Here is my 
change. 

diff --git a/BaseTools/BuildEnv b/BaseTools/BuildEnv
index 7c77454..7b00bc9 100755
--- a/BaseTools/BuildEnv
+++ b/BaseTools/BuildEnv
@@ -51,7 +51,12 @@ RestorePreviousConfiguration() {
   done
 fi
   fi
-  
+
+  if [ ! -d "$CONF_PATH" ]
+  then
+mkdir $CONF_PATH
+  fi
+
   PREVIOUS_CONF_FILE=$CONF_PATH/BuildEnv.sh
   if [ -e $PREVIOUS_CONF_FILE ]
   then
-- 
2.7.1.windows.2

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jordan Justen
> Sent: Saturday, March 05, 2016 2:23 AM
> To: Leahy, Leroy P; edk2-devel@lists.01.org; Bjorge, Erik C
> Cc: Kinney, Michael D
> Subject: Re: [edk2] [PATCH 1/9] edksetup.sh: Create the Conf directory if
> necessary
> 
> On 2016-03-04 08:54:44, Lee Leahy wrote:
> > Edit the shell script to determine if the Conf directory is present.  If
> > not then create the Conf directory.
> >
> 
> With PACKAGES_PATH, I think it might be possible that WORKSPACE is not
> defined.
> 
> Mike mentioned that in some scenario my usage of WORKSPACE in
> OvmfPkg/ResetVector/ResetVector.inf was causing an issue.
> 
> -Jordan
> 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Lee Leahy 
> > ---
> >  edksetup.sh | 4 
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/edksetup.sh b/edksetup.sh
> > index 57368b5..d89ef9d 100755
> > --- a/edksetup.sh
> > +++ b/edksetup.sh
> > @@ -72,6 +72,10 @@ function SetWorkspace()
> >
> >  function SetupEnv()
> >  {
> > +  if [ ! -d "$WORKSPACE/Conf" ]
> > +  then
> > +mkdir $WORKSPACE/Conf
> > +  fi
> >if [ -n "$EDK_TOOLS_PATH" ]
> >then
> >  . $EDK_TOOLS_PATH/BuildEnv $*
> > --
> > 1.9.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] MdeModulePkg ScsiDiskDxe: Fix hang issue when reconnecting an ISCSI device

[Hao Wu]

The 'Reset' function for BlockIO(2) in ScsiDiskDxe should return
EFI_SUCCESS instead of EFI_DEVICE_ERROR when a device does not support
reset feature.

Otherwise, a 'reconnect -r' action when an ISCSI device is attached will
cause system hang.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 18 +-
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index 1cacbb9..dfa5fa3 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -1,7 +1,7 @@
 /** @file
   SCSI disk driver that layers on every SCSI IO protocol in the system.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 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
@@ -466,8 +466,12 @@ ScsiDiskReset (
   Status  = ScsiDiskDevice->ScsiIo->ResetDevice 
(ScsiDiskDevice->ScsiIo);
 
   if (EFI_ERROR (Status)) {
-Status = EFI_DEVICE_ERROR;
-goto Done;
+if (Status == EFI_UNSUPPORTED) {
+  Status = EFI_SUCCESS;
+} else {
+  Status = EFI_DEVICE_ERROR;
+  goto Done;
+}
   }
 
   if (!ExtendedVerification) {
@@ -790,8 +794,12 @@ ScsiDiskResetEx (
   Status  = ScsiDiskDevice->ScsiIo->ResetDevice 
(ScsiDiskDevice->ScsiIo);
 
   if (EFI_ERROR (Status)) {
-Status = EFI_DEVICE_ERROR;
-goto Done;
+if (Status == EFI_UNSUPPORTED) {
+  Status = EFI_SUCCESS;
+} else {
+  Status = EFI_DEVICE_ERROR;
+  goto Done;
+}
   }
 
   if (!ExtendedVerification) {
-- 
1.9.5.msysgit.0

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

Re: [edk2] [PATCH v4] MdePkg: Add UEFI2.6 HII Image Ex and Image Decoder protocol definition.

[Dong, Eric]

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Cecil 
> Sheng
> Sent: Monday, March 07, 2016 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Cecil Sheng
> Subject: [edk2] [PATCH v4] MdePkg: Add UEFI2.6 HII Image Ex and Image Decoder 
> protocol definition.
> 
> Add the definition for the new UEFI 2.6 EFI_HII_IMAGE_EX_PROTOCOL and 
> EFI_IMAGE_DECODER_PROTOCOL.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cecil Sheng 
> Reviewed-by: Samer El-Haj-Mahmoud 
> Reviewed-by: Abner Chang 
> ---
>  MdePkg/Include/Protocol/HiiImageEx.h   | 245 
> +
>  MdePkg/Include/Protocol/ImageDecoder.h | 192 ++
>  MdePkg/MdePkg.dec  |  13 ++
>  3 files changed, 450 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/HiiImageEx.h
>  create mode 100644 MdePkg/Include/Protocol/ImageDecoder.h
> 
> diff --git a/MdePkg/Include/Protocol/HiiImageEx.h 
> b/MdePkg/Include/Protocol/HiiImageEx.h
> new file mode 100644
> index 000..9393a53
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/HiiImageEx.h
> @@ -0,0 +1,245 @@
> +/** @file
> +  Protocol which allows access to the images in the images database.
> +
> +(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 that 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.
> +
> +**/
> +
> +#ifndef EFI_HII_IMAGE_EX_H
> +#define EFI_HII_IMAGE_EX_H
> +
> +#include 
> +
> +//
> +// Global ID for the Hii Image Ex Protocol.
> +//
> +#define EFI_HII_IMAGE_EX_PROTOCOL_GUID \
> +  {0x1a1241e6, 0x8f19, 0x41a9,  { 0xbc, 0xe, 0xe8, 0xef, 0x39, 0xe0, 0x65, 
> 0x46 }}
> +
> +typedef struct _EFI_HII_IMAGE_EX_PROTOCOL EFI_HII_IMAGE_EX_PROTOCOL;
> +
> +/**
> +  The prototype of this extension function is the same with 
> EFI_HII_IMAGE_PROTOCOL.NewImage().
> +  Same with EFI_HII_IMAGE_PROTOCOL.NewImage().This protocol invokes
> +EFI_HII_IMAGE_PROTOCOL.NewImage() implicitly.
> +
> +  @param  This   A pointer to the EFI_HII_IMAGE_EX_PROTOCOL 
> instance.
> +  @param  PackageListHandle of the package list where this image 
> will
> + be added.
> +  @param  ImageIdOn return, contains the new image id, which 
> is
> + unique within PackageList.
> +  @param  Image  Points to the image.
> +
> +  @retval EFI_SUCCESSThe new image was added successfully.
> +  @retval EFI_NOT_FOUND  The specified PackageList could not be 
> found in
> + database.
> +  @retval EFI_OUT_OF_RESOURCES   Could not add the image due to lack of 
> resources.
> +  @retval EFI_INVALID_PARAMETER  Image is NULL or ImageId is NULL.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_HII_NEW_IMAGE_EX)(
> +  IN CONST  EFI_HII_IMAGE_EX_PROTOCOL  *This,
> +  INEFI_HII_HANDLE  PackageList,
> +  OUT   EFI_IMAGE_ID*ImageId,
> +  IN CONST  EFI_IMAGE_INPUT *Image
> +  );
> +
> +/**
> +  Return the information about the image, associated with the package list.
> +  The prototype of this extension function is the same with 
> EFI_HII_IMAGE_PROTOCOL.GetImage().
> +  Same with EFI_HII_IMAGE_PROTOCOL.SetImage(),this protocol invokes 
> EFI_HII_IMAGE_PROTOCOL.SetImage() implicitly.
> +
> +  @param  This   A pointer to the EFI_HII_IMAGE_EX_PROTOCOL 
> instance.
> +  @param  PackageListHandle of the package list where this image 
> will
> + be searched.
> +  @param  ImageIdThe image's id,, which is unique within
> + PackageList.
> +  @param  Image  Points to the image.
> +
> +  @retval EFI_SUCCESSThe new image was returned successfully.
> +  @retval EFI_NOT_FOUND  The image specified by ImageId is not in the
> + database. The specified PackageList is not 
> in
> + the database.
> +  @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too 
> small to
> + hold the image.
> +  @retval EFI_INVALID_PARAMETER  The Image or ImageSize was NULL.
> +  @retval EFI_OUT_OF_RESOURCES   The bitmap could not be retrieved because 
> there
> + was not enough memory.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_HII_GET_IMAGE_EX)(
> +  IN CONST  EFI_HII_IMAGE_EX_PROTOCOL

Re: [edk2] [PATCH] MdePkg: Add UEFI2.6 HII Image Ex and Image Decoder protocol definition.

[Dong, Eric]

Hi Cecil,

We found the V2 patch still has some issues need to be fixed before checking in 
the code.

ImageDecoder.h
1.  EFI_HII_IMAGE_DECODER_PROTOCOL_GUID  should be defined in ImageDecoder.h.
Like EFI_HII_IMAGE_EX_PROTOCOL_GUID in HiiImageEx.h
//
  // Global ID for the Hii Image Ex Protocol.
  //
  #define EFI_HII_IMAGE_EX_PROTOCOL_GUID \
{0x1a1241e6, 0x8f19, 0x41a9,  { 0xbc, 0xe, 0xe8, 0xef, 0x39, 0xe0, 
0x65, 0x46 }}
2.  The type of SizeOfImage is different in the spec and code.
 Code:
+typedef
+EFI_STATUS
+(EFIAPI *EFI_HII_IMAGE_DECODER_GET_IMAGE_INFO)(
+  IN  EFI_HII_IMAGE_DECODER_PROTOCOL   *This,
+  IN  VOID  *Image,
+  IN  UINT32SizeOfImage,
+  IN OUT  EFI_HII_IMAGE_DECODER_IMAGE_INFO_HEADER  **ImageInfo
+  );
Spec:
typedef
EFI_STATUS
(EFIAPI *EFI_HII_IMAGE_DECODER_GET_IMAGE_INFO)(
IN CONST EFI_HII_IMAGE_DECODER_PROTOCOL *This,
IN VOID *Image,
IN UINTN SizeOfImage,
IN OUT EFI_HII_IMAGE_DECODER_IMAGE_INFO **ImageInfo
);
3. the type of BitMap  is EFI_IMAGE_OUTPUT in spec but EFI_IMAGE_INPUT in code.
  Code:
  +typedef
  +EFI_STATUS
  +(EFIAPI *EFI_HII_IMAGE_DECODER_DECODE)(
  +  IN  EFI_HII_IMAGE_DECODER_PROTOCOL   *This,
  +  IN  VOID  *Image,
  +  IN  UINTN ImageRawDataSize,
  +  IN OUT  EFI_IMAGE_INPUT   **BitMap OPTIONAL,
  +  IN  BOOLEAN   Transparent
  +  );

Spec:
  typedef
  EFI_STATUS
  (EFIAPI *EFI_HII_IMAGE_DECODER_DECODE)(
  IN CONST EFI_HII_IMAGE_DECODER_PROTOCOL *This,
  IN VOID *Image,
  IN UINTN ImageRawDataSize,
  IN OUT EFI_IMAGE_OUTPUT **Bitmap OPTIONAL,
  IN BOOLEAN Transparent
  );

HiiImageEx.h
1.  Missing "extern EFI_GUID gEfiHiiImageExProtocolGuid;" in the HiiImageEx.h 
file.


Thanks,
Eric
> -Original Message-
> From: Sheng, Cecil (HPS SW) [mailto:cecil.sh...@hpe.com]
> Sent: Friday, March 04, 2016 9:36 AM
> To: Bi, Dandan; edk2-devel@lists.01.org
> Cc: Dong, Eric; Gao, Liming; Chang, Abner (HPS SW/FW Technologist)
> Subject: RE: [edk2] [PATCH] MdePkg: Add UEFI2.6 HII Image Ex and Image 
> Decoder protocol definition.
>
> Hi Dandan,
>
> I've corrected two places in the EFI_HII_IMAGE_DECODER_PROTOCOL.
>   1. NumberOfDecoderName in GetImageDecoderName() should be a pointer 
> because it's an OUT parameter.
>   2. the typedef for Decode() are not consistent between the protocol 
> definition and the function definition.
>
> There's an ECR to correct the second one but not the first one. However, they 
> are both incorrect in 2.6.
>
>
>
>
> Sincerely,
>
> Cecil Sheng
> ISS Firmware Development
> HPE Servers
>
> -Original Message-
> From: Bi, Dandan [mailto:dandan...@intel.com]
> Sent: Thursday, March 3, 2016 11:05 AM
> To: Sheng, Cecil (HPS SW); 
> edk2-devel@lists.01.org
> Cc: Dong, Eric; Gao, Liming
> Subject: RE: [edk2] [PATCH] MdePkg: Add UEFI2.6 HII Image Ex and Image 
> Decoder protocol definition.
>
> Hi Cecil,
>
>  I still find that the function description are not consistent with the spec 
> in the V2 patch, you use the Summary part not the Description
> part in the spec.
> You can refer  to the definition of other protocol to double check.
> And there are two same return status  (EFI_INVALID_PARAMETER )in 
> EFI_HII_DRAW_IMAGE_EX.
> +  @retval EFI_SUCCESSThe image was successfully drawn.
> +  @retval EFI_OUT_OF_RESOURCES   Unable to allocate an output buffer for Blt.
> +  @retval EFI_INVALID_PARAMETER  The Image or Blt was NULL.
> +  @retval EFI_INVALID_PARAMETER  Any combination of Flags is invalid.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_HII_DRAW_IMAGE_EX)(
> +  IN CONST  EFI_HII_IMAGE_EX_PROTOCOL   *This,
> +  INEFI_HII_DRAW_FLAGS  Flags,
> +  IN CONST  EFI_IMAGE_INPUT *Image,
> +  IN OUTEFI_IMAGE_OUTPUT**Blt,
> +  INUINTN   BltX,
> +  INUINTN   BltY
> +  );
>
> And could  you point out  the errors in the EFI_HII_IMAGE_DECODER_PROTOCOL 
> protocol definition in the spec?
> Since we don't know the issues in spec, it may has an effect on the review.
>
>
> Thanks,
> Dandan
>
>
>
>
> -Original Message-
> From: Sheng, Cecil (HPS SW) [mailto:cecil.sh...@hpe.com]
> Sent: Wednesday, March 2, 2016 10:58 AM
> To: Bi, Dandan >; 
> edk2-devel@lists.01.org
> Cc: Dong, Eric >; Gao, Liming 
> >
> Subject: RE: [edk2] [PATCH] MdePkg: Add UEFI2.6 HII Image Ex and Image 
> Decoder 

Re: [edk2] [Patch] MdeModulePkg/Bds: Fix VS2010/VS2012 build failure.

[Qiu, Shumin]

Reviewed-by: Qiu Shumin 

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, March 07, 2016 1:30 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu; Qiu, Shumin
Subject: [Patch] MdeModulePkg/Bds: Fix VS2010/VS2012 build failure.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Shumin Qiu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 18f835a..4225e80 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1667,6 +1667,8 @@ BmGetFileBufferFromLoadFileFileSystem (
 Handles = NULL;
 HandleCount = 0;
   }
+
+  Handle = NULL;
   for (Index = 0; Index < HandleCount; Index++) {
 Node = DevicePathFromHandle (Handles[Index]);
 Status = gBS->LocateDevicePath (, , );
-- 
2.7.0.windows.1

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

[edk2] [Patch] MdeModulePkg/Bds: Fix VS2010/VS2012 build failure.

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Shumin Qiu 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index 18f835a..4225e80 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1667,6 +1667,8 @@ BmGetFileBufferFromLoadFileFileSystem (
 Handles = NULL;
 HandleCount = 0;
   }
+
+  Handle = NULL;
   for (Index = 0; Index < HandleCount; Index++) {
 Node = DevicePathFromHandle (Handles[Index]);
 Status = gBS->LocateDevicePath (, , );
-- 
2.7.0.windows.1

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

Re: [edk2] [PATCH] ArmPlatformPkg: Allocate VRAM as RuntimeServicesData

[Ard Biesheuvel]

On 5 March 2016 at 00:55, Ard Biesheuvel  wrote:
> On 4 March 2016 at 18:40,   wrote:
>> From: Sami Mujawar 
>>
>> The UEFI specification allows the operating system (OS) to use the
>> Graphics Output Protocol (GOP) in the following scenarios:
>>  a. as part of the startup process and
>>  b. prior to loading of a high performance OS graphics driver
>>
>> If the VRAM is allocated as BootServicesData, then it is unmapped on
>> exit boot services. This prevents GOP usage by the OS post exit boot
>> services (the second scenario); as it results in a crash when the VRAM
>> is accessed.
>>
>> This patch fixes the issue by allocating VRAM as RuntimeServicesData.
>>
>> Code at: 
>> https://github.com/EvanLloyd/tianocore/commit/0b6b792886e5c6fa41f943b3f8775947fd4d3e8b
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Alexei Fedorov 
>> Signed-off-by: Girish Pathak 
>> Signed-off-by: Sami Mujawar 
>> Signed-off-by: Evan Lloyd 
>
> Isn't it the job of the loader to reserve this memory if it needs to
> hang on to it after ExitBootServices()?
> By the same logic, configuration tables are often loaded into
> BootServicesData memory, and it is up to the OS to reserve it if it
> needs to use it. Otherwise, OSes that don't care about this
> functionality are stuck with reserved regions that they cannot
> reclaim.
>

After some discussion, I realize that you are probably not in a
position where you can change the OS side to do the right thing here.

However, RuntimeServicesData is not the correct type to use. The
framebuffer region has special semantics throughout its lifetime, and
its lifetime is unbounded (i.e., the OS has no way to tell at which
point the framebuffer is released by the graphics hardware). Also, an
OS may elect not to use runtime services at all, and free all memory
related to it.

So the correct type to use here is EfiReservedMemoryType. But please,
also add a PCD to ArmPlatformPkg to make this behavior opt-in, since
losing 8 MB to a framebuffer that the OS may never use or care about
is a bit excessvie.

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

Re: [edk2] [PATCH] MdeModulePkg: Change the type of PcdMaxPeiPerformanceLogEntries to UINT16

[Shia, Cinnamon]

Hi Liming,

I see. Thanks for your explanation. Will fix it in the patch v2.

Thanks,
Cinnamon Shia


-Original Message-
From: Gao, Liming [mailto:liming@intel.com] 
Sent: Monday, March 7, 2016 10:49 AM
To: Shia, Cinnamon; Kinney, Michael D; edk2-devel@lists.01.org
Cc: Gao, Liming
Subject: RE: [edk2] [PATCH] MdeModulePkg: Change the type of 
PcdMaxPeiPerformanceLogEntries to UINT16

Cinnamon:
  You can see PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase 
and gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64. 
  In source code edk2\MdeModulePkg\Universal\Variable\Pei\Variable.c line 546, 
the consumer code is like below. 
NvStorageBase = (EFI_PHYSICAL_ADDRESS) (PcdGet64 
(PcdFlashNvStorageVariableBase64) != 0 ?
PcdGet64 
(PcdFlashNvStorageVariableBase64) :
PcdGet32 
(PcdFlashNvStorageVariableBase)
   );

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Shia, Cinnamon
> Sent: Monday, March 07, 2016 10:40 AM
> To: Kinney, Michael D; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: Change the type of 
> PcdMaxPeiPerformanceLogEntries to UINT16
> 
> Hi Mike,
> 
> Thanks for your feedback.
> 
> For solving the backward compatible problem, could you share to me 
> what the problem is?
> The PCD is accessed in override c files?
> 
> Regarding to introduce a new PCD, does it mean using the new PCD in c 
> files and leaving the existing one as it is?
> 
> Thanks,
> Cinnamon Shia
> 
> -Original Message-
> From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
> Sent: Sunday, March 6, 2016 12:47 AM
> To: Shia, Cinnamon; edk2-devel@lists.01.org; Kinney, Michael D
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Change the type of 
> PcdMaxPeiPerformanceLogEntries to UINT16
> 
> Cinnamon,
> 
> Changing the size of a PCD is a not backwards compatible change.
> 
> In the past when we have run into similar issues, we had to introduce 
> a new PCD.
> 
> Mike
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Cinnamon Shia
> > Sent: Saturday, March 5, 2016 5:19 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg: Change the type of 
> > PcdMaxPeiPerformanceLogEntries to UINT16
> >
> > Change the type of PcdMaxPeiPerformanceLogEntries from UINT8 to
> UINT16
> > to log more than 255 performance entries in PEI.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Cinnamon Shia 
> > ---
> >  .../Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 3 ++-
> >  MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c| 8
> 
> >  MdeModulePkg/MdeModulePkg.dec | 3 ++-
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git
> >
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b.c
> >
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b.c
> > index 0eb8e57..61491e6 100644
> > ---
> >
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b.c
> > +++
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b
> > +++ .c
> > @@ -11,6 +11,7 @@
> >Performance Protocol is installed at the very beginning of DXE phase.
> >
> >  Copyright (c) 2006 - 2015, 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 @@ -522,7 +523,7 @@ DxeCorePerformanceLibConstructor (
> >);
> >ASSERT_EFI_ERROR (Status);
> >
> > -  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet8 
> > (PcdMaxPeiPerformanceLogEntries);
> > +  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet16
> > (PcdMaxPeiPerformanceLogEntries);
> >
> >mGaugeData = AllocateZeroPool (sizeof (GAUGE_DATA_HEADER) +
> (sizeof
> > (GAUGE_DATA_ENTRY_EX) * mMaxGaugeRecords));
> >ASSERT (mGaugeData != NULL);
> > diff --git
> > a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > index 9674bbc..74e6300 100644
> > --- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > +++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > @@ -7,7 +7,7 @@
> >number of performance logging entry is specified by
> PcdMaxPeiPerformanceLogEntries.
> >
> >  Copyright (c) 2006 - 2015, Intel Corporation. All rights 
> > reserved.
> > -(C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > +(C) Copyright 2015-2016 Hewlett 

Re: [edk2] [PATCH 3/5] OvmfPkg: add DxePciLibI440FxQ35

[Jordan Justen]

On 2016-03-06 06:28:26, Laszlo Ersek wrote:
> On 03/05/16 00:47, Kinney, Michael D wrote:
> > Jordan,
> > 
> > Given these libs are Base type, a Dynamic PCD could not be used.  Only a 
> > fixed
> > or patchable PCD could be used, and if you go through path, you might as 
> > well
> > pick the right lib in the DSC at build time and get smaller size and better 
> > performance.
> 
> I'd like to discuss two things here.
> 
> * First, I fully agree with you that determining the branch in this
> unified library instance *at build time* is functionally equivalent (but
> is size- and perf-wise inferior) to simply picking the right one of the
> currently available PciLib instances, at build time.
> 
> I think that this library instance is special to virtualization. On the
> bare metal, you never face a situation where you don't know in advance
> whether your platform will be a PCIe system, or a PCI-only system. But,
> that's exactly what we have with QEMU (the i440fx and Q35 machine
> types), so this dynamism is needed. (In fact I find it very nice that
> the modularity of edk2 allowed me to do such a simple unification.)
> 
> Another possibility for influencing the branch to take would be a GUID
> HOB. In this particular case however I only saw downsides in a HOB. On
> the source code side, it would take another GUID definition, another
> structure definition, possibly another header file. And on the
> functionality side, the only "extension" it would buy us would be the
> usability of the library instance in the DXE_CORE. (The DXE_CORE cannot
> use PCDs, but it can look at HOBs.)
> 
> We couldn't use it in PEI anyway (without ugly depex trickery), because
> a PEIM using this library instance could be dispatched before the PEIM
> that produces the HOB.
> 
> But, in OVMF, PEI is perfectly fine with just 0xCF8 access anyway.
> 
> * Second, any given library instance has two kinds of "qualification" in
> its INF file, as far as client modules are concerned. One is
> MODULE_TYPE, and the other is the restriction list in LIBRARY_CLASS.
> 
> These qualifications seem to overlap, in a (superficially) confusing manner:
> 
> (a) the INF file spec says about MODULE_TYPE,
> 
> For Library Modules, the MODULE_TYPE must specify the MODULE_TYPE
> of the module that will use the driver.
> 
> Furthermore, in appendix G, MODULE_TYPE=BASE is explicitly allowed for
> libraries, and then BASE is documented as:
> 
> Modules of this type can be ported to any execution environment.
> This module type is intended to be use by silicon module developers
> to produce source code that is not tied to any specific execution
> environment.
> 
> (b) But the INF file also says
> 
> Each LIBRARY_CLASS statement must provide the name of the library
> class supported, followed by the pipe "|" field separator and then
> a space " " delimited list of module types the library instances
> supports.
> 
> Thus a conflict appears between
> 
>   MODULE_TYPE = BASE
> 
> and
> 
>   LIBRARY_CLASS = WhateverLib|DXE_DRIVER
> 
> because the former implies "any execution environment", whereas the
> second implies the DXE phase.
> 
> However, this apparent conflict is resolved by the INF spec in the
> following:
> 
> The MODULE_TYPE entry in the [Defines] section for a library only
> defines the module type that the build system must assume for
> building the library. It does not define the types of modules that
> a library may be linked with. Instead, the LIBRARY_CLASS entry in
> the [Defines] section optionally lists the supported module types
> that the library may be linked against.
> 
> Therefore, the MODULE_TYPE for a library instance only determines
> prototype / calling convention compatibility. In my experience, this
> practically boils down to two things, and nothing more:
> 
> - functions must return RETURN_STATUS vs. EFI_STATUS,
> 
> - the constructor function of the library instance (if any) gets VOID
>   if the MODULE_TYPE is BASE, vs. ImageHandle plus SystemTable if the
>   MODULE_TYPE is DXE_DRIVER.
> 
> That's it.
> 
> What *really* matters is the restriction list in LIBRARY_CLASS; that's
> where the author of the library instance ensures that the instance
> doesn't get linked into a module (and firmware phase) that cannot
> provide the library instance with the necessary environment.
> 
> Therefore, making a library instance MODULE_TYPE=BASE, and then
> restricting it to the DXE phase and later with LIBRARY_CLASS, is valid
> in my opinion. The library instance may not need ImageHandle and
> SystemTable for its own code at all, it is okay with returning
> RETURN_STATUS values, but it might need the environment to be DXE or later.
> 
> The question then emerges, how we should *name* the library instance. In
> my experience, the name is supposed to reflect the actual client module
> / firmware phase restrictions. Which is why I named this instance DxeXX.
> 
> In summary, if there 

[edk2] [PATCH v2 1/2] PerformancePkg/Dp_App: Support execution break

[Cinnamon Shia]

Support UEFI shell execution break.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
 PerformancePkg/Dp_App/Dp.c | 36 --
 PerformancePkg/Dp_App/DpInternal.h | 27 -
 PerformancePkg/Dp_App/DpTrace.c| 62 --
 3 files changed, 99 insertions(+), 26 deletions(-)

diff --git a/PerformancePkg/Dp_App/Dp.c b/PerformancePkg/Dp_App/Dp.c
index e052216..e36a032 100644
--- a/PerformancePkg/Dp_App/Dp.c
+++ b/PerformancePkg/Dp_App/Dp.c
@@ -14,7 +14,7 @@
   timer information to calculate elapsed time for each measurement.
  
   Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -195,11 +195,11 @@ InitCumulativeData (
   
   @param[in]  ImageHandle The image handle.
   @param[in]  SystemTable The system table.
-  
+
   @retval EFI_SUCCESSCommand completed successfully.
   @retval EFI_INVALID_PARAMETER  Command usage error.
+  @retval EFI_ABORTEDThe user aborts the operation.
   @retval value  Unknown error.
-  
 **/
 EFI_STATUS
 EFIAPI
@@ -443,7 +443,10 @@ InitializeDp (
 ProcessCumulative (CustomCumulativeData);
   } else if (AllMode) {
 if (TraceMode) {
-  DumpAllTrace( Number2Display, ExcludeMode);
+  Status = DumpAllTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+goto Done;
+  }
 }
 if (ProfileMode) {
   DumpAllProfile( Number2Display, ExcludeMode);
@@ -451,7 +454,10 @@ InitializeDp (
   }
   else if (RawMode) {
 if (TraceMode) {
-  DumpRawTrace( Number2Display, ExcludeMode);
+  Status = DumpRawTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+goto Done;
+  }
 }
 if (ProfileMode) {
   DumpRawProfile( Number2Display, ExcludeMode);
@@ -463,11 +469,21 @@ InitializeDp (
   ProcessPhases ( Ticker );
   if ( ! SummaryMode) {
 Status = ProcessHandles ( ExcludeMode);
-if ( ! EFI_ERROR( Status)) {
-  ProcessPeims ( );
-  ProcessGlobal ();
-  ProcessCumulative (NULL);
+if (Status == EFI_ABORTED) {
+  goto Done;
 }
+
+Status = ProcessPeims ();
+if (Status == EFI_ABORTED) {
+  goto Done;
+}
+
+Status = ProcessGlobal ();
+if (Status == EFI_ABORTED) {
+  goto Done;
+}
+
+ProcessCumulative (NULL);
   }
 }
 if (ProfileMode) {
@@ -480,6 +496,8 @@ InitializeDp (
 }
   }
 
+Done:
+
   //
   // Free the memory allocate from HiiGetString
   //
diff --git a/PerformancePkg/Dp_App/DpInternal.h 
b/PerformancePkg/Dp_App/DpInternal.h
index 0e97e1e..53c5fb2 100644
--- a/PerformancePkg/Dp_App/DpInternal.h
+++ b/PerformancePkg/Dp_App/DpInternal.h
@@ -7,7 +7,7 @@
   DpUtilities.c, DpTrace.c, and DpProfile.c are included here.
 
   Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -215,8 +215,11 @@ GatherStatistics(
   @param[in]Limit   The number of records to print.  Zero is ALL.
   @param[in]ExcludeFlag TRUE to exclude individual Cumulative items from 
display.
   
+  @retval EFI_SUCCESS   The operation was successful.
+  @retval EFI_ABORTED   The user aborts the operation.
+  @return Othersfrom a call to gBS->LocateHandleBuffer().
 **/
-VOID
+EFI_STATUS
 DumpAllTrace(
   IN UINTN Limit,
   IN BOOLEAN   ExcludeFlag
@@ -238,9 +241,11 @@ DumpAllTrace(
   
   @param[in]Limit   The number of records to print.  Zero is ALL.
   @param[in]ExcludeFlag TRUE to exclude individual Cumulative items from 
display.
-  
+
+  @retval EFI_SUCCESS   The operation was successful.
+  @retval EFI_ABORTED   The user aborts the operation.
 **/
-VOID
+EFI_STATUS
 DumpRawTrace(
   IN UINTN  Limit,
   IN BOOLEANExcludeFlag
@@ -262,8 +267,10 @@ ProcessPhases(
   Gather and print Handle data.
   
   @param[in]ExcludeFlag   TRUE to exclude individual Cumulative items from 
display.
- 

[edk2] [PATCH v2 2/2] ShellPkg/UefiDpLib: Support execution break

[Cinnamon Shia]

Support UEFI shell execution break.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
 ShellPkg/Library/UefiDpLib/Dp.c | 48 ---
 ShellPkg/Library/UefiDpLib/DpInternal.h | 24 ++
 ShellPkg/Library/UefiDpLib/DpTrace.c| 57 +
 ShellPkg/Library/UefiDpLib/UefiDpLib.h  |  6 
 4 files changed, 110 insertions(+), 25 deletions(-)

diff --git a/ShellPkg/Library/UefiDpLib/Dp.c b/ShellPkg/Library/UefiDpLib/Dp.c
index 6b06d20..0176e31 100644
--- a/ShellPkg/Library/UefiDpLib/Dp.c
+++ b/ShellPkg/Library/UefiDpLib/Dp.c
@@ -14,7 +14,7 @@
   timer information to calculate elapsed time for each measurement.
  
   Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -137,10 +137,10 @@ InitCumulativeData (
   @param[in]  ImageHandle The image handle.
   @param[in]  SystemTable The system table.
   
-  @retval EFI_SUCCESSCommand completed successfully.
-  @retval EFI_INVALID_PARAMETER  Command usage error.
-  @retval value  Unknown error.
-  
+  @retval SHELL_SUCCESSCommand completed successfully.
+  @retval SHELL_INVALID_PARAMETER  Command usage error.
+  @retval SHELL_ABORTEDThe user aborts the operation.
+  @retval valueUnknown error.
 **/
 SHELL_STATUS
 EFIAPI
@@ -168,6 +168,7 @@ ShellCommandRunDp (
   BOOLEAN   CumulativeMode;
   CONST CHAR16  *CustomCumulativeToken;
   PERF_CUM_DATA *CustomCumulativeData;
+  SHELL_STATUS  ShellStatus;
 
   StringPtr   = NULL;
   SummaryMode = FALSE;
@@ -179,6 +180,7 @@ ShellCommandRunDp (
   ExcludeMode = FALSE;
   CumulativeMode = FALSE;
   CustomCumulativeData = NULL;
+  ShellStatus = SHELL_SUCCESS;
 
   // Get DP's entry time as soon as possible.
   // This is used as the Shell-Phase end time.
@@ -329,14 +331,22 @@ ShellCommandRunDp (
 ProcessCumulative (CustomCumulativeData);
   } else if (AllMode) {
 if (TraceMode) {
-  DumpAllTrace( Number2Display, ExcludeMode);
+  Status = DumpAllTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+ShellStatus = SHELL_ABORTED;
+goto Done;
+  }
 }
 if (ProfileMode) {
   DumpAllProfile( Number2Display, ExcludeMode);
 }
   } else if (RawMode) {
 if (TraceMode) {
-  DumpRawTrace( Number2Display, ExcludeMode);
+  Status = DumpRawTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+ShellStatus = SHELL_ABORTED;
+goto Done;
+  }
 }
 if (ProfileMode) {
   DumpRawProfile( Number2Display, ExcludeMode);
@@ -347,11 +357,24 @@ ShellCommandRunDp (
   ProcessPhases ( Ticker );
   if ( ! SummaryMode) {
 Status = ProcessHandles ( ExcludeMode);
-if ( ! EFI_ERROR( Status)) {
-  ProcessPeims ();
-  ProcessGlobal ();
-  ProcessCumulative (NULL);
+if (Status == EFI_ABORTED) {
+  ShellStatus = SHELL_ABORTED;
+  goto Done;
+}
+
+Status = ProcessPeims ();
+if (Status == EFI_ABORTED) {
+  ShellStatus = SHELL_ABORTED;
+  goto Done;
+}
+
+Status = ProcessGlobal ();
+if (Status == EFI_ABORTED) {
+  ShellStatus = SHELL_ABORTED;
+  goto Done;
 }
+
+ProcessCumulative (NULL);
   }
 }
 if (ProfileMode) {
@@ -362,11 +385,12 @@ ShellCommandRunDp (
 DumpStatistics();
   }
 
+Done:
   SHELL_FREE_NON_NULL (StringPtr);
   if (CustomCumulativeData != NULL) {
 SHELL_FREE_NON_NULL (CustomCumulativeData->Name);
   }
   SHELL_FREE_NON_NULL (CustomCumulativeData);
 
-  return SHELL_SUCCESS;
+  return ShellStatus;
 }
diff --git a/ShellPkg/Library/UefiDpLib/DpInternal.h 
b/ShellPkg/Library/UefiDpLib/DpInternal.h
index aa07fea..b5ec5f0 100644
--- a/ShellPkg/Library/UefiDpLib/DpInternal.h
+++ b/ShellPkg/Library/UefiDpLib/DpInternal.h
@@ -7,7 +7,7 @@
   DpUtilities.c, DpTrace.c, and DpProfile.c are included here.
 
   Copyright (c) 2009 - 2013, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -198,8 +198,11 @@ GatherStatistics(
   @param[in]Limit   The number of records to print.  Zero is ALL.
  

Re: [edk2] [PATCH 1/2] PerformancePkg/Dp_App: Support execution break

[Shia, Cinnamon]

Hi Star,

Thanks for your feedbacks. Will fix it in the patch v2.

Thanks,
Cinnamon Shia


-Original Message-
From: Zeng, Star [mailto:star.z...@intel.com] 
Sent: Monday, March 7, 2016 10:50 AM
To: Shia, Cinnamon; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 1/2] PerformancePkg/Dp_App: Support execution break

Cinnamon,

Since you have updated code to replace gBS->LocateHandle with 
gBS->LocateHandleBuffer at previous commits. How about to update the
"gBS->LocateHandle" to "gBS->LocateHandleBuffer" in the new added comments for 
this patch series?

Other parts of this patch series are good to me.


Thanks,
Star

On 2016/3/5 19:35, Cinnamon Shia wrote:
> Support UEFI shell execution break.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cinnamon Shia 
> ---
>   PerformancePkg/Dp_App/Dp.c | 36 --
>   PerformancePkg/Dp_App/DpInternal.h | 27 -
>   PerformancePkg/Dp_App/DpTrace.c| 62 
> --
>   3 files changed, 99 insertions(+), 26 deletions(-)
>
> diff --git a/PerformancePkg/Dp_App/Dp.c b/PerformancePkg/Dp_App/Dp.c 
> index e052216..e36a032 100644
> --- a/PerformancePkg/Dp_App/Dp.c
> +++ b/PerformancePkg/Dp_App/Dp.c
> @@ -14,7 +14,7 @@
> timer information to calculate elapsed time for each measurement.
>
> Copyright (c) 2009 - 2015, Intel Corporation. All rights 
> reserved.
> -  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> +  (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
> which accompanies this distribution.  The full text of the license 
> may be found at @@ -195,11 +195,11 @@ InitCumulativeData (
>
> @param[in]  ImageHandle The image handle.
> @param[in]  SystemTable The system table.
> -
> +
> @retval EFI_SUCCESSCommand completed successfully.
> @retval EFI_INVALID_PARAMETER  Command usage error.
> +  @retval EFI_ABORTEDThe user aborts the operation.
> @retval value  Unknown error.
> -
>   **/
>   EFI_STATUS
>   EFIAPI
> @@ -443,7 +443,10 @@ InitializeDp (
>   ProcessCumulative (CustomCumulativeData);
> } else if (AllMode) {
>   if (TraceMode) {
> -  DumpAllTrace( Number2Display, ExcludeMode);
> +  Status = DumpAllTrace( Number2Display, ExcludeMode);
> +  if (Status == EFI_ABORTED) {
> +goto Done;
> +  }
>   }
>   if (ProfileMode) {
> DumpAllProfile( Number2Display, ExcludeMode); @@ -451,7 
> +454,10 @@ InitializeDp (
> }
> else if (RawMode) {
>   if (TraceMode) {
> -  DumpRawTrace( Number2Display, ExcludeMode);
> +  Status = DumpRawTrace( Number2Display, ExcludeMode);
> +  if (Status == EFI_ABORTED) {
> +goto Done;
> +  }
>   }
>   if (ProfileMode) {
> DumpRawProfile( Number2Display, ExcludeMode); @@ -463,11 
> +469,21 @@ InitializeDp (
> ProcessPhases ( Ticker );
> if ( ! SummaryMode) {
>   Status = ProcessHandles ( ExcludeMode);
> -if ( ! EFI_ERROR( Status)) {
> -  ProcessPeims ( );
> -  ProcessGlobal ();
> -  ProcessCumulative (NULL);
> +if (Status == EFI_ABORTED) {
> +  goto Done;
>   }
> +
> +Status = ProcessPeims ();
> +if (Status == EFI_ABORTED) {
> +  goto Done;
> +}
> +
> +Status = ProcessGlobal ();
> +if (Status == EFI_ABORTED) {
> +  goto Done;
> +}
> +
> +ProcessCumulative (NULL);
> }
>   }
>   if (ProfileMode) {
> @@ -480,6 +496,8 @@ InitializeDp (
>   }
> }
>
> +Done:
> +
> //
> // Free the memory allocate from HiiGetString
> //
> diff --git a/PerformancePkg/Dp_App/DpInternal.h 
> b/PerformancePkg/Dp_App/DpInternal.h
> index 0e97e1e..cd686d9 100644
> --- a/PerformancePkg/Dp_App/DpInternal.h
> +++ b/PerformancePkg/Dp_App/DpInternal.h
> @@ -7,7 +7,7 @@
> DpUtilities.c, DpTrace.c, and DpProfile.c are included here.
>
> Copyright (c) 2009 - 2014, Intel Corporation. All rights 
> reserved.
> -  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> +  (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
> which accompanies this distribution.  The full text of the license 
> may be found at @@ -215,8 +215,11 @@ GatherStatistics(
> @param[in]Limit   The number of records to print.  Zero is ALL.
> @param[in]ExcludeFlag TRUE to 

Re: [edk2] [PATCH v2] MdePkg: Add UEFI2.6 HII Image Ex and Image Decoder protocol definition.

[Dong, Eric]

Reviewed-by: Eric Dong 


Hi Feng,

Please help to check in the code.


Thanks,
Eric
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Cecil 
> Sheng
> Sent: Wednesday, March 02, 2016 10:56 AM
> To: edk2-devel@lists.01.org
> Cc: Cecil Sheng
> Subject: [edk2] [PATCH v2] MdePkg: Add UEFI2.6 HII Image Ex and Image Decoder 
> protocol definition.
> 
> Add the definition for the new UEFI 2.6 EFI_HII_IMAGE_EX_PROTOCOL and 
> EFI_IMAGE_DECODER_PROTOCOL.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cecil Sheng 
> Reviewed-by: Samer El-Haj-Mahmoud 
> Reviewed-by: Abner Chang 
> ---
>  MdePkg/Include/Protocol/HiiImageEx.h   | 234 
> +
>  MdePkg/Include/Protocol/ImageDecoder.h | 164 +++
>  MdePkg/MdePkg.dec  |  13 ++
>  3 files changed, 411 insertions(+)
>  create mode 100644 MdePkg/Include/Protocol/HiiImageEx.h
>  create mode 100644 MdePkg/Include/Protocol/ImageDecoder.h
> 
> diff --git a/MdePkg/Include/Protocol/HiiImageEx.h 
> b/MdePkg/Include/Protocol/HiiImageEx.h
> new file mode 100644
> index 000..3353c97
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/HiiImageEx.h
> @@ -0,0 +1,234 @@
> +/** @file
> +  Protocol which allows access to the images in the images database.
> +
> +(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 that 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.
> +
> +**/
> +
> +#ifndef EFI_HII_IMAGE_EX_H
> +#define EFI_HII_IMAGE_EX_H
> +
> +#include 
> +
> +//
> +// Global ID for the Hii Image Ex Protocol.
> +//
> +#define EFI_HII_IMAGE_EX_PROTOCOL_GUID \
> +  {0x1a1241e6, 0x8f19, 0x41a9,  { 0xbc, 0xe, 0xe8, 0xef, 0x39, 0xe0, 0x65, 
> 0x46 }}
> +
> +typedef struct _EFI_HII_IMAGE_EX_PROTOCOL EFI_HII_IMAGE_EX_PROTOCOL;
> +
> +/**
> +  The prototype of this extension function is the same with 
> EFI_HII_IMAGE_PROTOCOL.NewImage().
> +
> +  @param  This   A pointer to the EFI_HII_IMAGE_EX_PROTOCOL 
> instance.
> +  @param  PackageListHandle of the package list where this image 
> will
> + be added.
> +  @param  ImageIdOn return, contains the new image id, which 
> is
> + unique within PackageList.
> +  @param  Image  Points to the image.
> +
> +  @retval EFI_SUCCESSThe new image was added successfully.
> +  @retval EFI_NOT_FOUND  The specified PackageList could not be 
> found in
> + database.
> +  @retval EFI_OUT_OF_RESOURCES   Could not add the image due to lack of 
> resources.
> +  @retval EFI_INVALID_PARAMETER  Image is NULL or ImageId is NULL.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_HII_NEW_IMAGE_EX)(
> +  IN CONST  EFI_HII_IMAGE_EX_PROTOCOL  *This,
> +  INEFI_HII_HANDLE  PackageList,
> +  OUT   EFI_IMAGE_ID*ImageId,
> +  IN CONST  EFI_IMAGE_INPUT *Image
> +  );
> +
> +/**
> +  Return the information about the image, associated with the package list.
> +  The prototype of this extension function is the same with 
> EFI_HII_IMAGE_PROTOCOL.GetImage().
> +
> +  @param  This   A pointer to the EFI_HII_IMAGE_EX_PROTOCOL 
> instance.
> +  @param  PackageListHandle of the package list where this image 
> will
> + be searched.
> +  @param  ImageIdThe image's id,, which is unique within
> + PackageList.
> +  @param  Image  Points to the image.
> +
> +  @retval EFI_SUCCESSThe new image was returned successfully.
> +  @retval EFI_NOT_FOUND  The image specified by ImageId is not in the
> + database. The specified PackageList is not 
> in
> + the database.
> +  @retval EFI_BUFFER_TOO_SMALL   The buffer specified by ImageSize is too 
> small to
> + hold the image.
> +  @retval EFI_INVALID_PARAMETER  The Image or ImageSize was NULL.
> +  @retval EFI_OUT_OF_RESOURCES   The bitmap could not be retrieved because 
> there
> + was not enough memory.
> +
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *EFI_HII_GET_IMAGE_EX)(
> +  IN CONST  EFI_HII_IMAGE_EX_PROTOCOL   *This,
> +  INEFI_HII_HANDLE  PackageList,
> +  INEFI_IMAGE_IDImageId,
> +  

Re: [edk2] [PATCH 1/2] PerformancePkg/Dp_App: Support execution break

[Zeng, Star]

Cinnamon,

Since you have updated code to replace gBS->LocateHandle with 
gBS->LocateHandleBuffer at previous commits. How about to update the 
"gBS->LocateHandle" to "gBS->LocateHandleBuffer" in the new added 
comments for this patch series?


Other parts of this patch series are good to me.


Thanks,
Star

On 2016/3/5 19:35, Cinnamon Shia wrote:

Support UEFI shell execution break.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Cinnamon Shia 
---
  PerformancePkg/Dp_App/Dp.c | 36 --
  PerformancePkg/Dp_App/DpInternal.h | 27 -
  PerformancePkg/Dp_App/DpTrace.c| 62 --
  3 files changed, 99 insertions(+), 26 deletions(-)

diff --git a/PerformancePkg/Dp_App/Dp.c b/PerformancePkg/Dp_App/Dp.c
index e052216..e36a032 100644
--- a/PerformancePkg/Dp_App/Dp.c
+++ b/PerformancePkg/Dp_App/Dp.c
@@ -14,7 +14,7 @@
timer information to calculate elapsed time for each measurement.

Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
which accompanies this distribution.  The full text of the license may be 
found at
@@ -195,11 +195,11 @@ InitCumulativeData (

@param[in]  ImageHandle The image handle.
@param[in]  SystemTable The system table.
-
+
@retval EFI_SUCCESSCommand completed successfully.
@retval EFI_INVALID_PARAMETER  Command usage error.
+  @retval EFI_ABORTEDThe user aborts the operation.
@retval value  Unknown error.
-
  **/
  EFI_STATUS
  EFIAPI
@@ -443,7 +443,10 @@ InitializeDp (
  ProcessCumulative (CustomCumulativeData);
} else if (AllMode) {
  if (TraceMode) {
-  DumpAllTrace( Number2Display, ExcludeMode);
+  Status = DumpAllTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+goto Done;
+  }
  }
  if (ProfileMode) {
DumpAllProfile( Number2Display, ExcludeMode);
@@ -451,7 +454,10 @@ InitializeDp (
}
else if (RawMode) {
  if (TraceMode) {
-  DumpRawTrace( Number2Display, ExcludeMode);
+  Status = DumpRawTrace( Number2Display, ExcludeMode);
+  if (Status == EFI_ABORTED) {
+goto Done;
+  }
  }
  if (ProfileMode) {
DumpRawProfile( Number2Display, ExcludeMode);
@@ -463,11 +469,21 @@ InitializeDp (
ProcessPhases ( Ticker );
if ( ! SummaryMode) {
  Status = ProcessHandles ( ExcludeMode);
-if ( ! EFI_ERROR( Status)) {
-  ProcessPeims ( );
-  ProcessGlobal ();
-  ProcessCumulative (NULL);
+if (Status == EFI_ABORTED) {
+  goto Done;
  }
+
+Status = ProcessPeims ();
+if (Status == EFI_ABORTED) {
+  goto Done;
+}
+
+Status = ProcessGlobal ();
+if (Status == EFI_ABORTED) {
+  goto Done;
+}
+
+ProcessCumulative (NULL);
}
  }
  if (ProfileMode) {
@@ -480,6 +496,8 @@ InitializeDp (
  }
}

+Done:
+
//
// Free the memory allocate from HiiGetString
//
diff --git a/PerformancePkg/Dp_App/DpInternal.h 
b/PerformancePkg/Dp_App/DpInternal.h
index 0e97e1e..cd686d9 100644
--- a/PerformancePkg/Dp_App/DpInternal.h
+++ b/PerformancePkg/Dp_App/DpInternal.h
@@ -7,7 +7,7 @@
DpUtilities.c, DpTrace.c, and DpProfile.c are included here.

Copyright (c) 2009 - 2014, Intel Corporation. All rights reserved.
-  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
+  (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
which accompanies this distribution.  The full text of the license may be 
found at
@@ -215,8 +215,11 @@ GatherStatistics(
@param[in]Limit   The number of records to print.  Zero is ALL.
@param[in]ExcludeFlag TRUE to exclude individual Cumulative items from 
display.

+  @retval EFI_SUCCESS   The operation was successful.
+  @retval EFI_ABORTED   The user aborts the operation.
+  @return Othersfrom a call to gBS->LocateHandle().
  **/
-VOID
+EFI_STATUS
  DumpAllTrace(
IN UINTN Limit,
IN BOOLEAN   ExcludeFlag
@@ -238,9 +241,11 @@ DumpAllTrace(

@param[in]Limit   The number of records to print.  Zero is ALL.
@param[in]ExcludeFlag TRUE to exclude individual Cumulative items from 
display.
-
+
+  

Re: [edk2] [PATCH] MdeModulePkg: Change the type of PcdMaxPeiPerformanceLogEntries to UINT16

[Gao, Liming]

Cinnamon:
  You can see PCD gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase 
and gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64. 
  In source code edk2\MdeModulePkg\Universal\Variable\Pei\Variable.c line 546, 
the consumer code is like below. 
NvStorageBase = (EFI_PHYSICAL_ADDRESS) (PcdGet64 
(PcdFlashNvStorageVariableBase64) != 0 ?
PcdGet64 
(PcdFlashNvStorageVariableBase64) :
PcdGet32 
(PcdFlashNvStorageVariableBase)
   );

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Shia, Cinnamon
> Sent: Monday, March 07, 2016 10:40 AM
> To: Kinney, Michael D; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdeModulePkg: Change the type of
> PcdMaxPeiPerformanceLogEntries to UINT16
> 
> Hi Mike,
> 
> Thanks for your feedback.
> 
> For solving the backward compatible problem, could you share to me what
> the problem is?
> The PCD is accessed in override c files?
> 
> Regarding to introduce a new PCD, does it mean using the new PCD in c files
> and leaving the existing one as it is?
> 
> Thanks,
> Cinnamon Shia
> 
> -Original Message-
> From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
> Sent: Sunday, March 6, 2016 12:47 AM
> To: Shia, Cinnamon; edk2-devel@lists.01.org; Kinney, Michael D
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Change the type of
> PcdMaxPeiPerformanceLogEntries to UINT16
> 
> Cinnamon,
> 
> Changing the size of a PCD is a not backwards compatible change.
> 
> In the past when we have run into similar issues, we had to introduce a new
> PCD.
> 
> Mike
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Cinnamon Shia
> > Sent: Saturday, March 5, 2016 5:19 AM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH] MdeModulePkg: Change the type of
> > PcdMaxPeiPerformanceLogEntries to UINT16
> >
> > Change the type of PcdMaxPeiPerformanceLogEntries from UINT8 to
> UINT16
> > to log more than 255 performance entries in PEI.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Cinnamon Shia 
> > ---
> >  .../Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 3 ++-
> >  MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c| 8
> 
> >  MdeModulePkg/MdeModulePkg.dec | 3 ++-
> >  3 files changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git
> >
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b.c
> >
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b.c
> > index 0eb8e57..61491e6 100644
> > ---
> >
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b.c
> > +++
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi
> b
> > +++ .c
> > @@ -11,6 +11,7 @@
> >Performance Protocol is installed at the very beginning of DXE phase.
> >
> >  Copyright (c) 2006 - 2015, 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 @@ -522,7 +523,7 @@ DxeCorePerformanceLibConstructor (
> >);
> >ASSERT_EFI_ERROR (Status);
> >
> > -  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet8
> > (PcdMaxPeiPerformanceLogEntries);
> > +  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet16
> > (PcdMaxPeiPerformanceLogEntries);
> >
> >mGaugeData = AllocateZeroPool (sizeof (GAUGE_DATA_HEADER) +
> (sizeof
> > (GAUGE_DATA_ENTRY_EX) * mMaxGaugeRecords));
> >ASSERT (mGaugeData != NULL);
> > diff --git
> > a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > index 9674bbc..74e6300 100644
> > --- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > +++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> > @@ -7,7 +7,7 @@
> >number of performance logging entry is specified by
> PcdMaxPeiPerformanceLogEntries.
> >
> >  Copyright (c) 2006 - 2015, Intel Corporation. All rights
> > reserved.
> > -(C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > +(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  which
> > accompanies this distribution.  The full text of the license may be
> > found at @@ -71,11 +71,11 @@ InternalGetPerformanceHobLog (
> >  // PEI Performance HOB was not found, then build one.
> >  //
> >  

Re: [edk2] [patch] ShellPkg: Merge Ping6 and Ifconfig6 tools to Shell command.

[Zhang, Lubo]

Thank you for the comment, I will update the copyright year when I check in. As 
for the Side Note, it may be easier  for version sync in a patch in my opinion, 
need to wait for other people's Comments.

Thanks 
Lubo 

-Original Message-
From: Carsey, Jaben 
Sent: Saturday, March 05, 2016 1:33 AM
To: Zhang, Lubo ; edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Ye, Ting ; Fu, Siyuan 
; Carsey, Jaben 
Subject: RE: [patch] ShellPkg: Merge Ping6 and Ifconfig6 tools to Shell command.

The copyright change to ShellLibHiiGuid.h needs to be corrected.  It should be 
updated to 2011 - 2016, not replace 2011 with 2016.

If you fix that, then:
Reviewed-By: Jaben Carsey 

SideNote and I am not sure if this is reason enough to reject a patch.

This would be much cleaner to review/understand as a series of patches. My 
thought is this would be a clean order.
Patch 1 - add the GUID to the ShellLibHiiGuid.h and ShellPkg.DEC
Patch 2 - add the new NULL library instance
Patch 3 - update DSC file.

-Jaben

> -Original Message-
> From: Zhang, Lubo
> Sent: Tuesday, March 01, 2016 6:14 PM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Wu, Jiaxin
> ; Ye, Ting ; Fu, Siyuan
> 
> Subject: [patch] ShellPkg: Merge Ping6 and Ifconfig6 tools to Shell command.
> Importance: High
> 
> According to the new Shell spec, we add Network2 profile and
> merge Ping6 and Ifconfig6 tools to Shell command.
> 
> Cc: Carsey Jaben 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> ---
>  ShellPkg/Include/Guid/ShellLibHiiGuid.h|9 +-
>  .../UefiShellNetwork2CommandsLib/Ifconfig6.c   | 1839
> 
>  .../Library/UefiShellNetwork2CommandsLib/Ping6.c   | 1247 +
>  .../UefiShellNetwork2CommandsLib.c |   91 +
>  .../UefiShellNetwork2CommandsLib.h |   72 +
>  .../UefiShellNetwork2CommandsLib.inf   |   63 +
>  .../UefiShellNetwork2CommandsLib.uni   |  151 ++
>  ShellPkg/ShellPkg.dec  |4 +-
>  ShellPkg/ShellPkg.dsc  |8 +-
>  9 files changed, 3479 insertions(+), 5 deletions(-)
>  create mode 100644
> ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
>  create mode 100644
> ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
>  create mode 100644
> ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comma
> ndsLib.c
>  create mode 100644
> ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comma
> ndsLib.h
>  create mode 100644
> ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comma
> ndsLib.inf
>  create mode 100644
> ShellPkg/Library/UefiShellNetwork2CommandsLib/UefiShellNetwork2Comma
> ndsLib.uni
> 
> diff --git a/ShellPkg/Include/Guid/ShellLibHiiGuid.h
> b/ShellPkg/Include/Guid/ShellLibHiiGuid.h
> index 62c2e72..edbfd7c 100644
> --- a/ShellPkg/Include/Guid/ShellLibHiiGuid.h
> +++ b/ShellPkg/Include/Guid/ShellLibHiiGuid.h
> @@ -1,9 +1,9 @@
>  /** @file
>GUIDs for HII package list installed by Shell libraries.
> 
> -  Copyright (c) 2011, Intel Corporation. All rights reserved.
> +  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
> 
> @@ -52,10 +52,16 @@
> 
>  #define SHELL_NETWORK1_HII_GUID \
>{ \
>  0xf3d301bb, 0xf4a5, 0x45a8, { 0xb0, 0xb7, 0xfa, 0x99, 0x9c, 0x62, 0x37,
> 0xae } \
>}
> +
> +#define SHELL_NETWORK2_HII_GUID \
> +  { \
> +0x174b2b5, 0xf505, 0x4b12, { 0xaa, 0x60, 0x59, 0xdf, 0xf8, 0xd6, 0xea,
> 0x37 } \
> +  }
> +
>  #define SHELL_TFTP_HII_GUID \
>{ \
>  0x738a9314, 0x82c1, 0x4592, { 0x8f, 0xf7, 0xc1, 0xbd, 0xf1, 0xb2, 0x0e,
> 0xd4 } \
>}
> 
> @@ -71,9 +77,10 @@ extern EFI_GUID gShellDriver1HiiGuid;
>  extern EFI_GUID gShellInstall1HiiGuid;
>  extern EFI_GUID gShellLevel1HiiGuid;
>  extern EFI_GUID gShellLevel2HiiGuid;
>  extern EFI_GUID gShellLevel3HiiGuid;
>  extern EFI_GUID gShellNetwork1HiiGuid;
> +extern EFI_GUID gShellNetwork2HiiGuid;
>  extern EFI_GUID gShellTftpHiiGuid;
>  extern EFI_GUID gShellBcfgHiiGuid;
> 
>  #endif
> diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> new file mode 100644
> index 000..b1592e6
> --- /dev/null
> +++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ifconfig6.c
> @@ -0,0 

Re: [edk2] [PATCH] MdeModulePkg: Change the type of PcdMaxPeiPerformanceLogEntries to UINT16

[Shia, Cinnamon]

Hi Mike,

Thanks for your feedback.

For solving the backward compatible problem, could you share to me what the 
problem is?
The PCD is accessed in override c files?

Regarding to introduce a new PCD, does it mean using the new PCD in c files and 
leaving the existing one as it is?

Thanks,
Cinnamon Shia

-Original Message-
From: Kinney, Michael D [mailto:michael.d.kin...@intel.com] 
Sent: Sunday, March 6, 2016 12:47 AM
To: Shia, Cinnamon; edk2-devel@lists.01.org; Kinney, Michael D
Subject: RE: [edk2] [PATCH] MdeModulePkg: Change the type of 
PcdMaxPeiPerformanceLogEntries to UINT16

Cinnamon,

Changing the size of a PCD is a not backwards compatible change.

In the past when we have run into similar issues, we had to introduce a new PCD.

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Cinnamon Shia
> Sent: Saturday, March 5, 2016 5:19 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH] MdeModulePkg: Change the type of 
> PcdMaxPeiPerformanceLogEntries to UINT16
> 
> Change the type of PcdMaxPeiPerformanceLogEntries from UINT8 to UINT16 
> to log more than 255 performance entries in PEI.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Cinnamon Shia 
> ---
>  .../Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 3 ++-
>  MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c| 8 
> 
>  MdeModulePkg/MdeModulePkg.dec | 3 ++-
>  3 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git 
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> index 0eb8e57..61491e6 100644
> --- 
> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c
> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib
> +++ .c
> @@ -11,6 +11,7 @@
>Performance Protocol is installed at the very beginning of DXE phase.
> 
>  Copyright (c) 2006 - 2015, 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 @@ -522,7 +523,7 @@ DxeCorePerformanceLibConstructor (
>);
>ASSERT_EFI_ERROR (Status);
> 
> -  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet8 
> (PcdMaxPeiPerformanceLogEntries);
> +  mMaxGaugeRecords = INIT_DXE_GAUGE_DATA_ENTRIES + PcdGet16
> (PcdMaxPeiPerformanceLogEntries);
> 
>mGaugeData = AllocateZeroPool (sizeof (GAUGE_DATA_HEADER) + (sizeof
> (GAUGE_DATA_ENTRY_EX) * mMaxGaugeRecords));
>ASSERT (mGaugeData != NULL);
> diff --git 
> a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> index 9674bbc..74e6300 100644
> --- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> +++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
> @@ -7,7 +7,7 @@
>number of performance logging entry is specified by 
> PcdMaxPeiPerformanceLogEntries.
> 
>  Copyright (c) 2006 - 2015, Intel Corporation. All rights 
> reserved.
> -(C) Copyright 2015 Hewlett Packard Enterprise Development LP
> +(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  which 
> accompanies this distribution.  The full text of the license may be 
> found at @@ -71,11 +71,11 @@ InternalGetPerformanceHobLog (
>  // PEI Performance HOB was not found, then build one.
>  //
>  PeiPerformanceSize = sizeof (PEI_PERFORMANCE_LOG_HEADER) +
> - sizeof (PEI_PERFORMANCE_LOG_ENTRY) * PcdGet8
> (PcdMaxPeiPerformanceLogEntries);
> + sizeof (PEI_PERFORMANCE_LOG_ENTRY) * 
> + PcdGet16
> (PcdMaxPeiPerformanceLogEntries);
>  *PeiPerformanceLog = BuildGuidHob (,
> PeiPerformanceSize);
>  *PeiPerformanceLog = ZeroMem (*PeiPerformanceLog, 
> PeiPerformanceSize);
> 
> -PeiPerformanceSize = sizeof (UINT32) * PcdGet8
> (PcdMaxPeiPerformanceLogEntries);
> +PeiPerformanceSize = sizeof (UINT32) * PcdGet16
> (PcdMaxPeiPerformanceLogEntries);
>  *PeiPerformanceIdArray = BuildGuidHob 
> (, PeiPerformanceSize);
>  *PeiPerformanceIdArray = ZeroMem (*PeiPerformanceIdArray, 
> PeiPerformanceSize);
>}
> @@ -183,7 +183,7 @@ StartPerformanceMeasurementEx (
> 
>InternalGetPerformanceHobLog (, 
> );
> 
> -  if (PeiPerformanceLog->NumberOfEntries >= PcdGet8 
> (PcdMaxPeiPerformanceLogEntries)) {
> +  if (PeiPerformanceLog->NumberOfEntries >= PcdGet16 
> + (PcdMaxPeiPerformanceLogEntries))
> {
>  DEBUG ((DEBUG_ERROR, "PEI 

Re: [edk2] allocation of PMem64 BARs

[Ni, Ruiyu]

Thanks,
Ray

> 在 2016年3月6日，下午8:30，Laszlo Ersek  写道：
> 
>> On 03/05/16 10:54, Ni, Ruiyu wrote:
>> Laszlo,
>> 
>> Does [0|3|0] contain option rom? (video controller?)
>> 
>> If it contains option rom, PciBusDxe driver has a logic to
>> 
>> convert the PMem64 to PMem32. PMem32 is converted
>> 
>> to Mem32 because COMBINE_MEM_PMEM is set.
>> 
>> 
>> 
>> The detailed logic is in PciResourceSupport.c: DegradeResource().
> 
> Right; I had found that function before I sent my email, but it looked
> quite complex; so I thought I would ask my question without cluttering
> the email with doubts about that function.
> 
> You are correct: the device is a physical GTX750 GPU assigned to the
> guest, and it does have an option ROM.
> 
> I saw the comment in the source:
> 
>  //
>  // If any child device has both option ROM and 64-bit BAR, degrade its
>  // PMEM64/MEM64 requests in case that if a legacy option ROM image can
>  // not access 64-bit resources.
>  //
> 
> but I thought that this degradation would only happen if the option ROM
> in question were *for certain* a legacy option ROM *only*. In this case
> the GPU has a well-working 64-bit UEFI firmware, so I mostly ignored the
> code under the comment.
> 
> Now I'm noticing that the condition is:
> 
>Temp = PCI_IO_DEVICE_FROM_LINK (ChildDeviceLink);
>if (Temp->RomSize != 0) {
> 
> i.e., any kind of option ROM suffices to degrade the region.
> 
> So, my question is answered. Thank you!
You are welcome!

> Laszlo
> 
>> 
>> 
>> 
>> Regards,
>> 
>> Ray
>> 
>> 
>> 
>> *From:*Laszlo Ersek [mailto:ler...@redhat.com]
>> *Sent:* Saturday, March 5, 2016 5:41 AM
>> *To:* Ni, Ruiyu 
>> *Cc:* edk2-devel-ml01 
>> *Subject:* allocation of PMem64 BARs
>> 
>> 
>> 
>> Hi Ray,
>> 
>> on top of my pending series
>> 
>>  [edk2] [PATCH 0/5] OvmfPkg: enable PCIe on Q35
>>  http://thread.gmane.org/gmane.comp.bios.edk2.devel/8707
>> 
>> I also have some patches (not posted yet) that enable 64-bit MMIO. I
>> tested it and it seems to work, but something is strange to me.
>> 
>> Namely, there is enough room in the non-prefetchable 64-bit aperture for
>> allocating some PMem64 BARs (and EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM is
>> passed to the driver), but the driver decides to allocate those from the
>> (also non-prefetchable) 32-bit aperture instead.
>> 
>> It doesn't seem invalid, of course, but I thought that the driver would
>> conserve the 32-bit aperture, and allocate everything possible from the
>> 64-bit aperture.
>> 
>> I also have a Mem64 BAR, and that gets allocated high just fine.
>> 
>> Here's the initial output from the host bridge driver:
>> 
>>> RootBridge: PciRoot(0x0)
>>>  Support/Attr: 70069 / 70069
>>>DmaAbove4G: No
>>> NoExtConfSpace: No
>>> AllocAttr: 3 (CombineMemPMem Mem64Decode)
>>>   Bus: 0 - FF
>>>Io: C000 - 
>>>   Mem: C000 - FBFF
>>>MemAbove4G: 18000 - 97FFF
>>>  PMem: 0 - 0
>>>   PMemAbove4G: 0 - 0
>>> PciHostBridgeDxe: IntersectMemoryDescriptor: add [18000, 98000): 
>>> Success
>> 
>> Then the output from the PCI bus driver, interspersed with messages from
>> the host bridge driver:
>> 
>>> PCI Bus First Scanning
>>> PciBus: Discovered PCI @ [00|00|00]
>>> 
>>> PciBus: Discovered PCI @ [00|01|00]
>>>   BAR[0]: Type =  Mem32; Alignment = 0x3FF;  Length = 0x400;  
>>> Offset = 0x10
>>>   BAR[1]: Type =  Mem32; Alignment = 0x3FF;  Length = 0x400;  
>>> Offset = 0x14
>>>   BAR[2]: Type =  Mem32; Alignment = 0x1FFF;  Length = 0x2000;  Offset = 
>>> 0x18
>>>   BAR[3]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x1C
>>> 
>>> PciBus: Discovered PCI @ [00|02|00]
>>>   BAR[0]: Type =  Mem64; Alignment = 0x3FFF;  Length = 0x4000;  Offset = 
>>> 0x10
>>> 
>>> PciBus: Discovered PCI @ [00|03|00]
>>>   BAR[0]: Type =  Mem32; Alignment = 0xFF;  Length = 0x100;  Offset 
>>> = 0x10
>>>   BAR[1]: Type = PMem64; Alignment = 0xFFF;  Length = 0x1000;  
>>> Offset = 0x14
>>>   BAR[2]: Type = PMem64; Alignment = 0x1FF;  Length = 0x200;  
>>> Offset = 0x1C
>>>   BAR[3]: Type =   Io32; Alignment = 0x7F;  Length = 0x80;  Offset = 0x24
>>> 
>>> PciBus: Discovered PCI @ [00|04|00]
>>>   BAR[0]: Type =  Mem32; Alignment = 0x3FFF;  Length = 0x4000;  Offset = 
>>> 0x10
>>> 
>>> PciBus: Discovered PPB @ [00|1E|00]
>>> 
>>> PciBus: Discovered PPB @ [01|01|00]
>>>   BAR[0]: Type =  Mem64; Alignment = 0xFFF;  Length = 0x100;  Offset = 0x10
>>> 
>>> PciBus: Discovered PCI @ [02|01|00]
>>>   BAR[0]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x10
>>>   BAR[1]: Type =  Mem32; Alignment = 0xFFF;  Length = 0x1000;  Offset = 0x14
>>> 
>>> PciBus: Discovered PCI @ [02|02|00]
>>>   BAR[4]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x20
>>> 
>>> PciBus: Discovered PCI @ [02|02|01]
>>>   BAR[4]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x20
>>> 
>>> 

Re: [edk2] [Patch 1/2] MdeModulePkg/UefiBootManagerLib: Separate boot description functions.

[Fu, Siyuan]

Patch is good to me.
Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, March 4, 2016 6:15 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Tian, Feng ; Fu,
> Siyuan 
> Subject: [Patch 1/2] MdeModulePkg/UefiBootManagerLib: Separate boot
> description functions.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Feng Tian 
> Cc: Siyuan Fu 
> ---
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   | 576 ---
> -
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 586
> +
>  .../Library/UefiBootManagerLib/InternalBm.h|  69 +--
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   3 +-
>  4 files changed, 613 insertions(+), 621 deletions(-)
>  create mode 100644
> MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> 
> diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> index 18f835a..0b86cb9 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> @@ -15,19 +15,9 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #include "InternalBm.h"
> 
> -#define VENDOR_IDENTIFICATION_OFFSET 3
> -#define VENDOR_IDENTIFICATION_LENGTH 8
> -#define PRODUCT_IDENTIFICATION_OFFSET11
> -#define PRODUCT_IDENTIFICATION_LENGTH16
> -
> -CONST UINT16 mBmUsbLangId= 0x0409; // English
> -CHAR16   mBmUefiPrefix[] = L"UEFI ";
> -
>  EFI_BOOT_MANAGER_REFRESH_LEGACY_BOOT_OPTION
> mBmRefreshLegacyBootOption = NULL;
>  EFI_BOOT_MANAGER_LEGACY_BOOT mBmLegacyBoot  = 
> NULL;
> 
> -LIST_ENTRY mPlatformBootDescriptionHandlers =
> INITIALIZE_LIST_HEAD_VARIABLE (mPlatformBootDescriptionHandlers);
> -
>  ///
>  /// This GUID is used for an EFI Variable that stores the front device pathes
>  /// for a partial device path that starts with the HD node.
> @@ -75,107 +65,6 @@ BmIsAutoCreateBootOption (
>  }
> 
>  /**
> -  For a bootable Device path, return its boot type.
> -
> -  @param  DevicePath   The bootable device Path to check
> -
> -  @retval AcpiFloppyBoot   If given device path contains
> ACPI_DEVICE_PATH type device path node
> -   which HID is floppy device.
> -  @retval MessageAtapiBoot If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_ATAPI_DP.
> -  @retval MessageSataBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_SATA_DP.
> -  @retval MessageScsiBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_SCSI_DP.
> -  @retval MessageUsbBoot   If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_USB_DP.
> -  @retval MessageNetworkBoot   If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is
> MSG_MAC_ADDR_DP, MSG_VLAN_DP,
> -   MSG_IPv4_DP or MSG_IPv6_DP.
> -  @retval MessageHttpBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_URI_DP.
> -  @retval UnsupportedBoot  If tiven device path doesn't match the
> above condition, it's not supported.
> -
> -**/
> -BM_BOOT_TYPE
> -BmDevicePathType (
> -  IN  EFI_DEVICE_PATH_PROTOCOL *DevicePath
> -  )
> -{
> -  EFI_DEVICE_PATH_PROTOCOL  *Node;
> -  EFI_DEVICE_PATH_PROTOCOL  *NextNode;
> -
> -  ASSERT (DevicePath != NULL);
> -
> -  for (Node = DevicePath; !IsDevicePathEndType (Node); Node =
> NextDevicePathNode (Node)) {
> -switch (DevicePathType (Node)) {
> -
> -  case ACPI_DEVICE_PATH:
> -if (EISA_ID_TO_NUM (((ACPI_HID_DEVICE_PATH *) Node)->HID) ==
> 0x0604) {
> -  return BmAcpiFloppyBoot;
> -}
> -break;
> -
> -  case HARDWARE_DEVICE_PATH:
> -if (DevicePathSubType (Node) == HW_CONTROLLER_DP) {
> -  return BmHardwareDeviceBoot;
> -}
> -break;
> -
> -  case MESSAGING_DEVICE_PATH:
> -//
> -// Skip LUN device node
> -//
> -NextNode = Node;
> -do {
> -  NextNode = NextDevicePathNode (NextNode);

Re: [edk2] [Patch 2/2] MdeModulePkg/Bds: More user-friendly network boot option description

[Fu, Siyuan]

Patch is good to me.
Reviewed-by: Fu Siyuan 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, March 4, 2016 6:15 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Fu, Siyuan 
> Subject: [Patch 2/2] MdeModulePkg/Bds: More user-friendly network boot
> option description
> 
> The patch enhances the UefiBootManagerLib to use more user-friendly
> network boot option description.
> It builds description like below:
>  "PXEv6 (MAC:112233445566 VLAN1)"
>  "HTTPv4 (MAC:112233445566)"
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Siyuan Fu 
> ---
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 170
> -
>  .../Library/UefiBootManagerLib/InternalBm.h|   2 -
>  2 files changed, 133 insertions(+), 39 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> index 5c77e86..69a2914 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> @@ -28,24 +28,19 @@ LIST_ENTRY mPlatformBootDescriptionHandlers =
> INITIALIZE_LIST_HEAD_VARIABLE (mPl
>  /**
>For a bootable Device path, return its boot type.
> 
> -  @param  DevicePath   The bootable device Path to check
> -
> -  @retval AcpiFloppyBoot   If given device path contains
> ACPI_DEVICE_PATH type device path node
> -   which HID is floppy device.
> -  @retval MessageAtapiBoot If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_ATAPI_DP.
> -  @retval MessageSataBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_SATA_DP.
> -  @retval MessageScsiBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_SCSI_DP.
> -  @retval MessageUsbBoot   If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_USB_DP.
> -  @retval MessageNetworkBoot   If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is
> MSG_MAC_ADDR_DP, MSG_VLAN_DP,
> -   MSG_IPv4_DP or MSG_IPv6_DP.
> -  @retval MessageHttpBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> -   and its last device path node's 
> subtype is MSG_URI_DP.
> -  @retval UnsupportedBoot  If tiven device path doesn't match the
> above condition, it's not supported.
> +  @param  DevicePathThe bootable device Path to check
> +
> +  @retval AcpiFloppyBootIf given device path contains ACPI_DEVICE_PATH
> type device path node
> +which HID is floppy device.
> +  @retval MessageAtapiBoot  If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> +and its last device path node's subtype is 
> MSG_ATAPI_DP.
> +  @retval MessageSataBoot   If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> +and its last device path node's subtype is 
> MSG_SATA_DP.
> +  @retval MessageScsiBoot   If given device path contains
> MESSAGING_DEVICE_PATH type device path node
> +and its last device path node's subtype is 
> MSG_SCSI_DP.
> +  @retval MessageUsbBootIf given device path contains
> MESSAGING_DEVICE_PATH type device path node
> +and its last device path node's subtype is 
> MSG_USB_DP.
> +  @retval BmMiscBootIf tiven device path doesn't match the above
> condition.
> 
>  **/
>  BM_BOOT_TYPE
> @@ -108,17 +103,6 @@ BmDevicePathType (
>  case MSG_SCSI_DP:
>return BmMessageScsiBoot;
>break;
> -
> -case MSG_MAC_ADDR_DP:
> -case MSG_VLAN_DP:
> -case MSG_IPv4_DP:
> -case MSG_IPv6_DP:
> -  return BmMessageNetworkBoot;
> -  break;
> -
> -case MSG_URI_DP:
> -  return BmMessageHttpBoot;
> -  break;
>  }
>  }
>}
> @@ -352,6 +336,125 @@ BmGetUsbDescription (
>  }
> 
>  /**
> +  Return the description for network boot device.
> +
> +  @param HandleController handle.
> +
> +  @return  The description string.
> +**/
> +CHAR16 *
> 

Re: [edk2] [PATCH 1/2] ModulePkg/DxeHttpLib: Adding Functions to HttpLib

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ghazi Belaam
> Sent: Saturday, March 5, 2016 6:08 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng ; Wu, Jiaxin ; Fu,
> Siyuan ; Zeng, Star 
> Subject: [edk2] [PATCH 1/2] ModulePkg/DxeHttpLib: Adding Functions to
> HttpLib
> 
> There some usefull functions in edk2 private modules that could be used,
> so we added them to the httpLib
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ghazi Belaam 
> ---
>  MdeModulePkg/Include/Library/HttpLib.h | 129 ++
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.c   | 595
> ++---
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.h   |  89 
>  MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf |   6 +
>  4 files changed, 753 insertions(+), 66 deletions(-)
>  create mode 100644 MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.h
> 
> diff --git a/MdeModulePkg/Include/Library/HttpLib.h
> b/MdeModulePkg/Include/Library/HttpLib.h
> index cd97b64..af9ab5f 100644
> --- a/MdeModulePkg/Include/Library/HttpLib.h
> +++ b/MdeModulePkg/Include/Library/HttpLib.h
> @@ -3,6 +3,7 @@
>It provides the helper routines to parse the HTTP message byte stream.
> 
>  Copyright (c) 2015 - 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
> @@ -18,6 +19,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #include 
> 
> +
>  /**
>Decode a percent-encoded URI component to the ASCII character.
> 
> @@ -343,5 +345,132 @@ HttpFreeMsgParser (
>);
> 
> 
> +/**
> +  Find a specified header field according to the field name.
> +
> +  @param[in]   HeaderCount  Number of HTTP header structures in
> Headers list.
> +  @param[in]   Headers  Array containing list of HTTP headers.
> +  @param[in]   FieldNameNull terminated string which describes a 
> field
> name.
> +
> +  @returnPointer to the found header or NULL.
> +
> +**/
> +EFI_HTTP_HEADER *
> +EFIAPI
> +HttpFindHeader (
> +  IN  UINTNHeaderCount,
> +  IN  EFI_HTTP_HEADER  *Headers,
> +  IN  CHAR8*FieldName
> +  );
> +
> +/**
> +  Set FieldName and FieldValue into specified HttpHeader.
> +
> +  @param[in,out]  HttpHeader  Specified HttpHeader.
> +  @param[in]  FieldName   FieldName of this HttpHeader, a NULL
> terminated ASCII string.
> +  @param[in]  FieldValue  FieldValue of this HttpHeader, a NULL
> terminated ASCII string.
> +
> +
> +  @retval EFI_SUCCESS The FieldName and FieldValue are set into
> HttpHeader successfully.
> +  @retval EFI_OUT_OF_RESOURCESFailed to allocate resources.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HttpSetFieldNameAndValue (
> +   IN  OUT   EFI_HTTP_HEADER   *HttpHeader,
> +   IN  CONST CHAR8 *FieldName,
> +   IN  CONST CHAR8 *FieldValue
> +  );
> +
> +/**
> +  Get one key/value header pair from the raw string.
> +
> +  @param[in]  String Pointer to the raw string.
> +  @param[out] FieldName  Points directly to field name within
> 'HttpHeader'.
> +  @param[out] FieldValue Points directly to field value within
> 'HttpHeader'.
> +
> +  @return Pointer to the next raw string.
> +  @return NULL if no key/value header pair from this raw string.
> +
> +**/
> +CHAR8 *
> +EFIAPI
> +HttpGetFieldNameAndValue (
> +  IN CHAR8   *String,
> + OUT CHAR8   **FieldName,
> + OUT CHAR8   **FieldValue
> +  );
> +
> +/**
> +  Free existing HeaderFields.
> +
> +  @param[in]  HeaderFields   Pointer to array of key/value header pairs
> waiting for free.
> +  @param[in]  FieldCount The number of header pairs in HeaderFields.
> +
> +**/
> +VOID
> +EFIAPI
> +HttpFreeHeaderFields (
> +  IN  EFI_HTTP_HEADER  *HeaderFields,
> +  IN  UINTNFieldCount
> +  );
> +
> +/**
> +  Generate HTTP request string.
> +
> +  @param[in]   MessagePointer to storage containing HTTP message
> data.
> +  @param[in]   UrlThe URL of a remote host.
> +  @param[out]  RequestString  Pointer to the created HTTP request string.
> +  NULL if any error occured.
> +
> +  @return EFI_SUCCESS If HTTP request string was created 
> successfully
> +  @retval EFI_OUT_OF_RESOURCESFailed to allocate resources.
> +  @retval EFI_INVALID_PARAMETER   The input arguments are invalid
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +HttpGenRequestString (
> +  IN CONST EFI_HTTP_MESSAGE

Re: [edk2] [PATCH 2/2] NetworkPkg: Use the New Functions from HttpLib

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 


> -Original Message-
> From: Ghazi Belaam [mailto:ghazi.bel...@hpe.com]
> Sent: Saturday, March 5, 2016 6:08 AM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng ; Zeng, Star ; Fu,
> Siyuan ; Wu, Jiaxin ; samer.el-
> haj-mahm...@hpe.com; Ghazi Belaam 
> Subject: [PATCH 2/2] NetworkPkg: Use the New Functions from HttpLib
> 
> After submitting changes for HttpLib, other modules should be able to use
> those functions
> 1 remove the private function and their calls
> 2 update it with the functions from httpLib
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ghazi Belaam 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  43 +---
>  NetworkPkg/HttpDxe/HttpImpl.c  |   9 +-
>  NetworkPkg/HttpDxe/HttpProto.c | 112 +
>  NetworkPkg/HttpDxe/HttpProto.h |  33 +--
>  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.h |  94 +--
>  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesDxe.inf   |   3 +-
>  NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesImpl.c| 279 
> -
>  .../HttpUtilitiesDxe/HttpUtilitiesProtocol.c   |  27 +-
>  8 files changed, 35 insertions(+), 565 deletions(-)
>  delete mode 100644 NetworkPkg/HttpUtilitiesDxe/HttpUtilitiesImpl.c
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> index db2af78..5a22a17 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootSupport.c
> @@ -2,8 +2,9 @@
>Support functions implementation for UEFI HTTP boot driver.
> 
>  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 that accompanies this
> distribution.
> +(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 that accompanies this
> distribution.
>  The full text of the license may be found at
>  http://opensource.org/licenses/bsd-license.php.
> 
> @@ -548,44 +549,10 @@ HttpBootFreeHeader (
>  }
> 
>  /**
> -  Find a specified header field according to the field name.
> -
> -  @param[in]   HeaderCount  Number of HTTP header structures in Headers
> list.
> -  @param[in]   Headers  Array containing list of HTTP headers.
> -  @param[in]   FieldNameNull terminated string which describes a 
> field
> name.
> -
> -  @returnPointer to the found header or NULL.
> -
> -**/
> -EFI_HTTP_HEADER *
> -HttpBootFindHeader (
> -  IN  UINTNHeaderCount,
> -  IN  EFI_HTTP_HEADER  *Headers,
> -  IN  CHAR8*FieldName
> -  )
> -{
> -  UINTN Index;
> -
> -  if (HeaderCount == 0 || Headers == NULL || FieldName == NULL) {
> -return NULL;
> -  }
> -
> -  for (Index = 0; Index < HeaderCount; Index++){
> -//
> -// Field names are case-insensitive (RFC 2616).
> -//
> -if (AsciiStriCmp (Headers[Index].FieldName, FieldName) == 0) {
> -  return [Index];
> -}
> -  }
> -  return NULL;
> -}
> -
> -/**
>Set or update a HTTP header with the field name and corresponding value.
> 
>@param[in]  HttpIoHeader   Point to the HTTP header holder.
> -  @param[in]  FieldName  Null terminated string which describes a 
> field
> name.
> +  @param[in]  FieldName  Null terminated string which describes a 
> field
> name.
>@param[in]  FieldValue Null terminated string which describes the
> corresponding field value.
> 
>@retval  EFI_SUCCESS   The HTTP header has been set or updated.
> @@ -609,7 +576,7 @@ HttpBootSetHeader (
>  return EFI_INVALID_PARAMETER;
>}
> 
> -  Header = HttpBootFindHeader (HttpIoHeader->HeaderCount,
> HttpIoHeader->Headers, FieldName);
> +  Header = HttpFindHeader (HttpIoHeader->HeaderCount, HttpIoHeader-
> >Headers, FieldName);
>if (Header == NULL) {
>  //
>  // Add a new header.
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index a068cfb..63b683e 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -2,7 +2,7 @@
>Implementation of EFI_HTTP_PROTOCOL protocol interfaces.
> 
>Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> -  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> +  (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
> @@ -497,9 +497,10 @@ EfiHttpRequest (
>goto Error3;
>  }
>}
> -  RequestStr = 

Re: [edk2] [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot driver.

[Wu, Jiaxin]

Reviewed-by: Jiaxin Wu 

> -Original Message-
> From: Fu, Siyuan
> Sent: Friday, March 4, 2016 4:38 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting 
> Subject: [PATCH v3] NetworkPkg: Add URI configuration form to HTTP boot
> driver.
> 
> This patch updates the HTTP boot driver to produce a setup page for the
> boot
> file URI configuration. A new boot option will be created for the manual
> configured URI address. This change is made to support the HTTP boot usage
> in home environment.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> ---
>  NetworkPkg/HttpBootDxe/HttpBootClient.c|  99 +--
>  NetworkPkg/HttpBootDxe/HttpBootConfig.c| 723
> +
>  NetworkPkg/HttpBootDxe/HttpBootConfig.h|  78 +++
>  NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h |  43 ++
>  NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni   | Bin 0 -> 2926 bytes
>  NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr   |  53 ++
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.c | 111 +++-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp4.h |   9 +-
>  NetworkPkg/HttpBootDxe/HttpBootDhcp6.c |   6 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.c   |  44 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.h   |  33 +-
>  NetworkPkg/HttpBootDxe/HttpBootDxe.inf |  17 +-
>  NetworkPkg/HttpBootDxe/HttpBootImpl.c  |  71 +-
>  NetworkPkg/HttpBootDxe/HttpBootSupport.c   |  63 ++
>  NetworkPkg/HttpBootDxe/HttpBootSupport.h   |  18 +
>  NetworkPkg/Include/Guid/HttpBootConfigHii.h|  25 +
>  NetworkPkg/NetworkPkg.dec  |   5 +-
>  17 files changed, 1288 insertions(+), 110 deletions(-)
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.c
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfig.h
>  create mode 100644
> NetworkPkg/HttpBootDxe/HttpBootConfigNVDataStruc.h
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigStrings.uni
>  create mode 100644 NetworkPkg/HttpBootDxe/HttpBootConfigVfr.vfr
>  create mode 100644 NetworkPkg/Include/Guid/HttpBootConfigHii.h
> 
> diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> index 2ccac8c..aae4527 100644
> --- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
> +++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
> @@ -168,18 +168,35 @@ HttpBootDhcp4ExtractUriInfo (
>// HttpOffer contains the boot file URL.
>//
>SelectOffer = >OfferBuffer[SelectIndex].Dhcp4;
> -  if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) || (SelectOffer-
> >OfferType == HttpOfferTypeDhcpNameUriDns)) {
> -HttpOffer = SelectOffer;
> +  if (Private->FilePathUri == NULL) {
> +//
> +// In Corporate environment, we need a HttpOffer.
> +//
> +if ((SelectOffer->OfferType == HttpOfferTypeDhcpIpUri) ||
> +(SelectOffer->OfferType == HttpOfferTypeDhcpIpUriDns) ||
> +(SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns)) {
> +  HttpOffer = SelectOffer;
> +} else {
> +  ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
> +  ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
> +  HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
> +}
> +Private->BootFileUriParser = HttpOffer->UriParser;
> +Private->BootFileUri = (CHAR8*) HttpOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_BOOTFILE]->Data;
>} else {
> -ASSERT (Private->SelectProxyType != HttpOfferTypeMax);
> -ProxyIndex = Private->OfferIndex[Private->SelectProxyType][0];
> -HttpOffer = >OfferBuffer[ProxyIndex].Dhcp4;
> +//
> +// In Home environment the BootFileUri comes from the FilePath.
> +//
> +Private->BootFileUriParser = Private->FilePathUriParser;
> +Private->BootFileUri = Private->FilePathUri;
>}
> 
>//
>// Configure the default DNS server if server assigned.
>//
> -  if ((SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns) ||
> (SelectOffer->OfferType == HttpOfferTypeDhcpDns)) {
> +  if ((SelectOffer->OfferType == HttpOfferTypeDhcpNameUriDns) ||
> +  (SelectOffer->OfferType == HttpOfferTypeDhcpDns) ||
> +  (SelectOffer->OfferType == HttpOfferTypeDhcpIpUriDns)) {
>  Option = SelectOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_DNS_SERVER];
>  ASSERT (Option != NULL);
>  Status = HttpBootRegisterIp4Dns (
> @@ -196,8 +213,8 @@ HttpBootDhcp4ExtractUriInfo (
>// Extract the port from URL, and use default HTTP port 80 if not provided.
>//
>Status = HttpUrlGetPort (
> - (CHAR8*) HttpOffer-
> >OptList[HTTP_BOOT_DHCP4_TAG_INDEX_BOOTFILE]->Data,
> - HttpOffer->UriParser,
> + Private->BootFileUri,
> + Private->BootFileUriParser,
>   >Port
>   );
>if 

Re: [edk2] [PATCH 3/5] OvmfPkg: add DxePciLibI440FxQ35

[Laszlo Ersek]

On 03/05/16 00:47, Kinney, Michael D wrote:
> Jordan,
> 
> Given these libs are Base type, a Dynamic PCD could not be used.  Only a fixed
> or patchable PCD could be used, and if you go through path, you might as well
> pick the right lib in the DSC at build time and get smaller size and better 
> performance.

I'd like to discuss two things here.

* First, I fully agree with you that determining the branch in this
unified library instance *at build time* is functionally equivalent (but
is size- and perf-wise inferior) to simply picking the right one of the
currently available PciLib instances, at build time.

I think that this library instance is special to virtualization. On the
bare metal, you never face a situation where you don't know in advance
whether your platform will be a PCIe system, or a PCI-only system. But,
that's exactly what we have with QEMU (the i440fx and Q35 machine
types), so this dynamism is needed. (In fact I find it very nice that
the modularity of edk2 allowed me to do such a simple unification.)

Another possibility for influencing the branch to take would be a GUID
HOB. In this particular case however I only saw downsides in a HOB. On
the source code side, it would take another GUID definition, another
structure definition, possibly another header file. And on the
functionality side, the only "extension" it would buy us would be the
usability of the library instance in the DXE_CORE. (The DXE_CORE cannot
use PCDs, but it can look at HOBs.)

We couldn't use it in PEI anyway (without ugly depex trickery), because
a PEIM using this library instance could be dispatched before the PEIM
that produces the HOB.

But, in OVMF, PEI is perfectly fine with just 0xCF8 access anyway.

* Second, any given library instance has two kinds of "qualification" in
its INF file, as far as client modules are concerned. One is
MODULE_TYPE, and the other is the restriction list in LIBRARY_CLASS.

These qualifications seem to overlap, in a (superficially) confusing manner:

(a) the INF file spec says about MODULE_TYPE,

For Library Modules, the MODULE_TYPE must specify the MODULE_TYPE
of the module that will use the driver.

Furthermore, in appendix G, MODULE_TYPE=BASE is explicitly allowed for
libraries, and then BASE is documented as:

Modules of this type can be ported to any execution environment.
This module type is intended to be use by silicon module developers
to produce source code that is not tied to any specific execution
environment.

(b) But the INF file also says

Each LIBRARY_CLASS statement must provide the name of the library
class supported, followed by the pipe "|" field separator and then
a space " " delimited list of module types the library instances
supports.

Thus a conflict appears between

  MODULE_TYPE = BASE

and

  LIBRARY_CLASS = WhateverLib|DXE_DRIVER

because the former implies "any execution environment", whereas the
second implies the DXE phase.

However, this apparent conflict is resolved by the INF spec in the
following:

The MODULE_TYPE entry in the [Defines] section for a library only
defines the module type that the build system must assume for
building the library. It does not define the types of modules that
a library may be linked with. Instead, the LIBRARY_CLASS entry in
the [Defines] section optionally lists the supported module types
that the library may be linked against.

Therefore, the MODULE_TYPE for a library instance only determines
prototype / calling convention compatibility. In my experience, this
practically boils down to two things, and nothing more:

- functions must return RETURN_STATUS vs. EFI_STATUS,

- the constructor function of the library instance (if any) gets VOID
  if the MODULE_TYPE is BASE, vs. ImageHandle plus SystemTable if the
  MODULE_TYPE is DXE_DRIVER.

That's it.

What *really* matters is the restriction list in LIBRARY_CLASS; that's
where the author of the library instance ensures that the instance
doesn't get linked into a module (and firmware phase) that cannot
provide the library instance with the necessary environment.

Therefore, making a library instance MODULE_TYPE=BASE, and then
restricting it to the DXE phase and later with LIBRARY_CLASS, is valid
in my opinion. The library instance may not need ImageHandle and
SystemTable for its own code at all, it is okay with returning
RETURN_STATUS values, but it might need the environment to be DXE or later.

The question then emerges, how we should *name* the library instance. In
my experience, the name is supposed to reflect the actual client module
/ firmware phase restrictions. Which is why I named this instance DxeXX.

In summary, if there is one bit of inconsistency in this patch, then
that is the MODULE_TYPE setting. The name prefix (Dxe*) and the
LIBRARY_CLASS restriction list are correct, they reflect my intent and
reality.

If you or Jordan wish that I flip MODULE_TYPE to DXE_DRIVER, I can
certainly try 

Re: [edk2] [PATCH 2/5] OvmfPkg: PlatformPei: enable PCIEXBAR (aka MMCONFIG / ECAM) on Q35

[Laszlo Ersek]

On 03/06/16 11:54, Marcel Apfelbaum wrote:
> On 03/04/2016 04:46 PM, Laszlo Ersek wrote:
>> The comments in the code should speak for themselves; here we note only
>> two facts:
>>
>> - The PCI config space writes (to the PCIEXBAR register) are performed
>>using the 0xCF8 / 0xCFC IO ports, by virtue of PciLib being
>> resolved to
>>BasePciLibCf8. (This library resolution will permanently remain in
>> place
>>for the PEI phase.)
>>
>> - Since PCIEXBAR counts as a chipset register, it is the
>> responsibility of
>>the firmware to reprogram it at S3 resume. Therefore
>>PciExBarInitialization() is called regardless of the boot path.
>> (Marcel
>>recently posted patches for SeaBIOS that implement this.)
>>
>> This patch suffices to enable PCIEXBAR (and the dependent ACPI table
>> generation in QEMU), for the sake of "PCIeHotplug" in the Linux guest:
>>
>>ACPI: MCFG 0x7E193000 3C
>>  (v01 BOCHS  BXPCMCFG 0001 BXPC 0001)
>>PCI: MMCONFIG for domain  [bus 00-ff] at [mem
>> 0xb000-0xbfff]
>> (base 0xb000)
>>PCI: MMCONFIG at [mem 0xb000-0xbfff] reserved in E820
>>acpi PNP0A08:00: _OSC: OS supports
>> [ExtendedConfig ASPM ClockPM Segments MSI]
>>acpi PNP0A08:00: _OSC: OS now controls
>> [PCIeHotplug PME AER PCIeCapability]
>>
>> In the following patches, we'll equip the core PCI host bridge / root
>> bridge driver and the rest of DXE as well to utilize ECAM on Q35.
>>
>> Cc: Gabriel Somlo 
>> Cc: Jordan Justen 
>> Cc: Marcel Apfelbaum 
>> Cc: Michał Zegan 
>> Ref: https://github.com/tianocore/edk2/issues/32
>> Ref: http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/10548
>> Suggested-by: Marcel Apfelbaum 
>> Reported-by: Michał Zegan 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek 
>> ---
>>   OvmfPkg/OvmfPkgIa32.dsc |  8 ++
>>   OvmfPkg/OvmfPkgIa32X64.dsc  |  8 ++
>>   OvmfPkg/OvmfPkgX64.dsc  |  8 ++
>>   OvmfPkg/PlatformPei/PlatformPei.inf |  3 +
>>   OvmfPkg/PlatformPei/Platform.c  | 81 
>>   5 files changed, 108 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 2b928506e481..d1f91172044e 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -395,6 +395,14 @@ [PcdsFixedAtBuild]
>> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
>>   !endif
>>
>> +  # This PCD is used to set the base address of the PCI express
>> hierarchy. It
>> +  # is only consulted when OVMF runs on Q35. In that case it is
>> programmed into
>> +  # the PCIEXBAR register.
>> +  #
>> +  # The value we set below matches both the QEMU default, and the
>> value that
>> +  # SeaBIOS hard-codes.
>> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB000
>> +
>>   !ifdef $(SOURCE_DEBUG_ENABLE)
>> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>>   !endif
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 8d6271824f89..c9eeace4a134 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -400,6 +400,14 @@ [PcdsFixedAtBuild]
>> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
>>   !endif
>>
>> +  # This PCD is used to set the base address of the PCI express
>> hierarchy. It
>> +  # is only consulted when OVMF runs on Q35. In that case it is
>> programmed into
>> +  # the PCIEXBAR register.
>> +  #
>> +  # The value we set below matches both the QEMU default, and the
>> value that
>> +  # SeaBIOS hard-codes.
>> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB000
>> +
>>   !ifdef $(SOURCE_DEBUG_ENABLE)
>> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>>   !endif
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index e3f97f16f28b..b6f4b252ff03 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -400,6 +400,14 @@ [PcdsFixedAtBuild]
>> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
>>   !endif
>>
>> +  # This PCD is used to set the base address of the PCI express
>> hierarchy. It
>> +  # is only consulted when OVMF runs on Q35. In that case it is
>> programmed into
>> +  # the PCIEXBAR register.
>> +  #
>> +  # The value we set below matches both the QEMU default, and the
>> value that
>> +  # SeaBIOS hard-codes.
>> +  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB000
>> +
>>   !ifdef $(SOURCE_DEBUG_ENABLE)
>> gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
>>   !endif
>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf
>> b/OvmfPkg/PlatformPei/PlatformPei.inf
>> index 8480839efc8e..6dc5ff079f53 100644
>> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
>> +++ 

Re: [edk2] allocation of PMem64 BARs

[Laszlo Ersek]

On 03/05/16 10:54, Ni, Ruiyu wrote:
> Laszlo,
> 
> Does [0|3|0] contain option rom? (video controller?)
> 
> If it contains option rom, PciBusDxe driver has a logic to
> 
> convert the PMem64 to PMem32. PMem32 is converted
> 
> to Mem32 because COMBINE_MEM_PMEM is set.
> 
>  
> 
> The detailed logic is in PciResourceSupport.c: DegradeResource().

Right; I had found that function before I sent my email, but it looked
quite complex; so I thought I would ask my question without cluttering
the email with doubts about that function.

You are correct: the device is a physical GTX750 GPU assigned to the
guest, and it does have an option ROM.

I saw the comment in the source:

  //
  // If any child device has both option ROM and 64-bit BAR, degrade its
  // PMEM64/MEM64 requests in case that if a legacy option ROM image can
  // not access 64-bit resources.
  //

but I thought that this degradation would only happen if the option ROM
in question were *for certain* a legacy option ROM *only*. In this case
the GPU has a well-working 64-bit UEFI firmware, so I mostly ignored the
code under the comment.

Now I'm noticing that the condition is:

Temp = PCI_IO_DEVICE_FROM_LINK (ChildDeviceLink);
if (Temp->RomSize != 0) {

i.e., any kind of option ROM suffices to degrade the region.

So, my question is answered. Thank you!
Laszlo

> 
>  
> 
> Regards,
> 
> Ray
> 
>  
> 
> *From:*Laszlo Ersek [mailto:ler...@redhat.com]
> *Sent:* Saturday, March 5, 2016 5:41 AM
> *To:* Ni, Ruiyu 
> *Cc:* edk2-devel-ml01 
> *Subject:* allocation of PMem64 BARs
> 
>  
> 
> Hi Ray,
> 
> on top of my pending series
> 
>   [edk2] [PATCH 0/5] OvmfPkg: enable PCIe on Q35
>   http://thread.gmane.org/gmane.comp.bios.edk2.devel/8707
> 
> I also have some patches (not posted yet) that enable 64-bit MMIO. I
> tested it and it seems to work, but something is strange to me.
> 
> Namely, there is enough room in the non-prefetchable 64-bit aperture for
> allocating some PMem64 BARs (and EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM is
> passed to the driver), but the driver decides to allocate those from the
> (also non-prefetchable) 32-bit aperture instead.
> 
> It doesn't seem invalid, of course, but I thought that the driver would
> conserve the 32-bit aperture, and allocate everything possible from the
> 64-bit aperture.
> 
> I also have a Mem64 BAR, and that gets allocated high just fine.
> 
> Here's the initial output from the host bridge driver:
> 
>> RootBridge: PciRoot(0x0)
>>   Support/Attr: 70069 / 70069
>> DmaAbove4G: No
>> NoExtConfSpace: No
>>  AllocAttr: 3 (CombineMemPMem Mem64Decode)
>>Bus: 0 - FF
>> Io: C000 - 
>>Mem: C000 - FBFF
>> MemAbove4G: 18000 - 97FFF
>>   PMem: 0 - 0
>>PMemAbove4G: 0 - 0
>> PciHostBridgeDxe: IntersectMemoryDescriptor: add [18000, 98000): 
>> Success
> 
> Then the output from the PCI bus driver, interspersed with messages from
> the host bridge driver:
> 
>> PCI Bus First Scanning
>> PciBus: Discovered PCI @ [00|00|00]
>>
>> PciBus: Discovered PCI @ [00|01|00]
>>BAR[0]: Type =  Mem32; Alignment = 0x3FF;  Length = 0x400;  
>> Offset = 0x10
>>BAR[1]: Type =  Mem32; Alignment = 0x3FF;  Length = 0x400;  
>> Offset = 0x14
>>BAR[2]: Type =  Mem32; Alignment = 0x1FFF;  Length = 0x2000;  Offset = 
>> 0x18
>>BAR[3]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x1C
>>
>> PciBus: Discovered PCI @ [00|02|00]
>>BAR[0]: Type =  Mem64; Alignment = 0x3FFF;  Length = 0x4000;  Offset = 
>> 0x10
>>
>> PciBus: Discovered PCI @ [00|03|00]
>>BAR[0]: Type =  Mem32; Alignment = 0xFF;  Length = 0x100;  Offset 
>> = 0x10
>>BAR[1]: Type = PMem64; Alignment = 0xFFF;  Length = 0x1000;  
>> Offset = 0x14
>>BAR[2]: Type = PMem64; Alignment = 0x1FF;  Length = 0x200;  
>> Offset = 0x1C
>>BAR[3]: Type =   Io32; Alignment = 0x7F;  Length = 0x80;  Offset = 0x24
>>
>> PciBus: Discovered PCI @ [00|04|00]
>>BAR[0]: Type =  Mem32; Alignment = 0x3FFF;  Length = 0x4000;  Offset = 
>> 0x10
>>
>> PciBus: Discovered PPB @ [00|1E|00]
>>
>> PciBus: Discovered PPB @ [01|01|00]
>>BAR[0]: Type =  Mem64; Alignment = 0xFFF;  Length = 0x100;  Offset = 0x10
>>
>> PciBus: Discovered PCI @ [02|01|00]
>>BAR[0]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x10
>>BAR[1]: Type =  Mem32; Alignment = 0xFFF;  Length = 0x1000;  Offset = 0x14
>>
>> PciBus: Discovered PCI @ [02|02|00]
>>BAR[4]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x20
>>
>> PciBus: Discovered PCI @ [02|02|01]
>>BAR[4]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x20
>>
>> PciBus: Discovered PCI @ [02|02|02]
>>BAR[4]: Type =   Io32; Alignment = 0x1F;  Length = 0x20;  Offset = 0x20
>>
>> PciBus: Discovered PCI @ [02|02|07]
>>BAR[0]: Type =  Mem32; Alignment = 0xFFF;  Length = 0x1000;  Offset = 0x10
>>
>> 

Re: [edk2] [PATCH 2/5] OvmfPkg: PlatformPei: enable PCIEXBAR (aka MMCONFIG / ECAM) on Q35

[Marcel Apfelbaum]

On 03/04/2016 04:46 PM, Laszlo Ersek wrote:

The comments in the code should speak for themselves; here we note only
two facts:

- The PCI config space writes (to the PCIEXBAR register) are performed
   using the 0xCF8 / 0xCFC IO ports, by virtue of PciLib being resolved to
   BasePciLibCf8. (This library resolution will permanently remain in place
   for the PEI phase.)

- Since PCIEXBAR counts as a chipset register, it is the responsibility of
   the firmware to reprogram it at S3 resume. Therefore
   PciExBarInitialization() is called regardless of the boot path. (Marcel
   recently posted patches for SeaBIOS that implement this.)

This patch suffices to enable PCIEXBAR (and the dependent ACPI table
generation in QEMU), for the sake of "PCIeHotplug" in the Linux guest:

   ACPI: MCFG 0x7E193000 3C
 (v01 BOCHS  BXPCMCFG 0001 BXPC 0001)
   PCI: MMCONFIG for domain  [bus 00-ff] at [mem 0xb000-0xbfff]
(base 0xb000)
   PCI: MMCONFIG at [mem 0xb000-0xbfff] reserved in E820
   acpi PNP0A08:00: _OSC: OS supports
[ExtendedConfig ASPM ClockPM Segments MSI]
   acpi PNP0A08:00: _OSC: OS now controls
[PCIeHotplug PME AER PCIeCapability]

In the following patches, we'll equip the core PCI host bridge / root
bridge driver and the rest of DXE as well to utilize ECAM on Q35.

Cc: Gabriel Somlo 
Cc: Jordan Justen 
Cc: Marcel Apfelbaum 
Cc: Michał Zegan 
Ref: https://github.com/tianocore/edk2/issues/32
Ref: http://thread.gmane.org/gmane.comp.bios.coreboot.seabios/10548
Suggested-by: Marcel Apfelbaum 
Reported-by: Michał Zegan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
  OvmfPkg/OvmfPkgIa32.dsc |  8 ++
  OvmfPkg/OvmfPkgIa32X64.dsc  |  8 ++
  OvmfPkg/OvmfPkgX64.dsc  |  8 ++
  OvmfPkg/PlatformPei/PlatformPei.inf |  3 +
  OvmfPkg/PlatformPei/Platform.c  | 81 
  5 files changed, 108 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 2b928506e481..d1f91172044e 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -395,6 +395,14 @@ [PcdsFixedAtBuild]
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
  !endif

+  # This PCD is used to set the base address of the PCI express hierarchy. It
+  # is only consulted when OVMF runs on Q35. In that case it is programmed into
+  # the PCIEXBAR register.
+  #
+  # The value we set below matches both the QEMU default, and the value that
+  # SeaBIOS hard-codes.
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB000
+
  !ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
  !endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 8d6271824f89..c9eeace4a134 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -400,6 +400,14 @@ [PcdsFixedAtBuild]
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
  !endif

+  # This PCD is used to set the base address of the PCI express hierarchy. It
+  # is only consulted when OVMF runs on Q35. In that case it is programmed into
+  # the PCIEXBAR register.
+  #
+  # The value we set below matches both the QEMU default, and the value that
+  # SeaBIOS hard-codes.
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB000
+
  !ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
  !endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index e3f97f16f28b..b6f4b252ff03 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -400,6 +400,14 @@ [PcdsFixedAtBuild]
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F
  !endif

+  # This PCD is used to set the base address of the PCI express hierarchy. It
+  # is only consulted when OVMF runs on Q35. In that case it is programmed into
+  # the PCIEXBAR register.
+  #
+  # The value we set below matches both the QEMU default, and the value that
+  # SeaBIOS hard-codes.
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress|0xB000
+
  !ifdef $(SOURCE_DEBUG_ENABLE)
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2
  !endif
diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf 
b/OvmfPkg/PlatformPei/PlatformPei.inf
index 8480839efc8e..6dc5ff079f53 100644
--- a/OvmfPkg/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/PlatformPei/PlatformPei.inf
@@ -94,6 +94,9 @@ [Pcd]
gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable
gUefiCpuPkgTokenSpaceGuid.PcdCpuLocalApicBaseAddress

+[FixedPcd]
+  gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
+
  [FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire

diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c
index 7d0941209f25..f0a488bcfc2b 

Re: [edk2] [PATCH 1/5] OvmfPkg: IndustryStandard/Q35MchIch9.h: add PCIEXBAR macros

[Marcel Apfelbaum]

On 03/04/2016 04:46 PM, Laszlo Ersek wrote:

Section 5.1.16 ("PCIEXBAR -- PCI Express Register Range Base Address") in
Intel document #316966-002 (already referenced near the top of this header
file) describes the Q35 DRAM Controller register that configures the
memory-mapped PCI config space (also known as MMCONFIG, and ECAM /
Enhanced Configuration Access Method).

In this patch we add the macros we'll need later. We'll only support the
256 MB memory-mapped config space -- enough for buses [0, 255].

Cc: Gabriel Somlo 
Cc: Jordan Justen 
Cc: Marcel Apfelbaum 
Cc: Michał Zegan 
Ref: https://github.com/tianocore/edk2/issues/32
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
  OvmfPkg/Include/IndustryStandard/Q35MchIch9.h | 8 
  1 file changed, 8 insertions(+)

diff --git a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h 
b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
index 18b34a3d4f4e..4dc2c39901c1 100644
--- a/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
+++ b/OvmfPkg/Include/IndustryStandard/Q35MchIch9.h
@@ -33,6 +33,14 @@
  #define MCH_GGC   0x52
  #define MCH_GGC_IVD BIT1

+#define MCH_PCIEXBAR_LOW  0x60
+#define MCH_PCIEXBAR_LOWMASK0x0FFF
+#define MCH_PCIEXBAR_BUS_FF 0
+#define MCH_PCIEXBAR_EN BIT0
+
+#define MCH_PCIEXBAR_HIGH 0x64
+#define MCH_PCIEXBAR_HIGHMASK   0xFFF0
+
  #define MCH_SMRAM 0x9D
  #define MCH_SMRAM_D_LCK BIT4
  #define MCH_SMRAM_G_SMRAME  BIT3




Reviewed-by: Marcel Apfelbaum 

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