Re: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Yao, Jiewen]

HI Pete
I am glad that you like it. :)

I met some problem on extracting patch from email. So I reviewed the code in 
https://github.com/pbatard/EbcDebugger and I assume they are same.

However, the master contains some non-EBC-debugger related code.
The edk2-diff branch contains an old version of EBC driver code.

Next time, would you please post your V2 patches to a new branch, help me do 
the review efficiently?

Now I only checked the diff file. Here is some suggestion for your 
consideration:

1)  EFI_DEBUGGER_CONFIGURATION_PROTOCOL
Previously, we document a way to consume this protocol. In user manual:
https://sourceforge.net/projects/efidevkit/files/Documents/EBC%20Debugger%20User%20Manual.pdf/download
Appendix A. Configuring the EBC Debugger under EFI Shell.

But I cannot find the EbcDebuggerConfig application in EDKI. It brings you some 
confusing because you think no one is consuming this protocol.

So I post a EDK-I style APPLICATION to my personal git-hub - 
https://github.com/jyao1/EbcDebuggerApp
You can take a look. It is also BDS license code.

In order to make feature complete and match the user manual, I think we can:
1.A) Add DebuggerConfiguration.h to MdeModulePkg/Include/Protocol.
1.B) Keep EFI_DEBUGGER_CONFIGURATION_PROTOCOL in Ebc driver.
1.C) Port EbcDebuggerApp to EDKII and add it to MdeModulePkg/Application dir. 
Care need to be taken that EbcDebuggerApp should not depend on ShellPkg, the 
ARGC and ARGV can be got from SHELL_PARAMETERS_PROTOCOL.
1.D) I found the APP need EdbCommon.h to enable/disable debug , so I think we 
need expose some fields to the debug configuration protocol, such as:
  UINT32  EfiDebuggerRevision;
  UINT32  EbcVmRevision;
  UINT32  FeatureFlags;




2)  The code in EbdDebugger\*
I compare them with the previous one. Looks good to me.




3)  The update for original EbdDxe driver.
3.1) EFI_EBC_DEBUGGER_CODE()
Yes, I have similar thought as Mike.
In EDK-I, we do not have good infrastructure to enable/disable features. Using 
MACRO is the only choice.
In EDK-II, MACRO is not encouraged because it cannot cover build.
We may consider 2 options:
3.1.1) Use PCD - such as PcdEbcDebuggerEnable.
Then we can put all EbdDebugger related feature into this PCD.
This is an easy way, but it causes debugger code build every time.
As summary, we can:
3.1.1.A) Add a FeaturePcd:PcdEbcDebuggerEnable to MdeModulePkg.dec, and use 
this FeaturePcd to replace EFI_EBC_DEBUGGER_CODE.

3.1.2) Use Library - such EbcDebuggerLib.
The API in this library class is similar to the function in EbcDebuggerHook.h.
We can provide 2 instances: one is EbcDebuggerLib - real function one. The 
other is EbdDebuggerLibNull - null function one. The latter can be used by 
default to bring minimal build impact.

Personally, my preference is 3.1.2) because it can make code clean and align 
with our other debugger feature.
I found EbcDebugger code is using the OPCODE defined in EbcExecute.h. Maybe a 
better way is to move this UEFI specification define OPCODE to MdePkg Ebc.h. So 
that the EbcDebugger code does not need depend on EbcDriver.
As summary, we can:
3.1.2.A) Move UEFI specification defined EBC OPCODE from EbcExecute.h to 
MdePkg\Include\Protocol\Ebc.h
3.1.2.B) Add EbcDebuggerLib.h library class to MdeModulePkg/Include/Library
3.1.2.C) Add EbcDebuggerNullLib library instance to MdeModulePkg/Library/
3.1.2.D) Add EbcDebuggerLib library instance to MdeModulePkg/Library/



3.2) EbcExecute(), I cannot find below original code. Would you please double 
check?
//
// Verify the opcode is in range. Otherwise generate an exception.
//
if ((*VmPtr->Ip & OPCODE_M_OPCODE) >= (sizeof (mVmOpcodeTable) / sizeof 
(mVmOpcodeTable[0]))) {
  EbcDebugSignalException (EXCEPT_EBC_INVALID_OPCODE, EXCEPTION_FLAG_FATAL, 
VmPtr);
  Status = EFI_UNSUPPORTED;
  goto Done;
}

3.3) ExecuteBREAK(), I cannot find below original code. Would you please double 
check?
  case 3:
VmPtr->StopFlags |= STOPFLAG_BREAKPOINT;
VmPtr->Ip += 2; < here
//
// See if someone has registered a handler
//
EbcDebugSignalException (
  EXCEPT_EBC_BREAKPOINT,
  EXCEPTION_FLAG_NONE,
  VmPtr
  );
return EFI_SUCCESS; < here
break;

Thank You
Yao Jiewen



From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pete 
Batard
Sent: Saturday, November 12, 2016 8:44 AM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

Hi Yao - good to hear from you.

On 2016.11.11 23:48, Yao, Jiewen wrote:
> I have worked on this for EDK-I, but just did not have resource to do the 
> porting for EDK-II. Thank you for doing this.

I'll be glad if I can be of help.

The EBC Debugger has been tremendously useful during my development of
an EBC assembler as well as the 

Re: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Pete Batard]

Hi Yao - good to hear from you.

On 2016.11.11 23:48, Yao, Jiewen wrote:

I have worked on this for EDK-I, but just did not have resource to do the 
porting for EDK-II. Thank you for doing this.


I'll be glad if I can be of help.

The EBC Debugger has been tremendously useful during my development of 
an EBC assembler as well as the troubleshooting of related EBC code. So, 
while I understand that porting the EBC Debugger may not have been a 
high priority for EDK2, I'm very thankful for the work you have done on 
this in the past!



Please give me some time, and I will review the patch soon.


Sounds good. I'll be trying to post a v2 early next week, that takes 
into account the points Mike made. By the way, if you have some ideas 
for what you'd prefer for the PCD, or any other potential improvements, 
don't hesitate to let me know, since you're probably a lot more familiar 
with all of this than I am.


Regards,

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

Re: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Yao, Jiewen]

That is great news. Thank you Pete.

I have worked on this for EDK-I, but just did not have resource to do the 
porting for EDK-II. Thank you for doing this.

Please give me some time, and I will review the patch soon.

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Pete Batard
> Sent: Friday, November 11, 2016 11:51 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger
> 
> The EBC Debugger [1], which was present in Tianocore [2], is an
> invaluable tool for EBC development.
> This patch adds it back into the EDK2, allowing, for instance, the
> compilation of an AARCH64 EBC debugger.
> 
> Note 1: The patch is split in two, so that the changes to the existing
> EbcDxe code are clearer.
> 
> Note 2: The diff between the original and the new sources can be found
> at [3]. Most of the changes were for whitespaces/API names/compiler
> warnings, with the notable exception of:
> - Bumping of the EBCdebugger version to 1.0.
> - Dropping of the DebuggerConfiguration protocol (which would require
> introducing a new global GUID and protocol). I didn't see it as
> particularly useful to caary on and would rather see if there is actual
> demand for it, before adding it back.
> - Add of an EFIAPI qualifier for most of the support functions,
> especially the ones dealing with VPrint() ouput. This is required to
> avoid garbage text output in some instances.
> - Fixing of the erroneous display of 32 and 64 bit indexes in the
> disassembly.
> - Replacement of one assignation with CopyMem() to avoid an intrinsic
> memcpy().
> 
> Note 3: I tested the debugger built for AARCH64 and X64 using gcc on
> Linux (Debian/Sid) as well as the one built for IA32 and X64 using
> VS2015 on Windows. I haven't tested an IA64 version for lack of a toolchain.
> 
> Regards,
> 
> /Pete
> 
> [1] http://www.uefi.org/node/550
> [2]
> https://github.com/tianocore/edk/tree/master/Sample/Universal/Ebc/Dxe
> [3]
> https://github.com/pbatard/EbcDebugger/commit/906e87ed6ceab1c361ba
> 6f681bef48179baf549e
> ___
> 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] MdePkg Base.h: Update field name in VERIFY_UINTXX_ENUM_SIZE to follow style

[Kinney, Michael D]

Liming,

I agree enums should start with [A-Z].

These enums are only use to verify compiler config is right
and I wanted to make sure the enum names did not conflict
with any other names that might be used.  This is why
I added '__' to the beginning of these enum names.  These
enums would never be used my any code after the verify 
operation in the same .h file.

I don't expect name collisions without the '__', so I am
ok with this patch.

But, we could also consider adding a comment that the '__'
is on purpose.

Mike

> -Original Message-
> From: Gao, Liming
> Sent: Thursday, November 10, 2016 7:33 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D 
> Subject: [Patch] MdePkg Base.h: Update field name in VERIFY_UINTXX_ENUM_SIZE 
> to
> follow style
> 
> For field name in structure, its first character should be upper case.
> 
> Cc: Michael Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  MdePkg/Include/Base.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/MdePkg/Include/Base.h b/MdePkg/Include/Base.h
> index 5e24b5d..749c275 100644
> --- a/MdePkg/Include/Base.h
> +++ b/MdePkg/Include/Base.h
> @@ -64,15 +64,15 @@ VERIFY_SIZE_OF (CHAR8, 1);
>  VERIFY_SIZE_OF (CHAR16, 2);
> 
>  typedef enum {
> -  __VerifyUint8EnumValue = 0xff
> +  VerifyUint8EnumValue = 0xff
>  } __VERIFY_UINT8_ENUM_SIZE;
> 
>  typedef enum {
> -  __VerifyUint16EnumValue = 0x
> +  VerifyUint16EnumValue = 0x
>  } __VERIFY_UINT16_ENUM_SIZE;
> 
>  typedef enum {
> -  __VerifyUint32EnumValue = 0x
> +  VerifyUint32EnumValue = 0x
>  } __VERIFY_UINT32_ENUM_SIZE;
> 
>  VERIFY_SIZE_OF (__VERIFY_UINT8_ENUM_SIZE, 4);
> --
> 2.8.0.windows.1

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

Re: [edk2] [PATCH] ShellPkg/dmpstore: Support "-sfo"

[Shah, Tapan]

Ray,
   Chris and I looked at the patch closely again and saw that your change also 
includes -sfo support for -l and -s flags which is not supported according to 
the latest spec. See below syntax from spec:

dmpstore [-b] [-d] [-all | (-guid guid)] [variable] [-sfo]
dmpstore [-all | (-guid guid)] [variable] [-s file]
dmpstore [-all | (-guid guid)] [variable] [-l file]

If we want to include -l and -s support with -sfo flag, then we need to define 
new SFO tables and it requires UEFI Shell spec. update. Currently non-sfo mode 
for -l and -s flags do not display variable data and current -sfo table has 
variable data column defined.

We also saw that -l/-s -sfo mode suppresses error and that's confusing from 
user prospective as suppressing error does not provide enough information to 
user about what happened for an individual variable. This should be handled 
with new SFO tables for -l/-s flags and do it later after Shell spec. update.

Thanks,
Tapan

-Original Message-
From: Shah, Tapan 
Sent: Friday, November 11, 2016 11:21 AM
To: 'Ruiyu Ni' ; edk2-devel@lists.01.org
Cc: Chen A Chen ; Jaben Carsey 
Subject: RE: [PATCH] ShellPkg/dmpstore: Support "-sfo"

Two comments:
1. Missing help output update for -sfo support in dmpstore command.
2. BinaryToHexString() is missing input parameter NULL and out of bound size 
check before access.

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com]
Sent: Friday, November 11, 2016 4:14 AM
To: edk2-devel@lists.01.org
Cc: Chen A Chen ; Jaben Carsey ; 
Shah, Tapan 
Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"

The patch adds the "-sfo" support to "dmpstore" command.

1. For "-l" (load variable from file), when the variable content can be 
updated, the new variable data is displayed in SFO format, otherwise, the new 
variable data is not displayed. So that the SFO consumer can know which 
variables are updated by parsing the SFO.

2. For "-d" (delete variable), when the variable can be deleted successfully, 
only the variable name and GUID are displayed in SFO but the 
attribute/data/data size are displayed as empty to indicate the deletion 
happened; otherwise, the variable is not displayed.

3. For displaying variable, when the variable specified by name and GUID cannot 
be found, an error message is displayed; Otherwise, the SFO is displayed.
E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
as:
ShellCommand,"dmpstore"
VariableInfo,"","GuidThatDoesntExist","","",""

"dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
produces output as:
ShellCommand,"dmpstore"
dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name 
NameThatDoesntExist

The difference between the above 2 cases is that former one only specifies the 
GUID, but the latter one specifies both name and GUID.
Since not specifying GUID means to use GlobalVariableGuid, "dmpstore 
NameThatDoesntExist -sfo" produces the similar output as latter one.
I personally prefer to always produce SFO output for both cases.
But the above behavior is the discussion result between HPE engineers.

Signed-off-by: Ruiyu Ni 
Signed-off-by: Chen A Chen 
Cc: Jaben Carsey 
Cc: Tapan Shah 
---
 .../Library/UefiShellDebug1CommandsLib/DmpStore.c  | 269 +++--
 .../UefiShellDebug1CommandsLib.uni |   5 +
 2 files changed, 202 insertions(+), 72 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index 3427c99..aa8ad09 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -2,7 +2,7 @@
   Main file for DmpStore shell Debug1 function.

   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2005 - 2016, Intel Corporation. All rights 
+ reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at @@ -82,12 +82,46 @@ GetAttrType (  }
 
 /**
+  Convert binary to hex format string.
+
+  @param[in]  BufferSizeThe size in bytes of the binary data.
+  @param[in]  BufferThe binary data.
+  @param[in, out] HexString Hex format string.
+  @param[in]  HexStringSize The size in bytes of the string.
+
+  @return The hex format string.
+**/
+CHAR16*
+BinaryToHexString (
+  IN VOID*Buffer,
+  IN UINTN   BufferSize,
+  IN OUT CHAR16  *HexString,
+  IN UINTN   HexStringSize
+  )
+{
+  UINTN Index;
+  UINTN StringIndex;
+
+  for (Index = 0, StringIndex = 0; Index < 

Re: [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path

[Laszlo Ersek]

On 11/11/16 06:45, Jeff Fan wrote:
> On S3 path, we will wake up APs to restore CPU context in PiSmmCpuDxeSmm
> driver. In case, one NMI or SMI happens, APs may exit from hlt state and
> execute the instruction after HLT instruction.
> 
> But APs are not running on safe code, it leads OVMF S3 boot unstable.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=216
> 
> I tested real platform with 64bit DXE.
> 
> v2:
>   1. Make stack alignment per Laszlo's comment.
>   2. Trim whitespace at end of end per Laszlo's comment.
>   3. Update year mark in file header.
>   4. Enhancement on InterlockedDecrement() per Paolo's comment.
> 
> Jeff Fan (3):
>   UefiCpuPkg/PiSmmCpuDxeSmm: Put AP into safe hlt-loop code on S3 path
>   UefiCpuPkg/PiSmmCpuDxeSmm: Place AP to 32bit protected mode on S3 path
>   UefiCpuPkg/PiSmmCpuDxeSmm: Decrease mNumberToFinish in AP safe code
> 
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 33 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c | 29 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h| 15 +++
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  | 63 
> ++-
>  4 files changed, 136 insertions(+), 4 deletions(-)
> 

Applied this locally to master (ffd6b0b1b65e) for testing. I tested the
series with a suspend-resume loop -- not a busy loop, just manually. (So
there was always one second or so between adjacent steps.)

No crashes or emulation failures, but the "AP going lost" issue remains
present -- sometimes Linux cannot bring up one of the four VCPUs after
resume.

In the Ia32 case, this "AP lost" symptom surfaced after the 6th resume.

In the Ia32X64 case, I experienced the symptom after the 89th resume.

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

Re: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Pete Batard]

Hi Mike,

On 2016.11.11 17:41, Kinney, Michael D wrote:

I see the new INF files uses '..' in the [Sources]
section, which is not allowed.  Can we move that INF
file up one directory, so it can remove use of ..?


Sure. I'll work on this and submit a V2.


I also see that this code defined its own
EFI_EBC_DEBUGGER_CODE macro.  Could these be changed
to the standard DEBUG_CODE() macro that can be enabled
and disabled with a PCD?  Or do you think we should add
a new Feature Flag PCD to enable/disable the EBC
debugger?


I've been wondering about keeping the macro as well, which I mostly 
carried over from Tiano. If PCD is the more appropriate EDK2 practice, 
then I agree that we probably want to go with that.


I do feel however that we would need a new feature flag, as some people 
may want to compile an EBC module with the current debug PCD 
functionality enabled, but without the EBC debugger, especially as, in 
essence, the EBC debugger is designed to be intrusive and will break the 
flow of regular EBC execution (such as on program entry or thunk calls).


I'll explore this a little bit further, and try have a PCD proposal for V2.

Regards,

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

Re: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Kinney, Michael D]

Hi Pete,

I see the new INF files uses '..' in the [Sources]
section, which is not allowed.  Can we move that INF
file up one directory, so it can remove use of ..?

I also see that this code defined its own 
EFI_EBC_DEBUGGER_CODE macro.  Could these be changed
to the standard DEBUG_CODE() macro that can be enabled
and disabled with a PCD?  Or do you think we should add
a new Feature Flag PCD to enable/disable the EBC
debugger?

Thanks,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Pete
> Batard
> Sent: Friday, November 11, 2016 7:51 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger
> 
> The EBC Debugger [1], which was present in Tianocore [2], is an
> invaluable tool for EBC development.
> This patch adds it back into the EDK2, allowing, for instance, the
> compilation of an AARCH64 EBC debugger.
> 
> Note 1: The patch is split in two, so that the changes to the existing
> EbcDxe code are clearer.
> 
> Note 2: The diff between the original and the new sources can be found
> at [3]. Most of the changes were for whitespaces/API names/compiler
> warnings, with the notable exception of:
> - Bumping of the EBCdebugger version to 1.0.
> - Dropping of the DebuggerConfiguration protocol (which would require
> introducing a new global GUID and protocol). I didn't see it as
> particularly useful to caary on and would rather see if there is actual
> demand for it, before adding it back.
> - Add of an EFIAPI qualifier for most of the support functions,
> especially the ones dealing with VPrint() ouput. This is required to
> avoid garbage text output in some instances.
> - Fixing of the erroneous display of 32 and 64 bit indexes in the
> disassembly.
> - Replacement of one assignation with CopyMem() to avoid an intrinsic
> memcpy().
> 
> Note 3: I tested the debugger built for AARCH64 and X64 using gcc on
> Linux (Debian/Sid) as well as the one built for IA32 and X64 using
> VS2015 on Windows. I haven't tested an IA64 version for lack of a toolchain.
> 
> Regards,
> 
> /Pete
> 
> [1] http://www.uefi.org/node/550
> [2] https://github.com/tianocore/edk/tree/master/Sample/Universal/Ebc/Dxe
> [3]
> https://github.com/pbatard/EbcDebugger/commit/906e87ed6ceab1c361ba6f681bef48179ba
> f549e
> ___
> 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] ShellPkg/dmpstore: Support "-sfo"

