Re: [edk2] [PATCH v2 1/2] MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage

2018-09-20 Thread Zeng, Star

Jian,

The clarifications are very good.
There is a very superficial comment at below.

On 2018/9/20 14:02, Jian J Wang wrote:

v2 changes:
Newly added patch to clarify PCDs usage.


BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

The usage of following PCDs described in MdeModulePkg.dec don't match
the implementation exactly. This patch updates related description in
both .dec and .uni files to avoid confusion in platform configuration.

   PcdSetNxForStack
   PcdImageProtectionPolicy
   PcdDxeNxMemoryProtectionPolicy

The main change is at the statement on how to handle the FALSE or 0
setting value in those PCDs. Current statement says the implementation
should unset or disable related features but in fact the related code
just do nothing (leave it to AS-IS). That means the result might be
disabled, or might be not. It depends on other features or platform
policy.

For example, if one don't want to enforce NX onto stack memory, he/she
needs to set PcdSetNxForStack to FALSE as well as to clear BIT4 of
PcdDxeNxMemoryProtectionPolicy.

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
  MdeModulePkg/MdeModulePkg.dec | 20 +++-
  MdeModulePkg/MdeModulePkg.uni | 13 +
  2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 74a699cbb7..6566b57fd4 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1288,17 +1288,23 @@
## Set image protection policy. The policy is bitwise.
#  If a bit is set, the image will be protected by DxeCore if it is aligned.
#   The code section becomes read-only, and the data section becomes 
non-executable.
-  #  If a bit is clear, the image will not be protected.
+  #  If a bit is clear, nothing will be done to image code/data 
sections.
#BIT0   - Image from unknown device. 
#BIT1   - Image from firmware volume.
+  #  
+  #  Note: If a bit is cleared, the data section could be still non-executable 
if
+  #  PcdDxeNxMemoryProtectionPolicy is enabled for EfiLoaderData, 
EfiBootServicesData
+  #  and/or EfiRuntimeServicesData.
+  #  
# @Prompt Set image protection policy.
# @ValidRange 0x8002 | 0x - 0x001F

gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x0002|UINT32|0x1047
  
## Set DXE memory protection policy. The policy is bitwise.

#  If a bit is set, memory regions of the associated type will be mapped
-  #  non-executable.
-  #
+  #  non-executable.
+  #  If a bit is cleared, nothing will be done to associated type of 
memory.
+  #  
# Below is bit mask for this PCD: (Order is same as UEFI spec)
#  EfiReservedMemoryType  0x0001
#  EfiLoaderCode  0x0002
@@ -1890,8 +1896,12 @@
#  For the DxeIpl and the DxeCore are both X64, set NX for stack feature also 
require PcdDxeIplBuildPageTables be TRUE.
#  For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode 
is FALSE), set NX for stack feature also require
#  IA32 PAE is supported and Execute Disable Bit is available.
-  #   TRUE  - to set NX for stack.
-  #   FALSE - Not to set NX for stack.
+  #  
+  #  Note: If this PCD is set to FALSE, NX could be still applied to stack due 
to PcdDxeNxMemoryProtectionPolicy enabled for
+  #  EfiBootServicesData.


How about adding this sentence to be after TRUE/FALSE statements below?

Anyway, Reviewed-by: Star Zeng .

Thanks,
Star


+  #  
+  #   TRUE  - Set NX for stack.
+  #   FALSE - Do nothing for stack.
# @Prompt Set NX for stack.
gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f
  
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni

index 080b8a62c0..61befdc1e4 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -345,8 +345,9 @@
"For 
the DxeIpl and the DxeCore are both X64, set NX for stack feature also require 
PcdDxeIplBuildPageTables be TRUE."

"For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), 
set NX for stack feature also require"
"IA32 
PAE is supported and Execute Disable Bit is available."
-  "TRUE  
- to set NX for stack."
-  "FALSE 
- Not to set NX for stack."
+  "Note: 
If this PCD is set to FALSE, NX could be still applied to stack due to 

Re: [edk2] [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position

2018-09-20 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Gao, Liming
> Sent: Friday, September 21, 2018 8:56 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Dong, Eric ;
> Yao, Jiewen 
> Subject: [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry
> function run the same position
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191
> 
> Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp
> _SmiHandler
> is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved
> into rax,
> then call it. But, this code doesn't work in XCODE5 tool chain. Because
> XCODE5
> doesn't generated the absolute address in the EFI image. So, rax stores the
> relative address. Once this logic is moved to another place, it will not work.
> ;   jmp _SmiHandler ; instruction is not needed
> ...
> mov rax, ASM_PFX(CpuSmmDebugEntry)
> callrax
> 
> Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support
> XCODE5.
> One tricky way is selected to fix it. Although SmiEntry logic is copied to
> another place and run, but here jmp _SmiHandler is enabled to jmp the
> original
> code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative
> address.
> mov rax, strict qword 0 ;   mov rax, _SmiHandler
> _SmiHandlerAbsAddr:
> jmp rax
> ...
> callASM_PFX(CpuSmmDebugEntry)
> 
> Now, BZ 1191 raises the issue that SmiHandler should run in the copied
> address,
> can't run in the common address. So, jmp _SmiHandler is required to be
> removed,
> the code is kept to run in copied address. And, the relative address is
> requried to be fixed up to the absolute address. The necessary changes
> should
> not affect the behavior of platforms that already consume
> PiSmmCpuDxeSmm.
> OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has
> been verified.
> ...
> mov rax, strict qword 0 ;   call
> ASM_PFX(CpuSmmDebugEntry)
> CpuSmmDebugEntryAbsAddr:
> callrax
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Liming Gao 
> Cc: Laszlo Ersek 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42
> ++---
>  1 file changed, 33 insertions(+), 9 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> index 315d0f8..815f95b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
> @@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr:
>  mov gs, eax
>  mov ax, [rbx + DSC_SS]
>  mov ss, eax
> -mov rax, strict qword 0 ;   mov rax,
> _SmiHandler
> -_SmiHandlerAbsAddr:
> -jmp rax
> 
>  _SmiHandler:
>  mov rbx, [rsp + 0x8] ; rcx <- CpuIndex
> @@ -189,13 +186,19 @@ _SmiHandler:
>  add rsp, -0x20
> 
>  mov rcx, rbx
> -callASM_PFX(CpuSmmDebugEntry)
> +mov rax, strict qword 0 ;   call
> ASM_PFX(CpuSmmDebugEntry)
> +CpuSmmDebugEntryAbsAddr:
> +callrax
> 
>  mov rcx, rbx
> -callASM_PFX(SmiRendezvous)
> +mov rax, strict qword 0 ;   call
> ASM_PFX(SmiRendezvous)
> +SmiRendezvousAbsAddr:
> +callrax
> 
>  mov rcx, rbx
> -callASM_PFX(CpuSmmDebugExit)
> +mov rax, strict qword 0 ;   call
> ASM_PFX(CpuSmmDebugExit)
> +CpuSmmDebugExitAbsAddr:
> +callrax
> 
>  add rsp, 0x20
> 
> @@ -206,7 +209,8 @@ _SmiHandler:
> 
>  add rsp, 0x200
> 
> -lea rax, [ASM_PFX(mXdSupported)]
> +mov rax, strict qword 0 ;   lea rax,
> [ASM_PFX(mXdSupported)]
> +mXdSupportedAbsAddr:
>  mov al, [rax]
>  cmp al, 0
>  jz  .1
> @@ -224,13 +228,33 @@ _SmiHandler:
> 
>  ASM_PFX(gcSmiHandlerSize)DW  $ - _SmiEntryPoint
> 
> +;
> +; Retrieve the address and fill it into mov opcode.
> +;
> +; It is called in the driver entry point first.
> +; It is used to fix up the real address in mov opcode.
> +; Then, after the code logic is copied to the different location,
> +; the code can also run.
> +;
>  global ASM_PFX(PiSmmCpuSmiEntryFixupAddress)
>  ASM_PFX(PiSmmCpuSmiEntryFixupAddress):
>  learax, [ASM_PFX(gSmiHandlerIdtr)]
>  learcx, [SmiHandlerIdtrAbsAddr]
>  movqword [rcx - 8], rax
> 
> -learax, [_SmiHandler]
> -learcx, [_SmiHandlerAbsAddr]
> +learax, [ASM_PFX(CpuSmmDebugEntry)]
> +learcx, [CpuSmmDebugEntryAbsAddr]
> +movqword [rcx - 8], rax
> +
> +learax, [ASM_PFX(SmiRendezvous)]
> +learcx, [SmiRendezvousAbsAddr]
> +movqword [rcx - 8], rax
> +
> +learax, [ASM_PFX(CpuSmmDebugExit)]
> +learcx, [CpuSmmDebugExitAbsAddr]
> +movqword [rcx - 8], rax
> +
> +learax, [ASM_PFX(mXdSupported)]
> +learcx, [mXdSupportedAbsAddr]
>  movqword [rcx - 8], rax
>  ret
> --
> 

Re: [edk2] [PATCH 2/3] MdeModulePkg: Remove the missing PalLib in DSC file.

2018-09-20 Thread Zeng, Star
As I remember, I raised comment about removing the PalLib in MdeModulePkg.dsc 
at https://lists.01.org/pipermail/edk2-devel/2018-June/026079.html.

I'd like suggest updating the title and commit message a little.

For title: How about "MdeModulePkg: Remove PalLib in dsc which was missed at 
de00522" ?

For commit message: How about like below?

The PalLib is IPF specific and will be removed from MdePkg.
So this patch removes PalLib in MdeModulePkg.dsc which was missed at
de005223b77c473d45c9c8a11147f6968325f73e.

With them accepted, Reviewed-by: Star Zeng .


Thanks,
Star
-Original Message-
From: Chen, Chen A 
Sent: Friday, September 21, 2018 9:00 AM
To: edk2-devel@lists.01.org
Cc: Chen, Chen A ; Zeng, Star ; 
Dong, Eric ; Kinney, Michael D 
Subject: [PATCH 2/3] MdeModulePkg: Remove the missing PalLib in DSC file.

The PalLib will remove in MdePkg, so remove this lib from DSC file.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
---
 MdeModulePkg/MdeModulePkg.dsc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc 
index 8a81ea141f..3ff3b1213c 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -79,7 +79,6 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
   
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
   #
--
2.16.2.windows.1

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


Re: [edk2] [PATCH] MdeModulePkg CapsuleApp:Remove two redundant Guids

2018-09-20 Thread Zeng, Star
Reviewed-by: Star Zeng  and pushed it at 
8c06d18bc112c7b0547641e83a508c6784317f72.

Thanks,
Star
-Original Message-
From: Zhang, Shenglei 
Sent: Thursday, September 20, 2018 4:55 PM
To: edk2-devel@lists.01.org
Cc: Zeng, Star ; Dong, Eric 
Subject: [PATCH] MdeModulePkg CapsuleApp:Remove two redundant Guids

Remove two redundant Guids which are not used.
They are gEfiCertTypeRsa2048Sha256Guid and gEfiCertPkcs7Guid.This is an 
improved version of
https://bugzilla.tianocore.org/show_bug.cgi?id=1062
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
index 3a67c6b909..3e2217ea84 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
@@ -44,8 +44,6 @@
   gEfiCapsuleReportGuid  ## CONSUMES   ## GUID
   gEfiFmpCapsuleGuid ## CONSUMES   ## GUID
   gWindowsUxCapsuleGuid  ## CONSUMES   ## GUID
-  gEfiCertTypeRsa2048Sha256Guid  ## CONSUMES   ## GUID
-  gEfiCertPkcs7Guid  ## CONSUMES   ## GUID
   gEfiSystemResourceTableGuid## CONSUMES   ## GUID
 
 [Protocols]
--
2.18.0.windows.1

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


Re: [edk2] [PATCH v4 0/3] SdMmc fixes

2018-09-20 Thread Wu, Hao A
The series is good to me.
Reviewed-by: Hao Wu 

I will push the changes and update the relating Bugzilla trackers.

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marcin Wojtas
> Sent: Tuesday, September 18, 2018 4:59 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng; t...@semihalf.com; Wu, Hao A; nad...@marvell.com; Gao,
> Liming; Kinney, Michael D
> Subject: [edk2] [PATCH v4 0/3] SdMmc fixes
> 
> Hi,
> 
> The third version has shrunk once again - it turned
> out the bus width / bus mode sequence modification
> in eMMC high speed setting may not be needed.
> Other than that, for remaining two patches add
> bugzilla references and apply required modifications.
> More details can be found in the changelog below.
> 
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/sdmmc-fixes-r20180918
> 
> I'm looking forward to the comments and remarks.
> 
> Best regards,
> Marcin
> 
> Changelog:
> v3 -> v4
>  * rebase on top of the newest master
>  * add Hao Wu RB in 2/2
>  * add references to according bugzilla entries
>  * remove changes in EmmcSwitchToHighSpeed ()
>  * Use BIT0 directly and update error message in 1/2
> 
> v2 -> v3
>  * rebase on top of the newest master
>  * remove changes in EmmcSwitchToHS200 ()
>  * 2/3 - use BIT0 in macro
> 
> v1 -> v2
>  * rebase on top of the newest master
>  * resolve conflicts after taking fixes out from new features
>  * 3/4 - use macro instead of raw value in SdMmcHcReset
> 
> 
> Marcin Wojtas (1):
>   MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot
> 
> Tomasz Michalec (1):
>   MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 18
> --
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  8 
>  2 files changed, 16 insertions(+), 10 deletions(-)
> 
> --
> 2.7.4
> 
> ___
> 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 v1 1/5] MdePkg/BaseLib: Add new LoadFence API

2018-09-20 Thread Wu, Hao A
Hi Ard and Leif,

This commit aims to add to a new BaseLib API to implement the serializing
load operations functionality (for IA32/X64, called LFENCE).

For the 1st version of this commit, this API is named as 'LoadFence'. The
implementation only covers IA32/X64 arch, and does an empty implementation
for ARM/AARCH64.

But as Laszlo pointed out (comment (2) below), the empty implementation
for ARM/AARCH64 may be inappropriate, I would like to turn to you for some
helps or suggestions on the implementation of this API.

Also, as Laszlo pointed out in his comment (3) below, if the
implementations between ARM & AARCH64 are the same (not sure if this is
the case, I am not very familiar with ARM instructions). Do you have
comments or preference on the location of the codes?

A) Duplicate implementations under BaseLib\Arm and BaseLib\AArch64
(Like the IA32 & X64 implementation)
B) Use one common implementation under BaseLib\Arm


Thanks in advance.

