Re: [edk2] [Patch] BaseTools: correct to use specific arch to generate DSC database

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Yonghong Zhu
>Sent: Thursday, September 07, 2017 9:29 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [edk2] [Patch] BaseTools: correct to use specific arch to generate DSC
>database
>
>Not generic to use 'Common' arch, but use current build arch.
>
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>index fe2c7c1..b617221 100644
>--- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>+++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
>@@ -3107,11 +3107,11 @@ determine whether database file is out of
>date!\n")
>
> ## Summarize all packages in the database
> def GetPackageList(self, Platform, Arch, TargetName, ToolChainTag):
> self.Platform = Platform
> PackageList = []
>-Pa = self.BuildObject[self.Platform, 'COMMON']
>+Pa = self.BuildObject[self.Platform, Arch]
> #
> # Get Package related to Modules
> #
> for Module in Pa.Modules:
> ModuleObj = self.BuildObject[Module, Arch, TargetName,
>ToolChainTag]
>--
>2.6.1.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] WARNING: No source level debug

[Gao, Liming]

Please check the debuglib instance. 

MdeModulePkg.dsc sets 
DebugLib|MdePkg/Library/UefiDebugLibConOut/UefiDebugLibConOut.inf. This library 
instance prints the debug message to Console. 
Nt32.dsc sets 
DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf.
 This library instance prints the debug message to StatusCode. The StatusCode 
service prints the debug message into the different StatusCode handlers. NT32 
has its WinNtOemHookStatusCodeHandler. So, you can see the message log. 

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>david moheban
>Sent: Friday, September 08, 2017 12:03 PM
>To: edk2-devel@lists.01.org
>Subject: [edk2] WARNING: No source level debug
>
>Hi,
>
>Was having trouble getting the debug messages to show in the debug log list
>in the developer window that launches the simulated shell window. I started
>out following the driver building edk2 lab pdf instructions in building one
>of the test drivers. In the lab they have you enter the debug macro such as
>"DEBUG (L"[mydriver] Feature unsupported\r\n") and when the Nt32Pkg is
>compiled by issuing build and thus 'build run' that Debug message shows up
>in the developer window and message logs.
>
>Later I tried my hand at making another driver and compiled it like this:
>'build -p mdemodulepkg\mdemodulepkg.dsc -m mydriver\mydriver.inf'.
>Next I
>tried to issue a 'build run -p mdemodulepkg\mdemodulepkg.dsc' and at first
>failed to run secmain vm until I copied the secmain utility manually to the
>mdemodule build directory and then that worked. The problem I noticed was
>that those 'Debug' messages werent going through with MdeModulePkg and
>were
>just appearing on the shell window as if you typed 'Print'.
>
>At first I thought maybe it was my driver code or the Inf file or header
>file was missing something but running 'build -p nt32pkg' allowed all my
>Debug macro messages to appear on the developer window debug logs and I
>didn't get this 'Warning: No source level debug' error message in the logs.
>
>Also I added to
>[PCDfixedAtBuild]
>gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8040.
>This only allowed debug messages showing up in the shell and not in the
>other developer window logs as it should be. Heres my driver code though
>the fault is not with the code as it works when compiled against Nt32Pkg:
>
>Inf File:
>
>[Defines]
>  INF_VERSION   = 0x00010005
>  BASE_NAME = EmptyDriver
>  FILE_GUID = 785a3ef0-9016-11e7-a9b3-acd1b8bf61e6
>  MODULE_TYPE   = UEFI_DRIVER
>  VERSION_STRING= 1.0
>  ENTRY_POINT   = EmptyDriverDriverEntryPoint
>  UNLOAD_IMAGE  = EmptyDriverUnload
>  UEFI_HII_RESOURCE_SECTION = TRUE
>
>[Packages]
>  MdePkg/MdePkg.dec
>  MdeModulePkg/MdeModulePkg.dec
>  #IntelFrameworkPkg/IntelFrameworkPkg.dec
>  #IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>
>[Packages]
>  MdePkg/MdePkg.dec
>  MdeModulePkg/MdeModulePkg.dec
>
>[Sources]
>  EmptyDriver.h
>  EmptyDriver.c
>  ComponentName.c
>  ComponentName.h
>
>  SimpleTextOutput.c
>  SimpleTextOutput.h
>
>[LibraryClasses]
>  UefiDriverEntryPoint
>  UefiBootServicesTableLib
>  MemoryAllocationLib
>  BaseMemoryLib
>  BaseLib
>  UefiLib
>  DevicePathLib
>  DebugLib
>
>[Protocols]
>  gEfiDriverBindingProtocolGuid
>  gEfiPciIoProtocolGuid
>  gEfiDriverSupportedEfiVersionProtocolGuid
>  gEfiHiiPackageListProtocolGuid
>  gEfiHiiDatabaseProtocolGuid
>  gEfiComponentName2ProtocolGuid
>  gEfiComponentNameProtocolGuid
>  gEfiHiiConfigAccessProtocolGuid
>  gEfiSimpleTextOutProtocolGuid
>  gEfiSerialIoProtocolGuid
>  gEfiSimpleTextInputExProtocolGuid
>
>[Guids]
>
>[Depex]
>AFTER gFvSimpleFileSystemDxeGuid
>
>
>
>Header File:
>
>
>#ifndef __EFI_EMPTY_DRIVER_H__
>#define __EFI_EMPTY_DRIVER_H__
>
>
>#include 
>
>//
>// Libraries
>//
>#include 
>#include 
>#include 
>#include 
>#include 
>#include 
>#include 
>#include 
>
>
>
>//
>// UEFI Driver Model Protocols
>//
>#include 
>#include 
>#include 
>#include 
>#include 
>#include 
>#include 
>#include 
>
>//
>// Consumed Protocols
>//
>#include 
>#include 
>
>//
>// Produced Protocols
>//
>#include 
>#include 
>
>
>
>//
>// Guids
>//
>
>//
>// Driver Version
>//
>#define EMPTY_DRIVER_VERSION  0x
>
>//
>// Protocol instances
>//
>extern EFI_DRIVER_BINDING_PROTOCOL  gEmptyDriverDriverBinding;
>extern EFI_COMPONENT_NAME2_PROTOCOL
>gEmptyDriverComponentName2;
>extern EFI_COMPONENT_NAME_PROTOCOL  gEmptyDriverComponentName;
>
>//
>// Include files with function prototypes
>//
>#include "DriverBinding.h"
>#include "ComponentName.h"
>#include "SimpleTextOutput.h"
>
>#endif
>
>
>driver.C file:
>**/
>
>#include "EmptyDriver.h"
>
>
>static CHAR16* FileMemPath = L"\\file.efi";
>
>EFI_HANDLE MainImageHandle = NULL;
>
>#define SafeFree(p) do { FreePool(p); p = NULL;} while(0)
>
>
>
>
>
>///
>/// Driver Support EFI Version Protocol instance

[edk2] [Patch 0/2] Add CalculateCrc32() API in MdePkg BaseLib

[Liming Gao]

Add CalculateCrc32() as the public API for code sharing.

Liming Gao (2):
  MdePkg BaseLib: Add new API CalculateCrc32()
  MdeModulePkg: Update modules to consume CalculateCrc32()

 MdeModulePkg/Core/RuntimeDxe/Crc32.c   |  74 +-
 MdeModulePkg/Core/RuntimeDxe/Runtime.c |   5 -
 MdeModulePkg/Core/RuntimeDxe/Runtime.h |   9 -
 .../PeiCrc32GuidedSectionExtractLib.c  | 108 +---
 .../PeiCrc32GuidedSectionExtractLib.inf|   3 +-
 MdePkg/Include/Library/BaseLib.h   |  18 ++
 MdePkg/Library/BaseLib/CheckSum.c  | 294 +
 7 files changed, 323 insertions(+), 188 deletions(-)

-- 
2.8.0.windows.1

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

[edk2] [Patch 1/2] MdePkg BaseLib: Add new API CalculateCrc32()

[Liming Gao]

CalculateCrc32() bases on the initialized mCrcTable. When CalculateCrc32()
is used, mCrcTable will take 1KB size in the image. When CalculateCrc32() 
is not used, mCrcTable will not be built in the image, and no size impact.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 MdePkg/Include/Library/BaseLib.h  |  18 +++
 MdePkg/Library/BaseLib/CheckSum.c | 294 ++
 2 files changed, 312 insertions(+)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 40b96b7..de287a7 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -4829,6 +4829,24 @@ CalculateCheckSum64 (
   IN  UINTN Length
   );
 
+/**
+  Computes and returns a 32-bit CRC for a data buffer.
+
+  If Buffer is NULL, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param[in]  Buffer   A pointer to the buffer on which the 32-bit CRC is 
to be computed.
+  @param[in]  Length   The number of bytes in the buffer Data.
+
+  @retval Crc32 The 32-bit CRC was computed for the data buffer.
+
+**/
+UINT32
+EFIAPI
+CalculateCrc32(
+  IN  VOID *Buffer,
+  IN  UINTNLength
+  );
 
 //
 // Base Library CPU Functions
diff --git a/MdePkg/Library/BaseLib/CheckSum.c 
b/MdePkg/Library/BaseLib/CheckSum.c
index 2968829..788af19 100644
--- a/MdePkg/Library/BaseLib/CheckSum.c
+++ b/MdePkg/Library/BaseLib/CheckSum.c
@@ -334,4 +334,298 @@ CalculateCheckSum64 (
   return (UINT64) ((UINT64)(-1) - CheckSum + 1);
 }
 