[Shah, Tapan]

Two comments:
1. Missing help output update for -sfo support in dmpstore command.
2. BinaryToHexString() is missing input parameter NULL and out of bound size 
check before access.

-Original Message-
From: Ruiyu Ni [mailto:ruiyu...@intel.com] 
Sent: Friday, November 11, 2016 4:14 AM
To: edk2-devel@lists.01.org
Cc: Chen A Chen ; Jaben Carsey ; 
Shah, Tapan 
Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"

The patch adds the "-sfo" support to "dmpstore" command.

1. For "-l" (load variable from file), when the variable
content can be updated, the new variable data is displayed
in SFO format, otherwise, the new variable data is not
displayed. So that the SFO consumer can know which variables
are updated by parsing the SFO.

2. For "-d" (delete variable), when the variable can be deleted
successfully, only the variable name and GUID are displayed in
SFO but the attribute/data/data size are displayed as empty to
indicate the deletion happened; otherwise, the variable is not
displayed.

3. For displaying variable, when the variable specified by name
and GUID cannot be found, an error message is displayed; Otherwise,
the SFO is displayed.
E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
as:
ShellCommand,"dmpstore"
VariableInfo,"","GuidThatDoesntExist","","",""

"dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
produces output as:
ShellCommand,"dmpstore"
dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
NameThatDoesntExist

The difference between the above 2 cases is that former one only
specifies the GUID, but the latter one specifies both name and GUID.
Since not specifying GUID means to use GlobalVariableGuid,
"dmpstore NameThatDoesntExist -sfo" produces the similar output as
latter one.
I personally prefer to always produce SFO output for both cases.
But the above behavior is the discussion result between HPE engineers.

