Re: [edk2] testing Centos 7.1 boot from iscsi with EDK -> kernel crashes after launch by GRUB

[Laszlo Ersek]

On 11/13/15 12:22, P.A.M. van Dam (Pascal) wrote:
> On Thu, Nov 12, 2015 at 07:17:49PM +0100, Laszlo Ersek wrote:
> 
> Good afternoon all,
> 
>> Please don't drop the list and other CC's from the discussion when the
>> initial report is sent to the list. Keeping full context.
> 
> My apologies for that. I noticed it when I got your personal out-of-office 
> reply. :)
> 
> 
>>
>> On 11/11/15 08:38, P.A.M. van Dam (Pascal) wrote:
>>> On Mon, Nov 09, 2015 at 10:02:52PM +0100, Laszlo Ersek wrote:
 On 11/09/15 14:50, P.A.M. van Dam (Pascal) wrote:
>
> Good day all,
>
> I decided to do some testing with iSCSI boot from within a KVM. 
>
> My setup is a Centos 7.1 KVM host with several Fedora 22 and Centos 7.1 
> guests in it. The iSCSI storage has been configured
> from a LenovoEMC EPX6300 NAS.
>
> 1. I've configured the iSCSI bootlun to be available for the OS install.
> 2. Within the KVM guest OS install I assign the LUN to the Centos 7.1 OS.
> 3. The OS installs without errors ont he iSCSI LUN.
>
> After reboot of the freshly installed OS on the KVM
> I configure the iSCSI device in the OVMF user interface.
>
> 4. The iSCSI BOOT LUN is visible from the UEFI shell.
> 5. When booting from the LUN, GRUB correcly displays the bootmenu.

 Am I to understand that GRUB (and its config) are correctly loaded via
 iSCSI?
>>>
>>> Yep. You are correct in this. GRUB loads without issue and both the menu 
>>> and commandline work. I can even access the kernel and the initrd from 
>>> GRUB's commandline.
>>>

> 6. When selecting and starting a kernel/initrd pair the started kernel 
> quickly crashes reporting a fault.
>
> My questions;
>
> 1. Is iSCSI boot support in the OVMF/EDKII currrently usable?
>>>
>>>

 I never tested it, but I guess it should work (for some value of
 "work"). Since I never tested it, I can't say what code in OvmfPkg would
 require specific support (perhaps the boot order library... dunno).

> 2. Can I help in further analyzing the issue? It could be an issue in 
> EDKII (drivers?), in grub or in the Linux kernel.

 Your email is missing almost all of the important details, so let's
 start again. :)
>>>
>>> I will try to supply the info needed. This is my first time on this 
>>> mailinglist and it's long time since I've joined a Linux project.
>>>

 - Are you using libvirt? If so, what version? What is your domain XML?

>>>
>>> Yes, I am using libvirt. I am currently using 
>>> libvirt-1.2.8-16.el7_1.5.x86_64.
>>>
>>> [virtlabn17.xml]
>>>
>>> [root@kallista:38 qemu]# cat virtlabn17.xml
>>> 
>>>
>>> 
>>>   virtlabn17
>>>   ab2b120f-8a4f-4f90-bd32-fa78fedd51b0
>>>   Centos 7.1 UEFI/ISCSI BOOT virtlabn17
>>>   2097152
>>>   2097152
>>
>> Okay, so 2GB RAM.
>>
>>>   2
>>
>> 2 VCPUs
>>
>>>   
>>> hvm
>>> >> type='pflash'>/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd
>>> >> template='/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd'>/var/lib/libvirt/qemu/nvram/virtlabn17_VARS.fd
>>
>> Looks good. The OVMF binary is from Gerd.
> 
> Correct.
> 
>>
>>> 
>>
>> This is kinda useless here (you don't have a disk in your config). In
>> any case, as a side point, the  elements within device
>> elements are much more flexible than this. See
>> , and then search
>> that page for more occurrences of the string "boot order=". See also the
>> "boot" element's docs at
>> .
> 
> Ok. I will take a look at it. But as far as I know, it's not the cause of the 
> issue. :)
>>
>>> 
>>
>> This will drop you to the edk2 setup page; I guess it's fine.
> 
> Correct.
> 
>>
>>>   
>>>   
>>> 
>>> 
>>> 
>>>   
>>>   
>>> SandyBridge
>>>   
>>>   
>>> 
>>> 
>>> 
>>>   
>>>   destroy
>>>   restart
>>>   restart
>>>   
>>> /usr/libexec/qemu-kvm
>>> 
>>>   >> function='0x7'/>
>>> 
>>> 
>>>   
>>>   >> function='0x0' multifunction='on'/>
>>> 
>>> 
>>>   
>>>   >> function='0x1'/>
>>> 
>>> 
>>>   
>>>   >> function='0x2'/>
>>> 
>>> 
>>> 
>>>   >> function='0x0'/>
>>> 
>>> 
>>>   
>>>   
>>>   
>>>   >> function='0x0'/>
>>> 
>>
>> I see. This is the "Bridge to LAN" config
>> .
>>
>> You are using virtio-net, and the iPXE oprom will be present in the PCI
>> ROM BAR.
>>
>> Idea (1): not that it might matter a lot, but you could try with the
>> builtin virtio-net driver. For that, add the following element within
>> :
>>
>> 
>>
> 
> Yes. I implemented Idea(1) and it works! So there definately is something 
> with this ROM that cause the
> crash. (Looking further in the email, not the crash of the kernel, but 
> earlier in the process)
> 
> So; conclusion for 

[edk2] [Patch V2 07/12] MdeModulePkg: Add Platform recovery support

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Sunny Wang 
---
 MdeModulePkg/Include/Library/UefiBootManagerLib.h  |   1 +
 .../Library/UefiBootManagerLib/BmLoadOption.c  | 129 ++---
 .../Library/UefiBootManagerLib/InternalBm.h|   6 +-
 .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
 4 files changed, 120 insertions(+), 17 deletions(-)

