date:20170329

Re: [edk2] Using Intel UDk debugger

2017-03-29 Thread Fan, Jeff

Arka,

UDK Debugger tool supports your usage model.

Please see the section "9.11 Debugging a standalone module loaded in a UEFI 
shell" @ 
https://firmware.intel.com/sites/default/files/UDK_Debugger_Tool_User_Manual_V1.11.pdf

Thanks!
Jeff
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Arka 
Sharma
Sent: Thursday, March 30, 2017 1:15 PM
To: edk2-devel@lists.01.org
Subject: [edk2] Using Intel UDk debugger

Hi,

I am sorry if it is not a right place to ask this. I have installed WinDbg and 
Intel UDK debugger. I want to debug a driver and an application on an Asrock 
borad, but going through the UDK debugger user manual I realize that 
SourceLevelDebugPkg has to be included in target firmware image. Now in this 
case what option do I have to proceed with the debugging ? I am launching my 
application from shell and in the shell post codes are disabled, the 
application code get stuck randomly. So far I was trying to debug with 
AsciiPrints. Is there any way to use DEBUG macro to redirect debug messages 
from UEFI driver as well as application to some serial port in this case ?

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

Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm support

2017-03-29 Thread Gao, Liming

Michael:
  Thanks for point. I will create the patch to fix 2&3. For 1, I am not sure 
whether there is generic way to handle it. 

Thanks
Liming
>-Original Message-
>From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>Sent: Thursday, March 30, 2017 1:13 PM
>To: Gao, Liming 
>Cc: Song, BinX ; edk2-devel@lists.01.org
>Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>algorithm support
>
>Thanks you that worked. (sorry that I forgot about that)
>There were three things I had to workaround:
>
>1) since I'm using -nostdinc I had to remove the sys/types.h include
>from MdeModulePkg/Library/BrotliCustomDecompressLib/common/types.h
>and
>use BOOLEAN, TRUE and FALSE instead of bool, true and false.
>
>2) I had to chmod +x Brotli and BrotliCompress because basetools forgot that
>
>3) I had to run dos2unix on Brotli and BrotliCompress because bash
>doesn't support CRLF line endings.
>
>Thanks
>Michael
>
>On Thu, Mar 30, 2017 at 6:45 AM, Gao, Liming  wrote:
>> Michasel:
>>   Please delete cache Conf/tools_def.txt, and run edksetup again to apply
>new tools_def.txt.
>>
>> Thanks
>> Liming
>>>-Original Message-
>>>From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>>>Sent: Thursday, March 30, 2017 5:04 AM
>>>To: Gao, Liming 
>>>Cc: Song, BinX ; edk2-devel@lists.01.org
>>>Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>>>algorithm support
>>>
>>>How can I use this? If I change my compressed FV's GUID to
>>>3D532050-5CDA-4FD0-879E-0F7F630D5AFB I get the following error:
>>>
>>>GenFds.py...
>>>: error F003: No tool found with GUID 3D532050-5CDA-4FD0-879E-
>>>0F7F630D5AFB
>>>
>>>Thanks
>>>Michael
>>>
>>>On Mon, Mar 27, 2017 at 5:15 AM, Gao, Liming 
>wrote:
 Reviewed-by: Liming Gao 

>-Original Message-
>From: Song, BinX
>Sent: Thursday, March 23, 2017 2:05 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: RE: [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>algorithm
>support
>
>Hi All,
>
>The code is also in https://github.com/binxsong/edk2/tree/Brotli_V1
>
>Best Regards,
>Bell Song
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>Of
>Song,
>> BinX
>> Sent: Thursday, March 23, 2017 10:16 AM
>> To: edk2-devel@lists.01.org
>> Cc: Gao, Liming 
>> Subject: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>algorithm
>> support
>>
>> Brotli algorithm was released on the website
>https://github.com/google/brotli.
>> It has a little less compress ratio than Lzma, but has better decompress
>> performance than it.
>> Add Brotli algorithm support, include Brotli decompression library and
>tool
>set.
>>
>> Tested on:
>> OS: Windows
>> Arch: IA32/X64
>> Platform: Nt32Pkg
>> ToolChain: VS2015x86
>> Target: Release
>>
>> OS: Ubuntu
>> Arch: IA32/X64
>> Platform: OvmfPkgIa32.dsc/OvmfPkgX64.dsc
>> ToolChain: GCC5
>> Target: Release
>>
>> Cc: Liming Gao 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Bell Song 
>>
>> Bell Song (4):
>>   MdeModulePkg: Copy Brotli algorithm 3rd party source code for
>library
>>   MdeModulePkg: Add Brotli algorithm decompression library
>>   BaseTools: Copy Brotli algorithm 3rd party source code for tool
>>   BaseTools: Add Brotli algorithm tool
>>
>>  BaseTools/BinWrappers/PosixLike/Brotli |29 +
>>  BaseTools/BinWrappers/PosixLike/BrotliCompress |42 +
>>  BaseTools/Conf/tools_def.template  | 6 +
>>  .../Source/C/BrotliCompress/BrotliCompress.bat |48 +
>>  BaseTools/Source/C/BrotliCompress/GNUmakefile  |43 +
>>  BaseTools/Source/C/BrotliCompress/LICENSE  |19 +
>>  BaseTools/Source/C/BrotliCompress/Makefile |60 +
>>  BaseTools/Source/C/BrotliCompress/README.md|26 +
>>  BaseTools/Source/C/BrotliCompress/ReadMe.txt   | 2 +
>>  .../Source/C/BrotliCompress/common/constants.h |47 +
>>  .../Source/C/BrotliCompress/common/dictionary.c|  9474
>> 
>>  .../Source/C/BrotliCompress/common/dictionary.h|29 +
>>  BaseTools/Source/C/BrotliCompress/common/port.h|   107 +
>>  BaseTools/Source/C/BrotliCompress/common/types.h   |58 +
>>  BaseTools/Source/C/BrotliCompress/dec/bit_reader.c |48 +
>>  BaseTools/Source/C/BrotliCompress/dec/bit_reader.h |   383 +
>>  BaseTools/Source/C/BrotliCompress/dec/context.h|   251 +
>>

[edk2] Using Intel UDk debugger

2017-03-29 Thread Arka Sharma

Hi,

I am sorry if it is not a right place to ask this. I have installed
WinDbg and Intel UDK debugger. I want to debug a driver and an
application on an Asrock borad, but going through the UDK debugger
user manual I realize that SourceLevelDebugPkg has to be included in
target firmware image. Now in this case what option do I have to
proceed with the debugging ? I am launching my application from shell
and in the shell post codes are disabled, the application code get
stuck randomly. So far I was trying to debug with AsciiPrints. Is
there any way to use DEBUG macro to redirect debug messages from UEFI
driver as well as application to some serial port in this case ?

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

Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm support

2017-03-29 Thread Michael Zimmermann

Thanks you that worked. (sorry that I forgot about that)
There were three things I had to workaround:

1) since I'm using -nostdinc I had to remove the sys/types.h include
from MdeModulePkg/Library/BrotliCustomDecompressLib/common/types.h and
use BOOLEAN, TRUE and FALSE instead of bool, true and false.

2) I had to chmod +x Brotli and BrotliCompress because basetools forgot that

3) I had to run dos2unix on Brotli and BrotliCompress because bash
doesn't support CRLF line endings.

Thanks
Michael

On Thu, Mar 30, 2017 at 6:45 AM, Gao, Liming  wrote:
> Michasel:
>   Please delete cache Conf/tools_def.txt, and run edksetup again to apply new 
> tools_def.txt.
>
> Thanks
> Liming
>>-Original Message-
>>From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>>Sent: Thursday, March 30, 2017 5:04 AM
>>To: Gao, Liming 
>>Cc: Song, BinX ; edk2-devel@lists.01.org
>>Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>>algorithm support
>>
>>How can I use this? If I change my compressed FV's GUID to
>>3D532050-5CDA-4FD0-879E-0F7F630D5AFB I get the following error:
>>
>>GenFds.py...
>>: error F003: No tool found with GUID 3D532050-5CDA-4FD0-879E-
>>0F7F630D5AFB
>>
>>Thanks
>>Michael
>>
>>On Mon, Mar 27, 2017 at 5:15 AM, Gao, Liming  wrote:
>>> Reviewed-by: Liming Gao 
>>>
-Original Message-
From: Song, BinX
Sent: Thursday, March 23, 2017 2:05 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: RE: [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm
support

Hi All,

The code is also in https://github.com/binxsong/edk2/tree/Brotli_V1

Best Regards,
Bell Song

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
Song,
> BinX
> Sent: Thursday, March 23, 2017 10:16 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
algorithm
> support
>
> Brotli algorithm was released on the website
https://github.com/google/brotli.
> It has a little less compress ratio than Lzma, but has better decompress
> performance than it.
> Add Brotli algorithm support, include Brotli decompression library and 
> tool
set.
>
> Tested on:
> OS: Windows
> Arch: IA32/X64
> Platform: Nt32Pkg
> ToolChain: VS2015x86
> Target: Release
>
> OS: Ubuntu
> Arch: IA32/X64
> Platform: OvmfPkgIa32.dsc/OvmfPkgX64.dsc
> ToolChain: GCC5
> Target: Release
>
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Bell Song 
>
> Bell Song (4):
>   MdeModulePkg: Copy Brotli algorithm 3rd party source code for library
>   MdeModulePkg: Add Brotli algorithm decompression library
>   BaseTools: Copy Brotli algorithm 3rd party source code for tool
>   BaseTools: Add Brotli algorithm tool
>
>  BaseTools/BinWrappers/PosixLike/Brotli |29 +
>  BaseTools/BinWrappers/PosixLike/BrotliCompress |42 +
>  BaseTools/Conf/tools_def.template  | 6 +
>  .../Source/C/BrotliCompress/BrotliCompress.bat |48 +
>  BaseTools/Source/C/BrotliCompress/GNUmakefile  |43 +
>  BaseTools/Source/C/BrotliCompress/LICENSE  |19 +
>  BaseTools/Source/C/BrotliCompress/Makefile |60 +
>  BaseTools/Source/C/BrotliCompress/README.md|26 +
>  BaseTools/Source/C/BrotliCompress/ReadMe.txt   | 2 +
>  .../Source/C/BrotliCompress/common/constants.h |47 +
>  .../Source/C/BrotliCompress/common/dictionary.c|  9474
> 
>  .../Source/C/BrotliCompress/common/dictionary.h|29 +
>  BaseTools/Source/C/BrotliCompress/common/port.h|   107 +
>  BaseTools/Source/C/BrotliCompress/common/types.h   |58 +
>  BaseTools/Source/C/BrotliCompress/dec/bit_reader.c |48 +
>  BaseTools/Source/C/BrotliCompress/dec/bit_reader.h |   383 +
>  BaseTools/Source/C/BrotliCompress/dec/context.h|   251 +
>  BaseTools/Source/C/BrotliCompress/dec/decode.c |  2347 
>  BaseTools/Source/C/BrotliCompress/dec/decode.h |   188 +
>  BaseTools/Source/C/BrotliCompress/dec/huffman.c|   357 +
>  BaseTools/Source/C/BrotliCompress/dec/huffman.h|68 +
>  BaseTools/Source/C/BrotliCompress/dec/port.h   |   159 +
>  BaseTools/Source/C/BrotliCompress/dec/prefix.h |   751 ++
>  BaseTools/Source/C/BrotliCompress/dec/state.c  |   168 +
>  BaseTools/Source/C/BrotliCompress/dec/state.h  |   246 +
>  BaseTools/Source/C/BrotliCompress/dec/transform.h  |   300 +
>

Re: [edk2] [PATCH v3] MdePkg: BaseIoLibIntrinsic (IoLib class) library

2017-03-29 Thread Gao, Liming

Leo:
  Your change is good. My comment is not to add asm code. Nasm is enough. For 
new added features, nasm is only required. 

Thanks
Liming
>-Original Message-
>From: Leo Duran [mailto:leo.du...@amd.com]
>Sent: Thursday, March 30, 2017 12:07 AM
>To: edk2-de...@ml01.01.org
>Cc: Leo Duran ; Kinney, Michael D
>; Gao, Liming ; Brijesh
>Singh 
>Subject: [PATCH v3] MdePkg: BaseIoLibIntrinsic (IoLib class) library
>
>This patch adds an SEV-specific .INF and corresponding assembly
>files, to unroll REP INSx/OUTSx on IoRead/WriteFifo#() routines
>when the SEV feature is enabled under a hypervisor environment.
>
>The new .INF only supports the IA32 and X64 architectures.
>
>Cc: Michael D Kinney 
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Brijesh Singh 
>Signed-off-by: Leo Duran 
>---
> .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf   |  63 +
> .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm  | 297
>+
> .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293
>
> .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm   | 282
>+++
> .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 282
>+++
> MdePkg/MdePkg.dsc  |   2 +
> 6 files changed, 1219 insertions(+)
> create mode 100644
>MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
> create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
> create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
> create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm
> create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm
>
>diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>new file mode 100644
>index 000..6f14075
>--- /dev/null
>+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
>@@ -0,0 +1,63 @@
>+## @file
>+#  Instance of I/O Library using compiler intrinsics.
>+#
>+#  I/O Library that uses compiler intrinsics to perform IN and OUT 
>instructions
>+#  for IA-32 and x64.  On IPF, I/O port requests are translated into MMIO
>requests.
>+#  MMIO requests are forwarded directly to memory.  For EBC, I/O port
>requests
>+#  ASSERT().
>+#
>+#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
>+#  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
>+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
>+#
>+#  This program and the accompanying materials
>+#  are licensed and made available under the terms and conditions of the
>BSD License
>+#  which accompanies this distribution. The full text of the license may be
>found at
>+#  http://opensource.org/licenses/bsd-license.php.
>+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>BASIS,
>+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>+#
>+##
>+
>+[Defines]
>+  INF_VERSION= 0x00010005
>+  BASE_NAME  = BaseIoLibIntrinsicSev
>+  MODULE_UNI_FILE= BaseIoLibIntrinsic.uni
>+  FILE_GUID  = 93742f95-6e71-4581-b600-8e1da443f95a
>+  MODULE_TYPE= BASE
>+  VERSION_STRING = 1.0
>+  LIBRARY_CLASS  = IoLib
>+
>+
>+#
>+#  VALID_ARCHITECTURES   = IA32 X64
>+#
>+
>+[Sources]
>+  IoLibMmioBuffer.c
>+  BaseIoLibIntrinsicInternal.h
>+  IoHighLevel.c
>+
>+[Sources.IA32]
>+  IoLibGcc.c| GCC
>+  IoLibMsc.c| MSFT
>+  IoLibIcc.c| INTEL
>+  IoLib.c
>+  Ia32/IoFifoSev.nasm
>+  Ia32/IoFifoSev.asm
>+
>+[Sources.X64]
>+  IoLibGcc.c| GCC
>+  IoLibMsc.c| MSFT
>+  IoLibIcc.c| INTEL
>+  IoLib.c
>+  X64/IoFifoSev.nasm
>+  X64/IoFifoSev.asm
>+
>+[Packages]
>+  MdePkg/MdePkg.dec
>+
>+[LibraryClasses]
>+  DebugLib
>+  BaseLib
>+
>diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
>b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
>new file mode 100644
>index 000..d81871c
>--- /dev/null
>+++ b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
>@@ -0,0 +1,297 @@
>+;--
>+;
>+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
>+; Copyright (c) 2017, AMD Incorporated. 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,

Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm support

2017-03-29 Thread Gao, Liming

Michasel:
  Please delete cache Conf/tools_def.txt, and run edksetup again to apply new 
tools_def.txt. 

Thanks
Liming
>-Original Message-
>From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
>Sent: Thursday, March 30, 2017 5:04 AM
>To: Gao, Liming 
>Cc: Song, BinX ; edk2-devel@lists.01.org
>Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>algorithm support
>
>How can I use this? If I change my compressed FV's GUID to
>3D532050-5CDA-4FD0-879E-0F7F630D5AFB I get the following error:
>
>GenFds.py...
>: error F003: No tool found with GUID 3D532050-5CDA-4FD0-879E-
>0F7F630D5AFB
>
>Thanks
>Michael
>
>On Mon, Mar 27, 2017 at 5:15 AM, Gao, Liming  wrote:
>> Reviewed-by: Liming Gao 
>>
>>>-Original Message-
>>>From: Song, BinX
>>>Sent: Thursday, March 23, 2017 2:05 PM
>>>To: edk2-devel@lists.01.org
>>>Cc: Gao, Liming 
>>>Subject: RE: [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm
>>>support
>>>
>>>Hi All,
>>>
>>>The code is also in https://github.com/binxsong/edk2/tree/Brotli_V1
>>>
>>>Best Regards,
>>>Bell Song
>>>
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>>Song,
 BinX
 Sent: Thursday, March 23, 2017 10:16 AM
 To: edk2-devel@lists.01.org
 Cc: Gao, Liming 
 Subject: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>>>algorithm
 support

 Brotli algorithm was released on the website
>>>https://github.com/google/brotli.
 It has a little less compress ratio than Lzma, but has better decompress
 performance than it.
 Add Brotli algorithm support, include Brotli decompression library and tool
>>>set.

 Tested on:
 OS: Windows
 Arch: IA32/X64
 Platform: Nt32Pkg
 ToolChain: VS2015x86
 Target: Release

 OS: Ubuntu
 Arch: IA32/X64
 Platform: OvmfPkgIa32.dsc/OvmfPkgX64.dsc
 ToolChain: GCC5
 Target: Release

 Cc: Liming Gao 
 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Bell Song 

 Bell Song (4):
   MdeModulePkg: Copy Brotli algorithm 3rd party source code for library
   MdeModulePkg: Add Brotli algorithm decompression library
   BaseTools: Copy Brotli algorithm 3rd party source code for tool
   BaseTools: Add Brotli algorithm tool

  BaseTools/BinWrappers/PosixLike/Brotli |29 +
  BaseTools/BinWrappers/PosixLike/BrotliCompress |42 +
  BaseTools/Conf/tools_def.template  | 6 +
  .../Source/C/BrotliCompress/BrotliCompress.bat |48 +
  BaseTools/Source/C/BrotliCompress/GNUmakefile  |43 +
  BaseTools/Source/C/BrotliCompress/LICENSE  |19 +
  BaseTools/Source/C/BrotliCompress/Makefile |60 +
  BaseTools/Source/C/BrotliCompress/README.md|26 +
  BaseTools/Source/C/BrotliCompress/ReadMe.txt   | 2 +
  .../Source/C/BrotliCompress/common/constants.h |47 +
  .../Source/C/BrotliCompress/common/dictionary.c|  9474
 
  .../Source/C/BrotliCompress/common/dictionary.h|29 +
  BaseTools/Source/C/BrotliCompress/common/port.h|   107 +
  BaseTools/Source/C/BrotliCompress/common/types.h   |58 +
  BaseTools/Source/C/BrotliCompress/dec/bit_reader.c |48 +
  BaseTools/Source/C/BrotliCompress/dec/bit_reader.h |   383 +
  BaseTools/Source/C/BrotliCompress/dec/context.h|   251 +
  BaseTools/Source/C/BrotliCompress/dec/decode.c |  2347 
  BaseTools/Source/C/BrotliCompress/dec/decode.h |   188 +
  BaseTools/Source/C/BrotliCompress/dec/huffman.c|   357 +
  BaseTools/Source/C/BrotliCompress/dec/huffman.h|68 +
  BaseTools/Source/C/BrotliCompress/dec/port.h   |   159 +
  BaseTools/Source/C/BrotliCompress/dec/prefix.h |   751 ++
  BaseTools/Source/C/BrotliCompress/dec/state.c  |   168 +
  BaseTools/Source/C/BrotliCompress/dec/state.h  |   246 +
  BaseTools/Source/C/BrotliCompress/dec/transform.h  |   300 +
  .../docs/brotli-comparison-study-2015-09-22.pdf|   Bin 0 -> 215208
>bytes
  .../C/BrotliCompress/enc/backward_references.c |   892 ++
  .../C/BrotliCompress/enc/backward_references.h |99 +
  .../C/BrotliCompress/enc/backward_references_inc.h |   147 +
  BaseTools/Source/C/BrotliCompress/enc/bit_cost.c   |35 +
  BaseTools/Source/C/BrotliCompress/enc/bit_cost.h   |63 +
  .../Source/C/BrotliCompress/enc/bit_cost_inc.h |   127 +
  .../C/BrotliCompress/enc/block_encoder_inc.h   |33 +
  .../Source/C/BrotliCompress/enc/block_splitter.c   |   197 +
  .../Source/C/BrotliCompress/enc/block_splitter.h   |51 +

[edk2] [PATCH] SecureBoot UI Update

2017-03-29 Thread Zhang, Chao B

---
 .../SecureBootConfigDxe/SecureBootConfig.vfr   |  38 +++-
 .../SecureBootConfigDxe/SecureBootConfigImpl.c | 196 -
 .../SecureBootConfigDxe/SecureBootConfigImpl.h |  32 
 .../SecureBootConfigDxe/SecureBootConfigNvData.h   |   5 +
 .../SecureBootConfigStrings.uni|  13 +-
 5 files changed, 268 insertions(+), 16 deletions(-)

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
index 02ddf4a..e153eca 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
@@ -455,15 +455,35 @@ formset
 maxsize = SECURE_BOOT_GUID_SIZE,
 endstring;
 
-oneof name = SignatureFormatInDbx,
-  varid   = SECUREBOOT_CONFIGURATION.CertificateFormat,
-  prompt  = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
-  help= STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256), value 
= 0x2, flags = DEFAULT;
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384), value 
= 0x3, flags = 0;
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512), value 
= 0x4, flags = 0;
-  option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW), value = 
0x5, flags = 0;
-endoneof;
+disableif NOT ideqval SECUREBOOT_CONFIGURATION.FileEnrollType == 1;
+  oneof name = X509SignatureFormatInDbx,
+varid   = SECUREBOOT_CONFIGURATION.CertificateFormat,
+prompt  = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),
+help= STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP),
+option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA256), 
value = 0x2, flags = DEFAULT;
+option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA384), 
value = 0x3, flags = 0;
+option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_SHA512), 
value = 0x4, flags = 0;
+option text = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_RAW), value 
= 0x5, flags = 0;
+  endoneof;
+endif;
+
+disableif NOT ideqval SECUREBOOT_CONFIGURATION.FileEnrollType == 2;
+  grayoutif TRUE;
+text
+  help   = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),   // 
Help string
+  text   = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP), // 
Prompt string
+  text   = STRING_TOKEN(STR_DBX_PE_FORMAT_SHA256);// 
TextTwo
+ endif;
+endif;
+
+suppressif ideqval SECUREBOOT_CONFIGURATION.FileEnrollType == 3;
+  grayoutif TRUE;
+text
+  help   = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_PROMPT),   // 
Help string
+  text   = STRING_TOKEN(STR_DBX_CERTIFICATE_FORMAT_HELP), // 
Prompt string
+  text   = STRING_TOKEN(STR_DBX_AUTH_2_FORMAT);   // 
TextTwo
+  endif;
+endif;
 
 suppressif ideqval SECUREBOOT_CONFIGURATION.CertificateFormat == 5;
 checkbox varid  = SECUREBOOT_CONFIGURATION.AlwaysRevocation,
diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 6f58729..17fe120 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -120,6 +120,61 @@ IsDerEncodeCertificate (
 }
 
 /**
+  This code checks if the file content complies with 
EFI_VARIABLE_AUTHENTICATION_2 format
+The function reads file content but won't open/close given FileHandle.
+
+  @param[in] FileHandleThe FileHandle to be checked
+
+  @retvalTRUEThe content is EFI_VARIABLE_AUTHENTICATION_2 
format.
+  @retvalFALSE  The content is NOT a EFI_VARIABLE_AUTHENTICATION_2 
format.
+
+**/
+BOOLEAN
+IsAuthentication2Format (
+  IN   EFI_FILE_HANDLEFileHandle
+)
+{
+  EFI_STATUS Status;
+  EFI_VARIABLE_AUTHENTICATION_2  *Auth2;
+  BOOLEANIsAuth2Format;
+
+  IsAuth2Format = FALSE;
+
+  //
+  // Read the whole file content
+  //
+  Status = ReadFileContent(
+ FileHandle,
+ (VOID **) ,
+ ,
+ 0
+ );
+  if (EFI_ERROR (Status)) {
+goto ON_EXIT;
+  }
+
+  Auth2 = (EFI_VARIABLE_AUTHENTICATION_2 *)mImageBase;
+  if (Auth2->AuthInfo.Hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID) {
+goto ON_EXIT;
+  }
+
+  if (CompareGuid(, >AuthInfo.CertType)) {
+IsAuth2Format = TRUE;
+  }
+
+ON_EXIT:
+  //
+  // Do not close File. simply check file content
+  //
+  if (mImageBase != NULL) {
+FreePool (mImageBase);
+mImageBase = NULL;
+  }
+
+  return IsAuth2Format;
+}
+
+/**
   Set Secure

Re: [edk2] [RFC PATCH v1 0/2] OvmfPkg/QemuVideoDxe: Add VMWare SVGA2 framebuffer support

2017-03-29 Thread Laszlo Ersek

On 03/29/17 10:19, Phil Dennis-Jordan wrote:
> This extends the QemuVideoDxe driver to support the VMWare SVGA2 display
> device implemented by Qemu. Drivers for this device exist for guest OSes
> which do not support Qemu's other display adapters, so supporting it in
> OVMF is useful in conjunction with those OSes.
> 
> I've tried to follow the existing pattern for device-specific code in
> OVMF's QemuVideoDxe driver as much as possible, with the minimum of
> additional code. I've marked this patch as RFC for 2 main reasons:
> 
> 1. I've imported VMWare's own header file with device register constants
> etc. verbatim. (patch 1/2) This doesn't follow any of the EDK2 coding
> conventions, and it uses the MIT license, not BSD. Only a small percentage
> of symbols are actually used in the driver. On the other hand, it's
> obviously the authoritative source. I'm not sure what the correct
> etiquette is here, define our own constants, or import the authoritative
> header file?

The MIT license is OK (see "OvmfPkg/Contributions.txt").

I strongly prefer hand-crafted, minimal header files in OvmfPkg. I've
done that for all of the virtio stuff, for example.

At least one counter-example exists as well
("OvmfPkg/Include/IndustryStandard/Xen"), and I dislike that very much.

If hand-crafting the minimal required subset is not too much trouble,
and you don't expect frequent updates (header file syncs) from the
public (MIT-licensed) vmware-svga repo, I suggest that you write a brand
new header file, and place it under OvmfPkg/Include/IndustryStandard. As
reference you should indeed identify the original file (preferabily with
a commit hash / SVN revision identifier into the original repo, at which
the file looked like that). I think this new header file would still
qualify as derivative work, so it should be under MIT, and carry both
the original and your (C) notices. I think.

> 
> 2. For the functionality this driver uses, 2 I/O ports are used with
> 32-bit wide reads and writes. Unfortunately, one of them is not 32-bit
> aligned. This is fine as far as x86/x86-64 is concerned, but neither
> EDK2's IoLib nor other platforms support such an access pattern. It seems
> this issue was already encountered/discussed on the edk2-devel list 4
> years ago: http://edk2-devel.narkive.com/bwH3r0us/unaligned-i-o I couldn't
> find any code resulting from that discussion, and Qemu definitely uses
> unaligned port numbers for the SVGA2 device. (SVGA_IO_MUL is 1 in
> hw/display/vmware_vga.c) It does not appear to make any provision for
> non-x86 architectures, so I assume there's no sensible way to drive the
> device in those cases. The patch therefore only detects the device on x86,
> where it uses UnalignedIoWrite/Read32() helper functions which I've based
> on IoLib's aligned ones. I have only tested the GCC version of these.
> Feel free to suggest a better way of handling the issue.

Right now I can only say very generic things about patch 2:

- The idea to pull in & customize the primitives from IoLib matches
Jordan's idea from 4 years ago, so I think it's sane. Please do that in
a separate patch however.

- In the UnalignedIoRead32() and UnalignedIoWrite32() functions, you
should always return a value, even if you ASSERT(FALSE) first. Those
asserts can be compiled out.

- QemuVideDxe is used by ArmVirtPkg as well (it works OK on x86 TCG --
it's broken on aarch64 KVM); have you build tested the change with that
platform?

More on this later I hope.
Laszlo

> 
> Github feature branch: https://github.com/pmj/edk2/tree/ovmf_vmware_svga2_v1
> 
> Phil Dennis-Jordan (2):
>   OvmfPkg: Add SVGA2 device register definition header from VMWare
>   OvmfPkg: Add VMWare SVGA II support in QemuVideoDxe.
> 
>  OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |6 +
>  OvmfPkg/QemuVideoDxe/Qemu.h   |   50 +
>  OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h|   51 +
>  OvmfPkg/QemuVideoDxe/svga_reg.h   | 1558 
>  OvmfPkg/QemuVideoDxe/Driver.c |   67 +
>  OvmfPkg/QemuVideoDxe/Gop.c|   71 +-
>  OvmfPkg/QemuVideoDxe/Initialize.c |   88 ++
>  OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c |   59 +
>  OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c |   79 +
>  OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c |   81 +
>  OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c |   53 +
>  11 files changed, 2162 insertions(+), 1 deletion(-)
>  create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
>  create mode 100644 OvmfPkg/QemuVideoDxe/svga_reg.h
>  create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
>  create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
>  create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
>  create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c
> 

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

Re: [edk2] [Patch] NetworkPkg/DnsDxe: Fix zero StationIp configuration failure of DNSv6

2017-03-29 Thread Zhang, Lubo

Reviewed-by: Zhang Lubo 

-Original Message-
From: Wu, Jiaxin 
Sent: Friday, March 24, 2017 2:00 PM
To: edk2-devel@lists.01.org
Cc: Zhang, Lubo ; Ye, Ting ; Fu, 
Siyuan ; Wu, Jiaxin 
Subject: [Patch] NetworkPkg/DnsDxe: Fix zero StationIp configuration failure of 
DNSv6

According UEFI Spec, set to zero StationIp means to let the underlying
IPv6 driver choose a source address. But currently, DNSv6 always return 
EFI_NO_MAPPING. The issue is caused by below bugs in DnsDxe:
* Incorrect TPL(TPL_CALLBACK) usage during UDP configuration.
* Failed to create the timer used to get IPv6 mapping
* Doesn't check the Ip6Mode.IsStarted flag.

Cc: Zhang Lubo 
Cc: Ye Ting 
Cc: Fu Siyuan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 NetworkPkg/DnsDxe/DnsDriver.c   | 24 +++-
 NetworkPkg/DnsDxe/DnsImpl.c |  8 +---
 NetworkPkg/DnsDxe/DnsProtocol.c |  2 ++
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/NetworkPkg/DnsDxe/DnsDriver.c b/NetworkPkg/DnsDxe/DnsDriver.c 
index c000b5f..5dc9afe 100644
--- a/NetworkPkg/DnsDxe/DnsDriver.c
+++ b/NetworkPkg/DnsDxe/DnsDriver.c
@@ -1,9 +1,9 @@
 /** @file
 The driver binding and service binding protocol for DnsDxe driver.
 
-Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 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
 
@@ -277,22 +277,20 @@ DnsCreateService (
 
   //
   // Create the timer used to time out the procedure which is used to
   // get the default IP address.
   //
-  if (DnsSb->IpVersion == IP_VERSION_4) {
-Status = gBS->CreateEvent (
-EVT_TIMER,
-TPL_CALLBACK,
-NULL,
-NULL,
->TimerToGetMap
-);
-if (EFI_ERROR (Status)) {
-  FreePool (DnsSb);
-  return Status;
-}
+  Status = gBS->CreateEvent (
+  EVT_TIMER,
+  TPL_CALLBACK,
+  NULL,
+  NULL,
+  >TimerToGetMap
+  );
+  if (EFI_ERROR (Status)) {
+FreePool (DnsSb);
+return Status;
   }
   
   //
   // Create the timer to retransmit packets.
   //
diff --git a/NetworkPkg/DnsDxe/DnsImpl.c b/NetworkPkg/DnsDxe/DnsImpl.c index 
794df1d..ea3d27d 100644
--- a/NetworkPkg/DnsDxe/DnsImpl.c
+++ b/NetworkPkg/DnsDxe/DnsImpl.c
@@ -1,9 +1,9 @@
 /** @file
 DnsDxe support functions implementation.
   
-Copyright (c) 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2016 - 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
 
@@ -641,13 +641,15 @@ Dns6GetMapping (
 
   if (Ip6Mode.IcmpTypeList != NULL) {
 FreePool (Ip6Mode.IcmpTypeList);
   }
 
-  if (Ip6Mode.IsConfigured) {
+  if (!Ip6Mode.IsStarted || Ip6Mode.IsConfigured) {
 Udp->Configure (Udp, NULL);
-return (BOOLEAN) (Udp->Configure (Udp, UdpCfgData) == EFI_SUCCESS);
+if (Udp->Configure (Udp, UdpCfgData) == EFI_SUCCESS) {
+  return TRUE;
+}
   }
 }
   }
 
   return FALSE;
diff --git a/NetworkPkg/DnsDxe/DnsProtocol.c b/NetworkPkg/DnsDxe/DnsProtocol.c 
index 0e7ed34..bd189ae 100644
--- a/NetworkPkg/DnsDxe/DnsProtocol.c
+++ b/NetworkPkg/DnsDxe/DnsProtocol.c
@@ -1104,11 +1104,13 @@ Dns6Configure (
 }
 
 //
 // Config UDP
 //
+gBS->RestoreTPL (OldTpl);
 Status = Dns6ConfigUdp (Instance, Instance->UdpIo);
+OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
 if (EFI_ERROR (Status)) {
   if (Instance->Dns6CfgData.DnsServerList != NULL) {
 FreePool (Instance->Dns6CfgData.DnsServerList);
 Instance->Dns6CfgData.DnsServerList = NULL;
   }
--
1.9.5.msysgit.1

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

Re: [edk2] [PATCH v1 1/1] OvmfPkg: Allow multiple add-pointer linker commands to same ACPI table

2017-03-29 Thread Laszlo Ersek

This is a very good first patch!

I have a few requests. I'll generally rehash the points from
.

On 03/29/17 09:50, Phil Dennis-Jordan wrote:
> From: Phil Dennis-Jordan 
> 
> ACPI tables may contain multiple pointer fields to the same destination
> table. For example, in some revisions, the FADT contains both DSDT and
> X_DSDT fields, and they may both point to the DSDT. Indeed, some operating
> systems demand this to be the case.
> 
> Previously, if Qemu created "add pointer" linker commands for both fields,
> the linking process would fail, as AcpiProtocol->InstallAcpiTable() may
> only be called once for each destination table and otherwise returns an
> error.

(1) Please be a bit more precise here: passing the exact same table
twice to InstallAcpiTable() is wrong, but for two different reasons.

Reason #1 is what you named -- some, but not all, ACPI table types are
not allowed to have multiple instances, and that aborts the linking
process. Reason #2 is that successfully installing the exact same table,
of a type that does allow multiple instances (such as an SSDT, there
could be others) is just as wrong, although it doesn't abort the linking.

> 
> This change adds a memoisation data structure which tracks the table
> pointers that have already been installed; even if the same pointer is
> encountered multiple times, it is only installed once.

(2) Please s/installed/processed/g above -- see for more below.

> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Phil Dennis-Jordan 
> ---
>  OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 109 
>  1 file changed, 89 insertions(+), 20 deletions(-)
> 
> diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c 
> b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> index 7bb2e3f21821..cffa838623cc 100644
> --- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> +++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
> @@ -100,6 +100,39 @@ BlobCompare (
>  
>  
>  /**
> +  Comparator function for two opaque pointers, ordering on pointer value
> +  itself.
> +  Can be used as both Key and UserStruct comparator.
> +
> +  @param[in] Pointer1  First pointer.
> +
> +  @param[in] Pointer2  Second pointer.
> +
> +  @retval <0  If Pointer1 compares less than Pointer2.
> +
> +  @retval  0  If Pointer1 compares equal to Pointer2.
> +
> +  @retval >0  If Pointer1 compares greater than Pointer2.
> +**/
> +STATIC
> +INTN
> +EFIAPI
> +PointerCompare (
> +  IN CONST VOID *Pointer1,
> +  IN CONST VOID *Pointer2
> +  )
> +{
> +  if (Pointer1 == Pointer2) {
> +return 0;
> +  } else if ((INTN)Pointer1 < (INTN)Pointer2) {

(3) Please use UINTN here, not INTN.

Also, I slightly dislike an "else" after a "return", but I guess it's up
to you. :)

> +return -1;
> +  } else {
> +return 1;
> +  }
> +}
> +
> +
> +/**
>Process a QEMU_LOADER_ALLOCATE command.
>  
>@param[in] Allocate The QEMU_LOADER_ALLOCATE command to process.
> @@ -535,27 +568,32 @@ UndoCmdWritePointer (
>This function assumes that the entire QEMU linker/loader command file has
>been processed successfully in a prior first pass.
>  
> -  @param[in] AddPointerThe QEMU_LOADER_ADD_POINTER command to 
> process.
> +  @param[in] AddPointer  The QEMU_LOADER_ADD_POINTER command to 
> process.
>  
> -  @param[in] Tracker   The ORDERED_COLLECTION tracking the BLOB user
> -   structures.
> +  @param[in] Tracker The ORDERED_COLLECTION tracking the BLOB 
> user
> + structures.
>  
> -  @param[in] AcpiProtocol  The ACPI table protocol used to install 
> tables.
> +  @param[in] AcpiProtocolThe ACPI table protocol used to install 
> tables.
>  
> -  @param[in,out] InstalledKey  On input, an array of INSTALLED_TABLES_MAX 
> UINTN
> -   elements, allocated by the caller. On output,
> -   the function will have stored (appended) the
> -   AcpiProtocol-internal key of the ACPI table 
> that
> -   the function has installed, if the AddPointer
> -   command identified an ACPI table that is
> -   different from RSDT and XSDT.
> +  @param[in,out] InstalledKeyOn input, an array of INSTALLED_TABLES_MAX 
> UINTN
> + elements, allocated by the caller. On 
> output,
> + the function will have stored (appended) the
> + AcpiProtocol-internal key of the ACPI table 
> that
> + the function has installed, if the 
> AddPointer
> + command identified an ACPI table that is
> + different from RSDT and XSDT.
>  
> -  @param[in,out] NumInstalled  On input, the

Re: [edk2] [RFC PATCH 0/6] OVMF: HFS+ (and Mac OS X boot)

2017-03-29 Thread Phil Dennis-Jordan

Hi Gabriel,

First off, thanks for going to the effort of building an HFS+ driver!
Due to travel, I've only just got around to checking these out, sorry.

Before I can try to help out with cleaning the patches up to the
extent they can be upstreamed, I still need to get them working.
boot.efi seems to load, and it finds the kernel, so the HFS+ driver is
definitely working, but it fails during early boot with "Error loading
drivers." This is with existing VM disk images that work with an old
variant of Reza's patchset, so it's possible that the prelinked kernel
image assumes something about the EFI implementation that's not true
for this patchset. I'll try reinstalling the VM from scratch with the
instructions from your website [1] before I try to dig deeper. It
could also be something to do with the hardlink issue I mention later
on, or maybe the lack of support for fragmented files.

A few general comments so far on the HFS patches below:

On Tue, Mar 7, 2017 at 4:14 PM, Gabriel L. Somlo  wrote:
>
> - patch 3/6: A BSD-licensed implementation of an FSW HFS+ driver.
>  Based on Apple's HFS+ specification (TN1150), this is
>  a minimalistic, bare-bones set of FSW methods capable
>  of locating and loading files from an HFS+ volume.
>  Lots of functionality (e.g. stat, readdir, hard/sym
>  links, or accessing files with more than 8 fragments)
>  is unimplemented at this time. I only implemented the
>  methods necessary to support loading Apple's boot.efi
>  and kernel.

The OS X installer disk images produced by Apple's own
"createinstallmedia" tool creates hardlinks to the bootloader and
kernel image, so supporting hardlinks might be a useful feature we can
add, as it would simplify the installer image creation process
somewhat. But I guess the higher priority is getting rid of the FSW
wrapper.

> If not, it might be worth factoring out the common bits and roll the
> the whole thing up into a standalone HFS+ driver, which would be a
> significantly different direction to go.

Based on Laszlo's comments, that seems to be the preferred approach.
(Not entirely unexpectedly.) I'm unfamiliar with the FSW, so I'll try
to find some time to look over that and work out if I can make a
sufficient time investment to help out with distilling this down. Do
you think it'd be easier to start from zero, or to rip out FSW bits
one by one and hard-code them into the HFS+ driver, having the whole
thing working throughout?

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

Re: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm support

2017-03-29 Thread Michael Zimmermann

How can I use this? If I change my compressed FV's GUID to
3D532050-5CDA-4FD0-879E-0F7F630D5AFB I get the following error:

GenFds.py...
: error F003: No tool found with GUID 3D532050-5CDA-4FD0-879E-0F7F630D5AFB

Thanks
Michael

On Mon, Mar 27, 2017 at 5:15 AM, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>
>>-Original Message-
>>From: Song, BinX
>>Sent: Thursday, March 23, 2017 2:05 PM
>>To: edk2-devel@lists.01.org
>>Cc: Gao, Liming 
>>Subject: RE: [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli algorithm
>>support
>>
>>Hi All,
>>
>>The code is also in https://github.com/binxsong/edk2/tree/Brotli_V1
>>
>>Best Regards,
>>Bell Song
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>>Song,
>>> BinX
>>> Sent: Thursday, March 23, 2017 10:16 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Gao, Liming 
>>> Subject: [edk2] [PATCH 0/4] MdeModulePkg/BaseTools: Add Brotli
>>algorithm
>>> support
>>>
>>> Brotli algorithm was released on the website
>>https://github.com/google/brotli.
>>> It has a little less compress ratio than Lzma, but has better decompress
>>> performance than it.
>>> Add Brotli algorithm support, include Brotli decompression library and tool
>>set.
>>>
>>> Tested on:
>>> OS: Windows
>>> Arch: IA32/X64
>>> Platform: Nt32Pkg
>>> ToolChain: VS2015x86
>>> Target: Release
>>>
>>> OS: Ubuntu
>>> Arch: IA32/X64
>>> Platform: OvmfPkgIa32.dsc/OvmfPkgX64.dsc
>>> ToolChain: GCC5
>>> Target: Release
>>>
>>> Cc: Liming Gao 
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Bell Song 
>>>
>>> Bell Song (4):
>>>   MdeModulePkg: Copy Brotli algorithm 3rd party source code for library
>>>   MdeModulePkg: Add Brotli algorithm decompression library
>>>   BaseTools: Copy Brotli algorithm 3rd party source code for tool
>>>   BaseTools: Add Brotli algorithm tool
>>>
>>>  BaseTools/BinWrappers/PosixLike/Brotli |29 +
>>>  BaseTools/BinWrappers/PosixLike/BrotliCompress |42 +
>>>  BaseTools/Conf/tools_def.template  | 6 +
>>>  .../Source/C/BrotliCompress/BrotliCompress.bat |48 +
>>>  BaseTools/Source/C/BrotliCompress/GNUmakefile  |43 +
>>>  BaseTools/Source/C/BrotliCompress/LICENSE  |19 +
>>>  BaseTools/Source/C/BrotliCompress/Makefile |60 +
>>>  BaseTools/Source/C/BrotliCompress/README.md|26 +
>>>  BaseTools/Source/C/BrotliCompress/ReadMe.txt   | 2 +
>>>  .../Source/C/BrotliCompress/common/constants.h |47 +
>>>  .../Source/C/BrotliCompress/common/dictionary.c|  9474
>>> 
>>>  .../Source/C/BrotliCompress/common/dictionary.h|29 +
>>>  BaseTools/Source/C/BrotliCompress/common/port.h|   107 +
>>>  BaseTools/Source/C/BrotliCompress/common/types.h   |58 +
>>>  BaseTools/Source/C/BrotliCompress/dec/bit_reader.c |48 +
>>>  BaseTools/Source/C/BrotliCompress/dec/bit_reader.h |   383 +
>>>  BaseTools/Source/C/BrotliCompress/dec/context.h|   251 +
>>>  BaseTools/Source/C/BrotliCompress/dec/decode.c |  2347 
>>>  BaseTools/Source/C/BrotliCompress/dec/decode.h |   188 +
>>>  BaseTools/Source/C/BrotliCompress/dec/huffman.c|   357 +
>>>  BaseTools/Source/C/BrotliCompress/dec/huffman.h|68 +
>>>  BaseTools/Source/C/BrotliCompress/dec/port.h   |   159 +
>>>  BaseTools/Source/C/BrotliCompress/dec/prefix.h |   751 ++
>>>  BaseTools/Source/C/BrotliCompress/dec/state.c  |   168 +
>>>  BaseTools/Source/C/BrotliCompress/dec/state.h  |   246 +
>>>  BaseTools/Source/C/BrotliCompress/dec/transform.h  |   300 +
>>>  .../docs/brotli-comparison-study-2015-09-22.pdf|   Bin 0 -> 215208 
>>> bytes
>>>  .../C/BrotliCompress/enc/backward_references.c |   892 ++
>>>  .../C/BrotliCompress/enc/backward_references.h |99 +
>>>  .../C/BrotliCompress/enc/backward_references_inc.h |   147 +
>>>  BaseTools/Source/C/BrotliCompress/enc/bit_cost.c   |35 +
>>>  BaseTools/Source/C/BrotliCompress/enc/bit_cost.h   |63 +
>>>  .../Source/C/BrotliCompress/enc/bit_cost_inc.h |   127 +
>>>  .../C/BrotliCompress/enc/block_encoder_inc.h   |33 +
>>>  .../Source/C/BrotliCompress/enc/block_splitter.c   |   197 +
>>>  .../Source/C/BrotliCompress/enc/block_splitter.h   |51 +
>>>  .../C/BrotliCompress/enc/block_splitter_inc.h  |   432 +
>>>  .../C/BrotliCompress/enc/brotli_bit_stream.c   |  1334 +++
>>>  .../C/BrotliCompress/enc/brotli_bit_stream.h   |   107 +
>>>  BaseTools/Source/C/BrotliCompress/enc/cluster.c|56 +
>>>  BaseTools/Source/C/BrotliCompress/enc/cluster.h|48 +
>>>  .../Source/C/BrotliCompress/enc/cluster_inc.h  |   315 +
>>>  BaseTools/Source/C/BrotliCompress/enc/command.h|   163 +
>>>  .../C/BrotliCompress/enc/compress_fragment.c   |   747 ++
>>>

Re: [edk2] [PATCH v2] ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable override

2017-03-29 Thread Laszlo Ersek

On 03/29/17 21:10, Ard Biesheuvel wrote:
> On 29 March 2017 at 19:44, Laszlo Ersek  wrote:
>> On 03/29/17 19:50, Ard Biesheuvel wrote:
>>> In general, we should not present two separate (and inevitably different)
>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>> vice versa.
>>>
>>> However, this is arguably a regression for those who relied on DT
>>> descriptions being available, even if the former behavior can be
>>> restored by passing the -no-acpi switch to QEMU.
>>>
>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>> will result in ACPI to be disabled on subsequent boots even if -no-acpi
>>> was not passed on the QEMU command line.
>>>
>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceNoAcpi =01
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  ArmVirtPkg/ArmVirtPkg.dec| 9 +
>>>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 2 ++
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>>>  4 files changed, 19 insertions(+)
>>>
>>> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
>>> index efe83a383d55..a8603e1b80e5 100644
>>> --- a/ArmVirtPkg/ArmVirtPkg.dec
>>> +++ b/ArmVirtPkg/ArmVirtPkg.dec
>>> @@ -34,6 +34,8 @@ [Guids.common]
>>>gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 
>>> 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
>>>gEarlyPL011BaseAddressGuid   = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 
>>> 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
>>>
>>> +  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 
>>> 0x59, 0x59, 0x65, 0x16, 0xb0, 0x0a } }
>>> +
>>>  [Protocols]
>>>gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 
>>> 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>>>
>>> @@ -58,3 +60,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>>># EFI_VT_100_GUID.
>>>#
>>>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 
>>> 0xDF, 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
>>> 0x4D}|VOID*|0x0007
>>> +
>>> +[PcdsDynamic]
>>> +  #
>>> +  # Whether to force disable ACPI, regardless of the fw_cfg settings
>>> +  # exposed by QEMU
>>> +  #
>>> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|0x0|BOOLEAN|0x0003
>>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>>> index 4075b92aa2cb..76a7908105ab 100644
>>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>>> @@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>>>gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>>>
>>> +[PcdsDynamicHii]
>>> +  
>>> gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
>>> +
>>>  
>>> 
>>>  #
>>>  # Components Section - list of all EDK II Modules needed by this Platform
>>> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c 
>>> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
>>> index 8932dacabec5..da3cee645cfb 100644
>>> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
>>> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
>>> @@ -17,6 +17,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>
>>> @@ -37,6 +38,7 @@ PlatformHasAcpiDt (
>>>// errors here.
>>>//
>>>if (MAX_UINTN == MAX_UINT64 &&
>>> +  !PcdGetBool (PcdForceNoAcpi) &&
>>>!EFI_ERROR (
>>>   QemuFwCfgFindFile (
>>> "etc/table-loader",
>>> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf 
>>> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>>> index 4466bead57c2..08025f0c3722 100644
>>> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>>> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>>> @@ -25,6 +25,7 @@ [Sources]
>>>PlatformHasAcpiDtDxe.c
>>>
>>>  [Packages]
>>> +  ArmVirtPkg/ArmVirtPkg.dec
>>>EmbeddedPkg/EmbeddedPkg.dec
>>>MdePkg/MdePkg.dec
>>>OvmfPkg/OvmfPkg.dec
>>> @@ -32,6 +33,7 @@ [Packages]
>>>  [LibraryClasses]
>>>BaseLib
>>>DebugLib
>>> +  PcdLib
>>>QemuFwCfgLib
>>>UefiBootServicesTableLib
>>>UefiDriverEntryPoint
>>> @@ -40,5 +42,8 @@ [Guids]
>>>gEdkiiPlatformHasAcpiGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
>>>gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>>>
>>> +[Pcd]
>>> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
>>> +
>>>  [Depex]
>>>TRUE
>>>
>>
>> Technically

Re: [edk2] [PATCH v2] ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable override

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 19:44, Laszlo Ersek  wrote:
> On 03/29/17 19:50, Ard Biesheuvel wrote:
>> In general, we should not present two separate (and inevitably different)
>> hardware descriptions to the OS, in the form of ACPI tables and a device
>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>> vice versa.
>>
>> However, this is arguably a regression for those who relied on DT
>> descriptions being available, even if the former behavior can be
>> restored by passing the -no-acpi switch to QEMU.
>>
>> So allow a secret handshake with the UEFI Shell, to set a variable that
>> will result in ACPI to be disabled on subsequent boots even if -no-acpi
>> was not passed on the QEMU command line.
>>
>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceNoAcpi =01
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/ArmVirtPkg.dec| 9 +
>>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 2 ++
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>>  4 files changed, 19 insertions(+)
>>
>> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
>> index efe83a383d55..a8603e1b80e5 100644
>> --- a/ArmVirtPkg/ArmVirtPkg.dec
>> +++ b/ArmVirtPkg/ArmVirtPkg.dec
>> @@ -34,6 +34,8 @@ [Guids.common]
>>gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 
>> 0x2E, 0x36, 0x5B, 0x80, 0x63, 0x66 } }
>>gEarlyPL011BaseAddressGuid   = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 
>> 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
>>
>> +  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 
>> 0x59, 0x59, 0x65, 0x16, 0xb0, 0x0a } }
>> +
>>  [Protocols]
>>gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 
>> 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>>
>> @@ -58,3 +60,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
>># EFI_VT_100_GUID.
>>#
>>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
>> 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
>> 0x4D}|VOID*|0x0007
>> +
>> +[PcdsDynamic]
>> +  #
>> +  # Whether to force disable ACPI, regardless of the fw_cfg settings
>> +  # exposed by QEMU
>> +  #
>> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|0x0|BOOLEAN|0x0003
>> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
>> index 4075b92aa2cb..76a7908105ab 100644
>> --- a/ArmVirtPkg/ArmVirtQemu.dsc
>> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>> @@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
>>gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>>gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>>
>> +[PcdsDynamicHii]
>> +  
>> gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
>> +
>>  
>> 
>>  #
>>  # Components Section - list of all EDK II Modules needed by this Platform
>> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c 
>> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
>> index 8932dacabec5..da3cee645cfb 100644
>> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
>> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
>> @@ -17,6 +17,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>
>> @@ -37,6 +38,7 @@ PlatformHasAcpiDt (
>>// errors here.
>>//
>>if (MAX_UINTN == MAX_UINT64 &&
>> +  !PcdGetBool (PcdForceNoAcpi) &&
>>!EFI_ERROR (
>>   QemuFwCfgFindFile (
>> "etc/table-loader",
>> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf 
>> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> index 4466bead57c2..08025f0c3722 100644
>> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> @@ -25,6 +25,7 @@ [Sources]
>>PlatformHasAcpiDtDxe.c
>>
>>  [Packages]
>> +  ArmVirtPkg/ArmVirtPkg.dec
>>EmbeddedPkg/EmbeddedPkg.dec
>>MdePkg/MdePkg.dec
>>OvmfPkg/OvmfPkg.dec
>> @@ -32,6 +33,7 @@ [Packages]
>>  [LibraryClasses]
>>BaseLib
>>DebugLib
>> +  PcdLib
>>QemuFwCfgLib
>>UefiBootServicesTableLib
>>UefiDriverEntryPoint
>> @@ -40,5 +42,8 @@ [Guids]
>>gEdkiiPlatformHasAcpiGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
>>gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>>
>> +[Pcd]
>> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
>> +
>>  [Depex]
>>TRUE
>>
>
> Technically the patch is sound. I continue to disagree with its goal though.
>
> Technically, the patch could be improved (towards its wrong goal) by
> exposing the boolean knob with

Re: [edk2] Sec and Reset vector

2017-03-29 Thread Rafael Machado

Hi guys.

This e-mail if for the future generations, and proves everything you guys
commented to guide me when I did the first question.

So if you have a uefi firmware image, and would like to check how
everything happens using radare,
you need to do the following:

step 1: Check your .bin file size (in my case the image has 16MB)
step 2: convert the image size using a calculator (16MB * 1024 * 1024 =
0x100 )
step 3: Since everything at the first stages of the boot happens in real
mode, the instructions are 16 bits long, so you need to go back 16 bits,
this gives you the position 0xF0. Now you are at the reset vector

At radare you can do all this using these commands:

radare2.exe mybios.bin -b 16
> s 0x100
> s -16
> pd 10

The attached image shows an example of the result.
Thanks everyone for the comments and tips.


Rafael R. Machado

[image: FindingTheResetVector.png]


Em sex, 4 de nov de 2016 às 20:19, Rafael Machado <
rafaelrodrigues.mach...@gmail.com> escreveu:

> All the answers were really detailed and I agree with Andrew. These
> answers are a great help when we are learning.
>
> Thanks a lot guys. I learn a lot with this community.
>
> Thanks and regards
>
>
> Rafael
>
> Em sex, 4 de nov de 2016 19:28, Kinney, Michael D <
> michael.d.kin...@intel.com> escreveu:
>
> Rafael,
>
> The first instruction executed for IA32 SEC phase is typically 16-bytes
> from
> the end of the Firmware Device (FD) image generated by a build.
>
> If you look at QuarkPlatformPkg/Quark.dsc as an example build, it generates
> an 8MB  file called Quark.FD.  The reset vector is 16-bytes from the end of
> that file.  The reset vector and rest of SEC code are all in a special FFS
> file
> known at the Volume Top File(VFT).
>
> The source code for the reset vector is in the file:
>
> UefiCpuPkg/SecCore/Ia32/ResetVector.nasmb
>
> This source file contains the code that fills the last 0x40 bytes of the
> VTF
> which is also the last 0x40 bytes of Quark.FD.
>
> The build tools do some special fixups on this file, so 16-bit relative JMP
> instruction at line 79 is fixed up to jump to the symbol _ModuleEntryPoint.
>
> ResetHandler:
> nop
> nop
> ApStartup:
> ;
> ; Jmp Rel16 instruction
> ; Use machine code directly in case of the assembler
> optimization
> ; SEC entry point relative address will be fixed up by some
> build tool.
> ;
> ; Typically, SEC entry point is the function
> _ModuleEntryPoint() defined in
> ; SecEntry.asm
> ;
> DB  0e9h
> DW  -3
>
> For Quark.FD, _ModuleEntryPoint is from the PlatformSecLib library at:
>
> QuarkPlatformPkg/Library/PlatformSecLib
>
> Specifically Line l5 of the file:
>
> QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.asm
>
> _ModuleEntryEntryPoint starts in 16-bit real mode and transitions to
> 32-bit protected mode, initializes ESRAM to be used as a stack, and
> transfers control to the C function PlatformSecLibStartup() at line 220.
> The goal is to minimize the amount of assembly code and get into C code
> as quickly as possible.
>
> The PlatformSecLibStartup() function implementation is in the same
> PlatformSecLib library at line 68 of the file:
>
> QuarkPlatformPkg/Library/PlatformSecLib/PlatformSecLib.c
>
> Best regards,
>
> Mike
>
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Rafael Machado
> > Sent: Friday, November 4, 2016 12:34 PM
> > To: Andrew Fish 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] Sec and Reset vector
> >
> > Hi Andrew
> >
> > Maybe my question was not clear.
> > But thanks for the information you provided.
> >
> > I think we can simplify what I need based on your last comment. You told:
> >
> > *"The .com file contains the hardware real mode reset vector
> (0xFFF0).
> > So execution starts up high. The 1st far jmp you do gets you running in
> the
> > 0x000F range (the ROM is aliased to the old PC address)"*
> >
> > How could I find the .com file inside the dump of a bios (I have used
> > DediProg to get the dump)?
> > I'm hunting the first far jump the firmware has.
> >
> > Thanks
> > Rafael
> >
> > Em sex, 4 de nov de 2016 às 16:50, Andrew Fish 
> escreveu:
> >
> > > On Nov 4, 2016, at 10:48 AM, Rafael Machado <
> > > rafaelrodrigues.mach...@gmail.com> wrote:
> > >
> > > Hi everyone
> > >
> > > Thanks Andrew and Marvin for the clarification.
> > > Now things start to make sense.
> > >
> > > But I was still not able to understand were things start on a real
> binary
> > >
> > >
> > > Rafeal,
> > >
> > > I'm not sure what you are asking?
> > >
> > > The .com file contains the hardware real mode reset vector
> (0xFFF0).
> > > So execution starts up high. The 1st far jmp you do gets you running
> in the
> > > 0x000F range (the ROM is

Re: [edk2] [edk2-staging][PATCH 2/2] ShellPkg: Add acpiview tool to dump ACPI tables

2017-03-29 Thread Sami Mujawar

Hi Jeff,

Sadly, we do not have an environment setup to build tianocore-edk2 using Visual 
Studio 2015. We build the edk2 from the windows command prompt using the linaro 
gcc toolchain for ARM/AARCH64.
Is it possible to send the error log, please ?

Evan has made a request for creation of a branch on the edk2-staging 
repository. We would be very happy for you to contribute changes once this is 
in place.

Regards,

Sami Mujawar
-Original Message-
From: Jeff Westfahl [mailto:jeff.westf...@ni.com]
Sent: 28 March 2017 08:50
To: Sami Mujawar
Cc: Jeff Westfahl; Evan Lloyd; Ruiyu Ni Leif Lindholm; edk2-de...@ml01.01.org; 
Jaben Carsey; Jiewen Yao; Michael D Kinney; linaro@ml01.01.org; Dong Wei
Subject: RE: [edk2] [edk2-staging][PATCH 2/2] ShellPkg: Add acpiview tool to 
dump ACPI tables

Hi Sami,

Just another note, acpiview doesn't currently build on Windows, at least not 
with Visual Studio 2015. This isn't a problem for me as I don't usually build 
the shell on Windows, I just happened to notice it.

Regards,

Jeff Westfahl

On Fri, 17 Mar 2017, Sami Mujawar wrote:

> Hi Jeff,
>
> Thank you for letting us know these issues. I will provide an update to fix 
> this soon.
>
> Regards,
>
> Sami Mujawar
>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jeff Westfahl
> Sent: 16 March 2017 03:12
> To: Evan Lloyd
> Cc: Ruiyu Ni Leif Lindholm; edk2-de...@ml01.01.org; Jaben Carsey;
> Jiewen Yao; Michael D Kinney; linaro@ml01.01.org; Dong Wei
> Subject: Re: [edk2] [edk2-staging][PATCH 2/2] ShellPkg: Add acpiview
> tool to dump ACPI tables
>
> Hi Evan,
>
> I really appreciate this work, I've missed this functionality in the EFI 
> shell.
>
> Just a few minor things that I noticed:
>
> 1. In AcpiView.c, you use '\t' in a few places. For some reason, on my Intel 
> PC, these tabs are displayed as a reverse color '?'. I'm not sure where the 
> fault lies here. It seems like tab characters should be pretty universally 
> understood, but I also don't see any tab characters in other parts of the 
> shell code that are printed to the console.
>
> 2. In MadtParser.c, the error message at line 270 needs a newline.
> (No, my Intel PC doesn't have a GIC :)
>
> Regards,
> Jeff
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> IMPORTANT NOTICE: The contents of this email and any attachments are 
> confidential and may also be privileged. If you are not the intended 
> recipient, please notify the sender immediately and do not disclose the 
> contents to any other person, use it for any purpose, or store or copy the 
> information in any medium. Thank you.
>
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable override

2017-03-29 Thread Laszlo Ersek

On 03/29/17 19:50, Ard Biesheuvel wrote:
> In general, we should not present two separate (and inevitably different)
> hardware descriptions to the OS, in the form of ACPI tables and a device
> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
> vice versa.
> 
> However, this is arguably a regression for those who relied on DT
> descriptions being available, even if the former behavior can be
> restored by passing the -no-acpi switch to QEMU.
> 
> So allow a secret handshake with the UEFI Shell, to set a variable that
> will result in ACPI to be disabled on subsequent boots even if -no-acpi
> was not passed on the QEMU command line.
> 
>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceNoAcpi =01
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 9 +
>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 2 ++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>  4 files changed, 19 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index efe83a383d55..a8603e1b80e5 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -34,6 +34,8 @@ [Guids.common]
>gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 
> 0x36, 0x5B, 0x80, 0x63, 0x66 } }
>gEarlyPL011BaseAddressGuid   = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 
> 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
>  
> +  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 
> 0x59, 0x65, 0x16, 0xb0, 0x0a } }
> +
>  [Protocols]
>gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 
> 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>  
> @@ -58,3 +60,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
># EFI_VT_100_GUID.
>#
>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
> 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
> 0x4D}|VOID*|0x0007
> +
> +[PcdsDynamic]
> +  #
> +  # Whether to force disable ACPI, regardless of the fw_cfg settings
> +  # exposed by QEMU
> +  #
> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|0x0|BOOLEAN|0x0003
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 4075b92aa2cb..76a7908105ab 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>  
> +[PcdsDynamicHii]
> +  
> gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
> +
>  
> 
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c 
> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> index 8932dacabec5..da3cee645cfb 100644
> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> @@ -17,6 +17,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -37,6 +38,7 @@ PlatformHasAcpiDt (
>// errors here.
>//
>if (MAX_UINTN == MAX_UINT64 &&
> +  !PcdGetBool (PcdForceNoAcpi) &&
>!EFI_ERROR (
>   QemuFwCfgFindFile (
> "etc/table-loader",
> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf 
> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> index 4466bead57c2..08025f0c3722 100644
> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> @@ -25,6 +25,7 @@ [Sources]
>PlatformHasAcpiDtDxe.c
>  
>  [Packages]
> +  ArmVirtPkg/ArmVirtPkg.dec
>EmbeddedPkg/EmbeddedPkg.dec
>MdePkg/MdePkg.dec
>OvmfPkg/OvmfPkg.dec
> @@ -32,6 +33,7 @@ [Packages]
>  [LibraryClasses]
>BaseLib
>DebugLib
> +  PcdLib
>QemuFwCfgLib
>UefiBootServicesTableLib
>UefiDriverEntryPoint
> @@ -40,5 +42,8 @@ [Guids]
>gEdkiiPlatformHasAcpiGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
>gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>  
> +[Pcd]
> +  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
> +
>  [Depex]
>TRUE
> 

Technically the patch is sound. I continue to disagree with its goal though.

Technically, the patch could be improved (towards its wrong goal) by
exposing the boolean knob with an HII checkbox, called "disable ACPI
regardless of what the QEMU command line says". That would mirror Marc's
comments from earlier.

For now, I actually agree with you that we

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 19:30, Marc Zyngier wrote:
> On 29/03/17 18:15, Laszlo Ersek wrote:
>> On 03/29/17 19:01, Marc Zyngier wrote:
>>> On 29/03/17 17:40, Laszlo Ersek wrote:
 On 03/29/17 18:07, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
 On 03/29/17 17:19, Ard Biesheuvel wrote:
> In general, we should not present two separate (and inevitably 
> different)
> hardware descriptions to the OS, in the form of ACPI tables and a 
> device
> tree blob. For this reason, we recently added the logic to 
> ArmVirtQemu to
> only expose the ACPI 2.0 entry point if no DT binary is being passed, 
> and
> vice versa.
>
> However, this is arguably a regression for those who rely on both
> descriptions being available, even if the use cases in question are
> uncommon.
>
> So allow a secret handshake with the UEFI Shell, to set a variable 
> that
> will result in both descriptions being exposed on the next boot, if
> executing in the default 'ACPI-only' mode.
>
>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt 
> =01
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>  4 files changed, 22 insertions(+), 1 deletion(-)

>>>
>>> [snip the policy argumentation, I only care about the technical aspects]
>>>
 On the technical side:

 - I think a dynamic boolean PCD would be superior, if that is possible
 with HII (= directly nvvar-backed) PCDs -- absence of the variable
 should map to FALSE. I'm unsure though if that were as easy to set from
 the UEFI shell as a UINT8. So stick with the current data type if you
 deem that superior (maybe comment on it in the commit message).

 - please include  in the C source, to reflect the
 [LibraryClasses] update in the INF.
>>>
>>> My personal choice would be *not* to expose both tables at the same
>>> time, but instead to be able to shift the preference from one side or
>>> the other, similarly to what a menu on a bare metal system would do.
>>
>> Umm... Are we in violent agreement? This works already.
>>
>> If you invoke QEMU with the normal command like, like you've always
>> done, you get ACPI only (right now).
>>
>> If you pass the "-no-acpi" switch in addition to your normal command
>> line, you get DT only. This is documented in detail on the following commit:
>>
>> https://github.com/tianocore/edk2/commit/110316a995ed
>>
>> If you pass -no-acpi on your QEMU command line, you can perform the
>> whole guest kernel bisection using purely DT, without ever having to
>> re-launch QEMU.
>>
>>>
>>> Lets call the variable PreferDT (rather than ForceDT), with the
>>> following (exhaustive) behaviour :
>>>
>>> - PreferDT==0 and ACPI+DT present -> ACPI
>>> - PreferDT==0 and ACPI present-> ACPI
>>> - PreferDT==0 and DT present  -> DT
>>> - PreferDT==1 and ACPI+DT present -> DT
>>> - PreferDT==1 and ACPI present-> ACPI
>>> - PreferDT==1 and DT present  -> DT
>>>
>>> It allows people with existing setups to still have something that works
>>> with minimal effort (still need to set this variable though).
>>>
>>> Could people agree on something like this?
>>
>> It's overly complex. With QEMU (and the current edk2 master state) you
>> already have a single (host-side) master knob for controlling the ACPI
>> vs. DT exposure.
> 
> You're missing the essential bit. I don't want a knob in QEMU. Having to
> mess with QEMU means that I can't have a uniform way of running a VM. I
> want one way of booting a VM, and let the VM pick the description it has
> been configured for.
> 
> On bare metal, I can switch UEFI from a menu entry to pick ACPI or DT.
> And that's good. I want the same freedom in a VM, at the same level.
> There is no technical reason to have a different behaviour.

Correct, you can have the exact same menu entry (which is what I've been
calling the HII checkbox) in virtual firmware, assuming someone writes
the platform DXE driver that implements this kind of policy.

As I wrote, I expected Ard to submit (at some undeterminate time in the
future) such a driver, which would replace the current policy drivers in:

- the Xen build of ArmVirtQemu, which has no access to fw_cfg, and

- the upcoming "showcase QEMU" platform, which would entirely ignore

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 19:44, Mark Rutland wrote:
> Hi,
> 
> On Wed, Mar 29, 2017 at 06:55:26PM +0200, Laszlo Ersek wrote:
>> On 03/29/17 18:17, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:09, Jon Masters  wrote:
 Thanks Laszlo. A quick note from me that regardless of this
 discussion I will be pushing to ensure the version Red Hat ships
 makes ACPI the default with it being extremely painful to use DT.
 It is time the ecosystem got with the program we all agreed upon
 and there will be no more excuses.
>>>
>>> This has *nothing* to do with the ecosystem. This has to do with
>>> existing users of ArmVirtQemu (admittedly, a small fraction) that will
>>> be forced to compile their UEFI images from source because we made a
>>> backwards incompatible change.
>>>
>>> I am 100% on board with making ACPI and DT mutually exclusive. But I
>>> don't believe for a second that 'everybody just exposes ACPI and DT at
>>> the same time' if this gets merged.
>>
>> That's where we disagree, 100%.
>>
>>> That happens because people are clueless, not because they are
>>> deliberately spending time and effort on producing two hardware
>>> descriptions.
>>
>> If this were true, then the kernel's preference would have been changed
>> to ACPI aeons ago (assuming both DT and ACPI were present).
> 
> As has been covered elsewhere, unilaterally changing the default breaks
> existing systems, regardless of what vendors do from now on.
> 
>> ACPI is superior to DT (cue again Grant Likely's blog post), yet
>> kernel people resist it. 
> 
> In several respects, sure, though let's not be under the illusion that
> it is not free of novel technical problems.
> 
> The big deal with ACPI is that there are other major OS vendors who will
> support ACPI, but not DT. To avoid a fragmented market, commodity
> servers must use ACPI.
> 
>> That's not cluelessness. If the kernel's DT camp has any
>> influence on platform vendors (and that's rather more a "given that"
>> than an "if"), when they find out about this loophole, I expect them to
>> actively recommend it as a way to perpetuate the status quo.
> 
> Who is this "DT camp"?

I assume the people who have thus far prevented the preference
switch-over from DT to ACPI, provided a system exposed both.

I realize (as you say above) that this might render some systems
temporarily unbootable. Yes. But without that pain, no platform vendor
will *ever* be incentivized to get their ACPI tables in order. So the
idea here is to force that pain from the firmware side.

On one hand, it is a regression, yes. On the other hand, if it's not
broken (intentionally), why would anyone ever fix it? (This argument is
based on the fact that the shippers of broken ACPI tables actually label
their machines SBBR-conformant. Nominally, they *want* to support ACPI.)

> As a DT binding maintainer, I would be livid were people recommending
> using this in generic OS images.

The kernel still prefers DT over ACPI if both are present; that's just
the same mindset.

Laszlo

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 19:30, Ard Biesheuvel wrote:
> On 29 March 2017 at 18:23, Laszlo Ersek  wrote:
>> On 03/29/17 19:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 18:01, Marc Zyngier  wrote:
 On 29/03/17 17:40, Laszlo Ersek wrote:
> [...]
> On the technical side:
>
> - I think a dynamic boolean PCD would be superior, if that is possible
> with HII (= directly nvvar-backed) PCDs -- absence of the variable
> should map to FALSE. I'm unsure though if that were as easy to set from
> the UEFI shell as a UINT8. So stick with the current data type if you
> deem that superior (maybe comment on it in the commit message).
>
> - please include  in the C source, to reflect the
> [LibraryClasses] update in the INF.

 My personal choice would be *not* to expose both tables at the same
 time, but instead to be able to shift the preference from one side or
 the other, similarly to what a menu on a bare metal system would do.

>>>
>>> So to clarify, you want something sticky in the firmware settings
>>> rather than having to use the -no-acpi command line argument to QEMU?
>>
>> If you *really* want to control it within the guest, on the guest
>> firmware level, while keeping the exclusive nature, then there are
>> better options for that.
>>
>> Namely, a variation of your HII-exposed driver added in:
>>
>> https://github.com/tianocore/edk2/commit/779cc439e881
>>
>> In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
>> driver that exposes the same exclusive toggle as an HII checkbox. The
>> HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
>> vs EdkiiPlatformHasDeviceTreeGuid.
>>
>> Note that I disagree with *combining* a HII toggle with the current
>> fw_cfg-based knob (that is, the -no-acpi switch). That means there are
>> multiple masters for the same decision, which invariably leads to confusion.
>>
>> Therefore any ArmVirtQemu platform build should either offer the fw_cfg
>> swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
>> Never both.
>>
> 
> I don't see why we couldn't have two ways to disable ACPI.

I argue against it because it will confuse users and will lead to bug
reports we'll have to field. I'm sure you would not like two separate
config files, with two differently called options in them, for the same
daemon process. You would flip one setting, and wonder why the daemon
doesn't care.

> 
>> In fact I've fully expected Ard to post such an HII driver down the
>> road, for the (upcoming) "showcase QEMU" virtualization platform (and
>> QEMU  machine type). The target environment of that platform wouldn't be
>> the data center (where you really want to control most things from the
>> host side) -- the goal would be to model physical hardware very closely,
>> even as far as human interaction goes.
>>
>> So, let us figure out what we ultimately want (well, I know what I want,
>> what do you guys want)?
>>
>> - For the current (data center virtualization oriented case), the
>> DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
>> - For the "showcase QEMU" case, another HII driver would be necessary.
>> DT and ACPI would remain exclusive, but they would be controlled
>> (visually, interactively) from the guest firmware.
>> - Both of these control methods should never be enabled in the same
>> firmware build, at the same time. If Ard writes the HII driver, we can
>> introduce a build time flag that switches between the two control
>> methods. In no case would it be possible for DT and ACPI to appear together.
>>
> 
> Again, the aim is to recover some of the lost functionality for users
> with special requirements. If the alternative implementation will not
> be made available as widely, and needs to be built from source, it is
> not really an improvement over the current situation.
> 

I understand your point.

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

[edk2] [PATCH v2] ArmVirtPkg/PlatformHasAcpiDtDxe: allow guest level ACPI disable override

2017-03-29 Thread Ard Biesheuvel

In general, we should not present two separate (and inevitably different)
hardware descriptions to the OS, in the form of ACPI tables and a device
tree blob. For this reason, we recently added the logic to ArmVirtQemu to
only expose the ACPI 2.0 entry point if no DT binary is being passed, and
vice versa.

However, this is arguably a regression for those who relied on DT
descriptions being available, even if the former behavior can be
restored by passing the -no-acpi switch to QEMU.

So allow a secret handshake with the UEFI Shell, to set a variable that
will result in ACPI to be disabled on subsequent boots even if -no-acpi
was not passed on the QEMU command line.

  setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceNoAcpi =01

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtPkg.dec| 9 +
 ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 2 ++
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
 4 files changed, 19 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index efe83a383d55..a8603e1b80e5 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -34,6 +34,8 @@ [Guids.common]
   gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 
0x36, 0x5B, 0x80, 0x63, 0x66 } }
   gEarlyPL011BaseAddressGuid   = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 
0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
 
+  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 
0x59, 0x65, 0x16, 0xb0, 0x0a } }
+
 [Protocols]
   gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 
0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
 
@@ -58,3 +60,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # EFI_VT_100_GUID.
   #
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
0x4D}|VOID*|0x0007
+
+[PcdsDynamic]
+  #
+  # Whether to force disable ACPI, regardless of the fw_cfg settings
+  # exposed by QEMU
+  #
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi|0x0|BOOLEAN|0x0003
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 4075b92aa2cb..76a7908105ab 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
+[PcdsDynamicHii]
+  
gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c 
b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
index 8932dacabec5..da3cee645cfb 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -37,6 +38,7 @@ PlatformHasAcpiDt (
   // errors here.
   //
   if (MAX_UINTN == MAX_UINT64 &&
+  !PcdGetBool (PcdForceNoAcpi) &&
   !EFI_ERROR (
  QemuFwCfgFindFile (
"etc/table-loader",
diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf 
b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
index 4466bead57c2..08025f0c3722 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
@@ -25,6 +25,7 @@ [Sources]
   PlatformHasAcpiDtDxe.c
 
 [Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
@@ -32,6 +33,7 @@ [Packages]
 [LibraryClasses]
   BaseLib
   DebugLib
+  PcdLib
   QemuFwCfgLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
@@ -40,5 +42,8 @@ [Guids]
   gEdkiiPlatformHasAcpiGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
   gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
 
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdForceNoAcpi
+
 [Depex]
   TRUE
-- 
2.9.3

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Mark Rutland

Hi,

On Wed, Mar 29, 2017 at 06:55:26PM +0200, Laszlo Ersek wrote:
> On 03/29/17 18:17, Ard Biesheuvel wrote:
> > On 29 March 2017 at 17:09, Jon Masters  wrote:
> >> Thanks Laszlo. A quick note from me that regardless of this
> >> discussion I will be pushing to ensure the version Red Hat ships
> >> makes ACPI the default with it being extremely painful to use DT.
> >> It is time the ecosystem got with the program we all agreed upon
> >> and there will be no more excuses.
> > 
> > This has *nothing* to do with the ecosystem. This has to do with
> > existing users of ArmVirtQemu (admittedly, a small fraction) that will
> > be forced to compile their UEFI images from source because we made a
> > backwards incompatible change.
> > 
> > I am 100% on board with making ACPI and DT mutually exclusive. But I
> > don't believe for a second that 'everybody just exposes ACPI and DT at
> > the same time' if this gets merged.
> 
> That's where we disagree, 100%.
> 
> > That happens because people are clueless, not because they are
> > deliberately spending time and effort on producing two hardware
> > descriptions.
> 
> If this were true, then the kernel's preference would have been changed
> to ACPI aeons ago (assuming both DT and ACPI were present).

As has been covered elsewhere, unilaterally changing the default breaks
existing systems, regardless of what vendors do from now on.

> ACPI is superior to DT (cue again Grant Likely's blog post), yet
> kernel people resist it. 

In several respects, sure, though let's not be under the illusion that
it is not free of novel technical problems.

The big deal with ACPI is that there are other major OS vendors who will
support ACPI, but not DT. To avoid a fragmented market, commodity
servers must use ACPI.

> That's not cluelessness. If the kernel's DT camp has any
> influence on platform vendors (and that's rather more a "given that"
> than an "if"), when they find out about this loophole, I expect them to
> actively recommend it as a way to perpetuate the status quo.

Who is this "DT camp"?

As a DT binding maintainer, I would be livid were people recommending
using this in generic OS images.

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 18:23, Laszlo Ersek  wrote:
> On 03/29/17 19:07, Ard Biesheuvel wrote:
>> On 29 March 2017 at 18:01, Marc Zyngier  wrote:
>>> On 29/03/17 17:40, Laszlo Ersek wrote:
[...]
 On the technical side:

 - I think a dynamic boolean PCD would be superior, if that is possible
 with HII (= directly nvvar-backed) PCDs -- absence of the variable
 should map to FALSE. I'm unsure though if that were as easy to set from
 the UEFI shell as a UINT8. So stick with the current data type if you
 deem that superior (maybe comment on it in the commit message).

 - please include  in the C source, to reflect the
 [LibraryClasses] update in the INF.
>>>
>>> My personal choice would be *not* to expose both tables at the same
>>> time, but instead to be able to shift the preference from one side or
>>> the other, similarly to what a menu on a bare metal system would do.
>>>
>>
>> So to clarify, you want something sticky in the firmware settings
>> rather than having to use the -no-acpi command line argument to QEMU?
>
> If you *really* want to control it within the guest, on the guest
> firmware level, while keeping the exclusive nature, then there are
> better options for that.
>
> Namely, a variation of your HII-exposed driver added in:
>
> https://github.com/tianocore/edk2/commit/779cc439e881
>
> In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
> driver that exposes the same exclusive toggle as an HII checkbox. The
> HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
> vs EdkiiPlatformHasDeviceTreeGuid.
>
> Note that I disagree with *combining* a HII toggle with the current
> fw_cfg-based knob (that is, the -no-acpi switch). That means there are
> multiple masters for the same decision, which invariably leads to confusion.
>
> Therefore any ArmVirtQemu platform build should either offer the fw_cfg
> swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
> Never both.
>

I don't see why we couldn't have two ways to disable ACPI.

> In fact I've fully expected Ard to post such an HII driver down the
> road, for the (upcoming) "showcase QEMU" virtualization platform (and
> QEMU  machine type). The target environment of that platform wouldn't be
> the data center (where you really want to control most things from the
> host side) -- the goal would be to model physical hardware very closely,
> even as far as human interaction goes.
>
> So, let us figure out what we ultimately want (well, I know what I want,
> what do you guys want)?
>
> - For the current (data center virtualization oriented case), the
> DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
> - For the "showcase QEMU" case, another HII driver would be necessary.
> DT and ACPI would remain exclusive, but they would be controlled
> (visually, interactively) from the guest firmware.
> - Both of these control methods should never be enabled in the same
> firmware build, at the same time. If Ard writes the HII driver, we can
> introduce a build time flag that switches between the two control
> methods. In no case would it be possible for DT and ACPI to appear together.
>

Again, the aim is to recover some of the lost functionality for users
with special requirements. If the alternative implementation will not
be made available as widely, and needs to be built from source, it is
not really an improvement over the current situation.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 19:07, Ard Biesheuvel wrote:
> On 29 March 2017 at 18:01, Marc Zyngier  wrote:
>> On 29/03/17 17:40, Laszlo Ersek wrote:
>>> On 03/29/17 18:07, Ard Biesheuvel wrote:
 On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
> On 03/29/17 18:02, Ard Biesheuvel wrote:
>> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
 In general, we should not present two separate (and inevitably 
 different)
 hardware descriptions to the OS, in the form of ACPI tables and a 
 device
 tree blob. For this reason, we recently added the logic to ArmVirtQemu 
 to
 only expose the ACPI 2.0 entry point if no DT binary is being passed, 
 and
 vice versa.

 However, this is arguably a regression for those who rely on both
 descriptions being available, even if the use cases in question are
 uncommon.

 So allow a secret handshake with the UEFI Shell, to set a variable that
 will result in both descriptions being exposed on the next boot, if
 executing in the default 'ACPI-only' mode.

   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel 
 ---
  ArmVirtPkg/ArmVirtPkg.dec| 8 
  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
  4 files changed, 22 insertions(+), 1 deletion(-)
>>>
>>
>> [snip the policy argumentation, I only care about the technical aspects]
>>
>>> On the technical side:
>>>
>>> - I think a dynamic boolean PCD would be superior, if that is possible
>>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>>> should map to FALSE. I'm unsure though if that were as easy to set from
>>> the UEFI shell as a UINT8. So stick with the current data type if you
>>> deem that superior (maybe comment on it in the commit message).
>>>
>>> - please include  in the C source, to reflect the
>>> [LibraryClasses] update in the INF.
>>
>> My personal choice would be *not* to expose both tables at the same
>> time, but instead to be able to shift the preference from one side or
>> the other, similarly to what a menu on a bare metal system would do.
>>
> 
> So to clarify, you want something sticky in the firmware settings
> rather than having to use the -no-acpi command line argument to QEMU?

If you *really* want to control it within the guest, on the guest
firmware level, while keeping the exclusive nature, then there are
better options for that.

Namely, a variation of your HII-exposed driver added in:

https://github.com/tianocore/edk2/commit/779cc439e881

In this case, PlatformHasAcpiDtDxe should be replaced with a simple HII
driver that exposes the same exclusive toggle as an HII checkbox. The
HII checkbox would control the installation of EdkiiPlatformHasAcpiGuid
vs EdkiiPlatformHasDeviceTreeGuid.

Note that I disagree with *combining* a HII toggle with the current
fw_cfg-based knob (that is, the -no-acpi switch). That means there are
multiple masters for the same decision, which invariably leads to confusion.

Therefore any ArmVirtQemu platform build should either offer the fw_cfg
swtich (== -no-acpi QEMU command line option), *or* the HII toggle.
Never both.

In fact I've fully expected Ard to post such an HII driver down the
road, for the (upcoming) "showcase QEMU" virtualization platform (and
QEMU  machine type). The target environment of that platform wouldn't be
the data center (where you really want to control most things from the
host side) -- the goal would be to model physical hardware very closely,
even as far as human interaction goes.

So, let us figure out what we ultimately want (well, I know what I want,
what do you guys want)?

- For the current (data center virtualization oriented case), the
DT<->ACPI exclusion is controlled by QEMU's -no-acpi flag.
- For the "showcase QEMU" case, another HII driver would be necessary.
DT and ACPI would remain exclusive, but they would be controlled
(visually, interactively) from the guest firmware.
- Both of these control methods should never be enabled in the same
firmware build, at the same time. If Ard writes the HII driver, we can
introduce a build time flag that switches between the two control
methods. In no case would it be possible for DT and ACPI to appear together.

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 17:40, Laszlo Ersek  wrote:
> On 03/29/17 18:07, Ard Biesheuvel wrote:
>> On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
>>> On 03/29/17 18:02, Ard Biesheuvel wrote:
 On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
[..]
> NACK
>

 OK, fair enough. How do you propose to handle this regression then?

>>>
>>> I don't.
>>
>> I think I am entitled to a more constructive attitude from you. I
>> don't care about politics, but I do care about not breaking people's
>> workflows. So if you insist on getting on your high horse and throw
>> capitalized NACKs at me, you could at least *pretend* to care about
>> other users than *your* downstream, and come up with some alternative.
>
> Ard, I'm not on my high horse. I thought we were in agreement about
> this. It was obvious (to me at least) that this policy would be
> considered a regression by those who wanted both DT and ACPI in the
> guest. From my side, breaking that was all intentional.
>

Well, speaking for myself, I did not consider the need of some to have
both descriptions available. I take those needs very seriously.

> I'm not deliberately screwing with users (just because they have "niche"
> needs), and normally I would fully agree with you. The problem is that
> by providing an automated, upstream (centralized) opt-out from the
> mutual exclusion, the ecosystem will be allowed to suffer from the same
> nonchalance towards ACPI that has been the case until now. It's clear
> that your well-meaning change will be immediately exploited as the
> new-old default.
>
> Do you plan to add the same loophole to the HII-enabled driver that you
> recently committed as well?
>

No. That is mostly intended for new users (such as the Marvell
platform), though, which is why I only proposed to add it to TC2
(which is 32-bit so it does not use ACPI to begin with) and FVP (which
serves as a reference implementation for us.) Whether Juno can be
ported as well remains to be seen, but I would prefer to make ACPI and
DT mutually exclusive on that platform as well.

>
> Also, please don't accuse me of caring only about RH's downstream. The
> following blog post wasn't authored by a Red Hat employee:
>
> http://www.secretlab.ca/archives/151
>
> The following document is also not Red Hat exclusive:
>
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html
>
> What you are arguing for is, indirectly, to relicense platform vendors
> to continue ignoring ACPI, while (falsely) labeling their systems SBBR
> conformant. And then any OS vendor that actually cares about SBBR
> conformance (hint: it's not just Red Hat) sees brutal boot splats. That
> is not your intent (obviously), but that is the (seen, experienced)
> effect of providing both DT and ACPI.
>

Call me naive, but I really don't see how this change is going to
bring about all of that.

I understand that RedHat intends to start complaining noisily if ACPI
and DT are both exposed, and that having this wrong behavior in the
bundled VM firmware is undesirable, but that does not mean the world
is going to end for everybody if there is a manual override.

> My Nacked-by is not for the specific technical solution that you
> proposed. The technical solution is fine. The goal is wrong. Which makes
> any technical solution (original or alternative) wrong.
>
> If you want, you can go ahead and commit this patch, adding my Nacked-by
> from above. I won't get into a fight with you over this (or anything
> else), but I want my conviction -- namely, that this is entirely wrong
> -- clearly marked.
>

I'd rather have a solution that we can agree on.

> On the technical side:
>
> - I think a dynamic boolean PCD would be superior, if that is possible
> with HII (= directly nvvar-backed) PCDs -- absence of the variable
> should map to FALSE. I'm unsure though if that were as easy to set from
> the UEFI shell as a UINT8. So stick with the current data type if you
> deem that superior (maybe comment on it in the commit message).
>
> - please include  in the C source, to reflect the
> [LibraryClasses] update in the INF.
>

Much appreciated. I will keep this in mind.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 19:01, Marc Zyngier wrote:
> On 29/03/17 17:40, Laszlo Ersek wrote:
>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
 On 03/29/17 18:02, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>> In general, we should not present two separate (and inevitably 
>>> different)
>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu 
>>> to
>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, 
>>> and
>>> vice versa.
>>>
>>> However, this is arguably a regression for those who rely on both
>>> descriptions being available, even if the use cases in question are
>>> uncommon.
>>>
>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>> will result in both descriptions being exposed on the next boot, if
>>> executing in the default 'ACPI-only' mode.
>>>
>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>>>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>
> 
> [snip the policy argumentation, I only care about the technical aspects]
> 
>> On the technical side:
>>
>> - I think a dynamic boolean PCD would be superior, if that is possible
>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>> should map to FALSE. I'm unsure though if that were as easy to set from
>> the UEFI shell as a UINT8. So stick with the current data type if you
>> deem that superior (maybe comment on it in the commit message).
>>
>> - please include  in the C source, to reflect the
>> [LibraryClasses] update in the INF.
> 
> My personal choice would be *not* to expose both tables at the same
> time, but instead to be able to shift the preference from one side or
> the other, similarly to what a menu on a bare metal system would do.

Umm... Are we in violent agreement? This works already.

If you invoke QEMU with the normal command like, like you've always
done, you get ACPI only (right now).

If you pass the "-no-acpi" switch in addition to your normal command
line, you get DT only. This is documented in detail on the following commit:

https://github.com/tianocore/edk2/commit/110316a995ed

If you pass -no-acpi on your QEMU command line, you can perform the
whole guest kernel bisection using purely DT, without ever having to
re-launch QEMU.

> 
> Lets call the variable PreferDT (rather than ForceDT), with the
> following (exhaustive) behaviour :
> 
> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==0 and ACPI present-> ACPI
> - PreferDT==0 and DT present  -> DT
> - PreferDT==1 and ACPI+DT present -> DT
> - PreferDT==1 and ACPI present-> ACPI
> - PreferDT==1 and DT present  -> DT
> 
> It allows people with existing setups to still have something that works
> with minimal effort (still need to set this variable though).
> 
> Could people agree on something like this?

It's overly complex. With QEMU (and the current edk2 master state) you
already have a single (host-side) master knob for controlling the ACPI
vs. DT exposure.

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 18:01, Marc Zyngier  wrote:
> On 29/03/17 17:40, Laszlo Ersek wrote:
>> On 03/29/17 18:07, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
 On 03/29/17 18:02, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>> In general, we should not present two separate (and inevitably 
>>> different)
>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu 
>>> to
>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, 
>>> and
>>> vice versa.
>>>
>>> However, this is arguably a regression for those who rely on both
>>> descriptions being available, even if the use cases in question are
>>> uncommon.
>>>
>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>> will result in both descriptions being exposed on the next boot, if
>>> executing in the default 'ACPI-only' mode.
>>>
>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>>>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>
>
> [snip the policy argumentation, I only care about the technical aspects]
>
>> On the technical side:
>>
>> - I think a dynamic boolean PCD would be superior, if that is possible
>> with HII (= directly nvvar-backed) PCDs -- absence of the variable
>> should map to FALSE. I'm unsure though if that were as easy to set from
>> the UEFI shell as a UINT8. So stick with the current data type if you
>> deem that superior (maybe comment on it in the commit message).
>>
>> - please include  in the C source, to reflect the
>> [LibraryClasses] update in the INF.
>
> My personal choice would be *not* to expose both tables at the same
> time, but instead to be able to shift the preference from one side or
> the other, similarly to what a menu on a bare metal system would do.
>

So to clarify, you want something sticky in the firmware settings
rather than having to use the -no-acpi command line argument to QEMU?

> Lets call the variable PreferDT (rather than ForceDT), with the
> following (exhaustive) behaviour :
>
> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==0 and ACPI present-> ACPI
> - PreferDT==0 and DT present  -> DT
> - PreferDT==1 and ACPI+DT present -> DT
> - PreferDT==1 and ACPI present-> ACPI
> - PreferDT==1 and DT present  -> DT
>

DT is always available, so this condenses to

> - PreferDT==0 and ACPI+DT present -> ACPI
> - PreferDT==1 and ACPI+DT present -> DT

unless -no-acpi is set, which gives us

> - PreferDT==0 and DT present  -> DT
> - PreferDT==1 and DT present  -> DT

> It allows people with existing setups to still have something that works
> with minimal effort (still need to set this variable though).
>

For symmetry, it would make sense to call it ForceNoAcpi then, after
the command line param.

> Could people agree on something like this?
>

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 18:16, Marc Zyngier wrote:
> On 29/03/17 17:03, Laszlo Ersek wrote:
>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
 On 03/29/17 17:19, Ard Biesheuvel wrote:
> In general, we should not present two separate (and inevitably different)
> hardware descriptions to the OS, in the form of ACPI tables and a device
> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
> vice versa.
>
> However, this is arguably a regression for those who rely on both
> descriptions being available, even if the use cases in question are
> uncommon.
>
> So allow a secret handshake with the UEFI Shell, to set a variable that
> will result in both descriptions being exposed on the next boot, if
> executing in the default 'ACPI-only' mode.
>
>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>  4 files changed, 22 insertions(+), 1 deletion(-)

 Nacked-by: Laszlo Ersek 

 This will cause everyone *at all* to set the secret handshake, and we
 will be back to square one, where everyone just exposes ACPI and DT at
 the same time, and delegate the decision to the guest kernel.

 And then vendors will continue to ignore ACPI testing, and they will
 continue shipping crap in their ACPI tables.

 We might as well rip out the recent patches that implement the mechanism
 and the policy for the mutual exclusion.

 As Leif proved so eloquently (in the pub) in Budapest during Connect, no
 OS needs both descriptions at the same time. Virt users can make up
 their minds about what to expose. We (RH virt) had been worriedly
 planning to make the same proposal to Leif, you, et al, and then we were
 happy to see the violent agreement that ensued.

 Sorry for getting political, but the kernel's unwavering preference for
 DT over ACPI is political, and the recent edk2 patches only exist to
 rectify that, from the firmware side. Users don't lose DT. What they do
 lose is the (harmful) freedom of not choosing one over the other. That
 freedom has had a terrible effect on the quality of ACPI tables shipped
 with *allegedly* SBBR-compliant hardware.

 Feel free to diverge from this in downstream distributions, but this
 loophole has no place in upstream edk2.

 NACK

>>>
>>> OK, fair enough. How do you propose to handle this regression then?
>>>
>>
>> I don't.
> 
> Well, we then have an issue here. As a user of QEMU+UEFI, I expect my
> existing VM images to continue to work (mostly because I'm trying to
> track regression in KVM itself). And some are pretty old (circa 4.2). I
> upgrade my "firmware" (which is QEMU+UEFI), and my VM doesn't boot
> anymore, because someone decided that ACPI was better for my soul.

I understand. In my opinion, this can be considered an advanced enough
use case that justifies rebuilding the firmware on your side. The same
way I rebuild both QEMU and (occasionally) the kernel when I try to
track down something related to, or exposed by, the guest firmware.

(
Case in point:

https://www.mail-archive.com/qemu-devel@nongnu.org/msg440255.html

To me it seems like a recent vhost regression in the host kernel, and
I'd already be bisecting the kernel if I had hardware in my home where
the issue reproduces. But, I digress; a team mate is doing that now.
)

> I'm sorry, but I don't think that's the right thing to do. If RH decides
> to mandate ACPI VMs, that's RH's call. You can eviscerate DT support
> from your build of QEMU and UEFI, and I'm fine with it. Imposing this on
> all users with existing setups is just wrong.

What is wrong is the *practical* effect of the freedom that ACPI+DT
together provide. It undermines SBBR conformance in the wild. If people
*can* not care, they *will* not care. And SBBR is not just an RH preference.

(Sorry about the over-use of bold -- I'm not excited, just trying to get
the emphases over the wire.)

> I appreciate you may not care, but I had to say it.

I appreciate that you said it!

Thanks,
Laszlo

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 18:17, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:09, Jon Masters  wrote:
>> Thanks Laszlo. A quick note from me that regardless of this discussion I 
>> will be pushing to ensure the version Red Hat ships makes ACPI the default 
>> with it being extremely painful to use DT. It is time the ecosystem got with 
>> the program we all agreed upon and there will be no more excuses.
>>
> 
> This has *nothing* to do with the ecosystem. This has to do with
> existing users of ArmVirtQemu (admittedly, a small fraction) that will
> be forced to compile their UEFI images from source because we made a
> backwards incompatible change.
> 
> I am 100% on board with making ACPI and DT mutually exclusive. But I
> don't believe for a second that 'everybody just exposes ACPI and DT at
> the same time' if this gets merged.

That's where we disagree, 100%.

> That happens because people are
> clueless, not because they are deliberately spending time and effort
> on producing two hardware descriptions.

If this were true, then the kernel's preference would have been changed
to ACPI aeons ago (assuming both DT and ACPI were present). ACPI is
superior to DT (cue again Grant Likely's blog post), yet kernel people
resist it. That's not cluelessness. If the kernel's DT camp has any
influence on platform vendors (and that's rather more a "given that"
than an "if"), when they find out about this loophole, I expect them to
actively recommend it as a way to perpetuate the status quo.

IMO, with this patch you are eviscerating the work we've been doing in
the last few weeks. Well, politics is nasty.

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 18:07, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
>> On 03/29/17 18:02, Ard Biesheuvel wrote:
>>> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
 On 03/29/17 17:19, Ard Biesheuvel wrote:
> In general, we should not present two separate (and inevitably different)
> hardware descriptions to the OS, in the form of ACPI tables and a device
> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
> vice versa.
>
> However, this is arguably a regression for those who rely on both
> descriptions being available, even if the use cases in question are
> uncommon.
>
> So allow a secret handshake with the UEFI Shell, to set a variable that
> will result in both descriptions being exposed on the next boot, if
> executing in the default 'ACPI-only' mode.
>
>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>  4 files changed, 22 insertions(+), 1 deletion(-)

 Nacked-by: Laszlo Ersek 

 This will cause everyone *at all* to set the secret handshake, and we
 will be back to square one, where everyone just exposes ACPI and DT at
 the same time, and delegate the decision to the guest kernel.

 And then vendors will continue to ignore ACPI testing, and they will
 continue shipping crap in their ACPI tables.

 We might as well rip out the recent patches that implement the mechanism
 and the policy for the mutual exclusion.

 As Leif proved so eloquently (in the pub) in Budapest during Connect, no
 OS needs both descriptions at the same time. Virt users can make up
 their minds about what to expose. We (RH virt) had been worriedly
 planning to make the same proposal to Leif, you, et al, and then we were
 happy to see the violent agreement that ensued.

 Sorry for getting political, but the kernel's unwavering preference for
 DT over ACPI is political, and the recent edk2 patches only exist to
 rectify that, from the firmware side. Users don't lose DT. What they do
 lose is the (harmful) freedom of not choosing one over the other. That
 freedom has had a terrible effect on the quality of ACPI tables shipped
 with *allegedly* SBBR-compliant hardware.

 Feel free to diverge from this in downstream distributions, but this
 loophole has no place in upstream edk2.

 NACK

>>>
>>> OK, fair enough. How do you propose to handle this regression then?
>>>
>>
>> I don't.
> 
> I think I am entitled to a more constructive attitude from you. I
> don't care about politics, but I do care about not breaking people's
> workflows. So if you insist on getting on your high horse and throw
> capitalized NACKs at me, you could at least *pretend* to care about
> other users than *your* downstream, and come up with some alternative.

Ard, I'm not on my high horse. I thought we were in agreement about
this. It was obvious (to me at least) that this policy would be
considered a regression by those who wanted both DT and ACPI in the
guest. From my side, breaking that was all intentional.

I'm not deliberately screwing with users (just because they have "niche"
needs), and normally I would fully agree with you. The problem is that
by providing an automated, upstream (centralized) opt-out from the
mutual exclusion, the ecosystem will be allowed to suffer from the same
nonchalance towards ACPI that has been the case until now. It's clear
that your well-meaning change will be immediately exploited as the
new-old default.

Do you plan to add the same loophole to the HII-enabled driver that you
recently committed as well?

Also, please don't accuse me of caring only about RH's downstream. The
following blog post wasn't authored by a Red Hat employee:

http://www.secretlab.ca/archives/151

The following document is also not Red Hat exclusive:

http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0044b/index.html

What you are arguing for is, indirectly, to relicense platform vendors
to continue ignoring ACPI, while (falsely) labeling their systems SBBR
conformant. And then any OS vendor that actually cares about SBBR
conformance (hint: it's not just Red Hat) sees brutal boot splats. That
is not your intent (obviously), but that is the (seen, experienced)
effect of providing both DT and ACPI.

My Nacked-by is not for

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 17:09, Jon Masters  wrote:
> Thanks Laszlo. A quick note from me that regardless of this discussion I will 
> be pushing to ensure the version Red Hat ships makes ACPI the default with it 
> being extremely painful to use DT. It is time the ecosystem got with the 
> program we all agreed upon and there will be no more excuses.
>

This has *nothing* to do with the ecosystem. This has to do with
existing users of ArmVirtQemu (admittedly, a small fraction) that will
be forced to compile their UEFI images from source because we made a
backwards incompatible change.

I am 100% on board with making ACPI and DT mutually exclusive. But I
don't believe for a second that 'everybody just exposes ACPI and DT at
the same time' if this gets merged. That happens because people are
clueless, not because they are deliberately spending time and effort
on producing two hardware descriptions.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 17:03, Laszlo Ersek  wrote:
> On 03/29/17 18:02, Ard Biesheuvel wrote:
>> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
>>> On 03/29/17 17:19, Ard Biesheuvel wrote:
 In general, we should not present two separate (and inevitably different)
 hardware descriptions to the OS, in the form of ACPI tables and a device
 tree blob. For this reason, we recently added the logic to ArmVirtQemu to
 only expose the ACPI 2.0 entry point if no DT binary is being passed, and
 vice versa.

 However, this is arguably a regression for those who rely on both
 descriptions being available, even if the use cases in question are
 uncommon.

 So allow a secret handshake with the UEFI Shell, to set a variable that
 will result in both descriptions being exposed on the next boot, if
 executing in the default 'ACPI-only' mode.

   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel 
 ---
  ArmVirtPkg/ArmVirtPkg.dec| 8 
  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
  4 files changed, 22 insertions(+), 1 deletion(-)
>>>
>>> Nacked-by: Laszlo Ersek 
>>>
>>> This will cause everyone *at all* to set the secret handshake, and we
>>> will be back to square one, where everyone just exposes ACPI and DT at
>>> the same time, and delegate the decision to the guest kernel.
>>>
>>> And then vendors will continue to ignore ACPI testing, and they will
>>> continue shipping crap in their ACPI tables.
>>>
>>> We might as well rip out the recent patches that implement the mechanism
>>> and the policy for the mutual exclusion.
>>>
>>> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
>>> OS needs both descriptions at the same time. Virt users can make up
>>> their minds about what to expose. We (RH virt) had been worriedly
>>> planning to make the same proposal to Leif, you, et al, and then we were
>>> happy to see the violent agreement that ensued.
>>>
>>> Sorry for getting political, but the kernel's unwavering preference for
>>> DT over ACPI is political, and the recent edk2 patches only exist to
>>> rectify that, from the firmware side. Users don't lose DT. What they do
>>> lose is the (harmful) freedom of not choosing one over the other. That
>>> freedom has had a terrible effect on the quality of ACPI tables shipped
>>> with *allegedly* SBBR-compliant hardware.
>>>
>>> Feel free to diverge from this in downstream distributions, but this
>>> loophole has no place in upstream edk2.
>>>
>>> NACK
>>>
>>
>> OK, fair enough. How do you propose to handle this regression then?
>>
>
> I don't.

I think I am entitled to a more constructive attitude from you. I
don't care about politics, but I do care about not breaking people's
workflows. So if you insist on getting on your high horse and throw
capitalized NACKs at me, you could at least *pretend* to care about
other users than *your* downstream, and come up with some alternative.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH v3] MdePkg: BaseIoLibIntrinsic (IoLib class) library

2017-03-29 Thread Leo Duran

This patch adds an SEV-specific .INF and corresponding assembly
files, to unroll REP INSx/OUTSx on IoRead/WriteFifo#() routines
when the SEV feature is enabled under a hypervisor environment.

The new .INF only supports the IA32 and X64 architectures.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh 
Signed-off-by: Leo Duran 
---
 .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf   |  63 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm  | 297 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm   | 282 +++
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 282 +++
 MdePkg/MdePkg.dsc  |   2 +
 6 files changed, 1219 insertions(+)
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf 
b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
new file mode 100644
index 000..6f14075
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
@@ -0,0 +1,63 @@
+## @file
+#  Instance of I/O Library using compiler intrinsics.
+#
+#  I/O Library that uses compiler intrinsics to perform IN and OUT instructions
+#  for IA-32 and x64.  On IPF, I/O port requests are translated into MMIO 
requests.
+#  MMIO requests are forwarded directly to memory.  For EBC, I/O port requests
+#  ASSERT().
+#
+#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
+#  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution. The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php.
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = BaseIoLibIntrinsicSev
+  MODULE_UNI_FILE= BaseIoLibIntrinsic.uni
+  FILE_GUID  = 93742f95-6e71-4581-b600-8e1da443f95a
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = IoLib 
+
+
+#
+#  VALID_ARCHITECTURES   = IA32 X64
+#
+
+[Sources]
+  IoLibMmioBuffer.c
+  BaseIoLibIntrinsicInternal.h
+  IoHighLevel.c
+
+[Sources.IA32]
+  IoLibGcc.c| GCC
+  IoLibMsc.c| MSFT
+  IoLibIcc.c| INTEL
+  IoLib.c
+  Ia32/IoFifoSev.nasm
+  Ia32/IoFifoSev.asm
+
+[Sources.X64]
+  IoLibGcc.c| GCC
+  IoLibMsc.c| MSFT
+  IoLibIcc.c| INTEL
+  IoLib.c
+  X64/IoFifoSev.nasm
+  X64/IoFifoSev.asm
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  BaseLib
+
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm 
b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
new file mode 100644
index 000..d81871c
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
@@ -0,0 +1,297 @@
+;--
+;
+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. 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.
+;
+;--
+
+.586P
+.model  flat,C
+.code
+
+;--
+; Check whether we need to unroll the String I/O under SEV guest
+;
+; Return // eax   (1 - unroll, 0 - no unroll)
+;--
+SevNoRepIo PROC
+
+  ; CPUID clobbers ebx, ecx and edx
+  push  ebx
+  push  ecx
+  push  edx
+
+  ; Check if we are running under hypervisor
+  ; CPUID(1).ECX Bit 31
+  mov   eax, 1
+

[edk2] [PATCH v3] MdePkg: BaseIoLibIntrinsic (IoLib class) library

2017-03-29 Thread Leo Duran

This patch adds an SEV-specific .INF and corresponding assembly
files, to unroll REP INSx/OUTSx on IoRead/WriteFifo#() routines
when the SEV feature is enabled under a hypervisor environment.

The new .INF only supports the IA32 and X64 architectures.

This patch follows the series "[PATCH v3 00/10] IoLib class library",
which has already being pushed upstream.

Changes since v2:
- Add .INF entry into MdePkg.dsc

Leo Duran (1):
  MdePkg: BaseIoLibIntrinsic (IoLib class) library

 .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf   |  63 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm  | 297 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm   | 282 +++
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 282 +++
 MdePkg/MdePkg.dsc  |   2 +
 6 files changed, 1219 insertions(+)
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm

-- 
2.7.4

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 18:02, Ard Biesheuvel wrote:
> On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
>> On 03/29/17 17:19, Ard Biesheuvel wrote:
>>> In general, we should not present two separate (and inevitably different)
>>> hardware descriptions to the OS, in the form of ACPI tables and a device
>>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>>> vice versa.
>>>
>>> However, this is arguably a regression for those who rely on both
>>> descriptions being available, even if the use cases in question are
>>> uncommon.
>>>
>>> So allow a secret handshake with the UEFI Shell, to set a variable that
>>> will result in both descriptions being exposed on the next boot, if
>>> executing in the default 'ACPI-only' mode.
>>>
>>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>>>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>>>  4 files changed, 22 insertions(+), 1 deletion(-)
>>
>> Nacked-by: Laszlo Ersek 
>>
>> This will cause everyone *at all* to set the secret handshake, and we
>> will be back to square one, where everyone just exposes ACPI and DT at
>> the same time, and delegate the decision to the guest kernel.
>>
>> And then vendors will continue to ignore ACPI testing, and they will
>> continue shipping crap in their ACPI tables.
>>
>> We might as well rip out the recent patches that implement the mechanism
>> and the policy for the mutual exclusion.
>>
>> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
>> OS needs both descriptions at the same time. Virt users can make up
>> their minds about what to expose. We (RH virt) had been worriedly
>> planning to make the same proposal to Leif, you, et al, and then we were
>> happy to see the violent agreement that ensued.
>>
>> Sorry for getting political, but the kernel's unwavering preference for
>> DT over ACPI is political, and the recent edk2 patches only exist to
>> rectify that, from the firmware side. Users don't lose DT. What they do
>> lose is the (harmful) freedom of not choosing one over the other. That
>> freedom has had a terrible effect on the quality of ACPI tables shipped
>> with *allegedly* SBBR-compliant hardware.
>>
>> Feel free to diverge from this in downstream distributions, but this
>> loophole has no place in upstream edk2.
>>
>> NACK
>>
> 
> OK, fair enough. How do you propose to handle this regression then?
> 

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

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 17:00, Laszlo Ersek  wrote:
> On 03/29/17 17:19, Ard Biesheuvel wrote:
>> In general, we should not present two separate (and inevitably different)
>> hardware descriptions to the OS, in the form of ACPI tables and a device
>> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
>> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
>> vice versa.
>>
>> However, this is arguably a regression for those who rely on both
>> descriptions being available, even if the use cases in question are
>> uncommon.
>>
>> So allow a secret handshake with the UEFI Shell, to set a variable that
>> will result in both descriptions being exposed on the next boot, if
>> executing in the default 'ACPI-only' mode.
>>
>>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>>  4 files changed, 22 insertions(+), 1 deletion(-)
>
> Nacked-by: Laszlo Ersek 
>
> This will cause everyone *at all* to set the secret handshake, and we
> will be back to square one, where everyone just exposes ACPI and DT at
> the same time, and delegate the decision to the guest kernel.
>
> And then vendors will continue to ignore ACPI testing, and they will
> continue shipping crap in their ACPI tables.
>
> We might as well rip out the recent patches that implement the mechanism
> and the policy for the mutual exclusion.
>
> As Leif proved so eloquently (in the pub) in Budapest during Connect, no
> OS needs both descriptions at the same time. Virt users can make up
> their minds about what to expose. We (RH virt) had been worriedly
> planning to make the same proposal to Leif, you, et al, and then we were
> happy to see the violent agreement that ensued.
>
> Sorry for getting political, but the kernel's unwavering preference for
> DT over ACPI is political, and the recent edk2 patches only exist to
> rectify that, from the firmware side. Users don't lose DT. What they do
> lose is the (harmful) freedom of not choosing one over the other. That
> freedom has had a terrible effect on the quality of ACPI tables shipped
> with *allegedly* SBBR-compliant hardware.
>
> Feel free to diverge from this in downstream distributions, but this
> loophole has no place in upstream edk2.
>
> NACK
>

OK, fair enough. How do you propose to handle this regression then?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Laszlo Ersek

On 03/29/17 17:19, Ard Biesheuvel wrote:
> In general, we should not present two separate (and inevitably different)
> hardware descriptions to the OS, in the form of ACPI tables and a device
> tree blob. For this reason, we recently added the logic to ArmVirtQemu to
> only expose the ACPI 2.0 entry point if no DT binary is being passed, and
> vice versa.
> 
> However, this is arguably a regression for those who rely on both
> descriptions being available, even if the use cases in question are
> uncommon.
> 
> So allow a secret handshake with the UEFI Shell, to set a variable that
> will result in both descriptions being exposed on the next boot, if
> executing in the default 'ACPI-only' mode.
> 
>   setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtPkg.dec| 8 
>  ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
>  4 files changed, 22 insertions(+), 1 deletion(-)

Nacked-by: Laszlo Ersek 

This will cause everyone *at all* to set the secret handshake, and we
will be back to square one, where everyone just exposes ACPI and DT at
the same time, and delegate the decision to the guest kernel.

And then vendors will continue to ignore ACPI testing, and they will
continue shipping crap in their ACPI tables.

We might as well rip out the recent patches that implement the mechanism
and the policy for the mutual exclusion.

As Leif proved so eloquently (in the pub) in Budapest during Connect, no
OS needs both descriptions at the same time. Virt users can make up
their minds about what to expose. We (RH virt) had been worriedly
planning to make the same proposal to Leif, you, et al, and then we were
happy to see the violent agreement that ensued.

Sorry for getting political, but the kernel's unwavering preference for
DT over ACPI is political, and the recent edk2 patches only exist to
rectify that, from the firmware side. Users don't lose DT. What they do
lose is the (harmful) freedom of not choosing one over the other. That
freedom has had a terrible effect on the quality of ACPI tables shipped
with *allegedly* SBBR-compliant hardware.

Feel free to diverge from this in downstream distributions, but this
loophole has no place in upstream edk2.

NACK

Thanks
Laszlo

> 
> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index efe83a383d55..ae5473a3f3f3 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -34,6 +34,8 @@ [Guids.common]
>gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 
> 0x36, 0x5B, 0x80, 0x63, 0x66 } }
>gEarlyPL011BaseAddressGuid   = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 
> 0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
>  
> +  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 
> 0x59, 0x65, 0x16, 0xb0, 0x0a } }
> +
>  [Protocols]
>gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 
> 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>  
> @@ -58,3 +60,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
># EFI_VT_100_GUID.
>#
>gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
> 0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
> 0x4D}|VOID*|0x0007
> +
> +[PcdsDynamic]
> +  #
> +  # Whether to force the DT to be exposed to the OS, even in the presence of
> +  # ACPI tables.
> +  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|0x0|UINT8|0x0003
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 4075b92aa2cb..bbb517e40242 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
>gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>  
> +[PcdsDynamicHii]
> +  
> gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|L"ForceDt"|gArmVirtVariableGuid|0x0|0|NV,BS
> +
>  
> 
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c 
> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> index 8932dacabec5..4c53825d54ad 100644
> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
> @@ -58,7 +58,12 @@ PlatformHasAcpiDt (
>goto Failed;
>  }
>  
> -return Status;
> +if (!PcdGet8 (PcdAcpiDtForceDt)) {
> +  return EFI_SUCCESS;
> +}
> +DEBUG ((DEBUG_WARN,
> +  "%a: ForceDt is set: installing both ACPI and DT tables\n",
> +  __FUNCTION__));
>

[edk2] [PATCH v2] MdePkg: BaseIoLibIntrinsic (IoLib class) library

2017-03-29 Thread Leo Duran

This patch adds an SEV-specific .INF and corresponding assembly
files, to unroll REP INSx/OUTSx on IoRead/WriteFifo#() routines
when the SEV feature is enabled under a hypervisor environment.

The new .INF only supports the IA32 and X64 architectures.

This patch follows the series "[PATCH v3 00/10] IoLib class library",
which has already being pushed upstream.

Leo Duran (1):
  MdePkg: BaseIoLibIntrinsic (IoLib class) library

 .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf   |  63 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm  | 297 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm   | 282 +++
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 282 +++
 5 files changed, 1217 insertions(+)
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm

-- 
2.7.4

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

[edk2] [PATCH v2] MdePkg: BaseIoLibIntrinsic (IoLib class) library

2017-03-29 Thread Leo Duran

This patch adds an SEV-specific .INF and corresponding assembly
files, to unroll REP INSx/OUTSx on IoRead/WriteFifo#() routines
when the SEV feature is enabled under a hypervisor environment.

The new .INF only supports the IA32 and X64 architectures.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Brijesh Singh 
Signed-off-by: Leo Duran  
---
 .../BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf   |  63 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm  | 297 +
 .../Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm | 293 
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm   | 282 +++
 .../Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm  | 282 +++
 5 files changed, 1217 insertions(+)
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.nasm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.asm
 create mode 100644 MdePkg/Library/BaseIoLibIntrinsic/X64/IoFifoSev.nasm

diff --git a/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf 
b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
new file mode 100644
index 000..6f14075
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsicSev.inf
@@ -0,0 +1,63 @@
+## @file
+#  Instance of I/O Library using compiler intrinsics.
+#
+#  I/O Library that uses compiler intrinsics to perform IN and OUT instructions
+#  for IA-32 and x64.  On IPF, I/O port requests are translated into MMIO 
requests.
+#  MMIO requests are forwarded directly to memory.  For EBC, I/O port requests
+#  ASSERT().
+#
+#  Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
+#  Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
+#  Copyright (c) 2017, AMD Incorporated. All rights reserved.
+#
+#  This program and the accompanying materials
+#  are licensed and made available under the terms and conditions of the BSD 
License
+#  which accompanies this distribution. The full text of the license may be 
found at
+#  http://opensource.org/licenses/bsd-license.php.
+#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION= 0x00010005
+  BASE_NAME  = BaseIoLibIntrinsicSev
+  MODULE_UNI_FILE= BaseIoLibIntrinsic.uni
+  FILE_GUID  = 93742f95-6e71-4581-b600-8e1da443f95a
+  MODULE_TYPE= BASE
+  VERSION_STRING = 1.0
+  LIBRARY_CLASS  = IoLib 
+
+
+#
+#  VALID_ARCHITECTURES   = IA32 X64
+#
+
+[Sources]
+  IoLibMmioBuffer.c
+  BaseIoLibIntrinsicInternal.h
+  IoHighLevel.c
+
+[Sources.IA32]
+  IoLibGcc.c| GCC
+  IoLibMsc.c| MSFT
+  IoLibIcc.c| INTEL
+  IoLib.c
+  Ia32/IoFifoSev.nasm
+  Ia32/IoFifoSev.asm
+
+[Sources.X64]
+  IoLibGcc.c| GCC
+  IoLibMsc.c| MSFT
+  IoLibIcc.c| INTEL
+  IoLib.c
+  X64/IoFifoSev.nasm
+  X64/IoFifoSev.asm
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  DebugLib
+  BaseLib
+
diff --git a/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm 
b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
new file mode 100644
index 000..d81871c
--- /dev/null
+++ b/MdePkg/Library/BaseIoLibIntrinsic/Ia32/IoFifoSev.asm
@@ -0,0 +1,297 @@
+;--
+;
+; Copyright (c) 2006 - 2012, Intel Corporation. All rights reserved.
+; Copyright (c) 2017, AMD Incorporated. 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.
+;
+;--
+
+.586P
+.model  flat,C
+.code
+
+;--
+; Check whether we need to unroll the String I/O under SEV guest
+;
+; Return // eax   (1 - unroll, 0 - no unroll)
+;--
+SevNoRepIo PROC
+
+  ; CPUID clobbers ebx, ecx and edx
+  push  ebx
+  push  ecx
+  push  edx
+
+  ; Check if we are running under hypervisor
+  ; CPUID(1).ECX Bit 31
+  mov   eax, 1
+  cpuid
+  btecx, 31
+  jnc   @UseRepIo
+
+  ;

Re: [edk2] [PATCH] kvm: pass the virtual SEI syndrome to guest OS

2017-03-29 Thread Laszlo Ersek

On 03/29/17 16:48, Christoffer Dall wrote:
> On Wed, Mar 29, 2017 at 10:36:51PM +0800, gengdongjiu wrote:
>> 2017-03-29 18:36 GMT+08:00, Achin Gupta :

>>> Qemu is essentially fulfilling the role of secure firmware at the
>>> EL2/EL1 interface (as discussed with Christoffer below). So it
>>> should generate the CPER before injecting the error.
>>>
>>> This is corresponds to (1) above apart from notifying UEFI (I am
>>> assuming you mean guest UEFI). At this time, the guest OS already
>>> knows where to pick up the CPER from through the HEST. Qemu has
>>> to create the CPER and populate its address at the address
>>> exported in the HEST. Guest UEFI should not be involved in this 
>>> flow. Its job was to create the HEST at boot and that has been
>>> done by this stage.
>>
>> Sorry,  As I understand it, after Qemu generate the CPER table, it
>> should pass the CPER table to the guest UEFI, then Guest UEFI  place
>> this CPER table to the guest OS memory. In this flow, the Guest UEFI
>> should be involved, else the Guest OS can not see the CPER table.
>>
> 
> I think you need to explain the "pass the CPER table to the guest UEFI"
> concept in terms of what really happens, step by step, and when you say
> "then Guest UEFI place the CPER table to the guest OS memory", I'm
> curious who is running what code on the hardware when doing that.

I strongly suggest to keep the guest firmware's runtime involvement to
zero. Two reasons:

(1) As you explained above (... which I conveniently snipped), when you
inject an interrupt to the guest, the handler registered for that
interrupt will come from the guest kernel.

The only exception to this is when the platform provides a type of
interrupt whose handler can be registered and then locked down by the
firmware. On x86, this is the SMI.

In practice though,
- in OVMF (x86), we only do synchronous (software-initiated) SMIs (for
privileged UEFI varstore access),
- and in ArmVirtQemu (ARM / aarch64), none of the management mode stuff
exists at all.

I understand that the Platform Init 1.5 (or 1.6?) spec abstracted away
the MM (management mode) protocols from Intel SMM, but at this point
there is zero code in ArmVirtQemu for that. (And I'm unsure how much of
any eligible underlying hw emulation exists in QEMU.)

So you can't get the guest firmware to react to the injected interrupt
without the guest OS coming between first.

(2) Achin's description matches really-really closely what is possible,
and what should be done with QEMU, ArmVirtQemu, and the guest kernel.

In any solution for this feature, the firmware has to reserve some
memory from the OS at boot. The current facilities we have enable this.
As I described previously, the ACPI linker/loader actions can be mapped
more or less 1:1 to Achin's design. From a practical perspective, you
really want to keep the guest firmware as dumb as possible (meaning: as
generic as possible), and keep the ACPI specifics to the QEMU and the
guest kernel sides.

The error serialization actions -- the co-operation between guest kernel
and QEMU on the special memory areas -- that were mentioned earlier by
Michael and Punit look like a complication. But, IMO, they don't differ
from any other device emulation -- DMA actions in particular -- that
QEMU already does. Device models are what QEMU *does*. Read the command
block that the guest driver placed in guest memory, parse it, sanity
check it, verify it, execute it, write back the status code, inject an
interrupt (and/or let any polling guest driver notice it "soon after" --
use barriers as necessary).

Thus, I suggest to rely on the generic ACPI linker/loader interface
(between QEMU and guest firmware) *only* to make the firmware lay out
stuff (= reserve buffers, set up pointers, install QEMU's ACPI tables)
*at boot*. Then, at runtime, let the guest kernel and QEMU (the "device
model") talk to each other directly. Keep runtime firmware involvement
to zero.

You *really* don't want to debug three components at runtime, when you
can solve the thing with two. (Two components whose build systems won't
drive you mad, I should add.)

IMO, Achin's design nailed it. We can do that.

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

[edk2] [PATCH] ArmVirtPkg/PlatformHasAcpiDtDxe: allow manual override for DT installation

2017-03-29 Thread Ard Biesheuvel

In general, we should not present two separate (and inevitably different)
hardware descriptions to the OS, in the form of ACPI tables and a device
tree blob. For this reason, we recently added the logic to ArmVirtQemu to
only expose the ACPI 2.0 entry point if no DT binary is being passed, and
vice versa.

However, this is arguably a regression for those who rely on both
descriptions being available, even if the use cases in question are
uncommon.

So allow a secret handshake with the UEFI Shell, to set a variable that
will result in both descriptions being exposed on the next boot, if
executing in the default 'ACPI-only' mode.

  setvar -nv -bs -guid 50bea1e5-a2c5-46e9-9b3a-59596516b00a ForceDt =01

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtPkg.dec| 8 
 ArmVirtPkg/ArmVirtQemu.dsc   | 3 +++
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c   | 7 ++-
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 5 +
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index efe83a383d55..ae5473a3f3f3 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -34,6 +34,8 @@ [Guids.common]
   gArmVirtTokenSpaceGuid = { 0x0B6F5CA7, 0x4F53, 0x445A, { 0xB7, 0x6E, 0x2E, 
0x36, 0x5B, 0x80, 0x63, 0x66 } }
   gEarlyPL011BaseAddressGuid   = { 0xB199DEA9, 0xFD5C, 0x4A84, { 0x80, 
0x82, 0x2F, 0x41, 0x70, 0x78, 0x03, 0x05 } }
 
+  gArmVirtVariableGuid   = { 0x50bea1e5, 0xa2c5, 0x46e9, { 0x9b, 0x3a, 0x59, 
0x59, 0x65, 0x16, 0xb0, 0x0a } }
+
 [Protocols]
   gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 
0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
 
@@ -58,3 +60,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # EFI_VT_100_GUID.
   #
   gArmVirtTokenSpaceGuid.PcdTerminalTypeGuidBuffer|{0x65, 0x60, 0xA6, 0xDF, 
0x19, 0xB4, 0xD3, 0x11, 0x9A, 0x2D, 0x00, 0x90, 0x27, 0x3F, 0xC1, 
0x4D}|VOID*|0x0007
+
+[PcdsDynamic]
+  #
+  # Whether to force the DT to be exposed to the OS, even in the presence of
+  # ACPI tables.
+  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|0x0|UINT8|0x0003
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 4075b92aa2cb..bbb517e40242 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -210,6 +210,9 @@ [PcdsDynamicDefault.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
+[PcdsDynamicHii]
+  
gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt|L"ForceDt"|gArmVirtVariableGuid|0x0|0|NV,BS
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c 
b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
index 8932dacabec5..4c53825d54ad 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.c
@@ -58,7 +58,12 @@ PlatformHasAcpiDt (
   goto Failed;
 }
 
-return Status;
+if (!PcdGet8 (PcdAcpiDtForceDt)) {
+  return EFI_SUCCESS;
+}
+DEBUG ((DEBUG_WARN,
+  "%a: ForceDt is set: installing both ACPI and DT tables\n",
+  __FUNCTION__));
   }
 
   //
diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf 
b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
index 4466bead57c2..5bc9ea43c05b 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
@@ -25,6 +25,7 @@ [Sources]
   PlatformHasAcpiDtDxe.c
 
 [Packages]
+  ArmVirtPkg/ArmVirtPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   OvmfPkg/OvmfPkg.dec
@@ -32,6 +33,7 @@ [Packages]
 [LibraryClasses]
   BaseLib
   DebugLib
+  PcdLib
   QemuFwCfgLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
@@ -40,5 +42,8 @@ [Guids]
   gEdkiiPlatformHasAcpiGuid   ## SOMETIMES_PRODUCES ## PROTOCOL
   gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
 
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdAcpiDtForceDt
+
 [Depex]
   TRUE
-- 
2.9.3

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

Re: [edk2] [PATCH 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.

2017-03-29 Thread Ard Biesheuvel

On 29 March 2017 at 00:45, Yao, Jiewen  wrote:
> Agree. That is a good idea.
>
>
>
> I will add that in V2 patch.
>

Hello Jiewen,

As a bit of background, what I have in mind is an enhancement of the
PCI root bridge I/O allocate, map and unmap methods so that situations
that would currently lead to failure or to suboptimal performance are
left for the IOMMU protocol to handle if one is present. Note that
this may imply having IOMMU protocol instances for each PCI root
bridge, and for other masters as well. (For example, AMD Seattle has
separate IOMMUs for PCI and for the networking controllers, which are
wired to the internal interconnect directly)

So in RootBridgeIoMap(), for instance, we have this condition

  PhysicalAddress = (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress;
  if ((!RootBridge->DmaAbove4G ||
   (Operation != EfiPciOperationBusMasterRead64 &&
Operation != EfiPciOperationBusMasterWrite64 &&
Operation != EfiPciOperationBusMasterCommonBuffer64)) &&
  ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) {

to decide whether bounce buffering is necessary (or even possible).
The mapping between DeviceAddress and HostAddress could be supplied by
the IOMMU protocol instance, which also means we should reinterpret
DmaAbove4G and other variables related to 32-bit addressing to apply
to the device address and not the physical address.

Similarly, in RootBridgeIoAllocateBuffer(), a failure to allocate
below 4 GB may not be an error if the IOMMU protocol instance can
provide a 32-bit addressable mapping for it.

I am aware that this complicates matters for you, but having IOMMU
support in the generic PCI host bridge driver is extremely useful for
us. I am happy to help out in any way I can.

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

[edk2] [PATCH 3/6] ArmPlatformPkg/ArmShellCmdRunAxf: remove BdsLib dependency

2017-03-29 Thread Ard Biesheuvel

Remove ArmShellCmdRunAxf's dependency on the deprecated BdsLib by
cloning the ShutdownUefiBootServices() routine into a local source
file; this is the only BdsLib feature 'runaxf' depends on.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf |  1 -
 ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c  | 58 
+++-
 2 files changed, 57 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf 
b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
index 9a34f12a..7d15f6934608 100644
--- a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
+++ b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf
@@ -43,7 +43,6 @@ [Packages]
 [LibraryClasses]
   ArmLib
   BaseLib
-  BdsLib
   DebugLib
   HiiLib
   ShellLib
diff --git a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c 
b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c
index 2abfb6cc1053..9f4fc780307d 100644
--- a/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c
+++ b/ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c
@@ -21,7 +21,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -35,6 +34,63 @@
 typedef VOID (*ELF_ENTRYPOINT)(UINTN arg0, UINTN arg1,
UINTN arg2, UINTN arg3);
 
+STATIC
+EFI_STATUS
+ShutdownUefiBootServices (
+  VOID
+  )
+{
+  EFI_STATUS  Status;
+  UINTN   MemoryMapSize;
+  EFI_MEMORY_DESCRIPTOR   *MemoryMap;
+  UINTN   MapKey;
+  UINTN   DescriptorSize;
+  UINT32  DescriptorVersion;
+  UINTN   Pages;
+
+  MemoryMap = NULL;
+  MemoryMapSize = 0;
+  Pages = 0;
+
+  do {
+Status = gBS->GetMemoryMap (
+,
+MemoryMap,
+,
+,
+
+);
+if (Status == EFI_BUFFER_TOO_SMALL) {
+
+  Pages = EFI_SIZE_TO_PAGES (MemoryMapSize) + 1;
+  MemoryMap = AllocatePages (Pages);
+
+  //
+  // Get System MemoryMap
+  //
+  Status = gBS->GetMemoryMap (
+  ,
+  MemoryMap,
+  ,
+  ,
+  
+  );
+}
+
+// Don't do anything between the GetMemoryMap() and ExitBootServices()
+if (!EFI_ERROR(Status)) {
+  Status = gBS->ExitBootServices (gImageHandle, MapKey);
+  if (EFI_ERROR(Status)) {
+FreePages (MemoryMap, Pages);
+MemoryMap = NULL;
+MemoryMapSize = 0;
+  }
+}
+  } while (EFI_ERROR(Status));
+
+  return Status;
+}
+
 
 STATIC
 EFI_STATUS
-- 
2.9.3

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

[edk2] [PATCH 6/6] ArmPlatformPkg/ArmVExpressDxe: simply FDT handling in ArmFvpDxe

2017-03-29 Thread Ard Biesheuvel

Replace the elaborate but awkward handling of FDT images using device
paths and string PCDs initialized to 128 spaces with a simple scheme
involving a set of builtin DTBs and a bit of runtime logic to select
between them.

This is sufficient for ordinary use, which makes it more suitable as
reference code. Note that overriding the DTB presented to the OS can
easily be done with a UEFI application that simply installs a new DTB
image under the existing FDT configuration table GUID.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/AArch64/ArmFvpDxeAArch64.c |  60 
+++--
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.c| 134 
++--
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf  |  33 
++---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmVExpressCommon.c|  48 
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmVExpressInternal.h  |  52 
+---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec|  18 
---
 6 files changed, 35 insertions(+), 310 deletions(-)

diff --git 
a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/AArch64/ArmFvpDxeAArch64.c 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/AArch64/ArmFvpDxeAArch64.c
index c368957dcd3d..fe8e115c97eb 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/AArch64/ArmFvpDxeAArch64.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/AArch64/ArmFvpDxeAArch64.c
@@ -1,6 +1,7 @@
 /** @file
 
   Copyright (c) 2014-2015, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Linaro Ltd. All rights reserved.
 
   This program and the accompanying materials are licensed and made available
   under the terms and conditions of the BSD License which accompanies this
@@ -15,30 +16,11 @@
 #include "ArmVExpressInternal.h"
 #include 
 
-//
-// Description of the AARCH64 model platforms :
-// Platform ids are defined in ArmVExpressInternal.h for
-// all "ArmVExpress-like" platforms (AARCH64 or ARM architecture,
-// model or hardware platforms).
-//
-CONST ARM_VEXPRESS_PLATFORM ArmVExpressPlatforms[] = {
-  { ARM_FVP_VEXPRESS_AEMv8x4,  FixedPcdGetPtr 
(PcdFdtFvpVExpressAEMv8x4),L"rtsm_ve-aemv8a.dtb"  },
-  { ARM_FVP_BASE_AEMv8x4_AEMv8x4_GICV2,FixedPcdGetPtr 
(PcdFdtFvpBaseAEMv8x4GicV2),   L"fvp-base-gicv2-psci.dtb" },
-  { ARM_FVP_BASE_AEMv8x4_AEMv8x4_GICV2_LEGACY, FixedPcdGetPtr 
(PcdFdtFvpBaseAEMv8x4GicV2Legacy), L"fvp-base-gicv2legacy-psci.dtb"   },
-  { ARM_FVP_BASE_AEMv8x4_AEMv8x4_GICV3,FixedPcdGetPtr 
(PcdFdtFvpBaseAEMv8x4GicV3),   L"fvp-base-gicv3-psci.dtb" },
-  { ARM_FVP_FOUNDATION_GICV2,  FixedPcdGetPtr 
(PcdFdtFvpFoundationGicV2),L"fvp-foundation-gicv2-psci.dtb"   },
-  { ARM_FVP_FOUNDATION_GICV2_LEGACY,   FixedPcdGetPtr 
(PcdFdtFvpFoundationGicV2Legacy),  L"fvp-foundation-gicv2legacy-psci.dtb" },
-  { ARM_FVP_FOUNDATION_GICV3,  FixedPcdGetPtr 
(PcdFdtFvpFoundationGicV3),L"fvp-foundation-gicv3-psci.dtb"   },
-  { ARM_FVP_VEXPRESS_UNKNOWN }
-};
-
 /**
+
   Get information about the VExpress platform the firmware is running on.
 
-  @param[out]  Platform   Address where the pointer to the platform information
-  (type ARM_VEXPRESS_PLATFORM*) should be stored.
-  The returned pointer does not point to an allocated
-  memory area.
+  @param[out]  PlatformId ARM_VEXPRESS_PLATFORM_ID value of current platform
 
   @retval  EFI_SUCCESSThe platform information was returned.
   @retval  EFI_NOT_FOUND  The platform was not recognised.
@@ -46,19 +28,14 @@ CONST ARM_VEXPRESS_PLATFORM ArmVExpressPlatforms[] = {
 **/
 EFI_STATUS
 ArmVExpressGetPlatform (
-  OUT CONST ARM_VEXPRESS_PLATFORM** Platform
+  OUT   ARM_VEXPRESS_PLATFORM_ID*PlatformId
   )
 {
-  EFI_STATUSStatus;
   UINT32SysId;
   UINT32FvpSysId;
   UINT32VariantSysId;
   ARM_GIC_ARCH_REVISION GicRevision;
 
-  ASSERT (Platform != NULL);
-
-  Status = EFI_NOT_FOUND;
-
   SysId = MmioRead32 (ARM_VE_SYS_ID_REG);
   if (SysId != ARM_RTSM_SYS_ID) {
 // Remove the GIC variant to identify if we are running on the FVP Base or
@@ -70,44 +47,41 @@ ArmVExpressGetPlatform (
 
 if (FvpSysId == ARM_FVP_BASE_BOARD_SYS_ID) {
   if (VariantSysId == ARM_FVP_GIC_VE_MMAP) {
-// FVP Base Model with legacy GIC memory map
-Status = ArmVExpressGetPlatformFromId 
(ARM_FVP_BASE_AEMv8x4_AEMv8x4_GICV2_LEGACY, Platform);
+// FVP Base Model with legacy GIC memory map -- no longer supported
+goto NotFound;
   } else {
 GicRevision = ArmGicGetSupportedArchRevision ();
 
 if (GicRevision == ARM_GIC_ARCH_REVISION_2) {
   // FVP Base Model with GICv2 support
-

[edk2] [PATCH 4/6] ArmPlatformPkg/ArmVExpressDxe: remove ARM support

2017-03-29 Thread Ard Biesheuvel

The 32-bit ARM support in this driver is unused, and thus untested.
So let's just remove it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/Arm/ArmFvpDxeArm.c | 84 

 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf  |  9 ---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec| 10 ---
 3 files changed, 103 deletions(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/Arm/ArmFvpDxeArm.c 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/Arm/ArmFvpDxeArm.c
deleted file mode 100644
index 2057c6e2156a..
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/Arm/ArmFvpDxeArm.c
+++ /dev/null
@@ -1,84 +0,0 @@
-/** @file
-
-  Copyright (c) 2014, ARM Ltd. All rights reserved.
-
-  This program and the accompanying materials are licensed and made available
-  under the terms and conditions of the BSD License which accompanies this
-  distribution.  The full text of the license may be found at
-  http://opensource.org/licenses/bsd-license.php
-
-  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
-  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include "ArmVExpressInternal.h"
-#include   // To get Core Count
-
-//
-// Description of the four ARM model platforms :
-// Platform ids are defined in ArmVExpressInternal.h for
-// all "ArmVExpress-like" platforms (AARCH64 or ARM architecture,
-// model or hardware platforms).
-//
-CONST ARM_VEXPRESS_PLATFORM ArmVExpressPlatforms[] = {
-  { ARM_FVP_VEXPRESS_A9x4,  FixedPcdGetPtr (PcdFdtVExpressFvpA9x4),  
L"rtsm_ve-cortex_a9x4.dtb"  },
-  { ARM_FVP_VEXPRESS_A15x1, FixedPcdGetPtr (PcdFdtVExpressFvpA15x1), 
L"rtsm_ve-cortex_a15x1.dtb" },
-  { ARM_FVP_VEXPRESS_A15x2, FixedPcdGetPtr (PcdFdtVExpressFvpA15x2), 
L"rtsm_ve-cortex_a15x2.dtb" },
-  { ARM_FVP_VEXPRESS_A15x4, FixedPcdGetPtr (PcdFdtVExpressFvpA15x4), 
L"rtsm_ve-cortex_a15x4.dtb" },
-  { ARM_FVP_VEXPRESS_UNKNOWN, }
-};
-
-/**
-  Get information about the VExpress platform the firmware is running on.
-
-  @param[out]  Platform   Address where the pointer to the platform information
-  (type ARM_VEXPRESS_PLATFORM*) should be stored.
-  The returned pointer does not point to an allocated
-  memory area.
-
-  @retval  EFI_SUCCESSThe platform information was returned.
-  @retval  EFI_NOT_FOUND  The platform was not recognised.
-
-**/
-EFI_STATUS
-ArmVExpressGetPlatform (
-  OUT CONST ARM_VEXPRESS_PLATFORM** Platform
-  )
-{
-  UINT32SysId;
-  UINTN CpuType;
-  EFI_STATUSStatus;
-  UINTN CoreCount;
-
-  ASSERT (Platform != NULL);
-
-  CpuType   = 0;
-  Status= EFI_NOT_FOUND;
-  *Platform = NULL;
-
-  SysId = MmioRead32 (ARM_VE_SYS_ID_REG);
-  if (SysId == ARM_RTSM_SYS_ID) {
-// Get the Cortex-A version
-CpuType = (ArmReadMidr () >> 4) & ARM_CPU_TYPE_MASK;
-if (CpuType == ARM_CPU_TYPE_A9) {
-  Status = ArmVExpressGetPlatformFromId (ARM_FVP_VEXPRESS_A9x4, Platform);
-} else if (CpuType == ARM_CPU_TYPE_A15) {
-  CoreCount = ArmGetCpuCountPerCluster ();
-  if (CoreCount == 1) {
-Status = ArmVExpressGetPlatformFromId (ARM_FVP_VEXPRESS_A15x1, 
Platform);
-  } else if (CoreCount == 2) {
-Status = ArmVExpressGetPlatformFromId (ARM_FVP_VEXPRESS_A15x2, 
Platform);
-  } else if (CoreCount == 4) {
-Status = ArmVExpressGetPlatformFromId (ARM_FVP_VEXPRESS_A15x4, 
Platform);
-  }
-}
-  }
-
-  if (EFI_ERROR (Status)) {
-DEBUG ((EFI_D_ERROR, "Unsupported platform (SysId:0x%X, CpuType:0x%X)\n", 
SysId, CpuType));
-ASSERT_EFI_ERROR (Status);
-  }
-
-  return Status;
-}
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf
index 327c5101ddb5..2a8c8388a3b2 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf
@@ -24,9 +24,6 @@ [Sources.common]
   ArmFvpDxe.c
   ArmVExpressCommon.c
 
-[Sources.ARM]
-  Arm/ArmFvpDxeArm.c
-
 [Sources.AARCH64]
   AArch64/ArmFvpDxeAArch64.c
 
@@ -61,12 +58,6 @@ [Protocols]
 [FixedPcd]
   gArmVExpressTokenSpaceGuid.PcdFvpFdtDevicePathsBase
 
-[FixedPcd.ARM]
-  gArmVExpressTokenSpaceGuid.PcdFdtVExpressFvpA9x4
-  gArmVExpressTokenSpaceGuid.PcdFdtVExpressFvpA15x1
-  gArmVExpressTokenSpaceGuid.PcdFdtVExpressFvpA15x2
-  gArmVExpressTokenSpaceGuid.PcdFdtVExpressFvpA15x4
-
 [FixedPcd.AARCH64]
   gArmVExpressTokenSpaceGuid.PcdFdtFvpVExpressAEMv8x4
   gArmVExpressTokenSpaceGuid.PcdFdtFvpBaseAEMv8x4GicV2
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
index c774d97541e1..39f046541502 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec
+++

[edk2] [PATCH 0/6] Embedded|ArmPlatformPkg: spring cleaning + DtPlatformDxe switch

2017-03-29 Thread Ard Biesheuvel

This implements the upstream part of switching VExpress TC2 and the AArch64
FVP Foundation/Base models to the new DtPlatformDxe driver, which is much
simpler and only allows ACPI or DT to be enabled but never both.

Patches #1 and #2 tweak the new DtPlatformDxe so it can choose from several
builtin DTBs depending on the actual platform detected at runtime.

Patches #3, #4 and #5 are basically preparatory cleanup that allows patch #6
to radically change ArmFvpDxe without affecting other users.

Patch #6 removes all the handling of FDT device paths, string PCDs that
have to be initialized to 128 spaces and other awkwardness, and simply
sets the default DTB file section index based on the detected platform.

Ard Biesheuvel (6):
  EmbeddedPkg/DtPlatformDxe: allow multiple entries in DTB FV file
  EmbeddedPkg/DtPlatformDxe: declare symbolic name for FILE_GUID
  ArmPlatformPkg/ArmShellCmdRunAxf: remove BdsLib dependency
  ArmPlatformPkg/ArmVExpressDxe: remove ARM support
  ArmPlatformPkg/ArmVExpressDxe: remove unused cruft from ArmHwDxe
  ArmPlatformPkg/ArmVExpressDxe: simply FDT handling in ArmFvpDxe

 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/AArch64/ArmFvpDxeAArch64.c |  60 
+++--
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/Arm/ArmFvpDxeArm.c |  84 

 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.c| 134 
++--
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmFvpDxe.inf  |  42 
++
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.c |  43 
+--
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.inf   |   3 -
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmVExpressCommon.c|  48 
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmVExpressInternal.h  |  52 
+---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressPkg.dec|  28 

 ArmPlatformPkg/Library/ArmShellCmdRunAxf/ArmShellCmdRunAxf.inf  |   1 -
 ArmPlatformPkg/Library/ArmShellCmdRunAxf/RunAxf.c   |  58 
-
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   |   5 
+-
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf |   6 
+-
 EmbeddedPkg/EmbeddedPkg.dec |   6 +
 14 files changed, 108 insertions(+), 462 deletions(-)
 delete mode 100644 
ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/Arm/ArmFvpDxeArm.c
 delete mode 100644 
ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmVExpressCommon.c

-- 
2.9.3

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

[edk2] [PATCH 2/6] EmbeddedPkg/DtPlatformDxe: declare symbolic name for FILE_GUID

2017-03-29 Thread Ard Biesheuvel

Add a definition to the package .dec file to allow DEPEXes to refer
to DtPlatformDxe in a BEFORE/AFTER DEPEX.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf | 2 +-
 EmbeddedPkg/EmbeddedPkg.dec | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf 
b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
index c16202790ed9..64a674ad94b1 100644
--- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
@@ -16,7 +16,7 @@
 [Defines]
   INF_VERSION   = 0x00010019
   BASE_NAME = DtPlatformDxe
-  FILE_GUID = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC
+  FILE_GUID = FC097B3C-2EBD-4A75-A3DA-121DCAB365CC # 
gDtPlatformDxeFileGuid
   MODULE_TYPE   = DXE_DRIVER
   VERSION_STRING= 1.0
   ENTRY_POINT   = DtPlatformDxeEntryPoint
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index f1b7af347861..4d0322b73867 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -68,6 +68,9 @@ [Guids.common]
   # File GUID for default DTB image embedded in the firmware volume
   gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 
0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
 
+  # FILE_GUID defined in Drivers/DtPlatformDxe/DtPlatformDxe.inf
+  gDtPlatformDxeFileGuid = { 0xFC097B3C, 0x2EBD, 0x4A75, { 0xA3, 0xDA, 0x12, 
0x1D, 0xCA, 0xB3, 0x65, 0xCC } }
+
 [Protocols.common]
   gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 
0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
   gEfiDebugSupportPeriodicCallbackProtocolGuid = { 0x9546e07c, 0x2cbb, 0x4c88, 
{ 0x98, 0x6c, 0xcd, 0x34, 0x10, 0x86, 0xf0, 0x44 } }
-- 
2.9.3

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

[edk2] [PATCH 5/6] ArmPlatformPkg/ArmVExpressDxe: remove unused cruft from ArmHwDxe

2017-03-29 Thread Ard Biesheuvel

Remove unused cruft from ArmHwDxe -- the only thing that remains is
installation of the 'runaxf' shell command.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.c   | 43 
+---
 ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.inf |  3 --
 2 files changed, 1 insertion(+), 45 deletions(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.c 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.c
index 351c73312dc4..19efa3c23dea 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.c
@@ -12,49 +12,8 @@
 
 **/
 
-#include "ArmVExpressInternal.h"
 #include 
-
-CONST EFI_GUID ArmHwA9x4Guid = { 0x2fd21cf6, 0xe6e8, 0x4ff2, { 0xa9, 0xca, 
0x3b, 0x9f, 0x00, 0xe9, 0x28, 0x89 } };
-CONST EFI_GUID ArmHwA15x2A7x3Guid = { 0xd5e606eb, 0x83df, 0x4e90, { 0x81, 
0xe8, 0xc3, 0xdb, 0x2f, 0x77, 0x17, 0x9a } };
-CONST EFI_GUID ArmHwA15Guid = { 0x6b8947c2, 0x4287, 0x4d91, { 0x8f, 0xe0, 
0xa3, 0x81, 0xea, 0x5b, 0x56, 0x8f } };
-CONST EFI_GUID ArmHwA5Guid = { 0xa2cc7663, 0x4d7c, 0x448a, { 0xaa, 0xb5, 0x4c, 
0x03, 0x4b, 0x6f, 0xda, 0xb7 } };
-CONST EFI_GUID NullGuid = { 0x0, 0x0, 0x0, { 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 
0x0, 0x0 } };
-
-//
-// Description of the four hardware platforms :
-// just the platform id for the time being.
-// Platform ids are defined in ArmVExpressInternal.h for
-// all "ArmVExpress-like" platforms (AARCH64 or ARM architecture,
-// model or hardware platforms).
-//
-//Note: File extensions are stripped with the VExpress NOR Flash FileSystem
-CONST ARM_VEXPRESS_PLATFORM ArmVExpressPlatforms[] = {
-  { ARM_HW_A9x4, , L"ca9" },
-  { ARM_HW_A15x2_A7x3, , L"ca15a7" },
-  { ARM_HW_A15, , L"ca15a7" },
-  { ARM_HW_A5, , L"ca5s" },
-  { ARM_FVP_VEXPRESS_UNKNOWN, , NULL }
-};
-
-/**
-  Get information about the VExpress platform the firmware is running on.
-
-  @param[out]  Platform   Address where the pointer to the platform information
-  (type ARM_VEXPRESS_PLATFORM*) should be stored.
-  The returned pointer does not point to an allocated
-  memory area. Not used here.
-
-  @retval  EFI_NOT_FOUND  The platform was not recognised.
-
-**/
-EFI_STATUS
-ArmVExpressGetPlatform (
-  OUT CONST ARM_VEXPRESS_PLATFORM** Platform
-  )
-{
-  return EFI_NOT_FOUND;
-}
+#include 
 
 /**
  * Generic UEFI Entrypoint for 'ArmHwDxe' driver
diff --git a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.inf 
b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.inf
index 1a007627ad3f..1ecdbb0b231e 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.inf
+++ b/ArmPlatformPkg/ArmVExpressPkg/ArmVExpressDxe/ArmHwDxe.inf
@@ -22,12 +22,9 @@ [Defines]
 
 [Sources.common]
   ArmHwDxe.c
-  ArmVExpressCommon.c
 
 [Packages]
-  ArmPkg/ArmPkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
-  EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
 
 [LibraryClasses]
-- 
2.9.3

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

[edk2] [PATCH 1/6] EmbeddedPkg/DtPlatformDxe: allow multiple entries in DTB FV file

2017-03-29 Thread Ard Biesheuvel

To allow some dynamic behavior in selecting the DTB to expose to the
OS, allow the DTB FV file to contain multiple sections, and indirect
the choice of section via a fixed/dynamic PCD, which defaults to 0.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c   | 5 -
 EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf | 4 
 EmbeddedPkg/EmbeddedPkg.dec | 3 +++
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c 
b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
index 5778633b4985..72f9f5721cda 100644
--- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -121,7 +122,9 @@ DtPlatformDxeEntryPoint (
   //
   Dtb = NULL;
   Status = GetSectionFromAnyFv (,
- EFI_SECTION_RAW, 0, , );
+ EFI_SECTION_RAW,
+ PcdGet8 (PcdDtPlatformDefaultDtbSectionIndex),
+ , );
   if (EFI_ERROR (Status)) {
 DEBUG ((DEBUG_WARN, "%a: no DTB blob found, defaulting to ACPI\n",
   __FUNCTION__));
diff --git a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf 
b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
index b73877a6086b..c16202790ed9 100644
--- a/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
+++ b/EmbeddedPkg/Drivers/DtPlatformDxe/DtPlatformDxe.inf
@@ -43,6 +43,7 @@ [LibraryClasses]
   DxeServicesLib
   HiiLib
   MemoryAllocationLib
+  PcdLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
   UefiRuntimeServicesTableLib
@@ -56,3 +57,6 @@ [Guids]
 [Depex]
   gEfiVariableArchProtocolGuidAND
   gEfiVariableWriteArchProtocolGuid
+
+[Pcd]
+  gEmbeddedTokenSpaceGuid.PcdDtPlatformDefaultDtbSectionIndex
diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index 871fc5ff4016..f1b7af347861 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -198,3 +198,6 @@ [PcdsFixedAtBuild.X64]
 
 [PcdsFixedAtBuild.common, PcdsDynamic.common]
   gEmbeddedTokenSpaceGuid.PcdFdtDevicePaths|L""|VOID*|0x0055
+
+  # the section containing the default DTB for the current platform
+  
gEmbeddedTokenSpaceGuid.PcdDtPlatformDefaultDtbSectionIndex|0|UINT8|0x0057
-- 
2.9.3

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

Re: [edk2] [PATCH] kvm: pass the virtual SEI syndrome to guest OS

2017-03-29 Thread Laszlo Ersek

On 03/29/17 14:51, Michael S. Tsirkin wrote:
> On Wed, Mar 29, 2017 at 01:58:29PM +0200, Laszlo Ersek wrote:
>> (8) When QEMU gets SIGBUS from the kernel -- I hope that's going to come
>> through a signalfd -- QEMU can format the CPER right into guest memory,
>> and then inject whatever interrupt (or assert whatever GPIO line) is
>> necessary for notifying the guest.
> 
> I think I see a race condition potential - what if guest accesses
> CPER in guest memory while it's being written?

I'm not entirely sure about the data flow here (these parts of the ACPI
spec are particularly hard to read...), but I thought the OS wouldn't
look until it got a notification.

Or, are you concerned about the next CPER write by QEMU, while the OS is
reading the last one (and maybe the CPER area could wrap around?)

> 
> We can probably use another level of indirection to fix this:
> 
> allocate twice the space, add a pointer to where the valid
> table is located and update that after writing CPER completely.
> The pointer can be written atomically but also needs to
> be read atomically, so I suspect it should be a single byte
> as we don't know how are OSPMs implementing this.
> 

A-B-A problem? (Is that usually solved with a cookie or a wider
generation counter? But that would again require wider atomics.)

I do wonder though how this is handled on physical hardware. Assuming
the hardware error traps to the firmware first (which, on phys hw, is
responsible for depositing the CPER), in that scenario the phys firmware
would face the same issue (i.e., asynchronously interrupting the OS,
which could be reading the previously stored CPER).

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

Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library

2017-03-29 Thread Laszlo Ersek

On 03/29/17 11:49, Gao, Liming wrote:
> Laszlo:

> I agree GuidCName can't be used to initialize the global variable. If
> there is such requirement, GUID C MACRO will have to be defined.
> Then, its header file will be required.

> Now, we have no rule to forbid to add the header file if it has no
> other definitions except for GUID value, and we have no such
> discussion to define those rules. It is the developer choice to add
> it or not.

> Here, I want to mention that BaseTools has added "extern gGuidCName"
> into AutoGen header file. The consumer code can directly use
> gGuidCName without include its header file. If no GUID MACRO usage,
> its header file is not necessary.
> 
> For this case gEdkiiPlatformHasAcpiGuid, I know it will be as
> protocol dependency. I don't see its GUID C MACRO usage. So, I
> suggest not to add its header file. This is just my opinion.

Thank you for explaining the situation.

For now we've moved all of the generic/core patches in this series to
EmbeddedPkg (and the v4 series has been committed). Once Leif returns,
we can perhaps think about moving those GUIDs / lib instances to
MdeModulePkg, and we could agree at that point to drop the Guid/ header.

> For BaseTools enhancement to generated GUID C MACRO in autogen.h, I
> think it is valuable. To avoid the generated MACRO conflict with the
> existing MACRO, the generated MACRO will still have "G" prefix, such
> as gEdkiiPlatformHasAcpiGuid ==> G_EDKII_PLATFORM_HAS_ACPI_GUID.

Thanks -- I filed .
I marked it as a whishlist item.

Cheers
Laszlo

>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, March 28, 2017 3:50 PM
>> To: Gao, Liming ; Ard Biesheuvel
>> 
>> Cc: Zeng, Star ; Kinney, Michael D
>> ; af...@apple.com; Tian, Feng
>> ; edk2-devel-01 ; Leif
>> Lindholm 
>> Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>> ACPI Protocol, and plug-in library
>>
>> On 03/28/17 07:25, Gao, Liming wrote:
>>> Ersek:
>>>   I don't want to block your patch. You can still check in Guid header file 
>>> if you
>> think it is necessary.
>>
>> I don't want to check in without formal MdeModulePkg maintainer
>> approval. If I check in a patch without formal approval, that will only
>> lead to chaos. I don't want to circumvent the process; I want the
>> process to *work*.
>>
>> If MdeModulePkg maintainers agree with my
>>
>>  [PATCH v3 04/12] MdeModulePkg: introduce EDKII Platform Has ACPI GUID
>>
>> then they should please give their Reviewed-by for it.
>>
>> If they disagree with it, they should please explain why.
>>
>> The specific argument you raised, namely that a BaseTools utility
>> generates the necessary C language artifacts for GUID use, is not
>> precise. BaseTools generate *some* artifacts, but they do not generate a
>> macro that is usable for initializing a GUID object of static storage
>> duration (= global variable GUID, or a GUID field in a global variable
>> structure).
>>
>> This is what the ISO C99 standard says:
>>
>>  6.7.8 Initialization
>>
>>  [...]
>>
>>  Constraints
>>
>>  [...]
>>
>>  4 All the expressions in an initializer for an object that has static
>>storage duration shall be constant expressions or string literals.
>>
>>  [...]
>>
>>  16 Otherwise, the initializer for an object that has aggregate or
>> union type shall be a brace-enclosed list of initializers for the
>> elements or named members.
>>
>>  [...]
>>
>>  20 If the aggregate or union contains elements or members that are
>> aggregates or unions, these rules apply recursively to the
>> subaggregates or contained unions. [...]
>>
>> This is the GUID type definition:
>>
>> typedef struct {
>>  UINT32  Data1;
>>  UINT16  Data2;
>>  UINT16  Data3;
>>  UINT8   Data4[8];
>> } GUID;
>>
>> For this structure, the above standardese implies that we have to
>> provide integer constant expressions in the initializer.
>>
>>  6.6 Constant expressions
>>
>>  [...]
>>
>>  6 An /integer constant expression/ shall have integer type and shall
>>only have operands that are integer constants, enumeration
>>constants, character constants, /sizeof/ expressions whose results
>>are integer constants, and floating constants that are the
>>immediate operands of casts. Cast operators in an integer constant
>>expression shall only convert arithmetic types to integer types,
>>except as part of an operand to the /sizeof/ operator.
>>
>> The end result is that "gEdkiiPlatformHasAcpiGuid" is not usable in such
>> contexts, but the EDKII_PLATFORM_HAS_ACPI_GUID macro is.
>>
>>
>> If you want, I can file a TianoCore feature request BZ for generating
>> such macros as well in BaseTools, but for now, I don't think it should
>> either block my patch noted

Re: [edk2] [PATCH] kvm: pass the virtual SEI syndrome to guest OS

2017-03-29 Thread Laszlo Ersek

(This ought to be one of the longest address lists I've ever seen :)
Thanks for the CC. I'm glad Shannon is already on the CC list. For good
measure, I'm adding MST and Igor.)

On 03/29/17 12:36, Achin Gupta wrote:
> Hi gengdongjiu,
> 
> On Wed, Mar 29, 2017 at 05:36:37PM +0800, gengdongjiu wrote:
>>
>> Hi Laszlo/Biesheuvel/Qemu developer,
>>
>>Now I encounter a issue and want to consult with you in ARM64 platform， 
>> as described below:
>>
>> when guest OS happen synchronous or asynchronous abort, kvm needs
>> to send the error address to Qemu or UEFI through sigbus to
>> dynamically generate APEI table. from my investigation, there are
>> two ways:
>>
>> (1) Qemu get the error address, and generate the APEI table, then
>> notify UEFI to know this generation, then inject abort error to
>> guest OS, guest OS read the APEI table.
>> (2) Qemu get the error address, and let UEFI to generate the APEI
>> table, then inject abort error to guest OS, guest OS read the APEI
>> table.
> 
> Just being pedantic! I don't think we are talking about creating the APEI 
> table
> dynamically here. The issue is: Once KVM has received an error that is 
> destined
> for a guest it will raise a SIGBUS to Qemu. Now before Qemu can inject the 
> error
> into the guest OS, a CPER (Common Platform Error Record) has to be generated
> corresponding to the error source (GHES corresponding to memory subsystem,
> processor etc) to allow the guest OS to do anything meaningful with the
> error. So who should create the CPER is the question.
> 
> At the EL3/EL2 interface (Secure Firmware and OS/Hypervisor), an error arrives
> at EL3 and secure firmware (at EL3 or a lower secure exception level) is
> responsible for creating the CPER. ARM is experimenting with using a 
> Standalone
> MM EDK2 image in the secure world to do the CPER creation. This will avoid
> adding the same code in ARM TF in EL3 (better for security). The error will 
> then
> be injected into the OS/Hypervisor (through SEA/SEI/SDEI) through ARM Trusted
> Firmware.
> 
> Qemu is essentially fulfilling the role of secure firmware at the EL2/EL1
> interface (as discussed with Christoffer below). So it should generate the 
> CPER
> before injecting the error.
> 
> This is corresponds to (1) above apart from notifying UEFI (I am assuming you
> mean guest UEFI). At this time, the guest OS already knows where to pick up 
> the
> CPER from through the HEST. Qemu has to create the CPER and populate its 
> address
> at the address exported in the HEST. Guest UEFI should not be involved in this
> flow. Its job was to create the HEST at boot and that has been done by this
> stage.
> 
> Qemu folk will be able to add but it looks like support for CPER generation 
> will
> need to be added to Qemu. We need to resolve this.
> 
> Do shout if I am missing anything above.

After reading this email, the use case looks *very* similar to what
we've just done with VMGENID for QEMU 2.9.

We have a facility between QEMU and the guest firmware, called "ACPI
linker/loader", with which QEMU instructs the firmware to

- allocate and download blobs into guest RAM (AcpiNVS type memory) --
ALLOCATE command,

- relocate pointers in those blobs, to fields in other (or the same)
blobs -- ADD_POINTER command,

- set ACPI table checksums -- ADD_CHECKSUM command,

- and send GPAs of fields within such blobs back to QEMU --
WRITE_POINTER command.

This is how I imagine we can map the facility to the current use case
(note that this is the first time I read about HEST / GHES / CPER):

etc/acpi/tables etc/hardware_errors
 ==
 +---+
+--+ | address   | +-> +--+
|HEST  + | registers | |   | Error Status |
+ ++ | +-+ |   | Data Block 1 |
| | GHES   | --> | | address | +   | ++
| | GHES   | --> | | address | --+ | |  CPER  |
| | GHES   | --> | | address | + | | |  CPER  |
| | GHES   | --> | | address | -+  | | | |  CPER  |
+-++ +-+-+  |  | | +-++
|  | |
|  | +---> +--+
|  |   | Error Status |
|  |   | Data Block 2 |
|  |   | ++
|  |   | |  CPER  |
|  |   | |  CPER  |
|  |   +-++
|  |
|  +-> +--+
|  | Error Status |

Re: [edk2] [PATCH] kvm: pass the virtual SEI syndrome to guest OS

2017-03-29 Thread Achin Gupta

Hi gengdongjiu,

On Wed, Mar 29, 2017 at 05:36:37PM +0800, gengdongjiu wrote:
>
> Hi Laszlo/Biesheuvel/Qemu developer,
>
>Now I encounter a issue and want to consult with you in ARM64 platform， as 
> described below:
>
>when guest OS happen synchronous or asynchronous abort, kvm needs to send 
> the error address to Qemu or UEFI through sigbus to dynamically generate APEI 
> table. from my investigation, there are two ways:
>
>(1) Qemu get the error address, and generate the APEI table, then notify 
> UEFI to know this generation, then inject abort error to guest OS, guest OS 
> read the APEI table.
>(2) Qemu get the error address, and let UEFI to generate the APEI table, 
> then inject abort error to guest OS, guest OS read the APEI table.

Just being pedantic! I don't think we are talking about creating the APEI table
dynamically here. The issue is: Once KVM has received an error that is destined
for a guest it will raise a SIGBUS to Qemu. Now before Qemu can inject the error
into the guest OS, a CPER (Common Platform Error Record) has to be generated
corresponding to the error source (GHES corresponding to memory subsystem,
processor etc) to allow the guest OS to do anything meaningful with the
error. So who should create the CPER is the question.

At the EL3/EL2 interface (Secure Firmware and OS/Hypervisor), an error arrives
at EL3 and secure firmware (at EL3 or a lower secure exception level) is
responsible for creating the CPER. ARM is experimenting with using a Standalone
MM EDK2 image in the secure world to do the CPER creation. This will avoid
adding the same code in ARM TF in EL3 (better for security). The error will then
be injected into the OS/Hypervisor (through SEA/SEI/SDEI) through ARM Trusted
Firmware.

Qemu is essentially fulfilling the role of secure firmware at the EL2/EL1
interface (as discussed with Christoffer below). So it should generate the CPER
before injecting the error.

This is corresponds to (1) above apart from notifying UEFI (I am assuming you
mean guest UEFI). At this time, the guest OS already knows where to pick up the
CPER from through the HEST. Qemu has to create the CPER and populate its address
at the address exported in the HEST. Guest UEFI should not be involved in this
flow. Its job was to create the HEST at boot and that has been done by this
stage.

Qemu folk will be able to add but it looks like support for CPER generation will
need to be added to Qemu. We need to resolve this.

Do shout if I am missing anything above.

cheers,
Achin

>
>
>Do you think which modules generates the APEI table is better? UEFI or 
> Qemu?
>
>
>
>
> On 2017/3/28 21:40, James Morse wrote:
> > Hi gengdongjiu,
> >
> > On 28/03/17 13:16, gengdongjiu wrote:
> >> On 2017/3/28 19:54, Achin Gupta wrote:
> >>> On Tue, Mar 28, 2017 at 01:23:28PM +0200, Christoffer Dall wrote:
>  On Tue, Mar 28, 2017 at 11:48:08AM +0100, James Morse wrote:
> > On the host, part of UEFI is involved to generate the CPER records.
> > In a guest?, I don't know.
> > Qemu could generate the records, or drive some other component to do it.
> 
>  I think I am beginning to understand this a bit.  Since the guet UEFI
>  instance is specifically built for the machine it runs on, QEMU's virt
>  machine in this case, they could simply agree (by some contract) to
>  place the records at some specific location in memory, and if the guest
>  kernel asks its guest UEFI for that location, things should just work by
>  having logic in QEMU to process error reports and populate guest memory.
> 
>  Is this how others see the world too?
> >>>
> >>> I think so!
> >>>
> >>> AFAIU, the memory where CPERs will reside should be specified in a GHES 
> >>> entry in
> >>> the HEST. Is this not the case with a guest kernel i.e. the guest UEFI 
> >>> creates a
> >>> HEST for the guest Kernel?
> >>>
> >>> If so, then the question is how the guest UEFI finds out where QEMU 
> >>> (acting as
> >>> EL3 firmware) will populate the CPERs. This could either be a contract 
> >>> between
> >>> the two or a guest DXE driver uses the MM_COMMUNICATE call (see [1]) to 
> >>> ask QEMU
> >>> where the memory is.
> >>
> >> whether invoke the guest UEFI will be complex? not see the advantage. it 
> >> seems x86 Qemu
> >> directly generate the ACPI table, but I am not sure, we are checking the 
> >> qemu
> > logical.
> >> let Qemu generate CPER record may be clear.
> >
> > At boot UEFI in the guest will need to make sure the areas of memory that 
> > may be
> > used for CPER records are reserved. Whether UEFI or Qemu decides where 
> > these are
> > needs deciding, (but probably not here)...
> >
> > At runtime, when an error has occurred, I agree it would be simpler (fewer
> > components involved) if Qemu generates the CPER records. But if UEFI made 
> > the
> > memory choice above they need to interact and it gets complicated again. The
> > CPER records are defined in the UEFI spec, so

Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has ACPI Protocol, and plug-in library

2017-03-29 Thread Gao, Liming

Laszlo:
  I agree GuidCName can't be used to initialize the global variable. If there 
is such requirement, GUID C MACRO will have to be defined. Then, its header 
file will be required. 
  Now, we have no rule to forbid to add the header file if it has no other 
definitions except for GUID value, and we have no such discussion to define 
those rules. It is the developer choice to add it or not. 
  Here, I want to mention that BaseTools has added "extern gGuidCName" into 
AutoGen header file. The consumer code can directly use gGuidCName without 
include its header file. If no GUID MACRO usage, its header file is not 
necessary. 

  For this case gEdkiiPlatformHasAcpiGuid, I know it will be as protocol 
dependency. I don't see its GUID C MACRO usage. So, I suggest not to add its 
header file. This is just my opinion. 
  For BaseTools enhancement to generated GUID C MACRO in autogen.h, I think it 
is valuable. To avoid the generated MACRO conflict with the existing MACRO, the 
generated MACRO will still have "G" prefix, such as gEdkiiPlatformHasAcpiGuid 
==> G_EDKII_PLATFORM_HAS_ACPI_GUID.

Thanks
Liming
>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Tuesday, March 28, 2017 3:50 PM
>To: Gao, Liming ; Ard Biesheuvel
>
>Cc: Zeng, Star ; Kinney, Michael D
>; af...@apple.com; Tian, Feng
>; edk2-devel-01 ; Leif
>Lindholm 
>Subject: Re: [edk2] [PATCH v2 05/12] ArmPkg: introduce EDKII Platform Has
>ACPI Protocol, and plug-in library
>
>On 03/28/17 07:25, Gao, Liming wrote:
>> Ersek:
>>   I don't want to block your patch. You can still check in Guid header file 
>> if you
>think it is necessary.
>
>I don't want to check in without formal MdeModulePkg maintainer
>approval. If I check in a patch without formal approval, that will only
>lead to chaos. I don't want to circumvent the process; I want the
>process to *work*.
>
>If MdeModulePkg maintainers agree with my
>
>  [PATCH v3 04/12] MdeModulePkg: introduce EDKII Platform Has ACPI GUID
>
>then they should please give their Reviewed-by for it.
>
>If they disagree with it, they should please explain why.
>
>The specific argument you raised, namely that a BaseTools utility
>generates the necessary C language artifacts for GUID use, is not
>precise. BaseTools generate *some* artifacts, but they do not generate a
>macro that is usable for initializing a GUID object of static storage
>duration (= global variable GUID, or a GUID field in a global variable
>structure).
>
>This is what the ISO C99 standard says:
>
>  6.7.8 Initialization
>
>  [...]
>
>  Constraints
>
>  [...]
>
>  4 All the expressions in an initializer for an object that has static
>storage duration shall be constant expressions or string literals.
>
>  [...]
>
>  16 Otherwise, the initializer for an object that has aggregate or
> union type shall be a brace-enclosed list of initializers for the
> elements or named members.
>
>  [...]
>
>  20 If the aggregate or union contains elements or members that are
> aggregates or unions, these rules apply recursively to the
> subaggregates or contained unions. [...]
>
>This is the GUID type definition:
>
>typedef struct {
>  UINT32  Data1;
>  UINT16  Data2;
>  UINT16  Data3;
>  UINT8   Data4[8];
>} GUID;
>
>For this structure, the above standardese implies that we have to
>provide integer constant expressions in the initializer.
>
>  6.6 Constant expressions
>
>  [...]
>
>  6 An /integer constant expression/ shall have integer type and shall
>only have operands that are integer constants, enumeration
>constants, character constants, /sizeof/ expressions whose results
>are integer constants, and floating constants that are the
>immediate operands of casts. Cast operators in an integer constant
>expression shall only convert arithmetic types to integer types,
>except as part of an operand to the /sizeof/ operator.
>
>The end result is that "gEdkiiPlatformHasAcpiGuid" is not usable in such
>contexts, but the EDKII_PLATFORM_HAS_ACPI_GUID macro is.
>
>
>If you want, I can file a TianoCore feature request BZ for generating
>such macros as well in BaseTools, but for now, I don't think it should
>either block my patch noted above, or force me to drop the Guid/ header
>file from the patch. Right now, the Guid/ header adds something that is
>not available from BaseTools, and I wouldn't like to delay the v3 series
>any longer.
>
>Do you agree to the above method? I file a TianoCore Feature Request BZ
>for BaseTools, and MdeModulePkg maintainers formally approve the v3
>04/12 patch?
>
>
>(Side point: I think the Guid/ header file has another benefit:
>documentation. I don't think we can add the amount of documentation that
>is acceptable in a standalone Guid/ header to an in-line comment in the
>.DEC file. So, I think that the new

Re: [edk2] [Patch] BaseTools: tools_def.txt to append new option in VFRPP flag

2017-03-29 Thread Gao, Liming

Yonghong:
  Please consider the case that $(MODULE_NAME)ImgDefs.h is not generated. This 
patch may not work. 

Thanks
Liming
>-Original Message-
>From: Zhu, Yonghong
>Sent: Tuesday, March 28, 2017 5:37 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming 
>Subject: [Patch] BaseTools: tools_def.txt to append new option in VFRPP flag
>
>tools_def.txt to append new option (/FI$(MODULE_NAME)ImgDefs.h) in
>VFRPP
>flag.
>
>Cc: Liming Gao 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Yonghong Zhu 
>---
> BaseTools/Conf/tools_def.template | 172 +++
>---
> 1 file changed, 86 insertions(+), 86 deletions(-)
>
>diff --git a/BaseTools/Conf/tools_def.template
>b/BaseTools/Conf/tools_def.template
>index ab4f936..b537abe 100755
>--- a/BaseTools/Conf/tools_def.template
>+++ b/BaseTools/Conf/tools_def.template
>@@ -802,11 +802,11 @@ DEFINE SOURCERY_CYGWIN_TOOLS =
>/cygdrive/c/Program Files/CodeSourcery/Sourcery G
> *_VS2003_IA32_RC_PATH   = DEF(VS2003_BIN)\rc.exe
>
>   *_VS2003_IA32_MAKE_FLAGS  = /nologo
>   *_VS2003_IA32_APP_FLAGS   = /nologo /E /TC
>   *_VS2003_IA32_PP_FLAGS= /nologo /E /TC /FIAutoGen.h
>-  *_VS2003_IA32_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h
>+  *_VS2003_IA32_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h /FI$(MODULE_NAME)ImgDefs.h
>   DEBUG_VS2003_IA32_CC_FLAGS= /nologo /c /WX /W4 /Gs32768 /Gy /D
>UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /GX- /Zi /Gm
> RELEASE_VS2003_IA32_CC_FLAGS= /nologo /c /WX /W4 /Gs32768 /Gy /D
>UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /GX-
> NOOPT_VS2003_IA32_CC_FLAGS  = /nologo /c /WX /W4 /Gs32768 /Gy /D
>UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /GX- /Zi /Gm /Od
>
>   DEBUG_VS2003_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /coff /Cx /Zd
>/Zi
>@@ -836,11 +836,11 @@ NOOPT_VS2003_IA32_DLINK_FLAGS   = /NOLOGO
>/NODEFAULTLIB /IGNORE:4001 /OPT:RE
> *_VS2003_EBC_RC_PATH= DEF(VS2003_BIN)\rc.exe
>
> *_VS2003_EBC_MAKE_FLAGS = /nologo
> *_VS2003_EBC_PP_FLAGS   = /nologo /E /TC /FIAutoGen.h
> *_VS2003_EBC_CC_FLAGS   = /nologo /c /WX /W3 /FIAutoGen.h
>/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT)
>-*_VS2003_EBC_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h
>+*_VS2003_EBC_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h /FI$(MODULE_NAME)ImgDefs.h
> *_VS2003_EBC_SLINK_FLAGS= /lib /NOLOGO /MACHINE:EBC
> *_VS2003_EBC_DLINK_FLAGS= "C:\Program
>Files\Intel\EBC\Lib\EbcLib.lib" /NOLOGO /NODEFAULTLIB /MACHINE:EBC
>/OPT:REF /ENTRY:$(IMAGE_ENTRY_POINT)
>/SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /MAP /ALIGN:32 /DRIVER
>
>
>###
>#
> #
>@@ -880,11 +880,11 @@ NOOPT_VS2003_IA32_DLINK_FLAGS   = /NOLOGO
>/NODEFAULTLIB /IGNORE:4001 /OPT:RE
>
>
>   *_VS2003xASL_IA32_MAKE_FLAGS  = /nologo
>   *_VS2003xASL_IA32_APP_FLAGS   = /nologo /E /TC
>   *_VS2003xASL_IA32_PP_FLAGS= /nologo /E /TC /FIAutoGen.h
>-  *_VS2003xASL_IA32_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h
>+  *_VS2003xASL_IA32_VFRPP_FLAGS = /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h /FI$(MODULE_NAME)ImgDefs.h
>   DEBUG_VS2003xASL_IA32_CC_FLAGS= /nologo /c /WX /W4 /Gs32768
>/Gy /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /GX- /Zi /Gm
> RELEASE_VS2003xASL_IA32_CC_FLAGS= /nologo /c /WX /W4 /Gs32768
>/Gy /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /GX-
> NOOPT_VS2003xASL_IA32_CC_FLAGS  = /nologo /c /WX /W4 /Gs32768
>/Gy /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /GX- /Zi /Gm /Od
>
>   DEBUG_VS2003xASL_IA32_ASM_FLAGS   = /nologo /c /WX /W3 /coff /Cx
>/Zd /Zi
>@@ -914,11 +914,11 @@ NOOPT_VS2003xASL_IA32_DLINK_FLAGS   =
>/NOLOGO /NODEFAULTLIB /IGNORE:4001 /OP
> *_VS2003xASL_EBC_RC_PATH= DEF(VS2003_BIN)\rc.exe
>
> *_VS2003xASL_EBC_MAKE_FLAGS = /nologo
> *_VS2003xASL_EBC_PP_FLAGS   = /nologo /E /TC /FIAutoGen.h
> *_VS2003xASL_EBC_CC_FLAGS   = /nologo /c /WX /W3 /FIAutoGen.h
>/D$(MODULE_ENTRY_POINT)=$(ARCH_ENTRY_POINT)
>-*_VS2003xASL_EBC_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h
>+*_VS2003xASL_EBC_VFRPP_FLAGS= /nologo /E /TC /DVFRCOMPILE
>/FI$(MODULE_NAME)StrDefs.h /FI$(MODULE_NAME)ImgDefs.h
> *_VS2003xASL_EBC_SLINK_FLAGS= /lib /NOLOGO /MACHINE:EBC
> *_VS2003xASL_EBC_DLINK_FLAGS= "C:\Program
>Files\Intel\EBC\Lib\EbcLib.lib" /NOLOGO /NODEFAULTLIB /MACHINE:EBC
>/OPT:REF /ENTRY:$(IMAGE_ENTRY_POINT)
>/SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /MAP /ALIGN:32 /DRIVER
>
>
>###

Re: [edk2] [PATCH] MdeModulePkg/PeiCore: avoid EFI_IMAGE_MACHINE_TYPE_SUPPORTED to check arch

2017-03-29 Thread Gao, Liming

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
>Biesheuvel
>Sent: Monday, March 27, 2017 8:22 PM
>To: edk2-devel@lists.01.org; Gao, Liming 
>Cc: Ard Biesheuvel 
>Subject: [edk2] [PATCH] MdeModulePkg/PeiCore: avoid
>EFI_IMAGE_MACHINE_TYPE_SUPPORTED to check arch
>
>The EFI_IMAGE_MACHINE_TYPE_SUPPORTED() macro is abused in the
>PeiCore
>code to decide whether the system we are compiling for can deal with
>executable code being copied elsewhere and executed from there.
>
>As stated in the comment, this is fundamentally a property of the compiler
>target, and so this should be made dependent on MDE_CPU_xxx
>preprocessor
>defines, and not on whether or not the runtime target can deal with
>PE/COFF images of a certain machine type.
>
>On X86/IA32, this mostly boils down to the same thing, but not on other
>architectures, so let's clean this up.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ard Biesheuvel 
>---
> MdeModulePkg/Core/Pei/Image/Image.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/MdeModulePkg/Core/Pei/Image/Image.c
>b/MdeModulePkg/Core/Pei/Image/Image.c
>index c8bb2300a0a6..198541128512 100644
>--- a/MdeModulePkg/Core/Pei/Image/Image.c
>+++ b/MdeModulePkg/Core/Pei/Image/Image.c
>@@ -112,6 +112,7 @@ GetImageReadFunction (
>   IN  PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
>   )
> {
>+#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
>   PEI_CORE_INSTANCE *Private;
>   EFI_PHYSICAL_ADDRESS  MemoryBuffer;
>
>@@ -119,8 +120,7 @@ GetImageReadFunction (
>   MemoryBuffer = 0;
>
>   if (Private->PeiMemoryInstalled  && (((Private-
>>HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME)
>&& PcdGetBool (PcdShadowPeimOnBoot)) ||
>-  ((Private->HobList.HandoffInformationTable->BootMode ==
>BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot))) &&
>-  (EFI_IMAGE_MACHINE_TYPE_SUPPORTED(EFI_IMAGE_MACHINE_X64)
>|| EFI_IMAGE_MACHINE_TYPE_SUPPORTED(EFI_IMAGE_MACHINE_IA32))) {
>+  ((Private->HobList.HandoffInformationTable->BootMode ==
>BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot {
> //
> // Shadow algorithm makes lots of non ANSI C assumptions and only works
>for IA32 and X64
> //  compilers that have been tested
>@@ -136,7 +136,9 @@ GetImageReadFunction (
>   } else {
> ImageContext->ImageRead = PeiImageRead;
>   }
>-
>+#else
>+  ImageContext->ImageRead = PeiImageRead;
>+#endif
>   return EFI_SUCCESS;
> }
> /**
>--
>2.9.3
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] MdeModulePkg/DxeCore: add missing id-to-string mapping for AARCH64

2017-03-29 Thread Gao, Liming

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard
>Biesheuvel
>Sent: Monday, March 27, 2017 7:20 PM
>To: edk2-devel@lists.01.org; Zeng, Star ; Tian, Feng
>
>Cc: Ard Biesheuvel 
>Subject: [edk2] [PATCH] MdeModulePkg/DxeCore: add missing id-to-string
>mapping for AARCH64
>
>Add a mapping for EFI_IMAGE_MACHINE_AARCH64 to mMachineTypeInfo[]
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Ard Biesheuvel 
>---
> MdeModulePkg/Core/Dxe/Image/Image.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
>b/MdeModulePkg/Core/Dxe/Image/Image.c
>index b22b0fc786fc..91d70d5b6053 100644
>--- a/MdeModulePkg/Core/Dxe/Image/Image.c
>+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
>@@ -90,7 +90,8 @@ GLOBAL_REMOVE_IF_UNREFERENCED
>MACHINE_TYPE_INFO  mMachineTypeInfo[] = {
>   {EFI_IMAGE_MACHINE_IA32,   L"IA32"},
>   {EFI_IMAGE_MACHINE_IA64,   L"IA64"},
>   {EFI_IMAGE_MACHINE_X64,L"X64"},
>-  {EFI_IMAGE_MACHINE_ARMTHUMB_MIXED, L"ARM"}
>+  {EFI_IMAGE_MACHINE_ARMTHUMB_MIXED, L"ARM"},
>+  {EFI_IMAGE_MACHINE_AARCH64,L"AARCH64"}
> };
>
> UINT16 mDxeCoreImageMachineType = 0;
>--
>2.9.3
>
>___
>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] [RFC PATCH v1 1/2] OvmfPkg: Add SVGA2 device register definition header from VMWare

2017-03-29 Thread Phil Dennis-Jordan

From: Phil Dennis-Jordan 

Import a header file defining symbolic constants for the VMWare SVGA2
virtual display device in preparation for supporting it in QemuVideoDXE.
This file is made available by VMWare under the MIT license, for example
at http://vmware-svga.sourceforge.net/

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan 
---
 OvmfPkg/QemuVideoDxe/svga_reg.h | 1558 
 1 file changed, 1558 insertions(+)

diff --git a/OvmfPkg/QemuVideoDxe/svga_reg.h b/OvmfPkg/QemuVideoDxe/svga_reg.h
new file mode 100644
index ..abcd33abbd75
--- /dev/null
+++ b/OvmfPkg/QemuVideoDxe/svga_reg.h
@@ -0,0 +1,1558 @@
+/**
+ * Copyright 1998-2009 VMware, Inc.  All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ **/
+
+/*
+ * svga_reg.h --
+ *
+ *Virtual hardware definitions for the VMware SVGA II device.
+ */
+
+#ifndef _SVGA_REG_H_
+#define _SVGA_REG_H_
+
+typedef UINT32 uint32;
+typedef INT32 int32;
+
+/*
+ * PCI device IDs.
+ */
+#define PCI_VENDOR_ID_VMWARE0x15AD
+#define PCI_DEVICE_ID_VMWARE_SVGA2  0x0405
+
+/*
+ * SVGA_REG_ENABLE bit definitions.
+ */
+#define SVGA_REG_ENABLE_DISABLE 0
+#define SVGA_REG_ENABLE_ENABLE  1
+#define SVGA_REG_ENABLE_HIDE2
+#define SVGA_REG_ENABLE_ENABLE_HIDE (SVGA_REG_ENABLE_ENABLE |\
+SVGA_REG_ENABLE_HIDE)
+
+/*
+ * Legal values for the SVGA_REG_CURSOR_ON register in old-fashioned
+ * cursor bypass mode. This is still supported, but no new guest
+ * drivers should use it.
+ */
+#define SVGA_CURSOR_ON_HIDE0x0   /* Must be 0 to maintain backward 
compatibility */
+#define SVGA_CURSOR_ON_SHOW0x1   /* Must be 1 to maintain backward 
compatibility */
+#define SVGA_CURSOR_ON_REMOVE_FROM_FB  0x2   /* Remove the cursor from the 
framebuffer because we need to see what's under it */
+#define SVGA_CURSOR_ON_RESTORE_TO_FB   0x3   /* Put the cursor back in the 
framebuffer so the user can see it */
+
+/*
+ * The maximum framebuffer size that can traced for e.g. guests in VESA mode.
+ * The changeMap in the monitor is proportional to this number. Therefore, we'd
+ * like to keep it as small as possible to reduce monitor overhead (using
+ * SVGA_VRAM_MAX_SIZE for this increases the size of the shared area by over
+ * 4k!).
+ *
+ * NB: For compatibility reasons, this value must be greater than 0xff.
+ * See bug 335072.
+ */
+#define SVGA_FB_MAX_TRACEABLE_SIZE  0x100
+
+#define SVGA_MAX_PSEUDOCOLOR_DEPTH  8
+#define SVGA_MAX_PSEUDOCOLORS   (1 << SVGA_MAX_PSEUDOCOLOR_DEPTH)
+#define SVGA_NUM_PALETTE_REGS   (3 * SVGA_MAX_PSEUDOCOLORS)
+
+#define SVGA_MAGIC 0x90UL
+#define SVGA_MAKE_ID(ver)  (SVGA_MAGIC << 8 | (ver))
+
+/* Version 2 let the address of the frame buffer be unsigned on Win32 */
+#define SVGA_VERSION_2 2
+#define SVGA_ID_2  SVGA_MAKE_ID(SVGA_VERSION_2)
+
+/* Version 1 has new registers starting with SVGA_REG_CAPABILITIES so
+   PALETTE_BASE has moved */
+#define SVGA_VERSION_1 1
+#define SVGA_ID_1  SVGA_MAKE_ID(SVGA_VERSION_1)
+
+/* Version 0 is the initial version */
+#define SVGA_VERSION_0 0
+#define SVGA_ID_0  SVGA_MAKE_ID(SVGA_VERSION_0)
+
+/* "Invalid" value for all SVGA IDs. (Version ID, screen object ID, surface 
ID...) */
+#define SVGA_ID_INVALID0x
+
+/* Port offsets, relative to BAR0 */
+#define SVGA_INDEX_PORT 0x0
+#define SVGA_VALUE_PORT 0x1
+#define SVGA_BIOS_PORT  0x2
+#define SVGA_IRQSTATUS_PORT 0x8
+
+/*
+ * Interrupt source flags for IRQSTATUS_PORT and IRQMASK.
+ *
+ * Interrupts are only supported when

[edk2] [RFC PATCH v1 2/2] OvmfPkg: Add VMWare SVGA II support in QemuVideoDxe.

2017-03-29 Thread Phil Dennis-Jordan

From: Phil Dennis-Jordan 

In addition to the QXL, Cirrus, etc. VGA adapters, Qemu also implements
a basic version of VMWare's SVGA2 display device. Drivers for this device
exist for guest OSes which do not support Qemu's other display adapters,
so supporting it in OVMF is useful in conjunction with those OSes.

This change adds support for the SVGA2 device's framebuffer to
QemuVideoDxe, based on VMWare's documentation. The most basic
initialisation, framebuffer layout query, and mode setting are
implemented.

The device relies on port-based 32-bit I/O, unfortunately on misaligned
addresses. This requires the addition of unaligned I/O helper functions
for various compiler families, and limits the driver's support to the
x86 family of platforms.

Cc: Jordan Justen 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan 
---
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |  6 ++
 OvmfPkg/QemuVideoDxe/Qemu.h   | 50 +++
 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h| 51 
 OvmfPkg/QemuVideoDxe/Driver.c | 67 +++
 OvmfPkg/QemuVideoDxe/Gop.c| 71 +++-
 OvmfPkg/QemuVideoDxe/Initialize.c | 88 
 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c | 59 +
 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c | 79 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c | 81 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c | 53 
 10 files changed, 604 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf 
b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index affb6ffd88e0..346a5aed94fa 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -41,6 +41,12 @@ [Sources.common]
 
 [Sources.Ia32, Sources.X64]
   VbeShim.c
+  UnalignedIoGcc.c| GCC
+  UnalignedIoMsc.c| MSFT
+  UnalignedIoIcc.c| INTEL
+
+[Sources.IPF, Sources.EBC, Sources.ARM, Sources.AARCH64]
+  UnalignedIoUnsupported.c
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/OvmfPkg/QemuVideoDxe/Qemu.h b/OvmfPkg/QemuVideoDxe/Qemu.h
index 2ce37defc5b8..7be2cac63130 100644
--- a/OvmfPkg/QemuVideoDxe/Qemu.h
+++ b/OvmfPkg/QemuVideoDxe/Qemu.h
@@ -56,6 +56,10 @@ typedef struct {
   UINT32  HorizontalResolution;
   UINT32  VerticalResolution;
   UINT32  ColorDepth;
+  //
+  // VMWare specific:
+  //
+  UINT32  PixelsPerLine; // includes any dead space
 } QEMU_VIDEO_MODE_DATA;
 
 #define PIXEL_RED_SHIFT   0
@@ -92,6 +96,7 @@ typedef enum {
   QEMU_VIDEO_CIRRUS_5446,
   QEMU_VIDEO_BOCHS,
   QEMU_VIDEO_BOCHS_MMIO,
+  QEMU_VIDEO_VMWARE_SVGA2,
 } QEMU_VIDEO_VARIANT;
 
 typedef struct {
@@ -119,6 +124,8 @@ typedef struct {
   QEMU_VIDEO_VARIANTVariant;
   FRAME_BUFFER_CONFIGURE*FrameBufferBltConfigure;
   UINTN FrameBufferBltConfigureSize;
+
+  UINT16VMWareSVGA2_BasePort;
 } QEMU_VIDEO_PRIVATE_DATA;
 
 ///
@@ -459,6 +466,13 @@ outw (
   UINT16  Data
   );
 
+VOID
+outl (
+  QEMU_VIDEO_PRIVATE_DATA  *Private,
+  UINTNAddress,
+  UINT32   Data
+  );
+
 UINT8
 inb (
   QEMU_VIDEO_PRIVATE_DATA  *Private,
@@ -471,6 +485,12 @@ inw (
   UINTN   Address
   );
 
+UINT32
+inl (
+  QEMU_VIDEO_PRIVATE_DATA  *Private,
+  UINTNAddress
+  );
+
 VOID
 BochsWrite (
   QEMU_VIDEO_PRIVATE_DATA  *Private,
@@ -502,9 +522,39 @@ QemuVideoBochsModeSetup (
   BOOLEAN  IsQxl
   );
 
+EFI_STATUS
+QemuVideoVmwareModeSetup (
+  QEMU_VIDEO_PRIVATE_DATA *Private
+  );
+
 VOID
 InstallVbeShim (
   IN CONST CHAR16 *CardName,
   IN EFI_PHYSICAL_ADDRESS FrameBufferBase
   );
+
+VOID
+QemuVideoVMWSVGA2RegisterWrite (
+  QEMU_VIDEO_PRIVATE_DATA *Private,
+  UINT16  reg,
+  UINT32  value
+  );
+
+UINT32
+QemuVideoVMWSVGA2RegisterRead (
+  QEMU_VIDEO_PRIVATE_DATA *Private,
+  UINT16  reg
+  );
+
+EFI_STATUS
+QemuVideoVMWSVGA2CompleteModeData (
+  IN  QEMU_VIDEO_PRIVATE_DATA   *Private,
+  OUT EFI_GRAPHICS_OUTPUT_PROTOCOL_MODE *Mode
+  );
+
+void InitializeVMWSVGA2GraphicsMode (
+  QEMU_VIDEO_PRIVATE_DATA  *Private,
+  QEMU_VIDEO_BOCHS_MODES   *ModeData
+  );
+
 #endif
diff --git a/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h 
b/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
new file mode 100644
index ..1b9f71afb6db
--- /dev/null
+++ b/OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
@@ -0,0 +1,51 @@
+/** @file
+  Unaligned port I/O, with implementations for various x86 compilers and a 
dummy
+  for platforms which do not support unaligned port I/O.
+
+  Copyright (c) 2017, Phil Dennis-Jordan.
+  This program and the accompanying materials
+  are licensed and made

[edk2] [RFC PATCH v1 0/2] OvmfPkg/QemuVideoDxe: Add VMWare SVGA2 framebuffer support

2017-03-29 Thread Phil Dennis-Jordan

This extends the QemuVideoDxe driver to support the VMWare SVGA2 display
device implemented by Qemu. Drivers for this device exist for guest OSes
which do not support Qemu's other display adapters, so supporting it in
OVMF is useful in conjunction with those OSes.

I've tried to follow the existing pattern for device-specific code in
OVMF's QemuVideoDxe driver as much as possible, with the minimum of
additional code. I've marked this patch as RFC for 2 main reasons:

1. I've imported VMWare's own header file with device register constants
etc. verbatim. (patch 1/2) This doesn't follow any of the EDK2 coding
conventions, and it uses the MIT license, not BSD. Only a small percentage
of symbols are actually used in the driver. On the other hand, it's
obviously the authoritative source. I'm not sure what the correct
etiquette is here, define our own constants, or import the authoritative
header file?

2. For the functionality this driver uses, 2 I/O ports are used with
32-bit wide reads and writes. Unfortunately, one of them is not 32-bit
aligned. This is fine as far as x86/x86-64 is concerned, but neither
EDK2's IoLib nor other platforms support such an access pattern. It seems
this issue was already encountered/discussed on the edk2-devel list 4
years ago: http://edk2-devel.narkive.com/bwH3r0us/unaligned-i-o I couldn't
find any code resulting from that discussion, and Qemu definitely uses
unaligned port numbers for the SVGA2 device. (SVGA_IO_MUL is 1 in
hw/display/vmware_vga.c) It does not appear to make any provision for
non-x86 architectures, so I assume there's no sensible way to drive the
device in those cases. The patch therefore only detects the device on x86,
where it uses UnalignedIoWrite/Read32() helper functions which I've based
on IoLib's aligned ones. I have only tested the GCC version of these.
Feel free to suggest a better way of handling the issue.

Github feature branch: https://github.com/pmj/edk2/tree/ovmf_vmware_svga2_v1

Phil Dennis-Jordan (2):
  OvmfPkg: Add SVGA2 device register definition header from VMWare
  OvmfPkg: Add VMWare SVGA II support in QemuVideoDxe.

 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |6 +
 OvmfPkg/QemuVideoDxe/Qemu.h   |   50 +
 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h|   51 +
 OvmfPkg/QemuVideoDxe/svga_reg.h   | 1558 
 OvmfPkg/QemuVideoDxe/Driver.c |   67 +
 OvmfPkg/QemuVideoDxe/Gop.c|   71 +-
 OvmfPkg/QemuVideoDxe/Initialize.c |   88 ++
 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c |   59 +
 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c |   79 +
 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c |   81 +
 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c |   53 +
 11 files changed, 2162 insertions(+), 1 deletion(-)
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoInternal.h
 create mode 100644 OvmfPkg/QemuVideoDxe/svga_reg.h
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoGcc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoIcc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoMsc.c
 create mode 100644 OvmfPkg/QemuVideoDxe/UnalignedIoUnsupported.c

-- 
2.3.2 (Apple Git-55)

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

[edk2] [PATCH v1 1/1] OvmfPkg: Allow multiple add-pointer linker commands to same ACPI table

2017-03-29 Thread Phil Dennis-Jordan

From: Phil Dennis-Jordan 

ACPI tables may contain multiple pointer fields to the same destination
table. For example, in some revisions, the FADT contains both DSDT and
X_DSDT fields, and they may both point to the DSDT. Indeed, some operating
systems demand this to be the case.

Previously, if Qemu created "add pointer" linker commands for both fields,
the linking process would fail, as AcpiProtocol->InstallAcpiTable() may
only be called once for each destination table and otherwise returns an
error.

This change adds a memoisation data structure which tracks the table
pointers that have already been installed; even if the same pointer is
encountered multiple times, it is only installed once.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Phil Dennis-Jordan 
---
 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 109 
 1 file changed, 89 insertions(+), 20 deletions(-)

diff --git a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c 
b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
index 7bb2e3f21821..cffa838623cc 100644
--- a/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
+++ b/OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c
@@ -100,6 +100,39 @@ BlobCompare (
 
 
 /**
+  Comparator function for two opaque pointers, ordering on pointer value
+  itself.
+  Can be used as both Key and UserStruct comparator.
+
+  @param[in] Pointer1  First pointer.
+
+  @param[in] Pointer2  Second pointer.
+
+  @retval <0  If Pointer1 compares less than Pointer2.
+
+  @retval  0  If Pointer1 compares equal to Pointer2.
+
+  @retval >0  If Pointer1 compares greater than Pointer2.
+**/
+STATIC
+INTN
+EFIAPI
+PointerCompare (
+  IN CONST VOID *Pointer1,
+  IN CONST VOID *Pointer2
+  )
+{
+  if (Pointer1 == Pointer2) {
+return 0;
+  } else if ((INTN)Pointer1 < (INTN)Pointer2) {
+return -1;
+  } else {
+return 1;
+  }
+}
+
+
+/**
   Process a QEMU_LOADER_ALLOCATE command.
 
   @param[in] Allocate The QEMU_LOADER_ALLOCATE command to process.
@@ -535,27 +568,32 @@ UndoCmdWritePointer (
   This function assumes that the entire QEMU linker/loader command file has
   been processed successfully in a prior first pass.
 
-  @param[in] AddPointerThe QEMU_LOADER_ADD_POINTER command to process.
+  @param[in] AddPointer  The QEMU_LOADER_ADD_POINTER command to 
process.
 
-  @param[in] Tracker   The ORDERED_COLLECTION tracking the BLOB user
-   structures.
+  @param[in] Tracker The ORDERED_COLLECTION tracking the BLOB user
+ structures.
 
-  @param[in] AcpiProtocol  The ACPI table protocol used to install tables.
+  @param[in] AcpiProtocolThe ACPI table protocol used to install 
tables.
 
-  @param[in,out] InstalledKey  On input, an array of INSTALLED_TABLES_MAX UINTN
-   elements, allocated by the caller. On output,
-   the function will have stored (appended) the
-   AcpiProtocol-internal key of the ACPI table that
-   the function has installed, if the AddPointer
-   command identified an ACPI table that is
-   different from RSDT and XSDT.
+  @param[in,out] InstalledKeyOn input, an array of INSTALLED_TABLES_MAX 
UINTN
+ elements, allocated by the caller. On output,
+ the function will have stored (appended) the
+ AcpiProtocol-internal key of the ACPI table 
that
+ the function has installed, if the AddPointer
+ command identified an ACPI table that is
+ different from RSDT and XSDT.
 
-  @param[in,out] NumInstalled  On input, the number of entries already used in
-   InstalledKey; it must be in [0,
-   INSTALLED_TABLES_MAX] inclusive. On output, the
-   parameter is incremented if the AddPointer
-   command identified an ACPI table that is
-   different from RSDT and XSDT.
+  @param[in,out] NumInstalledOn input, the number of entries already used 
in
+ InstalledKey; it must be in [0,
+ INSTALLED_TABLES_MAX] inclusive. On output, 
the
+ parameter is incremented if the AddPointer
+ command identified an ACPI table that is
+ different from RSDT and XSDT.
+
+  @param[in,out] InstalledTables The ORDERED_COLLECTION tracking the ACPI 
tables
+ which have already been installed. If a new
+ table is encountered by the function, it is
+

[edk2] [PATCH v1 0/1] OvmfPkg/AcpiPlatformDxe: Fix bug 368, multiply pointed-to tables

2017-03-29 Thread Phil Dennis-Jordan

From: Phil Dennis-Jordan 

This fixes the bug in OVMF's Qemu ACPI table linker which caused it to
fail when multiple fields point to the same table. Previously, each
pointer caused the pointed-to table to be installed via the
EFI_ACPI_TABLE_PROTOCOL. However, each table must only be installed once
via this mechanism.

The patch fixes this (as previously suggested by Laszlo) via memoisation
of the pointers. If it encounters the same pointer twice, it will no
longer try to re-install it. I hope I got the implementation details
right. I've tested it successfully with Windows 10, a recent Ubuntu
version, and OS X as guest OSes. (further OVMF patches required for 
booting the latter)

I found this bug while trying to patch Qemu to generate a Rev3 FADT (as
per ACPI 2.0) instead of a Rev1 FADT (ACPI 1.0). Said patch has missed
the Qemu 2.9 merge window, but has been provisionally accepted for 2.10
(save for some minor tweaks) and will break the ACPI table linker
on unpatched OVMF. (When this bug is triggered, OVMF reverts to its
built-in ACPI tables, ignoring those provided by Qemu.)

Previous discussion: 
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg06679.html

Qemu patch: https://lists.gnu.org/archive/html/qemu-devel/2017-03/msg02837.html

Bugzilla: https://bugzilla.tianocore.org/show_bug.cgi?id=368

Github feature branch: https://github.com/pmj/edk2/tree/bug_368_v1

Phil Dennis-Jordan (1):
  OvmfPkg: Allow multiple add-pointer linker commands to same ACPI table

 OvmfPkg/AcpiPlatformDxe/QemuFwCfgAcpi.c | 109 
 1 file changed, 89 insertions(+), 20 deletions(-)

-- 
2.3.2 (Apple Git-55)

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

Re: [edk2] [Patch] BaseTools: Update Pkcs7 and RSA2048 tool with shell=True

2017-03-29 Thread Gao, Liming

Reviewed-by: Liming Gao 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Yonghong Zhu
> Sent: Tuesday, March 28, 2017 5:38 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [Patch] BaseTools: Update Pkcs7 and RSA2048 tool with 
> shell=True
> 
> Pkcs7Sign, Rsa2048Sha256Sign and Rsa2048Sha256GenerateKeys doesn't work
> on Linux. It needs to be changed with shell=True.
> 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Yonghong Zhu 
> ---
>  BaseTools/Source/Python/Pkcs7Sign/Pkcs7Sign.py  | 4 ++--
>  .../Source/Python/Rsa2048Sha256Sign/Rsa2048Sha256GenerateKeys.py| 6 
> +++---
>  BaseTools/Source/Python/Rsa2048Sha256Sign/Rsa2048Sha256Sign.py  | 6 
> +++---
>  3 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Pkcs7Sign/Pkcs7Sign.py 
> b/BaseTools/Source/Python/Pkcs7Sign/Pkcs7Sign.py
> index 6412587..ef79f80 100644
> --- a/BaseTools/Source/Python/Pkcs7Sign/Pkcs7Sign.py
> +++ b/BaseTools/Source/Python/Pkcs7Sign/Pkcs7Sign.py
> @@ -201,11 +201,11 @@ if __name__ == '__main__':
>  FullInputFileBuffer = struct.pack(format, args.InputFileBuffer, 
> args.MonotonicCountValue)
> 
>  #
>  # Sign the input file using the specified private key and capture 
> signature from STDOUT
>  #
> -Process = subprocess.Popen('%s smime -sign -binary -signer "%s" -outform 
> DER -md sha256 -certfile "%s"' % (OpenSslCommand,
> args.SignerPrivateCertFileName, args.OtherPublicCertFileName), 
> stdin=subprocess.PIPE, stdout=subprocess.PIPE,
> stderr=subprocess.PIPE)
> +Process = subprocess.Popen('%s smime -sign -binary -signer "%s" -outform 
> DER -md sha256 -certfile "%s"' % (OpenSslCommand,
> args.SignerPrivateCertFileName, args.OtherPublicCertFileName), 
> stdin=subprocess.PIPE, stdout=subprocess.PIPE,
> stderr=subprocess.PIPE, shell=True)
>  Signature = Process.communicate(input=FullInputFileBuffer)[0]
>  if Process.returncode <> 0:
>sys.exit(Process.returncode)
> 
>  #
> @@ -270,11 +270,11 @@ if __name__ == '__main__':
>  open(args.OutputFileName, 'wb').write(FullInputFileBuffer)
> 
>  #
>  # Verify signature
>  #
> -Process = subprocess.Popen('%s smime -verify -inform DER -content %s 
> -CAfile %s' % (OpenSslCommand, args.OutputFileName,
> args.TrustedPublicCertFileName), stdin=subprocess.PIPE, 
> stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> +Process = subprocess.Popen('%s smime -verify -inform DER -content %s 
> -CAfile %s' % (OpenSslCommand, args.OutputFileName,
> args.TrustedPublicCertFileName), stdin=subprocess.PIPE, 
> stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
>  Process.communicate(input=args.SignatureBuffer)[0]
>  if Process.returncode <> 0:
>print 'ERROR: Verification failed'
>os.remove (args.OutputFileName)
>sys.exit(Process.returncode)
> diff --git 
> a/BaseTools/Source/Python/Rsa2048Sha256Sign/Rsa2048Sha256GenerateKeys.py
> b/BaseTools/Source/Python/Rsa2048Sha256Sign/Rsa2048Sha256GenerateKeys.py
> index 2dd6c20..df2d989 100644
> --- a/BaseTools/Source/Python/Rsa2048Sha256Sign/Rsa2048Sha256GenerateKeys.py
> +++ b/BaseTools/Source/Python/Rsa2048Sha256Sign/Rsa2048Sha256GenerateKeys.py
> @@ -96,11 +96,11 @@ if __name__ == '__main__':
>Item.close()
> 
>#
># Generate private key and save it to output file in a PEM file format
>#
> -  Process = subprocess.Popen('%s genrsa -out %s 2048' % (OpenSslCommand, 
> Item.name), stdout=subprocess.PIPE,
> stderr=subprocess.PIPE)
> +  Process = subprocess.Popen('%s genrsa -out %s 2048' % (OpenSslCommand, 
> Item.name), stdout=subprocess.PIPE,
> stderr=subprocess.PIPE, shell=True)
>Process.communicate()
>if Process.returncode <> 0:
>  print 'ERROR: RSA 2048 key generation failed'
>  sys.exit(Process.returncode)
> 
> @@ -118,11 +118,11 @@ if __name__ == '__main__':
>PublicKeyHash = ''
>for Item in args.PemFileName:
>  #
>  # Extract public key from private key into STDOUT
>  #
> -Process = subprocess.Popen('%s rsa -in %s -modulus -noout' % 
> (OpenSslCommand, Item), stdout=subprocess.PIPE,
> stderr=subprocess.PIPE)
> +Process = subprocess.Popen('%s rsa -in %s -modulus -noout' % 
> (OpenSslCommand, Item), stdout=subprocess.PIPE,
> stderr=subprocess.PIPE, shell=True)
>  PublicKeyHexString = Process.communicate()[0].split('=')[1].strip()
>  if Process.returncode <> 0:
>print 'ERROR: Unable to extract public key from private key'
>sys.exit(Process.returncode)
>  PublicKey = ''
> @@ -130,11 +130,11 @@ if __name__ == '__main__':
>PublicKey = PublicKey + chr(int(PublicKeyHexString[Index:Index + 2], 
> 16))
> 
>  #
>  # Generate SHA 256 hash of RSA 2048 bit

Re: [edk2] [PATCH 1/2] MdeModulePkg/Tcp4Dxe: Fix unconditional window shrinking

2017-03-29 Thread Tian, Feng

Adding network module owners for further review.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
ate...@kraftway.ru
Sent: Tuesday, March 28, 2017 3:20 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH 1/2] MdeModulePkg/Tcp4Dxe: Fix unconditional window 
shrinking

Moving Right window edge to the left on sender side without additional checks 
leads to the situation when sender assumes the receiver shrunk its rcv buffer, 
when, in fact, it only reduced window size. This is a TCP deadlock situation. 
Receiver ACKs proper segment, while sender discards it for future ACK. Add 
check for negative usable window to prevent erroneous window shrinking.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Andrey Tepin 
---
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Input.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Input.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Input.c
index 1000538..ea0766a 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Input.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Input.c
@@ -703,6 +703,7 @@ TcpInput (
   TCP_SEG *Seg;
   TCP_SEQNO   Right;
   TCP_SEQNO   Urg;
+  INT32   UsableWnd;
 
   NET_CHECK_SIGNATURE (Nbuf, NET_BUF_SIGNATURE);
 
@@ -1188,7 +1189,10 @@ TcpInput (
 
   if (TCP_SEQ_LT (Right, Tcb->SndNxt)) {
 
-Tcb->SndNxt = Right;
+UsableWnd = Tcb->SndUna + Tcb->SndWnd - Tcb->SndNxt;
+if (UsableWnd < 0) {
+  Tcb->SndNxt = Right;
+}
 
 if (Right == Tcb->SndUna) {
 
--
1.9.1
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

64 matches

Mail list logo