Signed-off-by: Ruiyu Ni 
Signed-off-by: Chen A Chen 
Cc: Jaben Carsey 
Cc: Tapan Shah 
---
 .../Library/UefiShellDebug1CommandsLib/DmpStore.c  | 269 +++--
 .../UefiShellDebug1CommandsLib.uni |   5 +
 2 files changed, 202 insertions(+), 72 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index 3427c99..aa8ad09 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -2,7 +2,7 @@
   Main file for DmpStore shell Debug1 function.

   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -82,12 +82,46 @@ GetAttrType (
 }
 
 /**
+  Convert binary to hex format string.
+
+  @param[in]  BufferSizeThe size in bytes of the binary data.
+  @param[in]  BufferThe binary data.
+  @param[in, out] HexString Hex format string.
+  @param[in]  HexStringSize The size in bytes of the string.
+
+  @return The hex format string.
+**/
+CHAR16*
+BinaryToHexString (
+  IN VOID*Buffer,
+  IN UINTN   BufferSize,
+  IN OUT CHAR16  *HexString,
+  IN UINTN   HexStringSize
+  )
+{
+  UINTN Index;
+  UINTN StringIndex;
+
+  for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
+StringIndex +=
+  UnicodeSPrint (
+[StringIndex],
+HexStringSize - StringIndex * sizeof (CHAR16),
+L"%02x",
+((UINT8 *) Buffer)[Index]
+);
+  }
+  return HexString;
+}
+
+/**
   Load the variable data from file and set to variable data base.
 
-  @param[in]  FileHandle The file to be read.
-  @param[in]  Name   The name of the variables to be loaded.
-  @param[in]  Guid   The guid of the variables to be loaded.
-  @param[out] Found  TRUE when at least one variable was loaded and 
set.
+  @param[in]  FileHandle   The file to be read.
+  @param[in]  Name The name of the variables to be loaded.
+  @param[in]  Guid The guid of the variables to be loaded.
+  @param[out] FoundTRUE when at least one variable was loaded 
and set.
+  @param[in]  StandardFormatOutput TRUE indicates Standard-Format Output.
 
   @retval SHELL_DEVICE_ERROR  Cannot access the file.
   @retval SHELL_VOLUME_CORRUPTED  The file is in bad format.
@@ -99,7 +133,8 @@ LoadVariablesFromFile (
   IN SHELL_FILE_HANDLE FileHandle,
   IN CONST CHAR16  *Name,
   IN CONST EFI_GUID*Guid,
-  OUT BOOLEAN  *Found
+  OUT 

[edk2] [PATCH 0/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Pete Batard]

The EBC Debugger [1], which was present in Tianocore [2], is an 
invaluable tool for EBC development.
This patch adds it back into the EDK2, allowing, for instance, the 
compilation of an AARCH64 EBC debugger.


Note 1: The patch is split in two, so that the changes to the existing 
EbcDxe code are clearer.


Note 2: The diff between the original and the new sources can be found 
at [3]. Most of the changes were for whitespaces/API names/compiler 
warnings, with the notable exception of:

- Bumping of the EBCdebugger version to 1.0.
- Dropping of the DebuggerConfiguration protocol (which would require 
introducing a new global GUID and protocol). I didn't see it as 
particularly useful to caary on and would rather see if there is actual 
demand for it, before adding it back.
- Add of an EFIAPI qualifier for most of the support functions, 
especially the ones dealing with VPrint() ouput. This is required to 
avoid garbage text output in some instances.
- Fixing of the erroneous display of 32 and 64 bit indexes in the 
disassembly.
- Replacement of one assignation with CopyMem() to avoid an intrinsic 
memcpy().


Note 3: I tested the debugger built for AARCH64 and X64 using gcc on 
Linux (Debian/Sid) as well as the one built for IA32 and X64 using 
VS2015 on Windows. I haven't tested an IA64 version for lack of a toolchain.


Regards,

/Pete

[1] http://www.uefi.org/node/550
[2] https://github.com/tianocore/edk/tree/master/Sample/Universal/Ebc/Dxe
[3] 
https://github.com/pbatard/EbcDebugger/commit/906e87ed6ceab1c361ba6f681bef48179baf549e

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

[edk2] [PATCH 1/2] MdeModulePkg/EbcDxe: add EBC Debugger

[Pete Batard]

This patch introduces EbcDebuggerHook.h and inserts the required 
EBCDebugger references into the existing EBC source files.
Since none of the introduced code is active, EbcDxe module compilation 
should be unaffected by this patch.


Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Pete Batard 
---
 MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c |   7 ++
 MdeModulePkg/Universal/EbcDxe/EbcDebuggerHook.h| 124 
+

 MdeModulePkg/Universal/EbcDxe/EbcDxe.inf   |   3 +-
 MdeModulePkg/Universal/EbcDxe/EbcExecute.c |  67 +++
 MdeModulePkg/Universal/EbcDxe/EbcInt.c |  10 ++
 MdeModulePkg/Universal/EbcDxe/Ia32/EbcSupport.c|  11 +-
 MdeModulePkg/Universal/EbcDxe/Ipf/EbcSupport.c |  13 ++-
 MdeModulePkg/Universal/EbcDxe/X64/EbcSupport.c |  11 +-
 8 files changed, 239 insertions(+), 7 deletions(-)
 create mode 100644 MdeModulePkg/Universal/EbcDxe/EbcDebuggerHook.h

diff --git a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c 
b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c

index c5cc76d..0b990c8 100644
--- a/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c
+++ b/MdeModulePkg/Universal/EbcDxe/AArch64/EbcSupport.c
@@ -18,6 +18,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
EITHER EXPRESS OR IMPLIED.


 #include "EbcInt.h"
 #include "EbcExecute.h"
+#include "EbcDebuggerHook.h"

 //
 // Amount of space that is not used in the stack
@@ -225,6 +226,9 @@ EbcInterpret (
   //
   // Begin executing the EBC code
   //
+  EFI_EBC_DEBUGGER_CODE (
+EbcDebuggerHookEbcInterpret ();
+  )
   EbcExecute ();

   //
@@ -336,6 +340,9 @@ ExecuteEbcImageEntryPoint (
   //
   // Begin executing the EBC code
   //
+  EFI_EBC_DEBUGGER_CODE (
+EbcDebuggerHookExecuteEbcImageEntryPoint ();
+  )
   EbcExecute ();

   //
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDebuggerHook.h 
b/MdeModulePkg/Universal/EbcDxe/EbcDebuggerHook.h

new file mode 100644
index 000..34e9815
--- /dev/null
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDebuggerHook.h
@@ -0,0 +1,124 @@
+/*++
+
+Copyright (c) 2007, 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.

+
+Module Name:
+
+  EbcDebuggerHook.h
+
+Abstract:
+
+--*/
+
+#ifndef _EFI_EBC_DEBUGGER_HOOK_H_
+#define _EFI_EBC_DEBUGGER_HOOK_H_
+
+#ifdef   EFI_EBC_DEBUGGER_ENABLED
+#define  EFI_EBC_DEBUGGER_CODE(a)   a
+#else
+#define  EFI_EBC_DEBUGGER_CODE(a)
+#endif
+
+//
+// Hook in EbcInt.c
+//
+VOID
+EbcDebuggerHookInit (
+  IN EFI_HANDLE  Handle,
+  IN EFI_DEBUG_SUPPORT_PROTOCOL  *EbcDebugProtocol
+  );
+
+VOID
+EbcDebuggerHookUnload (
+  VOID
+  );
+
+VOID
+EbcDebuggerHookEbcUnloadImage (
+  IN EFI_HANDLE  Handle
+  );
+
+//
+// Hook in EbcSupport.c
+//
+VOID
+EbcDebuggerHookExecuteEbcImageEntryPoint (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookEbcInterpret (
+  IN VM_CONTEXT *VmPtr
+  );
+
+//
+// Hook in EbcExecute.c
+//
+VOID
+EbcDebuggerHookExecuteStart (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookExecuteEnd (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookCALLStart (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookCALLEnd (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookCALLEXStart (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookCALLEXEnd (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookRETStart (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookRETEnd (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookJMPStart (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookJMPEnd (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookJMP8Start (
+  IN VM_CONTEXT *VmPtr
+  );
+
+VOID
+EbcDebuggerHookJMP8End (
+  IN VM_CONTEXT *VmPtr
+  );
+
+#endif
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf 
b/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf

index e9a0b28..dc769f9 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
+++ b/MdeModulePkg/Universal/EbcDxe/EbcDxe.inf
@@ -33,6 +33,7 @@
 #

 [Sources]
+  EbcDebuggerHook.h
   EbcExecute.h
   EbcExecute.c
   EbcInt.h
@@ -88,4 +89,4 @@
 # EVENT_TYPE_PERIODIC_TIMER ## CONSUMES

 [UserExtensions.TianoCore."ExtraFiles"]
-  EbcDxeExtra.uni
\ No newline at end of file
+  EbcDxeExtra.uni
diff --git a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c 
b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c

index d9c17f4..f71ecb8 100644
--- a/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
+++ b/MdeModulePkg/Universal/EbcDxe/EbcExecute.c
@@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
EITHER EXPRESS OR IMPLIED.


 #include "EbcInt.h"
 

Re: [edk2] [PATCH] ShellPkg/dmpstore: Support "-sfo"

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Ni, Ruiyu
> Sent: Friday, November 11, 2016 2:14 AM
> To: edk2-devel@lists.01.org
> Cc: Chen, Chen A ; Carsey, Jaben
> ; Tapan Shah 
> Subject: [PATCH] ShellPkg/dmpstore: Support "-sfo"
> Importance: High
> 
> The patch adds the "-sfo" support to "dmpstore" command.
> 
> 1. For "-l" (load variable from file), when the variable
> content can be updated, the new variable data is displayed
> in SFO format, otherwise, the new variable data is not
> displayed. So that the SFO consumer can know which variables
> are updated by parsing the SFO.
> 
> 2. For "-d" (delete variable), when the variable can be deleted
> successfully, only the variable name and GUID are displayed in
> SFO but the attribute/data/data size are displayed as empty to
> indicate the deletion happened; otherwise, the variable is not
> displayed.
> 
> 3. For displaying variable, when the variable specified by name
> and GUID cannot be found, an error message is displayed; Otherwise,
> the SFO is displayed.
> E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
> as:
> ShellCommand,"dmpstore"
> VariableInfo,"","GuidThatDoesntExist","","",""
> 
> "dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
> produces output as:
> ShellCommand,"dmpstore"
> dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
> NameThatDoesntExist
> 
> The difference between the above 2 cases is that former one only
> specifies the GUID, but the latter one specifies both name and GUID.
> Since not specifying GUID means to use GlobalVariableGuid,
> "dmpstore NameThatDoesntExist -sfo" produces the similar output as
> latter one.
> I personally prefer to always produce SFO output for both cases.
> But the above behavior is the discussion result between HPE engineers.
> 
> Signed-off-by: Ruiyu Ni 
> Signed-off-by: Chen A Chen 
> Cc: Jaben Carsey 
> Cc: Tapan Shah 
> ---
>  .../Library/UefiShellDebug1CommandsLib/DmpStore.c  | 269
> +++--
>  .../UefiShellDebug1CommandsLib.uni |   5 +
>  2 files changed, 202 insertions(+), 72 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> index 3427c99..aa8ad09 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
> @@ -2,7 +2,7 @@
>Main file for DmpStore shell Debug1 function.
> 
>(C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
> -  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of the BSD
> License
>which accompanies this distribution.  The full text of the license may be
> found at
> @@ -82,12 +82,46 @@ GetAttrType (
>  }
> 
>  /**
> +  Convert binary to hex format string.
> +
> +  @param[in]  BufferSizeThe size in bytes of the binary data.
> +  @param[in]  BufferThe binary data.
> +  @param[in, out] HexString Hex format string.
> +  @param[in]  HexStringSize The size in bytes of the string.
> +
> +  @return The hex format string.
> +**/
> +CHAR16*
> +BinaryToHexString (
> +  IN VOID*Buffer,
> +  IN UINTN   BufferSize,
> +  IN OUT CHAR16  *HexString,
> +  IN UINTN   HexStringSize
> +  )
> +{
> +  UINTN Index;
> +  UINTN StringIndex;
> +
> +  for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
> +StringIndex +=
> +  UnicodeSPrint (
> +[StringIndex],
> +HexStringSize - StringIndex * sizeof (CHAR16),
> +L"%02x",
> +((UINT8 *) Buffer)[Index]
> +);
> +  }
> +  return HexString;
> +}
> +
> +/**
>Load the variable data from file and set to variable data base.
> 
> -  @param[in]  FileHandle The file to be read.
> -  @param[in]  Name   The name of the variables to be loaded.
> -  @param[in]  Guid   The guid of the variables to be loaded.
> -  @param[out] Found  TRUE when at least one variable was loaded and
> set.
> +  @param[in]  FileHandle   The file to be read.
> +  @param[in]  Name The name of the variables to be loaded.
> +  @param[in]  Guid The guid of the variables to be loaded.
> +  @param[out] FoundTRUE when at least one variable was loaded
> and set.
> +  @param[in]  StandardFormatOutput TRUE indicates Standard-Format
> Output.
> 
>@retval SHELL_DEVICE_ERROR  Cannot access the file.
>@retval SHELL_VOLUME_CORRUPTED  The file is in bad format.
> @@ -99,7 +133,8 @@ LoadVariablesFromFile (
>IN SHELL_FILE_HANDLE 

Re: [edk2] File mode problem on Github edk2-BaseTools-win32

[Gao, Liming]

Evan:
  Yes. I expect you send the patch into this mail list. After I review and 
test, I will help push it into edk2-BaseTools-win32 repo. As you mention, this 
repo is still the mirror of svn. Any change in this repo will impact the mirror 
sync. So, we expect the patch to be applied.

Thanks
Liming
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Evan 
Lloyd
Sent: Friday, November 11, 2016 9:13 PM
To: Laszlo Ersek ; edk2-devel (edk2-devel@lists.01.org) 

Cc: Gao, Liming ; Leif Lindholm 
Subject: Re: [edk2] File mode problem on Github edk2-BaseTools-win32

Hi Laszlo.

>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: 11 November 2016 11:24
>To: Evan Lloyd; edk2-devel 
>(edk2-devel@lists.01.org)
>Cc: Leif Lindholm; liming@intel.com
>Subject: Re: [edk2] File mode problem on Github edk2-BaseTools-win32
...
>> Liming,
>> Because this is purely a permission problem in the Git repository, and .exe
>files are not amenable to patching,
>
>They are -- I think if you change the file mode bits, git will see that, and 
>will
>create a patch that has no content hunks, just the file mode changes.

A semantic quibble; ".exe files are not amenable to patching" is true, the file 
ATTRIBUTES may be.
Patches against a .exe (or .dll) should surely start alarm bells ringing for 
most people.

>
>For example, in the BaseTools/Conf/ directory, we happen have two
>template files that have gratuitous execute permissions. If I remove those
>permissions, "git diff" shows
>
...
>(The above patch is one I could submit genuinely, but I'm too lazy. :))

And somebody on the list would only object, so why bother? ;-)

>
>> I have raised a pull request on https://github.com/tianocore/edk2-
>BaseTools-win32/pulls
>> This is only a minor thing, but I would deem it a great favour were you to
>accept the pull request.
>> It has me tearing my hair out, and I have little enough to begin with. :-{
>
>It is fine to send pull requests, but:
>- they should be mailed to the list (not opened on github),
>- the patches have to be reviewed first, anyway.
>
>(Speaking about the edk2 repo at least -- I realize this is a different repo.)

As you point out, this is for a different repo; provided (I think) as a 
convenience, and is ancillary to edk2.
My viewpoint is that this is a specialised aspect, of interest to very few 
people. (Does anyone else use Cygwin Git and the Win32 binaries?)
The only reason for publishing this request here was one of awareness. Most 
people will, I expect, be blissfully unconcerned.

I am happy to submit a patch though, should those responsible (Liming?) want 
that.
Until that is confirmed though, I'm assuming that the GitHub repo is a mirror 
of a Subversion original (which will not record modes), so applying a patch 
might involve a lot more work than accepting the pull request on GitHub.

Regards,
Evan

>
>Thanks
>Laszlo
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] File mode problem on Github edk2-BaseTools-win32

[Evan Lloyd]

Hi Laszlo.

>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: 11 November 2016 11:24
>To: Evan Lloyd; edk2-devel (edk2-devel@lists.01.org)
>Cc: Leif Lindholm; liming@intel.com
>Subject: Re: [edk2] File mode problem on Github edk2-BaseTools-win32
...
>> Liming,
>> Because this is purely a permission problem in the Git repository, and .exe
>files are not amenable to patching,
>
>They are -- I think if you change the file mode bits, git will see that, and 
>will
>create a patch that has no content hunks, just the file mode changes.

A semantic quibble; ".exe files are not amenable to patching" is true, the file 
ATTRIBUTES may be.
Patches against a .exe (or .dll) should surely start alarm bells ringing for 
most people.

>
>For example, in the BaseTools/Conf/ directory, we happen have two
>template files that have gratuitous execute permissions. If I remove those
>permissions, "git diff" shows
>
...
>(The above patch is one I could submit genuinely, but I'm too lazy. :))

And somebody on the list would only object, so why bother?;-)

>
>> I have raised a pull request on  https://github.com/tianocore/edk2-
>BaseTools-win32/pulls
>> This is only a minor thing, but I would deem it a great favour were you to
>accept the pull request.
>> It has me tearing my hair out, and I have little enough to begin with.  :-{
>
>It is fine to send pull requests, but:
>- they should be mailed to the list (not opened on github),
>- the patches have to be reviewed first, anyway.
>
>(Speaking about the edk2 repo at least -- I realize this is a different repo.)

As you point out, this is for a different repo; provided (I think) as a 
convenience, and is ancillary to edk2.
My viewpoint is that this is a specialised aspect, of interest to very few 
people. (Does anyone else use Cygwin Git and the Win32 binaries?)
The only reason for publishing this request here was one of awareness.  Most 
people will, I expect,  be blissfully unconcerned.

I am happy to submit a patch though, should those responsible (Liming?) want 
that.
Until that is confirmed though, I'm assuming that the GitHub repo is a mirror 
of a Subversion original (which will not record modes), so applying a patch 
might involve a lot more work than accepting the pull request on GitHub.

Regards,
Evan

>
>Thanks
>Laszlo
IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/11/16 13:53, Yao, Jiewen wrote:
> Sorry, I did not explain it clear enough before.
> 
> Jeff is right. The NX fix is introduced in this patch series.
> The reason is that when we update page table to protect the SMRAM, we would 
> like enable the protection as early as possible.
> We moved the NX enabling code from C function to ASM function to achieve that.
> 
> You can see my GIT message in 5/6.
> ==
> The XD enabling code is moved to SmiEntry to let NX take effect.
> ==
> 
> Unfortunately, I introduced a bug there. You help to catch it and now I fix 
> it.
> It is not related to current code.
> 
> What about your idea?

Right, I missed that the XD handling in SmiEntry.nasm was brand new code
added by this series. So, the current structure of both series should be
fine; I should be able to start testing them (hopefully) soon.

Thanks!
Laszlo

> Thank you
> Yao Jiewen
> 
> From: Fan, Jeff
> Sent: Friday, November 11, 2016 8:38 PM
> To: Laszlo Ersek ; Yao, Jiewen ; 
> edk2-devel@lists.01.org 
> Cc: Tian, Feng ; Kinney, Michael D 
> ; Paolo Bonzini ; Zeng, Star 
> 
> Subject: RE: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
> 
> Laszlo,
> 
> NX support and fix in SmiEntry.asm is new code for UefiCpuPkg. So, that means 
> we still have other unknown problem on S3 boot issue.
> 
> Jeff
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Friday, November 11, 2016 8:26 PM
> To: Yao, Jiewen; edk2-devel@lists.01.org; 
> Fan, Jeff
> Cc: Tian, Feng; Kinney, Michael D; Paolo Bonzini; Zeng, Star
> Subject: Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
> 
> Jiewen, Jeff,
> 
> On 11/11/16 10:12, Yao, Jiewen wrote:
>> HI Laszlo
>> I fixed the IA32 boot issue in this patch
> 
> Thank you.
> 
> Jiewen, I'd like to request the following:
> 
> - Please separate the fix for the incorrect parameter passing to
>   SmiRendezvous() out to a separate patch. It is my understanding that
>   the issue exists already in the master branch. Is that right? If it
>   is, then the fix should not be tied to the SMM page level protection
>   feature.
> 
> - Please give that patch to Jeff.
> 
> Jeff,
> 
> can you please repost your series
> 
>   [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path
> 
> to edk2-devel, as v3, with Jiewen's patch from above included, as patch#4? 
> Because, I would like to see a patch series that addresses all known S3 
> issues that we've uncovered in this investigation.
> 
> The first three patches should fix BZ#216, yes.
> 
> The last (4th) patch, from Jiewen, is unrelated to that BZ indeed, but it 
> nonetheless addresses an existent issue in PiSmmCpuDxeSmm that can be hit 
> during S3.
> 
> My goal is to apply that series (the first 3 patches from Jeff, and the 
> fourth patch from Jiewen), and to test it as one unit. I'd like to see if 
> those changes fix the infrequent, but still triggerable issues with
> S3+SMM, for both Ia32 and Ia32X64. If everything works fine, then that
> series should be committed.
> 
> After that, I'd like to test Jiewen's v4 series for the SMM page level 
> protection, separately.
> 
> In other words, first we should fix the existent bugs that Jiewen's SMM page 
> level protection feature only amplifies (but doesn't introduce) on QEMU/KVM + 
> OVMF. Once the known bugs are fixed, I'll be glad to test the new feature.
> 
> Would this work for you guys?
> 
> Thank you,
> Laszlo
> 
>> with DEBUG message update you suggested.
>>
>> My unit test failed before. Now it can pass.
>> I validated on a real IA32 and Windows OVMF with and without XD.
>>
>>
>> For QEMU installation, it is still on progress.
>> We have setup a Fedora 24 host, download QEMU, and install it.
>> But we are still struggling to make QEMU boot on Fedora.
>> Your step by step is great. There is still some minor place we stuck in due 
>> to my ignorance.
>> My goal is still to setup an environment like yours for our validation or 
>> issue reproduce.
>> It just need take some time, more than I expected. sign...
>>
>> Thank you
>> Yao Jiewen
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>>> Of Jiewen Yao
>>> Sent: Friday, November 11, 2016 5:01 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Tian, Feng >; Kinney, 
>>> Michael D
>>> >; Paolo 
>>> Bonzini >;
>>> Laszlo Ersek >; Fan, Jeff 
>>> >;
>>> Zeng, Star >
>>> Subject: [edk2] [PATCH V3 0/6] Enable SMM 

Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Yao, Jiewen]

Sorry, I did not explain it clear enough before.

Jeff is right. The NX fix is introduced in this patch series.
The reason is that when we update page table to protect the SMRAM, we would 
like enable the protection as early as possible.
We moved the NX enabling code from C function to ASM function to achieve that.

You can see my GIT message in 5/6.
==
The XD enabling code is moved to SmiEntry to let NX take effect.
==

Unfortunately, I introduced a bug there. You help to catch it and now I fix it.
It is not related to current code.

What about your idea?

Thank you
Yao Jiewen

From: Fan, Jeff
Sent: Friday, November 11, 2016 8:38 PM
To: Laszlo Ersek ; Yao, Jiewen ; 
edk2-devel@lists.01.org 
Cc: Tian, Feng ; Kinney, Michael D 
; Paolo Bonzini ; Zeng, Star 

Subject: RE: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

Laszlo,

NX support and fix in SmiEntry.asm is new code for UefiCpuPkg. So, that means 
we still have other unknown problem on S3 boot issue.

Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Friday, November 11, 2016 8:26 PM
To: Yao, Jiewen; edk2-devel@lists.01.org; Fan, 
Jeff
Cc: Tian, Feng; Kinney, Michael D; Paolo Bonzini; Zeng, Star
Subject: Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

Jiewen, Jeff,

On 11/11/16 10:12, Yao, Jiewen wrote:
> HI Laszlo
> I fixed the IA32 boot issue in this patch

Thank you.

Jiewen, I'd like to request the following:

- Please separate the fix for the incorrect parameter passing to
  SmiRendezvous() out to a separate patch. It is my understanding that
  the issue exists already in the master branch. Is that right? If it
  is, then the fix should not be tied to the SMM page level protection
  feature.

- Please give that patch to Jeff.

Jeff,

can you please repost your series

  [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path

to edk2-devel, as v3, with Jiewen's patch from above included, as patch#4? 
Because, I would like to see a patch series that addresses all known S3 issues 
that we've uncovered in this investigation.

The first three patches should fix BZ#216, yes.

The last (4th) patch, from Jiewen, is unrelated to that BZ indeed, but it 
nonetheless addresses an existent issue in PiSmmCpuDxeSmm that can be hit 
during S3.

My goal is to apply that series (the first 3 patches from Jeff, and the fourth 
patch from Jiewen), and to test it as one unit. I'd like to see if those 
changes fix the infrequent, but still triggerable issues with
S3+SMM, for both Ia32 and Ia32X64. If everything works fine, then that
series should be committed.

After that, I'd like to test Jiewen's v4 series for the SMM page level 
protection, separately.

In other words, first we should fix the existent bugs that Jiewen's SMM page 
level protection feature only amplifies (but doesn't introduce) on QEMU/KVM + 
OVMF. Once the known bugs are fixed, I'll be glad to test the new feature.

Would this work for you guys?

Thank you,
Laszlo

> with DEBUG message update you suggested.
>
> My unit test failed before. Now it can pass.
> I validated on a real IA32 and Windows OVMF with and without XD.
>
>
> For QEMU installation, it is still on progress.
> We have setup a Fedora 24 host, download QEMU, and install it.
> But we are still struggling to make QEMU boot on Fedora.
> Your step by step is great. There is still some minor place we stuck in due 
> to my ignorance.
> My goal is still to setup an environment like yours for our validation or 
> issue reproduce.
> It just need take some time, more than I expected. sign...
>
> Thank you
> Yao Jiewen
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
>> Of Jiewen Yao
>> Sent: Friday, November 11, 2016 5:01 PM
>> To: edk2-devel@lists.01.org
>> Cc: Tian, Feng >; Kinney, 
>> Michael D
>> >; Paolo 
>> Bonzini >;
>> Laszlo Ersek >; Fan, Jeff 
>> >;
>> Zeng, Star >
>> Subject: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
>>
>>
>>  below is V3 description 
>> 1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition.
>> (Many thanks to Laszlo Ersek > 
>> for catching it.)
>> 2) PiSmmCpu: Add ASSERT for CpuIndex check.
>> 3) PiSmmCpu: Use DEBUG_VERBOSE for page table update.
>> 4) PiSmmCpu: Do not report DEBUG message for Ap non present when
>> PcdCpuSmmSyncMode==1 (Relex mode).
>> 5) PiSmmCpu: Do not report DEBUG 

Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Laszlo Ersek]

On 11/11/16 13:38, Fan, Jeff wrote:
> Laszlo,
> 
> NX support and fix in SmiEntry.asm is new code for UefiCpuPkg.

Ah, you are right. Sorry, I missed that. I've checked the v2 series now
and I see the SkipXd label and friends are introduced by v2; it's not
preexistent code.

This means it should be okay for me to test these series as they are. No
need to move patches between them.

> So, that means we still have other unknown problem on S3 boot issue.

Hm, with your above explanation, I'm actually not sure about that. I
think that your series

[edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path

has a good chance to fix it all. I hope I can test it today. (And once
I'm done with it, I'll move to Jiewen's v3.)

Thanks!
Laszlo



> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Friday, November 11, 2016 8:26 PM
> To: Yao, Jiewen; edk2-devel@lists.01.org; Fan, Jeff
> Cc: Tian, Feng; Kinney, Michael D; Paolo Bonzini; Zeng, Star
> Subject: Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
> 
> Jiewen, Jeff,
> 
> On 11/11/16 10:12, Yao, Jiewen wrote:
>> HI Laszlo
>> I fixed the IA32 boot issue in this patch
> 
> Thank you.
> 
> Jiewen, I'd like to request the following:
> 
> - Please separate the fix for the incorrect parameter passing to
>   SmiRendezvous() out to a separate patch. It is my understanding that
>   the issue exists already in the master branch. Is that right? If it
>   is, then the fix should not be tied to the SMM page level protection
>   feature.
> 
> - Please give that patch to Jeff.
> 
> Jeff,
> 
> can you please repost your series
> 
>   [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path
> 
> to edk2-devel, as v3, with Jiewen's patch from above included, as patch#4? 
> Because, I would like to see a patch series that addresses all known S3 
> issues that we've uncovered in this investigation.
> 
> The first three patches should fix BZ#216, yes.
> 
> The last (4th) patch, from Jiewen, is unrelated to that BZ indeed, but it 
> nonetheless addresses an existent issue in PiSmmCpuDxeSmm that can be hit 
> during S3.
> 
> My goal is to apply that series (the first 3 patches from Jeff, and the 
> fourth patch from Jiewen), and to test it as one unit. I'd like to see if 
> those changes fix the infrequent, but still triggerable issues with
> S3+SMM, for both Ia32 and Ia32X64. If everything works fine, then that
> series should be committed.
> 
> After that, I'd like to test Jiewen's v4 series for the SMM page level 
> protection, separately.
> 
> In other words, first we should fix the existent bugs that Jiewen's SMM page 
> level protection feature only amplifies (but doesn't introduce) on QEMU/KVM + 
> OVMF. Once the known bugs are fixed, I'll be glad to test the new feature.
> 
> Would this work for you guys?
> 
> Thank you,
> Laszlo
> 
>> with DEBUG message update you suggested.
>>
>> My unit test failed before. Now it can pass.
>> I validated on a real IA32 and Windows OVMF with and without XD.
>>
>>
>> For QEMU installation, it is still on progress.
>> We have setup a Fedora 24 host, download QEMU, and install it.
>> But we are still struggling to make QEMU boot on Fedora.
>> Your step by step is great. There is still some minor place we stuck in due 
>> to my ignorance.
>> My goal is still to setup an environment like yours for our validation or 
>> issue reproduce.
>> It just need take some time, more than I expected. sign...
>>
>> Thank you
>> Yao Jiewen
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>>> Of Jiewen Yao
>>> Sent: Friday, November 11, 2016 5:01 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Tian, Feng ; Kinney, Michael D 
>>> ; Paolo Bonzini ; 
>>> Laszlo Ersek ; Fan, Jeff ; 
>>> Zeng, Star 
>>> Subject: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
>>>
>>>
>>>  below is V3 description 
>>> 1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition.
>>> (Many thanks to Laszlo Ersek  for catching it.)
>>> 2) PiSmmCpu: Add ASSERT for CpuIndex check.
>>> 3) PiSmmCpu: Use DEBUG_VERBOSE for page table update.
>>> 4) PiSmmCpu: Do not report DEBUG message for Ap non present when 
>>> PcdCpuSmmSyncMode==1 (Relex mode).
>>> 5) PiSmmCpu: Do not report DEBUG message for AP removed when 
>>> PcdCpuHotPlugSupport==TRUE.
>>>
>>> Tested combination:
>>> 1) XD disabled
>>> 2) XD enabled in SMM and disabled in non-SMM.
>>> 3) XD enabled in SMM and enabled in non-SMM.
>>>
>>>  below is V2 description 
>>> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
>>> 2) PiSmmCpu: Add debug info on StartupAp() fails.
>>> 3) PiSmmCpu: Add ASSERT for AllocatePages().
>>> 4) PiSmmCpu: Add protection detail in commit message.
>>> 5) UefiCpuPkg.dsc: Add page table footprint info in commit 

Re: [edk2] [PATCH v2 2/2] MdeModulePkg/Ip4Dxe: Correct the return status

[Laszlo Ersek]

On 11/11/16 06:18, Jiaxin Wu wrote:
> This patch made the following change:
> * DataItem->Status should be updated to the status code.
> * Data should not be freed if EFI_NOT_READY returned.
> 
> Cc: Santhapur Naveen 
> Cc: Laszlo Ersek 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++---
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c 
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index 5b01b35..88ead9d 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1280,25 +1280,21 @@ Ip4Config2SetMaunualAddress (
>DataItem->DataSize = DataSize;
>DataItem->Status   = EFI_NOT_READY;
>  
>IpSb->Reconfig = TRUE;
>Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
> -  if (EFI_ERROR (Status)) {
> -goto ON_EXIT;
> -  }  
>  
> -  DataItem->Status = EFI_SUCCESS;   
> +  DataItem->Status = Status; 
>  
> -ON_EXIT:
> -  if (EFI_ERROR (DataItem->Status)) {
> +  if (EFI_ERROR (DataItem->Status) && DataItem->Status != EFI_NOT_READY) {
>  if (Ptr != NULL) {
>FreePool (Ptr);
>  }
>  DataItem->Data.Ptr = NULL; 
>}
>  
> -  return EFI_SUCCESS;
> +  return Status;
>  }
>  
>  /**
>The work function is to set the gateway addresses manually for the EFI 
> IPv4 
>network stack that is running on the communication device that this EFI 
> IPv4 
> 

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

Re: [edk2] [PATCH v2 1/2] MdeModulePkg/Ip4Dxe: Add wrong/invalid subnet check

[Laszlo Ersek]

On 11/11/16 06:18, Jiaxin Wu wrote:
> v2:
> * Separate out the return status fix.
> * Replace IP4_MASK_MAX with IP4_MASK_MAX.
> * Remove the ON_EXIT label.
> 
> This patch is used to add the wrong/invalid subnet check.
> 
> Cc: Santhapur Naveen 
> Cc: Laszlo Ersek 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c | 10 +++---
>  MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c  |  8 +---
>  2 files changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c 
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> index a931bb3..5b01b35 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4Config2Impl.c
> @@ -1253,10 +1253,17 @@ Ip4Config2SetMaunualAddress (
>  return EFI_WRITE_PROTECTED;
>}
>  
>NewAddress = *((EFI_IP4_CONFIG2_MANUAL_ADDRESS *) Data);
>  
> +  StationAddress = EFI_NTOHL (NewAddress.Address);
> +  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
> +
> +  if (NetGetMaskLength (SubnetMask) == IP4_MASK_NUM) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>//
>// Store the new data, and init the DataItem status to EFI_NOT_READY 
> because
>// we may have an asynchronous configuration process.
>//
>Ptr = AllocateCopyPool (DataSize, Data);
> @@ -1271,13 +1278,10 @@ Ip4Config2SetMaunualAddress (
>
>DataItem->Data.Ptr = Ptr;
>DataItem->DataSize = DataSize;
>DataItem->Status   = EFI_NOT_READY;
>  
> -  StationAddress = EFI_NTOHL (NewAddress.Address);
> -  SubnetMask = EFI_NTOHL (NewAddress.SubnetMask);
> -
>IpSb->Reconfig = TRUE;
>Status = Ip4Config2SetDefaultAddr (IpSb, StationAddress, SubnetMask);
>if (EFI_ERROR (Status)) {
>  goto ON_EXIT;
>}  
> diff --git a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c 
> b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> index 9cd5dd5..b0cc6a3 100644
> --- a/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> +++ b/MdeModulePkg/Universal/Network/Ip4Dxe/Ip4If.c
> @@ -562,10 +562,15 @@ Ip4SetAddress (
>EFI_STATUSStatus;
>INTN  Len;
>  
>NET_CHECK_SIGNATURE (Interface, IP4_INTERFACE_SIGNATURE);
>  
> +  Len = NetGetMaskLength (SubnetMask);
> +  if (Len == IP4_MASK_NUM) {
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>//
>// Set the ip/netmask, then compute the subnet broadcast
>// and network broadcast for easy access. When computing
>// nework broadcast, the subnet mask is most like longer
>// than the default netmask (not subneted) as defined in
> @@ -573,13 +578,10 @@ Ip4SetAddress (
>// networks, use the subnet's mask instead.
>//
>Interface->Ip = IpAddr;
>Interface->SubnetMask = SubnetMask;
>Interface->SubnetBrdcast  = (IpAddr | ~SubnetMask);
> -
> -  Len   = NetGetMaskLength (SubnetMask);
> -  ASSERT (Len <= IP4_MASK_MAX);
>Interface->NetBrdcast = (IpAddr | ~SubnetMask);
>  
>//
>// Do clean up for Arp child
>//
> 

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/2] Place APs to suitable state on Legacy OS boot

[Fan, Jeff]

Laszlo,

Got you. It's a good suggestion!

Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Friday, November 11, 2016 8:33 PM
To: Fan, Jeff
Cc: edk2-de...@ml01.01.org; Tian, Feng; Yao, Jiewen; Kinney, Michael D; Paolo 
Bonzini
Subject: Re: [edk2] [PATCH 0/2] Place APs to suitable state on Legacy OS boot

On 11/11/16 12:57, Jeff Fan wrote:
> Currently, DxeMpLib only places APs into specified c-state in Exit 
> Boot Service callback function for UEFI OS boot. We need to put APs 
> into specified c-state for legacy OS boot also.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=210
> 
> Jeff Fan (2):
>   UefiCpuPkg/DxeMpLib: Rename MpInitExitBootServicesCallback()
>   UefiCpuPkg/DxeMpLib: Place APs to suitable state on Legacy OS boot
> 
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   | 18 +++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 

>From a cursory look, it looks good to me, but I think I'll leave the official 
>review on this to Mike et al :)

Also, a meta-hint: when you post a cover letter for a patch series, it is best 
to collect all the CC's from the patches, sort them uniquely, and then add the 
resulting CC list to the cover letter as well. This way every CC'd person will 
receive the high level description for the entire set.

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

* finally, add all of the Cc: tags to the cover letter that you
  used across all of the patches. This will ensure that even if a
  maintainer is involved in reviewing one or two of your patches
  across the series, he or she will get a copy of your cover
  letter, which outlines the full feature or bugfix.

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

Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Fan, Jeff]

Laszlo,

NX support and fix in SmiEntry.asm is new code for UefiCpuPkg. So, that means 
we still have other unknown problem on S3 boot issue.

Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Friday, November 11, 2016 8:26 PM
To: Yao, Jiewen; edk2-devel@lists.01.org; Fan, Jeff
Cc: Tian, Feng; Kinney, Michael D; Paolo Bonzini; Zeng, Star
Subject: Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

Jiewen, Jeff,

On 11/11/16 10:12, Yao, Jiewen wrote:
> HI Laszlo
> I fixed the IA32 boot issue in this patch

Thank you.

Jiewen, I'd like to request the following:

- Please separate the fix for the incorrect parameter passing to
  SmiRendezvous() out to a separate patch. It is my understanding that
  the issue exists already in the master branch. Is that right? If it
  is, then the fix should not be tied to the SMM page level protection
  feature.

- Please give that patch to Jeff.

Jeff,

can you please repost your series

  [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path

to edk2-devel, as v3, with Jiewen's patch from above included, as patch#4? 
Because, I would like to see a patch series that addresses all known S3 issues 
that we've uncovered in this investigation.

The first three patches should fix BZ#216, yes.

The last (4th) patch, from Jiewen, is unrelated to that BZ indeed, but it 
nonetheless addresses an existent issue in PiSmmCpuDxeSmm that can be hit 
during S3.

My goal is to apply that series (the first 3 patches from Jeff, and the fourth 
patch from Jiewen), and to test it as one unit. I'd like to see if those 
changes fix the infrequent, but still triggerable issues with
S3+SMM, for both Ia32 and Ia32X64. If everything works fine, then that
series should be committed.

After that, I'd like to test Jiewen's v4 series for the SMM page level 
protection, separately.

In other words, first we should fix the existent bugs that Jiewen's SMM page 
level protection feature only amplifies (but doesn't introduce) on QEMU/KVM + 
OVMF. Once the known bugs are fixed, I'll be glad to test the new feature.

Would this work for you guys?

Thank you,
Laszlo

> with DEBUG message update you suggested.
> 
> My unit test failed before. Now it can pass.
> I validated on a real IA32 and Windows OVMF with and without XD.
> 
> 
> For QEMU installation, it is still on progress.
> We have setup a Fedora 24 host, download QEMU, and install it.
> But we are still struggling to make QEMU boot on Fedora.
> Your step by step is great. There is still some minor place we stuck in due 
> to my ignorance.
> My goal is still to setup an environment like yours for our validation or 
> issue reproduce.
> It just need take some time, more than I expected. sign...
> 
> Thank you
> Yao Jiewen
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>> Of Jiewen Yao
>> Sent: Friday, November 11, 2016 5:01 PM
>> To: edk2-devel@lists.01.org
>> Cc: Tian, Feng ; Kinney, Michael D 
>> ; Paolo Bonzini ; 
>> Laszlo Ersek ; Fan, Jeff ; 
>> Zeng, Star 
>> Subject: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
>>
>>
>>  below is V3 description 
>> 1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition.
>> (Many thanks to Laszlo Ersek  for catching it.)
>> 2) PiSmmCpu: Add ASSERT for CpuIndex check.
>> 3) PiSmmCpu: Use DEBUG_VERBOSE for page table update.
>> 4) PiSmmCpu: Do not report DEBUG message for Ap non present when 
>> PcdCpuSmmSyncMode==1 (Relex mode).
>> 5) PiSmmCpu: Do not report DEBUG message for AP removed when 
>> PcdCpuHotPlugSupport==TRUE.
>>
>> Tested combination:
>> 1) XD disabled
>> 2) XD enabled in SMM and disabled in non-SMM.
>> 3) XD enabled in SMM and enabled in non-SMM.
>>
>>  below is V2 description 
>> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
>> 2) PiSmmCpu: Add debug info on StartupAp() fails.
>> 3) PiSmmCpu: Add ASSERT for AllocatePages().
>> 4) PiSmmCpu: Add protection detail in commit message.
>> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message.
>>
>>  below is V1 description 
>> This series patch enables SMM page level protection.
>> Features are:
>> 1) PiSmmCore reports SMM PE image code/data information in 
>> EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable and set XD for 
>> data page and RO for code page.
>> 3) PiSmmCpu enables Static Paging for X64 according to 
>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G is 
>> used as long as it is supported.
>> 4) PiSmmCpu sets importance data structure to be read only, such as 
>> Gdt, Idt, SmmEntrypoint, and PageTable itself.
>>
>> tested platform:
>> 1) Intel internal platform (X64).
>> 2) EDKII Quark IA32
>> 3) EDKII Vlv2  X64
>> 4) 

Re: [edk2] [PATCH 0/2] Place APs to suitable state on Legacy OS boot

[Laszlo Ersek]

On 11/11/16 12:57, Jeff Fan wrote:
> Currently, DxeMpLib only places APs into specified c-state in Exit Boot 
> Service
> callback function for UEFI OS boot. We need to put APs into specified c-state
> for legacy OS boot also.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=210
> 
> Jeff Fan (2):
>   UefiCpuPkg/DxeMpLib: Rename MpInitExitBootServicesCallback()
>   UefiCpuPkg/DxeMpLib: Place APs to suitable state on Legacy OS boot
> 
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   | 18 +++---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 

>From a cursory look, it looks good to me, but I think I'll leave the
official review on this to Mike et al :)

Also, a meta-hint: when you post a cover letter for a patch series, it
is best to collect all the CC's from the patches, sort them uniquely,
and then add the resulting CC list to the cover letter as well. This way
every CC'd person will receive the high level description for the entire
set.

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

* finally, add all of the Cc: tags to the cover letter that you
  used across all of the patches. This will ensure that even if a
  maintainer is involved in reviewing one or two of your patches
  across the series, he or she will get a copy of your cover
  letter, which outlines the full feature or bugfix.

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

Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Laszlo Ersek]

Jiewen, Jeff,

On 11/11/16 10:12, Yao, Jiewen wrote:
> HI Laszlo
> I fixed the IA32 boot issue in this patch

Thank you.

Jiewen, I'd like to request the following:

- Please separate the fix for the incorrect parameter passing to
  SmiRendezvous() out to a separate patch. It is my understanding that
  the issue exists already in the master branch. Is that right? If it
  is, then the fix should not be tied to the SMM page level protection
  feature.

- Please give that patch to Jeff.

Jeff,

can you please repost your series

  [edk2] [PATCH v2 0/3] Put AP into safe hlt-loop code on S3 path

to edk2-devel, as v3, with Jiewen's patch from above included, as
patch#4? Because, I would like to see a patch series that addresses all
known S3 issues that we've uncovered in this investigation.

The first three patches should fix BZ#216, yes.

The last (4th) patch, from Jiewen, is unrelated to that BZ indeed, but
it nonetheless addresses an existent issue in PiSmmCpuDxeSmm that can be
hit during S3.

My goal is to apply that series (the first 3 patches from Jeff, and the
fourth patch from Jiewen), and to test it as one unit. I'd like to see
if those changes fix the infrequent, but still triggerable issues with
S3+SMM, for both Ia32 and Ia32X64. If everything works fine, then that
series should be committed.

After that, I'd like to test Jiewen's v4 series for the SMM page level
protection, separately.

In other words, first we should fix the existent bugs that Jiewen's SMM
page level protection feature only amplifies (but doesn't introduce) on
QEMU/KVM + OVMF. Once the known bugs are fixed, I'll be glad to test the
new feature.

Would this work for you guys?

Thank you,
Laszlo

> with DEBUG message update you suggested.
> 
> My unit test failed before. Now it can pass.
> I validated on a real IA32 and Windows OVMF with and without XD.
> 
> 
> For QEMU installation, it is still on progress.
> We have setup a Fedora 24 host, download QEMU, and install it.
> But we are still struggling to make QEMU boot on Fedora.
> Your step by step is great. There is still some minor place we stuck in due 
> to my ignorance.
> My goal is still to setup an environment like yours for our validation or 
> issue reproduce.
> It just need take some time, more than I expected. sign...
> 
> Thank you
> Yao Jiewen
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Jiewen Yao
>> Sent: Friday, November 11, 2016 5:01 PM
>> To: edk2-devel@lists.01.org
>> Cc: Tian, Feng ; Kinney, Michael D
>> ; Paolo Bonzini ;
>> Laszlo Ersek ; Fan, Jeff ; Zeng,
>> Star 
>> Subject: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
>>
>>
>>  below is V3 description 
>> 1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition.
>> (Many thanks to Laszlo Ersek  for catching it.)
>> 2) PiSmmCpu: Add ASSERT for CpuIndex check.
>> 3) PiSmmCpu: Use DEBUG_VERBOSE for page table update.
>> 4) PiSmmCpu: Do not report DEBUG message for Ap non present
>> when PcdCpuSmmSyncMode==1 (Relex mode).
>> 5) PiSmmCpu: Do not report DEBUG message for AP removed
>> when PcdCpuHotPlugSupport==TRUE.
>>
>> Tested combination:
>> 1) XD disabled
>> 2) XD enabled in SMM and disabled in non-SMM.
>> 3) XD enabled in SMM and enabled in non-SMM.
>>
>>  below is V2 description 
>> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
>> 2) PiSmmCpu: Add debug info on StartupAp() fails.
>> 3) PiSmmCpu: Add ASSERT for AllocatePages().
>> 4) PiSmmCpu: Add protection detail in commit message.
>> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message.
>>
>>  below is V1 description 
>> This series patch enables SMM page level protection.
>> Features are:
>> 1) PiSmmCore reports SMM PE image code/data information
>> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
>> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
>> and set XD for data page and RO for code page.
>> 3) PiSmmCpu enables Static Paging for X64 according to
>> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
>> is used as long as it is supported.
>> 4) PiSmmCpu sets importance data structure to be read only,
>> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
>>
>> tested platform:
>> 1) Intel internal platform (X64).
>> 2) EDKII Quark IA32
>> 3) EDKII Vlv2  X64
>> 4) EDKII OVMF IA32 and IA32X64. (with -smp 8)
>>
>> Cc: Jeff Fan 
>> Cc: Feng Tian 
>> Cc: Star Zeng 
>> Cc: Michael D Kinney 
>> Cc: Laszlo Ersek 
>> Cc: Paolo Bonzini 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jiewen Yao 
>>
>> Jiewen Yao (6):
>>   MdeModulePkg/Include: Add 

Re: [edk2] [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem for AsmRelocateApLoopFunc

[Fan, Jeff]

Laszlo,

Thanks your updating and pushing. 

Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Friday, November 11, 2016 8:08 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Paolo Bonzini; Yao, Jiewen
Subject: Re: [edk2] [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem for 
AsmRelocateApLoopFunc

On 11/11/16 11:51, Laszlo Ersek wrote:
> On 11/11/16 09:56, Jeff Fan wrote:
>> Current implementation just allocates reserve memory for 
>> AsmRelocateApLoopFunc.
>> It not be safe because APs will be placed into 32bit protected mode 
>> on long mode DXE. This reserve memory must be located below 4GB memory.
>>
>> This fix is to allocate < 4GB memory for AsmRelocateApLoopFunc.
>>
>> Cc: Laszlo Ersek 
>> Cc: Paolo Bonzini 
>> Cc: Jiewen Yao 
>> Cc: Michael D Kinney 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Jeff Fan 
>> ---
>>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 27 
>> ---
>>  1 file changed, 20 insertions(+), 7 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
>> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>> index eb36d6f..4b929ff 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
>> @@ -286,7 +286,8 @@ InitMpGlobalData (
>>IN CPU_MP_DATA   *CpuMpData
>>)
>>  {
>> -  EFI_STATUS Status;
>> +  EFI_STATUS Status;
>> +  EFI_PHYSICAL_ADDRESS   Address;
>>  
>>SaveCpuMpData (CpuMpData);
>>  
>> @@ -298,16 +299,28 @@ InitMpGlobalData (
>>}
>>  
>>//
>> -  // Avoid APs access invalid buff data which allocated by 
>> BootServices,
>> -  // so we will allocate reserved data for AP loop code.
>> +  // Avoid APs access invalid buffer data which allocated by 
>> + BootServices,  // so we will allocate reserved data for AP loop 
>> + code. We also need to

There was a superfluous space character at the end of this line (reported by 
git-am), but I removed it.

>> +  // allocate this buffer below 4GB due to APs may be transferred to 
>> + 32bit  // protected mode on long mode DXE.
>>// Allocating it in advance since memory services are not available in
>>// Exit Boot Services callback function.
>>//
>> -  mReservedApLoopFunc = AllocateReservedCopyPool (
>> -  CpuMpData->AddressMap.RelocateApLoopFuncSize,
>> -  CpuMpData->AddressMap.RelocateApLoopFuncAddress
>> -  );
>> +  Address = BASE_4GB - 1;
>> +  Status  = gBS->AllocatePages (
>> +   AllocateMaxAddress,
>> +   EfiReservedMemoryType,
>> +   EFI_SIZE_TO_PAGES (sizeof 
>> (CpuMpData->AddressMap.RelocateApLoopFuncSize)),
>> +   
>> +   );
>> +  ASSERT_EFI_ERROR (Status);
>> +  mReservedApLoopFunc = (VOID *) (UINTN) Address;
>>ASSERT (mReservedApLoopFunc != NULL);
>> +  CopyMem (
>> +mReservedApLoopFunc,
>> +CpuMpData->AddressMap.RelocateApLoopFuncAddress,
>> +CpuMpData->AddressMap.RelocateApLoopFuncSize
>> +);
>>  
>>Status = gBS->CreateEvent (
>>EVT_TIMER | EVT_NOTIFY_SIGNAL,
>>
> 
> Reviewed-by: Laszlo Ersek 
> 

Tested-by: Laszlo Ersek 
[ler...@redhat.com: strip whitespace at EOL]
Signed-off-by: Laszlo Ersek 

Commit ffd6b0b1b65e.

(I pushed the patch because it's simple -- Jeff is the sole maintainer, 
according to Maintainers.txt, of UefiCpuPkg, and I thought he'd push this 
simple patch with just my review anyway. I'm trying to flush my review / test 
queue, simplifying the possible orderings between the pending patch sets.)

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

[edk2] [PATCH 2/2] UefiCpuPkg/DxeMpLib: Place APs to suitable state on Legacy OS boot

[Jeff Fan]

Currently, DxeMpLib only places APs into specified c-state in Exit Boot Service
callback function for UEFI OS boot. We need to put APs into specified c-state
for legacy OS boot also.

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

Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  1 +
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   | 12 
 2 files changed, 13 insertions(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf 
b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 03a8994..972c9ad 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -58,6 +58,7 @@
 
 [Guids]
   gEfiEventExitBootServicesGuid ## CONSUMES  ## Event
+  gEfiEventLegacyBootGuid   ## CONSUMES  ## Event
 
 [Pcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## CONSUMES
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index 7ba4b80..793d947 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -22,6 +22,7 @@
 CPU_MP_DATA  *mCpuMpData = NULL;
 EFI_EVENTmCheckAllApsEvent = NULL;
 EFI_EVENTmMpInitExitBootServicesEvent = NULL;
+EFI_EVENTmLegacyBootEvent = NULL;
 volatile BOOLEAN mStopCheckAllApsStatus = TRUE;
 VOID *mReservedApLoopFunc = NULL;
 
@@ -340,6 +341,7 @@ InitMpGlobalData (
   AP_CHECK_INTERVAL
   );
   ASSERT_EFI_ERROR (Status);
+
   Status = gBS->CreateEvent (
   EVT_SIGNAL_EXIT_BOOT_SERVICES,
   TPL_CALLBACK,
@@ -348,6 +350,16 @@ InitMpGlobalData (
   
   );
   ASSERT_EFI_ERROR (Status);
+
+  Status = gBS->CreateEventEx (
+  EVT_NOTIFY_SIGNAL,
+  TPL_CALLBACK,
+  MpInitChangeApLoopCallback,
+  NULL,
+  ,
+  
+  );
+  ASSERT_EFI_ERROR (Status);
 }
 
 /**
-- 
2.9.3.windows.2

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

Re: [edk2] File mode problem on Github edk2-BaseTools-win32

[Laszlo Ersek]

On 11/11/16 12:05, Evan Lloyd wrote:
> There is a minor, but annoying, problem with file modes on the Github 
> edk2-BaseTools-win32 repository.
> Git maintains a limited internal record of the Unix style file modes.
> edk2-BaseTools-win32 currently causes Git to set the file's modes to 660.
> So, after a checkout or pull of master, the builds fail because the files do 
> not have Windows' "Read & Execute" permission.
> This is simple to fix, but one has to remember (or, ofttimes, get reminded) 
> to do it every time there is an update.
> 
> Liming,
> Because this is purely a permission problem in the Git repository, and .exe 
> files are not amenable to patching,

They are -- I think if you change the file mode bits, git will see that, and 
will create a patch that has no content hunks, just the file mode changes.

For example, in the BaseTools/Conf/ directory, we happen have two template 
files that have gratuitous execute permissions. If I remove those permissions, 
"git diff" shows

> diff --git a/BaseTools/Conf/build_rule.template 
> b/BaseTools/Conf/build_rule.template
> old mode 100755
> new mode 100644
> diff --git a/BaseTools/Conf/tools_def.template 
> b/BaseTools/Conf/tools_def.template
> old mode 100755
> new mode 100644

The same should be possible on Windows too. (You just need the inverse 
operation right now, chmod +x.)

(The above patch is one I could submit genuinely, but I'm too lazy. :))

> I have raised a pull request on  
> https://github.com/tianocore/edk2-BaseTools-win32/pulls
> This is only a minor thing, but I would deem it a great favour were you to 
> accept the pull request.
> It has me tearing my hair out, and I have little enough to begin with.  :-{

It is fine to send pull requests, but:
- they should be mailed to the list (not opened on github),
- the patches have to be reviewed first, anyway.

(Speaking about the edk2 repo at least -- I realize this is a different repo.)

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

Re: [edk2] [PATCH v2 6/9] OvmfPkg/PlatformBds: Dispatch deferred images after EndOfDxe

[Ni, Ruiyu]

Sorry for that! :(
I really forgot it.

发自我的 iPhone

> 在 2016年11月11日，下午7:16，Laszlo Ersek  写道：
> 
> Ray,
> 
>> On 11/08/16 14:04, Laszlo Ersek wrote:
>>> On 11/08/16 13:29, Ruiyu Ni wrote:
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ruiyu Ni 
>>> Cc: Jordan Justen 
>>> Cc: Laszlo Ersek 
>>> ---
>>> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 5 +
>>> 1 file changed, 5 insertions(+)
>>> 
>>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
>>> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>> index 66ee590..cc35630 100644
>>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>> @@ -389,6 +389,11 @@ Returns:
>>>   NULL);
>>>   ASSERT_EFI_ERROR (Status);
>>> 
>>> +  //
>>> +  // Dispatch deferred images after EndOfDxe event and ReadyToLock 
>>> installation.
>>> +  //
>>> +  EfiBootManagerDispatchDeferredImages ();
>>> +
>>>   PlatformInitializeConsole (gPlatformConsole);
>>>   PcdStatus = PcdSet16S (PcdPlatformBootTimeOut,
>>> GetFrontPageTimeoutFromQemu ());
>>> 
>> 
>> Can you please add a note to the commit message that, if the platform
>> installs EfiDxeSmmReadyToLockProtocol, then the deferred images must not
>> be dispatched prior to that either, not just prior to EndOfDxe?
>> 
>> Simultaneously, I propose the following subject (72 chars):
>> 
>> OvmfPkg/PlatformBds: Dispatch deferred images after EndOfDxe/ReadyToLock
>> 
>> No need to repost just because of this; the commit message can be
>> updated before you commit the series.
>> 
>> With the commit message update:
>> 
>> Reviewed-by: Laszlo Ersek 
> 
> I see that you added my R-b to the commit message.
> 
> 9789894e3ba3dae87bfc384e97f946caeab389e0
> 
> But, you didn't update the patch like I requested. :(
> 
> Please pay more attention. Otherwise next time I'll have to ask for a
> full repost, to make sure that my request is addressed.
> 
> 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 v2 6/9] OvmfPkg/PlatformBds: Dispatch deferred images after EndOfDxe

[Laszlo Ersek]

Ray,

On 11/08/16 14:04, Laszlo Ersek wrote:
> On 11/08/16 13:29, Ruiyu Ni wrote:
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ruiyu Ni 
>> Cc: Jordan Justen 
>> Cc: Laszlo Ersek 
>> ---
>>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
>> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> index 66ee590..cc35630 100644
>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> @@ -389,6 +389,11 @@ Returns:
>>NULL);
>>ASSERT_EFI_ERROR (Status);
>>  
>> +  //
>> +  // Dispatch deferred images after EndOfDxe event and ReadyToLock 
>> installation.
>> +  //
>> +  EfiBootManagerDispatchDeferredImages ();
>> +
>>PlatformInitializeConsole (gPlatformConsole);
>>PcdStatus = PcdSet16S (PcdPlatformBootTimeOut,
>>  GetFrontPageTimeoutFromQemu ());
>>
> 
> Can you please add a note to the commit message that, if the platform
> installs EfiDxeSmmReadyToLockProtocol, then the deferred images must not
> be dispatched prior to that either, not just prior to EndOfDxe?
> 
> Simultaneously, I propose the following subject (72 chars):
> 
> OvmfPkg/PlatformBds: Dispatch deferred images after EndOfDxe/ReadyToLock
> 
> No need to repost just because of this; the commit message can be
> updated before you commit the series.
> 
> With the commit message update:
> 
> Reviewed-by: Laszlo Ersek 

I see that you added my R-b to the commit message.

9789894e3ba3dae87bfc384e97f946caeab389e0

But, you didn't update the patch like I requested. :(

Please pay more attention. Otherwise next time I'll have to ask for a
full repost, to make sure that my request is addressed.

Laszlo

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

[edk2] File mode problem on Github edk2-BaseTools-win32

[Evan Lloyd]

There is a minor, but annoying, problem with file modes on the Github 
edk2-BaseTools-win32 repository.
Git maintains a limited internal record of the Unix style file modes.
edk2-BaseTools-win32 currently causes Git to set the file's modes to 660.
So, after a checkout or pull of master, the builds fail because the files do 
not have Windows' "Read & Execute" permission.
This is simple to fix, but one has to remember (or, ofttimes, get reminded) to 
do it every time there is an update.

Liming,
Because this is purely a permission problem in the Git repository, and .exe 
files are not amenable to patching, I have raised a pull request on  
https://github.com/tianocore/edk2-BaseTools-win32/pulls
This is only a minor thing, but I would deem it a great favour were you to 
accept the pull request.
It has me tearing my hair out, and I have little enough to begin with.  :-{

Regards,
Evan

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem for AsmRelocateApLoopFunc

[Laszlo Ersek]

On 11/11/16 09:56, Jeff Fan wrote:
> Current implementation just allocates reserve memory for 
> AsmRelocateApLoopFunc.
> It not be safe because APs will be placed into 32bit protected mode on long 
> mode
> DXE. This reserve memory must be located below 4GB memory.
> 
> This fix is to allocate < 4GB memory for AsmRelocateApLoopFunc.
> 
> Cc: Laszlo Ersek 
> Cc: Paolo Bonzini 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 27 ---
>  1 file changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> index eb36d6f..4b929ff 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
> @@ -286,7 +286,8 @@ InitMpGlobalData (
>IN CPU_MP_DATA   *CpuMpData
>)
>  {
> -  EFI_STATUS Status;
> +  EFI_STATUS Status;
> +  EFI_PHYSICAL_ADDRESS   Address;
>  
>SaveCpuMpData (CpuMpData);
>  
> @@ -298,16 +299,28 @@ InitMpGlobalData (
>}
>  
>//
> -  // Avoid APs access invalid buff data which allocated by BootServices,
> -  // so we will allocate reserved data for AP loop code.
> +  // Avoid APs access invalid buffer data which allocated by BootServices,
> +  // so we will allocate reserved data for AP loop code. We also need to 
> +  // allocate this buffer below 4GB due to APs may be transferred to 32bit
> +  // protected mode on long mode DXE.
>// Allocating it in advance since memory services are not available in
>// Exit Boot Services callback function.
>//
> -  mReservedApLoopFunc = AllocateReservedCopyPool (
> -  CpuMpData->AddressMap.RelocateApLoopFuncSize,
> -  CpuMpData->AddressMap.RelocateApLoopFuncAddress
> -  );
> +  Address = BASE_4GB - 1;
> +  Status  = gBS->AllocatePages (
> +   AllocateMaxAddress,
> +   EfiReservedMemoryType,
> +   EFI_SIZE_TO_PAGES (sizeof 
> (CpuMpData->AddressMap.RelocateApLoopFuncSize)),
> +   
> +   );
> +  ASSERT_EFI_ERROR (Status);
> +  mReservedApLoopFunc = (VOID *) (UINTN) Address;
>ASSERT (mReservedApLoopFunc != NULL);
> +  CopyMem (
> +mReservedApLoopFunc,
> +CpuMpData->AddressMap.RelocateApLoopFuncAddress,
> +CpuMpData->AddressMap.RelocateApLoopFuncSize
> +);
>  
>Status = gBS->CreateEvent (
>EVT_TIMER | EVT_NOTIFY_SIGNAL,
> 

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

Re: [edk2] [PATCH v2 3/3] UefiCpuPkg/PiSmmCpuDxeSmm: Decrease mNumberToFinish in AP safe code

[Paolo Bonzini]

On 11/11/2016 06:45, Jeff Fan wrote:
> We will put APs into hlt-loop in safe code. But we decrease mNumberToFinish
> before APs enter into the safe code. Paolo pointed out this gap.
> 
> This patch is to move mNumberToFinish decreasing to the safe code. It could
> make sure BSP could wait for all APs are running in safe code.
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=216
> 
> Reported-by: Paolo Bonzini 
> Cc: Laszlo Ersek 
> Cc: Paolo Bonzini 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 17 +++--
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c |  6 --
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h|  4 +++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c  |  6 --
>  4 files changed, 18 insertions(+), 15 deletions(-)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> index e53e096..34547e0 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c
> @@ -79,9 +79,11 @@ BOOLEAN  mAcpiS3Enable = TRUE;
>  
>  UINT8*mApHltLoopCode = NULL;
>  UINT8mApHltLoopCodeTemplate[] = {
> -   0xFA,// cli
> -   0xF4,// hlt
> -   0xEB, 0xFC   // jmp $-2
> +   0x8B, 0x44, 0x24, 0x04,  // mov  eax, dword 
> ptr [esp+4]
> +   0xF0, 0xFF, 0x08,// lock dec  dword 
> ptr [eax]
> +   0xFA,// cli
> +   0xF4,// hlt
> +   0xEB, 0xFC   // jmp $-2
> };
>  
>  /**
> @@ -399,17 +401,12 @@ MPRendezvousProcedure (
>}
>  
>//
> -  // Count down the number with lock mechanism.
> -  //
> -  InterlockedDecrement ();
> -
> -  //
> -  // Place AP into the safe code
> +  // Place AP into the safe code, count down the number with lock mechanism 
> in the safe code.
>//
>TopOfStack  = (UINT32) (UINTN) Stack + sizeof (Stack);
>TopOfStack &= ~(UINT32) (CPU_STACK_ALIGNMENT - 1);
>CopyMem ((VOID *) (UINTN) mApHltLoopCode, mApHltLoopCodeTemplate, sizeof 
> (mApHltLoopCodeTemplate));
> -  TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack);
> +  TransferApToSafeState ((UINT32) (UINTN) mApHltLoopCode, TopOfStack, 
> );
>  }
>  
>  /**
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> index 8b880d6..9760373 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c
> @@ -100,17 +100,19 @@ InitGdt (
>  
>@param[in] ApHltLoopCodeThe 32-bit address of the safe hlt-loop 
> function.
>@param[in] TopOfStack   A pointer to the new stack to use for the 
> ApHltLoopCode.
> +  @param[in] NumberToFinish   Semaphore of APs finish count.
>  
>  **/
>  VOID
>  TransferApToSafeState (
>IN UINT32 ApHltLoopCode,
> -  IN UINT32 TopOfStack
> +  IN UINT32 TopOfStack,
> +  IN UINT32 *NumberToFinish
>)
>  {
>SwitchStack (
>  (SWITCH_STACK_ENTRY_POINT) (UINTN) ApHltLoopCode,
> -NULL,
> +NumberToFinish,
>  NULL,
>  (VOID *) (UINTN) TopOfStack
>  );
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 6c98659..88d9c85 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -830,12 +830,14 @@ GetAcpiS3EnableFlag (
>  
>@param[in] ApHltLoopCodeThe 32-bit address of the safe hlt-loop 
> function.
>@param[in] TopOfStack   A pointer to the new stack to use for the 
> ApHltLoopCode.
> +  @param[in] NumberToFinish   Semaphore of APs finish count.
>  
>  **/
>  VOID
>  TransferApToSafeState (
>IN UINT32 ApHltLoopCode,
> -  IN UINT32 TopOfStack
> +  IN UINT32 TopOfStack,
> +  IN UINT32 *NumberToFinish
>);
>  
>  #endif
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> index 62338b7..6844c3f 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c
> @@ -105,18 +105,20 @@ GetProtectedModeCS (
>  
>@param[in] ApHltLoopCodeThe 32-bit address of the safe hlt-loop 
> function.
>@param[in] TopOfStack   A pointer to the new stack to use for the 
> ApHltLoopCode.
> +  @param[in] NumberToFinish   Semaphore of APs finish count.
>  
>  **/
>  VOID
>  

[edk2] [PATCH] ShellPkg/dmpstore: Support "-sfo"

[Ruiyu Ni]

The patch adds the "-sfo" support to "dmpstore" command.

1. For "-l" (load variable from file), when the variable
content can be updated, the new variable data is displayed
in SFO format, otherwise, the new variable data is not
displayed. So that the SFO consumer can know which variables
are updated by parsing the SFO.

2. For "-d" (delete variable), when the variable can be deleted
successfully, only the variable name and GUID are displayed in
SFO but the attribute/data/data size are displayed as empty to
indicate the deletion happened; otherwise, the variable is not
displayed.

3. For displaying variable, when the variable specified by name
and GUID cannot be found, an error message is displayed; Otherwise,
the SFO is displayed.
E.g.: "dmpstore -guid GuidThatDoesntExist -sfo" produces output
as:
ShellCommand,"dmpstore"
VariableInfo,"","GuidThatDoesntExist","","",""

"dmpstore NameThatDoesntExist -guid GuidThatDoesntExist -sfo"
produces output as:
ShellCommand,"dmpstore"
dmpstore: No matching variables found. Guid GuidThatDoesntExist, Name
NameThatDoesntExist

The difference between the above 2 cases is that former one only
specifies the GUID, but the latter one specifies both name and GUID.
Since not specifying GUID means to use GlobalVariableGuid,
"dmpstore NameThatDoesntExist -sfo" produces the similar output as
latter one.
I personally prefer to always produce SFO output for both cases.
But the above behavior is the discussion result between HPE engineers.

Signed-off-by: Ruiyu Ni 
Signed-off-by: Chen A Chen 
Cc: Jaben Carsey 
Cc: Tapan Shah 
---
 .../Library/UefiShellDebug1CommandsLib/DmpStore.c  | 269 +++--
 .../UefiShellDebug1CommandsLib.uni |   5 +
 2 files changed, 202 insertions(+), 72 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
index 3427c99..aa8ad09 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/DmpStore.c
@@ -2,7 +2,7 @@
   Main file for DmpStore shell Debug1 function.

   (C) Copyright 2013-2015 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2005 - 2015, Intel Corporation. All rights reserved.
+  Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -82,12 +82,46 @@ GetAttrType (
 }
 
 /**
+  Convert binary to hex format string.
+
+  @param[in]  BufferSizeThe size in bytes of the binary data.
+  @param[in]  BufferThe binary data.
+  @param[in, out] HexString Hex format string.
+  @param[in]  HexStringSize The size in bytes of the string.
+
+  @return The hex format string.
+**/
+CHAR16*
+BinaryToHexString (
+  IN VOID*Buffer,
+  IN UINTN   BufferSize,
+  IN OUT CHAR16  *HexString,
+  IN UINTN   HexStringSize
+  )
+{
+  UINTN Index;
+  UINTN StringIndex;
+
+  for (Index = 0, StringIndex = 0; Index < BufferSize; Index += 1) {
+StringIndex +=
+  UnicodeSPrint (
+[StringIndex],
+HexStringSize - StringIndex * sizeof (CHAR16),
+L"%02x",
+((UINT8 *) Buffer)[Index]
+);
+  }
+  return HexString;
+}
+
+/**
   Load the variable data from file and set to variable data base.
 
-  @param[in]  FileHandle The file to be read.
-  @param[in]  Name   The name of the variables to be loaded.
-  @param[in]  Guid   The guid of the variables to be loaded.
-  @param[out] Found  TRUE when at least one variable was loaded and 
set.
+  @param[in]  FileHandle   The file to be read.
+  @param[in]  Name The name of the variables to be loaded.
+  @param[in]  Guid The guid of the variables to be loaded.
+  @param[out] FoundTRUE when at least one variable was loaded 
and set.
+  @param[in]  StandardFormatOutput TRUE indicates Standard-Format Output.
 
   @retval SHELL_DEVICE_ERROR  Cannot access the file.
   @retval SHELL_VOLUME_CORRUPTED  The file is in bad format.
@@ -99,7 +133,8 @@ LoadVariablesFromFile (
   IN SHELL_FILE_HANDLE FileHandle,
   IN CONST CHAR16  *Name,
   IN CONST EFI_GUID*Guid,
-  OUT BOOLEAN  *Found
+  OUT BOOLEAN  *Found,
+  IN BOOLEAN   StandardFormatOutput
   )
 {
   EFI_STATUS   Status;
@@ -116,6 +151,7 @@ LoadVariablesFromFile (
   CHAR16   *Attributes;
   UINT8*Buffer;
   UINT32   Crc32;
+  CHAR16   *HexString;
 
   Status = ShellGetFileSize (FileHandle, );
   if (EFI_ERROR (Status)) {
@@ -221,11 +257,6 @@ LoadVariablesFromFile (
 ((Guid == NULL) || CompareGuid (>Guid, Guid))
) {
   

Re: [edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Yao, Jiewen]

HI Laszlo
I fixed the IA32 boot issue in this patch with DEBUG message update you 
suggested.

My unit test failed before. Now it can pass.
I validated on a real IA32 and Windows OVMF with and without XD.


For QEMU installation, it is still on progress.
We have setup a Fedora 24 host, download QEMU, and install it.
But we are still struggling to make QEMU boot on Fedora.
Your step by step is great. There is still some minor place we stuck in due to 
my ignorance.
My goal is still to setup an environment like yours for our validation or issue 
reproduce.
It just need take some time, more than I expected. sign...

Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiewen Yao
> Sent: Friday, November 11, 2016 5:01 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng ; Kinney, Michael D
> ; Paolo Bonzini ;
> Laszlo Ersek ; Fan, Jeff ; Zeng,
> Star 
> Subject: [edk2] [PATCH V3 0/6] Enable SMM page level protection.
> 
> 
>  below is V3 description 
> 1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition.
> (Many thanks to Laszlo Ersek  for catching it.)
> 2) PiSmmCpu: Add ASSERT for CpuIndex check.
> 3) PiSmmCpu: Use DEBUG_VERBOSE for page table update.
> 4) PiSmmCpu: Do not report DEBUG message for Ap non present
> when PcdCpuSmmSyncMode==1 (Relex mode).
> 5) PiSmmCpu: Do not report DEBUG message for AP removed
> when PcdCpuHotPlugSupport==TRUE.
> 
> Tested combination:
> 1) XD disabled
> 2) XD enabled in SMM and disabled in non-SMM.
> 3) XD enabled in SMM and enabled in non-SMM.
> 
>  below is V2 description 
> 1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
> 2) PiSmmCpu: Add debug info on StartupAp() fails.
> 3) PiSmmCpu: Add ASSERT for AllocatePages().
> 4) PiSmmCpu: Add protection detail in commit message.
> 5) UefiCpuPkg.dsc: Add page table footprint info in commit message.
> 
>  below is V1 description 
> This series patch enables SMM page level protection.
> Features are:
> 1) PiSmmCore reports SMM PE image code/data information
> in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
> 2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
> and set XD for data page and RO for code page.
> 3) PiSmmCpu enables Static Paging for X64 according to
> PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
> is used as long as it is supported.
> 4) PiSmmCpu sets importance data structure to be read only,
> such as Gdt, Idt, SmmEntrypoint, and PageTable itself.
> 
> tested platform:
> 1) Intel internal platform (X64).
> 2) EDKII Quark IA32
> 3) EDKII Vlv2  X64
> 4) EDKII OVMF IA32 and IA32X64. (with -smp 8)
> 
> Cc: Jeff Fan 
> Cc: Feng Tian 
> Cc: Star Zeng 
> Cc: Michael D Kinney 
> Cc: Laszlo Ersek 
> Cc: Paolo Bonzini 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiewen Yao 
> 
> Jiewen Yao (6):
>   MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
>   MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
>   MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
>   UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
>   UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
>   QuarkPlatformPkg/dsc: enable Smm paging protection.
> 
>  MdeModulePkg/Core/PiSmmCore/Dispatcher.c   |   66 +
>  MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c| 1509
> 
>  MdeModulePkg/Core/PiSmmCore/Page.c |  775
> +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c|   40
> +
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   91
> ++
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |2
> +
>  MdeModulePkg/Core/PiSmmCore/Pool.c |   16
> +
>  MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h |   51 +
>  MdeModulePkg/MdeModulePkg.dec  |
> 3 +
>  QuarkPlatformPkg/Quark.dsc |6 +
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   71
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |   75
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|   75
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |   79
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S  |  226
> +--
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm|   36
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |   36
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c  |   37
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c|4
> +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  135
> +-
>  

[edk2] [Patch] MdePkg IndustryStandard: Add DDR3, DDR4 and LPDDR definition per SPD spec

[Liming Gao]

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

Cc: Giri P Mudusuru 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdePkg/Include/IndustryStandard/SdramSpd.h | 1682 +++-
 1 file changed, 1670 insertions(+), 12 deletions(-)

diff --git a/MdePkg/Include/IndustryStandard/SdramSpd.h 
b/MdePkg/Include/IndustryStandard/SdramSpd.h
index 2b2012b..e3f6eea 100644
--- a/MdePkg/Include/IndustryStandard/SdramSpd.h
+++ b/MdePkg/Include/IndustryStandard/SdramSpd.h
@@ -1,14 +1,24 @@
 /** @file
   This file contains definitions for the SPD fields on an SDRAM.
-
-  Copyright (c) 2007 - 2008, 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. 
+
+  Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD 
License
+  which accompanies this distribution.  The full text of the license may be 
found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+  @par Revision Reference:
+- Serial Presence Detect (SPD) for DDR3 SDRAM Modules Document Release 6
+  http://www.jedec.org/sites/default/files/docs/4_01_02_11R21A.pdf
+
+- Serial Presence Detect (SPD) for DDR4 SDRAM Modules Document Release 4
+  http://www.jedec.org/standards-documents/docs/spd412l-4
+
+- Serial Presence Detect (SPD) for LPDDR3 and LPDDR4 SDRAM Modules 
Document Release 2
+  http://www.jedec.org/standards-documents/docs/spd412m-2
 **/
 
 #ifndef _SDRAM_SPD_H_
@@ -47,9 +57,14 @@
 //
 // Memory Type Definitions
 //
-#define SPD_VAL_SDR_TYPE  4 ///< SDR SDRAM memory
-#define SPD_VAL_DDR_TYPE  7 ///< DDR SDRAM memory
-#define SPD_VAL_DDR2_TYPE 8 ///< DDR2 SDRAM memory
+#define SPD_VAL_SDR_TYPE  4  ///< SDR SDRAM memory
+#define SPD_VAL_DDR_TYPE  7  ///< DDR SDRAM memory
+#define SPD_VAL_DDR2_TYPE 8  ///< DDR2 SDRAM memory
+#define SPD_VAL_DDR3_TYPE 11 ///< DDR3 SDRAM memory
+#define SPD_VAL_DDR4_TYPE 12 ///< DDR4 SDRAM memory
+#define SPD_VAL_LPDDR3_TYPE 15 ///< LPDDR3 SDRAM memory
+#define SPD_VAL_LPDDR4_TYPE 16 ///< LPDDR4 SDRAM memory
+
 //
 // ECC Type Definitions
 //
@@ -62,4 +77,1647 @@
 #define SPD_BUFFERED0x01
 #define SPD_REGISTERED  0x02
 
+#pragma pack (push, 1)
+
+typedef union {
+  struct {
+UINT8  BytesUsed   :  4; ///< Bits 3:0
+UINT8  BytesTotal  :  3; ///< Bits 6:4
+UINT8  CrcCoverage :  1; ///< Bits 7:7
+  } Bits;
+  UINT8  Data;
+} SPD_DEVICE_DESCRIPTION_STRUCT;
+
+typedef union {
+  struct {
+UINT8  Minor   :  4; ///< Bits 3:0
+UINT8  Major   :  4; ///< Bits 7:4
+  } Bits;
+  UINT8  Data;
+} SPD_REVISION_STRUCT;
+
+typedef union {
+  struct {
+UINT8  Type:  8; ///< Bits 7:0
+  } Bits;
+  UINT8  Data;
+} SPD_DRAM_DEVICE_TYPE_STRUCT;
+
+typedef union {
+  struct {
+UINT8  ModuleType  :  4; ///< Bits 3:0
+UINT8  Reserved:  4; ///< Bits 7:4
+  } Bits;
+  UINT8  Data;
+} SPD_MODULE_TYPE_STRUCT;
+
+typedef union {
+  struct {
+UINT8  Density :  4; ///< Bits 3:0
+UINT8  BankAddress :  3; ///< Bits 6:4
+UINT8  Reserved:  1; ///< Bits 7:7
+  } Bits;
+  UINT8  Data;
+} SPD_SDRAM_DENSITY_BANKS_STRUCT;
+
+typedef union {
+  struct {
+UINT8  ColumnAddress   :  3; ///< Bits 2:0
+UINT8  RowAddress  :  3; ///< Bits 5:3
+UINT8  Reserved:  2; ///< Bits 7:6
+  } Bits;
+  UINT8  Data;
+} SPD_SDRAM_ADDRESSING_STRUCT;
+
+typedef union {
+  struct {
+UINT8  OperationAt1_50 :  1; ///< Bits 0:0
+UINT8  OperationAt1_35 :  1; ///< Bits 1:1
+UINT8  OperationAt1_25 :  1; ///< Bits 2:2
+UINT8  Reserved:  5; ///< Bits 7:3
+  } Bits;
+  UINT8  Data;
+} SPD_MODULE_NOMINAL_VOLTAGE_STRUCT;
+
+typedef union {
+  struct {
+UINT8  SdramDeviceWidth:  3; ///< Bits 2:0
+UINT8  

[edk2] [PATCH V3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.

[Jiewen Yao]

PiSmmCpuDxeSmm consumes SmmAttributesTable and setup page table:
1) Code region is marked as read-only and Data region is non-executable,
if the PE image is 4K aligned.
2) Important data structure is set to RO, such as GDT/IDT.
3) SmmSaveState is set to non-executable,
and SmmEntrypoint is set to read-only.
4) If static page is supported, page table is read-only.

We use page table to protect other components, and itself.

If we use dynamic paging, we can still provide *partial* protection.
And hope page table is not modified by other components.

The XD enabling code is moved to SmiEntry to let NX take effect.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |  71 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |  75 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|  75 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |  79 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S  | 226 +
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm|  36 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |  36 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c  |  37 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c|   4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  | 135 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 144 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 156 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   5 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 871 
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |  39 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h |  15 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 274 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   |  59 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm |  62 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm|  69 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.S   | 250 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.asm |  35 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.nasm|  31 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c   |  30 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c |   7 +-
 25 files changed, 2044 insertions(+), 777 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
index a871bef..65f09e5 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
@@ -58,7 +58,7 @@ SmmInitPageTable (
   if (FeaturePcdGet (PcdCpuSmmStackGuard)) {
 InitializeIDTSmmStackGuard ();
   }
-  return Gen4GPageTable (0, TRUE);
+  return Gen4GPageTable (TRUE);
 }
 
 /**
@@ -99,7 +99,7 @@ SmiPFHandler (
   if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
   (PFAddress >= mCpuHotPlugData.SmrrBase) &&
   (PFAddress < (mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize))) {
-DEBUG ((EFI_D_ERROR, "SMM stack overflow!\n"));
+DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
 CpuDeadLoop ();
   }
 
@@ -109,7 +109,7 @@ SmiPFHandler (
   if ((PFAddress < mCpuHotPlugData.SmrrBase) ||
   (PFAddress >= mCpuHotPlugData.SmrrBase + mCpuHotPlugData.SmrrSize)) {
 if ((SystemContext.SystemContextIa32->ExceptionData & IA32_PF_EC_ID) != 0) 
{
-  DEBUG ((EFI_D_ERROR, "Code executed on IP(0x%x) out of SMM range after 
SMM is locked!\n", PFAddress));
+  DEBUG ((DEBUG_ERROR, "Code executed on IP(0x%x) out of SMM range after 
SMM is locked!\n", PFAddress));
   DEBUG_CODE (
 DumpModuleInfoByIp (*(UINTN 
*)(UINTN)SystemContext.SystemContextIa32->Esp);
   );
@@ -128,3 +128,68 @@ SmiPFHandler (
 
   ReleaseSpinLock (mPFLock);
 }
+
+/**
+  This function sets memory attribute for page table.
+**/
+VOID
+SetPageTableAttributes (
+  VOID
+  )
+{
+  UINTN Index2;
+  UINTN Index3;
+  UINT64*L1PageTable;
+  UINT64*L2PageTable;
+  UINT64*L3PageTable;
+  BOOLEAN   IsSplitted;
+  BOOLEAN   PageTableSplitted;
+
+  DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));
+
+  //
+  // Disable write protection, because we need mark page table to be write 
protected.
+  // We need *write* page table memory, to mark itself to be *read only*.
+  //
+  AsmWriteCr0 (AsmReadCr0() & ~CR0_WP);
+
+  do {
+DEBUG ((DEBUG_INFO, "Start...\n"));
+PageTableSplitted = FALSE;
+
+L3PageTable = (UINT64 *)GetPageTableBase ();
+
+SmmSetMemoryAttributesEx ((EFI_PHYSICAL_ADDRESS)(UINTN)L3PageTable, 
SIZE_4KB, EFI_MEMORY_RO, );
+PageTableSplitted = (PageTableSplitted || IsSplitted);
+
+for (Index3 = 0; Index3 < 4; Index3++) {

[edk2] [PATCH V3 6/6] QuarkPlatformPkg/dsc: enable Smm paging protection.

[Jiewen Yao]

Cc: Michael D Kinney 
Cc: Kelly Steele 
Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 QuarkPlatformPkg/Quark.dsc | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/QuarkPlatformPkg/Quark.dsc b/QuarkPlatformPkg/Quark.dsc
index d5988da..9804b70 100644
--- a/QuarkPlatformPkg/Quark.dsc
+++ b/QuarkPlatformPkg/Quark.dsc
@@ -891,3 +891,9 @@
 
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096
+
+# Force PE/COFF sections to be aligned at 4KB boundaries to support page level 
protection of DXE_SMM_DRIVER/SMM_CORE modules
+[BuildOptions.common.EDKII.DXE_SMM_DRIVER, BuildOptions.common.EDKII.SMM_CORE]
+  MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
-- 
2.7.4.windows.1

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

[edk2] [PATCH V3 2/6] MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.

[Jiewen Yao]

This table describes the SMM memory attributes.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/MdeModulePkg.dec | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 74b8700..99a028f 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -355,6 +355,9 @@
   ## Include/Guid/PiSmmCommunicationRegionTable.h
   gEdkiiPiSmmCommunicationRegionTableGuid = { 0x4e28ca50, 0xd582, 0x44ac, 
{0xa1, 0x1f, 0xe3, 0xd5, 0x65, 0x26, 0xdb, 0x34}}
 
+  ## Include/Guid/PiSmmMemoryAttributesTable.h
+  gEdkiiPiSmmMemoryAttributesTableGuid = { 0x6b9fd3f7, 0x16df, 0x45e8, {0xbd, 
0x39, 0xb9, 0x4a, 0x66, 0x54, 0x1a, 0x5d}}
+
 [Ppis]
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid   = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 
0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
-- 
2.7.4.windows.1

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

[edk2] [PATCH V3 4/6] UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.

[Jiewen Yao]

If enabled, SMM will not use on-demand paging.
SMM will build static page table for all memory.

The page table size depend on 2 things:
1) The 1G paging capability.
2) The whole system memory/MMIO addressing capability.

A) If the system only supports 2M paging,
When the whole memory/MMIO is 32bit, we only need 1+1+4=6 pages for 4G.
When the whole memory/MMIO is 39bit, we need 1+1+256 pages (~ 1M)
When the whole memory/MMIO is 48bit, we need 1+256+256*256 pages (~ 257M)

B) If the system supports 1G paging.
When the whole memory/MMIO is 32bit, we only need 1+1+4=6 pages for 4G.
(We still generate 2M page for maintenance consideration.)
When the whole memory/MMIO is 39bit, we still need 6 pages.
(We setup 1G paging for >1G.)
When the whole memory/MMIO is 48bit, we need 1+256 pages (~ 1M).

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 UefiCpuPkg/UefiCpuPkg.dec | 8 
 1 file changed, 8 insertions(+)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 8674533..a110820 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -199,6 +199,14 @@
   # @Prompt The specified AP target C-state for Mwait.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate|0|UINT8|0x0007
 
+  ## Indicates if SMM uses static page table.
+  #  If enabled, SMM will not use on-demand paging. SMM will build static page 
table for all memory.
+  #  This flag only impacts X64 build, because SMM alway builds static page 
table for IA32.
+  #   TRUE  - SMM uses static page table for all memory.
+  #   FALSE - SMM uses static page table for below 4G memory and use on-demand 
paging for above 4G memory.
+  # @Prompt Use static page table for all memory in SMM.
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable|TRUE|BOOLEAN|0x3213210D
+
 [PcdsDynamic, PcdsDynamicEx]
   ## Contains the pointer to a CPU S3 data buffer of structure ACPI_CPU_DATA.
   # @Prompt The pointer to a CPU S3 data buffer.
-- 
2.7.4.windows.1

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

[edk2] [PATCH V3 0/6] Enable SMM page level protection.

[Jiewen Yao]

 below is V3 description 
1) PiSmmCpu: Fix CpuIndex corruption issue due to stack malposition.
(Many thanks to Laszlo Ersek  for catching it.)
2) PiSmmCpu: Add ASSERT for CpuIndex check.
3) PiSmmCpu: Use DEBUG_VERBOSE for page table update.
4) PiSmmCpu: Do not report DEBUG message for Ap non present
when PcdCpuSmmSyncMode==1 (Relex mode).
5) PiSmmCpu: Do not report DEBUG message for AP removed
when PcdCpuHotPlugSupport==TRUE.

Tested combination:
1) XD disabled
2) XD enabled in SMM and disabled in non-SMM.
3) XD enabled in SMM and enabled in non-SMM.

 below is V2 description 
1) PiSmmCpu: resolve OVMF multiple processors boot hang issue.
2) PiSmmCpu: Add debug info on StartupAp() fails.
3) PiSmmCpu: Add ASSERT for AllocatePages().
4) PiSmmCpu: Add protection detail in commit message.
5) UefiCpuPkg.dsc: Add page table footprint info in commit message.

 below is V1 description 