Best Regards,
Hao Wu

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Thursday, September 20, 2018 9:13 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Kinney, Michael D; Yao, Jiewen; Gao, Liming
> Subject: Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API
> 
> On 09/20/18 08:40, Hao Wu wrote:
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1193
> >
> > This commit will add a new BaseLib API LoadFence(). This API will perform
> > a serializing operation on all load-from-memory instructions that were
> > issued prior to the call of this function.
> >
> > The purpose of adding this API is to mitigate of the [CVE-2017-5753]
> > Bounds Check Bypass issue when untrusted data are being processed within
> > SMM. More details can be referred at the 'Bounds check bypass mitigation'
> > section at the below link:
> >
> > https://software.intel.com/security-software-guidance/insights/host-
> firmware-speculative-execution-side-channel-mitigation
> >
> > Cc: Ard Biesheuvel 
> > Cc: Laszlo Ersek 
> > Cc: Jiewen Yao 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu 
> > ---
> >  MdePkg/Include/Library/BaseLib.h   | 12 +++
> >  MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 ++
> >  MdePkg/Library/BaseLib/BaseLib.inf |  4 +++
> >  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 +++-
> >  MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37 +++
> >  MdePkg/Library/BaseLib/X64/LoadFence.nasm  | 38
> 
> >  6 files changed, 131 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdePkg/Include/Library/BaseLib.h
> b/MdePkg/Include/Library/BaseLib.h
> > index 123ae19dc2..194726ca35 100644
> > --- a/MdePkg/Include/Library/BaseLib.h
> > +++ b/MdePkg/Include/Library/BaseLib.h
> > @@ -4939,6 +4939,18 @@ MemoryFence (
> >
> >
> >  /**
> > +  Performs a serializing operation on all load-from-memory instructions 
> > that
> > +  were issued prior to the call of this function.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +LoadFence (
> > +  VOID
> > +  );
> > +
> > +
> > +/**
> >Saves the current CPU context that can be restored with a call to 
> > LongJump()
> >and returns 0.
> >
> > diff --git a/MdePkg/Library/BaseLib/Arm/LoadFence.c
> b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > new file mode 100644
> > index 00..69f0c3a07e
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > @@ -0,0 +1,26 @@
> > +/** @file
> > +  LoadFence() function for ARM.
> > +
> > +  Copyright (C) 2018, Intel Corporation. All rights reserved.
> > +
> > +  This program and the accompanying materials are licensed and made
> available
> > +  under the terms and conditions of the BSD License which accompanies this
> > +  distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php.
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS, WITHOUT
> > +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> > +**/
> > +
> > +/**
> > +  Performs a serializing operation on all load-from-memory instructions 
> > that
> > +  were issued prior to the call of this function.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +LoadFence (
> > +  VOID
> > +  )
> > +{
> > +}
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> > index a1b5ec4b75..f028fbc75a 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -68,6 +68,7 @@
> >
> >  [Sources.Ia32]
> >Ia32/WriteTr.nasm
> > +  Ia32/LoadFence.nasm
> >
> >Ia32/Wbinvd.c | MSFT
> >Ia32/WriteMm7.c | MSFT
> > @@ -346,6 +347,7 @@
> >X64/EnableCache.nasm
> >X64/DisableCache.nasm
> >X64/WriteTr.nasm
> > +  X64/LoadFence.nasm
> >
> >X64/CpuBreakpoint.c | MSFT
> >X64/WriteMsr64.c | MSFT
> > @@ -580,6 +582,7 @@
> >  

Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API

2018-09-20 Thread Yao, Jiewen
Thanks Laszlo.

That is very good feedback. For ARM, I think we need use *CSDB*. :-)

Thank you
Yao Jiewen

> -Original Message-
> From: Wu, Hao A
> Sent: Friday, September 21, 2018 10:15 AM
> To: Laszlo Ersek ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Yao, Jiewen
> ; Gao, Liming 
> Subject: RE: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo
> > Ersek
> > Sent: Thursday, September 20, 2018 9:13 PM
> > To: Wu, Hao A; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D; Yao, Jiewen; Gao, Liming
> > Subject: Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence
> API
> >
> > On 09/20/18 08:40, Hao Wu wrote:
> > > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1193
> > >
> > > This commit will add a new BaseLib API LoadFence(). This API will
> perform
> > > a serializing operation on all load-from-memory instructions that were
> > > issued prior to the call of this function.
> > >
> > > The purpose of adding this API is to mitigate of the [CVE-2017-5753]
> > > Bounds Check Bypass issue when untrusted data are being processed
> within
> > > SMM. More details can be referred at the 'Bounds check bypass
> mitigation'
> > > section at the below link:
> > >
> > > https://software.intel.com/security-software-guidance/insights/host-
> > firmware-speculative-execution-side-channel-mitigation
> > >
> > > Cc: Ard Biesheuvel 
> > > Cc: Laszlo Ersek 
> > > Cc: Jiewen Yao 
> > > Cc: Michael D Kinney 
> > > Cc: Liming Gao 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Hao Wu 
> > > ---
> > >  MdePkg/Include/Library/BaseLib.h   | 12 +++
> > >  MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 ++
> > >  MdePkg/Library/BaseLib/BaseLib.inf |  4 +++
> > >  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 +++-
> > >  MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37
> +++
> > >  MdePkg/Library/BaseLib/X64/LoadFence.nasm  | 38
> > 
> > >  6 files changed, 131 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdePkg/Include/Library/BaseLib.h
> > b/MdePkg/Include/Library/BaseLib.h
> > > index 123ae19dc2..194726ca35 100644
> > > --- a/MdePkg/Include/Library/BaseLib.h
> > > +++ b/MdePkg/Include/Library/BaseLib.h
> > > @@ -4939,6 +4939,18 @@ MemoryFence (
> > >
> > >
> > >  /**
> > > +  Performs a serializing operation on all load-from-memory
> instructions that
> > > +  were issued prior to the call of this function.
> > > +
> > > +**/
> > > +VOID
> > > +EFIAPI
> > > +LoadFence (
> > > +  VOID
> > > +  );
> > > +
> > > +
> > > +/**
> > >Saves the current CPU context that can be restored with a call to
> LongJump()
> > >and returns 0.
> > >
> > > diff --git a/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > > new file mode 100644
> > > index 00..69f0c3a07e
> > > --- /dev/null
> > > +++ b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > > @@ -0,0 +1,26 @@
> > > +/** @file
> > > +  LoadFence() function for ARM.
> > > +
> > > +  Copyright (C) 2018, Intel Corporation. All rights reserved.
> > > +
> > > +  This program and the accompanying materials are licensed and made
> > available
> > > +  under the terms and conditions of the BSD License which
> accompanies this
> > > +  distribution.  The full text of the license may be found at
> > > +  http://opensource.org/licenses/bsd-license.php.
> > > +
> > > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> IS"
> > BASIS, WITHOUT
> > > +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR
> > IMPLIED.
> > > +**/
> > > +
> > > +/**
> > > +  Performs a serializing operation on all load-from-memory
> instructions that
> > > +  were issued prior to the call of this function.
> > > +
> > > +**/
> > > +VOID
> > > +EFIAPI
> > > +LoadFence (
> > > +  VOID
> > > +  )
> > > +{
> > > +}
> > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> > b/MdePkg/Library/BaseLib/BaseLib.inf
> > > index a1b5ec4b75..f028fbc75a 100644
> > > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > > @@ -68,6 +68,7 @@
> > >
> > >  [Sources.Ia32]
> > >Ia32/WriteTr.nasm
> > > +  Ia32/LoadFence.nasm
> > >
> > >Ia32/Wbinvd.c | MSFT
> > >Ia32/WriteMm7.c | MSFT
> > > @@ -346,6 +347,7 @@
> > >X64/EnableCache.nasm
> > >X64/DisableCache.nasm
> > >X64/WriteTr.nasm
> > > +  X64/LoadFence.nasm
> > >
> > >X64/CpuBreakpoint.c | MSFT
> > >X64/WriteMsr64.c | MSFT
> > > @@ -580,6 +582,7 @@
> > >  [Sources.ARM]
> > >Arm/InternalSwitchStack.c
> > >Arm/Unaligned.c
> > > +  Arm/LoadFence.c
> > >Math64.c   | RVCT
> > >Math64.c   | MSFT
> > >
> > > @@ -613,6 +616,7 @@
> > >  [Sources.AARCH64]
> > >Arm/InternalSwitchStack.c
> > >Arm/Unaligned.c
> > > +  Arm/LoadFence.c
> > >Math64.c
> > 

Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API

2018-09-20 Thread Wu, Hao A
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo
> Ersek
> Sent: Thursday, September 20, 2018 9:13 PM
> To: Wu, Hao A; edk2-devel@lists.01.org
> Cc: Kinney, Michael D; Yao, Jiewen; Gao, Liming
> Subject: Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API
> 
> On 09/20/18 08:40, Hao Wu wrote:
> > REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1193
> >
> > This commit will add a new BaseLib API LoadFence(). This API will perform
> > a serializing operation on all load-from-memory instructions that were
> > issued prior to the call of this function.
> >
> > The purpose of adding this API is to mitigate of the [CVE-2017-5753]
> > Bounds Check Bypass issue when untrusted data are being processed within
> > SMM. More details can be referred at the 'Bounds check bypass mitigation'
> > section at the below link:
> >
> > https://software.intel.com/security-software-guidance/insights/host-
> firmware-speculative-execution-side-channel-mitigation
> >
> > Cc: Ard Biesheuvel 
> > Cc: Laszlo Ersek 
> > Cc: Jiewen Yao 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Hao Wu 
> > ---
> >  MdePkg/Include/Library/BaseLib.h   | 12 +++
> >  MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 ++
> >  MdePkg/Library/BaseLib/BaseLib.inf |  4 +++
> >  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 +++-
> >  MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37 +++
> >  MdePkg/Library/BaseLib/X64/LoadFence.nasm  | 38
> 
> >  6 files changed, 131 insertions(+), 1 deletion(-)
> >
> > diff --git a/MdePkg/Include/Library/BaseLib.h
> b/MdePkg/Include/Library/BaseLib.h
> > index 123ae19dc2..194726ca35 100644
> > --- a/MdePkg/Include/Library/BaseLib.h
> > +++ b/MdePkg/Include/Library/BaseLib.h
> > @@ -4939,6 +4939,18 @@ MemoryFence (
> >
> >
> >  /**
> > +  Performs a serializing operation on all load-from-memory instructions 
> > that
> > +  were issued prior to the call of this function.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +LoadFence (
> > +  VOID
> > +  );
> > +
> > +
> > +/**
> >Saves the current CPU context that can be restored with a call to 
> > LongJump()
> >and returns 0.
> >
> > diff --git a/MdePkg/Library/BaseLib/Arm/LoadFence.c
> b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > new file mode 100644
> > index 00..69f0c3a07e
> > --- /dev/null
> > +++ b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> > @@ -0,0 +1,26 @@
> > +/** @file
> > +  LoadFence() function for ARM.
> > +
> > +  Copyright (C) 2018, Intel Corporation. All rights reserved.
> > +
> > +  This program and the accompanying materials are licensed and made
> available
> > +  under the terms and conditions of the BSD License which accompanies this
> > +  distribution.  The full text of the license may be found at
> > +  http://opensource.org/licenses/bsd-license.php.
> > +
> > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS, WITHOUT
> > +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR
> IMPLIED.
> > +**/
> > +
> > +/**
> > +  Performs a serializing operation on all load-from-memory instructions 
> > that
> > +  were issued prior to the call of this function.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +LoadFence (
> > +  VOID
> > +  )
> > +{
> > +}
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> > index a1b5ec4b75..f028fbc75a 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -68,6 +68,7 @@
> >
> >  [Sources.Ia32]
> >Ia32/WriteTr.nasm
> > +  Ia32/LoadFence.nasm
> >
> >Ia32/Wbinvd.c | MSFT
> >Ia32/WriteMm7.c | MSFT
> > @@ -346,6 +347,7 @@
> >X64/EnableCache.nasm
> >X64/DisableCache.nasm
> >X64/WriteTr.nasm
> > +  X64/LoadFence.nasm
> >
> >X64/CpuBreakpoint.c | MSFT
> >X64/WriteMsr64.c | MSFT
> > @@ -580,6 +582,7 @@
> >  [Sources.ARM]
> >Arm/InternalSwitchStack.c
> >Arm/Unaligned.c
> > +  Arm/LoadFence.c
> >Math64.c   | RVCT
> >Math64.c   | MSFT
> >
> > @@ -613,6 +616,7 @@
> >  [Sources.AARCH64]
> >Arm/InternalSwitchStack.c
> >Arm/Unaligned.c
> > +  Arm/LoadFence.c
> >Math64.c
> >
> >AArch64/MemoryFence.S | GCC
> > diff --git a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> > index 9b7d875664..a79461cfbf 100644
> > --- a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> > +++ b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >Base Library CPU Functions for EBC
> >
> > -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
> > +  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> >This program and the accompanying materials
> >are licensed and made available under the terms and conditions of 

Re: [edk2] [PATCH v1 0/5] [CVE-2017-5753] Bounds Check Bypass issue in SMI handlers

2018-09-20 Thread Wu, Hao A
Hi Mike,

We found that this API needs to be inserted within file:
MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c

which is in module:
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf

This module (INF file) is consumed by the AARCH64/ARM architectures as
well. That is the reason I do not make this API as IA32/X64 specific.

Best Regards,
Hao Wu


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Kinney, Michael D
> Sent: Thursday, September 20, 2018 9:59 PM
> To: Wu, Hao A; edk2-devel@lists.01.org; Kinney, Michael D
> Cc: Dong, Eric; Gao, Liming; Yao, Jiewen; Laszlo Ersek; Zeng, Star
> Subject: Re: [edk2] [PATCH v1 0/5] [CVE-2017-5753] Bounds Check Bypass issue
> in SMI handlers
> 
> Hao Wu,
> 
> I see that implementations of this API are only
> provided for IA32 and X64.  Should this be an IA32/X64
> specific API in BaseLib?  Also, since the API is providing
> a C callable function to execute a specific IA32/X64
> instruction, should the API be prefixed with Asm to
> match the convention of other APIs in BaseLib?
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Wu, Hao A
> > Sent: Wednesday, September 19, 2018 11:41 PM
> > To: edk2-devel@lists.01.org
> > Cc: Wu, Hao A ; Ard Biesheuvel
> > ; Laszlo Ersek
> > ; Yao, Jiewen
> > ; Kinney, Michael D
> > ; Gao, Liming
> > ; Zeng, Star
> > ; Dong, Eric 
> > Subject: [PATCH v1 0/5] [CVE-2017-5753] Bounds Check
> > Bypass issue in SMI handlers
> >
> > The series aims to mitigate the Bounds Check Bypass
> > (CVE-2017-5753) issues
> > within SMI handlers.
> >
> > A more detailed explanation of the purpose of the
> > series is under the
> > 'Bounds check bypass mitigation' section of the below
> > link:
> > https://software.intel.com/security-software-
> > guidance/insights/host-firmware-speculative-execution-
> > side-channel-mitigation
> >
> > And the document at:
> > https://software.intel.com/security-software-
> > guidance/api-app/sites/default/files/337879-analyzing-
> > potential-bounds-Check-bypass-vulnerabilities.pdf
> >
> > Cc: Ard Biesheuvel 
> > Cc: Laszlo Ersek 
> > Cc: Jiewen Yao 
> > Cc: Michael D Kinney 
> > Cc: Liming Gao 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> >
> > Hao Wu (5):
> >   MdePkg/BaseLib: Add new LoadFence API
> >   MdeModulePkg/FaultTolerantWrite:[CVE-2017-5753]Fix
> > bounds check bypass
> >   MdeModulePkg/SmmLockBox: [CVE-2017-5753] Fix bounds
> > check bypass
> >   MdeModulePkg/Variable: [CVE-2017-5753] Fix bounds
> > check bypass
> >   UefiCpuPkg/PiSmmCpuDxeSmm: [CVE-2017-5753] Fix bounds
> > check bypass
> >
> >
> > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultToler
> > antWriteSmm.c   |  2 ++
> >
> > MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultToler
> > antWriteSmm.inf |  1 +
> >  MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> > |  2 ++
> >  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> > |  1 +
> >
> > MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.
> > c   |  3 ++
> >  MdePkg/Include/Library/BaseLib.h
> > | 12 +++
> >  MdePkg/Library/BaseLib/Arm/LoadFence.c
> > | 26 ++
> >  MdePkg/Library/BaseLib/BaseLib.inf
> > |  4 +++
> >  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> > | 15 +++-
> >  MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
> > | 37 +++
> >  MdePkg/Library/BaseLib/X64/LoadFence.nasm
> > | 38 
> >  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> > |  1 +
> >  12 files changed, 141 insertions(+), 1 deletion(-)
> >  create mode 100644
> > MdePkg/Library/BaseLib/Arm/LoadFence.c
> >  create mode 100644
> > MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
> >  create mode 100644
> > MdePkg/Library/BaseLib/X64/LoadFence.nasm
> >
> > --
> > 2.12.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 3/3] SignedCapsulePkg: Remove the missing PalLib in DSC file.

2018-09-20 Thread Yao, Jiewen
Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Chen, Chen A
> Sent: Friday, September 21, 2018 9:00 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A ; Yao, Jiewen
> ; Kinney, Michael D 
> Subject: [PATCH 3/3] SignedCapsulePkg: Remove the missing PalLib in DSC
> file.
> 
> The PalLib will remove in MdePkg, so remove this lib from DSC file.
> 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Chen A Chen 
> ---
>  SignedCapsulePkg/SignedCapsulePkg.dsc | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/SignedCapsulePkg/SignedCapsulePkg.dsc
> b/SignedCapsulePkg/SignedCapsulePkg.dsc
> index 7ea74d7346..db7f176166 100644
> --- a/SignedCapsulePkg/SignedCapsulePkg.dsc
> +++ b/SignedCapsulePkg/SignedCapsulePkg.dsc
> @@ -77,7 +77,6 @@
> 
> SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.in
> f
> 
> CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.in
> f
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> -  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
> 
> CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/Custo
> mizedDisplayLib.inf
>#
># Misc
> --
> 2.16.2.windows.1

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


[edk2] [PATCH 3/3] SignedCapsulePkg: Remove the missing PalLib in DSC file.

2018-09-20 Thread Chen A Chen
The PalLib will remove in MdePkg, so remove this lib from DSC file.

Cc: Jiewen Yao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
---
 SignedCapsulePkg/SignedCapsulePkg.dsc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/SignedCapsulePkg/SignedCapsulePkg.dsc 
b/SignedCapsulePkg/SignedCapsulePkg.dsc
index 7ea74d7346..db7f176166 100644
--- a/SignedCapsulePkg/SignedCapsulePkg.dsc
+++ b/SignedCapsulePkg/SignedCapsulePkg.dsc
@@ -77,7 +77,6 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
   
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   #
   # Misc
-- 
2.16.2.windows.1

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


[edk2] [PATCH 2/3] MdeModulePkg: Remove the missing PalLib in DSC file.

2018-09-20 Thread Chen A Chen
The PalLib will remove in MdePkg, so remove this lib from DSC file.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
---
 MdeModulePkg/MdeModulePkg.dsc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index 8a81ea141f..3ff3b1213c 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -79,7 +79,6 @@
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
-  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
   
CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
   
FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLib.inf
   #
-- 
2.16.2.windows.1

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


[edk2] [PATCH 1/3] IntelFrameworkModulePkg: Remove the missing PalLib in DSC file.

2018-09-20 Thread Chen A Chen
The PalLib will remove in MdePkg, so remove this lib from DSC file.

Cc: Liming Gao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chen A Chen 
---
 IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc 
b/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
index 84e1d890b5..894c5340a0 100644
--- a/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
+++ b/IntelFrameworkModulePkg/IntelFrameworkModulePkg.dsc
@@ -75,7 +75,6 @@
   
DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
   UefiRuntimeLib|MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-  PalLib|MdePkg/Library/BasePalLibNull/BasePalLibNull.inf
 
 [LibraryClasses.common.PEIM]
   HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
-- 
2.16.2.windows.1

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


[edk2] [PATCH v2] UefiCpuPkg PiSmmCpuDxeSmm: Update SmiEntry function run the same position

2018-09-20 Thread Liming Gao
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=1191

Before commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89, jmp _SmiHandler
is commented. And below code, ASM_PFX(CpuSmmDebugEntry) is moved into rax,
then call it. But, this code doesn't work in XCODE5 tool chain. Because XCODE5
doesn't generated the absolute address in the EFI image. So, rax stores the
relative address. Once this logic is moved to another place, it will not work.
;   jmp _SmiHandler ; instruction is not needed
...
mov rax, ASM_PFX(CpuSmmDebugEntry)
callrax

Commit e21e355e2ca7fefb15b4df7078f995d3fb9c2b89 is to support XCODE5.
One tricky way is selected to fix it. Although SmiEntry logic is copied to
another place and run, but here jmp _SmiHandler is enabled to jmp the original
code place, then call ASM_PFX(CpuSmmDebugEntry) with the relative address.
mov rax, strict qword 0 ;   mov rax, _SmiHandler
_SmiHandlerAbsAddr:
jmp rax
...
callASM_PFX(CpuSmmDebugEntry)

Now, BZ 1191 raises the issue that SmiHandler should run in the copied address,
can't run in the common address. So, jmp _SmiHandler is required to be removed,
the code is kept to run in copied address. And, the relative address is
requried to be fixed up to the absolute address. The necessary changes should
not affect the behavior of platforms that already consume PiSmmCpuDxeSmm.
OVMF SMM boot to shell with VS2017, GCC5 and XCODE5 tool chain has been 
verified.
...
mov rax, strict qword 0 ;   callASM_PFX(CpuSmmDebugEntry)
CpuSmmDebugEntryAbsAddr:
callrax

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
Cc: Laszlo Ersek 
Cc: Eric Dong 
Cc: Jiewen Yao 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm | 42 ++---
 1 file changed, 33 insertions(+), 9 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm 
b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
index 315d0f8..815f95b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm
@@ -173,9 +173,6 @@ SmiHandlerIdtrAbsAddr:
 mov gs, eax
 mov ax, [rbx + DSC_SS]
 mov ss, eax
-mov rax, strict qword 0 ;   mov rax, _SmiHandler
-_SmiHandlerAbsAddr:
-jmp rax
 
 _SmiHandler:
 mov rbx, [rsp + 0x8] ; rcx <- CpuIndex
@@ -189,13 +186,19 @@ _SmiHandler:
 add rsp, -0x20
 
 mov rcx, rbx
-callASM_PFX(CpuSmmDebugEntry)
+mov rax, strict qword 0 ;   callASM_PFX(CpuSmmDebugEntry)
+CpuSmmDebugEntryAbsAddr:
+callrax
 
 mov rcx, rbx
-callASM_PFX(SmiRendezvous)
+mov rax, strict qword 0 ;   callASM_PFX(SmiRendezvous)
+SmiRendezvousAbsAddr:
+callrax
 
 mov rcx, rbx
-callASM_PFX(CpuSmmDebugExit)
+mov rax, strict qword 0 ;   callASM_PFX(CpuSmmDebugExit)
+CpuSmmDebugExitAbsAddr:
+callrax
 
 add rsp, 0x20
 
@@ -206,7 +209,8 @@ _SmiHandler:
 
 add rsp, 0x200
 
-lea rax, [ASM_PFX(mXdSupported)]
+mov rax, strict qword 0 ;   lea rax, 
[ASM_PFX(mXdSupported)]
+mXdSupportedAbsAddr:
 mov al, [rax]
 cmp al, 0
 jz  .1
@@ -224,13 +228,33 @@ _SmiHandler:
 
 ASM_PFX(gcSmiHandlerSize)DW  $ - _SmiEntryPoint
 
+;
+; Retrieve the address and fill it into mov opcode.
+;
+; It is called in the driver entry point first.
+; It is used to fix up the real address in mov opcode.
+; Then, after the code logic is copied to the different location, 
+; the code can also run.
+;
 global ASM_PFX(PiSmmCpuSmiEntryFixupAddress)
 ASM_PFX(PiSmmCpuSmiEntryFixupAddress):
 learax, [ASM_PFX(gSmiHandlerIdtr)]
 learcx, [SmiHandlerIdtrAbsAddr]
 movqword [rcx - 8], rax
 
-learax, [_SmiHandler]
-learcx, [_SmiHandlerAbsAddr]
+learax, [ASM_PFX(CpuSmmDebugEntry)]
+learcx, [CpuSmmDebugEntryAbsAddr]
+movqword [rcx - 8], rax
+
+learax, [ASM_PFX(SmiRendezvous)]
+learcx, [SmiRendezvousAbsAddr]
+movqword [rcx - 8], rax
+
+learax, [ASM_PFX(CpuSmmDebugExit)]
+learcx, [CpuSmmDebugExitAbsAddr]
+movqword [rcx - 8], rax
+
+learax, [ASM_PFX(mXdSupported)]
+learcx, [mXdSupportedAbsAddr]
 movqword [rcx - 8], rax
 ret
-- 
2.10.0.windows.1

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


Re: [edk2] question about uefi shell-current working directory and shell-script

2018-09-20 Thread krishnaLee
Carsey,
  The shell may not have a file system assigned,
yes but when it find the stript(startup.nsh) file,it has the default file 
system-->the startup.nsh's file system,
so the default cwd should exist,I think it is strange that start doing script 
without a default cwd,


currently, without a default cwd,some people have to write ugly script to make 
a default cwd like this:
if exist fs0:\mytool.efi then
fs0:
goto work_label
if exist fs1:\mytool.efi then
fs1:
goto work_label
if exist fs2:\mytool.efi then
fs2:
goto work_label
if exist fs3:\mytool.efi then
fs3:
goto work_label
...
















At 2018-09-20 22:21:58, "Carsey, Jaben"  wrote:
>When the shell starts, it the prompt "shell>" or something like "fs0:>"... The 
>shell may not have a file system assigned yet so you cannot change directories 
>until you pick a file system.
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> krishnaLee
>> Sent: Thursday, September 20, 2018 2:22 AM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] question about uefi shell-current working directory and 
>> shell-
>> script
>> Importance: High
>> 
>> Hi,
>> I wonder if it is a bug:
>> I have a usb Fat32-disk,installed with EDK2 shell2.6 or 2.7,
>> the root directory has a startup.nsh,the startup.nsh has only one line:"cd 
>> \",
>> I boot my cannolake machine with this usb disk,
>> in the uefi shell,I got a message:"cd: current directory not specified".it 
>> means
>> the cwd environment variable was not set when start doing script.
>> 
>> 
>> I think it may be  an error,because the uefi shell can find the 
>> startup.nsh,why
>> not set cwd(current working directory) before execute this script,so the cd
>> command works fine.
>> I mention it because I found if I put a uefi application(using 
>> efi_shell_protocol
>> to access .\logfile.txt) in this startup.nsh,the uefi application can't 
>> access files,
>>  because I found the efi_shell_protocol need cwd to work too,such as
>> efi_shell_protocol.OpenFileByName().
>> 
>> 
>> 
>> 
>> 
>> 
>> thank you,
>> krishna
>> 
>> 
>> 
>> ___
>> 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] SPI Flash Corruption

2018-09-20 Thread Yao, Jiewen
thank you, Samah. 
Would you please file a tracker in edkii bugzilla ?

The term VPD might lead confusion here. 
Ideally VPD region is independent with UEFI variable region. It is a special 
region to hold PCD with VPD type. 
I just look at the code. The open source minnowmax puts variable region in the 
VPD region. As such there is discussion about variable atomicity. But the 
variable atomicity cannot guarantee the integrity of FV header. Additional 
check need to be done in platform  FVB driver. 

If you can add a detailed reproducing step in the bugzilla, it will be helpful 
for us to understand the problem. 

thank you!
Yao, Jiewen


> 在 2018年9月20日,下午11:47,Samah Mansour  写道:
> 
> Hi Laszlo,
> Thanks for your reply.
> Actually what I see is that VPD (Vital Product Area between addresses
> 44000->47DFF0  ) is completely wiped which causes the failure to boot!
> Without the VPD unit cannot boot.
> I will take a look at the white paper.
> It would be helpful to know what's the impact of disabling the ability of
> the firmware to write those non volatile variables to flash.
> 
> Samah
> 
> 
>> On Thu, Sep 20, 2018 at 9:48 AM Laszlo Ersek  wrote:
>> 
>>> On 09/19/18 16:26, Samah Mansour wrote:
>>> Hello,
>>> 
>>> 
>>> Our product uses a Baytrail with Minnowboard Max bios firmware ( version
>>> 0.93). Every now and then we see SPI flash corruption due to power cuts
>>> while the unit is booting which causes the unit not to boot anymore.
>> After
>>> investigation we noticed that the VPD area is all FFs (address
>>> 44000->47DFF0).
>>> 
>>> 
>>> We have noticed that the Bios while booting writes to the flash from
>>> several places in the code, which is if interrupted most probably is
>>> causing the corruption.
>>> 
>>> 
>>> Why is the bios writing all these configurations to flash while booting,
>> is
>>> it to optimize boot time? is it ok if we disable the bios writing to
>> flash
>>> completely to protect ourselves from corruption?
>> 
>> The firmware is at liberty to write various non-volatile UEFI variables
>> during boot. Some of those variables are standardized, some others may
>> be specific to UEFI drivers (with correspondingly private namespace
>> GUIDs for the variables).
>> 
>> Power loss during flash write (and resultant flash corruption) is
>> expected. My understanding is that the Fault Tolerant Write protocol /
>> driver, sitting between the FVB (firmware volume block, i.e. flash)
>> protocol / driver, and the variable write protocol / driver, implements
>> a kind of journaling. It is described in the Intel whitepaper
>> 
>>  A Tour Beyond BIOS
>>  Implementing UEFI Authenticated Variables in SMM with EDKII
>>  September 2015
>> 
>> My expectation has been that the platform should recover from
>> interrupted writes. That is, for a single given UEFI variable, you
>> should either see "before" or "after" status, never "middle". (The
>> whitepaper says that "Individual variable atomicity" is maintained even
>> through a failed "reclaim", with the help of FTW.)
>> 
>> If multiple variables should be in sync with each other, that's a
>> different question. If the variables are not in sync, I think "failure
>> to boot" may be a reasonable outcome. But, "failure to boot" means a lot
>> of things, and I hope one should be at least dropped to the setup
>> utility or the shell. Are you seeing an actual crash?
>> 
>> Laszlo
>> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/3] IntelFsp2Pkg: Fix typo in SplitFspBin

2018-09-20 Thread Chiu, Chasel

Reviewed-by: Chasel Chiu 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Desimone, Nathaniel L
Sent: Tuesday, September 18, 2018 11:04 PM
To: Patrick Georgi ; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 2/3] IntelFsp2Pkg: Fix typo in SplitFspBin

Reviewed-by: Nate DeSimone 

On 9/18/18, 6:32 AM, "edk2-devel on behalf of Patrick Georgi" 
 wrote:

Signed-off-by: Patrick Georgi 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 IntelFsp2Pkg/Tools/SplitFspBin.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/IntelFsp2Pkg/Tools/SplitFspBin.py 
b/IntelFsp2Pkg/Tools/SplitFspBin.py
index bd5507c2fa..ac864492e8 100755
--- a/IntelFsp2Pkg/Tools/SplitFspBin.py
+++ b/IntelFsp2Pkg/Tools/SplitFspBin.py
@@ -726,7 +726,7 @@ def SplitFspBin (fspfile, outdir, nametemplate):
 fspname, ext = os.path.splitext(os.path.basename(nametemplate))
 filename = os.path.join(outdir, fspname + '_' + fsp.Type + ext)
 hfsp = open(filename, 'wb')
-print ("Ceate FSP component file '%s'" % filename)
+print ("Create FSP component file '%s'" % filename)
 for fvidx in fsp.FvIdxList:
 fv = fd.FvList[fvidx]
 hfsp.write(fv.FvData)
-- 
2.19.0.397.gdd90340f6a-goog

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


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


[edk2] [PATCH v3 6/7] MdePkg/UefiBaseType.h: treat EBC as a non-native machine type

2018-09-20 Thread Ard Biesheuvel
Instead of classifying EBC as a supported machine type and have special
handling in DXE core for loading EBC images, make it a foreign type and
rely on the EDK2 PE/COFF image emulator protocol to claim the image when
the DXE core finds that it cannot be supported natively.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Include/Uefi/UefiBaseType.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Include/Uefi/UefiBaseType.h 
b/MdePkg/Include/Uefi/UefiBaseType.h
index 401db7f620b3..e52121809deb 100644
--- a/MdePkg/Include/Uefi/UefiBaseType.h
+++ b/MdePkg/Include/Uefi/UefiBaseType.h
@@ -250,21 +250,21 @@ typedef union {
 #if   defined (MDE_CPU_IA32)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_IA32) || ((Machine) == 
EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_IA32)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) ((Machine) == 
EFI_IMAGE_MACHINE_X64)
 
 #elif defined (MDE_CPU_IPF)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_IA64) || ((Machine) == 
EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_IA64)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) (FALSE)
 
 #elif defined (MDE_CPU_X64)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_X64) || ((Machine) == 
EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_X64)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) ((Machine) == 
EFI_IMAGE_MACHINE_IA32)
 
@@ -277,7 +277,7 @@ typedef union {
 #elif defined (MDE_CPU_AARCH64)
 
 #define EFI_IMAGE_MACHINE_TYPE_SUPPORTED(Machine) \
-  (((Machine) == EFI_IMAGE_MACHINE_AARCH64) || ((Machine) == 
EFI_IMAGE_MACHINE_EBC))
+  ((Machine) == EFI_IMAGE_MACHINE_AARCH64)
 
 #define EFI_IMAGE_MACHINE_CROSS_TYPE_SUPPORTED(Machine) (FALSE)
 
-- 
2.17.1

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


[edk2] [PATCH v3 3/7] MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option ROMs

2018-09-20 Thread Ard Biesheuvel
When enumerating option ROM images, take into account whether an emulator
exists that would allow dispatch of PE/COFF images built for foreign
architectures.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h  |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c | 53 +++-
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 55eb3a5a8070..dc57d4876c0f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -33,6 +33,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index a21dd2b5edf4..c8b861093292 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -96,6 +96,7 @@
   gEfiIncompatiblePciDeviceSupportProtocolGuid## SOMETIMES_CONSUMES
   gEfiLoadFile2ProtocolGuid   ## SOMETIMES_PRODUCES
   gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiLoadedImageDevicePathProtocolGuid   ## CONSUMES
 
 [FeaturePcd]
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
index c2be85a906af..085bd5d571bd 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciOptionRomSupport.c
@@ -651,6 +651,55 @@ RomDecode (
   }
 }
 
+STATIC
+BOOLEAN
+IsImageTypeSupported (
+  IN  UINT16MachineType,
+  IN  UINT16SubSystem,
+  IN  EFI_DEVICE_PATH_PROTOCOL  *DevicePath
+  )
+{
+  EFI_STATUSStatus;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
+  UINTN HandleCount;
+  EFI_HANDLE*HandleBuffer;
+  BOOLEAN   ReturnValue;
+  UINTN Index;
+
+  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType)) {
+return TRUE;
+  }
+
+  Status = gBS->LocateHandleBuffer (
+  ByProtocol,
+  ,
+  NULL,
+  ,
+  
+  );
+  if (EFI_ERROR (Status)) {
+return FALSE;
+  }
+
+  ReturnValue = FALSE;
+  for (Index = 0; Index < HandleCount; Index++) {
+Status = gBS->HandleProtocol (
+HandleBuffer[Index],
+,
+(VOID **)
+);
+ASSERT_EFI_ERROR (Status);
+
+if (Emu->IsImageSupported (Emu, MachineType, SubSystem, DevicePath)) {
+  ReturnValue = TRUE;
+  break;
+}
+  }
+
+  FreePool (HandleBuffer);
+  return ReturnValue;
+}
+
 /**
   Load and start the Option Rom image.
 
@@ -715,7 +764,9 @@ ProcessOpRomImage (
 //
 // Skip the EFI PCI Option ROM image if its machine type is not supported
 //
-if (!EFI_IMAGE_MACHINE_TYPE_SUPPORTED (EfiRomHeader->EfiMachineType)) {
+if (!IsImageTypeSupported(EfiRomHeader->EfiMachineType,
+  EfiRomHeader->EfiSubsystem,
+  PciDevice->DevicePath)) {
   goto NextImage;
 }
 
-- 
2.17.1

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


[edk2] [PATCH v3 5/7] MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol

2018-09-20 Thread Ard Biesheuvel
Implement the new EDK2 PE/COFF image emulator protocol so that we can
remove the EBC specific handling in the DXE core and other places in
the core code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Universal/EbcDxe/EbcDxe.inf |   3 +
 MdeModulePkg/Universal/EbcDxe/EbcInt.c   | 127 
 MdeModulePkg/Universal/EbcDxe/EbcInt.h   |   3 +
 3 files changed, 133 insertions(+)

diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf 
b/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
index 8f128b121d0b..9cba1d0a917b 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
@@ -62,7 +62,9 @@
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
+  CacheMaintenanceLib
   MemoryAllocationLib
+  PeCoffLib
   UefiBootServicesTableLib
   BaseMemoryLib
   UefiDriverEntryPoint
@@ -73,6 +75,7 @@
 [Protocols]
   gEfiDebugSupportProtocolGuid  ## PRODUCES
   gEfiEbcProtocolGuid   ## PRODUCES
+  gEdkiiPeCoffImageEmulatorProtocolGuid ## PRODUCES
   gEfiEbcVmTestProtocolGuid ## SOMETIMES_PRODUCES
   gEfiEbcSimpleDebuggerProtocolGuid ## SOMETIMES_CONSUMES
 
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcInt.c 
b/MdeModulePkg/Universal/EbcDxe/EbcInt.c
index 727ba8bcae44..45e5da1823e7 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcInt.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcInt.c
@@ -349,6 +349,123 @@ UINTN  mStackNum = 0;
 EFI_EVENT  mEbcPeriodicEvent;
 VM_CONTEXT *mVmPtr = NULL;
 
+/**
+  Check whether the emulator supports executing a certain PE/COFF image
+
+  @param[in] This This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+  structure
+  @param[in] MachineType  The machine type for which the image was built
+  @param[in] ImageTypeWhether the image is an application, a boot time
+  driver or a runtime driver.
+  @param[in] DevicePath   Path to device where the image originated
+  (e.g., a PCI option ROM)
+
+  @retval TRUEThe image is supported by the emulator
+  @retval FALSE   The image is not supported by the emulator.
+**/
+BOOLEAN
+EFIAPI
+EbcIsImageSupported (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+  IN  UINT16  MachineType,
+  IN  UINT16  ImageType,
+  IN  EFI_DEVICE_PATH_PROTOCOL*DevicePath   OPTIONAL
+  )
+{
+  if (MachineType != EFI_IMAGE_MACHINE_EBC) {
+return FALSE;
+  }
+
+  if (ImageType != EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION &&
+  ImageType != EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER) {
+return FALSE;
+  }
+  return TRUE;
+}
+
+/**
+  Register a supported PE/COFF image with the emulator. After this call
+  completes successfully, the PE/COFF image may be started as usual, and
+  it is the responsibility of the emulator implementation that any branch
+  into the code section of the image (including returns from functions called
+  from the foreign code) is executed as if it were running on the machine
+  type it was built for.
+
+  @param[in]  This  This pointer for
+EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL structure
+  @param[in]  ImageBase The base address in memory of the PE/COFF image
+  @param[in]  ImageSize The size in memory of the PE/COFF image
+  @param[in,out]  EntryPointThe entry point of the PE/COFF image. Passed by
+reference so that the emulator may modify it.
+
+  @retval EFI_SUCCESS   The image was registered with the emulator and
+can be started as usual.
+  @retval other The image could not be registered.
+
+  If the PE/COFF machine type or image type are not supported by the emulator,
+  then ASSERT().
+**/
+EFI_STATUS
+EFIAPI
+EbcRegisterImage (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+  IN  EFI_PHYSICAL_ADDRESSImageBase,
+  IN  UINT64  ImageSize,
+  IN  OUT EFI_IMAGE_ENTRY_POINT   *EntryPoint
+  )
+{
+  DEBUG_CODE_BEGIN ();
+PE_COFF_LOADER_IMAGE_CONTEXT  ImageContext;
+EFI_STATUSStatus;
+
+ZeroMem (, sizeof (ImageContext));
+
+ImageContext.Handle= (VOID *)(UINTN)ImageBase;
+ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
+
+Status = PeCoffLoaderGetImageInfo ();
+if (EFI_ERROR (Status)) {
+  return Status;
+}
+
+ASSERT (ImageContext.Machine == EFI_IMAGE_MACHINE_EBC);
+ASSERT (ImageContext.ImageType == EFI_IMAGE_SUBSYSTEM_EFI_APPLICATION ||
+ImageContext.ImageType == 
EFI_IMAGE_SUBSYSTEM_EFI_BOOT_SERVICE_DRIVER);
+  DEBUG_CODE_END ();
+
+  EbcRegisterICacheFlush (NULL,
+(EBC_ICACHE_FLUSH)InvalidateInstructionCacheRange);
+

[edk2] [PATCH v3 1/7] MdeModulePkg: introduce PE/COFF image emulator protocol

2018-09-20 Thread Ard Biesheuvel
Introduce a protocol that can be invoked by the image loading services
to execute foreign architecture PE/COFF images via an emulator.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h | 102 
 MdeModulePkg/MdeModulePkg.dec   |   4 +
 2 files changed, 106 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h 
b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
new file mode 100644
index ..27bad556209c
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h
@@ -0,0 +1,102 @@
+/** @file
+  Copyright (c) 2018, Linaro, Ltd. All rights reserved.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID_H
+#define PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID_H
+
+#define EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL_GUID \
+  { 0x96F46153, 0x97A7, 0x4793, { 0xAC, 0xC1, 0xFA, 0x19, 0xBF, 0x78, 0xEA, 
0x97 } }
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL 
EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+/**
+  Check whether the emulator supports executing a certain PE/COFF image
+
+  @param[in] This This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+  structure
+  @param[in] MachineType  The machine type for which the image was built
+  @param[in] ImageTypeWhether the image is an application, a boot time
+  driver or a runtime driver.
+  @param[in] DevicePath   Path to device where the image originated
+  (e.g., a PCI option ROM)
+
+  @retval TRUEThe image is supported by the emulator
+  @retval FALSE   The image is not supported by the emulator.
+**/
+typedef
+BOOLEAN
+(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTED) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+  IN  UINT16  MachineType,
+  IN  UINT16  ImageType,
+  IN  EFI_DEVICE_PATH_PROTOCOL*DevicePath   OPTIONAL
+  );
+
+/**
+  Register a supported PE/COFF image with the emulator. After this call
+  completes successfully, the PE/COFF image may be started as usual, and
+  it is the responsibility of the emulator implementation that any branch
+  into the code section of the image (including returns from functions called
+  from the foreign code) is executed as if it were running on the machine
+  type it was built for.
+
+  @param[in]  This  This pointer for
+EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL structure
+  @param[in]  ImageBase The base address in memory of the PE/COFF image
+  @param[in]  ImageSize The size in memory of the PE/COFF image
+  @param[in,out]  EntryPointThe entry point of the PE/COFF image. Passed by
+reference so that the emulator may modify it.
+
+  @retval EFI_SUCCESS   The image was registered with the emulator and
+can be started as usual.
+  @retval other The image could not be registered.
+
+  If the PE/COFF machine type or image type are not supported by the emulator,
+  then ASSERT().
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGE) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+  IN  EFI_PHYSICAL_ADDRESSImageBase,
+  IN  UINT64  ImageSize,
+  IN  OUT EFI_IMAGE_ENTRY_POINT   *EntryPoint
+  );
+
+/**
+  Unregister a PE/COFF image that has been registered with the emulator.
+  This should be done before the image is unloaded from memory.
+
+  @param[in] This This pointer for EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL
+  structure
+  @param[in] ImageBaseThe base address in memory of the PE/COFF image
+
+  @retval EFI_SUCCESS The image was unregistered with the emulator.
+  @retval other   Image could not be unloaded.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE) (
+  IN  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL*This,
+  IN  EFI_PHYSICAL_ADDRESSImageBase
+  );
+
+typedef struct _EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL {
+  EDKII_PECOFF_IMAGE_EMULATOR_IS_IMAGE_SUPPORTEDIsImageSupported;
+  EDKII_PECOFF_IMAGE_EMULATOR_REGISTER_IMAGERegisterImage;
+  EDKII_PECOFF_IMAGE_EMULATOR_UNREGISTER_IMAGE  UnregisterImage;
+} EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL;
+
+extern EFI_GUID 

[edk2] [PATCH v3 7/7] MdeModulePkg/DxeCore: remove explicit EBC handling

2018-09-20 Thread Ard Biesheuvel
Now that the EBC machine type is no longer classified as a
natively supported machine type on the architectures that can
support it via the EBC interpreter, the EBC specific handling
in DXE core is no longer used and can be removed.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/DxeMain.h |  3 --
 MdeModulePkg/Core/Dxe/DxeMain.inf   |  1 -
 MdeModulePkg/Core/Dxe/Image/Image.c | 53 ++--
 3 files changed, 3 insertions(+), 54 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index ff2418c5ae5e..c473006813fe 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -42,7 +42,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -228,8 +227,6 @@ typedef struct {
   BASE_LIBRARY_JUMP_BUFFER*JumpContext;
   /// Machine type from PE image
   UINT16  Machine;
-  /// EBC Protocol pointer
-  EFI_EBC_PROTOCOL*Ebc;
   /// PE/COFF Image Emulator Protocol pointer
   EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *PeCoffEmu;
   /// Runtime image list
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 63e650ee7c27..a969b869b331 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -161,7 +161,6 @@
   gEfiLoadedImageProtocolGuid   ## PRODUCES
   gEfiLoadedImageDevicePathProtocolGuid ## PRODUCES
   gEfiHiiPackageListProtocolGuid## SOMETIMES_PRODUCES
-  gEfiEbcProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiSmmBase2ProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiBlockIoProtocolGuid   ## SOMETIMES_CONSUMES
   gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index dd987f7fcea7..53d526fddc7d 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -74,7 +74,6 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage  = {
   NULL,   // JumpBuffer
   NULL,   // JumpContext
   0,  // Machine
-  NULL,   // Ebc
   NULL,   // PeCoffEmu
   NULL,   // RuntimeData
   NULL// LoadedImageDevicePath
@@ -91,9 +90,6 @@ typedef struct {
   CHAR16  *MachineTypeName;
 } MACHINE_TYPE_INFO;
 
-//
-// EBC machine is not listed in this table, because EBC is in the default 
supported scopes of other machine type.
-//
 GLOBAL_REMOVE_IF_UNREFERENCED MACHINE_TYPE_INFO  mMachineTypeInfo[] = {
   {EFI_IMAGE_MACHINE_IA32,   L"IA32"},
   {EFI_IMAGE_MACHINE_IA64,   L"IA64"},
@@ -742,51 +738,15 @@ CoreLoadPeImage (
   InvalidateInstructionCacheRange ((VOID 
*)(UINTN)Image->ImageContext.ImageAddress, 
(UINTN)Image->ImageContext.ImageSize);
 
   //
-  // Copy the machine type from the context to the image private data. This
-  // is needed during image unload to know if we should call an EBC protocol
-  // to unload the image.
+  // Copy the machine type from the context to the image private data.
   //
   Image->Machine = Image->ImageContext.Machine;
 
   //
-  // Get the image entry point. If it's an EBC image, then call into the
-  // interpreter to create a thunk for the entry point and use the returned
-  // value for the entry point.
+  // Get the image entry point.
   //
   Image->EntryPoint   = 
(EFI_IMAGE_ENTRY_POINT)(UINTN)Image->ImageContext.EntryPoint;
-  if (Image->ImageContext.Machine == EFI_IMAGE_MACHINE_EBC) {
-//
-// Locate the EBC interpreter protocol
-//
-Status = CoreLocateProtocol (, NULL, (VOID 
**)>Ebc);
-if (EFI_ERROR(Status) || Image->Ebc == NULL) {
-  DEBUG ((DEBUG_LOAD | DEBUG_ERROR, "CoreLoadPeImage: There is no EBC 
interpreter for an EBC image.\n"));
-  goto Done;
-}
-
-//
-// Register a callback for flushing the instruction cache so that created
-// thunks can be flushed.
-//
-Status = Image->Ebc->RegisterICacheFlush (Image->Ebc, 
(EBC_ICACHE_FLUSH)InvalidateInstructionCacheRange);
-if (EFI_ERROR(Status)) {
-  goto Done;
-}
-
-//
-// Create a thunk for the image's entry point. This will be the new
-// entry point for the image.
-//
-Status = Image->Ebc->CreateThunk (
-   Image->Ebc,
-   Image->Handle,
-   (VOID *)(UINTN) Image->ImageContext.EntryPoint,
-   (VOID **) >EntryPoint
-   );
-if (EFI_ERROR(Status)) {
-  goto Done;
-}
-  } else if (Image->PeCoffEmu != NULL) {
+  if (Image->PeCoffEmu != NULL) {
 Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
 

[edk2] [PATCH v3 4/7] MdeModulePkg/UefiBootManagerLib: allow foreign Driver#### images

2018-09-20 Thread Ard Biesheuvel
Allow PE/COFF images that must execute under emulation for Driver
options, by relaxing the machine type check to include support for
machine types that is provided by an emulator.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c | 51 
+++-
 MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h   |  1 +
 MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf |  1 +
 3 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 7bf96646c690..f6fda8f2c3f7 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -1185,6 +1185,54 @@ EfiBootManagerFreeLoadOptions (
   return EFI_SUCCESS;
 }
 
+STATIC
+BOOLEAN
+BmIsImageTypeSupported (
+  IN  UINT16MachineType,
+  IN  UINT16SubSystem
+  )
+{
+  EFI_STATUSStatus;
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *Emu;
+  UINTN HandleCount;
+  EFI_HANDLE*HandleBuffer;
+  BOOLEAN   ReturnValue;
+  UINTN Index;
+
+  if (EFI_IMAGE_MACHINE_TYPE_SUPPORTED (MachineType)) {
+return TRUE;
+  }
+
+  Status = gBS->LocateHandleBuffer (
+  ByProtocol,
+  ,
+  NULL,
+  ,
+  
+  );
+  if (EFI_ERROR (Status)) {
+return FALSE;
+  }
+
+  ReturnValue = FALSE;
+  for (Index = 0; Index < HandleCount; Index++) {
+Status = gBS->HandleProtocol (
+HandleBuffer[Index],
+,
+(VOID **)
+);
+ASSERT_EFI_ERROR (Status);
+
+if (Emu->IsImageSupported (Emu, MachineType, SubSystem, NULL)) {
+  ReturnValue = TRUE;
+  break;
+}
+  }
+
+  FreePool (HandleBuffer);
+  return ReturnValue;
+}
+
 /**
   Return whether the PE header of the load option is valid or not.
 
@@ -1235,7 +1283,8 @@ BmIsLoadOptionPeHeaderValid (
   OptionalHeader = (EFI_IMAGE_OPTIONAL_HEADER32 *) 
>Pe32.OptionalHeader;
   if ((OptionalHeader->Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC ||
OptionalHeader->Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) &&
-  EFI_IMAGE_MACHINE_TYPE_SUPPORTED (PeHeader->Pe32.FileHeader.Machine)
+  BmIsImageTypeSupported (PeHeader->Pe32.FileHeader.Machine,
+  OptionalHeader->Subsystem)
   ) {
 //
 // Check the Subsystem:
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 978fbff966f6..5f64ef304b87 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -47,6 +47,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf 
b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
index 72c5ca1cd59e..09e2134acf8e 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+++ b/MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
@@ -104,6 +104,7 @@
   gEfiDevicePathProtocolGuid## SOMETIMES_CONSUMES
   gEfiBootLogoProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiSimpleTextInputExProtocolGuid ## SOMETIMES_CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
   gEdkiiVariableLockProtocolGuid## SOMETIMES_CONSUMES
   gEfiGraphicsOutputProtocolGuid## SOMETIMES_CONSUMES
   gEfiUsbIoProtocolGuid ## SOMETIMES_CONSUMES
-- 
2.17.1

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


[edk2] [PATCH v3 2/7] MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images

2018-09-20 Thread Ard Biesheuvel
When encountering PE/COFF images that cannot be supported natively,
attempt to locate an instance of the PE/COFF image emulator protocol,
and if it supports the image, proceed with loading it and register it
with the emulator.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/DxeMain.h |   3 +
 MdeModulePkg/Core/Dxe/DxeMain.inf   |   1 +
 MdeModulePkg/Core/Dxe/Image/Image.c | 139 ++--
 MdeModulePkg/Core/Dxe/Image/Image.h |   1 +
 4 files changed, 133 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 7ec82388a3f9..ff2418c5ae5e 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -53,6 +53,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -229,6 +230,8 @@ typedef struct {
   UINT16  Machine;
   /// EBC Protocol pointer
   EFI_EBC_PROTOCOL*Ebc;
+  /// PE/COFF Image Emulator Protocol pointer
+  EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *PeCoffEmu;
   /// Runtime image list
   EFI_RUNTIME_IMAGE_ENTRY *RuntimeData;
   /// Pointer to Loaded Image Device Path Protocol
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 68fa0a01d9bd..63e650ee7c27 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -164,6 +164,7 @@
   gEfiEbcProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiSmmBase2ProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiBlockIoProtocolGuid   ## SOMETIMES_CONSUMES
+  gEdkiiPeCoffImageEmulatorProtocolGuid ## SOMETIMES_CONSUMES
 
   # Arch Protocols
   gEfiBdsArchProtocolGuid   ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c 
b/MdeModulePkg/Core/Dxe/Image/Image.c
index eddca140ee1a..dd987f7fcea7 100644
--- a/MdeModulePkg/Core/Dxe/Image/Image.c
+++ b/MdeModulePkg/Core/Dxe/Image/Image.c
@@ -29,6 +29,14 @@ LOAD_PE32_IMAGE_PRIVATE_DATA  mLoadPe32PrivateData = {
   }
 };
 
+STATIC
+EDKII_PECOFF_IMAGE_EMULATOR_PROTOCOL  *mAvailableEmulators[MAX_NUM_EMULATORS];
+
+STATIC
+EFI_EVENT mPeCoffEmuProtocolRegistrationEvent;
+
+STATIC
+VOID  *mPeCoffEmuProtocolNotifyRegistration;
 
 //
 // This code is needed to build the Image handle for the DXE Core
@@ -67,6 +75,7 @@ LOADED_IMAGE_PRIVATE_DATA mCorePrivateImage  = {
   NULL,   // JumpContext
   0,  // Machine
   NULL,   // Ebc
+  NULL,   // PeCoffEmu
   NULL,   // RuntimeData
   NULL// LoadedImageDevicePath
 };
@@ -118,6 +127,41 @@ GetMachineTypeName (
   return L"";
 }
 
+/**
+  Notification event handler registered by CoreInitializeImageServices () to
+  keep track of which PE/COFF image emulators are available.
+
+  @param  Event  The Event that is being processed, not used.
+  @param  ContextEvent Context, not used.
+
+**/
+STATIC
+VOID
+EFIAPI
+PeCoffEmuProtocolNotify (
+  IN  EFI_EVENT  Event,
+  IN  VOID   *Context
+  )
+{
+  EFI_STATUS  Status;
+  UINTN   Index;
+
+  for (Index = 0; Index < MAX_NUM_EMULATORS; Index++) {
+if (mAvailableEmulators[Index] == NULL) {
+  break;
+}
+  }
+
+  // ensure that there is still room in the emulator protocol array
+  ASSERT (Index < MAX_NUM_EMULATORS);
+
+  Status = CoreLocateProtocol (,
+   mPeCoffEmuProtocolNotifyRegistration,
+   (VOID **)[Index]
+   );
+  ASSERT_EFI_ERROR (Status);
+}
+
 /**
   Add the Image Services to EFI Boot Services Table and install the protocol
   interfaces for this image.
@@ -192,6 +236,28 @@ CoreInitializeImageServices (
   gDxeCoreImageHandle = Image->Handle;
   gDxeCoreLoadedImage = >Info;
 
+  //
+  // Create the PE/COFF emulator protocol registration event
+  //
+  Status = CoreCreateEvent (
+ EVT_NOTIFY_SIGNAL,
+ TPL_CALLBACK,
+ PeCoffEmuProtocolNotify,
+ NULL,
+ 
+ );
+  ASSERT_EFI_ERROR(Status);
+
+  //
+  // Register for protocol notifications on this event
+  //
+  Status = CoreRegisterProtocolNotify (
+ ,
+ mPeCoffEmuProtocolRegistrationEvent,
+ 
+ );
+  ASSERT_EFI_ERROR(Status);
+
   if (FeaturePcdGet (PcdFrameworkCompatibilitySupport)) {
 //
 // Export DXE Core PE Loader functionality for backward compatibility.
@@ -425,6 +491,41 @@ GetPeCoffImageFixLoadingAssignedAddress(
DEBUG ((EFI_D_INFO|EFI_D_LOAD, "LOADING MODULE FIXED INFO: Loading module 
at fixed address 0x%11p. Status = %r \n", (VOID 

[edk2] [PATCH v3 0/7] MdeModulePkg: add support for dispatching foreign arch PE/COFF images

2018-09-20 Thread Ard Biesheuvel
Add the basic plumbing to DXE core, the PCI bus driver and the boot manager
to allow PE/COFF images to be dispatched that target an architecture that is
not native for the platform, but which is supported by one of potentially
several available emulators.

One implementation of such an emulator can be found here:
https://github.com/ardbiesheuvel/X86EmulatorPkg

This also allows us to get rid of the special treatment of EBC images in
core code. Instead, the EbcDxe driver is augmented with an implementation
of the EDK2 PE/COFF image emulator so that internal knowledge of how EBC
is implemented (I-cache flushing, thunks) is removed from the DXE core.

Changes since v2:
- incorporate feedback from Andrew Fish (delivered in person):
  * pass a device path into the IsImageSupported() protocol method so that an
implementation can blacklist or whitelist certain devices, or implement
other policies that depend on the device where the driver originated
  * allow the emulator to supersede the native loading of the image - this
permits things like X86 on X86 emulators for security sandboxing or debug

Changes since v1:
- subsume the EBC handling into the EDK2 emulator protocol and abstract
  away from EBC specifics in core code.
- allow multiple emulator implementations to co-exist
- incorporate Star's review feedback

Cc: Vincent Zimmer 
Cc: Brian Richardson 
Cc: Michael D Kinney 
Cc: Andrew Fish 
Cc: Leif Lindholm 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Ruiyu Ni 
Cc: Liming Gao 
Cc: Jaben Carsey 
Cc: Steven Shi 

Ard Biesheuvel (7):
  MdeModulePkg: introduce PE/COFF image emulator protocol
  MdeModulePkg/DxeCore: invoke the emulator protocol for foreign images
  MdeModulePkg/PciBusDxe: invoke PE/COFF emulator for foreign option
ROMs
  MdeModulePkg/UefiBootManagerLib: allow foreign Driver images
  MdeModulePkg/EbcDxe: implement the PE/COFF emulator protocol
  MdePkg/UefiBaseType.h: treat EBC as a non-native machine type
  MdeModulePkg/DxeCore: remove explicit EBC handling

 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h   |   1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf  |   1 +
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  53 +-
 MdeModulePkg/Core/Dxe/DxeMain.h   |   6 +-
 MdeModulePkg/Core/Dxe/DxeMain.inf |   2 +-
 MdeModulePkg/Core/Dxe/Image/Image.c   | 178 --
 MdeModulePkg/Core/Dxe/Image/Image.h   |   1 +
 .../Include/Protocol/PeCoffImageEmulator.h| 102 ++
 .../Library/UefiBootManagerLib/BmLoadOption.c |  51 -
 .../Library/UefiBootManagerLib/InternalBm.h   |   1 +
 .../UefiBootManagerLib/UefiBootManagerLib.inf |   1 +
 MdeModulePkg/MdeModulePkg.dec |   4 +
 MdeModulePkg/Universal/EbcDxe/EbcDxe.inf  |   3 +
 MdeModulePkg/Universal/EbcDxe/EbcInt.c| 127 +
 MdeModulePkg/Universal/EbcDxe/EbcInt.h|   3 +
 MdePkg/Include/Uefi/UefiBaseType.h|   8 +-
 16 files changed, 478 insertions(+), 64 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/PeCoffImageEmulator.h

-- 
2.17.1

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


Re: [edk2] Stack issue after warm UEFI reset and MMU enabling on an Armv8 platform

2018-09-20 Thread Vladimir Olovyannikov
On Wed, Sep 19, 2018 at 5:21 PM Bill Paul  wrote:
>
> Of all the gin joints in all the towns in all the world, Vladimir
> Olovyannikov
> had to walk into mine at 16:58 on Wednesday 19 September 2018 and say:
>
> > >From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > >Sent: Wednesday, September 19, 2018 4:38 PM
> > >To: Vladimir Olovyannikov
> > >Cc: edk2-devel@lists.01.org
> > >Subject: Re: Stack issue after warm UEFI reset and MMU enabling on an
> > >Armv8 platform
> > >
> > >
> > >On 19 September 2018 at 15:55, Vladimir Olovyannikov
> > >
> > > wrote:
> > >>Hi All,
> > >>
> > >>I need UEFI experts help on the problem with Armv8 board on warm UEFI
> > >>reset.
> > >>Cold reset works fine.
> > >>
> > >>Here is how I set up a warm reset:
> > >>
> > >>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;
> > >>
> > >>}
> > >>
> > >>Then perform
> > >>ArmCleanDataCache ();
> > >>ArmInvalidateDataCache ();
> > >>ArmDisableInstructionCache ();
> > >>ArmInvalidateInstructionCache ();
> > >
> > >These don't do anything useful on ARM. You can only reliably perform
> > >cache
> > >maintenance by virtual address.
> >
> > So, should I just remove them altogether?
> >
> > >>ArmDisableMmu ();
> > >
> > >... so after this call returns, all bets are off with regards to
> > >whether
> > >what is popped from the stack is actually what we pushed when we
> > >entered
> > >the function.
> >
> > OK, thank you for explanation.
> > But this call returns back into ResetLib implementation as it should,
> > and
> > then there is a direct jump to the start of FV.
> > Am I doing anything wrong here?
> > Then, up to the point of enabling of MMU the stack is OK. But right
> > after
> > enabling MMU it points at _ModuleEntryPoint end of function in
> > DxeCoreEntryPoint.c
> > Am I missing anything? Maybe some stack cleanup before jumping to the
> > start
> > of FV?
>
> When the MMU is enabled, does the mapping for the stack pages change? That
> is,
> could the stack now be mapped to different physical page now?
Thanks for ideas Bill,
No, the mapping stays the same.
The issue is only with warm reset, and only on an A72 board.
There is another platform on A53 sharing the same code, which has no issues
with warm reset.
I cannot explain why.
>
> Instead of showing a stack trace, can you dump the stack pages and compare
> the
> before and after contents?
I can clearly see that before and after contents are different.
>
> Assuming the same physical memory pages are still being used, then there
> could
> be a cache flushing problem. What could happen is:
>
> - some stack memory has been touched recently and is now in the data cache
> - changes are made, which are written to the cache, but not yet flushed
> out to
> RAM
> - enabling the MMU causes a full invalidate of the cache
>
> Now when you look at the stack, you see the earlier contents that were in
> RAM
> -- the changes previously only written to the cache have been lost.
>
> Enabling/disabling caches and MMU is always tricky. I can't say for sure,
> but
> I wouldn't be surprised if there's some subtle bug that causes a flush
> operation to be missed and things may just work by coincidence in the cold
> start case.
I might be missing something preparing for warm reset.
Disabling interrupts does not help though.
Ard, I switched off all DMA-capable devices, 

Re: [edk2] [Question] Testing environment regarding SMM driver

2018-09-20 Thread Andrew Fish



> On Sep 20, 2018, at 7:30 AM, poxyran  wrote:
> 
> Hello,
> 
> I have a specific question regarding SMM drivers. I'm trying to create a
> mixed driver as mentioned here
> http://blog.cr4.sh/2015/07/building-reliable-smm-backdoor-for-uefi.html
> and my first try is to create a kind of 'Hello World'. My try is to
> install a SMI handler and call it from a user-mode app once the OS
> booted up. The testing aproach mentioned in the blog post is not
> practical, from my pooint of view. My question is, is it possible to
> test this kind of drivers from the UEFI shell? or do I need to setup a
> dedicated machine as mentioned here

poxyran,

I'm not sure what you are asking?  Indirectly referencing a 10,000 word article 
is not very helpful. Feel free to ask a specific question. 

Thanks,

Andrew Fish

> https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt?
> 
> BR,
> poxyran
> 
> ___
> 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] UEFIPayload build issue

2018-09-20 Thread Wim Vervoorn


Hello,

I am trying to build the new UEFIPayload from the staging repo.

The build proceeds pretty well but then I got this message:

Traceback (most recent call last):
  File "C:\Python27\lib\site-packages\cx_Freeze\initscripts\Console.py", line 
27, in 
exec(code, m.__dict__)
  File "GenFds\GenFds.py", line 24, in 
ValueError: Attempted relative import in non-package


build.exe...
: error 7000: Failed to execute command
GenFds -f 
C:\git\SlimBootPayload\UEFIPayload\UefiPayloadPkg\UefiPayloadPkg.fdf 
--conf=c:\git\slimbootpayload\edk2\conf -o 
c:\git\slimbootpayload\edk2\Build\UefiPayloadPkgX64\DEBUG_VS2015x86 -t 
VS2015x86 -b DEBUG -p 
C:\git\SlimBootPayload\UEFIPayload\UefiPayloadPkg\UefiPayloadPkgIA32X64.dsc -a 
IA32,X64  -D "EFI_SOURCE=c:\\git\\slimbootpayload\\edk2\\edkcompatibilitypkg"  
-D "EDK_SOURCE=c:\\git\\slimbootpayload\\edk2\\edkcompatibilitypkg"  -D 
"TOOL_CHAIN_TAG=VS2015x86"  -D "TOOLCHAIN=VS2015x86"  -D "TARGET=DEBUG"  -D 
"FAMILY=MSFT"  -D "WORKSPACE=c:\\git\\slimbootpayload\\edk2"  -D 
"EDK_TOOLS_PATH=c:\\git\\slimbootpayload\\edk2\\basetools"  -D 
"BD_ARCH=IA32X64"  -D "ARCH=IA32 X64"  -D 
"ECP_SOURCE=c:\\git\\slimbootpayload\\edk2\\edkcompatibilitypkg" 
[C:\git\SlimBootPayload\edk2]

- Failed -

So far I haven’t figured out what is causing this issue. It is good to note 
that use the tools from the master of the edk2 repo (status of today).

Suggestions are welcome.

Wim Vervoorn

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


[edk2] [PATCH] [edk2-platforms/devel-IntelAtomProcessorE3900] Clean up build scripts

2018-09-20 Thread Steele, Kelly
>From 62aef5b6fdb4e85878bafed4ab9c41631689b69d Mon Sep 17 00:00:00 2001
From: Kelly Steele 
Date: Thu, 20 Sep 2018 10:47:06 -0700
Subject: [PATCH] [edk2-platforms/devel-IntelAtomProcessorE3900] Clean up build
scripts

Went thru the build scripts and cleaned them up. There was a couple of
"magic" numbers being used in BuildBxtBios.bat that are derived from
defines.dsc. I switched from "magic" numbers to parsing defines.dsc to
use what is set there.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Kelly Steele 
---
BuildBIOS.bat|  12 +-
Platform/BroxtonPlatformPkg/BuildBxtBios.bat | 232 -
Platform/BroxtonPlatformPkg/BuildIFWI.bat| 497 ++-
3 files changed, 410 insertions(+), 331 deletions(-)

diff --git a/BuildBIOS.bat b/BuildBIOS.bat
index d8275aca14..500c2244c9 100644
--- a/BuildBIOS.bat
+++ b/BuildBIOS.bat
@@ -34,13 +34,14 @@ echo  Call Build Script of Broxton 
 if not exist Platform\%PlatformName%PlatformPkg\BuildIFWI.bat (
   echo Platform %PlatformName%PlatformPkg does not exist
-  echo. & echo Error - Unsupported Platform name: %1
+  echo. & echo Error - Unsupported Platform name: %1
   echo.
   goto Usage
)
-echo calling : Platform\%PlatformName%PlatformPkg\BuildIFWI.bat  %BuildFlags%  
/fspw MINN %BuildTarget%
-call Platform\%PlatformName%PlatformPkg\BuildIFWI.bat  %BuildFlags%  /fspw 
MINN %BuildTarget%
+echo calling : Platform\%PlatformName%PlatformPkg\BuildIFWI.bat  %BuildFlags%  
/fspw MINN %BuildTarget%
+call Platform\%PlatformName%PlatformPkg\BuildIFWI.bat  %BuildFlags%  /fspw 
MINN %BuildTarget%
+set ExitCode=%ErrorLevel%
 goto Exit
@@ -75,4 +76,7 @@ echo%thisscript% /vs13 /LH /B /x64 Broxton Debug
set exitCode=1
 :Exit
-exit /b %exitCode%
+(
+  EndLocal
+  exit /b %exitCode%
+)
diff --git a/Platform/BroxtonPlatformPkg/BuildBxtBios.bat 
b/Platform/BroxtonPlatformPkg/BuildBxtBios.bat
index 9858ceae58..2bc2556a8a 100644
--- a/Platform/BroxtonPlatformPkg/BuildBxtBios.bat
+++ b/Platform/BroxtonPlatformPkg/BuildBxtBios.bat
@@ -73,8 +73,9 @@ set FSP_BIN_PKG_NAME=BroxtonFspBinPkg
set STITCH_PATH=%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\Stitch
set ResetVectorPath=%WORKSPACE%\%PLATFORM_RC_PACKAGE%\Cpu\ResetVector
-PATH=%PATH%;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\GenBiosId;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\nasm\Win32
-PATH=%PATH%;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\FCE;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\nasm\Win32
+PATH=%PATH%;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\FCE
+PATH=%PATH%;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\GenBiosId
+PATH=%PATH%;%WORKSPACE%\%PLATFORM_PATH%\Common\Tools\nasm\Win32
 ::**
:: Parse command line arguments
@@ -268,8 +269,8 @@ if "%Arch%"=="IA32" (
 echo DEFINE X64_CONFIG  = TRUE  >> 
%Build_Macros%
)
-echo DEFINE UP2_BOARD= %UP2_BOARD%   >> 
%Build_Macros%
-echo DEFINE MINNOW3_MODULE_BOARD = %MINNOW3_MODULE_BOARD%>> 
%Build_Macros%
+echo DEFINE UP2_BOARD   = %UP2_BOARD%   >> 
%Build_Macros%
+echo DEFINE MINNOW3_MODULE_BOARD= %MINNOW3_MODULE_BOARD%>> 
%Build_Macros%
 ::Stage of copy of BiosId.env in Conf/ with Platform_Type and Build_Target 
values removed
@@ -288,43 +289,63 @@ if /i "%~2" == "RELEASE" (
 echo BUILD_TYPE = D >> Conf\BiosId.env
)
-if %BoardId%==BG (
-  if %FabId%==B (
+if "%BoardId%" == "BG" (
+  if "%FabId%" == "A" (
+echo BOARD_REV = A >> Conf\BiosId.env
+  ) else if "%FabId%" == "B" (
 echo BOARD_REV = B >> Conf\BiosId.env
   ) else (
-echo BOARD_REV = A >> Conf\BiosId.env
+echo ERROR: Benson Glacier currently only supports Board Fab A & B^^^!
+goto BldFail
   )
)
-if %BoardId%==AG (
-  echo BOARD_REV = A >> Conf\BiosId.env
+if "%BoardId%" == "AG" (
+  if "%FabId%" == "A" (
+echo BOARD_REV = A >> Conf\BiosId.env
+  ) else (
+echo ERROR: Aurora Glacier currently only supports Board Fab A^^^!
+goto BldFail
+  )
)
-if %BoardId%==MN (
-  if %FabId%==B (
+if "%BoardId%" == "MN" (
+  if "%FabId%" == "A" (
+echo BOARD_REV = A >> Conf\BiosId.env
+  ) else if "%FabId%" == "B" (
 echo BOARD_REV = B >> Conf\BiosId.env
   ) else (
-echo BOARD_REV = A >> Conf\BiosId.env
+echo ERROR: Minnow Baord v3 currently only supports Board Fab A & B^^^!
+goto BldFail
   )
)
-if %BoardId%==MX (
-  if %FabId%==C (
+if "%BoardId%" == "MX" (
+  if "%FabId%" == "A" (
+echo BOARD_REV = A >> Conf\BiosId.env
+  ) else if "%FabId%" == "C" (
 echo BOARD_REV = C >> Conf\BiosId.env
   ) else (
-echo BOARD_REV = A >> Conf\BiosId.env
+echo ERROR: Minnow Baord v3 Module currently only supports Board Fab A, B 
& C^^^!
+goto BldFail
   )
)
-if %BoardId%==LH (
-  if %FabId%==D (
+if "%BoardId%" == "LH" (
+  if "%FabId%" == "D" (
 echo BOARD_REV = D >> Conf\BiosId.env
+  ) else (
+echo ERROR: Leaf Hill currently only 

Re: [edk2] SPI Flash Corruption

2018-09-20 Thread Samah Mansour
Hi Laszlo,
Thanks for your reply.
Actually what I see is that VPD (Vital Product Area between addresses
44000->47DFF0  ) is completely wiped which causes the failure to boot!
Without the VPD unit cannot boot.
I will take a look at the white paper.
It would be helpful to know what's the impact of disabling the ability of
the firmware to write those non volatile variables to flash.

Samah


On Thu, Sep 20, 2018 at 9:48 AM Laszlo Ersek  wrote:

> On 09/19/18 16:26, Samah Mansour wrote:
> > Hello,
> >
> >
> > Our product uses a Baytrail with Minnowboard Max bios firmware ( version
> > 0.93). Every now and then we see SPI flash corruption due to power cuts
> > while the unit is booting which causes the unit not to boot anymore.
> After
> > investigation we noticed that the VPD area is all FFs (address
> > 44000->47DFF0).
> >
> >
> > We have noticed that the Bios while booting writes to the flash from
> > several places in the code, which is if interrupted most probably is
> > causing the corruption.
> >
> >
> > Why is the bios writing all these configurations to flash while booting,
> is
> > it to optimize boot time? is it ok if we disable the bios writing to
> flash
> > completely to protect ourselves from corruption?
>
> The firmware is at liberty to write various non-volatile UEFI variables
> during boot. Some of those variables are standardized, some others may
> be specific to UEFI drivers (with correspondingly private namespace
> GUIDs for the variables).
>
> Power loss during flash write (and resultant flash corruption) is
> expected. My understanding is that the Fault Tolerant Write protocol /
> driver, sitting between the FVB (firmware volume block, i.e. flash)
> protocol / driver, and the variable write protocol / driver, implements
> a kind of journaling. It is described in the Intel whitepaper
>
>   A Tour Beyond BIOS
>   Implementing UEFI Authenticated Variables in SMM with EDKII
>   September 2015
>
> My expectation has been that the platform should recover from
> interrupted writes. That is, for a single given UEFI variable, you
> should either see "before" or "after" status, never "middle". (The
> whitepaper says that "Individual variable atomicity" is maintained even
> through a failed "reclaim", with the help of FTW.)
>
> If multiple variables should be in sync with each other, that's a
> different question. If the variables are not in sync, I think "failure
> to boot" may be a reasonable outcome. But, "failure to boot" means a lot
> of things, and I hope one should be at least dropped to the setup
> utility or the shell. Are you seeing an actual crash?
>
> Laszlo
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Question] Testing environment regarding SMM driver

2018-09-20 Thread poxyran
Hello,

I have a specific question regarding SMM drivers. I'm trying to create a
mixed driver as mentioned here
http://blog.cr4.sh/2015/07/building-reliable-smm-backdoor-for-uefi.html
and my first try is to create a kind of 'Hello World'. My try is to
install a SMI handler and call it from a user-mode app once the OS
booted up. The testing aproach mentioned in the blog post is not
practical, from my pooint of view. My question is, is it possible to
test this kind of drivers from the UEFI shell? or do I need to setup a
dedicated machine as mentioned here
https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt?

BR,
poxyran

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


Re: [edk2] question about uefi shell-current working directory and shell-script

2018-09-20 Thread Carsey, Jaben
When the shell starts, it the prompt "shell>" or something like "fs0:>"... The 
shell may not have a file system assigned yet so you cannot change directories 
until you pick a file system.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> krishnaLee
> Sent: Thursday, September 20, 2018 2:22 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] question about uefi shell-current working directory and shell-
> script
> Importance: High
> 
> Hi,
> I wonder if it is a bug:
> I have a usb Fat32-disk,installed with EDK2 shell2.6 or 2.7,
> the root directory has a startup.nsh,the startup.nsh has only one line:"cd \",
> I boot my cannolake machine with this usb disk,
> in the uefi shell,I got a message:"cd: current directory not specified".it 
> means
> the cwd environment variable was not set when start doing script.
> 
> 
> I think it may be  an error,because the uefi shell can find the 
> startup.nsh,why
> not set cwd(current working directory) before execute this script,so the cd
> command works fine.
> I mention it because I found if I put a uefi application(using 
> efi_shell_protocol
> to access .\logfile.txt) in this startup.nsh,the uefi application can't 
> access files,
>  because I found the efi_shell_protocol need cwd to work too,such as
> efi_shell_protocol.OpenFileByName().
> 
> 
> 
> 
> 
> 
> thank you,
> krishna
> 
> 
> 
> ___
> 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 v1 0/5] [CVE-2017-5753] Bounds Check Bypass issue in SMI handlers

2018-09-20 Thread Kinney, Michael D
Hao Wu,

I see that implementations of this API are only
provided for IA32 and X64.  Should this be an IA32/X64
specific API in BaseLib?  Also, since the API is providing
a C callable function to execute a specific IA32/X64 
instruction, should the API be prefixed with Asm to 
match the convention of other APIs in BaseLib?

Thanks,

Mike

> -Original Message-
> From: Wu, Hao A
> Sent: Wednesday, September 19, 2018 11:41 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Ard Biesheuvel
> ; Laszlo Ersek
> ; Yao, Jiewen
> ; Kinney, Michael D
> ; Gao, Liming
> ; Zeng, Star
> ; Dong, Eric 
> Subject: [PATCH v1 0/5] [CVE-2017-5753] Bounds Check
> Bypass issue in SMI handlers
> 
> The series aims to mitigate the Bounds Check Bypass
> (CVE-2017-5753) issues
> within SMI handlers.
> 
> A more detailed explanation of the purpose of the
> series is under the
> 'Bounds check bypass mitigation' section of the below
> link:
> https://software.intel.com/security-software-
> guidance/insights/host-firmware-speculative-execution-
> side-channel-mitigation
> 
> And the document at:
> https://software.intel.com/security-software-
> guidance/api-app/sites/default/files/337879-analyzing-
> potential-bounds-Check-bypass-vulnerabilities.pdf
> 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Star Zeng 
> Cc: Eric Dong 
> 
> Hao Wu (5):
>   MdePkg/BaseLib: Add new LoadFence API
>   MdeModulePkg/FaultTolerantWrite:[CVE-2017-5753]Fix
> bounds check bypass
>   MdeModulePkg/SmmLockBox: [CVE-2017-5753] Fix bounds
> check bypass
>   MdeModulePkg/Variable: [CVE-2017-5753] Fix bounds
> check bypass
>   UefiCpuPkg/PiSmmCpuDxeSmm: [CVE-2017-5753] Fix bounds
> check bypass
> 
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultToler
> antWriteSmm.c   |  2 ++
> 
> MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultToler
> antWriteSmm.inf |  1 +
>  MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
> |  2 ++
>  MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
> |  1 +
> 
> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.
> c   |  3 ++
>  MdePkg/Include/Library/BaseLib.h
> | 12 +++
>  MdePkg/Library/BaseLib/Arm/LoadFence.c
> | 26 ++
>  MdePkg/Library/BaseLib/BaseLib.inf
> |  4 +++
>  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> | 15 +++-
>  MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
> | 37 +++
>  MdePkg/Library/BaseLib/X64/LoadFence.nasm
> | 38 
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> |  1 +
>  12 files changed, 141 insertions(+), 1 deletion(-)
>  create mode 100644
> MdePkg/Library/BaseLib/Arm/LoadFence.c
>  create mode 100644
> MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
>  create mode 100644
> MdePkg/Library/BaseLib/X64/LoadFence.nasm
> 
> --
> 2.12.0.windows.1

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


Re: [edk2] [PATCH v2 3/3] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

2018-09-20 Thread Zhang, Chao B
Hi Ard:
  I am good with this patch. I will help to push it.

From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Thursday, September 20, 2018 5:47 AM
To: Laszlo Ersek 
Cc: edk2-devel@lists.01.org; Zeng, Star ; Wang, Jian J 
; Kinney, Michael D ; Gao, 
Liming ; Zhang, Chao B ; Yao, 
Jiewen ; Leif Lindholm 
Subject: Re: [PATCH v2 3/3] SecurityPkg: remove PE/COFF header workaround for 
ELILO on IPF

On 7 September 2018 at 01:28, Laszlo Ersek 
mailto:ler...@redhat.com>> wrote:
> On 09/07/18 07:42, Ard Biesheuvel wrote:
>> Now that Itanium support has been dropped, we can remove the various
>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> mailto:ard.biesheu...@linaro.org>>
>> ---
>>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c   
>>  | 47 
>>  SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c 
>>  | 27 +++
>>  SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c 
>>  | 27 +++
>>  
>> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
>> | 25 +++
>>  4 files changed, 25 insertions(+), 101 deletions(-)
>
> Reviewed-by: Laszlo Ersek mailto:ler...@redhat.com>>
>

Chao, Jiewen: any concerns?
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] SPI Flash Corruption

2018-09-20 Thread Laszlo Ersek
On 09/19/18 16:26, Samah Mansour wrote:
> Hello,
> 
> 
> Our product uses a Baytrail with Minnowboard Max bios firmware ( version
> 0.93). Every now and then we see SPI flash corruption due to power cuts
> while the unit is booting which causes the unit not to boot anymore. After
> investigation we noticed that the VPD area is all FFs (address
> 44000->47DFF0).
> 
> 
> We have noticed that the Bios while booting writes to the flash from
> several places in the code, which is if interrupted most probably is
> causing the corruption.
> 
> 
> Why is the bios writing all these configurations to flash while booting, is
> it to optimize boot time? is it ok if we disable the bios writing to flash
> completely to protect ourselves from corruption?

The firmware is at liberty to write various non-volatile UEFI variables
during boot. Some of those variables are standardized, some others may
be specific to UEFI drivers (with correspondingly private namespace
GUIDs for the variables).

Power loss during flash write (and resultant flash corruption) is
expected. My understanding is that the Fault Tolerant Write protocol /
driver, sitting between the FVB (firmware volume block, i.e. flash)
protocol / driver, and the variable write protocol / driver, implements
a kind of journaling. It is described in the Intel whitepaper

  A Tour Beyond BIOS
  Implementing UEFI Authenticated Variables in SMM with EDKII
  September 2015

My expectation has been that the platform should recover from
interrupted writes. That is, for a single given UEFI variable, you
should either see "before" or "after" status, never "middle". (The
whitepaper says that "Individual variable atomicity" is maintained even
through a failed "reclaim", with the help of FTW.)

If multiple variables should be in sync with each other, that's a
different question. If the variables are not in sync, I think "failure
to boot" may be a reasonable outcome. But, "failure to boot" means a lot
of things, and I hope one should be at least dropped to the setup
utility or the shell. Are you seeing an actual crash?

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


Re: [edk2] [PATCH v1 0/5] [CVE-2017-5753] Bounds Check Bypass issue in SMI handlers

2018-09-20 Thread Laszlo Ersek
On 09/20/18 08:40, Hao Wu wrote:
> The series aims to mitigate the Bounds Check Bypass (CVE-2017-5753) issues
> within SMI handlers.
> 
> A more detailed explanation of the purpose of the series is under the
> 'Bounds check bypass mitigation' section of the below link:
> https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation
> 
> And the document at:
> https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf
> 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Star Zeng 
> Cc: Eric Dong 

I'd like to test this series, but before I do that, I'll wait a bit
longer for other review feedback. Please ping me, should I forget.

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


Re: [edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API

2018-09-20 Thread Laszlo Ersek
On 09/20/18 08:40, Hao Wu wrote:
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1193
> 
> This commit will add a new BaseLib API LoadFence(). This API will perform
> a serializing operation on all load-from-memory instructions that were
> issued prior to the call of this function.
> 
> The purpose of adding this API is to mitigate of the [CVE-2017-5753]
> Bounds Check Bypass issue when untrusted data are being processed within
> SMM. More details can be referred at the 'Bounds check bypass mitigation'
> section at the below link:
> 
> https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation
> 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  MdePkg/Include/Library/BaseLib.h   | 12 +++
>  MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 ++
>  MdePkg/Library/BaseLib/BaseLib.inf |  4 +++
>  MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 +++-
>  MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37 +++
>  MdePkg/Library/BaseLib/X64/LoadFence.nasm  | 38 
>  6 files changed, 131 insertions(+), 1 deletion(-)
> 
> diff --git a/MdePkg/Include/Library/BaseLib.h 
> b/MdePkg/Include/Library/BaseLib.h
> index 123ae19dc2..194726ca35 100644
> --- a/MdePkg/Include/Library/BaseLib.h
> +++ b/MdePkg/Include/Library/BaseLib.h
> @@ -4939,6 +4939,18 @@ MemoryFence (
>  
>  
>  /**
> +  Performs a serializing operation on all load-from-memory instructions that
> +  were issued prior to the call of this function.
> +
> +**/
> +VOID
> +EFIAPI
> +LoadFence (
> +  VOID
> +  );
> +
> +
> +/**
>Saves the current CPU context that can be restored with a call to 
> LongJump()
>and returns 0.
>  
> diff --git a/MdePkg/Library/BaseLib/Arm/LoadFence.c 
> b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> new file mode 100644
> index 00..69f0c3a07e
> --- /dev/null
> +++ b/MdePkg/Library/BaseLib/Arm/LoadFence.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  LoadFence() function for ARM.
> +
> +  Copyright (C) 2018, Intel Corporation. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +/**
> +  Performs a serializing operation on all load-from-memory instructions that
> +  were issued prior to the call of this function.
> +
> +**/
> +VOID
> +EFIAPI
> +LoadFence (
> +  VOID
> +  )
> +{
> +}
> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
> b/MdePkg/Library/BaseLib/BaseLib.inf
> index a1b5ec4b75..f028fbc75a 100644
> --- a/MdePkg/Library/BaseLib/BaseLib.inf
> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> @@ -68,6 +68,7 @@
>  
>  [Sources.Ia32]
>Ia32/WriteTr.nasm
> +  Ia32/LoadFence.nasm
>  
>Ia32/Wbinvd.c | MSFT
>Ia32/WriteMm7.c | MSFT
> @@ -346,6 +347,7 @@
>X64/EnableCache.nasm
>X64/DisableCache.nasm
>X64/WriteTr.nasm
> +  X64/LoadFence.nasm
>  
>X64/CpuBreakpoint.c | MSFT
>X64/WriteMsr64.c | MSFT
> @@ -580,6 +582,7 @@
>  [Sources.ARM]
>Arm/InternalSwitchStack.c
>Arm/Unaligned.c
> +  Arm/LoadFence.c
>Math64.c   | RVCT
>Math64.c   | MSFT
>  
> @@ -613,6 +616,7 @@
>  [Sources.AARCH64]
>Arm/InternalSwitchStack.c
>Arm/Unaligned.c
> +  Arm/LoadFence.c
>Math64.c
>  
>AArch64/MemoryFence.S | GCC
> diff --git a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c 
> b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> index 9b7d875664..a79461cfbf 100644
> --- a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> +++ b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
> @@ -1,7 +1,7 @@
>  /** @file
>Base Library CPU Functions for EBC
>  
> -  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
> +  Copyright (c) 2006 - 2018, 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
> @@ -52,6 +52,19 @@ MemoryFence (
>  }
>  
>  /**
> +  Performs a serializing operation on all load-from-memory instructions that
> +  were issued prior to the call of this function.
> +
> +**/
> +VOID
> +EFIAPI
> +LoadFence (
> +  VOID
> +  )
> +{
> +}
> +
> +/**
>Disables CPU interrupts.
>  
>  **/
> diff --git a/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm 
> b/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
> new file mode 100644
> index 00..11600bea76
> --- 

Re: [edk2] [PATCH v2 0/2] clarify NXE enabling logic

2018-09-20 Thread Laszlo Ersek
On 09/20/18 08:02, Jian J Wang wrote:
>> v2 changes:
>>Incorporates review comments from Laszlo and Star.
> 
> BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116
> 
> Test:
> a. try all related PCDs combinations and check the page table attributes
> b. boot to shell on real intel platform with valid PCD setting combinations
> (IA32/X64)
> c. boot to fedora26, ubuntu18.04, windows 7 and windows 10 on OVMF emulated
> platform (X64)
> 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Cc: Ard Biesheuvel 
> Cc: Ruiyu Ni 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> 
> Jian J Wang (2):
>   MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage
>   MdeModulePkg/DxeIpl: support more NX related PCDs
> 
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 ++
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  4 ++--
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 30 
> +++-
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 24 +++
>  MdeModulePkg/MdeModulePkg.dec| 20 
>  MdeModulePkg/MdeModulePkg.uni| 13 ++
>  6 files changed, 81 insertions(+), 12 deletions(-)
> 

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


Re: [edk2] [Patch 0/5] Support windowsize to benefit tftp/pxe download performance.

2018-09-20 Thread Laszlo Ersek
On 09/20/18 07:54, Wu, Jiaxin wrote:
> Hi Laszlo,
>
> I agree there is no document to describe the detailed difference
> against the overlapped network drivers the between NetworkPkg and
> MdeModulePkg (except IPv4/IPv6 support ). We only declared that those
> drivers should not be used at the same
> (https://github.com/tianocore/tianocore.github.io/wiki/NetworkPkg-Getting-Started-Guide).
>
> Actually, the problem you mentioned here only exists in ISCSI/TCP/PXE
> drivers - Tcp4Dxe VS TcpDxe, IScsiDxe VS IScsiDxe,  UefiPxeBcDxe VS
> UefiPxeBcDxe. So, as you said, it's the time for us to add some
> declaration somewhere (inf or wiki) -- For those three drivers in
> MdeModulePkg,  they will remain unchanged until there is any specific
> requirement that we need fix any issue. That will greatly reduce the
> effort to maintain/test those combine of two drivers.  So, we don't
> recommend to use those three drivers in MdeModulePkg because they
> might some issues, which has been fixed in the NetworkPkg drivers. If
> you agree, we will add some statement in the corresponding *.inf
> files.

In OVMF, we now have:

> !if $(NETWORK_IP6_ENABLE) == TRUE
>   NetworkPkg/Ip6Dxe/Ip6Dxe.inf
>   NetworkPkg/TcpDxe/TcpDxe.inf
>   NetworkPkg/Udp6Dxe/Udp6Dxe.inf
>   NetworkPkg/Dhcp6Dxe/Dhcp6Dxe.inf
>   NetworkPkg/Mtftp6Dxe/Mtftp6Dxe.inf
>   NetworkPkg/UefiPxeBcDxe/UefiPxeBcDxe.inf
>   NetworkPkg/IScsiDxe/IScsiDxe.inf
> !else
>   MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Dxe.inf
>   MdeModulePkg/Universal/Network/UefiPxeBcDxe/UefiPxeBcDxe.inf
>   MdeModulePkg/Universal/Network/IScsiDxe/IScsiDxe.inf
> !endif

My understanding has been that the three drivers from
MdeModulePkg/Universal/Network/, namely Tcp4Dxe, UefiPxeBcDxe, IScsiDxe,
only lack features (mainly IPv6, but perhaps some more).

However, your email suggests that the three drivers in question could
even miss fixes to known bugs.

If that's the case, should we consider the three drivers under
"MdeModulePkg/Universal/Network/" deprecated, and should we abandon them
completely?

If that's the case, then it is really important to document.

Because, if a user reports a networking-related error (after building
OVMF without NETWORK_IP6_ENABLE), then I wouldn't like to start
investigating, just to find out that the issue was fixed in NetworkPkg a
year earlier.

Furthermore, if the MdeModulePkg/Universal/Network/ drivers are
deprecated, when do we intend to actually remove them from the tree?

Oh, wait, is this related to OpenSSL? When including IScsiDxe from
NetworkPkg, OpenSSL is required. When including IScsiDxe from
MdeModulePkg, OpenSSL is not required -- but the networking
functionality may have some known bugs (which are already fixed in
NetworkPkg).

Is this the real trade-off?

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


[edk2] question about uefi shell-current working directory and shell-script

2018-09-20 Thread krishnaLee
Hi,
I wonder if it is a bug:
I have a usb Fat32-disk,installed with EDK2 shell2.6 or 2.7,
the root directory has a startup.nsh,the startup.nsh has only one line:"cd \",
I boot my cannolake machine with this usb disk,
in the uefi shell,I got a message:"cd: current directory not specified".it 
means the cwd environment variable was not set when start doing script.


I think it may be  an error,because the uefi shell can find the startup.nsh,why 
not set cwd(current working directory) before execute this script,so the cd 
command works fine.
I mention it because I found if I put a uefi application(using 
efi_shell_protocol to access .\logfile.txt) in this startup.nsh,the uefi 
application can't access files,
 because I found the efi_shell_protocol need cwd to work too,such as 
efi_shell_protocol.OpenFileByName().






thank you,
krishna



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


Re: [edk2] [Patch] MdeModulePkg RegularExpressionDxe: Update Oniguruma to 6.9.0

2018-09-20 Thread Shia, Cinnamon
Hi Liming and Dongao,

Thank you for making this change. It looks good to me and verified it with 
VS2015.

Reviewed-by: Cinnamon Shia 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Friday, September 7, 2018 10:13 PM
To: edk2-devel@lists.01.org
Cc: Cecil Sheng ; Dongao Guo 
Subject: [edk2] [Patch] MdeModulePkg RegularExpressionDxe: Update Oniguruma to 
6.9.0

From: Dongao Guo 

The change is in https://github.com/lgao4/edk2/tree/Oniguruma

Update Oniguruma to the latest version v6.9.0.
Oniguruma https://github.com/kkos/oniguruma Verify VS2017, GCC5 build.
Verify RegularExpressionProtocol GetInfo() and Match() function.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dongao Guo 
Reviewed-by: Liming Gao 
Cc: Cecil Sheng 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH] MdeModulePkg CapsuleApp:Remove two redundant Guids

2018-09-20 Thread shenglei
Remove two redundant Guids which are not used.
They are gEfiCertTypeRsa2048Sha256Guid and
gEfiCertPkcs7Guid.This is an improved version of
https://bugzilla.tianocore.org/show_bug.cgi?id=1062
Cc: Star Zeng 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf | 2 --
 1 file changed, 2 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
index 3a67c6b909..3e2217ea84 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.inf
@@ -44,8 +44,6 @@
   gEfiCapsuleReportGuid  ## CONSUMES   ## GUID
   gEfiFmpCapsuleGuid ## CONSUMES   ## GUID
   gWindowsUxCapsuleGuid  ## CONSUMES   ## GUID
-  gEfiCertTypeRsa2048Sha256Guid  ## CONSUMES   ## GUID
-  gEfiCertPkcs7Guid  ## CONSUMES   ## GUID
   gEfiSystemResourceTableGuid## CONSUMES   ## GUID
 
 [Protocols]
-- 
2.18.0.windows.1

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


Re: [edk2] [PATCH v2] UefiCpuPkg: Remove redundant library classes, Ppis and GUIDs

2018-09-20 Thread Dong, Eric
Reviewed-by: Eric Dong 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> shenglei
> Sent: Wednesday, September 19, 2018 11:02 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric 
> Subject: [edk2] [PATCH v2] UefiCpuPkg: Remove redundant library classes,
> Ppis and GUIDs
> 
> Some redundant library classes Ppis and GUIDs
> have been removed in inf, .c and .h files.
> 
> v2:
> 1.Remove ReadOnlyVariable2.h in S3Resume.c which should be
>   deleted in last version in which gEfiPeiReadOnlyVariable2PpiGuid
>   was removed.
> 2.Remove the library class BaseLib in CpuPageTable.c
>   which is included elsewhere.
> 3.Add library classes in SecCore.inf which are removed
>   at last version.
>   They are DebugAgentLib and CpuExceptionHandlerLib.
> 4.Add two Ppis in SecCore.inf which are removed
>   at last version.
>   They are gEfiSecPlatformInformationPpiGuid and
>   gEfiSecPlatformInformation2PpiGuid.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1043
> https://bugzilla.tianocore.org/show_bug.cgi?id=1013
> https://bugzilla.tianocore.org/show_bug.cgi?id=1032
> https://bugzilla.tianocore.org/show_bug.cgi?id=1016
> 
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei 
> ---
>  UefiCpuPkg/CpuDxe/CpuPageTable.c| 7 ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h  | 1 -
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf| 2 --
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h  | 1 -
>  UefiCpuPkg/SecCore/FindPeiCore.c| 2 --
>  UefiCpuPkg/SecCore/SecCore.inf  | 4 
>  UefiCpuPkg/SecCore/SecMain.h| 2 --
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c   | 4 +---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 3 ---
>  9 files changed, 1 insertion(+), 25 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> index 0a980b9753..33e8ee2d2c 100644
> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> @@ -16,17 +16,10 @@
> 
>  #include 
>  #include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
> -#include 
>  #include 
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index e3c7cff81c..8c7f4996d1 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -38,7 +38,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index a7fb7b0b14..95a4511225 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -77,7 +77,6 @@
>  [LibraryClasses]
>UefiDriverEntryPoint
>UefiRuntimeServicesTableLib
> -  CacheMaintenanceLib
>PcdLib
>DebugLib
>BaseLib
> @@ -113,7 +112,6 @@
> 
>  [Guids]
>gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it 
> is
> used for S3 boot.
> -  gEfiGlobalVariableGuid   ## SOMETIMES_PRODUCES ##
> Variable:L"SmmProfileData"
>gEfiAcpi20TableGuid  ## SOMETIMES_CONSUMES ##
> SystemTable
>gEfiAcpi10TableGuid  ## SOMETIMES_CONSUMES ##
> SystemTable
>gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> index bacb2f8ad3..18a7fe3b07 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h
> @@ -15,7 +15,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #ifndef _SMM_PROFILE_INTERNAL_H_
>  #define _SMM_PROFILE_INTERNAL_H_
> 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git a/UefiCpuPkg/SecCore/FindPeiCore.c
> b/UefiCpuPkg/SecCore/FindPeiCore.c
> index 60ccaa9667..bb9c434d1e 100644
> --- a/UefiCpuPkg/SecCore/FindPeiCore.c
> +++ b/UefiCpuPkg/SecCore/FindPeiCore.c
> @@ -13,8 +13,6 @@
>  **/
> 
>  #include 
> -#include 
> -#include 
> 
>  #include "SecMain.h"
> 
> diff --git a/UefiCpuPkg/SecCore/SecCore.inf
> b/UefiCpuPkg/SecCore/SecCore.inf
> index 7bcd4f8a96..b228610757 100644
> --- a/UefiCpuPkg/SecCore/SecCore.inf
> +++ b/UefiCpuPkg/SecCore/SecCore.inf
> @@ -50,7 +50,6 @@
>  [LibraryClasses]
>BaseMemoryLib
>DebugLib
> -  BaseLib
>PlatformSecLib
>PcdLib
>DebugAgentLib
> @@ -71,9 +70,6 @@
>## SOMETIMES_PRODUCES
>gEfiSecPlatformInformation2PpiGuid
>gEfiTemporaryRamDonePpiGuid  ## 

Re: [edk2] [PATCH v2 0/9] BaseTools: refactor Workspace classes

2018-09-20 Thread Zhu, Yonghong
Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jaben 
Carsey
Sent: Tuesday, September 11, 2018 6:18 AM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2 0/9] BaseTools: refactor Workspace classes

update the classes for the following:
1) use decorators for property
2) use decorators for caching property and caching function
  - this allows for objects to reduce in size as they get used
