Re: [edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching max perf log entries

2015-11-01 Thread Zeng, Star

On 2015/10/31 7:12, Samer El-Haj-Mahmoud wrote:

Add a DEBUG statement when the number of PEI perf log entries
exceeds PcdMaxPeiPerformanceLogEntries

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
  MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c | 2 ++
  1 file changed, 2 insertions(+)


Reviewed-by: Star Zeng 



diff --git a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c 
b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
index 0b5a717..fdc0ae0 100644
--- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
+++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
@@ -7,6 +7,7 @@
number of performance logging entry is specified by 
PcdMaxPeiPerformanceLogEntries.

  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
  This program and the accompanying materials
  are licensed and made available under the terms and conditions of the BSD 
License
  which accompanies this distribution.  The full text of the license may be 
found at
@@ -183,6 +184,7 @@ StartPerformanceMeasurementEx (
InternalGetPerformanceHobLog (&PeiPerformanceLog, &PeiPerformanceIdArray);

if (PeiPerformanceLog->NumberOfEntries >= PcdGet8 
(PcdMaxPeiPerformanceLogEntries)) {
+DEBUG ((DEBUG_ERROR, "PEI performance log arrray out of resources\n"));
  return RETURN_OUT_OF_RESOURCES;
}
Index   = PeiPerformanceLog->NumberOfEntries++;



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


Re: [edk2] SecurityPkg: PeiRsa2048Sha256GuidedSectionExtractLib error handling

2015-11-01 Thread Zhang, Chao B
Eugene:
May I know details about your problem?

1.   What kind of FV does the RSA2048Sha256 section exist in? BFV, Other FV 
must be processed in PEI (containing required Peim or DxeCore) or FV can be 
processed in DXE

2.   What is the behavior of your Authentication Failure Handler




Thanks & Best regards
Chao Zhang

From: Cohen, Eugene [mailto:eug...@hp.com]
Sent: Friday, October 30, 2015 7:45 PM
To: Zhang, Chao B
Cc: edk2-devel@lists.01.org
Subject: SecurityPkg: PeiRsa2048Sha256GuidedSectionExtractLib error handling

Dear SecurityPkg maintainer,

I'm trying to track down the best way for platform policy to handle an 
authentication failure in the PEI Rsa2048Sha256 guided section extraction 
library and ran across this curious state near the end of 
Rsa2048Sha256GuidedSectionHandler.

  //
  // Temp solution until PeiCore checks AUTH Status.
  //
  if ((*AuthenticationStatus & (EFI_AUTH_STATUS_TEST_FAILED | 
EFI_AUTH_STATUS_NOT_TESTED)) != 0) {
Status = EFI_ACCESS_DENIED;
  }

>From what I can tell the caller is checking AuthenticationStatus so this may 
>be some development code that was not removed.

Setting the return status to EFI_ACCESS_DENIED prevents us from getting to 
platform policy code (via the EFI_PEI_SECURITY2_PPI)

   Status = ParentFvPpi->FindSectionByType (
ParentFvPpi,
EFI_SECTION_FIRMWARE_VOLUME_IMAGE,
ParentFvFileHandle,
(VOID **)&FvHeader
);
  }
  if (EFI_ERROR (Status)) {
return Status;  <--- we bail out here
  }

  Status = VerifyPeim (PrivateData, ParentFvHandle, ParentFvFileHandle, 
AuthenticationStatus); <-- this would have called the EFI_PEI_SECURITY2_PPI
  if (Status == EFI_SECURITY_VIOLATION) {
return Status;
  }

Because of this issue we cannot run a policy handler that we'd like to because 
we remain in the PEI core going down error paths.

I think this fix may be as simple as removing the 'Temp solution' block but 
since I'm unfamiliar with the intent I wanted to check with you what the 
intended flow is for platform code discovering and handling authentication 
errors.

Thanks,

Eugene

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


Re: [edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching max perf log entries

2015-11-01 Thread Zeng, Star

On 2015/11/2 9:28, Zeng, Star wrote:

On 2015/10/31 7:12, Samer El-Haj-Mahmoud wrote:

Add a DEBUG statement when the number of PEI perf log entries
exceeds PcdMaxPeiPerformanceLogEntries

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
  MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c | 2 ++
  1 file changed, 2 insertions(+)


Reviewed-by: Star Zeng 


Oh, add one minor comment below.





diff --git
a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
index 0b5a717..fdc0ae0 100644
--- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
+++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
@@ -7,6 +7,7 @@
number of performance logging entry is specified by
PcdMaxPeiPerformanceLogEntries.

  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
  This program and the accompanying materials
  are licensed and made available under the terms and conditions of
the BSD License
  which accompanies this distribution.  The full text of the license
may be found at
@@ -183,6 +184,7 @@ StartPerformanceMeasurementEx (
InternalGetPerformanceHobLog (&PeiPerformanceLog,
&PeiPerformanceIdArray);

if (PeiPerformanceLog->NumberOfEntries >= PcdGet8
(PcdMaxPeiPerformanceLogEntries)) {
+DEBUG ((DEBUG_ERROR, "PEI performance log arrray out of
resources\n"));


The "arrray" should be "array", right?
If you are ok, I will help commit this patch with the correct "array".

Thanks,
Star


  return RETURN_OUT_OF_RESOURCES;
}
Index   = PeiPerformanceLog->NumberOfEntries++;



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


Re: [edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching max perf log entries

2015-11-01 Thread El-Haj-Mahmoud, Samer
Yes please fix the typo and commit it thank you for your help.




-Original Message-
From: Zeng, Star [star.z...@intel.com]
Received: Sunday, 01 Nov 2015, 7:57PM
To: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]; 
edk2-devel@lists.01.org [edk2-devel@lists.01.org]
CC: feng.t...@intel.com [feng.t...@intel.com]
Subject: Re: [edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching 
max perf log entries

On 2015/11/2 9:28, Zeng, Star wrote:
> On 2015/10/31 7:12, Samer El-Haj-Mahmoud wrote:
>> Add a DEBUG statement when the number of PEI perf log entries
>> exceeds PcdMaxPeiPerformanceLogEntries
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Samer El-Haj-Mahmoud 
>> ---
>>   MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c | 2 ++
>>   1 file changed, 2 insertions(+)
>
> Reviewed-by: Star Zeng 

Oh, add one minor comment below.

>
>>
>> diff --git
>> a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
>> b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
>> index 0b5a717..fdc0ae0 100644
>> --- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
>> +++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
>> @@ -7,6 +7,7 @@
>> number of performance logging entry is specified by
>> PcdMaxPeiPerformanceLogEntries.
>>
>>   Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
>> +(C) Copyright 2015 Hewlett Packard Enterprise Development LP
>>   This program and the accompanying materials
>>   are licensed and made available under the terms and conditions of
>> the BSD License
>>   which accompanies this distribution.  The full text of the license
>> may be found at
>> @@ -183,6 +184,7 @@ StartPerformanceMeasurementEx (
>> InternalGetPerformanceHobLog (&PeiPerformanceLog,
>> &PeiPerformanceIdArray);
>>
>> if (PeiPerformanceLog->NumberOfEntries >= PcdGet8
>> (PcdMaxPeiPerformanceLogEntries)) {
>> +DEBUG ((DEBUG_ERROR, "PEI performance log arrray out of
>> resources\n"));

The "arrray" should be "array", right?
If you are ok, I will help commit this patch with the correct "array".

Thanks,
Star

>>   return RETURN_OUT_OF_RESOURCES;
>> }
>> Index   = PeiPerformanceLog->NumberOfEntries++;
>>

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


Re: [edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching max perf log entries

2015-11-01 Thread Zeng, Star

On 2015/11/2 10:24, El-Haj-Mahmoud, Samer wrote:

Yes please fix the typo and commit it thank you for your help.



Committed at R18714.

Thanks,
Star





-Original Message-
From: Zeng, Star [star.z...@intel.com]
Received: Sunday, 01 Nov 2015, 7:57PM
To: El-Haj-Mahmoud, Samer [samer.el-haj-mahm...@hpe.com]; 
edk2-devel@lists.01.org [edk2-devel@lists.01.org]
CC: feng.t...@intel.com [feng.t...@intel.com]
Subject: Re: [edk2] [PATCH v2] MdeModulePkg: Add DEBUG statement when reaching 
max perf log entries

On 2015/11/2 9:28, Zeng, Star wrote:

On 2015/10/31 7:12, Samer El-Haj-Mahmoud wrote:

Add a DEBUG statement when the number of PEI perf log entries
exceeds PcdMaxPeiPerformanceLogEntries

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
   MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c | 2 ++
   1 file changed, 2 insertions(+)


Reviewed-by: Star Zeng 


Oh, add one minor comment below.





diff --git
a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
index 0b5a717..fdc0ae0 100644
--- a/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
+++ b/MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.c
@@ -7,6 +7,7 @@
 number of performance logging entry is specified by
PcdMaxPeiPerformanceLogEntries.

   Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+(C) Copyright 2015 Hewlett Packard Enterprise Development LP
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of
the BSD License
   which accompanies this distribution.  The full text of the license
may be found at
@@ -183,6 +184,7 @@ StartPerformanceMeasurementEx (
 InternalGetPerformanceHobLog (&PeiPerformanceLog,
&PeiPerformanceIdArray);

 if (PeiPerformanceLog->NumberOfEntries >= PcdGet8
(PcdMaxPeiPerformanceLogEntries)) {
+DEBUG ((DEBUG_ERROR, "PEI performance log arrray out of
resources\n"));


The "arrray" should be "array", right?
If you are ok, I will help commit this patch with the correct "array".

Thanks,
Star


   return RETURN_OUT_OF_RESOURCES;
 }
 Index   = PeiPerformanceLog->NumberOfEntries++;



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


Re: [edk2] [Patch 2/2] UefiCpuPkg/CpuDxe: Fix one dead lock issue in ProcessorToIdleState()

2015-11-01 Thread Fan, Jeff
Laszlo,

Your updating patch looks better. But when I tried it on my platform, Sometime, 
it hang at syncing MTRRs from BSP to Aps. I am investigating it and will 
feedback you later.

Thanks!
Jeff

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Wednesday, October 28, 2015 8:16 PM
To: Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Justen, Jordan L; Chen Fan
Subject: Re: [Patch 2/2] UefiCpuPkg/CpuDxe: Fix one dead lock issue in 
ProcessorToIdleState()

On 10/28/15 07:42, Jeff Fan wrote:
> There is one dead lock issue between BSP and AP. AP has one logic to 
> enter into Sleep state (hlt) if it found its state is Idle state in one loop.
> In ProcessorToIdleState() function, AP will get current AP state, if 
> its state is Sleep state, then it will go into real sleep(hlt) state.
> The dead lock may happen if BSP found AP state is Sleep state and send 
> INIT-SIPI -SIPI to AP. But AP just acquried the lock to get its state. 
> If AP is reset, the lock will not released correctly.
> This fix is to let AP enter into real sleep(hlt) state immediately 
> after it changes its state is set to Sleep and needn't get lock again.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jeff Fan 
> Cc: Michael Kinney 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Chen Fan 
> ---
>  UefiCpuPkg/CpuDxe/CpuMp.c | 12 
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c 
> index 3de2aa5..357ee42 100644
> --- a/UefiCpuPkg/CpuDxe/CpuMp.c
> +++ b/UefiCpuPkg/CpuDxe/CpuMp.c
> @@ -1289,14 +1289,18 @@ ProcessorToIdleState (
>GetMpSpinLock (CpuData);
>if (CpuData->State == CpuStateIdle) {
>CpuData->State = CpuStateSleeping;
> +ReleaseMpSpinLock (CpuData);
> +while (TRUE) {
> +  //
> +  // Place AP into cli-hlt loop to handle RSM from SMM case
> +  //
> +  DisableInterrupts ();
> +  CpuSleep ();
> +}
>}
>ReleaseMpSpinLock (CpuData);
>  }
>  
> -if (GetApState (CpuData) == CpuStateSleeping) {
> -  CpuSleep ();
> -}
> -
>  CpuPause ();
>}
>  
> 

I think the idea in this patch is good, but I recommend a *much* more sweeping 
change for this function. From my past multi-threaded programming experience, 
it is always best to hold the lock *by default* in a critical section, and only 
release it *temporarily* while yielding, or invoking a callback, or invoking a 
blocking operation.

- It makes no sense to release and reacquire a mutex or splinlock if our thread 
or CPU has actual, fast work to do, without wanting to yield intentionally.

- For loops especially, this means that the loop body is both *entered* and 
*exited* with the lock held.

- This approach tends to improve throughput and happens to eliminate a *bunch* 
of TOCTTOU races.

Therefore my proposal would be:

> diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c 
> index 98976a1..31c6222 100644
> --- a/UefiCpuPkg/CpuDxe/CpuMp.c
> +++ b/UefiCpuPkg/CpuDxe/CpuMp.c
> @@ -1288,40 +1288,42 @@ ProcessorToIdleState (
>  CpuData->Procedure = NULL;
>}
>CpuData->State = CpuStateIdle;
> -  ReleaseMpSpinLock (CpuData);
>  
>while (TRUE) {
> -GetMpSpinLock (CpuData);
>  ProcedureArgument = CpuData->Parameter;
>  Procedure = CpuData->Procedure;
> -ReleaseMpSpinLock (CpuData);
>  
>  if (Procedure != NULL) {
> -  SetApState (CpuData, CpuStateBusy);
> +  CpuData->State = CpuStateBusy;
> +  ReleaseMpSpinLock (CpuData);
>  
>Procedure ((VOID*) ProcedureArgument);
>  
>GetMpSpinLock (CpuData);
>CpuData->Procedure = NULL;
>CpuData->State = CpuStateFinished;
> -  ReleaseMpSpinLock (CpuData);
>  } else {
>//
>// if no procedure to execution, we simply put AP
>// into sleeping state, and waiting BSP sent SIPI.
>//
> -  GetMpSpinLock (CpuData);
>if (CpuData->State == CpuStateIdle) {
> -  CpuData->State = CpuStateSleeping;
> +CpuData->State = CpuStateSleeping;
> +ReleaseMpSpinLock (CpuData);
> +
> +//
> +// wait for INIT-SIPI-SIPI
> +//
> +while (TRUE) {
> +  DisableInterrupts ();
> +  CpuSleep ();
> +}
>}
> -  ReleaseMpSpinLock (CpuData);
> -}
> -
> -if (GetApState (CpuData) == CpuStateSleeping) {
> -  CpuSleep ();
>  }
>  
> +ReleaseMpSpinLock (CpuData);
>  CpuPause ();
> +GetMpSpinLock (CpuData);
>}
>  
>CpuSleep ();

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


Re: [edk2] [PATCH v2 00/16] unify GCC command line options

2015-11-01 Thread Ard Biesheuvel
On 8 October 2015 at 21:56, Bill Paul  wrote:
> Of all the gin joints in all the towns in all the world, Bill Paul had to walk
> into mine at 10:30:26 on Monday 24 August 2015 and say:
>
>> Of all the gin joints in all the towns in all the world, Ard Biesheuvel had
>> to
>>
>> walk into mine at 10:22:59 on Monday 24 August 2015 and say:
>> > On 24 August 2015 at 19:20, Bill Paul  wrote:
>> > > Of all the gin joints in all the towns in all the world, Ard Biesheuvel
>> > > had to
>> > >
>> > > walk into mine at 10:06:10 on Monday 24 August 2015 and say:
>> > >> On 24 August 2015 at 19:02, Bill Paul  wrote:
>> > >> > Of all the gin joints in all the towns in all the world, Ard
>> > >> > Biesheuvel had to
>> >
>> > >> > walk into mine at 09:54:08 on Monday 24 August 2015 and say:
>> > [...]
>> >
>> > >> >> Jordan suggested to drop UNIXGCC as well, and introduce MINGW
>> > >> >> instead iff we want the MinGW PE/COFF GCC, and I think we do, if
>> > >> >> only so that we have a LLP64 environment for X64 available to
>> > >> >> those without the possibility or the desire to run a MS toolchains
>> > >> >> under Windows.
>> > >> >
>> > >> > People should be able to build a known-good crossbuild toolchain.
>> > >> > This is the simplest way to provide that option.
>> > >>
>> > >> Meh. The primary audience of this feature are people building UEFI for
>> > >> X64 on X64, in which case the GCC4x options are arguably simpler. But
>> > >> apparently we agree that we should keep it /and/ support it.
>> > >>
>> > >> > By the way, do you think I can get you to update the
>> > >> > mingw-gcc-build.py script while you're at it? :)
>> > >>
>> > >> I proposed some updates here
>> > >> http://thread.gmane.org/gmane.comp.bios.edk2.devel/1297
>> > >> (with you on cc). Care to ack those?
>> > >
>> > > Is there a particular reason why you chose to use binutils from
>> > > www.kernel.org rather than from ftpmirror.gnu.org (other than "that's
>> > > what it was doing before")?
>> >
>> > Nope, that was it :-)
>> >
>> > In fact, I vaguely remember noticing the kernel.org URL and thinking
>> > "hmm that's odd" but for some reason, it did not provoke any action on
>> > my part
>>
>> My attention was drawn to it before because the specific version the script
>> was looking for previously ceased to exist on www.kernel.org, which broke
>> the script.
>>
>> > > In my testing I used binutins 2.25 from gnu.org, and it worked ok. I
>> > > thought it made more sense to get both packages from the same place.
>> > >
>> > > source_files_common = {
>> > >
>> > > 'binutils': {
>> > >
>> > > 'url': 'http://ftpmirror.gnu.org/binutils/' + \
>> > >
>> > >'binutils-$version.tar.bz2',
>> > >
>> > > 'version': '2.25',
>> > > 'md5': 'd9f3303f802a5b6b0bb73a335ab89d66',
>> > > },
>> > >
>> > > }
>> >
>> > Yes, 2.25 would be even better. In fact, it might make sense to wait
>> > for 2.26 to appear, since it adds support for --gc-sections (see the
>> > other part of this thread) which brings performance of mingw in line
>> > with ELF based GCC regarding code size.
>>
>> Fair enough, as long as we don't have to wait too long. In any case, aside
>> from this, the changes look ok to me.
>>
>> -Bill
>
> So... about that "as long as we don't have to wait too long" thing? I think
> it's been too long. :)
>

Well, we never decided the 'UNIXGCC shall remain at the same
GCC/binutils version into eternity' vs 'UNIXGCC shall reflect the
current stable combo of GCC/binutils' debate, and to be honest, I kind
of lost interest, since my primary focus is AARCH64/ARM and
apparently, nobody else cares about UNIXGCC anyway (except Dave
perhaps?)

Apparently, for some reason, Larry Hauch needs to give his blessing if
we decide to change anything at all in this regard, but his name does
not come up in the git log for over a year, and I don't think I ever
saw emails from him on the list. So it is really a matter of deciding
whether toolchains tags have immutable versions associated with them
or not.

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


Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

2015-11-01 Thread Wang, Sunny (HPS SW)
Hi Ray,
That's great. If we can directly use your current platform recovery 
implementation, that can save our effort to replace the short-term fix with the 
permanent fix in the future. 
Thanks for your help! :) 
In addition, could you help to take a look into the attached patch 
which adds the other BDS hook platform function 
PlatformBootManagerBeforeDefaultBoot() as well? For 
PlatformBootManagerBeforeDefaultBoot(), do you also prefer to add a 
REPORT_STATUS_CODE before doing DefaultBootBehavior() while-loop or some other 
way? Or adding PlatformBootManagerBeforeDefaultBoot() is acceptable for this 
case?

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Saturday, October 31, 2015 6:56 AM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
I don't like to add a library interface as a short term fix. I will provide a 
patch next week to enable platform recovery (We are still working on OS 
recovery) so that you don't need PlatformBootManagerDefaultBootFail() interface.

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 6:14 PM
To: Ni, Ruiyu 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Yes, I think OS/Platform recovery feature can meet our requirement.
However, OS/Platform recovery feature has not yet been completely implemented, 
so we would like to have a short-term solution for this.
Just like Samer said, if you are OK with introducing a new StatusCode in either 
BootAllBootOptions() or DefaultBootBehavior(), I can update patch for this. 
Of cause, If you think my current patch (adding hook function) could be used 
for short-term, it would be great.
By the way, I have the other similar need as the attached email.  If you have 
time, please help to review it. Thanks! 

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Friday, October 30, 2015 2:41 PM
To: Wang, Sunny (HPS SW)
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib
Importance: High

Sunny,
Can the OS/Platform recovery feature in UEFI spec can meet your requirement?

Thanks,
Ray

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang, 
Sunny (HPS SW)
Sent: Friday, October 30, 2015 11:46 AM
To: Ni, Ruiyu 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Hi Ray,
Thanks for the information.
However, the Status codes you mentioned in EfiBootManagerBoot() is for 
indicating booting a specific boot option failure rather than booting ALL boot 
options failure, so I can't use these Status codes for my purpose. The 
following two are detailed reasons:
-  I can't figure out how to know that all the boot options are booted in the 
Status code handler which is triggered by the REPORT_STATUS_CODE in 
EfiBootManagerBoot()
-  If there is no boot option in the boot order, this status code will not work 
either because BDS code doesn't call EfiBootManagerBoot().

Regards,
Sunny Wang

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com]
Sent: Thursday, October 29, 2015 1:07 PM
To: El-Haj-Mahmoud, Samer
Cc: Wang, Sunny (HPS SW); edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add a BDS platform hook function 
PlatformBootManagerDefaultBootFail to PlatformBootManagerLib

Samer,
EfiBootManagerBoot() function reports the failure to load boot option and the 
failure to start boot option through status code.

//
// Report Status Code to indicate that the failure to load boot option // 
REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_LOAD_ERROR)
  );
...

//
// Report Status Code to indicate that boot failure // REPORT_STATUS_CODE (
  EFI_ERROR_CODE | EFI_ERROR_MINOR,
  (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_EC_BOOT_OPTION_FAILED)
  );

Platform can hook the above two status code so that the 
PlatformBootManagerDefaultBootFail() can be avoided.

We are working on the Platform Recovery feature right now. But not sure whether 
it can be done by the end of this year.
Some spec clarification/discussion are on-going.

Thanks,
Ray

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com]
Sent: Wednesday, October 28, 2015 10:49 PM
To: Ni, Ruiyu 
Cc: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org; 
El-Haj-Mahmoud, Samer 
Subject: RE: [