This series patch enables SMM page level protection.
Features are:
1) PiSmmCore reports SMM PE image code/data information
in EdkiiPiSmmMemoryAttributeTable, if the SMM image is page aligned.
2) PiSmmCpu consumes EdkiiPiSmmMemoryAttributeTable
and set XD for data page and RO for code page.
3) PiSmmCpu enables Static Paging for X64 according to
PcdCpuSmmStaticPageTable. If it is true, 1G paging for above 4G
is used as long as it is supported.
4) PiSmmCpu sets importance data structure to be read only,
such as Gdt, Idt, SmmEntrypoint, and PageTable itself.

tested platform:
1) Intel internal platform (X64).
2) EDKII Quark IA32
3) EDKII Vlv2  X64
4) EDKII OVMF IA32 and IA32X64. (with -smp 8)

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 

Jiewen Yao (6):
  MdeModulePkg/Include: Add PiSmmMemoryAttributesTable.h
  MdeModulePkg/dec: Add gEdkiiPiSmmMemoryAttributesTableGuid.
  MdeModulePkg/PiSmmCore: Add MemoryAttributes support.
  UefiCpuPkg/dec: Add PcdCpuSmmStaticPageTable.
  UefiCpuPkg/PiSmmCpuDxeSmm: Add paging protection.
  QuarkPlatformPkg/dsc: enable Smm paging protection.

 MdeModulePkg/Core/PiSmmCore/Dispatcher.c   |   66 +
 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c| 1509 

 MdeModulePkg/Core/PiSmmCore/Page.c |  775 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c|   40 +
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   91 ++
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf  |2 +
 MdeModulePkg/Core/PiSmmCore/Pool.c |   16 +
 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h |   51 +
 MdeModulePkg/MdeModulePkg.dec  |3 +
 QuarkPlatformPkg/Quark.dsc |6 +
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c   |   71 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.S  |   75 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.asm|   75 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiEntry.nasm   |   79 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.S  |  226 +--
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.asm|   36 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm   |   36 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmFuncsArch.c  |   37 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmmProfileArch.c|4 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c  |  135 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c |  144 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  156 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |5 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c |  871 +++
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c |   39 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h |   15 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c|  274 +++-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.S   |   59 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.asm |   62 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiEntry.nasm|   69 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.S   |  250 +---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.asm |   35 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmiException.nasm|   31 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c   |   30 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmProfileArch.c |7 +-
 UefiCpuPkg/UefiCpuPkg.dec  |8 +
 36 files changed, 4585 insertions(+), 803 deletions(-)
 create mode 100644 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c
 create mode 100644 MdeModulePkg/Include/Guid/PiSmmMemoryAttributesTable.h
 create mode 100644 