3) remove unused variables and properties
4) use tuple instead of custom class when apropriate
5) remove callers from accessing "private" data and use the existing properties
6) removed a circular dependency between APIs

v2:
fix error where class attribute M was accidentally removed.

Jaben Carsey (9):
  BaseTools: Refactor PlatformAutoGen
  BaseTools: AutoGen refactor WorkspaceAutoGen class
  BaseTools: AutoGen - refactor class properties
  BaseTools: refactor class properties
  BaseTools: Workspace classes refactor properties
  BaseTools: refactor Build Database objects
  BaseTools: Don't save unused workspace data
  BaseTools: refactor to not overcreate ModuleAutoGen objects
  BaseTools: refactor to cache InfBuildData data

 BaseTools/Source/Python/AutoGen/AutoGen.py | 692 +++---
 BaseTools/Source/Python/AutoGen/GenMake.py |  20 +-
 BaseTools/Source/Python/Common/Misc.py |  90 +-
 BaseTools/Source/Python/GenFds/FfsInfStatement.py  |   4 +-
 BaseTools/Source/Python/Workspace/BuildClassObject.py  |  39 +-
 BaseTools/Source/Python/Workspace/DecBuildData.py  |  65 +-
 BaseTools/Source/Python/Workspace/DscBuildData.py  | 151 ++--
 BaseTools/Source/Python/Workspace/InfBuildData.py  | 954 