diff --git a/MdeModulePkg/Include/Library/UefiBootManagerLib.h 
b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
index 54a6713..afb4271 100644
--- a/MdeModulePkg/Include/Library/UefiBootManagerLib.h
+++ b/MdeModulePkg/Include/Library/UefiBootManagerLib.h
@@ -31,6 +31,7 @@ typedef enum {
   LoadOptionTypeDriver,
   LoadOptionTypeSysPrep,
   LoadOptionTypeBoot,
+  LoadOptionTypePlatformRecovery,
   LoadOptionTypeMax
 } EFI_BOOT_MANAGER_LOAD_OPTION_TYPE;
 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 6fcd23b..999647c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -19,14 +19,16 @@ GLOBAL_REMOVE_IF_UNREFERENCED
   CHAR16 *mBmLoadOptionName[] = {
 L"Driver",
 L"SysPrep",
-L"Boot"
+L"Boot",
+L"PlatformRecovery"
   };
 
 GLOBAL_REMOVE_IF_UNREFERENCED
   CHAR16 *mBmLoadOptionOrderName[] = {
 EFI_DRIVER_ORDER_VARIABLE_NAME,
 EFI_SYS_PREP_ORDER_VARIABLE_NAME,
-EFI_BOOT_ORDER_VARIABLE_NAME
+EFI_BOOT_ORDER_VARIABLE_NAME,
+NULL  // PlatformRecovery doesn't have associated *Order variable
   };
 
 /**
@@ -154,8 +156,9 @@ BmGetFreeOptionNumber (
 }
 
 /**
-  Create the Boot, Driver, SysPrep, variable from the load option.
-  
+  Create the Boot, Driver, SysPrep, PlatformRecovery variable
+  from the load option.
+
   @param  LoadOption  Pointer to the load option.
 
   @retval EFI_SUCCESS The variable was created.
@@ -167,12 +170,14 @@ EfiBootManagerLoadOptionToVariable (
   IN CONST EFI_BOOT_MANAGER_LOAD_OPTION *Option
   )
 {
+  EFI_STATUS   Status;
   UINTNVariableSize;
   UINT8*Variable;
   UINT8*Ptr;
   CHAR16   OptionName[BM_OPTION_NAME_LEN];
   CHAR16   *Description;
   CHAR16   NullChar;
+  EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock;
   UINT32   VariableAttributes;
 
   if ((Option->OptionNumber == LoadOptionNumberUnassigned) ||
@@ -233,6 +238,17 @@ structure.
   UnicodeSPrint (OptionName, sizeof (OptionName), L"%s%04x", 
mBmLoadOptionName[Option->OptionType], Option->OptionNumber);
 
   VariableAttributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE;
+  if (Option->OptionType == LoadOptionTypePlatformRecovery) {
+//
+// Lock the PlatformRecovery
+//
+Status = gBS->LocateProtocol (, NULL, (VOID 
**) );
+if (!EFI_ERROR (Status)) {
+  Status = VariableLock->RequestToLock (VariableLock, OptionName, 
);
+  ASSERT_EFI_ERROR (Status);
+}
+VariableAttributes = EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS;
+  }
 
   return gRT->SetVariable (
 OptionName,
@@ -550,6 +566,7 @@ EfiBootManagerDeleteLoadOptionVariable (
   UINTN OptionOrderSize;
   EFI_STATUSStatus;
   UINTN Index;
+  CHAR16OptionName[BM_OPTION_NAME_LEN];
 
   if (((UINT32) OptionType >= LoadOptionTypeMax) || (OptionNumber >= 
LoadOptionNumberMax)) {
 return EFI_INVALID_PARAMETER;
@@ -581,6 +598,18 @@ EfiBootManagerDeleteLoadOptionVariable (
 if (OptionOrder != NULL) {
   FreePool (OptionOrder);
 }
+  } else if (OptionType == LoadOptionTypePlatformRecovery) {
+//
+// PlatformRecovery doesn't have assiciated PlatformRecoveryOrder, 
remove the PlatformRecovery itself.
+//
+UnicodeSPrint (OptionName, sizeof (OptionName), L"%s%04x", 
mBmLoadOptionName[OptionType], OptionNumber);
+Status = gRT->SetVariable (
+OptionName,
+,
+0,
+0,
+NULL
+);
   }
 
   return Status;
@@ -677,7 +706,8 @@ BmStrSizeEx (
 }
 
 /**
-  Validate the Boot, Driver, SysPrep variable (VendorGuid/Name)
+  Validate the Boot, Driver, SysPrep and PlatformRecovery
+  variable (VendorGuid/Name)
 
   @param  Variable  The variable data.
   @param  VariableSize  The variable size.
@@ -920,6 +950,62 @@ EfiBootManagerVariableToLoadOption (
   return EfiBootManagerVariableToLoadOptionEx (VariableName, 
, Option);
 }
 
+typedef struct {
+  

[edk2] [Patch V2 09/12] MdeModulePkg: Use UefiSpec.h defined macro to replace L"xxx" string

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Sunny Wang 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index c889892..4154d41 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -672,7 +672,7 @@ BdsFormalizeOSIndicationVariable (
   //
   OsIndicationSupport = EFI_OS_INDICATIONS_BOOT_TO_FW_UI;
   Status = gRT->SetVariable (
-  L"OsIndicationsSupported",
+  EFI_OS_INDICATIONS_SUPPORT_VARIABLE_NAME,
   ,
   EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS,
   sizeof(UINT64),
@@ -694,7 +694,7 @@ BdsFormalizeOSIndicationVariable (
   Attributes = 0;
   DataSize = sizeof(UINT64);
   Status = gRT->GetVariable (
-  L"OsIndications",
+  EFI_OS_INDICATIONS_VARIABLE_NAME,
   ,
   ,
   ,
@@ -711,7 +711,7 @@ BdsFormalizeOSIndicationVariable (
 
 DEBUG ((EFI_D_ERROR, "[Bds] Unformalized OsIndications variable exists. 
Delete it\n"));
 Status = gRT->SetVariable (
-L"OsIndications",
+EFI_OS_INDICATIONS_VARIABLE_NAME,
 ,
 0,
 0,
@@ -737,9 +737,9 @@ BdsFormalizeEfiGlobalVariable (
   //
   // Validate Console variable.
   //
-  BdsFormalizeConsoleVariable (L"ConIn");
-  BdsFormalizeConsoleVariable (L"ConOut");
-  BdsFormalizeConsoleVariable (L"ErrOut");
+  BdsFormalizeConsoleVariable (EFI_CON_IN_VARIABLE_NAME);
+  BdsFormalizeConsoleVariable (EFI_CON_OUT_VARIABLE_NAME);
+  BdsFormalizeConsoleVariable (EFI_ERR_OUT_VARIABLE_NAME);
 
   //
   // Validate OSIndication related variable.
@@ -1045,7 +1045,7 @@ BdsEntry (
   //
   DataSize = sizeof (UINT64);
   Status = gRT->GetVariable (
-  L"OsIndications",
+  EFI_OS_INDICATIONS_VARIABLE_NAME,
   ,
   NULL,
   ,
@@ -1062,7 +1062,7 @@ BdsEntry (
   if (BootFwUi) {
 OsIndication &= ~((UINT64) EFI_OS_INDICATIONS_BOOT_TO_FW_UI);
 Status = gRT->SetVariable (
-   L"OsIndications",
+   EFI_OS_INDICATIONS_VARIABLE_NAME,
,
EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | 
EFI_VARIABLE_NON_VOLATILE,
sizeof(UINT64),
-- 
1.9.5.msysgit.1

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

[edk2] [Patch V2 10/12] MdeModulePkg: Add PlatformRecovery#### pointing to default file path

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Sunny Wang 
---
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 38 +---
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 4154d41..b632ada 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -823,11 +823,12 @@ BdsEntry (
   UINT16  BootTimeOut;
   EDKII_VARIABLE_LOCK_PROTOCOL*VariableLock;
   UINTN   Index;
-  EFI_BOOT_MANAGER_LOAD_OPTIONBootOption;
+  EFI_BOOT_MANAGER_LOAD_OPTIONLoadOption;
   UINT16  *BootNext;
   CHAR16  BootNextVariableName[sizeof ("Boot")];
   EFI_BOOT_MANAGER_LOAD_OPTIONBootManagerMenu;
   BOOLEAN BootFwUi;
+  EFI_DEVICE_PATH_PROTOCOL*FilePath;
 
   HotkeyTriggered = NULL;
   Status  = EFI_SUCCESS;
@@ -943,6 +944,27 @@ BdsEntry (
   InitializeLanguage (TRUE);
 
   //
+  // System firmware must include a PlatformRecovery variable specifying
+  // a short-form File Path Media Device Path containing the platform default
+  // file path for removable media
+  //
+  FilePath = FileDevicePath (NULL, EFI_REMOVABLE_MEDIA_FILE_NAME);
+  Status = EfiBootManagerInitializeLoadOption (
+ ,
+ 0,
+ LoadOptionTypePlatformRecovery,
+ LOAD_OPTION_ACTIVE,
+ L"Default PlatformRecovery",
+ FilePath,
+ NULL,
+ 0
+ );
+  ASSERT_EFI_ERROR (Status);
+  EfiBootManagerLoadOptionToVariable ();
+  EfiBootManagerFreeLoadOption ();
+  FreePool (FilePath);
+
+  //
   // Report Status Code to indicate connecting drivers will happen
   //
   REPORT_STATUS_CODE (
@@ -1120,17 +1142,17 @@ BdsEntry (
   //
   if (BootNext != NULL) {
 UnicodeSPrint (BootNextVariableName, sizeof (BootNextVariableName), 
L"Boot%04x", *BootNext);
-Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, 
);
+Status = EfiBootManagerVariableToLoadOption (BootNextVariableName, 
);
 if (!EFI_ERROR (Status)) {
-  EfiBootManagerBoot ();
-  EfiBootManagerFreeLoadOption ();
-  if (BootOption.Status == EFI_SUCCESS) {
+  EfiBootManagerBoot ();
+  EfiBootManagerFreeLoadOption ();
+  if (LoadOption.Status == EFI_SUCCESS) {
 //
 // Boot to Boot Manager Menu upon EFI_SUCCESS
 //
-EfiBootManagerGetBootManagerMenu ();
-EfiBootManagerBoot ();
-EfiBootManagerFreeLoadOption ();
+EfiBootManagerGetBootManagerMenu ();
+EfiBootManagerBoot ();
+EfiBootManagerFreeLoadOption ();
   }
 }
   }
-- 
1.9.5.msysgit.1

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

[edk2] [Patch V2 11/12] MdeModulePkg: Enable PlatformRecovery in BdsDxe driver

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Sunny Wang 
---
 MdeModulePkg/Universal/BdsDxe/Bds.h  |   3 -
 MdeModulePkg/Universal/BdsDxe/BdsEntry.c | 406 ---
 2 files changed, 95 insertions(+), 314 deletions(-)

diff --git a/MdeModulePkg/Universal/BdsDxe/Bds.h 
b/MdeModulePkg/Universal/BdsDxe/Bds.h
index 2171d14..ac54482 100644
--- a/MdeModulePkg/Universal/BdsDxe/Bds.h
+++ b/MdeModulePkg/Universal/BdsDxe/Bds.h
@@ -24,9 +24,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
-#include 
-#include 
-#include 
 
 #include 
 #include 
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c 
b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index b632ada..f06684c 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -51,10 +51,10 @@ CHAR16  *mReadOnlyVariables[] = {
 CHAR16 *mBdsLoadOptionName[] = {
   L"Driver",
   L"SysPrep",
-  L"Boot"
+  L"Boot",
+  L"PlatformRecovery"
 };
 
-CHAR16  mRecoveryBoot[] = L"Recovery Boot";
 /**
   Event to Connect ConIn.
 
@@ -122,187 +122,6 @@ BdsInitialize (
 }
 
 /**
-  Emuerate all possible bootable medias in the following order:
-  1. Removable BlockIo- The boot option only points to the 
removable media
-device, like USB key, DVD, Floppy etc.
-  2. Fixed BlockIo- The boot option only points to a Fixed 
blockIo device,
-like HardDisk.
-  3. Non-BlockIo SimpleFileSystem - The boot option points to a device 
supporting
-SimpleFileSystem Protocol, but not 
supporting BlockIo
-protocol.
-  4. LoadFile - The boot option points to the media 
supporting 
-LoadFile protocol.
-  Reference: UEFI Spec chapter 3.3 Boot Option Variables Default Boot Behavior
-
-  @param BootOptionCount   Return the boot option count which has been found.
-
-  @retval   Pointer to the boot option array.
-**/
-EFI_BOOT_MANAGER_LOAD_OPTION *
-BdsEnumerateBootOptions (
-  UINTN *BootOptionCount
-  )
-{
-  EFI_STATUSStatus;
-  EFI_BOOT_MANAGER_LOAD_OPTION  *BootOptions;
-  UINTN HandleCount;
-  EFI_HANDLE*Handles;
-  EFI_BLOCK_IO_PROTOCOL *BlkIo;
-  UINTN Removable;
-  UINTN Index;
-
-  ASSERT (BootOptionCount != NULL);
-
-  *BootOptionCount = 0;
-  BootOptions  = NULL;
-
-  //
-  // Parse removable block io followed by fixed block io
-  //
-  gBS->LocateHandleBuffer (
- ByProtocol,
- ,
- NULL,
- ,
- 
- );
-
-  for (Removable = 0; Removable < 2; Removable++) {
-for (Index = 0; Index < HandleCount; Index++) {
-  Status = gBS->HandleProtocol (
-  Handles[Index],
-  ,
-  (VOID **) 
-  );
-  if (EFI_ERROR (Status)) {
-continue;
-  }
-
-  //
-  // Skip the logical partitions
-  //
-  if (BlkIo->Media->LogicalPartition) {
-continue;
-  }
-
-  //
-  // Skip the fixed block io then the removable block io
-  //
-  if (BlkIo->Media->RemovableMedia == ((Removable == 0) ? FALSE : TRUE)) {
-continue;
-  }
-
-  BootOptions = ReallocatePool (
-  sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * 
(*BootOptionCount),
-  sizeof (EFI_BOOT_MANAGER_LOAD_OPTION) * 
(*BootOptionCount + 1),
-  BootOptions
-  );
-  ASSERT (BootOptions != NULL);
-
-  Status = EfiBootManagerInitializeLoadOption (
- [(*BootOptionCount)++],
- LoadOptionNumberUnassigned,
- LoadOptionTypeBoot,
- LOAD_OPTION_ACTIVE,
- mRecoveryBoot,
- DevicePathFromHandle (Handles[Index]),
- NULL,
- 0
- );
-  ASSERT_EFI_ERROR (Status);
-}
-  }
-
-  if (HandleCount != 0) {
-FreePool (Handles);
-  }
-
-  //
-  // Parse simple file system not based on block io
-  //
-  gBS->LocateHandleBuffer (
- ByProtocol,
- ,
- NULL,
- ,
- 
- );
-  for (Index = 0; Index < HandleCount; Index++) {
-Status = gBS->HandleProtocol (
-Handles[Index],
-,
-(VOID **) 
-);
- if (!EFI_ERROR (Status)) {
-  //
-  //  Skip if the file system handle supports a BlkIo protocol, which 
we've handled in above
-  //
-  continue;
-}
-BootOptions = ReallocatePool (
-  

[edk2] [Patch V2 06/12] MdeModulePkg: Support to expand File device path

[Ruiyu Ni]

To support platform recovery, File device path expanding capability
is added.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Sunny Wang 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c | 76 
 1 file changed, 76 insertions(+)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
index aef2e7b..70ec216 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
@@ -1073,6 +1073,76 @@ BmExpandUsbDevicePath (
 }
 
 /**
+  Expand File-path device path node to be full device path in platform.
+
+  @param FilePath  The device path pointing to a load option.
+   It could be a short-form device path.
+  @param FullPath  Return the full device path of the load option after
+   short-form device path expanding.
+   Caller is responsible to free it.
+  @param FileSize  Return the load option size.
+
+  @return The load option buffer. Caller is responsible to free the memory.
+**/
+VOID *
+BmExpandFileDevicePath (
+  IN  EFI_DEVICE_PATH_PROTOCOL*FilePath,
+  OUT EFI_DEVICE_PATH_PROTOCOL**FullPath,
+  OUT UINTN   *FileSize
+  )
+{
+  EFI_STATUS  Status;
+  UINTN   Index;
+  UINTN   HandleCount;
+  EFI_HANDLE  *Handles;
+  EFI_BLOCK_IO_PROTOCOL   *BlockIo;
+  UINTN   MediaType;
+  EFI_DEVICE_PATH_PROTOCOL*FullDevicePath;
+  VOID*FileBuffer;
+  UINT32  AuthenticationStatus;
+  
+  EfiBootManagerConnectAll ();
+  Status = gBS->LocateHandleBuffer (ByProtocol, 
, NULL, , );
+  if (EFI_ERROR (Status)) {
+HandleCount = 0;
+Handles = NULL;
+  }
+
+  //
+  // Enumerate all removable media devices followed by all fixed media devices,
+  //   followed by media devices which don't layer on block io.
+  //
+  for (MediaType = 0; MediaType < 3; MediaType++) {
+for (Index = 0; Index < HandleCount; Index++) {
+  Status = gBS->HandleProtocol (Handles[Index], , 
(VOID *) );
+  if (EFI_ERROR (Status)) {
+BlockIo = NULL;
+  }
+  if ((MediaType == 0 && BlockIo != NULL && 
BlockIo->Media->RemovableMedia) ||
+  (MediaType == 1 && BlockIo != NULL && 
!BlockIo->Media->RemovableMedia) ||
+  (MediaType == 2 && BlockIo == NULL)
+  ) {
+FullDevicePath = AppendDevicePath (DevicePathFromHandle 
(Handles[Index]), FilePath);
+FileBuffer = GetFileBufferByFilePath (TRUE, FullDevicePath, FileSize, 
);
+if (FileBuffer != NULL) {
+  *FullPath = FullDevicePath;
+  FreePool (Handles);
+  return FileBuffer;
+}
+FreePool (FullDevicePath);
+  }
+}
+  }
+
+  if (Handles != NULL) {
+FreePool (Handles);
+  }
+
+  *FullPath = NULL;
+  return NULL;
+}
+
+/**
   Save the partition DevicePath to the CachedDevicePath as the first instance.
 
   @param CachedDevicePath  The device path cache.
@@ -1473,6 +1543,12 @@ BmGetLoadOptionBuffer (
 // Expand the Harddrive device path
 //
 return BmExpandPartitionDevicePath (FilePath, FullPath, FileSize);
+  } else if ((DevicePathType (FilePath) == MEDIA_DEVICE_PATH) &&
+ (DevicePathSubType (FilePath) == MEDIA_FILEPATH_DP)) {
+//
+// Expand the File-path device path
+//
+return BmExpandFileDevicePath (FilePath, FullPath, FileSize);
   } else {
 for (Node = FilePath; !IsDevicePathEnd (Node); Node = NextDevicePathNode 
(Node)) {
   if ((DevicePathType (Node) == MESSAGING_DEVICE_PATH) &&
-- 
1.9.5.msysgit.1

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

[edk2] [Patch V2 04/12] MdeModulePkg: Use BM_OPTION_NAME_LEN instead of sizeof L"Boot####"

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Reviewed-by: Sunny Wang 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index ff1cbe9..f902357 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -45,7 +45,7 @@ BmIsKeyOptionValid (
   IN EFI_BOOT_MANAGER_KEY_OPTION *KeyOption
 )
 {
-  UINT16   OptionName[sizeof (L"Boot")];
+  UINT16   OptionName[BM_OPTION_NAME_LEN];
   UINT8*BootOption;
   UINTNBootOptionSize;
   UINT32   Crc;
@@ -349,7 +349,7 @@ BmHotkeyCallback (
 {
   LIST_ENTRY*Link;
   BM_HOTKEY *Hotkey;
-  CHAR16OptionName[sizeof ("Boot")];
+  CHAR16OptionName[BM_OPTION_NAME_LEN];
   EFI_STATUSStatus;
   EFI_KEY_DATA  *HotkeyData;
 
@@ -905,13 +905,13 @@ EfiBootManagerAddKeyOptionVariable (
   VA_LISTArgs;
   VOID   *BootOption;
   UINTN  BootOptionSize;
-  CHAR16 BootOptionName[sizeof (L"Boot")];
+  CHAR16 BootOptionName[BM_OPTION_NAME_LEN];
   EFI_BOOT_MANAGER_KEY_OPTIONKeyOption;
   EFI_BOOT_MANAGER_KEY_OPTION*KeyOptions;
   UINTN  KeyOptionCount;
   UINTN  Index;
   UINTN  KeyOptionNumber;
-  CHAR16 KeyOptionName[sizeof (L"Key")];
+  CHAR16 KeyOptionName[sizeof ("Key")];
 
   UnicodeSPrint (BootOptionName, sizeof (BootOptionName), L"Boot%04x", 
BootOptionNumber);
   GetEfiGlobalVariable2 (BootOptionName, , );
@@ -1018,7 +1018,7 @@ EfiBootManagerDeleteKeyOptionVariable (
   BM_HOTKEY  *Hotkey;
   UINT32 ShiftState;
   BOOLEANMatch;
-  CHAR16 KeyOptionName[sizeof (L"Key")];
+  CHAR16 KeyOptionName[sizeof ("Key")];
 
   ZeroMem (, sizeof (EFI_BOOT_MANAGER_KEY_OPTION));
   VA_START (Args, Modifier);
-- 
1.9.5.msysgit.1

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

[edk2] [Patch V2 03/12] MdeModulePkg: Use BmCharToUint in BmIsKeyOptionVariable

[Ruiyu Ni]

The patch also moves the BmCharToUint to BmMisc.c because it
belongs to misc functions.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Reviewed-by: Sunny Wang 
---
 MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c | 10 -
 .../Library/UefiBootManagerLib/BmLoadOption.c  | 23 ---
 MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c   | 26 ++
 .../Library/UefiBootManagerLib/InternalBm.h| 13 +++
 4 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
index 8d398fb..ff1cbe9 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmHotkey.c
@@ -88,6 +88,7 @@ BmIsKeyOptionVariable (
   )
 {
   UINTN Index;
+  UINTN Uint;
   
   if (!CompareGuid (Guid, ) ||
   (StrSize (Name) != sizeof (L"Key")) ||
@@ -98,12 +99,11 @@ BmIsKeyOptionVariable (
 
   *OptionNumber = 0;
   for (Index = 3; Index < 7; Index++) {
-if ((Name[Index] >= L'0') && (Name[Index] <= L'9')) {
-  *OptionNumber = *OptionNumber * 16 + Name[Index] - L'0';
-} else if ((Name[Index] >= L'A') && (Name[Index] <= L'F')) {
-  *OptionNumber = *OptionNumber * 16 + Name[Index] - L'A' + 10;
-} else {
+Uint = BmCharToUint (Name[Index]);
+if (Uint == -1) {
   return FALSE;
+} else {
+  *OptionNumber = (UINT16) Uint + *OptionNumber * 0x10;
 }
   }
 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
index 07e6249..6fcd23b 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmLoadOption.c
@@ -587,29 +587,6 @@ EfiBootManagerDeleteLoadOptionVariable (
 }
 
 /**
-  Convert a single character to number.
-  It assumes the input Char is in the scope of L'0' ~ L'9' and L'A' ~ L'F'
-
-  @paramChar   The input char which need to convert to int.
-**/
-UINTN
-BmCharToUint (
-  IN CHAR16   Char
-  )
-{
-  if ((Char >= L'0') && (Char <= L'9')) {
-return (UINTN) (Char - L'0');
-  }
-
-  if ((Char >= L'A') && (Char <= L'F')) {
-return (UINTN) (Char - L'A' + 0xA);
-  }
-
-  ASSERT (FALSE);
-  return (UINTN) -1;
-}
-
-/**
   Returns the size of a device path in bytes.
 
   This function returns the size, in bytes, of the device path data structure 
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c 
b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
index 97d1a48..cc2032c 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmMisc.c
@@ -384,3 +384,29 @@ BmPrintDp (
 FreePool (Str);
   }
 }
+
+/**
+  Convert a single character to number.
+  It assumes the input Char is in the scope of L'0' ~ L'9' and L'A' ~ L'F'
+
+  @paramChar   The input char which need to convert to int.
+
+  @return  The converted 8-bit number or (UINTN) -1 if conversion failed.
+**/
+UINTN
+BmCharToUint (
+  IN CHAR16   Char
+  )
+{
+  if ((Char >= L'0') && (Char <= L'9')) {
+return (UINTN) (Char - L'0');
+  }
+
+  if ((Char >= L'A') && (Char <= L'F')) {
+return (UINTN) (Char - L'A' + 0xA);
+  }
+
+  ASSERT (FALSE);
+  return (UINTN) -1;
+}
+
diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
index 290ce3f..862811e 100644
--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
@@ -434,4 +434,17 @@ BmPrintDp (
   EFI_DEVICE_PATH_PROTOCOL*DevicePath
   );
 
+/**
+  Convert a single character to number.
+  It assumes the input Char is in the scope of L'0' ~ L'9' and L'A' ~ L'F'
+
+  @paramChar   The input char which need to convert to int.
+
+  @return  The converted 8-bit number or (UINTN) -1 if conversion failed.
+**/
+UINTN
+BmCharToUint (
+  IN CHAR16   Char
+  );
+
 #endif // _INTERNAL_BM_H_
-- 
1.9.5.msysgit.1

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

[edk2] [Patch V2 01/12] MdePkg: Add Platform Recovery definitions.

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Reviewed-by: Sunny Wang 
---
 MdePkg/Include/Uefi/UefiSpec.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h
index 2fb6ea0..909b8a1 100644
--- a/MdePkg/Include/Uefi/UefiSpec.h
+++ b/MdePkg/Include/Uefi/UefiSpec.h
@@ -1750,6 +1750,7 @@ EFI_STATUS
 #define EFI_OS_INDICATIONS_FILE_CAPSULE_DELIVERY_SUPPORTED  0x0004
 #define EFI_OS_INDICATIONS_FMP_CAPSULE_SUPPORTED0x0008
 #define EFI_OS_INDICATIONS_CAPSULE_RESULT_VAR_SUPPORTED 0x0010
+#define EFI_OS_INDICATIONS_START_PLATFORM_RECOVERY  0x0040
 
 //
 // EFI Runtime Services Table
-- 
1.9.5.msysgit.1

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

Re: [edk2] Help: edk2 Ovfm pkg building error:

[Laszlo Ersek]

On 11/13/15 09:44, maojunhong wrote:
> 
> 
> Help: edk2 Ovfm pkg building error:
> 
> : error C0DE: Unknown fatal error when processing 
> [e:\edk2\OvmfPkg\PlatformDxe\Platform.inf]
> 
> 
> 
> SVN version: edk2 18610
> 
> 
> 
> 
> 
> 
> 
> 
> 
> E:\edk2>build -a IA32 -p OvmfPkg\OvmfPkgIa32.dsc
> 
> Build environment: Windows-7-6.1.7601-SP1
> 
> Build start time: 15:33:23, Nov.13 2015
> 
> 
> 
> WORKSPACE= e:\edk2
> 
> ECP_SOURCE   = e:\edk2\edkcompatibilitypkg
> 
> EDK_SOURCE   = e:\edk2\edkcompatibilitypkg
> 
> EFI_SOURCE   = e:\edk2\edkcompatibilitypkg
> 
> EDK_TOOLS_PATH   = e:\edk2\basetools
> 
> 
> 
> 
> 
> Architecture(s)  = IA32
> 
> Build target = DEBUG
> 
> Toolchain= VS2008x86
> 
> 
> 
> Active Platform  = e:\edk2\OvmfPkg\OvmfPkgIa32.dsc
> 
> Flash Image Definition   = e:\edk2\OvmfPkg\OvmfPkgIa32.fdf
> 
> 
> 
> Processing meta-data .
> 
> 
> 
> 
> 
> build...
> 
> : error C0DE: Unknown fatal error when processing 
> [e:\edk2\OvmfPkg\PlatformDxe\Platform.inf]
> 
> 
> 
> (Please send email to 
> edk2-de...@lists.sourceforge.net for 
> help, attaching following call stack trace!)
> 
> 
> 
> (Python 2.7.3 on win32) Traceback (most recent call last):
> 
>   File "build.py", line 2036, in Main
> 
>   File "build.py", line 1792, in Launch
> 
>   File "build.py", line 1622, in _MultiThreadBuildPlatform
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\AutoGen.py",
>  line 3446, in CreateCodeFile
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\AutoGen.py",
>  line 2854, in _GetAutoGenFileList
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\GenC.py",
>  line 1552, in CreateCode
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\GenC.py",
>  line 1456, in CreateUnicodeStringCode
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\StrGather.py",
>  line 600, in GetStringFiles
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\UniClassObject.py",
>  line 203, in __init__
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\UniClassObject.py",
>  line 463, in LoadUniFiles
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\UniClassObject.py",
>  line 377, in LoadUniFile
> 
>   File 
> "c:\Users\Public\Documents\BuildPool\BaseTools\build\Source\Python\AutoGen\UniClassObject.py",
>  line 318, in PreProcess
> 
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 684, in next
> 
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 615, in next
> 
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 530, in readline
> 
>   File "C:\Python\32-bit\2.7\lib\codecs.py", line 477, in read
> 
>   File "C:\Python\32-bit\2.7\lib\encodings\utf_16.py", line 112, in decode
> 
> UnicodeError: UTF-16 stream does not start with BOM
> 
> 
> 
> 
> 
> - Failed -
> 
> Build end time: 15:33:37, Nov.13 2015
> 
> Build total time: 00:00:13

Sorry, no clue. I don't build on Windows, plus everything under
OvmfPkg/PlatformDxe has been pure ASCII since SVN r17700
("OvmfPkg/PlatformDxe: Convert Platform.uni to UTF-8").

Maybe someone who builds OVMF on Windows can help. I can only recommend
(as a best guess) to clean your Conf/ subdirectory before running
edksetup.bat and building OVMF.

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

Re: [edk2] [PATCH v2 2/4] ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as device

[Laszlo Ersek]

On 11/13/15 15:55, Ard Biesheuvel wrote:
> The ARM architecture version 7 and later mandates that device mappings
> have the XN (non-executable) bit set, to prevent speculative instruction
> fetches from read-sensitive regions. This implies that we should not map
> regions as device if we want to execute from them, so the NOR region that
> contains our FD image should be mapped as normal memory instead.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c | 58 
>  1 file changed, 47 insertions(+), 11 deletions(-)
> 
> diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c 
> b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
> index d5d288fb1b48..f5198fff9402 100644
> --- a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
> +++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
> @@ -22,7 +22,7 @@
>  #include 
>  
>  // Number of Virtual Memory Map Descriptors
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  4
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  6
>  
>  // DDR attributes
>  #define DDR_ATTRIBUTES_CACHEDARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> @@ -52,6 +52,7 @@ ArmPlatformGetVirtualMemoryMap (
>  {
>ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
>ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +  UINTN Index;
>  
>ASSERT (VirtualMemoryMap != NULL);
>  
> @@ -88,20 +89,55 @@ ArmPlatformGetVirtualMemoryMap (
>VirtualMemoryTable[0].VirtualBase,
>VirtualMemoryTable[0].Length));
>  
> -  // Peripheral space before DRAM
> -  VirtualMemoryTable[1].PhysicalBase = 0x0;
> -  VirtualMemoryTable[1].VirtualBase  = 0x0;
> -  VirtualMemoryTable[1].Length   = VirtualMemoryTable[0].PhysicalBase;
> -  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  //
> +  // We cannot map the NOR flash that contains the FD image as a device, 
> since
> +  // that may imply XN on v7 and later, and we may still be executing from 
> it at
> +  // this point. So explicitly map the FD as normal memory if it does not
> +  // intersect with system memory.
> +  //
> +
> +  // Disregard the case where the FD sits above DRAM, to keep the logic 
> simple.
> +  ASSERT (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize) <=
> +  PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize));
> +
> +  Index = 0;
> +  if (FixedPcdGet64(PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) {
> +// FD is loaded in DRAM, map everything below DRAM as device
> +VirtualMemoryTable[++Index].PhysicalBase= 0x0;
> +VirtualMemoryTable[Index].VirtualBase   = 0x0;
> +VirtualMemoryTable[Index].Length= PcdGet64 
> (PcdSystemMemoryBase);
> +VirtualMemoryTable[Index].Attributes= 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  } else {
> +// FD is loaded below DRAM, map everything except the FD as device
> +if (FixedPcdGet64(PcdFdBaseAddress) > 0) {
> +  // Peripheral space before flash device
> +  VirtualMemoryTable[++Index].PhysicalBase  = 0x0;
> +  VirtualMemoryTable[Index].VirtualBase = 0x0;
> +  VirtualMemoryTable[Index].Length  = 
> FixedPcdGet64(PcdFdBaseAddress);
> +  VirtualMemoryTable[Index].Attributes  = 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +}
> +
> +// Map the FD as normal memory
> +VirtualMemoryTable[++Index].PhysicalBase= 
> FixedPcdGet64(PcdFdBaseAddress);
> +VirtualMemoryTable[Index].VirtualBase   = 
> VirtualMemoryTable[Index].PhysicalBase;
> +VirtualMemoryTable[Index].Length= FixedPcdGet32(PcdFdSize);
> +VirtualMemoryTable[Index].Attributes= CacheAttributes;
> +
> +// Peripheral space between flash device and DRAM
> +VirtualMemoryTable[++Index].PhysicalBase= 
> FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize);
> +VirtualMemoryTable[Index].VirtualBase   = 
> VirtualMemoryTable[Index].PhysicalBase;
> +VirtualMemoryTable[Index].Length= PcdGet64 
> (PcdSystemMemoryBase) - VirtualMemoryTable[Index].PhysicalBase;
> +VirtualMemoryTable[Index].Attributes= 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  }
>  
>// Peripheral space after DRAM
> -  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + 
> VirtualMemoryTable[1].Length;
> -  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
> -  VirtualMemoryTable[2].Length   = ArmGetPhysAddrTop () - 
> VirtualMemoryTable[2].PhysicalBase;
> -  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 
> (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize);
> +  VirtualMemoryTable[Index].VirtualBase = 
> VirtualMemoryTable[Index].PhysicalBase;
> +  VirtualMemoryTable[Index].Length  = ArmGetPhysAddrTop () 

Re: [edk2] Alter OVMF/UEFI iSCSI settings outside of the OVMF UI

[Laszlo Ersek]

On 11/13/15 13:39, P.A.M. van Dam (Pascal) wrote:
> Good morning all,
> 
> What I would like to do is pre-configure the NVRAM of the Virtual Machines 
> (guests) with the
> ISCSI config. So have initiator, iSCSI host and target preconfigured in the 
> NVRAM before the guest
> gets started. This way, RHEL/CENTOS Kickstart should be able to pickup the 
> iSCSI config and use
> the UEFI discovered and enabled bootdrive in the configuration.

I've been expecting this question.

> My questions:
> 
> 1) Is this possible. And yes, what tool can I use?