+GLOBAL_REMOVE_IF_UNREFERENCED CONST UINT32  mCrcTable[256] = {
+  0x,
+  0x77073096,
+  0xEE0E612C,
+  0x990951BA,
+  0x076DC419,
+  0x706AF48F,
+  0xE963A535,
+  0x9E6495A3,
+  0x0EDB8832,
+  0x79DCB8A4,
+  0xE0D5E91E,
+  0x97D2D988,
+  0x09B64C2B,
+  0x7EB17CBD,
+  0xE7B82D07,
+  0x90BF1D91,
+  0x1DB71064,
+  0x6AB020F2,
+  0xF3B97148,
+  0x84BE41DE,
+  0x1ADAD47D,
+  0x6DDDE4EB,
+  0xF4D4B551,
+  0x83D385C7,
+  0x136C9856,
+  0x646BA8C0,
+  0xFD62F97A,
+  0x8A65C9EC,
+  0x14015C4F,
+  0x63066CD9,
+  0xFA0F3D63,
+  0x8D080DF5,
+  0x3B6E20C8,
+  0x4C69105E,
+  0xD56041E4,
+  0xA2677172,
+  0x3C03E4D1,
+  0x4B04D447,
+  0xD20D85FD,
+  0xA50AB56B,
+  0x35B5A8FA,
+  0x42B2986C,
+  0xDBBBC9D6,
+  0xACBCF940,
+  0x32D86CE3,
+  0x45DF5C75,
+  0xDCD60DCF,
+  0xABD13D59,
+  0x26D930AC,
+  0x51DE003A,
+  0xC8D75180,
+  0xBFD06116,
+  0x21B4F4B5,
+  0x56B3C423,
+  0xCFBA9599,
+  0xB8BDA50F,
+  0x2802B89E,
+  0x5F058808,
+  0xC60CD9B2,
+  0xB10BE924,
+  0x2F6F7C87,
+  0x58684C11,
+  0xC1611DAB,
+  0xB6662D3D,
+  0x76DC4190,
+  0x01DB7106,
+  0x98D220BC,
+  0xEFD5102A,
+  0x71B18589,
+  0x06B6B51F,
+  0x9FBFE4A5,
+  0xE8B8D433,
+  0x7807C9A2,
+  0x0F00F934,
+  0x9609A88E,
+  0xE10E9818,
+  0x7F6A0DBB,
+  0x086D3D2D,
+  0x91646C97,
+  0xE6635C01,
+  0x6B6B51F4,
+  0x1C6C6162,
+  0x856530D8,
+  0xF262004E,
+  0x6C0695ED,
+  0x1B01A57B,
+  0x8208F4C1,
+  0xF50FC457,
+  0x65B0D9C6,
+  0x12B7E950,
+  0x8BBEB8EA,
+  0xFCB9887C,
+  0x62DD1DDF,
+  0x15DA2D49,
+  0x8CD37CF3,
+  0xFBD44C65,
+  0x4DB26158,
+  0x3AB551CE,
+  0xA3BC0074,
+  0xD4BB30E2,
+  0x4ADFA541,
+  0x3DD895D7,
+  0xA4D1C46D,
+  0xD3D6F4FB,
+  0x4369E96A,
+  0x346ED9FC,
+  0xAD678846,
+  0xDA60B8D0,
+  0x44042D73,
+  0x33031DE5,
+  0xAA0A4C5F,
+  0xDD0D7CC9,
+  0x5005713C,
+  0x270241AA,
+  0xBE0B1010,
+  0xC90C2086,
+  0x5768B525,
+  0x206F85B3,
+  0xB966D409,
+  0xCE61E49F,
+  0x5EDEF90E,
+  0x29D9C998,
+  0xB0D09822,
+  0xC7D7A8B4,
+  0x59B33D17,
+  0x2EB40D81,
+  0xB7BD5C3B,
+  0xC0BA6CAD,
+  0xEDB88320,
+  0x9ABFB3B6,
+  0x03B6E20C,
+  0x74B1D29A,
+  0xEAD54739,
+  0x9DD277AF,
+  0x04DB2615,
+  0x73DC1683,
+  0xE3630B12,
+  0x94643B84,
+  0x0D6D6A3E,
+  0x7A6A5AA8,
+  0xE40ECF0B,
+  0x9309FF9D,
+  0x0A00AE27,
+  0x7D079EB1,
+  0xF00F9344,
+  0x8708A3D2,
+  0x1E01F268,
+  0x6906C2FE,
+  0xF762575D,
+  0x806567CB,
+  0x196C3671,
+  0x6E6B06E7,
+  0xFED41B76,
+  0x89D32BE0,
+  0x10DA7A5A,
+  0x67DD4ACC,
+  0xF9B9DF6F,
+  0x8EBEEFF9,
+  0x17B7BE43,
+  0x60B08ED5,
+  0xD6D6A3E8,
+  0xA1D1937E,
+  0x38D8C2C4,
+  0x4FDFF252,
+  0xD1BB67F1,
+  0xA6BC5767,
+  0x3FB506DD,
+  0x48B2364B,
+  0xD80D2BDA,
+  0xAF0A1B4C,
+  0x36034AF6,
+  0x41047A60,
+  0xDF60EFC3,
+  0xA867DF55,
+  0x316E8EEF,
+  0x4669BE79,
+  0xCB61B38C,
+  0xBC66831A,
+  0x256FD2A0,
+  0x5268E236,
+  0xCC0C7795,
+  0xBB0B4703,
+  0x220216B9,
+  0x5505262F,
+  0xC5BA3BBE,
+  0xB2BD0B28,
+  0x2BB45A92,
+  0x5CB36A04,
+  0xC2D7FFA7,
+  0xB5D0CF31,
+  0x2CD99E8B,
+  0x5BDEAE1D,
+  0x9B64C2B0,
+  0xEC63F226,
+  0x756AA39C,
+  0x026D930A,
+  0x9C0906A9,
+  0xEB0E363F,
+  0x72076785,
+  0x05005713,
+  0x95BF4A82,
+  0xE2B87A14,
+  0x7BB12BAE,
+  0x0CB61B38,
+  0x92D28E9B,
+  0xE5D5BE0D,
+  0x7CDCEFB7,
+  0x0BDBDF21,
+  0x86D3D2D4,
+  0xF1D4E242,
+  0x68DDB3F8,
+  0x1FDA836E,
+  0x81BE16CD,
+  0xF6B9265B,
+  0x6FB077E1,
+  0x18B74777,
+  0x88085AE6,
+  0xFF0F6A70,

[edk2] [Patch 2/2] MdeModulePkg: Update modules to consume CalculateCrc32()

[Liming Gao]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Core/RuntimeDxe/Crc32.c   |  74 +-
 MdeModulePkg/Core/RuntimeDxe/Runtime.c |   5 -
 MdeModulePkg/Core/RuntimeDxe/Runtime.h |   9 --
 .../PeiCrc32GuidedSectionExtractLib.c  | 108 ++---
 .../PeiCrc32GuidedSectionExtractLib.inf|   3 +-
 5 files changed, 11 insertions(+), 188 deletions(-)

diff --git a/MdeModulePkg/Core/RuntimeDxe/Crc32.c 
b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
index a6fe77f..3e91e08 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Crc32.c
+++ b/MdeModulePkg/Core/RuntimeDxe/Crc32.c
@@ -7,7 +7,7 @@
   EFI Runtime Services Table are converted from physical address to
   virtual addresses.  This requires that the 32-bit CRC be recomputed.
 
-Copyright (c) 2006 - 2008, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, 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
@@ -20,8 +20,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 
 #include 
-
-UINT32  mCrcTable[256];
+#include 
 
 /**
   Calculate CRC32 for target data.
@@ -43,73 +42,6 @@ RuntimeDriverCalculateCrc32 (
   OUT UINT32  *CrcOut
   )
 {
-  UINT32  Crc;
-  UINTN   Index;
-  UINT8   *Ptr;
-
-  if (Data == NULL || DataSize == 0 || CrcOut == NULL) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  Crc = 0x;
-  for (Index = 0, Ptr = Data; Index < DataSize; Index++, Ptr++) {
-Crc = (Crc >> 8) ^ mCrcTable[(UINT8) Crc ^ *Ptr];
-  }
-
-  *CrcOut = Crc ^ 0x;
+  *CrcOut = CalculateCrc32 (Data, DataSize);
   return EFI_SUCCESS;
 }
-
-
-/**
-  This internal function reverses bits for 32bit data.
-
-  @param  Value The data to be reversed.
-
-  @return   Data reversed.
-
-**/
-UINT32
-ReverseBits (
-  UINT32  Value
-  )
-{
-  UINTN   Index;
-  UINT32  NewValue;
-
-  NewValue = 0;
-  for (Index = 0; Index < 32; Index++) {
-if ((Value & (1 << Index)) != 0) {
-  NewValue = NewValue | (1 << (31 - Index));
-}
-  }
-
-  return NewValue;
-}
-
-/**
-  Initialize CRC32 table.
-
-**/
-VOID
-RuntimeDriverInitializeCrc32Table (
-  VOID
-  )
-{
-  UINTN   TableEntry;
-  UINTN   Index;
-  UINT32  Value;
-
-  for (TableEntry = 0; TableEntry < 256; TableEntry++) {
-Value = ReverseBits ((UINT32) TableEntry);
-for (Index = 0; Index < 8; Index++) {
-  if ((Value & 0x8000) != 0) {
-Value = (Value << 1) ^ 0x04c11db7;
-  } else {
-Value = Value << 1;
-  }
-}
-
-mCrcTable[TableEntry] = ReverseBits (Value);
-  }
-}
diff --git a/MdeModulePkg/Core/RuntimeDxe/Runtime.c 
b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
index c61301c..0557457 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Runtime.c
+++ b/MdeModulePkg/Core/RuntimeDxe/Runtime.c
@@ -401,11 +401,6 @@ RuntimeDriverInitialize (
   mMyImageBase = MyLoadedImage->ImageBase;
 
   //
-  // Initialize the table used to compute 32-bit CRCs
-  //
-  RuntimeDriverInitializeCrc32Table ();
-
-  //
   // Fill in the entries of the EFI Boot Services and EFI Runtime Services 
Tables
   //
   gBS->CalculateCrc32   = RuntimeDriverCalculateCrc32;
diff --git a/MdeModulePkg/Core/RuntimeDxe/Runtime.h 
b/MdeModulePkg/Core/RuntimeDxe/Runtime.h
index f2cee9c..506915e 100644
--- a/MdeModulePkg/Core/RuntimeDxe/Runtime.h
+++ b/MdeModulePkg/Core/RuntimeDxe/Runtime.h
@@ -104,15 +104,6 @@ RuntimeDriverSetVirtualAddressMap (
   );
 
 /**
-  Initialize CRC32 table.
-
-**/
-VOID
-RuntimeDriverInitializeCrc32Table (
-  VOID
-  );
-
-/**
   Install Runtime AP. This code includes the EfiRuntimeLib, but it only
   functions at RT in physical mode.
 
diff --git 
a/MdeModulePkg/Library/PeiCrc32GuidedSectionExtractLib/PeiCrc32GuidedSectionExtractLib.c
 
b/MdeModulePkg/Library/PeiCrc32GuidedSectionExtractLib/PeiCrc32GuidedSectionExtractLib.c
index f979bdf..34f1e17 100644
--- 
a/MdeModulePkg/Library/PeiCrc32GuidedSectionExtractLib/PeiCrc32GuidedSectionExtractLib.c
+++ 
b/MdeModulePkg/Library/PeiCrc32GuidedSectionExtractLib/PeiCrc32GuidedSectionExtractLib.c
@@ -3,7 +3,7 @@
   This library registers CRC32 guided section handler 
   to parse CRC32 encapsulation section and extract raw data.
 
-Copyright (c) 2007 - 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 2017, 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
@@ -16,6 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include 
 #include 

[edk2] WARNING: No source level debug

[david moheban]

Hi,

Was having trouble getting the debug messages to show in the debug log list
in the developer window that launches the simulated shell window. I started
out following the driver building edk2 lab pdf instructions in building one
of the test drivers. In the lab they have you enter the debug macro such as
"DEBUG (L"[mydriver] Feature unsupported\r\n") and when the Nt32Pkg is
compiled by issuing build and thus 'build run' that Debug message shows up
in the developer window and message logs.

Later I tried my hand at making another driver and compiled it like this:
'build -p mdemodulepkg\mdemodulepkg.dsc -m mydriver\mydriver.inf'.  Next I
tried to issue a 'build run -p mdemodulepkg\mdemodulepkg.dsc' and at first
failed to run secmain vm until I copied the secmain utility manually to the
mdemodule build directory and then that worked. The problem I noticed was
that those 'Debug' messages werent going through with MdeModulePkg and were
just appearing on the shell window as if you typed 'Print'.

At first I thought maybe it was my driver code or the Inf file or header
file was missing something but running 'build -p nt32pkg' allowed all my
Debug macro messages to appear on the developer window debug logs and I
didn't get this 'Warning: No source level debug' error message in the logs.

Also I added to
[PCDfixedAtBuild] gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8040.
This only allowed debug messages showing up in the shell and not in the
other developer window logs as it should be. Heres my driver code though
the fault is not with the code as it works when compiled against Nt32Pkg:

Inf File:

[Defines]
  INF_VERSION   = 0x00010005
  BASE_NAME = EmptyDriver
  FILE_GUID = 785a3ef0-9016-11e7-a9b3-acd1b8bf61e6
  MODULE_TYPE   = UEFI_DRIVER
  VERSION_STRING= 1.0
  ENTRY_POINT   = EmptyDriverDriverEntryPoint
  UNLOAD_IMAGE  = EmptyDriverUnload
  UEFI_HII_RESOURCE_SECTION = TRUE

[Packages]
  MdePkg/MdePkg.dec
  MdeModulePkg/MdeModulePkg.dec
  #IntelFrameworkPkg/IntelFrameworkPkg.dec
  #IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec

[Packages]
  MdePkg/MdePkg.dec
  MdeModulePkg/MdeModulePkg.dec

[Sources]
  EmptyDriver.h
  EmptyDriver.c
  ComponentName.c
  ComponentName.h

  SimpleTextOutput.c
  SimpleTextOutput.h

[LibraryClasses]
  UefiDriverEntryPoint
  UefiBootServicesTableLib
  MemoryAllocationLib
  BaseMemoryLib
  BaseLib
  UefiLib
  DevicePathLib
  DebugLib

[Protocols]
  gEfiDriverBindingProtocolGuid
  gEfiPciIoProtocolGuid
  gEfiDriverSupportedEfiVersionProtocolGuid
  gEfiHiiPackageListProtocolGuid
  gEfiHiiDatabaseProtocolGuid
  gEfiComponentName2ProtocolGuid
  gEfiComponentNameProtocolGuid
  gEfiHiiConfigAccessProtocolGuid
  gEfiSimpleTextOutProtocolGuid
  gEfiSerialIoProtocolGuid
  gEfiSimpleTextInputExProtocolGuid

[Guids]

[Depex]
AFTER gFvSimpleFileSystemDxeGuid



Header File:


#ifndef __EFI_EMPTY_DRIVER_H__
#define __EFI_EMPTY_DRIVER_H__


#include 

//
// Libraries
//
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 



//
// UEFI Driver Model Protocols
//
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

//
// Consumed Protocols
//
#include 
#include 

//
// Produced Protocols
//
#include 
#include 



//
// Guids
//

//
// Driver Version
//
#define EMPTY_DRIVER_VERSION  0x

//
// Protocol instances
//
extern EFI_DRIVER_BINDING_PROTOCOL  gEmptyDriverDriverBinding;
extern EFI_COMPONENT_NAME2_PROTOCOL  gEmptyDriverComponentName2;
extern EFI_COMPONENT_NAME_PROTOCOL  gEmptyDriverComponentName;

//
// Include files with function prototypes
//
#include "DriverBinding.h"
#include "ComponentName.h"
#include "SimpleTextOutput.h"

#endif


driver.C file:
**/

#include "EmptyDriver.h"


static CHAR16* FileMemPath = L"\\file.efi";

EFI_HANDLE MainImageHandle = NULL;

#define SafeFree(p) do { FreePool(p); p = NULL;} while(0)





///
/// Driver Support EFI Version Protocol instance
///
GLOBAL_REMOVE_IF_UNREFERENCED
EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL
gEmptyDriverDriverSupportedEfiVersion = {
  sizeof (EFI_DRIVER_SUPPORTED_EFI_VERSION_PROTOCOL),
  0x0002001E
};

///
/// Driver Binding Protocol instance
///
EFI_DRIVER_BINDING_PROTOCOL gEmptyDriverDriverBinding = {
  EmptyDriverDriverBindingSupported,
  EmptyDriverDriverBindingStart,
  EmptyDriverDriverBindingStop,
  EMPTY_DRIVER_VERSION,
  NULL,
  NULL
};

/**
  Unloads an image.

  @param  ImageHandle   Handle that identifies the image to be
unloaded.

  @retval EFI_SUCCESS   The image has been unloaded.
  @retval EFI_INVALID_PARAMETER ImageHandle is not a valid image handle.

**/
EFI_STATUS
EFIAPI
EmptyDriverUnload (
  IN EFI_HANDLE  ImageHandle
  )
{
  EFI_STATUS  Status;
  EFI_HANDLE  *HandleBuffer;
  UINTN   HandleCount;
  UINTN   Index;

  Status = EFI_SUCCESS;
  //
  // Retrieve array of all handles in the handle database
  //
  Status = 

Re: [edk2] [PATCH v5 0/6] read-only UDF file system support

[Paulo Alcantara]

Hi,

On 07/09/2017 23:41, Ni, Ruiyu wrote:

Paulo,
Did you mix the usage of "\r\n" and "\n"? because I saw a ^M in your diff.
However, the diff content looks good to me.


No, I didn't. My local repo just lacked the 'git config core.whitespace 
cr-at-eol' setting so 'git diff' showed the CRLFs as whitespace errors. 
Just ignore them :-)


Thanks!
Paulo



Thanks/Ray


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Friday, September 8, 2017 8:56 AM
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek ; Justen, Jordan L
; Andrew Fish ; Kinney,
Michael D ; Gao, Liming
; Zeng, Star ; Dong, Eric
; Doran, Mark ; Ni, Ruiyu
; Wu, Hao A 
Subject: Re: [PATCH v5 0/6] read-only UDF file system support

Ray,

On 07/09/2017 20:13, Paulo Alcantara wrote:

v5:
   - Fixed OVMF IA32 build.
   - Fixed a typo in UdfDriveBindingStop() ("This" -> "SimpleFs") which
 broke retrieval of private fs data from SimpleFs protocol --
 identified by 'reconnect -r' command in UEFI shell.


Follow the diff between v4 and v5 for Mde*Pkg changes (forgot to include it
when resending):

diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 8ad14fe594..2dbcff0be4 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -372,7 +372,7 @@ UdfRead (
 PrivFileData->FileSize,
 >FilePosition,
 Buffer,
-  BufferSize
+  (UINT64 *)(UINTN)BufferSize^M
 );
 } else if (IS_FID_DIRECTORY_FILE (Parent->FileIdentifierDesc)) {
   if (ReadDirInfo->FidOffset == 0 && PrivFileData->FilePosition > 0) { 
diff --
git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
index 9f10c78ca9..49dc7077b7 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
@@ -264,7 +264,7 @@ UdfDriverBindingStop (
   EFI_OPEN_PROTOCOL_GET_PROTOCOL
   );
 if (!EFI_ERROR (Status)) {
-PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (This);
+PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS
(SimpleFs);^M

   //
   // Uninstall child handle


Thanks,
Paulo



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

Re: [edk2] Curious question regarding lack of debug in MdeModulePkg

[Marvin Häuser]

Hey,

Did you make sure you use the proper DebugLib instance in your .dsc (or if you 
build MdeModulePkg directly, adapted its)? You likely want UefiDebugLibConOut.
Also make sure you enabled the necessary debug levels via PCD.

Regards,
Marvin.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> david moheban
> Sent: Friday, September 8, 2017 4:53 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Curious question regarding lack of debug in MdeModulePkg
> 
> Hi,
> 
> Was trying to figure out why I could not output debug messages with a
> simple driver using the Debug macro compiled against the MdeModulePkg
> until I discovered that the same driver code works when compiled against
> Nt32Pkg and all my debug messages get posted in the debug console. Would
> anyone happen to know why?
> 
> Thank you
> ___
> 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] PI 1.6: Issues within the SPI sections.

[Marvin Häuser]

Thank you very much Andrew, hadn't seen it yet. Sorry for spamming here.

Best regards,
Marvin.

From: af...@apple.com [mailto:af...@apple.com]
Sent: Wednesday, September 6, 2017 5:31 PM
To: Marvin H?user 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] PI 1.6: Issues within the SPI sections.

Marvin,

There is a public forum for feedback.

http://www.uefi.org/FWOSForum

Thanks,

Andrew Fish


On Sep 5, 2017, at 8:21 PM, Marvin H?user 
> wrote:

Dear UEFI contributors,

I am not an UEFI contributor and hence cannot submit changes. Could somebody 
please take note of the following?


 1.  I do not see EFI_SPI_TRANSACTION_TYPE defined anywhere. There is a list of 
names for possible values with descriptions (PI 1.6, Vol. 5, page 368), though 
not assigned to numerical values. The size of the type is unknown, as far as I 
can see.
 2.  Some parameter names in the protocol descriptions are flawed (e.g. PI 1.6, 
Vol. 5, page 368: "Read Bytes", "Read Buffer").
 3.  Some protorype return types are flawed (e.g. PI 1.6, Vol. 5, page 367: 
"EFI STATUS" (space instead of underscore); page 364: "FI STATUS" (same as 
before, also "E" missing)).
 4.  Some prototype parameter types are flawed (e.g. PI 1.6, Vol. 5, page 364: 
"EFI- LEGACY- SPI- FLASH- PROTOCOL").
 5.  Some status code descriptions are flawed (e.g. PI 1.6, Vol. 5, page 364: 
"BLocksToProtect" (capital "L")).
 6.  Some formatations are flawed (e.g. PI 1.6, Vol. 5, page 358: multiple 
parameters in one line).
 7.  Some function decorators are flawed (e.g. PI 1.6, Vol. 5, page 357: "In" 
(lower-case "n")).
 8.  Some function parameter names are flawed (e.g. PI 1.6, Vol. 5, page 356: 
"LengthinBytes" (lower-case "i")).
 9.  Some descriptions contain spaces in inappropiate places (e.g. PI 1.5, Vol. 
5, page 349: "EFI_SPI_P ART").
 10. Occasionally incorrect punctuation (e.g. PI 1.6, Vol. 5, page 346: "[...] 
SPI busses, The SPI bus layer [...]" (comma instead of period)).
 11. PI 1.6, Vol. 5, page 349: The description of "SpiPeripheralDriverGuid" 
speaks of a "Driversupported" routine. Is 
EFI_DRIVER_BINDING_PROTOCOL.Supported() meant by this?
 12. PI 1.6, Vol. 5, page 350: The description of "ChipSelectParameter" 
contains spaces, though they are barely noticable when having the PDF opened 
with the latest version of Acrobat DC. Can this be fixed?
 13. UEFI PI 1.6, Vol. 5, 18.2 contains a reference to "EDK2". Is this intended?

Please note that this list is not complete. Maybe the entire section 18 should 
be reviewed again?

Thank you in advance!

Regards,
Marvin.
___
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] Curious question regarding lack of debug in MdeModulePkg

[david moheban]

Hi,

Was trying to figure out why I could not output debug messages with a
simple driver using the Debug macro compiled against the MdeModulePkg until
I discovered that the same driver code works when compiled against Nt32Pkg
and all my debug messages get posted in the debug console. Would anyone
happen to know why?

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

Re: [edk2] Fwd: StartImage with Secure Boot on Self-Signed App

[David F.]

Thanks, looking forward, can the people on the board dealing with the
specification please consider revising EFI_LOADED_IMAGE_PROTOCOL to
include a new "Flags" field and one of the bits allows StartImage to
start the image even if LoadImage reported a EFI_SECURITY_VIOLATION
was reported.  defined bit name could be #define
EFI_LOADED_IMAGE_PROTOCOL_FLAG_SELF_VALIDATED  0x0001ULL.
 This provides a clean interface for applications without having to
hack StartImage() with a potential conflict with future changes to the
internal firmware.


On Thu, Sep 7, 2017 at 7:11 PM, Gary Lin  wrote:
> On Thu, Sep 07, 2017 at 01:00:03PM -0700, David F. wrote:
>> Hello,
>>
>> What is the proper way to allow running another app that is verified
>> with a self-signed certificate?
>>
>> Example, App1 is signed with one that allows secure boot booting (in
>> firmware) and has a public key embedded in the signed code, App2 is
>> verified by App1 and so is allowed to run, but because the key is not
>> in secure boot firmware, StartImage will not run it (although
>> LoadImage did what it needed to do and already reported the security
>> violation potential).   Do we have to roll our own StartImage?  or is
>> something already in place?  I can't rely on changing an internal
>> private structure field to allow StartImage to work since each
>> firmware platform may change the way it all works, looking for the
>> proper method as designed.
>>
> The major linux distros are using shim(*) to verify the bootloaders and
> kernels signed by ourselves, and shim implements its own StartImage.
>
> If your application is going to be deployed to the newer UEFI, instead
> of using the built-in openssl, you can try EFI_PKCS7_VERIFY_PROTOCOL to
> verify the UEFI images. It will make your application much slimmer and
> easier to maintain.
>
> Cheers,
>
> Gary Lin
>
> (*) https://github.com/rhboot/shim
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v5 0/6] read-only UDF file system support

[Ni, Ruiyu]

Paulo,
Did you mix the usage of "\r\n" and "\n"? because I saw a ^M in your diff.
However, the diff content looks good to me.

Thanks/Ray

> -Original Message-
> From: Paulo Alcantara [mailto:pca...@zytor.com]
> Sent: Friday, September 8, 2017 8:56 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Justen, Jordan L
> ; Andrew Fish ; Kinney,
> Michael D ; Gao, Liming
> ; Zeng, Star ; Dong, Eric
> ; Doran, Mark ; Ni, Ruiyu
> ; Wu, Hao A 
> Subject: Re: [PATCH v5 0/6] read-only UDF file system support
> 
> Ray,
> 
> On 07/09/2017 20:13, Paulo Alcantara wrote:
> > v5:
> >   - Fixed OVMF IA32 build.
> >   - Fixed a typo in UdfDriveBindingStop() ("This" -> "SimpleFs") which
> > broke retrieval of private fs data from SimpleFs protocol --
> > identified by 'reconnect -r' command in UEFI shell.
> 
> Follow the diff between v4 and v5 for Mde*Pkg changes (forgot to include it
> when resending):
> 
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> index 8ad14fe594..2dbcff0be4 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> @@ -372,7 +372,7 @@ UdfRead (
> PrivFileData->FileSize,
> >FilePosition,
> Buffer,
> -  BufferSize
> +  (UINT64 *)(UINTN)BufferSize^M
> );
> } else if (IS_FID_DIRECTORY_FILE (Parent->FileIdentifierDesc)) {
>   if (ReadDirInfo->FidOffset == 0 && PrivFileData->FilePosition > 0) { 
> diff --
> git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> index 9f10c78ca9..49dc7077b7 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> @@ -264,7 +264,7 @@ UdfDriverBindingStop (
>   EFI_OPEN_PROTOCOL_GET_PROTOCOL
>   );
> if (!EFI_ERROR (Status)) {
> -PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (This);
> +PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS
> (SimpleFs);^M
> 
>   //
>   // Uninstall child handle
> 
> 
> Thanks,
> Paulo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] MdeModulePkg: Update PiDxeS3BootScriptLib Internal function name

[Liming Gao]

To avoid the function name conflict, update the internal function name
to be the specific one.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
index b865d44..8761d69 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptExecute.c
@@ -57,7 +57,7 @@
 
 **/
 EFI_STATUS
-SmbusExecute (
+InternalSmbusExecute (
   IN UINTNSmbusAddress,
   IN EFI_SMBUS_OPERATION  Operation,
   IN OUT UINTN*Length,
@@ -1053,7 +1053,7 @@ BootScriptExecuteSmbusExecute (
 
   SmBusAddress = (UINTN)SmbusExecuteEntry.SmBusAddress;
   DataSize = (UINTN) SmbusExecuteEntry.DataSize;
-  return SmbusExecute (
+  return InternalSmbusExecute (
SmBusAddress,
(EFI_SMBUS_OPERATION) SmbusExecuteEntry.Operation,
,
-- 
2.8.0.windows.1

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

Re: [edk2] [PATCH v5 6/6] Nt32Pkg: Enable UDF file system support

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: Paulo Alcantara [mailto:pca...@zytor.com]
> Sent: Friday, September 8, 2017 7:13 AM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara ; Ni, Ruiyu ; Wu,
> Hao A ; Laszlo Ersek 
> Subject: [PATCH v5 6/6] Nt32Pkg: Enable UDF file system support
> 
> This patch enables UDF file system support by default.
> 
> Cc: Ruiyu Ni 
> Cc: Hao Wu 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara 
> ---
>  Nt32Pkg/Nt32Pkg.dsc | 1 +
>  Nt32Pkg/Nt32Pkg.fdf | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc index
> cb6b33f3e0..22c8a5595c 100644
> --- a/Nt32Pkg/Nt32Pkg.dsc
> +++ b/Nt32Pkg/Nt32Pkg.dsc
> @@ -444,6 +444,7 @@
>MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
> 
> MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
>FatPkg/EnhancedFatDxe/Fat.inf
> +  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
>MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf ##This driver
> follows UEFI specification definition
>MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf##This driver
> follows UEFI specification definition
> diff --git a/Nt32Pkg/Nt32Pkg.fdf b/Nt32Pkg/Nt32Pkg.fdf index
> e03999b0cb..d104cb7645 100644
> --- a/Nt32Pkg/Nt32Pkg.fdf
> +++ b/Nt32Pkg/Nt32Pkg.fdf
> @@ -295,6 +295,7 @@ INF  EdkShellBinPkg/FullShell/FullShell.inf
>  !endif
> 
>  INF FatPkg/EnhancedFatDxe/Fat.inf
> +INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
> 
>  INF MdeModulePkg/Logo/LogoDxe.inf
>  INF MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
> --
> 2.11.0

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

Re: [edk2] Fwd: StartImage with Secure Boot on Self-Signed App

[Gary Lin]

On Thu, Sep 07, 2017 at 01:00:03PM -0700, David F. wrote:
> Hello,
> 
> What is the proper way to allow running another app that is verified
> with a self-signed certificate?
> 
> Example, App1 is signed with one that allows secure boot booting (in
> firmware) and has a public key embedded in the signed code, App2 is
> verified by App1 and so is allowed to run, but because the key is not
> in secure boot firmware, StartImage will not run it (although
> LoadImage did what it needed to do and already reported the security
> violation potential).   Do we have to roll our own StartImage?  or is
> something already in place?  I can't rely on changing an internal
> private structure field to allow StartImage to work since each
> firmware platform may change the way it all works, looking for the
> proper method as designed.
> 
The major linux distros are using shim(*) to verify the bootloaders and
kernels signed by ourselves, and shim implements its own StartImage.

If your application is going to be deployed to the newer UEFI, instead
of using the built-in openssl, you can try EFI_PKCS7_VERIFY_PROTOCOL to
verify the UEFI images. It will make your application much slimmer and
easier to maintain.

Cheers,

Gary Lin

(*) https://github.com/rhboot/shim
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v5 0/6] read-only UDF file system support

[Paulo Alcantara]

Ray,

On 07/09/2017 20:13, Paulo Alcantara wrote:

v5:
  - Fixed OVMF IA32 build.
  - Fixed a typo in UdfDriveBindingStop() ("This" -> "SimpleFs") which
broke retrieval of private fs data from SimpleFs protocol --
identified by 'reconnect -r' command in UEFI shell.


Follow the diff between v4 and v5 for Mde*Pkg changes (forgot to include 
it when resending):


diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c 
b/MdeModulePkg/Universal/Disk/UdfDxe/File.c

index 8ad14fe594..2dbcff0be4 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -372,7 +372,7 @@ UdfRead (
   PrivFileData->FileSize,
   >FilePosition,
   Buffer,
-  BufferSize
+  (UINT64 *)(UINTN)BufferSize^M
   );
   } else if (IS_FID_DIRECTORY_FILE (Parent->FileIdentifierDesc)) {
 if (ReadDirInfo->FidOffset == 0 && PrivFileData->FilePosition > 0) {
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c 
b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c

index 9f10c78ca9..49dc7077b7 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
@@ -264,7 +264,7 @@ UdfDriverBindingStop (
 EFI_OPEN_PROTOCOL_GET_PROTOCOL
 );
   if (!EFI_ERROR (Status)) {
-PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (This);
+PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (SimpleFs);^M

 //
 // Uninstall child handle


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

[edk2] [PATCH v5 5/6] ArmVirtPkg: Enable UDF file system support

[Paulo Alcantara]

This patch enables UDF file system support by default.

Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
Reviewed-by: Ard Biesheuvel 
Reviewed-by: Laszlo Ersek 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 3 ++-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 3 ++-
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 3 ++-
 ArmVirtPkg/ArmVirtXen.dsc| 3 ++-
 ArmVirtPkg/ArmVirtXen.fdf| 1 +
 5 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 2e6e762249..609697f293 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -300,12 +300,13 @@
   OvmfPkg/VirtioRngDxe/VirtioRng.inf
 
   #
-  # FAT filesystem + GPT/MBR partitioning
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
   #
   MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
   MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
   #
   # Bds
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc 
b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 3194aa3edc..e54eeea41e 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -83,12 +83,13 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 
   #
-  # FAT filesystem + GPT/MBR partitioning
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
   #
   INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
   INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
   INF FatPkg/EnhancedFatDxe/Fat.inf
   INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+  INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
   #
   # Platform Driver
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 69de887277..cb3704727b 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -291,12 +291,13 @@
   OvmfPkg/VirtioRngDxe/VirtioRng.inf
 
   #
-  # FAT filesystem + GPT/MBR partitioning
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
   #
   MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
   MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
   #
   # Bds
diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
index eb37137f27..e9437066ca 100644
--- a/ArmVirtPkg/ArmVirtXen.dsc
+++ b/ArmVirtPkg/ArmVirtXen.dsc
@@ -191,12 +191,13 @@
   ArmVirtPkg/FdtClientDxe/FdtClientDxe.inf
 
   #
-  # FAT filesystem + GPT/MBR partitioning
+  # FAT filesystem + GPT/MBR partitioning + UDF filesystem
   #
   MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
   MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
   #
   # Bds
diff --git a/ArmVirtPkg/ArmVirtXen.fdf b/ArmVirtPkg/ArmVirtXen.fdf
index 67fde73e69..70e76df228 100644
--- a/ArmVirtPkg/ArmVirtXen.fdf
+++ b/ArmVirtPkg/ArmVirtXen.fdf
@@ -163,6 +163,7 @@ READ_LOCK_STATUS   = TRUE
   INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
   INF FatPkg/EnhancedFatDxe/Fat.inf
   INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
+  INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
   #
   # UEFI application (Shell Embedded Boot Loader)
-- 
2.11.0

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

[edk2] [PATCH v5 2/6] MdeModulePkg/PartitionDxe: Add UDF file system support

[Paulo Alcantara]

Scan for UDF file systems on all block devices, as specified by OSTA
Universal Disk Format Specification 2.60, and install a Vendor-Defined
Media Device Path for each file system found.

The Vendor-Defined Media Device Path for the UDF file system is then
checked by UdfDxe to decide whether or not start the driver.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
Reviewed-by: Ruiyu Ni 
---
 .../Universal/Disk/PartitionDxe/Partition.c|   9 +-
 .../Universal/Disk/PartitionDxe/Partition.h|  32 ++-
 .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |   3 +-
 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 318 +
 4 files changed, 355 insertions(+), 7 deletions(-)
 create mode 100644 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c

diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c 
b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
index 5a7d119b43..f6030e0897 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.c
@@ -1,7 +1,7 @@
 /** @file
   Partition driver that produces logical BlockIo devices from a physical
   BlockIo device. The logical BlockIo devices are based on the format
-  of the raw block devices media. Currently "El Torito CD-ROM", Legacy
+  of the raw block devices media. Currently "El Torito CD-ROM", UDF, Legacy
   MBR, and GPT partition schemes are supported.
 
 Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
@@ -45,6 +45,7 @@ PARTITION_DETECT_ROUTINE mPartitionDetectRoutineTable[] = {
   PartitionInstallGptChildHandles,
   PartitionInstallElToritoChildHandles,
   PartitionInstallMbrChildHandles,
+  PartitionInstallUdfChildHandles,
   NULL
 };
 
@@ -305,9 +306,9 @@ PartitionDriverBindingStart (
   if (BlockIo->Media->MediaPresent ||
   (BlockIo->Media->RemovableMedia && !BlockIo->Media->LogicalPartition)) {
 //
-// Try for GPT, then El Torito, and then legacy MBR partition types. If the
-// media supports a given partition type install child handles to represent
-// the partitions described by the media.
+// Try for GPT, then El Torito, then UDF, and then legacy MBR partition
+// types. If the media supports a given partition type install child 
handles
+// to represent the partitions described by the media.
 //
 Routine = [0];
 while (*Routine != NULL) {
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h 
b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h
index f2f6185317..c763c676a9 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Partition.h
@@ -1,7 +1,7 @@
 /** @file
   Partition driver that produces logical BlockIo devices from a physical 
   BlockIo device. The logical BlockIo devices are based on the format
-  of the raw block devices media. Currently "El Torito CD-ROM", Legacy 
+  of the raw block devices media. Currently "El Torito CD-ROM", UDF, Legacy
   MBR, and GPT partition schemes are supported.
 
 Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
@@ -39,7 +39,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include 
 #include 
-
+#include 
 
 //
 // Partition private data
@@ -445,6 +445,34 @@ PartitionInstallMbrChildHandles (
   IN  EFI_DEVICE_PATH_PROTOCOL *DevicePath
   );
 
+/**
+  Install child handles if the Handle supports UDF/ECMA-167 volume format.
+
+  @param[in]  ThisCalling context.
+  @param[in]  Handle  Parent Handle.
+  @param[in]  DiskIo  Parent DiskIo interface.
+  @param[in]  DiskIo2 Parent DiskIo2 interface.
+  @param[in]  BlockIo Parent BlockIo interface.
+  @param[in]  BlockIo2Parent BlockIo2 interface.
+  @param[in]  DevicePath  Parent Device Path
+
+
+  @retval EFI_SUCCESS Child handle(s) was added.
+  @retval EFI_MEDIA_CHANGED   Media changed Detected.
+  @retval other   no child handle was added.
+
+**/
+EFI_STATUS
+PartitionInstallUdfChildHandles (
+  IN  EFI_DRIVER_BINDING_PROTOCOL  *This,
+  IN  EFI_HANDLE   Handle,
+  IN  EFI_DISK_IO_PROTOCOL *DiskIo,
+  IN  EFI_DISK_IO2_PROTOCOL*DiskIo2,
+  IN  EFI_BLOCK_IO_PROTOCOL*BlockIo,
+  IN  EFI_BLOCK_IO2_PROTOCOL   *BlockIo2,
+  IN  EFI_DEVICE_PATH_PROTOCOL *DevicePath
+  );
+
 typedef
 EFI_STATUS
 (*PARTITION_DETECT_ROUTINE) (
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf 
b/MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
index 48212773e8..fb2ea87a9d 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
@@ -4,7 +4,7 @@
 #  This module produces the logical Block I/O device that 

[edk2] [PATCH v5 6/6] Nt32Pkg: Enable UDF file system support

[Paulo Alcantara]

This patch enables UDF file system support by default.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---
 Nt32Pkg/Nt32Pkg.dsc | 1 +
 Nt32Pkg/Nt32Pkg.fdf | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc
index cb6b33f3e0..22c8a5595c 100644
--- a/Nt32Pkg/Nt32Pkg.dsc
+++ b/Nt32Pkg/Nt32Pkg.dsc
@@ -444,6 +444,7 @@
   MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
   MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf ##This driver follows 
UEFI specification definition
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf##This driver follows 
UEFI specification definition
diff --git a/Nt32Pkg/Nt32Pkg.fdf b/Nt32Pkg/Nt32Pkg.fdf
index e03999b0cb..d104cb7645 100644
--- a/Nt32Pkg/Nt32Pkg.fdf
+++ b/Nt32Pkg/Nt32Pkg.fdf
@@ -295,6 +295,7 @@ INF  EdkShellBinPkg/FullShell/FullShell.inf
 !endif
 
 INF FatPkg/EnhancedFatDxe/Fat.inf
+INF MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
 INF MdeModulePkg/Logo/LogoDxe.inf
 INF MdeModulePkg/Universal/LoadFileOnFv2/LoadFileOnFv2.inf
-- 
2.11.0

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

[edk2] [PATCH v5 0/6] read-only UDF file system support

[Paulo Alcantara]

Hi,

This series introduces read-only UDF file system support in EDK2. As
Laszlo (or Red Hat) seemed to be interested in such support, I'm posting
it again after ~3 years.

The idea is not replacing the default FAT file system, nor breaking any
existing file system support, but extending EDK2 with a new file system
that might be useful for some people who are looking for specific file
system features that current FAT doesn't support.

Originally the driver was written to support UDF file systems as
specified by OSTA Universal Disk Format Specification 2.60. However,
some Windows 10 Enterprise ISO (UDF bridge) images that I tested
supported a revision of 1.02 thus I had to rework the driver a little
bit to support such revision as well.

v2:
 - Rework to _partially_ support UDF revisions <2.60.
 - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h
   instead of creating another one (UDF_VOLUME_DESCRIPTOR).
 - Fixed UdfDxe to correctly follow UEFI driver model.
 - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one.
 - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver
   check for specific UDF device path to decide whether or not install
   SimpleFs protocol.
 - Place MdePkg changes in a separate patch.
v3:
 - Install UDF partition child handles with a Vendor-Defined Media
   Device Path.
 - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a
   specific UDF file system GUID when determining to whether or not
   start the driver.
 - Removed leading TAB chars in some source files identified by
   PatchCheck.py tool.
v4:
 - Added missing R-b's.
v5:
 - Fixed OVMF IA32 build.
 - Fixed a typo in UdfDriveBindingStop() ("This" -> "SimpleFs") which
   broke retrieval of private fs data from SimpleFs protocol --
   identified by 'reconnect -r' command in UEFI shell.

Repo:   https://github.com/pcacjr/edk2.git
Branch: udf-fs-v5

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Cc: Andrew Fish 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Mark Doran 
Cc: Ruiyu Ni 
Cc: hao.a...@intel.com
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
---

Paulo Alcantara (6):
  MdePkg: Add UDF volume structure definitions
  MdeModulePkg/PartitionDxe: Add UDF file system support
  MdeModulePkg: Initial UDF/ECMA-167 file system support
  OvmfPkg: Enable UDF file system support
  ArmVirtPkg: Enable UDF file system support
  Nt32Pkg: Enable UDF file system support

 ArmVirtPkg/ArmVirtQemu.dsc |3 +-
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc   |3 +-
 ArmVirtPkg/ArmVirtQemuKernel.dsc   |3 +-
 ArmVirtPkg/ArmVirtXen.dsc  |3 +-
 ArmVirtPkg/ArmVirtXen.fdf  |1 +
 .../Universal/Disk/PartitionDxe/Partition.c|9 +-
 .../Universal/Disk/PartitionDxe/Partition.h|   32 +-
 .../Universal/Disk/PartitionDxe/PartitionDxe.inf   |3 +-
 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c |  318 +++
 MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c |  185 ++
 MdeModulePkg/Universal/Disk/UdfDxe/File.c  |  903 
 MdeModulePkg/Universal/Disk/UdfDxe/FileName.c  |  195 ++
 .../Universal/Disk/UdfDxe/FileSystemOperations.c   | 2447 
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c   |  344 +++
 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h   | 1244 ++
 MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf  |   66 +
 MdePkg/Include/IndustryStandard/Udf.h  |   60 +
 Nt32Pkg/Nt32Pkg.dsc|1 +
 Nt32Pkg/Nt32Pkg.fdf|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 +
 25 files changed, 5816 insertions(+), 11 deletions(-)
 create mode 100644 MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/ComponentName.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/File.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/FileName.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
 create mode 100644 MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 create mode 100644 MdePkg/Include/IndustryStandard/Udf.h

-- 
2.11.0

___
edk2-devel 

[edk2] [PATCH v5 4/6] OvmfPkg: Enable UDF file system support

[Paulo Alcantara]

This patch enables UDF file system support by default.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
Reviewed-by: Laszlo Ersek 
---
 OvmfPkg/OvmfPkgIa32.dsc| 1 +
 OvmfPkg/OvmfPkgIa32.fdf| 1 +
 OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
 OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
 OvmfPkg/OvmfPkgX64.dsc | 1 +
 OvmfPkg/OvmfPkgX64.fdf | 1 +
 6 files changed, 6 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 5a14325f73..92e943d4a0 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -685,6 +685,7 @@
   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
   OvmfPkg/SataControllerDxe/SataControllerDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 5e5ade2a1f..7515224118 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -282,6 +282,7 @@ INF  
MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
 INF  FatPkg/EnhancedFatDxe/Fat.inf
+INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
 !ifndef $(USE_OLD_SHELL)
 INF  ShellPkg/Application/Shell/Shell.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 2f17a70db8..7f9220ccb9 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -694,6 +694,7 @@
   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
   OvmfPkg/SataControllerDxe/SataControllerDxe.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index aa0d8c69f3..f1a2044fb7 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -283,6 +283,7 @@ INF  
MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
 INF  FatPkg/EnhancedFatDxe/Fat.inf
+INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
 !ifndef $(USE_OLD_SHELL)
 INF  ShellPkg/Application/Shell/Shell.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index c0bd5d0ea6..36c60fc19c 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -692,6 +692,7 @@
   MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
   MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
   FatPkg/EnhancedFatDxe/Fat.inf
+  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
   OvmfPkg/SataControllerDxe/SataControllerDxe.inf
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 98a0cf17da..32000a3b93 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -283,6 +283,7 @@ INF  
MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
 INF  
MdeModulePkg/Universal/Acpi/BootGraphicsResourceTableDxe/BootGraphicsResourceTableDxe.inf
 
 INF  FatPkg/EnhancedFatDxe/Fat.inf
+INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
 
 !ifndef $(USE_OLD_SHELL)
 INF  ShellPkg/Application/Shell/Shell.inf
-- 
2.11.0

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

[edk2] [PATCH v5 1/6] MdePkg: Add UDF volume structure definitions

[Paulo Alcantara]

This patch adds basic volume structure definitions necessary to identify
a valid UDF file system on a block device, as specified by OSTA
Universal Disk Format Specification 2.60.

Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara 
Reviewed-by: Ruiyu Ni 
---
 MdePkg/Include/IndustryStandard/Udf.h | 60 +++
 1 file changed, 60 insertions(+)
 create mode 100644 MdePkg/Include/IndustryStandard/Udf.h

diff --git a/MdePkg/Include/IndustryStandard/Udf.h 
b/MdePkg/Include/IndustryStandard/Udf.h
new file mode 100644
index 00..0febb4bcda
--- /dev/null
+++ b/MdePkg/Include/IndustryStandard/Udf.h
@@ -0,0 +1,60 @@
+/** @file
+  OSTA Universal Disk Format (UDF) definitions.
+
+  Copyright (C) 2014-2017 Paulo Alcantara 
+
+  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 __UDF_H__
+#define __UDF_H__
+
+#define UDF_BEA_IDENTIFIER   "BEA01"
+#define UDF_NSR2_IDENTIFIER  "NSR02"
+#define UDF_NSR3_IDENTIFIER  "NSR03"
+#define UDF_TEA_IDENTIFIER   "TEA01"
+
+#define UDF_LOGICAL_SECTOR_SHIFT  11
+#define UDF_LOGICAL_SECTOR_SIZE   ((UINT64)(1ULL << UDF_LOGICAL_SECTOR_SHIFT))
+#define UDF_VRS_START_OFFSET  ((UINT64)(16ULL << UDF_LOGICAL_SECTOR_SHIFT))
+
+#define _GET_TAG_ID(_Pointer) \
+  (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
+
+#define IS_AVDP(_Pointer) \
+  ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
+
+#pragma pack(1)
+
+typedef struct {
+  UINT16  TagIdentifier;
+  UINT16  DescriptorVersion;
+  UINT8   TagChecksum;
+  UINT8   Reserved;
+  UINT16  TagSerialNumber;
+  UINT16  DescriptorCRC;
+  UINT16  DescriptorCRCLength;
+  UINT32  TagLocation;
+} UDF_DESCRIPTOR_TAG;
+
+typedef struct {
+  UINT32  ExtentLength;
+  UINT32  ExtentLocation;
+} UDF_EXTENT_AD;
+
+typedef struct {
+  UDF_DESCRIPTOR_TAG  DescriptorTag;
+  UDF_EXTENT_AD   MainVolumeDescriptorSequenceExtent;
+  UDF_EXTENT_AD   ReserveVolumeDescriptorSequenceExtent;
+  UINT8   Reserved[480];
+} UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
+
+#pragma pack()
+
+#endif
-- 
2.11.0

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

Re: [edk2] [PATCH 0/5] OvmfPkg/VirtioNetDxe: map host address to device address

[Laszlo Ersek]

Hi Brijesh,

On 09/01/17 13:24, Brijesh Singh wrote:
> The patch updates VirtioNetDxe to use IOMMU-like member functions to
> map the system physical address to device address for buffers
> (including vring, device specific request and response pointed by
> vring descriptor, and any furter memory reference by those request and
> response).
> 
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Cc: Tom Lendacky 
> Cc: Laszlo Ersek 
> 
> Repo: https://github.com/codomania/edk2
> Branch: virtio-net-1
> 
> Brijesh Singh (5):
>   OvmfPkg/VirtioNetDxe: map VRING using VirtioRingMap()
>   OvmfPkg/VirtioNetDxe: alloc RxBuf using AllocateSharedPages()
>   OvmfPkg/VirtioNetDxe: dynamically alloc transmit header
>   OvmfPkg/VirtioNetDxe: map virtio-net transmit request buffer
>   OvmfPkg/VirtioNetDxe: negotiate VIRTIO_F_IOMMU_PLATFORM
> 
>  OvmfPkg/VirtioNetDxe/VirtioNet.inf  |   1 +
>  OvmfPkg/VirtioNetDxe/VirtioNet.h|  27 ++-
>  OvmfPkg/VirtioNetDxe/Events.c   |  19 ++
>  OvmfPkg/VirtioNetDxe/SnpGetStatus.c |  30 +++-
>  OvmfPkg/VirtioNetDxe/SnpInitialize.c| 185 
>  OvmfPkg/VirtioNetDxe/SnpSharedHelpers.c | 171 +-
>  OvmfPkg/VirtioNetDxe/SnpShutdown.c  |   2 +
>  OvmfPkg/VirtioNetDxe/SnpTransmit.c  |  37 +++-
>  8 files changed, 427 insertions(+), 45 deletions(-)
> 

just adding a pointer so that my comment is linked under this thread as well:

  [edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at
   ExitBootServices
  https://lists.01.org/pipermail/edk2-devel/2017-September/014304.html

On 09/08/17 00:41, Laszlo Ersek wrote:
> (The conversion of VirtioNetDxe to device addresses is still in
> progress -- Brijesh, when you submit v2 of that, under this approach,
> there is no need to change VirtioNetExitBoot() relative to current
> upstream, and you can use VirtioOperationBusMasterRead to map outgoing
> packets.)

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

[edk2] [PATCH 06/10] OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()

[Laszlo Ersek]

In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.

In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioRngExitBoot(),
originally added in commit 0a568ccbcbd1 ("OvmfPkg/VirtioRngDxe: map host
address to device address", 2017-08-23).

Add a DEBUG message so we can observe the ordering between
VirtioRngExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/VirtioRngDxe/VirtioRng.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/OvmfPkg/VirtioRngDxe/VirtioRng.c b/OvmfPkg/VirtioRngDxe/VirtioRng.c
index 80380bcdf8bf..3c733ea4db66 100644
--- a/OvmfPkg/VirtioRngDxe/VirtioRng.c
+++ b/OvmfPkg/VirtioRngDxe/VirtioRng.c
@@ -433,25 +433,20 @@ VirtioRngExitBoot (
   IN  VOID  *Context
   )
 {
   VIRTIO_RNG_DEV *Dev;
 
+  DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context));
   //
   // Reset the device. This causes the hypervisor to forget about the virtio
   // ring.
   //
   // We allocated said ring in EfiBootServicesData type memory, and code
   // executing after ExitBootServices() is permitted to overwrite it.
   //
   Dev = Context;
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
-
-  //
-  // Unmap the ring buffer so that hypervisor will not be able to get readable
-  // data after device reset.
-  //
-  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
 }
 
 
 //
 // Probe, start and stop functions of this driver, called by the DXE core for
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 02/10] MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM DMA

[Laszlo Ersek]

In AtaAtapiPassThruStop(), if the device has been operating in AHCI mode,
we unmap the DMA buffers and then disable the device (including bus master
DMA). The order of these actions is wrong; we shouldn't unmap DMA buffers
until bus master DMA is turned off. Reverse the steps.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 24 
++--
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index b7fdb8dd4876..a48b295d26aa 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -911,16 +911,26 @@ AtaAtapiPassThruStop (
   //
   // Free allocated resource
   //
   DestroyDeviceInfoList (Instance);
 
+  PciIo = Instance->PciIo;
+
+  //
+  // Disable this ATA host controller.
+  //
+  PciIo->Attributes (
+   PciIo,
+   EfiPciIoAttributeOperationDisable,
+   Instance->EnabledPciAttributes,
+   NULL
+   );
+
   //
   // If the current working mode is AHCI mode, then pre-allocated resource
   // for AHCI initialization should be released.
   //
-  PciIo = Instance->PciIo;
-
   if (Instance->Mode == EfiAtaAhciMode) {
 AhciRegisters = >AhciRegisters;
 PciIo->Unmap (
  PciIo,
  AhciRegisters->MapCommandTable
@@ -948,20 +958,10 @@ AtaAtapiPassThruStop (
  EFI_SIZE_TO_PAGES ((UINTN) AhciRegisters->MaxReceiveFisSize),
  AhciRegisters->AhciRFis
  );
   }
 
-  //
-  // Disable this ATA host controller.
-  //
-  PciIo->Attributes (
-   PciIo,
-   EfiPciIoAttributeOperationDisable,
-   Instance->EnabledPciAttributes,
-   NULL
-   );
-
   //
   // Restore original PCI attributes
   //
   Status = PciIo->Attributes (
 PciIo,
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 10/10] OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()

[Laszlo Ersek]

Register an ExitBootServices() callback that tears down all IOMMU
mappings, without modifying the UEFI memory map.

The trick is that in the ExitBootServices() callback, we don't immediately
do the work; instead we signal another (private) event.

Normally the dispatch order of ExitBootServices() callbacks is unspecified
(within the same task priority level anyway). By queueing another
function, we delay the unmapping until after all PciIo and Virtio drivers
abort -- in their own ExitBootServices() callbacks -- the pending DMA
operations of their respective controllers.

Furthermore, the fact that IoMmuUnmapWorker() rewrites client-owned memory
when it unmaps a Write or CommonBuffer bus master operation, is safe even
in this context. The existence of any given "MapInfo" in "mMapInfos"
implies that the client buffer pointed-to by "MapInfo->CryptedAddress" was
live when ExitBootServices() was entered. And, after entering
ExitBootServices(), nothing must have changed the UEFI memory map, hence
the client buffer at "MapInfo->CryptedAddress" still exists.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 145 
 1 file changed, 145 insertions(+)

diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index 34e1c6ee4a74..6b7df8d8aa3d 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -746,10 +746,111 @@ EDKII_IOMMU_PROTOCOL  mAmdSev = {
   IoMmuUnmap,
   IoMmuAllocateBuffer,
   IoMmuFreeBuffer,
 };
 
+/**
+  Notification function that is queued when gBS->ExitBootServices() signals the
+  EFI_EVENT_GROUP_EXIT_BOOT_SERVICES event group. This function signals another
+  event, received as Context, and returns.
+
+  Signaling an event in this context is safe. The UEFI spec allows
+  gBS->SignalEvent() to return EFI_SUCCESS only; EFI_OUT_OF_RESOURCES is not
+  listed, hence memory is not allocated. The edk2 implementation also does not
+  release memory (and we only have to care about the edk2 implementation
+  because EDKII_IOMMU_PROTOCOL is edk2-specific anyway).
+
+  @param[in] Event  Event whose notification function is being invoked.
+Event is permitted to request the queueing of this
+function at TPL_CALLBACK or TPL_NOTIFY task
+priority level.
+
+  @param[in] EventToSignal  Identifies the EFI_EVENT to signal. EventToSignal
+is permitted to request the queueing of its
+notification function only at TPL_CALLBACK level.
+**/
+STATIC
+VOID
+EFIAPI
+AmdSevExitBoot (
+  IN EFI_EVENT Event,
+  IN VOID  *EventToSignal
+  )
+{
+  //
+  // (1) The NotifyFunctions of all the events in
+  // EFI_EVENT_GROUP_EXIT_BOOT_SERVICES will have been queued before
+  // AmdSevExitBoot() is entered.
+  //
+  // (2) AmdSevExitBoot() is executing minimally at TPL_CALLBACK.
+  //
+  // (3) AmdSevExitBoot() has been queued in unspecified order relative to the
+  // NotifyFunctions of all the other events in
+  // EFI_EVENT_GROUP_EXIT_BOOT_SERVICES whose NotifyTpl is the same as
+  // Event's.
+  //
+  // Consequences:
+  //
+  // - If Event's NotifyTpl is TPL_CALLBACK, then some other NotifyFunctions
+  //   queued at TPL_CALLBACK may be invoked after AmdSevExitBoot() returns.
+  //
+  // - If Event's NotifyTpl is TPL_NOTIFY, then some other NotifyFunctions
+  //   queued at TPL_NOTIFY may be invoked after AmdSevExitBoot() returns; plus
+  //   *all* NotifyFunctions queued at TPL_CALLBACK will be invoked strictly
+  //   after all NotifyFunctions queued at TPL_NOTIFY, including
+  //   AmdSevExitBoot(), have been invoked.
+  //
+  // - By signaling EventToSignal here, whose NotifyTpl is TPL_CALLBACK, we
+  //   queue EventToSignal's NotifyFunction after the NotifyFunctions of *all*
+  //   events in EFI_EVENT_GROUP_EXIT_BOOT_SERVICES.
+  //
+  DEBUG ((DEBUG_VERBOSE, "%a\n", __FUNCTION__));
+  gBS->SignalEvent (EventToSignal);
+}
+
+/**
+  Notification function that is queued after the notification functions of all
+  events in the EFI_EVENT_GROUP_EXIT_BOOT_SERVICES event group. The same memory
+  map restrictions apply.
+
+  This function unmaps all currently existing IOMMU mappings.
+
+  @param[in] EventEvent whose notification function is being invoked. Event
+  is permitted to request the queueing of this function
+  only at TPL_CALLBACK task priority level.
+
+  @param[in] Context  Ignored.
+**/
+STATIC
+VOID
+EFIAPI
+AmdSevUnmapAllMappings (
+  IN EFI_EVENT Event,
+  IN VOID  *Context
+  )
+{
+  LIST_ENTRY *Node;
+  LIST_ENTRY *NextNode;
+  MAP_INFO   *MapInfo;
+
+  DEBUG 

[edk2] [PATCH 05/10] OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at ExitBootServices

[Laszlo Ersek]

In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.

In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() calls from VirtioGpuExitBoot(),
originally added in commit 9bc5026c19a5 ("OvmfPkg/VirtioGpuDxe: map VRING
for bus master common buffer operation", 2017-08-26) and commit
f10ae923665f ("OvmfPkg/VirtioGpuDxe: map backing store to bus master
device address", 2017-08-26).

Add a DEBUG message so we can observe the ordering between
VirtioGpuExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/VirtioGpuDxe/Commands.c | 23 +---
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/OvmfPkg/VirtioGpuDxe/Commands.c b/OvmfPkg/VirtioGpuDxe/Commands.c
index 6e70b1c33f65..6ce21976c918 100644
--- a/OvmfPkg/VirtioGpuDxe/Commands.c
+++ b/OvmfPkg/VirtioGpuDxe/Commands.c
@@ -351,34 +351,13 @@ VirtioGpuExitBoot (
   IN VOID  *Context
   )
 {
   VGPU_DEV *VgpuDev;
 
+  DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context));
   VgpuDev = Context;
   VgpuDev->VirtIo->SetDeviceStatus (VgpuDev->VirtIo, 0);
-
-  //
-  // If VirtioGpuDriverBindingStart() and VirtioGpuDriverBindingStop() have
-  // been called thus far in such a sequence that right now our (sole) child
-  // handle exists -- with the GOP on it standing for head (scanout) #0 --,
-  // then we have to unmap the current video mode's backing store.
-  //
-  if (VgpuDev->Child != NULL) {
-//
-// The current video mode is guaranteed to have a valid and mapped backing
-// store, due to the first Gop.SetMode() call, made internally in
-// InitVgpuGop().
-//
-ASSERT (VgpuDev->Child->BackingStore != NULL);
-
-VgpuDev->VirtIo->UnmapSharedBuffer (
-   VgpuDev->VirtIo,
-   VgpuDev->Child->BackingStoreMap
-   );
-  }
-
-  VgpuDev->VirtIo->UnmapSharedBuffer (VgpuDev->VirtIo, VgpuDev->RingMap);
 }
 
 /**
   Internal utility function that sends a request to the VirtIo GPU device
   model, awaits the answer from the host, and returns a status.
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 07/10] OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()

[Laszlo Ersek]

In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.

In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioScsiExitBoot(),
originally added in commit fc2168feb248 ("OvmfPkg/VirtioScsiDxe: map VRING
using VirtioRingMap()", 2017-08-31).

Add a DEBUG message so we can observe the ordering between
VirtioScsiExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c 
b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
index 7b8c3d22c8de..1a68f062106c 100644
--- a/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
+++ b/OvmfPkg/VirtioScsiDxe/VirtioScsi.c
@@ -1221,25 +1221,20 @@ VirtioScsiExitBoot (
   IN  VOID  *Context
   )
 {
   VSCSI_DEV *Dev;
 
+  DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context));
   //
   // Reset the device. This causes the hypervisor to forget about the virtio
   // ring.
   //
   // We allocated said ring in EfiBootServicesData type memory, and code
   // executing after ExitBootServices() is permitted to overwrite it.
   //
   Dev = Context;
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
-
-  //
-  // Unmap the ring buffer so that hypervisor will not be able to get
-  // readable data after device reset.
-  //
-  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
 }
 
 
 //
 // Probe, start and stop functions of this driver, called by the DXE core for
-- 
2.14.1.3.gb7cf6e02401b


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

[edk2] [PATCH 00/10] MdeModulePkg, OvmfPkg: unmap DMA buffers at ExitBootServices

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: iommu_exit_boot

This series is the result of the discussion under

  [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common
 buffers at ExitBootServices()
  https://lists.01.org/pipermail/edk2-devel/2017-September/014099.html

At ExitBootServices(), PCI and VirtIo drivers should only care about
aborting pending DMA on the devices. Cleaning up PciIo mappings (which
ultimately boil down to IOMMU mappings) for those aborted DMA operations
should be the job of the IOMMU driver.

Patches 01 through 03 clean up the AtaAtapiPassThru driver in
MdeModulePkg a little bit, because at present, (a) it unmaps the buffers
and disables BMDMA in the wrong order in its DriverBindingStop()
function, (b) it doesn't abort pending DMA at ExitBootServices().

This subset can be treated separately from the rest of the series, but I
thought they belonged loosely together (given that AtaAtapiPassThru is
used on QEMU's Q35 machine type).

Patches 04 through 07 remove VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer()
calls from the VirtIo drivers' ExitBootServices() handlers.

(The conversion of VirtioNetDxe to device addresses is still in progress
-- Brijesh, when you submit v2 of that, under this approach, there is no
need to change VirtioNetExitBoot() relative to current upstream, and you
can use VirtioOperationBusMasterRead to map outgoing packets.)

Patches 08 through 10 make OvmfPkg/IoMmuDxe track all mappings, and
unmap all mappings (Read, Write, CommonBuffer) that are in effect when
ExitBootServices() is called. It is ensured that PCI and VirtIo drivers
abort pending DMA first, and IoMmuDxe clean up the mappings last.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Star Zeng 

Thanks
Laszlo

Laszlo Ersek (10):
  MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes
  MdeModulePkg/AtaAtapiPassThru: unmap DMA buffers after disabling BM
DMA
  MdeModulePkg/AtaAtapiPassThru: disable the device at
ExitBootServices()
  OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/VirtioGpuDxe: don't unmap VRING & BackingStore at
ExitBootServices
  OvmfPkg/VirtioRngDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/VirtioScsiDxe: don't unmap VRING at ExitBootServices()
  OvmfPkg/IoMmuDxe: track all mappings
  OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()
  OvmfPkg/IoMmuDxe: unmap all IOMMU mappings at ExitBootServices()

 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 103 +---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |   7 +
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c   | 246 
+---
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c |   7 +-
 OvmfPkg/VirtioGpuDxe/Commands.c  |  23 +-
 OvmfPkg/VirtioRngDxe/VirtioRng.c |   7 +-
 OvmfPkg/VirtioScsiDxe/VirtioScsi.c   |   7 +-
 7 files changed, 299 insertions(+), 101 deletions(-)

-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH 08/10] OvmfPkg/IoMmuDxe: track all mappings

[Laszlo Ersek]

The "mRecycledMapInfos" list implements an internal pool of unused
MAP_INFO structures between the IoMmuUnmap() and IoMmuMap() functions. The
original goal was to allow IoMmuUnmap() to tear down CommonBuffer mappings
without releasing any memory: IoMmuUnmap() would recycle the MAP_INFO
structure to the list, and IoMmuMap() would always check the list first,
before allocating a brand new MAP_INFO structure.

In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.

For this, rename and repurpose the list to track all live mappings.

This means that IoMmuUnmap() will always release a MAP_INFO structure
(even when cleaning up a CommonBuffer operation). That's fine (for now),
because device drivers are no longer expected to call Unmap() in their
ExitBootServices() notification functions.

In theory, we could also move the allocation and freeing of the stash
buffer from IoMmuAllocateBuffer() and IoMmuFreeBuffer(), respectively, to
IoMmuMap() and IoMmuUnmap(). However, this would require allocating and
freeing a stash buffer in *both* IoMmuMap() and IoMmuUnmap(), as
IoMmuMap() performs in-place decryption for CommonBuffer operations, and
IoMmuUnmap() performs in-place encryption for the same.

By keeping the stash buffer allocation as-is, not only do we keep the code
almost fully undisturbed, but

- we also continue to guarantee that IoMmuUnmap() succeeds: allocating a
  stash buffer in IoMmuUnmap(), for in-place encryption after a
  CommonBuffer operation, could fail;

- we also keep IoMmuUnmap() largely reusable for ExitBootServices()
  callback context: allocating a stash buffer in IoMmuUnmap() would simply
  be forbidden in that context.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 49 +++-
 1 file changed, 18 insertions(+), 31 deletions(-)

diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index bc57de5b572b..c86e73498555 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -31,18 +31,15 @@ typedef struct {
   EFI_PHYSICAL_ADDRESS  CryptedAddress;
   EFI_PHYSICAL_ADDRESS  PlainTextAddress;
 } MAP_INFO;
 
 //
-// List of MAP_INFO structures recycled by Unmap().
+// List of the MAP_INFO structures that have been set up by IoMmuMap() and not
+// yet torn down by IoMmuUnmap(). The list represents the full set of mappings
+// currently in effect.
 //
-// Recycled MAP_INFO structures are equally good for future recycling and
-// freeing.
-//
-STATIC LIST_ENTRY mRecycledMapInfos = INITIALIZE_LIST_HEAD_VARIABLE (
-mRecycledMapInfos
-);
+STATIC LIST_ENTRY mMapInfos = INITIALIZE_LIST_HEAD_VARIABLE (mMapInfos);
 
 #define COMMON_BUFFER_SIG SIGNATURE_64 ('C', 'M', 'N', 'B', 'U', 'F', 'F', 'R')
 
 //
 // ASCII names for EDKII_IOMMU_OPERATION constants, for debug logging.
@@ -121,11 +118,10 @@ IoMmuMap (
   OUTEFI_PHYSICAL_ADDRESS   *DeviceAddress,
   OUTVOID   **Mapping
   )
 {
   EFI_STATUSStatus;
-  LIST_ENTRY*RecycledMapInfo;
   MAP_INFO  *MapInfo;
   EFI_ALLOCATE_TYPE AllocateType;
   COMMON_BUFFER_HEADER  *CommonBufferHeader;
   VOID  *DecryptionSource;
 
@@ -148,23 +144,14 @@ IoMmuMap (
 
   //
   // Allocate a MAP_INFO structure to remember the mapping when Unmap() is
   // called later.
   //
-  RecycledMapInfo = GetFirstNode ();
-  if (RecycledMapInfo == ) {
-//
-// No recycled MAP_INFO structure, allocate a new one.
-//
-MapInfo = AllocatePool (sizeof (MAP_INFO));
-if (MapInfo == NULL) {
-  Status = EFI_OUT_OF_RESOURCES;
-  goto Failed;
-}
-  } else {
-MapInfo = CR (RecycledMapInfo, MAP_INFO, Link, MAP_INFO_SIG);
-RemoveEntryList (RecycledMapInfo);
+  MapInfo = AllocatePool (sizeof (MAP_INFO));
+  if (MapInfo == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Failed;
   }
 
   //
   // Initialize the MAP_INFO structure, except the PlainTextAddress field
   //
@@ -296,10 +283,14 @@ IoMmuMap (
   DecryptionSource,
   MapInfo->NumberOfBytes
   );
   }
 
+  //
+  // Track all MAP_INFO structures.
+  //
+  InsertHeadList (, >Link);
   //
   // Populate output 

[edk2] [PATCH 01/10] MdeModulePkg/AtaAtapiPassThru: cache EnabledPciAttributes

[Laszlo Ersek]

Both AtaAtapiPassThruStart() and AtaAtapiPassThruStop() fetch the
supported attributes of the device, just so they can toggle the
IO+MMIO+BusMaster subset.

After we compute this bitmask in AtaAtapiPassThruStart(), we can cache it
for later, and save the fetch in AtaAtapiPassThruStop().

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |  1 +
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 32 

 2 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 4f327dc30b60..85e5a5553953 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -98,10 +98,11 @@ typedef struct {
 
   //
   // The attached device list
   //
   LIST_ENTRYDeviceList;
+  UINT64EnabledPciAttributes;
   UINT64OriginalPciAttributes;
 
   //
   // For AtaPassThru protocol, using the following bytes to record the 
previous call in
   // GetNextPort()/GetNextDevice().
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index 795443ef74f6..b7fdb8dd4876 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -92,10 +92,11 @@ ATA_ATAPI_PASS_THRU_INSTANCE 
gAtaAtapiPassThruInstanceTemplate = {
   },
   {   // DeviceList
 NULL,
 NULL
   },
+  0,  // EnabledPciAttributes
   0,  // OriginalAttributes
   0,  // PreviousPort
   0,  // PreviousPortMultiplier
   0,  // PreviousTargetId
   0,  // PreviousLun
@@ -668,11 +669,11 @@ AtaAtapiPassThruStart (
 {
   EFI_STATUSStatus;
   EFI_IDE_CONTROLLER_INIT_PROTOCOL  *IdeControllerInit;
   ATA_ATAPI_PASS_THRU_INSTANCE  *Instance;
   EFI_PCI_IO_PROTOCOL   *PciIo;
-  UINT64Supports;
+  UINT64EnabledPciAttributes;
   UINT64OriginalPciAttributes;
 
   Status= EFI_SUCCESS;
   IdeControllerInit = NULL;
   Instance  = NULL;
@@ -720,18 +721,18 @@ AtaAtapiPassThruStart (
 
   Status = PciIo->Attributes (
 PciIo,
 EfiPciIoAttributeOperationSupported,
 0,
-
+
 );
   if (!EFI_ERROR (Status)) {
-Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
+EnabledPciAttributes &= (UINT64)EFI_PCI_DEVICE_ENABLE;
 Status = PciIo->Attributes (
   PciIo,
   EfiPciIoAttributeOperationEnable,
-  Supports,
+  EnabledPciAttributes,
   NULL
   );
   }
 
   if (EFI_ERROR (Status)) {
@@ -747,10 +748,11 @@ AtaAtapiPassThruStart (
   }
 
   Instance->ControllerHandle  = Controller;
   Instance->IdeControllerInit = IdeControllerInit;
   Instance->PciIo = PciIo;
+  Instance->EnabledPciAttributes  = EnabledPciAttributes;
   Instance->OriginalPciAttributes = OriginalPciAttributes;
   Instance->AtaPassThru.Mode  = >AtaPassThruMode;
   Instance->ExtScsiPassThru.Mode  = >ExtScsiPassThruMode;
   InitializeListHead(>DeviceList);
   InitializeListHead(>NonBlockingTaskList);
@@ -857,11 +859,10 @@ AtaAtapiPassThruStop (
   EFI_STATUSStatus;
   ATA_ATAPI_PASS_THRU_INSTANCE  *Instance;
   EFI_ATA_PASS_THRU_PROTOCOL*AtaPassThru;
   EFI_PCI_IO_PROTOCOL   *PciIo;
   EFI_AHCI_REGISTERS*AhciRegisters;
-  UINT64Supports;
 
   DEBUG ((EFI_D_INFO, "==AtaAtapiPassThru Stop== Controller = %x\n", 
Controller));
 
   Status = gBS->OpenProtocol (
   Controller,
@@ -950,25 +951,16 @@ AtaAtapiPassThruStop (
   }
 
   //
   // Disable this ATA host controller.
   //
-  Status = PciIo->Attributes (
-PciIo,
-EfiPciIoAttributeOperationSupported,
-0,
-
-);
-  if (!EFI_ERROR (Status)) {
-Supports &= (UINT64)EFI_PCI_DEVICE_ENABLE;
-PciIo->Attributes (
- PciIo,
- EfiPciIoAttributeOperationDisable,
- Supports,
- NULL
- );
-  }
+  PciIo->Attributes (
+   PciIo,
+   

[edk2] [PATCH 03/10] MdeModulePkg/AtaAtapiPassThru: disable the device at ExitBootServices()

[Laszlo Ersek]

The AtaAtapiPassThru driver maps three system memory regions for Bus
Master Common Buffer operation on the following call path, if the
controller has PCI_CLASS_MASS_STORAGE_SATADPA class code:

  AtaAtapiPassThruStart()
EnumerateAttachedDevice()
  AhciModeInitialization()
AhciCreateTransferDescriptor()

The device is disabled (including Bus Master DMA) when the controller is
unbound, in AtaAtapiPassThruStop(). Then the regions are unmapped.

The former step should also be done when we exit the boot services, and
the OS gains ownership of system memory.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h |  6 ++
 MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 59 
+++-
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
index 85e5a5553953..8d6eac706c0b 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.h
@@ -119,10 +119,16 @@ typedef struct {
   //
   // For Non-blocking.
   //
   EFI_EVENT TimerEvent;
   LIST_ENTRYNonBlockingTaskList;
+
+  //
+  // For disabling the device (especially Bus Master DMA) at
+  // ExitBootServices().
+  //
+  EFI_EVENT ExitBootEvent;
 } ATA_ATAPI_PASS_THRU_INSTANCE;
 
 //
 // Task for Non-blocking mode.
 //
diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c 
b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
index a48b295d26aa..09064dda18b7 100644
--- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
+++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
@@ -102,11 +102,12 @@ ATA_ATAPI_PASS_THRU_INSTANCE 
gAtaAtapiPassThruInstanceTemplate = {
   0,  // PreviousLun
   NULL,   // Timer event
   {   // NonBlocking TaskList
 NULL,
 NULL
-  }
+  },
+  NULL,   // ExitBootEvent
 };
 
 ATAPI_DEVICE_PATHmAtapiDevicePathTemplate = {
   {
 MESSAGING_DEVICE_PATH,
@@ -476,10 +477,42 @@ InitializeAtaAtapiPassThru (
   ASSERT_EFI_ERROR (Status);
 
   return Status;
 }
 
+/**
+  Disable the device (especially Bus Master DMA) when exiting the boot
+  services.
+
+  @param[in] EventEvent for which this notification function is being
+  called.
+  @param[in] Context  Pointer to the ATA_ATAPI_PASS_THRU_INSTANCE that
+  represents the HBA.
+**/
+VOID
+EFIAPI
+AtaPassThruExitBootServices (
+  IN EFI_EVENT Event,
+  IN VOID  *Context
+  )
+{
+  ATA_ATAPI_PASS_THRU_INSTANCE *Instance;
+  EFI_PCI_IO_PROTOCOL  *PciIo;
+
+  DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context));
+
+  Instance = Context;
+  PciIo = Instance->PciIo;
+
+  PciIo->Attributes (
+   PciIo,
+   EfiPciIoAttributeOperationDisable,
+   Instance->EnabledPciAttributes,
+   NULL
+   );
+}
+
 /**
   Tests to see if this driver supports a given controller. If a child device 
is provided,
   it further tests to see if this driver supports creating a handle for the 
specified child device.
 
   This function checks to see if the driver specified by This supports the 
device specified by
@@ -755,10 +788,21 @@ AtaAtapiPassThruStart (
   Instance->AtaPassThru.Mode  = >AtaPassThruMode;
   Instance->ExtScsiPassThru.Mode  = >ExtScsiPassThruMode;
   InitializeListHead(>DeviceList);
   InitializeListHead(>NonBlockingTaskList);
 
+  Status = gBS->CreateEvent (
+  EVT_SIGNAL_EXIT_BOOT_SERVICES,
+  TPL_CALLBACK,
+  AtaPassThruExitBootServices,
+  Instance,
+  >ExitBootEvent
+  );
+  if (EFI_ERROR (Status)) {
+goto ErrorExit;
+  }
+
   Instance->TimerEvent = NULL;
 
   Status = gBS->CreateEvent (
   EVT_TIMER | EVT_NOTIFY_SIGNAL,
   TPL_NOTIFY,
@@ -808,10 +852,14 @@ ErrorExit:
 
   if ((Instance != NULL) && (Instance->TimerEvent != NULL)) {
 gBS->CloseEvent (Instance->TimerEvent);
   }
 
+  if ((Instance != NULL) && (Instance->ExitBootEvent != NULL)) {
+gBS->CloseEvent (Instance->ExitBootEvent);
+  }
+
   //
   // Remove all inserted ATA devices.
   //
   DestroyDeviceInfoList(Instance);
 
@@ -906,10 +954,19 @@ AtaAtapiPassThruStop (
   if (Instance->TimerEvent != NULL) {
 gBS->CloseEvent (Instance->TimerEvent);
 Instance->TimerEvent = NULL;
   }
   DestroyAsynTaskList (Instance, FALSE);
+
+  //
+  // Close event signaled at gBS->ExitBootServices().
+  

[edk2] [PATCH 09/10] OvmfPkg/IoMmuDxe: generalize IoMmuUnmap() to IoMmuUnmapWorker()

[Laszlo Ersek]

IoMmuUnmapWorker() is identical to IoMmuUnmap(), it just takes an
additional BOOLEAN parameter called "MemoryMapLocked".  If the memory map
is locked, IoMmuUnmapWorker() does its usual job, but it purposely leaks
memory rather than freeing it. This makes it callable from
ExitBootServices() context.

Turn IoMmuUnmap() into a thin wrapper around IoMmuUnmapWorker() that
passes constant FALSE for "MemoryMapLocked".

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 60 +---
 1 file changed, 53 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index c86e73498555..34e1c6ee4a74 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -316,32 +316,46 @@ Failed:
 }
 
 /**
   Completes the Map() operation and releases any corresponding resources.
 
+  This is an internal worker function that only extends the Map() API with
+  the MemoryMapLocked parameter.
+
   @param  This  The protocol instance pointer.
   @param  Mapping   The mapping value returned from Map().
+  @param  MemoryMapLocked   The function is executing on the stack of
+gBS->ExitBootServices(); changes to the UEFI
+memory map are forbidden.
 
   @retval EFI_SUCCESS   The range was unmapped.
   @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by
 Map().
   @retval EFI_DEVICE_ERROR  The data was not committed to the target system
 memory.
 **/
+STATIC
 EFI_STATUS
 EFIAPI
-IoMmuUnmap (
+IoMmuUnmapWorker (
   IN  EDKII_IOMMU_PROTOCOL *This,
-  IN  VOID *Mapping
+  IN  VOID *Mapping,
+  IN  BOOLEAN  MemoryMapLocked
   )
 {
   MAP_INFO *MapInfo;
   EFI_STATUS   Status;
   COMMON_BUFFER_HEADER *CommonBufferHeader;
   VOID *EncryptionTarget;
 
-  DEBUG ((DEBUG_VERBOSE, "%a: Mapping=0x%p\n", __FUNCTION__, Mapping));
+  DEBUG ((
+DEBUG_VERBOSE,
+"%a: Mapping=0x%p MemoryMapLocked=%d\n",
+__FUNCTION__,
+Mapping,
+MemoryMapLocked
+));
 
   if (Mapping == NULL) {
 return EFI_INVALID_PARAMETER;
   }
 
@@ -410,11 +424,12 @@ IoMmuUnmap (
   //
   // For BusMasterCommonBuffer[64] operations, copy the stashed data to the
   // original (now encrypted) location.
   //
   // For all other operations, fill the late bounce buffer (which existed as
-  // plaintext at some point) with zeros, and then release it.
+  // plaintext at some point) with zeros, and then release it (unless the UEFI
+  // memory map is locked).
   //
   if (MapInfo->Operation == EdkiiIoMmuOperationBusMasterCommonBuffer ||
   MapInfo->Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) {
 CopyMem (
   (VOID *)(UINTN)MapInfo->CryptedAddress,
@@ -424,22 +439,53 @@ IoMmuUnmap (
   } else {
 ZeroMem (
   (VOID *)(UINTN)MapInfo->PlainTextAddress,
   EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages)
   );
-gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
+if (!MemoryMapLocked) {
+  gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
+}
   }
 
   //
-  // Forget and free the MAP_INFO structure.
+  // Forget the MAP_INFO structure, then free it (unless the UEFI memory map is
+  // locked).
   //
   RemoveEntryList (>Link);
-  FreePool (MapInfo);
+  if (!MemoryMapLocked) {
+FreePool (MapInfo);
+  }
 
   return EFI_SUCCESS;
 }
 
+/**
+  Completes the Map() operation and releases any corresponding resources.
+
+  @param  This  The protocol instance pointer.
+  @param  Mapping   The mapping value returned from Map().
+
+  @retval EFI_SUCCESS   The range was unmapped.
+  @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by
+Map().
+  @retval EFI_DEVICE_ERROR  The data was not committed to the target system
+memory.
+**/
+EFI_STATUS
+EFIAPI
+IoMmuUnmap (
+  IN  EDKII_IOMMU_PROTOCOL *This,
+  IN  VOID *Mapping
+  )
+{
+  return IoMmuUnmapWorker (
+   This,
+   Mapping,
+   FALSE// MemoryMapLocked
+   );
+}
+
 /**
   Allocates pages that are suitable for an OperationBusMasterCommonBuffer or
   OperationBusMasterCommonBuffer64 mapping.
 
   @param  This  The protocol instance pointer.
-- 
2.14.1.3.gb7cf6e02401b


___

[edk2] [PATCH 04/10] OvmfPkg/VirtioBlkDxe: don't unmap VRING at ExitBootServices()

[Laszlo Ersek]

In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it
unmaps all existent bus master operations (CommonBuffer, Read, Write) at
ExitBootServices(), strictly after the individual device drivers abort
pending DMA on the devices they manage, in their own ExitBootServices()
notification functions.

In preparation, remove the explicit
VIRTIO_DEVICE_PROTOCOL.UnmapSharedBuffer() call from VirtioBlkExitBoot(),
originally added in commit 19165130470f ("OvmfPkg/VirtioBlkDxe: map VRING
using VirtioRingMap()", 2017-08-27).

Add a DEBUG message so we can observe the ordering between
VirtioBlkExitBoot() and the upcoming cleanup of mappings in
OvmfPkg/IoMmuDxe.

Cc: Ard Biesheuvel 
Cc: Brijesh Singh 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/VirtioBlkDxe/VirtioBlk.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
index 5a63986b3f39..55598843455d 100644
--- a/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
+++ b/OvmfPkg/VirtioBlkDxe/VirtioBlk.c
@@ -1017,25 +1017,20 @@ VirtioBlkExitBoot (
   IN  VOID  *Context
   )
 {
   VBLK_DEV *Dev;
 
+  DEBUG ((DEBUG_VERBOSE, "%a: Context=0x%p\n", __FUNCTION__, Context));
   //
   // Reset the device. This causes the hypervisor to forget about the virtio
   // ring.
   //
   // We allocated said ring in EfiBootServicesData type memory, and code
   // executing after ExitBootServices() is permitted to overwrite it.
   //
   Dev = Context;
   Dev->VirtIo->SetDeviceStatus (Dev->VirtIo, 0);
-
-  //
-  // Unmap the ring buffer so that hypervisor will not be able to get
-  // readable data after device is reset.
-  //
-  Dev->VirtIo->UnmapSharedBuffer (Dev->VirtIo, Dev->RingMap);
 }
 
 /**
 
   After we've pronounced support for a specific device in
-- 
2.14.1.3.gb7cf6e02401b


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

Re: [edk2] Fwd: Boot####, Key####

[Laszlo Ersek]

On 09/07/17 22:32, Andrew Fish wrote:
> 
>> On Sep 7, 2017, at 1:01 PM, David F.  wrote:
>>
>> Hi,
>>
>> Implementing support for UEFI inside different OS environments, I
>> don't see anything in the spec about being able to find the various
>> Boot and Key values without trying 0-0x which is slow
>> (takes 10+ seconds).What's the better way?
>>
> 
> Usually there is another variable that implies the Boot value that is in 
> use, like BootOrder. 

There's also GetNextVariableName() -- iterating through, say, ~50
variables that all exist is likely faster than querying ~65500 that don't.

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

Re: [edk2] Fwd: Boot####, Key####

[Andrew Fish]

> On Sep 7, 2017, at 1:01 PM, David F.  wrote:
> 
> Hi,
> 
> Implementing support for UEFI inside different OS environments, I
> don't see anything in the spec about being able to find the various
> Boot and Key values without trying 0-0x which is slow
> (takes 10+ seconds).What's the better way?
> 

Usually there is another variable that implies the Boot value that is in 
use, like BootOrder. 

Thanks,

Andrew Fish


> Thanks.
> ___
> 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] Fwd: Boot####, Key####

[David F.]

Hi,

Implementing support for UEFI inside different OS environments, I
don't see anything in the spec about being able to find the various
Boot and Key values without trying 0-0x which is slow
(takes 10+ seconds).What's the better way?

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

[edk2] Fwd: StartImage with Secure Boot on Self-Signed App

[David F.]

Hello,

What is the proper way to allow running another app that is verified
with a self-signed certificate?

Example, App1 is signed with one that allows secure boot booting (in
firmware) and has a public key embedded in the signed code, App2 is
verified by App1 and so is allowed to run, but because the key is not
in secure boot firmware, StartImage will not run it (although
LoadImage did what it needed to do and already reported the security
violation potential).   Do we have to roll our own StartImage?  or is
something already in place?  I can't rely on changing an internal
private structure field to allow StartImage to work since each
firmware platform may change the way it all works, looking for the
proper method as designed.

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

Re: [edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds

[Laszlo Ersek]

On 09/07/17 17:04, Gao, Liming wrote:
> I suggest to also add this option in GCC49 tool chain RELEASE builds. GCC49 
> can be used for GCC 4.9 or the above version. It doesn't enable LTO. 

Good idea. There could be several reasons for picking GCC49; see e.g.
commit 432f1d83f77a ("OvmfPkg/build.sh: Use GCC49 toolchains with GCC
6.[0-2]", 2016-12-06).

Thomas, can you please CC Ard on v2?

Thank you!
Laszlo


>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Thomas Lamprecht
>> Sent: Thursday, September 7, 2017 10:55 PM
>> To: edk2-devel@lists.01.org
>> Cc: Laszlo Ersek ; Gao, Liming 
>> Subject: [edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on 
>> RELEASE builds
>>
>> TianoCore BZ#700 
>>
>> This fixes the RELEASE build of OVMF with GCC in version 6 or newer.
>> GCC 6 added the '-Wunused-const-variable' warning, which gets
>> activated by '-Wunused-variable' and has the following behavior:
>> "Warn whenever a constant static variable is unused aside from its
>> declaration" [1]
>>
>> Commit 2ad6ba80a1bd58382bde6b994070f7c01d2fb48d introduced a case
>> where exactly this happens on a RELEASE build. All uses of the static
>> const variable are located in debug code only, which gets thrown out
>> by the compiler on RELEASE builds and thus triggers the
>> unused-const-variable warning.
>>
>> There is currently no GCC 6 toolchain target defined and doing so
>> would add a lot of boilerplate code. Instead, use the fact that GCC
>> ignores unkown '-Wno-*' options:
>>
>> "[...] if the -Wno- form is used [...] no diagnostic is produced for
>> -Wno-unknown-warning unless other diagnostics are being produced"
>>
>> This behavior is available in GCC 5 [2] (and also earlier, for that
>> matter), so add the flag to the GCC5 toolchain, even if GCC 5 itself
>> does not supports it.
>>
>> Orient the changes on 20d00edf21d2 which moved the
>> '-Wno-unused-but-set-variable' flag to RELEASE builds only, as there
>> it ensure that it does not gets raised if the only usage of a
>> variable is in (then collapsed) debug code.
>>
>> [1] 
>> https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html#index-Wunused-const-variable
>> [2] https://gcc.gnu.org/onlinedocs/gcc-5.4.0/gcc/Warning-Options.html
>>
>> Cc: Yonghong Zhu 
>> Cc: Liming Gao 
>> Cc: Laszlo Ersek 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Thomas Lamprecht 
>> ---
>>
>> I hope I CCed the correct people and got the style right :)
>>
>>  BaseTools/Conf/tools_def.template | 8 
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/BaseTools/Conf/tools_def.template 
>> b/BaseTools/Conf/tools_def.template
>> index ba1d1a16de..7db7a174c3 100755
>> --- a/BaseTools/Conf/tools_def.template
>> +++ b/BaseTools/Conf/tools_def.template
>> @@ -5326,7 +5326,7 @@ RELEASE_GCC49_AARCH64_DLINK_FLAGS  = 
>> DEF(GCC49_AARCH64_DLINK_FLAGS)
>>DEBUG_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os
>>DEBUG_GCC5_IA32_DLINK_FLAGS= DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os 
>> -Wl,-m,elf_i386,--oformat=elf32-i386
>>
>> -RELEASE_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os 
>> -Wno-unused-but-set-variable
>> +RELEASE_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os 
>> -Wno-unused-but-set-variable
>> -Wno-unused-const-variable
>>  RELEASE_GCC5_IA32_DLINK_FLAGS= DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os 
>> -Wl,-m,elf_i386,--oformat=elf32-i386
>>
>>NOOPT_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -O0
>> @@ -5358,7 +5358,7 @@ RELEASE_GCC5_IA32_DLINK_FLAGS= 
>> DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os -Wl,
>>DEBUG_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
>> -Os
>>DEBUG_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
>>
>> -RELEASE_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
>> -Os -Wno-unused-but-set-variable
>> +RELEASE_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
>> -Os -Wno-unused-but-set-variable
>> -Wno-unused-const-variable
>>  RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
>>
>>NOOPT_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -O0
>> @@ -5393,7 +5393,7 @@ RELEASE_GCC5_X64_DLINK_FLAGS = 
>> DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
>>DEBUG_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -O0
>>DEBUG_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS)
>>
>> -RELEASE_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -flto 
>> -Wno-unused-but-set-variable
>> +RELEASE_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -flto 
>> -Wno-unused-but-set-variable
>> -Wno-unused-const-variable
>>  RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os 
>> 

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Laszlo Ersek]

On 09/07/17 16:48, Yao, Jiewen wrote:
> Great. Thanks to confirm that. Now it is clear to me.
> 
> Then let's wait Laszlo's new patch to make all DMA buffer to private. :)

I've got the patches ready and smoke-tested. Let me look a bit more at
the logs, and re-review the patches. I hope I can post the series still
today (well, in my timezone anyway :) )

Thanks!
Laszlo

> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Brijesh Singh
> Sent: Thursday, September 7, 2017 10:40 PM
> To: Laszlo Ersek ; Yao, Jiewen ; 
> Zeng, Star ; edk2-devel-01 
> Cc: brijesh.si...@amd.com; Dong, Eric 
> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap 
> common buffers at ExitBootServices()
> 
> Hi Jiewen,
> 
> On 09/07/2017 06:46 AM, Laszlo Ersek wrote:
>> On 09/07/17 06:46, Yao, Jiewen wrote:
>>> Thanks for the sharing, Brijesh.
>>>
>>> "If a page was marked as "shared"
>>> then its guest responsibility to make it "private" after its done 
>>> communicating with
>>> hypervisor."
>>>
>>> I believe I have same understanding - a *guest* should guarantee that.
>>>
>>> My question is: During the *transition* from firmware to OS, *which guest* 
>>> should make the shared buffer to be private? Is it "guest firmware" or 
>>> "guest OS"?
>>>
>>> Maybe I can ask the specific question to get it more clear.
>>>
>>> 1)   If the common DMA buffer is not unmapped at ExitBootService, is 
>>> that treated as an issue?
>>>
>>> 2)   If the read/write DMA buffer is not unmapped at ExitBootService, 
>>> is that treated as an issue?
>>
>> Very good questions, totally to the point.
>>
>> On the authoritative answer, I defer to Brijesh.
>>
> 
> 
> Both the above cases (#1 and #2) are problems. Since buffers was owned by 
> firmware
> and firmware made it "shared" hence firmware is responsible to mark them as 
> private
> after its done with the buffer. In other words, we must call Unmap() from 
> ExitBootServices
> to ensure that buffers mapped using 
> BusMasterCommonBuffer/BusMasterRead/BusMasterWrite
> is marked as "private" before we make it available to the guest OS. (we do 
> similar thing
> in Linux OS).
> 
> Having any kind of side channel to communicate the encryption status of a page
> will not work -- we should be able to support a usecase where we boot OVMF in
> 64-bit but launch 32-bit Linux guest. When Linux boots in 32-bit mode it does 
> not have
> access to encryption bit (C-bit is bit-47 in page table) and can't mark the 
> page as
> private (even if we provide some kind of side-channel).
> 
> thank you very much for all your help.
> 
>> (
>>
>> My personal opinion is that both situations (#1 and #2) are problems,
>> because they break the *practical* security invariant for SEV guests:
>>
>> - most memory should be encrypted at all times, *and*
>>
>> - any memory that is decrypted must have an owner that is responsible
>>for re-encrypting the memory eventually.
>>
>> Therefore, *either* the firmware has to re-encrypt all remaining DMA
>> buffers at ExitBootServices(), *or* a new information channel must be
>> designed, from firmware to OS, to carry over the decryption status.
>>
>> I strongly prefer the first option, for the following reason: the same
>> questions apply to all EDK2 IOMMU protocol interfaces, not just the one
>> exported by the SEV driver.
>>
>> )
>>
>> Thanks,
>> Laszlo
>>
>>> From: Brijesh Singh [mailto:brijesh.si...@amd.com]
>>> Sent: Wednesday, September 6, 2017 11:40 PM
>>> To: Laszlo Ersek >; Yao, Jiewen 
>>> >; Zeng, Star 
>>> >; edk2-devel-01 
>>> >
>>> Cc: brijesh.si...@amd.com; Dong, Eric 
>>> >
>>> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap 
>>> common buffers at ExitBootServices()
>>>
>>>
>>>
>>> On 09/06/2017 07:14 AM, Laszlo Ersek wrote:
 On 09/06/17 06:37, Yao, Jiewen wrote:
> Thanks for the clarification. Comment in line.
>
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, September 6, 2017 1:57 AM
> To: Yao, Jiewen 
> >>;
>  Zeng, Star 
> >>;
>  edk2-devel-01 
> >>
> Cc: Dong, Eric 
> >>;
>  Brijesh Singh 
> 

Re: [edk2] [PATCH v3 1/2] MMC : Added missing __FUNCTION__ macro.

[Leif Lindholm]

Thanks!

For the series:
Reviewed-by: Leif Lindholm 

Pushed as c50596a701..ea21f1d98d

/
Leif


On Thu, Sep 07, 2017 at 03:19:34PM +, Meenakshi Aggarwal wrote:
> Hi Leif,
> 
> Yes, I agree to contribute under TianoCore Contribution Agreement 1.1
> 
> Thanks,
> Meenakshi
> 
> > -Original Message-
> > From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> > Sent: Thursday, September 07, 2017 8:07 PM
> > To: Meenakshi Aggarwal 
> > Cc: jun@linaro.org; haojian.zhu...@linaro.org; edk2-devel@lists.01.org
> > Subject: Re: [PATCH v3 1/2] MMC : Added missing __FUNCTION__ macro.
> > 
> > On Thu, Sep 07, 2017 at 07:47:52PM +0530, Meenakshi Aggarwal wrote:
> > > We want to print name of the function resulted in error, but
> > > __FUNCTION__ macro was missing.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Meenakshi Aggarwal 
> > 
> > Like for 2/2, you appear to have left out edk2-devel@lists.01.org in this
> > submission (added).
> > 
> > Like for 2/2, happy to push this if you can confirm you are contributing 
> > under
> > TianoCore Contribution Agreement 1.1.
> > 
> > /
> > Leif
> > 
> > > ---
> > >  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 14 +++-
> > --
> > >  1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > > b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > > index 7f74c54..27e 100644
> > > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > > @@ -388,12 +388,12 @@ InitializeSdMmcDevice (
> > >
> > >Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);
> > >if (EFI_ERROR (Status)) {
> > > -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n",
> > Status));
> > > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n",
> > > + __FUNCTION__, Status));
> > >  return Status;
> > >}
> > >Status = MmcHost->ReceiveResponse (MmcHost,
> > MMC_RESPONSE_TYPE_R1, Response);
> > >if (EFI_ERROR (Status)) {
> > > -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n",
> > Status));
> > > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n",
> > > + __FUNCTION__, Status));
> > >  return Status;
> > >}
> > >if ((Response[0] & MMC_STATUS_APP_CMD) == 0) { @@ -445,12 +445,12
> > > @@ InitializeSdMmcDevice (
> > >  CmdArg |= 1 << (0 * 4);
> > >  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
> > >  if (EFI_ERROR (Status)) {
> > > -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n",
> > Status));
> > > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> > > + __FUNCTION__, Status));
> > > return Status;
> > >  } else {
> > >Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);
> > >if (EFI_ERROR (Status)) {
> > > -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): ReadBlockData Error and
> > Status = %r\n", Status));
> > > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error
> > and
> > > + Status = %r\n", __FUNCTION__, Status));
> > >  return Status;
> > >}
> > >  }
> > > @@ -459,20 +459,20 @@ InitializeSdMmcDevice (
> > >  CmdArg = MmcHostInstance->CardInfo.RCA << 16;
> > >  Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);
> > >  if (EFI_ERROR (Status)) {
> > > -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n",
> > Status));
> > > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status =
> > %r\n",
> > > + __FUNCTION__, Status));
> > >return Status;
> > >  }
> > >  /* Width: 4 */
> > >  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, 2);
> > >  if (EFI_ERROR (Status)) {
> > > -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n",
> > Status));
> > > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> > > + __FUNCTION__, Status));
> > >return Status;
> > >  }
> > >}
> > >if (MMC_HOST_HAS_SETIOS(MmcHost)) {
> > >  Status = MmcHost->SetIos (MmcHost, 26 * 1000 * 1000, 4,
> > EMMCBACKWARD);
> > >  if (EFI_ERROR (Status)) {
> > > -  DEBUG ((EFI_D_ERROR, "%a(SetIos): Error and Status = %r\n", 
> > > Status));
> > > +  DEBUG ((DEBUG_ERROR, "%a (SetIos): Error and Status = %r\n",
> > > + __FUNCTION__, Status));
> > >return Status;
> > >  }
> > >}
> > > --
> > > 1.9.1
> > >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 1/2] MMC : Added missing __FUNCTION__ macro.

[Meenakshi Aggarwal]

Hi Leif,

Yes, I agree to contribute under TianoCore Contribution Agreement 1.1

Thanks,
Meenakshi

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Thursday, September 07, 2017 8:07 PM
> To: Meenakshi Aggarwal 
> Cc: jun@linaro.org; haojian.zhu...@linaro.org; edk2-devel@lists.01.org
> Subject: Re: [PATCH v3 1/2] MMC : Added missing __FUNCTION__ macro.
> 
> On Thu, Sep 07, 2017 at 07:47:52PM +0530, Meenakshi Aggarwal wrote:
> > We want to print name of the function resulted in error, but
> > __FUNCTION__ macro was missing.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Meenakshi Aggarwal 
> 
> Like for 2/2, you appear to have left out edk2-devel@lists.01.org in this
> submission (added).
> 
> Like for 2/2, happy to push this if you can confirm you are contributing under
> TianoCore Contribution Agreement 1.1.
> 
> /
> Leif
> 
> > ---
> >  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 14 +++-
> --
> >  1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > index 7f74c54..27e 100644
> > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > @@ -388,12 +388,12 @@ InitializeSdMmcDevice (
> >
> >Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);
> >if (EFI_ERROR (Status)) {
> > -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n",
> Status));
> > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n",
> > + __FUNCTION__, Status));
> >  return Status;
> >}
> >Status = MmcHost->ReceiveResponse (MmcHost,
> MMC_RESPONSE_TYPE_R1, Response);
> >if (EFI_ERROR (Status)) {
> > -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n",
> Status));
> > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n",
> > + __FUNCTION__, Status));
> >  return Status;
> >}
> >if ((Response[0] & MMC_STATUS_APP_CMD) == 0) { @@ -445,12 +445,12
> > @@ InitializeSdMmcDevice (
> >  CmdArg |= 1 << (0 * 4);
> >  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
> >  if (EFI_ERROR (Status)) {
> > -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n",
> Status));
> > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> > + __FUNCTION__, Status));
> > return Status;
> >  } else {
> >Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);
> >if (EFI_ERROR (Status)) {
> > -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): ReadBlockData Error and
> Status = %r\n", Status));
> > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error
> and
> > + Status = %r\n", __FUNCTION__, Status));
> >  return Status;
> >}
> >  }
> > @@ -459,20 +459,20 @@ InitializeSdMmcDevice (
> >  CmdArg = MmcHostInstance->CardInfo.RCA << 16;
> >  Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);
> >  if (EFI_ERROR (Status)) {
> > -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n",
> Status));
> > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status =
> %r\n",
> > + __FUNCTION__, Status));
> >return Status;
> >  }
> >  /* Width: 4 */
> >  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, 2);
> >  if (EFI_ERROR (Status)) {
> > -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n",
> Status));
> > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> > + __FUNCTION__, Status));
> >return Status;
> >  }
> >}
> >if (MMC_HOST_HAS_SETIOS(MmcHost)) {
> >  Status = MmcHost->SetIos (MmcHost, 26 * 1000 * 1000, 4,
> EMMCBACKWARD);
> >  if (EFI_ERROR (Status)) {
> > -  DEBUG ((EFI_D_ERROR, "%a(SetIos): Error and Status = %r\n", Status));
> > +  DEBUG ((DEBUG_ERROR, "%a (SetIos): Error and Status = %r\n",
> > + __FUNCTION__, Status));
> >return Status;
> >  }
> >}
> > --
> > 1.9.1
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 2/2] SD : Updated CMD 6 implememtation.