+---
 BaseTools/Source/Python/Workspace/MetaFileParser.py|  18 +-
 BaseTools/Source/Python/Workspace/WorkspaceDatabase.py |  16 +-
 BaseTools/Source/Python/build/build.py |   4 +-
 11 files changed, 933 insertions(+), 1120 deletions(-)

--
2.16.2.windows.1

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


Re: [edk2] [Patch] SecurityPkg/TcgStorageOpalLib: Fixed correct user password not works issue.

2018-09-20 Thread Wu, Hao A
Reviewed-by: Hao Wu 

Best Regards,
Hao Wu


> -Original Message-
> From: Dong, Eric
> Sent: Monday, September 17, 2018 11:11 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A; Chu, Maggie
> Subject: [Patch] SecurityPkg/TcgStorageOpalLib: Fixed correct user password
> not works issue.
> 
> After admin password reach the TryLimit value, code logic will direct return
> error password result no matter which password been inputted. So even correct
> user password will return TryLimit error.
> 
> Now update code logic to also check user password. Only when both
> user/admin
> password reach the TryLimit count, code will return exceed TryLimit error.
> 
> Cc: Wu Hao 
> Cc: Chu Maggie 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> index 3c4a8e9001..f2febc0a0c 100644
> --- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> +++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalUtil.c
> @@ -763,7 +763,6 @@ OpalUtilUpdateGlobalLockingRange(
> 
>if (MethodStatus ==
> TCG_METHOD_STATUS_CODE_AUTHORITY_LOCKED_OUT) {
>  DEBUG ((DEBUG_INFO, "unlock as admin failed with
> AUTHORITY_LOCKED_OUT\n"));
> -goto done;
>}
> 
>//
> --
> 2.15.0.windows.1

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


[edk2] [PATCH v1 5/5] UefiCpuPkg/PiSmmCpuDxeSmm: [CVE-2017-5753] Fix bounds check bypass

2018-09-20 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1194

Speculative execution is used by processor to avoid having to wait for
data to arrive from memory, or for previous operations to finish, the
processor may speculate as to what will be executed.

If the speculation is incorrect, the speculatively executed instructions
might leave hints such as which memory locations have been brought into
cache. Malicious actors can use the bounds check bypass method (code
gadgets with controlled external inputs) to infer data values that have
been used in speculative operations to reveal secrets which should not
otherwise be accessed.

It is possible for SMI handler(s) to call EFI_SMM_CPU_PROTOCOL service
ReadSaveState() and use the content in the 'CommBuffer' (controlled
external inputs) as the 'CpuIndex'. So this commit will insert LoadFence
API to mitigate the bounds check bypass issue within SmmReadSaveState().

For SmmReadSaveState():

The 'CpuIndex' will be passed into function ReadSaveStateRegister(). And
then in to ReadSaveStateRegisterByIndex().

With the call:
ReadSaveStateRegisterByIndex (
  CpuIndex,
  SMM_SAVE_STATE_REGISTER_IOMISC_INDEX,
  sizeof(IoMisc.Uint32),
  
  );

The 'IoMisc' can be a cross boundary access during speculative execution.
Later, 'IoMisc' is used as the index to access buffers 'mSmmCpuIoWidth'
and 'mSmmCpuIoType'. One can observe which part of the content within
those buffers was brought into cache to possibly reveal the value of
'IoMisc'.

Hence, this commit adds a LoadFence after the check of 'CpuIndex' within
function SmmReadSaveState() to prevent the speculative execution.

A more detailed explanation of the purpose of commit is under the
'Bounds check bypass mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation

And the document at:
https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf

Cc: Laszlo Ersek 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
index fbf74e8d90..256a8bbb94 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
@@ -237,6 +237,7 @@ SmmReadSaveState (
   if ((CpuIndex >= gSmst->NumberOfCpus) || (Buffer == NULL)) {
 return EFI_INVALID_PARAMETER;
   }
+  LoadFence ();
 
   //
   // Check for special EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID
-- 
2.12.0.windows.1

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


[edk2] [PATCH v1 2/5] MdeModulePkg/FaultTolerantWrite:[CVE-2017-5753]Fix bounds check bypass

2018-09-20 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1194

Speculative execution is used by processor to avoid having to wait for
data to arrive from memory, or for previous operations to finish, the
processor may speculate as to what will be executed.

If the speculation is incorrect, the speculatively executed instructions
might leave hints such as which memory locations have been brought into
cache. Malicious actors can use the bounds check bypass method (code
gadgets with controlled external inputs) to infer data values that have
been used in speculative operations to reveal secrets which should not
otherwise be accessed.

This commit will focus on the SMI handler(s) registered within the
FaultTolerantWriteDxe driver and insert LoadFence API to mitigate the
bounds check bypass issue.

For SMI handler SmmFaultTolerantWriteHandler():

Under "case FTW_FUNCTION_WRITE:", 'SmmFtwWriteHeader->Length' can be a
potential cross boundary access of the 'CommBuffer' (controlled external
inputs) during speculative execution. This cross boundary access is later
passed as parameter 'Length' into function FtwWrite().

Within function FtwWrite(), the value of 'Length' can be inferred by code:
"CopyMem (MyBuffer + Offset, Buffer, Length);". One can observe which part
of the content within 'Buffer' was brought into cache to possibly reveal
the value of 'Length'.

Hence, this commit adds a LoadFence after the boundary/range checks of
'CommBuffer' to prevent the speculative execution.

A more detailed explanation of the purpose of commit is under the
'Bounds check bypass mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation

And the document at:
https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf

Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c   | 2 ++
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf | 1 +
 2 files changed, 3 insertions(+)

diff --git 
a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c 
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
index 632313f076..2ed1bb9498 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c
@@ -57,6 +57,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include "FaultTolerantWrite.h"
 #include "FaultTolerantWriteSmmCommon.h"
@@ -417,6 +418,7 @@ SmmFaultTolerantWriteHandler (
  
  );
   if (!EFI_ERROR (Status)) {
+LoadFence ();
 Status = FtwWrite(
>FtwInstance,
SmmFtwWriteHeader->Lba,
diff --git 
a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf 
b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
index 85d109e8d9..606cc2266b 100644
--- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
+++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf
@@ -55,6 +55,7 @@
   PcdLib
   ReportStatusCodeLib
   SmmMemLib
+  BaseLib
 
 [Guids]
   #
-- 
2.12.0.windows.1

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


[edk2] [PATCH v1 3/5] MdeModulePkg/SmmLockBox: [CVE-2017-5753] Fix bounds check bypass

2018-09-20 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1194

Speculative execution is used by processor to avoid having to wait for
data to arrive from memory, or for previous operations to finish, the
processor may speculate as to what will be executed.

If the speculation is incorrect, the speculatively executed instructions
might leave hints such as which memory locations have been brought into
cache. Malicious actors can use the bounds check bypass method (code
gadgets with controlled external inputs) to infer data values that have
been used in speculative operations to reveal secrets which should not
otherwise be accessed.

This commit will focus on the SMI handler(s) registered within the
SmmLockBox driver and insert LoadFence API to mitigate the
bounds check bypass issue.

For SMI handler SmmLockBoxHandler():

Under "case EFI_SMM_LOCK_BOX_COMMAND_SAVE:", the 'CommBuffer' (controlled
external inputs) is passed to function SmmLockBoxSave().

'TempLockBoxParameterSave.Length' can be a potential cross boundary access
of the 'CommBuffer' during speculative execution. This cross boundary
access is later passed as parameter 'Length' into function SaveLockBox().

Within function SaveLockBox(), the value of 'Length' can be inferred by
code:
"CopyMem ((VOID *)(UINTN)SmramBuffer, (VOID *)(UINTN)Buffer, Length);".
One can observe which part of the content within 'Buffer' was brought into
cache to possibly reveal the value of 'Length'.

Hence, this commit adds a LoadFence after the boundary/range checks of
'CommBuffer' to prevent the speculative execution.

And there is a similar case under "case EFI_SMM_LOCK_BOX_COMMAND_UPDATE:"
function SmmLockBoxUpdate() as well. This commits also handles it.

A more detailed explanation of the purpose of commit is under the
'Bounds check bypass mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation

And the document at:
https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf

Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c 
b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
index 5a11743cb9..87b4947908 100644
--- a/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
+++ b/MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c
@@ -76,6 +76,7 @@ SmmLockBoxSave (
 LockBoxParameterSave->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
 return ;
   }
+  LoadFence ();
 
   //
   // Save data
@@ -160,6 +161,7 @@ SmmLockBoxUpdate (
 LockBoxParameterUpdate->Header.ReturnStatus = (UINT64)EFI_ACCESS_DENIED;
 return ;
   }
+  LoadFence ();
 
   //
   // Update data
-- 
2.12.0.windows.1

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


[edk2] [PATCH v1 4/5] MdeModulePkg/Variable: [CVE-2017-5753] Fix bounds check bypass

2018-09-20 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1194

Speculative execution is used by processor to avoid having to wait for
data to arrive from memory, or for previous operations to finish, the
processor may speculate as to what will be executed.

If the speculation is incorrect, the speculatively executed instructions
might leave hints such as which memory locations have been brought into
cache. Malicious actors can use the bounds check bypass method (code
gadgets with controlled external inputs) to infer data values that have
been used in speculative operations to reveal secrets which should not
otherwise be accessed.

This commit will focus on the SMI handler(s) registered within the
Variable\RuntimeDxe driver and insert LoadFence API to mitigate the
bounds check bypass issue.

For SMI handler SmmVariableHandler():

Under "case SMM_VARIABLE_FUNCTION_GET_VARIABLE:",
'SmmVariableHeader->NameSize' can be a potential cross boundary access of
the 'CommBuffer' (controlled external input) during speculative execution.

This cross boundary access is later used as the index to access array
'SmmVariableHeader->Name' by code:
"SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1]"
One can observe which part of the content within array was brought into
cache to possibly reveal the value of 'SmmVariableHeader->NameSize'.

Hence, this commit adds a LoadFence after the boundary/range checks of
'CommBuffer' to prevent the speculative execution.

And there are 2 similar cases under
"case SMM_VARIABLE_FUNCTION_SET_VARIABLE:" and
"case SMM_VARIABLE_FUNCTION_VAR_CHECK_VARIABLE_PROPERTY_GET:" as well.
This commits also handles them.

Also, under "case SMM_VARIABLE_FUNCTION_SET_VARIABLE:",
'(UINT8 *)SmmVariableHeader->Name + SmmVariableHeader->NameSize' points to
the 'CommBuffer' (with some offset) and then passed as parameter 'Data' to
function VariableServiceSetVariable().

Within function VariableServiceSetVariable(), there is a sanity check for
EFI_VARIABLE_AUTHENTICATION_2 descriptor for the data pointed by 'Data'.
If this check is speculatively bypassed, potential cross-boundary data
access for 'Data' is possible to be revealed via the below function calls
sequence during speculative execution:

AuthVariableLibProcessVariable()
ProcessVarWithPk() or ProcessVarWithKek()

Within function ProcessVarWithPk() or ProcessVarWithKek(), for the code
"PayloadSize = DataSize - AUTHINFO2_SIZE (Data);", 'AUTHINFO2_SIZE (Data)'
can be a cross boundary access during speculative execution.

Then, 'PayloadSize' is possible to be revealed by the function call
sequence:

AuthServiceInternalUpdateVariableWithTimeStamp()
mAuthVarLibContextIn->UpdateVariable()
VariableExLibUpdateVariable()
UpdateVariable()
CopyMem()

Hence, this commit adds a LoadFence after the sanity check for
EFI_VARIABLE_AUTHENTICATION_2 descriptor upon 'Data' within function
VariableServiceSetVariable() to prevent the speculative execution.

A more detailed explanation of the purpose of commit is under the
'Bounds check bypass mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation

And the document at:
https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf

Cc: Jiewen Yao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c| 1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
index 1ea2f84dda..52af56c4c0 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c
@@ -3198,6 +3198,7 @@ VariableServiceSetVariable (
   ((EFI_VARIABLE_AUTHENTICATION_2 *) Data)->AuthInfo.Hdr.dwLength < 
OFFSET_OF (WIN_CERTIFICATE_UEFI_GUID, CertData)) {
   return EFI_SECURITY_VIOLATION;
 }
+LoadFence ();
 PayloadSize = DataSize - AUTHINFO2_SIZE (Data);
   } else {
 PayloadSize = DataSize;
diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c 
b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
index e495d971a0..0bbed71a76 100644
--- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
+++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c
@@ -537,6 +537,7 @@ SmmVariableHandler (
 goto EXIT;
   }
 
+  LoadFence ();
   if (SmmVariableHeader->NameSize < sizeof (CHAR16) || 
SmmVariableHeader->Name[SmmVariableHeader->NameSize/sizeof (CHAR16) - 1] != 
L'\0') {
 //
 // Make sure VariableName is A Null-terminated string.
@@ -631,6 +632,7 @@ SmmVariableHandler (
 goto EXIT;
   }
 
+  LoadFence ();
   if 

[edk2] [PATCH v1 1/5] MdePkg/BaseLib: Add new LoadFence API

2018-09-20 Thread Hao Wu
REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1193

This commit will add a new BaseLib API LoadFence(). This API will perform
a serializing operation on all load-from-memory instructions that were
issued prior to the call of this function.

The purpose of adding this API is to mitigate of the [CVE-2017-5753]
Bounds Check Bypass issue when untrusted data are being processed within
SMM. More details can be referred at the 'Bounds check bypass mitigation'
section at the below link:

https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation

Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdePkg/Include/Library/BaseLib.h   | 12 +++
 MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 ++
 MdePkg/Library/BaseLib/BaseLib.inf |  4 +++
 MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 +++-
 MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37 +++
 MdePkg/Library/BaseLib/X64/LoadFence.nasm  | 38 
 6 files changed, 131 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 123ae19dc2..194726ca35 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -4939,6 +4939,18 @@ MemoryFence (
 
 
 /**
+  Performs a serializing operation on all load-from-memory instructions that
+  were issued prior to the call of this function.
+
+**/
+VOID
+EFIAPI
+LoadFence (
+  VOID
+  );
+
+
+/**
   Saves the current CPU context that can be restored with a call to LongJump()
   and returns 0.
 
diff --git a/MdePkg/Library/BaseLib/Arm/LoadFence.c 
b/MdePkg/Library/BaseLib/Arm/LoadFence.c
new file mode 100644
index 00..69f0c3a07e
--- /dev/null
+++ b/MdePkg/Library/BaseLib/Arm/LoadFence.c
@@ -0,0 +1,26 @@
+/** @file
+  LoadFence() function for ARM.
+
+  Copyright (C) 2018, Intel Corporation. All rights reserved.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+/**
+  Performs a serializing operation on all load-from-memory instructions that
+  were issued prior to the call of this function.
+
+**/
+VOID
+EFIAPI
+LoadFence (
+  VOID
+  )
+{
+}
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf 
b/MdePkg/Library/BaseLib/BaseLib.inf
index a1b5ec4b75..f028fbc75a 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -68,6 +68,7 @@
 
 [Sources.Ia32]
   Ia32/WriteTr.nasm
+  Ia32/LoadFence.nasm
 
   Ia32/Wbinvd.c | MSFT
   Ia32/WriteMm7.c | MSFT
@@ -346,6 +347,7 @@
   X64/EnableCache.nasm
   X64/DisableCache.nasm
   X64/WriteTr.nasm
+  X64/LoadFence.nasm
 
   X64/CpuBreakpoint.c | MSFT
   X64/WriteMsr64.c | MSFT
@@ -580,6 +582,7 @@
 [Sources.ARM]
   Arm/InternalSwitchStack.c
   Arm/Unaligned.c
+  Arm/LoadFence.c
   Math64.c   | RVCT
   Math64.c   | MSFT
 
@@ -613,6 +616,7 @@
 [Sources.AARCH64]
   Arm/InternalSwitchStack.c
   Arm/Unaligned.c
+  Arm/LoadFence.c
   Math64.c
 
   AArch64/MemoryFence.S | GCC
diff --git a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c 
b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
index 9b7d875664..a79461cfbf 100644
--- a/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
+++ b/MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c
@@ -1,7 +1,7 @@
 /** @file
   Base Library CPU Functions for EBC
 
-  Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2018, 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
@@ -52,6 +52,19 @@ MemoryFence (
 }
 
 /**
+  Performs a serializing operation on all load-from-memory instructions that
+  were issued prior to the call of this function.
+
+**/
+VOID
+EFIAPI
+LoadFence (
+  VOID
+  )
+{
+}
+
+/**
   Disables CPU interrupts.
 
 **/
diff --git a/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm 
b/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
new file mode 100644
index 00..11600bea76
--- /dev/null
+++ b/MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
@@ -0,0 +1,37 @@
+;--
 ;
+; Copyright (c) 2018, 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 

[edk2] [PATCH v1 0/5] [CVE-2017-5753] Bounds Check Bypass issue in SMI handlers

2018-09-20 Thread Hao Wu
The series aims to mitigate the Bounds Check Bypass (CVE-2017-5753) issues
within SMI handlers.

A more detailed explanation of the purpose of the series is under the
'Bounds check bypass mitigation' section of the below link:
https://software.intel.com/security-software-guidance/insights/host-firmware-speculative-execution-side-channel-mitigation

And the document at:
https://software.intel.com/security-software-guidance/api-app/sites/default/files/337879-analyzing-potential-bounds-Check-bypass-vulnerabilities.pdf

Cc: Ard Biesheuvel 
Cc: Laszlo Ersek 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Star Zeng 
Cc: Eric Dong 

Hao Wu (5):
  MdePkg/BaseLib: Add new LoadFence API
  MdeModulePkg/FaultTolerantWrite:[CVE-2017-5753]Fix bounds check bypass
  MdeModulePkg/SmmLockBox: [CVE-2017-5753] Fix bounds check bypass
  MdeModulePkg/Variable: [CVE-2017-5753] Fix bounds check bypass
  UefiCpuPkg/PiSmmCpuDxeSmm: [CVE-2017-5753] Fix bounds check bypass

 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.c   |  2 ++
 MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteSmm.inf |  1 +
 MdeModulePkg/Universal/LockBox/SmmLockBox/SmmLockBox.c |  2 ++
 MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c  |  1 +
 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c   |  3 ++
 MdePkg/Include/Library/BaseLib.h   | 12 
+++
 MdePkg/Library/BaseLib/Arm/LoadFence.c | 26 
++
 MdePkg/Library/BaseLib/BaseLib.inf |  4 +++
 MdePkg/Library/BaseLib/Ebc/CpuBreakpoint.c | 15 
+++-
 MdePkg/Library/BaseLib/Ia32/LoadFence.nasm | 37 
+++
 MdePkg/Library/BaseLib/X64/LoadFence.nasm  | 38 

 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  1 +
 12 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 MdePkg/Library/BaseLib/Arm/LoadFence.c
 create mode 100644 MdePkg/Library/BaseLib/Ia32/LoadFence.nasm
 create mode 100644 MdePkg/Library/BaseLib/X64/LoadFence.nasm

-- 
2.12.0.windows.1

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


[edk2] [PATCH v2 2/2] MdeModulePkg/DxeIpl: support more NX related PCDs

2018-09-20 Thread Jian J Wang
> v2 changes:
>a. remove macros no longer needed
>b. remove DEBUG and ASSERT in ToEnableExecuteDisableFeature()
>c. change ToEnableExecuteDisableFeature to EnableNonExec

BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Currently IA32_EFER.NXE is only set against PcdSetNxForStack. This
confuses developers because following two other PCDs also need NXE
to be set, but actually not.

PcdDxeNxMemoryProtectionPolicy
PcdImageProtectionPolicy

This patch solves this issue by adding logic to enable IA32_EFER.NXE
if any of those PCDs have anything enabled.

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 ++
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  4 ++--
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 30 +++-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 24 +++
 4 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index fd82657404..068e700074 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -117,6 +117,8 @@
 
 [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack   ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## 
SOMETIMES_CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy   ## 
SOMETIMES_CONSUMES
 
 [Depex]
   gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index d28baa3615..ccd30f964b 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -245,7 +245,7 @@ ToBuildPageTable (
 return TRUE;
   }
 
-  if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) {
+  if (EnableNonExec ()) {
 return TRUE;
   }
 
@@ -436,7 +436,7 @@ HandOffToDxeCore (
 BuildPageTablesIa32Pae = ToBuildPageTable ();
 if (BuildPageTablesIa32Pae) {
   PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE);
-  if (IsExecuteDisableBitAvailable ()) {
+  if (EnableNonExec ()) {
 EnableExecuteDisableBit();
   }
 }
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c 
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 496e219913..73b0f67c6b 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -106,6 +106,31 @@ IsNullDetectionEnabled (
   return ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) != 0);
 }
 
+/**
+  Check if Execute Disable Bit (IA32_EFER.NXE) should be enabled or not.
+
+  @retval TRUEIA32_EFER.NXE should be enabled.
+  @retval FALSE   IA32_EFER.NXE should not be enabled.
+
+**/
+BOOLEAN
+EnableNonExec (
+  VOID
+  )
+{
+  if (!IsExecuteDisableBitAvailable ()) {
+return FALSE;
+  }
+
+  //
+  // XD flag (BIT63) in page table entry is only valid if IA32_EFER.NXE is set.
+  // Features controlled by Following PCDs need this feature to be enabled.
+  //
+  return (PcdGetBool (PcdSetNxForStack) ||
+  PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 ||
+  PcdGet32 (PcdImageProtectionPolicy) != 0);
+}
+
 /**
   Enable Execute Disable Bit.
 
@@ -755,7 +780,10 @@ CreateIdentityMappingPageTables (
   //
   EnablePageTableProtection ((UINTN)PageMap, TRUE);
 
-  if (PcdGetBool (PcdSetNxForStack)) {
+  //
+  // Set IA32_EFER.NXE if necessary.
+  //
+  if (EnableNonExec ()) {
 EnableExecuteDisableBit ();
   }
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h 
b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
index 85457ff937..09085312aa 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
@@ -179,6 +179,30 @@ typedef struct {
   UINTN   FreePages;
 } PAGE_TABLE_POOL;
 
+/**
+  Check if Execute Disable Bit (IA32_EFER.NXE) should be enabled or not.
+
+  @retval TRUEIA32_EFER.NXE should be enabled.
+  @retval FALSE   IA32_EFER.NXE should not be enabled.
+
+**/
+BOOLEAN
+EnableNonExec (
+  VOID
+  );
+
+/**
+  The function will check if Execute Disable Bit is available.
+
+  @retval TRUE  Execute Disable Bit is available.
+  @retval FALSE Execute Disable Bit is not available.
+
+**/
+BOOLEAN
+IsExecuteDisableBitAvailable (
+  VOID
+  );
+
 /**
   Enable Execute Disable Bit.
 
-- 
2.16.2.windows.1

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


[edk2] [PATCH v2 1/2] MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage

2018-09-20 Thread Jian J Wang
> v2 changes:
>Newly added patch to clarify PCDs usage.

BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

The usage of following PCDs described in MdeModulePkg.dec don't match
the implementation exactly. This patch updates related description in
both .dec and .uni files to avoid confusion in platform configuration.

  PcdSetNxForStack
  PcdImageProtectionPolicy
  PcdDxeNxMemoryProtectionPolicy

The main change is at the statement on how to handle the FALSE or 0
setting value in those PCDs. Current statement says the implementation
should unset or disable related features but in fact the related code
just do nothing (leave it to AS-IS). That means the result might be
disabled, or might be not. It depends on other features or platform
policy.

For example, if one don't want to enforce NX onto stack memory, he/she
needs to set PcdSetNxForStack to FALSE as well as to clear BIT4 of
PcdDxeNxMemoryProtectionPolicy.

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/MdeModulePkg.dec | 20 +++-
 MdeModulePkg/MdeModulePkg.uni | 13 +
 2 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 74a699cbb7..6566b57fd4 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1288,17 +1288,23 @@
   ## Set image protection policy. The policy is bitwise.
   #  If a bit is set, the image will be protected by DxeCore if it is aligned.
   #   The code section becomes read-only, and the data section becomes 
non-executable.
-  #  If a bit is clear, the image will not be protected.
+  #  If a bit is clear, nothing will be done to image code/data 
sections.
   #BIT0   - Image from unknown device. 
   #BIT1   - Image from firmware volume.
+  #  
+  #  Note: If a bit is cleared, the data section could be still non-executable 
if
+  #  PcdDxeNxMemoryProtectionPolicy is enabled for EfiLoaderData, 
EfiBootServicesData
+  #  and/or EfiRuntimeServicesData.
+  #  
   # @Prompt Set image protection policy.
   # @ValidRange 0x8002 | 0x - 0x001F
   
gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x0002|UINT32|0x1047
 
   ## Set DXE memory protection policy. The policy is bitwise.
   #  If a bit is set, memory regions of the associated type will be mapped
-  #  non-executable.
-  #
+  #  non-executable.
+  #  If a bit is cleared, nothing will be done to associated type of 
memory.
+  #  
   # Below is bit mask for this PCD: (Order is same as UEFI spec)
   #  EfiReservedMemoryType  0x0001
   #  EfiLoaderCode  0x0002
@@ -1890,8 +1896,12 @@
   #  For the DxeIpl and the DxeCore are both X64, set NX for stack feature 
also require PcdDxeIplBuildPageTables be TRUE.
   #  For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode 
is FALSE), set NX for stack feature also require
   #  IA32 PAE is supported and Execute Disable Bit is available.
-  #   TRUE  - to set NX for stack.
-  #   FALSE - Not to set NX for stack.
+  #  
+  #  Note: If this PCD is set to FALSE, NX could be still applied to stack due 
to PcdDxeNxMemoryProtectionPolicy enabled for
+  #  EfiBootServicesData.
+  #  
+  #   TRUE  - Set NX for stack.
+  #   FALSE - Do nothing for stack.
   # @Prompt Set NX for stack.
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f
 
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 080b8a62c0..61befdc1e4 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -345,8 +345,9 @@

   "For the DxeIpl and the DxeCore are both X64, set NX for stack feature also 
require PcdDxeIplBuildPageTables be TRUE."

   "For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is 
FALSE), set NX for stack feature also require"

   "IA32 PAE is supported and Execute Disable Bit is available."
-   
   "TRUE  - to set NX for stack."
-   
   "FALSE - Not to set NX for stack."
+   
   "Note: If this PCD is set to FALSE, NX could be still applied to stack due 
to PcdDxeNxMemoryProtectionPolicy enabled for EfiBootServicesData."
+   
   "TRUE  - Set NX for stack."
+   
   "FALSE - Do nothing for stack."
 
 

[edk2] [PATCH v2 0/2] clarify NXE enabling logic

2018-09-20 Thread Jian J Wang
> v2 changes:
>Incorporates review comments from Laszlo and Star.

BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116

Test:
a. try all related PCDs combinations and check the page table attributes
b. boot to shell on real intel platform with valid PCD setting combinations
(IA32/X64)
c. boot to fedora26, ubuntu18.04, windows 7 and windows 10 on OVMF emulated
platform (X64)

Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Ard Biesheuvel 
Cc: Ruiyu Ni 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 

Jian J Wang (2):
  MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage
  MdeModulePkg/DxeIpl: support more NX related PCDs

 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf  |  2 ++
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c  |  4 ++--
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 30 +++-
 MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 24 +++
 MdeModulePkg/MdeModulePkg.dec| 20 
 MdeModulePkg/MdeModulePkg.uni| 13 ++
 6 files changed, 81 insertions(+), 12 deletions(-)

-- 
2.16.2.windows.1

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