[edk2] [PATCH] MdeModulePkg/CapsuleApp: remove unused definition.

[Jiewen Yao]

remove EFI_CAPSULE_FROM_FILE_DIR
remove EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED.
no one uses them.

Cc: Michael D Kinney 
Cc: Feng Tian 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Application/CapsuleApp/CapsuleApp.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c 
b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
index 81a98ae..23672ae 100644
--- a/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
+++ b/MdeModulePkg/Application/CapsuleApp/CapsuleApp.c
@@ -38,9 +38,6 @@
 #define SYSTEM_FIRMWARE_FLAG 0x5
 #define DEVICE_FIRMWARE_FLAG 0x78010
 
-#define EFI_CAPSULE_FROM_FILE_DIR   
L"\\EFI\\UpdateCapsule\\"
-#define EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED  0x0004
-
 #define MAJOR_VERSION   1
 #define MINOR_VERSION   0
 
-- 
2.7.4.windows.1

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

[edk2] [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem for AsmRelocateApLoopFunc

[Jeff Fan]

Current implementation just allocates reserve memory for AsmRelocateApLoopFunc.
It not be safe because APs will be placed into 32bit protected mode on long mode
DXE. This reserve memory must be located below 4GB memory.

This fix is to allocate < 4GB memory for AsmRelocateApLoopFunc.

Cc: Laszlo Ersek 
Cc: Paolo Bonzini 
Cc: Jiewen Yao 
Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan 
---
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 27 ---
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
index eb36d6f..4b929ff 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c
@@ -286,7 +286,8 @@ InitMpGlobalData (
   IN CPU_MP_DATA   *CpuMpData
   )
 {
-  EFI_STATUS Status;
+  EFI_STATUS Status;
+  EFI_PHYSICAL_ADDRESS   Address;
 
   SaveCpuMpData (CpuMpData);
 
@@ -298,16 +299,28 @@ InitMpGlobalData (
   }
 
   //
-  // Avoid APs access invalid buff data which allocated by BootServices,
-  // so we will allocate reserved data for AP loop code.
+  // Avoid APs access invalid buffer data which allocated by BootServices,
+  // so we will allocate reserved data for AP loop code. We also need to 
+  // allocate this buffer below 4GB due to APs may be transferred to 32bit
+  // protected mode on long mode DXE.
   // Allocating it in advance since memory services are not available in
   // Exit Boot Services callback function.
   //
-  mReservedApLoopFunc = AllocateReservedCopyPool (
-  CpuMpData->AddressMap.RelocateApLoopFuncSize,
-  CpuMpData->AddressMap.RelocateApLoopFuncAddress
-  );
+  Address = BASE_4GB - 1;
+  Status  = gBS->AllocatePages (
+   AllocateMaxAddress,
+   EfiReservedMemoryType,
+   EFI_SIZE_TO_PAGES (sizeof 
(CpuMpData->AddressMap.RelocateApLoopFuncSize)),
+   
+   );
+  ASSERT_EFI_ERROR (Status);
+  mReservedApLoopFunc = (VOID *) (UINTN) Address;
   ASSERT (mReservedApLoopFunc != NULL);
+  CopyMem (
+mReservedApLoopFunc,
+CpuMpData->AddressMap.RelocateApLoopFuncAddress,
+CpuMpData->AddressMap.RelocateApLoopFuncSize
+);
 
   Status = gBS->CreateEvent (
   EVT_TIMER | EVT_NOTIFY_SIGNAL,
-- 
2.9.3.windows.2

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

Re: [edk2] [patch] MdePkg: Fix spec mismatch in string representation of EMMC dev node

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Feng Tian
> Sent: Thursday, November 10, 2016 1:17 PM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Jin, Eric 
> Subject: [edk2] [patch] MdePkg: Fix spec mismatch in string representation
> of EMMC dev node
> 
> Cc: Eric Jin 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Feng Tian 
> ---
>  MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c | 2 +-
>  MdePkg/Library/UefiDevicePathLib/DevicePathToText.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> index c167b4c..8a3a470 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathFromText.c
> @@ -3529,7 +3529,7 @@ GLOBAL_REMOVE_IF_UNREFERENCED
> DEVICE_PATH_FROM_TEXT_TABLE mUefiDevicePathLibDevP
>{L"NVMe",DevPathFromTextNVMe},
>{L"UFS", DevPathFromTextUfs },
>{L"SD",  DevPathFromTextSd  },
> -  {L"Emmc",DevPathFromTextEmmc},
> +  {L"eMMC",DevPathFromTextEmmc},
>{L"DebugPort",   DevPathFromTextDebugPort   },
>{L"MAC", DevPathFromTextMAC },
>{L"IPv4",DevPathFromTextIPv4},
> diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> index 5922dee..87eca23 100644
> --- a/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathToText.c
> @@ -850,7 +850,7 @@ DevPathToTextEmmc (
>Emmc = DevPath;
>UefiDevicePathLibCatPrint (
>  Str,
> -L"Emmc(0x%x)",
> +L"eMMC(0x%x)",
>  Emmc->SlotNumber
>  );
>  }
> --
> 2.7.1.windows.2
> 
> ___
> 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] DuetPkg: Add POSTBUILD in DSC files to run post-build automatically

[Ni, Ruiyu]

Hao,
Thanks for making this helpful updates.

Reviewed-by: Ruiyu Ni 

>-Original Message-
>From: Wu, Hao A
>Sent: Friday, November 11, 2016 4:27 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A ; Ni, Ruiyu 
>Subject: [PATCH 3/3] DuetPkg: Add POSTBUILD in DSC files to run post-build 
>automatically
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=220
>
>Currently, the post-build scripts PostBuild.bat/PostBuild.sh in DuetPkg
>need to be run manually. Especially for Windows batch script, it also
>requires users to set the build options (like tool chain, target and arch)
>in file Conf/target.txt. If users using command line options via '-t' or
>'-a', the post-build script won't work properly.
>
>The package DSC files now support the feature to execute post-build script
>automatically by adding a 'POSTBUILD' definition. This feature also passes
>the build options into the post-build script as parameters. This commit
>uses this feature to make the post-build works for DuetPkg more
>user-friendly. Also, ReadMe.txt is updated to reflect the new steps for
>UEFI Emulation (DUET) development.
>
>Cc: Ruiyu Ni 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Hao Wu 
>---
> DuetPkg/CreateBootDisk.bat |  8 ++
> DuetPkg/CreateBootDisk.sh  | 10 ++-
> DuetPkg/DuetPkgIa32.dsc|  5 
> DuetPkg/DuetPkgX64.dsc |  5 
> DuetPkg/GetVariables.bat   | 39 -
> DuetPkg/PostBuild.bat  | 49 
> DuetPkg/PostBuild.sh   | 60 ---
> DuetPkg/ReadMe.txt | 71 +-
> DuetPkg/build32.sh |  4 +--
> DuetPkg/build64.sh |  4 +--
> 10 files changed, 105 insertions(+), 150 deletions(-)
> delete mode 100644 DuetPkg/GetVariables.bat
>
>diff --git a/DuetPkg/CreateBootDisk.bat b/DuetPkg/CreateBootDisk.bat
>index 7265837..cee04b8 100644
>--- a/DuetPkg/CreateBootDisk.bat
>+++ b/DuetPkg/CreateBootDisk.bat
>@@ -1,7 +1,7 @@
> @echo off
> @REM ## @file
> @REM #
>-@REM #  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
>+@REM #  Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
> @REM #
> @REM #  This program and the accompanying materials
> @REM #  are licensed and made available under the terms and conditions of the 
> BSD License
>@@ -15,14 +15,11 @@
>
> @REM Set up environment at first.
>
>-set BASETOOLS_DIR=%WORKSPACE_TOOLS_PATH%\Bin\Win32
>+set BASETOOLS_DIR=%EDK_TOOLS_BIN%
> set BOOTSECTOR_BIN_DIR=%WORKSPACE%\DuetPkg\BootSector\bin
> set DISK_LABEL=DUET
> set PROCESSOR=""
> set STEP=1
>-call %WORKSPACE%\DuetPkg\GetVariables.bat
>-
>-echo on
>
> if "%1"=="" goto Help
> if "%2"=="" goto Help
>@@ -35,6 +32,7 @@ set EFI_BOOT_DISK=%2
> if "%TARGET_ARCH%"=="IA32" set PROCESSOR=IA32
> if "%TARGET_ARCH%"=="X64" set PROCESSOR=X64
> if %PROCESSOR%=="" goto WrongArch
>+call %WORKSPACE%\DuetPkg\SetEnv_%PROCESSOR%.bat
> set BUILD_DIR=%WORKSPACE%\Build\DuetPkg%PROCESSOR%\%TARGET%_%TOOL_CHAIN_TAG%
>
> if "%1"=="floppy" goto CreateFloppy
>diff --git a/DuetPkg/CreateBootDisk.sh b/DuetPkg/CreateBootDisk.sh
>index fa00408..897ba9b 100755
>--- a/DuetPkg/CreateBootDisk.sh
>+++ b/DuetPkg/CreateBootDisk.sh
>@@ -34,7 +34,7 @@ if [ \
>  "$*" = "--help" \
>]
> then
>-echo "Usage: CreateBootDisk [usb|floppy|ide|file] MediaPath DevicePath 
>[FAT12|FAT16|FAT32] [IA32|X64]
>[GCC44|UNIXGCC]"
>+echo "Usage: CreateBootDisk [usb|floppy|ide|file] MediaPath DevicePath 
>[FAT12|FAT16|FAT32] [IA32|X64]"
> echo "e.g. : CreateBootDisk floppy /media/floppy0 /dev/fd0 FAT12 IA32"
> PROCESS_MARK=FALSE
> fi
>@@ -51,13 +51,7 @@ case "$5" in
>  return 1
> esac
>
>-if [ -z "$6" ]
>-then
>-  TOOLCHAIN=GCC44
>-else
>-  TOOLCHAIN=$6
>-fi
>-
>+. $WORKSPACE/DuetPkg/SetEnv_$PROCESSOR.sh
> export BUILD_DIR=$WORKSPACE/Build/DuetPkg$PROCESSOR/DEBUG_$TOOLCHAIN
>
>
>diff --git a/DuetPkg/DuetPkgIa32.dsc b/DuetPkg/DuetPkgIa32.dsc
>index 86346f3..3b59343 100644
>--- a/DuetPkg/DuetPkgIa32.dsc
>+++ b/DuetPkg/DuetPkgIa32.dsc
>@@ -31,6 +31,11 @@
>   BUILD_TARGETS  = DEBUG
>   SKUID_IDENTIFIER   = DEFAULT
>   FLASH_DEFINITION   = DuetPkg/DuetPkg.fdf
>+!if $(TOOL_CHAIN_TAG) == GCC47 || $(TOOL_CHAIN_TAG) == GCC48 || 
>$(TOOL_CHAIN_TAG) == GCC49 ||
>$(TOOL_CHAIN_TAG) == GCC5
>+  POSTBUILD  = DuetPkg/PostBuild.sh
>+!else
>+  POSTBUILD  = DuetPkg/PostBuild.bat
>+!endif
>
> 
> #
>diff --git a/DuetPkg/DuetPkgX64.dsc b/DuetPkg/DuetPkgX64.dsc
>index e0aeb5c..c23354a 100644
>--- a/DuetPkg/DuetPkgX64.dsc
>+++ b/DuetPkg/DuetPkgX64.dsc
>@@ -31,6 +31,11 @@
>   BUILD_TARGETS  = DEBUG
>   SKUID_IDENTIFIER   = DEFAULT
>   FLASH_DEFINITION   = DuetPkg/DuetPkg.fdf

Re: [edk2] [PATCH 1/3] DuetPkg: Resolve white-space issues for post-build scripts & ReadMe

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Regards,
Ray

>-Original Message-
>From: Wu, Hao A
>Sent: Friday, November 11, 2016 4:27 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A ; Ni, Ruiyu 
>Subject: [PATCH 1/3] DuetPkg: Resolve white-space issues for post-build 
>scripts & ReadMe
>
>Cc: Ruiyu Ni 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Hao Wu 
>---
> DuetPkg/CreateBootDisk.bat |  14 +--
> DuetPkg/CreateBootDisk.sh  | 304 ++---
> DuetPkg/PostBuild.bat  |   2 +-
> DuetPkg/PostBuild.sh   |  58 -
> DuetPkg/ReadMe.txt |  44 +++
> 5 files changed, 211 insertions(+), 211 deletions(-)
>
>diff --git a/DuetPkg/CreateBootDisk.bat b/DuetPkg/CreateBootDisk.bat
>index 541de81..214b5b6 100644
>--- a/DuetPkg/CreateBootDisk.bat
>+++ b/DuetPkg/CreateBootDisk.bat
>@@ -56,7 +56,7 @@ goto Help
> @copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
> @%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
> @REM @del FDBS.com
>-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
>+@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
> @del FDBs-1.com
> @echo Done.
> @copy %BUILD_DIR%\FV\EfiLdr %EFI_BOOT_DISK%
>@@ -70,7 +70,7 @@ goto Help
> @copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
> @%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
> @REM @del FDBS.com
>-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
>+@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
> @del FDBs-1.com
> @echo Done.
> @goto end
>@@ -89,8 +89,8 @@ goto Help
> @del FormatCommandInput.txt
> @echo Create boot sector ...
> @%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o UsbBs16.com
>-@copy %BOOTSECTOR_BIN_DIR%\Bs16.com Bs16-1.com
>-@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs16.com Bs16-1.com -f
>+@copy %BOOTSECTOR_BIN_DIR%\Bs16.com Bs16-1.com
>+@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs16.com Bs16-1.com -f
> @%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i Bs16-1.com
> @del Bs16-1.com
> @%BASETOOLS_DIR%\Genbootsector.exe -m -o %EFI_BOOT_DISK% -i 
> %BOOTSECTOR_BIN_DIR%\Mbr.com
>@@ -110,15 +110,15 @@ goto Help
> @del FormatCommandInput.txt
> @echo Create boot sector ...
> @%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o UsbBs32.com
>-@copy %BOOTSECTOR_BIN_DIR%\Bs32.com Bs32-1.com
>-@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs32.com Bs32-1.com -f
>+@copy %BOOTSECTOR_BIN_DIR%\Bs32.com Bs32-1.com
>+@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs32.com Bs32-1.com -f
> @del UsbBs32.com
> @%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i Bs32-1.com
> @del Bs32-1.com
> @%BASETOOLS_DIR%\Genbootsector.exe -m -o %EFI_BOOT_DISK% -i 
> %BOOTSECTOR_BIN_DIR%\Mbr.com
> @echo Done.
> @echo PLEASE UNPLUG USB, THEN PLUG IT AGAIN!
>-@goto end
>+@goto end
>
> :CreateUsb_FAT32_step2
> @copy %BUILD_DIR%\FV\EfiLdr20 %EFI_BOOT_DISK%
>diff --git a/DuetPkg/CreateBootDisk.sh b/DuetPkg/CreateBootDisk.sh
>index f2ff571..fa00408 100755
>--- a/DuetPkg/CreateBootDisk.sh
>+++ b/DuetPkg/CreateBootDisk.sh
>@@ -34,9 +34,9 @@ if [ \
>  "$*" = "--help" \
>]
> then
>-  echo "Usage: CreateBootDisk [usb|floppy|ide|file] MediaPath DevicePath 
>[FAT12|FAT16|FAT32] [IA32|X64]
>[GCC44|UNIXGCC]"
>-  echo "e.g. : CreateBootDisk floppy /media/floppy0 /dev/fd0 FAT12 IA32"
>-  PROCESS_MARK=FALSE
>+echo "Usage: CreateBootDisk [usb|floppy|ide|file] MediaPath DevicePath 
>[FAT12|FAT16|FAT32] [IA32|X64]
>[GCC44|UNIXGCC]"
>+echo "e.g. : CreateBootDisk floppy /media/floppy0 /dev/fd0 FAT12 IA32"
>+PROCESS_MARK=FALSE
> fi
>
> case "$5" in
>@@ -66,153 +66,153 @@ export EFI_BOOT_DEVICE=$3
>
> if [ "$PROCESS_MARK" = TRUE ]
> then
>-  case "$1" in
>-  floppy)
>-  if [ "$4" = FAT12 ]
>-  then
>-  echo Start to create floppy boot disk ...
>-  echo Format $EFI_BOOT_MEDIA ...
>-  ## Format floppy disk
>-  umount $EFI_BOOT_MEDIA
>-  mkfs.msdos $EFI_BOOT_DEVICE
>-  mount $EFI_BOOT_DEVICE $EFI_BOOT_MEDIA
>-  echo Create boot sector ...
>-  ## Linux version of GenBootSector has not pass 
>build yet.
>-  $BASETOOLS_DIR/GnuGenBootSector -i 
>$EFI_BOOT_DEVICE -o FDBs.com
>-  cp $BOOTSECTOR_BIN_DIR/bootsect.com FDBs-1.com
>-  $BASETOOLS_DIR/BootSectImage -g FDBs.com 
>FDBs-1.com -f
>-  $BASETOOLS_DIR/GnuGenBootSector -o 
>$EFI_BOOT_DEVICE -i FDBs-1.com
>-  rm FDBs-1.com
>-  cp $BUILD_DIR/FV/Efildr $EFI_BOOT_MEDIA
>-
>-  mkdir -p 

Re: [edk2] [PATCH 2/3] DuetPkg: Use 'echo off' in BATCH script files

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Regards,
Ray

>-Original Message-
>From: Wu, Hao A
>Sent: Friday, November 11, 2016 4:27 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A ; Ni, Ruiyu 
>Subject: [PATCH 2/3] DuetPkg: Use 'echo off' in BATCH script files
>
>Instead of putting a '@' at the beginning of every command in BATCH script
>files, use 'echo off' at the beginning of each file.
>
>Cc: Ruiyu Ni 
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Hao Wu 
>---
> DuetPkg/CreateBootDisk.bat | 197 +++--
> DuetPkg/GetVariables.bat   |  23 +++---
> DuetPkg/PostBuild.bat  |  69 
> 3 files changed, 146 insertions(+), 143 deletions(-)
>
>diff --git a/DuetPkg/CreateBootDisk.bat b/DuetPkg/CreateBootDisk.bat
>index 214b5b6..7265837 100644
>--- a/DuetPkg/CreateBootDisk.bat
>+++ b/DuetPkg/CreateBootDisk.bat
>@@ -1,3 +1,4 @@
>+@echo off
> @REM ## @file
> @REM #
> @REM #  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
>@@ -14,133 +15,133 @@
>
> @REM Set up environment at first.
>
>-@set BASETOOLS_DIR=%WORKSPACE_TOOLS_PATH%\Bin\Win32
>-@set BOOTSECTOR_BIN_DIR=%WORKSPACE%\DuetPkg\BootSector\bin
>-@set DISK_LABEL=DUET
>-@set PROCESSOR=""
>-@set STEP=1
>-@call %WORKSPACE%\DuetPkg\GetVariables.bat
>-
>-@echo on
>-
>-@if "%1"=="" goto Help
>-@if "%2"=="" goto Help
>-@if "%3"=="" goto Help
>-@if "%4"=="" goto Set_BootDisk
>-@if "%4"=="step2" (@set STEP=2) else @set TARGET_ARCH=%4
>-@if "%5"=="step2" @set STEP=2
>+set BASETOOLS_DIR=%WORKSPACE_TOOLS_PATH%\Bin\Win32
>+set BOOTSECTOR_BIN_DIR=%WORKSPACE%\DuetPkg\BootSector\bin
>+set DISK_LABEL=DUET
>+set PROCESSOR=""
>+set STEP=1
>+call %WORKSPACE%\DuetPkg\GetVariables.bat
>+
>+echo on
>+
>+if "%1"=="" goto Help
>+if "%2"=="" goto Help
>+if "%3"=="" goto Help
>+if "%4"=="" goto Set_BootDisk
>+if "%4"=="step2" (@set STEP=2) else @set TARGET_ARCH=%4
>+if "%5"=="step2" @set STEP=2
> :Set_BootDisk
>-@set EFI_BOOT_DISK=%2
>-@if "%TARGET_ARCH%"=="IA32" set PROCESSOR=IA32
>-@if "%TARGET_ARCH%"=="X64" set PROCESSOR=X64
>-@if %PROCESSOR%=="" goto WrongArch
>-@set BUILD_DIR=%WORKSPACE%\Build\DuetPkg%PROCESSOR%\%TARGET%_%TOOL_CHAIN_TAG%
>+set EFI_BOOT_DISK=%2
>+if "%TARGET_ARCH%"=="IA32" set PROCESSOR=IA32
>+if "%TARGET_ARCH%"=="X64" set PROCESSOR=X64
>+if %PROCESSOR%=="" goto WrongArch
>+set BUILD_DIR=%WORKSPACE%\Build\DuetPkg%PROCESSOR%\%TARGET%_%TOOL_CHAIN_TAG%
>
>-@if "%1"=="floppy" goto CreateFloppy
>-@if "%1"=="file" goto CreateFile
>-@if "%1"=="usb" goto CreateUsb
>-@if "%1"=="ide" goto CreateIde
>+if "%1"=="floppy" goto CreateFloppy
>+if "%1"=="file" goto CreateFile
>+if "%1"=="usb" goto CreateUsb
>+if "%1"=="ide" goto CreateIde
>
> goto Help
>
> :CreateFloppy
>-@if NOT "%3"=="FAT12" goto WrongFATType
>-@echo Start to create floppy boot disk ...
>-@echo Format %EFI_BOOT_DISK% ...
>-@echo.> FormatCommandInput.txt
>-@echo.n>> FormatCommandInput.txt
>-@format /v:%DISK_LABEL% /q %EFI_BOOT_DISK% < FormatCommandInput.txt > NUL
>-@del FormatCommandInput.txt
>-@echo Create boot sector ...
>-@%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o FDBs.com
>-@copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
>-@%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
>+if NOT "%3"=="FAT12" goto WrongFATType
>+echo Start to create floppy boot disk ...
>+echo Format %EFI_BOOT_DISK% ...
>+echo.> FormatCommandInput.txt
>+echo.n>> FormatCommandInput.txt
>+format /v:%DISK_LABEL% /q %EFI_BOOT_DISK% < FormatCommandInput.txt > NUL
>+del FormatCommandInput.txt
>+echo Create boot sector ...
>+%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o FDBs.com
>+copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
>+%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
> @REM @del FDBS.com
>-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
>-@del FDBs-1.com
>-@echo Done.
>-@copy %BUILD_DIR%\FV\EfiLdr %EFI_BOOT_DISK%
>-@goto CreateBootFile
>+%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
>+del FDBs-1.com
>+echo Done.
>+copy %BUILD_DIR%\FV\EfiLdr %EFI_BOOT_DISK%
>+goto CreateBootFile
>
> :CreateFile
>-@if NOT "%3"=="FAT12" goto WrongFATType
>-@echo Start to create file boot disk ...
>-@echo Create boot sector ...
>+if NOT "%3"=="FAT12" goto WrongFATType
>+echo Start to create file boot disk ...
>+echo Create boot sector ...
> %BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o FDBs.com
>-@copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
>-@%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
>-@REM @del FDBS.com
>-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
>-@del FDBs-1.com
>-@echo Done.
>-@goto end
>+copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
>+%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
>+REM @del FDBS.com
>+%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
>+del FDBs-1.com
>+echo Done.
>+goto 

[edk2] [PATCH 0/3] Enable POSTBUILD feature in DuetPkg DSC files

[Hao Wu]

This patch series uses the POSTBUILD feature in package DSC files to make
the post-build steps for DuetPkg more user-friendly.

Cc: Ruiyu Ni 

Hao Wu (3):
  DuetPkg: Resolve white-space issues for post-build scripts & ReadMe
  DuetPkg: Use 'echo off' in BATCH script files
  DuetPkg: Add POSTBUILD in DSC files to run post-build automatically

 DuetPkg/CreateBootDisk.bat | 199 ++---
 DuetPkg/CreateBootDisk.sh  | 312 ++---
 DuetPkg/DuetPkgIa32.dsc|   5 +
 DuetPkg/DuetPkgX64.dsc |   5 +
 DuetPkg/GetVariables.bat   |  38 --
 DuetPkg/PostBuild.bat  |  98 --
 DuetPkg/PostBuild.sh   | 112 
 DuetPkg/ReadMe.txt |  85 ++--
 DuetPkg/build32.sh |   4 +-
 DuetPkg/build64.sh |   4 +-
 10 files changed, 410 insertions(+), 452 deletions(-)
 delete mode 100644 DuetPkg/GetVariables.bat

-- 
1.9.5.msysgit.0

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

[edk2] [PATCH 1/3] DuetPkg: Resolve white-space issues for post-build scripts & ReadMe

[Hao Wu]

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 DuetPkg/CreateBootDisk.bat |  14 +--
 DuetPkg/CreateBootDisk.sh  | 304 ++---
 DuetPkg/PostBuild.bat  |   2 +-
 DuetPkg/PostBuild.sh   |  58 -
 DuetPkg/ReadMe.txt |  44 +++
 5 files changed, 211 insertions(+), 211 deletions(-)

diff --git a/DuetPkg/CreateBootDisk.bat b/DuetPkg/CreateBootDisk.bat
index 541de81..214b5b6 100644
--- a/DuetPkg/CreateBootDisk.bat
+++ b/DuetPkg/CreateBootDisk.bat
@@ -56,7 +56,7 @@ goto Help
 @copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
 @%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
 @REM @del FDBS.com
-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com 
+@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
 @del FDBs-1.com
 @echo Done.
 @copy %BUILD_DIR%\FV\EfiLdr %EFI_BOOT_DISK%
@@ -70,7 +70,7 @@ goto Help
 @copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
 @%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
 @REM @del FDBS.com
-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com 
+@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
 @del FDBs-1.com
 @echo Done.
 @goto end
@@ -89,8 +89,8 @@ goto Help
 @del FormatCommandInput.txt
 @echo Create boot sector ...
 @%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o UsbBs16.com
-@copy %BOOTSECTOR_BIN_DIR%\Bs16.com Bs16-1.com 
-@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs16.com Bs16-1.com -f 
+@copy %BOOTSECTOR_BIN_DIR%\Bs16.com Bs16-1.com
+@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs16.com Bs16-1.com -f
 @%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i Bs16-1.com
 @del Bs16-1.com
 @%BASETOOLS_DIR%\Genbootsector.exe -m -o %EFI_BOOT_DISK% -i 
%BOOTSECTOR_BIN_DIR%\Mbr.com
@@ -110,15 +110,15 @@ goto Help
 @del FormatCommandInput.txt
 @echo Create boot sector ...
 @%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o UsbBs32.com
-@copy %BOOTSECTOR_BIN_DIR%\Bs32.com Bs32-1.com 
-@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs32.com Bs32-1.com -f 
+@copy %BOOTSECTOR_BIN_DIR%\Bs32.com Bs32-1.com
+@%BASETOOLS_DIR%\Bootsectimage.exe -g UsbBs32.com Bs32-1.com -f
 @del UsbBs32.com
 @%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i Bs32-1.com
 @del Bs32-1.com
 @%BASETOOLS_DIR%\Genbootsector.exe -m -o %EFI_BOOT_DISK% -i 
%BOOTSECTOR_BIN_DIR%\Mbr.com
 @echo Done.
 @echo PLEASE UNPLUG USB, THEN PLUG IT AGAIN!
-@goto end  
+@goto end
 
 :CreateUsb_FAT32_step2
 @copy %BUILD_DIR%\FV\EfiLdr20 %EFI_BOOT_DISK%
diff --git a/DuetPkg/CreateBootDisk.sh b/DuetPkg/CreateBootDisk.sh
index f2ff571..fa00408 100755
--- a/DuetPkg/CreateBootDisk.sh
+++ b/DuetPkg/CreateBootDisk.sh
@@ -34,9 +34,9 @@ if [ \
  "$*" = "--help" \
]
 then
-   echo "Usage: CreateBootDisk [usb|floppy|ide|file] MediaPath DevicePath 
[FAT12|FAT16|FAT32] [IA32|X64] [GCC44|UNIXGCC]"
-   echo "e.g. : CreateBootDisk floppy /media/floppy0 /dev/fd0 FAT12 IA32"
-   PROCESS_MARK=FALSE
+echo "Usage: CreateBootDisk [usb|floppy|ide|file] MediaPath DevicePath 
[FAT12|FAT16|FAT32] [IA32|X64] [GCC44|UNIXGCC]"
+echo "e.g. : CreateBootDisk floppy /media/floppy0 /dev/fd0 FAT12 IA32"
+PROCESS_MARK=FALSE
 fi
 
 case "$5" in
@@ -66,153 +66,153 @@ export EFI_BOOT_DEVICE=$3
 
 if [ "$PROCESS_MARK" = TRUE ]
 then
-   case "$1" in
-   floppy)
-   if [ "$4" = FAT12 ]
-   then
-   echo Start to create floppy boot disk ...
-   echo Format $EFI_BOOT_MEDIA ...
-   ## Format floppy disk
-   umount $EFI_BOOT_MEDIA
-   mkfs.msdos $EFI_BOOT_DEVICE
-   mount $EFI_BOOT_DEVICE $EFI_BOOT_MEDIA
-   echo Create boot sector ...
-   ## Linux version of GenBootSector has not pass 
build yet.
-   $BASETOOLS_DIR/GnuGenBootSector -i 
$EFI_BOOT_DEVICE -o FDBs.com
-   cp $BOOTSECTOR_BIN_DIR/bootsect.com FDBs-1.com
-   $BASETOOLS_DIR/BootSectImage -g FDBs.com 
FDBs-1.com -f
-   $BASETOOLS_DIR/GnuGenBootSector -o 
$EFI_BOOT_DEVICE -i FDBs-1.com
-   rm FDBs-1.com
-   cp $BUILD_DIR/FV/Efildr $EFI_BOOT_MEDIA
-   
-   mkdir -p $EFI_BOOT_MEDIA/efi
-   mkdir -p $EFI_BOOT_MEDIA/efi/boot
-   if [ "$5" = IA32 ]
-   then
-   cp 
$WORKSPACE/ShellBinPkg/UefiShell/Ia32/Shell.efi 
$EFI_BOOT_MEDIA/efi/boot/boot$5.efi
-   else
-   if [ "$5" = X64 ]
-