[Meenakshi Aggarwal]

Hi Leif,

I agree to contribute under TianoCore Contribution Agreement 1.1

Thanks,
Meenakshi

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Thursday, September 07, 2017 8:05 PM
> To: Meenakshi Aggarwal 
> Cc: jun@linaro.org; haojian.zhu...@linaro.org; edk2-devel@lists.01.org
> Subject: Re: [PATCH v3 2/2] SD : Updated CMD 6 implememtation.
> 
> On Thu, Sep 07, 2017 at 07:47:53PM +0530, Meenakshi Aggarwal wrote:
> > For setting high speed in SD card,
> > First CMD 6 (Switch) is send to check if card supports High Speed and
> > Second command is send to switch card to high speed mode.
> >
> > In current inplementation, CMD 6 was sent only once to switch the card
> > into HS mode without checking if card supports HS or not, which is not
> > as per specification and also we are not setting the HS i.e. 5000
> > but directly asking the card to switch to 2600 which is incorrect
> > as SD card supports either 2500 or 5000.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> 
> Ok, I would say this one is good to go, but can you please confirm if you are
> happy to contribute under TianoCore Contribution Agreement 1.1, which is
> the version currently described in Contributions.txt?
> 
> If so, I can fold in this change and push.
> 
> Oh, you also appear to have left out edk2-devel@lists.01.org in this
> submission (added).
> 
> Best Regards,
> 
> Leif
> 
> > Signed-off-by: Meenakshi Aggarwal 
> > ---
> >  EmbeddedPkg/Universal/MmcDxe/Mmc.h   |  8 
> >  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 53
> > +---
> >  2 files changed, 56 insertions(+), 5 deletions(-)  mode change 100644
> > => 100755 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> >
> > diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > index f3e56ff..a77ba41 100644
> > --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> > @@ -64,6 +64,14 @@
> >  #define EMMC_CMD6_ARG_VALUE(x)  (((x) & 0xFF) << 8)
> >  #define EMMC_CMD6_ARG_CMD_SET(x)(((x) & 0x7) << 0)
> >
> > +#define SWITCH_CMD_DATA_LENGTH  64
> > +#define SD_HIGH_SPEED_SUPPORTED 0x2
> > +#define SD_DEFAULT_SPEED2500
> > +#define SD_HIGH_SPEED   5000
> > +#define SWITCH_CMD_SUCCESS_MASK 0x0f00
> > +
> > +#define BUSWIDTH_4  4
> > +
> >  typedef enum {
> >UNKNOWN_CARD,
> >MMC_CARD,  //MMC card
> > diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > old mode 100644
> > new mode 100755
> > index 27e..f661a0c
> > --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> > @@ -318,6 +318,23 @@ InitializeEmmcDevice (  }
> >
> >  STATIC
> > +UINT32
> > +CreateSwitchCmdArgument (
> > +  IN  UINT32 Mode,
> > +  IN  UINT8  Group,
> > +  IN  UINT8  Value
> > +  )
> > +{
> > +  UINT32 Argument;
> > +
> > +  Argument = Mode << 31 | 0x00FF;  Argument &= ~(0xF << (Group *
> > + 4));  Argument |= Value << (Group * 4);
> > +
> > +  return Argument;
> > +}
> > +
> > +STATIC
> >  EFI_STATUS
> >  InitializeSdMmcDevice (
> >IN  MMC_HOST_INSTANCE   *MmcHostInstance
> > @@ -326,6 +343,7 @@ InitializeSdMmcDevice (
> >UINT32CmdArg;
> >UINT32Response[4];
> >UINT32Buffer[128];
> > +  UINT32Speed;
> >UINTN BlockSize;
> >UINTN CardSize;
> >UINTN NumBlocks;
> > @@ -334,6 +352,7 @@ InitializeSdMmcDevice (
> >EFI_STATUSStatus;
> >EFI_MMC_HOST_PROTOCOL *MmcHost;
> >
> > +  Speed = SD_DEFAULT_SPEED;
> >MmcHost = MmcHostInstance->MmcHost;
> >
> >// Send a command to get Card specific data @@ -439,20 +458,44 @@
> > InitializeSdMmcDevice (
> >  }
> >}
> >if (CccSwitch) {
> > +/* SD Switch, Mode:0, Group:0, Value:0 */
> > +CmdArg = CreateSwitchCmdArgument(0, 0, 0);
> > +Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
> > +if (EFI_ERROR (Status)) {
> > +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n",
> __FUNCTION__, Status));
> > +   return Status;
> > +} else {
> > +  Status = MmcHost->ReadBlockData (MmcHost, 0,
> SWITCH_CMD_DATA_LENGTH, Buffer);
> > +  if (EFI_ERROR (Status)) {
> > +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error
> and Status = %r\n", __FUNCTION__, Status));
> > +return Status;
> > +  }
> > +}
> > +
> > +if (!(Buffer[3] & SD_HIGH_SPEED_SUPPORTED)) {
> > +  DEBUG ((DEBUG_ERROR, "%a : High Speed not supported by Card
> %r\n", __FUNCTION__, Status));
> > +  return Status;
> > +}
> > +
> > +Speed = SD_HIGH_SPEED;
> > +
> >  /* SD Switch, 

Re: [edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds

[Gao, Liming]

I suggest to also add this option in GCC49 tool chain RELEASE builds. GCC49 can 
be used for GCC 4.9 or the above version. It doesn't enable LTO. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Thomas 
> Lamprecht
> Sent: Thursday, September 7, 2017 10:55 PM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Gao, Liming 
> Subject: [edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on 
> RELEASE builds
> 
> TianoCore BZ#700 
> 
> This fixes the RELEASE build of OVMF with GCC in version 6 or newer.
> GCC 6 added the '-Wunused-const-variable' warning, which gets
> activated by '-Wunused-variable' and has the following behavior:
> "Warn whenever a constant static variable is unused aside from its
> declaration" [1]
> 
> Commit 2ad6ba80a1bd58382bde6b994070f7c01d2fb48d introduced a case
> where exactly this happens on a RELEASE build. All uses of the static
> const variable are located in debug code only, which gets thrown out
> by the compiler on RELEASE builds and thus triggers the
> unused-const-variable warning.
> 
> There is currently no GCC 6 toolchain target defined and doing so
> would add a lot of boilerplate code. Instead, use the fact that GCC
> ignores unkown '-Wno-*' options:
> 
> "[...] if the -Wno- form is used [...] no diagnostic is produced for
> -Wno-unknown-warning unless other diagnostics are being produced"
> 
> This behavior is available in GCC 5 [2] (and also earlier, for that
> matter), so add the flag to the GCC5 toolchain, even if GCC 5 itself
> does not supports it.
> 
> Orient the changes on 20d00edf21d2 which moved the
> '-Wno-unused-but-set-variable' flag to RELEASE builds only, as there
> it ensure that it does not gets raised if the only usage of a
> variable is in (then collapsed) debug code.
> 
> [1] 
> https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html#index-Wunused-const-variable
> [2] https://gcc.gnu.org/onlinedocs/gcc-5.4.0/gcc/Warning-Options.html
> 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Thomas Lamprecht 
> ---
> 
> I hope I CCed the correct people and got the style right :)
> 
>  BaseTools/Conf/tools_def.template | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Conf/tools_def.template 
> b/BaseTools/Conf/tools_def.template
> index ba1d1a16de..7db7a174c3 100755
> --- a/BaseTools/Conf/tools_def.template
> +++ b/BaseTools/Conf/tools_def.template
> @@ -5326,7 +5326,7 @@ RELEASE_GCC49_AARCH64_DLINK_FLAGS  = 
> DEF(GCC49_AARCH64_DLINK_FLAGS)
>DEBUG_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os
>DEBUG_GCC5_IA32_DLINK_FLAGS= DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os 
> -Wl,-m,elf_i386,--oformat=elf32-i386
> 
> -RELEASE_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os 
> -Wno-unused-but-set-variable
> +RELEASE_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os 
> -Wno-unused-but-set-variable
> -Wno-unused-const-variable
>  RELEASE_GCC5_IA32_DLINK_FLAGS= DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os 
> -Wl,-m,elf_i386,--oformat=elf32-i386
> 
>NOOPT_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -O0
> @@ -5358,7 +5358,7 @@ RELEASE_GCC5_IA32_DLINK_FLAGS= 
> DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os -Wl,
>DEBUG_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
> -Os
>DEBUG_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
> 
> -RELEASE_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
> -Os -Wno-unused-but-set-variable
> +RELEASE_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
> -Os -Wno-unused-but-set-variable
> -Wno-unused-const-variable
>  RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
> 
>NOOPT_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -O0
> @@ -5393,7 +5393,7 @@ RELEASE_GCC5_X64_DLINK_FLAGS = 
> DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
>DEBUG_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -O0
>DEBUG_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS)
> 
> -RELEASE_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -flto 
> -Wno-unused-but-set-variable
> +RELEASE_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -flto 
> -Wno-unused-but-set-variable
> -Wno-unused-const-variable
>  RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os 
> -L$(WORKSPACE)/ArmPkg/Library/GccLto
> -llto-arm -Wl,-plugin-opt=-pass-through=-llto-arm
> 
>NOOPT_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -O0
> @@ -5428,7 +5428,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = 
> DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS
>DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z 
> 

[edk2] [PATCH] BaseTools/GCC5: set -Wno-unused-const-variables on RELEASE builds

[Thomas Lamprecht]

TianoCore BZ#700 

This fixes the RELEASE build of OVMF with GCC in version 6 or newer.
GCC 6 added the '-Wunused-const-variable' warning, which gets
activated by '-Wunused-variable' and has the following behavior:
"Warn whenever a constant static variable is unused aside from its
declaration" [1]

Commit 2ad6ba80a1bd58382bde6b994070f7c01d2fb48d introduced a case
where exactly this happens on a RELEASE build. All uses of the static
const variable are located in debug code only, which gets thrown out
by the compiler on RELEASE builds and thus triggers the
unused-const-variable warning.

There is currently no GCC 6 toolchain target defined and doing so
would add a lot of boilerplate code. Instead, use the fact that GCC
ignores unkown '-Wno-*' options:

"[...] if the -Wno- form is used [...] no diagnostic is produced for
-Wno-unknown-warning unless other diagnostics are being produced"

This behavior is available in GCC 5 [2] (and also earlier, for that
matter), so add the flag to the GCC5 toolchain, even if GCC 5 itself
does not supports it.

Orient the changes on 20d00edf21d2 which moved the
'-Wno-unused-but-set-variable' flag to RELEASE builds only, as there
it ensure that it does not gets raised if the only usage of a
variable is in (then collapsed) debug code.

[1] 
https://gcc.gnu.org/onlinedocs/gcc-6.4.0/gcc/Warning-Options.html#index-Wunused-const-variable
[2] https://gcc.gnu.org/onlinedocs/gcc-5.4.0/gcc/Warning-Options.html

Cc: Yonghong Zhu 
Cc: Liming Gao 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Thomas Lamprecht 
---

I hope I CCed the correct people and got the style right :)

 BaseTools/Conf/tools_def.template | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index ba1d1a16de..7db7a174c3 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -5326,7 +5326,7 @@ RELEASE_GCC49_AARCH64_DLINK_FLAGS  = 
DEF(GCC49_AARCH64_DLINK_FLAGS)
   DEBUG_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os
   DEBUG_GCC5_IA32_DLINK_FLAGS= DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os 
-Wl,-m,elf_i386,--oformat=elf32-i386
 
-RELEASE_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os 
-Wno-unused-but-set-variable
+RELEASE_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -flto -Os 
-Wno-unused-but-set-variable -Wno-unused-const-variable
 RELEASE_GCC5_IA32_DLINK_FLAGS= DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os 
-Wl,-m,elf_i386,--oformat=elf32-i386
 
   NOOPT_GCC5_IA32_CC_FLAGS   = DEF(GCC5_IA32_CC_FLAGS) -O0
@@ -5358,7 +5358,7 @@ RELEASE_GCC5_IA32_DLINK_FLAGS= 
DEF(GCC5_IA32_X64_DLINK_FLAGS) -flto -Os -Wl,
   DEBUG_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO -Os
   DEBUG_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
 
-RELEASE_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
-Os -Wno-unused-but-set-variable
+RELEASE_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -flto -DUSING_LTO 
-Os -Wno-unused-but-set-variable -Wno-unused-const-variable
 RELEASE_GCC5_X64_DLINK_FLAGS = DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
 
   NOOPT_GCC5_X64_CC_FLAGS= DEF(GCC5_X64_CC_FLAGS) -O0
@@ -5393,7 +5393,7 @@ RELEASE_GCC5_X64_DLINK_FLAGS = 
DEF(GCC5_X64_DLINK_FLAGS) -flto -Os
   DEBUG_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -O0
   DEBUG_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS)
 
-RELEASE_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -flto 
-Wno-unused-but-set-variable
+RELEASE_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -flto 
-Wno-unused-but-set-variable -Wno-unused-const-variable
 RELEASE_GCC5_ARM_DLINK_FLAGS = DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os 
-L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-arm 
-Wl,-plugin-opt=-pass-through=-llto-arm
 
   NOOPT_GCC5_ARM_CC_FLAGS= DEF(GCC5_ARM_CC_FLAGS) -O0
@@ -5428,7 +5428,7 @@ RELEASE_GCC5_ARM_DLINK_FLAGS = 
DEF(GCC5_ARM_DLINK_FLAGS) -flto -Os -L$(WORKS
   DEBUG_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -z 
common-page-size=0x1000
   DEBUG_GCC5_AARCH64_DLINK_XIPFLAGS = -z common-page-size=0x20
 
-RELEASE_GCC5_AARCH64_CC_FLAGS= DEF(GCC5_AARCH64_CC_FLAGS) -flto 
-Wno-unused-but-set-variable -mcmodel=tiny -fomit-frame-pointer
+RELEASE_GCC5_AARCH64_CC_FLAGS= DEF(GCC5_AARCH64_CC_FLAGS) -flto 
-Wno-unused-but-set-variable -Wno-unused-const-variable -mcmodel=tiny 
-fomit-frame-pointer
 RELEASE_GCC5_AARCH64_DLINK_FLAGS = DEF(GCC5_AARCH64_DLINK_FLAGS) -flto -Os 
-L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 
-Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wno-lto-type-mismatch
 
   NOOPT_GCC5_AARCH64_CC_FLAGS= DEF(GCC5_AARCH64_CC_FLAGS) -O0 
-mcmodel=small
-- 
2.11.0


___

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Yao, Jiewen]

Great. Thanks to confirm that. Now it is clear to me.

Then let's wait Laszlo's new patch to make all DMA buffer to private. :)

Thank you
Yao, Jiewen

From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Brijesh 
Singh
Sent: Thursday, September 7, 2017 10:40 PM
To: Laszlo Ersek ; Yao, Jiewen ; Zeng, 
Star ; edk2-devel-01 
Cc: brijesh.si...@amd.com; Dong, Eric 
Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common 
buffers at ExitBootServices()

Hi Jiewen,

On 09/07/2017 06:46 AM, Laszlo Ersek wrote:
> On 09/07/17 06:46, Yao, Jiewen wrote:
>> Thanks for the sharing, Brijesh.
>>
>> "If a page was marked as "shared"
>> then its guest responsibility to make it "private" after its done 
>> communicating with
>> hypervisor."
>>
>> I believe I have same understanding - a *guest* should guarantee that.
>>
>> My question is: During the *transition* from firmware to OS, *which guest* 
>> should make the shared buffer to be private? Is it "guest firmware" or 
>> "guest OS"?
>>
>> Maybe I can ask the specific question to get it more clear.
>>
>> 1)   If the common DMA buffer is not unmapped at ExitBootService, is 
>> that treated as an issue?
>>
>> 2)   If the read/write DMA buffer is not unmapped at ExitBootService, is 
>> that treated as an issue?
>
> Very good questions, totally to the point.
>
> On the authoritative answer, I defer to Brijesh.
>


Both the above cases (#1 and #2) are problems. Since buffers was owned by 
firmware
and firmware made it "shared" hence firmware is responsible to mark them as 
private
after its done with the buffer. In other words, we must call Unmap() from 
ExitBootServices
to ensure that buffers mapped using 
BusMasterCommonBuffer/BusMasterRead/BusMasterWrite
is marked as "private" before we make it available to the guest OS. (we do 
similar thing
in Linux OS).

Having any kind of side channel to communicate the encryption status of a page
will not work -- we should be able to support a usecase where we boot OVMF in
64-bit but launch 32-bit Linux guest. When Linux boots in 32-bit mode it does 
not have
access to encryption bit (C-bit is bit-47 in page table) and can't mark the 
page as
private (even if we provide some kind of side-channel).

thank you very much for all your help.

> (
>
> My personal opinion is that both situations (#1 and #2) are problems,
> because they break the *practical* security invariant for SEV guests:
>
> - most memory should be encrypted at all times, *and*
>
> - any memory that is decrypted must have an owner that is responsible
>for re-encrypting the memory eventually.
>
> Therefore, *either* the firmware has to re-encrypt all remaining DMA
> buffers at ExitBootServices(), *or* a new information channel must be
> designed, from firmware to OS, to carry over the decryption status.
>
> I strongly prefer the first option, for the following reason: the same
> questions apply to all EDK2 IOMMU protocol interfaces, not just the one
> exported by the SEV driver.
>
> )
>
> Thanks,
> Laszlo
>
>> From: Brijesh Singh [mailto:brijesh.si...@amd.com]
>> Sent: Wednesday, September 6, 2017 11:40 PM
>> To: Laszlo Ersek >; Yao, Jiewen 
>> >; Zeng, Star 
>> >; edk2-devel-01 
>> >
>> Cc: brijesh.si...@amd.com; Dong, Eric 
>> >
>> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap 
>> common buffers at ExitBootServices()
>>
>>
>>
>> On 09/06/2017 07:14 AM, Laszlo Ersek wrote:
>>> On 09/06/17 06:37, Yao, Jiewen wrote:
 Thanks for the clarification. Comment in line.

 From: Laszlo Ersek [mailto:ler...@redhat.com]
 Sent: Wednesday, September 6, 2017 1:57 AM
 To: Yao, Jiewen 
 >>;
  Zeng, Star 
 >>;
  edk2-devel-01 
 >>
 Cc: Dong, Eric 
 >>;
  Brijesh Singh 
 >>
 Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap 
 common buffers at ExitBootServices()
>>>
> Then after ExitBootService, the OS will take control of CR3 and set 
> correct
> encryption bit.

 This is true, the guest OS 

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Brijesh Singh]

Hi Jiewen,

On 09/07/2017 06:46 AM, Laszlo Ersek wrote:

On 09/07/17 06:46, Yao, Jiewen wrote:

Thanks for the sharing, Brijesh.

"If a page was marked as "shared"
then its guest responsibility to make it "private" after its done communicating 
with
hypervisor."

I believe I have same understanding - a *guest* should guarantee that.

My question is: During the *transition* from firmware to OS, *which guest* should make the shared 
buffer to be private? Is it "guest firmware" or "guest OS"?

Maybe I can ask the specific question to get it more clear.

1)   If the common DMA buffer is not unmapped at ExitBootService, is that 
treated as an issue?

2)   If the read/write DMA buffer is not unmapped at ExitBootService, is 
that treated as an issue?


Very good questions, totally to the point.

On the authoritative answer, I defer to Brijesh.




Both the above cases (#1 and #2) are problems. Since buffers was owned by 
firmware
and firmware made it "shared" hence firmware is responsible to mark them as 
private
after its done with the buffer. In other words, we must call Unmap() from 
ExitBootServices
to ensure that buffers mapped using 
BusMasterCommonBuffer/BusMasterRead/BusMasterWrite
is marked as "private" before we make it available to the guest OS. (we do 
similar thing
in Linux OS).

Having any kind of side channel to communicate the encryption status of a page
will not work -- we should be able to support a usecase where we boot OVMF in
64-bit but launch 32-bit Linux guest. When Linux boots in 32-bit mode it does 
not have
access to encryption bit (C-bit is bit-47 in page table) and can't mark the 
page as
private (even if we provide some kind of side-channel).

thank you very much for all your help.


(

My personal opinion is that both situations (#1 and #2) are problems,
because they break the *practical* security invariant for SEV guests:

- most memory should be encrypted at all times, *and*

- any memory that is decrypted must have an owner that is responsible
   for re-encrypting the memory eventually.

Therefore, *either* the firmware has to re-encrypt all remaining DMA
buffers at ExitBootServices(), *or* a new information channel must be
designed, from firmware to OS, to carry over the decryption status.

I strongly prefer the first option, for the following reason: the same
questions apply to all EDK2 IOMMU protocol interfaces, not just the one
exported by the SEV driver.

)

Thanks,
Laszlo


From: Brijesh Singh [mailto:brijesh.si...@amd.com]
Sent: Wednesday, September 6, 2017 11:40 PM
To: Laszlo Ersek ; Yao, Jiewen ; Zeng, Star 
; edk2-devel-01 
Cc: brijesh.si...@amd.com; Dong, Eric 
Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common 
buffers at ExitBootServices()



On 09/06/2017 07:14 AM, Laszlo Ersek wrote:

On 09/06/17 06:37, Yao, Jiewen wrote:

Thanks for the clarification. Comment in line.

From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Wednesday, September 6, 2017 1:57 AM
To: Yao, Jiewen >; Zeng, Star 
>; edk2-devel-01 
>
Cc: Dong, Eric >; Brijesh Singh 
>
Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common 
buffers at ExitBootServices()



Then after ExitBootService, the OS will take control of CR3 and set correct
encryption bit.


This is true, the guest OS will set up new page tables, and in those
PTEs, the C-bit ("encrypt") will likely be set by default.

However, the guest OS will not *rewrite* all of the memory, with the
C-bit set. This means that any memory that the firmware didn't actually
re-encrypt (in the IOMMU driver) will still expose stale data to the
hypervisor.
[Jiewen] That is an interesting question.
Is there any security concern associated?


I can't tell for sure. Answering this question is up to the proponents
of SEV, who have designed the threat model for SEV.

My operating assumption is that we should keep memory as tightly
encrypted as possible at the firmware --> OS control transfer. It could
be an exaggerated expectation from my side; I'd just better be safe than
sorry :)




Let me give some brief intro on SEV (Secure Encrypted Virtualization) and then
we can discuss a security model (if needed).

SEV is an extension to the AMD-V architecture which supports running encrypted
virtual machines (VMs) under the control of a hypervisor. Encrypted VMs have 
their
pages (code and data) secured such that only the guest itself has access to the
unencrypted version. Each encrypted VMs is associated with a unique encryption
key; if its data is accessed by a different entity using a different key the
encrypted guest data will be incorrectly 

Re: [edk2] [PATCH v3 1/2] MMC : Added missing __FUNCTION__ macro.

[Leif Lindholm]

On Thu, Sep 07, 2017 at 07:47:52PM +0530, Meenakshi Aggarwal wrote:
> We want to print name of the function resulted in error,
> but __FUNCTION__ macro was missing.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Meenakshi Aggarwal 

Like for 2/2, you appear to have left out edk2-devel@lists.01.org in
this submission (added).

Like for 2/2, happy to push this if you can confirm you are
contributing under TianoCore Contribution Agreement 1.1.

/
Leif

> ---
>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 14 +++---
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
> b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> index 7f74c54..27e 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> @@ -388,12 +388,12 @@ InitializeSdMmcDevice (
>  
>Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);
>if (EFI_ERROR (Status)) {
> -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n", Status));
> +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", 
> __FUNCTION__, Status));
>  return Status;
>}
>Status = MmcHost->ReceiveResponse (MmcHost, MMC_RESPONSE_TYPE_R1, 
> Response);
>if (EFI_ERROR (Status)) {
> -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n", Status));
> +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", 
> __FUNCTION__, Status));
>  return Status;
>}
>if ((Response[0] & MMC_STATUS_APP_CMD) == 0) {
> @@ -445,12 +445,12 @@ InitializeSdMmcDevice (
>  CmdArg |= 1 << (0 * 4);
>  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
>  if (EFI_ERROR (Status)) {
> -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n", Status));
> +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n", 
> __FUNCTION__, Status));
> return Status;
>  } else {
>Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);
>if (EFI_ERROR (Status)) {
> -DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): ReadBlockData Error and Status = 
> %r\n", Status));
> +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error and Status 
> = %r\n", __FUNCTION__, Status));
>  return Status;
>}
>  }
> @@ -459,20 +459,20 @@ InitializeSdMmcDevice (
>  CmdArg = MmcHostInstance->CardInfo.RCA << 16;
>  Status = MmcHost->SendCommand (MmcHost, MMC_CMD55, CmdArg);
>  if (EFI_ERROR (Status)) {
> -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD55): Error and Status = %r\n", 
> Status));
> +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD55): Error and Status = %r\n", 
> __FUNCTION__, Status));
>return Status;
>  }
>  /* Width: 4 */
>  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, 2);
>  if (EFI_ERROR (Status)) {
> -  DEBUG ((EFI_D_ERROR, "%a(MMC_CMD6): Error and Status = %r\n", Status));
> +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n", 
> __FUNCTION__, Status));
>return Status;
>  }
>}
>if (MMC_HOST_HAS_SETIOS(MmcHost)) {
>  Status = MmcHost->SetIos (MmcHost, 26 * 1000 * 1000, 4, EMMCBACKWARD);
>  if (EFI_ERROR (Status)) {
> -  DEBUG ((EFI_D_ERROR, "%a(SetIos): Error and Status = %r\n", Status));
> +  DEBUG ((DEBUG_ERROR, "%a (SetIos): Error and Status = %r\n", 
> __FUNCTION__, Status));
>return Status;
>  }
>}
> -- 
> 1.9.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v3 2/2] SD : Updated CMD 6 implememtation.

