Re: [edk2] [Patch] Nt32Pkg PlatformBootManagerLib: Enable BootManagerMenuApp.

2016-08-25 Thread Ni, Ruiyu


Reviewed-by: Ruiyu Ni 
>-Original Message-
>From: Dong, Eric
>Sent: Wednesday, August 24, 2016 4:22 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu 
>Subject: [Patch] Nt32Pkg PlatformBootManagerLib: Enable BootManagerMenuApp.
>
>Enable BootManagerMenuApp application for Nt32 platform.
>Also enable F7 hotkey to select this boot option.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Eric Dong 
>Cc: Ruiyu Ni 
>---
> .../PlatformBootManagerLib/PlatformBootManager.c   | 169 -
> .../PlatformBootManagerLib/PlatformBootManager.h   |   1 +
> .../PlatformBootManagerLib.inf |   1 +
> Nt32Pkg/Nt32Pkg.dsc|   4 +
> Nt32Pkg/Nt32Pkg.fdf|   1 +
> 5 files changed, 173 insertions(+), 3 deletions(-)
>
>diff --git a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
>b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
>index 4b23eb1..e75b849 100644
>--- a/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
>+++ b/Nt32Pkg/Library/PlatformBootManagerLib/PlatformBootManager.c
>@@ -15,6 +15,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>EXPRESS OR IMPLIED.
>
> #include "PlatformBootManager.h"
>
>+EFI_GUID mBootMenuFile = {
>+  0xEEC25BDC, 0x67F2, 0x4D95, { 0xB1, 0xD5, 0xF8, 0x1B, 0x20, 0x39, 0xD1, 
>0x1D }
>+};
>+
> /**
>   Perform the platform diagnostic, such like test memory. OEM/IBV also
>   can customize this function to support specific platform diagnostic.
>@@ -144,6 +148,154 @@ CompareBootOption (
> }
>
> /**
>+  Generate device path include the input file guid info.
>+
>+  @param  FileGuid Input file guid for the BootManagerMenuApp.
>+
>+  @retval DevicePath for BootManagerMenuApp.
>+**/
>+EFI_DEVICE_PATH *
>+FvFilePath (
>+  EFI_GUID *FileGuid
>+  )
>+{
>+
>+  EFI_STATUS Status;
>+  EFI_LOADED_IMAGE_PROTOCOL  *LoadedImage;
>+  MEDIA_FW_VOL_FILEPATH_DEVICE_PATH  FileNode;
>+
>+  EfiInitializeFwVolDevicepathNode (, FileGuid);
>+
>+  Status = gBS->HandleProtocol (
>+  gImageHandle,
>+  ,
>+  (VOID **) 
>+  );
>+  ASSERT_EFI_ERROR (Status);
>+
>+  return AppendDevicePathNode (
>+   DevicePathFromHandle (LoadedImage->DeviceHandle),
>+   (EFI_DEVICE_PATH_PROTOCOL *) 
>+   );
>+}
>+
>+/**
>+  Create one boot option for BootManagerMenuApp.
>+
>+  @param  FileGuid  Input file guid for the BootManagerMenuApp.
>+  @param  Description   Description of the BootManagerMenuApp boot option.
>+  @param  Position  Position of the new load option to put in the 
>Order variable.
>+  @param  IsBootCategoryWhether this is a boot category.
>+
>+
>+  @retval OptionNumber  Return the option number info.
>+
>+**/
>+UINTN
>+RegisterBootManagerMenuAppBootOption (
>+  EFI_GUID *FileGuid,
>+  CHAR16   *Description,
>+  UINTNPosition,
>+  BOOLEAN  IsBootCategory
>+  )
>+{
>+  EFI_STATUS   Status;
>+  EFI_BOOT_MANAGER_LOAD_OPTION NewOption;
>+  EFI_DEVICE_PATH_PROTOCOL *DevicePath;
>+  UINTNOptionNumber;
>+
>+  DevicePath = FvFilePath (FileGuid);
>+  Status = EfiBootManagerInitializeLoadOption (
>+ ,
>+ LoadOptionNumberUnassigned,
>+ LoadOptionTypeBoot,
>+ IsBootCategory ? LOAD_OPTION_ACTIVE : LOAD_OPTION_CATEGORY_APP,
>+ Description,
>+ DevicePath,
>+ NULL,
>+ 0
>+ );
>+  ASSERT_EFI_ERROR (Status);
>+  FreePool (DevicePath);
>+
>+  Status = EfiBootManagerAddLoadOptionVariable (, Position);
>+  ASSERT_EFI_ERROR (Status);
>+
>+  OptionNumber = NewOption.OptionNumber;
>+
>+  EfiBootManagerFreeLoadOption ();
>+
>+  return OptionNumber;
>+}
>+
>+/**
>+  Check if it's a Device Path pointing to BootManagerMenuApp.
>+
>+  @param  DevicePath Input device path.
>+
>+  @retval TRUE   The device path is BootManagerMenuApp File Device Path.
>+  @retval FALSE  The device path is NOT BootManagerMenuApp File Device Path.
>+**/
>+BOOLEAN
>+IsBootManagerMenuAppFilePath (
>+  EFI_DEVICE_PATH_PROTOCOL *DevicePath
>+)
>+{
>+  EFI_HANDLE  FvHandle;
>+  VOID*NameGuid;
>+  EFI_STATUS  Status;
>+
>+  Status = gBS->LocateDevicePath (, 
>, );
>+  if (!EFI_ERROR (Status)) {
>+NameGuid = EfiGetNameGuidFromFwVolDevicePathNode ((CONST 
>MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)
>DevicePath);
>+if (NameGuid != NULL) {
>+  return CompareGuid (NameGuid, );
>+}
>+  }
>+
>+  return FALSE;
>+}
>+
>+/**
>+  Return the boot option number to the BootManagerMenuApp.
>+
>+  If not found it in the current 

Re: [edk2] [Patch] Nt32Pkg: Add Eric Dong as Nt32Pkg owner.

2016-08-25 Thread Ni, Ruiyu


Reviewed-by: Ruiyu Ni 
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Eric 
>Dong
>Sent: Wednesday, August 24, 2016 4:22 PM
>To: edk2-devel@lists.01.org
>Cc: Ni, Ruiyu 
>Subject: [edk2] [Patch] Nt32Pkg: Add Eric Dong as Nt32Pkg owner.
>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Eric Dong 
>Cc: Ruiyu Ni 
>---
> Maintainers.txt | 1 +
> 1 file changed, 1 insertion(+)
>
>diff --git a/Maintainers.txt b/Maintainers.txt
>index d0d5f5c..60c83bc 100644
>--- a/Maintainers.txt
>+++ b/Maintainers.txt
>@@ -153,6 +153,7 @@ M: Jiaxin Wu 
> Nt32Pkg
> W: https://github.com/tianocore/tianocore.github.io/wiki/Nt32Pkg
> M: Ruiyu Ni 
>+M: Eric Dong 
>
> Omap35xxPkg
> W: https://github.com/tianocore/tianocore.github.io/wiki/Omap35xxPkg
>--
>2.6.4.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] [RESEND] Platforms/ARM/Juno: Add support for ACPI 6.0 LPI(Low Power Idle) states

2016-08-25 Thread Sudeep Holla
ACPI 6.0 introduced LPI(Low Power Idle) states that provides an alternate
method to describe processor idle states.

LPI extensions leverages the processor container device(again introduced
in ACPI 6.0) allowing to express which parts of the system are affected
by a given LPI state. It defines the local power states for each node
in a hierarchical processor topology. The OSPM can use _LPI object to
select a local power state for each level of processor hierarchy in the
system.

This patch adds LPI support on Juno.

Contributed-under: TianoCore Contribution Agreement 1.0
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Evan Lloyd 
Signed-off-by: Sudeep Holla 
---
 Platforms/ARM/Juno/AcpiTables/Dsdt.asl | 232 ++---
 1 file changed, 213 insertions(+), 19 deletions(-)

Hi Lief,

Assuming Evan has lost the patches during his holidays, I am resending it.

The support in the kernel is now added(latest mainline as of today) and
is enabled in defconfig. It's easy to test, just examine the sysfs entries:

/sys/devices/system/cpu/cpu*/cpuidle/state*/desc - Describes the cpuidle state
/sys/devices/system/cpu/cpu*/cpuidle/state*/{usage,time} - Should keep ticking
with time signifying that the idle states are entered and exited
correctly

