Re: [edk2] [RFC] EDK2 Platforms Proposal

[Gao, Liming]

Mike:
  I have some comments.
1) If the platforms wants to base on edk2 master, it is not suggested to be 
placed into edk2-platform repo. Right?
2) On add a new platform to edk2-platforms, who approves the request? Edk2 
maintainer? Now, edk2 maintainer are edk2 package owner. So, for new platform, 
their owner will become edk2 maintainer.  
3) On update source in edk2-platform branch, the change in core package is 
allowed? For example, Platform feature may detect core issue, and this issue is 
not fixed in edk2 master. Could platform owners add their fix first or they 
need wait for the formal fix in edk2 master, then sync the fix to their branch? 
3) On remove an edk2-platforms branch, I think we need to collect the feedback 
from the platform owner.  

Thanks
Liming
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Kinney, 
Michael D
Sent: Wednesday, May 4, 2016 12:31 AM
To: edk2-devel@lists.01.org; Kinney, Michael D 
Subject: [edk2] [RFC] EDK2 Platforms Proposal

Hello,



Similar to edk2-staging, we also have a need to manage platforms that have been

ported to edk2.  Jordan has created a repository called edk2-platforms and

has created a branch for the minnowboard-max that uses a validated release

of the UDK 2015 for the dependent packages:



https://github.com/tianocore/edk2-platforms



https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015





Instead of creating a branch per feature in edk2-staging, the proposal is to

create a branch per platform in edk2-platforms.  The maintainer(s) that create

and support a platform branch can decide if the platform is synced to 
edk2/master

for dependent packages, or uses a stable release of the edk2 for dependent 
packages.



This proposal provides an area for platform development so we can minimize the

number of platforms that are included in edk2/master.  It is important to keep

some platforms in edk2/master so we can use those platforms to validate features

in non-platform packages in edk2/master.  If a new platform does not add feature

coverage to edk2/master, then a new edk2-platforms branch would be recommended.



Please review the proposal below for edk2-platforms.



If this proposal is accepted, then a review of the platform in edk2/master can

Be done to see if any of them should be moved to branches in edk2-platforms.







Problem statement

=

Need place on tianocore.org where platforms can be maintained by the EDK II

community.  This serves several purposes:



* Encourage more platforms sources to be shared earlier in the development 
process

* Allow platform sources to be shared that may not yet meet all edk2 required 
quality criteria

* Allow platform source to be shared so the EDK II community may choose to help 
finish and validate

* Allow more platforms to be used as part of the edk2 validation and release 
cycle.

* Not intended to be used for bug fixes.



Proposal



1) Create a new repo called edk2-platforms

 a) edk2-platforms is a fork of edk2

 b) edk2-platforms/master tracks edk2/master



2) edk2-platforms discussions can use the existing edk2-devel mailing list for 
design/patch/test.

 Use the following style for discussion of a platform branch in 
edk2-platforms repo.



 [platforms/branch]: Subject



3) All commits to edk2-platforms must follow same edk2 rules (e.g. Tiano 
Contributor's Agreement)



4) Process to add a new platform to edk2-platforms

 a) Maintainer sends patch email to edk2-devel mailing list announcing the 
creation of a new

platform branch in edk2-platforms with Readme.MD.  Readme.MD is in root 
of platform branch

with summary, owners, status, build instructions, target update 
instructions, OS compatibility,

known issues/limitations, links to related materials, and anything else 
a developer would need

to use the platform branch.

 b) Maintainer creates platform branch in edk2-platforms

 c) Maintainer is responsible syncing platform to edk2/master or supported 
edk2 branch.



5) Process to update sources in feature branch

 a) Commit message subject format:



  [platforms/branch PATCH]: Package/Module: Subject



 b) Directly commit changes to platform branch or if community review is 
desired,

then use edk2-devel review process.



7) Process to remove an edk2-platforms branch

 a) Stewards may periodically review of platform branches in edk2-platforms 
(once a quarter?)

 b) If no activity for extended period of time and platform is not being 
maintained and is no

longer functional then stewards send email to edk2-devel to request 
deletion of platform

branch.

 c) If no objections from EDK II community, then platform branch is deleted 
and archived at

https://github.com/tianocore/edk2-archive.



8) How to evaluate a platform in 

[edk2] [Patch 5/7] SecurityPkg OpalPasswordDxe: Check BlockSid capability before send command.

[Eric Dong]

