Re: [edk2] [Patch v3] SecurityPkg OpalPasswordDxe: Error handling enhance when input password.

[Tian, Feng]

Suggest to updated the comment log like below:

1. When the device is unlocked at BIOS phase and system does a warm reboot, the 
device may be still in unlock status if it uses external power. For such case, 
we would still popup password window to ask user input. If user presses ESC key 
here, we would force the system shut down or ask user input again to avoid 
security hole.

Others look good to me
Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric Dong
Sent: Wednesday, May 11, 2016 11:15 AM
To: edk2-devel@lists.01.org
Cc: Tian, Feng 
Subject: [edk2] [Patch v3] SecurityPkg OpalPasswordDxe: Error handling enhance 
when input password.

V2 -> V3: Refine the message in the popup dialog:

1. 
L"Confirm: Not unlock device and continue boot?.",
L"Press ENTER to confirm, Press Esc to input password",   -> L"Press ENTER to 
skip password, Press ESC to input password",

2. 
L"Warning: system in unknown status, must shutdown!",
L"Press ENTER to shutdown.",  -> L"Press ENTER to 
shutdown, Press ESC to input password",

3. Add more explaination in the check in log.



Enhance the error handling:
1. When device use external power and do a hot reboot, device will be in unlock 
status. If user input ESC, force shutdown or return to input password.
2. When user reach max retry count, force shutdown.

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

diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
index 7c6deb8..3764b24 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
@@ -263,6 +263,7 @@ OpalDriverRequestPassword (
   EFI_INPUT_KEY   Key;
   OPAL_SESSIONSession;
   BOOLEAN PressEsc;
+  BOOLEAN Locked;
 
   if (Dev == NULL) {
 return;
@@ -277,33 +278,61 @@ OpalDriverRequestPassword (
 Session.MediaId = Dev->OpalDisk.MediaId;
 Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
 
+Locked = OpalDeviceLocked (>OpalDisk.SupportedAttributes, 
+ >OpalDisk.LockingFeature);
+
 while (Count < MAX_PASSWORD_TRY_COUNT) {
   Password = OpalDriverPopUpHddPassword (Dev, );
   if (PressEsc) {
-//
-// User not input password and press ESC, keep device in lock status 
and continue boot.
-//
-do {
-  CreatePopUp (
-  EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
-  ,
-  L"Confirm: Not unlock device and continue boot?.",
-  L"Press ENTER to confirm, Press Esc to input password",
-  NULL
-  );
-} while ((Key.ScanCode != SCAN_ESC) && (Key.UnicodeChar != 
CHAR_CARRIAGE_RETURN));
-
-if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
-  gST->ConOut->ClearScreen(gST->ConOut);
+if (Locked) {
   //
-  // Keep lock and continue boot.
+  // Current device in the lock status and
+  // User not input password and press ESC,
+  // keep device in lock status and continue boot.
   //
-  return;
+  do {
+CreatePopUp (
+EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+,
+L"Press ENTER to skip password, Press ESC to input 
password",
+NULL
+);
+  } while ((Key.ScanCode != SCAN_ESC) && (Key.UnicodeChar != 
+ CHAR_CARRIAGE_RETURN));
+
+  if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
+gST->ConOut->ClearScreen(gST->ConOut);
+//
+// Keep lock and continue boot.
+//
+return;
+  } else {
+//
+// Let user input password again.
+//
+continue;
+  }
 } else {
   //
-  // Let user input password again.
+  // Current device in the unlock status and
+  // User not input password and press ESC,
+  // Shutdown the device.
   //
-  continue;
+  do {
+CreatePopUp (
+EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+,
+L"Press ENTER to shutdown, Press ESC to input password",
+NULL
+);
+  } while ((Key.ScanCode != SCAN_ESC) && (Key.UnicodeChar != 
+ CHAR_CARRIAGE_RETURN));
+
+  if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
+gRT->ResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+  } else {
+//
+// Let user input 

Re: [edk2] OVMF boot order and efivars persistence questions

[Gary Lin]

On Tue, May 10, 2016 at 05:29:33PM +0200, Laszlo Ersek wrote:
> On 05/10/16 06:34, Gary Lin wrote:
> > On Mon, May 09, 2016 at 02:58:00PM +0200, Laszlo Ersek wrote:
> >> On 05/09/16 13:39, Thomas Lamprecht wrote:
[snip]
> > Speaking of the boot order, I would like to propose to postpone the
> > registration of the internal shell after 
> > EfiBootManagerRefreshAllBootOption()
> > in PlatformBootManagerAfterConsole(). The reason is to align to the
> > behavior of the old BDS. Besides, Xen doesn't support fw_cfg and pflash,
> > so it is really a headache to change the boot order in Xen :(
> > With the current order, the Xen HVM always ran into the shell instead of
> > DVD or the hard disk if there was no one to interfere. Lowering the
> > priority of shell gives the block devices the chance to boot at least.
> > I know the best solution is to make Xen support both fw_cfg and pflash,
> > but it seems not going to happen in the short term.
> 
> I think this makes sense.
> 
> However, in this case it's not just the UEFI shell, but also the memory
> mapped UiApp application (practically the setup UI). If you move one to
> the end, you should probably move the other one as well.
> 
We don't have to move UiApp. The attribute of UiApp has is 
"LOAD_OPTION_CATEGORY_APP | LOAD_OPTION_ACTIVE | LOAD_OPTION_HIDDEN"
and BootBootOptions will just ignore UiApp. It only will be triggered by
the hotkeys or OsIndications.

> Gary, can you please submit a patch that works for you, CC'ing Ray in
> addition to the OvmfPkg maintainers? The new BDS is, well, pretty new in
> OvmfPkg, so for now I'd like some help from Ray in reviewing such changes.
> 
Ok, I am working on it and will submit a patch later.

> ... The problem I see here is that PlatformBootManagerBeforeConsole()
> *should* be exited with those boot options (and their hotkeys!)
> existing. Namely, right after PlatformBootManagerBeforeConsole() returns
> to BdsEntry(), you can see a call to EfiBootManagerStartHotkeyService().
> I think by that time the hotkeys (and their boot options) must be in
> place already.
> 
> So, I believe the right to do is *not* to postpone the registration
> until AfterConsole. Instead, how about de-registering those two options
> right before EfiBootManagerRefreshAllBootOption(), and re-adding them
> right after EfiBootManagerRefreshAllBootOption()?
> 
> Also, since this may have a performance impact, I would prefer if this
> de-register / re-register logic was restricted to Xen only. More
> precisely, call QemuFwCfgIsAvailable() -- it's not about Xen
> specifically; we want to know if SetBootOrderFromQemu() will have a
> chance to do anything.
> 
Since the order of UiApp doesn't matter, we don't have to worry about
the hotkeys. Do you have any other concern about the delayed registration
of the shell?

Thanks,

Gary Lin

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

[edk2] [Patch v3] SecurityPkg OpalPasswordDxe: Error handling enhance when input password.

[Eric Dong]

V2 -> V3: Refine the message in the popup dialog:

1. 
L"Confirm: Not unlock device and continue boot?.",
L"Press ENTER to confirm, Press Esc to input password",   -> L"Press ENTER to 
skip password, Press ESC to input password",

2. 
L"Warning: system in unknown status, must shutdown!",
L"Press ENTER to shutdown.",  -> L"Press ENTER to 
shutdown, Press ESC to input password",

3. Add more explaination in the check in log.



Enhance the error handling:
1. When device use external power and do a hot reboot, device will be
in unlock status. If user input ESC, force shutdown or return to input password.
2. When user reach max retry count, force shutdown.

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

diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
index 7c6deb8..3764b24 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
@@ -263,6 +263,7 @@ OpalDriverRequestPassword (
   EFI_INPUT_KEY   Key;
   OPAL_SESSIONSession;
   BOOLEAN PressEsc;
+  BOOLEAN Locked;
 
   if (Dev == NULL) {
 return;
@@ -277,33 +278,61 @@ OpalDriverRequestPassword (
 Session.MediaId = Dev->OpalDisk.MediaId;
 Session.OpalBaseComId = Dev->OpalDisk.OpalBaseComId;
 
+Locked = OpalDeviceLocked (>OpalDisk.SupportedAttributes, 
>OpalDisk.LockingFeature);
+
 while (Count < MAX_PASSWORD_TRY_COUNT) {
   Password = OpalDriverPopUpHddPassword (Dev, );
   if (PressEsc) {
-//
-// User not input password and press ESC, keep device in lock status 
and continue boot.
-//
-do {
-  CreatePopUp (
-  EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
-  ,
-  L"Confirm: Not unlock device and continue boot?.",
-  L"Press ENTER to confirm, Press Esc to input password",
-  NULL
-  );
-} while ((Key.ScanCode != SCAN_ESC) && (Key.UnicodeChar != 
CHAR_CARRIAGE_RETURN));
-
-if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
-  gST->ConOut->ClearScreen(gST->ConOut);
+if (Locked) {
   //
-  // Keep lock and continue boot.
+  // Current device in the lock status and
+  // User not input password and press ESC,
+  // keep device in lock status and continue boot.
   //
-  return;
+  do {
+CreatePopUp (
+EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+,
+L"Press ENTER to skip password, Press ESC to input 
password",
+NULL
+);
+  } while ((Key.ScanCode != SCAN_ESC) && (Key.UnicodeChar != 
CHAR_CARRIAGE_RETURN));
+
+  if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
+gST->ConOut->ClearScreen(gST->ConOut);
+//
+// Keep lock and continue boot.
+//
+return;
+  } else {
+//
+// Let user input password again.
+//
+continue;
+  }
 } else {
   //
-  // Let user input password again.
+  // Current device in the unlock status and
+  // User not input password and press ESC,
+  // Shutdown the device.
   //
-  continue;
+  do {
+CreatePopUp (
+EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
+,
+L"Press ENTER to shutdown, Press ESC to input password",
+NULL
+);
+  } while ((Key.ScanCode != SCAN_ESC) && (Key.UnicodeChar != 
CHAR_CARRIAGE_RETURN));
+
+  if (Key.UnicodeChar == CHAR_CARRIAGE_RETURN) {
+gRT->ResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+  } else {
+//
+// Let user input password again.
+//
+continue;
+  }
 }
   }
 
@@ -313,7 +342,7 @@ OpalDriverRequestPassword (
   }
   PasswordLen = (UINT32) AsciiStrLen(Password);
 
-  if (OpalDeviceLocked (>OpalDisk.SupportedAttributes, 
>OpalDisk.LockingFeature)) {
+  if (Locked) {
 Ret = OpalSupportUnlock(, Password, PasswordLen, 
Dev->OpalDevicePath);
   } else {
 Ret = OpalSupportLock(, Password, PasswordLen, 
Dev->OpalDevicePath);
@@ -349,12 +378,13 @@ OpalDriverRequestPassword (
 CreatePopUp (
 EFI_LIGHTGRAY | EFI_BACKGROUND_BLUE,
 ,
-L"Opal password retry count is expired. Keep lock and continue 
boot.",
-L"Press ENTER to continue",
+

Re: [edk2] [patch] MdeModulePkg/UsbMouseAbsolutePointerDxe: fix VS2015 NOOPT build error

[Qiu, Shumin]

Reviewed-by: Qiu Shumin 
-Original Message-
From: Tian, Feng 
Sent: Wednesday, May 11, 2016 10:58 AM
To: Qiu, Shumin
Cc: edk2-devel@lists.01.org
Subject: [patch] MdeModulePkg/UsbMouseAbsolutePointerDxe: fix VS2015 NOOPT 
build error

Cc: Shumin Qiu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 .../Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c  | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c 
b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
index bf3d853..9fe9244 100644
--- a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
+++ b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
@@ -678,9 +678,9 @@ InitializeUsbMouseDevice (
   // Let the cursor's starting position is in the center of the screen.
   //
   UsbMouseAbsolutePointerDev->State.CurrentX =
-(UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinX) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinX, 2);
   UsbMouseAbsolutePointerDev->State.CurrentY =
-(UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinY) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinY, 2);
 
   //
   // Set boot protocol for the USB mouse.
@@ -942,9 +942,9 @@ UsbMouseAbsolutePointerReset (
   // Let the cursor's starting position is in the center of the screen.
   //
   UsbMouseAbsolutePointerDevice->State.CurrentX =
-(UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinX) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinX, 2);
   UsbMouseAbsolutePointerDevice->State.CurrentY =
-(UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinY) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinY, 2);
 
   UsbMouseAbsolutePointerDevice->StateChanged = FALSE;
 
-- 
2.7.1.windows.2

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

[edk2] [patch] MdeModulePkg/UsbMouseAbsolutePointerDxe: fix VS2015 NOOPT build error

[Feng Tian]

Cc: Shumin Qiu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 .../Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c  | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git 
a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c 
b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
index bf3d853..9fe9244 100644
--- a/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
+++ b/MdeModulePkg/Bus/Usb/UsbMouseAbsolutePointerDxe/UsbMouseAbsolutePointer.c
@@ -678,9 +678,9 @@ InitializeUsbMouseDevice (
   // Let the cursor's starting position is in the center of the screen.
   //
   UsbMouseAbsolutePointerDev->State.CurrentX =
-(UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinX) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinX, 2);
   UsbMouseAbsolutePointerDev->State.CurrentY =
-(UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinY) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDev->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDev->Mode.AbsoluteMinY, 2);
 
   //
   // Set boot protocol for the USB mouse.
@@ -942,9 +942,9 @@ UsbMouseAbsolutePointerReset (
   // Let the cursor's starting position is in the center of the screen.
   //
   UsbMouseAbsolutePointerDevice->State.CurrentX =
-(UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinX) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxX + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinX, 2);
   UsbMouseAbsolutePointerDevice->State.CurrentY =
-(UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinY) / 2;
+DivU64x32 (UsbMouseAbsolutePointerDevice->Mode.AbsoluteMaxY + 
UsbMouseAbsolutePointerDevice->Mode.AbsoluteMinY, 2);
 
   UsbMouseAbsolutePointerDevice->StateChanged = FALSE;
 
-- 
2.7.1.windows.2

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

[edk2] [patch] NetworkPkg: Bug fix of iSCSI to support MPIO

[Zhang Lubo]

If two attempts added on different NIC and enable
MPIO attribute, then change the attempts order. If
both two attempts succeed to connect the target,it
should abort the later one in the order and uninstall
ExtScsiPassThruProtocol Interface, But now it
unistall it twice.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Wu Jiaxin 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
---
 NetworkPkg/IScsiDxe/IScsiDriver.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c 
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 12095cb..5a121ce 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -863,14 +863,26 @@ IScsiStart (
 IScsiRemoveNic (ExistPrivate->Controller);
 if (ExistPrivate->Session != NULL) {
   IScsiSessionAbort (ExistPrivate->Session);
 }
 
-Status = IScsiCleanDriverData (ExistPrivate);
-if (EFI_ERROR (Status)) {
-  goto ON_ERROR;
+if (ExistPrivate->DevicePath != NULL) {
+  Status = gBS->UninstallProtocolInterface (
+  ExistPrivate->ExtScsiPassThruHandle,
+  ,
+  ExistPrivate->DevicePath
+  );
+  if (EFI_ERROR (Status)) {
+goto ON_ERROR;
+  }
+
+  FreePool (ExistPrivate->DevicePath);
 }
+
+gBS->CloseEvent (ExistPrivate->ExitBootServiceEvent);
+FreePool (ExistPrivate);
+
   }
 } else {
   //
   // Use the attempt in earlier order as boot selected in single path mode.
   //
-- 
1.9.5.msysgit.1

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

[edk2] [Patch] BaseTools/GenFds: enhance to get TOOL_CHAIN_TAG and TARGET value

[Yonghong Zhu]

when user don't set TOOL_CHAIN_TAG and TARGET by –D Flag, then GenFds
would report failure for format:
FILE DATA = $(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/testfile
so this patch enhance to get the TOOL_CHAIN_TAG and TARGET value by
following priority (high to low): 1. the Macro value set by -D Flag;
2. Get the value by the -t/-b option. 3. get the value from target.txt
file. Besides, this patch also remove the error checking for missing
-t/-b option.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/GenFds/GenFds.py | 32 +++-
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFds.py 
b/BaseTools/Source/Python/GenFds/GenFds.py
index 672d103..68232c5 100644
--- a/BaseTools/Source/Python/GenFds/GenFds.py
+++ b/BaseTools/Source/Python/GenFds/GenFds.py
@@ -117,17 +117,13 @@ def main():
 else:
 EdkLogger.error("GenFds", OPTION_MISSING, "Missing FDF filename")
 
 if (Options.BuildTarget):
 GenFdsGlobalVariable.TargetName = Options.BuildTarget
-else:
-EdkLogger.error("GenFds", OPTION_MISSING, "Missing build target")
 
 if (Options.ToolChain):
 GenFdsGlobalVariable.ToolChainTag = Options.ToolChain
-else:
-EdkLogger.error("GenFds", OPTION_MISSING, "Missing tool chain tag")
 
 if (Options.activePlatform):
 ActivePlatform = Options.activePlatform
 ActivePlatform = 
GenFdsGlobalVariable.ReplaceWorkspaceMacro(ActivePlatform)
 
@@ -159,11 +155,27 @@ def main():
 # Get standard WORKSPACE/Conf, use the absolute path to the 
WORKSPACE/Conf
 ConfDirectoryPath = mws.join(GenFdsGlobalVariable.WorkSpaceDir, 
'Conf')
 GenFdsGlobalVariable.ConfDir = ConfDirectoryPath
 BuildConfigurationFile = 
os.path.normpath(os.path.join(ConfDirectoryPath, "target.txt"))
 if os.path.isfile(BuildConfigurationFile) == True:
-TargetTxtClassObject.TargetTxtClassObject(BuildConfigurationFile)
+TargetTxt = TargetTxtClassObject.TargetTxtClassObject()
+TargetTxt.LoadTargetTxtFile(BuildConfigurationFile)
+# if no build target given in command line, get it from target.txt
+if not GenFdsGlobalVariable.TargetName:
+BuildTargetList = 
TargetTxt.TargetTxtDictionary[DataType.TAB_TAT_DEFINES_TARGET]
+if len(BuildTargetList) != 1:
+EdkLogger.error("GenFds", OPTION_VALUE_INVALID, 
ExtraData="Only allows one instance for Target.")
+GenFdsGlobalVariable.TargetName = BuildTargetList[0]
+
+# if no tool chain given in command line, get it from target.txt
+if not GenFdsGlobalVariable.ToolChainTag:
+ToolChainList = 
TargetTxt.TargetTxtDictionary[DataType.TAB_TAT_DEFINES_TOOL_CHAIN_TAG]
+if ToolChainList == None or len(ToolChainList) == 0:
+EdkLogger.error("GenFds", RESOURCE_NOT_AVAILABLE, 
ExtraData="No toolchain given. Don't know how to build.")
+if len(ToolChainList) != 1:
+EdkLogger.error("GenFds", OPTION_VALUE_INVALID, 
ExtraData="Only allows one instance for ToolChain.")
+GenFdsGlobalVariable.ToolChainTag = ToolChainList[0]
 else:
 EdkLogger.error("GenFds", FILE_NOT_FOUND, 
ExtraData=BuildConfigurationFile)
 
 #Set global flag for build mode
 GlobalData.gIgnoreSource = Options.IgnoreSources
@@ -174,10 +186,12 @@ def main():
 Pair = Pair[1:]
 if Pair.endswith('"'):
 Pair = Pair[:-1]
 List = Pair.split('=')
 if len(List) == 2:
+if not List[1].strip():
+EdkLogger.error("GenFds", OPTION_VALUE_INVALID, 
ExtraData="No Value given for Macro %s" %List[0])
 if List[0].strip() == "EFI_SOURCE":
 GlobalData.gEfiSource = List[1].strip()
 GlobalData.gGlobalDefines["EFI_SOURCE"] = 
GlobalData.gEfiSource
 continue
 elif List[0].strip() == "EDK_SOURCE":
@@ -190,10 +204,18 @@ def main():
 GlobalData.gCommandLineDefines[List[0].strip()] = 
List[1].strip()
 else:
 GlobalData.gCommandLineDefines[List[0].strip()] = "TRUE"
 os.environ["WORKSPACE"] = Workspace
 
+# Use the -t and -b option as gGlobalDefines's TOOLCHAIN and TARGET if 
they are not defined
+if "TARGET" not in GlobalData.gGlobalDefines.keys():
+GlobalData.gGlobalDefines["TARGET"] = 
GenFdsGlobalVariable.TargetName
+if "TOOLCHAIN" not in GlobalData.gGlobalDefines.keys():
+

Re: [edk2] [Patch v2] report line number for command errors in a script.

[Qiu, Shumin]

Reviewed-by: Qiu Shumin 

-Original Message-
From: Carsey, Jaben 
Sent: Wednesday, May 11, 2016 5:02 AM
To: edk2-devel@lists.01.org
Cc: Qiu, Shumin
Subject: [Patch v2] report line number for command errors in a script.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jaben Carsey 

---
 ShellPkg/Application/Shell/Shell.c   | 6 +-
 ShellPkg/Application/Shell/Shell.uni | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index 12daff9..b06c1ef 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -2518,7 +2518,11 @@ SetupAndRunCommandOrFile(
   // Now print errors
   //
   if (EFI_ERROR(Status)) {
-ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR), 
ShellInfoObject.HiiHandle, (VOID*)(Status));
+if (ShellCommandGetCurrentScriptFile() == NULL || 
ShellCommandGetCurrentScriptFile()->CurrentCommand == NULL) {
+  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR), 
ShellInfoObject.HiiHandle, (VOID*)(Status));
+} else {
+  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR_SCRIPT), 
ShellInfoObject.HiiHandle, (VOID*)(Status), 
ShellCommandGetCurrentScriptFile()->CurrentCommand->Line);
+}
   }
 
   //
diff --git a/ShellPkg/Application/Shell/Shell.uni 
b/ShellPkg/Application/Shell/Shell.uni
index 3947d01..ef69f89 100644
--- a/ShellPkg/Application/Shell/Shell.uni
+++ b/ShellPkg/Application/Shell/Shell.uni
@@ -40,6 +40,7 @@
 #string STR_SHELL_CRLF#language en-US "\r\n"
 
 #string STR_SHELL_ERROR   #language en-US  "%NCommand Error 
Status: %r\r\n"
+#string STR_SHELL_ERROR_SCRIPT#language en-US  "%NScript Error Status: 
%r (line number %d)\r\n"
 
 #string STR_SHELL_INVALID_MAPPING #language en-US "%N'%B%s%N' is not a 
valid mapping.\r\n"
 #string STR_SHELL_INVALID_SPLIT   #language en-US "Invalid use of pipe 
(%B|%N).\r\n"
-- 
2.7.2.windows.1

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

Re: [edk2] [Patch] SecurityPkg: SecureBootConfigDxe: Add NULL pointer check

[Qiu, Shumin]

Reviewed-by: Qiu Shumin 

-Original Message-
From: El-Haj-Mahmoud, Samer [mailto:samer.el-haj-mahm...@hpe.com] 
Sent: Wednesday, May 11, 2016 12:24 AM
To: Zhang, Chao B; edk2-devel@lists.01.org
Cc: Qiu, Shumin
Subject: RE: [edk2] [Patch] SecurityPkg: SecureBootConfigDxe: Add NULL pointer 
check

Reviewed-by: Samer El-Haj-Mahmoud 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang, 
Chao B
Sent: Tuesday, May 10, 2016 2:52 AM
To: edk2-devel@lists.01.org
Cc: shumin@intel.com; Chao Zhang 
Subject: [edk2] [Patch] SecurityPkg: SecureBootConfigDxe: Add NULL pointer check

Add SecureBoot NULL pointer check before reference it.

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

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 088fa26..3f80441 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -2933,7 +2933,7 @@ SecureBootExtractConfigFromVariable (
   //
   // Fix Pk, SecureBootEnable inconsistence
   //
-  if ((*SetupMode) == USER_MODE) {
+  if ((SetupMode != NULL) && (*SetupMode) == USER_MODE) {
 ConfigData->HideSecureBoot = FALSE;
 if ((SecureBootEnable != NULL) && (*SecureBootEnable == 
SECURE_BOOT_ENABLE)) {
   ConfigData->AttemptSecureBoot = TRUE;
-- 
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] SecurityPkg TcgStorageOpalLib: Avoid using special word in comments.

[Qiu, Shumin]

Reviewed-by: Qiu Shumin 

-Original Message-
From: Dong, Eric 
Sent: Tuesday, May 10, 2016 5:57 PM
To: edk2-devel@lists.01.org
Cc: Qiu, Shumin
Subject: [Patch] SecurityPkg TcgStorageOpalLib: Avoid using special word in 
comments.

Cc: Shumin Qiu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
---
 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
index cc8d5ef..a0eac33 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
@@ -927,7 +927,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
 if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
   DEBUG ((DEBUG_INFO, "Update ACE for GLOBALRANGE_GENKEY failed\n"));
   //
-  // TODO do we want to disable user1 if all permissions are not granted
+  // Disable user1 if all permissions are not granted.
   //
   return TcgResultFailure;
 }
-- 
2.6.4.windows.1

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

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

[Tim Lewis]

Yes.

Tim

-Original Message-
From: Yao, Jiewen [mailto:jiewen@intel.com] 
Sent: Monday, May 09, 2016 6:42 PM
To: Kinney, Michael D ; Tim Lewis 
; Mudusuru, Giri P ; 
edk2-devel@lists.01.org
Cc: Mudusuru, Giri P ; Zimmer, Vincent 
; Rangarajan, Ravi P 
Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split FspInitPei 
to FspmWrapperPeim and FspsWrapperPeim.

Good discussion on PACKAGES_PATH usage.

For this specific case (FSP2 without FSP1.1 support), do we need change to 
IntelFsp2Pkg?

Thank you
Yao Jiewen

> -Original Message-
> From: Kinney, Michael D
> Sent: Friday, May 6, 2016 10:07 AM
> To: Tim Lewis ; Mudusuru, Giri P
> ; Yao, Jiewen ;
> edk2-devel@lists.01.org; Kinney, Michael D 
> Cc: Mudusuru, Giri P ; Zimmer, Vincent
> ; Rangarajan, Ravi P
> 
> Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split
> FspInitPei to FspmWrapperPeim and FspsWrapperPeim.
> 
> Tim,
> 
> I agree multiple repos in a WORKSPACE can potentially be confusing.
> Especially if the
> same package dir exists in more than of the repos.  To alleviate this issue, a
> repo can
> be pruned and the .gitignore feature can be used for git to ignore the
> packages that
> have been pruned.
> 
> The reason I am asking these questions is not related to FSP.
> 
> There have been prior discussions on edk2-devel to organize the packages in
> edk2
> into subdirectories, and I am working on a proposal for that.  However,
> moving
> packages into subdirectories would require the use of the PACKAGES_PATH
> feature.
> 
> Is PACKAGES_PATH something you could add support for in your tools?
> 
> Thanks,
> 
> Mike
> 
> > -Original Message-
> > From: Tim Lewis [mailto:tim.le...@insyde.com]
> > Sent: Thursday, May 5, 2016 6:53 PM
> > To: Kinney, Michael D ; Mudusuru, Giri P
> > ; Yao, Jiewen ; edk2-
> > de...@lists.01.org
> > Cc: Mudusuru, Giri P ; Zimmer, Vincent
> > ; Rangarajan, Ravi P
> 
> > Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split
> FspInitPei to
> > FspmWrapperPeim and FspsWrapperPeim.
> >
> > Mike --
> >
> > At this time, no. Our internal tools do not recognize it, and would fail
> during tree
> > analysis.
> >
> > Likewise, our policy does not allow it because we find our customers are
> confused by
> > multiple roots. It makes it hard for the engineers looking at a downstream
> file to
> > predict where the final file resides. That leads to bad bug reports, etc.
> >
> > Tim
> >
> > -Original Message-
> > From: Kinney, Michael D [mailto:michael.d.kin...@intel.com]
> > Sent: Thursday, May 05, 2016 6:14 PM
> > To: Tim Lewis ; Mudusuru, Giri P
> ;
> > Yao, Jiewen ; edk2-devel@lists.01.org; Kinney,
> Michael D
> > 
> > Cc: Mudusuru, Giri P ; Zimmer, Vincent
> > ; Rangarajan, Ravi P
> 
> > Subject: RE: [edk2] [PATCH 17/19] IntelFspWrapperPkg/FspInit: Split
> FspInitPei to
> > FspmWrapperPeim and FspsWrapperPeim.
> >
> > Hi Tim,
> >
> > I wanted to follow up on the general concern on the use of the
> PACKAGES_PATH feature.
> >
> > The EDK II BaseTools still use a single WORKSPACE and can use
> PACKAGES_PATH for
> > additional search paths for packages.  PACKAGES_PATH can point to
> directories
> > Below WORKSPACE or outside WORKSPACE, so it is very flexible.
> >
> >
> https://github.com/tianocore/tianocore.github.io/wiki/Multiple_Workspace
> >
> > For your specific use case, could you use PACKAGES_PATH if all the paths
> listed
> > in PACKAGES_PATH are below WORKSPACE?
> >
> > For example, if you have the following dir structure:
> >
> > tianocore/
> > edk2/
> > Udk2015/
> >
> > You can set WORKSPACE to tianocore/ and PACKAGES_PATH to
> > tianocore/edk2;tianocore/Udk2015.
> >
> > This means an additional directory level is added, so all the packages are
> not in the
> > root of WORKSPACE.  This is how we can support pulling content from
> multiple git
> > repos/tags/branches for a single WSORKSPACE build environment without
> having to update
> > the [Packages] section in every INF.
> >
> > Thanks,
> >
> > Mike
> >
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf
> Of Tim Lewis
> > > Sent: Thursday, May 5, 2016 12:09 PM
> > > To: Mudusuru, Giri P ; Yao, Jiewen
> 

[edk2] [PATCH 7/7] CorebootPayloadPkg: Add BdsDxe support

[Lee Leahy]

Add define to select the MdeModulePkg/Universal/BdsDxe instead of
IntelFrameworkModulePkg/Universal/BdsDxe.

Change-Id: I0930b375e46fd72a199567efc422df5bb535798c
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkg.fdf  |  14 +-
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc  |  22 +-
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc   |  22 +-
 .../PlatformBootManagerLib/PlatformBootManager.c   | 339 +
 .../PlatformBootManagerLib/PlatformBootManager.h   |  50 +++
 .../PlatformBootManagerLib.inf |  73 +
 .../Library/PlatformBootManagerLib/PlatformData.c  | 281 +
 7 files changed, 794 insertions(+), 7 deletions(-)
 create mode 100644 
CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
 create mode 100644 
CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.h
 create mode 100644 
CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
 create mode 100644 
CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformData.c

diff --git a/CorebootPayloadPkg/CorebootPayloadPkg.fdf 
b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
index 432155f..df771ef 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkg.fdf
+++ b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
@@ -86,7 +86,11 @@ INF 
IntelFrameworkModulePkg/Universal/StatusCode/RuntimeDxe/StatusCodeRuntimeDxe
 
 INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
 INF UefiCpuPkg/CpuDxe/CpuDxe.inf
+!if $(BDS_TYPE) == IntelFrameworkModulePkg
 INF IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
+!else
+INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+!endif
 INF PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
 INF MdeModulePkg/Universal/Metronome/Metronome.inf
 INF MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
@@ -112,12 +116,16 @@ INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 #
 INF 
CorebootModulePkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
 INF CorebootModulePkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
-INF CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
 
 #
-# ISA Support
+# Serial Support
 #
-INF CorebootModulePkg/SerialDxe/SerialDxe.inf
+!if $(BDS_TYPE) == IntelFrameworkModulePkg
+
+  INF CorebootModulePkg/SerialDxe/SerialDxe.inf
+!else
+  INF CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
+!endif
 
 #
 # Console Support
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index a9e78a5..57c2dce 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -34,6 +34,11 @@
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
   #
+  # BDS Options: [IntelFrameworkModulePkg, MdeModulePkg]
+  #
+  DEFINE BDS_TYPE= IntelFrameworkModulePkg
+
+  #
   # CPU options
   #
   DEFINE MAX_LOGICAL_PROCESSORS  = 64
@@ -162,7 +167,13 @@
   ResetSystemLib|CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.inf
   
SerialPortLib|CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
   
PlatformHookLib|CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
+!if $(BDS_TYPE) == IntelFrameworkModulePkg
   PlatformBdsLib|CorebootPayloadPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
+!else
+  
PlatformBootManagerLib|CorebootPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+  SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
+  
UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
+!endif
 
   #
   # Misc
@@ -353,7 +364,11 @@
   #
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
+!if $(BDS_TYPE) == IntelFrameworkModulePkg
   IntelFrameworkModulePkg/Universal/BdsDxe/BdsDxe.inf
+!else
+  MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
+!endif
   PcAtChipsetPkg/8254TimerDxe/8254Timer.inf
   MdeModulePkg/Universal/Metronome/Metronome.inf
   MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
@@ -398,7 +413,6 @@
   #
   
CorebootModulePkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
   CorebootModulePkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
-  CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
 
   #
   # SCSI/ATA/IDE/DISK Support
@@ -436,9 +450,13 @@
   CorebootModulePkg/OhciDxe/OhciDxe.inf
 
   #
-  # ISA Support
+  # Serial Support
   #
+!if $(BDS_TYPE) == IntelFrameworkModulePkg
   CorebootModulePkg/SerialDxe/SerialDxe.inf
+!else
+  CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
+!endif
 
   #
   # Console Support
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 5dd17cb..d377535 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -34,6 +34,11 @@
   DEFINE SOURCE_DEBUG_ENABLE = FALSE
 
   #
+  

[edk2] [PATCH 2/7] CorebootPayloadPkg: Assume no PCI serial devices

[Lee Leahy]

Set the vendor to 0x which indicates the end of the list.

Change-Id: If6475e04d3675f0a932571a85d1dd3f301416b6a
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 4 ++--
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index 907e952..cc88502 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -67,10 +67,10 @@
   #UINT8   Reserved[2];
   #  } PCI_SERIAL_PARAMETER;
   #
-  # Vendor  Device  Prog Interface 1, BAR #0, Offset 0, Stride = 1, 
Clock 1843200 (0x1c2000)
+  # Vendor  Device  Prog Interface 1, BAR #0, Offset 0, Stride = 1, 
Clock 1843200 (0x1c2000)
   #
   #   [Vendor]   [Device]  
[ClockRate---]  [Offset---] [Bar] [Stride] [RxFifo] 
[TxFifo]   [Rsvd]   [Vendor]
-  DEFINE PCI_SERIAL_PARAMETERS= {0x00,0x00, 0x00,0x00, 
0x0,0x20,0x1c,0x00, 0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0, 0x00,0x01, 0x0,0x0, 
0x0,0x0, 0x0,0x0, 0xff,0xff}
+  DEFINE PCI_SERIAL_PARAMETERS= {0xff,0xff, 0x00,0x00, 
0x0,0x20,0x1c,0x00, 0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0, 0x00,0x01, 0x0,0x0, 
0x0,0x0, 0x0,0x0, 0xff,0xff}
 
   #
   # Shell options: [BUILD_SHELL, FULL_BIN, MIN_BIN, NONE, UEFI]
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 90a484d..77a33a9 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -67,10 +67,10 @@
   #UINT8   Reserved[2];
   #  } PCI_SERIAL_PARAMETER;
   #
-  # Vendor  Device  Prog Interface 1, BAR #0, Offset 0, Stride = 1, 
Clock 1843200 (0x1c2000)
+  # Vendor  Device  Prog Interface 1, BAR #0, Offset 0, Stride = 1, 
Clock 1843200 (0x1c2000)
   #
   #   [Vendor]   [Device]  
[ClockRate---]  [Offset---] [Bar] [Stride] [RxFifo] 
[TxFifo]   [Rsvd]   [Vendor]
-  DEFINE PCI_SERIAL_PARAMETERS= {0x00,0x00, 0x00,0x00, 
0x0,0x20,0x1c,0x00, 0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0, 0x00,0x01, 0x0,0x0, 
0x0,0x0, 0x0,0x0, 0xff,0xff}
+  DEFINE PCI_SERIAL_PARAMETERS= {0xff,0xff, 0x00,0x00, 
0x0,0x20,0x1c,0x00, 0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0, 0x00,0x01, 0x0,0x0, 
0x0,0x0, 0x0,0x0, 0xff,0xff}
 
   #
   # Shell options: [BUILD_SHELL, FULL_BIN, MIN_BIN, NONE, UEFI]
-- 
1.9.1

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

[edk2] [PATCH 3/7] CorebootPayloadPkg: Use correct BaseSerialPortLib16550

[Lee Leahy]

Use the BaseSerialPortLib16550 which sets RTS and DTR during
initialization.  This fixes the mis-matched flow control issue when
the flow control signals are connected between the host and target
and the host has flow control enabled.

Change-Id: I3505e129b2de3c5c17fff23c62553f15cd892dca
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 2 +-
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index cc88502..9be96e3 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -160,7 +160,7 @@
   #
   TimerLib|CorebootPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf
   ResetSystemLib|CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.inf
-  
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  
SerialPortLib|CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
   
PlatformHookLib|CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
   PlatformBdsLib|CorebootPayloadPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
 
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 77a33a9..561c0c9 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -162,7 +162,7 @@
   #
   TimerLib|CorebootPayloadPkg/Library/AcpiTimerLib/AcpiTimerLib.inf
   ResetSystemLib|CorebootPayloadPkg/Library/ResetSystemLib/ResetSystemLib.inf
-  
SerialPortLib|MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+  
SerialPortLib|CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
   
PlatformHookLib|CorebootPayloadPkg/Library/PlatformHookLib/PlatformHookLib.inf
   PlatformBdsLib|CorebootPayloadPkg/Library/PlatformBdsLib/PlatformBdsLib.inf
 
-- 
1.9.1

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

[edk2] [PATCH 4/7] CorebootPayloadPkg: Set the proper Shell file GUID

[Lee Leahy]

Set the proper Shell file GUID so that the BDS transfers control to the
Shell.

Change-Id: I816636a340bbe2f76ac1973b9cb685084c4f88a0
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 11 +++
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 11 +++
 2 files changed, 22 insertions(+)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index 9be96e3..72b69f2 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -288,6 +288,17 @@
 
   
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|$(MAX_LOGICAL_PROCESSORS)
 
+  #
+  # Set the proper Shell file GUID
+  #
+  !if $(SHELL_TYPE) == FULL_BIN
+  # c57ad6b7-0515-40a8-9d21-551652854e37
+  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0xB7, 0xD6, 0x7A, 
0xC5, 0x15, 0x05, 0xA8, 0x40, 0x9D, 0x21, 0x55, 0x16, 0x52, 0x85, 0x4E, 0x37 }
+  !else
+  # 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
+  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 
0x7C, 0x3E, 0x9E, 0x1c, 0x4f, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
+  !endif
+
 

 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 561c0c9..21e2548 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -292,6 +292,17 @@
 
   
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|$(MAX_LOGICAL_PROCESSORS)
 
+  #
+  # Set the proper Shell file GUID
+  #
+  !if $(SHELL_TYPE) == FULL_BIN
+  # c57ad6b7-0515-40a8-9d21-551652854e37
+  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0xB7, 0xD6, 0x7A, 
0xC5, 0x15, 0x05, 0xA8, 0x40, 0x9D, 0x21, 0x55, 0x16, 0x52, 0x85, 0x4E, 0x37 }
+  !else
+  # 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1
+  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile|{ 0x83, 0xA5, 0x04, 
0x7C, 0x3E, 0x9E, 0x1c, 0x4f, 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
+  !endif
+
 

 #
 # Pcd Dynamic Section - list of all EDK II PCD Entries defined by this Platform
-- 
1.9.1

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

[edk2] [PATCH 5/7] CorebootPayloadPkg: Add SD/eMMC support

[Lee Leahy]

Add SD and eMMC DXE driver support to CorebootPayloadPkg.

Change-Id: Ibfd3a2cc32a653ce51e38d9157ea3c6da25a5474
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkg.fdf| 7 +++
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 7 +++
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 7 +++
 3 files changed, 21 insertions(+)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkg.fdf 
b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
index 7b52f40..cb18828 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkg.fdf
+++ b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
@@ -142,6 +142,13 @@ INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 INF FatPkg/EnhancedFatDxe/Fat.inf
 
 #
+# SD/eMMC Support
+#
+INF MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+INF MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+INF MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
+#
 # Usb Support
 #
 INF MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
index 72b69f2..73af2dd 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc
@@ -414,6 +414,13 @@
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
   #
+  # SD/eMMC Support
+  #
+  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+  MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
+  #
   # Usb Support
   #
   MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
diff --git a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc 
b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
index 21e2548..95ec8ce 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
+++ b/CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc
@@ -418,6 +418,13 @@
   MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
   #
+  # SD/eMMC Support
+  #
+  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+  MdeModulePkg/Bus/Sd/EmmcDxe/EmmcDxe.inf
+  MdeModulePkg/Bus/Sd/SdDxe/SdDxe.inf
+
+  #
   # Usb Support
   #
   MdeModulePkg/Bus/Pci/UhciDxe/UhciDxe.inf
-- 
1.9.1

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

[edk2] [PATCH] IntelFrameworkModulePkg/BootMaint: Fix ASSERT condition

[Lee Leahy]

GroupMultipleLegacyBootOption4SameType ASSERTs with EFI_NOT_FOUND when
the BootOrder variable is not found.  This occurs because BootOrderSize
is zero and BootOrder is NULL.  This patch eliminates the two ASSERTs
which occur.

Change-Id: I6b4f713575011da7c7442fe25ebdbd8379b0303b
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 .../Universal/BdsDxe/BootMaint/BBSsupport.c| 41 --
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BBSsupport.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BBSsupport.c
index 6a0b525..379a951 100644
--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BBSsupport.c
+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BBSsupport.c
@@ -53,9 +53,9 @@ OrderLegacyBootOption4SameType (
   UINTNBootOrderSize;
   UINTNIndex;
   UINTNStartPosition;
-  
+
   BDS_COMMON_OPTION*BootOption;
-  
+
   CHAR16   OptionName[sizeof ("Boot")];
   UINT16   *BbsIndexArray;
   UINT16   *DeviceTypeArray;
@@ -82,12 +82,12 @@ OrderLegacyBootOption4SameType (
   ASSERT (*DisBootOption != NULL);
 
   for (Index = 0; Index < BootOrderSize / sizeof (UINT16); Index++) {
-  
+
 UnicodeSPrint (OptionName, sizeof (OptionName), L"Boot%04x", 
BootOrder[Index]);
 InitializeListHead ();
 BootOption = BdsLibVariableToOption (, OptionName);
 ASSERT (BootOption != NULL);
-
+
 if ((DevicePathType (BootOption->DevicePath) == BBS_DEVICE_PATH) &&
 (DevicePathSubType (BootOption->DevicePath) == BBS_BBS_DP)) {
   //
@@ -119,7 +119,7 @@ OrderLegacyBootOption4SameType (
   if (BbsIndexArray[Index] == (DevOrder[DevOrderCount] & 0xFF)) {
 StartPosition = MIN (StartPosition, Index);
 NewBootOption[DevOrderCount] = BootOrder[Index];
-
+
 if ((DevOrder[DevOrderCount] & 0xFF00) == 0xFF00) {
   (*DisBootOption)[*DisBootOptionCount] = BootOrder[Index];
   (*DisBootOptionCount)++;
@@ -157,7 +157,7 @@ OrderLegacyBootOption4SameType (
 /**
   Group the legacy boot options in the BootOption.
 
-  The routine assumes the boot options in the beginning that covers all the 
device 
+  The routine assumes the boot options in the beginning that covers all the 
device
   types are ordered properly and re-position the following boot options just 
after
   the corresponding boot options with the same device type.
   For example:
@@ -194,7 +194,6 @@ GroupMultipleLegacyBootOption4SameType (
 ,
 
 );
-
   for (Index = 0; Index < BootOrderSize / sizeof (UINT16); Index++) {
 UnicodeSPrint (OptionName, sizeof (OptionName), L"Boot%04x", 
BootOrder[Index]);
 InitializeListHead ();
@@ -238,17 +237,21 @@ GroupMultipleLegacyBootOption4SameType (
 FreePool (BootOption);
   }
 
-  Status = gRT->SetVariable (
-  L"BootOrder",
-  ,
-  EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
-  BootOrderSize,
-  BootOrder
-  );
-  //
-  // Changing content without increasing its size with current variable 
implementation shouldn't fail.
-  //
-  ASSERT_EFI_ERROR (Status);
-  FreePool (BootOrder);
+  if (BootOrder != NULL) {
+if (BootOrderSize != 0) {
+  Status = gRT->SetVariable (
+  L"BootOrder",
+  ,
+  EFI_VARIABLE_BOOTSERVICE_ACCESS | 
EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE,
+  BootOrderSize,
+  BootOrder
+  );
+  //
+  // Changing content without increasing its size with current variable 
implementation shouldn't fail.
+  //
+  ASSERT_EFI_ERROR (Status);
+}
+FreePool (BootOrder);
+  }
 }
 
-- 
1.9.1

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

[edk2] [PATCH 1/7] CorebootPayloadPkg: Use DOS line endings

[Lee Leahy]

Convert to using DOS line endings.

Change-Id: Ie2f148867d9b2b386d556583afb6716ec21399e9
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Lee Leahy 
---
 CorebootPayloadPkg/CorebootPayloadPkg.fdf|  84 ++---
 CorebootPayloadPkg/CorebootPayloadPkgIa32.dsc| 412 +++---
 CorebootPayloadPkg/CorebootPayloadPkgIa32X64.dsc | 419 +++
 3 files changed, 455 insertions(+), 460 deletions(-)

diff --git a/CorebootPayloadPkg/CorebootPayloadPkg.fdf 
b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
index 85748a6..7b52f40 100644
--- a/CorebootPayloadPkg/CorebootPayloadPkg.fdf
+++ b/CorebootPayloadPkg/CorebootPayloadPkg.fdf
@@ -1,16 +1,16 @@
 ## @file
 # Coreboot Payload Package
 #
-# Provides drivers and definitions to create uefi payload for coreboot.
+# Provides drivers and definitions to create uefi payload for coreboot.
 #
-# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
-# This program and the accompanying materials are licensed and made available 
under
-# the terms and conditions of the BSD License that accompanies this 
distribution.
+# Copyright (c) 2014 - 2016, Intel Corporation. All rights reserved.
+# This program and the accompanying materials are licensed and made available 
under
+# the terms and conditions of the BSD License that accompanies this 
distribution.
 # The full text of the license may be found at
-# http://opensource.org/licenses/bsd-license.php.
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+# http://opensource.org/licenses/bsd-license.php.
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #
 ##
 
@@ -93,31 +93,31 @@ INF 
MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
 INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
 INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
 INF 
MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
-INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
-INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
+INF MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf
 INF MdeModulePkg/Universal/Variable/EmuRuntimeDxe/EmuVariableRuntimeDxe.inf
 
 INF UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
 INF MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
-INF PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
+INF PcAtChipsetPkg/8259InterruptControllerDxe/8259.inf
 INF MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
-INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
+INF MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
 INF MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
-INF CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
+INF CorebootModulePkg/CbSupportDxe/CbSupportDxe.inf
 
 INF MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
 #
 # PCI Support
 #
-INF 
CorebootModulePkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
-INF CorebootModulePkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
-INF CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
+INF 
CorebootModulePkg/PciRootBridgeNoEnumerationDxe/PciRootBridgeNoEnumeration.inf
+INF CorebootModulePkg/PciBusNoEnumerationDxe/PciBusNoEnumeration.inf
+INF CorebootModulePkg/PciSioSerialDxe/PciSioSerialDxe.inf
 
 #
 # ISA Support
 #
-INF CorebootModulePkg/SerialDxe/SerialDxe.inf
+INF CorebootModulePkg/SerialDxe/SerialDxe.inf
 
 #
 # Console Support
@@ -133,13 +133,13 @@ INF 
MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
 INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
 INF MdeModulePkg/Universal/Disk/PartitionDxe/PartitionDxe.inf
 INF MdeModulePkg/Universal/Disk/UnicodeCollation/EnglishDxe/EnglishDxe.inf
-INF CorebootModulePkg/SataControllerDxe/SataControllerDxe.inf
+INF CorebootModulePkg/SataControllerDxe/SataControllerDxe.inf
 INF MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
 INF MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
 INF MdeModulePkg/Bus/Scsi/ScsiBusDxe/ScsiBusDxe.inf
 INF MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDiskDxe.inf
 
-INF FatPkg/EnhancedFatDxe/Fat.inf
+INF FatPkg/EnhancedFatDxe/Fat.inf
 
 #
 # Usb Support
@@ -154,40 +154,40 @@ INF 
MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 #
 # Shell
 #
-!if $(SHELL_TYPE) == BUILD_SHELL
-INF  ShellPkg/Application/Shell/Shell.inf
-!endif
-
-!if $(SHELL_TYPE) == FULL_BIN
+!if $(SHELL_TYPE) == BUILD_SHELL
+INF ShellPkg/Application/Shell/Shell.inf
+!endif
+
+!if $(SHELL_TYPE) == FULL_BIN
 !if $(ARCH) == IA32
 INF  RuleOverride = BINARY USE = IA32 EdkShellBinPkg/FullShell/FullShell.inf
 

Re: [edk2] [Patch v2] report line number for command errors in a script.

[Bjorge, Erik C]

Reviewed-by: Erik Bjorge 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jaben Carsey
> Sent: Tuesday, May 10, 2016 2:02 PM
> To: edk2-devel@lists.01.org
> Cc: Qiu, Shumin 
> Subject: [edk2] [Patch v2] report line number for command errors in a
> script.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jaben Carsey 
> 
> ---
>  ShellPkg/Application/Shell/Shell.c   | 6 +-
>  ShellPkg/Application/Shell/Shell.uni | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 12daff9..b06c1ef 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -2518,7 +2518,11 @@ SetupAndRunCommandOrFile(
>// Now print errors
>//
>if (EFI_ERROR(Status)) {
> -ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR),
> ShellInfoObject.HiiHandle, (VOID*)(Status));
> +if (ShellCommandGetCurrentScriptFile() == NULL ||
> ShellCommandGetCurrentScriptFile()->CurrentCommand == NULL) {
> +  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR),
> ShellInfoObject.HiiHandle, (VOID*)(Status));
> +} else {
> +  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> (STR_SHELL_ERROR_SCRIPT), ShellInfoObject.HiiHandle, (VOID*)(Status),
> ShellCommandGetCurrentScriptFile()->CurrentCommand->Line);
> +}
>}
> 
>//
> diff --git a/ShellPkg/Application/Shell/Shell.uni
> b/ShellPkg/Application/Shell/Shell.uni
> index 3947d01..ef69f89 100644
> --- a/ShellPkg/Application/Shell/Shell.uni
> +++ b/ShellPkg/Application/Shell/Shell.uni
> @@ -40,6 +40,7 @@
>  #string STR_SHELL_CRLF#language en-US "\r\n"
> 
>  #string STR_SHELL_ERROR   #language en-US  "%NCommand Error
> Status: %r\r\n"
> +#string STR_SHELL_ERROR_SCRIPT#language en-US  "%NScript Error
> Status: %r (line number %d)\r\n"
> 
>  #string STR_SHELL_INVALID_MAPPING #language en-US "%N'%B%s%N' is
> not a valid mapping.\r\n"
>  #string STR_SHELL_INVALID_SPLIT   #language en-US "Invalid use of
> pipe (%B|%N).\r\n"
> --
> 2.7.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch v2] report line number for command errors in a script.

[Jaben Carsey]

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jaben Carsey 

---
 ShellPkg/Application/Shell/Shell.c   | 6 +-
 ShellPkg/Application/Shell/Shell.uni | 1 +
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index 12daff9..b06c1ef 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -2518,7 +2518,11 @@ SetupAndRunCommandOrFile(
   // Now print errors
   //
   if (EFI_ERROR(Status)) {
-ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR), 
ShellInfoObject.HiiHandle, (VOID*)(Status));
+if (ShellCommandGetCurrentScriptFile() == NULL || 
ShellCommandGetCurrentScriptFile()->CurrentCommand == NULL) {
+  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR), 
ShellInfoObject.HiiHandle, (VOID*)(Status));
+} else {
+  ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_SHELL_ERROR_SCRIPT), 
ShellInfoObject.HiiHandle, (VOID*)(Status), 
ShellCommandGetCurrentScriptFile()->CurrentCommand->Line);
+}
   }
 
   //
diff --git a/ShellPkg/Application/Shell/Shell.uni 
b/ShellPkg/Application/Shell/Shell.uni
index 3947d01..ef69f89 100644
--- a/ShellPkg/Application/Shell/Shell.uni
+++ b/ShellPkg/Application/Shell/Shell.uni
@@ -40,6 +40,7 @@
 #string STR_SHELL_CRLF#language en-US "\r\n"
 
 #string STR_SHELL_ERROR   #language en-US  "%NCommand Error 
Status: %r\r\n"
+#string STR_SHELL_ERROR_SCRIPT#language en-US  "%NScript Error Status: 
%r (line number %d)\r\n"
 
 #string STR_SHELL_INVALID_MAPPING #language en-US "%N'%B%s%N' is not a 
valid mapping.\r\n"
 #string STR_SHELL_INVALID_SPLIT   #language en-US "Invalid use of pipe 
(%B|%N).\r\n"
-- 
2.7.2.windows.1

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

Re: [edk2] [PATCH v2] ArmVirtPkg: set PcdMaxVariableSize and PcdMaxAuthVariableSize

[Laszlo Ersek]

On 05/10/16 20:16, Linn Crosetto wrote:
> To support UEFI Secure Boot and the Linux persistent store with UEFI
> variables, set PcdMaxVariableSize to 0x2000 bytes as is done in OvmfPkg.
> For reference, the related Ovmf commits: 8cee3de7 2d441ca9
> 
> Also increase the maximum size for Authenticated variables in order to
> handle a larger Signature List size as is done in OvmfPkg. Related Ovmf
> commit: f5404a3e
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Linn Crosetto 
> ---
> v2:
> - Set PcdMaxAuthVariableSize [Laszlo Ersek]
> 
>  ArmVirtPkg/ArmVirtQemu.dsc   | 2 ++
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
>  2 files changed, 4 insertions(+)
> 
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 1d48cd0..d07593b 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -111,6 +111,8 @@ [PcdsFixedAtBuild.common]
>  
>gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
>gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>  
># Size of the region used by UEFI in permanent memory (Reserved 64MB)
>gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc 
> b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index d4134f1..65e6c87 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -100,6 +100,8 @@ [PcdsFixedAtBuild.common]
>  
>gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
>gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>  
># Size of the region used by UEFI in permanent memory (Reserved 64MB)
>gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
> 

Reviewed-by: Laszlo Ersek 

Commit 2d063646b9f5.

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

Re: [edk2] [Patch] BaseTools: fix a bug for uni file \x####\ format handling

[Palmer, Thomas]

This fixes a build "hang" we've experienced after a recent EDK2 update.  Thank 
you!  

Please consider this patch for commit to master

Thomas

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Tuesday, May 10, 2016 5:17 AM
To: edk2-devel@lists.01.org
Cc: Liming Gao 
Subject: [edk2] [Patch] BaseTools: fix a bug for uni file \x\ format 
handling

It should start from the last '\x' position + 1 to find next '\x'
character.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/UniClassObject.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/UniClassObject.py 
b/BaseTools/Source/Python/AutoGen/UniClassObject.py
index d28fd3a..183b2b2 100644
--- a/BaseTools/Source/Python/AutoGen/UniClassObject.py
+++ b/BaseTools/Source/Python/AutoGen/UniClassObject.py
@@ -442,11 +442,11 @@ class UniFileClassObject(object):
 if EndStr.startswith(u'\\x') and len(EndStr) >= 7:
 if EndStr[6] == u'\\' and 
re.match('[a-fA-F0-9]{4}', EndStr[2 : 6], re.UNICODE):
 Line = Line[0 : StartPos] + UniStr + EndStr
 else:
 Line = Line[0 : StartPos] + UniStr + EndStr[1:]
-StartPos = Line.find(u'\\x', StartPos)
+StartPos = Line.find(u'\\x', StartPos + 1)
 
 IncList = gIncludePattern.findall(Line)
 if len(IncList) == 1:
 for Dir in [File.Dir] + self.IncludePathList:
 IncFile = PathClass(str(IncList[0]), Dir)
-- 
2.6.1.windows.1

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

Re: [edk2] [PATCH v2 7/7] OvmfPkg/PciHostBridgeLib: Scan for root bridges when running over Xen

[Laszlo Ersek]

On 05/10/16 07:53, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Gary Lin 
> Cc: Laszlo Ersek 
> ---
>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h   |  75 
>  .../Library/PciHostBridgeLib/PciHostBridgeLib.c|   6 +-
>  .../Library/PciHostBridgeLib/PciHostBridgeLib.inf  |   3 +
>  OvmfPkg/Library/PciHostBridgeLib/XenSupport.c  | 456 
> +
>  4 files changed, 539 insertions(+), 1 deletion(-)
>  create mode 100644 OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h
>  create mode 100644 OvmfPkg/Library/PciHostBridgeLib/XenSupport.c
> 
> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h 
> b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h
> new file mode 100644
> index 000..c23d40c
> --- /dev/null
> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h
> @@ -0,0 +1,75 @@
> +/** @file
> +  Header file of OVMF instance of PciHostBridgeLib.
> +
> +  Copyright (c) 2016, Intel Corporation. All rights reserved.
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php.
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, 
> WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +PCI_ROOT_BRIDGE *
> +ScanForRootBridges (
> +  UINTN  *NumberOfRootBridges
> +);
> +
> +/**
> +  Initialize a PCI_ROOT_BRIDGE structure.
> +
> +  @param[in]  Supports Supported attributes.
> +
> +  @param[in]  Attributes   Initial attributes.
> +
> +  @param[in]  AllocAttributes  Allocation attributes.
> +
> +  @param[in]  RootBusNumberThe bus number to store in RootBus.
> +
> +  @param[in]  MaxSubBusNumber  The inclusive maximum bus number that can be
> +   assigned to any subordinate bus found behind 
> any
> +   PCI bridge hanging off this root bus.
> +
> +   The caller is repsonsible for ensuring that
> +   RootBusNumber <= MaxSubBusNumber. If
> +   RootBusNumber equals MaxSubBusNumber, then the
> +   root bus has no room for subordinate buses.
> +
> +  @param[in]  Io   IO aperture.
> +
> +  @param[in]  Mem  MMIO aperture.
> +
> +  @param[in]  MemAbove4G   MMIO aperture above 4G.
> +
> +  @param[in]  PMem Prefetchable MMIO aperture.
> +
> +  @param[in]  PMemAbove4G  Prefetchable MMIO aperture above 4G.
> +
> +  @param[out] RootBus  The PCI_ROOT_BRIDGE structure (allocated by 
> the
> +   caller) that should be filled in by this
> +   function.
> +
> +  @retval EFI_SUCCESS   Initialization successful. A device path
> +consisting of an ACPI device path node, with
> +UID = RootBusNumber, has been allocated and
> +linked into RootBus.
> +
> +  @retval EFI_OUT_OF_RESOURCES  Memory allocation failed.
> +**/
> +EFI_STATUS
> +InitRootBridge (
> +  IN  UINT64   Supports,
> +  IN  UINT64   Attributes,
> +  IN  UINT64   AllocAttributes,
> +  IN  UINT8RootBusNumber,
> +  IN  UINT8MaxSubBusNumber,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *Io,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *Mem,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *PMem,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G,
> +  OUT PCI_ROOT_BRIDGE  *RootBus
> +  );
> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c 
> b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> index aeb0bdf..6ba0ca6 100644
> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include "PciHostBridge.h"
>  
>  
>  #pragma pack(1)
> @@ -113,7 +114,6 @@ STATIC PCI_ROOT_BRIDGE_APERTURE mNonExistAperture = { 
> MAX_UINT64, 0 };
>  
>@retval EFI_OUT_OF_RESOURCES  Memory allocation failed.
>  **/
> -STATIC
>  EFI_STATUS
>  InitRootBridge (
>IN  UINT64   Supports,
> @@ -217,6 +217,10 @@ PciHostBridgeGetRootBridges (
>PCI_ROOT_BRIDGE_APERTURE Mem;
>PCI_ROOT_BRIDGE_APERTURE MemAbove4G;
>  
> +  if (PcdGetBool (PcdPciDisableBusEnumeration)) {
> +return ScanForRootBridges (Count);
> +  }
> +
>Attributes = EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO |
>  EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO |
>  EFI_PCI_ATTRIBUTE_ISA_IO_16 |
> diff --git 

[edk2] [PATCH v2] ArmVirtPkg: set PcdMaxVariableSize and PcdMaxAuthVariableSize

[Linn Crosetto]

To support UEFI Secure Boot and the Linux persistent store with UEFI
variables, set PcdMaxVariableSize to 0x2000 bytes as is done in OvmfPkg.
For reference, the related Ovmf commits: 8cee3de7 2d441ca9

Also increase the maximum size for Authenticated variables in order to
handle a larger Signature List size as is done in OvmfPkg. Related Ovmf
commit: f5404a3e

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Linn Crosetto 
---
v2:
- Set PcdMaxAuthVariableSize [Laszlo Ersek]

 ArmVirtPkg/ArmVirtQemu.dsc   | 2 ++
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 1d48cd0..d07593b 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -111,6 +111,8 @@ [PcdsFixedAtBuild.common]
 
   gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
   gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
 
   # Size of the region used by UEFI in permanent memory (Reserved 64MB)
   gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index d4134f1..65e6c87 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -100,6 +100,8 @@ [PcdsFixedAtBuild.common]
 
   gArmPlatformTokenSpaceGuid.PcdCPUCoresStackBase|0x4007c000
   gArmPlatformTokenSpaceGuid.PcdCPUCorePrimaryStackSize|0x4000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
 
   # Size of the region used by UEFI in permanent memory (Reserved 64MB)
   gArmPlatformTokenSpaceGuid.PcdSystemMemoryUefiRegionSize|0x0400
-- 
2.8.0.rc3

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

Re: [edk2] [PATCH v2 6/7] OvmfPkg/PciHostBridgeLib: Change InitRootBridge prototype

[Laszlo Ersek]

On 05/10/16 07:53, Ruiyu Ni wrote:
> The patch re-factors the code without functionality impact.
> Next patch will add code to support OVMF above Xen.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Gary Lin 
> Cc: Laszlo Ersek 
> ---
>  .../Library/PciHostBridgeLib/PciHostBridgeLib.c| 131 
> +++--
>  1 file changed, 93 insertions(+), 38 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c 
> b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> index 9e01498..aeb0bdf 100644
> --- a/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> +++ b/OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.c
> @@ -70,13 +70,20 @@ OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH 
> mRootBridgeDevicePathTemplate = {
>}
>  };
>  
> +STATIC PCI_ROOT_BRIDGE_APERTURE mNonExistAperture = { MAX_UINT64, 0 };
>  
>  /**
>Initialize a PCI_ROOT_BRIDGE structure.
>  
> -  param[in]  RootBusNumber The bus number to store in RootBus.
> +  @param[in]  Supports Supported attributes.
>  
> -  param[in]  MaxSubBusNumber   The inclusive maximum bus number that can be
> +  @param[in]  Attributes   Initial attributes.
> +
> +  @param[in]  AllocAttributes  Allocation attributes.
> +
> +  @param[in]  RootBusNumberThe bus number to store in RootBus.
> +
> +  @param[in]  MaxSubBusNumber  The inclusive maximum bus number that can be
> assigned to any subordinate bus found behind 
> any
> PCI bridge hanging off this root bus.
>  
> @@ -85,7 +92,17 @@ OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH 
> mRootBridgeDevicePathTemplate = {
> RootBusNumber equals MaxSubBusNumber, then the
> root bus has no room for subordinate buses.
>  
> -  param[out] RootBus   The PCI_ROOT_BRIDGE structure (allocated by 
> the
> +  @param[in]  Io   IO aperture.
> +
> +  @param[in]  Mem  MMIO aperture.
> +
> +  @param[in]  MemAbove4G   MMIO aperture above 4G.
> +
> +  @param[in]  PMem Prefetchable MMIO aperture.
> +
> +  @param[in]  PMemAbove4G  Prefetchable MMIO aperture above 4G.
> +
> +  @param[out] RootBus  The PCI_ROOT_BRIDGE structure (allocated by 
> the
> caller) that should be filled in by this
> function.
>  
> @@ -99,9 +116,17 @@ OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH 
> mRootBridgeDevicePathTemplate = {
>  STATIC
>  EFI_STATUS
>  InitRootBridge (
> -  IN  UINT8   RootBusNumber,
> -  IN  UINT8   MaxSubBusNumber,
> -  OUT PCI_ROOT_BRIDGE *RootBus
> +  IN  UINT64   Supports,
> +  IN  UINT64   Attributes,
> +  IN  UINT64   AllocAttributes,
> +  IN  UINT8RootBusNumber,
> +  IN  UINT8MaxSubBusNumber,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *Io,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *Mem,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *MemAbove4G,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *PMem,
> +  IN  PCI_ROOT_BRIDGE_APERTURE *PMemAbove4G,
> +  OUT PCI_ROOT_BRIDGE  *RootBus
>)
>  {
>OVMF_PCI_ROOT_BRIDGE_DEVICE_PATH *DevicePath;
> @@ -113,39 +138,19 @@ InitRootBridge (
>  
>RootBus->Segment = 0;
>  
> -  RootBus->Supports   = EFI_PCI_ATTRIBUTE_IDE_PRIMARY_IO |
> -EFI_PCI_ATTRIBUTE_IDE_SECONDARY_IO |
> -EFI_PCI_ATTRIBUTE_ISA_IO_16 |
> -EFI_PCI_ATTRIBUTE_ISA_MOTHERBOARD_IO |
> -EFI_PCI_ATTRIBUTE_VGA_MEMORY |
> -EFI_PCI_ATTRIBUTE_VGA_IO_16  |
> -EFI_PCI_ATTRIBUTE_VGA_PALETTE_IO_16;
> -  RootBus->Attributes = RootBus->Supports;
> +  RootBus->Supports   = Supports;
> +  RootBus->Attributes = Attributes;
>  
>RootBus->DmaAbove4G = FALSE;
>  
> -  RootBus->AllocationAttributes = EFI_PCI_HOST_BRIDGE_COMBINE_MEM_PMEM;
> -  RootBus->PMem.Base= MAX_UINT64;
> -  RootBus->PMem.Limit   = 0;
> -  RootBus->PMemAbove4G.Base = MAX_UINT64;
> -  RootBus->PMemAbove4G.Limit= 0;
> -  RootBus->MemAbove4G.Base  = MAX_UINT64;
> -  RootBus->MemAbove4G.Limit = 0;
> -
> -  if (PcdGet64 (PcdPciMmio64Size) > 0) {
> -RootBus->AllocationAttributes |= EFI_PCI_HOST_BRIDGE_MEM64_DECODE;
> -RootBus->MemAbove4G.Base   = PcdGet64 (PcdPciMmio64Base);
> -RootBus->MemAbove4G.Limit  = PcdGet64 (PcdPciMmio64Base) +
> - (PcdGet64 (PcdPciMmio64Size) - 1);
> -  }
> -
> +  RootBus->AllocationAttributes = AllocAttributes;
>RootBus->Bus.Base  = RootBusNumber;
>RootBus->Bus.Limit = MaxSubBusNumber;
> -  RootBus->Io.Base   = PcdGet64 (PcdPciIoBase);
> -  RootBus->Io.Limit  = PcdGet64 (PcdPciIoBase) + (PcdGet64 (PcdPciIoSize) - 
> 1);
> -  RootBus->Mem.Base  = PcdGet64 (PcdPciMmio32Base);
> -  

Re: [edk2] edk2 llvm branch

[Andrew Fish]

> On May 10, 2016, at 8:05 AM, Shi, Steven  wrote:
> 
> Hi Andrew,
> Thank you for the suggestion. I will try your suggestion and response other 
> questions in your email later. I don't have XCODE5 environment, but could do 
> me a favor and  let me know what current XCODE5 code size for PeiCore.efi and 
> DxeCore.efi in your side? In my side, as below data show, I see the LTO can 
> bring big code size improvement which is quite important for firmware in many 
> scenarios.

I forgot to mention. LTO or not it is good to check to make sure the assembly 
files are getting dead stripped. For example check to make sure you are not 
getting all the assembly functions in the BaseLib included in your executable.  
Some of the assembly is .S and some is .nasm so you may see different behavior 
depending on which assembler was used. 

It is also useful to start looking at the smallest PEIM/DXE drivers 1st as it 
may be easier to spot what is different. 

> Maybe it is also a good idea to enable LTO in XCODE.

For Xcode you add -object_path_lto $(DEST_DIR_DEBUG)/$(BASE_NAME).lto to   
*_XCODE5_*_DLINK_FLAGS. This places the intermediate link code gen in the 
Build/ director vs. a temp director and is important for source level 
debugging. To turn LTO on and off you add -flto to *_XCODE5_*_CC_FLAGS .

We ended up making LTO a configurable build option, so we control it in the DSC 
file. git

[BuildOptions]
!if $(PEI_LTO_ENABLE)
  XCODE:*_*_IA32_PLATFORM_FLAGS = -flto
!endif

!if $(DXE_LTO_ENABLE)
  XCODE:*_*_X64_PLATFORM_FLAGS = -flto
!endif

I included the Xcode 6.3.2 Numbers:

>  
>  
> IA32 DEBUG PeiCore.efi on Ovmf build code size example:
> ToolChainName  PeiCore.efi file size  
>   
> VS2013x86: 40KB 
> CLANGLTO38:42KB   
Xcode  61K

> GCCLTO53:  44KB   
>   
> GCC49:  55KB  
>
> CLANG38:60KB  
>  
>  
> IA32 RELEASE PeiCore.efi on Ovmf build code size example:
> ToolChainName  PeiCore.efi file size  
>   
> VS2013x86: 20KB   
>   
> GCCLTO53:  23KB   
>   
> CLANGLTO38:24KB   
Xcode31K

>   
> GCC49:27KB 
> Clang38:   29KB 
>  
>  
> X64 DEBUG DxeCore.efi on Ovmf build code size example:
> ToolChainName  .efi file size  LZMA 
> Compressed size
> VS2013x86:  137KB 
>57KB
> CLANGLTO38:145KB
> 61KB
Xcode157K  68K

> GCCLTO53:  161KB  
>   63KB
> GCC49:273KB69KB
> CLANG38:205KB 
>72KB
>  
>  
>  
> X64 RELEASE DxeCore.efi on Ovmf build code size example:
> ToolChainName  .efi file size  LZMA 
> Compressed size
> VS2013x86: 95KB   
>44KB
> GCCLTO53:  101KB  
>   46KB
> CLANGLTO38:107KB
> 48KB
Xcode104K  49K

> GCC49:184KB52KB
> CLANG38:133KB 
>53KB
>  

Can you send my linker map files for VS2013 & CLANGLTO38 off list. 

Thanks,

Andrew Fish

> Steven Shi
> Intel\SSG\STO\UEFI Firmware
>  
> Tel: +86 021-61166522
> iNet: 821-6522
>  
>  <>> -Original Message-
> > From: af...@apple.com  [mailto:af...@apple.com 
> > ]
> > Sent: Tuesday, May 10, 2016 1:12 PM
> > To: Shi, Steven >
> > Cc: edk2-devel@lists.01.org 
> > Subject: Re: [edk2] edk2 llvm branch
> > 
> > 

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

Re: [edk2] [PATCH v2 5/7] MdeModulePkg/PciHostBridgeDxe: Honor ResourceAssigned

[Laszlo Ersek]

Hi Ray,

this is a nice patch; I have one cosmetic comment:

On 05/10/16 07:53, Ruiyu Ni wrote:
> Change PciHostBridgeDxe driver to not install the
> PciHostBridgeResourceAllocation protocol and let
> PciRootBridgeIo.Configuration() return the correct PCI resource
> assignment information when the ResourceAssigned is TRUE.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jeff Fan 
> Cc: Laszlo Ersek 
> ---
>  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   | 92 +++--
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |  4 +-
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 95 
> ++
>  3 files changed, 133 insertions(+), 58 deletions(-)

[snip]

> @@ -174,14 +178,41 @@ CreateRootBridge (
>CopyMem (>PMemAbove4G, >PMemAbove4G, sizeof 
> (PCI_ROOT_BRIDGE_APERTURE));
>  
>for (Index = TypeIo; Index < TypeMax; Index++) {
> -RootBridge->ResAllocNode[Index].Type   = Index;
> -RootBridge->ResAllocNode[Index].Base   = 0;
> -RootBridge->ResAllocNode[Index].Length = 0;
> -RootBridge->ResAllocNode[Index].Status = ResNone;
> +switch (Index) {
> +case TypeBus:
> +  Aperture = >Bus;
> +  break;
> +case TypeIo:
> +  Aperture = >Io;
> +  break;
> +case TypeMem32:
> +  Aperture = >Mem;
> +  break;
> +case TypeMem64:
> +  Aperture = >MemAbove4G;
> +  break;
> +case TypePMem32:
> +  Aperture = >PMem;
> +  break;
> +case TypePMem64:
> +  Aperture = >PMemAbove4G;
> +  break;
> +default:

can you add an ASSERT (FALSE); here?

Reviewed-by: Laszlo Ersek 

Thanks!
Laszlo

> +  break;
> +}
> +RootBridge->ResAllocNode[Index].Type = Index;
> +if (Bridge->ResourceAssigned && (Aperture->Limit >= Aperture->Base)) {
> +  RootBridge->ResAllocNode[Index].Base   = Aperture->Base;
> +  RootBridge->ResAllocNode[Index].Length = Aperture->Limit - 
> Aperture->Base + 1;
> +  RootBridge->ResAllocNode[Index].Status = ResAllocated;
> +} else {
> +  RootBridge->ResAllocNode[Index].Base   = 0;
> +  RootBridge->ResAllocNode[Index].Length = 0;
> +  RootBridge->ResAllocNode[Index].Status = ResNone;
> +}
>}
>  
>RootBridge->RootBridgeIo.SegmentNumber  = Bridge->Segment;
> -  RootBridge->RootBridgeIo.ParentHandle   = HostBridgeHandle;
>RootBridge->RootBridgeIo.PollMem= RootBridgeIoPollMem;
>RootBridge->RootBridgeIo.PollIo = RootBridgeIoPollIo;
>RootBridge->RootBridgeIo.Mem.Read   = RootBridgeIoMemRead;
> 

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

Re: [edk2] [PATCH] BaseTools AARCH64/ARM: remove -save-temps GCC option

[Andrew Fish]

> On May 10, 2016, at 8:53 AM, Ard Biesheuvel  wrote:
> 
> On 10 May 2016 at 17:18, Evan Lloyd  wrote:
>> Hi Ard.
>> This is not a major inconvenience, but we do occasionally find the 
>> -save-temps flag vital when debugging.
>> What does seem strange is using GCC_ARM_CC_FLAGS, etc for a CLANG compiler.
> 
> Since Clang is mostly GCC compatible in terms of command line options,
> the ARM and AARCH64 CLANG35 toolchain tag is part of the GCC toolchain
> family. That by itself is not a reason to have completely separate CC
> flag definitions, though. I just never thought about it.
> 
>> Surely there ought to be CLANG_*_CC_FLAGS (which could be played with to 
>> your hearts content, without disrupting other compilers).
>> 
> 

It looks like Steven is working on things clang, including LTO. 

On the Xcode/clang side there is a linker flag, -object_path_lto 
$(DEST_DIR_DEBUG)/$(BASE_NAME).lto, that needs to get added to store temp 
output of the LTO generation to support debug, but there is not a compiler flag 
required. The Xcode clang flags are very GCC like, but the linker is totally 
different.

Thanks,

Andrew Fish



> Indeed. Will go that route instead.
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

Re: [edk2] [Patch] SecurityPkg: SecureBootConfigDxe: Add NULL pointer check

[El-Haj-Mahmoud, Samer]

Reviewed-by: Samer El-Haj-Mahmoud 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Zhang, 
Chao B
Sent: Tuesday, May 10, 2016 2:52 AM
To: edk2-devel@lists.01.org
Cc: shumin@intel.com; Chao Zhang 
Subject: [edk2] [Patch] SecurityPkg: SecureBootConfigDxe: Add NULL pointer check

Add SecureBoot NULL pointer check before reference it.

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

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 088fa26..3f80441 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -2933,7 +2933,7 @@ SecureBootExtractConfigFromVariable (
   //
   // Fix Pk, SecureBootEnable inconsistence
   //
-  if ((*SetupMode) == USER_MODE) {
+  if ((SetupMode != NULL) && (*SetupMode) == USER_MODE) {
 ConfigData->HideSecureBoot = FALSE;
 if ((SecureBootEnable != NULL) && (*SecureBootEnable == 
SECURE_BOOT_ENABLE)) {
   ConfigData->AttemptSecureBoot = TRUE;
-- 
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] BaseTools AARCH64/ARM: remove -save-temps GCC option

[Ard Biesheuvel]

On 10 May 2016 at 17:18, Evan Lloyd  wrote:
> Hi Ard.
> This is not a major inconvenience, but we do occasionally find the 
> -save-temps flag vital when debugging.
> What does seem strange is using GCC_ARM_CC_FLAGS, etc for a CLANG compiler.

Since Clang is mostly GCC compatible in terms of command line options,
the ARM and AARCH64 CLANG35 toolchain tag is part of the GCC toolchain
family. That by itself is not a reason to have completely separate CC
flag definitions, though. I just never thought about it.

> Surely there ought to be CLANG_*_CC_FLAGS (which could be played with to your 
> hearts content, without disrupting other compilers).
>

Indeed. Will go that route instead.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] EDK2 Platforms Proposal

[Kinney, Michael D]

Leif,

You raise some very good concerns.  I do not think edk2-platforms is intended to
solve all of these, and I agree that a branch per platform can make some things 
much worse, especially if there are many different core overrides in platform 
branches.

We need to consider different types of platform ports and figure out the right 
landing zone for each.

1) Platforms required to validate a core feature in edk2/master
2) Platforms that require a validated/stable release of edk2
3) Platforms that are always synced with edk2/master, but do not improve edk2 
core feature coverage.
4) Platforms that are under active initial porting efforts, have stability 
issues, or code quality issues.

edk2/master works for (1)
edk2-platforms/ works for (2) and (4)
As platforms in category (4) mature they migrate into (1), (2), or (3)

We do not have a landing zone for (3).  Maybe we can have a special branch in 
edk2-platforms that contains all the platforms in this category, so we can make 
sure things like common overrides and features common to many different 
platforms can be easily identified.  This special branch can be
auto synced to edk2/master so incompatibilities introduced by platform changes 
or edk2/master
changes can be quickly identified.

I am looking into a different proposal to add some tree organization to 
edk2/master.  Part of that 
includes a landing zone for vendor specific UEFI/PI device drivers that can be 
used by many
different platforms.

Let's continue to refine the set of proposals here to address all of the 
important issues you have raised.

Best regards,

Mike

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Tuesday, May 10, 2016 3:06 AM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC] EDK2 Platforms Proposal
> 
> Hi Mike,
> 
> Apologies for delay in responding, this was a topic I wanted to leave
> some time to think about.
> 
> On Tue, May 03, 2016 at 04:30:36PM +, Kinney, Michael D wrote:
> > Similar to edk2-staging, we also have a need to manage platforms
> > that have been ported to edk2.  Jordan has created a repository
> > called edk2-platforms and has created a branch for the
> > minnowboard-max that uses a validated release of the UDK 2015 for
> > the dependent packages:
> >
> > https://github.com/tianocore/edk2-platforms
> >
> > https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015
> >
> > Instead of creating a branch per feature in edk2-staging, the
> > proposal is to create a branch per platform in edk2-platforms.  The
> > maintainer(s) that create and support a platform branch can decide
> > if the platform is synced to edk2/master for dependent packages, or
> > uses a stable release of the edk2 for dependent packages.
> >
> > This proposal provides an area for platform development so we can
> > minimize the number of platforms that are included in edk2/master.
> > It is important to keep some platforms in edk2/master so we can use
> > those platforms to validate features in non-platform packages in
> > edk2/master.  If a new platform does not add feature coverage to
> > edk2/master, then a new edk2-platforms branch would be recommended.
> >
> > Please review the proposal below for edk2-platforms.
> >
> > If this proposal is accepted, then a review of the platform in
> > edk2/master can Be done to see if any of them should be moved to
> > branches in edk2-platforms.
> 
> First of all, let me reiterate - I very much want to see more public
> platform code. To the extent this helps that happen, I am a strong
> supporter.
> 
> However, this proposal does not really address the issues I've been
> pursuing, and am currently staging in my OpenPlatformPkg tree:
> - The ability to see when a core change breaks some/all platform
>   ports.
> - The ability to easily spot common code that should be turned into
>   core libraries.
>   - The ability to easily spot near-identical local overrides to
> common libraries that should turn into options in an existing core
> library.
> - The ability to share drivers for common components between different
>   platforms, benefitting from feature additions and bugfixes.
> 
> It may not be intending to do this, which is fine, but I'd like to
> share a few warnings - seen both in Linaro's Linux kernel work and in
> Linaro's UEFI work when the "platform feature branch" approach was
> used (also try an Internet search for "vendor kernels"):
> - Keeping each platform port in a separate branch will inevitably lead
>   to incompatible changes to core components.
>   - Some of these changes will be fundamentally incompatible with
> other platforms, because they were made in complete isolation from
> those other platforms.
>   - Mandating that all ports are based on a UDK version reduces the
> scope of these incompatibilities, but ports will inevitably want
> access to newer features that what is 

Re: [edk2] OVMF boot order and efivars persistence questions

[Laszlo Ersek]

On 05/10/16 06:34, Gary Lin wrote:
> On Mon, May 09, 2016 at 02:58:00PM +0200, Laszlo Ersek wrote:
>> On 05/09/16 13:39, Thomas Lamprecht wrote:
> [snip]
>>> I caught a few changes regarding the boot
>>> order and its settings.
>>>
>>> Is it by design that not all devices listed in the boot order section
>>> get probed (anymore)?
>>> E.g. it tries only EFI CD/ROM which is the first by default and then
>>> gives up printing an error and after an timeout I get the UEFI shell.
>>> If I open the UEFI settings and directly choose "EFI Misc Device" I get
>>> the working grub2 of my VM, but this was not needed in the earlier
>>> version, there it probed all devices until something "bootable" found
>>> and only if all were tried it did throw an error.
>>>
>>> Am I'm missing something here, or using it incorrect?
>>
>> I think you may be encountering a change where it is out of scope for
>> OVMF to ensure stability.
>>
>> (You can try to build OVMF with -D USE_OLD_BDS to see if the
>> IntelFrameworkModulePkg BDS is whose behavior you are missing.)
>>
>> There are two things to consider when talking about the boot order. The
>> first is the generic BDS library from edk2 that OVMF uses to enumerate
>> all possible boot options. The second is the boot option filtering and
>> reordering that OVMF does itself, staring out from the full enumeration.
>>
>> * Regarding the first part, the BDS library (and the related BDS
>> driver), OVMF has been very recently migrated from the
>> IntelFrameworkModulePkg BDS to the new core BDS, under MdeModulePkg. See
>> . Setting -D USE_OLD_BDS
>> allows users to turn off this change (temporarily -- we'll soon remove
>> the fallback).
>>
>> The reason for the new BDS infrastructure is that it follows the UEFI
>> spec much more closely, and that it separates out UI and BDS driver
>> responsibilities to separate modules.
>>
>> * Regarding the second part, OVMF consults the "bootorder" firmware
>> config file from QEMU, translates it to UEFI boot option fragments, and
>> reorders and filters the fully enumerated boot option list (from the
>> previous step) against it.
>>
>> Given that some UEFI boot options (that are auto-generated and useful,
>> like the UEFI shell) are not expressible by QEMU in the "bootorder"
>> firmware config file, OVMF preserves such UEFI boot options at the end
>> of the boot order.
>>
>> The observable behavior is that:
>>
>> - what you pass in the "bootorder" fw_cfg file, will be tried for
>> booting, in the order that you passed it in,
>> - what you do not pass, *although you could* (because QEMU could express
>> it), will not be booted,
>> - what you do not pass (because QEMU cannot express it), will be
>> preserved at the end of the boot order list, in some unspecified order.
>>
>> If you don't pass a "bootorder" fw_cfg file at all, then the first two
>> items fall away, and only the last item takes effect.
>>
>> I suspect that the boot order change you are witnessing is due to that
>> "unspecified order", in the last item. The auto-generation of boot
>> options (by the generic BDS library) keeps preexistent boot options at
>> the beginning of the full set, and adds new ones at the end, but the
>> order of "new ones" at the end may have changed, due to the patches for
>> issue #62.
>>
>> Again, keeping this order "stable" (as far as item #3 on the above list
>> is concerned) is out of scope for OVMF. If you want a stable boot order,
>> you must instruct QEMU to expose the "boorder" fw_cfg file to OVMF.
>>
>> On the QEMU command line, this is possible by adding the "bootindex=N"
>> property to the "-device " options. For example,
>>
>>   -device virtio-blk-pci,...,bootindex=2
>>
>> In the libvirt domain XML, it is possible by adding the following
>> element to disk, network, assigned device (= hostdev) etc elements:
>>
>>   
>>
> Speaking of the boot order, I would like to propose to postpone the
> registration of the internal shell after EfiBootManagerRefreshAllBootOption()
> in PlatformBootManagerAfterConsole(). The reason is to align to the
> behavior of the old BDS. Besides, Xen doesn't support fw_cfg and pflash,
> so it is really a headache to change the boot order in Xen :(
> With the current order, the Xen HVM always ran into the shell instead of
> DVD or the hard disk if there was no one to interfere. Lowering the
> priority of shell gives the block devices the chance to boot at least.
> I know the best solution is to make Xen support both fw_cfg and pflash,
> but it seems not going to happen in the short term.

I think this makes sense.

However, in this case it's not just the UEFI shell, but also the memory
mapped UiApp application (practically the setup UI). If you move one to
the end, you should probably move the other one as well.

Gary, can you please submit a patch that works for you, CC'ing Ray in
addition to the OvmfPkg maintainers? The new BDS is, well, pretty new in
OvmfPkg, so for now I'd like 

Re: [edk2] [PATCH] BaseTools AARCH64/ARM: remove -save-temps GCC option

[Evan Lloyd]

Hi Ard.
This is not a major inconvenience, but we do occasionally find the -save-temps 
flag vital when debugging.
What does seem strange is using GCC_ARM_CC_FLAGS, etc for a CLANG compiler.
Surely there ought to be CLANG_*_CC_FLAGS (which could be played with to your 
hearts content, without disrupting other compilers).

Regards,
Evan

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: 10 May 2016 14:56
To: edk2-devel@lists.01.org; leif.lindh...@linaro.org
Cc: Ard Biesheuvel
Subject: [edk2] [PATCH] BaseTools AARCH64/ARM: remove -save-temps GCC option

Recent CLANG version choke on the -save-temps options. Of course, this is not 
GCC's fault, but since it seems we don't need this option, just remove it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2065fa34998f..616a27871978 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4323,8 +4323,8 @@ DEFINE GCC_ALL_CC_FLAGS= -g -Os -fshort-wchar 
-fno-strict-aliasing -
 DEFINE GCC_IA32_CC_FLAGS   = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double 
-freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe
 DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
-Wno-address -mno-stack-arg-probe
 DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-minline-int-divide-min-latency
-DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -save-temps -funsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
-DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
+DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
+DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
 DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
--
2.7.4

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

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.

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

Re: [edk2] edk2 llvm branch

[Shi, Steven]

Hi Andrew,

Thank you for the suggestion. I will try your suggestion and response other 
questions in your email later. I don't have XCODE5 environment, but could do me 
a favor and  let me know what current XCODE5 code size for PeiCore.efi and 
DxeCore.efi in your side? In my side, as below data show, I see the LTO can 
bring big code size improvement which is quite important for firmware in many 
scenarios. Maybe it is also a good idea to enable LTO in XCODE.





IA32 DEBUG PeiCore.efi on Ovmf build code size example:

ToolChainName  PeiCore.efi file size

VS2013x86: 40KB

CLANGLTO38:42KB

GCCLTO53:  44KB

GCC49:  55KB

CLANG38:60KB





IA32 RELEASE PeiCore.efi on Ovmf build code size example:

ToolChainName  PeiCore.efi file size

VS2013x86: 20KB

GCCLTO53:  23KB

CLANGLTO38:24KB

GCC49:27KB

Clang38:   29KB





X64 DEBUG DxeCore.efi on Ovmf build code size example:

ToolChainName  .efi file size  LZMA 
Compressed size

VS2013x86:  137KB   
 57KB

CLANGLTO38:145KB61KB

GCCLTO53:  161KB
63KB

GCC49:273KB69KB

CLANG38:205KB   
 72KB







X64 RELEASE DxeCore.efi on Ovmf build code size example:

ToolChainName  .efi file size  LZMA 
Compressed size

VS2013x86: 95KB 
 44KB

GCCLTO53:  101KB
46KB

CLANGLTO38:107KB48KB

GCC49:184KB52KB

CLANG38:133KB   
 53KB



Steven Shi

Intel\SSG\STO\UEFI Firmware



Tel: +86 021-61166522

iNet: 821-6522



> -Original Message-

> From: af...@apple.com [mailto:af...@apple.com]

> Sent: Tuesday, May 10, 2016 1:12 PM

> To: Shi, Steven 

> Cc: edk2-devel@lists.01.org

> Subject: Re: [edk2] edk2 llvm branch

>

>


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

[edk2] [PATCH] BaseTools AARCH64/ARM: remove -save-temps GCC option

[Ard Biesheuvel]

Recent CLANG version choke on the -save-temps options. Of course, this
is not GCC's fault, but since it seems we don't need this option, just
remove it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 2065fa34998f..616a27871978 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4323,8 +4323,8 @@ DEFINE GCC_ALL_CC_FLAGS= -g -Os -fshort-wchar 
-fno-strict-aliasing -
 DEFINE GCC_IA32_CC_FLAGS   = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double 
-freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe
 DEFINE GCC_X64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mno-red-zone 
-Wno-address -mno-stack-arg-probe
 DEFINE GCC_IPF_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-minline-int-divide-min-latency
-DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -save-temps -funsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
-DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -save-temps -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
+DEFINE GCC_ARM_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections 
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
+DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address 
-fno-asynchronous-unwind-tables
 DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
-- 
2.7.4

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

Re: [edk2] [PATCH 0/4] EmbeddedPkg/Lan9118Dxe MMIO fixes

[Ard Biesheuvel]

On 9 May 2016 at 11:22, Mark Rutland  wrote:
> On Sat, May 07, 2016 at 10:43:45AM +0200, Ard Biesheuvel wrote:
>> On 6 May 2016 at 19:19, Mark Rutland  wrote:
>> > The LAN9118 driver uses memory fences in a novel but erroneous fashion, 
>> > due to
>> > a misunderstanding of some under-commented code. This series fixes these
>> > erroneous uses, documenting the unusual requirements of the LAN9118 chip 
>> > that
>> > lead us to this situation, and introduces new helpers to handle this in a 
>> > more
>> > consistent fashion.
>> >
>> > The LAN9118 datasheet is publicly available at:
>> >
>> > http://www.microchip.com/wwwproducts/en/LAN9118
>> >
>>
>> Thanks a lot for getting to the bottom of this! I particularly like
>> the way how you folded the required delays into the MMIO read/write
>> functions, which makes the top level code a lot cleaner.
>>
>> I can't test this, but the code looks fine to me.
>>
>> Reviewed-by: Ard Biesheuvel 
>
> Cheers!
>
> FWIW, I've tested each patch on Juno R1, and I haven't seen any
> regression as a result of this. That said, I haven't been able to
> trigger issues even without this series.
>
> There's another latent bug that this doesn't solve, in that if the PHY
> negotiates full-duplex operation (at 100Mb/s or 10Mb/s), but that
> appears to be unrelated.
>

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

Re: [edk2] [PATCH 0/4] EmbeddedPkg/Lan9118Dxe MMIO fixes

[Mark Rutland]

On Mon, May 09, 2016 at 07:02:16PM +0100, Ryan Harkin wrote:
> On 9 May 2016 at 11:07, Ryan Harkin  wrote:
> > On 9 May 2016 at 10:22, Mark Rutland  wrote:
> >> On Sat, May 07, 2016 at 10:43:45AM +0200, Ard Biesheuvel wrote:
> >>> On 6 May 2016 at 19:19, Mark Rutland  wrote:
> >>> > The LAN9118 driver uses memory fences in a novel but erroneous fashion, 
> >>> > due to
> >>> > a misunderstanding of some under-commented code. This series fixes these
> >>> > erroneous uses, documenting the unusual requirements of the LAN9118 
> >>> > chip that
> >>> > lead us to this situation, and introduces new helpers to handle this in 
> >>> > a more
> >>> > consistent fashion.
> >>> >
> >>> > The LAN9118 datasheet is publicly available at:
> >>> >
> >>> > http://www.microchip.com/wwwproducts/en/LAN9118
> >>> >
> >>>
> >>> Thanks a lot for getting to the bottom of this! I particularly like
> >>> the way how you folded the required delays into the MMIO read/write
> >>> functions, which makes the top level code a lot cleaner.
> >>>
> >>> I can't test this, but the code looks fine to me.
> >>>
> >
> > I'll test it later today on TC2 and Juno R0/1/2.  But I like the look
> > of it, it seems like a huge improvement.
> >
> 
> This seems to work on TC2 and Juno R0, R1 and R2, although I'm not
> 100% sure it's reliable, so I need to do more testing.
> 
> When attempting to install debian over the network, my Juno R1 has
> just reported:
> 
> LAN9118: There was an error transmitting. TxStatus=0x8401:- No carrier
> - Packet tx was deferred
> 
> The 2nd attempt seems to be working fine.  Juno R0 and R2 seem happy enough.

I suspect you're hitting the latent, intermittent issue with full duplex
operation that I mentioned in my other reply. This occurs without these
patches, and was what I was trying to fix when I noticed the suspicious
barrier usage.

If you early return from AutoNegotiate (e.g. because the timeout is
short enough), or remove the full duplex bits from the Feature register,
this works, this issue does not manifest (at least in my experience), as
the HW will auto-negotiate a half-duplex connection anyway.

Evidently, we don't have a good test case for this. I've found that it's
fairly reproducable when using TFTP from GRUB (using SNP).

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

Re: [edk2] [PATCH 4/5] ArmPkg/ArmDmaLib: reject consistent DMA mappings of cached memory

[Leif Lindholm]

On Tue, May 10, 2016 at 12:58:08PM +0200, Ard Biesheuvel wrote:
> On 9 May 2016 at 22:17, Leif Lindholm  wrote:
> > On Mon, May 09, 2016 at 06:44:42PM +0200, Ard Biesheuvel wrote:
> >> On 9 May 2016 at 18:37, Leif Lindholm  wrote:
> >> > On Tue, Apr 19, 2016 at 04:55:33PM +0200, Ard Biesheuvel wrote:
> >> >> DmaMap () operations of type MapOperationBusMasterCommonBuffer should
> >> >> return a mapping that is coherent between the CPU and the device. For
> >> >> this reason, the API only allows DmaMap () to be called with this 
> >> >> operation
> >> >> type if the memory to be mapped was allocated by DmaAllocateBuffer (),
> >> >> which in this implementation guarantees the coherency by using uncached
> >> >> mappings on the CPU side.
> >> >>
> >> >> This means that, if we encounter a cached mapping in DmaMap () with this
> >> >> operation type, the code is either broken, or someone is violating the
> >> >> API, but simply proceeding with a double buffer makes no sense at all,
> >> >> and can only cause problems.
> >> >>
> >> >> So instead, actively reject this operation type for cached memory 
> >> >> mappings.
> >> >>
> >> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> >> Signed-off-by: Ard Biesheuvel 
> >> >> ---
> >> >>  ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 18 --
> >> >>  1 file changed, 16 insertions(+), 2 deletions(-)
> >> >>
> >> >> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c 
> >> >> b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> >> >> index 7f6598318a91..7e518ed3b83e 100644
> >> >> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> >> >> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
> >> >> @@ -103,6 +103,18 @@ DmaMap (
> >> >>  // If the mapped buffer is not an uncached buffer
> >> >>  if ((GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) 
> >> >> != 0) {
> >> >>//
> >> >> +  // Operations of type MapOperationBusMasterCommonBuffer are only 
> >> >> allowed
> >> >> +  // on uncached buffers.
> >> >> +  //
> >> >> +  if (Operation == MapOperationBusMasterCommonBuffer) {
> >> >> +DEBUG ((EFI_D_ERROR,
> >> >> +  "%a: Operation type 'MapOperationBusMasterCommonBuffer' is 
> >> >> only supported\n"
> >> >> +  "on memory regions that were allocated using 
> >> >> DmaAllocateBuffer ()\n",
> >> >> +  __FUNCTION__));
> >> >> +return EFI_UNSUPPORTED;
> >> >> +  }
> >> >> +
> >> >> +  //
> >> >>// If the buffer does not fill entire cache lines we must double 
> >> >> buffer into
> >> >>// uncached memory. Device (PCI) address becomes uncached page.
> >> >>//
> >> >> @@ -112,7 +124,7 @@ DmaMap (
> >> >>  return Status;
> >> >>}
> >> >>
> >> >> -  if ((Operation == MapOperationBusMasterRead) || (Operation == 
> >> >> MapOperationBusMasterCommonBuffer)) {
> >> >> +  if (Operation == MapOperationBusMasterRead) {
> >> >>  CopyMem (Buffer, HostAddress, *NumberOfBytes);
> >> >>}
> >> >>
> >> >> @@ -168,7 +180,9 @@ DmaUnmap (
> >> >>Map = (MAP_INFO_INSTANCE *)Mapping;
> >> >>
> >> >>if (Map->DoubleBuffer) {
> >> >> -if ((Map->Operation == MapOperationBusMasterWrite) || 
> >> >> (Map->Operation == MapOperationBusMasterCommonBuffer)) {
> >> >> +ASSERT (Map->Operation != MapOperationBusMasterCommonBuffer);
> >> >
> >> > Would it be more correct to return EFI_DEVICE_ERROR if this
> >> > condition became true?
> >>
> >> I don't think so. We should never create double buffer mappings for
> >> MapOperationBusMasterCommonBuffer operations, so in the unmap path, an
> >> ASSERT () is appropriate, since the code is doing something *very* if
> >> this ever occurs.
> >
> > Yeah, that was kind of my meaning:
> > The only way it could happen would be through
> > * memory corruption
> > * intentional manipulation of the structure
> > both of which would make it hard to guarantee that the data had been
> > "committed to the target system memory".
> >
> 
> Indeed. So an ASSERT () is appropriate here, but I guess you were
> looking for an equivalent in RELEASE builds? In that case,
> EFI_INVALID_PARAMETER seems more appropriate

Yeah - so could we have both?
If so, just change and push.

> > Anyway, it's not a hard requirement, more of a discussion point.
> >
> > Reviewed-by: Leif Lindholm 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] I$ maintenance in ArmCacheMaintenanceLib.c

[Achin Gupta]

BTW, digging it a bit deeper and adding edk2-devel belatedly!

For ARMv7 (assuming that is where the code was lifted from), it might be that
the entire I$ was invaidated since it did the same to the branch predictor. The
recommended sequence, as per the ARMv7 ARM, Section A3.5.4, should be:

DCCMVAU [instruction location] ; Clean data cache by MVA to point of unification
DSB; Ensure visibility of the data cleaned from the 
cache
ICIMVAU [instruction location] ; Invalidate instruction cache by MVA to PoU
BPIMVAU [instruction location] ; Invalidate branch predictor by MVA to PoU
DSB; Ensure completion of the invalidations
ISB; Synchronize fetched instruction stream

thanks,
Achin

On Tue, May 10, 2016 at 09:46:03AM +0100, Achin Gupta wrote:
> Thanks Eugene! Thought as much but wanted to be sure. I will put up something
> for review in a bit.
>
> On Mon, May 09, 2016 at 10:53:52PM +, Cohen, Eugene wrote:
> >
> > Through the power of 'git blame' it looks like this dates back to the dawn 
> > of time (Andrew Fish and Olivier) and predates ARMv8.
> >
> > On AArch64 this is implemented thusly:
> >
> >   ic  iallu   // Invalidate entire instruction cache
> >   dsb sy
> >   isb
> >   ret
> >
> > ARMv8 shows an IC IVAU at C5.4.11 so I think we can switch this to a cache 
> > range operation.
> >
> > Eugene
> >
> > > -Original Message-
> > > From: Achin Gupta [mailto:achin.gu...@arm.com]
> > > Sent: Monday, May 09, 2016 1:33 PM
> > > To: Ard Biesheuvel ; Leif Lindholm
> > > ; Cohen, Eugene 
> > > Subject: I$ maintenance in ArmCacheMaintenanceLib.c
> > >
> > > Hi All,
> > >
> > > Does anyone of you know why on ARM platforms, the entire instruction
> > > cache is invalidated in the following function :
> > >
> > > VOID *
> > > EFIAPI
> > > InvalidateInstructionCacheRange (
> > >   IN  VOID  *Address,
> > >   IN  UINTN Length
> > >   )
> > > {
> > >   CacheRangeOperation (Address, Length,
> > > ArmCleanDataCacheEntryToPoUByMVA);
> > >   ArmInvalidateInstructionCache ();
> > >   return Address;
> > > }
> > >
> > > Afaics, there is no instruction to perform a "IC IVAU" in
> > > ArmPkg/Library/ArmLib/AArch64/AArch64Support.S. Instead of
> > > ArmInvalidateInstructionCache() there should be another
> > > CacheRangeOperation() for the ISide.
> > >
> > > There is another problem that CacheRangeOperation() caters for only
> > > the Data cache line length even though the function name is generic.
> > >
> > > I am looking at the Tianocore edk2 tree (commit 9f64a83). Please let me
> > > know if I am missing anything, else I will cook up some patches to fix the
> > > problem.
> > >
> > > Thanks,
> > > Achin
> > >

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

Re: [edk2] [PATCH 4/5] ArmPkg/ArmDmaLib: reject consistent DMA mappings of cached memory

[Ard Biesheuvel]

On 9 May 2016 at 22:17, Leif Lindholm  wrote:
> On Mon, May 09, 2016 at 06:44:42PM +0200, Ard Biesheuvel wrote:
>> On 9 May 2016 at 18:37, Leif Lindholm  wrote:
>> > On Tue, Apr 19, 2016 at 04:55:33PM +0200, Ard Biesheuvel wrote:
>> >> DmaMap () operations of type MapOperationBusMasterCommonBuffer should
>> >> return a mapping that is coherent between the CPU and the device. For
>> >> this reason, the API only allows DmaMap () to be called with this 
>> >> operation
>> >> type if the memory to be mapped was allocated by DmaAllocateBuffer (),
>> >> which in this implementation guarantees the coherency by using uncached
>> >> mappings on the CPU side.
>> >>
>> >> This means that, if we encounter a cached mapping in DmaMap () with this
>> >> operation type, the code is either broken, or someone is violating the
>> >> API, but simply proceeding with a double buffer makes no sense at all,
>> >> and can only cause problems.
>> >>
>> >> So instead, actively reject this operation type for cached memory 
>> >> mappings.
>> >>
>> >> Contributed-under: TianoCore Contribution Agreement 1.0
>> >> Signed-off-by: Ard Biesheuvel 
>> >> ---
>> >>  ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 18 --
>> >>  1 file changed, 16 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c 
>> >> b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>> >> index 7f6598318a91..7e518ed3b83e 100644
>> >> --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>> >> +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
>> >> @@ -103,6 +103,18 @@ DmaMap (
>> >>  // If the mapped buffer is not an uncached buffer
>> >>  if ((GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) != 
>> >> 0) {
>> >>//
>> >> +  // Operations of type MapOperationBusMasterCommonBuffer are only 
>> >> allowed
>> >> +  // on uncached buffers.
>> >> +  //
>> >> +  if (Operation == MapOperationBusMasterCommonBuffer) {
>> >> +DEBUG ((EFI_D_ERROR,
>> >> +  "%a: Operation type 'MapOperationBusMasterCommonBuffer' is 
>> >> only supported\n"
>> >> +  "on memory regions that were allocated using DmaAllocateBuffer 
>> >> ()\n",
>> >> +  __FUNCTION__));
>> >> +return EFI_UNSUPPORTED;
>> >> +  }
>> >> +
>> >> +  //
>> >>// If the buffer does not fill entire cache lines we must double 
>> >> buffer into
>> >>// uncached memory. Device (PCI) address becomes uncached page.
>> >>//
>> >> @@ -112,7 +124,7 @@ DmaMap (
>> >>  return Status;
>> >>}
>> >>
>> >> -  if ((Operation == MapOperationBusMasterRead) || (Operation == 
>> >> MapOperationBusMasterCommonBuffer)) {
>> >> +  if (Operation == MapOperationBusMasterRead) {
>> >>  CopyMem (Buffer, HostAddress, *NumberOfBytes);
>> >>}
>> >>
>> >> @@ -168,7 +180,9 @@ DmaUnmap (
>> >>Map = (MAP_INFO_INSTANCE *)Mapping;
>> >>
>> >>if (Map->DoubleBuffer) {
>> >> -if ((Map->Operation == MapOperationBusMasterWrite) || 
>> >> (Map->Operation == MapOperationBusMasterCommonBuffer)) {
>> >> +ASSERT (Map->Operation != MapOperationBusMasterCommonBuffer);
>> >
>> > Would it be more correct to return EFI_DEVICE_ERROR if this
>> > condition became true?
>>
>> I don't think so. We should never create double buffer mappings for
>> MapOperationBusMasterCommonBuffer operations, so in the unmap path, an
>> ASSERT () is appropriate, since the code is doing something *very* if
>> this ever occurs.
>
> Yeah, that was kind of my meaning:
> The only way it could happen would be through
> * memory corruption
> * intentional manipulation of the structure
> both of which would make it hard to guarantee that the data had been
> "committed to the target system memory".
>

Indeed. So an ASSERT () is appropriate here, but I guess you were
looking for an equivalent in RELEASE builds? In that case,
EFI_INVALID_PARAMETER seems more appropriate

> Anyway, it's not a hard requirement, more of a discussion point.
>
> Reviewed-by: Leif Lindholm 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] BaseTools: fix a bug for uni file \x####\ format handling

[Yonghong Zhu]

It should start from the last '\x' position + 1 to find next '\x'
character.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/UniClassObject.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/Python/AutoGen/UniClassObject.py 
b/BaseTools/Source/Python/AutoGen/UniClassObject.py
index d28fd3a..183b2b2 100644
--- a/BaseTools/Source/Python/AutoGen/UniClassObject.py
+++ b/BaseTools/Source/Python/AutoGen/UniClassObject.py
@@ -442,11 +442,11 @@ class UniFileClassObject(object):
 if EndStr.startswith(u'\\x') and len(EndStr) >= 7:
 if EndStr[6] == u'\\' and 
re.match('[a-fA-F0-9]{4}', EndStr[2 : 6], re.UNICODE):
 Line = Line[0 : StartPos] + UniStr + EndStr
 else:
 Line = Line[0 : StartPos] + UniStr + EndStr[1:]
-StartPos = Line.find(u'\\x', StartPos)
+StartPos = Line.find(u'\\x', StartPos + 1)
 
 IncList = gIncludePattern.findall(Line)
 if len(IncList) == 1:
 for Dir in [File.Dir] + self.IncludePathList:
 IncFile = PathClass(str(IncList[0]), Dir)
-- 
2.6.1.windows.1

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

[edk2] [Patch] BaseTools: support private package definition

[Yonghong Zhu]

EDKII build spec and DEC spec updated to support private package
definition.
If GUID, Protocol or PPI is listed in a DEC file, where the  Private
modifier is used in the section tag ([Guids.common.Private] for example),
only modules within the package are permitted to use the GUID, Protocol
or PPI. If a module or library instance outside of the package attempts
to use the item, the build must fail with an appropriate error message.

Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Yonghong Zhu 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py | 10 ++--
 BaseTools/Source/Python/Common/Misc.py | 29 ---
 .../Source/Python/Workspace/MetaFileParser.py  | 12 +
 BaseTools/Source/Python/Workspace/MetaFileTable.py |  4 +-
 .../Source/Python/Workspace/WorkspaceDatabase.py   | 59 +-
 5 files changed, 90 insertions(+), 24 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index ae0f8a6..2f2bed0 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2172,11 +2172,11 @@ class PlatformAutoGen(AutoGen):
 self._OverridePcd(PcdInModule, PcdInPlatform, Module)
 # resolve the VariableGuid value
 for SkuId in PcdInModule.SkuInfoList:
 Sku = PcdInModule.SkuInfoList[SkuId]
 if Sku.VariableGuid == '': continue
-Sku.VariableGuidValue = GuidValue(Sku.VariableGuid, 
self.PackageList)
+Sku.VariableGuidValue = GuidValue(Sku.VariableGuid, 
self.PackageList, self.MetaFile.Path)
 if Sku.VariableGuidValue == None:
 PackageList = "\n\t".join([str(P) for P in 
self.PackageList])
 EdkLogger.error(
 'build',
 RESOURCE_NOT_AVAILABLE,
@@ -3392,11 +3392,15 @@ class ModuleAutoGen(AutoGen):
 
 for Package in self.Module.Packages:
 PackageDir = mws.join(self.WorkspaceDir, Package.MetaFile.Dir)
 if PackageDir not in self._IncludePathList:
 self._IncludePathList.append(PackageDir)
-for Inc in Package.Includes:
+IncludesList = Package.Includes
+if Package._PrivateIncludes:
+if not self.MetaFile.Path.startswith(PackageDir):
+IncludesList = 
list(set(Package.Includes).difference(set(Package._PrivateIncludes)))
+for Inc in IncludesList:
 if Inc not in self._IncludePathList:
 self._IncludePathList.append(str(Inc))
 return self._IncludePathList
 
 def _GetIncludePathLength(self):
@@ -3459,11 +3463,11 @@ class ModuleAutoGen(AutoGen):
 if Pcd.Type != TAB_PCDS_DYNAMIC_EX_HII:
 continue
 for SkuName in Pcd.SkuInfoList:
 SkuInfo = Pcd.SkuInfoList[SkuName]
 Name = ConvertStringToByteArray(SkuInfo.VariableName)
-Value = GuidValue(SkuInfo.VariableGuid, 
self.PlatformInfo.PackageList)
+Value = GuidValue(SkuInfo.VariableGuid, 
self.PlatformInfo.PackageList, self.MetaFile.Path)
 if not Value:
 continue
 Guid = GuidStructureStringToGuidString(Value)
 if (Name, Guid) in NameGuids and Pcd not in HiiExPcds:
 HiiExPcds.append(Pcd)
diff --git a/BaseTools/Source/Python/Common/Misc.py 
b/BaseTools/Source/Python/Common/Misc.py
index 777450d..c99716d 100644
--- a/BaseTools/Source/Python/Common/Misc.py
+++ b/BaseTools/Source/Python/Common/Misc.py
@@ -1,9 +1,9 @@
 ## @file
 # Common routines used by all tools
 #
-# Copyright (c) 2007 - 2015, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
 # http://opensource.org/licenses/bsd-license.php
 #
@@ -792,45 +792,60 @@ def GetRelPath(Path1, Path2):
 
 ## Get GUID value from given packages
 #
 #   @param  CName   The CName of the GUID
 #   @param  PackageList List of packages looking-up in
+#   @param  Inffile The driver file
 #
 #   @retval GuidValue   if the CName is found in any given package
 #   @retval Noneif the CName is not found in all given packages
 #
-def GuidValue(CName, PackageList):
+def GuidValue(CName, PackageList, Inffile = None):
 for P in PackageList:
-if CName in P.Guids:
+GuidKeys = P.Guids.keys()
+if Inffile and P._PrivateGuids:
+if not 

Re: [edk2] [RFC] EDK2 Platforms Proposal

[Leif Lindholm]

Hi Mike,

Apologies for delay in responding, this was a topic I wanted to leave
some time to think about.

On Tue, May 03, 2016 at 04:30:36PM +, Kinney, Michael D wrote:
> Similar to edk2-staging, we also have a need to manage platforms
> that have been ported to edk2.  Jordan has created a repository
> called edk2-platforms and has created a branch for the
> minnowboard-max that uses a validated release of the UDK 2015 for
> the dependent packages:
> 
> https://github.com/tianocore/edk2-platforms
> 
> https://github.com/tianocore/edk2-platforms/tree/minnowboard-max-udk2015
> 
> Instead of creating a branch per feature in edk2-staging, the
> proposal is to create a branch per platform in edk2-platforms.  The
> maintainer(s) that create and support a platform branch can decide
> if the platform is synced to edk2/master for dependent packages, or
> uses a stable release of the edk2 for dependent packages.
> 
> This proposal provides an area for platform development so we can
> minimize the number of platforms that are included in edk2/master.
> It is important to keep some platforms in edk2/master so we can use
> those platforms to validate features in non-platform packages in
> edk2/master.  If a new platform does not add feature coverage to
> edk2/master, then a new edk2-platforms branch would be recommended.
> 
> Please review the proposal below for edk2-platforms.
> 
> If this proposal is accepted, then a review of the platform in
> edk2/master can Be done to see if any of them should be moved to
> branches in edk2-platforms.

First of all, let me reiterate - I very much want to see more public
platform code. To the extent this helps that happen, I am a strong
supporter.

However, this proposal does not really address the issues I've been
pursuing, and am currently staging in my OpenPlatformPkg tree:
- The ability to see when a core change breaks some/all platform
  ports.
- The ability to easily spot common code that should be turned into
  core libraries.
  - The ability to easily spot near-identical local overrides to
common libraries that should turn into options in an existing core
library.
- The ability to share drivers for common components between different
  platforms, benefitting from feature additions and bugfixes.

It may not be intending to do this, which is fine, but I'd like to
share a few warnings - seen both in Linaro's Linux kernel work and in
Linaro's UEFI work when the "platform feature branch" approach was
used (also try an Internet search for "vendor kernels"):
- Keeping each platform port in a separate branch will inevitably lead
  to incompatible changes to core components.
  - Some of these changes will be fundamentally incompatible with
other platforms, because they were made in complete isolation from
those other platforms.
  - Mandating that all ports are based on a UDK version reduces the
scope of these incompatibilities, but ports will inevitably want
access to newer features that what is provided in UDK, so will
cherry-pick ... and then some will change that.
- But EDK2 has reasonably high churn on internal interfaces, and
  taking the step from one UDK to the next could mean a
  substantial amount of work at once, instead of progressively
  adjusting to core changes. (Acknowledging that this may well be
  preferable for some.)
  - Preventing this would cause higher maintainer overhead than
letting the platforms into the main tree.

So I guess my main question is - should this be seen as a complement
to what I am looking for, or should I write an alternative proposal
trying to incorporate the use-cases this one covers as well as the
ones I am looking for?

Regards,

Leif

> 
> 
> 
> 
> Problem statement
> =
> Need place on tianocore.org where platforms can be maintained by the EDK II
> community.  This serves several purposes:
> 
> * Encourage more platforms sources to be shared earlier in the
>   development process
> * Allow platform sources to be shared that may not yet meet all edk2
>   required quality criteria
> * Allow platform source to be shared so the EDK II community may
>   choose to help finish and validate
> * Allow more platforms to be used as part of the edk2 validation and
>   release cycle.
> * Not intended to be used for bug fixes.
> 
> 
> Proposal
> 
> 1) Create a new repo called edk2-platforms
> 
>  a) edk2-platforms is a fork of edk2
> 
>  b) edk2-platforms/master tracks edk2/master
> 
> 
> 2) edk2-platforms discussions can use the existing edk2-devel
>mailing list for design/patch/test.
> 
>  Use the following style for discussion of a platform branch in
>  edk2-platforms repo.
> 
>  [platforms/branch]: Subject
> 
> 
> 3) All commits to edk2-platforms must follow same edk2 rules
>(e.g. Tiano Contributor's Agreement)
>
>
> 4) Process to add a new platform to edk2-platforms
> 
>  a) Maintainer sends patch email to edk2-devel mailing list
>

[edk2] [Patch 2/2] SecurityPkg OpalPasswordDxe: Avoid saving password in browser.

[Eric Dong]

Change callback handler type to avoid saving password in
browser temp buffer.

Cc: Feng Tian 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
---
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c | 25 +++---
 .../Tcg/Opal/OpalPasswordDxe/OpalHiiPrivate.h  |  4 +++-
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
index bae2b2b..e50a683 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
@@ -464,14 +464,12 @@ DriverCallback(
 
   case HII_KEY_ID_ENTER_PASSWORD:
 return HiiPasswordEntered(Value->string);
+
+  case HII_KEY_ID_ENTER_PSID:
+return HiiPsidRevert(Value->string);
 }
   } else if (Action == EFI_BROWSER_ACTION_CHANGED) {
 switch (HiiKeyId) {
-  case HII_KEY_ID_ENTER_PSID:
-HiiPsidRevert();
-*ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_APPLY;
-return EFI_SUCCESS;
-
   case HII_KEY_ID_BLOCKSID:
 HiiSetBlockSid(Value->b);
 *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_APPLY;
@@ -594,12 +592,14 @@ HiiPopulateDiskInfoForm(
 /**
   Reverts the Opal disk to factory default.
 
+  @param   PsidStringId  The string id for the PSID info.
+
   @retval  EFI_SUCCESS   Do the required action success.
 
 **/
 EFI_STATUS
 HiiPsidRevert(
-  VOID
+  EFI_STRING_ID PsidStringId
   )
 {
   CHAR8 Response[DEFAULT_RESPONSE_SIZE];
@@ -607,12 +607,21 @@ HiiPsidRevert(
   OPAL_DISK *OpalDisk;
   TCG_RESULTRet;
   OPAL_SESSION  Session;
+  CHAR16*UnicodeStr;
 
   Ret = TcgResultFailure;
 
-  OpalHiiGetBrowserData();
+  UnicodeStr = HiiGetString (gHiiPackageListHandle, PsidStringId, NULL);
+
+  UnicodeStrToAsciiStr(UnicodeStr, (CHAR8*)Psid.Psid);
+
+  //
+  // For security concern, clean up the password after use it.
+  //
+  HiiSetString (gHiiPackageListHandle, PsidStringId, L"", NULL);
 
-  UnicodeStrToAsciiStr(gHiiConfiguration.Psid, (CHAR8*)Psid.Psid);
+  ZeroMem (UnicodeStr, StrSize (UnicodeStr));
+  FreePool (UnicodeStr);
 
   OpalDisk = HiiGetOpalDiskCB (gHiiConfiguration.SelectedDiskIndex);
   if (OpalDisk != NULL) {
diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHiiPrivate.h 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHiiPrivate.h
index 366cd38..e873a24 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHiiPrivate.h
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHiiPrivate.h
@@ -225,12 +225,14 @@ HiiSetBlockSid (
 /**
   Reverts the Opal disk to factory default.
 
+  @param   PsidStringId  The string id for the PSID info.
+
   @retval  EFI_SUCCESS   Do the required action success.
 
 **/
 EFI_STATUS
 HiiPsidRevert(
-  VOID
+  EFI_STRING_ID PsidStringId
   );
 
 /**
-- 
2.6.4.windows.1

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

[edk2] [Patch 1/2] SecurityPkg OpalPasswordDxe: Clean Password buffer after using it.

[Eric Dong]

For security concern, clean password buffer after using it.

Cc: Feng Tian 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
---
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c |  5 +
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c| 27 +++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
index 7c6deb8..1c31291 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c
@@ -241,6 +241,11 @@ OpalDriverPopUpHddPassword (
 
   UnicodeStrToAsciiStr(Unicode, Ascii);
 
+  //
+  // For security concern, must clean the passord memory.
+  //
+  ZeroMem (Unicode, sizeof (Unicode));
+
   return Ascii;
 }
 
diff --git a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c 
b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
index 3fb3553..bae2b2b 100644
--- a/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
+++ b/SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c
@@ -624,6 +624,8 @@ HiiPsidRevert(
 Ret = OpalSupportPsidRevert(, Psid.Psid, 
(UINT32)sizeof(Psid.Psid), OpalDisk->OpalDevicePath);
   }
 
+  ZeroMem (, sizeof (PsidStringId));
+
   if (Ret == TcgResultSuccess) {
 AsciiSPrint( Response, DEFAULT_RESPONSE_SIZE, "%a", "PSID Revert: Success" 
);
   } else {
@@ -1029,8 +1031,8 @@ HiiPasswordEntered(
   EFI_STRING_IDStr
   )
 {
-  OPAL_DISK*OpalDisk;
-  CHAR8 Password[MAX_PASSWORD_CHARACTER_LENGTH + 1];
+  OPAL_DISK*   OpalDisk;
+  CHAR8Password[MAX_PASSWORD_CHARACTER_LENGTH + 1];
   CHAR16*  UniStr;
   UINT32   PassLength;
   EFI_STATUS   Status;
@@ -1054,15 +1056,30 @@ HiiPasswordEntered(
   if (UniStr == NULL) {
 return EFI_NOT_FOUND;
   }
+
+  //
+  // For security concern, must clean the password saved in string package.
+  //
+  HiiSetString(gHiiPackageListHandle, Str, L"", NULL);
+
   PassLength = (UINT32) StrLen (UniStr);
   if (PassLength >= sizeof(Password)) {
 HiiSetFormString(STRING_TOKEN(STR_ACTION_STATUS), "Password too long");
-gBS->FreePool(UniStr);
+//
+// For security concern, must clean the password saved in string package.
+//
+ZeroMem (UniStr, StrSize (UniStr));
+FreePool(UniStr);
 return EFI_BUFFER_TOO_SMALL;
   }
 
   UnicodeStrToAsciiStr(UniStr, Password);
-  gBS->FreePool(UniStr);
+
+  //
+  // For security concern, must clean the password saved in string package.
+  //
+  ZeroMem (UniStr, StrSize (UniStr));
+  FreePool(UniStr);
 
   if (gHiiConfiguration.SelectedAction == HII_KEY_ID_GOTO_UNLOCK) {
 Status = HiiUnlock (OpalDisk, Password, PassLength);
@@ -1077,6 +1094,8 @@ HiiPasswordEntered(
 Status = HiiSetPassword(OpalDisk, Password, PassLength);
   }
 
+  ZeroMem (Password, sizeof (Password));
+
   OpalHiiSetBrowserData ();
 
   return Status;
-- 
2.6.4.windows.1

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

[edk2] [Patch 0/2] Clean password buffer after using it.

[Eric Dong]

For security concern, clean the password buffer after using it.

Eric Dong (2):
  SecurityPkg OpalPasswordDxe: Clean Password buffer after using it.
  SecurityPkg OpalPasswordDxe: Avoid saving password in browser.

 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c  |  5 +++
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.c | 52 +-
 .../Tcg/Opal/OpalPasswordDxe/OpalHiiPrivate.h  |  4 +-
 3 files changed, 48 insertions(+), 13 deletions(-)

-- 
2.6.4.windows.1

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

[edk2] [Patch] SecurityPkg TcgStorageOpalLib: Avoid using special word in comments.

[Eric Dong]

Cc: Shumin Qiu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Eric Dong 
---
 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
index cc8d5ef..a0eac33 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalCore.c
@@ -927,7 +927,7 @@ OpalSetLockingSpAuthorityEnabledAndPin(
 if (*MethodStatus != TCG_METHOD_STATUS_CODE_SUCCESS) {
   DEBUG ((DEBUG_INFO, "Update ACE for GLOBALRANGE_GENKEY failed\n"));
   //
-  // TODO do we want to disable user1 if all permissions are not granted
+  // Disable user1 if all permissions are not granted.
   //
   return TcgResultFailure;
 }
-- 
2.6.4.windows.1

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

Re: [edk2] [PATCH v2 1/1] NetworkPkg:HttpDxe: Code changes to support HTTP PUT/POST operations

[Hegde, Nagaraj P]

Hi Jiaxin,

After an attempt of HTTPBoot, we keep the State as HTTP_STATE_TCP_CONNECTED. 
With your suggestion, I see that alternate attempts of HTTPBoot is failing. In 
Wireshark, I see that we send a RST following a GET request for each failed 
alternate boot attempts. So, we might not want to have that condition in place.

Hi Siyuan,

Do you have any additional feedback on this patch?

Regards,
Nagaraj.

-Original Message-
From: Wu, Jiaxin [mailto:jiaxin...@intel.com] 
Sent: Monday, May 09, 2016 7:28 AM
To: Hegde, Nagaraj P ; edk2-devel@lists.01.org
Cc: Fu, Siyuan ; Ye, Ting ; 
El-Haj-Mahmoud, Samer 
Subject: RE: [PATCH v2 1/1] NetworkPkg:HttpDxe: Code changes to support HTTP 
PUT/POST operations

Hi Nagaraj,

In every EfiHttpRequest() for PUT/POST, HttpInitTcp() will be called even the 
TCP state is established (get from TCP4/6 GetModeData each time) and return 
EFI_SUCCESS. 

To avoid this repeating function call and improve the performance, we can check 
the current Http state, if required, then call it:

if (HttpInstance->State != HTTP_STATE_TCP_CONNECTED) {
  Status = HttpInitTcp (HttpInstance, Wrap, Configure);
   if (EFI_ERROR (Status)) {
 goto Error2;
   }
}

Others is good to me.
Reviewed-By: Wu Jiaxin 

Thanks.
Jiaxin


> -Original Message-
> From: Nagaraj Hegde [mailto:nagaraj-p.he...@hpe.com]
> Sent: Friday, May 6, 2016 6:20 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Fu, Siyuan 
> ; Ye, Ting ; 
> samer.el-haj-mahm...@hpe.com
> Subject: [PATCH v2 1/1] NetworkPkg:HttpDxe: Code changes to support 
> HTTP PUT/POST operations
> 
> v2: Add comments for the code changes.
> 
> Code changes enables HttpDxe to handle PUT/POST operations.
> EfiHttpRequest assumes "Request" and "HttpMsg->Headers" can never be 
> NULL. Also, HttpResponseWorker assumes HTTP Reponse will contain 
> headers. We could have response which could contain only a string 
> (HTTP 100
> Continue) and no headers. Code changes tries to do-away from these 
> assumptions, which would enable HttpDxe to support PUT/POST operations.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hegde, Nagaraj P nagaraj-p.he...@hpe.com
> ---
>  NetworkPkg/HttpDxe/HttpDriver.c |   3 +
>  NetworkPkg/HttpDxe/HttpImpl.c   | 445 
>  NetworkPkg/HttpDxe/HttpProto.h  |   1 +
>  3 files changed, 264 insertions(+), 185 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpDriver.c 
> b/NetworkPkg/HttpDxe/HttpDriver.c index 2518f4e..da6d087 100644
> --- a/NetworkPkg/HttpDxe/HttpDriver.c
> +++ b/NetworkPkg/HttpDxe/HttpDriver.c
> @@ -2,6 +2,7 @@
>The driver binding and service binding protocol for HttpDxe driver.
> 
>Copyright (c) 2015, Intel Corporation. All rights reserved.
> +  (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> 
>This program and the accompanying materials
>are licensed and made available under the terms and conditions of 
> the BSD License @@ -939,6 +940,8 @@ HttpServiceBindingCreateChild (
> 
>HttpInstance->Signature = HTTP_PROTOCOL_SIGNATURE;
>HttpInstance->Service   = HttpService;
> +  HttpInstance->Method = HttpMethodMax;
> +
>CopyMem (>Http, , sizeof 
> (HttpInstance->Http));
>NetMapInit (>TxTokens);
>NetMapInit (>RxTokens); diff --git 
> a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c index 
> 7a236f4..9bc0f8a 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -248,151 +248,185 @@ EfiHttpRequest (
>HTTP_TOKEN_WRAP   *Wrap;
>CHAR8 *FileUrl;
>UINTN RequestMsgSize;
> -
> +
> +  //
> +  // Initializations
> +  //
> +  Url = NULL;
> +  HostName = NULL;
> +  RequestMsg = NULL;
> +  HostNameStr = NULL;
> +  Wrap = NULL;
> +  FileUrl = NULL;
> +
>if ((This == NULL) || (Token == NULL)) {
>  return EFI_INVALID_PARAMETER;
>}
> 
>HttpMsg = Token->Message;
> -  if ((HttpMsg == NULL) || (HttpMsg->Headers == NULL)) {
> +  if (HttpMsg == NULL) {
>  return EFI_INVALID_PARAMETER;
>}
> 
> -  //
> -  // Current implementation does not support POST/PUT method.
> -  // If future version supports these two methods, Request could be 
> NULL for a special case that to send large amounts
> -  // of data. For this case, the implementation need check whether 
> previous call to Request() has been completed or not.
> -  //
> -  //
>Request = HttpMsg->Data.Request;
> -  if ((Request == NULL) || (Request->Url == NULL)) {
> -return EFI_INVALID_PARAMETER;
> -  }
> 
>//
> -  // Only support GET and HEAD method in current implementation.
> +  // Only support GET, HEAD, PUT and POST method in current
> implementation.
>//
> -  if ((Request->Method != HttpMethodGet) && 

[edk2] [Patch] SecurityPkg: SecureBootConfigDxe: Add NULL pointer check

[Zhang, Chao B]

Add SecureBoot NULL pointer check before reference it.

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

diff --git 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 088fa26..3f80441 100644
--- 
a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ 
b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -2933,7 +2933,7 @@ SecureBootExtractConfigFromVariable (
   //
   // Fix Pk, SecureBootEnable inconsistence
   //
-  if ((*SetupMode) == USER_MODE) {
+  if ((SetupMode != NULL) && (*SetupMode) == USER_MODE) {
 ConfigData->HideSecureBoot = FALSE;
 if ((SecureBootEnable != NULL) && (*SecureBootEnable == 
SECURE_BOOT_ENABLE)) {
   ConfigData->AttemptSecureBoot = TRUE;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH v2 0/7] PciHostBridgeDxe: Bug fix and OVMF above Xen support

[Gary Lin]

On Tue, May 10, 2016 at 01:53:24PM +0800, Ruiyu Ni wrote:
> The patch serials fix two bugs in PciHostBridgeDxe driver.
> And it adds a new field in PCI_ROOT_BRIDGE structure to support OVMF above
> Xen support.
> 
> v2 splits 6/6 in v1 to 6/7 and 7/7.
> v2 fixes a bug in 6/6 which causes OVMF above KVM hang.
> 
This series work for me!
Tested-by: Gary Lin 

Thanks!

Gary Lin

> 
> Ruiyu Ni (7):
>   MdeModulePkg/PciHostBridgeDxe: Don't miss prefetchable MMIO aperture
>   MdeModulePkg/PciHostBridgeDxe: Fix a Base/Limit comparing bug
>   OvmfPkg/PciHostBridgeLib: Set correct Base/Limit for absent resource
>   MdeModulePkg/PciHostBridgeLib: Add ResourceAssigned field
>   MdeModulePkg/PciHostBridgeDxe: Honor ResourceAssigned
>   OvmfPkg/PciHostBridgeLib: Change InitRootBridge prototype
>   OvmfPkg/PciHostBridgeLib: Scan for root bridges when running over Xen
> 
>  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  96 +++--
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   4 +-
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 106 +++--
>  MdeModulePkg/Include/Library/PciHostBridgeLib.h|   2 +
>  OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h   |  75 
>  .../Library/PciHostBridgeLib/PciHostBridgeLib.c| 137 +--
>  .../Library/PciHostBridgeLib/PciHostBridgeLib.inf  |   3 +
>  OvmfPkg/Library/PciHostBridgeLib/XenSupport.c  | 456 
> +
>  8 files changed, 775 insertions(+), 104 deletions(-)
>  create mode 100644 OvmfPkg/Library/PciHostBridgeLib/PciHostBridge.h
>  create mode 100644 OvmfPkg/Library/PciHostBridgeLib/XenSupport.c
> 
> -- 
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] MdeModulePkg/PciSioSerialDxe: Do not flush the UART

[Jin, Eric]

Reviewed-by: Eric Jin 

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, May 9, 2016 1:04 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Jin, Eric 
Subject: [Patch] MdeModulePkg/PciSioSerialDxe: Do not flush the UART

The patch aligns to the IntelFrameworkModulePkg/Bus/Isa/IsaSerialDxe
driver not flush the UART in Reset() and SetAttributes() function.
It was found the flush causes hang on certain PCI serial devices.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Eric Jin 
---
 MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c | 27 +
 1 file changed, 1 insertion(+), 26 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c 
b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
index f1870f3..cce61d7 100644
--- a/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
+++ b/MdeModulePkg/Bus/Pci/PciSioSerialDxe/SerialIo.c
@@ -1,7 +1,7 @@
 /** @file
   SerialIo implementation for PCI or SIO UARTs.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at @@ -442,27 +442,6 
@@ SerialReceiveTransmit (
   return EFI_SUCCESS;
 }
 
-/**
-  Flush the serial hardware transmit FIFO and shift register.
-
-  @param SerialDevice  The device to flush.
-**/
-VOID
-SerialFlushTransmitFifo (
-  SERIAL_DEV  *SerialDevice
-  )
-{
-  SERIAL_PORT_LSR  Lsr;
-
-  //
-  // Wait for the serial port to be ready, to make sure both the transmit FIFO
-  // and shift register empty.
-  //
-  do {
-Lsr.Data = READ_LSR (SerialDevice);
-  } while (Lsr.Bits.Temt == 0);
-}
-
 //
 // Interface Functions
 //
@@ -503,8 +482,6 @@ SerialReset (
 
   Tpl = gBS->RaiseTPL (TPL_NOTIFY);
 
-  SerialFlushTransmitFifo (SerialDevice);
-
   //
   // Make sure DLAB is 0.
   //
@@ -683,8 +660,6 @@ SerialSetAttributes (
 
   Tpl = gBS->RaiseTPL (TPL_NOTIFY);
 
-  SerialFlushTransmitFifo (SerialDevice);
-
   //
   // Put serial port on Divisor Latch Mode
   //
--
2.7.0.windows.1

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