Regards,
Sudeep

diff --git a/Platforms/ARM/Juno/AcpiTables/Dsdt.asl 
b/Platforms/ARM/Juno/AcpiTables/Dsdt.asl
index c324ded471f2..07e32bae21f8 100644
--- a/Platforms/ARM/Juno/AcpiTables/Dsdt.asl
+++ b/Platforms/ARM/Juno/AcpiTables/Dsdt.asl
@@ -19,29 +19,223 @@ DefinitionBlock("DsdtTable.aml", "DSDT", 1, "ARMLTD", 
"ARM-JUNO", EFI_ACPI_ARM_O
 //
 // A57x2-A53x4 Processor declaration
 //
-Device(CPU0) { // A53-0: Cluster 1, Cpu 0
-  Name(_HID, "ACPI0007")
-  Name(_UID, 0)
+Method (_OSC, 4, Serialized)  { // _OSC: Operating System Capabilities
+  CreateDWordField (Arg3, 0x00, STS0)
+  CreateDWordField (Arg3, 0x04, CAP0)
+  If ((Arg0 == ToUUID ("0811b06e-4a27-44f9-8d60-3cbbc22e7b48") /* 
Platform-wide Capabilities */)) {
+If (!(Arg1 == One)) {
+  STS0 &= ~0x1F
+  STS0 |= 0x0A
+} Else {
+  If ((CAP0 & 0x100)) {
+CAP0 &= ~0x100 /* No support for OS Initiated LPI */
+STS0 &= ~0x1F
+STS0 |= 0x12
+  }
+}
+  } Else {
+STS0 &= ~0x1F
+STS0 |= 0x06
+  }
+  Return (Arg3)
 }
-Device(CPU1) { // A53-1: Cluster 1, Cpu 1
-  Name(_HID, "ACPI0007")
+Device (CLU0) { // Cluster0 state
+  Name(_HID, "ACPI0010")
   Name(_UID, 1)
+  Name (_LPI, Package() {
+0, // Version
+0, // Level Index
+1, // Count
+Package() { // Power Gating state for Cluster
+  2500, // Min residency (uS)
+  1150, // Wake latency (uS)
+  1, // Flags
+  1, // Arch Context Flags
+  100, //Residency Counter Frequency
+  0, // No Parent State
+  0x0100, // Integer Entry method
+  ResourceTemplate() { // Null Residency Counter
+Register (SystemMemory, 0, 0, 0, 0)
+  },
+  ResourceTemplate() { // Null Usage Counter
+Register (SystemMemory, 0, 0, 0, 0)
+  },
+  "CluPwrDn"
+},
+  })
+  Name(PLPI, Package() {
+0, // Version
+1, // Level Index
+2, // Count
+Package() { // WFI for CPU
+  1, // Min residency (uS)
+  1, // Wake latency (uS)
+  1, // Flags
+  0, // Arch Context Flags
+  100, //Residency Counter Frequency
+  0, // No parent state
+  ResourceTemplate () {
+// Register Entry method
+Register (FFixedHW,
+  0x20,   // Bit Width
+  0x00,   // Bit Offset
+  0x, // Address
+  0x03,   // Access Size
+  )
+  },
+  ResourceTemplate() { // Null Residency Counter
+Register (SystemMemory, 0, 0, 0, 0)
+  },
+  ResourceTemplate() { // Null Usage Counter
+Register (SystemMemory, 0, 0, 0, 0)
+  },
+  "WFI",
+},
+Package() { // Power Gating state for CPU
+  150, // Min residency (uS)
+  350, // Wake latency (uS)
+  1, // Flags
+  1, // Arch Context Flags
+  100, //Residency Counter Frequency
+  1, // Parent node can be in any state
+  ResourceTemplate () {
+// Register Entry method
+Register (FFixedHW,
+  0x20,   // Bit Width
+  0x00,   // Bit Offset
+  0x0001, // Address
+  0x03,   // Access Size
+  )
+  },
+  ResourceTemplate() { // Null Residency Counter
+

[edk2] [PATCH] Platforms/ARM/Juno: limit ACPI support to v5.0 and higher

2016-08-25 Thread Sudeep Holla
The ACPI spec predates the AARCH64 architecture by 5 versions, so there
is no point in supporting anything below v5.0. So set the PCD that
controls the ACPI table generation to the appropriate value.

Based on the commit e0692789058e ("ArmVirtPkg/ArmVirtQemu: limit ACPI
support to v5.0 and higher") in the upstream TianoCore by Ard Biesheuvel

Contributed-under: TianoCore Contribution Agreement 1.0
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Evan Lloyd 
Signed-off-by: Sudeep Holla 
---
 Platforms/ARM/Juno/ArmJuno.dsc | 4 
 1 file changed, 4 insertions(+)

diff --git a/Platforms/ARM/Juno/ArmJuno.dsc b/Platforms/ARM/Juno/ArmJuno.dsc
index c51d8f2e21ab..302bc66f 100644
--- a/Platforms/ARM/Juno/ArmJuno.dsc
+++ b/Platforms/ARM/Juno/ArmJuno.dsc
@@ -183,6 +183,10 @@
   #
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosVersion|0x0300
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
+  #
+  # ACPI Table Version
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
 
 [PcdsPatchableInModule]
   # Console Resolution (Full HD)
-- 
2.7.4

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


Re: [edk2] I found a fun bug in the Shell today. Looks like we have been getting lucky?

2016-08-25 Thread Andrew Fish

> On Aug 25, 2016, at 9:17 AM, Carsey, Jaben  wrote:
> 
> Doh!  Thanks!  I didn't realize that was the bug.  I will have to finally 
> learn that servers name one of these days...
> 

No worries I thought the URL would be more obvious, I should have labeled it.

Thanks,

Andrew Fish

>> -Original Message-
>> From: af...@apple.com [mailto:af...@apple.com]
>> Sent: Thursday, August 25, 2016 9:08 AM
>> To: Carsey, Jaben 
>> Cc: edk2-devel ; Ni, Ruiyu 
>> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we have
>> been getting lucky?
>> Importance: High
>> 
>> 
>>> On Aug 25, 2016, at 9:05 AM, Carsey, Jaben 
>> wrote:
>>> 
>>> Andrew,
>>> 
>>> Can you file a Bugzilla issue so we can track this issue properly?
>>> 
>> 
>> Jaben,
>> 
>> I attached the URL at the end of the original mail:
>> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> -Jaben
>>> 
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
 Andrew Fish
 Sent: Wednesday, August 24, 2016 6:26 PM
 To: edk2-devel 
 Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we
>> have
 been getting lucky?
 Importance: High
 
 
> On Aug 24, 2016, at 5:59 PM, Andrew Fish  wrote:
> 
> I was tracking down a data corruption issue when paging was enabled on
>> an
 edk2 shell command. The crash was in a custom ConSpliter over writing a
>> DXE
 Core data structure. The buffer overflow seemed to be caused by the
 Console getting confused on the location of the end of the screen. I set a
 watchpoint on gST->ConOut->Mode->CursorRow and found the shell was
 the one corrupting the Mode data.
> 
> UEFI Spec: The following data values in the
>> SIMPLE_TEXT_OUTPUT_MODE
 interface are read-only and are changed by using the appropriate
>> interface
 functions:
> 
> (master)>git grep "OurConOut.Mode"
> Application/Shell/ConsoleLogger.c:72:  (*ConsoleInfo)-
>>> OurConOut.Mode
 = gST->ConOut->Mode;
> Application/Shell/ConsoleLogger.c:647://ShellInfoObject.ConsoleInfo-
> OurConOut.Mode->CursorRow= 0;
> Application/Shell/ConsoleLogger.c:648://ShellInfoObject.ConsoleInfo-
> OurConOut.Mode->CursorColumn = 0;
> Application/Shell/ConsoleLogger.c:704:  if (ConsoleInfo-
> OurConOut.Mode->CursorColumn > 0) {
> Application/Shell/ConsoleLogger.c:705:ConsoleInfo-
> OurConOut.Mode->CursorColumn--;
> Application/Shell/ConsoleLogger.c:734:  ConsoleInfo-
>>> OurConOut.Mode-
> CursorRow++;
> Application/Shell/ConsoleLogger.c:741:  ConsoleInfo-
>>> OurConOut.Mode-
> CursorColumn = 0;
> Application/Shell/ConsoleLogger.c:747:  ConsoleInfo-
>>> OurConOut.Mode-
> CursorColumn++;
> Application/Shell/ConsoleLogger.c:751:  if ((INTN)ConsoleInfo-
> ColsPerScreen == ConsoleInfo->OurConOut.Mode->CursorColumn + 1) {
> Application/Shell/ConsoleLogger.c:781:ConsoleInfo-
> OurConOut.Mode->CursorRow++;
> Application/Shell/ConsoleLogger.c:782:ConsoleInfo-
> OurConOut.Mode->CursorColumn = 0;
> Application/Shell/ConsoleLogger.c:976:ConsoleInfo-
>>> OurConOut.Mode =
 ConsoleInfo->OldConOut->Mode;
> 
> 
> I'm not exactly sure what this code is trying to do as the console should
 update Mode structure directly? Maybe the intent was to have a copy of
 gST->ConOut->Mode and keep it in sync? It seems like this should cause
 more issues, but maybe the edk2 ConSplitter is not broken by this
>> behavior
 and we are getting lucky?
> 
 
 I forgot to mention that setting the Mode->CursorRow in the console
>> code
 back to the last row if was larger looks like it hides this bug in the 
 shell.
 
 Thanks,
 
 Andrew Fish
 
 
> Thanks,
> 
> Andrew Fish
> 
> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
> ___
> 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] I found a fun bug in the Shell today. Looks like we have been getting lucky?

2016-08-25 Thread Carsey, Jaben
Doh!  Thanks!  I didn't realize that was the bug.  I will have to finally learn 
that servers name one of these days...

> -Original Message-
> From: af...@apple.com [mailto:af...@apple.com]
> Sent: Thursday, August 25, 2016 9:08 AM
> To: Carsey, Jaben 
> Cc: edk2-devel ; Ni, Ruiyu 
> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we have
> been getting lucky?
> Importance: High
> 
> 
> > On Aug 25, 2016, at 9:05 AM, Carsey, Jaben 
> wrote:
> >
> > Andrew,
> >
> > Can you file a Bugzilla issue so we can track this issue properly?
> >
> 
> Jaben,
> 
> I attached the URL at the end of the original mail:
> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
> 
> Thanks,
> 
> Andrew Fish
> 
> > -Jaben
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Andrew Fish
> >> Sent: Wednesday, August 24, 2016 6:26 PM
> >> To: edk2-devel 
> >> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we
> have
> >> been getting lucky?
> >> Importance: High
> >>
> >>
> >>> On Aug 24, 2016, at 5:59 PM, Andrew Fish  wrote:
> >>>
> >>> I was tracking down a data corruption issue when paging was enabled on
> an
> >> edk2 shell command. The crash was in a custom ConSpliter over writing a
> DXE
> >> Core data structure. The buffer overflow seemed to be caused by the
> >> Console getting confused on the location of the end of the screen. I set a
> >> watchpoint on gST->ConOut->Mode->CursorRow and found the shell was
> >> the one corrupting the Mode data.
> >>>
> >>> UEFI Spec: The following data values in the
> SIMPLE_TEXT_OUTPUT_MODE
> >> interface are read-only and are changed by using the appropriate
> interface
> >> functions:
> >>>
> >>> (master)>git grep "OurConOut.Mode"
> >>> Application/Shell/ConsoleLogger.c:72:  (*ConsoleInfo)-
> >OurConOut.Mode
> >> = gST->ConOut->Mode;
> >>> Application/Shell/ConsoleLogger.c:647://ShellInfoObject.ConsoleInfo-
> >>> OurConOut.Mode->CursorRow= 0;
> >>> Application/Shell/ConsoleLogger.c:648://ShellInfoObject.ConsoleInfo-
> >>> OurConOut.Mode->CursorColumn = 0;
> >>> Application/Shell/ConsoleLogger.c:704:  if (ConsoleInfo-
> >>> OurConOut.Mode->CursorColumn > 0) {
> >>> Application/Shell/ConsoleLogger.c:705:ConsoleInfo-
> >>> OurConOut.Mode->CursorColumn--;
> >>> Application/Shell/ConsoleLogger.c:734:  ConsoleInfo-
> >OurConOut.Mode-
> >>> CursorRow++;
> >>> Application/Shell/ConsoleLogger.c:741:  ConsoleInfo-
> >OurConOut.Mode-
> >>> CursorColumn = 0;
> >>> Application/Shell/ConsoleLogger.c:747:  ConsoleInfo-
> >OurConOut.Mode-
> >>> CursorColumn++;
> >>> Application/Shell/ConsoleLogger.c:751:  if ((INTN)ConsoleInfo-
> >>> ColsPerScreen == ConsoleInfo->OurConOut.Mode->CursorColumn + 1) {
> >>> Application/Shell/ConsoleLogger.c:781:ConsoleInfo-
> >>> OurConOut.Mode->CursorRow++;
> >>> Application/Shell/ConsoleLogger.c:782:ConsoleInfo-
> >>> OurConOut.Mode->CursorColumn = 0;
> >>> Application/Shell/ConsoleLogger.c:976:ConsoleInfo-
> >OurConOut.Mode =
> >> ConsoleInfo->OldConOut->Mode;
> >>>
> >>>
> >>> I'm not exactly sure what this code is trying to do as the console should
> >> update Mode structure directly? Maybe the intent was to have a copy of
> >> gST->ConOut->Mode and keep it in sync? It seems like this should cause
> >> more issues, but maybe the edk2 ConSplitter is not broken by this
> behavior
> >> and we are getting lucky?
> >>>
> >>
> >> I forgot to mention that setting the Mode->CursorRow in the console
> code
> >> back to the last row if was larger looks like it hides this bug in the 
> >> shell.
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>
> >>> Thanks,
> >>>
> >>> Andrew Fish
> >>>
> >>> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
> >>> ___
> >>> 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] I found a fun bug in the Shell today. Looks like we have been getting lucky?

2016-08-25 Thread Andrew Fish

> On Aug 25, 2016, at 9:05 AM, Carsey, Jaben  wrote:
> 
> Andrew,
> 
> Can you file a Bugzilla issue so we can track this issue properly?
> 

Jaben,

I attached the URL at the end of the original mail: 
https://tianocore.acgmultimedia.com/show_bug.cgi?id=105

Thanks,

Andrew Fish

> -Jaben
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> Andrew Fish
>> Sent: Wednesday, August 24, 2016 6:26 PM
>> To: edk2-devel 
>> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we have
>> been getting lucky?
>> Importance: High
>> 
>> 
>>> On Aug 24, 2016, at 5:59 PM, Andrew Fish  wrote:
>>> 
>>> I was tracking down a data corruption issue when paging was enabled on an
>> edk2 shell command. The crash was in a custom ConSpliter over writing a DXE
>> Core data structure. The buffer overflow seemed to be caused by the
>> Console getting confused on the location of the end of the screen. I set a
>> watchpoint on gST->ConOut->Mode->CursorRow and found the shell was
>> the one corrupting the Mode data.
>>> 
>>> UEFI Spec: The following data values in the SIMPLE_TEXT_OUTPUT_MODE
>> interface are read-only and are changed by using the appropriate interface
>> functions:
>>> 
>>> (master)>git grep "OurConOut.Mode"
>>> Application/Shell/ConsoleLogger.c:72:  (*ConsoleInfo)->OurConOut.Mode
>> = gST->ConOut->Mode;
>>> Application/Shell/ConsoleLogger.c:647://ShellInfoObject.ConsoleInfo-
>>> OurConOut.Mode->CursorRow= 0;
>>> Application/Shell/ConsoleLogger.c:648://ShellInfoObject.ConsoleInfo-
>>> OurConOut.Mode->CursorColumn = 0;
>>> Application/Shell/ConsoleLogger.c:704:  if (ConsoleInfo-
>>> OurConOut.Mode->CursorColumn > 0) {
>>> Application/Shell/ConsoleLogger.c:705:ConsoleInfo-
>>> OurConOut.Mode->CursorColumn--;
>>> Application/Shell/ConsoleLogger.c:734:  ConsoleInfo->OurConOut.Mode-
>>> CursorRow++;
>>> Application/Shell/ConsoleLogger.c:741:  ConsoleInfo->OurConOut.Mode-
>>> CursorColumn = 0;
>>> Application/Shell/ConsoleLogger.c:747:  ConsoleInfo->OurConOut.Mode-
>>> CursorColumn++;
>>> Application/Shell/ConsoleLogger.c:751:  if ((INTN)ConsoleInfo-
>>> ColsPerScreen == ConsoleInfo->OurConOut.Mode->CursorColumn + 1) {
>>> Application/Shell/ConsoleLogger.c:781:ConsoleInfo-
>>> OurConOut.Mode->CursorRow++;
>>> Application/Shell/ConsoleLogger.c:782:ConsoleInfo-
>>> OurConOut.Mode->CursorColumn = 0;
>>> Application/Shell/ConsoleLogger.c:976:ConsoleInfo->OurConOut.Mode =
>> ConsoleInfo->OldConOut->Mode;
>>> 
>>> 
>>> I'm not exactly sure what this code is trying to do as the console should
>> update Mode structure directly? Maybe the intent was to have a copy of
>> gST->ConOut->Mode and keep it in sync? It seems like this should cause
>> more issues, but maybe the edk2 ConSplitter is not broken by this behavior
>> and we are getting lucky?
>>> 
>> 
>> I forgot to mention that setting the Mode->CursorRow in the console code
>> back to the last row if was larger looks like it hides this bug in the shell.
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>> 
>>> Thanks,
>>> 
>>> Andrew Fish
>>> 
>>> https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
>>> ___
>>> 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] I found a fun bug in the Shell today. Looks like we have been getting lucky?

2016-08-25 Thread Carsey, Jaben
Andrew,

Can you file a Bugzilla issue so we can track this issue properly?

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Wednesday, August 24, 2016 6:26 PM
> To: edk2-devel 
> Subject: Re: [edk2] I found a fun bug in the Shell today. Looks like we have
> been getting lucky?
> Importance: High
> 
> 
> > On Aug 24, 2016, at 5:59 PM, Andrew Fish  wrote:
> >
> > I was tracking down a data corruption issue when paging was enabled on an
> edk2 shell command. The crash was in a custom ConSpliter over writing a DXE
> Core data structure. The buffer overflow seemed to be caused by the
> Console getting confused on the location of the end of the screen. I set a
> watchpoint on gST->ConOut->Mode->CursorRow and found the shell was
> the one corrupting the Mode data.
> >
> > UEFI Spec: The following data values in the SIMPLE_TEXT_OUTPUT_MODE
> interface are read-only and are changed by using the appropriate interface
> functions:
> >
> > (master)>git grep "OurConOut.Mode"
> > Application/Shell/ConsoleLogger.c:72:  (*ConsoleInfo)->OurConOut.Mode
> = gST->ConOut->Mode;
> > Application/Shell/ConsoleLogger.c:647://ShellInfoObject.ConsoleInfo-
> >OurConOut.Mode->CursorRow= 0;
> > Application/Shell/ConsoleLogger.c:648://ShellInfoObject.ConsoleInfo-
> >OurConOut.Mode->CursorColumn = 0;
> > Application/Shell/ConsoleLogger.c:704:  if (ConsoleInfo-
> >OurConOut.Mode->CursorColumn > 0) {
> > Application/Shell/ConsoleLogger.c:705:ConsoleInfo-
> >OurConOut.Mode->CursorColumn--;
> > Application/Shell/ConsoleLogger.c:734:  ConsoleInfo->OurConOut.Mode-
> >CursorRow++;
> > Application/Shell/ConsoleLogger.c:741:  ConsoleInfo->OurConOut.Mode-
> >CursorColumn = 0;
> > Application/Shell/ConsoleLogger.c:747:  ConsoleInfo->OurConOut.Mode-
> >CursorColumn++;
> > Application/Shell/ConsoleLogger.c:751:  if ((INTN)ConsoleInfo-
> >ColsPerScreen == ConsoleInfo->OurConOut.Mode->CursorColumn + 1) {
> > Application/Shell/ConsoleLogger.c:781:ConsoleInfo-
> >OurConOut.Mode->CursorRow++;
> > Application/Shell/ConsoleLogger.c:782:ConsoleInfo-
> >OurConOut.Mode->CursorColumn = 0;
> > Application/Shell/ConsoleLogger.c:976:ConsoleInfo->OurConOut.Mode =
> ConsoleInfo->OldConOut->Mode;
> >
> >
> > I'm not exactly sure what this code is trying to do as the console should
> update Mode structure directly? Maybe the intent was to have a copy of
> gST->ConOut->Mode and keep it in sync? It seems like this should cause
> more issues, but maybe the edk2 ConSplitter is not broken by this behavior
> and we are getting lucky?
> >
> 
> I forgot to mention that setting the Mode->CursorRow in the console code
> back to the last row if was larger looks like it hides this bug in the shell.
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> > Thanks,
> >
> > Andrew Fish
> >
> > https://tianocore.acgmultimedia.com/show_bug.cgi?id=105
> > ___
> > 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

2016-08-25 Thread Zhang, Lubo
  Yes, the SVN version number I provide is not consistent with you. 
  The attachment is latest patch log details of the GitHub, you can make a  
contrast when you download the code. And then do some test.
  
Best Regards
Lubo
 
  

-Original Message-
From: Santhapur Naveen [mailto:nave...@amiindia.co.in] 
Sent: Thursday, August 25, 2016 5:09 PM
To: Zhang, Lubo 
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Lubo,

I believe the way we refer the version numbers seem different. I use 
tortoise SVN to get the details of the edk2. When I checked the revision 
numbers 22104 and 21740 for HttpBootDxe and HttpDxe drivers respectively, I 
couldn't find anything. Please refer the attachments. By any means is it 
possible for you to provide the way you refer the revision details?

However, I've downloaded the latest source from the URL you have 
provided and I'm trying to verify the same now. Will update you my result.

Thank you,
Naveen

-Original Message-
From: Zhang, Lubo [mailto:lubo.zh...@intel.com] 
Sent: Thursday, August 25, 2016 7:19 AM
To: Santhapur Naveen
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Naveen
   I update the  SVN and now the revision is 22467.
   The HttpBootDxe revision is 22104,patch is - Fix IPv6 HTTPClient vendor 
class data
   The HttpDxe revision is 21740, patch is - NetworkPkg: Fix typos in 
comments.
   But I suggest you to download  the https://github.com/tianocore/edk2, it 
is latest edk2 code.

Best Regards
Lubo

-Original Message-
From: Santhapur Naveen [mailto:nave...@amiindia.co.in] 
Sent: Wednesday, August 24, 2016 7:10 PM
To: Zhang, Lubo 
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Lubo,

Can you please provide me any reference revision number to download and 
check the same?

Thank you,
Naveen

-Original Message-
From: Zhang, Lubo [mailto:lubo.zh...@intel.com] 
Sent: Wednesday, August 24, 2016 4:23 PM
To: Santhapur Naveen
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Naveen
  I have checked the issue you mentioned. On my side, I updated the code to 
latest and do some test on NT32 and Denlow platform, both the first http boot 
and 2nd are successful. 
  Until now , we have fixed few bugs which may lead the 2nd http boot fail. 
As for the patch you mentioned is one case to fix the bug, So I suggest you to 
update the code firstly and at least make sure the HttpDxe , HttpBootDxe and 
DxeHttpLib are latest. 
   
 If any result, Pls let me know.

Best Regards
Lubo 
 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Santhapur Naveen
Sent: Wednesday, August 24, 2016 4:53 PM
To: Zhang, Lubo 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Lubo,

Please find the following details that may help.

NetworkPkg:Fix a bug the 2nd httpboot fail issue.

Httpboot over Ipv4 or Ipv6 stack,for both Identity and chunked transfer 
mode,when the last data has been parsed by HttpLib, the HttpInstance->NextMsg 
pointer should point a correct location.Now after the first  successful 
httpboot for ipv4 or ipv6,the HttpInstance->NextMsgpoint the character after 
the last byte, it may be a bad buffer if we don't receive another HttpHeader, 
so if call a 2nd httpboot, the wrong NextMsg pointer will cause the httpboot 
fail, so we need to check this case in HttpBodyParserCallback function in the 
first http boot process.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Reviewed-by: Fu Siyuan 
Reviewed-by: Ye Ting 
Reviewed-by: Wu Jiaxin 

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19423 
6f19259b-4bc3-4df7-8a09-765794883524


Thanks,
Naveen

-Original Message-
From: Zhang, Lubo [mailto:lubo.zh...@intel.com] 
Sent: Wednesday, August 24, 2016 2:13 PM
To: Santhapur Naveen
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Naveen
 Could you list the patch name or Git SHA value which fixed the 2nd 
HttpBootfail issue. On my side, the SVN version 24729 is not exist.

Thanks 
Lubo 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Santhapur Naveen
Sent: Wednesday, August 24, 2016 3:40 PM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hello,

  Revision 24729 of edk2 says 2nd HttpBoot fail issue has been fixed. 
So, I have taken the changes from the file 

Re: [edk2] Question on iBFT for iSCSI

2016-08-25 Thread Subramanian, Sriram (EG Servers Platform SW)
Any feedback anyone? Is it reasonable to make the change to publish the 
configured/DHCP provided target IP address rather than the redirected IP to 
iBFT?

Thanks,
Sriram.

-Original Message-
From: Subramanian, Sriram (EG Servers Platform SW) 
Sent: Tuesday, August 23, 2016 7:01 PM
To: 'Ye Ting' ; 'Fu Siyuan' ; 'Wu 
Jiaxin' 
Cc: 'Zimmer, Vincent' ; Li, Ruth ; 
edk2-devel@lists.01.org
Subject: Question on iBFT for iSCSI

All,

The IScsiDxe implementation in EDK2 publishes the iBFT with the target IP 
address to which it connected to, and not the configured IP. These IPs will be 
the same if there is no iSCSI redirection. However, if the configured target 
redirects the initiator to another target (as part of iSCSI Login response), 
the Session data is updated to the new target IP.

To my knowledge, there doesn't look to be any standard/specification governing 
this, so I believe this is open to debate. 

The reason for this question is that the configured IP address may be that of a 
server that's doing the redirect to achieve load balancing or high availability 
in the cluster.

Now if we set the Target IP address in the iBFT to the final IP to which we 
connected, we lose the redirection capability in case the target at the final 
IP goes down, and the OS wants to re-establish the connection. 

If we published the original configured IP, then the OS will attempt to 
reconnect to the configured IP in the iBFT, and will get redirected to another 
target which is up.

So do you think it makes sense to publish the original configured static IP (or 
the original IP provided via DHCP) instead of the redirected IP? What are your 
thoughts on this?

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


Re: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

2016-08-25 Thread Santhapur Naveen
Hi Lubo,

I believe the way we refer the version numbers seem different. I use 
tortoise SVN to get the details of the edk2. When I checked the revision 
numbers 22104 and 21740 for HttpBootDxe and HttpDxe drivers respectively, I 
couldn't find anything. Please refer the attachment. By any means is it 
possible for you to provide the way you refer the revision details?

However, I've downloaded the latest source from the URL you have 
provided and I'm trying to verify the same now. Will update you my result.

Thank you,
Naveen

-Original Message-
From: Zhang, Lubo [mailto:lubo.zh...@intel.com] 
Sent: Thursday, August 25, 2016 7:19 AM
To: Santhapur Naveen
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Naveen
   I update the  SVN and now the revision is 22467.
   The HttpBootDxe revision is 22104,patch is - Fix IPv6 HTTPClient vendor 
class data
   The HttpDxe revision is 21740, patch is - NetworkPkg: Fix typos in 
comments.
   But I suggest you to download  the https://github.com/tianocore/edk2, it 
is latest edk2 code.

Best Regards
Lubo

-Original Message-
From: Santhapur Naveen [mailto:nave...@amiindia.co.in] 
Sent: Wednesday, August 24, 2016 7:10 PM
To: Zhang, Lubo 
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Lubo,

Can you please provide me any reference revision number to download and 
check the same?

Thank you,
Naveen

-Original Message-
From: Zhang, Lubo [mailto:lubo.zh...@intel.com] 
Sent: Wednesday, August 24, 2016 4:23 PM
To: Santhapur Naveen
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Naveen
  I have checked the issue you mentioned. On my side, I updated the code to 
latest and do some test on NT32 and Denlow platform, both the first http boot 
and 2nd are successful. 
  Until now , we have fixed few bugs which may lead the 2nd http boot fail. 
As for the patch you mentioned is one case to fix the bug, So I suggest you to 
update the code firstly and at least make sure the HttpDxe , HttpBootDxe and 
DxeHttpLib are latest. 
   
 If any result, Pls let me know.

Best Regards
Lubo 
 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Santhapur Naveen
Sent: Wednesday, August 24, 2016 4:53 PM
To: Zhang, Lubo 
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Lubo,

Please find the following details that may help.

NetworkPkg:Fix a bug the 2nd httpboot fail issue.

Httpboot over Ipv4 or Ipv6 stack,for both Identity and chunked transfer 
mode,when the last data has been parsed by HttpLib, the HttpInstance->NextMsg 
pointer should point a correct location.Now after the first  successful 
httpboot for ipv4 or ipv6,the HttpInstance->NextMsgpoint the character after 
the last byte, it may be a bad buffer if we don't receive another HttpHeader, 
so if call a 2nd httpboot, the wrong NextMsg pointer will cause the httpboot 
fail, so we need to check this case in HttpBodyParserCallback function in the 
first http boot process.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Zhang Lubo 
Reviewed-by: Fu Siyuan 
Reviewed-by: Ye Ting 
Reviewed-by: Wu Jiaxin 

git-svn-id: https://svn.code.sf.net/p/edk2/code/trunk/edk2@19423 
6f19259b-4bc3-4df7-8a09-765794883524


Thanks,
Naveen

-Original Message-
From: Zhang, Lubo [mailto:lubo.zh...@intel.com] 
Sent: Wednesday, August 24, 2016 2:13 PM
To: Santhapur Naveen
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hi Naveen
 Could you list the patch name or Git SHA value which fixed the 2nd 
HttpBootfail issue. On my side, the SVN version 24729 is not exist.

Thanks 
Lubo 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
Santhapur Naveen
Sent: Wednesday, August 24, 2016 3:40 PM
To: edk2-devel@lists.01.org
Subject: Re: [edk2] 2nd HttpBoot fails even after upgrading to revision 24729

Hello,

  Revision 24729 of edk2 says 2nd HttpBoot fail issue has been fixed. 
So, I have taken the changes from the file NetworkPkg/HttpDxe/HttpImpl.c of the 
revsision 24729 and still the 2nd  HttpBoot fails. Please let me know if I have 
missed any.

P.S: The first http boot happens successfully though. (downloading a Shell.efi)

Thanks,
Naveen
___
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] BootableImageSupportTest\StorageSecurityCommandProtocolTest

2016-08-25 Thread Ramesh R .
Hi,

   When the we run the 
"BootableImageSupportTest\StorageSecurityCommandProtocolTest" test on the NVME 
devices we are getting into error because of the below testing code.

//
// According to TCG definition, when the Security Protocol field is set to 
00h, and SP
// Specific is set to h in a TRUSTED RECEIVE command, return security 
protocol
// information. This Command is not associated with a security send command
//
Status = StorageSecurityCommand->ReceiveData (
   StorageSecurityCommand,
   BlockIo->Media->MediaId,
   1,// Timeout 
10-sec
   0,// 
SecurityProtocol
   0,// 
SecurityProtocolSpecifcData
   10,   // 
PayloadBufferSize,
   DataBuffer,   // 
PayloadBuffer
   
   );
//
// for ATA8-ACS SecurityProtocol, 512 byte is a request
//
if (IsAtaDevice) {
  if((Status == EFI_DEVICE_ERROR) || (Status == EFI_WARN_BUFFER_TOO_SMALL)){
AssertionType = EFI_TEST_ASSERTION_PASSED;
  } else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
  }
} else {
  if((!EFI_ERROR(Status)) || (Status == EFI_WARN_BUFFER_TOO_SMALL)){
AssertionType = EFI_TEST_ASSERTION_PASSED;
  } else {
AssertionType = EFI_TEST_ASSERTION_FAILED;
  }
}

For Ata devices, EFI_DEVICE_ERROR considered as valid error case and for the 
Nvme ( Non ATA) device it's considered as error. Could you please let us know 
why there is difference in this case ?.

Thanks,
Ramesh


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


Re: [edk2] [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add gEfiVariableArchProtocolGuid dependency

2016-08-25 Thread Fan, Jeff
Laszlo,

After discussed with Star, I understood OVMF's circle dependency on Variable 
Arch protocol and SMM CPU driver.

I will defer to check-in this patch till found the better solution.

Before the proper fix adopted, our platforms might not set the HII types 
dynamic PCDs used by PiSmmCpuDxeSmm driver.

Thanks!
Jeff

-Original Message-
From: Zeng, Star 
Sent: Wednesday, August 24, 2016 9:42 PM
To: Laszlo Ersek; Fan, Jeff; edk2-de...@ml01.01.org
Cc: Kinney, Michael D; Justen, Jordan L; Tian, Feng
Subject: Re: [edk2] [Patch v5 48/48] UefiCpuPkg/PiSmmCpuDxeSmm: Add 
gEfiVariableArchProtocolGuid dependency

[Snipped]


 I am not so clear about "the pristine "OVMF_VARS.fd" varstore 
 template that falls right out of the OVMF build".

 Variable driver depends on PcdFlashNvStorageVariableBase(64) be set 
 correctly to produce gEfiVariableArchProtocolGuid protocol.

 After PiSmmCpuDxeSmm adds gEfiVariableArchProtocolGuid dependency 
 and FvbServicesSmm adds gEfiSmmCpuProtocolGuid dependency, there 
 will be a dependency circle below.
  - PiSmmCpuDxeSmm depends on Variable driver to produce 
 gEfiVariableArchProtocolGuid protocol.
  - FvbServicesSmm depends on PiSmmCpuDxeSmm to produce 
 gEfiSmmCpuProtocolGuid protocol.
  - Variable driver depends on FvbServicesSmm to set
 PcdFlashNvStorageVariableBase(64) PCD.
>>>
>>> I understand, thank you for the explanation. The 64-bit PCD that you 
>>> mention is set at the end of the entry point function.
>>>
 Are below approaches possible in OVMF?
 1. Do InitializeVariableFvHeader () and
 PcdFlashNvStorageVariableBase(64) PCD set at PEI phase.
>>>
>>> InitializeVariableFvHeader() writes to the pflash chip, which is 
>>> only possible if the code runs in SMM (under the use case we are 
>>> discussing now). So running it as part of a PEIM would not be right.
>>>
 2. Do InitializeVariableFvHeader () and
 PcdFlashNvStorageVariableBase(64) PCD set in entrypoint with 
 dependency = TRUE, and do other part of code in FvbInitialize() in 
 another gEfiSmmCpuProtocolGuid notification function?
>>>
>>> That's again not good, because it would allow the entry point 
>>> function to exit with success (and to produce the FVB protocol 
>>> interface) even if the pflash configuration on the QEMU command line 
>>> is incorrect (that is, a -D SMM_REQUIRE OVMF build is being run with 
>>> "-bios", rather than the pflash chips).
>>
>> I am a little curious about what makes "writing to the pflash chip is 
>> only possible if the code runs in SMM" in OVMF?
>
> QEMU does that.
>
> That is the way QEMU protects the pflash chip from direct writes by 
> the guest OS. If you pass
>
>   -global driver=cfi.pflash01,property=secure,value=on
>
> to QEMU (which we do), then all write accesses to the chip will be 
> rejected, unless the code doing the write access runs in SMM.
>
>> I meant the code flow for approach 2) I mentioned is like below, I am 
>> not sure if it works or not. :)
>>
>> FvbInitialize() - This may could be done at PEI phase.
>>   InitializeVariableFvHeader()
>> if ValidateFvHeader () returns error status # mark1
>>   allocate buffer to hold data from GetFvbInfo() # mark2
>> endif
>> return PcdOvmfFlashNvStorageVariableBase or the buffer
>>   (allocated at mark2) pointer
>>   set PcdFlashNvStorageVariableBase64/
>>   PcdFlashNvStorageFtwWorkingBase/
>>   PcdFlashNvStorageFtwSpareBase
>>   according to the return from InitializeVariableFvHeader()
>>
>> FvbWriteInitialize() - Notification function on gEfiSmmCpuProtocolGuid
>>   QemuFlashInitialize()
>>   ...
>>   Free buffer(allocated at mark2)
>>   set PcdFlashNvStorageVariableBase64/
>>   PcdFlashNvStorageFtwWorkingBase/
>>   PcdFlashNvStorageFtwSpareBase
>>   again if mark1 returned success
>>   Install FVB protocol
>>   ...
>
> I don't understand the purpose of the buffer allocated at mark2.
>
> Also, currently InitializeVariableFvHeader() erases blocks, after it 
> prints "Variable FV header is not valid. It will be reinitialized". As 
> I said, that part cannot be done in PEI.
>
>  Anyway, I just tried to set PcdFlashNvStorageVariableBase64 / 
> PcdFlashNvStorageFtwWorkingBase / PcdFlashNvStorageFtwSpareBase right 
> in the FDF file, at build time -- that is, even earlier than in PEI 
> --, to the correct values, as dynamic defaults.
>
> (I even verified those values in the build report file.)
>
> It doesn't work: I get the exact same assertion failure. In other 
> words, even if those PCDs are set to the correct values, VariableSmm 
> blows up with "Firmware Volume for Variable Store is corrupted".
>
> . Ahh, I realized why this happens! QEMU prevents even *read
> accesses* if they don't come from code running in SMM.
> 
>
> Independently, I have further thoughts