Modifying the varstore file directly from the host side is not
supported, and such suppport is also explicitly not planned. I've gone
through the reasons (in various discussions, here and elsewhere) many times.

The summary is that the host-visible binary contents of the file are the
*functional composition* of three separate drivers in edk2:
- the platform (ie. OVMF) specific flash driver (including the flash
  size and layout)
- the universal fault tolerant write driver
- the universal variable driver (with secure boot / SSL supported or
  not)

A robust host-side utility to modify the varstore file would have to
implement the same three layers independently of edk2, which would be a
*huge* task. Consider that the fault tolerant write driver deals with
and recovers from aborted / halfway performed writes (physical case:
power is lost while writing the flash -- your UEFI box must not be
bricked by this); the variable driver does reclaim / defragmentation,
and so on.

In addition, any future changes to the edk2 driver stack mentioned above
would have to be ported to the host-side utility ad infinitum.

> 
> 2) If not, where can I find the documentation about how the NVRAM is built,
>so I can try to create such a tool myself.

Please don't even try (or ask me to support it, ever).

There are some Intel whitepapers that more or less describe the design;
for example:

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

And the drivers that implement the above are (with their supporting
libraries -- see the OVMF DSC files):

  OvmfPkg/QemuFlashFvbServicesRuntimeDxe/FvbServicesRuntimeDxe.inf
  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf

if you want to spend weeks reading code, and see why I'm right.

Instead, the recommended ways for non-interactively setting UEFI
variables are:

* Write a UEFI application or a UEFI shell script (containing UEFI
  shell commands) that perform the variable settings you would like to
  see. You can then invoke this utility or shell script in at least two
  ways:

  (a) Using "expect", communicating with the UEFI shell over the
  virtual serial console. (You can think of this as a crude "UEFI
  guest agent".)

  (b) Preparing EFI-bootable media (a disk or ISO image) that
  automatically boots your utility in the guest.

* Alternatively, pre-populate a varstore in some way, from within the
  guest (for example: using (a) or (b) above, or even the manual /
  interactive configuration that you seem to be doing now), shut down
  the VM ("reset -s" UEFI shell command), and then stash the varstore
  file just written, as a varstore template for further virtual
  machines.


Also note that QEMU has no way of presenting an OpenFirmware device path
to OVMF's QemuBootOrderLib that the latter could map to a UEFI iSCSI
Device Path. Meaning, if you provide any boot order setting in the
domain XML or on the QEMU command line (like -boot X, or -device
...,bootorder=N), *and* there is a match against the preexistent or
auto-enumerated UEFI boot options, even in the second or third place,
then OVMF will silently remove your preconfigured iSCSI boot option
(because it has no way of matching).

I guess the previous paragraph is hard to interpret. In simpler terms,
it means that when you have such a pre-set iscsi boot option in your
varstore, then you should never pass -boot X, -device ZZZ,bootorder=N,
nor their corresponding elements in libvirt's domain XML. Otherwise your
existent iSCSI boot option will be removed.

Summary:
- Set it up once manually, from within the guest, then stash the
  varstore file. Use that file as the varstore *template* when creating
  further guests. See virt-install --boot ...,nvram_template=...,...

- If you would like to pass in the iSCSI details dynamically before
  each guest is created, then the above won't work, and you'll have to
  do (a) or (b).

- Whenever booting such guests, don't pass boot order related settings
  to QEMU.

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

[edk2] [PATCH v2 1/4] ArmPkg/AArch64Mmu: remove unused GcdAttributeToArmAttribute()

[Ard Biesheuvel]

The function GcdAttributeToArmAttribute() is not used anywhere in the
code base, and is only defined for AARCH64 and not for ARM. It also
fails to set the bits for shareability and non-executability that we
require for correct operation. So remove it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Include/Chipset/AArch64.h   |  5 ---
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 42 
 2 files changed, 47 deletions(-)

diff --git a/ArmPkg/Include/Chipset/AArch64.h b/ArmPkg/Include/Chipset/AArch64.h
index 47993ec9fc3b..f6a89012898c 100644
--- a/ArmPkg/Include/Chipset/AArch64.h
+++ b/ArmPkg/Include/Chipset/AArch64.h
@@ -178,11 +178,6 @@ PageAttributeToGcdAttribute (
   IN UINT64 PageAttributes
   );
 
-UINT64
-GcdAttributeToPageAttribute (
-  IN UINT64 GcdAttributes
-  );
-
 UINTN
 ArmWriteCptr (
   IN  UINT64 Cptr
diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c 
b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
index 8829c6286b36..c8b3d4a121b1 100644
--- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
+++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c
@@ -96,48 +96,6 @@ PageAttributeToGcdAttribute (
   return GcdAttributes;
 }
 
-UINT64
-GcdAttributeToPageAttribute (
-  IN UINT64 GcdAttributes
-  )
-{
-  UINT64  PageAttributes;
-
-  switch (GcdAttributes & 0xFF) {
-  case EFI_MEMORY_UC:
-PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY;
-break;
-  case EFI_MEMORY_WC:
-PageAttributes = TT_ATTR_INDX_MEMORY_NON_CACHEABLE;
-break;
-  case EFI_MEMORY_WT:
-PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_THROUGH;
-break;
-  case EFI_MEMORY_WB:
-PageAttributes = TT_ATTR_INDX_MEMORY_WRITE_BACK;
-break;
-  default:
-DEBUG ((EFI_D_ERROR, "GcdAttributeToPageAttribute: 0x%X attributes is not 
supported.\n", GcdAttributes));
-ASSERT (0);
-// If no match has been found then we mark the memory as device memory.
-// The only side effect of using device memory should be a slow down in 
the performance.
-PageAttributes = TT_ATTR_INDX_DEVICE_MEMORY;
-  }
-
-  // Determine protection attributes
-  if (GcdAttributes & EFI_MEMORY_WP) {
-// Read only cases map to write-protect
-PageAttributes |= TT_AP_RO_RO;
-  }
-
-  // Process eXecute Never attribute
-  if (GcdAttributes & EFI_MEMORY_XP) {
-PageAttributes |= (TT_PXN_MASK | TT_UXN_MASK);
-  }
-
-  return PageAttributes;
-}
-
 ARM_MEMORY_REGION_ATTRIBUTES
 GcdAttributeToArmAttribute (
   IN UINT64 GcdAttributes
-- 
1.9.1

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

[edk2] [PATCH v2 0/4] improve handling of device attributes on ARM/AARCH64

[Ard Biesheuvel]

This is a followup to the 2-patch series I sent out earlier today.

This series fixes some issues that exist in the code with regard to how device
mappings are created. According to the architecture, read-sensitive devices
should be mapped with the non-execute bits (XN/PXN/UXN) to prevent speculative
instruction fetches from accessing those regions.

Patch #1 removes GcdAttributeToArmAttribute() rather than fixing it, since it
is unused anyway. Note that it fails to set shareability attributes on cached
mappings as well, so it is broken in more than one way. (identical to v1)

Patch #2 ensures that the ArmVirtQemu firmware is still executable in place
after changing the device mapping attribute set to include the non-exec
attributes. (new in v2)

Patch #3 does the same for FVP-AArch64 and RTSM-A15_MPCore. (new in v2)

Patch #4 makes the changes to ensure that all device mappings have the XN bit
set. The v2 version now covers ARM as well.

Ard Biesheuvel (4):
  ArmPkg/AArch64Mmu: remove unused GcdAttributeToArmAttribute()
  ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as
device
  ArmVExpressPkg/ArmVExpressLibRTSM: map NOR flash as cached
  ArmPkg/Mmu: set required XN attributes for device mappings

 ArmPkg/Include/Chipset/AArch64.h   |  5 --
 ArmPkg/Include/Chipset/ArmV7Mmu.h  |  2 +
 ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 47 
++--
 ArmPkg/Library/ArmLib/ArmV7/ArmV7Mmu.c |  2 +-
 ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c |  2 +-
 ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c| 58 

 6 files changed, 55 insertions(+), 61 deletions(-)

-- 
1.9.1

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

[edk2] [PATCH v2 2/4] ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as device

[Ard Biesheuvel]

The ARM architecture version 7 and later mandates that device mappings
have the XN (non-executable) bit set, to prevent speculative instruction
fetches from read-sensitive regions. This implies that we should not map
regions as device if we want to execute from them, so the NOR region that
contains our FD image should be mapped as normal memory instead.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c | 58 
 1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c 
b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
index d5d288fb1b48..f5198fff9402 100644
--- a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
+++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
@@ -22,7 +22,7 @@
 #include 
 
 // Number of Virtual Memory Map Descriptors
-#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  4
+#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  6
 
 // DDR attributes
 #define DDR_ATTRIBUTES_CACHEDARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
@@ -52,6 +52,7 @@ ArmPlatformGetVirtualMemoryMap (
 {
   ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
   ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
+  UINTN Index;
 
   ASSERT (VirtualMemoryMap != NULL);
 
@@ -88,20 +89,55 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[0].VirtualBase,
   VirtualMemoryTable[0].Length));
 
-  // Peripheral space before DRAM
-  VirtualMemoryTable[1].PhysicalBase = 0x0;
-  VirtualMemoryTable[1].VirtualBase  = 0x0;
-  VirtualMemoryTable[1].Length   = VirtualMemoryTable[0].PhysicalBase;
-  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  //
+  // We cannot map the NOR flash that contains the FD image as a device, since
+  // that may imply XN on v7 and later, and we may still be executing from it 
at
+  // this point. So explicitly map the FD as normal memory if it does not
+  // intersect with system memory.
+  //
+
+  // Disregard the case where the FD sits above DRAM, to keep the logic simple.
+  ASSERT (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize) <=
+  PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize));
+
+  Index = 0;
+  if (FixedPcdGet64(PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) {
+// FD is loaded in DRAM, map everything below DRAM as device
+VirtualMemoryTable[++Index].PhysicalBase= 0x0;
+VirtualMemoryTable[Index].VirtualBase   = 0x0;
+VirtualMemoryTable[Index].Length= PcdGet64 
(PcdSystemMemoryBase);
+VirtualMemoryTable[Index].Attributes= 
ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  } else {
+// FD is loaded below DRAM, map everything except the FD as device
+if (FixedPcdGet64(PcdFdBaseAddress) > 0) {
+  // Peripheral space before flash device
+  VirtualMemoryTable[++Index].PhysicalBase  = 0x0;
+  VirtualMemoryTable[Index].VirtualBase = 0x0;
+  VirtualMemoryTable[Index].Length  = 
FixedPcdGet64(PcdFdBaseAddress);
+  VirtualMemoryTable[Index].Attributes  = 
ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+}
+
+// Map the FD as normal memory
+VirtualMemoryTable[++Index].PhysicalBase= 
FixedPcdGet64(PcdFdBaseAddress);
+VirtualMemoryTable[Index].VirtualBase   = 
VirtualMemoryTable[Index].PhysicalBase;
+VirtualMemoryTable[Index].Length= FixedPcdGet32(PcdFdSize);
+VirtualMemoryTable[Index].Attributes= CacheAttributes;
+
+// Peripheral space between flash device and DRAM
+VirtualMemoryTable[++Index].PhysicalBase= 
FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize);
+VirtualMemoryTable[Index].VirtualBase   = 
VirtualMemoryTable[Index].PhysicalBase;
+VirtualMemoryTable[Index].Length= PcdGet64 
(PcdSystemMemoryBase) - VirtualMemoryTable[Index].PhysicalBase;
+VirtualMemoryTable[Index].Attributes= 
ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  }
 
   // Peripheral space after DRAM
-  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + 
VirtualMemoryTable[1].Length;
-  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
-  VirtualMemoryTable[2].Length   = ArmGetPhysAddrTop () - 
VirtualMemoryTable[2].PhysicalBase;
-  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 
(PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize);
+  VirtualMemoryTable[Index].VirtualBase = 
VirtualMemoryTable[Index].PhysicalBase;
+  VirtualMemoryTable[Index].Length  = ArmGetPhysAddrTop () - 
VirtualMemoryTable[Index].PhysicalBase;
+  VirtualMemoryTable[Index].Attributes  = 
ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 
   // End of Table
-  ZeroMem ([3], sizeof (ARM_MEMORY_REGION_DESCRIPTOR));
+  ZeroMem ([++Index], sizeof 

Re: [edk2] [PATCH v2 2/4] ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as device

[Ard Biesheuvel]

On 13 November 2015 at 17:55, Ard Biesheuvel  wrote:
> On 13 November 2015 at 17:40, Laszlo Ersek  wrote:
>> On 11/13/15 15:55, Ard Biesheuvel wrote:
>>> The ARM architecture version 7 and later mandates that device mappings
>>> have the XN (non-executable) bit set, to prevent speculative instruction
>>> fetches from read-sensitive regions. This implies that we should not map
>>> regions as device if we want to execute from them, so the NOR region that
>>> contains our FD image should be mapped as normal memory instead.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c | 58 
>>>  1 file changed, 47 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c 
>>> b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
>>> index d5d288fb1b48..f5198fff9402 100644
>>> --- a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
>>> +++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
>>> @@ -22,7 +22,7 @@
>>>  #include 
>>>
>>>  // Number of Virtual Memory Map Descriptors
>>> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  4
>>> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  6
>>>
>>>  // DDR attributes
>>>  #define DDR_ATTRIBUTES_CACHEDARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
>>> @@ -52,6 +52,7 @@ ArmPlatformGetVirtualMemoryMap (
>>>  {
>>>ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
>>>ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
>>> +  UINTN Index;
>>>
>>>ASSERT (VirtualMemoryMap != NULL);
>>>
>>> @@ -88,20 +89,55 @@ ArmPlatformGetVirtualMemoryMap (
>>>VirtualMemoryTable[0].VirtualBase,
>>>VirtualMemoryTable[0].Length));
>>>
>>> -  // Peripheral space before DRAM
>>> -  VirtualMemoryTable[1].PhysicalBase = 0x0;
>>> -  VirtualMemoryTable[1].VirtualBase  = 0x0;
>>> -  VirtualMemoryTable[1].Length   = VirtualMemoryTable[0].PhysicalBase;
>>> -  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>>> +  //
>>> +  // We cannot map the NOR flash that contains the FD image as a device, 
>>> since
>>> +  // that may imply XN on v7 and later, and we may still be executing from 
>>> it at
>>> +  // this point. So explicitly map the FD as normal memory if it does not
>>> +  // intersect with system memory.
>>> +  //
>>> +
>>> +  // Disregard the case where the FD sits above DRAM, to keep the logic 
>>> simple.
>>> +  ASSERT (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize) <=
>>> +  PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize));
>>> +
>>> +  Index = 0;
>>> +  if (FixedPcdGet64(PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) {
>>> +// FD is loaded in DRAM, map everything below DRAM as device
>>> +VirtualMemoryTable[++Index].PhysicalBase= 0x0;
>>> +VirtualMemoryTable[Index].VirtualBase   = 0x0;
>>> +VirtualMemoryTable[Index].Length= PcdGet64 
>>> (PcdSystemMemoryBase);
>>> +VirtualMemoryTable[Index].Attributes= 
>>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>>> +  } else {
>>> +// FD is loaded below DRAM, map everything except the FD as device
>>> +if (FixedPcdGet64(PcdFdBaseAddress) > 0) {
>>> +  // Peripheral space before flash device
>>> +  VirtualMemoryTable[++Index].PhysicalBase  = 0x0;
>>> +  VirtualMemoryTable[Index].VirtualBase = 0x0;
>>> +  VirtualMemoryTable[Index].Length  = 
>>> FixedPcdGet64(PcdFdBaseAddress);
>>> +  VirtualMemoryTable[Index].Attributes  = 
>>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>>> +}
>>> +
>>> +// Map the FD as normal memory
>>> +VirtualMemoryTable[++Index].PhysicalBase= 
>>> FixedPcdGet64(PcdFdBaseAddress);
>>> +VirtualMemoryTable[Index].VirtualBase   = 
>>> VirtualMemoryTable[Index].PhysicalBase;
>>> +VirtualMemoryTable[Index].Length= FixedPcdGet32(PcdFdSize);
>>> +VirtualMemoryTable[Index].Attributes= CacheAttributes;
>>> +
>>> +// Peripheral space between flash device and DRAM
>>> +VirtualMemoryTable[++Index].PhysicalBase= 
>>> FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize);
>>> +VirtualMemoryTable[Index].VirtualBase   = 
>>> VirtualMemoryTable[Index].PhysicalBase;
>>> +VirtualMemoryTable[Index].Length= PcdGet64 
>>> (PcdSystemMemoryBase) - VirtualMemoryTable[Index].PhysicalBase;
>>> +VirtualMemoryTable[Index].Attributes= 
>>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>>> +  }
>>>
>>>// Peripheral space after DRAM
>>> -  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + 
>>> VirtualMemoryTable[1].Length;
>>> -  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
>>> -  VirtualMemoryTable[2].Length   = ArmGetPhysAddrTop () - 
>>> VirtualMemoryTable[2].PhysicalBase;
>>> -  

[edk2] [PATCH v2 3/4] ArmVExpressPkg/ArmVExpressLibRTSM: map NOR flash as cached

[Ard Biesheuvel]

Some users of this library (i.e., FVP-AArch64 and RTSM-A15_MPCore)
may be built to execute straight from NOR flash. Since device mappings
should have the XN attribute set (according to the architecture), mapping
the NOR flash as a device may prevent it from being executable.

Since the NOR flash DXE driver is perfectly capable of setting the correct
attributes for the region it needs to write to, and since we will be
executing from DRAM by that time anyway, we can simply map the NOR flash
as normal memory initially.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c 
b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
index 6c5e494d502c..c6df2e1b3de4 100644
--- a/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
+++ b/ArmPlatformPkg/ArmVExpressPkg/Library/ArmVExpressLibRTSM/RTSMMem.c
@@ -126,7 +126,7 @@ ArmPlatformGetVirtualMemoryMap (
   VirtualMemoryTable[++Index].PhysicalBase = ARM_VE_SMB_NOR0_BASE;
   VirtualMemoryTable[Index].VirtualBase  = ARM_VE_SMB_NOR0_BASE;
   VirtualMemoryTable[Index].Length   = ARM_VE_SMB_NOR0_SZ + 
ARM_VE_SMB_NOR1_SZ;
-  VirtualMemoryTable[Index].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
+  VirtualMemoryTable[Index].Attributes   = CacheAttributes;
 
   // SMB CS2 - SRAM
   VirtualMemoryTable[++Index].PhysicalBase = ARM_VE_SMB_SRAM_BASE;
-- 
1.9.1

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

Re: [edk2] [PATCH v2 2/4] ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as device

[Ard Biesheuvel]

On 13 November 2015 at 17:40, Laszlo Ersek  wrote:
> On 11/13/15 15:55, Ard Biesheuvel wrote:
>> The ARM architecture version 7 and later mandates that device mappings
>> have the XN (non-executable) bit set, to prevent speculative instruction
>> fetches from read-sensitive regions. This implies that we should not map
>> regions as device if we want to execute from them, so the NOR region that
>> contains our FD image should be mapped as normal memory instead.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c | 58 
>>  1 file changed, 47 insertions(+), 11 deletions(-)
>>
>> diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c 
>> b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
>> index d5d288fb1b48..f5198fff9402 100644
>> --- a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
>> +++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
>> @@ -22,7 +22,7 @@
>>  #include 
>>
>>  // Number of Virtual Memory Map Descriptors
>> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  4
>> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  6
>>
>>  // DDR attributes
>>  #define DDR_ATTRIBUTES_CACHEDARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
>> @@ -52,6 +52,7 @@ ArmPlatformGetVirtualMemoryMap (
>>  {
>>ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
>>ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
>> +  UINTN Index;
>>
>>ASSERT (VirtualMemoryMap != NULL);
>>
>> @@ -88,20 +89,55 @@ ArmPlatformGetVirtualMemoryMap (
>>VirtualMemoryTable[0].VirtualBase,
>>VirtualMemoryTable[0].Length));
>>
>> -  // Peripheral space before DRAM
>> -  VirtualMemoryTable[1].PhysicalBase = 0x0;
>> -  VirtualMemoryTable[1].VirtualBase  = 0x0;
>> -  VirtualMemoryTable[1].Length   = VirtualMemoryTable[0].PhysicalBase;
>> -  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>> +  //
>> +  // We cannot map the NOR flash that contains the FD image as a device, 
>> since
>> +  // that may imply XN on v7 and later, and we may still be executing from 
>> it at
>> +  // this point. So explicitly map the FD as normal memory if it does not
>> +  // intersect with system memory.
>> +  //
>> +
>> +  // Disregard the case where the FD sits above DRAM, to keep the logic 
>> simple.
>> +  ASSERT (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize) <=
>> +  PcdGet64 (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize));
>> +
>> +  Index = 0;
>> +  if (FixedPcdGet64(PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) {
>> +// FD is loaded in DRAM, map everything below DRAM as device
>> +VirtualMemoryTable[++Index].PhysicalBase= 0x0;
>> +VirtualMemoryTable[Index].VirtualBase   = 0x0;
>> +VirtualMemoryTable[Index].Length= PcdGet64 
>> (PcdSystemMemoryBase);
>> +VirtualMemoryTable[Index].Attributes= 
>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>> +  } else {
>> +// FD is loaded below DRAM, map everything except the FD as device
>> +if (FixedPcdGet64(PcdFdBaseAddress) > 0) {
>> +  // Peripheral space before flash device
>> +  VirtualMemoryTable[++Index].PhysicalBase  = 0x0;
>> +  VirtualMemoryTable[Index].VirtualBase = 0x0;
>> +  VirtualMemoryTable[Index].Length  = 
>> FixedPcdGet64(PcdFdBaseAddress);
>> +  VirtualMemoryTable[Index].Attributes  = 
>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>> +}
>> +
>> +// Map the FD as normal memory
>> +VirtualMemoryTable[++Index].PhysicalBase= 
>> FixedPcdGet64(PcdFdBaseAddress);
>> +VirtualMemoryTable[Index].VirtualBase   = 
>> VirtualMemoryTable[Index].PhysicalBase;
>> +VirtualMemoryTable[Index].Length= FixedPcdGet32(PcdFdSize);
>> +VirtualMemoryTable[Index].Attributes= CacheAttributes;
>> +
>> +// Peripheral space between flash device and DRAM
>> +VirtualMemoryTable[++Index].PhysicalBase= 
>> FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize);
>> +VirtualMemoryTable[Index].VirtualBase   = 
>> VirtualMemoryTable[Index].PhysicalBase;
>> +VirtualMemoryTable[Index].Length= PcdGet64 
>> (PcdSystemMemoryBase) - VirtualMemoryTable[Index].PhysicalBase;
>> +VirtualMemoryTable[Index].Attributes= 
>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>> +  }
>>
>>// Peripheral space after DRAM
>> -  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + 
>> VirtualMemoryTable[1].Length;
>> -  VirtualMemoryTable[2].VirtualBase  = VirtualMemoryTable[2].PhysicalBase;
>> -  VirtualMemoryTable[2].Length   = ArmGetPhysAddrTop () - 
>> VirtualMemoryTable[2].PhysicalBase;
>> -  VirtualMemoryTable[2].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
>> +  VirtualMemoryTable[++Index].PhysicalBase  = PcdGet64 
>> (PcdSystemMemoryBase) + PcdGet64 (PcdSystemMemorySize);
>> +  

Re: [edk2] Alter OVMF/UEFI iSCSI settings outside of the OVMF UI

[Andrew Fish]

> On Nov 13, 2015, at 4:39 AM, P.A.M. van Dam (Pascal) 
>  wrote:
> 
> Good morning all,
> 
> What I would like to do is pre-configure the NVRAM of the Virtual Machines 
> (guests) with the
> ISCSI config. So have initiator, iSCSI host and target preconfigured in the 
> NVRAM before the guest
> gets started. This way, RHEL/CENTOS Kickstart should be able to pickup the 
> iSCSI config and use
> the UEFI discovered and enabled bootdrive in the configuration.
> 
> My questions:
> 
> 1) Is this possible. And yes, what tool can I use?
> 
> 2) If not, where can I find the documentation about how the NVRAM is built,
>  so I can try to create such a tool myself.
> 

The NVRAM format is not standard as it is abstracted by the UEFI Variable 
services that exist at runtime, so the OS can use them too. So the easy answer 
is run some EFI/OS code that calls gRT->SetVariable(). 

For the edk2 the format is defined here:
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Include/Guid/VariableFormat.h

This is the DXE Driver that reads and writes the variables. 
https://github.com/tianocore/edk2/tree/master/MdeModulePkg/Universal/Variable/RuntimeDxe

Thanks,

Andrew Fish

> Thanks for your help!
> 
> Kind regards,
> 
>   Pascal van Dam
> 
> 
> ___
> 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/4] ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as device

[Laszlo Ersek]

On 11/13/15 18:16, Ard Biesheuvel wrote:
> On 13 November 2015 at 17:55, Ard Biesheuvel  
> wrote:
>> On 13 November 2015 at 17:40, Laszlo Ersek  wrote:
>>> On 11/13/15 15:55, Ard Biesheuvel wrote:
 The ARM architecture version 7 and later mandates that device mappings
 have the XN (non-executable) bit set, to prevent speculative instruction
 fetches from read-sensitive regions. This implies that we should not map
 regions as device if we want to execute from them, so the NOR region that
 contains our FD image should be mapped as normal memory instead.

 Contributed-under: TianoCore Contribution Agreement 1.0
 Signed-off-by: Ard Biesheuvel 
 ---
  ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c | 58 
  1 file changed, 47 insertions(+), 11 deletions(-)

 diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c 
 b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
 index d5d288fb1b48..f5198fff9402 100644
 --- a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
 +++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
 @@ -22,7 +22,7 @@
  #include 

  // Number of Virtual Memory Map Descriptors
 -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  4
 +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  6

  // DDR attributes
  #define DDR_ATTRIBUTES_CACHEDARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
 @@ -52,6 +52,7 @@ ArmPlatformGetVirtualMemoryMap (
  {
ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
 +  UINTN Index;

ASSERT (VirtualMemoryMap != NULL);

 @@ -88,20 +89,55 @@ ArmPlatformGetVirtualMemoryMap (
VirtualMemoryTable[0].VirtualBase,
VirtualMemoryTable[0].Length));

 -  // Peripheral space before DRAM
 -  VirtualMemoryTable[1].PhysicalBase = 0x0;
 -  VirtualMemoryTable[1].VirtualBase  = 0x0;
 -  VirtualMemoryTable[1].Length   = VirtualMemoryTable[0].PhysicalBase;
 -  VirtualMemoryTable[1].Attributes   = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 +  //
 +  // We cannot map the NOR flash that contains the FD image as a device, 
 since
 +  // that may imply XN on v7 and later, and we may still be executing 
 from it at
 +  // this point. So explicitly map the FD as normal memory if it does not
 +  // intersect with system memory.
 +  //
 +
 +  // Disregard the case where the FD sits above DRAM, to keep the logic 
 simple.
 +  ASSERT (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize) <=
 +  PcdGet64 (PcdSystemMemoryBase) + PcdGet64 
 (PcdSystemMemorySize));
 +
 +  Index = 0;
 +  if (FixedPcdGet64(PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) {
 +// FD is loaded in DRAM, map everything below DRAM as device
 +VirtualMemoryTable[++Index].PhysicalBase= 0x0;
 +VirtualMemoryTable[Index].VirtualBase   = 0x0;
 +VirtualMemoryTable[Index].Length= PcdGet64 
 (PcdSystemMemoryBase);
 +VirtualMemoryTable[Index].Attributes= 
 ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 +  } else {
 +// FD is loaded below DRAM, map everything except the FD as device
 +if (FixedPcdGet64(PcdFdBaseAddress) > 0) {
 +  // Peripheral space before flash device
 +  VirtualMemoryTable[++Index].PhysicalBase  = 0x0;
 +  VirtualMemoryTable[Index].VirtualBase = 0x0;
 +  VirtualMemoryTable[Index].Length  = 
 FixedPcdGet64(PcdFdBaseAddress);
 +  VirtualMemoryTable[Index].Attributes  = 
 ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 +}
 +
 +// Map the FD as normal memory
 +VirtualMemoryTable[++Index].PhysicalBase= 
 FixedPcdGet64(PcdFdBaseAddress);
 +VirtualMemoryTable[Index].VirtualBase   = 
 VirtualMemoryTable[Index].PhysicalBase;
 +VirtualMemoryTable[Index].Length= 
 FixedPcdGet32(PcdFdSize);
 +VirtualMemoryTable[Index].Attributes= CacheAttributes;
 +
 +// Peripheral space between flash device and DRAM
 +VirtualMemoryTable[++Index].PhysicalBase= 
 FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize);
 +VirtualMemoryTable[Index].VirtualBase   = 
 VirtualMemoryTable[Index].PhysicalBase;
 +VirtualMemoryTable[Index].Length= PcdGet64 
 (PcdSystemMemoryBase) - VirtualMemoryTable[Index].PhysicalBase;
 +VirtualMemoryTable[Index].Attributes= 
 ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
 +  }

// Peripheral space after DRAM
 -  VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + 
 VirtualMemoryTable[1].Length;
 -  VirtualMemoryTable[2].VirtualBase  = 

Re: [edk2] [PATCH v2 2/4] ArmVirtPkg/ArmVirtPlatformLib: do not map executable NOR region as device

[Ard Biesheuvel]

On 13 November 2015 at 18:33, Laszlo Ersek  wrote:
> On 11/13/15 18:16, Ard Biesheuvel wrote:
>> On 13 November 2015 at 17:55, Ard Biesheuvel  
>> wrote:
>>> On 13 November 2015 at 17:40, Laszlo Ersek  wrote:
 On 11/13/15 15:55, Ard Biesheuvel wrote:
> The ARM architecture version 7 and later mandates that device mappings
> have the XN (non-executable) bit set, to prevent speculative instruction
> fetches from read-sensitive regions. This implies that we should not map
> regions as device if we want to execute from them, so the NOR region that
> contains our FD image should be mapped as normal memory instead.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
>  ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c | 58 
>  1 file changed, 47 insertions(+), 11 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c 
> b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
> index d5d288fb1b48..f5198fff9402 100644
> --- a/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
> +++ b/ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c
> @@ -22,7 +22,7 @@
>  #include 
>
>  // Number of Virtual Memory Map Descriptors
> -#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  4
> +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS  6
>
>  // DDR attributes
>  #define DDR_ATTRIBUTES_CACHEDARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK
> @@ -52,6 +52,7 @@ ArmPlatformGetVirtualMemoryMap (
>  {
>ARM_MEMORY_REGION_ATTRIBUTES  CacheAttributes;
>ARM_MEMORY_REGION_DESCRIPTOR  *VirtualMemoryTable;
> +  UINTN Index;
>
>ASSERT (VirtualMemoryMap != NULL);
>
> @@ -88,20 +89,55 @@ ArmPlatformGetVirtualMemoryMap (
>VirtualMemoryTable[0].VirtualBase,
>VirtualMemoryTable[0].Length));
>
> -  // Peripheral space before DRAM
> -  VirtualMemoryTable[1].PhysicalBase = 0x0;
> -  VirtualMemoryTable[1].VirtualBase  = 0x0;
> -  VirtualMemoryTable[1].Length   = 
> VirtualMemoryTable[0].PhysicalBase;
> -  VirtualMemoryTable[1].Attributes   = 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  //
> +  // We cannot map the NOR flash that contains the FD image as a device, 
> since
> +  // that may imply XN on v7 and later, and we may still be executing 
> from it at
> +  // this point. So explicitly map the FD as normal memory if it does not
> +  // intersect with system memory.
> +  //
> +
> +  // Disregard the case where the FD sits above DRAM, to keep the logic 
> simple.
> +  ASSERT (FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize) <=
> +  PcdGet64 (PcdSystemMemoryBase) + PcdGet64 
> (PcdSystemMemorySize));
> +
> +  Index = 0;
> +  if (FixedPcdGet64(PcdFdBaseAddress) >= PcdGet64 (PcdSystemMemoryBase)) 
> {
> +// FD is loaded in DRAM, map everything below DRAM as device
> +VirtualMemoryTable[++Index].PhysicalBase= 0x0;
> +VirtualMemoryTable[Index].VirtualBase   = 0x0;
> +VirtualMemoryTable[Index].Length= PcdGet64 
> (PcdSystemMemoryBase);
> +VirtualMemoryTable[Index].Attributes= 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  } else {
> +// FD is loaded below DRAM, map everything except the FD as device
> +if (FixedPcdGet64(PcdFdBaseAddress) > 0) {
> +  // Peripheral space before flash device
> +  VirtualMemoryTable[++Index].PhysicalBase  = 0x0;
> +  VirtualMemoryTable[Index].VirtualBase = 0x0;
> +  VirtualMemoryTable[Index].Length  = 
> FixedPcdGet64(PcdFdBaseAddress);
> +  VirtualMemoryTable[Index].Attributes  = 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +}
> +
> +// Map the FD as normal memory
> +VirtualMemoryTable[++Index].PhysicalBase= 
> FixedPcdGet64(PcdFdBaseAddress);
> +VirtualMemoryTable[Index].VirtualBase   = 
> VirtualMemoryTable[Index].PhysicalBase;
> +VirtualMemoryTable[Index].Length= 
> FixedPcdGet32(PcdFdSize);
> +VirtualMemoryTable[Index].Attributes= CacheAttributes;
> +
> +// Peripheral space between flash device and DRAM
> +VirtualMemoryTable[++Index].PhysicalBase= 
> FixedPcdGet64(PcdFdBaseAddress) + FixedPcdGet32(PcdFdSize);
> +VirtualMemoryTable[Index].VirtualBase   = 
> VirtualMemoryTable[Index].PhysicalBase;
> +VirtualMemoryTable[Index].Length= PcdGet64 
> (PcdSystemMemoryBase) - VirtualMemoryTable[Index].PhysicalBase;
> +VirtualMemoryTable[Index].Attributes= 
> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE;
> +  }
>
> 

[edk2] [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without EFI_PEI_SMM_COMMUNICATION_PPI

[Laszlo Ersek]

The RestoreLockBox() and RestoreAllLockBoxInPlace() functions handle the
case when EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() returns
EFI_NOT_STARTED: they access the SMRAM directly, for restoring LockBox
data.

This occurs if a PEIM needs to restore LockBox data *before* the SMBASE is
relocated and the SMI handler is installed for all processors.

One such PEIM is UefiCpuPkg/Universal/Acpi/S3Resume2Pei. On the S3 resume
path, in function S3RestoreConfig2(), LockBox data are restored *before*
the SmmRestoreCpu() function of UefiCpuPkg/PiSmmCpuDxeSmm is called via
SmmS3ResumeState->SmmS3ResumeEntryPoint. (The latter SmmRestoreCpu()
function is responsible for the SMBASE relocation.)

If a platform knows that its PEIMs restore LockBox data *only* before
SMBASE relocation -- e.g., due to S3Resume2Pei being the platform's only
SmmLockBoxPeiLib client --, then the platform might not want to include
"UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf" at all (hence
not provide EFI_PEI_SMM_COMMUNICATION_PPI) -- because all of those
restores would be serviced by direct SMRAM access anyway.

Currently the absence of EFI_PEI_SMM_COMMUNICATION_PPI is not supported by
SmmLockBoxPeiLib, but it's not hard to implement. Handle it the same as
when EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() returns EFI_NOT_STARTED:
restore LockBox data directly from SMRAM.

Suggested-by: Jiewen Yao 
Cc: Jiewen Yao 
Cc: Michael Kinney 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
This allows me to drop the null implementation of
EFI_PEI_SMM_COMMUNICATION_PPI from the patch "OvmfPkg: add PEIM for
providing TSEG-as-SMRAM during PEI".

 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index bd3204b..cea1fed 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -563,7 +563,10 @@ RestoreLockBox (
  (VOID **)
  );
   if (EFI_ERROR (Status)) {
-return EFI_NOT_STARTED;
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status));
+Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib RestoreLockBox - Exit (%r)\n", 
Status));
+return Status;
   }
 
   //