Not all opal device support BlockSid feature. So Add
code logic to check the capability before send BlockSid
command.

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
---
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c | 25 ++-
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
index 4a4fa6a..7c6deb8 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
@@ -388,14 +388,11 @@ ReadyToBootCallback (
 {
   EFI_STATUS  Status;
   OPAL_DRIVER_DEVICE* Itr;
-  UINT8   Count;
   TCG_RESULT  Result;
   OPAL_EXTRA_INFO_VAR OpalExtraInfo;
   UINTN   DataSize;
   OPAL_SESSIONSession;
 
-  Count = 0;
-
   gBS->CloseEvent (Event);
 
   DataSize = sizeof (OPAL_EXTRA_INFO_VAR);
@@ -415,21 +412,21 @@ ReadyToBootCallback (
 // Send BlockSID command to each Opal disk
 //
 Itr = mOpalDriver.DeviceList;
-Count = 0;
 while (Itr != NULL) {
-  ZeroMem(, sizeof(Session));
-  Session.Sscp = Itr->OpalDisk.Sscp;
-  Session.MediaId = Itr->OpalDisk.MediaId;
-  Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
-
-  Result = OpalBlockSid (, TRUE);  // HardwareReset must always be 
TRUE
-  if (Result != TcgResultSuccess) {
-DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
-break;
+  if (Itr->OpalDisk.SupportedAttributes.BlockSid) {
+ZeroMem(, sizeof(Session));
+Session.Sscp = Itr->OpalDisk.Sscp;
+Session.MediaId = Itr->OpalDisk.MediaId;
+Session.OpalBaseComId = Itr->OpalDisk.OpalBaseComId;
+
+Result = OpalBlockSid (, TRUE);  // HardwareReset must always 
be TRUE
+if (Result != TcgResultSuccess) {
+  DEBUG ((DEBUG_ERROR, "OpalBlockSid fail\n"));
+  break;
+}
   }
 
   Itr = Itr->Next;
-  Count++;
 }
   }
 }
-- 
2.6.4.windows.1

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

[edk2] [Patch 2/7] MdePkg: Add TCG_BLOCK_SID_FEATURE_DESCRIPTOR definition.

[Eric Dong]

Cc: Feng Tian 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
---
 MdePkg/Include/IndustryStandard/TcgStorageCore.h | 12 
 MdePkg/Include/IndustryStandard/TcgStorageOpal.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/MdePkg/Include/IndustryStandard/TcgStorageCore.h 
b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
index 9549c00..26b39a5 100644
--- a/MdePkg/Include/IndustryStandard/TcgStorageCore.h
+++ b/MdePkg/Include/IndustryStandard/TcgStorageCore.h
@@ -228,6 +228,7 @@ typedef enum {
 #define TCG_FEATURE_OPAL_SSC_V2_0_0 (UINT16)0x0203
 #define TCG_FEATURE_OPAL_SSC_LITE   (UINT16)0x0301
 #define TCG_FEATURE_PYRITE_SSC  (UINT16)0x0302
+#define TCG_FEATURE_BLOCK_SID   (UINT16)0x0402
 
 // ACE Expression values
 #define TCG_ACE_EXPRESSION_AND 0x0
@@ -300,6 +301,17 @@ typedef struct {
 
 typedef struct {
   TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
+  UINT8SIDValueState : 1;
+  UINT8SIDBlockedState : 1;
+  UINT8Reserved4 : 6;
+  UINT8HardwareReset : 1;
+  UINT8Reserved5 : 7;
+  UINT8Reserved615[10];
+} TCG_BLOCK_SID_FEATURE_DESCRIPTOR;
+
+
+typedef struct {
+  TCG_LEVEL0_FEATURE_DESCRIPTOR_HEADER Header;
   UINT8SyncSupported : 1;
   UINT8AsyncSupported : 1;
   UINT8AckNakSupported : 1;
diff --git a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h 
b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
index 66027da..91d5008 100644
--- a/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
+++ b/MdePkg/Include/IndustryStandard/TcgStorageOpal.h
@@ -173,6 +173,7 @@ typedef union {
   OPAL_SSCV2_FEATURE_DESCRIPTOROpalSscV2;
   OPAL_SSCLITE_FEATURE_DESCRIPTOR  OpalSscLite;
   PYRITE_SSC_FEATURE_DESCRIPTORPyriteSsc;
+  TCG_BLOCK_SID_FEATURE_DESCRIPTOR BlockSid;
 } OPAL_LEVEL0_FEATURE_DESCRIPTOR;
 
 #pragma pack()
-- 
2.6.4.windows.1

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

[edk2] [patch] MdeModulePkg/Sd: add Erase Block support on sd/emmc device

[Feng Tian]

It's done by producing EFI_ERASE_BLOCK_PROTOCOL protocol instance.

Cc: Wu, Hao A 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 411 ++
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.h |  39 ++-
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.c |  74 +-
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.h |   5 +
 MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf   |   3 +-
 MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c | 384 
 MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.h |  39 ++-
 MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c |  15 ++
 MdeModulePkg/Bus/Sd/SdDxe/SdDxe.h |   7 +-
 MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf   |   3 +-
 10 files changed, 974 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c 
b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
index edb438b..a12cd63 100644
--- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
+++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
@@ -47,6 +47,8 @@ AsyncIoCallback (
 
   if (EFI_ERROR (Request->Packet.TransactionStatus)) {
 Request->Token->TransactionStatus = Request->Packet.TransactionStatus;
+  } else {
+Request->Token->TransactionStatus = EFI_SUCCESS;
   }
 
   RemoveEntryList (>Link);
@@ -1589,3 +1591,412 @@ EmmcSecurityProtocolOut (
   return Status;
 }
 
+/**
+  Set the erase start address through sync or async I/O request.
+
+  @param[in]  Partition A pointer to the EMMC_PARTITION instance.
+  @param[in]  StartLba  The starting logical block address to be 
erased.
+  @param[in]  Token A pointer to the token associated with the 
transaction.
+  @param[in]  IsEnd A boolean to show whether it's the last cmd in 
a series of cmds.
+This parameter is only meaningful in async I/O 
request.
+
+  @retval EFI_SUCCESS   The request is executed successfully.
+  @retval EFI_OUT_OF_RESOURCES  The request could not be executed due to a 
lack of resources.
+  @retval OthersThe request could not be executed successfully.
+
+**/
+EFI_STATUS
+EmmcEraseBlockStart (
+  IN  EMMC_PARTITION*Partition,
+  IN  EFI_LBA   StartLba,
+  IN  EFI_BLOCK_IO2_TOKEN   *Token,
+  IN  BOOLEAN   IsEnd
+  )
+{
+  EFI_STATUS   Status;
+  EFI_SD_MMC_PASS_THRU_PROTOCOL*PassThru;
+  EMMC_DEVICE  *Device;
+  EMMC_REQUEST *EraseBlockStart;
+  EFI_TPL  OldTpl;
+
+  EraseBlockStart = NULL;
+
+  Device   = Partition->Device;
+  PassThru = Device->Private->PassThru;
+
+  EraseBlockStart = AllocateZeroPool (sizeof (EMMC_REQUEST));
+  if (EraseBlockStart == NULL) {
+Status = EFI_OUT_OF_RESOURCES;
+goto Error;
+  }
+
+  EraseBlockStart->Signature = EMMC_REQUEST_SIGNATURE;
+  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
+  InsertTailList (>Queue, >Link);
+  gBS->RestoreTPL (OldTpl);
+  EraseBlockStart->Packet.SdMmcCmdBlk= >SdMmcCmdBlk;
+  EraseBlockStart->Packet.SdMmcStatusBlk = >SdMmcStatusBlk;
+  EraseBlockStart->Packet.Timeout= EMMC_GENERIC_TIMEOUT;
+
+  EraseBlockStart->SdMmcCmdBlk.CommandIndex = EMMC_ERASE_GROUP_START;
+  EraseBlockStart->SdMmcCmdBlk.CommandType  = SdMmcCommandTypeAc;
+  EraseBlockStart->SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1;
+
+  if (Device->SectorAddressing) {
+EraseBlockStart->SdMmcCmdBlk.CommandArgument = (UINT32)StartLba;
+  } else {
+EraseBlockStart->SdMmcCmdBlk.CommandArgument = (UINT32)MultU64x32 
(StartLba, Partition->BlockMedia.BlockSize);
+  }
+
+  EraseBlockStart->IsEnd = IsEnd;
+  EraseBlockStart->Token = Token;
+
+  if ((Token != NULL) && (Token->Event != NULL)) {
+Status = gBS->CreateEvent (
+EVT_NOTIFY_SIGNAL,
+TPL_CALLBACK,
+AsyncIoCallback,
+EraseBlockStart,
+>Event
+);
+if (EFI_ERROR (Status)) {
+  goto Error;
+}
+  } else {
+EraseBlockStart->Event = NULL;
+  }
+
+  Status = PassThru->PassThru (PassThru, Device->Slot, 
>Packet, EraseBlockStart->Event);
+
+Error:
+  if ((Token != NULL) && (Token->Event != NULL)) {
+//
+// For asynchronous operation, only free request and event in error case.
+// The request and event will be freed in asynchronous callback for 
success case.
+//
+if (EFI_ERROR (Status) && (EraseBlockStart != NULL)) {
+  RemoveEntryList (>Link);
+  if (EraseBlockStart->Event != NULL) {
+gBS->CloseEvent (EraseBlockStart->Event);
+  }
+  FreePool (EraseBlockStart);
+}
+  } else {
+//
+// For synchronous operation, free request whatever the execution result 
is.
+//
+if (EraseBlockStart != NULL) {
+  RemoveEntryList (>Link);
+  FreePool (EraseBlockStart);
+}
+  }
+
+  return 

Re: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Ni, Ruiyu]

2 comments below.

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>Egranov
>Sent: Wednesday, May 4, 2016 9:34 AM
>To: edk2-devel@lists.01.org
>Cc: Fan, Jeff 
>Subject: [edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot 
>timeout message
>
>The PlatformBdsShowProgress() supports graphics mode only, which is not
>applicable for RS-232 serial console. Show the progress message as a console
>text message in case PlatformBdsShowProgress() fails.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Daniil Egranov 
>---
> IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
>diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>index 6958979..d1a5c05 100644
>--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
>@@ -925,7 +925,7 @@ ShowProgress (
> // Show progress
> //
> if (TmpStr != NULL) {
>-  PlatformBdsShowProgress (
>+  Status = PlatformBdsShowProgress (
> Foreground,
> Background,
> TmpStr,
>@@ -933,12 +933,19 @@ ShowProgress (
> ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
> 0
> );
>+  if (EFI_ERROR(Status)) {
>+//if graphics mode is not supported (serial console) show text 
>progress message
>+AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
>", TimeoutRemain);
>+  }
1. Why use AsciiPrint but not Print(L"")?
I agree they are the same but normally we use Print().

> }
>   }
> }
>
> if (TmpStr != NULL) {
>   gBS->FreePool (TmpStr);
>+if (EFI_ERROR(Status)) {
>+  AsciiPrint ("\n");
>+}
2. What's the purpose of the EOL here?

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

Re: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS boot timeout

[Ni, Ruiyu]

3 comments below.

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>Egranov
>Sent: Wednesday, May 4, 2016 9:34 AM
>To: edk2-devel@lists.01.org
>Cc: Fan, Jeff 
>Subject: [edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS 
>boot timeout
>
>The patch loads timeout value from the "Timeout" global variable and passes
>it to PlatformBdsEnterFrontPage(), which handles delay and key input.
>The PcdPlatformBootTimeOut is only used at the BDS entry point and updates
>the "Timeout" value. This will allow the modification of the timeout value
>through the BDS menu and overwrite it if PcdPlatformBootTimeOut has been
>set.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Daniil Egranov 
>---
> IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c| 18 +++---
> .../Universal/BdsDxe/BootMaint/BootMaint.c | 17 -
> .../Universal/BdsDxe/BootMaint/UpdatePage.c| 13 +++--
> 3 files changed, 38 insertions(+), 10 deletions(-)
>
>diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>index ae7ad21..1d80fca 100644
>--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
>@@ -123,6 +123,7 @@ BdsBootDeviceSelect (
>   BOOLEAN   BootNextExist;
>   LIST_ENTRY*LinkBootNext;
>   EFI_EVENT ConnectConInEvent;
>+  UINTN Size;
>
>   //
>   // Got the latest boot option
>@@ -214,6 +215,18 @@ BdsBootDeviceSelect (
>   if (Link == NULL) {
> return ;
>   }
>+
>+  //Read boot timeout variable
>+  Status = gRT->GetVariable (L"Timeout",
>+ ,
>+ NULL,
>+ ,
>+ (VOID *) );
>+
>+  if (EFI_ERROR (Status)) {
>+Timeout = 0x;
>+  }
>+
>   //
>   // Here we make the boot in a loop, every boot success will
>   // return to the front page
>@@ -222,7 +235,7 @@ BdsBootDeviceSelect (
> //
> // Check the boot option list first
> //
>-if (Link == ) {
>+if (Link ==  || Timeout != 0x) {
>   //
>   // When LazyConIn enabled, signal connect ConIn event before enter UI
>   //
>@@ -238,12 +251,11 @@ BdsBootDeviceSelect (
>   //one is success to boot, then we back here to allow user
>   //add new active boot option
>   //
>-  Timeout = 0x;
>   PlatformBdsEnterFrontPage (Timeout, FALSE);
>+  Timeout = 0x;

1. The old code is to enter front page unconditionally and immediately while you
changed the behavior to wait for Timeout seconds before entering front page.
Why?


>   InitializeListHead ();
>   BdsLibBuildOptionFromVar (, L"BootOrder");
>   Link = BootLists.ForwardLink;
>-  continue;
> }
> //
> // Get the boot option from the link list
>diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>index d4b4475..cc2d656 100644
>--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
>@@ -111,6 +111,9 @@ InitializeBmmConfig (
>   BM_MENU_ENTRY   *NewMenuEntry;
>   BM_LOAD_CONTEXT *NewLoadContext;
>   UINT16  Index;
>+  UINT16  BootTimeOut;
>+  EFI_STATUS  Status;
>+  UINTN   Size;
>
>   ASSERT (CallbackData != NULL);
>
>@@ -128,7 +131,19 @@ InitializeBmmConfig (
> }
>   }
>
>-  CallbackData->BmmFakeNvData.BootTimeOut = PcdGet16 (PcdPlatformBootTimeOut);
>+  //Read boot timeout variable. If PcdPlatformBootTimeOut has been set,
>+  //the Timeout variable will be initialized as part of the BDS startup 
>procedure
>+  Status = gRT->GetVariable ( L"Timeout",
>+  ,
>+  NULL,
>+  ,
>+  (VOID *) );
>+
>+  if (EFI_ERROR (Status)) {
>+  BootTimeOut = 0;
>+  }
>+
>+  CallbackData->BmmFakeNvData.BootTimeOut = BootTimeOut;

2. With the platform DSC maps the PcdPlatformBootTimeout to L"Timeout" variable 
correctly,
the above change is not necessary. right?


>
>   //
>   // Initialize data which located in Boot Options Menu
>diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
>b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
>index b13ed11..c872766 100644
>--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
>+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
>@@ -722,18 +722,21 @@ UpdateTimeOutPage (
>   IN BMM_CALLBACK_DATA*CallbackData
>   )
> {
>-  UINT16  BootTimeOut;
>   VOID*DefaultOpCodeHandle;
>
>   CallbackData->BmmAskSaveOrNot = TRUE;
>
>   UpdatePageStart 

Re: [edk2] [PATCH v2 1/3] IntelFrameworkModulePkg/GenericBdsLib: Check for invalid device handle

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daniil 
>Egranov
>Sent: Wednesday, May 4, 2016 9:34 AM
>To: edk2-devel@lists.01.org
>Cc: Fan, Jeff 
>Subject: [edk2] [PATCH v2 1/3] IntelFrameworkModulePkg/GenericBdsLib: Check 
>for invalid device handle
>
>Added error control for invalid device handle when LocateDevicePath()
>fails.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Daniil Egranov 
>---
> IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 6 +-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
>b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
>index 68b32f3..330d6b5 100644
>--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
>+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
>@@ -3626,7 +3626,11 @@ BdsLibGetBootableHandle (
>   //
>   UpdatedDevicePath = DevicePath;
>   Status= gBS->LocateDevicePath (, 
> , );
>-  gBS->ConnectController (Handle, NULL, NULL, TRUE);
>+  if (!EFI_ERROR (Status)) {
>+gBS->ConnectController (Handle, NULL, NULL, TRUE);
>+  } else {
>+return (EFI_HANDLE) NULL;
>+  }
> }
>   } else {
> //
>--
>2.7.4
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.

[Ni, Ruiyu]

I encountered the same issue weeks ago.
A quick but dirty fix is to change:
UefiCpuPkg/CpuS3DataDxe/CpuS3Data.c: CpuS3DataInitialize()
  //
  // Allocate a 4KB reserved page below 1MB
  //
-  AcpiCpuData->StartupVector = BASE_1MB - 1;
+  ApiCpuData->StartupVector = BASE_512KB - 1;


Copying Jeff.

Regards,
Ray

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
>Ersek
>Sent: Monday, April 25, 2016 7:22 PM
>To: wq...@aliyun.com
>Cc: edk2-devel ; Kevin O'Connor ; 
>David Woodhouse
>
>Subject: Re: [edk2] Csm16.bin(seabios) failed to work in OVMF.
>
>On 04/23/16 08:39, wq...@aliyun.com wrote:
>> Hi everyone,
>> I build the Csm16.bin from source code of seabios-1.9.2.tar.gz . And 
>> copy the Csm16.bin to
>OvmfPkg/Csm/Csm16/Csm16.bin , Build the OVMF_CODE.fd and OVMF_VARS.fd with 
>command build -D CSM_ENABLE.
>>Unfortunately, I test the OVMF_CODE.fd and OVMF_VARS.fd in qemu. It 
>> failed!
>> The log is:
>> ASSERT_EFI_ERROR (Status = Not Found)
>> ASSERT 
>> /root/tianocore-edk2/IntelFrameworkModulePkg/Csm/LegacyBiosDxe/LegacyBios.c(982):
>>  !EFI_ERROR
>(Status)
>> Any information you can provide me would be greatly appreciated.
>
>Sorry, no idea. The assert is about a memory allocation failure (you can
>check the source code at the location captured in the assert):
>
>  Status = AllocateLegacyMemory (
> AllocateAddress,
> CONVENTIONAL_MEMORY_TOP - MemorySize,
> EFI_SIZE_TO_PAGES (MemorySize),
> 
> );
>  ASSERT_EFI_ERROR (Status);
>
>I don't know why it fails.
>
>Perhaps you can experiment with older SeaBIOS releases and narrow it
>down a little. (You could also experiment with OVMF builds at different
>git commits, but I don't readily recall anything that could cause this.)
>
>I'm adding David, Kevin and Gerd to the CC list.
>
>Thanks
>Laszlo
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] SecurityPkg: SecureBootConfigDxe: Disable SecureBoot Enable/Disable in some case

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 



> -Original Message-
> From: Zhang, Chao B
> Sent: Wednesday, May 4, 2016 10:35 AM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Zhang, Chao B
> 
> Subject: [PATCH] SecurityPkg: SecureBootConfigDxe: Disable SecureBoot
> Enable/Disable in some case
> 
> Disable SecureBoot Enable/Disable feature when PhysicalPresence is not
> available,
> Since SecureBootEnable is protected with PhysicalPresence.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  .../VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr  | 2
> +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
> g.vfr
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> ig.vfr
> index fefbfbf..02ddf4a 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfi
> g.vfr
> +++
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConf
> ig.vfr
> @@ -41,7 +41,7 @@ formset
>  //
>  // Display of Check Box: Attempt Secure Boot
>  //
> -grayoutif ideqval SECUREBOOT_CONFIGURATION.HideSecureBoot == 1;
> +grayoutif ideqval SECUREBOOT_CONFIGURATION.HideSecureBoot == 1 OR
> NOT ideqval SECUREBOOT_CONFIGURATION.PhysicalPresent == 1;
>  checkbox varid = SECUREBOOT_CONFIGURATION.AttemptSecureBoot,
>questionid = KEY_SECURE_BOOT_ENABLE,
>prompt = STRING_TOKEN(STR_SECURE_BOOT_PROMPT),
> --
> 1.9.5.msysgit.1

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

[edk2] [PATCH] SecurityPkg: SecureBootConfigDxe: Disable SecureBoot Enable/Disable in some case

[Zhang, Chao B]

Disable SecureBoot Enable/Disable feature when PhysicalPresence is not 
available,
Since SecureBootEnable is protected with PhysicalPresence.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang 
---
 .../VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
index fefbfbf..02ddf4a 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr
@@ -41,7 +41,7 @@ formset
 //
 // Display of Check Box: Attempt Secure Boot
 //
-grayoutif ideqval SECUREBOOT_CONFIGURATION.HideSecureBoot == 1;
+grayoutif ideqval SECUREBOOT_CONFIGURATION.HideSecureBoot == 1 OR NOT 
ideqval SECUREBOOT_CONFIGURATION.PhysicalPresent == 1;
 checkbox varid = SECUREBOOT_CONFIGURATION.AttemptSecureBoot,
   questionid = KEY_SECURE_BOOT_ENABLE,
   prompt = STRING_TOKEN(STR_SECURE_BOOT_PROMPT),
-- 
1.9.5.msysgit.1

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

[edk2] [Patch] NetworkPkg: Use UefiBootManagerLib API to create load option.

[Fu Siyuan]

This patch updates the HTTP boot driver to use the API in UefiBootManagerLib to
create new load option, to avoid duplicate code.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Cc: Ni Ruiyu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/HttpBootDxe/HttpBootConfig.c | 138 ++--
 NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |   1 +
 NetworkPkg/NetworkPkg.dsc   |   8 ++
 3 files changed, 34 insertions(+), 113 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c 
b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 00e4782..b2d6628 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 **/
 
 #include "HttpBootDxe.h"
+#include 
 
 CHAR16  mHttpBootConfigStorageName[] = L"HTTP_BOOT_CONFIG_IFR_NVDATA";
 
@@ -36,31 +37,19 @@ HttpBootAddBootOption (
   IN   CHAR16   *Uri
   )
 {
-  EFI_DEV_PATH   *Node;
-  EFI_DEVICE_PATH_PROTOCOL   *TmpDevicePath;
-  EFI_DEVICE_PATH_PROTOCOL   *NewDevicePath;
-  UINTN  Length;
-  CHAR8  AsciiUri[URI_STR_MAX_SIZE];
-  CHAR16 *CurrentOrder;
-  EFI_STATUS Status;
-  UINTN  OrderCount;
-  UINTN  TargetLocation;
-  BOOLEANFound;
-  UINT8  *TempByteBuffer;
-  UINT8  *TempByteStart;
-  UINTN  DescSize;
-  UINTN  FilePathSize;
-  CHAR16 OptionStr[10];
-  UINT16 *NewOrder;
-  UINTN  Index;
-
-  NewOrder  = NULL;
-  TempByteStart = NULL;
+  EFI_DEV_PATH  *Node;
+  EFI_DEVICE_PATH_PROTOCOL  *TmpDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL  *NewDevicePath;
+  UINTN Length;
+  CHAR8 AsciiUri[URI_STR_MAX_SIZE];
+  EFI_STATUSStatus;
+  UINTN Index;
+  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
+
   NewDevicePath = NULL;
-  NewOrder  = NULL;
   Node  = NULL;
   TmpDevicePath = NULL;
-  CurrentOrder  = NULL;
+  ZeroMem (, sizeof (EFI_BOOT_MANAGER_LOAD_OPTION));
 
   if (StrLen (Description) == 0) {
 return EFI_INVALID_PARAMETER;
@@ -137,108 +126,31 @@ HttpBootAddBootOption (
   }
 
   //
-  // Get current "BootOrder" variable and find a free target.
+  // Add a new load option.
   //
-  Length = 0;
-  Status = GetVariable2 (
- L"BootOrder",
- ,
- (VOID **),
-  
- );
-  if (EFI_ERROR (Status) && Status != EFI_NOT_FOUND) {
-goto ON_EXIT;
-  }
-  OrderCount = Length / sizeof (UINT16);
-  Found = FALSE;
-  for (TargetLocation=0; TargetLocation < 0x; TargetLocation++) {
-Found = TRUE;
-for (Index = 0; Index < OrderCount; Index++) {
-  if (CurrentOrder[Index] == TargetLocation) {
-Found = FALSE;
-break;
-  }
-}
-if (Found) {
-  break;
-}
-  }
-
-  if (TargetLocation == 0x) {
-DEBUG ((EFI_D_ERROR, "Could not find unused target index.\n"));
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  } else {
-DEBUG ((EFI_D_INFO, "TargetIndex = %04x.\n", TargetLocation));
-  }
-  
-  //
-  // Construct and set the "Boot" variable
-  //
-  DescSize = StrSize(Description);
-  FilePathSize = GetDevicePathSize (NewDevicePath);
-  TempByteBuffer = AllocateZeroPool(sizeof(EFI_LOAD_OPTION) + DescSize + 
FilePathSize);
-  if (TempByteBuffer == NULL) {
-Status = EFI_OUT_OF_RESOURCES;
-goto ON_EXIT;
-  }
-
-  TempByteStart = TempByteBuffer;
-  *((UINT32 *) TempByteBuffer) = LOAD_OPTION_ACTIVE;  // Attributes
-  TempByteBuffer += sizeof (UINT32);
-
-  *((UINT16 *) TempByteBuffer) = (UINT16)FilePathSize;// FilePathListLength
-  TempByteBuffer += sizeof (UINT16);
-
-  CopyMem (TempByteBuffer, Description, DescSize);
-  TempByteBuffer += DescSize;
-  CopyMem (TempByteBuffer, NewDevicePath, FilePathSize);
-
-  UnicodeSPrint (OptionStr, sizeof(OptionStr), L"%s%04x", L"Boot", 
TargetLocation);
-  Status = gRT->SetVariable (
-  OptionStr,
-  ,
-  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS 
| EFI_VARIABLE_RUNTIME_ACCESS,
-  sizeof(UINT32) + sizeof(UINT16) + DescSize + FilePathSize,
-  TempByteStart
-  );
+  Status = EfiBootManagerInitializeLoadOption (
+ ,
+ LoadOptionNumberUnassigned,
+ LoadOptionTypeBoot,
+ LOAD_OPTION_ACTIVE,
+ Description,
+ NewDevicePath,
+ NULL,
+ 0
+ 

[edk2] [PATCH v2 0/3] IntelFrameworkModulePkg: Fixes for Intel BDS

[Daniil Egranov]

This set of patches fixes the following:
Crash when BDS loads a device handle based onthe device path but 
the device driver has not been loaded. 
BDS boot timeout.
Timeout progress message for RS-232 serial console. 

Changes since v1:
Fixed patch 2/3 compilation issue. 

Thanks,
Daniil

Daniil Egranov (3):
  IntelFrameworkModulePkg/GenericBdsLib: Check for invalid device handle
  IntelFrameworkModulePkg/BdsDxe: Fix for the BDS boot timeout
  IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

 .../Library/GenericBdsLib/BdsBoot.c|  6 +-
 IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c| 18 +++---
 .../Universal/BdsDxe/BootMaint/BootMaint.c | 17 -
 .../Universal/BdsDxe/BootMaint/UpdatePage.c| 13 +++--
 IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c   |  9 -
 5 files changed, 51 insertions(+), 12 deletions(-)

-- 
2.7.4

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

[edk2] [PATCH v2 1/3] IntelFrameworkModulePkg/GenericBdsLib: Check for invalid device handle

[Daniil Egranov]

Added error control for invalid device handle when LocateDevicePath() 
fails. 

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c 
b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
index 68b32f3..330d6b5 100644
--- a/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
+++ b/IntelFrameworkModulePkg/Library/GenericBdsLib/BdsBoot.c
@@ -3626,7 +3626,11 @@ BdsLibGetBootableHandle (
   //
   UpdatedDevicePath = DevicePath;
   Status= gBS->LocateDevicePath (, 
, );
-  gBS->ConnectController (Handle, NULL, NULL, TRUE);
+  if (!EFI_ERROR (Status)) {
+gBS->ConnectController (Handle, NULL, NULL, TRUE);
+  } else {
+return (EFI_HANDLE) NULL;
+  }
 }
   } else {
 //
-- 
2.7.4

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

[edk2] [PATCH v2 2/3] IntelFrameworkModulePkg/BdsDxe: Fix for the BDS boot timeout

[Daniil Egranov]

The patch loads timeout value from the "Timeout" global variable and passes
it to PlatformBdsEnterFrontPage(), which handles delay and key input.
The PcdPlatformBootTimeOut is only used at the BDS entry point and updates
the "Timeout" value. This will allow the modification of the timeout value 
through the BDS menu and overwrite it if PcdPlatformBootTimeOut has been 
set.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c| 18 +++---
 .../Universal/BdsDxe/BootMaint/BootMaint.c | 17 -
 .../Universal/BdsDxe/BootMaint/UpdatePage.c| 13 +++--
 3 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
index ae7ad21..1d80fca 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -123,6 +123,7 @@ BdsBootDeviceSelect (
   BOOLEAN   BootNextExist;
   LIST_ENTRY*LinkBootNext;
   EFI_EVENT ConnectConInEvent;
+  UINTN Size;
 
   //
   // Got the latest boot option
@@ -214,6 +215,18 @@ BdsBootDeviceSelect (
   if (Link == NULL) {
 return ;
   }
+
+  //Read boot timeout variable
+  Status = gRT->GetVariable (L"Timeout",
+ ,
+ NULL,
+ ,
+ (VOID *) );
+
+  if (EFI_ERROR (Status)) {
+Timeout = 0x;
+  }
+
   //
   // Here we make the boot in a loop, every boot success will
   // return to the front page
@@ -222,7 +235,7 @@ BdsBootDeviceSelect (
 //
 // Check the boot option list first
 //
-if (Link == ) {
+if (Link ==  || Timeout != 0x) {
   //
   // When LazyConIn enabled, signal connect ConIn event before enter UI
   //
@@ -238,12 +251,11 @@ BdsBootDeviceSelect (
   //one is success to boot, then we back here to allow user
   //add new active boot option
   //
-  Timeout = 0x;
   PlatformBdsEnterFrontPage (Timeout, FALSE);
+  Timeout = 0x;
   InitializeListHead ();
   BdsLibBuildOptionFromVar (, L"BootOrder");
   Link = BootLists.ForwardLink;
-  continue;
 }
 //
 // Get the boot option from the link list
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
index d4b4475..cc2d656 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootMaint.c
@@ -111,6 +111,9 @@ InitializeBmmConfig (
   BM_MENU_ENTRY   *NewMenuEntry;
   BM_LOAD_CONTEXT *NewLoadContext;
   UINT16  Index;
+  UINT16  BootTimeOut;
+  EFI_STATUS  Status;
+  UINTN   Size;
 
   ASSERT (CallbackData != NULL);
 
@@ -128,7 +131,19 @@ InitializeBmmConfig (
 }
   }
 
-  CallbackData->BmmFakeNvData.BootTimeOut = PcdGet16 (PcdPlatformBootTimeOut);
+  //Read boot timeout variable. If PcdPlatformBootTimeOut has been set,
+  //the Timeout variable will be initialized as part of the BDS startup 
procedure
+  Status = gRT->GetVariable ( L"Timeout",
+  ,
+  NULL,
+  ,
+  (VOID *) );
+
+  if (EFI_ERROR (Status)) {
+  BootTimeOut = 0;
+  }
+
+  CallbackData->BmmFakeNvData.BootTimeOut = BootTimeOut;
 
   //
   // Initialize data which located in Boot Options Menu
diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
index b13ed11..c872766 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
@@ -722,18 +722,21 @@ UpdateTimeOutPage (
   IN BMM_CALLBACK_DATA*CallbackData
   )
 {
-  UINT16  BootTimeOut;
   VOID*DefaultOpCodeHandle;
 
   CallbackData->BmmAskSaveOrNot = TRUE;
 
   UpdatePageStart (CallbackData);
 
-  BootTimeOut = PcdGet16 (PcdPlatformBootTimeOut);
-
   DefaultOpCodeHandle = HiiAllocateOpCodeHandle ();
   ASSERT (DefaultOpCodeHandle != NULL);
-  HiiCreateDefaultOpCode (DefaultOpCodeHandle, EFI_HII_DEFAULT_CLASS_STANDARD, 
EFI_IFR_TYPE_NUM_SIZE_16, BootTimeOut);
+
+  HiiCreateDefaultOpCode (
+DefaultOpCodeHandle,
+EFI_HII_DEFAULT_CLASS_STANDARD,
+EFI_IFR_TYPE_NUM_SIZE_16,
+CallbackData->BmmFakeNvData.BootTimeOut
+);
 
   HiiCreateNumericOpCode (
 mStartOpCodeHandle,
@@ -752,8 +755,6 @@ UpdateTimeOutPage (
   
   HiiFreeOpCodeHandle (DefaultOpCodeHandle);
 
-  //CallbackData->BmmFakeNvData.BootTimeOut = BootTimeOut;
-
   UpdatePageEnd (CallbackData);
 }
 
-- 
2.7.4

[edk2] [PATCH v2 3/3] IntelFrameworkModulePkg/BdsDxe: Show boot timeout message

[Daniil Egranov]

The PlatformBdsShowProgress() supports graphics mode only, which is not
applicable for RS-232 serial console. Show the progress message as a console
text message in case PlatformBdsShowProgress() fails.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov 
---
 IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
index 6958979..d1a5c05 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/FrontPage.c
@@ -925,7 +925,7 @@ ShowProgress (
 // Show progress
 //
 if (TmpStr != NULL) {
-  PlatformBdsShowProgress (
+  Status = PlatformBdsShowProgress (
 Foreground,
 Background,
 TmpStr,
@@ -933,12 +933,19 @@ ShowProgress (
 ((TimeoutDefault - TimeoutRemain) * 100 / TimeoutDefault),
 0
 );
+  if (EFI_ERROR(Status)) {
+//if graphics mode is not supported (serial console) show text 
progress message
+AsciiPrint ("\rPress any key to enter Boot Menu in %d seconds 
", TimeoutRemain);
+  }
 }
   }
 }
 
 if (TmpStr != NULL) {
   gBS->FreePool (TmpStr);
+if (EFI_ERROR(Status)) {
+  AsciiPrint ("\n");
+}
 }
 
 //
-- 
2.7.4

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

[edk2] [Patch] MdeModulePkg: Refine SNP driver's media status check logic.

[Fu Siyuan]

Some UNDI drivers may not support the cable detect in UNDI INITIALIZE command,
but support the media present check in UNDI GET_STATUS command. Current SNP
driver will set the MediaPresentSupported field to FALSE in 
EFI_SIMPLE_NETWORK_MODE
for such case, which forbid the media detect from the callers.

This patch updates the SNP driver to support such kind of UNDIs for media 
detect.
MediaPresentSupported will be set to TRUE, and a GET_STATUS command will be 
issued
immediately after INITIALIZE command in SNP->Initialize() function, to refresh
the value of MediaPresent in SNP mode data.

Cc: Ye Ting 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/SnpDxe/Get_status.c |  3 ++-
 MdeModulePkg/Universal/Network/SnpDxe/Initialize.c | 13 +-
 MdeModulePkg/Universal/Network/SnpDxe/Snp.c|  8 --
 MdeModulePkg/Universal/Network/SnpDxe/Snp.h| 30 ++
 4 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
index edbc0f2..3c6bf39 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Get_status.c
@@ -18,7 +18,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 /**
   Call undi to get the status of the interrupts, get the list of recycled 
transmit
   buffers that completed transmitting. The recycled transmit buffer address 
will
-  be saved into Snp->RecycledTxBuf.
+  be saved into Snp->RecycledTxBuf. This function will also update the 
MediaPresent
+  field of EFI_SIMPLE_NETWORK_MODE if UNDI support it.
 
   @param  Snp Pointer to snp driver structure.
   @param  InterruptStatusPtr  A non null pointer to contain the interrupt
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
index 3f40ef3..2151375 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Initialize.c
@@ -229,7 +229,10 @@ SnpUndi32Initialize (
   //
   Snp->TxRxBufferSize = (UINT32) (Snp->InitInfo.MemoryRequired + 
ExtraRxBufferSize + ExtraTxBufferSize);
 
-  if (Snp->Mode.MediaPresentSupported) {
+  //
+  // If UNDI support cable detect for INITIALIZE command, try it first.
+  //
+  if (Snp->CableDetectSupported) {
 if (PxeInit (Snp, PXE_OPFLAGS_INITIALIZE_DETECT_CABLE) == EFI_SUCCESS) {
   Snp->Mode.MediaPresent = TRUE;
   goto ON_EXIT;
@@ -242,6 +245,14 @@ SnpUndi32Initialize (
 
   if (EFI_ERROR (EfiStatus)) {
 gBS->CloseEvent (Snp->Snp.WaitForPacket);
+goto ON_EXIT;
+  }
+
+  //
+  // Try to update the MediaPresent field of EFI_SIMPLE_NETWORK_MODE if the 
UNDI support it.
+  //
+  if (Snp->MediaStatusSupported) {
+PxeGetStatus (Snp, NULL, FALSE);
   }
 
 ON_EXIT:
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c 
b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
index 5ff294f..9f61aee 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.c
@@ -554,12 +554,12 @@ SimpleNetworkDriverStart (
 
   switch (InitStatFlags & PXE_STATFLAGS_CABLE_DETECT_MASK) {
   case PXE_STATFLAGS_CABLE_DETECT_SUPPORTED:
-Snp->Mode.MediaPresentSupported = TRUE;
+Snp->CableDetectSupported = TRUE;
 break;
 
   case PXE_STATFLAGS_CABLE_DETECT_NOT_SUPPORTED:
   default:
-Snp->Mode.MediaPresentSupported = FALSE;
+Snp->CableDetectSupported = FALSE;
   }
 
   switch (InitStatFlags & PXE_STATFLAGS_GET_STATUS_NO_MEDIA_MASK) {
@@ -572,6 +572,10 @@ SimpleNetworkDriverStart (
 Snp->MediaStatusSupported = FALSE;
   }
 
+  if (Snp->CableDetectSupported || Snp->MediaStatusSupported) {
+Snp->Mode.MediaPresentSupported = TRUE;
+  }
+
   if ((Pxe->hw.Implementation & PXE_ROMID_IMP_STATION_ADDR_SETTABLE) != 0) {
 Snp->Mode.MacAddressChangeable = TRUE;
   } else {
diff --git a/MdeModulePkg/Universal/Network/SnpDxe/Snp.h 
b/MdeModulePkg/Universal/Network/SnpDxe/Snp.h
index 67f65ff..f802dfb 100644
--- a/MdeModulePkg/Universal/Network/SnpDxe/Snp.h
+++ b/MdeModulePkg/Universal/Network/SnpDxe/Snp.h
@@ -135,6 +135,12 @@ typedef struct {
   BOOLEANMediaStatusSupported;
 
   //
+  // Whether UNDI support cable detect for INITIALIZE command,
+  // i.e. PXE_STATFLAGS_CABLE_DETECT_NOT_SUPPORTED
+  //
+  BOOLEANCableDetectSupported;
+
+  //
   // Array of the recycled transmit buffer address from UNDI.
   //
   UINT64 *RecycledTxBuf;
@@ -234,6 +240,30 @@ PxeGetStnAddr (
   );
 
 /**
+  Call undi to get the status of the interrupts, get the list of recycled 
transmit
+  buffers that completed transmitting. The recycled transmit buffer address 
will
+  be saved into Snp->RecycledTxBuf. This function will also update the 

Re: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken "Supported VPD Pages" VPD page

[Tian, Feng]

Laszlo, 

Could you explain more? 

Usb Flash drive is managed by UsbMassStorage driver, which is irrelevant with 
UefiScsiLib lib or ScsiDisk driver.

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Wednesday, May 4, 2016 12:30 AM
To: edk2-devel-01 
Cc: Ni, Ruiyu ; Paolo Bonzini ; Tian, 
Feng ; Zeng, Star 
Subject: [edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken "Supported 
VPD Pages" VPD page

The USB flash drive with Vendor ID 0x1516 (CompUSA) and Product ID 0x6221 
returns a broken "Supported VPD Pages" VPD page. In particular, the PageLength 
field has the invalid value 0x0602 (decimal 1538).

This prevents the loop from terminating that scans for the Block Limits VPD 
page code in ScsiDiskInquiryDevice():

for (Index = 0; Index < PageLength; Index++) {

because the Index variable has type UINT8, and it wraps from 255 to 0, without 
ever reaching PageLength (1538), and because 
EFI_SCSI_PAGE_CODE_BLOCK_LIMITS_VPD does not occur at offsets 0 through 255.

* The fix is not to change the type of Index to UINT16 or a wider type.
  Namely, section

7.8.14 Supported VPD Pages VPD page

  in the "SCSI Primary Commands - 4" (SPC-4) specification names the
  following requirement:

The supported VPD page list shall contain a list of all VPD page codes
(see 7.8) implemented by the logical unit in ascending order beginning
with page code 00h.

  Since page codes are 8-bit unsigned quantities, it follows that the
  maximum size for the Supported VPD Pages VPD page is 0x100 bytes, in
  which every possible page code (0x00 through 0xFF) will be found, before
  the UINT8 offset wraps around.

  (EFI_SCSI_SUPPORTED_VPD_PAGES_VPD_PAGE.SupportedVpdPageList is correctly
  sized as well, in "MdePkg/Include/IndustryStandard/Scsi.h".)

* Instead, add sanity checks that enforce the above requirement. If the
  device breaks the spec, simply fall back to the "Block Limits page
  absent" case.

Cc: Feng Tian 
Cc: Paolo Bonzini 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1330955
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 37 
 1 file changed, 37 insertions(+)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index dfa5fa32e635..1b75d55231a6 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -1493,7 +1493,44 @@ ScsiDiskInquiryDevice (
   if (!EFI_ERROR (Status)) {
 PageLength = (SupportedVpdPages->PageLength2 << 8)
|  SupportedVpdPages->PageLength1;
+
+//
+// Sanity checks for coping with broken devices
+//
+if (PageLength > sizeof SupportedVpdPages->SupportedVpdPageList) {
+  DEBUG ((EFI_D_WARN,
+"%a: invalid PageLength (%u) in Supported VPD Pages page\n",
+__FUNCTION__, (UINT32)PageLength));
+  PageLength = 0;
+}
+
+if ((PageLength > 0) &&
+(SupportedVpdPages->SupportedVpdPageList[0] !=
+ EFI_SCSI_PAGE_CODE_SUPPORTED_VPD)) {
+  DEBUG ((EFI_D_WARN,
+"%a: Supported VPD Pages page doesn't start with code 0x%02x\n",
+__FUNCTION__, EFI_SCSI_PAGE_CODE_SUPPORTED_VPD));
+  PageLength = 0;
+}
+
+//
+// Locate the code for the Block Limits VPD page
+//
 for (Index = 0; Index < PageLength; Index++) {
+  //
+  // Sanity check
+  //
+  if ((Index > 0) &&
+  (SupportedVpdPages->SupportedVpdPageList[Index] <=
+   SupportedVpdPages->SupportedVpdPageList[Index - 1])) {
+DEBUG ((EFI_D_WARN,
+  "%a: non-ascending code in Supported VPD Pages page @ %u\n",
+  __FUNCTION__, Index));
+Index = 0;
+PageLength = 0;
+break;
+  }
+
   if (SupportedVpdPages->SupportedVpdPageList[Index] == 
EFI_SCSI_PAGE_CODE_BLOCK_LIMITS_VPD) {
 break;
   }
--
1.8.3.1

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

Re: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei to FspmWrapperPeim and FspsWrapperPeim.

[Tim Lewis]

1.   Because we, per our custom tools and per our company policy, require 
that all source code exist underneath $(WORKSPACE).  As previously discussed on 
this list, we think multiple workspaces makes our projects harder to maintain 
and harder to scan. Multiple workspaces was meant to be an OPTIONAL feature 
and, while some may use it, making it a requirement that all codebases follow 
this path to support what is today already supported seems a bit steep.

2.   There is NO PRODUCTION silicon that uses FSP 2.0 at this time that I 
am aware of. That means Intel is upgrading the Wrapper package to support 
silicon that is not yet shipping and removing support for the (by my count) 5 
publicly announced chipsets, and probably a few others that are in alpha or 
beta.

3.   Many of these chipsets have a long life, which means new projects 
start on them well after initial product launch. Because of features and 
security concerns, we continually upgrade our chipsets to use newer kernel 
code. This puts an extra variable.

Regards,

Tim Lewis
CTO, Insyde Software
www.insyde.com

From: Mudusuru, Giri P [mailto:giri.p.mudus...@intel.com]
Sent: Tuesday, May 03, 2016 4:07 PM
To: Tim Lewis ; Yao, Jiewen ; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P ; Yarlagadda, Satya P 
; Zimmer, Vincent ; 
Mudusuru, Giri P 
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.

Hi Tim,
Yes we considered creating a separate package name along with moving the 
package under Deprecated folder and just leave it in the UDK2015 branch.

There are pro's and con's for all options.

Can you please help me understand on why multi-workspace is not an option?

Thanks,
-Giri

From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, May 03, 2016 8:47 AM
To: Mudusuru, Giri P 
>; Yao, Jiewen 
>; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P 
>; Yarlagadda, 
Satya P >; 
Zimmer, Vincent >; 
Mudusuru, Giri P >
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.


Giri ,

I suggest you create a new package name, rather than use the same one with 
different semantics.



Multiple workspaces is not ah option for us.



Tim



Sent from my Windows 10 phone



From: Mudusuru, Giri P
Sent: Tuesday, May 3, 2016 8:07 AM
To: Tim Lewis; Yao, 
Jiewen; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P; Yarlagadda, Satya 
P; Zimmer, 
Vincent; Mudusuru, Giri 
P
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.


Hi Tim,
The main reason to deprecate the v1.x support was to reduce the complexity and 
focus on v2.0 going forward. At the same time as Jiewen mentioned below our 
plan is to continue maintaining v1.1 in the UDK2015 branch for the existing 
chipsets.

Understand your usage with single tree. For this usage, suggest to use the 
multi-workspace capability so you can have both in a single tree.

Thanks,
-Giri

-Original Message-
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, May 3, 2016 4:48 AM
To: Yao, Jiewen >; 
edk2-devel@lists.01.org
Cc: Mudusuru, Giri P 
>; Rangarajan, Ravi 
P >
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.

Is this really a good idea, dropping 1.1 support? We don't maintain two 
separate trees for our products, just 1. There are several chipsets which have 
long life that will need additional features, but are only 1.1

Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiewen 
Yao
Sent: Monday, May 02, 2016 9:51 PM
To: edk2-devel@lists.01.org
Cc: Giri P Mudusuru 
>; Ravi P 
Rangarajan >
Subject: [edk2] [PATCH 17/19] 

[edk2] [RFC] EDK2 Platforms Proposal

[Kinney, Michael D]

Hello,



Similar to edk2-staging, we also have a need to manage platforms that have been

ported to edk2.  Jordan has created a repository called edk2-platforms and

has created a branch for the minnowboard-max that uses a validated release

of the UDK 2015 for the dependent packages:



https://github.com/tianocore/edk2-platforms



https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015





Instead of creating a branch per feature in edk2-staging, the proposal is to

create a branch per platform in edk2-platforms.  The maintainer(s) that create

and support a platform branch can decide if the platform is synced to 
edk2/master

for dependent packages, or uses a stable release of the edk2 for dependent 
packages.



This proposal provides an area for platform development so we can minimize the

number of platforms that are included in edk2/master.  It is important to keep

some platforms in edk2/master so we can use those platforms to validate features

in non-platform packages in edk2/master.  If a new platform does not add feature

coverage to edk2/master, then a new edk2-platforms branch would be recommended.



Please review the proposal below for edk2-platforms.



If this proposal is accepted, then a review of the platform in edk2/master can

Be done to see if any of them should be moved to branches in edk2-platforms.







Problem statement

=

Need place on tianocore.org where platforms can be maintained by the EDK II

community.  This serves several purposes:



* Encourage more platforms sources to be shared earlier in the development 
process

* Allow platform sources to be shared that may not yet meet all edk2 required 
quality criteria

* Allow platform source to be shared so the EDK II community may choose to help 
finish and validate

* Allow more platforms to be used as part of the edk2 validation and release 
cycle.

* Not intended to be used for bug fixes.



Proposal



1) Create a new repo called edk2-platforms

 a) edk2-platforms is a fork of edk2

 b) edk2-platforms/master tracks edk2/master



2) edk2-platforms discussions can use the existing edk2-devel mailing list for 
design/patch/test.

 Use the following style for discussion of a platform branch in 
edk2-platforms repo.



 [platforms/branch]: Subject



3) All commits to edk2-platforms must follow same edk2 rules (e.g. Tiano 
Contributor's Agreement)



4) Process to add a new platform to edk2-platforms

 a) Maintainer sends patch email to edk2-devel mailing list announcing the 
creation of a new

platform branch in edk2-platforms with Readme.MD.  Readme.MD is in root 
of platform branch

with summary, owners, status, build instructions, target update 
instructions, OS compatibility,

known issues/limitations, links to related materials, and anything else 
a developer would need

to use the platform branch.

 b) Maintainer creates platform branch in edk2-platforms

 c) Maintainer is responsible syncing platform to edk2/master or supported 
edk2 branch.



5) Process to update sources in feature branch

 a) Commit message subject format:



  [platforms/branch PATCH]: Package/Module: Subject



 b) Directly commit changes to platform branch or if community review is 
desired,

then use edk2-devel review process.



7) Process to remove an edk2-platforms branch

 a) Stewards may periodically review of platform branches in edk2-platforms 
(once a quarter?)

 b) If no activity for extended period of time and platform is not being 
maintained and is no

longer functional then stewards send email to edk2-devel to request 
deletion of platform

branch.

 c) If no objections from EDK II community, then platform branch is deleted 
and archived at

https://github.com/tianocore/edk2-archive.



8) How to evaluate a platform in edk2-platforms

 a) Clone edk2-platforms/[branch name]

 b) Following instructions in Readme.MD to build firmware and update target 
platform




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

[edk2] [PATCH] MdeModulePkg: ScsiDiskDxe: cope with broken "Supported VPD Pages" VPD page

[Laszlo Ersek]

The USB flash drive with Vendor ID 0x1516 (CompUSA) and Product ID 0x6221
returns a broken "Supported VPD Pages" VPD page. In particular, the
PageLength field has the invalid value 0x0602 (decimal 1538).

This prevents the loop from terminating that scans for the Block Limits
VPD page code in ScsiDiskInquiryDevice():

for (Index = 0; Index < PageLength; Index++) {

because the Index variable has type UINT8, and it wraps from 255 to 0,
without ever reaching PageLength (1538), and because
EFI_SCSI_PAGE_CODE_BLOCK_LIMITS_VPD does not occur at offsets 0 through
255.

* The fix is not to change the type of Index to UINT16 or a wider type.
  Namely, section

7.8.14 Supported VPD Pages VPD page

  in the "SCSI Primary Commands - 4" (SPC-4) specification names the
  following requirement:

The supported VPD page list shall contain a list of all VPD page codes
(see 7.8) implemented by the logical unit in ascending order beginning
with page code 00h.

  Since page codes are 8-bit unsigned quantities, it follows that the
  maximum size for the Supported VPD Pages VPD page is 0x100 bytes, in
  which every possible page code (0x00 through 0xFF) will be found, before
  the UINT8 offset wraps around.

  (EFI_SCSI_SUPPORTED_VPD_PAGES_VPD_PAGE.SupportedVpdPageList is correctly
  sized as well, in "MdePkg/Include/IndustryStandard/Scsi.h".)

* Instead, add sanity checks that enforce the above requirement. If the
  device breaks the spec, simply fall back to the "Block Limits page
  absent" case.

Cc: Feng Tian 
Cc: Paolo Bonzini 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Ref: https://bugzilla.redhat.com/show_bug.cgi?id=1330955
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 37 
 1 file changed, 37 insertions(+)

diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c 
b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
index dfa5fa32e635..1b75d55231a6 100644
--- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
+++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
@@ -1493,7 +1493,44 @@ ScsiDiskInquiryDevice (
   if (!EFI_ERROR (Status)) {
 PageLength = (SupportedVpdPages->PageLength2 << 8)
|  SupportedVpdPages->PageLength1;
+
+//
+// Sanity checks for coping with broken devices
+//
+if (PageLength > sizeof SupportedVpdPages->SupportedVpdPageList) {
+  DEBUG ((EFI_D_WARN,
+"%a: invalid PageLength (%u) in Supported VPD Pages page\n",
+__FUNCTION__, (UINT32)PageLength));
+  PageLength = 0;
+}
+
+if ((PageLength > 0) &&
+(SupportedVpdPages->SupportedVpdPageList[0] !=
+ EFI_SCSI_PAGE_CODE_SUPPORTED_VPD)) {
+  DEBUG ((EFI_D_WARN,
+"%a: Supported VPD Pages page doesn't start with code 0x%02x\n",
+__FUNCTION__, EFI_SCSI_PAGE_CODE_SUPPORTED_VPD));
+  PageLength = 0;
+}
+
+//
+// Locate the code for the Block Limits VPD page
+//
 for (Index = 0; Index < PageLength; Index++) {
+  //
+  // Sanity check
+  //
+  if ((Index > 0) &&
+  (SupportedVpdPages->SupportedVpdPageList[Index] <=
+   SupportedVpdPages->SupportedVpdPageList[Index - 1])) {
+DEBUG ((EFI_D_WARN,
+  "%a: non-ascending code in Supported VPD Pages page @ %u\n",
+  __FUNCTION__, Index));
+Index = 0;
+PageLength = 0;
+break;
+  }
+
   if (SupportedVpdPages->SupportedVpdPageList[Index] == 
EFI_SCSI_PAGE_CODE_BLOCK_LIMITS_VPD) {
 break;
   }
-- 
1.8.3.1

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

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Andrew Fish]

> On May 3, 2016, at 8:59 AM, Kinney, Michael D  
> wrote:
> 
> Shaveta,
> 
> As long as all PCI I/O Protocol instances in the platform conform to UEFI 
> spec, it can work.  
> 
> Your original question was about how to tell that a handle has the PCI I/O 
> Protocol produced
> By PCI Bus Driver or your emulation of PCI.  If the PCI I/O Protocols are 
> conformant, then
> It should not matter.  Just write a PCI Device Driver that manages a PCI 
> controller based on
> PCI Class code or Vendor ID/Device ID matching and it should work.
> 

I would also point out that Subsystem ID and Subsystem Vendor ID can be used 
for this type of binding too. The Subsystem values exist to deal with cases 
when the same chip is used on different vendors PCI plug in cards. 

Thanks,

Andrew Fish

> Mike
> 
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Shaveta Leekha
>> Sent: Tuesday, May 3, 2016 3:27 AM
>> To: Laszlo Ersek 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how 
>> to open
>> correct one?
>> 
>> 
>> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, May 03, 2016 1:15 PM
>> To: Shaveta Leekha 
>> Cc: edk2-devel@lists.01.org 
>> Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how 
>> to open
>> correct one?
>> 
>> On 05/03/16 08:54, Shaveta Leekha wrote:
>>> Hi,
>>> 
>>> I have a scenario where two separate drivers are installing/producing "PCI 
>>> IO"
>> protocol with same GUID (gEfiPciIoProtocolGuid).
>>> 
>>> Where:
>>> 
>>> (1)One of the PCI Io protocol instance is produced by
>> "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like 
>> E1000/NIC card
>> to use)
>>> 
>>> (2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
>>> created
>> for USB and SATA kind of controllers. This protocol (APIs) is intended to be 
>> used by
>> USB and SATA controller drivers.
>>> 
>>> Now when I use "OpenProtocol" in my "Platform specific Sata controller 
>>> driver" using:
>>> 
>>> Status = gBS->OpenProtocol (
>>>  Controller,
>>>  ,
>>>  (VOID **) ,
>>>  This->DriverBindingHandle,
>>>  Controller,
>>>  EFI_OPEN_PROTOCOL_GET_PROTOCOL
>>>  );
>>> 
>>> How can I make sure that PCI Io protocol that is produced by "PciEmulation" 
>>> layer
>> driver is getting opened?
>>> 
>>> As "gEfiPciIoProtocolGuid" is same for both protocol instances.
>>> 
>>> How to handle such scenarios?
>> 
>> The UEFI spec forbids installing more than one protocol interface with the 
>> same GUID on
>> the same handle. See
>> EFI_BOOT_SERVICES.InstallProtocolInterface():
>> 
>>  [... The same GUID cannot be installed more than once onto the same
>>  handle. If installation of a duplicate GUID on a handle is attempted,
>>  an EFI_INVALID_PARAMETER will result. [...]
>> 
>>  EFI_INVALID_PARAMETER  Protocol is already installed on the handle
>> specified by Handle.
>> 
>> If you have two separate UEFI drivers that install PciIo interfaces on 
>> handles, then
>> their EFI_DRIVER_BINDING_PROTOCOL.Supported() methods may report success 
>> only on
>> distinct sets of handles. There must be no overlap.
>> 
>> But, in any case, I think it's very unusual to have two drivers that produce 
>> PciIo
>> interfaces. According to the "Driver Writer's Guide for UEFI 2.3.1" 
>> (03/08/2012,
>> Version 1.01), section "18.2 PCI Bus Drivers":
>> 
>>  TIP:  PCI Bus Driver customizations are *strongly discouraged*
>>because the PCI Bus Driver is designed to be conformant with
>>the /PCI Specification/. Instead, focus platform specific
>>customizations on the Root Bridge Driver that produced
>>EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL and its PCI Device Drivers.
>> 
>> 
>> [Shaveta]
>> Yes, it is un-usual and we had discussed over it also.
>> 
>> So the scenario is:
>> 
>> On LS2080 platform, we have real PCI controllers that have E1000 card on 
>> them(used for
>> networking purposes) And USB and SATA controllers are on-board.
>> So to provide SATA and USB, a PCIe emulation layer, a PCI IO protocol is 
>> installed in
>> PciEmulation code.
>> PciEmulation code is very thin layer that simply produce PciRootBridgeIo 
>> protocol and
>> hence PCI IO protocol,
>> Hence bypass the need of "real Pci Bus" code.
>> 
>> Reference is taken from ArmJune platform code in EDK2. In it, it install :
>> (1) PCI IO protocol using "gEfiPciIoProtocolGuid" in
>> ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/PciEmulation.c
>> (2) It also install " gEfiPciRootBridgeIoProtocolGuid" protocol, using which
>> "PciBusDxe" will produce PciIo protocol
>> 
>> The same we are 

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Kinney, Michael D]

Shaveta,

As long as all PCI I/O Protocol instances in the platform conform to UEFI spec, 
it can work.  

Your original question was about how to tell that a handle has the PCI I/O 
Protocol produced
By PCI Bus Driver or your emulation of PCI.  If the PCI I/O Protocols are 
conformant, then
It should not matter.  Just write a PCI Device Driver that manages a PCI 
controller based on
PCI Class code or Vendor ID/Device ID matching and it should work.

Mike


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Shaveta Leekha
> Sent: Tuesday, May 3, 2016 3:27 AM
> To: Laszlo Ersek 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how 
> to open
> correct one?
> 
> 
> 
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, May 03, 2016 1:15 PM
> To: Shaveta Leekha 
> Cc: edk2-devel@lists.01.org 
> Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how 
> to open
> correct one?
> 
> On 05/03/16 08:54, Shaveta Leekha wrote:
> > Hi,
> >
> > I have a scenario where two separate drivers are installing/producing "PCI 
> > IO"
> protocol with same GUID (gEfiPciIoProtocolGuid).
> >
> > Where:
> >
> > (1)One of the PCI Io protocol instance is produced by
> "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like 
> E1000/NIC card
> to use)
> >
> > (2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
> > created
> for USB and SATA kind of controllers. This protocol (APIs) is intended to be 
> used by
> USB and SATA controller drivers.
> >
> > Now when I use "OpenProtocol" in my "Platform specific Sata controller 
> > driver" using:
> >
> > Status = gBS->OpenProtocol (
> >   Controller,
> >   ,
> >   (VOID **) ,
> >   This->DriverBindingHandle,
> >   Controller,
> >   EFI_OPEN_PROTOCOL_GET_PROTOCOL
> >   );
> >
> > How can I make sure that PCI Io protocol that is produced by "PciEmulation" 
> > layer
> driver is getting opened?
> >
> > As "gEfiPciIoProtocolGuid" is same for both protocol instances.
> >
> > How to handle such scenarios?
> 
> The UEFI spec forbids installing more than one protocol interface with the 
> same GUID on
> the same handle. See
> EFI_BOOT_SERVICES.InstallProtocolInterface():
> 
>   [... The same GUID cannot be installed more than once onto the same
>   handle. If installation of a duplicate GUID on a handle is attempted,
>   an EFI_INVALID_PARAMETER will result. [...]
> 
>   EFI_INVALID_PARAMETER  Protocol is already installed on the handle
>  specified by Handle.
> 
> If you have two separate UEFI drivers that install PciIo interfaces on 
> handles, then
> their EFI_DRIVER_BINDING_PROTOCOL.Supported() methods may report success only 
> on
> distinct sets of handles. There must be no overlap.
> 
> But, in any case, I think it's very unusual to have two drivers that produce 
> PciIo
> interfaces. According to the "Driver Writer's Guide for UEFI 2.3.1" 
> (03/08/2012,
> Version 1.01), section "18.2 PCI Bus Drivers":
> 
>   TIP:  PCI Bus Driver customizations are *strongly discouraged*
> because the PCI Bus Driver is designed to be conformant with
> the /PCI Specification/. Instead, focus platform specific
> customizations on the Root Bridge Driver that produced
> EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL and its PCI Device Drivers.
> 
> 
> [Shaveta]
> Yes, it is un-usual and we had discussed over it also.
> 
> So the scenario is:
> 
> On LS2080 platform, we have real PCI controllers that have E1000 card on 
> them(used for
> networking purposes) And USB and SATA controllers are on-board.
> So to provide SATA and USB, a PCIe emulation layer, a PCI IO protocol is 
> installed in
> PciEmulation code.
> PciEmulation code is very thin layer that simply produce PciRootBridgeIo 
> protocol and
> hence PCI IO protocol,
> Hence bypass the need of "real Pci Bus" code.
> 
> Reference is taken from ArmJune platform code in EDK2. In it, it install :
> (1) PCI IO protocol using "gEfiPciIoProtocolGuid" in
> ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/PciEmulation.c
> (2) It also install " gEfiPciRootBridgeIoProtocolGuid" protocol, using which
> "PciBusDxe" will produce PciIo protocol
> 
> The same we are doing for our platform.
> For real PCI controller, we have " installed Root Bridge PCI IO protocol with
> gEfiPciRootBridgeIoProtocolGuid"
> And for emulation layer, the code direcly install PCI IO protocol with
> gEfiPciIoProtocolGuid"
> 
> Regards,
> Shaveta
> 
> Laszlo
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei to FspmWrapperPeim and FspsWrapperPeim.

[Tim Lewis]

Giri ,

I suggest you create a new package name, rather than use the same one with 
different semantics.



Multiple workspaces is not ah option for us.



Tim



Sent from my Windows 10 phone



From: Mudusuru, Giri P
Sent: Tuesday, May 3, 2016 8:07 AM
To: Tim Lewis; Yao, 
Jiewen; 
edk2-devel@lists.01.org
Cc: Rangarajan, Ravi P; Yarlagadda, Satya 
P; Zimmer, 
Vincent; Mudusuru, Giri 
P
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.



Hi Tim,
The main reason to deprecate the v1.x support was to reduce the complexity and 
focus on v2.0 going forward. At the same time as Jiewen mentioned below our 
plan is to continue maintaining v1.1 in the UDK2015 branch for the existing 
chipsets.

Understand your usage with single tree. For this usage, suggest to use the 
multi-workspace capability so you can have both in a single tree.

Thanks,
-Giri

-Original Message-
From: Tim Lewis [mailto:tim.le...@insyde.com]
Sent: Tuesday, May 3, 2016 4:48 AM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Mudusuru, Giri P ; Rangarajan, Ravi P 

Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.

Is this really a good idea, dropping 1.1 support? We don't maintain two 
separate trees for our products, just 1. There are several chipsets which have 
long life that will need additional features, but are only 1.1

Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiewen 
Yao
Sent: Monday, May 02, 2016 9:51 PM
To: edk2-devel@lists.01.org
Cc: Giri P Mudusuru ; Ravi P Rangarajan 

Subject: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei to 
FspmWrapperPeim and FspsWrapperPeim.

Update FSP to follow:
https://firmware.intel.com/sites/default/files/FSP_EAS_v2.0_Draft%20External.pdf

Align to FSP2.0.
Remove 1.1 support from FspInit.
Split it into FspmWrapperPeim and FspsWrapperPeim, so they can build in
different FV.

The FSP1.1 compatibility is NOT maintained.

The new Intel platform will follow FSP2.0.
The old platform can either use an old EDK branch,
or move FSP1.1 support to platform directory.

Cc: Giri P Mudusuru 
Cc: Maurice Ma 
Cc: Ravi P Rangarajan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
Reviewed-by: Giri P Mudusuru 
Reviewed-by: Maurice Ma 
Reviewed-by: Ravi P Rangarajan 
---
 IntelFspWrapperPkg/FspInitPei/FindPeiCore.c| 199 
 IntelFspWrapperPkg/FspInitPei/FspInitPei.c |  66 
 IntelFspWrapperPkg/FspInitPei/FspInitPei.h |  64 
 IntelFspWrapperPkg/FspInitPei/FspInitPei.inf   |  90 --
 IntelFspWrapperPkg/FspInitPei/FspInitPeiV1.c   | 182 ---
 IntelFspWrapperPkg/FspInitPei/FspInitPeiV2.c   | 338 

 IntelFspWrapperPkg/FspInitPei/FspNotifyS3.c|  80 -
 IntelFspWrapperPkg/FspInitPei/SecMain.c| 310 --
 IntelFspWrapperPkg/FspInitPei/SecMain.h| 116 ---
 IntelFspWrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c   | 161 ++
 IntelFspWrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf |  77 +
 IntelFspWrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c   | 313 ++
 IntelFspWrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf |  79 +
 13 files changed, 630 insertions(+), 1445 deletions(-)

diff --git a/IntelFspWrapperPkg/FspInitPei/FindPeiCore.c 
b/IntelFspWrapperPkg/FspInitPei/FindPeiCore.c
deleted file mode 100644
index ce003d0..000
--- a/IntelFspWrapperPkg/FspInitPei/FindPeiCore.c
+++ /dev/null
@@ -1,199 +0,0 @@
-/** @file
-  Locate the entry point for the PEI Core
-
-  Copyright (c) 2014, 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.
-
-**/
-
-#include 
-#include 
-#include 
-
-#include "SecMain.h"
-
-/**
-  Find core image base.
-
-  @param[in]  BootFirmwareVolumePtrPoint to the boot firmware volume.
-  

Re: [edk2] [PATCH v3 1/2] ShellPkg/UefiDpLib: Fix the error message "Timer library instance error!"

[Carsey, Jaben]

Thanks!

> -Original Message-
> From: Zeng, Star
> Sent: Monday, May 02, 2016 9:39 PM
> To: Carsey, Jaben ; Cinnamon Shia
> ; edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; samer.el-haj-mahm...@hpe.com
> Subject: RE: [PATCH v3 1/2] ShellPkg/UefiDpLib: Fix the error message "Timer
> library instance error!"
> Importance: High
> 
> Pushed at 69af847603ca1f6439031c4e1fee844fdb06ca6f and
> 0cd1e699e5f76b3323b5881932af8a62bc5edf2a.
> 
> Thanks,
> Star
> -Original Message-
> From: Carsey, Jaben
> Sent: Monday, May 2, 2016 11:33 PM
> To: Zeng, Star ; Cinnamon Shia
> ; edk2-devel@lists.01.org
> Cc: Qiu, Shumin ; samer.el-haj-mahm...@hpe.com;
> Carsey, Jaben 
> Subject: RE: [PATCH v3 1/2] ShellPkg/UefiDpLib: Fix the error message "Timer
> library instance error!"
> 
> Reviewed-by: Jaben Carsey 
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Monday, May 02, 2016 1:25 AM
> > To: Cinnamon Shia ; edk2-devel@lists.01.org
> > Cc: Qiu, Shumin ; Carsey, Jaben
> > ; samer.el-haj-mahm...@hpe.com
> > Subject: RE: [PATCH v3 1/2] ShellPkg/UefiDpLib: Fix the error message
> > "Timer library instance error!"
> > Importance: High
> >
> > Reviewed-by: Star Zeng  to series.
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Cinnamon Shia [mailto:cinnamon.s...@hpe.com]
> > Sent: Monday, May 2, 2016 12:34 PM
> > To: edk2-devel@lists.01.org
> > Cc: Qiu, Shumin ; Zeng, Star
> > ; Carsey, Jaben ;
> > samer.el-haj-mahm...@hpe.com; Cinnamon Shia
> 
> > Subject: [PATCH v3 1/2] ShellPkg/UefiDpLib: Fix the error message
> > "Timer library instance error!"
> >
> > When executing shell dp command, there is an error message "Timer
> > library instance error!"
> >
> > The error message "Timer library instance error!" should be for the
> > case about duration > EndTimeStamp if CountUp or duration >
> > StartTimeStamp if CountDown.
> >
> > But if the EndTimeStamp of an entry is not added, it should not the
> > case to catch.
> >
> > This change fixes the error message "Timer library instance error!"
> > from the "BdsAttempt" entry which is logged when trying to boot a boot
> option.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Cinnamon Shia 
> > ---
> >  ShellPkg/Library/UefiDpLib/DpUtilities.c | 6 +-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/ShellPkg/Library/UefiDpLib/DpUtilities.c
> > b/ShellPkg/Library/UefiDpLib/DpUtilities.c
> > index 032e7b4..fbdd938 100644
> > --- a/ShellPkg/Library/UefiDpLib/DpUtilities.c
> > +++ b/ShellPkg/Library/UefiDpLib/DpUtilities.c
> > @@ -2,7 +2,7 @@
> >Utility functions used by the Dp application.
> >
> >Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> > -  (C) Copyright 2015 Hewlett Packard Enterprise Development LP
> > +  (C) Copyright 2015-2016 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 @@ -68,6 +68,10 @@ GetDuration (
> >UINT64Duration;
> >BOOLEAN   Error;
> >
> > +  if (Measurement->EndTimeStamp == 0) {
> > +return 0;
> > +  }
> > +
> >// PERF_START macros are called with a value of 1 to indicate
> >// the beginning of time.  So, adjust the start ticker value
> >// to the real beginning of time.
> > --
> > 2.8.1.windows.1

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

Re: [edk2] [RFC PATCH 2/2] ArmVirtQemu: restrict RWX mappings

[Ard Biesheuvel]

On 3 May 2016 at 16:01, Laszlo Ersek  wrote:
> On 05/03/16 14:45, Ard Biesheuvel wrote:
>> This reduces the amount of memory mapped as both writable and executable
>> to the absolute minimum, by mapping all of memory non-executable by
>> default, and using PermissionsPeCoffExtraActionLib to only map those
>> regions executable that require it for execution. If possible, these
>> regions are remapped read-only at the same time, but in some cases
>> (runtime drivers, TE images, images with < 4 KB section alignment) we
>> cannot avoid having to map the entire PE/COFF image RWX.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/ArmVirtQemu.dsc   | 9 
>> -
>>  ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 3 +++
>>  2 files changed, 11 insertions(+), 1 deletion(-)
>
> (1) Could you include a comprehensive table in the commit message that
> describes what handling affects what driver type, and where each
> treatment is established?
>

Sure

> For example, you mention TE images as exempt. According to
> "ArmVirtQemu.fdf", this means SEC, PEI_CORE, PEIM. However, below you
> link the library into PeiMain.inf (i.e., the PEI_CORE) as well. Why is
> that correct?
>

PeiCore and DxeCore both contain PE/COFF loaders, which is why they
need this library resolution. This is independent of whether these
modules are TE themselves.

> Also, why must e.g. runtime drivers be mapped fully RWX?
>

Runtime drivers are relocated a second time when
SetVirtualAddressMap() is called, and the boot time MMU configuration
may still be live at this point. Since these relocations point into
the .text section as well (e.g., to relocate const structures
containing function pointers), they cannot be mapped read-only. This
is a bit disappointing, actually, and I wonder if there is a better
solution. In fact, i was hoping for some discussion on this topic in
general, not necessarily on the patches.

> (2) Shouldn't ArmVirtQemuKernel.dsc be modified as well? (Maybe not
> identically, since it runs entirely from DRAM, but still?)
>

The fact that it runs from DRAM is a problem. In fact, this patch will
break if applies as is. The reason is that remapping all of RAM noexec
will affect the code that is currently being executed. Here,
ArmVirtQemu has a big advantage since NOR flash is R-X by nature.

> (3) I think it's worth keeping in mind that on X64 and Ia32, some
> drivers generate code dynamically. For example, in the implementation of
> EFI_MP_SERVICES_PROTOCOL. I don't think it should block this series, but
> perhaps make a note somewhere (code? commit message?) that such drivers
> will have to massage their mappings individually on aarch64.
>

This is exactly the kind of feedback i was hoping for (hence the RFC
in the subject line). There may be other cases where executable code
is copied (and now that I think of it, we have such code in AArch64 as
well). When it is arch-specific code, that can be handled ad hoc, but
if there are generic ways of putting code on the heap, it would
require some standardized protocol to manage this.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC PATCH 2/2] ArmVirtQemu: restrict RWX mappings

[Laszlo Ersek]

On 05/03/16 14:45, Ard Biesheuvel wrote:
> This reduces the amount of memory mapped as both writable and executable
> to the absolute minimum, by mapping all of memory non-executable by
> default, and using PermissionsPeCoffExtraActionLib to only map those
> regions executable that require it for execution. If possible, these
> regions are remapped read-only at the same time, but in some cases
> (runtime drivers, TE images, images with < 4 KB section alignment) we
> cannot avoid having to map the entire PE/COFF image RWX.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/ArmVirtQemu.dsc   | 9 
> -
>  ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 3 +++
>  2 files changed, 11 insertions(+), 1 deletion(-)

(1) Could you include a comprehensive table in the commit message that
describes what handling affects what driver type, and where each
treatment is established?

For example, you mention TE images as exempt. According to
"ArmVirtQemu.fdf", this means SEC, PEI_CORE, PEIM. However, below you
link the library into PeiMain.inf (i.e., the PEI_CORE) as well. Why is
that correct?

Also, why must e.g. runtime drivers be mapped fully RWX?

(2) Shouldn't ArmVirtQemuKernel.dsc be modified as well? (Maybe not
identically, since it runs entirely from DRAM, but still?)

(3) I think it's worth keeping in mind that on X64 and Ia32, some
drivers generate code dynamically. For example, in the implementation of
EFI_MP_SERVICES_PROTOCOL. I don't think it should block this series, but
perhaps make a note somewhere (code? commit message?) that such drivers
will have to massage their mappings individually on aarch64.

Thanks!
Laszlo

> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 34323bf83d64..fbddd7fac7b5 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -82,6 +82,9 @@ [BuildOptions]
>GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 
> -I$(WORKSPACE)/ArmVirtPkg/Include
>*_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
>  
> +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER]
> +  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1000
> +
>  
>  
> 
>  #
> @@ -243,7 +246,10 @@ [Components.common]
># PEI Phase modules
>#
>ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
> -  MdeModulePkg/Core/Pei/PeiMain.inf
> +  MdeModulePkg/Core/Pei/PeiMain.inf {
> +
> +  
> PeCoffExtraActionLib|ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
> +  }
>MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
>  
>PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> @@ -265,6 +271,7 @@ [Components.common]
>MdeModulePkg/Core/Dxe/DxeMain.inf {
>  
>
> NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +  
> PeCoffExtraActionLib|ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
>}
>MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>  
> diff --git 
> a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c 
> b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> index f6c69152848e..c5899aa2ac3c 100644
> --- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> +++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
> @@ -111,6 +111,9 @@ MemoryPeim (
>// Build Memory Allocation Hob
>InitMmu ();
>  
> +  ArmSetMemoryRegionNoExec ((EFI_PHYSICAL_ADDRESS)PcdGet64 
> (PcdSystemMemoryBase),
> +PcdGet64 (PcdSystemMemorySize));
> +
>if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
>  // Optional feature that helps prevent EFI memory map fragmentation.
>  BuildMemoryTypeInformationHob ();
> 

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

Re: [edk2] [RFC PATCH 0/2] ARM: reduce RWX mappings to absolute minimum

[Ard Biesheuvel]

On 3 May 2016 at 15:01, Michael Zimmermann  wrote:
> wouldn't it be even better to map everything (the whole 32/64bit addr range)
> without any permissions(fault on read, write or exec), and then require the
> platforms to map IO-registers properly?
>

We already do that. While we use a 1:1 mapping, we are still enabling
the MMU and so we need to map memory and I/O with different
attributes, and regions that we leave unmapped will fault. However,
the default mapping for memory is RWX, and we only deviate from that
at boot time for the stack in some cases, while it would be much
better to map all of RAM RW- by default, and only switch to R-X when
necessary.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC PATCH 0/2] ARM: reduce RWX mappings to absolute minimum

[Michael Zimmermann]

wouldn't it be even better to map everything (the whole 32/64bit addr
range) without any permissions(fault on read, write or exec), and then
require the platforms to map IO-registers properly?

Michael

On Tue, May 3, 2016 at 2:45 PM, Ard Biesheuvel 
wrote:

> From a security perspective, having any memory with both write and execute
> permissions is undesirable, and EDK2 at boot time has all of memory mapped
> RWX by default, so there is obviously some room for improvement here.
>
> This series aims to reduce the attack surface, by mapping the whole of
> system RAM without executable permissions by default, and only giving
> such permissions on PE/COFF sections containing executable code, while
> revoking the write permissions on such regions at the same time [to the
> extent possible].
>
> Patch #1 implements a PeCoffExtraActionLib that remaps executable PE/COFF
> sections with execute permissions, and removes the writable permissions at
> at the same time as well.
>
> Patch #2 enables this functionality for ArmVirtQemu, by mapping system
> memory as non-exec, and using the library from patch #1. Also, it increases
> the PE/COFF section alignment to 4 KB for DXE_CORE, DXE_DRIVER and
> UEFI_DRIVER modules.
>
> Comments welcome.
>
> Ard Biesheuvel (2):
>   ArmPkg/PermissionsPeCoffExtraActionLib: introduce new library
>   ArmVirtQemu: restrict RWX mappings
>
>  
> ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
>  | 202 
>  
> ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
> |  45 +
>  ArmVirtPkg/ArmVirtQemu.dsc
>  |   9 +-
>  ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
>  |   3 +
>  4 files changed, 258 insertions(+), 1 deletion(-)
>  create mode 100644
> ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
>  create mode 100644
> ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
>
> --
> 2.7.4
>
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [RFC PATCH 2/2] ArmVirtQemu: restrict RWX mappings

[Ard Biesheuvel]

This reduces the amount of memory mapped as both writable and executable
to the absolute minimum, by mapping all of memory non-executable by
default, and using PermissionsPeCoffExtraActionLib to only map those
regions executable that require it for execution. If possible, these
regions are remapped read-only at the same time, but in some cases
(runtime drivers, TE images, images with < 4 KB section alignment) we
cannot avoid having to map the entire PE/COFF image RWX.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/ArmVirtQemu.dsc   | 9 
-
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c | 3 +++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 34323bf83d64..fbddd7fac7b5 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -82,6 +82,9 @@ [BuildOptions]
   GCC:*_*_ARM_PLATFORM_FLAGS == -mcpu=cortex-a15 
-I$(WORKSPACE)/ArmVirtPkg/Include
   *_*_AARCH64_PLATFORM_FLAGS == -I$(WORKSPACE)/ArmVirtPkg/Include
 
+[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER]
+  GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1000
+
 
 

 #
@@ -243,7 +246,10 @@ [Components.common]
   # PEI Phase modules
   #
   ArmPlatformPkg/PrePeiCore/PrePeiCoreUniCore.inf
-  MdeModulePkg/Core/Pei/PeiMain.inf
+  MdeModulePkg/Core/Pei/PeiMain.inf {
+
+  
PeCoffExtraActionLib|ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
+  }
   MdeModulePkg/Universal/PCD/Pei/Pcd.inf {
 
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
@@ -265,6 +271,7 @@ [Components.common]
   MdeModulePkg/Core/Dxe/DxeMain.inf {
 
   
NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
+  
PeCoffExtraActionLib|ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
   }
   MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
 
diff --git 
a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c 
b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
index f6c69152848e..c5899aa2ac3c 100644
--- a/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
+++ b/ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c
@@ -111,6 +111,9 @@ MemoryPeim (
   // Build Memory Allocation Hob
   InitMmu ();
 
+  ArmSetMemoryRegionNoExec ((EFI_PHYSICAL_ADDRESS)PcdGet64 
(PcdSystemMemoryBase),
+PcdGet64 (PcdSystemMemorySize));
+
   if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
 // Optional feature that helps prevent EFI memory map fragmentation.
 BuildMemoryTypeInformationHob ();
-- 
2.7.4

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

[edk2] [RFC PATCH 1/2] ArmPkg/PermissionsPeCoffExtraActionLib: introduce new library

[Ard Biesheuvel]

This introduces a new implementation of PeCoffExtraActionLib that remaps
PE/COFF executable sections as read-only if the section attributes allow
it (and if the module is not a runtime driver)

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 
ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
   | 202 
 
ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
 |  45 +
 2 files changed, 247 insertions(+)

diff --git 
a/ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
 
b/ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
new file mode 100644
index ..565cbe4db57f
--- /dev/null
+++ 
b/ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
@@ -0,0 +1,202 @@
+/**@file
+
+PeCoff extra action library for PEI and DXE phase to set strict permissions on
+PE/COFF executables using ArmLib
+
+Copyright (c) 2016, Linaro Ltd. All rights reserved.
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD 
License
+which accompanies this distribution.  The full text of the license may be 
found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+typedef RETURN_STATUS (*REGION_PERMISSION_UPDATE_FUNC) (
+  IN  EFI_PHYSICAL_ADDRESS  BaseAddress,
+  IN  UINT64Length
+  );
+
+STATIC
+RETURN_STATUS
+UpdatePeCoffPermissions (
+  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
+  IN  REGION_PERMISSION_UPDATE_FUNC   NoExecUpdater,
+  IN  REGION_PERMISSION_UPDATE_FUNC   ReadOnlyUpdater
+  )
+{
+  RETURN_STATUS Status;
+  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION   Hdr;
+  EFI_IMAGE_OPTIONAL_HEADER_UNION   HdrData;
+  UINTN Size;
+  UINTN ReadSize;
+  UINT32SectionHeaderOffset;
+  UINTN NumberOfSections;
+  UINTN Index;
+  EFI_IMAGE_SECTION_HEADER  SectionHeader;
+  PE_COFF_LOADER_IMAGE_CONTEXT  TmpContext;
+  EFI_PHYSICAL_ADDRESS  Base;
+
+  //
+  // We need to copy ImageContext since PeCoffLoaderGetImageInfo ()
+  // will mangle the ImageAddress field
+  //
+  CopyMem (, ImageContext, sizeof (TmpContext));
+
+  if (TmpContext.PeCoffHeaderOffset == 0) {
+Status = PeCoffLoaderGetImageInfo ();
+if (RETURN_ERROR (Status)) {
+  DEBUG ((EFI_D_ERROR,
+"%a: PeCoffLoaderGetImageInfo () failed (Status = %r)\n",
+__FUNCTION__, Status));
+  return Status;
+}
+  }
+
+  if (TmpContext.IsTeImage &&
+  TmpContext.ImageAddress == ImageContext->ImageAddress) {
+DEBUG ((EFI_D_INFO, "%a: ignoring XIP TE image at 0x%lx\n", __FUNCTION__,
+  ImageContext->ImageAddress));
+return RETURN_SUCCESS;
+  }
+
+  if (TmpContext.SectionAlignment < EFI_PAGE_SIZE) {
+//
+// The sections need to be at least 4 KB aligned, since that is the
+// granularity at which we can tighten permissions. So just clear the
+// noexec permissions on the entire region.
+//
+if (!TmpContext.IsTeImage) {
+  DEBUG ((EFI_D_WARN,
+"%a: non-TE Image at 0x%lx has SectionAlignment < 4 KB (%lu)\n",
+__FUNCTION__, ImageContext->ImageAddress, 
TmpContext.SectionAlignment));
+}
+Base = ImageContext->ImageAddress & ~(EFI_PAGE_SIZE - 1);
+Size = ImageContext->ImageAddress - Base + ImageContext->ImageSize;
+return NoExecUpdater (Base, ALIGN_VALUE (Size, EFI_PAGE_SIZE));
+  }
+
+  //
+  // Read the PE/COFF Header. For PE32 (32-bit) this will read in too much
+  // data, but that should not hurt anything. Hdr.Pe32->OptionalHeader.Magic
+  // determines if this is a PE32 or PE32+ image. The magic is in the same
+  // location in both images.
+  //
+  Hdr.Union = 
+  Size = sizeof (EFI_IMAGE_OPTIONAL_HEADER_UNION);
+  ReadSize = Size;
+  Status = TmpContext.ImageRead (TmpContext.Handle,
+ TmpContext.PeCoffHeaderOffset, , Hdr.Pe32);
+  if (RETURN_ERROR (Status) || (Size != ReadSize)) {
+DEBUG ((EFI_D_ERROR,
+  "%a: TmpContext.ImageRead () failed (Status = %r)\n",
+  __FUNCTION__, Status));
+return Status;
+  }
+
+  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
+
+  SectionHeaderOffset = TmpContext.PeCoffHeaderOffset + sizeof (UINT32) +
+sizeof (EFI_IMAGE_FILE_HEADER);
+  NumberOfSections= (UINTN)(Hdr.Pe32->FileHeader.NumberOfSections);
+
+  switch (Hdr.Pe32->OptionalHeader.Magic) {
+case 

[edk2] [RFC PATCH 0/2] ARM: reduce RWX mappings to absolute minimum

[Ard Biesheuvel]

>From a security perspective, having any memory with both write and execute
permissions is undesirable, and EDK2 at boot time has all of memory mapped
RWX by default, so there is obviously some room for improvement here.

This series aims to reduce the attack surface, by mapping the whole of
system RAM without executable permissions by default, and only giving
such permissions on PE/COFF sections containing executable code, while
revoking the write permissions on such regions at the same time [to the
extent possible].

Patch #1 implements a PeCoffExtraActionLib that remaps executable PE/COFF
sections with execute permissions, and removes the writable permissions at
at the same time as well.

Patch #2 enables this functionality for ArmVirtQemu, by mapping system
memory as non-exec, and using the library from patch #1. Also, it increases
the PE/COFF section alignment to 4 KB for DXE_CORE, DXE_DRIVER and
UEFI_DRIVER modules.

Comments welcome.

Ard Biesheuvel (2):
  ArmPkg/PermissionsPeCoffExtraActionLib: introduce new library
  ArmVirtQemu: restrict RWX mappings

 
ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
   | 202 
 
ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf
 |  45 +
 ArmVirtPkg/ArmVirtQemu.dsc 
|   9 +-
 ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c   
|   3 +
 4 files changed, 258 insertions(+), 1 deletion(-)
 create mode 100644 
ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.c
 create mode 100644 
ArmPkg/Library/PermissionsPeCoffExtraActionLib/PermissionsPeCoffExtraActionLib.inf

-- 
2.7.4

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

Re: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei to FspmWrapperPeim and FspsWrapperPeim.

[Tim Lewis]

Is this really a good idea, dropping 1.1 support? We don't maintain two 
separate trees for our products, just 1. There are several chipsets which have 
long life that will need additional features, but are only 1.1

Tim

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiewen 
Yao
Sent: Monday, May 02, 2016 9:51 PM
To: edk2-devel@lists.01.org
Cc: Giri P Mudusuru ; Ravi P Rangarajan 

Subject: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei to 
FspmWrapperPeim and FspsWrapperPeim.

Update FSP to follow:
https://firmware.intel.com/sites/default/files/FSP_EAS_v2.0_Draft%20External.pdf

Align to FSP2.0.
Remove 1.1 support from FspInit.
Split it into FspmWrapperPeim and FspsWrapperPeim, so they can build in
different FV.

The FSP1.1 compatibility is NOT maintained.

The new Intel platform will follow FSP2.0.
The old platform can either use an old EDK branch,
or move FSP1.1 support to platform directory.

Cc: Giri P Mudusuru 
Cc: Maurice Ma 
Cc: Ravi P Rangarajan 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
Reviewed-by: Giri P Mudusuru 
Reviewed-by: Maurice Ma 
Reviewed-by: Ravi P Rangarajan 
---
 IntelFspWrapperPkg/FspInitPei/FindPeiCore.c| 199 
 IntelFspWrapperPkg/FspInitPei/FspInitPei.c |  66 
 IntelFspWrapperPkg/FspInitPei/FspInitPei.h |  64 
 IntelFspWrapperPkg/FspInitPei/FspInitPei.inf   |  90 --
 IntelFspWrapperPkg/FspInitPei/FspInitPeiV1.c   | 182 ---
 IntelFspWrapperPkg/FspInitPei/FspInitPeiV2.c   | 338 

 IntelFspWrapperPkg/FspInitPei/FspNotifyS3.c|  80 -
 IntelFspWrapperPkg/FspInitPei/SecMain.c| 310 --
 IntelFspWrapperPkg/FspInitPei/SecMain.h| 116 ---
 IntelFspWrapperPkg/FspmWrapperPeim/FspmWrapperPeim.c   | 161 ++
 IntelFspWrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf |  77 +
 IntelFspWrapperPkg/FspsWrapperPeim/FspsWrapperPeim.c   | 313 ++
 IntelFspWrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf |  79 +
 13 files changed, 630 insertions(+), 1445 deletions(-)

diff --git a/IntelFspWrapperPkg/FspInitPei/FindPeiCore.c 
b/IntelFspWrapperPkg/FspInitPei/FindPeiCore.c
deleted file mode 100644
index ce003d0..000
--- a/IntelFspWrapperPkg/FspInitPei/FindPeiCore.c
+++ /dev/null
@@ -1,199 +0,0 @@
-/** @file
-  Locate the entry point for the PEI Core
-
-  Copyright (c) 2014, 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.
-
-**/
-
-#include 
-#include 
-#include 
-
-#include "SecMain.h"
-
-/**
-  Find core image base.
-
-  @param[in]  BootFirmwareVolumePtrPoint to the boot firmware volume.
-  @param[out] SecCoreImageBase The base address of the SEC core image.
-  @param[out] PeiCoreImageBase The base address of the PEI core image.
-
-**/
-EFI_STATUS
-EFIAPI
-FindImageBase (
-  IN  EFI_FIRMWARE_VOLUME_HEADER   *BootFirmwareVolumePtr,
-  OUT EFI_PHYSICAL_ADDRESS *SecCoreImageBase,
-  OUT EFI_PHYSICAL_ADDRESS *PeiCoreImageBase
-  )
-{
-  EFI_PHYSICAL_ADDRESSCurrentAddress;
-  EFI_PHYSICAL_ADDRESSEndOfFirmwareVolume;
-  EFI_FFS_FILE_HEADER *File;
-  UINT32  Size;
-  EFI_PHYSICAL_ADDRESSEndOfFile;
-  EFI_COMMON_SECTION_HEADER   *Section;
-  EFI_PHYSICAL_ADDRESSEndOfSection;
-
-  *SecCoreImageBase = 0;
-  *PeiCoreImageBase = 0;
-
-  CurrentAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) BootFirmwareVolumePtr;
-  EndOfFirmwareVolume = CurrentAddress + BootFirmwareVolumePtr->FvLength;
-
-  //
-  // Loop through the FFS files in the Boot Firmware Volume
-  //
-  for (EndOfFile = CurrentAddress + BootFirmwareVolumePtr->HeaderLength; ; ) {
-
-CurrentAddress = (EndOfFile + 7) & 0xfff8ULL;
-if (CurrentAddress > EndOfFirmwareVolume) {
-  return EFI_NOT_FOUND;
-}
-
-File = (EFI_FFS_FILE_HEADER*)(UINTN) CurrentAddress;
-if (IS_FFS_FILE2 (File)) {
-  Size = FFS_FILE2_SIZE (File);
-  if (Size <= 0x00FF) {
-return EFI_NOT_FOUND;
-  }
-} else {
-  Size = FFS_FILE_SIZE (File);
-  if (Size < sizeof (EFI_FFS_FILE_HEADER)) {
-return EFI_NOT_FOUND;
-  }
-}
-
-EndOfFile = CurrentAddress + 

Re: [edk2] [PATCH] ArmPkg/ArmDmaLib: assert that consistent mappings are uncached

[Ryan Harkin]

On 20 April 2016 at 09:40, Ard Biesheuvel  wrote:
> DmaMap () only allows uncached mappings to be used for creating consistent
> mappings with operation type MapOperationBusMasterCommonBuffer. However,
> if the buffer passed to DmaMap () happens to be aligned to the CWG, there
> is no need for a bounce buffer, and we perform the cache maintenance
> directly without ever checking if the memory attributes of the buffer
> adhere to the API.
>
> So add some debug code that asserts that the operation type and the memory
> attributes are consistent.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
Tested-by: Ryan Harkin 

Tested on TC2, Juno R0, R1 and R2, FVP Base AEMv8 and FVP Foundation models.

On FVP models, I just checked that they still booted.  On my Juno
boards, I checked that the USB disks were still readable and that I
could read my SATA drive.  On TC2, I made sure I could read an MMC
card; there is USB support on TC2.  I'm not even sure if the MMC card
driver uses DMA, but I thought I'd give it a spin anyway.


> ---
>
> This applies on top of the ArmDmaLib patches that I sent out yesterday.
> Based on the original code which contained some dodgy looking workarounds,
> I expect this to actually break something, but Olivier is not around anymore
> to tell me what he was trying to fix (and the broken 'promote cache
> mainantenance by VA to set/way above a certain threshold' may well have been
> the culprit here)
>
> Since the PCI emulation for non-coherent devices relies on this DMA code,
> any non-coherent real platform that uses the SATA or EHCI/UCHI code could
> be used to test this, but I don't have access to any of those. Suggestions
> are welcome, as are donations of a Juno or a TC2.
>
> Thanks.
>
>  ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 17 +
>  1 file changed, 17 insertions(+)
>
> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c 
> b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> index 83f4d38a8a60..329756efc268 100644
> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> @@ -135,6 +135,23 @@ DmaMap (
>} else {
>  Map->DoubleBuffer  = FALSE;
>
> +DEBUG_CODE_BEGIN ();
> +
> +//
> +// The operation type check above only executes if the buffer happens to 
> be
> +// misaligned with respect to CWG, but even if it is aligned, we should 
> not
> +// allow arbitrary buffers to be used for creating consistent mappings.
> +// So duplicate the check here when running in DEBUG mode, just to assert
> +// that we are not trying to create a consistent mapping for cached 
> memory.
> +//
> +Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, );
> +ASSERT_EFI_ERROR(Status);
> +
> +ASSERT (Operation != MapOperationBusMasterCommonBuffer ||
> +(GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) == 
> 0);
> +
> +DEBUG_CODE_END ();
> +
>  // Flush the Data Cache (should not have any effect if the memory region 
> is uncached)
>  gCpu->FlushDataCache (gCpu, *DeviceAddress, *NumberOfBytes, 
> EfiCpuFlushTypeWriteBackInvalidate);
>}
> --
> 2.5.0
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/3] Use new BDS and UiApp for OvmfPkg

[David Woodhouse]

On Tue, 2016-05-03 at 12:37 +0200, Laszlo Ersek wrote:
> 
> However, CSM is apparently generally borked at the moment (see
> ), and I don't think I can
> spend time on analyzing and fixing that.
> 
> I guess this is not good news, but I thought it better to report than to
> drop the item silently.

Thank you for the explicit prod. Once I've sorted out the new
regressions in building against newly-opaque structures in OpenSSL
HEAD, I'll do a 'git bisect' and see when this broke.

I just hope the history is preserved accurately enough to point the
finger correctly, and that there *is* a point in the (apparent) history
during which it actually worked... :)

-- 
dwmw2



smime.p7s
Description: S/MIME cryptographic signature
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch 0/3] Use new BDS and UiApp for OvmfPkg

[Laszlo Ersek]

On 08/04/15 13:07, Laszlo Ersek wrote:
> On 08/04/15 12:42, David Woodhouse wrote:
>> On Tue, 2015-08-04 at 03:16 +, Ni, Ruiyu wrote:
>>> I forgot to emphasize that I only tested the QEMU boot timeout 
>>> feature after changing the QemuBootOrderLib. I don't know how to test 
>>> the boot order feature.
>>> For the SeaBIOS CSM, can you tell me how to test or test it for me if 
>>> it's very quick?
>>
>> It's been a quite since I've done it; Laszlo probably has more up-to
>> -date instructions and a prepackaged SeaBIOS .config file (perhaps in
>> the RPM packages).
>>
>> But basically you first build the SeaBIOS CSM image as described at
>> http://www.seabios.org/Build_overview#Build_as_a_UEFI_Compatibility_Support_Module_.28CSM.29
>> and then drop it into OvmfPkg/Csm/Csm16/Csm16.bin and build with 
>> -DCSM_ENABLE.
>>
>> Then (if the BDS gets it right) you should be able to boot legacy
>> targets as well as native UEFI targets.
> 
> Okay, if you don't have your guests any longer, I guess I can add this
> to the end of my queue too.

I'm de-queueing this item for myself:

Ray's BDS work (for ) is
nearing completion (his v5 series at
 is fully
reviewed, and the test results are good), so the next item for me would
be to try to look into this.

However, CSM is apparently generally borked at the moment (see
), and I don't think I can
spend time on analyzing and fixing that.

I guess this is not good news, but I thought it better to report than to
drop the item silently.

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

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Tuesday, May 03, 2016 1:15 PM
To: Shaveta Leekha 
Cc: edk2-devel@lists.01.org 
Subject: Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to 
open correct one?

On 05/03/16 08:54, Shaveta Leekha wrote:
> Hi,
> 
> I have a scenario where two separate drivers are installing/producing "PCI 
> IO" protocol with same GUID (gEfiPciIoProtocolGuid).
> 
> Where:
> 
> (1)One of the PCI Io protocol instance is produced by 
> "MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like 
> E1000/NIC card to use)
> 
> (2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
> created for USB and SATA kind of controllers. This protocol (APIs) is 
> intended to be used by USB and SATA controller drivers.
> 
> Now when I use "OpenProtocol" in my "Platform specific Sata controller 
> driver" using:
> 
> Status = gBS->OpenProtocol (
>   Controller,
>   ,
>   (VOID **) ,
>   This->DriverBindingHandle,
>   Controller,
>   EFI_OPEN_PROTOCOL_GET_PROTOCOL
>   );
> 
> How can I make sure that PCI Io protocol that is produced by "PciEmulation" 
> layer driver is getting opened?
> 
> As "gEfiPciIoProtocolGuid" is same for both protocol instances.
> 
> How to handle such scenarios?

The UEFI spec forbids installing more than one protocol interface with the same 
GUID on the same handle. See
EFI_BOOT_SERVICES.InstallProtocolInterface():

  [... The same GUID cannot be installed more than once onto the same
  handle. If installation of a duplicate GUID on a handle is attempted,
  an EFI_INVALID_PARAMETER will result. [...]

  EFI_INVALID_PARAMETER  Protocol is already installed on the handle
 specified by Handle.

If you have two separate UEFI drivers that install PciIo interfaces on handles, 
then their EFI_DRIVER_BINDING_PROTOCOL.Supported() methods may report success 
only on distinct sets of handles. There must be no overlap.

But, in any case, I think it's very unusual to have two drivers that produce 
PciIo interfaces. According to the "Driver Writer's Guide for UEFI 2.3.1" 
(03/08/2012, Version 1.01), section "18.2 PCI Bus Drivers":

  TIP:  PCI Bus Driver customizations are *strongly discouraged*
because the PCI Bus Driver is designed to be conformant with
the /PCI Specification/. Instead, focus platform specific
customizations on the Root Bridge Driver that produced
EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL and its PCI Device Drivers.


[Shaveta] 
Yes, it is un-usual and we had discussed over it also.

So the scenario is:

On LS2080 platform, we have real PCI controllers that have E1000 card on 
them(used for networking purposes) And USB and SATA controllers are on-board.
So to provide SATA and USB, a PCIe emulation layer, a PCI IO protocol is 
installed in PciEmulation code.
PciEmulation code is very thin layer that simply produce PciRootBridgeIo 
protocol and hence PCI IO protocol,
Hence bypass the need of "real Pci Bus" code.

Reference is taken from ArmJune platform code in EDK2. In it, it install :
(1) PCI IO protocol using "gEfiPciIoProtocolGuid" in 
ArmPlatformPkg/ArmJunoPkg/Drivers/ArmJunoDxe/PciEmulation.c
(2) It also install " gEfiPciRootBridgeIoProtocolGuid" protocol, using which 
"PciBusDxe" will produce PciIo protocol

The same we are doing for our platform.
For real PCI controller, we have " installed Root Bridge PCI IO protocol with 
gEfiPciRootBridgeIoProtocolGuid"
And for emulation layer, the code direcly install PCI IO protocol with 
gEfiPciIoProtocolGuid"

Regards,
Shaveta

Laszlo

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

Re: [edk2] [Patch v5 23/24] OvmfPkg: Use MdeModulePkg/BDS

[Laszlo Ersek]

Ray,

On 05/03/16 07:35, Ruiyu Ni wrote:
> By default the new MdeModulePkg/BDS is used.
> If USE_OLD_BDS is defined to TRUE, IntelFrameworkModulePkg/BDS
> is used.
> 
> Fixes: https://github.com/tianocore/edk2/issues/62
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Reviewed-by: Laszlo Ersek 
> ---
>  OvmfPkg/OvmfPkgIa32.dsc| 41 +
>  OvmfPkg/OvmfPkgIa32.fdf|  5 +
>  OvmfPkg/OvmfPkgIa32X64.dsc | 41 +
>  OvmfPkg/OvmfPkgIa32X64.fdf |  5 +
>  OvmfPkg/OvmfPkgX64.dsc | 41 +
>  OvmfPkg/OvmfPkgX64.fdf |  5 +
>  6 files changed, 126 insertions(+), 12 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 0206dda..ad3e336 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -37,6 +37,7 @@ [Defines]
>DEFINE NETWORK_IP6_ENABLE  = FALSE
>DEFINE HTTP_BOOT_ENABLE= FALSE
>DEFINE SMM_REQUIRE = FALSE
> +  DEFINE USE_OLD_BDS = FALSE
>  
>  [BuildOptions]
>GCC:*_UNIXGCC_*_CC_FLAGS = -DMDEPKG_NDEBUG
> @@ -75,7 +76,13 @@ [LibraryClasses]
>
> UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
>
> UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
>HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
> +  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
> +!if $(USE_OLD_BDS) == TRUE
>
> GenericBdsLib|IntelFrameworkModulePkg/Library/GenericBdsLib/GenericBdsLib.inf
> +!else
> +  
> UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
> +!endif
> +  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
>CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
>DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>
> DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
> @@ -275,7 +282,13 @@ [LibraryClasses.common.DXE_DRIVER]
>IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
>UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
>DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
> +!if $(USE_OLD_BDS) == TRUE
>PlatformBdsLib|OvmfPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
> +  QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> +!else
> +  
> PlatformBootManagerLib|OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +  QemuBootOrderLib|OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.inf
> +!endif
>
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>  !if $(SMM_REQUIRE) == TRUE
>LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
> @@ -285,8 +298,6 @@ [LibraryClasses.common.DXE_DRIVER]
>  !ifdef $(SOURCE_DEBUG_ENABLE)
>DebugAgentLib|SourceLevelDebugPkg/Library/DebugAgent/DxeDebugAgentLib.inf
>  !endif
> -  QemuBootOrderLib|OvmfPkg/Library/QemuBootOrderLib/QemuBootOrderLib.inf
> -  FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
>PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
> @@ -294,6 +305,7 @@ [LibraryClasses.common.UEFI_APPLICATION]
>TimerLib|OvmfPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> +  
> ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>  !ifdef $(DEBUG_ON_SERIAL_PORT)
>DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
>  !else
> @@ -428,6 +440,9 @@ [PcdsFixedAtBuild]
># IRQs 5, 9, 10, 11 are level-triggered
>gPcAtChipsetPkgTokenSpaceGuid.Pcd8259LegacyModeEdgeLevel|0x0E20
>  
> +  # Point to the MdeModulePkg/Application/UiApp/UiApp.inf
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 
> 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> +
>  
> 
>  #
>  # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this 
> Platform
> @@ -551,13 +566,32 @@ [Components]
>PcAtChipsetPkg/KbcResetDxe/Reset.inf
>MdeModulePkg/Universal/Metronome/Metronome.inf
>PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
> +!if $(USE_OLD_BDS) == TRUE
>IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf {
>  
>  !ifdef $(CSM_ENABLE)
>NULL|OvmfPkg/Csm/CsmSupportLib/CsmSupportLib.inf
> +  !endif

You forgot to un-indent this !endif directive. Please fix it up before
pushing this patch.

Thanks
Laszlo

> +  }
> +!else
> +  

Re: [edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

Please find the replies and doubts in-lined.

Regards,
Shaveta

-Original Message-
From: Tian, Feng [mailto:feng.t...@intel.com] 
Sent: Tuesday, May 03, 2016 1:02 PM
To: Shaveta Leekha ; edk2-devel@lists.01.org
Cc: Tian, Feng 
Subject: RE: Two PCI IO protocols getting produced by same GUID, how to open 
correct one?

In your driver binding supported() of "Platform specific Sata controller 
driver", check the class code (PCI config space offset 09/0a/0bh) after getting 
the PciIo instance to see if it's the device you want to manage. If not, then 
it's not you want

[Shaveta] Doing the same, the moment I OpenProtocol, I read class code to check 
for the device(In my SATA controller driver).
But the issue is that, if some real device on PCI like NIC card open this PCI 
IO(Emulated protocol instance)?


PS: which platform are you running? Why your platform could have Pci bus and 
non-pci sata/usb host controller co-exist at the same time?

[Shaveta]
Yes on LS2080 platform, we have real PCI controllers that have E1000 card on 
them(used for networking purposes)
And USB and SATA controllers are on-board.
So to provide SATA and USB, a PCIe emulation layer, a PCI IO protocol is 
installed in PciEmulation code.


Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Shaveta 
Leekha
Sent: Tuesday, May 3, 2016 2:55 PM
To: edk2-devel@lists.01.org
Subject: [edk2] Two PCI IO protocols getting produced by same GUID, how to open 
correct one?

Hi,

I have a scenario where two separate drivers are installing/producing "PCI IO" 
protocol with same GUID (gEfiPciIoProtocolGuid).

Where:

(1)One of the PCI Io protocol instance is produced by 
"MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like 
E1000/NIC card to use)

(2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
created for USB and SATA kind of controllers. This protocol (APIs) is intended 
to be used by USB and SATA controller drivers.

Now when I use "OpenProtocol" in my "Platform specific Sata controller driver" 
using:

Status = gBS->OpenProtocol (
  Controller,
  ,
  (VOID **) ,
  This->DriverBindingHandle,
  Controller,
  EFI_OPEN_PROTOCOL_GET_PROTOCOL
  );

How can I make sure that PCI Io protocol that is produced by "PciEmulation" 
layer driver is getting opened?

As "gEfiPciIoProtocolGuid" is same for both protocol instances.

How to handle such scenarios?

Thanks and Regards,
Shaveta
___
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 v5 12/24] OvmfPkg/PlatformBootManagerLib: Register boot options and hot keys

[Laszlo Ersek]

Hi Ray,

On 05/03/16 07:35, Ruiyu Ni wrote:
> The patch registers "Enter" key as the continue key (hot key to skip
> the boot timeout wait), maps "F2" key to UI, and registers Shell
> boot option.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 108 
> +
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |   1 +
>  .../PlatformBootManagerLib.inf |   2 +
>  3 files changed, 111 insertions(+)

I consider this patch done (I also tested it), so:

Reviewed-by: Laszlo Ersek 
Tested-by: Laszlo Ersek 

I have two comments:

> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 3432e02..1b8f484 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -88,6 +88,112 @@ InstallDevicePathCallback (
>VOID
>);
>  
> +VOID
> +PlatformRegisterFvBootOption (
> +  EFI_GUID *FileGuid,
> +  CHAR16   *Description,
> +  UINT32   Attributes
> +  )
> +{
> +  EFI_STATUSStatus;
> +  INTN  OptionIndex;
> +  EFI_BOOT_MANAGER_LOAD_OPTION  NewOption;
> +  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
> +  UINTN BootOptionCount;
> +  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH FileNode;
> +  EFI_LOADED_IMAGE_PROTOCOL *LoadedImage;
> +  EFI_DEVICE_PATH_PROTOCOL  *DevicePath;
> +
> +  Status = gBS->HandleProtocol (
> +  gImageHandle,
> +  ,
> +  (VOID **) 
> +  );
> +  ASSERT_EFI_ERROR (Status);
> +
> +  EfiInitializeFwVolDevicepathNode (, FileGuid);
> +  DevicePath = DevicePathFromHandle (LoadedImage->DeviceHandle);
> +  ASSERT (DevicePath != NULL);
> +  DevicePath = AppendDevicePathNode (
> + DevicePath,
> + (EFI_DEVICE_PATH_PROTOCOL *) 
> + );
> +  ASSERT (DevicePath != NULL);
> +
> +  Status = EfiBootManagerInitializeLoadOption (
> + ,
> + LoadOptionNumberUnassigned,
> + LoadOptionTypeBoot,
> + Attributes,
> + Description,
> + DevicePath,
> + NULL,
> + 0
> + );
> +  FreePool (DevicePath);
> +
> +  if (!EFI_ERROR (Status)) {

(1) In the v3 review, I asked that an EFI_D_WARN message be logged, if
the EfiBootManagerInitializeLoadOption() function fails. You either
didn't like that request, or missed it -- it's not a problem. If you
missed it, you can still add it; up to you.

(2) My other comment is for UefiBootManagerLib:

> +BootOptions = EfiBootManagerGetLoadOptions (
> +, LoadOptionTypeBoot
> +);
> +
> +OptionIndex = EfiBootManagerFindLoadOption (
> +, BootOptions, BootOptionCount
> +);

The EfiBootManagerFindLoadOption() function requires the Description
fields to match as well, in order for a full match. I believe that it is
not the best idea.

First, the Description field is purely cosmetic, it doesn't affect behavior.

Second, if you have an installed OVMF virtual machine, from an OVMF
build that used the IntelFrameworkModulePkg BDS, one of your boot
options is likely the internal UEFI shell, called "EFI Internal Shell".
This patch ensures that the UEFI shell option exists (which is good),
and also makes an effort to avoid duplicate shell options (also good).

However, the Description that this patch associates with the shell
option is "UEFI Shell". Because EfiBootManagerFindLoadOption() requires
the descriptions to match as well, we end up with two options for the
exact same shell: one called "EFI Internal Shell", another called "UEFI
Shell".

So, as second comment, I suggest (for later) that
EfiBootManagerFindLoadOption() ignore the Description fields when
comparing boot options. *Or else*:

> +
> +if (OptionIndex == -1) {
> +  Status = EfiBootManagerAddLoadOptionVariable (, MAX_UINTN);
> +  ASSERT_EFI_ERROR (Status);
> +}
> +EfiBootManagerFreeLoadOption ();
> +EfiBootManagerFreeLoadOptions (BootOptions, BootOptionCount);
> +  }
> +}
> +
> +VOID
> +PlatformRegisterOptionsAndKeys (
> +  VOID
> +  )
> +{
> +  EFI_STATUS   Status;
> +  EFI_INPUT_KEYEnter;
> +  EFI_INPUT_KEYF2;
> +  EFI_INPUT_KEYEsc;
> +  EFI_BOOT_MANAGER_LOAD_OPTION BootOption;
> +
> +  //
> +  // Register ENTER as CONTINUE key
> +  //
> +  Enter.ScanCode= SCAN_NULL;
> +  Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
> +  Status = EfiBootManagerRegisterContinueKeyOption 

Re: [edk2] [PATCH 1/5] MdePkg: Revert AuditMode/DeployedMode name definition

[Fu, Siyuan]

Series reviewed-by: Fu Siyuan 



> -Original Message-
> From: Zhang, Chao B
> Sent: Thursday, April 28, 2016 3:46 PM
> To: edk2-devel@lists.01.org
> Cc: Fu, Siyuan ; Zeng, Star ;
> Zhang, Chao B 
> Subject: [PATCH 1/5] MdePkg: Revert AuditMode/DeployedMode name
> definition
> 
> Revert AuditMode/DeployedMode definition from Customized Secure Boot
> feature defined in UEFI2.5 Mantis 1263.
> The feature has been moved to
>   https://github.com/tianocore/edk2-staging/tree/Customized-Secure-Boot
> Previous check-in hash is
>   SHA-1: 79e7b6472797f156d1ff28f3022b25d9c6f250f9
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chao Zhang 
> ---
>  MdePkg/Include/Guid/GlobalVariable.h  | 16 +---
>  MdePkg/Include/Guid/ImageAuthentication.h | 15 ++-
>  2 files changed, 3 insertions(+), 28 deletions(-)
> 
> diff --git a/MdePkg/Include/Guid/GlobalVariable.h
> b/MdePkg/Include/Guid/GlobalVariable.h
> index e58f7a1..0804236 100644
> --- a/MdePkg/Include/Guid/GlobalVariable.h
> +++ b/MdePkg/Include/Guid/GlobalVariable.h
> @@ -1,7 +1,7 @@
>  /** @file
>GUID for EFI (NVRAM) Variables.
> 
> -  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2006 - 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
> @@ -126,20 +126,6 @@ extern EFI_GUID gEfiGlobalVariableGuid;
>  ///
>  #define EFI_SETUP_MODE_NAME L"SetupMode"
>  ///
> -/// Whether the system is operating in audit mode (1) or not (0).
> -/// All other values are reserved. Should be treated as read-only except when
> DeployedMode is 0.
> -/// Always becomes read-only after ExitBootServices() is called.
> -/// Its attribute is BS+RT.
> -///
> -#define EFI_AUDIT_MODE_NAME L"AuditMode"
> -///
> -/// Whether the system is operating in deployed mode (1) or not (0).
> -/// All other values are reserved. Should be treated as read-only when its
> value is 1.
> -/// Always becomes read-only after ExitBootServices() is called.
> -/// Its attribute is BS+RT.
> -///
> -#define EFI_DEPLOYED_MODE_NAME  L"DeployedMode"
> -///
>  /// The Key Exchange Key Signature Database.
>  /// Its attribute is NV+BS+RT+AT.
>  ///
> diff --git a/MdePkg/Include/Guid/ImageAuthentication.h
> b/MdePkg/Include/Guid/ImageAuthentication.h
> index c733643..3b3b7b5 100644
> --- a/MdePkg/Include/Guid/ImageAuthentication.h
> +++ b/MdePkg/Include/Guid/ImageAuthentication.h
> @@ -1,7 +1,7 @@
>  /** @file
>Image signature database are defined for the signed image validation.
> 
> -  Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.
> +  Copyright (c) 2009 - 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
> @@ -43,20 +43,9 @@
> 
>  #define SECURE_BOOT_MODE_ENABLE   1
>  #define SECURE_BOOT_MODE_DISABLE  0
> -///
> -/// Depricated value definition for SetupMode variable
> -///
> +
>  #define SETUP_MODE1
>  #define USER_MODE 0
> -///
> -/// Value definition for SetupMode/DeployedMode/AuditMode variable
> -///
> -#define SETUP_MODE_ENABLE 1
> -#define SETUP_MODE_DISABLE0
> -#define DEPLOYED_MODE_ENABLE  1
> -#define DEPLOYED_MODE_DISABLE 0
> -#define AUDIT_MODE_ENABLE 1
> -#define AUDIT_MODE_DISABLE0
> 
> 
> //**
> *
>  // Signature Database
> --
> 1.9.5.msysgit.1

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

Re: [edk2] [Patch v5 04/24] OvmfPkg/QemuNewBootOrderLib: Build with UefiBootManagerLib

[Laszlo Ersek]

I wanted to mention an opportunity for a micro-optimization:

On 05/03/16 11:16, Laszlo Ersek wrote:
> On 05/03/16 07:35, Ruiyu Ni wrote:
>> NOTE: SetBootOrderFromQemu() interface is not changed.
>> But when the old IntelFrameworkModulePkg/BDS is no longer used in
>> OVMF and ArmVirtPkg, additional patch will be submitted to change
>> this interface to remove parameter BootOptionList.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ruiyu Ni 
>> Cc: Jordan Justen 
>> Cc: Laszlo Ersek 
>> ---
>>  .../Library/QemuNewBootOrderLib/QemuBootOrderLib.c | 160 
>> ++---
>>  .../QemuNewBootOrderLib/QemuBootOrderLib.inf   |   4 +-
>>  2 files changed, 113 insertions(+), 51 deletions(-)
> 
> This looks fine and works fine as well.
> 
> Reviewed-by: Laszlo Ersek 
> Tested-by: Laszlo Ersek 
> 
> Thanks Ray!
> Laszlo
> 
>> diff --git a/OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c 
>> b/OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>> index 15065b7..cea3219 100644
>> --- a/OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>> +++ b/OvmfPkg/Library/QemuNewBootOrderLib/QemuBootOrderLib.c
>> @@ -2,7 +2,7 @@
>>Rewrite the BootOrder NvVar based on QEMU's "bootorder" fw_cfg file.
>>  
>>Copyright (C) 2012 - 2014, Red Hat, Inc.
>> -  Copyright (c) 2013, Intel Corporation. All rights reserved.
>> +  Copyright (c) 2013 - 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
>> @@ -16,7 +16,7 @@
>>  #include 
>>  #include 
>>  #include 
>> -#include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -253,8 +253,10 @@ typedef struct {
>>LOAD_OPTION_ACTIVE attribute.
>>  **/
>>  typedef struct {
>> -  CONST BDS_COMMON_OPTION *BootOption; // reference only, no ownership
>> -  BOOLEAN Appended;// has been added to a BOOT_ORDER?
>> +  CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOption; // reference only, no
>> +  //   ownership
>> +  BOOLEANAppended;// has been added to a
>> +  //   BOOT_ORDER?
>>  } ACTIVE_OPTION;
>>  
>>  
>> @@ -300,7 +302,7 @@ BootOrderAppend (
>>}
>>  
>>BootOrder->Data[BootOrder->Produced++] =
>> - 
>> ActiveOption->BootOption->BootCurrent;
>> +   (UINT16) 
>> ActiveOption->BootOption->OptionNumber;
>>ActiveOption->Appended = TRUE;
>>return RETURN_SUCCESS;
>>  }
>> @@ -308,22 +310,25 @@ BootOrderAppend (
>>  
>>  /**
>>  
>> -  Create an array of ACTIVE_OPTION elements for a boot option list.
>> +  Create an array of ACTIVE_OPTION elements for a boot option array.
>>  
>> -  @param[in]  BootOptionList  A boot option list, created with
>> -  BdsLibEnumerateAllBootOption().
>> +  @param[in]  BootOptions  A boot option array, created with
>> +   EfiBootManagerRefreshAllBootOption () and
>> +   EfiBootManagerGetLoadOptions ().
>>  
>> -  @param[out] ActiveOptionPointer to the first element in the new array.
>> -  The caller is responsible for freeing the 
>> array
>> -  with FreePool() after use.
>> +  @param[in]  BootOptionCount  The number of elements in BootOptions.
>>  
>> -  @param[out] Count   Number of elements in the new array.
>> +  @param[out] ActiveOption Pointer to the first element in the new 
>> array.
>> +   The caller is responsible for freeing the 
>> array
>> +   with FreePool() after use.
>> +
>> +  @param[out] CountNumber of elements in the new array.
>>  
>>  
>>@retval RETURN_SUCCESS   The ActiveOption array has been created.
>>  
>>@retval RETURN_NOT_FOUND No active entry has been found in
>> -   BootOptionList.
>> +   BootOptions.
>>  
>>@retval RETURN_OUT_OF_RESOURCES  Memory allocation failed.
>>  
>> @@ -331,11 +336,13 @@ BootOrderAppend (
>>  STATIC
>>  RETURN_STATUS
>>  CollectActiveOptions (
>> -  IN   CONST LIST_ENTRY *BootOptionList,
>> -  OUT  ACTIVE_OPTION**ActiveOption,
>> -  OUT  UINTN*Count
>> +  IN   CONST EFI_BOOT_MANAGER_LOAD_OPTION *BootOptions,
>> +  IN   UINTN  BootOptionCount,
>> +  OUT  ACTIVE_OPTION  **ActiveOption,
>> +  OUT  UINTN  *Count
>>)
>>  {
>> +  UINTN Index;
>>UINTN ScanMode;
>>  
>>*ActiveOption = NULL;
>> @@ -346,22 +353,15 @@ CollectActiveOptions (
>>// - 

[edk2] Two PCI IO protocols getting produced by same GUID, how to open correct one?

[Shaveta Leekha]

Hi,

I have a scenario where two separate drivers are installing/producing "PCI IO" 
protocol with same GUID (gEfiPciIoProtocolGuid).

Where:

(1)One of the PCI Io protocol instance is produced by 
"MdeModulePkg/Bus/Pci/PciBus" driver for real PCI devices to use( like 
E1000/NIC card to use)

(2)Other PCI Io protocol instance is produced by "Pci Emulation" layer 
created for USB and SATA kind of controllers. This protocol (APIs) is intended 
to be used by USB and SATA controller drivers.

Now when I use "OpenProtocol" in my "Platform specific Sata controller driver" 
using:

Status = gBS->OpenProtocol (
  Controller,
  ,
  (VOID **) ,
  This->DriverBindingHandle,
  Controller,
  EFI_OPEN_PROTOCOL_GET_PROTOCOL
  );

How can I make sure that PCI Io protocol that is produced by "PciEmulation" 
layer driver is getting opened?

As "gEfiPciIoProtocolGuid" is same for both protocol instances.

How to handle such scenarios?

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