[edk2] [PATCH 2/3] DuetPkg: Use 'echo off' in BATCH script files

[Hao Wu]

Instead of putting a '@' at the beginning of every command in BATCH script
files, use 'echo off' at the beginning of each file.

Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 DuetPkg/CreateBootDisk.bat | 197 +++--
 DuetPkg/GetVariables.bat   |  23 +++---
 DuetPkg/PostBuild.bat  |  69 
 3 files changed, 146 insertions(+), 143 deletions(-)

diff --git a/DuetPkg/CreateBootDisk.bat b/DuetPkg/CreateBootDisk.bat
index 214b5b6..7265837 100644
--- a/DuetPkg/CreateBootDisk.bat
+++ b/DuetPkg/CreateBootDisk.bat
@@ -1,3 +1,4 @@
+@echo off
 @REM ## @file
 @REM #
 @REM #  Copyright (c) 2010 - 2014, Intel Corporation. All rights reserved.
@@ -14,133 +15,133 @@
 
 @REM Set up environment at first.
 
-@set BASETOOLS_DIR=%WORKSPACE_TOOLS_PATH%\Bin\Win32
-@set BOOTSECTOR_BIN_DIR=%WORKSPACE%\DuetPkg\BootSector\bin
-@set DISK_LABEL=DUET
-@set PROCESSOR=""
-@set STEP=1
-@call %WORKSPACE%\DuetPkg\GetVariables.bat
-
-@echo on
-
-@if "%1"=="" goto Help
-@if "%2"=="" goto Help
-@if "%3"=="" goto Help
-@if "%4"=="" goto Set_BootDisk
-@if "%4"=="step2" (@set STEP=2) else @set TARGET_ARCH=%4
-@if "%5"=="step2" @set STEP=2
+set BASETOOLS_DIR=%WORKSPACE_TOOLS_PATH%\Bin\Win32
+set BOOTSECTOR_BIN_DIR=%WORKSPACE%\DuetPkg\BootSector\bin
+set DISK_LABEL=DUET
+set PROCESSOR=""
+set STEP=1
+call %WORKSPACE%\DuetPkg\GetVariables.bat
+
+echo on
+
+if "%1"=="" goto Help
+if "%2"=="" goto Help
+if "%3"=="" goto Help
+if "%4"=="" goto Set_BootDisk
+if "%4"=="step2" (@set STEP=2) else @set TARGET_ARCH=%4
+if "%5"=="step2" @set STEP=2
 :Set_BootDisk