@@ -682,7 +685,10 @@ RestoreAllLockBoxInPlace (
  (VOID **)
  );
   if (EFI_ERROR (Status)) {
-return EFI_NOT_STARTED;
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status));
+Status = InternalRestoreAllLockBoxInPlaceFromSmram ();
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib RestoreAllLockBoxInPlace - Exit 
(%r)\n", Status));
+return Status;
   }
 
   //
-- 
1.8.3.1

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

Re: [edk2] Armv8 64bit: System error booting linux from the UEFI

[Vladimir Olovyannikov]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Friday, November 13, 2015 3:00 AM
> To: Vladimir Olovyannikov
> Cc: edk2-devel@lists.01.org
> Subject: Re: Armv8 64bit: System error booting linux from the UEFI
> 
(removed extra text)

> >> This is a firmware problem, not a kernel problem. The kernel is
> >> entered with an pending asynchronous external abort, which may have
> >> several causes. You need to instrument the firmware (i.e., unmask the
> >> aborts earlier and run in the debugger) to get a feeling for what is
> >> going on there. It could be something like speculative accesses to
> >> unassigned physical ranges that are mapped cacheable inadvertently,
> >> but it could be lots of other things as well.
> >
> >
> 
> FYI http://article.gmane.org/gmane.comp.bios.edk2.devel/4246
> 
> This fixes a bug that could potentially result in speculative reads to
> device regions. We have also made some other changes regarding
> cacheability and shareability recently (and the patch above will
> likely go in today) so it is probably worthwhile to make sure you are
> based on the latest upstream.