[Leif Lindholm]

On Thu, Sep 07, 2017 at 07:47:53PM +0530, Meenakshi Aggarwal wrote:
> For setting high speed in SD card,
> First CMD 6 (Switch) is send to check if card supports High Speed and
> Second command is send to switch card to high speed mode.
> 
> In current inplementation, CMD 6 was sent only once to switch the
> card into HS mode without checking if card supports HS or not, which is
> not as per specification and also we are not setting the HS i.e. 5000
> but directly asking the card to switch to 2600 which is incorrect as
> SD card supports either 2500 or 5000.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0

Ok, I would say this one is good to go, but can you please confirm
if you are happy to contribute under TianoCore Contribution Agreement
1.1, which is the version currently described in Contributions.txt?

If so, I can fold in this change and push.

Oh, you also appear to have left out edk2-devel@lists.01.org in this
submission (added).

Best Regards,

Leif

> Signed-off-by: Meenakshi Aggarwal 
> ---
>  EmbeddedPkg/Universal/MmcDxe/Mmc.h   |  8 
>  EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c | 53 
> +---
>  2 files changed, 56 insertions(+), 5 deletions(-)
>  mode change 100644 => 100755 EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> 
> diff --git a/EmbeddedPkg/Universal/MmcDxe/Mmc.h 
> b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> index f3e56ff..a77ba41 100644
> --- a/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> +++ b/EmbeddedPkg/Universal/MmcDxe/Mmc.h
> @@ -64,6 +64,14 @@
>  #define EMMC_CMD6_ARG_VALUE(x)  (((x) & 0xFF) << 8)
>  #define EMMC_CMD6_ARG_CMD_SET(x)(((x) & 0x7) << 0)
>  
> +#define SWITCH_CMD_DATA_LENGTH  64
> +#define SD_HIGH_SPEED_SUPPORTED 0x2
> +#define SD_DEFAULT_SPEED2500
> +#define SD_HIGH_SPEED   5000
> +#define SWITCH_CMD_SUCCESS_MASK 0x0f00
> +
> +#define BUSWIDTH_4  4
> +
>  typedef enum {
>UNKNOWN_CARD,
>MMC_CARD,  //MMC card
> diff --git a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c 
> b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> old mode 100644
> new mode 100755
> index 27e..f661a0c
> --- a/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> +++ b/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c
> @@ -318,6 +318,23 @@ InitializeEmmcDevice (
>  }
>  
>  STATIC
> +UINT32
> +CreateSwitchCmdArgument (
> +  IN  UINT32 Mode,
> +  IN  UINT8  Group,
> +  IN  UINT8  Value
> +  )
> +{
> +  UINT32 Argument;
> +
> +  Argument = Mode << 31 | 0x00FF;
> +  Argument &= ~(0xF << (Group * 4));
> +  Argument |= Value << (Group * 4);
> +
> +  return Argument;
> +}
> +
> +STATIC
>  EFI_STATUS
>  InitializeSdMmcDevice (
>IN  MMC_HOST_INSTANCE   *MmcHostInstance
> @@ -326,6 +343,7 @@ InitializeSdMmcDevice (
>UINT32CmdArg;
>UINT32Response[4];
>UINT32Buffer[128];
> +  UINT32Speed;
>UINTN BlockSize;
>UINTN CardSize;
>UINTN NumBlocks;
> @@ -334,6 +352,7 @@ InitializeSdMmcDevice (
>EFI_STATUSStatus;
>EFI_MMC_HOST_PROTOCOL *MmcHost;
>  
> +  Speed = SD_DEFAULT_SPEED;
>MmcHost = MmcHostInstance->MmcHost;
>  
>// Send a command to get Card specific data
> @@ -439,20 +458,44 @@ InitializeSdMmcDevice (
>  }
>}
>if (CccSwitch) {
> +/* SD Switch, Mode:0, Group:0, Value:0 */
> +CmdArg = CreateSwitchCmdArgument(0, 0, 0);
> +Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
> +if (EFI_ERROR (Status)) {
> +  DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n", 
> __FUNCTION__, Status));
> +   return Status;
> +} else {
> +  Status = MmcHost->ReadBlockData (MmcHost, 0, SWITCH_CMD_DATA_LENGTH, 
> Buffer);
> +  if (EFI_ERROR (Status)) {
> +DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): ReadBlockData Error and Status 
> = %r\n", __FUNCTION__, Status));
> +return Status;
> +  }
> +}
> +
> +if (!(Buffer[3] & SD_HIGH_SPEED_SUPPORTED)) {
> +  DEBUG ((DEBUG_ERROR, "%a : High Speed not supported by Card %r\n", 
> __FUNCTION__, Status));
> +  return Status;
> +}
> +
> +Speed = SD_HIGH_SPEED;
> +
>  /* SD Switch, Mode:1, Group:0, Value:1 */
> -CmdArg = 1 << 31 | 0x00FF;
> -CmdArg &= ~(0xF << (0 * 4));
> -CmdArg |= 1 << (0 * 4);
> +CmdArg = CreateSwitchCmdArgument(1, 0, 1);
>  Status = MmcHost->SendCommand (MmcHost, MMC_CMD6, CmdArg);
>  if (EFI_ERROR (Status)) {
>DEBUG ((DEBUG_ERROR, "%a (MMC_CMD6): Error and Status = %r\n", 
> __FUNCTION__, Status));
> return Status;
>  } else {
> -  Status = MmcHost->ReadBlockData (MmcHost, 0, 64, Buffer);
> +  Status = MmcHost->ReadBlockData (MmcHost, 0, SWITCH_CMD_DATA_LENGTH, 
> Buffer);
>if (EFI_ERROR (Status)) {
>  DEBUG ((DEBUG_ERROR, "%a 

Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

[Paulo Alcantara]

Laszlo,

On 07/09/2017 08:35, Laszlo Ersek wrote:

Paulo,

On 09/07/17 02:23, Ni, Ruiyu wrote:

Sure. Please.


Before pushing the series, I wanted to slightly regression-test OVMF and
ArmVirtQemu with it.

However, I've found that the series doesn't build for OVMF IA32. I get
an error like this:


MdeModulePkg/Universal/Disk/UdfDxe/File.c: In function 'UdfRead':
MdeModulePkg/Universal/Disk/UdfDxe/File.c:376:7: error: passing argument 8 of 
'ReadFileData' from incompatible pointer type [-Werror]
);
^
In file included from MdeModulePkg/Universal/Disk/UdfDxe/File.c:15:0:
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h:1008:1: note: expected 'UINT64 *' but 
argument is of type 'UINTN *'
  ReadFileData (


There could be further such errors (I didn't check, the build stopped
here).

Can you please submit a v5 that is 32-bit and 64-bit clean?


Sure. Thanks for catching that up. I'll also make sure to re-test it 
after the changes.




I expect that any changes you make will be in the first three patches
somewhere. If they are not trivial, please drop Ray's Reviewed-by from
the affected patches so he can review them again.

I'm not sure if Ray uses any tools for supporting incremental review;
you could help him by including a cumulative diff between v4 and v5 in
the v5 blurb.


OK.

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

[edk2] [staging/Intel_UNDI]: Creation of new branch (Intel_UNDI) and initial commit

[Rabeda, Maciej]

Hi edk2-devel members,

As per agreement with Michael D. Kinney 
(michael.d.kin...@intel.com), Intel PreBoot 
team has submitted a new feature branch to edk2-staging repository.
Intel_UNDI branch introduces open-source UEFI UNDI Drivers code supporting 
Intel(r) server Ethernet adapters to EDK2.
Branch also contains Readme.MD with necessary details, branch owner and issues 
to be fixed before incorporating the drivers code to master branch.

Thanks!
Maciej Rabeda
DCG CG ND SW ITP PreBoot Dev



Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] BaseTools: correct to use specific arch to generate DSC database

[Yonghong Zhu]

Not generic to use 'Common' arch, but use current build arch.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py 
b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
index fe2c7c1..b617221 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py
@@ -3107,11 +3107,11 @@ determine whether database file is out of date!\n")
 
 ## Summarize all packages in the database
 def GetPackageList(self, Platform, Arch, TargetName, ToolChainTag):
 self.Platform = Platform
 PackageList = []
-Pa = self.BuildObject[self.Platform, 'COMMON']
+Pa = self.BuildObject[self.Platform, Arch]
 #
 # Get Package related to Modules
 #
 for Module in Pa.Modules:
 ModuleObj = self.BuildObject[Module, Arch, TargetName, 
ToolChainTag]
-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Laszlo Ersek]

On 09/07/17 06:34, Yao, Jiewen wrote:
> Thanks.
> It seems the discussion becomes too long. I try to make it short.
> 
> 
> 1)   As long as we use same mechanism to handle both common buffer and 
> read/write buffer. I have no concern at all. :)
> 
> 
> 2)   I am sorry that I do not have an immediate answer for your question 
> on event handling in ExitBootServices.
> Although it seems tricky, I believe it works. Can you have a try?
> 
> 
> 3)   Looking at the code (DxeMain.c), I have another idea for your 
> consideration only.
> 
>   //
>   // Notify other drivers that we are exiting boot services.
>   //
>   CoreNotifySignalList ();
> 
>   //
>   // Report that ExitBootServices() has been called
>   //
>   REPORT_STATUS_CODE (
> EFI_PROGRESS_CODE,
> (EFI_SOFTWARE_EFI_BOOT_SERVICE | EFI_SW_BS_PC_EXIT_BOOT_SERVICES)
> );
> 
> The cleanup driver may register a report status code handle to do the 
> cleanup. :)
> 
> We already have such example in 
> MdeModulePkg\Universal\Acpi\FirmwarePerformanceDataTableDxe\ 
> FirmwarePerformanceDxe.c.
> FpdtStatusCodeListenerDxe()
> 
>   } else if (Value == (EFI_SOFTWARE_EFI_BOOT_SERVICE | 
> EFI_SW_BS_PC_EXIT_BOOT_SERVICES)) {
> ..
> 
> The requirement will become: If a platform wants to add cleanup, it must use 
> a real report status code library.

I think I prefer option #2; it presents fewer requirements for the platform.

In addition: I've just realized that we don't need the UEFI spec to
promise us that option #2 works. We only have to care whether it works
in edk2.

The reason is that EDKII_IOMMU_PROTOCOL is *already* edk2-specific; it
is a platform (DXE) driver, not a UEFI driver. In other words, we would
use option #2 only inside edk2.

So, I'll try option #2.

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

Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices()

[Laszlo Ersek]

On 09/07/17 06:46, Yao, Jiewen wrote:
> Thanks for the sharing, Brijesh.
> 
> "If a page was marked as "shared"
> then its guest responsibility to make it "private" after its done 
> communicating with
> hypervisor."
> 
> I believe I have same understanding - a *guest* should guarantee that.
> 
> My question is: During the *transition* from firmware to OS, *which guest* 
> should make the shared buffer to be private? Is it "guest firmware" or "guest 
> OS"?
> 
> Maybe I can ask the specific question to get it more clear.
> 
> 1)   If the common DMA buffer is not unmapped at ExitBootService, is that 
> treated as an issue?
> 
> 2)   If the read/write DMA buffer is not unmapped at ExitBootService, is 
> that treated as an issue?

