[edk2] [PATCH v3] MdeModulePkg: Skip registering BootManagerMenu if its FFS is not found

[Sunny Wang]

This is a enhancement to support the case when platform firmware doesn’t 
support Boot Manager Menu.
For now, if BootManagerMenu FFS can not be retrieved from FV, BDS core code 
will still register a boot option for it. Then, this non-functional boot option 
will still be booted by user's request (like HotKey or Exit from shell) to 
cause additional boot time and error status code reported. Therefore, it would 
be good to skip BootManagerMenu boot option registration and then return error 
status and Invalid BootOption data for this case so that the BootManagerBoot() 
or other consumers can directly return without doing anything.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Sunny Wang 
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h | 10 +++--
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c  | 39 +++
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c  | 46 +++
 3 files changed, 70 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 0fdb23d..e333ffd 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -418,12 +418,16 @@ EfiBootManagerBoot (
   );
 
 /**
-  Return the Boot Manager Menu.
- 
+  Return the boot option corresponding to the Boot Manager Menu.
+  It may automatically create one if the boot option hasn't been created yet.
+
   @param BootOptionReturn the Boot Manager Menu.
 
   @retval EFI_SUCCESS   The Boot Manager Menu is successfully returned.
-  @retval EFI_NOT_FOUND The Boot Manager Menu is not found.
+  @retval EFI_NOT_FOUND The Boot Manager Menu cannot be found.
+  @retval othersReturn status of gRT->SetVariable (). BootOption still 
points
+to the Boot Manager Menu even the Status is not 
EFI_SUCCESS
+and EFI_NOT_FOUND.
 **/
 EFI_STATUS
 EFIAPI
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index d016517..4da401d 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -2,7 +2,7 @@
   Library functions which relates with booting.
 
 Copyright (c) 2011 - 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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -2159,7 +2159,7 @@ EfiBootManagerRefreshAllBootOption (
 }
 
 /**
-  This function is called to create the boot option for the Boot Manager Menu.
+  This function is called to get or create the boot option for the Boot 
Manager Menu.
 
   The Boot Manager Menu is shown after successfully booting a boot option.
   Assume the BootManagerMenuFile is in the same FV as the module links to this 
library.
@@ -2167,8 +2167,10 @@ EfiBootManagerRefreshAllBootOption (
   @param  BootOptionReturn the boot option of the Boot Manager Menu
 
   @retval EFI_SUCCESS   Successfully register the Boot Manager Menu.
-  @retval StatusReturn status of gRT->SetVariable (). BootOption still 
points
-to the Boot Manager Menu even the Status is not 
EFI_SUCCESS.
+  @retval EFI_NOT_FOUND The Boot Manager Menu cannot be found.
+  @retval othersReturn status of gRT->SetVariable (). BootOption still 
points
+to the Boot Manager Menu even the Status is not 
EFI_SUCCESS
+and EFI_NOT_FOUND.
 **/
 EFI_STATUS
 BmRegisterBootManagerMenu (
@@ -2181,7 +2183,28 @@ BmRegisterBootManagerMenu (
   EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
   EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage;
   MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;
+  VOID   *Data;
+  UINTN  DataSize;
 
+  Data = NULL;
+  Status = GetSectionFromFv (
+ PcdGetPtr (PcdBootManagerMenuFile),
+ EFI_SECTION_PE32,
+ 0,
+ (VOID **) ,
+ 
+ );
+  if (Data != NULL) {
+FreePool (Data);
+  }
+  if (EFI_ERROR (Status)) {
+DEBUG ((EFI_D_WARN, "[Bds]BootManagerMenu FFS section can not be found, 
skip its boot option registration\n"));
+return EFI_NOT_FOUND;
+  }
+
+  //
+  // Get BootManagerMenu application's description from EFI User Interface 
Section.
+  //
   Status = GetSectionFromFv (
  PcdGetPtr (PcdBootManagerMenuFile),
  EFI_SECTION_USER_INTERFACE,
@@ -2237,12 +2260,14 @@ BmRegisterBootManagerMenu (
 /**
   Return the boot option corresponding to the Boot Manager Menu.
   It may automatically create one if the boot option hasn't been 

Re: [edk2] [PATCH 10/10] BaseTools Scripts: Add MemoryProfileSymbolGen.py

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Zeng, Star
> Sent: Sunday, June 26, 2016 1:36 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Gao, Liming
> ; Zhu, Yonghong 
> Subject: [PATCH 10/10] BaseTools Scripts: Add MemoryProfileSymbolGen.py
> 
> This tool depends on DIA2Dump.exe (VS) or nm (gcc) to parse debug entry.
> 
> Usage: MemoryProfileSymbolGen.py [--version] [-h] [--help] [-i inputfile
> [-o outputfile]]
> 
> Copyright (c) 2016, Intel Corporation. All rights reserved.
> 
> Options:
>   --version show program's version number and exit
>   -h, --helpshow this help message and exit
>   -i INPUTFILENAME, --inputfile=INPUTFILENAME
> The input memory profile info file output from
> MemoryProfileInfo application in MdeModulePkg
>   -o OUTPUTFILENAME, --outputfile=OUTPUTFILENAME
> The output memory profile info file with symbol,
> MemoryProfileInfoSymbol.txt will be used if it is not
> 
> Cc: Jiewen Yao 
> Cc: Liming Gao 
> Cc: Yonghong Zhu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  BaseTools/Scripts/MemoryProfileSymbolGen.py | 281
> 
>  1 file changed, 281 insertions(+)
>  create mode 100644 BaseTools/Scripts/MemoryProfileSymbolGen.py
> 
> diff --git a/BaseTools/Scripts/MemoryProfileSymbolGen.py
> b/BaseTools/Scripts/MemoryProfileSymbolGen.py
> new file mode 100644
> index ..a9790d8883c8
> --- /dev/null
> +++ b/BaseTools/Scripts/MemoryProfileSymbolGen.py
> @@ -0,0 +1,281 @@
> +##
> +# Generate symbal for memory profile info.
> +#
> +# This tool depends on DIA2Dump.exe (VS) or nm (gcc) to parse debug
> entry.
> +#
> +# 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 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.
> +#
> +##
> +
> +import os
> +import re
> +import sys
> +from optparse import OptionParser
> +
> +versionNumber = "1.0"
> +__copyright__ = "Copyright (c) 2016, Intel Corporation. All rights reserved."
> +
> +class Symbols:
> +def __init__(self):
> +self.listLineAddress = []
> +self.pdbName = ""
> +# Cache for function
> +self.functionName = ""
> +# Cache for line
> +self.sourceName = ""
> +
> +
> +def getSymbol (self, rva):
> +index = 0
> +lineName  = 0
> +sourceName = "??"
> +while index + 1 < self.lineCount :
> +if self.listLineAddress[index][0] <= rva and 
> self.listLineAddress[index
> + 1][0] > rva :
> +offset = rva - self.listLineAddress[index][0]
> +functionName = self.listLineAddress[index][1]
> +lineName = self.listLineAddress[index][2]
> +sourceName = self.listLineAddress[index][3]
> +if lineName == 0 :
> +  return " (" + self.listLineAddress[index][1] + "() - " + 
> ")"
> +else :
> +  return " (" + self.listLineAddress[index][1] + "() - " + 
> sourceName +
> ":" + str(lineName) + ")"
> +index += 1
> +
> +return " (unknown)"
> +
> +def parse_debug_file(self, driverName, pdbName):
> +if cmp (pdbName, "") == 0 :
> +return
> +self.pdbName = pdbName;
> +
> +try:
> +nmCommand = "nm"
> +nmLineOption = "-l"
> +print "parsing (debug) - " + pdbName
> +os.system ('%s %s %s > nmDump.line.log' % (nmCommand,
> nmLineOption, pdbName))
> +except :
> +print 'ERROR: nm command not available.  Please verify PATH'
> +return
> +
> +#
> +# parse line
> +#
> +linefile = open("nmDump.line.log")
> +reportLines = linefile.readlines()
> +linefile.close()
> +
> +# 000113ca T AllocatePoolc:\home\edk-
> ii\MdePkg\Library\UefiMemoryAllocationLib\MemoryAllocationLib.c:399
> +patchLineFileMatchString = "([0-9a-fA-
> F]{8})\s+[T|D|t|d]\s+(\w+)\s*((?:[a-zA-Z]:)?[\w+\-./_a-zA-Z0-9]*):?([0-
> 9]*)"
> +
> +for reportLine in reportLines:
> +#print "check - " + reportLine
> +match = re.match(patchLineFileMatchString, reportLine)
> +if match is not None:
> +#print "match - " + reportLine[:-1]
> +#print "0 - 

Re: [edk2] [PATCH] UefiCpuPkg S3Resume2Pei: Report status code when allocate memory is failed

[Fan, Jeff]

Reviewed-by: Jeff Fan 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star Zeng
Sent: Friday, July 01, 2016 9:55 AM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen; Fan, Jeff
Subject: [edk2] [PATCH] UefiCpuPkg S3Resume2Pei: Report status code when 
allocate memory is failed

Cc: Jiewen Yao 
Cc: Jeff Fan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c 
b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index a79fc59bdfa6..0ccf3a42b5ff 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -4,7 +4,7 @@
   This module will excute the boot script saved during last boot and after 
that,
   control is passed to OS waking up handler.
 
-  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 @@ -822,7 
+822,13 @@ S3ResumeExecuteBootScript (
 // Make sure the newly allcated IDT align with 16-bytes
 // 
 IdtBuffer = AllocatePages (EFI_SIZE_TO_PAGES((IdtDescriptor->Limit + 1) + 
16));
-ASSERT (IdtBuffer != NULL);
+if (IdtBuffer == NULL) {
+  REPORT_STATUS_CODE (
+EFI_ERROR_CODE | EFI_ERROR_MAJOR,
+(EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_EC_S3_RESUME_FAILED)
+);
+  ASSERT (FALSE);
+}
 //
 // Additional 16 bytes allocated to save IA32 IDT descriptor and Pei 
Service Table Pointer
 // IA32 IDT descriptor will be used to setup IA32 IDT table for 32-bit 
Framework Boot Script code @@ -852,7 +858,13 @@ S3ResumeExecuteBootScript (
   // Prepare data for return back
   //
   PeiS3ResumeState = AllocatePool (sizeof(*PeiS3ResumeState));
-  ASSERT (PeiS3ResumeState != NULL);
+  if (PeiS3ResumeState == NULL) {
+REPORT_STATUS_CODE (
+  EFI_ERROR_CODE | EFI_ERROR_MAJOR,
+  (EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_EC_S3_RESUME_FAILED)
+  );
+ASSERT (FALSE);
+  }
   DEBUG (( EFI_D_ERROR, "PeiS3ResumeState - %x\r\n", PeiS3ResumeState));
   PeiS3ResumeState->ReturnCs   = 0x10;
   PeiS3ResumeState->ReturnEntryPoint   = 
(EFI_PHYSICAL_ADDRESS)(UINTN)S3ResumeBootOs;
--
2.7.0.windows.1

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

Re: [edk2] [PATCH] UefiCpuPkg S3Resume2Pei: Report status code when allocate memory is failed

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Star Zeng
> Sent: Friday, July 1, 2016 9:55 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Fan, Jeff 
> Subject: [edk2] [PATCH] UefiCpuPkg S3Resume2Pei: Report status code
> when allocate memory is failed
> 
> Cc: Jiewen Yao 
> Cc: Jeff Fan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng 
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 18
> +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index a79fc59bdfa6..0ccf3a42b5ff 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -4,7 +4,7 @@
>This module will excute the boot script saved during last boot and after
> that,
>control is passed to OS waking up handler.
> 
> -  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
> @@ -822,7 +822,13 @@ S3ResumeExecuteBootScript (
>  // Make sure the newly allcated IDT align with 16-bytes
>  //
>  IdtBuffer = AllocatePages (EFI_SIZE_TO_PAGES((IdtDescriptor->Limit +
> 1) + 16));
> -ASSERT (IdtBuffer != NULL);
> +if (IdtBuffer == NULL) {
> +  REPORT_STATUS_CODE (
> +EFI_ERROR_CODE | EFI_ERROR_MAJOR,
> +(EFI_SOFTWARE_PEI_MODULE |
> EFI_SW_PEI_EC_S3_RESUME_FAILED)
> +);
> +  ASSERT (FALSE);
> +}
>  //
>  // Additional 16 bytes allocated to save IA32 IDT descriptor and Pei
> Service Table Pointer
>  // IA32 IDT descriptor will be used to setup IA32 IDT table for 32-bit
> Framework Boot Script code
> @@ -852,7 +858,13 @@ S3ResumeExecuteBootScript (
>// Prepare data for return back
>//
>PeiS3ResumeState = AllocatePool (sizeof(*PeiS3ResumeState));
> -  ASSERT (PeiS3ResumeState != NULL);
> +  if (PeiS3ResumeState == NULL) {
> +REPORT_STATUS_CODE (
> +  EFI_ERROR_CODE | EFI_ERROR_MAJOR,
> +  (EFI_SOFTWARE_PEI_MODULE |
> EFI_SW_PEI_EC_S3_RESUME_FAILED)
> +  );
> +ASSERT (FALSE);
> +  }
>DEBUG (( EFI_D_ERROR, "PeiS3ResumeState - %x\r\n",
> PeiS3ResumeState));
>PeiS3ResumeState->ReturnCs   = 0x10;
>PeiS3ResumeState->ReturnEntryPoint   =
> (EFI_PHYSICAL_ADDRESS)(UINTN)S3ResumeBootOs;
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] UefiCpuPkg S3Resume2Pei: Report status code when allocate memory is failed

[Star Zeng]

Cc: Jiewen Yao 
Cc: Jeff Fan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng 
---
 UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c 
b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
index a79fc59bdfa6..0ccf3a42b5ff 100644
--- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
+++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
@@ -4,7 +4,7 @@
   This module will excute the boot script saved during last boot and after 
that,
   control is passed to OS waking up handler.
 
-  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
@@ -822,7 +822,13 @@ S3ResumeExecuteBootScript (
 // Make sure the newly allcated IDT align with 16-bytes
 // 
 IdtBuffer = AllocatePages (EFI_SIZE_TO_PAGES((IdtDescriptor->Limit + 1) + 
16));
-ASSERT (IdtBuffer != NULL);
+if (IdtBuffer == NULL) {
+  REPORT_STATUS_CODE (
+EFI_ERROR_CODE | EFI_ERROR_MAJOR,
+(EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_EC_S3_RESUME_FAILED)
+);
+  ASSERT (FALSE);
+}
 //
 // Additional 16 bytes allocated to save IA32 IDT descriptor and Pei 
Service Table Pointer
 // IA32 IDT descriptor will be used to setup IA32 IDT table for 32-bit 
Framework Boot Script code
@@ -852,7 +858,13 @@ S3ResumeExecuteBootScript (
   // Prepare data for return back
   //
   PeiS3ResumeState = AllocatePool (sizeof(*PeiS3ResumeState));
-  ASSERT (PeiS3ResumeState != NULL);
+  if (PeiS3ResumeState == NULL) {
+REPORT_STATUS_CODE (
+  EFI_ERROR_CODE | EFI_ERROR_MAJOR,
+  (EFI_SOFTWARE_PEI_MODULE | EFI_SW_PEI_EC_S3_RESUME_FAILED)
+  );
+ASSERT (FALSE);
+  }
   DEBUG (( EFI_D_ERROR, "PeiS3ResumeState - %x\r\n", PeiS3ResumeState));
   PeiS3ResumeState->ReturnCs   = 0x10;
   PeiS3ResumeState->ReturnEntryPoint   = 
(EFI_PHYSICAL_ADDRESS)(UINTN)S3ResumeBootOs;
-- 
2.7.0.windows.1

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

[edk2] [PATCH 2/4] MdeModulePkg/PciBusDxe: look for the right capability in IsSHPC()

[Laszlo Ersek]

The PCI Hot Plug capability register block is marked with capability ID
0x0C (EFI_PCI_CAPABILITY_ID_SHPC), not 0x06
(EFI_PCI_CAPABILITY_ID_HOTPLUG).

This bug prevents PciBusDxe from recognizing whether a PCI-to-PCI bridge
supports hotplug. In turn the platform's EFI_PCI_HOT_PLUG_INIT_PROTOCOL is
not consulted for resource padding information:

  GatherPpbInfo()[PciEnumeratorSupport.c]
GetResourcePaddingPpb()  [PciResourceSupport.c]
  GetResourcePaddingForHpb() [PciHotPlugSupport.c]
IsPciHotPlugBus()[PciHotPlugSupport.c]
  IsSHPC()   [PciHotPlugSupport.c]
//
// returns FALSE
//
//
// the following is not reached:
//
gPciHotPlugInit->GetResourcePadding()

Look for the correct capability ID.

Cc: "Johnson, Brian J." 
Cc: Alex Williamson 
Cc: Andrew Fish 
Cc: Feng Tian 
Cc: Jordan Justen 
Cc: Marcel Apfelbaum 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c
index 257874b8b03e..ca8766869ae7 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c
@@ -300,7 +300,7 @@ IsSHPC (
   Offset = 0;
   Status = LocateCapabilityRegBlock (
 PciIoDevice,
-EFI_PCI_CAPABILITY_ID_HOTPLUG,
+EFI_PCI_CAPABILITY_ID_SHPC,
 ,
 NULL
 );
-- 
1.8.3.1


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

[edk2] [PATCH 1/4] MdePkg/IndustryStandard: introduce EFI_PCI_CAPABILITY_ID_SHPC

[Laszlo Ersek]

The "Pci22.h" header file defines the macro EFI_PCI_CAPABILITY_ID_HOTPLUG
with value 0x06. According to all of:
- later parts of the same header file,
- Appendix H ("Capability IDs") of the PCI Local Bus Specification
  Revision 2.3,
- and Chapter 2 ("Capability IDs") of the PCI Code and ID Assignment
  Specification Revision 0.9,

0x06 means "CompactPCI Hot Swap". It does not mean "PCI Hot-Plug": that
capability is described by ID 0x0C:

  0Ch  PCI Hot-Plug -- This Capability ID indicates that the associated
   device conforms to the Standard Hot-Plug Controller model.

Therefore EFI_PCI_CAPABILITY_ID_HOTPLUG is arguably a misnomer. PciBusDxe
(mis-)uses EFI_PCI_CAPABILITY_ID_HOTPLUG in the IsSHPC() helper function
to identify PCI Hot-Plug capability.

In order to preserve compatibility with existent code, leave
EFI_PCI_CAPABILITY_ID_HOTPLUG alone, and introduce
EFI_PCI_CAPABILITY_ID_SHPC with the right ID value.

Cc: "Johnson, Brian J." 
Cc: Alex Williamson 
Cc: Andrew Fish 
Cc: Feng Tian 
Cc: Jordan Justen 
Cc: Liming Gao 
Cc: Marcel Apfelbaum 
Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdePkg/Include/IndustryStandard/Pci22.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/Include/IndustryStandard/Pci22.h 
b/MdePkg/Include/IndustryStandard/Pci22.h
index db24a153e15e..4cf8389c699b 100644
--- a/MdePkg/Include/IndustryStandard/Pci22.h
+++ b/MdePkg/Include/IndustryStandard/Pci22.h
@@ -635,6 +635,7 @@ typedef union {
 #define EFI_PCI_CAPABILITY_ID_SLOTID  0x04
 #define EFI_PCI_CAPABILITY_ID_MSI 0x05
 #define EFI_PCI_CAPABILITY_ID_HOTPLUG 0x06
+#define EFI_PCI_CAPABILITY_ID_SHPC0x0C
 
 ///
 /// Capabilities List Header
-- 
1.8.3.1


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

[edk2] [PATCH 4/4] OvmfPkg: add PciHotPlugInitDxe

[Laszlo Ersek]

After IncompatiblePciDeviceSupportDxe, this is another small driver /
protocol implementation that tweaks the behavior of the PCI bus driver in
edk2.

The protocol is specified in the Platform Init Spec v1.4a, Volume 5,
Chapter 12.6 "PCI Hot Plug PCI Initialization Protocol". This
implementation steers the PCI bus driver to reserve the following
resources ("padding") for each PCI bus, in addition to the BARs of the
devices on that PCI bus:
- 2MB of 64-bit non-prefetchable MMIO aperture,
- 512B of IO port space.

The goal is to reserve room for devices hot-plugged at runtime even if the
bridge receiving the device is empty at boot time.

The 2MB MMIO size is inspired by SeaBIOS. The 512B IO port size is
actually only 1/8th of the PCI spec mandated reservation, but the
specified size of 4096 has proved wasteful (given the limited size of our
IO port space -- see commit bba734ab4c7c). Especially on Q35, where every
PCIe root port and downstream port qualifies as a separate bridge (capable
of accepting a single device).

Test results for this patch:
- regardless of our request for 64-bit MMIO reservation, it is downgraded
  to 32-bit,
- although we request 512B alignment for the IO port space reservation,
  the next upstream bridge rounds it up to 4096B.

Cc: "Johnson, Brian J." 
Cc: Alex Williamson 
Cc: Andrew Fish 
Cc: Feng Tian 
Cc: Jordan Justen 
Cc: Marcel Apfelbaum 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Suggested-by: Andrew Fish 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/OvmfPkgIa32.dsc  |   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc   |   1 +
 OvmfPkg/OvmfPkgX64.dsc   |   1 +
 OvmfPkg/OvmfPkgIa32.fdf  |   1 +
 OvmfPkg/OvmfPkgIa32X64.fdf   |   1 +
 OvmfPkg/OvmfPkgX64.fdf   |   1 +
 OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf |  46 +++
 OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c   | 352 
 8 files changed, 404 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 6d074641bede..805650059e96 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -552,6 +552,7 @@ [Components]
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
+  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
 
   PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 25fcb38aaf84..7615ee96dff2 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -561,6 +561,7 @@ [Components.X64]
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
+  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
 
   PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index cb7ccf79618d..7f8a5c25a5c0 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -559,6 +559,7 @@ [Components]
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
   OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
+  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
 
   PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 59a4024ff026..386fd6bc1301 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -208,6 +208,7 @@ [FV.DXEFV]
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
+INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
 INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
 INF  PcAtChipsetPkg/KbcResetDxe/Reset.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index c6167a4176af..3a777a3dba88 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -208,6 +208,7 @@ [FV.DXEFV]
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
 INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
+INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
 INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
 INF  

[edk2] [PATCH 0/4] PCI resource reservation changes for better hotpluggability

[Laszlo Ersek]

This series makes the following test case succeed:

- Set the CODE env variable to the OVMF_CODE.fd file in the build
  output.

- Set the TMPL env variable to the OVMF_VARS.fd file in the build
  output.

- Set the ISO env variable to the pathname of a Linux LiveCD.

- Create a copy of the varstore template:

  cp $TMPL vars.fd

- Create an empty disk image:

  qemu-img create -f qcow2 test.qcow2 1G

- Start QEMU as follows.

  qemu-system-x86_64 \
-m 2048 \
\
-machine q35,accel=kvm \
-device VGA \
\
-drive if=pflash,format=raw,file=$CODE,readonly \
-drive if=pflash,format=raw,file=vars.fd \
\
-drive id=cdrom,if=none,readonly,format=raw,file=$ISO \
-drive id=disk,if=none,format=qcow2,file=test.qcow2 \
\
-chardev file,id=debugfile,path=test.log \
-device isa-debugcon,iobase=0x402,chardev=debugfile \
\
-chardev stdio,id=char0,signal=off,mux=on \
-mon chardev=char0,mode=readline,default \
-serial chardev:char0 \
\
-device ioh3420,id=root_port,bus=pcie.0 \
\
-device x3130-upstream,id=upstream_port,bus=root_port \
\
-device xio3130-downstream,id=downstream_port1,bus=upstream_port,chassis=1 \
-device virtio-scsi-pci,id=scsi0,bus=downstream_port1 \
-device scsi-cd,bus=scsi0.0,drive=cdrom,bootindex=0 \
\
-device xio3130-downstream,id=downstream_port2,bus=upstream_port,chassis=2

  This command line creates a PCIe root port, and cold-plugs a PCIe
  switch into it. The PCIe switch has one upstream port and two
  downstream ports. Into one of those downstream ports, a virtio-scsi
  controller is cold-plugged, and the LiveCD is made available to the
  guest as a SCSI CD-ROM on that virtio-scsi controller. The other
  downstream port of the PCIe switch is left empty.

- After booting the LiveCD, enter [Ctrl-A C] to switch the terminal I/O
  from the guest's serial port to the QEMU monitor, then hotplug a
  virtio-block device with the following command into the second
  downstream port:

  device_add virtio-blk-pci,drive=disk,bus=downstream_port2

- Without the patches, the guest kernel will spew PCI resource
  allocation errors to the syslog, and it might even hang eventually.
  With the patches, the hotplugged disk can be used (/dev/vda).

Public branch: .

Cc: "Johnson, Brian J." 
Cc: Alex Williamson 
Cc: Andrew Fish 
Cc: Feng Tian 
Cc: Jordan Justen 
Cc: Liming Gao 
Cc: Marcel Apfelbaum 
Cc: Michael D Kinney 
Cc: Ruiyu Ni 
Cc: Star Zeng 

Thanks
Laszlo

Laszlo Ersek (4):
  MdePkg/IndustryStandard: introduce EFI_PCI_CAPABILITY_ID_SHPC
  MdeModulePkg/PciBusDxe: look for the right capability in IsSHPC()
  MdeModulePkg/PciBusDxe: recognize hotplug-capable PCIe ports
  OvmfPkg: add PciHotPlugInitDxe

 MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c |  73 +++-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h |  19 ++
 MdePkg/Include/IndustryStandard/Pci22.h|   1 +
 OvmfPkg/OvmfPkgIa32.dsc|   1 +
 OvmfPkg/OvmfPkgIa32.fdf|   1 +
 OvmfPkg/OvmfPkgIa32X64.dsc |   1 +
 OvmfPkg/OvmfPkgIa32X64.fdf |   1 +
 OvmfPkg/OvmfPkgX64.dsc |   1 +
 OvmfPkg/OvmfPkgX64.fdf |   1 +
 OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c | 352 
 OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf   |  46 +++
 11 files changed, 496 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 create mode 100644 OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.c

-- 
1.8.3.1

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

[edk2] [PATCH 3/4] MdeModulePkg/PciBusDxe: recognize hotplug-capable PCIe ports

[Laszlo Ersek]

Section 7.8.2 of the PCI Express specification (r4.0 v0.3), entitled "PCI
Express Capabilities Register (Offset 02h)", describes the conditions when
a PCIe port should be considered "supporting hotplug":
- it should be a root complex port or a switch downstream port, and
- it should have the "Slot Implemented" bit set.

This logic is already implemented in at least two open source projects I
could find:

- in SeaBIOS by Marcel Apfelbaum: "hw/pci: reserve IO and mem for pci
  express downstream ports with no devices attached"
  ,

- in edk2 itself, in the implementation of the "PCI" UEFI Shell command:
  see the "PcieExplainTypeSlot" case label in function
  PciExplainPciExpress(), file
  "ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c".

PciBusDxe recognizes such PCIe ports as bridges, but it doesn't realize
they support hotplug. In turn PciBusDxe omits getting any resource padding
information from the platform's EFI_PCI_HOT_PLUG_INIT_PROTOCOL for these
bridges:

  GatherPpbInfo()[PciEnumeratorSupport.c]
GetResourcePaddingPpb()  [PciResourceSupport.c]
  GetResourcePaddingForHpb() [PciHotPlugSupport.c]
IsPciHotPlugBus()[PciHotPlugSupport.c]
  //
  // returns FALSE
  //
//
// the following is not reached:
//
gPciHotPlugInit->GetResourcePadding()

Implement a function called SupportsPcieHotplug() for identifying such
ports, and call it from IsPciHotPlugBus() (after the call to IsSHPC()).

Cc: "Johnson, Brian J." 
Cc: Alex Williamson 
Cc: Andrew Fish 
Cc: Feng Tian 
Cc: Jordan Justen 
Cc: Marcel Apfelbaum 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h | 19 ++
 MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c | 71 
 2 files changed, 90 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h
index 1fb9ba972091..6e02b8b98bf0 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.h
@@ -177,6 +177,25 @@ IsSHPC (
   );
 
 /**
+  Check whether PciIoDevice supports PCIe hotplug.
+
+  This is equivalent to the following condition:
+  - the device is either a PCIe switch downstream port or a root port,
+  - and the device has the SlotImplemented bit set in its PCIe capability
+register.
+
+  @param[in] PciIoDevice  The device being checked.
+
+  @retval TRUE   PciIoDevice is a PCIe port that accepts a hotplugged device.
+  @retval FALSE  Otherwise.
+
+**/
+BOOLEAN
+SupportsPcieHotplug (
+  IN PCI_IO_DEVICE  *PciIoDevice
+  );
+
+/**
   Get resource padding if the specified PCI bridge is a hot plug bus.
 
   @param PciIoDevicePCI bridge instance.
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c
index ca8766869ae7..19dd97a4528d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciHotPlugSupport.c
@@ -317,6 +317,69 @@ IsSHPC (
 }
 
 /**
+  Check whether PciIoDevice supports PCIe hotplug.
+
+  This is equivalent to the following condition:
+  - the device is either a PCIe switch downstream port or a root port,
+  - and the device has the SlotImplemented bit set in its PCIe capability
+register.
+
+  @param[in] PciIoDevice  The device being checked.
+
+  @retval TRUE   PciIoDevice is a PCIe port that accepts a hotplugged device.
+  @retval FALSE  Otherwise.
+
+**/
+BOOLEAN
+SupportsPcieHotplug (
+  IN PCI_IO_DEVICE  *PciIoDevice
+  )
+{
+  UINT32  Offset;
+  EFI_STATUS  Status;
+  PCI_REG_PCIE_CAPABILITY Capability;
+
+  if (PciIoDevice == NULL) {
+return FALSE;
+  }
+
+  //
+  // Read the PCI Express Capabilities Register
+  //
+  if (!PciIoDevice->IsPciExp) {
+return FALSE;
+  }
+  Offset = PciIoDevice->PciExpressCapabilityOffset +
+   OFFSET_OF (PCI_CAPABILITY_PCIEXP, Capability);
+
+  Status = PciIoDevice->PciIo.Pci.Read (
+>PciIo,
+EfiPciIoWidthUint16,
+Offset,
+1,
+
+);
+  if (EFI_ERROR (Status)) {
+return FALSE;
+  }
+
+  //
+  // Check the contents of the register
+  //
+  switch (Capability.Bits.DevicePortType) {
+  case PCIE_DEVICE_PORT_TYPE_ROOT_PORT:
+  case PCIE_DEVICE_PORT_TYPE_DOWNSTREAM_PORT:
+break;
+  default:
+return FALSE;
+  }
+  if 

Re: [edk2] [RFC V2] Add more flexible PCD value formats in DEC/INF/DSC/FDF files

[Yao, Jiewen]

HI Mike
One possible "skip byte" usage is to align with ACPI ASL OPREGION global NVS 
structure.
For example,
  Field(GNVS,AnyAcc,Lock,Preserve)
  {
  //
  // Miscellaneous Dynamic Registers:
  //
  Offset(0),  , 16,
  Offset(4),  , 8,
  Offset(6),  , 8,

I think we can claim to assign 0 to skipped field.

Thank you
Yao Jiewen

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Kinney, 
Michael D
Sent: Friday, July 1, 2016 4:04 AM
To: Gao, Liming ; edk2-devel@lists.01.org; Kinney, 
Michael D 
Subject: Re: [edk2] [RFC V2] Add more flexible PCD value formats in 
DEC/INF/DSC/FDF files

Liming,

Responses below.

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Wednesday, June 22, 2016 6:33 PM
> To: Kinney, Michael D 
> >; 
> edk2-devel@lists.01.org; Kinney,
> Michael D >
> Subject: RE: [RFC V2] Add more flexible PCD value formats in DEC/INF/DSC/FDF 
> files
>
> Mike:
>   For GUID C Name in DEC file, how to find its value? Or require it be 
> defined in the
> same package.

DEC File - GUID C name must be defined in same package
INF File - GUID C name must be defined in dependent package from [Packages] 
section
DSC/FDF File - GUID C name must be defined in dependent package of module 
referenced in DSC/FDF

>   For VOID* byte array, could we support the usage to set the value in the 
> specified
> offset. For example, VOID*   { 0, 1, 2, OFFSET(8) 3, 4, 5 }

Is this used to skip init of bytes?  What value is assigned to skipped bytes?

>
> Thanks
> Liming
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Kinney, Michael D
> > Sent: Thursday, June 23, 2016 8:28 AM
> > To: edk2-devel@lists.01.org; Kinney, 
> > Michael D >
> > Subject: [edk2] [RFC V2] Add more flexible PCD value formats in
> > DEC/INF/DSC/FDF files
> >
> > Hi
> >
> > I have updated this RFC to V2 that includes a concepts provided by
> > Tim Lewis that includes extensions to initialize a VOID* PCD that
> > cannot be described with a C structure (e.g. UEFI Load Option).
> >
> > The changes for V2 are:
> >
> > * Update examples to cover new syntax cases
> > * Clarify that this proposal is for PCD values in DEC/INF/DSC/FDF
> >   files.  Not just DEC/DSC.
> > * Add support for multi character ASCII and UCS16 char constants
> >   and strings for VOID* PCD byte arrays
> > * Add support for GUID() macro in VOID* PCD byte arrays
> > * Add support for DEVICE_PATH() macro in VOID* PCD byte arrays
> > * Add support for LABEL() and OFFSET_OF() macros in VOID* PCD
> >   byte arrays.
> > * Add support for expressions to compute values.
> >
> > 
> >
> > I would like to propose more flexible value formats for PCDs
> > in DEC/INF/DSC/FDF files and --pcd command line option.
> >
> > This would include the following additions:
> >
> > * ASCII character values using single quotes(e.g. 'A').
> > * ASCII multi-character values using single quotes(e.g. 'ABCD').
> >   No null-terminator.
> > * UCS16 character values using L and single quotes(e.g. L'A').
> > * UCS16 multi-character values using L and single quotes(e.g. L'ABCD').
> >   No null-terminator.
> > * Support TRUE/FALSE for UINT8/16/32/64 values
> > * Support TRUE/FALSE in VOID* byte array
> > * Support decimal values in VOID* byte array
> > * Support ASCII single quote character values in VOID* byte arrays
> > * Support byte arrays, ASCII string, Unicode String,
> >   ASCII multi-character strings, UCS16 multi-character strings for
> >   UINT8/16/32/64 values as long as they fit in the target type.
> > * Support multi-character values using single quotes(e.g. 'ABCD') in
> >   VOID* byte arrays that are expanded to a sequence of byte values.
> > * Support multi-character values using double quotes(e.g. "ABCD") in
> >   VOID* byte arrays that are expanded to a sequence of byte values.
> > * Support UCS16 multi-character values using L and single quotes(e.g.
> > L'ABCD')
> >   in VOID* byte arrays that are expanded to a sequence of byte values.
> > * Support UCS16 multi-character values using L and double quotes(e.g.
> > L"ABCD")
> >   in VOID* byte arrays that are expanded to a sequence of byte values.
> > * Support UINT8(), UINT16, UINT32(), and UINT64() macros in
> >   VOID* byte arrays that are expanded to a sequence of byte values.
> > * Support GUID() macro in VOID* byte arrays that are expanded to a
> >   sequence of 16 byte values.  The GUID() operand supports Registry
> >   format GUIDs, C struct style GUIDs, and GUID C names.
> > * Support DEVICE_PATH() macro in VOID* byte arrays that are expanded
> >   to a sequence of byte values.  The DEVICE_PATH() operand is a
> >   double quoted string that 

Re: [edk2] [PATCH] ShellPkg: UefiHandleParsingLib: remove tautological comparison

[Laszlo Ersek]

On 06/30/16 23:09, Carsey, Jaben wrote:
> Reviewed-by: Jaben Carsey 

Thank you! Commit 42cb90685275.

Cheers
Laszlo

>> On Jun 30, 2016, at 2:05 PM, Laszlo Ersek  wrote:
>>
>> The code being removed in this patch dates back to git commit a405b86d274d
>> (Sep 14, 2010; "udk2010.up2.shell initial release."). The condition always
>> evaluates to true, and it breaks DEBUG builds of ArmVirtPkg with gcc-6.1:
>>
>>  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c:
>>In function 'ParseHandleDatabaseByRelationshipWithType':
>>  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c:2465:76:
>>error: self-comparison always evaluates to true
>>[-Werror=tautological-compare]
>> ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
>>
>> Cc: Ard Biesheuvel 
>> Cc: Gerd Hoffmann 
>> Cc: Jaben Carsey 
>> Cc: Michael Zimmermann 
>> Reported-by: Gerd Hoffmann 
>> Reported-by: Michael Zimmermann 
>> Suggested-by: Jaben Carsey 
>> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/13794/focus=13939
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek 
>> ---
>> ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 4 
>> 1 file changed, 4 deletions(-)
>>
>> diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
>> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
>> index b82f925c9218..63710865251d 100644
>> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
>> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
>> @@ -2460,10 +2460,6 @@ ParseHandleDatabaseByRelationshipWithType (
>> (*HandleType)[HandleIndex] |= (UINTN)HR_COMPONENT_NAME_HANDLE;
>>   } else if (CompareGuid (ProtocolGuidArray[ProtocolIndex], 
>> )  ) {
>> (*HandleType)[HandleIndex] |= (UINTN)HR_DEVICE_HANDLE;
>> -  } else {
>> -DEBUG_CODE_BEGIN();
>> -ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
>> -DEBUG_CODE_END();
>>   }
>>   //
>>   // Retrieve the list of agents that have opened each protocol
>> -- 
>> 1.8.3.1
>>

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

Re: [edk2] [PATCH] ShellPkg: UefiHandleParsingLib: remove tautological comparison

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

-Jaben

> On Jun 30, 2016, at 2:05 PM, Laszlo Ersek  wrote:
> 
> The code being removed in this patch dates back to git commit a405b86d274d
> (Sep 14, 2010; "udk2010.up2.shell initial release."). The condition always
> evaluates to true, and it breaks DEBUG builds of ArmVirtPkg with gcc-6.1:
> 
>  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c:
>In function 'ParseHandleDatabaseByRelationshipWithType':
>  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c:2465:76:
>error: self-comparison always evaluates to true
>[-Werror=tautological-compare]
> ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
> 
> Cc: Ard Biesheuvel 
> Cc: Gerd Hoffmann 
> Cc: Jaben Carsey 
> Cc: Michael Zimmermann 
> Reported-by: Gerd Hoffmann 
> Reported-by: Michael Zimmermann 
> Suggested-by: Jaben Carsey 
> Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/13794/focus=13939
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
> ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 4 
> 1 file changed, 4 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> index b82f925c9218..63710865251d 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> @@ -2460,10 +2460,6 @@ ParseHandleDatabaseByRelationshipWithType (
> (*HandleType)[HandleIndex] |= (UINTN)HR_COMPONENT_NAME_HANDLE;
>   } else if (CompareGuid (ProtocolGuidArray[ProtocolIndex], 
> )  ) {
> (*HandleType)[HandleIndex] |= (UINTN)HR_DEVICE_HANDLE;
> -  } else {
> -DEBUG_CODE_BEGIN();
> -ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
> -DEBUG_CODE_END();
>   }
>   //
>   // Retrieve the list of agents that have opened each protocol
> -- 
> 1.8.3.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ShellPkg: UefiHandleParsingLib: remove tautological comparison

[Laszlo Ersek]

The code being removed in this patch dates back to git commit a405b86d274d
(Sep 14, 2010; "udk2010.up2.shell initial release."). The condition always
evaluates to true, and it breaks DEBUG builds of ArmVirtPkg with gcc-6.1:

  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c:
In function 'ParseHandleDatabaseByRelationshipWithType':
  ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c:2465:76:
error: self-comparison always evaluates to true
[-Werror=tautological-compare]
 ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);

Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Jaben Carsey 
Cc: Michael Zimmermann 
Reported-by: Gerd Hoffmann 
Reported-by: Michael Zimmermann 
Suggested-by: Jaben Carsey 
Ref: http://thread.gmane.org/gmane.comp.bios.edk2.devel/13794/focus=13939
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index b82f925c9218..63710865251d 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -2460,10 +2460,6 @@ ParseHandleDatabaseByRelationshipWithType (
 (*HandleType)[HandleIndex] |= (UINTN)HR_COMPONENT_NAME_HANDLE;
   } else if (CompareGuid (ProtocolGuidArray[ProtocolIndex], 
)  ) {
 (*HandleType)[HandleIndex] |= (UINTN)HR_DEVICE_HANDLE;
-  } else {
-DEBUG_CODE_BEGIN();
-ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
-DEBUG_CODE_END();
   }
   //
   // Retrieve the list of agents that have opened each protocol
-- 
1.8.3.1

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

Re: [edk2] PCIe hotplug into downstream port

[Bill Paul]

Of all the gin joints in all the towns in all the world, Laszlo Ersek had to 
walk into mine at 11:58:29 on Thursday 30 June 2016 and say:

> On 06/30/16 20:14, Brian J. Johnson wrote:
> > On 06/30/2016 11:47 AM, Laszlo Ersek wrote:
> >> On 06/30/16 18:39, Marcel Apfelbaum wrote:
> >>> On 06/30/2016 07:21 PM, Marcel Apfelbaum wrote:
>  On 06/30/2016 04:07 PM, Laszlo Ersek wrote:
> > Hi,
[snip] 
> > We can safely skip
> > allocating IO ports to those cards, saving significant space in the
> > overall IO port map.  We've worked with the card vendors on cleaning up
> > their PCIe advertisements and eliminating the use of IO ports, but it
> > takes time.
> 
> I surprisingly often hear about non-conformance or unjustified
> behavior... I guess with huge, elaborate specs, this is unavoidable. No
> single programmer might be able to internalize it all.

This is one of my pet peeves. Things that've thrown me for a loop are:

- Customers with their own special PCI devices that want _enormous_ amounts of 
MMIO space -- there was one case recently where the device wanted an 8GB 
window. (Even if this is legal according to the spec, it strikes me as a 
little unfriendly.)

- Oddball PCI/PCIe controller implementations in SoCs. For example, some 
devices implement support for MSI but don't support the ability to dynamically 
allocate interrupt vectors for multiple MSI sources. This means there's often 
just one vector for MSI events and it's shared by all devices on the bus. (The 
Freescale/NXP i.MX6 is one example of this.)

- FPGA-based PCIe controller logic blocks. To be fair, I've only worked with 
one of these, namely the Xilinx Zynq7k. Maybe that one is just particularly 
irritating. Out of the box, the Zynq doesn't support PCIe: you have to load a 
bitstream file into it to make the internal FPGA think it's a PCIe controller. 
I think it's the case that the customer can customize the logic to add or 
remove features, probably to optimize the use of the available logic gates in 
the FPGA. Unfortunately this can make driver development difficult because it 
makes the "hardware" a moving target. It also means the "hardware" design 
might be kept very simple and a lot of heavy lifting is deferred to the driver 
software.

The Zynq PCIe controller has only one interrupt vector, and it uses it for 
_all_ events (INTx interrupts, MSI interrupts and controller errors). For the 
INTx interrupts, you have to implement level-triggered interrupt semantics in 
software. If you do it wrong, you could miss an event and get totally stuck. 
(And last time I looked at it, I wasn't 100% convinced that the Linux driver 
was doing it exactly right either.) If you only ever want to connect one PCIe 
device to the controller, you can get away with some fairly simple logic, but 
hardly any of our customers would be satisfied with that.

Also, the documentation for the PCIe controller block says that it supports 
both MMIO and I/O space BARs, but the particular bitstream file that I was 
given to work with only seemed to support MMIO. (The registers documented to 
configure the outbound window for I/O transactions didn't seem to do 
anything.)

While I realize that I/O space BARs and INTx interrupts are old and busted and 
MMIO and MSI are the new hotness, we sometimes have customers with highly 
specialized legacy hardware and I would prefer not to throw a spanner in their 
works if I can avoid it.

-Bill

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

-- 
=
-Bill Paul(510) 749-2329 | Senior Member of Technical Staff,
 wp...@windriver.com | Master of Unix-Fu - Wind River Systems
=
   "I put a dollar in a change machine. Nothing changed." - George Carlin
=
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PCIe hotplug into downstream port

[Brian J. Johnson]

On 06/30/2016 11:47 AM, Laszlo Ersek wrote:

On 06/30/16 18:39, Marcel Apfelbaum wrote:

On 06/30/2016 07:21 PM, Marcel Apfelbaum wrote:

On 06/30/2016 04:07 PM, Laszlo Ersek wrote:

Hi,

does anyone on this list have experience with $SUBJECT, using physical
UEFI firmware derived from edk2?

The problem we're seeing with OVMF is the following: PCIe hotplug works
with PCIe root ports, but it doesn't work with PCIe downstream ports.
The error messages reported by both Linux and Windows guests indicate
"lack of resources" for the device being plugged.

In my understanding:

* Each PCIe downstream port qualifies as a separate bus / bridge.

* Bridges need IO and MMIO resources for themselves. When a device is
behind a bridge at boot, the device's IO and MMIO BARs are carved out
of the bridge's IO and MMIO apertures. For IO, the standard bridge
aperture size is 4096 (byte-sized) ports.

* If no device behind a bridge needs a specific type of resource (for
example, no devices behind a bridge have IO BARs), then that kind of
aperture is not necessary for the bridge either.

A special case of this is when there are no devices behind a bridge
at all. Example: PCIe downstream port empty at boot. In this case,
the bridge needs no resources at all, at boot.

* Different firmwares seem to follow different policies in the last
case (-> bridge empty at boot). SeaBIOS for example allocates the
0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
allocates no apertures for bridges that have no devices behind them
at boot.

This appears to be a trade-off: if the bridge remains empty for the
entire lifetime of the OS, then any IO ports that have been allocated
in advance will be in vain (wasted). However, if the aperture is not
allocated at boot, then hotplug under the bridge may not work --
resources for the device being hot-plugged cannot be allocated from
nonexistent bridge apertures.



Hi Laszlo,
I fully agree with your assessment.

  From the firmware point of view it seems like a waste to allocate an
IO/MEM range for
an unused PCI/PCIe-bridge indeed. Especially IO.

But what about MEM?
Let's have a look to this special case: PCIe downstream ports on
64-bit systems.

1. PCIe spec requires devices to work correctly even without enabling
their IO BARs.
2. 64-bit Memory space is pretty large, decent CPUs support 46-bit
addresses, so even if the system uses Terabytes of RAM
 and has dozens of switches we still have enough unused MEM space.

My point is we can skip assigning IO for PCIe ports but we can assign
a 4K MEM range in 64-bit memory for them
without being too greedy.


I meant 2M MEM range, sorry.  4k is the min range for IO, of course.




One thing to watch out for is prefetchable vs. non-prefetchable MEM 
ranges.  On Intel chipsets, the high (64-bit) MMIO range is always 
prefetchable, which mainly makes it useful for frame buffers, 
accelerator cards with RAM mapped to MMIO in the host, and the like. 
Only the low (32-bit) MMIO space is non-prefetchable, and suitable for 
registers and things which need precise ordering of loads and stores.


So you can demote a prefetchable BAR into non-prefetchable MMIO space if 
necessary, but not the other way around.  Which is a shame, because the 
32-bit MMIO space is much more limited in size than the 64-bit space.



I've just been looking at the values reserved by SeaBIOS [src/fw/pciinit.c]:

#define PCI_BRIDGE_MEM_MIN(1<<21)  // 2M == hugepage size
#define PCI_BRIDGE_IO_MIN  0x1000  // mandated by pci bridge spec

2MB sounds perfectly fine for MEM, especially given that OVMF supports a
64-bit MMIO aperture (32GB in size by default).

We discussed the IO size earlier -- I think 4K is overkill (despite
being required by the standard). We considered half K, 1K, 2K earlier...
I think I'll take a stab at half K first.


It depends on what your hardware (physical or virtual) supports.  A 
standards-compliant bridge may not be able to map I/O ports to targets 
on less than a 4k boundary.  Yes, 4k is ridiculous overkill, but it may 
be your only choice, depending on the details of the h/w and/or VMM.


We've had to add special cases in BIOS for certain cards we support 
which request IO space, but don't actually use it.  We can safely skip 
allocating IO ports to those cards, saving significant space in the 
overall IO port map.  We've worked with the card vendors on cleaning up 
their PCIe advertisements and eliminating the use of IO ports, but it 
takes time.

--

Brian



  "My software never has bugs.  It just develops random features."
   -- Quoted by Khan Tran
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PCIe hotplug into downstream port

[Laszlo Ersek]

On 06/30/16 18:39, Marcel Apfelbaum wrote:
> On 06/30/2016 07:21 PM, Marcel Apfelbaum wrote:
>> On 06/30/2016 04:07 PM, Laszlo Ersek wrote:
>>> Hi,
>>>
>>> does anyone on this list have experience with $SUBJECT, using physical
>>> UEFI firmware derived from edk2?
>>>
>>> The problem we're seeing with OVMF is the following: PCIe hotplug works
>>> with PCIe root ports, but it doesn't work with PCIe downstream ports.
>>> The error messages reported by both Linux and Windows guests indicate
>>> "lack of resources" for the device being plugged.
>>>
>>> In my understanding:
>>>
>>> * Each PCIe downstream port qualifies as a separate bus / bridge.
>>>
>>> * Bridges need IO and MMIO resources for themselves. When a device is
>>>behind a bridge at boot, the device's IO and MMIO BARs are carved out
>>>of the bridge's IO and MMIO apertures. For IO, the standard bridge
>>>aperture size is 4096 (byte-sized) ports.
>>>
>>> * If no device behind a bridge needs a specific type of resource (for
>>>example, no devices behind a bridge have IO BARs), then that kind of
>>>aperture is not necessary for the bridge either.
>>>
>>>A special case of this is when there are no devices behind a bridge
>>>at all. Example: PCIe downstream port empty at boot. In this case,
>>>the bridge needs no resources at all, at boot.
>>>
>>> * Different firmwares seem to follow different policies in the last
>>>case (-> bridge empty at boot). SeaBIOS for example allocates the
>>>0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
>>>allocates no apertures for bridges that have no devices behind them
>>>at boot.
>>>
>>>This appears to be a trade-off: if the bridge remains empty for the
>>>entire lifetime of the OS, then any IO ports that have been allocated
>>>in advance will be in vain (wasted). However, if the aperture is not
>>>allocated at boot, then hotplug under the bridge may not work --
>>>resources for the device being hot-plugged cannot be allocated from
>>>nonexistent bridge apertures.
>>>
>>
>> Hi Laszlo,
>> I fully agree with your assessment.
>>
>>  From the firmware point of view it seems like a waste to allocate an
>> IO/MEM range for
>> an unused PCI/PCIe-bridge indeed. Especially IO.
>>
>> But what about MEM?
>> Let's have a look to this special case: PCIe downstream ports on
>> 64-bit systems.
>>
>> 1. PCIe spec requires devices to work correctly even without enabling
>> their IO BARs.
>> 2. 64-bit Memory space is pretty large, decent CPUs support 46-bit
>> addresses, so even if the system uses Terabytes of RAM
>> and has dozens of switches we still have enough unused MEM space.
>>
>> My point is we can skip assigning IO for PCIe ports but we can assign
>> a 4K MEM range in 64-bit memory for them
>> without being too greedy.
> 
> I meant 2M MEM range, sorry.  4k is the min range for IO, of course.

I've just been looking at the values reserved by SeaBIOS [src/fw/pciinit.c]:

#define PCI_BRIDGE_MEM_MIN(1<<21)  // 2M == hugepage size
#define PCI_BRIDGE_IO_MIN  0x1000  // mandated by pci bridge spec

2MB sounds perfectly fine for MEM, especially given that OVMF supports a
64-bit MMIO aperture (32GB in size by default).

We discussed the IO size earlier -- I think 4K is overkill (despite
being required by the standard). We considered half K, 1K, 2K earlier...
I think I'll take a stab at half K first.

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

Re: [edk2] PCIe hotplug into downstream port

[Marcel Apfelbaum]

On 06/30/2016 07:21 PM, Marcel Apfelbaum wrote:

On 06/30/2016 04:07 PM, Laszlo Ersek wrote:

Hi,

does anyone on this list have experience with $SUBJECT, using physical
UEFI firmware derived from edk2?

The problem we're seeing with OVMF is the following: PCIe hotplug works
with PCIe root ports, but it doesn't work with PCIe downstream ports.
The error messages reported by both Linux and Windows guests indicate
"lack of resources" for the device being plugged.

In my understanding:

* Each PCIe downstream port qualifies as a separate bus / bridge.

* Bridges need IO and MMIO resources for themselves. When a device is
   behind a bridge at boot, the device's IO and MMIO BARs are carved out
   of the bridge's IO and MMIO apertures. For IO, the standard bridge
   aperture size is 4096 (byte-sized) ports.

* If no device behind a bridge needs a specific type of resource (for
   example, no devices behind a bridge have IO BARs), then that kind of
   aperture is not necessary for the bridge either.

   A special case of this is when there are no devices behind a bridge
   at all. Example: PCIe downstream port empty at boot. In this case,
   the bridge needs no resources at all, at boot.

* Different firmwares seem to follow different policies in the last
   case (-> bridge empty at boot). SeaBIOS for example allocates the
   0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
   allocates no apertures for bridges that have no devices behind them
   at boot.

   This appears to be a trade-off: if the bridge remains empty for the
   entire lifetime of the OS, then any IO ports that have been allocated
   in advance will be in vain (wasted). However, if the aperture is not
   allocated at boot, then hotplug under the bridge may not work --
   resources for the device being hot-plugged cannot be allocated from
   nonexistent bridge apertures.



Hi Laszlo,
I fully agree with your assessment.

 From the firmware point of view it seems like a waste to allocate an IO/MEM 
range for
an unused PCI/PCIe-bridge indeed. Especially IO.

But what about MEM?
Let's have a look to this special case: PCIe downstream ports on 64-bit systems.

1. PCIe spec requires devices to work correctly even without enabling their IO 
BARs.
2. 64-bit Memory space is pretty large, decent CPUs support 46-bit addresses, 
so even if the system uses Terabytes of RAM
and has dozens of switches we still have enough unused MEM space.

My point is we can skip assigning IO for PCIe ports but we can assign a 4K MEM 
range in 64-bit memory for them
without being too greedy.


I meant 2M MEM range, sorry.  4k is the min range for IO, of course.


Thanks,
Marcel



Would this be a decent trade-off *for OVMF* ?

Thanks,
Marcel


And that's what we are experiencing with OVMF.

My questions are:

- Is the above behavior of the edk2 PCI bus driver intentional?

- Does PCIe hotplug into downstream ports work with any (phys) firmware
   forked from edk2? Brian, Samer, do you guys have experience with this?

- What could be the difference between root ports and downstream ports?
   (Hotplug into root ports seems to work fine.) Are OSes entitled to
   allocate any unused address space (MMIO and IO) right when a device is
   hot-plugged into a root port?

Thanks!
Laszlo





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

Re: [edk2] PCIe hotplug into downstream port

[Marcel Apfelbaum]

On 06/30/2016 04:07 PM, Laszlo Ersek wrote:

Hi,

does anyone on this list have experience with $SUBJECT, using physical
UEFI firmware derived from edk2?

The problem we're seeing with OVMF is the following: PCIe hotplug works
with PCIe root ports, but it doesn't work with PCIe downstream ports.
The error messages reported by both Linux and Windows guests indicate
"lack of resources" for the device being plugged.

In my understanding:

* Each PCIe downstream port qualifies as a separate bus / bridge.

* Bridges need IO and MMIO resources for themselves. When a device is
   behind a bridge at boot, the device's IO and MMIO BARs are carved out
   of the bridge's IO and MMIO apertures. For IO, the standard bridge
   aperture size is 4096 (byte-sized) ports.

* If no device behind a bridge needs a specific type of resource (for
   example, no devices behind a bridge have IO BARs), then that kind of
   aperture is not necessary for the bridge either.

   A special case of this is when there are no devices behind a bridge
   at all. Example: PCIe downstream port empty at boot. In this case,
   the bridge needs no resources at all, at boot.

* Different firmwares seem to follow different policies in the last
   case (-> bridge empty at boot). SeaBIOS for example allocates the
   0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
   allocates no apertures for bridges that have no devices behind them
   at boot.

   This appears to be a trade-off: if the bridge remains empty for the
   entire lifetime of the OS, then any IO ports that have been allocated
   in advance will be in vain (wasted). However, if the aperture is not
   allocated at boot, then hotplug under the bridge may not work --
   resources for the device being hot-plugged cannot be allocated from
   nonexistent bridge apertures.



Hi Laszlo,
I fully agree with your assessment.

From the firmware point of view it seems like a waste to allocate an IO/MEM 
range for
an unused PCI/PCIe-bridge indeed. Especially IO.

But what about MEM?
Let's have a look to this special case: PCIe downstream ports on 64-bit systems.

1. PCIe spec requires devices to work correctly even without enabling their IO 
BARs.
2. 64-bit Memory space is pretty large, decent CPUs support 46-bit addresses, 
so even if the system uses Terabytes of RAM
   and has dozens of switches we still have enough unused MEM space.

My point is we can skip assigning IO for PCIe ports but we can assign a 4K MEM 
range in 64-bit memory for them
without being too greedy.

Would this be a decent trade-off *for OVMF* ?

Thanks,
Marcel


And that's what we are experiencing with OVMF.

My questions are:

- Is the above behavior of the edk2 PCI bus driver intentional?

- Does PCIe hotplug into downstream ports work with any (phys) firmware
   forked from edk2? Brian, Samer, do you guys have experience with this?

- What could be the difference between root ports and downstream ports?
   (Hotplug into root ports seems to work fine.) Are OSes entitled to
   allocate any unused address space (MMIO and IO) right when a device is
   hot-plugged into a root port?

Thanks!
Laszlo



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

Re: [edk2] PCIe hotplug into downstream port

[Marcel Apfelbaum]

On 06/30/2016 06:06 PM, Alex Williamson wrote:

On Thu, 30 Jun 2016 15:07:27 +0200
Laszlo Ersek  wrote:

- What could be the difference between root ports and downstream ports?
   (Hotplug into root ports seems to work fine.)


Hi Laszlo,

It should be no difference IMHO. Both are treated the same by the PCIe spec as 
far as I know.

Are OSes entitled to

   allocate any unused address space (MMIO and IO) right when a device is
   hot-plugged into a root port?




Yes, resources "re-banlancing"| can be triggered by the OS and at least Windows 
is actually
doing so if you hot-plug devices behind the bridge using more IO/MEM than what 
is allocated
for the bridge by the firmware. I don't know for sure, but I think Linux will 
try this too.


A possible difference is simply the depth of the hierarchy, the
apertures on a root port come directly from the host bridge and there's
no affect to other devices to disable and resize the root port
apertures.

 In order to resize a switch downstream port aperture, the

OS would need to touch multiple levels, which could affect peer devices
already in operation.


But this should be the same as with nested PCI bridges, right? And If I would 
have to guess,
there is no problem with them.


  Does hotplug to a downstream switch port work if

the hot added device is the only endpoint within that sub-hierarchy?


I don't see why not. If the firmware pre-allocates IO/MEM ranges there is no 
problem.
Otherwise the OS uses the free IO/MEM ranges specified by the _CRS for the 
respective host-bridge.

Thanks,
Marcel

 I

wouldn't necessarily be surprised either way, it seems like a
complicated resource runtime reallocation issue that probably isn't
very prevalent on real hardware.  Thanks,

Alex



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

Re: [edk2] weird ShellPkg line

[Carsey, Jaben]

Odd for sure! That little block will be added to my low priority chopping queue.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael Zimmermann
> Sent: Thursday, June 30, 2016 8:48 AM
> To: Carsey, Jaben 
> Cc: edk2-devel@lists.01.org; Qiu, Shumin 
> Subject: Re: [edk2] weird ShellPkg line
> Importance: High
> 
> that operation must have taken place during the initial development process
> then because the line exists since the Initial commitof that file 'a405b86
> udk2010.up2.shell initial release.'
> 
> jcarsey  is to blame :)
> 
> On Thu, Jun 30, 2016 at 5:18 PM, Carsey, Jaben 
> wrote:
> 
> > Interesting… My guess would be that is used to do something and is the
> > side effect of a find and replace operation and it should have been
> > removed, but wasn’t…
> >
> >
> >
> > -Jaben
> >
> >
> >
> >
> >
> >
> >
> > *From:* peter.kirme...@ts.fujitsu.com [mailto:
> > peter.kirme...@ts.fujitsu.com]
> > *Sent:* Wednesday, June 29, 2016 1:22 AM
> > *To:* Michael Zimmermann 
> > *Cc:* edk2-devel@lists.01.org; Carsey, Jaben ;
> > Qiu, Shumin 
> > *Subject:* RE: [edk2] weird ShellPkg line
> > *Importance:* High
> >
> >
> >
> > Hi,
> >
> >
> >
> > yes that’s right. I found those TRUE-Asserts on various other projects as
> > well and I never saw any good reason for this.
> >
> > My most popular assumptions are that this is some kind of “prepare an
> > assert here, so it can be enabled easily”.
> >
> > Or “let the debug message be part of the binary, so I can find this line
> > of code more easily in the debugger without symbols”.
> >
> >
> >
> > Maybe someone here has any good reason for such constructs. I lived with
> > my assumptions about that.
> >
> >
> >
> > Best Regards,
> >
> >   Peter
> >
> >
> >
> > *From:* Michael Zimmermann [mailto:sigmaepsilo...@gmail.com
> > ]
> > *Sent:* Wednesday, June 29, 2016 10:12 AM
> > *To:* Kirmeier, Peter
> > *Cc:* edk2-devel@lists.01.org; Jaben Carsey; Shumin Qiu
> > *Subject:* Re: [edk2] weird ShellPkg line
> >
> >
> >
> > Hi,
> >
> >
> >
> > but doesn't ASSERT only trigger if the condition is not true?
> >
> > to me it looks like this ASSERT would never show an error.
> >
> >
> >
> > Thanks,
> >
> > Michael
> >
> >
> >
> > On Wed, Jun 29, 2016 at 9:53 AM, peter.kirme...@ts.fujitsu.com <
> > peter.kirme...@ts.fujitsu.com> wrote:
> >
> > Hi Michael,
> >
> > for me it looks like an ASSERT(TRUE) when no GUID comparison matched.
> > It actually produces a more readable debug message rather than "TRUE"
> > which allows you to find the actual ASSERT faster.
> >
> > Best Regards,
> >   Peter
> >
> >
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Michael Zimmermann
> > Sent: Wednesday, June 29, 2016 9:42 AM
> > To: edk2-devel@lists.01.org
> > Cc: Jaben Carsey; Shumin Qiu
> > Subject: [edk2] weird ShellPkg line
> >
> > Hi,
> >
> > I found this weird line in ShellPkg(which also throws a warning in GCC6).
> > What does it do?
> >
> > ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
> >
> >
> >
> https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiHandl
> eParsingLib/UefiHandleParsingLib.c#L2465
> >
> >
> > Thanks
> > Michael
> >
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> >
> >
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] weird ShellPkg line

[Michael Zimmermann]

that operation must have taken place during the initial development process
then because the line exists since the Initial commitof that file 'a405b86
udk2010.up2.shell initial release.'

jcarsey  is to blame :)

On Thu, Jun 30, 2016 at 5:18 PM, Carsey, Jaben 
wrote:

> Interesting… My guess would be that is used to do something and is the
> side effect of a find and replace operation and it should have been
> removed, but wasn’t…
>
>
>
> -Jaben
>
>
>
>
>
>
>
> *From:* peter.kirme...@ts.fujitsu.com [mailto:
> peter.kirme...@ts.fujitsu.com]
> *Sent:* Wednesday, June 29, 2016 1:22 AM
> *To:* Michael Zimmermann 
> *Cc:* edk2-devel@lists.01.org; Carsey, Jaben ;
> Qiu, Shumin 
> *Subject:* RE: [edk2] weird ShellPkg line
> *Importance:* High
>
>
>
> Hi,
>
>
>
> yes that’s right. I found those TRUE-Asserts on various other projects as
> well and I never saw any good reason for this.
>
> My most popular assumptions are that this is some kind of “prepare an
> assert here, so it can be enabled easily”.
>
> Or “let the debug message be part of the binary, so I can find this line
> of code more easily in the debugger without symbols”.
>
>
>
> Maybe someone here has any good reason for such constructs. I lived with
> my assumptions about that.
>
>
>
> Best Regards,
>
>   Peter
>
>
>
> *From:* Michael Zimmermann [mailto:sigmaepsilo...@gmail.com
> ]
> *Sent:* Wednesday, June 29, 2016 10:12 AM
> *To:* Kirmeier, Peter
> *Cc:* edk2-devel@lists.01.org; Jaben Carsey; Shumin Qiu
> *Subject:* Re: [edk2] weird ShellPkg line
>
>
>
> Hi,
>
>
>
> but doesn't ASSERT only trigger if the condition is not true?
>
> to me it looks like this ASSERT would never show an error.
>
>
>
> Thanks,
>
> Michael
>
>
>
> On Wed, Jun 29, 2016 at 9:53 AM, peter.kirme...@ts.fujitsu.com <
> peter.kirme...@ts.fujitsu.com> wrote:
>
> Hi Michael,
>
> for me it looks like an ASSERT(TRUE) when no GUID comparison matched.
> It actually produces a more readable debug message rather than "TRUE"
> which allows you to find the actual ASSERT faster.
>
> Best Regards,
>   Peter
>
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Michael Zimmermann
> Sent: Wednesday, June 29, 2016 9:42 AM
> To: edk2-devel@lists.01.org
> Cc: Jaben Carsey; Shumin Qiu
> Subject: [edk2] weird ShellPkg line
>
> Hi,
>
> I found this weird line in ShellPkg(which also throws a warning in GCC6).
> What does it do?
>
> ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);
>
>
> https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c#L2465
>
>
> Thanks
> Michael
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
>
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/6] fix ASSERT_EFI_ERROR() misuse around the tree

[Laszlo Ersek]

On 06/28/16 15:25, Laszlo Ersek wrote:
> Gerd's Jenkins CI worker has recently tried to build a few edk2 packages
> with gcc-6. Gcc-6 introduces a new warning (among many others, likely)
> called "bool-compare":
> 
>   
> https://gcc.gnu.org/onlinedocs/gcc-6.1.0/gcc/Warning-Options.html#index-Wbool-compare-453
> 
> The build broke subsequently, exposing an actual bug in the code:
> 
>> "gcc" -g -fshort-wchar -fno-strict-aliasing -Wall -Werror
>> -Wno-array-bounds -ffunction-sections -fdata-sections -c -include
>> AutoGen.h -fno-common -DSTRING_ARRAY_NAME=PiSmmCpuDxeSmmStrings -m64
>> -fno-stack-protector "-DEFIAPI=__attribute__((ms_abi))"
>> -DNO_BUILTIN_VA_FUNCS -mno-red-zone -Wno-address -mcmodel=large
>> -fno-asynchronous-unwind-tables -Wno-address -mno-mmx -mno-sse -o
>> Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm/OUTPUT/X64/SmmProfileArch.obj
>> -IUefiCpuPkg/PiSmmCpuDxeSmm/X64 -IUefiCpuPkg/PiSmmCpuDxeSmm
>> -IBuild/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm/DEBUG
>> -IMdePkg -IMdePkg/Include -IMdePkg/Include/X64 -IMdeModulePkg
>> -IMdeModulePkg/Include -IUefiCpuPkg -IUefiCpuPkg/Include
>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c
>>
>> In file included from 
>> Build/OvmfX64/DEBUG_GCC49/X64/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm/DEBUG/AutoGen.h:16:0,
>>  from :0:
>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c: In function 
>> 'InitPagesForPFHandler':
>> MdePkg/Include/Base.h:882:75: error: comparison of constant '0' with boolean 
>> expression is always false [-Werror=bool-compare]
>>  #define RETURN_ERROR(StatusCode) (((INTN)(RETURN_STATUS)(StatusCode)) < 
>> 0)
>>^
>> MdePkg/Include/Uefi/UefiBaseType.h:169:35: note: in expansion of macro 
>> 'RETURN_ERROR'
>>  #define EFI_ERROR(A)  RETURN_ERROR(A)
>>^~~~
>> MdePkg/Include/Library/DebugLib.h:341:13: note: in expansion of macro 
>> 'EFI_ERROR'
>>  if (EFI_ERROR (StatusParameter)) {  
>>  \
>>  ^
>> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c:81:3: note: in expansion of 
>> macro 'ASSERT_EFI_ERROR'
>>ASSERT_EFI_ERROR (Address != NULL);
>>^~~~
>> cc1: all warnings being treated as errors
> 
> This is fixed by one of the patches in the series (and I added Gerd's
> Reported-by there). Then I searched the tree for further misuses of
> ASSERT_EFI_ERROR, and fixed what I found.
> 
> Cc: David Wei 
> Cc: Gerd Hoffmann 
> Cc: Jaben Carsey 
> Cc: Jeff Fan 
> Cc: Jiaxin Wu 
> Cc: Kelly Steele 
> Cc: Liming Gao 
> Cc: Michael D Kinney 
> Cc: Shumin Qiu 
> Cc: Siyuan Fu 
> Cc: Tim He 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (6):
>   EdkCompatibilityPkg: fix ASSERT_EFI_ERROR() typos
>   NetworkPkg: fix ASSERT_EFI_ERROR() typos
>   QuarkPlatformPkg: fix ASSERT_EFI_ERROR() typos
>   ShellPkg: don't call functions with side effects in ASSERT_EFI_ERROR()
>   UefiCpuPkg: fix ASSERT_EFI_ERROR() typos
>   Vlv2TbltDevicePkg: fix ASSERT_EFI_ERROR() typos
> 
>  EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/HiiDatabase.c | 
>  2 +-
>  EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/Utility.c | 
>  2 +-
>  NetworkPkg/IpSecDxe/Ikev2/Sa.c | 
>  2 +-
>  QuarkPlatformPkg/Acpi/Dxe/AcpiPlatform/AcpiPlatform.c  | 
>  2 +-
>  QuarkPlatformPkg/Acpi/Dxe/AcpiPlatform/MadtPlatform.c  | 
>  2 +-
>  QuarkPlatformPkg/Library/PlatformHelperLib/PlatformHelperDxe.c | 
>  2 +-
>  QuarkPlatformPkg/Platform/Pei/PlatformInit/MrcWrapper.c| 
>  2 +-
>  ShellPkg/Library/UefiShellLevel1CommandsLib/If.c   | 
> 10 --
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c   | 
>  5 -
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c | 
>  2 +-
>  Vlv2TbltDevicePkg/FvbRuntimeDxe/FvbService.c   | 
>  6 +++---
>  Vlv2TbltDevicePkg/Library/MultiPlatformLib/BoardClkGens/BoardClkGens.c | 
>  8 
>  Vlv2TbltDevicePkg/Library/MultiPlatformLib/PlatformInfoHob.c   | 
>  2 +-
>  13 files changed, 28 insertions(+), 19 deletions(-)
> 

Thanks everyone for the reviews; series committed as
05b39efb669e..2bfd84ed45b2.

Gerd, can you pleae kick Jenkins?

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

Re: [edk2] weird ShellPkg line

[Carsey, Jaben]

Interesting… My guess would be that is used to do something and is the side 
effect of a find and replace operation and it should have been removed, but 
wasn’t…

-Jaben



From: peter.kirme...@ts.fujitsu.com [mailto:peter.kirme...@ts.fujitsu.com]
Sent: Wednesday, June 29, 2016 1:22 AM
To: Michael Zimmermann 
Cc: edk2-devel@lists.01.org; Carsey, Jaben ; Qiu, 
Shumin 
Subject: RE: [edk2] weird ShellPkg line
Importance: High

Hi,

yes that’s right. I found those TRUE-Asserts on various other projects as well 
and I never saw any good reason for this.
My most popular assumptions are that this is some kind of “prepare an assert 
here, so it can be enabled easily”.
Or “let the debug message be part of the binary, so I can find this line of 
code more easily in the debugger without symbols”.

Maybe someone here has any good reason for such constructs. I lived with my 
assumptions about that.

Best Regards,
  Peter

From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
Sent: Wednesday, June 29, 2016 10:12 AM
To: Kirmeier, Peter
Cc: edk2-devel@lists.01.org; Jaben Carsey; 
Shumin Qiu
Subject: Re: [edk2] weird ShellPkg line

Hi,

but doesn't ASSERT only trigger if the condition is not true?
to me it looks like this ASSERT would never show an error.

Thanks,
Michael

On Wed, Jun 29, 2016 at 9:53 AM, 
peter.kirme...@ts.fujitsu.com 
> wrote:
Hi Michael,

for me it looks like an ASSERT(TRUE) when no GUID comparison matched.
It actually produces a more readable debug message rather than "TRUE" which 
allows you to find the actual ASSERT faster.

Best Regards,
  Peter

-Original Message-
From: edk2-devel 
[mailto:edk2-devel-boun...@lists.01.org]
 On Behalf Of Michael Zimmermann
Sent: Wednesday, June 29, 2016 9:42 AM
To: edk2-devel@lists.01.org
Cc: Jaben Carsey; Shumin Qiu
Subject: [edk2] weird ShellPkg line

Hi,

I found this weird line in ShellPkg(which also throws a warning in GCC6).
What does it do?

ASSERT((*HandleType)[HandleIndex] == (*HandleType)[HandleIndex]);

https://github.com/tianocore/edk2/blob/master/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c#L2465


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

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

Re: [edk2] [PATCH 4/6] ShellPkg: don't call functions with side effects in ASSERT_EFI_ERROR()

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, June 30, 2016 4:07 AM
> To: Carsey, Jaben ; Qiu, Shumin
> 
> Cc: edk2-devel-01 
> Subject: Re: [edk2] [PATCH 4/6] ShellPkg: don't call functions with side
> effects in ASSERT_EFI_ERROR()
> Importance: High
> 
> Jaben, Shumin,
> 
> On 06/28/16 15:25, Laszlo Ersek wrote:
> > When ASSERT_EFI_ERROR() is compiled out, dependent on build flags, only
> > the status checking should be removed; the function calls should stay.
> >
> > Cc: Jaben Carsey 
> > Cc: Shumin Qiu 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Laszlo Ersek 
> > ---
> >
> > Notes:
> > build tested
> >
> >  ShellPkg/Library/UefiShellLevel1CommandsLib/If.c | 10 --
> >  ShellPkg/Library/UefiShellLib/UefiShellLib.c |  5 -
> >  2 files changed, 12 insertions(+), 3 deletions(-)
> 
> Can I please get a maintainer review for this patch?
> 
> Thanks
> Laszlo
> 
> > diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> b/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> > index 7abfd8944b92..dc96bffde7d3 100644
> > --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> > +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> > @@ -991,8 +991,11 @@ ShellCommandRunElse (
> >IN EFI_SYSTEM_TABLE  *SystemTable
> >)
> >  {
> > +  EFI_STATUS  Status;
> >SCRIPT_FILE *CurrentScriptFile;
> > -  ASSERT_EFI_ERROR(CommandInit());
> > +
> > +  Status = CommandInit ();
> > +  ASSERT_EFI_ERROR (Status);
> >
> >if (gEfiShellParametersProtocol->Argc > 1) {
> >  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> gShellLevel1HiiHandle, L"if");
> > @@ -1066,8 +1069,11 @@ ShellCommandRunEndIf (
> >IN EFI_SYSTEM_TABLE  *SystemTable
> >)
> >  {
> > +  EFI_STATUS  Status;
> >SCRIPT_FILE *CurrentScriptFile;
> > -  ASSERT_EFI_ERROR(CommandInit());
> > +
> > +  Status = CommandInit ();
> > +  ASSERT_EFI_ERROR (Status);
> >
> >if (gEfiShellParametersProtocol->Argc > 1) {
> >  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> gShellLevel1HiiHandle, L"if");
> > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > index cf89a4ac87ed..35a1a7169c8b 100644
> > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> > @@ -373,6 +373,8 @@ EFIAPI
> >  ShellInitialize (
> >)
> >  {
> > +  EFI_STATUS Status;
> > +
> >//
> >// if auto initialize is not false then skip
> >//
> > @@ -383,7 +385,8 @@ ShellInitialize (
> >//
> >// deinit the current stuff
> >//
> > -  ASSERT_EFI_ERROR(ShellLibDestructor(gImageHandle, gST));
> > +  Status = ShellLibDestructor (gImageHandle, gST);
> > +  ASSERT_EFI_ERROR (Status);
> >
> >//
> >// init the new stuff
> >

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

Re: [edk2] PCIe hotplug into downstream port

[Alex Williamson]

On Thu, 30 Jun 2016 15:07:27 +0200
Laszlo Ersek  wrote:
> - What could be the difference between root ports and downstream ports?
>   (Hotplug into root ports seems to work fine.) Are OSes entitled to
>   allocate any unused address space (MMIO and IO) right when a device is
>   hot-plugged into a root port?

A possible difference is simply the depth of the hierarchy, the
apertures on a root port come directly from the host bridge and there's
no affect to other devices to disable and resize the root port
apertures.  In order to resize a switch downstream port aperture, the
OS would need to touch multiple levels, which could affect peer devices
already in operation.  Does hotplug to a downstream switch port work if
the hot added device is the only endpoint within that sub-hierarchy?  I
wouldn't necessarily be surprised either way, it seems like a
complicated resource runtime reallocation issue that probably isn't
very prevalent on real hardware.  Thanks,

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

Re: [edk2] PCIe hotplug into downstream port

[Laszlo Ersek]

On 06/30/16 16:48, Brian J. Johnson wrote:
> On 06/30/2016 09:35 AM, Laszlo Ersek wrote:
>> On 06/30/16 16:24, Brian J. Johnson wrote:
>>> On 06/30/2016 08:07 AM, Laszlo Ersek wrote:
>>
 - Does PCIe hotplug into downstream ports work with any (phys) firmware
 forked from edk2? Brian, Samer, do you guys have experience with
 this?
>>>
>>> Yes, we support it on our UV server line.  Physically, hotplug is only
>>> possible in expansion chassis designed for that purpose.  (That's not a
>>> problem with virtual machines, of course.)  But we've had to do quite a
>>> bit of work on the host bridge driver to allowing scaling to the sizes
>>> of systems we support (hundreds of sockets and PCIe slots, dozens of TB
>>> of RAM), including reserving resources for hotplug only in the specific
>>> cases where it's possible.
>>
>> Thanks a lot for your answer! Could you perhaps check for me if your
>> platform firmware implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL, highlighted
>> by Andrew?
> 
> 
> Yes, we do.  We have a customized implementation of this protocol which
> provides padding values optimized for the expansion chassis we support.

Awesome. That's what I'm looking at in the PI spec right now.

I'm being led to believe that I won't need to return any
EFI_HPC_LOCATIONs from GetRootHpcList(), and correspondingly I can stub
out InitializeRootHpc() as well. The PI spec allows this:

* As stated above, root HPCs may require firmware initialization.
  PI Architecture firmware must be capable of supporting root HPCs
  that are initialized by hardware and do not require any firmware
  initialization.

[...]

Scan all the devices in the specified bus range and the specified
segment, one bus at a time. If the device is a PCI-to-PCI bridge,
update the bus numbers and program the bus number registers in the
PCI-to-PCI bridge hardware. If the device path of a device matches
that of a root HPC and it is not a PCI-to-CardBus bridge, it must
be initialized by calling
EFI_PCI_HOT_PLUG_INIT_PROTOCOL.InitializeRootHpc() before the bus it
controls can be fully enumerated.

[...]

Root HPC

These HPCs must be initialized by calling InitializeRootHpc()
during the enumeration process. These HPCs will also require
resource padding. The platform code must have a priori
knowledge of these devices and must know how to initialize
them. There may not be any way to access their PCI
configuration space before the PCI enumerator programs all the
upstream bridges and thus enables the path to these devices.

The recursive enumeration works just fine already. On QEMU's virtual
hardware, there doesn't seem to be any need for this kind of
initialization. It's the resource padding bit that I have to figure out.

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

Re: [edk2] PCIe hotplug into downstream port

[Brian J. Johnson]

On 06/30/2016 09:35 AM, Laszlo Ersek wrote:

On 06/30/16 16:24, Brian J. Johnson wrote:

On 06/30/2016 08:07 AM, Laszlo Ersek wrote:



- Does PCIe hotplug into downstream ports work with any (phys) firmware
forked from edk2? Brian, Samer, do you guys have experience with this?


Yes, we support it on our UV server line.  Physically, hotplug is only
possible in expansion chassis designed for that purpose.  (That's not a
problem with virtual machines, of course.)  But we've had to do quite a
bit of work on the host bridge driver to allowing scaling to the sizes
of systems we support (hundreds of sockets and PCIe slots, dozens of TB
of RAM), including reserving resources for hotplug only in the specific
cases where it's possible.


Thanks a lot for your answer! Could you perhaps check for me if your
platform firmware implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL, highlighted
by Andrew?



Yes, we do.  We have a customized implementation of this protocol which 
provides padding values optimized for the expansion chassis we support.

--

Brian J. Johnson



  My statements are my own, are not authorized by SGI, and do not
  necessarily represent SGI’s positions.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] PCIe hotplug into downstream port

[Laszlo Ersek]

On 06/30/16 16:24, Brian J. Johnson wrote:
> On 06/30/2016 08:07 AM, Laszlo Ersek wrote:

>> - Does PCIe hotplug into downstream ports work with any (phys) firmware
>>forked from edk2? Brian, Samer, do you guys have experience with this?
> 
> Yes, we support it on our UV server line.  Physically, hotplug is only
> possible in expansion chassis designed for that purpose.  (That's not a
> problem with virtual machines, of course.)  But we've had to do quite a
> bit of work on the host bridge driver to allowing scaling to the sizes
> of systems we support (hundreds of sockets and PCIe slots, dozens of TB
> of RAM), including reserving resources for hotplug only in the specific
> cases where it's possible.

Thanks a lot for your answer! Could you perhaps check for me if your
platform firmware implements EFI_PCI_HOT_PLUG_INIT_PROTOCOL, highlighted
by Andrew?

Thanks!
Laszlo

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

Re: [edk2] [PATCH 1/6] EdkCompatibilityPkg: fix ASSERT_EFI_ERROR() typos

[Mudusuru, Giri P]

Thanks Laszlo for notifying. Sorry, it was by accident and didn't mean to send 
multiple R-b's :)

Thanks,
-Giri

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, June 30, 2016 3:23 AM
> To: Mudusuru, Giri P 
> Cc: Gao, Liming ; edk2-devel-01  de...@ml01.01.org>
> Subject: Re: [edk2] [PATCH 1/6] EdkCompatibilityPkg: fix
> ASSERT_EFI_ERROR() typos
> 
> Hi Giri,
> 
> On 06/30/16 06:40, Mudusuru, Giri P wrote:
> > Reviewed-by: Giri P Mudusuru 
> 
> Thank you for the reviews -- I did find and pick up your R-b messages
> soon after you sent them.
> 
> There's no need to send repeated R-b's to the same patches. For example,
> this patch has two R-b's from you at the moment:
> 
> http://thread.gmane.org/gmane.comp.bios.edk2.devel/13736/focus=13764
> http://thread.gmane.org/gmane.comp.bios.edk2.devel/13736/focus=13884
> 
> I've been waiting for a maintainer R-b for patch #4 -- that's the one
> piece missing for me to commit the series.
> 
> Thank you
> Laszlo

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

Re: [edk2] PCIe hotplug into downstream port

[Laszlo Ersek]

On 06/30/16 16:22, Andrew Fish wrote:
> 
>> On Jun 30, 2016, at 6:07 AM, Laszlo Ersek  wrote:
>>
>> Hi,
>>
>> does anyone on this list have experience with $SUBJECT, using physical
>> UEFI firmware derived from edk2?
>>
> 
> Laszlo,
> 
> I don't know the edk2 specific answer from memory, but in general as
> a system gets larger it is harder and harder to efficiently over
> allocate resources. So there is usually some mechanism to configure
> the over allocation of resources and identify which ports support hot
> plug.
> 
> Maybe: 
> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/PciHotPlugInit.h

That seems to be exactly what OVMF should implement -- thank you!

Now I just need to understand how it plays together with the PCI bus
driver, and what values we should expose through it :)

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

Re: [edk2] PCIe hotplug into downstream port

[Brian J. Johnson]

On 06/30/2016 08:07 AM, Laszlo Ersek wrote:

Hi,

does anyone on this list have experience with $SUBJECT, using physical
UEFI firmware derived from edk2?

The problem we're seeing with OVMF is the following: PCIe hotplug works
with PCIe root ports, but it doesn't work with PCIe downstream ports.
The error messages reported by both Linux and Windows guests indicate
"lack of resources" for the device being plugged.

In my understanding:

* Each PCIe downstream port qualifies as a separate bus / bridge.

* Bridges need IO and MMIO resources for themselves. When a device is
   behind a bridge at boot, the device's IO and MMIO BARs are carved out
   of the bridge's IO and MMIO apertures. For IO, the standard bridge
   aperture size is 4096 (byte-sized) ports.

* If no device behind a bridge needs a specific type of resource (for
   example, no devices behind a bridge have IO BARs), then that kind of
   aperture is not necessary for the bridge either.

   A special case of this is when there are no devices behind a bridge
   at all. Example: PCIe downstream port empty at boot. In this case,
   the bridge needs no resources at all, at boot.

* Different firmwares seem to follow different policies in the last
   case (-> bridge empty at boot). SeaBIOS for example allocates the
   0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
   allocates no apertures for bridges that have no devices behind them
   at boot.

   This appears to be a trade-off: if the bridge remains empty for the
   entire lifetime of the OS, then any IO ports that have been allocated
   in advance will be in vain (wasted). However, if the aperture is not
   allocated at boot, then hotplug under the bridge may not work --
   resources for the device being hot-plugged cannot be allocated from
   nonexistent bridge apertures.

And that's what we are experiencing with OVMF.


Laszlo,

I believe that's an accurate summary of PCIe behavior.



My questions are:

- Is the above behavior of the edk2 PCI bus driver intentional?


I'm not familiar with the history of the code  But any PCI bridge 
which wants to support hotplug needs resources allocated to it at boot 
time.  As you say, that's a trade-off between wasting resources 
(especially I/O ports) and allowing hotplug.  The authors of the code 
must have opted for avoiding waste vs. allowing hotplug.




- Does PCIe hotplug into downstream ports work with any (phys) firmware
   forked from edk2? Brian, Samer, do you guys have experience with this?


Yes, we support it on our UV server line.  Physically, hotplug is only 
possible in expansion chassis designed for that purpose.  (That's not a 
problem with virtual machines, of course.)  But we've had to do quite a 
bit of work on the host bridge driver to allowing scaling to the sizes 
of systems we support (hundreds of sockets and PCIe slots, dozens of TB 
of RAM), including reserving resources for hotplug only in the specific 
cases where it's possible.




- What could be the difference between root ports and downstream ports?
   (Hotplug into root ports seems to work fine.) Are OSes entitled to
   allocate any unused address space (MMIO and IO) right when a device is
   hot-plugged into a root port?


I'm not sure what the OS rules for this are
--

Brian



   "The national budget must be balanced.  The public debt must be
reduced; the arrogance of the authorities must be moderated and
controlled.  Payments to foreign governments must be reduced, if
the nation doesn't want to go bankrupt.  People must again learn
to work, instead of living on public assistance."
  -- Marcus Tullius Cicero, 55 BC
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 2/3] MdeModulePkg: Add FrameBufferBltLib

[Laszlo Ersek]

On 06/30/16 15:40, Michael Zimmermann wrote:
> Is this the same as OptionRomPkg/Library/FrameBufferBltLib ?

AIUI, it has the same purpose, but it has improvements. For example, the
API for the OptionRomPkg library forces all conforming library
implementations to keep their configration in internal, static data.
This limits the number of parallel configurations (per client module) to
one. The new library moves the configuration into client storage, and
makes it an explicit part of each API, so several configs can coexist
within the same client module.

Thanks
Laszlo

> On Thu, Jun 30, 2016 at 1:23 PM, Laszlo Ersek  > wrote:
> 
> On 06/30/16 07:09, Ruiyu Ni wrote:
> > This library provides interfaces to perform UEFI Graphics
> > Output Protocol Video BLT operations.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ruiyu Ni >
> > Cc: Feng Tian >
> > Cc: Justen Jordan  >
> > Cc: Laszlo Ersek >
> > ---
> >  MdeModulePkg/Include/Library/FrameBufferBltLib.h   |  94 +++
> >  .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 704 
> +
> >  .../FrameBufferBltLib/FrameBufferBltLib.inf|  34 +
> >  MdeModulePkg/MdeModulePkg.dec  |   4 +
> >  MdeModulePkg/MdeModulePkg.dsc  |   1 +
> >  5 files changed, 837 insertions(+)
> >  create mode 100644 MdeModulePkg/Include/Library/FrameBufferBltLib.h
> >  create mode 100644 
> MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
> >  create mode 100644 
> MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
> 
> In
>  I
> made some comments; from them, the following seem to affect this patch:
> 
> (1) use FRAMEBUFFER_CONFIGURE* rather than VOID*
> (3) FrameBufferBltLib.inf should get a new FILE_GUID
> (6) the git history should remain bisectable
> 
> All of these have been addressed in this patch, so:
> 
> Acked-by: Laszlo Ersek >
> 
> (for this patch).
> 
> Jordan requests that this patch be split in two: lib class header and
> lib instance (implementation). I agree that's a good idea: when you do
> it, please add my Acked-by to both of the resultant patches.
> 
> Jordan also requests that you re-add the OvmfPkg patch. In the message
> linked above, I commented on that patch as well -- please consider those
> comments. I would like to see your new version of that patch before
> giving my R-b for it.
> 
> Thanks!
> Laszlo
> 
> > diff --git a/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> b/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > new file mode 100644
> > index 000..c92eb94
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > @@ -0,0 +1,94 @@
> > +/** @file
> > +  Library for performing UEFI GOP Blt operations on a framebuffer
> > +
> > +  Copyright (c) 2009 - 2016, Intel Corporation. All rights
> reserved.
> > +
> > +  This program and the accompanying materials are licensed and
> made available
> > +  under the terms and conditions of the BSD License which
> accompanies this
> > +  distribution. The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  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 __FRAMEBUFFER_BLT_LIB__
> > +#define __FRAMEBUFFER_BLT_LIB__
> > +
> > +#include 
> > +
> > +//
> > +// Opaque structure for the frame buffer configure.
> > +//
> > +typedef struct FRAME_BUFFER_CONFIGURE FRAME_BUFFER_CONFIGURE;
> > +
> > +/**
> > +  Create the configuration for a video frame buffer.
> > +
> > +  The configuration is returned in the caller provided buffer.
> > +
> > +  @param[in] FrameBuffer   Pointer to the start of the frame
> buffer.
> > +  @param[in] FrameBufferInfo   Describes the frame buffer
> characteristics.
> > +  @param[in,out] Configure The created configuration information.
> > +  @param[in,out] ConfigureSize Size of the configuration information.
> > +
> > +  @retval RETURN_SUCCESSThe configuration was
> successful created.
> > +  @retval RETURN_BUFFER_TOO_SMALL   The Configure is to too
> small. The required
> > +   

Re: [edk2] PCIe hotplug into downstream port

[Andrew Fish]

> On Jun 30, 2016, at 6:07 AM, Laszlo Ersek  wrote:
> 
> Hi,
> 
> does anyone on this list have experience with $SUBJECT, using physical
> UEFI firmware derived from edk2?
> 

Laszlo,

I don't know the edk2 specific answer from memory, but in general as a system 
gets larger it is harder and harder to efficiently over allocate resources. So 
there is usually some mechanism to configure the over allocation of resources 
and identify which ports support hot plug.

Maybe: 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/PciHotPlugInit.h

Thanks,

Andrew Fish

> The problem we're seeing with OVMF is the following: PCIe hotplug works
> with PCIe root ports, but it doesn't work with PCIe downstream ports.
> The error messages reported by both Linux and Windows guests indicate
> "lack of resources" for the device being plugged.
> 
> In my understanding:
> 
> * Each PCIe downstream port qualifies as a separate bus / bridge.
> 
> * Bridges need IO and MMIO resources for themselves. When a device is
>  behind a bridge at boot, the device's IO and MMIO BARs are carved out
>  of the bridge's IO and MMIO apertures. For IO, the standard bridge
>  aperture size is 4096 (byte-sized) ports.
> 
> * If no device behind a bridge needs a specific type of resource (for
>  example, no devices behind a bridge have IO BARs), then that kind of
>  aperture is not necessary for the bridge either.
> 
>  A special case of this is when there are no devices behind a bridge
>  at all. Example: PCIe downstream port empty at boot. In this case,
>  the bridge needs no resources at all, at boot.
> 
> * Different firmwares seem to follow different policies in the last
>  case (-> bridge empty at boot). SeaBIOS for example allocates the
>  0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
>  allocates no apertures for bridges that have no devices behind them
>  at boot.
> 
>  This appears to be a trade-off: if the bridge remains empty for the
>  entire lifetime of the OS, then any IO ports that have been allocated
>  in advance will be in vain (wasted). However, if the aperture is not
>  allocated at boot, then hotplug under the bridge may not work --
>  resources for the device being hot-plugged cannot be allocated from
>  nonexistent bridge apertures.
> 
> And that's what we are experiencing with OVMF.
> 
> My questions are:
> 
> - Is the above behavior of the edk2 PCI bus driver intentional?
> 
> - Does PCIe hotplug into downstream ports work with any (phys) firmware
>  forked from edk2? Brian, Samer, do you guys have experience with this?
> 
> - What could be the difference between root ports and downstream ports?
>  (Hotplug into root ports seems to work fine.) Are OSes entitled to
>  allocate any unused address space (MMIO and IO) right when a device is
>  hot-plugged into a root port?
> 
> Thanks!
> Laszlo
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] Memory Profile question.

[Andrew Fish]

> On Jun 30, 2016, at 12:05 AM, Yao, Jiewen  wrote:
> 
> Yes, Andrew. You are right.
> We encounter similar problem, but it seems MSVC _ReturnAddress() does not 
> take parameter.
>  #define RETURN_ADDRESS(L) ((L == 0) ? _ReturnAddress() : (VOID *) 0)
> 

For GCC family you can potentially get more info. 

#define RETURN_ADDRESS(L) __builtin_return_address (L)

> So we enhanced MemoryAllocationLib to support add record from 
> MemoryAllocationLib.
> 

Does that mean the logging gets moved to the library? Or does the library log 
just update the log in the DXE Core?

> The final log is like this:
> 
>  CountSize   RVA  Action
> ==  == == 
> ()
> 0x006B  0x0001D31F <== 0xAE7C (gBS->AllocatePool) 
> (InternalAllocatePool() - 
> c:\home\edk-ii\mdemodulepkg\library\smmmemoryallocationlibprofile\memoryallocationlib.c:567)
> 0x0001  0x0020 <== 0xBCB8 (Lib:AllocatePool) 
> (SmmMemLibConstructor() - 
> c:\home\edk-ii\mdepkg\library\smmmemlib\smmmemlib.c:309)
> 0x0001  0x0070 <== 0x7F2D 
> (Lib:AllocateZeroRuntimePool) (VariableCommonInitialize() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:4055)
> 0x0001  0xC000 <== 0x715A 
> (Lib:AllocateZeroRuntimePool) (InitNonVolatileVariableStore() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:3692)
> 0x0001  0x00010400 <== 0x8121 
> (Lib:AllocateRuntimePool) (VariableCommonInitialize() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:4113)
> 0x0001  0x0404 <== 0x2957 (gBS->AllocatePool) 
> (VariableServiceInitialize() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variablesmm.c:956)
> 0x0007  0x01CE <== 0xC42E 
> (Lib:AllocateZeroRuntimePool) (VarCheckLibVariablePropertySet() - 
> c:\home\edk-ii\mdemodulepkg\library\varchecklib\varchecklib.c:521)
> 0x0001  0x0020 <== 0xC071 
> (Lib:ReallocateRuntimePool) (VarCheckAddTableEntry() - 
> c:\home\edk-ii\mdemodulepkg\library\varchecklib\varchecklib.c:228)
> 0x0001  0x0030 <== 0x2E08 (Lib:AllocateZeroPool) 
> (UpdateVariableInfo() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:196)
> 0x0001  0x0044 <== 0x2E68 (Lib:AllocateZeroPool) 
> (UpdateVariableInfo() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:200)
> 0x002C  0x0840 <== 0x2F98 (Lib:AllocateZeroPool) 
> (UpdateVariableInfo() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:232)
> 0x002C  0x037C <== 0x2FF6 (Lib:AllocateZeroPool) 
> (UpdateVariableInfo() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:236)
> 0x0001  0x000D <== 0x4DC9 
> (Lib:AllocateCopyRuntimePool) (AutoUpdateLangVariable() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:1960)
> 0x0001  0x0012 <== 0x4CC4 
> (Lib:AllocateCopyRuntimePool) (AutoUpdateLangVariable() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:1930)
> 0x0001  0x0012 <== 0x4D28 
> (Lib:AllocateRuntimePool) (AutoUpdateLangVariable() - 
> c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:1940)
> 
> 

Are the address ranges allocated tracked? Is it possible to look up an address 
and figure out where it got allocated? 

Any plans to add logging for stalls? You can dump events with a debugger 
script, so that is not as important. 

Thanks,

Andrew Fish

> 
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Andrew Fish
>> Sent: Thursday, June 30, 2016 2:38 PM
>> To: edk2-devel 
>> Subject: [edk2] Memory Profile question.
>> 
>> I've done some experimentation on the memory logging and if possible it is
>> very useful to have 4 stack frames (non-LTO)  as it is common for the
>> MemoryAllocationLib to to call a sequence of Internal functions, so to find
>> the calling spot in the driver you need 4 entries.
>> For example:
>> FunctionThatAllocatePool()
>> AllocateZeroPool()
>> InternalAllocateZeroPool()
>> InternalAllocatePool()
>> 
>> I see that only RETURN_ADDRESS (0) is logged.
>> 
>> MdeModulePkg/Core/Dxe/Mem/Page.c:1338:CoreUpdateProfile
>> ((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0),
>> MemoryProfileActionAllocatePages, MemoryType, EFI_PAGES_TO_SIZE
>> (NumberOfPages), (VOID *) (UINTN) *Memory);
>> MdeModulePkg/Core/Dxe/Mem/Page.c:1447:CoreUpdateProfile
>> ((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0),
>> 

Re: [edk2] [PATCH v3 2/3] MdeModulePkg: Add FrameBufferBltLib

[Michael Zimmermann]

Is this the same as OptionRomPkg/Library/FrameBufferBltLib ?

Thanks
Michael

On Thu, Jun 30, 2016 at 1:23 PM, Laszlo Ersek  wrote:

> On 06/30/16 07:09, Ruiyu Ni wrote:
> > This library provides interfaces to perform UEFI Graphics
> > Output Protocol Video BLT operations.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ruiyu Ni 
> > Cc: Feng Tian 
> > Cc: Justen Jordan 
> > Cc: Laszlo Ersek 
> > ---
> >  MdeModulePkg/Include/Library/FrameBufferBltLib.h   |  94 +++
> >  .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 704
> +
> >  .../FrameBufferBltLib/FrameBufferBltLib.inf|  34 +
> >  MdeModulePkg/MdeModulePkg.dec  |   4 +
> >  MdeModulePkg/MdeModulePkg.dsc  |   1 +
> >  5 files changed, 837 insertions(+)
> >  create mode 100644 MdeModulePkg/Include/Library/FrameBufferBltLib.h
> >  create mode 100644
> MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
> >  create mode 100644
> MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
>
> In
>  I
> made some comments; from them, the following seem to affect this patch:
>
> (1) use FRAMEBUFFER_CONFIGURE* rather than VOID*
> (3) FrameBufferBltLib.inf should get a new FILE_GUID
> (6) the git history should remain bisectable
>
> All of these have been addressed in this patch, so:
>
> Acked-by: Laszlo Ersek 
>
> (for this patch).
>
> Jordan requests that this patch be split in two: lib class header and
> lib instance (implementation). I agree that's a good idea: when you do
> it, please add my Acked-by to both of the resultant patches.
>
> Jordan also requests that you re-add the OvmfPkg patch. In the message
> linked above, I commented on that patch as well -- please consider those
> comments. I would like to see your new version of that patch before
> giving my R-b for it.
>
> Thanks!
> Laszlo
>
> > diff --git a/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> b/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > new file mode 100644
> > index 000..c92eb94
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> > @@ -0,0 +1,94 @@
> > +/** @file
> > +  Library for performing UEFI GOP Blt operations on a framebuffer
> > +
> > +  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> > +
> > +  This program and the accompanying materials are licensed and made
> available
> > +  under the terms and conditions of the BSD License which accompanies
> this
> > +  distribution. The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php
> > +
> > +  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 __FRAMEBUFFER_BLT_LIB__
> > +#define __FRAMEBUFFER_BLT_LIB__
> > +
> > +#include 
> > +
> > +//
> > +// Opaque structure for the frame buffer configure.
> > +//
> > +typedef struct FRAME_BUFFER_CONFIGURE FRAME_BUFFER_CONFIGURE;
> > +
> > +/**
> > +  Create the configuration for a video frame buffer.
> > +
> > +  The configuration is returned in the caller provided buffer.
> > +
> > +  @param[in] FrameBuffer   Pointer to the start of the frame buffer.
> > +  @param[in] FrameBufferInfo   Describes the frame buffer
> characteristics.
> > +  @param[in,out] Configure The created configuration information.
> > +  @param[in,out] ConfigureSize Size of the configuration information.
> > +
> > +  @retval RETURN_SUCCESSThe configuration was successful
> created.
> > +  @retval RETURN_BUFFER_TOO_SMALL   The Configure is to too small. The
> required
> > +size is returned in ConfigureSize.
> > +  @retval RETURN_UNSUPPORTEDThe requested mode is not supported
> by
> > +this implementaion.
> > +**/
> > +RETURN_STATUS
> > +EFIAPI
> > +FrameBufferBltConfigure (
> > +  IN  VOID  *FrameBuffer,
> > +  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *FrameBufferInfo,
> > +  IN OUT  FRAME_BUFFER_CONFIGURE*Configure,
> > +  IN OUT  UINTN *ConfigureSize
> > +  );
> > +
> > +/**
> > +  Performs a UEFI Graphics Output Protocol Blt operation.
> > +
> > +  @param[in] ConfigurePointer to a configuration which was
> successfully
> > +  created by FrameBufferBltConfigure ().
> > +  @param[in,out] BltBufferThe data to transfer to screen.
> > +  @param[in] BltOperation The operation to perform.
> > +  @param[in] SourceX  The X coordinate of the source for
> BltOperation.
> > +  @param[in] SourceY  The Y coordinate of the source for
> 

[edk2] PCIe hotplug into downstream port

[Laszlo Ersek]

Hi,

does anyone on this list have experience with $SUBJECT, using physical
UEFI firmware derived from edk2?

The problem we're seeing with OVMF is the following: PCIe hotplug works
with PCIe root ports, but it doesn't work with PCIe downstream ports.
The error messages reported by both Linux and Windows guests indicate
"lack of resources" for the device being plugged.

In my understanding:

* Each PCIe downstream port qualifies as a separate bus / bridge.

* Bridges need IO and MMIO resources for themselves. When a device is
  behind a bridge at boot, the device's IO and MMIO BARs are carved out
  of the bridge's IO and MMIO apertures. For IO, the standard bridge
  aperture size is 4096 (byte-sized) ports.

* If no device behind a bridge needs a specific type of resource (for
  example, no devices behind a bridge have IO BARs), then that kind of
  aperture is not necessary for the bridge either.

  A special case of this is when there are no devices behind a bridge
  at all. Example: PCIe downstream port empty at boot. In this case,
  the bridge needs no resources at all, at boot.

* Different firmwares seem to follow different policies in the last
  case (-> bridge empty at boot). SeaBIOS for example allocates the
  0x1000 IO ports nonetheless. The PCI bus driver in edk2 does not: it
  allocates no apertures for bridges that have no devices behind them
  at boot.

  This appears to be a trade-off: if the bridge remains empty for the
  entire lifetime of the OS, then any IO ports that have been allocated
  in advance will be in vain (wasted). However, if the aperture is not
  allocated at boot, then hotplug under the bridge may not work --
  resources for the device being hot-plugged cannot be allocated from
  nonexistent bridge apertures.

And that's what we are experiencing with OVMF.

My questions are:

- Is the above behavior of the edk2 PCI bus driver intentional?

- Does PCIe hotplug into downstream ports work with any (phys) firmware
  forked from edk2? Brian, Samer, do you guys have experience with this?

- What could be the difference between root ports and downstream ports?
  (Hotplug into root ports seems to work fine.) Are OSes entitled to
  allocate any unused address space (MMIO and IO) right when a device is
  hot-plugged into a root port?

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

Re: [edk2] [PATCH v3 2/3] MdeModulePkg: Add FrameBufferBltLib

[Laszlo Ersek]

On 06/30/16 07:09, Ruiyu Ni wrote:
> This library provides interfaces to perform UEFI Graphics
> Output Protocol Video BLT operations.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Feng Tian 
> Cc: Justen Jordan 
> Cc: Laszlo Ersek 
> ---
>  MdeModulePkg/Include/Library/FrameBufferBltLib.h   |  94 +++
>  .../Library/FrameBufferBltLib/FrameBufferBltLib.c  | 704 
> +
>  .../FrameBufferBltLib/FrameBufferBltLib.inf|  34 +
>  MdeModulePkg/MdeModulePkg.dec  |   4 +
>  MdeModulePkg/MdeModulePkg.dsc  |   1 +
>  5 files changed, 837 insertions(+)
>  create mode 100644 MdeModulePkg/Include/Library/FrameBufferBltLib.h
>  create mode 100644 MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.c
>  create mode 100644 
> MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf

In
 I
made some comments; from them, the following seem to affect this patch:

(1) use FRAMEBUFFER_CONFIGURE* rather than VOID*
(3) FrameBufferBltLib.inf should get a new FILE_GUID
(6) the git history should remain bisectable

All of these have been addressed in this patch, so:

Acked-by: Laszlo Ersek 

(for this patch).

Jordan requests that this patch be split in two: lib class header and
lib instance (implementation). I agree that's a good idea: when you do
it, please add my Acked-by to both of the resultant patches.

Jordan also requests that you re-add the OvmfPkg patch. In the message
linked above, I commented on that patch as well -- please consider those
comments. I would like to see your new version of that patch before
giving my R-b for it.

Thanks!
Laszlo

> diff --git a/MdeModulePkg/Include/Library/FrameBufferBltLib.h 
> b/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> new file mode 100644
> index 000..c92eb94
> --- /dev/null
> +++ b/MdeModulePkg/Include/Library/FrameBufferBltLib.h
> @@ -0,0 +1,94 @@
> +/** @file
> +  Library for performing UEFI GOP Blt operations on a framebuffer
> +
> +  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  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 __FRAMEBUFFER_BLT_LIB__
> +#define __FRAMEBUFFER_BLT_LIB__
> +
> +#include 
> +
> +//
> +// Opaque structure for the frame buffer configure.
> +//
> +typedef struct FRAME_BUFFER_CONFIGURE FRAME_BUFFER_CONFIGURE;
> +
> +/**
> +  Create the configuration for a video frame buffer.
> +
> +  The configuration is returned in the caller provided buffer.
> +
> +  @param[in] FrameBuffer   Pointer to the start of the frame buffer.
> +  @param[in] FrameBufferInfo   Describes the frame buffer characteristics.
> +  @param[in,out] Configure The created configuration information.
> +  @param[in,out] ConfigureSize Size of the configuration information.
> +
> +  @retval RETURN_SUCCESSThe configuration was successful created.
> +  @retval RETURN_BUFFER_TOO_SMALL   The Configure is to too small. The 
> required
> +size is returned in ConfigureSize.
> +  @retval RETURN_UNSUPPORTEDThe requested mode is not supported by
> +this implementaion.
> +**/
> +RETURN_STATUS
> +EFIAPI
> +FrameBufferBltConfigure (
> +  IN  VOID  *FrameBuffer,
> +  IN  EFI_GRAPHICS_OUTPUT_MODE_INFORMATION  *FrameBufferInfo,
> +  IN OUT  FRAME_BUFFER_CONFIGURE*Configure,
> +  IN OUT  UINTN *ConfigureSize
> +  );
> +
> +/**
> +  Performs a UEFI Graphics Output Protocol Blt operation.
> +
> +  @param[in] ConfigurePointer to a configuration which was 
> successfully
> +  created by FrameBufferBltConfigure ().
> +  @param[in,out] BltBufferThe data to transfer to screen.
> +  @param[in] BltOperation The operation to perform.
> +  @param[in] SourceX  The X coordinate of the source for 
> BltOperation.
> +  @param[in] SourceY  The Y coordinate of the source for 
> BltOperation.
> +  @param[in] DestinationX The X coordinate of the destination for
> +  BltOperation.
> +  @param[in] DestinationY The Y coordinate of the destination for
> +  BltOperation.
> +  @param[in] WidthThe width of a rectangle in the blt rectangle
> +  in 

Re: [edk2] [PATCH 4/6] ShellPkg: don't call functions with side effects in ASSERT_EFI_ERROR()

[Laszlo Ersek]

Jaben, Shumin,

On 06/28/16 15:25, Laszlo Ersek wrote:
> When ASSERT_EFI_ERROR() is compiled out, dependent on build flags, only
> the status checking should be removed; the function calls should stay.
> 
> Cc: Jaben Carsey 
> Cc: Shumin Qiu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> build tested
> 
>  ShellPkg/Library/UefiShellLevel1CommandsLib/If.c | 10 --
>  ShellPkg/Library/UefiShellLib/UefiShellLib.c |  5 -
>  2 files changed, 12 insertions(+), 3 deletions(-)

Can I please get a maintainer review for this patch?

Thanks
Laszlo

> diff --git a/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c 
> b/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> index 7abfd8944b92..dc96bffde7d3 100644
> --- a/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> +++ b/ShellPkg/Library/UefiShellLevel1CommandsLib/If.c
> @@ -991,8 +991,11 @@ ShellCommandRunElse (
>IN EFI_SYSTEM_TABLE  *SystemTable
>)
>  {
> +  EFI_STATUS  Status;
>SCRIPT_FILE *CurrentScriptFile;
> -  ASSERT_EFI_ERROR(CommandInit());
> +
> +  Status = CommandInit ();
> +  ASSERT_EFI_ERROR (Status);
>  
>if (gEfiShellParametersProtocol->Argc > 1) {
>  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), 
> gShellLevel1HiiHandle, L"if");  
> @@ -1066,8 +1069,11 @@ ShellCommandRunEndIf (
>IN EFI_SYSTEM_TABLE  *SystemTable
>)
>  {
> +  EFI_STATUS  Status;
>SCRIPT_FILE *CurrentScriptFile;
> -  ASSERT_EFI_ERROR(CommandInit());
> +
> +  Status = CommandInit ();
> +  ASSERT_EFI_ERROR (Status);
>  
>if (gEfiShellParametersProtocol->Argc > 1) {
>  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY), 
> gShellLevel1HiiHandle, L"if");  
> diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c 
> b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> index cf89a4ac87ed..35a1a7169c8b 100644
> --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c
> @@ -373,6 +373,8 @@ EFIAPI
>  ShellInitialize (
>)
>  {
> +  EFI_STATUS Status;
> +
>//
>// if auto initialize is not false then skip
>//
> @@ -383,7 +385,8 @@ ShellInitialize (
>//
>// deinit the current stuff
>//
> -  ASSERT_EFI_ERROR(ShellLibDestructor(gImageHandle, gST));
> +  Status = ShellLibDestructor (gImageHandle, gST);
> +  ASSERT_EFI_ERROR (Status);
>  
>//
>// init the new stuff
> 

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

Re: [edk2] [PATCH 1/6] EdkCompatibilityPkg: fix ASSERT_EFI_ERROR() typos

[Laszlo Ersek]

Hi Giri,

On 06/30/16 06:40, Mudusuru, Giri P wrote:
> Reviewed-by: Giri P Mudusuru  

Thank you for the reviews -- I did find and pick up your R-b messages
soon after you sent them.

There's no need to send repeated R-b's to the same patches. For example,
this patch has two R-b's from you at the moment:

http://thread.gmane.org/gmane.comp.bios.edk2.devel/13736/focus=13764
http://thread.gmane.org/gmane.comp.bios.edk2.devel/13736/focus=13884

I've been waiting for a maintainer R-b for patch #4 -- that's the one
piece missing for me to commit the series.

Thank you
Laszlo

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

Re: [edk2] [PATCH v2] MdeModulePkg/MemoryStatusCode: Expose the DXE memory status code table.

[Laszlo Ersek]

On 06/30/16 04:40, Gao, Liming wrote:
> Laszlo:
> I have sent the patch to fix it.

Thanks!

> Besides, I suggest to update Ovmf
> DSC/FDF to use StatusCode Router and Handler from MdeModulePkg
> instead of IntelFrameworkModulePkg.

What's the difference between them?

Actually, what do these drivers do? (I don't know why we include them in
OVMF.)

Thanks!
Laszlo

> 
> Thanks
> Liming
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Gao, Liming
>> Sent: Thursday, June 30, 2016 9:02 AM
>> To: Laszlo Ersek ; Bruce Cran ;
>> Cinnamon Shia ; edk2-de...@ml01.01.org
>> Cc: Tian, Feng ; Zeng, Star 
>> Subject: Re: [edk2] [PATCH v2] MdeModulePkg/MemoryStatusCode: Expose
>> the DXE memory status code table.
>>
>> Yes. I will make one patch to fix it. Thanks for your report.
>>
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, June 30, 2016 1:40 AM
>> To: Bruce Cran ; Cinnamon Shia
>> ; edk2-de...@ml01.01.org
>> Cc: Tian, Feng ; Gao, Liming ;
>> Zeng, Star 
>> Subject: Re: [edk2] [PATCH v2] MdeModulePkg/MemoryStatusCode: Expose
>> the DXE memory status code table.
>>
>> On 06/29/16 16:47, Bruce Cran wrote:
>>> On 6/27/2016 1:25 AM, Cinnamon Shia wrote:
 Let data of DXE memory status code can be used by other modules.
 1. Save the address of DXE memory status code table to
 DxeConfigurationTable.
 2. Save the address of SMM memory status code table to
 SmmConfigurationTable.
 3. Move RUNTIME_MEMORY_STATUSCODE_HEADER to its public header
>> file.
>>>
>>> I'm getting an error building OVMF today, which appears related:
>>>
>>> In file included from
>>>
>> /home/bcran/workspace/edk2/IntelFrameworkModulePkg/Universal/Statu
>> sCode/RuntimeDxe/SerialStatusCodeWorker.c:15:0:
>>>
>>>
>> /home/bcran/workspace/edk2/IntelFrameworkModulePkg/Universal/Statu
>> sCode/RuntimeDxe/StatusCodeRuntimeDxe.h:63:3:
>>> error: conflicting types for 'RUNTIME_MEMORY_STATUSCODE_HEADER'
>>> } RUNTIME_MEMORY_STATUSCODE_HEADER;
>>> ^~~~
>>> In file included from
>>>
>> /home/bcran/workspace/edk2/IntelFrameworkModulePkg/Universal/Statu
>> sCode/RuntimeDxe/StatusCodeRuntimeDxe.h:22:0,
>>>
>>> from
>>>
>> /home/bcran/workspace/edk2/IntelFrameworkModulePkg/Universal/Statu
>> sCode/RuntimeDxe/SerialStatusCodeWorker.c:15:
>>>
>>>
>> /home/bcran/workspace/edk2/MdeModulePkg/Include/Guid/MemoryStat
>> usCodeRecord.h:76:3:
>>> note: previous declaration of 'RUNTIME_MEMORY_STATUSCODE_HEADER'
>> was here
>>> } RUNTIME_MEMORY_STATUSCODE_HEADER;
>>> ^~~~
>>>
>>
>> Yes, the file
>>
>> IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCode
>> RuntimeDxe.h
>>
>> includes
>>
>> MdeModulePkg/Include/Guid/MemoryStatusCodeRecord.h
>>
>> but also defines the RUNTIME_MEMORY_STATUSCODE_HEADER type, which
>> has
>> now become redundant.
>>
>> As far as I can see, it should be fixable by removing the
>> RUNTIME_MEMORY_STATUSCODE_HEADER typedef from
>> "IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCod
>> eRuntimeDxe.h".
>>
>> Thanks
>> Laszlo
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] [PATCH] OvmfPkg/build.sh: update gcc detection

[Laszlo Ersek]

On 06/30/16 07:00, Olaf Hering wrote:
> On Wed, Jun 29, Jordan Justen wrote:
> 
>> Missing Contributed-under. (See OvmfPkg/Contributions.txt)
> 
> Looks like this project tries to avoid simple fixes from third party.

It's not the case. Every project has its contribution rules (I reckon
xen-devel is no exception). It takes some time to set everything up in
order to contribute in accordance with the rules, but that's a one time
cost for every contributor.

If we encouraged drive-by patches without regard to the process, then it
would be a constant cost for long term participants / maintainers.

I do agree with you on two points -- I find these unjustified /
gratuitous burdens:
- the Contributed-under line
- having to subscribe to the mailing list in order to post

The first is alas a legal requirement, and I can't do anything about it.
Luckily, it can be automated at least. See for example

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-07

The second (the subscription requirement) is terrible. No other open
source project I'm aware of follows that requirement. I've raised it
several times, but other subscribers on the list disagree with me.

> I'm done with it.

I'm sorry to hear that. I don't really see why you can't post a v2 of
your patch, with all the remarks addressed, considering you will likely
carry such a patch in your downstream anyway. I encourage you to send a v2.

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

Re: [edk2] [Patch 0/2] Update-PXE-driver-to-follow-edk2-coding-standards

[Wu, Jiaxin]

Series Reviewed-By: Wu Jiaxin 

Best Regards!
Jiaxin

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Fu
> Siyuan
> Sent: Thursday, June 30, 2016 10:08 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch 0/2] Update-PXE-driver-to-follow-edk2-coding-
> standards
> 
> 
> Fu Siyuan (2):
>   MdeModulePkg: Update PXE driver to follow edk2 coding standards.
>   NetworkPkg: Update PXE driver to follow edk2 coding standards.
> 
>  MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.c | 4 ++--
> MdeModulePkg/Universal/Network/UefiPxeBcDxe/PxeBcSupport.h | 4 ++--
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.c | 6 +++---
>  NetworkPkg/UefiPxeBcDxe/PxeBcSupport.h | 4 ++--
>  4 files changed, 9 insertions(+), 9 deletions(-)
> 
> --
> 2.7.4.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [patch] MdeModulePkg/EmmcDxe: Don't expose BlockIo interface for RPMB partition

[Wu, Hao A]

The patch is good to me.

Reviewed-by: Hao Wu 

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Feng
> Tian
> Sent: Wednesday, June 29, 2016 11:18 AM
> To: Wu, Hao A
> Cc: edk2-devel@lists.01.org
> Subject: [edk2] [patch] MdeModulePkg/EmmcDxe: Don't expose BlockIo
> interface for RPMB partition
> 
> This change is to avoid UEFI SCT failure as UEFI SCT has no knowledge
> about how to accessing a EMMC RPMB partition.
> 
> The user needs to access RPMB partition should get access through
> EFI_SD_MMC_PASS_THRU protocol with authentication key & mac.
> 
> Cc: Hao Wu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Feng Tian 
> ---
>  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c | 138 +++---
> 
>  1 file changed, 45 insertions(+), 93 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
> index d3602a5..5040882 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c
> @@ -443,77 +443,60 @@ InstallProtocolOnPartition (
>  //
>  // Install BlkIo/BlkIo2/Ssp for the specified partition
>  //
> -Status = gBS->InstallMultipleProtocolInterfaces (
> ->Handle,
> -,
> -Partition->DevicePath,
> -,
> ->BlockIo,
> -,
> ->BlockIo2,
> -NULL
> -);
> -if (EFI_ERROR (Status)) {
> -  goto Error;
> -}
> -
>  if (Partition->PartitionType != EmmcPartitionRPMB) {
> -  Status = gBS->InstallProtocolInterface (
> +  Status = gBS->InstallMultipleProtocolInterfaces (
>>Handle,
> +  ,
> +  Partition->DevicePath,
> +  ,
> +  >BlockIo,
> +  ,
> +  >BlockIo2,
>,
> -  EFI_NATIVE_INTERFACE,
> -  >EraseBlock
> +  >EraseBlock,
> +  NULL
>);
>if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces (
> -   >Handle,
> -   ,
> -   Partition->DevicePath,
> -   ,
> -   >BlockIo,
> -   ,
> -   >BlockIo2,
> -   NULL
> -   );
>  goto Error;
>}
> -}
> 
> -if (((Partition->PartitionType == EmmcPartitionUserData) ||
> -(Partition->PartitionType == EmmcPartitionBoot1) ||
> -(Partition->PartitionType == EmmcPartitionBoot2)) &&
> -((Device->Csd.Ccc & BIT10) != 0)) {
> -  Status = gBS->InstallProtocolInterface (
> -  >Handle,
> -  ,
> -  EFI_NATIVE_INTERFACE,
> -  >StorageSecurity
> -  );
> -  if (EFI_ERROR (Status)) {
> -gBS->UninstallMultipleProtocolInterfaces (
> -   >Handle,
> -   ,
> -   Partition->DevicePath,
> -   ,
> -   >BlockIo,
> -   ,
> -   >BlockIo2,
> -   ,
> -   >EraseBlock,
> -   NULL
> -   );
> -goto Error;
> +  if (((Partition->PartitionType == EmmcPartitionUserData) ||
> +  (Partition->PartitionType == EmmcPartitionBoot1) ||
> +  (Partition->PartitionType == EmmcPartitionBoot2)) &&
> +  ((Device->Csd.Ccc & BIT10) != 0)) {
> +Status = gBS->InstallProtocolInterface (
> +>Handle,
> +,
> +EFI_NATIVE_INTERFACE,
> +>StorageSecurity
> +);
> +if (EFI_ERROR (Status)) {
> +  gBS->UninstallMultipleProtocolInterfaces (
> + >Handle,
> + ,
> + Partition->DevicePath,
> + ,
> + >BlockIo,
> + ,
> + >BlockIo2,
> + ,
> + >EraseBlock,
> + NULL
> + );
> +  goto Error;
> +}
>}
> +
> +  gBS->OpenProtocol (
> + Device->Private->Controller,
> + ,
> + (VOID **) &(Device->Private->PassThru),
> + Device->Private->DriverBindingHandle,
> + Partition->Handle,
> + EFI_OPEN_PROTOCOL_BY_CHILD_CONTROLLER
> + );
>  }
> 
> -gBS->OpenProtocol (
> -   Device->Private->Controller,
> -   ,
> -   (VOID **) &(Device->Private->PassThru),
> -   Device->Private->DriverBindingHandle,
> -   

[edk2] [Patch] MdeModulePkg: Fix IPv4 stack potential disappeared issue

[Jiaxin Wu]

IP4_CONFIG2_INSTANCE->DataItem is used to save the configuration
data to NV variable. When the policy is changed from static to
DHCP, DnsServers info will be cleaned from DataItem first
(See Ip4Config2SetPolicy), it's correct because DnsServers info
should not be saved to NV variable.
But if there is any DnsServers info received from DHCP message, it
will be reset to DataItem again (See Ip4Config2SetDnsServerWorker),
which may cause the NV variable contain the DnsServers info while
the policy is DHCP (See Ip4Config2WriteConfigData).
Then, while the platform is reset, the issue happened. Because
Ip4Config2DataTypeDnsServer is set under DHCP policy, which is not
allowed by UEFI Spec and error returned.

This patch is used to resolve this potential issue.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 12 +++-
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.h |  1 +
 MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c  |  4 
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
index 028c61d..f91a935 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
@@ -1058,11 +1058,10 @@ Ip4Config2GetIfInfo (
   IN IP4_CONFIG2_INSTANCE *Instance,
   IN OUT UINTN*DataSize,
   IN VOID *Data  OPTIONAL
   )
 {
-
   IP4_SERVICE*IpSb;
   UINTN  Length;
   IP4_CONFIG2_DATA_ITEM  *Item;
   EFI_IP4_CONFIG2_INTERFACE_INFO *IfInfo;
   IP4_ADDR   Address;
@@ -1177,10 +1176,11 @@ Ip4Config2SetPolicy (
 FreePool (DataItem->Data.Ptr);
   }
   DataItem->Data.Ptr = NULL;
   DataItem->DataSize = 0;
   DataItem->Status   = EFI_NOT_FOUND;
+  SET_DATA_ATTRIB (DataItem->Attribute, DATA_ATTRIB_VOLATILE);
   NetMapIterate (>EventMap, Ip4Config2SignalEvent, NULL);
 } else {
   //
   // The policy is changed from dhcp to static. Stop the DHCPv4 process
   // and destroy the DHCPv4 child.
@@ -1457,14 +1457,24 @@ Ip4Config2SetDnsServer (
   IN IP4_CONFIG2_INSTANCE *Instance,
   IN UINTNDataSize,
   IN VOID *Data
   )
 {
+  IP4_CONFIG2_DATA_ITEM *Item;
+
+  Item = NULL;
+
   if (Instance->Policy != Ip4Config2PolicyStatic) {
 return EFI_WRITE_PROTECTED;
   }
 
+  Item = >DataItem[Ip4Config2DataTypeDnsServer];
+
+  if (DATA_ATTRIB_SET (Item->Attribute, DATA_ATTRIB_VOLATILE)) {
+REMOVE_DATA_ATTRIB (Item->Attribute, DATA_ATTRIB_VOLATILE);
+  }
+
   return Ip4Config2SetDnsServerWorker (Instance, DataSize, Data);
 }
 
 /**
   Generate the operational state of the interface this IP4 config2 instance 
manages
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.h 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.h
index b2665bd..b6da11f 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.h
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.h
@@ -25,10 +25,11 @@
 #define DATA_ATTRIB_SIZE_FIXED  0x1
 #define DATA_ATTRIB_VOLATILE0x2
 
 #define DATA_ATTRIB_SET(Attrib, Bits)   (BOOLEAN)((Attrib) & (Bits))
 #define SET_DATA_ATTRIB(Attrib, Bits)   ((Attrib) |= (Bits))
+#define REMOVE_DATA_ATTRIB(Attrib, Bits)((Attrib) &= (~Bits))
 
 typedef struct _IP4_CONFIG2_INSTANCE IP4_CONFIG2_INSTANCE;
 
 #define IP4_CONFIG2_INSTANCE_FROM_PROTOCOL(Proto) \
   CR ((Proto), \
diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c 
b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
index fcd3ccb..20bc21f 100644
--- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
+++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Driver.c
@@ -596,10 +596,14 @@ Ip4DriverBindingStart (
   DataItem->Data.Ptr
   );
   if (EFI_ERROR(Status)) {
 goto UNINSTALL_PROTOCOL;
   }
+  
+  if (Index == Ip4Config2DataTypePolicy && (*(DataItem->Data.Policy) == 
Ip4Config2PolicyDhcp)) {
+break;
+  } 
 }
   }
  
   //
   // Ready to go: start the receiving and timer.
-- 
1.9.5.msysgit.1

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

Re: [edk2] Memory Profile question.

[Yao, Jiewen]

Yes, Andrew. You are right.
We encounter similar problem, but it seems MSVC _ReturnAddress() does not take 
parameter.
  #define RETURN_ADDRESS(L) ((L == 0) ? _ReturnAddress() : (VOID *) 0)

So we enhanced MemoryAllocationLib to support add record from 
MemoryAllocationLib.

The final log is like this:

  CountSize   RVA  Action
==  == == 
()
0x006B  0x0001D31F <== 0xAE7C (gBS->AllocatePool) 
(InternalAllocatePool() - 
c:\home\edk-ii\mdemodulepkg\library\smmmemoryallocationlibprofile\memoryallocationlib.c:567)
0x0001  0x0020 <== 0xBCB8 (Lib:AllocatePool) 
(SmmMemLibConstructor() - 
c:\home\edk-ii\mdepkg\library\smmmemlib\smmmemlib.c:309)
0x0001  0x0070 <== 0x7F2D 
(Lib:AllocateZeroRuntimePool) (VariableCommonInitialize() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:4055)
0x0001  0xC000 <== 0x715A 
(Lib:AllocateZeroRuntimePool) (InitNonVolatileVariableStore() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:3692)
0x0001  0x00010400 <== 0x8121 (Lib:AllocateRuntimePool) 
(VariableCommonInitialize() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:4113)
0x0001  0x0404 <== 0x2957 (gBS->AllocatePool) 
(VariableServiceInitialize() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variablesmm.c:956)
0x0007  0x01CE <== 0xC42E 
(Lib:AllocateZeroRuntimePool) (VarCheckLibVariablePropertySet() - 
c:\home\edk-ii\mdemodulepkg\library\varchecklib\varchecklib.c:521)
0x0001  0x0020 <== 0xC071 
(Lib:ReallocateRuntimePool) (VarCheckAddTableEntry() - 
c:\home\edk-ii\mdemodulepkg\library\varchecklib\varchecklib.c:228)
0x0001  0x0030 <== 0x2E08 (Lib:AllocateZeroPool) 
(UpdateVariableInfo() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:196)
0x0001  0x0044 <== 0x2E68 (Lib:AllocateZeroPool) 
(UpdateVariableInfo() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:200)
0x002C  0x0840 <== 0x2F98 (Lib:AllocateZeroPool) 
(UpdateVariableInfo() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:232)
0x002C  0x037C <== 0x2FF6 (Lib:AllocateZeroPool) 
(UpdateVariableInfo() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:236)
0x0001  0x000D <== 0x4DC9 
(Lib:AllocateCopyRuntimePool) (AutoUpdateLangVariable() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:1960)
0x0001  0x0012 <== 0x4CC4 
(Lib:AllocateCopyRuntimePool) (AutoUpdateLangVariable() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:1930)
0x0001  0x0012 <== 0x4D28 (Lib:AllocateRuntimePool) 
(AutoUpdateLangVariable() - 
c:\home\edk-ii\mdemodulepkg\universal\variable\runtimedxe\variable.c:1940)




> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Thursday, June 30, 2016 2:38 PM
> To: edk2-devel 
> Subject: [edk2] Memory Profile question.
> 
> I've done some experimentation on the memory logging and if possible it is
> very useful to have 4 stack frames (non-LTO)  as it is common for the
> MemoryAllocationLib to to call a sequence of Internal functions, so to find
> the calling spot in the driver you need 4 entries.
> For example:
> FunctionThatAllocatePool()
> AllocateZeroPool()
> InternalAllocateZeroPool()
> InternalAllocatePool()
> 
> I see that only RETURN_ADDRESS (0) is logged.
> 
> MdeModulePkg/Core/Dxe/Mem/Page.c:1338:CoreUpdateProfile
> ((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0),
> MemoryProfileActionAllocatePages, MemoryType, EFI_PAGES_TO_SIZE
> (NumberOfPages), (VOID *) (UINTN) *Memory);
> MdeModulePkg/Core/Dxe/Mem/Page.c:1447:CoreUpdateProfile
> ((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0),
> MemoryProfileActionFreePages, MemoryType, EFI_PAGES_TO_SIZE
> (NumberOfPages), (VOID *) (UINTN) Memory);
> MdeModulePkg/Core/Dxe/Mem/Pool.c:279:CoreUpdateProfile
> ((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0),
> MemoryProfileActionAllocatePool, PoolType, Size, *Buffer);
> MdeModulePkg/Core/Dxe/Mem/Pool.c:508:CoreUpdateProfile
> ((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0),
> MemoryProfileActionFreePool, PoolType, 0, Buffer);
> 
> On platforms that emit frame pointers you can do RETURN_ADDRESS (0),
> RETURN_ADDRESS (1), RETURN_ADDRESS (2), and RETURN_ADDRESS (3)
> and it is very useful to find the location of the allocation. Is it possible 
> to log
> more data on platforms that can support it? Is there some 

[edk2] Memory Profile question.

[Andrew Fish]

I've done some experimentation on the memory logging and if possible it is very 
useful to have 4 stack frames (non-LTO)  as it is common for the 
MemoryAllocationLib to to call a sequence of Internal functions, so to find the 
calling spot in the driver you need 4 entries. 
For example:
FunctionThatAllocatePool()
AllocateZeroPool()
InternalAllocateZeroPool()
InternalAllocatePool()

I see that only RETURN_ADDRESS (0) is logged. 

MdeModulePkg/Core/Dxe/Mem/Page.c:1338:CoreUpdateProfile 
((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0), 
MemoryProfileActionAllocatePages, MemoryType, EFI_PAGES_TO_SIZE 
(NumberOfPages), (VOID *) (UINTN) *Memory);
MdeModulePkg/Core/Dxe/Mem/Page.c:1447:CoreUpdateProfile 
((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0), 
MemoryProfileActionFreePages, MemoryType, EFI_PAGES_TO_SIZE (NumberOfPages), 
(VOID *) (UINTN) Memory);
MdeModulePkg/Core/Dxe/Mem/Pool.c:279:CoreUpdateProfile 
((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0), 
MemoryProfileActionAllocatePool, PoolType, Size, *Buffer);
MdeModulePkg/Core/Dxe/Mem/Pool.c:508:CoreUpdateProfile 
((EFI_PHYSICAL_ADDRESS) (UINTN) RETURN_ADDRESS (0), 
MemoryProfileActionFreePool, PoolType, 0, Buffer);

On platforms that emit frame pointers you can do RETURN_ADDRESS (0), 
RETURN_ADDRESS (1), RETURN_ADDRESS (2), and RETURN_ADDRESS (3) and it is very 
useful to find the location of the allocation. Is it possible to log more data 
on platforms that can support it? Is there some alternate scheme that is planed 
to find the driver code calling the MemoryAllocationLib function?

Thanks,

Andrew Fish


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

Re: [edk2] [Patch] IntelFrameworkModulePkg StatusCode RuntimeDxe: Remove duplicated structure.

[Shia, Cinnamon]

Reviewed-by: Cinnamon Shia 

-Original Message-
From: Liming Gao [mailto:liming@intel.com] 
Sent: Thursday, June 30, 2016 10:38 AM
To: edk2-devel@lists.01.org
Cc: Shia, Cinnamon 
Subject: [Patch] IntelFrameworkModulePkg StatusCode RuntimeDxe: Remove 
duplicated structure.

RUNTIME_MEMORY_STATUSCODE_HEADER has been moved into MdeModulePkg public header 
file Include/Guid/MemoryStatusCodeRecord.h. It should be removed from the 
driver.

Cc: Cinnamon Shia 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 .../Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.h   | 9 -
 1 file changed, 9 deletions(-)

diff --git 
a/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.h
 
b/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.h
index 9d0ed19..ae20f5b 100644
--- 
a/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe.h
+++ b/IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCode
+++ RuntimeDxe.h
@@ -53,15 +53,6 @@ typedef struct {
 } DATAHUB_STATUSCODE_RECORD;
 
 
-//
-// Runtime memory status code worker definition -// -typedef struct {
-  UINT32   RecordIndex;
-  UINT32   NumberOfRecords;
-  UINT32   MaxRecordsNumber;
-} RUNTIME_MEMORY_STATUSCODE_HEADER;
-
 extern RUNTIME_MEMORY_STATUSCODE_HEADER  *mRtMemoryStatusCodeTable;
 
 /**
--
2.8.0.windows.1

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