Thank you Ard, I got the latest upstream. Same issue... It is not related to 
the cache.

I investigated and found that as soon as I do
msr DAIFClr, #4 (or call ArmEnableAsynchronousAbort()) anywhere in the UEFI, 
I immediately get this exception.

Here is slightly modified efi_entry.S (immediately on ENTRY):

ENTRY(efi_stub_entry)
0x995a440   msr DAIFClr, #4 <-- crashes here right away
/*
 * Create a stack frame to save FP/LR with extra space
 * for image_addr variable passed to efi_entry().
 */
0x995a444 stp   x29, x30, [sp, #-32]!
...

So for me just performing this instruction causes SError immediately.
Here is the excerpt from the UEFI exception handler on this command:

SError Exception at 0xA995A444

  X0 0xBEDB8798   X1 0xBEFE0018   X2 0xA995A440   X3 
0x00B0
  X4 0xBEDB8598   X5 0xBF4BDE8C   X6 0x005C   X7 
0x
  X8 0xB986DF9C   X9 0xB9F8  X10 0x  X11 
0xD800
 X12 0xDC00  X13 0x200C  X14 0x0003  X15 
0x001D
 X16 0xB5B0  X17 0x0A880C885C60448A  X18 0x0A880C88  X19 
0xBF4D0780
 X20 0x00B8  X21 0xB986DF9B  X22 0xBE31A040  X23 
0xB98E727A
 X24 0xB98E7274  X25 0xB998D7A4  X26 0x  X27 
0x
 X28 0x   FP 0xB630   LR 0xBF4A36C0  
...


  
 SP 0xB5B0  ELR 0xA995A444  SPSR 0x6209  FPSR 
0x  
   
 ESR 0xBF00  FAR 0x 

  


  
 ESR : EC 0x2F  IL 0x1  ISS 0x0100  

  
ASSERT  
/uefi/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c(186):
 ((BOOLEAN)(0==1))

The boot sequence is BL2->BL3.1->UEFI (plus grub.efi app)->Linux
UEFI is running in EL2.
Maybe I am missing some initialization in the UEFI and that is the reason any 
msr DAIFClr,#4 causes this SError exception?

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

Re: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for Python 2.7.10.

[Bjorge, Erik C]

Daryl,

It actually looks like you are adding a two blank lines instead of removing 
them.  Is the commit message wrong?

Thanks,
-Erik

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daryl 
McDaniel
Sent: Thursday, November 12, 2015 2:53 PM
To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
Erik C 
Subject: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for 
Python 2.7.10.

AppPkg/Python-2.7.10: Update file for Python-2.7.10 inclusion.

Add copyright message.
Remove some redundant blank lines.
Remove a superfluous call to setup_confname_tables(m) from INITFUNC().

Signed-off-by: Daryl McDaniel 
---
 .../Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git 
a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c 
b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
index 4cf09ca..42a9aea 100644
--- a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
+++ b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
@@ -2,6 +2,7 @@
 OS-specific module implementation for EDK II and UEFI.
 Derived from posixmodule.c in Python 2.7.2.

+Copyright (c) 2015, Daryl McDaniel. All rights reserved.
 Copyright (c) 2011 - 2012, 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 that accompanies this 
distribution.
@@ -2248,6 +2249,7 @@ edk2_getpid(PyObject *self, PyObject *noargs)
 return PyLong_FromPid(getpid());
 }

+
 #ifdef HAVE_GETLOGIN
 PyDoc_STRVAR(edk2_getlogin__doc__,
 "getlogin() -> string\n\n\
@@ -2340,7 +2342,6 @@ PyDoc_STRVAR(edk2_popen__doc__,
 "popen(command [, mode='r' [, bufsize]]) -> pipe\n\n\
 Open a pipe to/from a command returning a file object.");

-/* standard posix version of popen() support */
 static PyObject *
 edk2_popen(PyObject *self, PyObject *args)
 {
@@ -2369,6 +2370,7 @@ edk2_popen(PyObject *self, PyObject *args)

 #endif /* HAVE_POPEN */

+
 #if defined(HAVE_WAIT3) || defined(HAVE_WAIT4)
 static PyObject *
 wait_helper(pid_t pid, int status, struct rusage *ru)
@@ -4187,9 +4189,6 @@ INITFUNC(void)
 if (all_ins(m))
 return;

-if (setup_confname_tables(m))
-return;
-
 Py_INCREF(PyExc_OSError);
 PyModule_AddObject(m, "error", PyExc_OSError);

--
1.9.5.msysgit.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 v4 13/41] OvmfPkg: LockBox: -D SMM_REQUIRE excludes our fake lockbox

[Laszlo Ersek]

On 11/11/15 22:25, Jordan Justen wrote:
> On 2015-11-03 13:00:49, Laszlo Ersek wrote:
>> When the user builds OVMF with -D SMM_REQUIRE, our LockBox implementation
>> must not be used, since it doesn't actually protect data in the LockBox
>> from the runtime guest OS. Add an according assert to
>> LockBoxLibInitialize().
>>
>> Furthermore, since our LockBox must not be selected with -D SMM_REQUIRE,
>> it makes sense to set aside memory for it only if -D SMM_REQUIRE is
>> absent. Modify InitializeRamRegions() accordingly.
>>
>> This patch completes the -D SMM_REQUIRE-related tweaking of the special
>> OVMF memory areas.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek 
>> ---
>>  OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf |  3 ++
>>  OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  |  3 ++
>>  OvmfPkg/Library/LockBoxLib/LockBoxLib.c   |  2 +
> 
> It seems like the LockBoxLib changes fit better with either the next
> patch, or in a patch of their own.
> 
> With those move into a new patch, or into patch 14
> 
> 13-14 Reviewed-by: Jordan Justen 
> 
> (+ the possible new patch.)

I split this patch in two:
- OvmfPkg: LockBoxLib: -D SMM_REQUIRE excludes our fake lockbox

  OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf | 3 +++
  OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf  | 3 +++
  OvmfPkg/Library/LockBoxLib/LockBoxLib.c   | 2 ++
  3 files changed, 8 insertions(+)

- OvmfPkg: PlatformPei: don't allocate fake lockbox if SMM_REQUIRE

  OvmfPkg/PlatformPei/MemDetect.c | 40 +---
  1 file changed, 21 insertions(+), 19 deletions(-)

I added your R-b to both. (Also to the next patch in the series.)

Thanks!
Laszlo