Very good questions, totally to the point.

On the authoritative answer, I defer to Brijesh.

(

My personal opinion is that both situations (#1 and #2) are problems,
because they break the *practical* security invariant for SEV guests:

- most memory should be encrypted at all times, *and*

- any memory that is decrypted must have an owner that is responsible
  for re-encrypting the memory eventually.

Therefore, *either* the firmware has to re-encrypt all remaining DMA
buffers at ExitBootServices(), *or* a new information channel must be
designed, from firmware to OS, to carry over the decryption status.

I strongly prefer the first option, for the following reason: the same
questions apply to all EDK2 IOMMU protocol interfaces, not just the one
exported by the SEV driver.

)

Thanks,
Laszlo

> From: Brijesh Singh [mailto:brijesh.si...@amd.com]
> Sent: Wednesday, September 6, 2017 11:40 PM
> To: Laszlo Ersek ; Yao, Jiewen ; 
> Zeng, Star ; edk2-devel-01 
> Cc: brijesh.si...@amd.com; Dong, Eric 
> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap 
> common buffers at ExitBootServices()
> 
> 
> 
> On 09/06/2017 07:14 AM, Laszlo Ersek wrote:
>> On 09/06/17 06:37, Yao, Jiewen wrote:
>>> Thanks for the clarification. Comment in line.
>>>
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Wednesday, September 6, 2017 1:57 AM
>>> To: Yao, Jiewen >; Zeng, 
>>> Star >; edk2-devel-01 
>>> >
>>> Cc: Dong, Eric >; Brijesh 
>>> Singh >
>>> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap 
>>> common buffers at ExitBootServices()
>>
 Then after ExitBootService, the OS will take control of CR3 and set correct
 encryption bit.
>>>
>>> This is true, the guest OS will set up new page tables, and in those
>>> PTEs, the C-bit ("encrypt") will likely be set by default.
>>>
>>> However, the guest OS will not *rewrite* all of the memory, with the
>>> C-bit set. This means that any memory that the firmware didn't actually
>>> re-encrypt (in the IOMMU driver) will still expose stale data to the
>>> hypervisor.
>>> [Jiewen] That is an interesting question.
>>> Is there any security concern associated?
>>
>> I can't tell for sure. Answering this question is up to the proponents
>> of SEV, who have designed the threat model for SEV.
>>
>> My operating assumption is that we should keep memory as tightly
>> encrypted as possible at the firmware --> OS control transfer. It could
>> be an exaggerated expectation from my side; I'd just better be safe than
>> sorry :)
>>
>>
> 
> Let me give some brief intro on SEV (Secure Encrypted Virtualization) and then
> we can discuss a security model (if needed).
> 
> SEV is an extension to the AMD-V architecture which supports running encrypted
> virtual machines (VMs) under the control of a hypervisor. Encrypted VMs have 
> their
> pages (code and data) secured such that only the guest itself has access to 
> the
> unencrypted version. Each encrypted VMs is associated with a unique encryption
> key; if its data is accessed by a different entity using a different key the
> encrypted guest data will be incorrectly decrypted, leading to unintelligible 
> data.
> You can also find some detail for SEV in white paper [1].
> 
> SEV encrypted Vs have the concept of private and shared memory. The private 
> memory
> is encrypted with the guest-specific key, while shared memory may be encrypted
> with hypervisor key. SEV guest VMs can choose which pages they would like to
> be private. But the instruction pages and guest page tables are always treated
> as private by the hardware. The DMA operation inside the guest must be 
> performed
> on shared pages -- this is mainly because in virtualization world the device
> DMA needs some assistance from the hypervisor.
> 
> The security model provided by the SEV ensure that hypervisor will no longer 
> able
> to 