-@set EFI_BOOT_DISK=%2
-@if "%TARGET_ARCH%"=="IA32" set PROCESSOR=IA32
-@if "%TARGET_ARCH%"=="X64" set PROCESSOR=X64
-@if %PROCESSOR%=="" goto WrongArch
-@set BUILD_DIR=%WORKSPACE%\Build\DuetPkg%PROCESSOR%\%TARGET%_%TOOL_CHAIN_TAG%
+set EFI_BOOT_DISK=%2
+if "%TARGET_ARCH%"=="IA32" set PROCESSOR=IA32
+if "%TARGET_ARCH%"=="X64" set PROCESSOR=X64
+if %PROCESSOR%=="" goto WrongArch
+set BUILD_DIR=%WORKSPACE%\Build\DuetPkg%PROCESSOR%\%TARGET%_%TOOL_CHAIN_TAG%
 
-@if "%1"=="floppy" goto CreateFloppy
-@if "%1"=="file" goto CreateFile
-@if "%1"=="usb" goto CreateUsb
-@if "%1"=="ide" goto CreateIde
+if "%1"=="floppy" goto CreateFloppy
+if "%1"=="file" goto CreateFile
+if "%1"=="usb" goto CreateUsb
+if "%1"=="ide" goto CreateIde
 
 goto Help
 
 :CreateFloppy