> 
>>  OvmfPkg/PlatformPei/MemDetect.c   | 40 ++--
>>  4 files changed, 29 insertions(+), 19 deletions(-)
>>
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> index 7203d07..81c893e 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf
>> @@ -42,3 +42,6 @@ [LibraryClasses]
>>  [Pcd]
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +[FeaturePcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> index a4d27a5..08973a4 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
>> @@ -43,3 +43,6 @@ [LibraryClasses]
>>  [Pcd]
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
>>gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
>> +
>> +[FeaturePcd]
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
>> diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c 
>> b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> index 89050ac..45481b9 100644
>> --- a/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> +++ b/OvmfPkg/Library/LockBoxLib/LockBoxLib.c
>> @@ -44,6 +44,8 @@ LockBoxLibInitialize (
>>  {
>>UINTN NumEntries;
>>  
>> +  ASSERT (!FeaturePcdGet (PcdSmmSmramRequire));
>> +
>>if (PcdGet32 (PcdOvmfLockBoxStorageSize) < sizeof (LOCK_BOX_GLOBAL)) {
>>  return RETURN_UNSUPPORTED;
>>}
>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c 
>> b/OvmfPkg/PlatformPei/MemDetect.c
>> index 1bdc2df..455fcbb 100644
>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>> @@ -407,25 +407,27 @@ InitializeRamRegions (
>>}
>>  
>>if (mBootMode != BOOT_ON_S3_RESUME) {
>> -//
>> -// Reserve the lock box storage area
>> -//
>> -// Since this memory range will be used on S3 resume, it must be
>> -// reserved as ACPI NVS.
>> -//
>> -// If S3 is unsupported, then various drivers might still write to the
>> -// LockBox area. We ought to prevent DXE from serving allocation 
>> requests
>> -// such that they would overlap the LockBox storage.
>> -//
>> -ZeroMem (
>> -  (VOID*)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> -  (UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize)
>> -  );
>> -BuildMemoryAllocationHob (
>> -  (EFI_PHYSICAL_ADDRESS)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageBase),
>> -  (UINT64)(UINTN) PcdGet32 (PcdOvmfLockBoxStorageSize),
>> -  mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>> -  );
>> +if (!FeaturePcdGet (PcdSmmSmramRequire)) {
>> +  //
>> +  // Reserve the lock box storage area
>> +  //
>> +  // Since this memory range will be used on S3 resume, it must be
>> +  // reserved as ACPI NVS.
>> +  //
>> +  // If S3 is unsupported, then various drivers might still write to the
>> +  // LockBox area. We ought to prevent DXE from serving allocation 
>> requests
>> +  // such that they would overlap the LockBox storage.
>> + 

Re: [edk2] [PATCH 2/3] AppPkg/Python-2.7.10/edk2module.c: Rename posix_ to edk2_.

[Bjorge, Erik C]

Reviewed-by: Erik Bjorge 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daryl 
McDaniel
Sent: Thursday, November 12, 2015 2:53 PM
To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
Erik C 
Subject: [edk2] [PATCH 2/3] AppPkg/Python-2.7.10/edk2module.c: Rename posix_ to 
edk2_.

AppPkg/Python-2.7.10: Rename identifiers beginning with "posix_" to "edk2_".

Rename identifiers to have an edk2_ prefix instead of posix_ in order to
conform to the convention used in other environment-specific Python modules.
This also makes the names match the module instead of implying that these are
Posix compatible routines.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daryl McDaniel 
---
 .../PyMod-2.7.10/Modules/edk2module.c  | 794 ++---
 1 file changed, 397 insertions(+), 397 deletions(-)

diff --git 
a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c 
b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
index d9d4916..4cf09ca 100644
--- a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
+++ b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
@@ -201,19 +201,19 @@ convertenviron(void)
 /* Set a POSIX-specific error from errno, and return NULL */

 static PyObject *
-posix_error(void)
+edk2_error(void)
 {
 return PyErr_SetFromErrno(PyExc_OSError);
 }
 static PyObject *
-posix_error_with_filename(char* name)
+edk2_error_with_filename(char* name)
 {
 return PyErr_SetFromErrnoWithFilename(PyExc_OSError, name);
 }


 static PyObject *
-posix_error_with_allocated_filename(char* name)
+edk2_error_with_allocated_filename(char* name)
 {
 PyObject *rc = PyErr_SetFromErrnoWithFilename(PyExc_OSError, name);
 PyMem_Free(name);
@@ -224,7 +224,7 @@ posix_error_with_allocated_filename(char* name)

 #ifndef UEFI_C_SOURCE
   static PyObject *
-  posix_fildes(PyObject *fdobj, int (*func)(int))
+  edk2_fildes(PyObject *fdobj, int (*func)(int))
   {
   int fd;
   int res;
@@ -232,19 +232,19 @@ posix_error_with_allocated_filename(char* name)
   if (fd < 0)
   return NULL;
   if (!_PyVerify_fd(fd))
-  return posix_error();
+  return edk2_error();
   Py_BEGIN_ALLOW_THREADS
   res = (*func)(fd);
   Py_END_ALLOW_THREADS
   if (res < 0)
-  return posix_error();
+  return edk2_error();
   Py_INCREF(Py_None);
   return Py_None;
   }
 #endif  /* UEFI_C_SOURCE */

 static PyObject *
-posix_1str(PyObject *args, char *format, int (*func)(const char*))
+edk2_1str(PyObject *args, char *format, int (*func)(const char*))
 {
 char *path1 = NULL;
 int res;
@@ -255,14 +255,14 @@ posix_1str(PyObject *args, char *format, int 
(*func)(const char*))
 res = (*func)(path1);
 Py_END_ALLOW_THREADS
 if (res < 0)
-return posix_error_with_allocated_filename(path1);
+return edk2_error_with_allocated_filename(path1);
 PyMem_Free(path1);
 Py_INCREF(Py_None);
 return Py_None;
 }

 static PyObject *
-posix_2str(PyObject *args,
+edk2_2str(PyObject *args,
char *format,
int (*func)(const char *, const char *))
 {
@@ -279,7 +279,7 @@ posix_2str(PyObject *args,
 PyMem_Free(path2);
 if (res != 0)
 /* XXX how to report both path1 and path2??? */
-return posix_error();
+return edk2_error();
 Py_INCREF(Py_None);
 return Py_None;
 }
@@ -486,7 +486,7 @@ fill_time(PyObject *v, int index, time_t sec, unsigned long 
nsec)
 }

 /* pack a system stat C structure into the Python stat tuple
-   (used by posix_stat() and posix_fstat()) */
+   (used by edk2_stat() and edk2_fstat()) */
 static PyObject*
 _pystat_fromstructstat(STRUCT_STAT *st)
 {
@@ -556,7 +556,7 @@ _pystat_fromstructstat(STRUCT_STAT *st)
 }

 static PyObject *
-posix_do_stat(PyObject *self, PyObject *args,
+edk2_do_stat(PyObject *self, PyObject *args,
   char *format,
   int (*statfunc)(const char *, STRUCT_STAT *),
   char *wformat,
@@ -578,7 +578,7 @@ posix_do_stat(PyObject *self, PyObject *args,
 Py_END_ALLOW_THREADS

 if (res != 0) {
-result = posix_error_with_filename(pathfree);
+result = edk2_error_with_filename(pathfree);
 }
 else
 result = _pystat_fromstructstat();
@@ -589,7 +589,7 @@ posix_do_stat(PyObject *self, PyObject *args,

 /* POSIX methods */

-PyDoc_STRVAR(posix_access__doc__,
+PyDoc_STRVAR(edk2_access__doc__,
 "access(path, mode) -> True if granted, False otherwise\n\n\
 Use the real uid/gid to test for access to a path.  Note that most\n\
 operations will use the effective uid/gid, therefore this routine can\n\
@@ -598,7 +598,7 @@ specified access to the path.  The mode argument can be 
F_OK to test\n\
 existence, or the 

Re: [edk2] [PATCH v4 07/41] OvmfPkg: add PEIM for providing TSEG-as-SMRAM during PEI

[Laszlo Ersek]

On 11/12/15 04:33, Yao, Jiewen wrote:
> Thanks Jorden, for your reminder.
> 
> Hi Laszlo
> Per my understand, you add NULL implementation for PiSmmCommunicationPpi, 
> just because SmmLockBoxPeiLib depends on it. Right?

Yes.

> How  about we update SmmLockBoxPeiLib to call 
> InternalRestoreLockBoxFromSmram(), if PiSmmCommunicationPpi is not located?
> ==
>   Status = PeiServicesLocatePpi (
>  ,
>  0,
>  NULL,
>  (VOID **)
>  );
>   if (EFI_ERROR (Status)) {
> -return EFI_NOT_STARTED;
> +return = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
>   }
> ==
> 
> SmmAccess is design to be silicon specific driver. I really do not want to 
> let it touch generic infrastructure code like SmmCommunication.
> 
> Would you please do me a favor to try if it works?
> If yes, we can update SmmLockBoxPeiLib.
> If no, please let us know where is wrong, and we can try to figure out other 
> better way.

Your suggestion works very well, thanks for it.

I just submitted it as a separate patch:

  [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without
  EFI_PEI_SMM_COMMUNICATION_PPI

  http://thread.gmane.org/gmane.comp.bios.edk2.devel/4283

If you think it is fine, I can commit it (please review it in its own
thread), and then I can drop the SmmCommunication PPI null
implementation from this OVMF patch. (I've tested S3 that way.)

Thanks!
Laszlo

> 
> Thank you
> Yao Jiewen
> 
> -Original Message-
> From: Justen, Jordan L 
> Sent: Thursday, November 12, 2015 6:41 AM
> To: Kinney, Michael D; Yao, Jiewen; Laszlo Ersek
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH v4 07/41] OvmfPkg: add PEIM for providing 
> TSEG-as-SMRAM during PEI
> 
> On 2015-11-05 07:45:21, Kinney, Michael D wrote:
>> Laszlo,
>>
>> Thanks for the details.  Your implementation looks good.
>>
>> I will discuss this further with Jiewen to see if it makes sense to 
>> provide additional implementations of this PPI that could be shared by 
>> multiple platforms.
> 
> Mike, Jiewen,
> 
> Can you guys review Laszlo's patches 7-9?
> 
> Obviously I'd like to avoid adding forked versions of any modules under OVMF 
> if the goal had been to have common modules that should work for all 
> platforms.
> 
> You guys probably know better if we ought to be able to use common modules 
> instead. If the common modules don't work, then hopefully it is something we 
> can fix.
> 
> Laszlo,
> 
> Maybe it would be good to note in the commit message the common module 
> location that were forced to fork, and why you needed to fork it.
> 
> -Jordan
> 
>>
>>> -Original Message-
>>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
>>> Of Laszlo Ersek
>>> Sent: Thursday, November 05, 2015 3:15 AM
>>> To: Kinney, Michael D; edk2-de...@ml01.01.org
>>> Cc: Justen, Jordan L
>>> Subject: Re: [edk2] [PATCH v4 07/41] OvmfPkg: add PEIM for providing 
>>> TSEG- as-SMRAM during PEI
>>>
>>> On 11/05/15 02:32, Kinney, Michael D wrote:
 Laszlo,

 For the EFI_PEI_COMMUNICATION_PPI, is there a reason you are not 
 using UefiCpuPkg\PiSmmCommunication\PiSmmCommunicationPei.inf to 
 produce
>>> that PPI?
>>>
>>> Yes.
>>>
>>> When I wrote this patch originally on April 27th and the days after, 
>>> there was no EFI_PEI_COMMUNICATION_PPI implementation in edk2; 
>>> neither were plans known to add one.
>>>
>>> In addition, we had discussed the behavior of SmmLockBoxPeiLib at 
>>> length, in an off-list thread that you had been CC'd on. Please 
>>> search your archives for the message with the following metadata, and 
>>> the sub-thread rooted in it:
>>>
>>> Message-ID: <553f96cc.9050...@redhat.com>
>>> Date:   Tue, 28 Apr 2015 16:18:52 +0200
>>> From:   Laszlo Ersek 
>>> To: Yao, Jiewen 
>>> CC: Justen, Jordan L ,
>>>Zimmer, Vincent ,
>>>Kinney, Michael D ,
>>>Paolo Bonzini ,
>>>Gerd Hoffmann ,
>>>Michael S. Tsirkin 
>>> Subject:Re: open-sourcing Intel's "IA32FamilyCpuPkg/PiSmmCpuDxeSmm"
>>>
>>> In that message, I asked:
>>>
 In the whitepaper entitled "A Tour Beyond BIOS: Implementing S3 
 Resume with EDKII", page 25 says, under the heading "PEI Instance":

 The PEI instance of SmmLockboxLib has two ways to communicate with 
 the LockBox in SMRAM.

 1) It uses the SMM_COMMUNICATION PPI to communicate the SmmLockbox 
 service provider, similar as DXE instance.
 2) When the PEI instance is used before SMM ready, the 
 SMM_COMMUNICATION PPI will return EFI_NOT_STARTED. In this case, 
 PEI SmmLockBoxLib needs to search the SMRAM region directly to find 
 LockBox content.

 The question is why 

Re: [edk2] [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without EFI_PEI_SMM_COMMUNICATION_PPI

[Yao, Jiewen]

Good. Thanks!
Reviewed by: jiewen@intel.com



-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Saturday, November 14, 2015 4:52 AM
To: edk2-de...@ml01.01.org
Cc: Yao, Jiewen; Kinney, Michael D; Justen, Jordan L
Subject: [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without 
EFI_PEI_SMM_COMMUNICATION_PPI

The RestoreLockBox() and RestoreAllLockBoxInPlace() functions handle the case 
when EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() returns
EFI_NOT_STARTED: they access the SMRAM directly, for restoring LockBox data.

This occurs if a PEIM needs to restore LockBox data *before* the SMBASE is 
relocated and the SMI handler is installed for all processors.

One such PEIM is UefiCpuPkg/Universal/Acpi/S3Resume2Pei. On the S3 resume path, 
in function S3RestoreConfig2(), LockBox data are restored *before* the 
SmmRestoreCpu() function of UefiCpuPkg/PiSmmCpuDxeSmm is called via
SmmS3ResumeState->SmmS3ResumeEntryPoint. (The latter SmmRestoreCpu()
function is responsible for the SMBASE relocation.)

If a platform knows that its PEIMs restore LockBox data *only* before SMBASE 
relocation -- e.g., due to S3Resume2Pei being the platform's only 
SmmLockBoxPeiLib client --, then the platform might not want to include 
"UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf" at all (hence not 
provide EFI_PEI_SMM_COMMUNICATION_PPI) -- because all of those restores would 
be serviced by direct SMRAM access anyway.

Currently the absence of EFI_PEI_SMM_COMMUNICATION_PPI is not supported by 
SmmLockBoxPeiLib, but it's not hard to implement. Handle it the same as when 
EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() returns EFI_NOT_STARTED:
restore LockBox data directly from SMRAM.

Suggested-by: Jiewen Yao 
Cc: Jiewen Yao 
Cc: Michael Kinney 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---

Notes:
This allows me to drop the null implementation of
EFI_PEI_SMM_COMMUNICATION_PPI from the patch "OvmfPkg: add PEIM for
providing TSEG-as-SMRAM during PEI".

 MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
index bd3204b..cea1fed 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c
@@ -563,7 +563,10 @@ RestoreLockBox (
  (VOID **)
  );
   if (EFI_ERROR (Status)) {
-return EFI_NOT_STARTED;
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status));
+Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib RestoreLockBox - Exit (%r)\n", 
Status));
+return Status;
   }
 
   //
@@ -682,7 +685,10 @@ RestoreAllLockBoxInPlace (
  (VOID **)
  );
   if (EFI_ERROR (Status)) {
-return EFI_NOT_STARTED;
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status));
+Status = InternalRestoreAllLockBoxInPlaceFromSmram ();
+DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib RestoreAllLockBoxInPlace - Exit 
(%r)\n", Status));
+return Status;
   }
 
   //
--
1.8.3.1

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

Re: [edk2] Armv8 64bit: System error booting linux from the UEFI

[Mark Rutland]

On Fri, Nov 13, 2015 at 10:39:34PM +, Vladimir Olovyannikov wrote:
> 
> 
> > -Original Message-
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: Friday, November 13, 2015 3:00 AM
> > To: Vladimir Olovyannikov
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: Armv8 64bit: System error booting linux from the UEFI
> > 
> (removed extra text)
> 
> > >> This is a firmware problem, not a kernel problem. The kernel is
> > >> entered with an pending asynchronous external abort, which may have
> > >> several causes. You need to instrument the firmware (i.e., unmask the
> > >> aborts earlier and run in the debugger) to get a feeling for what is
> > >> going on there. It could be something like speculative accesses to
> > >> unassigned physical ranges that are mapped cacheable inadvertently,
> > >> but it could be lots of other things as well.
> > >
> > >
> > 
> > FYI http://article.gmane.org/gmane.comp.bios.edk2.devel/4246
> > 
> > This fixes a bug that could potentially result in speculative reads to
> > device regions. We have also made some other changes regarding
> > cacheability and shareability recently (and the patch above will
> > likely go in today) so it is probably worthwhile to make sure you are
> > based on the latest upstream.
> 
> Thank you Ard, I got the latest upstream. Same issue... It is not related to 
> the cache.
> 
> I investigated and found that as soon as I do
> msr DAIFClr, #4 (or call ArmEnableAsynchronousAbort()) anywhere in the UEFI, 
> I immediately get this exception.

This implies that either code very early in EDK2, or code prior to EDK2 is the
problem, given that when masked, SError will pend until it is unmasked
(whereupon it should be taken).

What is the earliest point in EDK2 that you have unmasked SError?

Are you doing this in PrePeiCoreEntryPoint.S, or later?

> The boot sequence is BL2->BL3.1->UEFI (plus grub.efi app)->Linux

I take it per the naming that you are running ARM Trusted Firmware.

Are you able to unmask SError during BL2 or BL3.1, and if so, does it fire
prior to entering EDK2?

[...]

> UEFI is running in EL2.
> Maybe I am missing some initialization in the UEFI and that is the reason any
> msr DAIFClr,#4 causes this SError exception?

It is possible that some initialisation is missing at an early stage in the
boot sequence, and that this contributes ot the SError.

As an asynchronous exception, SError pends until it is unmasked, and should
trigger as soon as it is unmasked. The DAIFClr itself is vanishingly unlikely
to be the root cause of the SError, but rather makes it apparent.

The earlier you are able to unmask SError, the closer you will be able to get
to the root cause. Are you able to capture it were it earlier in the boot
sequence (e.g. can you register a handler earlier, or capture the exception
with a hardware debugger)?

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

Re: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for Python 2.7.10.

[Daryl McDaniel]

Hi Erik,

The commit message is wrong.
I'll fix it when I do the actual commit.

Daryl McDaniel

-Original Message-
From: Bjorge, Erik C [mailto:erik.c.bjo...@intel.com] 
Sent: Friday, November 13, 2015 2:10 PM
To: Daryl McDaniel ; edk2-devel@lists.01.org; 
Carsey, Jaben 
Subject: RE: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for 
Python 2.7.10.

Daryl,

It actually looks like you are adding a two blank lines instead of removing 
them.  Is the commit message wrong?

Thanks,
-Erik

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daryl 
McDaniel
Sent: Thursday, November 12, 2015 2:53 PM
To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
Erik C 
Subject: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for 
Python 2.7.10.

AppPkg/Python-2.7.10: Update file for Python-2.7.10 inclusion.

Add copyright message.
Remove some redundant blank lines.
Remove a superfluous call to setup_confname_tables(m) from INITFUNC().

Signed-off-by: Daryl McDaniel 
---
 .../Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git 
a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
index 4cf09ca..42a9aea 100644
--- a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
+++ b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
@@ -2,6 +2,7 @@
 OS-specific module implementation for EDK II and UEFI.
 Derived from posixmodule.c in Python 2.7.2.

+Copyright (c) 2015, Daryl McDaniel. All rights reserved.
 Copyright (c) 2011 - 2012, 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 that accompanies this 
distribution.
@@ -2248,6 +2249,7 @@ edk2_getpid(PyObject *self, PyObject *noargs)
 return PyLong_FromPid(getpid());
 }

+
 #ifdef HAVE_GETLOGIN
 PyDoc_STRVAR(edk2_getlogin__doc__,
 "getlogin() -> string\n\n\
@@ -2340,7 +2342,6 @@ PyDoc_STRVAR(edk2_popen__doc__,
 "popen(command [, mode='r' [, bufsize]]) -> pipe\n\n\
 Open a pipe to/from a command returning a file object.");

-/* standard posix version of popen() support */
 static PyObject *
 edk2_popen(PyObject *self, PyObject *args)
 {
@@ -2369,6 +2370,7 @@ edk2_popen(PyObject *self, PyObject *args)

 #endif /* HAVE_POPEN */

+
 #if defined(HAVE_WAIT3) || defined(HAVE_WAIT4)
 static PyObject *
 wait_helper(pid_t pid, int status, struct rusage *ru)
@@ -4187,9 +4189,6 @@ INITFUNC(void)
 if (all_ins(m))
 return;

-if (setup_confname_tables(m))
-return;
-
 Py_INCREF(PyExc_OSError);
 PyModule_AddObject(m, "error", PyExc_OSError);

--
1.9.5.msysgit.1

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

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

Re: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for Python 2.7.10.

[Bjorge, Erik C]

Reviewed-by: Erik Bjorge 

-Original Message-
From: Daryl McDaniel [mailto:edk2-li...@mc2research.org] 
Sent: Friday, November 13, 2015 3:24 PM
To: Bjorge, Erik C ; edk2-devel@lists.01.org; Carsey, 
Jaben 
Subject: RE: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for 
Python 2.7.10.

Hi Erik,

The commit message is wrong.
I'll fix it when I do the actual commit.

Daryl McDaniel

-Original Message-
From: Bjorge, Erik C [mailto:erik.c.bjo...@intel.com] 
Sent: Friday, November 13, 2015 2:10 PM
To: Daryl McDaniel ; edk2-devel@lists.01.org; 
Carsey, Jaben 
Subject: RE: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for 
Python 2.7.10.

Daryl,

It actually looks like you are adding a two blank lines instead of removing 
them.  Is the commit message wrong?

Thanks,
-Erik

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Daryl 
McDaniel
Sent: Thursday, November 12, 2015 2:53 PM
To: edk2-devel@lists.01.org; Carsey, Jaben ; Bjorge, 
Erik C 
Subject: [edk2] [PATCH 3/3] AppPkg/Python-2.7.10/edk2module.c: Update for 
Python 2.7.10.

AppPkg/Python-2.7.10: Update file for Python-2.7.10 inclusion.

Add copyright message.
Remove some redundant blank lines.
Remove a superfluous call to setup_confname_tables(m) from INITFUNC().

Signed-off-by: Daryl McDaniel 
---
 .../Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git 
a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
index 4cf09ca..42a9aea 100644
--- a/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
+++ b/AppPkg/Applications/Python/Python-2.7.10/PyMod-2.7.10/Modules/edk2module.c
@@ -2,6 +2,7 @@
 OS-specific module implementation for EDK II and UEFI.
 Derived from posixmodule.c in Python 2.7.2.

+Copyright (c) 2015, Daryl McDaniel. All rights reserved.
 Copyright (c) 2011 - 2012, 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 that accompanies this 
distribution.
@@ -2248,6 +2249,7 @@ edk2_getpid(PyObject *self, PyObject *noargs)
 return PyLong_FromPid(getpid());
 }

+
 #ifdef HAVE_GETLOGIN
 PyDoc_STRVAR(edk2_getlogin__doc__,
 "getlogin() -> string\n\n\
@@ -2340,7 +2342,6 @@ PyDoc_STRVAR(edk2_popen__doc__,
 "popen(command [, mode='r' [, bufsize]]) -> pipe\n\n\
 Open a pipe to/from a command returning a file object.");

-/* standard posix version of popen() support */
 static PyObject *
 edk2_popen(PyObject *self, PyObject *args)
 {
@@ -2369,6 +2370,7 @@ edk2_popen(PyObject *self, PyObject *args)

 #endif /* HAVE_POPEN */

+
 #if defined(HAVE_WAIT3) || defined(HAVE_WAIT4)
 static PyObject *
 wait_helper(pid_t pid, int status, struct rusage *ru)
@@ -4187,9 +4189,6 @@ INITFUNC(void)
 if (all_ins(m))
 return;

-if (setup_confname_tables(m))
-return;
-
 Py_INCREF(PyExc_OSError);
 PyModule_AddObject(m, "error", PyExc_OSError);

--
1.9.5.msysgit.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] EFI GOP with manual vsync trigger

[Michael Zimmermann]

I think I've made some progress.
Thx to the fact that the only thing stuck is the ARM thread mode and the
timer interrupts are still fired(I verified that my callback is called from
an interrupt, being it a fluke or not) I could easily dump the current PC
of the DxeCore context.

This way I discovered that the code is stuck in the delay functions of the
timerLib because they're not thread safe.

The reason is that a hw counter gets reset and started for this and if this
happens from an interrupt(UART uses delay functions) while the thread mode
is whiting for the counter to reach an specific value it will just wait
forever because the counter gets disabled after the delay.

Code:
usecs = (usecs * 33 + 1000 - 33) / 1000;

writel(0, GPT_CLEAR);
writel(0, GPT_ENABLE);
while (readl(GPT_COUNT_VAL) != 0) ;

writel(GPT_ENABLE_EN, GPT_ENABLE);
while (readl(GPT_COUNT_VAL) < usecs) ;

writel(0, GPT_ENABLE);
writel(0, GPT_CLEAR);

I guess I need to try keeping the counter running and detect when it rolled
over to make it thread-safe.

Michael

On Fri, Nov 13, 2015 at 1:14 AM, Andrew Fish  wrote:

>
> > On Nov 12, 2015, at 11:21 AM, Michael Zimmermann <
> sigmaepsilo...@gmail.com> wrote:
> >
> > thx for this information.
> > I don't have any debug hw but UART.
> >
> > After some careful DEBUG printing and tracing the call stack using
> > '__builtin_return_address(0)'
>
> FYI the edk2 has a portable form of this RETURN_ADDRESS(0)
>
> > I found that the application's dead happens
> > at 'WaitForEvent'
>
> The WaitForEvent() is what happens at the shell prompt when it is waiting
> for keyboard input. So that is functioning as intended. gBS->WaitForEvent()
> is the power efficient way to wait.
>
> If you look at CoreWaitForEvent() it checks all the events, and then calls
> CoreSignalEvent with the gIdleLoopEvent. If the DXE Cpu driver supports
> this feature gIdleLoopEvent will put the CPU in a low power state until the
> next timer tick. Since  CoreWaitForEvent() checked all the event state it
> can not change until the next timer tick so this lets the system spend its
> idle time in a lower power mode.
>
>
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Event/Event.c#L685
>   for(;;) {
>
> for(Index = 0; Index < NumberOfEvents; Index++) {
>
>   Status = CoreCheckEvent (UserEvents[Index]);
>
>   //
>   // provide index of event that caused problem
>   //
>   if (Status != EFI_NOT_READY) {
> if (UserIndex != NULL) {
>   *UserIndex = Index;
> }
> return Status;
>   }
> }
>
> //
> // Signal the Idle event
> //
> CoreSignalEvent (gIdleLoopEvent);
>   }
>
> So if you are really stuck here it could imply your Simple Text In is
> hung? Maybe your issue is data corruption vs an event issue?
>
> > and that there aren't any long running events(it returns
> > from CoreDispatchEventNotifies and CoreRestoreTpl everytime).
> >
> > I'm not sure this is always the case though because without the DEBUG's
> the
> > application can even stop when the edk2 shell is printing the device map
> > and WaitForEvent usually is only used for key input.
> >
> > isn't there some GCC option to save the stacktrace so I can dump it after
> > some seconds runtime(when the system died) and just print the whole
> > backtrace?(like the linux kernel does).
> >
>
> Well this is when it gets tricky. For VC++ you can only _ReturnAddress(),
> and that is equivalent of __builtin_return_address(0) on the GCC side. But
> it gets worse... To get a stack trace for X64 with VC++ you need symbols as
> the unwind is in the .PDB file.
>
> For clang, and I think with the right GCC compiler flags, you can walk the
> stack as each routine saves the frame pointer on entry. This is a simple
> X64 C function that just returns 0. The push of the %rbp and saving %rsp
> allow the stack to be unwound in software.
>
> pushq   %rbp
> movq%rsp, %rbp
> xorl%eax, %eax
> popq%rbp
> retq
>
> So it is possible to unwind the stack in software.
>
> Simple answer is try RETURN_ADDRESS(1), RETURN_ADDRESS(2), etc. If it is
> supported the compiler will generate the code to unwind for you.
>
> Complex answer is write the stack unwind code, if it is possible? Which
> Processor and I assume you are using GCC?
>
> You can use the GetImageName() function in
> https://github.com/tianocore/edk2/blob/master/ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerUefi.c#L64
> to see how you can use the PE/COFF library functions to convert a
> FaultAddress to the ImageBase, and name of the image. If you know the
> offset into the image relative to the start you can load image in gdb from
> the build output directory and see what code it maps to.
>
> The X86 version of all this lives:
> https://github.com/tianocore/edk2/blob/master/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
>
> Thanks,
>
> Andrew 

Re: [edk2] testing Centos 7.1 boot from iscsi with EDK -> kernel crashes after launch by GRUB

[P.A.M. van Dam (Pascal)]

On Thu, Nov 12, 2015 at 07:17:49PM +0100, Laszlo Ersek wrote:

Good afternoon all,

> Please don't drop the list and other CC's from the discussion when the
> initial report is sent to the list. Keeping full context.

My apologies for that. I noticed it when I got your personal out-of-office 
reply. :)


> 
> On 11/11/15 08:38, P.A.M. van Dam (Pascal) wrote:
> > On Mon, Nov 09, 2015 at 10:02:52PM +0100, Laszlo Ersek wrote:
> >> On 11/09/15 14:50, P.A.M. van Dam (Pascal) wrote:
> >>>
> >>> Good day all,
> >>>
> >>> I decided to do some testing with iSCSI boot from within a KVM. 
> >>>
> >>> My setup is a Centos 7.1 KVM host with several Fedora 22 and Centos 7.1 
> >>> guests in it. The iSCSI storage has been configured
> >>> from a LenovoEMC EPX6300 NAS.
> >>>
> >>> 1. I've configured the iSCSI bootlun to be available for the OS install.
> >>> 2. Within the KVM guest OS install I assign the LUN to the Centos 7.1 OS.
> >>> 3. The OS installs without errors ont he iSCSI LUN.
> >>>
> >>> After reboot of the freshly installed OS on the KVM
> >>> I configure the iSCSI device in the OVMF user interface.
> >>>
> >>> 4. The iSCSI BOOT LUN is visible from the UEFI shell.
> >>> 5. When booting from the LUN, GRUB correcly displays the bootmenu.
> >>
> >> Am I to understand that GRUB (and its config) are correctly loaded via
> >> iSCSI?
> > 
> > Yep. You are correct in this. GRUB loads without issue and both the menu 
> > and commandline work. I can even access the kernel and the initrd from 
> > GRUB's commandline.
> > 
> >>
> >>> 6. When selecting and starting a kernel/initrd pair the started kernel 
> >>> quickly crashes reporting a fault.
> >>>
> >>> My questions;
> >>>
> >>> 1. Is iSCSI boot support in the OVMF/EDKII currrently usable?
> > 
> > 
> >>
> >> I never tested it, but I guess it should work (for some value of
> >> "work"). Since I never tested it, I can't say what code in OvmfPkg would
> >> require specific support (perhaps the boot order library... dunno).
> >>
> >>> 2. Can I help in further analyzing the issue? It could be an issue in 
> >>> EDKII (drivers?), in grub or in the Linux kernel.
> >>
> >> Your email is missing almost all of the important details, so let's
> >> start again. :)
> > 
> > I will try to supply the info needed. This is my first time on this 
> > mailinglist and it's long time since I've joined a Linux project.
> > 
> >>
> >> - Are you using libvirt? If so, what version? What is your domain XML?
> >>
> > 
> > Yes, I am using libvirt. I am currently using 
> > libvirt-1.2.8-16.el7_1.5.x86_64.
> > 
> > [virtlabn17.xml]
> > 
> > [root@kallista:38 qemu]# cat virtlabn17.xml
> > 
> > 
> > 
> >   virtlabn17
> >   ab2b120f-8a4f-4f90-bd32-fa78fedd51b0
> >   Centos 7.1 UEFI/ISCSI BOOT virtlabn17
> >   2097152
> >   2097152
> 
> Okay, so 2GB RAM.
> 
> >   2
> 
> 2 VCPUs
> 
> >   
> > hvm
> >  > type='pflash'>/usr/share/edk2.git/ovmf-x64/OVMF_CODE-pure-efi.fd
> >  > template='/usr/share/edk2.git/ovmf-x64/OVMF_VARS-pure-efi.fd'>/var/lib/libvirt/qemu/nvram/virtlabn17_VARS.fd
> 
> Looks good. The OVMF binary is from Gerd.

Correct.

> 
> > 
> 
> This is kinda useless here (you don't have a disk in your config). In
> any case, as a side point, the  elements within device
> elements are much more flexible than this. See
> , and then search
> that page for more occurrences of the string "boot order=". See also the
> "boot" element's docs at
> .

Ok. I will take a look at it. But as far as I know, it's not the cause of the 
issue. :)
> 
> > 
> 
> This will drop you to the edk2 setup page; I guess it's fine.

Correct.

> 
> >   
> >   
> > 
> > 
> > 
> >   
> >   
> > SandyBridge
> >   
> >   
> > 
> > 
> > 
> >   
> >   destroy
> >   restart
> >   restart
> >   
> > /usr/libexec/qemu-kvm
> > 
> >> function='0x7'/>
> > 
> > 
> >   
> >> function='0x0' multifunction='on'/>
> > 
> > 
> >   
> >> function='0x1'/>
> > 
> > 
> >   
> >> function='0x2'/>
> > 
> > 
> > 
> >> function='0x0'/>
> > 
> > 
> >   
> >   
> >   
> >> function='0x0'/>
> > 
> 
> I see. This is the "Bridge to LAN" config
> .
> 
> You are using virtio-net, and the iPXE oprom will be present in the PCI
> ROM BAR.
> 
> Idea (1): not that it might matter a lot, but you could try with the
> builtin virtio-net driver. For that, add the following element within
> :
> 
> 
>

Yes. I implemented Idea(1) and it works! So there definately is something with 
this ROM that cause the
crash. (Looking further in the email, not the crash of the kernel, but earlier 
in the process)

So; conclusion for Idea(1); it's a solution. The system boots fine. It spits 
out some udev messages about 
the dubious use of 

[edk2] Alter OVMF/UEFI iSCSI settings outside of the OVMF UI

[P.A.M. van Dam (Pascal)]

Good morning all,

What I would like to do is pre-configure the NVRAM of the Virtual Machines 
(guests) with the
ISCSI config. So have initiator, iSCSI host and target preconfigured in the 
NVRAM before the guest
gets started. This way, RHEL/CENTOS Kickstart should be able to pickup the 
iSCSI config and use
the UEFI discovered and enabled bootdrive in the configuration.

My questions:

1) Is this possible. And yes, what tool can I use?

2) If not, where can I find the documentation about how the NVRAM is built,
   so I can try to create such a tool myself.

Thanks for your help!

Kind regards,

Pascal van Dam


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

Re: [edk2] ShellPkg: Network commands (ifconfig/ping) broken

[Conen, Johannes]

Hi everyone,

now don't get me wrong, I understand why the shell was written the way it is 
now and that this is probably the cleanest solution. Things change during 
development and not always is backward compatibility the best option or even 
possible. The point I was trying to make is:

- not every UEFI developer follows every change that happens in the dev version 
or the edk2 mailing list
- therefore, if backward-compatibility gets broken, it should be stated in a 
clear and unmistakeable way in the release notes (and not just in a side 
mention): X got removed, therefore Y will break and you have to change Z to 
make it work again
- If certain features only work in or above certain UEFI revisions, this should 
be stated the same way
- instead of immediately removing things and breaking backward compatibility, 
it would be better to mark them as "deprecated", keep them so they are still 
usable and remove them in the next version so developers have time to adapt 
their programs instead of being left with non-working things from one day to 
another. In this case e.g.: Keep the backward compatibility in UDK2015, but 
mention clearly in the release notes that EFI_IP4_CONFIG_PROTOCOL got 
deprecated and will be removed in the next version

The ideal solution, but probably also the most complicated and the one 
involving most work, would be a build-option approach like Samer suggested. 
E.g., you can only use deprecated things with the build option "-unsafe", 
otherwise the compiler will throw an error about using deprecated things. One 
could extend this to UEFI revisions - e.g. if you provide the build option 
"-uefi240", the compiler will throw an error if something is used that is only 
available in an UEFI revision > 2.40 - or isn't available in 2.40 anymore.

Now to something completely unrelated, for which I hope to get an answer 
nevertheless (if I shall "open" a new e-mail "thread", please tell me). One of 
the applications I wanted to include in the shell is made using EADK, 
specifically the socket library. I got it integrated and can run it from the 
shell, however all EADK related things don't seem to work - e.g. if I use 
printf(), nothing happens, the UEFI Print() however works. Is this a mistake on 
my side or does EADK generally not work when used from applications integrated 
into the shell? I saw the know issue in the readme.txt from AppPkg (2.  
Applications must be launched from within the EFI Shell.), but thought running 
it from the shell or integrating it into the shell would make no technical 
difference.

Greetings
Johannes


-Original Message-
From: af...@apple.com [mailto:af...@apple.com] 
Sent: Thursday, November 12, 2015 5:11 AM
To: El-Haj-Mahmoud, Samer
Cc: Conen, Johannes; ler...@redhat.com; jiaxin...@intel.com; 
edk2-de...@ml01.01.org; jaben.car...@intel.com; ting...@intel.com; 
leif.lindh...@linaro.org
Subject: Re: [edk2] ShellPkg: Network commands (ifconfig/ping) broken


> On Nov 11, 2015, at 8:03 PM, El-Haj-Mahmoud, Samer 
>  wrote:
> 
> I remember we specifically discussed this in the UEFI Forum and decided to 
> drop the If4ConfigProtocol compatability from the spec.
> 
> If edk2 would like to keep it, it can be done, but will complicate things. I 
> would recommend a build option to disable /remove the legacy support as new 
> platforms don't care about this in their shell (if the shell is embedded or 
> targets specific platforms).
> 