Re: [edk2] [PATCH v4 0/6] read-only UDF file system support

[Laszlo Ersek]

Paulo,

On 09/07/17 02:23, Ni, Ruiyu wrote:
> Sure. Please.

Before pushing the series, I wanted to slightly regression-test OVMF and
ArmVirtQemu with it.

However, I've found that the series doesn't build for OVMF IA32. I get
an error like this:

> MdeModulePkg/Universal/Disk/UdfDxe/File.c: In function 'UdfRead':
> MdeModulePkg/Universal/Disk/UdfDxe/File.c:376:7: error: passing argument 8 of 
> 'ReadFileData' from incompatible pointer type [-Werror]
>);
>^
> In file included from MdeModulePkg/Universal/Disk/UdfDxe/File.c:15:0:
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h:1008:1: note: expected 'UINT64 *' 
> but argument is of type 'UINTN *'
>  ReadFileData (

There could be further such errors (I didn't check, the build stopped
here).

Can you please submit a v5 that is 32-bit and 64-bit clean?

I expect that any changes you make will be in the first three patches
somewhere. If they are not trivial, please drop Ray's Reviewed-by from
the affected patches so he can review them again.

I'm not sure if Ray uses any tools for supporting incremental review;
you could help him by including a cumulative diff between v4 and v5 in
the v5 blurb.

Thanks!
Laszlo



> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, September 6, 2017 10:28 PM
> To: Ni, Ruiyu ; Zeng, Star 
> Cc: Paulo Alcantara ; edk2-devel@lists.01.org; Dong, Eric 
> ; Wu, Hao A ; Justen, Jordan L 
> ; Andrew Fish ; Gao, Liming 
> ; Kinney, Michael D 
> Subject: Re: [edk2] [PATCH v4 0/6] read-only UDF file system support
>
> Ray, Star,
>
> On 08/24/17 19:56, Paulo Alcantara wrote:
>
>> v2:
>>  - Rework to _partially_ support UDF revisions <2.60.
>>  - Use existing CDROM_VOLUME_DESCRIPTOR structure defined in Eltorito.h
>>instead of creating another one (UDF_VOLUME_DESCRIPTOR).
>>  - Fixed UdfDxe to correctly follow UEFI driver model.
>>  - Use HARDDRIVE_DEVICE_PATH instead of a vendor-defined one.
>>  - Detect UDF file systems only in PartitionDxe, and let UdfDxe driver
>>check for specific UDF device path to decide whether or not install
>>SimpleFs protocol.
>>  - Place MdePkg changes in a separate patch.
>> v3:
>>  - Install UDF partition child handles with a Vendor-Defined Media
>>Device Path.
>>  - Changed UdfDxe to check for Vendor-Defined Media Device Paths with a
>>specific UDF file system GUID when determining to whether or not
>>start the driver.
>>  - Removed leading TAB chars in some source files identified by
>>PatchCheck.py tool.
>> v4:
>>  - Added missing R-b's.
>
> Looks like the series is now fully reviewed (with the Mde*Pkg patches having 
> Ray's R-b). Are you guys OK if I push the v4 set?
>
> 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] ASSERT in QemuVideoDxe driver during reset

[Yao, Jiewen]

Agree. We should not break Window7.
Let’s see the result.

Thank you
Yao Jiewen

From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Thursday, September 7, 2017 6:58 PM
To: Yao, Jiewen ; Wang, Jian J ; 
Justen, Jordan L 
Cc: edk2-devel@lists.01.org; Kinney, Michael D 
Subject: Re: ASSERT in QemuVideoDxe driver during reset

On 09/07/17 03:28, Yao, Jiewen wrote:
> OK. Got you.
>
> I think we can move the zero memory to DxeIpl.
> The DxeIpl can check if the 0 is allocated in MemoryAllocationHob, and zero 
> it if it is not allocated.

Regarding the zeroing out of page 0, this solution seems good to me as
well. (I agree that page 0 should be zeroed out even if the null pointer
detection is enabled.)

Regarding the question whether QemuVideoDxe should, or should not,
manually change the access permissions to page 0, before and after
setting up the Int0x10 vector, if null pointer detection is enabled:

This depends on Windows 7. If Windows 7 can boot fine with the null
pointer detection enabled, then I agree QemuVideoDxe should be updated
as described above. (For massaging the page protection, it should use
DXE services, not CpuArch services.) In this case it is also fine for
OVMF to inherit the default TRUE setting for the PCD.

If Windows 7 cannot boot with null pointer detection enabled, then OVMF
should set the PCD to FALSE, and QemuVideoDxe should be changed to not
call InstallVbeShim() at all if the PCD is TRUE.

Thanks
Laszlo

> From: Wang, Jian J
> Sent: Thursday, September 7, 2017 8:41 AM
> To: Yao, Jiewen >; Laszlo 
> Ersek >; Justen, Jordan L 
> >
> Cc: edk2-devel@lists.01.org; Kinney, Michael 
> D >
> Subject: RE: ASSERT in QemuVideoDxe driver during reset
>
> Hi Jiewen and Laszlo,
>
> According to both of your comments:
>
>
> 1.   VBE SHIM must be installed to avoid boot failure of Windows 7.
>
> 2.   Enabling NULL detection should not break existing platforms.
>
> Let me clarify a few things in advance. I think there’s a little 
> misunderstanding on this issue (Sorry for my poor description originally).
>
> 1.   NULL detection is implemented by disable first 4K page (0-4095). Let 
> me call it page 0.
>
> 2.   The ASSERT was not caused by accessing page 0. I do enabling page 0 
> before access int10 vector in QemuVideoDxe driver while NULL detection is 
> enabled.
>
> 3.  The ASSERT will only be triggered by reset because QemuVideoDxe will 
> write int10 vector and memory at that place keeps intact during reset.
>
> 4.  The root cause of ASSERT is the fact that page 0 is always cleared in DXE 
> core except to NULL detection enabled. I changed CoreAddRange() in page.c in 
> DXC core like below to avoid page fault in core.
> //
>   // If memory of type EfiConventionalMemory is being added that includes the 
> page
>   // starting at address 0, then zero the page starting at address 0.  This 
> has
>   // two benifits.  It helps find NULL pointer bugs and it also maximizes
>   // compatibility with operating systems that may evaluate memory in this 
> page
>   // for legacy data structures.  If memory of any other type is added 
> starting
>   // at address 0, then do not zero the page at address 0 because the page is 
> being
>   // used for other purposes.
>   //
>   if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 
> 1)) {
> if ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) == 0) {
>   SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0);
> }
>   }
>
> I think the best solution at present is to update above code to make sure 
> page 0 is always cleared no matter NULL detection is enable or not. To make 
> it possible, I have to enable page 0 before memory clearing and disable it 
> again after then. My concern is that I cannot use CPU arch protocol to do so 
> in CoreAddRange() function because it’s not ready at that time. There’re two 
> options here. One is “manually” disabling page 0 (read cr3, then pm4l, then 
> pde…) in above code; another is moving above code to another place where 
> paging operation is at hand, like DxeIpl or CPU driver. My personal opinion 
> is second one would be better. What’s your opinions? Or any better ideas?
>
> Thanks,
> Wang, Jian J
>
> From: Yao, Jiewen
> Sent: Wednesday, September 06, 2017 7:17 PM
> To: Laszlo Ersek 
> >>;
>  Wang, Jian J 
> >>;
>  Justen, Jordan L 
> 