-@if NOT "%3"=="FAT12" goto WrongFATType
-@echo Start to create floppy boot disk ...
-@echo Format %EFI_BOOT_DISK% ...
-@echo.> FormatCommandInput.txt
-@echo.n>> FormatCommandInput.txt
-@format /v:%DISK_LABEL% /q %EFI_BOOT_DISK% < FormatCommandInput.txt > NUL
-@del FormatCommandInput.txt
-@echo Create boot sector ...
-@%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o FDBs.com
-@copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
-@%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
+if NOT "%3"=="FAT12" goto WrongFATType
+echo Start to create floppy boot disk ...
+echo Format %EFI_BOOT_DISK% ...
+echo.> FormatCommandInput.txt
+echo.n>> FormatCommandInput.txt
+format /v:%DISK_LABEL% /q %EFI_BOOT_DISK% < FormatCommandInput.txt > NUL
+del FormatCommandInput.txt
+echo Create boot sector ...
+%BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o FDBs.com
+copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
+%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
 @REM @del FDBS.com
-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
-@del FDBs-1.com
-@echo Done.
-@copy %BUILD_DIR%\FV\EfiLdr %EFI_BOOT_DISK%
-@goto CreateBootFile
+%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
+del FDBs-1.com
+echo Done.
+copy %BUILD_DIR%\FV\EfiLdr %EFI_BOOT_DISK%
+goto CreateBootFile
 
 :CreateFile
-@if NOT "%3"=="FAT12" goto WrongFATType
-@echo Start to create file boot disk ...
-@echo Create boot sector ...
+if NOT "%3"=="FAT12" goto WrongFATType
+echo Start to create file boot disk ...
+echo Create boot sector ...
 %BASETOOLS_DIR%\Genbootsector.exe -i %EFI_BOOT_DISK% -o FDBs.com
-@copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
-@%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
-@REM @del FDBS.com
-@%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
-@del FDBs-1.com
-@echo Done.
-@goto end
+copy %BOOTSECTOR_BIN_DIR%\Bootsect.com FDBs-1.com
+%BASETOOLS_DIR%\Bootsectimage.exe -g FDBs.com FDBs-1.com -f
+REM @del FDBS.com
+%BASETOOLS_DIR%\Genbootsector.exe -o %EFI_BOOT_DISK% -i FDBs-1.com
+del FDBs-1.com
+echo Done.
+goto end
 
 :CreateUsb
-@echo Start to create usb boot disk ...
-@if "%3"=="FAT16" goto CreateUsb_FAT16
-@if "%3"=="FAT32" goto CreateUsb_FAT32
-@if "%3"=="FAT12" goto WrongFATType
+echo Start to create usb boot disk ...
+if "%3"=="FAT16" goto CreateUsb_FAT16
+if "%3"=="FAT32" goto CreateUsb_FAT32
+if "%3"=="FAT12" goto WrongFATType
 
 :CreateUsb_FAT16
-@if "%STEP%"=="2" goto CreateUsb_FAT16_step2
-@echo Format %EFI_BOOT_DISK% ...
-@echo.>