Samer,

I don't think this is a spec conformance issue. It is easy for a ROM to pick a 
given version of UEFI to conform to. The problem is an application as Johannes 
has pointed out can run on different ROMs that implement different versions of 
UEFI. 

>From an implementation standpoint it should be possible for the Application to 
>check the UEFI revision and "do the correct thing". So I think the objection 
>is the shell was written to strictly conform to the latest UEFI specification, 
>when it could have been made backward compatible with a range of UEFI 
>specification versions. 

Thanks,

Andrew Fish

> 
> Thanks
> Samer
> 
> 
> 
> Sent from my Android phone using Symantec TouchDown (www.symantec.com)
> 
> -Original Message-
> From: Ye, Ting [ting...@intel.com]
> Received: Wednesday, 11 Nov 2015, 7:41PM
> To: Conen, Johannes [johannes.co...@siemens.com]; Laszlo Ersek 
> [ler...@redhat.com]; Wu, Jiaxin [jiaxin...@intel.com]; 
> edk2-devel@lists.01.org [edk2-de...@ml01.01.org]; Carsey, Jaben 
> [jaben.car...@intel.com]
> CC: Leif Lindholm (Linaro address) [leif.lindh...@linaro.org]
> Subject: Re: [edk2] ShellPkg: Network commands (ifconfig/ping) broken
> 
> One solution to support UEFI 2.4 network modules with UEFI 2.5 ifconfig is to 
> add the code back to use HII services to perform setting. The definition of 
> NIC_IP4_CONFIG_INFO and a private GUID gEfiNicIp4ConfigVariableGuid should be 
> added into ShellPkg as UEFI 2.5 no longer keep the definition in 
> MdeModulePkg. This