[edk2] [PATCH] MdeModulePkg Xhci: Correct description of Timeout param in XhciReg.h

[Star Zeng]

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=653

Correct description of Timeout param in XhciReg.h to be matched with
XhciReg.c.

Cc: Alexei Fedorov 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h | 10 +-
 MdeModulePkg/Bus/Pci/XhciPei/XhciReg.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h 
b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
index b748c8d39739..838a44628c27 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.h
@@ -2,7 +2,7 @@
 
   This file contains the register definition of XHCI host controller.
 
-Copyright (c) 2011 - 2013, Intel Corporation. All rights reserved.
+Copyright (c) 2011 - 2017, 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
@@ -399,7 +399,7 @@ XhcClearOpRegBit (
   @param  Offset   The offset of the operational register.
   @param  Bit  The bit of the register to wait for.
   @param  WaitToSetWait the bit to set or clear.
-  @param  Timeout  The time to wait before abort (in microsecond, us).
+  @param  Timeout  The time to wait before abort (in millisecond, ms).
 
   @retval EFI_SUCCESS  The bit successfully changed by host controller.
   @retval EFI_TIMEOUT  The time out occurred.
@@ -521,7 +521,7 @@ XhcIsSysError (
   Reset the XHCI host controller.
 
   @param  Xhc  The XHCI Instance.
-  @param  Timeout  Time to wait before abort (in microsecond, us).
+  @param  Timeout  Time to wait before abort (in millisecond, ms).
 
   @retval EFI_SUCCESS  The XHCI host controller is reset.
   @return Others   Failed to reset the XHCI before Timeout.
@@ -537,7 +537,7 @@ XhcResetHC (
   Halt the XHCI host controller.
 
   @param  Xhc  The XHCI Instance.
-  @param  Timeout  Time to wait before abort (in microsecond, us).
+  @param  Timeout  Time to wait before abort (in millisecond, ms).
 
   @return EFI_SUCCESS  The XHCI host controller is halt.
   @return EFI_TIMEOUT  Failed to halt the XHCI before Timeout.
@@ -553,7 +553,7 @@ XhcHaltHC (
   Set the XHCI host controller to run.
 
   @param  Xhc  The XHCI Instance.
-  @param  Timeout  Time to wait before abort (in microsecond, us).
+  @param  Timeout  Time to wait before abort (in millisecond, ms).
 
   @return EFI_SUCCESS  The XHCI host controller is running.
   @return EFI_TIMEOUT  Failed to set the XHCI to run before Timeout.
diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhciReg.h 
b/MdeModulePkg/Bus/Pci/XhciPei/XhciReg.h
index 1a6256066599..0297072ffd6b 100644
--- a/MdeModulePkg/Bus/Pci/XhciPei/XhciReg.h
+++ b/MdeModulePkg/Bus/Pci/XhciPei/XhciReg.h
@@ -1,7 +1,7 @@
 /** @file
 Private Header file for Usb Host Controller PEIM
 
-Copyright (c) 2014, Intel Corporation. All rights reserved.
+Copyright (c) 2014 - 2017, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -287,7 +287,7 @@ XhcPeiClearOpRegBit (
   @param  OffsetThe offset of the operational register.
   @param  Bit   The bit of the register to wait for.
   @param  WaitToSet Wait the bit to set or clear.
-  @param  Timeout   The time to wait before abort (in microsecond, us).
+  @param  Timeout   The time to wait before abort (in millisecond, ms).
 
   @retval EFI_SUCCESS   The bit successfully changed by host controller.
   @retval EFI_TIMEOUT   The time out occurred.
-- 
2.7.0.windows.1

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

Re: [edk2] ASSERT in QemuVideoDxe driver during reset

[Laszlo Ersek]

On 09/07/17 03:28, Yao, Jiewen wrote:
> OK. Got you.
> 
> I think we can move the zero memory to DxeIpl.
> The DxeIpl can check if the 0 is allocated in MemoryAllocationHob, and zero 
> it if it is not allocated.

Regarding the zeroing out of page 0, this solution seems good to me as
well. (I agree that page 0 should be zeroed out even if the null pointer
detection is enabled.)

Regarding the question whether QemuVideoDxe should, or should not,
manually change the access permissions to page 0, before and after
setting up the Int0x10 vector, if null pointer detection is enabled:

This depends on Windows 7. If Windows 7 can boot fine with the null
pointer detection enabled, then I agree QemuVideoDxe should be updated
as described above. (For massaging the page protection, it should use
DXE services, not CpuArch services.) In this case it is also fine for
OVMF to inherit the default TRUE setting for the PCD.

If Windows 7 cannot boot with null pointer detection enabled, then OVMF
should set the PCD to FALSE, and QemuVideoDxe should be changed to not
call InstallVbeShim() at all if the PCD is TRUE.

Thanks
Laszlo

> From: Wang, Jian J
> Sent: Thursday, September 7, 2017 8:41 AM
> To: Yao, Jiewen ; Laszlo Ersek ; 
> Justen, Jordan L 
> Cc: edk2-devel@lists.01.org; Kinney, Michael D 
> Subject: RE: ASSERT in QemuVideoDxe driver during reset
> 
> Hi Jiewen and Laszlo,
> 
> According to both of your comments:
> 
> 
> 1.   VBE SHIM must be installed to avoid boot failure of Windows 7.
> 
> 2.   Enabling NULL detection should not break existing platforms.
> 
> Let me clarify a few things in advance. I think there’s a little 
> misunderstanding on this issue (Sorry for my poor description originally).
> 
> 1.   NULL detection is implemented by disable first 4K page (0-4095). Let 
> me call it page 0.
> 
> 2.   The ASSERT was not caused by accessing page 0. I do enabling page 0 
> before access int10 vector in QemuVideoDxe driver while NULL detection is 
> enabled.
> 
> 3.  The ASSERT will only be triggered by reset because QemuVideoDxe will 
> write int10 vector and memory at that place keeps intact during reset.
> 
> 4.  The root cause of ASSERT is the fact that page 0 is always cleared in DXE 
> core except to NULL detection enabled. I changed CoreAddRange() in page.c in 
> DXC core like below to avoid page fault in core.
> //
>   // If memory of type EfiConventionalMemory is being added that includes the 
> page
>   // starting at address 0, then zero the page starting at address 0.  This 
> has
>   // two benifits.  It helps find NULL pointer bugs and it also maximizes
>   // compatibility with operating systems that may evaluate memory in this 
> page
>   // for legacy data structures.  If memory of any other type is added 
> starting
>   // at address 0, then do not zero the page at address 0 because the page is 
> being
>   // used for other purposes.
>   //
>   if (Type == EfiConventionalMemory && Start == 0 && (End >= EFI_PAGE_SIZE - 
> 1)) {
> if ((PcdGet8(PcdNullPointerDetectionPropertyMask) & BIT0) == 0) {
>   SetMem ((VOID *)(UINTN)Start, EFI_PAGE_SIZE, 0);
> }
>   }
> 
> I think the best solution at present is to update above code to make sure 
> page 0 is always cleared no matter NULL detection is enable or not. To make 
> it possible, I have to enable page 0 before memory clearing and disable it 
> again after then. My concern is that I cannot use CPU arch protocol to do so 
> in CoreAddRange() function because it’s not ready at that time. There’re two 
> options here. One is “manually” disabling page 0 (read cr3, then pm4l, then 
> pde…) in above code; another is moving above code to another place where 
> paging operation is at hand, like DxeIpl or CPU driver. My personal opinion 
> is second one would be better. What’s your opinions? Or any better ideas?
> 
> Thanks,
> Wang, Jian J
> 
> From: Yao, Jiewen
> Sent: Wednesday, September 06, 2017 7:17 PM
> To: Laszlo Ersek >; Wang, Jian J 
> >; Justen, Jordan L 
> >
> Cc: edk2-devel@lists.01.org; Kinney, Michael 
> D >
> Subject: RE: ASSERT in QemuVideoDxe driver during reset
> 
> HI
> I think the NULL detection feature should *never* break any existing platform.
> No real function should be skipped for PcdNullDetection.
> 
> If InstallVbeShim () is needed for CSM, we should always call 
> InstallVbeShim() for CSM.
> 
> I suggest below options:
> 
> 1)   In OVMF, if CSM is enabled, disable PcdNullDetection.
> 
> 2)   In OVMF, if any driver need access first 4K page, the code should 
> explicit call SetAttribute(0, 4K, READ|WRITE), if PcdNullDetection is enabled.
> 
> Either #1 

Re: [edk2] [Patch] BaseTools: Not show *B when Pcd value in build option same with DEC

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Yonghong Zhu
>Sent: Monday, September 04, 2017 4:42 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [edk2] [Patch] BaseTools: Not show *B when Pcd value in build
>option same with DEC
>
>Per build spec, If the value obtained from either a build option, the
>DSC or FDF is the same as the value in the DEC, then *B , *P or *F
>will not be shown in the report.
>
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Source/Python/build/BuildReport.py | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/BaseTools/Source/Python/build/BuildReport.py
>b/BaseTools/Source/Python/build/BuildReport.py
>index a7cbb6a..38ee26d 100644
>--- a/BaseTools/Source/Python/build/BuildReport.py
>+++ b/BaseTools/Source/Python/build/BuildReport.py
>@@ -969,14 +969,14 @@ class PcdReport(object):
> DscMatch = (DscDefaultValue.strip() == 
> PcdValue.strip())
>
> #
> # Report PCD item according to their override relationship
> #
>-if BuildOptionMatch:
>-FileWrite(File, ' *B %-*s: %6s %10s = %-22s' % 
>(self.MaxLen,
>PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', PcdValue.strip()))
>-elif DecMatch and InfMatch:
>+if DecMatch and InfMatch:
> FileWrite(File, '%-*s: %6s %10s = %-22s' % 
> (self.MaxLen,
>PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', PcdValue.strip()))
>+elif BuildOptionMatch:
>+FileWrite(File, ' *B %-*s: %6s %10s = %-22s' % 
>(self.MaxLen,
>PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', PcdValue.strip()))
> else:
> if DscMatch:
> if (Pcd.TokenCName, Key) in self.FdfPcdSet:
> FileWrite(File, ' *F %-*s: %6s %10s = %-22s' 
> % (self.MaxLen,
>PcdTokenCName, TypeName, '(' + Pcd.DatumType + ')', PcdValue.strip()))
> else:
>--
>2.6.1.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][edk2-platforms] Ubuntu boot

[Liu, XianhuiX]

Hi all,
Please ignore this patch. The subject is not quite right. 
I have re-submitted a patch with subject: 
[Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Ubuntu boot. Thanks.

Best Regards
Liu Xianhui
-Original Message-
From: Wei, David 
Sent: Thursday, September 7, 2017 11:15 AM
To: edk2-devel@lists.01.org
Cc: Liu, XianhuiX 
Subject: [PATCH][edk2-platforms] Ubuntu boot

From: xianhu2x 

Add Ubuntu boot loader file path into known OS loader list.

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: xianhu2x 
---
 .../Library/GenericBdsLib/BdsBoot.c| 26 ++
 Core/MdePkg/Include/Uefi/UefiSpec.h| 11 -
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c 
b/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
index d1da635f3..dec5d8cef 100644
--- a/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
+++ b/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
@@ -2384,6 +2384,21 @@ BdsLibBootViaBootOption (
   0,
   
   );
+  //
+  //Try UBUNTU boot loader
+  //
+  if (EFI_ERROR(Status)) {
+FilePath = FileDevicePath (Handle, 
EFI_REMOVABLE_MEDIA_FILE_NAME_UBUNTU);
+Status = gBS->LoadImage (
+TRUE,
+gImageHandle,
+FilePath,
+NULL,
+0,
+
+);
+  }
+   
 }
   }
 }
@@ -3721,6 +3736,17 @@ BdsLibGetBootableHandle (
  ,
  Hdr
  );
+  //
+  //Try UBUNTU boot loader
+  //
+  if (EFI_ERROR(Status)) {
+Status = BdsLibGetImageHeader (
+   SimpleFileSystemHandles[Index],
+   EFI_REMOVABLE_MEDIA_FILE_NAME_UBUNTU,
+   ,
+   Hdr
+   );
+  }
   if (!EFI_ERROR (Status) &&
 EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Hdr.Pe32->FileHeader.Machine) &&
 Hdr.Pe32->OptionalHeader.Subsystem == 
EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION) { diff --git 
a/Core/MdePkg/Include/Uefi/UefiSpec.h b/Core/MdePkg/Include/Uefi/UefiSpec.h
index 57cb4e804..e5556952b 100644
--- a/Core/MdePkg/Include/Uefi/UefiSpec.h
+++ b/Core/MdePkg/Include/Uefi/UefiSpec.h
@@ -2166,11 +2166,12 @@ typedef struct {  //  // EFI File location to boot from 
on removable media devices  //
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA32L"\\EFI\\BOOT\\BOOTIA32.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA64L"\\EFI\\BOOT\\BOOTIA64.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_X64 L"\\EFI\\BOOT\\BOOTX64.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_ARM L"\\EFI\\BOOT\\BOOTARM.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_AARCH64 L"\\EFI\\BOOT\\BOOTAA64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA32   L"\\EFI\\BOOT\\BOOTIA32.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA64   L"\\EFI\\BOOT\\BOOTIA64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_X64L"\\EFI\\BOOT\\BOOTX64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_ARML"\\EFI\\BOOT\\BOOTARM.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_AARCH64L"\\EFI\\BOOT\\BOOTAA64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_UBUNTU_X64 L"\\EFI\\UBUNTU\\GRUBX64.EFI"
 
 #if   defined (MDE_CPU_IA32)
   #define EFI_REMOVABLE_MEDIA_FILE_NAME   EFI_REMOVABLE_MEDIA_FILE_NAME_IA32
--
2.14.1.windows.1

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

[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Ubuntu boot

[xianhu2x]

Add Ubuntu boot loader file path \\EFI\\UBUNTU\\GRUBX64.EFI into known OS 
loader list.

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: xianhu2x 
---
 .../Library/GenericBdsLib/BdsBoot.c| 26 ++
 Core/MdePkg/Include/Uefi/UefiSpec.h| 11 -
 2 files changed, 32 insertions(+), 5 deletions(-)

diff --git a/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c 
b/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
index d1da635f3..dec5d8cef 100644
--- a/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
+++ b/Core/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
@@ -2384,6 +2384,21 @@ BdsLibBootViaBootOption (
   0,
   
   );
+  //
+  //Try UBUNTU boot loader
+  //
+  if (EFI_ERROR(Status)) {
+FilePath = FileDevicePath (Handle, 
EFI_REMOVABLE_MEDIA_FILE_NAME_UBUNTU);
+Status = gBS->LoadImage (
+TRUE,
+gImageHandle,
+FilePath,
+NULL,
+0,
+
+);
+  }
+   
 }
   }
 }
@@ -3721,6 +3736,17 @@ BdsLibGetBootableHandle (
  ,
  Hdr
  );
+  //
+  //Try UBUNTU boot loader
+  //
+  if (EFI_ERROR(Status)) {
+Status = BdsLibGetImageHeader (
+   SimpleFileSystemHandles[Index],
+   EFI_REMOVABLE_MEDIA_FILE_NAME_UBUNTU,
+   ,
+   Hdr
+   );
+  }
   if (!EFI_ERROR (Status) &&
 EFI_IMAGE_MACHINE_TYPE_SUPPORTED (Hdr.Pe32->FileHeader.Machine) &&
 Hdr.Pe32->OptionalHeader.Subsystem == 
EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION) {
diff --git a/Core/MdePkg/Include/Uefi/UefiSpec.h 
b/Core/MdePkg/Include/Uefi/UefiSpec.h
index 57cb4e804..e5556952b 100644
--- a/Core/MdePkg/Include/Uefi/UefiSpec.h
+++ b/Core/MdePkg/Include/Uefi/UefiSpec.h
@@ -2166,11 +2166,12 @@ typedef struct {
 //
 // EFI File location to boot from on removable media devices
 //
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA32L"\\EFI\\BOOT\\BOOTIA32.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA64L"\\EFI\\BOOT\\BOOTIA64.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_X64 L"\\EFI\\BOOT\\BOOTX64.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_ARM L"\\EFI\\BOOT\\BOOTARM.EFI"
-#define EFI_REMOVABLE_MEDIA_FILE_NAME_AARCH64 L"\\EFI\\BOOT\\BOOTAA64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA32   L"\\EFI\\BOOT\\BOOTIA32.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_IA64   L"\\EFI\\BOOT\\BOOTIA64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_X64L"\\EFI\\BOOT\\BOOTX64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_ARML"\\EFI\\BOOT\\BOOTARM.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_AARCH64L"\\EFI\\BOOT\\BOOTAA64.EFI"
+#define EFI_REMOVABLE_MEDIA_FILE_NAME_UBUNTU_X64 L"\\EFI\\UBUNTU\\GRUBX64.EFI"
 
 #if   defined (MDE_CPU_IA32)
   #define EFI_REMOVABLE_MEDIA_FILE_NAME   EFI_REMOVABLE_MEDIA_FILE_NAME_IA32
-- 
2.14.1.windows.1

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

Re: [edk2] [PATCH 1/1] OvmfPkg/IoMmuDxe: shut up "unused-const-variable" gcc-6 warning in RELEASE

[Laszlo Ersek]

On 09/07/17 07:47, Thomas Lamprecht wrote:
> On 09/07/2017 05:38 AM, Gao, Liming wrote:
>> Laszlo:
>>    I add -Wno-unused-const-variable option in GCC5 RELEASE option, and
>> build OvmfPkg with GCC5.3. I don't meet any warning or error. Do you
>> meet any issue? So, you think we can't add this option in GCC5 tool
>> chain.
>>
> 
> Yes, he is right, looking at:
> https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#Warning-Options
> 
>> When an unrecognized warning option is requested (e.g.,
>> -Wunknown-warning), GCC emits a diagnostic stating that the option is
>> not recognized. However, if the -Wno- form is used, the behavior is
>> slightly different: no diagnostic is produced for -Wno-unknown-warning
>> unless other diagnostics are being produced. This allows the use of new
>> -Wno- options with old compilers, but if something goes wrong, the
>> compiler warns that an unrecognized option is present.
> 
> Also the older ones support it:
> https://gcc.gnu.org/onlinedocs/gcc-4.9.4/gcc/Warning-Options.html#Warning-Options
> 
> https://gcc.gnu.org/onlinedocs/gcc-5.4.0/gcc/Warning-Options.html#Warning-Options
> 
> 
> I verified this behavior on gcc 5.4 and on gcc version 4.4.5 (Debian
> 4.4.5-8)
> (spun up an squeeze container, fun times)
> 
> This seems like an easy solution, hopefully also a valid one in the edk2
> context.
> The gcc rationale seems good, if there is such a warning suppress it, if
> not we
> wouldn't output any warning anyhow, so there is nothing to suppress.
> 
> The only drawback seems that if something goes wrong the user could
> wonder if
> those "unrecognized command line options" have something to do with his
> problem.
> 
> It looks like this (gcc 4.4.5):
> 
> tom@olddeb:~# gcc -Wall -Wno-foo-bar test.c
> test.c: In function 'main':
> test.c:4: warning: unused variable 'test'
> At top level:
> cc1: warning: unrecognized command line option "-Wno-foo-bar"
> 
> And very similar on gcc 6:
> 
> tom@plain-stretch:~# gcc -Wall -Wno-foo-bar test.c
> test.c: In function ‘main’:
> test.c:7:9: warning: unused variable ‘test’ [-Wunused-variable]
>  int test;
>  ^~~~
> test.c: At top level:
> cc1: warning: unrecognized command line option ‘-Wno-foo-bar’

Sounds great, thank you guys for discovering this. I believe this would
be a simple solution to BZ#700. Please feel free to update the subject
of that bug as appropriate.

Correspondingly, this patch can be ignored.

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

Re: [edk2] [PATCH v1] MdePkg: Add definitions for the SPI protocols introduced in UEFI PI 1.6.

[Ni, Ruiyu]

Marvin,
Thank you for your contribution. We will need some time to review the 
definitions against PI Spec.
If there is a need to post V2, it might be better to separate the header files 
in different groups.
For example, LegacySpi group, SPI group, SMM SPI group.

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marvin Häuser
> Sent: Wednesday, September 6, 2017 5:21 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> 
> Subject: [edk2] [PATCH v1] MdePkg: Add definitions for the SPI protocols
> introduced in UEFI PI 1.6.
> 
> This commit adds header files for the SPI protocols introduced in the
> UEFI PI 1.6 specification, as well as their GUIDs to MdePkg.dec.
> 
> EFI_SPI_TRANSACTION_TYPE assumes an enum with its members ordered
> the
> way they are listed in the specification, as there are no values given
> explicitely.
> EFI_LEGACY_SPI_CONTROLLER_GUID assumes the character 'l' used in the
> specification was meant to be '1'.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser 
> ---
>  MdePkg/Include/Protocol/LegacySpiController.h| 265
> ++
>  MdePkg/Include/Protocol/LegacySpiFlash.h | 201 ++
>  MdePkg/Include/Protocol/LegacySpiSmmController.h |  36 +++
>  MdePkg/Include/Protocol/LegacySpiSmmFlash.h  |  36 +++
>  MdePkg/Include/Protocol/SpiConfiguration.h   | 293
> 
>  MdePkg/Include/Protocol/SpiHc.h  | 194 +
>  MdePkg/Include/Protocol/SpiIo.h  | 292 +++
>  MdePkg/Include/Protocol/SpiNorFlash.h| 289
> +++
>  MdePkg/Include/Protocol/SpiSmmConfiguration.h|  36 +++
>  MdePkg/Include/Protocol/SpiSmmHc.h   |  36 +++
>  MdePkg/MdePkg.dec|  31 +++
>  11 files changed, 1709 insertions(+)
> 
> diff --git a/MdePkg/Include/Protocol/LegacySpiController.h
> b/MdePkg/Include/Protocol/LegacySpiController.h
> new file mode 100644
> index ..2d36eaefc0ee
> --- /dev/null
> +++ b/MdePkg/Include/Protocol/LegacySpiController.h
> @@ -0,0 +1,265 @@
> +/** @file
> +  This file defines the Legacy SPI Controller Protocol.
> +
> +  Copyright (c) 2017, Intel Corporation. All rights reserved.
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD
> +  License which accompanies this distribution. The full text of the license
> may
> +  be found at http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> +
> +  @par Revision Reference:
> +This Protocol was introduced in UEFI PI Specification 1.6.
> +
> +**/
> +
> +#ifndef __LEGACY_SPI_CONTROLLER_PROTOCOL_H__
> +#define __LEGACY_SPI_CONTROLLER_PROTOCOL_H__
> +
> +///
> +/// Note: The UEFI PI 1.6 specification uses the character 'l' in the GUID
> +///   definition. This definition assumes it was supposed to be '1'.
> +///
> +/// Global ID for the Legacy SPI Controller Protocol
> +///
> +#define EFI_LEGACY_SPI_CONTROLLER_GUID  \
> +  { 0x39136fc7, 0x1a11, 0x49de, \
> +{ 0xbf, 0x35, 0x0e, 0x78, 0xdd, 0xb5, 0x24, 0xfc }}
> +
> +typedef
> +struct _EFI_LEGACY_SPI_CONTROLLER_PROTOCOL
> +EFI_LEGACY_SPI_CONTROLLER_PROTOCOL;
> +
> +/**
> +  Set the erase block opcode.
> +
> +  This routine must be called at or below TPL_NOTIFY.
> +  The menu table contains SPI transaction opcodes which are accessible
> after
> +  the legacy SPI flash controller's configuration is locked. The board layer
> +  specifies the erase block size for the SPI NOR flash part. The SPI NOR 
> flash
> +  peripheral driver selects the erase block opcode which matches the erase
> +  block size and uses this API to load the opcode into the opcode menu table.
> +
> +  @param[in] This  Pointer to an
> EFI_LEGACY_SPI_CONTROLLER_PROTOCOL
> +   structure.
> +  @param[in] EraseBlockOpcode  Erase block opcode to be placed into the
> opcode
> +   menu table.
> +
> +  @retval EFI_SUCCESS   The opcode menu table was updated
> +  @retval EFI_ACCESS_ERROR  The SPI controller is locked
> +
> +**/
> +typedef EFI_STATUS
> +(EFIAPI
> *EFI_LEGACY_SPI_CONTROLLER_PROTOCOL_ERASE_BLOCK_OPCODE) (
> +  IN CONST EFI_LEGACY_SPI_CONTROLLER_PROTOCOL  *This,
> +  IN UINT8 EraseBlockOpcode
> +  );
> +
> +/**
> +  Set the write status prefix opcode.
> +
> +  This routine must be called at or below TPL_NOTIFY.
> +  The prefix table contains SPI transaction write prefix opcodes which are
> +  accessible after the legacy SPI flash controller's configuration is locked.
> +  The board layer