[edk2] [PATCH v3 0/2] Report ACPI NFIT for reserved memory RAM disks

[Hao Wu]

Changes compared with V2:
1. RamDiskDxe driver now will register an EFI event in the Ready To Boot
   Event Group to a). detect whether EFI_ACPI_TABLE_PROTOCOL and
   EFI_ACPI_SDT_PROTOCOL are both produced; b). publish all the reserved
   memory type RAM disks registered at this point to the NFIT if both
   protocols are produced.

2. The RamDiskUnpublishNfit() now will uninstall NFIT and SSDT that is
   used to report the NVDIMM root device from the ACPI table when there is
   no NFIT structure in NFIT after removing a RAM disk.

3. Instead of adding a new rule in OvmfPkg FDF files, the patch now lists
   the asl and aml options in the main [Rule.Common.DXE_DRIVER] field.


Changes compared with V1:
1. Instead of creating a new NFIT for each registered reserved memory RAM
   disk, new cotent of the NFIT is appended to the existing one.

2. Report an NVDIMM root device in the \SB scope if there is no NFIT in
   the ACPI table.

3. Modify FDF files in OvmfPkg to make sure the report of the NVDIMM root
   device will be done correctly.


Hao Wu (2):
  MdeModulePkg RamDiskDxe: Report ACPI NFIT for reserved memory RAM
disks
  OvmfPkg: Modify fdf files due to RamDiskDxe driver's adding ASL code

 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDisk.asl |  44 ++
 .../Universal/Disk/RamDiskDxe/RamDiskDriver.c  |  80 
 .../Universal/Disk/RamDiskDxe/RamDiskDxe.inf   |  12 +
 .../Universal/Disk/RamDiskDxe/RamDiskImpl.h|  28 ++
 .../Universal/Disk/RamDiskDxe/RamDiskProtocol.c| 494 +
 OvmfPkg/OvmfPkgIa32.fdf|   2 +
 OvmfPkg/OvmfPkgIa32X64.fdf |   2 +
 OvmfPkg/OvmfPkgX64.fdf |   2 +
 8 files changed, 664 insertions(+)
 create mode 100644 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDisk.asl

-- 
1.9.5.msysgit.0

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

[edk2] [PATCH v3 2/2] OvmfPkg: Modify fdf files due to RamDiskDxe driver's adding ASL code

[Hao Wu]

The RamDiskDxe driver in MdeModulePkg now will sometimes report a NVDIMM
Root Device using ASL code which is put in a Secondary System Description
Table (SSDT) according to the ACPI 6.1 spec.

Locating the SSDT requires modifying the fdf files in OvmfPkg.

Cc: Laszlo Ersek 
Cc: Jordan Justen 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu 
---
 OvmfPkg/OvmfPkgIa32.fdf| 2 ++
 OvmfPkg/OvmfPkgIa32X64.fdf | 2 ++
 OvmfPkg/OvmfPkgX64.fdf | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 93a51a5..287737c 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -444,6 +444,8 @@ FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
 PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
+RAW ACPI  Optional   |.acpi
+RAW ASL   Optional   |.aml
   }
 
 [Rule.Common.DXE_RUNTIME_DRIVER]
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index aad16a6..cc1587a 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -444,6 +444,8 @@ FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
 PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
+RAW ACPI  Optional   |.acpi
+RAW ASL   Optional   |.aml
   }
 
 [Rule.Common.DXE_RUNTIME_DRIVER]
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 387b808..1b52381 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -444,6 +444,8 @@ FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
 PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
+RAW ACPI  Optional   |.acpi
+RAW ASL   Optional   |.aml
   }
 
 [Rule.Common.DXE_RUNTIME_DRIVER]
-- 
1.9.5.msysgit.0

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

Re: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor PcdAcpiS3Enable

[Yao, Jiewen]

Reviewed by: jiewen@intel.com


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, April 28, 2016 3:21 AM
> To: edk2-devel-01 
> Cc: Tian, Feng ; Yao, Jiewen ;
> Justen, Jordan L ; Ni, Ruiyu
> ; Zeng, Star 
> Subject: [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor
> PcdAcpiS3Enable
> 
> In the edk2 tree, there are currently four drivers that consume
> PcdAcpiS3Enable:
> 
> 
> IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.in
> f
> 
> MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutor
> Dxe.inf
>   MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>   MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.inf
> 
> From these, AcpiS3SaveDxe is the only one that isn't also a client of the
> S3BootScriptLib class; all the others (BootScriptExecutorDxe,
> S3SaveStateDxe, SmmS3SaveState) are clients of the S3BootScriptLib class.
> 
> In turn, the edk2 tree contains only one non-Null instance of the
> S3BootScriptLib class:
> 
>   MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> 
> Therefore we can safely state that BootScriptExecutorDxe, S3SaveStateDxe,
> and SmmS3SaveState are all linked against PiDxeS3BootScriptLib.
> 
> Now, if PcdAcpiS3Enable is FALSE when either of BootScriptExecutorDxe,
> SmmS3SaveState, or SmmS3SaveState is dispatched, then the following
> happens:
> 
> - The constructor of PiDxeS3BootScriptLib, function
>   S3BootScriptLibInitialize(), registers a protocol installation callback
>   for gEfiDxeSmmReadyToLockProtocolGuid. Namely, the function
>   S3BootScriptEventCallBack().
> 
> - The driver immediately exits with EFI_UNSUPPORTED from its entry point
>   function, upon seeing PcdAcpiS3Enable == FALSE. (See commits
>   800c02fbe2da6, 125e093876414, and d2d38610603f6.)
> 
> - This leaves a dangling callback pointer in the DXE core.
> 
> - When Platform BDS installs gEfiDxeSmmReadyToLockProtocolGuid (which
> is a
>   valid thing to do for locking down SMM, even in the absence of S3
>   support!), things blow up.
> 
> Fix this issue by returning immediately from S3BootScriptLibInitialize()
> if PcdAcpiS3Enable is FALSE -- it is useless to initialize the library
> instance if the containing driver module exits first thing in its entry
> point.
> 
> Cc: Feng Tian 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Ruiyu Ni 
> Cc: Star Zeng 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 1 +
>  MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   | 4
> 
>  2 files changed, 5 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> index 6b7540a5eab9..4e0919ea2c79 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> @@ -67,7 +67,8 @@ [Pcd]
>## SOMETIMES_PRODUCES
>gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateDataPtr
>## CONSUMES
>## SOMETIMES_PRODUCES
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateSmmDataPt
> r
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptRuntimeTableReservePa
> geNumber   ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
> ## CONSUMES
> 
> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> index e7d2a2492e84..d954503f864d 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> @@ -434,12 +434,16 @@ S3BootScriptLibInitialize (
>EFI_SMM_BASE2_PROTOCOL *SmmBase2;
>BOOLEANInSmm;
>EFI_SMM_SYSTEM_TABLE2  *Smst;
>EFI_PHYSICAL_ADDRESS   Buffer;
>EFI_EVENT  Event;
> 
> +  if (!PcdGetBool (PcdAcpiS3Enable)) {
> +return RETURN_SUCCESS;
> +  }
> +
>S3TablePtr =
> (SCRIPT_TABLE_PRIVATE_DATA*)(UINTN)PcdGet64(PcdS3BootScriptTablePriv
> ateDataPtr);
>//
>// The Boot script private data is not be initialized. create it
>//
>if (S3TablePtr == 0) {
>  Buffer = SIZE_4GB - 1;
> --
> 1.8.3.1
> 

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

Re: [edk2] [Patch 0/3] Improve Fixed-MTRR programming performance

[Tian, Feng]

For patch #1, I have comment: 
1. the parameter name is StartMsrNum, but the parameter comment is MsrNum.
2. the algorithm is calculating the MsrNum from *StartMsrNum + 1, it seems not 
match the comment "The start index of the fixed MTRR MSR to program".

Other looks good to me.
Reviewed-by: Feng Tian 

Thanks
Feng


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jeff Fan
Sent: Friday, April 22, 2016 2:06 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [Patch 0/3] Improve Fixed-MTRR programming performance

Internal function ProgramFixedMtrr() is used to calculate Fixed-MTRR and/or 
mask values. It used 3 loops to search Fixed-MTRR index and bytes offset, and 
to calculate mask values. We optimized this function to reduce the loops time 
and got at least 50% performance improvement on real platform. 

Jeff Fan (3):
  UefiCpuPkg/MtrrLib: Reduce the loop time to get fixed-MTRR MSR index
  UefiCpuPkg/MtrrLib: Remove the loop of calculating byte offset in MSR
  UefiCpuPkg/MtrrLib: Remove the loop of calculating Fixed-MTRR Mask

 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 66 ++--
 1 file changed, 33 insertions(+), 33 deletions(-)

--
2.7.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

Re: [edk2] [PATCH 0/3] OvmfPkg: tighten the SMM lockdown

[Zeng, Star]

On 2016/4/28 3:20, Laszlo Ersek wrote:

The first patch (for MdeModulePkg) fixes a bug that is exposed
(triggered) by the third patch.

The second and third patches fix a security vulnerability in OVMF that I
reported to the UEFI SRT more than three weeks ago:

   To: secur...@uefi.org, secal...@redhat.com [...]
   From: Laszlo Ersek 
   Subject: OVMF PlatformBds allows circumvention of SMM
   Message-ID: <5701256d.8010...@redhat.com>
   Date: Sun, 3 Apr 2016 16:15:09 +0200

I have not received any response thus far.

As can be seen above, I also reported the issue to the Red Hat SRT.
While I received acknowledgement about my report, there has been no
technical feedback either.

Now, this issue has very low impact in my opinion:

- Configurations (that is, (host kernel, QEMU, OVMF firmware) triplets)
   on which the issue being fixed is *actually* a vulnerability count as
   "very recent" and "sporadic" at best. I'm not aware of any deployments
   where such a configuration is put to use in a production environment.

- If Secure Boot is enabled, then the attacker's job is much harder: he
   cannot install just any UEFI driver in DriverOrder (see the second
   patch for more explanation), he must instead exploit a bug in an
   already signed UEFI driver, before that driver is blacklisted in DBX.

Independently, Ray's work for porting OvmfPkg to MdeModulePkg/BDS
includes a patch, namely

   [edk2] [Patch v3 11/23] OvmfPkg/PlatformBds: Initialize console
   variables in *BeforeConsole()
   http://thread.gmane.org/gmane.comp.bios.edk2.devel/10859/focus=11039

that needs to connect the PCI root bridges on the call stack of
PlatformBdsInit(), not the current PlatformBdsPolicyBehavior(). Since
patch #2 in this series implements a superset of that requirement, and
given the low impact of the security issue (and the unresponsiveness of
the USRT), it makes sense for me to post this small series first, and
for Ray to rebase his work on top second.

I tested these changes in OVMF, with

   { S3 enabled, S3 disabled } x { SMM enabled, SMM disabled },

using Fedora guests.

Public branch: .

Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Cc: Star Zeng 

Thanks
Laszlo

Laszlo Ersek (3):
   MdeModulePkg: PiDxeS3BootScriptLib: honor PcdAcpiS3Enable
   OvmfPkg: PlatformBdsLib: lock down SMM in PlatformBdsInit()
   OvmfPkg: PlatformBdsLib: lock down SMM regardless of S3

  MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf |  1 +
  MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   |  4 +
  OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 89 

  3 files changed, 58 insertions(+), 36 deletions(-)



Reviewed-by: Star Zeng  to [1/3]
And, you can have my Acked-by for other patches.

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

Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial IO mode

[Andrew Fish]

> On Apr 27, 2016, at 8:10 PM, Ni, Ruiyu  wrote:
> 
>> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi 
>> Guo
>> Sent: Saturday, April 23, 2016 4:54 PM
>> To: edk2-devel@lists.01.org
>> Cc: Kinney, Michael D ; Heyi Guo 
>> ; Tian, Feng ;
>> Zeng, Star 
>> Subject: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial 
>> IO mode
>> 
>> Calculate serial input polling rate according to parameters from
>> serial IO mode as below, to fix potential input truncation.
>> 
>> Polling interval (100ns) =
>> FifoDepth * (StartBit + DataBits + StopBits + ParityBits) * 10,000,000
>> / BaudRate
>> (StopBits is assumed to be 1 and ParityBits is ignored for simplicity.
>> 
>> However, as the event is time sensitive, we need to align the interval
>> to timer interrupt period to make sure the event will be triggered at
>> the expected rate. E.g. if the interval is 2.7ms and timer interrupt
>> period is 1ms, the event will be triggered by time slice sequence as
>> below:
>> 3ms 3ms 2ms 3ms 3ms 2ms...
>> 
>> In such case we will adjust the polling interval to be 2ms.
> 
> Heyi,
> I remember we had a discussion about the polling interval calculation
> equation a month ago. The equation used in the patch looks good to me.
> Because the timer handler is called sometimes every 3ms but not 2.7ms
> resulting the key loss?
> Just want to confirm with you.
> 

Is it safe to make assumptions based on an implementation of an AP? Other 
platforms could be using different code?

Thanks,

Andrew Fish

> 
> Regards,
> Ray
> 
>> 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Heyi Guo 
>> Cc: Feng Tian 
>> Cc: Star Zeng 
>> Cc: Michael D Kinney 
>> ---
>> .../Universal/Console/TerminalDxe/Terminal.c   |  5 +-
>> .../Universal/Console/TerminalDxe/Terminal.h   | 28 ++-
>> .../Universal/Console/TerminalDxe/TerminalConIn.c  | 92 
>> ++
>> .../Universal/Console/TerminalDxe/TerminalDxe.inf  |  1 +
>> 4 files changed, 123 insertions(+), 3 deletions(-)
>> 
>> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>> index 6fde3b2..2944707 100644
>> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>> @@ -71,6 +71,7 @@ TERMINAL_DEV  mTerminalDevTemplate = {
>>  },
>>  NULL, // TerminalConsoleModeData
>>  0,  // SerialInTimeOut
>> +  0,  // KeyboardTimerInterval
>> 
>>  NULL, // RawFifo
>>  NULL, // UnicodeFiFo
>> @@ -984,10 +985,12 @@ TerminalDriverBindingStart (
>>);
>>ASSERT_EFI_ERROR (Status);
>> 
>> +TerminalDevice->KeyboardTimerInterval = GetKeyboardTimerInterval (Mode);
>> +
>>Status = gBS->SetTimer (
>>TerminalDevice->TimerEvent,
>>TimerPeriodic,
>> -KEYBOARD_TIMER_INTERVAL
>> +TerminalDevice->KeyboardTimerInterval
>>);
>>ASSERT_EFI_ERROR (Status);
>> 
>> diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> index 269d2ae..a1ff595 100644
>> --- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> +++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>> @@ -28,6 +28,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR IMPLIED.
>> #include 
>> #include 
>> #include 
>> +#include 
>> 
>> #include 
>> #include 
>> @@ -68,8 +69,6 @@ typedef struct {
>>  UINTN   Rows;
>> } TERMINAL_CONSOLE_MODE_DATA;
>> 
>> -#define KEYBOARD_TIMER_INTERVAL 20  // 0.02s
>> -
>> #define TERMINAL_DEV_SIGNATURE  SIGNATURE_32 ('t', 'm', 'n', 'l')
>> 
>> #define TERMINAL_CONSOLE_IN_EX_NOTIFY_SIGNATURE SIGNATURE_32 ('t', 'm', 'e', 
>> 'n')
>> @@ -91,6 +90,7 @@ typedef struct {
>>  EFI_SIMPLE_TEXT_OUTPUT_MODE SimpleTextOutputMode;
>>  TERMINAL_CONSOLE_MODE_DATA  *TerminalConsoleModeData;
>>  UINTN   SerialInTimeOut;
>> +  UINT64  KeyboardTimerInterval;
>>  RAW_DATA_FIFO   *RawFiFo;
>>  UNICODE_FIFO*UnicodeFiFo;
>>  EFI_KEY_FIFO*EfiKeyFiFo;
>> @@ -1358,4 +1358,28 @@ TerminalConInTimerHandler (
>>  IN EFI_EVENTEvent,
>>  IN VOID *Context
>>  );
>> +
>> +/**
>> +  Calculate input polling timer interval by serial IO mode.
>> +
>> +  @param  Mode  Pointer to serial IO mode.
>> +
>> +  @retval The required polling timer interval in 100ns.
>> +
>> +**/
>> +UINT64
>> +GetKeyboardTimerInterval (
>> +  IN EFI_SERIAL_IO_MODE   *Mode
>> +  );
>> +
>> +/**
>> +  Update 

Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial IO mode

[Ni, Ruiyu]

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi Guo
>Sent: Saturday, April 23, 2016 4:54 PM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Heyi Guo 
>; Tian, Feng ;
>Zeng, Star 
>Subject: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial 
>IO mode
>
>Calculate serial input polling rate according to parameters from
>serial IO mode as below, to fix potential input truncation.
>
>Polling interval (100ns) =
>FifoDepth * (StartBit + DataBits + StopBits + ParityBits) * 10,000,000
>/ BaudRate
>(StopBits is assumed to be 1 and ParityBits is ignored for simplicity.
>
>However, as the event is time sensitive, we need to align the interval
>to timer interrupt period to make sure the event will be triggered at
>the expected rate. E.g. if the interval is 2.7ms and timer interrupt
>period is 1ms, the event will be triggered by time slice sequence as
>below:
>3ms 3ms 2ms 3ms 3ms 2ms...
>
>In such case we will adjust the polling interval to be 2ms.

Heyi,
I remember we had a discussion about the polling interval calculation
equation a month ago. The equation used in the patch looks good to me.
Because the timer handler is called sometimes every 3ms but not 2.7ms
resulting the key loss?
Just want to confirm with you.


Regards,
Ray

>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Heyi Guo 
>Cc: Feng Tian 
>Cc: Star Zeng 
>Cc: Michael D Kinney 
>---
> .../Universal/Console/TerminalDxe/Terminal.c   |  5 +-
> .../Universal/Console/TerminalDxe/Terminal.h   | 28 ++-
> .../Universal/Console/TerminalDxe/TerminalConIn.c  | 92 ++
> .../Universal/Console/TerminalDxe/TerminalDxe.inf  |  1 +
> 4 files changed, 123 insertions(+), 3 deletions(-)
>
>diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>index 6fde3b2..2944707 100644
>--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
>@@ -71,6 +71,7 @@ TERMINAL_DEV  mTerminalDevTemplate = {
>   },
>   NULL, // TerminalConsoleModeData
>   0,  // SerialInTimeOut
>+  0,  // KeyboardTimerInterval
>
>   NULL, // RawFifo
>   NULL, // UnicodeFiFo
>@@ -984,10 +985,12 @@ TerminalDriverBindingStart (
> );
> ASSERT_EFI_ERROR (Status);
>
>+TerminalDevice->KeyboardTimerInterval = GetKeyboardTimerInterval (Mode);
>+
> Status = gBS->SetTimer (
> TerminalDevice->TimerEvent,
> TimerPeriodic,
>-KEYBOARD_TIMER_INTERVAL
>+TerminalDevice->KeyboardTimerInterval
> );
> ASSERT_EFI_ERROR (Status);
>
>diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>index 269d2ae..a1ff595 100644
>--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.h
>@@ -28,6 +28,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>EXPRESS OR IMPLIED.
> #include 
> #include 
> #include 
>+#include 
>
> #include 
> #include 
>@@ -68,8 +69,6 @@ typedef struct {
>   UINTN   Rows;
> } TERMINAL_CONSOLE_MODE_DATA;
>
>-#define KEYBOARD_TIMER_INTERVAL 20  // 0.02s
>-
> #define TERMINAL_DEV_SIGNATURE  SIGNATURE_32 ('t', 'm', 'n', 'l')
>
> #define TERMINAL_CONSOLE_IN_EX_NOTIFY_SIGNATURE SIGNATURE_32 ('t', 'm', 'e', 
> 'n')
>@@ -91,6 +90,7 @@ typedef struct {
>   EFI_SIMPLE_TEXT_OUTPUT_MODE SimpleTextOutputMode;
>   TERMINAL_CONSOLE_MODE_DATA  *TerminalConsoleModeData;
>   UINTN   SerialInTimeOut;
>+  UINT64  KeyboardTimerInterval;
>   RAW_DATA_FIFO   *RawFiFo;
>   UNICODE_FIFO*UnicodeFiFo;
>   EFI_KEY_FIFO*EfiKeyFiFo;
>@@ -1358,4 +1358,28 @@ TerminalConInTimerHandler (
>   IN EFI_EVENTEvent,
>   IN VOID *Context
>   );
>+
>+/**
>+  Calculate input polling timer interval by serial IO mode.
>+
>+  @param  Mode  Pointer to serial IO mode.
>+
>+  @retval The required polling timer interval in 100ns.
>+
>+**/
>+UINT64
>+GetKeyboardTimerInterval (
>+  IN EFI_SERIAL_IO_MODE   *Mode
>+  );
>+
>+/**
>+  Update period of polling timer event.
>+
>+  @param  TerminalDeviceThe terminal device to update.
>+**/
>+VOID
>+UpdatePollingRate (
>+  IN TERMINAL_DEV *TerminalDevice
>+  );
>+
> #endif
>diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
>b/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c
>index 3be877b..e7788a0 100644
>--- a/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConIn.c

Re: [edk2] [Patch] ShellPkg: Enhance ping to select the interface automatically

[Fu, Siyuan]

Hi, Jiaxin

A interface with only link local address should be allowed as the src if the 
dest is also an link local address, right?

And the 2 if conditions in line 1052 is better to change to a "if 
(UnspecifiedSrc) else ..." so it's more readable.

If (UnspecifiedSrc && !NetIp6IsUnspecifiedAddr (Addr) && 
!NetIp6IsLinkLocalAddr (Addr)){
}
if (!UnspecifiedSrc && EFI_IP6_EQUAL (>SrcAddress, Addr)) {
}

Best Regards
Siyuan

> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, April 19, 2016 9:52 AM
> To: edk2-devel@lists.01.org
> Cc: David Van Arnem ; Bhupesh Sharma
> ; Carsey, Jaben ; Ye,
> Ting ; Fu, Siyuan 
> Subject: [Patch] ShellPkg: Enhance ping to select the interface automatically
> 
> This patch is used to support no source IP specified case
> while multiple NICs existed in the platform. The command
> will select the first both connected and configured interface
> automatically.
> 
> Cc: David Van Arnem 
> Cc: Bhupesh Sharma 
> Cc: Jaben Carsey 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  .../Library/UefiShellNetwork1CommandsLib/Ping.c| 224 --
> ---
>  1 file changed, 127 insertions(+), 97 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> index 13bcdde..6b05884 100644
> --- a/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> +++ b/ShellPkg/Library/UefiShellNetwork1CommandsLib/Ping.c
> @@ -874,20 +874,24 @@ PingCreateIpInstance (
>  {
>EFI_STATUS   Status;
>UINTNHandleIndex;
>UINTNHandleNum;
>EFI_HANDLE   *HandleBuffer;
> +  BOOLEAN  UnspecifiedSrc;
> +  BOOLEAN  MediaPresent;
>EFI_SERVICE_BINDING_PROTOCOL *EfiSb;
>VOID *IpXCfg;
>EFI_IP6_CONFIG_DATA  Ip6Config;
>EFI_IP4_CONFIG_DATA  Ip4Config;
>VOID *IpXInterfaceInfo;
>UINTNIfInfoSize;
>EFI_IPv6_ADDRESS *Addr;
>UINTNAddrIndex;
> 
>HandleBuffer  = NULL;
> +  UnspecifiedSrc= FALSE;
> +  MediaPresent  = TRUE;
>EfiSb = NULL;
>IpXInterfaceInfo  = NULL;
>IfInfoSize= 0;
> 
>//
> @@ -923,139 +927,165 @@ PingCreateIpInstance (
>ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN (STR_GEN_PARAM_INV),
> gShellNetwork1HiiHandle, L"ping", mSrcString);
>Status = EFI_INVALID_PARAMETER;
>goto ON_ERROR;
>  }
>}
> +
> +  if (Private->IpChoice == PING_IP_CHOICE_IP6 ? NetIp6IsUnspecifiedAddr
> ((EFI_IPv6_ADDRESS*)>SrcAddress) : \
> +  PingNetIp4IsUnspecifiedAddr ((EFI_IPv4_ADDRESS*)
> >SrcAddress)) {
> +//
> +// SrcAddress is unspecified. So, both connected and configured interface
> will be automatic selected.
> +//
> +UnspecifiedSrc = TRUE;
> +  }
> +
>//
>// For each ip6 protocol, check interface addresses list.
>//
>for (HandleIndex = 0; HandleIndex < HandleNum; HandleIndex++) {
> -
>  EfiSb = NULL;
>  IpXInterfaceInfo  = NULL;
>  IfInfoSize= 0;
> 
> +if (UnspecifiedSrc) {
> +  //
> +  // Check media.
> +  //
> +  NetLibDetectMedia (HandleBuffer[HandleIndex], );
> +  if (!MediaPresent) {
> +//
> +// Skip this one.
> +//
> +continue;
> +  }
> +}
> +
>  Status = gBS->HandleProtocol (
>  HandleBuffer[HandleIndex],
>  Private->IpChoice ==
> PING_IP_CHOICE_IP6?:
> ndingProtocolGuid,
>  (VOID **) 
>  );
>  if (EFI_ERROR (Status)) {
>goto ON_ERROR;
>  }
> 
> -if (Private->IpChoice == PING_IP_CHOICE_IP6?NetIp6IsUnspecifiedAddr
> ((EFI_IPv6_ADDRESS*)>SrcAddress):PingNetIp4IsUnspecifiedAddr
> ((EFI_IPv4_ADDRESS*)>SrcAddress)) {
> -  //
> -  // No need to match interface address.
> -  //
> -  break;
> +//
> +// Ip6config protocol and ip6 service binding protocol are installed
> +// on the same handle.
> +//
> +Status = gBS->HandleProtocol (
> +HandleBuffer[HandleIndex],
> +Private->IpChoice ==
> PING_IP_CHOICE_IP6?:
> uid,
> +(VOID **) 
> +);
> +
> +if (EFI_ERROR (Status)) {
> +  goto ON_ERROR;
> +}
> +//
> +// Get the interface information size.
> +//
> +if (Private->IpChoice == PING_IP_CHOICE_IP6) {
> +  

Re: [edk2] [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jiaxin Wu
> Sent: Tuesday, April 26, 2016 6:39 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Zhang, Lubo ; Fu,
> Siyuan 
> Subject: [edk2] [Patch] NetworkPkg: Avoid the indefinite wait case in HttpDxe
> 
> Need the timer check to avoid the indefinite wait case
> in HttpDxe driver
> A.HTTP receive Header process in HttpTcpReceiveHeader();
> B.HTTP receive Body process in HttpTcpReceiveBody();
> 
> Cc: Hegde Nagaraj P 
> Cc: El-Haj-Mahmoud Samer 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> Cc: Zhang Lubo 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Jiaxin Wu 
> ---
>  NetworkPkg/HttpDxe/HttpImpl.c  | 60
> --
>  NetworkPkg/HttpDxe/HttpProto.c | 59
> ++---
>  NetworkPkg/HttpDxe/HttpProto.h | 15 +++
>  3 files changed, 117 insertions(+), 17 deletions(-)
> 
> diff --git a/NetworkPkg/HttpDxe/HttpImpl.c
> b/NetworkPkg/HttpDxe/HttpImpl.c
> index 63b683e..fe2acbc 100644
> --- a/NetworkPkg/HttpDxe/HttpImpl.c
> +++ b/NetworkPkg/HttpDxe/HttpImpl.c
> @@ -174,10 +174,11 @@ EfiHttpConfigure (
>  >IPv4Node,
>  HttpConfigData->AccessPoint.IPv4Node,
>  sizeof (HttpInstance->IPv4Node)
>  );
>  }
> +
>  //
>  // Creat Tcp child
>  //
>  Status = HttpInitProtocol (HttpInstance, 
> HttpInstance->LocalAddressIsIPv6);
>  if (EFI_ERROR (Status)) {
> @@ -894,11 +895,39 @@ HttpResponseWorker (
>  }
> 
>  HttpInstance->EndofHeader = 
>  HttpInstance->HttpHeaders = 
> 
> -Status = HttpTcpReceiveHeader (HttpInstance, ,
> );
> +
> +if (HttpInstance->TimeoutEvent == NULL) {
> +  //
> +  // Create TimeoutEvent for response
> +  //
> +  Status = gBS->CreateEvent (
> +  EVT_TIMER,
> +  TPL_CALLBACK,
> +  NULL,
> +  NULL,
> +  >TimeoutEvent
> +  );
> +  if (EFI_ERROR (Status)) {
> +goto Error;
> +  }
> +}
> +
> +//
> +// Start the timer, and wait Timeout seconds to receive the header 
> packet.
> +//
> +Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative,
> HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
> +if (EFI_ERROR (Status)) {
> +  goto Error;
> +}
> +
> +Status = HttpTcpReceiveHeader (HttpInstance, ,
> , HttpInstance->TimeoutEvent);
> +
> +gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
> +
>  if (EFI_ERROR (Status)) {
>goto Error;
>  }
> 
>  ASSERT (HttpHeaders != NULL);
> @@ -1095,14 +1124,41 @@ HttpResponseWorker (
>  }
>}
> 
>ASSERT (HttpInstance->MsgParser != NULL);
> 
> +  if (HttpInstance->TimeoutEvent == NULL) {
> +//
> +// Create TimeoutEvent for response
> +//
> +Status = gBS->CreateEvent (
> +EVT_TIMER,
> +TPL_CALLBACK,
> +NULL,
> +NULL,
> +>TimeoutEvent
> +);
> +if (EFI_ERROR (Status)) {
> +  goto Error;
> +}
> +  }
> +
> +  //
> +  // Start the timer, and wait Timeout seconds to receive the body packet.
> +  //
> +  Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative,
> HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
> +  if (EFI_ERROR (Status)) {
> +goto Error;
> +  }
> +
>//
>// We still need receive more data when there is no cache data and
> MsgParser is not NULL;
>//
> -  Status = HttpTcpReceiveBody (Wrap, HttpMsg);
> +  Status = HttpTcpReceiveBody (Wrap, HttpMsg, HttpInstance-
> >TimeoutEvent);
> +
> +  gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
> +
>if (EFI_ERROR (Status)) {
>  goto Error;
>}
> 
>return Status;
> diff --git a/NetworkPkg/HttpDxe/HttpProto.c
> b/NetworkPkg/HttpDxe/HttpProto.c
> index 156f138..eb2af7f 100644
> --- a/NetworkPkg/HttpDxe/HttpProto.c
> +++ b/NetworkPkg/HttpDxe/HttpProto.c
> @@ -813,10 +813,15 @@ HttpCleanProtocol (
>  {
>HttpCloseConnection (HttpInstance);
> 
>HttpCloseTcpConnCloseEvent (HttpInstance);
> 
> +  if (HttpInstance->TimeoutEvent != NULL) {
> +gBS->CloseEvent (HttpInstance->TimeoutEvent);
> +HttpInstance->TimeoutEvent = NULL;
> +  }
> +
>if (HttpInstance->CacheBody != NULL) {
>  FreePool (HttpInstance->CacheBody);
>  HttpInstance->CacheBody = NULL;
>  HttpInstance->NextMsg   = NULL;
>}
> @@ -1537,20 +1542,22 @@ HttpTcpReceive (
>Receive the HTTP header by processing the associated HTTP token.
> 
>@param[in]   HttpInstance The HTTP instance private data.
>

Re: [edk2] [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

[Kinney, Michael D]

With that update, 

Reviewed-by: Michael Kinney 

Mike

From: Gao, Liming 
Sent: Wednesday, April 27, 2016 5:51 PM
To: Ard Biesheuvel 
Cc: edk2-devel-01 ; Kinney, Michael D 

Subject: RE: [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

Good catch. I will update it. 

From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Thursday, April 28, 2016 12:05 AM
To: Gao, Liming 
Cc: edk2-devel-01 ; Kinney, Michael D 

Subject: Re: [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

On 27 April 2016 at 08:40, Liming Gao wrote:
> Use 128 bytes as the start size region to be same to previous one.
>
> 64 bytes is small as the first range. On X64 arch, POOL_OVERHEAD
> takes 40 bytes, the pool data less than 24 bytes can be fit into
> it. But, the real allocation is few that can't reduce its free pool
> link list. And, the second range (64~128) has more allocation
> that also increases the free pool link list of the first range.
> Then, the link list will become longer and longer. When LinkList
> check enable in DEBUG tip, the long link list will bring the
> additional overhead and bad performance. Here is the performance
> data collected in our X64 platform with DEBUG enable.
> 64 byte: 22 seconds in BDS phase
> 128 byte: 19.6 seconds in BDS phase
>
> Cc: Michael Kinney 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
> MdeModulePkg/Core/Dxe/Mem/Pool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c 
> b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> index 5eced88..934b4ca 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> @@ -52,7 +52,7 @@ typedef struct {
> // as we would in a strict power-of-2 sequence
> //
> STATIC CONST UINT16 mPoolSizeTable[] = {
> - 64, 128, 192, 320, 512, 832, 1344, 2176, 3520, 5696, 9216, 14912, 24128
> + 128, 256, 384, 640, 1024, 1664, 2688, 5352, 7040, 11392, 18432, 29824

5352 should be 4352

Other than that, this change looks fine to me

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

Re: [edk2] [Patch v3 02/23] OvmfPkg/PlatformPei: Add memory above 4GB as tested

[Ni, Ruiyu]

Regards,
Ray

>-Original Message-
>From: Laszlo Ersek [mailto:ler...@redhat.com]
>Sent: Tuesday, April 26, 2016 5:27 AM
>To: Ni, Ruiyu 
>Cc: edk2-de...@ml01.01.org; Justen, Jordan L 
>Subject: Re: [edk2] [Patch v3 02/23] OvmfPkg/PlatformPei: Add memory above 4GB 
>as tested
>
>On 04/21/16 08:57, Ruiyu Ni wrote:
>> Since PlatformBootManagerLib do not run memory test
>> to convert untested memory to tested.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ruiyu Ni 
>> Cc: Jordan Justen 
>> Cc: Laszlo Ersek 
>> ---
>>  OvmfPkg/PlatformPei/MemDetect.c |  4 ++--
>>  OvmfPkg/PlatformPei/Platform.c  | 29 -
>>  OvmfPkg/PlatformPei/Platform.h  | 14 +-
>>  OvmfPkg/PlatformPei/Xen.c   |  8 ++--
>>  4 files changed, 5 insertions(+), 50 deletions(-)
>>
>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c 
>> b/OvmfPkg/PlatformPei/MemDetect.c
>> index ed13c57..7991ba2 100644
>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>> @@ -1,7 +1,7 @@
>>  /**@file
>>Memory Detection for Virtual Machines.
>>
>> -  Copyright (c) 2006 - 2014, 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
>> @@ -422,7 +422,7 @@ QemuInitializeRam (
>>  }
>>
>>  if (UpperMemorySize != 0) {
>> -  AddUntestedMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
>> +  AddMemoryBaseSizeHob (BASE_4GB, UpperMemorySize);
>>  }
>>}
>
>Hmmm. I'm reminded of the following discussions:
>
>http://thread.gmane.org/gmane.comp.bios.edk2.devel/1023/focus=1124
>http://thread.gmane.org/gmane.comp.bios.edk2.devel/2385/focus=2425
>
>The worry here is that adding the >=4GB memory descriptor as tested
>(rather than as untested) could cause CoreInitializeMemoryServices() to
>change its behavior, moving the very first memory region -- that is
>declared at the end of the function -- from within OVMF's permanent PEI
>RAM to someplace else. We don't want that.
>
>... Actually, I think this should be safe at this point. The worry I
>described was real before Star's commit 3a05b13106d15 (= the subject of
>the second thread that I linked above). However, with commit
>3a05b13106d15 in place, I think the DXE core will prefer OVMF's
>permanent PEI RAM for initializing the memory services *even if* a >=4GB
>*tested* memory descriptor is present.
>
>(1) Ray, can you please verify this? It is not hard:
>
>(1a) first boot OVMF without your patches applied, and from the debug
>log, save the following lines:
>
>  PeiInstallPeiMemory MemoryBegin ..., MemoryLength ...
>
>  CoreInitializeMemoryServices:
>BaseAddress - ... Length - ...
>
>The area printed by CoreInitializeMemoryServices is currently inside the
>area printed by PeiInstallPeiMemory. That's what we should preserve.
>
>(1b) Then please boot OVMF with your patches applied, and look at the
>same lines. The addresses need not be exactly identical, but the area
>printed by CoreInitializeMemoryServices should continue to be a subset
>of the area printed by PeiInstallPeiMemory. Can you confirm this?
>
>... I cherry-picked this patch (commit 9e6e5876471d) from your Ovmf_Bds2
>branch, on top of current master, and I tested it in isolation. (I
>couldn't test the Xen change, but I tested the QEMU change.)
>
>I checked a number of RAM sizes (4GB, 6GB, 16GB) and a number of configs
>(i440fx (-> without SMM), including S3, and Q35 (with SMM), including
>S3). Everything seemed to work.
>
>I also compared the OVMF logs; there were practically no differences
>(definitely not in the area where I worried). The amount of RAM seen
>within the guest OSes was fine as well.
>
>For this patch:
>
>Tested-by: Laszlo Ersek 
>Reviewed-by: Laszlo Ersek 

You already did the boot test using big RAM size.
Do you still need I do that as well?

>
>(2) A general question: why has the memory testing (= conversion from
>untested to tested) been removed from BDS?
Is your question
"why has the memory testing been removed from BDS *core*?"
The answer is:
1. BDS core just follows the UEFI Spec chapter 3 while that chapter doesn't
say memory testing should be performed in BDS
2. Memory test involves user interaction and screen updates (progress bar,
ESC to skip, test failure...). I don't want BDS core contain these UI. UI is
the component that everyone has different ideas. I want the BDS core to be
stable.



>
>It wasn't doing anything useful for OVMF anyway, I'm just curious what
>else is supposed to do this memory testing on physical platforms, when
>using BDS from MdeModulePkg.
I 

Re: [edk2] [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

[Gao, Liming]

Good catch. I will update it.

From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Thursday, April 28, 2016 12:05 AM
To: Gao, Liming 
Cc: edk2-devel-01 ; Kinney, Michael D 

Subject: Re: [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

On 27 April 2016 at 08:40, Liming Gao wrote:
> Use 128 bytes as the start size region to be same to previous one.
>
> 64 bytes is small as the first range. On X64 arch, POOL_OVERHEAD
> takes 40 bytes, the pool data less than 24 bytes can be fit into
> it. But, the real allocation is few that can't reduce its free pool
> link list. And, the second range (64~128) has more allocation
> that also increases the free pool link list of the first range.
> Then, the link list will become longer and longer. When LinkList
> check enable in DEBUG tip, the long link list will bring the
> additional overhead and bad performance. Here is the performance
> data collected in our X64 platform with DEBUG enable.
> 64 byte: 22 seconds in BDS phase
> 128 byte: 19.6 seconds in BDS phase
>
> Cc: Michael Kinney
> Cc: Ard Biesheuvel
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao
> ---
> MdeModulePkg/Core/Dxe/Mem/Pool.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c 
> b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> index 5eced88..934b4ca 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> @@ -52,7 +52,7 @@ typedef struct {
> // as we would in a strict power-of-2 sequence
> //
> STATIC CONST UINT16 mPoolSizeTable[] = {
> - 64, 128, 192, 320, 512, 832, 1344, 2176, 3520, 5696, 9216, 14912, 24128
> + 128, 256, 384, 640, 1024, 1664, 2688, 5352, 7040, 11392, 18432, 29824

5352 should be 4352

Other than that, this change looks fine to me

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

Re: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor PcdAcpiS3Enable

[Carsey, Jaben]

thanks for the explanation.

I agree that libraries must be designed for this feature from the beginning.  I 
also know that people use libraries for purposes other than their original 
intent.  I think it would be best to plan for this by having properly coded 
destructors in libraries whenever possible.

I mean it's a lot harder to use a DXE/PEI DRIVER in an application or 
unloadable driver than it is to link to a library.  I just think that libraries 
should inherently be self-contained.

That does not mean that you should add one in this case though...  

Reviewed-by: Jaben Carsey 


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Wednesday, April 27, 2016 1:12 PM
> To: Carsey, Jaben ; edk2-devel-01  de...@ml01.01.org>
> Cc: Ni, Ruiyu ; Justen, Jordan L
> ; Tian, Feng ; Yao, Jiewen
> ; Zeng, Star 
> Subject: Re: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib:
> honor PcdAcpiS3Enable
> Importance: High
> 
> On 04/27/16 21:45, Carsey, Jaben wrote:
> > Laszlo,
> >
> > Does the library destructor not get called?  Shouldn't that destructor
> unregister the protocol notify and leave the callback pointer in DXE core
> correct?
> 
> PiDxeS3BootScriptLib doesn't have a DESTRUCTOR at the moment.
> 
> I would prefer not attempting to add a destructor from scratch.
> 
> The constructor does quite a bit of work that would all have to be
> undone in the destructor.
> 
> The commit message here only points out the protocol notify, but in fact
> S3BootScriptLibInitialize() implements a "shared responsibility memory
> allocation" as well, through PcdS3BootScriptTablePrivateDataPtr.
> 
> All modules that are linked against this library instance, and are
> dispatched through the same boot, are prepared to "be the first" and
> allocate the region. The guys that come second and later just reuse the
> area.
> 
> Undoing this is messy; I think it would require reference counting (in
> another PCD, or in the data structure pointed-to by the current PCD).
> 
> The constructor also has some sophisticated logic for determining if it
> is running as part of a DXE_SMM_DRIVER module, or just as part of a
> DXE_(RUNTIME_)DRIVER module. Even without my patch, the constructor has
> five (5) RETURN_SUCCESS exit points. Rolling back all those (valid)
> library states in a destructor would be a nightmare.
> 
> I think that DESTRUCTOR is a clever facility of edk2, but similarly to
> how a DXE_DRIVER must be designed from the ground up for unloading
> (UNLOAD_IMAGE=... in the INF file), a library instance must also be
> designed from the ground up if it is to have a DESTRUCTOR. I think that
> none of BootScriptExecutorDxe, S3SaveStateDxe, SmmS3SaveState, and
> PiDxeS3BootScriptLib qualify.
> 
> (I do see that the shell command libraries use DESTRUCTORs. That makes
> perfect sense: the shell binary that they are linked into with NULL
> library resolution is a UEFI application (not a driver), which is by
> definition unload-capable. Hence I even believe that for the shell
> command libs, DESTRUCTORs are actually unavoidable.)
> 
> Thanks!
> Laszlo
> 
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Wednesday, April 27, 2016 12:21 PM
> >> To: edk2-devel-01 
> >> Cc: Ni, Ruiyu ; Justen, Jordan L
> >> ; Tian, Feng ; Yao,
> Jiewen
> >> ; Zeng, Star 
> >> Subject: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor
> >> PcdAcpiS3Enable
> >>
> >> In the edk2 tree, there are currently four drivers that consume
> >> PcdAcpiS3Enable:
> >>
> >>
> >>
> IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.in
> >> f
> >>
> >>
> MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorD
> >> xe.inf
> >>   MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
> >>   MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.inf
> >>
> >> From these, AcpiS3SaveDxe is the only one that isn't also a client of the
> >> S3BootScriptLib class; all the others (BootScriptExecutorDxe,
> >> S3SaveStateDxe, SmmS3SaveState) are clients of the S3BootScriptLib class.
> >>
> >> In turn, the edk2 tree contains only one non-Null instance of the
> >> S3BootScriptLib class:
> >>
> >>   MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> >>
> >> Therefore we can safely state that BootScriptExecutorDxe,
> S3SaveStateDxe,
> >> and SmmS3SaveState are all linked against PiDxeS3BootScriptLib.
> >>
> >> Now, if PcdAcpiS3Enable is FALSE when either of BootScriptExecutorDxe,
> >> SmmS3SaveState, or SmmS3SaveState is dispatched, then the following
> >> happens:
> >>
> >> - The constructor of 

Re: [edk2] [PATCH] ShellPkg: Fix pci command for '_e' option

[Carsey, Jaben]

Done.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Attar, Abdul Lateef
> Sent: Tuesday, April 26, 2016 3:42 AM
> To: Qiu, Shumin ; Carsey, Jaben
> ; Ni, Ruiyu ; edk2-
> de...@lists.01.org
> Cc: Parthasarathy, Mohan (HPE Servers) ;
> Lakshminarasimha, Sunil Vishwanathpur 
> Subject: Re: [edk2] [PATCH] ShellPkg: Fix pci command for '_e' option
> Importance: High
> 
> Hi,
> Could someone please commit this change, if have no objection.
> Thanks
> AbduL
> 
> From: Qiu, Shumin 
> Sent: Friday, April 22, 2016 7:50:48 AM
> To: Attar, Abdul Lateef; Carsey, Jaben; Ni, Ruiyu; edk2-devel@lists.01.org
> Cc: Parthasarathy, Mohan (HPE Servers); Lakshminarasimha, Sunil
> Vishwanathpur
> Subject: RE: [edk2] [PATCH] ShellPkg: Fix pci command for '_e' option
> 
> Reviewed-by: Qiu Shumin 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Abdul Lateef Attar
> Sent: Thursday, April 21, 2016 7:24 PM
> To: Carsey, Jaben; Ni, Ruiyu; Qiu, Shumin; edk2-devel@lists.01.org
> Cc: mohan_parthasara...@hpe.com; sunil...@hpe.com
> Subject: [edk2] [PATCH] ShellPkg: Fix pci command for '_e' option
> 
> ShellPkg: Fix pci command for '_e' option
> 
> Processing of '_e' argument was missing.
> Added fix, to process the '_e' option
> for printing additional AER information.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Abdul Lateef Attar 
> ---
>  ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> index 4a8a97b..337495e 100644
> --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Pci.c
> @@ -2370,6 +2370,7 @@ PCI_CONFIG_SPACE  *mConfigSpace = NULL;
> STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>{L"-s", TypeValue},
>{L"-i", TypeFlag},
> +  {L"-_e", TypeFlag},
>{NULL, TypeMax}
>};
> 
> --
> 1.9.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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ShellPkg: Update smbiosview for latest Type 17 devices

[Samer El-Haj-Mahmoud]

Update smbiosview to understand latest SMBIOS Type 17 devices from
SMBIOS 3.0.0 spec

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Samer El-Haj-Mahmoud 
---
 .../SmbiosView/QueryTable.c | 21 +
 1 file changed, 21 insertions(+)

diff --git 
a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
index 759f486..d0106c0 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/SmbiosView/QueryTable.c
@@ -3,6 +3,7 @@
   And give a interface of query a string out of a table.
 
   Copyright (c) 2005 - 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
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -2303,6 +2304,26 @@ TABLE_ITEM  MemoryDeviceTypeTable[] = {
   {
 0x19,
 L"  FBD2"
+  },
+  {
+0x1A,
+L"  DDR4"
+  },
+  {
+0x1B,
+L"  LPDDR"
+  },
+  {
+0x1C,
+L"  LPDDR2"
+  },
+  {
+0x1D,
+L"  LPDDR3"
+  },
+  {
+0x1E,
+L"  LPDDR4"
   }
 };
 
-- 
2.6.3.windows.1

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

[edk2] [PATCH] [BaseTools]/Build: Better DSC arch filtering

[Thomas Palmer]

Description:
When building for any specific architecture, the build script today is loading
DSC sections for other architectures not in the build. The build process should
disregard DSC sections that are not relevant to the build.

My previous patch only fixed issue for one section type (Components). This
patch will handle all section types by updating the MetaFileParser class, which
now takes a Arch argument and will filter the DSC table results as they are
returned from the database.  The database still contains all information from
DSCs for when builds support multiple arch's

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Thomas Palmer 
---
 .../Source/Python/Workspace/MetaFileParser.py  | 40 ++
 .../Source/Python/Workspace/WorkspaceDatabase.py   |  7 +---
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py 
b/BaseTools/Source/Python/Workspace/MetaFileParser.py
index 2811fd1..209f47c 100644
--- a/BaseTools/Source/Python/Workspace/MetaFileParser.py
+++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py
@@ -2,7 +2,7 @@
 # This file is used to parse meta files
 #
 # Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.
-# Copyright (c) 2015, Hewlett Packard Enterprise Development, L.P.
+# (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD 
License
 # which accompanies this distribution.  The full text of the license may be 
found at
@@ -144,14 +144,15 @@ class MetaFileParser(object):
 #
 #   @param  FilePathThe path of platform description file
 #   @param  FileTypeThe raw data of DSC file
+#   @param  ArchDefault Arch value for filtering sections
 #   @param  Table   Database used to retrieve module/package 
information
-#   @param  Macros  Macros used for replacement in file
 #   @param  Owner   Owner ID (for sub-section parsing)
 #   @param  FromID from which the data comes (for !INCLUDE 
directive)
 #
-def __init__(self, FilePath, FileType, Table, Owner= -1, From= -1):
+def __init__(self, FilePath, FileType, Arch, Table, Owner= -1, From= -1):
 self._Table = Table
 self._RawTable = Table
+self._Arch = Arch
 self._FileType = FileType
 self.MetaFile = FilePath
 self._FileDir = self.MetaFile.Dir
@@ -211,6 +212,15 @@ class MetaFileParser(object):
 def _SetFinished(self, Value):
 self._Finished = Value
 
+## Remove records that do not match given Filter Arch
+def _FilterRecordList(self, RecordList, FilterArch):
+NewRecordList = []
+for Record in RecordList:
+Arch = Record[3]
+if Arch == 'COMMON' or Arch == FilterArch:
+NewRecordList.append(Record)
+return NewRecordList
+
 ## Use [] style to query data in table, just for readability
 #
 #   DataInfo = [data_type, scope1(arch), scope2(platform/moduletype)]
@@ -230,13 +240,13 @@ class MetaFileParser(object):
 
 # No specific ARCH or Platform given, use raw data
 if self._RawTable and (len(DataInfo) == 1 or DataInfo[1] == None):
-return self._RawTable.Query(*DataInfo)
+return self._FilterRecordList(self._RawTable.Query(*DataInfo), 
self._Arch)
 
 # Do post-process if necessary
 if not self._PostProcessed:
 self._PostProcess()
 
-return self._Table.Query(*DataInfo)
+return self._FilterRecordList(self._Table.Query(*DataInfo), 
DataInfo[1])
 
 ## Data parser for the common format in different type of file
 #
@@ -490,14 +500,14 @@ class InfParser(MetaFileParser):
 #
 #   @param  FilePathThe path of module description file
 #   @param  FileTypeThe raw data of DSC file
+#   @param  ArchDefault Arch value for filtering sections
 #   @param  Table   Database used to retrieve module/package 
information
-#   @param  Macros  Macros used for replacement in file
 #
-def __init__(self, FilePath, FileType, Table):
+def __init__(self, FilePath, FileType, Arch, Table):
 # prevent re-initialization
 if hasattr(self, "_Table"):
 return
-MetaFileParser.__init__(self, FilePath, FileType, Table)
+MetaFileParser.__init__(self, FilePath, FileType, Arch, Table)
 self.PcdsDict = {}
 
 ## Parser starter
@@ -848,16 +858,16 @@ class DscParser(MetaFileParser):
 #
 #   @param  FilePathThe path of platform description file
 #   @param  FileTypeThe raw data of DSC file
+#   @param  ArchDefault Arch value for filtering sections
 

Re: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor PcdAcpiS3Enable

[Carsey, Jaben]

Laszlo,

Does the library destructor not get called?  Shouldn't that destructor 
unregister the protocol notify and leave the callback pointer in DXE core 
correct?

-Jaben

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, April 27, 2016 12:21 PM
> To: edk2-devel-01 
> Cc: Ni, Ruiyu ; Justen, Jordan L
> ; Tian, Feng ; Yao, Jiewen
> ; Zeng, Star 
> Subject: [edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor
> PcdAcpiS3Enable
> 
> In the edk2 tree, there are currently four drivers that consume
> PcdAcpiS3Enable:
> 
> 
> IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.in
> f
> 
> MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorD
> xe.inf
>   MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
>   MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.inf
> 
> From these, AcpiS3SaveDxe is the only one that isn't also a client of the
> S3BootScriptLib class; all the others (BootScriptExecutorDxe,
> S3SaveStateDxe, SmmS3SaveState) are clients of the S3BootScriptLib class.
> 
> In turn, the edk2 tree contains only one non-Null instance of the
> S3BootScriptLib class:
> 
>   MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> 
> Therefore we can safely state that BootScriptExecutorDxe, S3SaveStateDxe,
> and SmmS3SaveState are all linked against PiDxeS3BootScriptLib.
> 
> Now, if PcdAcpiS3Enable is FALSE when either of BootScriptExecutorDxe,
> SmmS3SaveState, or SmmS3SaveState is dispatched, then the following
> happens:
> 
> - The constructor of PiDxeS3BootScriptLib, function
>   S3BootScriptLibInitialize(), registers a protocol installation callback
>   for gEfiDxeSmmReadyToLockProtocolGuid. Namely, the function
>   S3BootScriptEventCallBack().
> 
> - The driver immediately exits with EFI_UNSUPPORTED from its entry point
>   function, upon seeing PcdAcpiS3Enable == FALSE. (See commits
>   800c02fbe2da6, 125e093876414, and d2d38610603f6.)
> 
> - This leaves a dangling callback pointer in the DXE core.
> 
> - When Platform BDS installs gEfiDxeSmmReadyToLockProtocolGuid (which is
> a
>   valid thing to do for locking down SMM, even in the absence of S3
>   support!), things blow up.
> 
> Fix this issue by returning immediately from S3BootScriptLibInitialize()
> if PcdAcpiS3Enable is FALSE -- it is useless to initialize the library
> instance if the containing driver module exits first thing in its entry
> point.
> 
> Cc: Feng Tian 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Ruiyu Ni 
> Cc: Star Zeng 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
>  MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 1 +
>  MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   | 4 
>  2 files changed, 5 insertions(+)
> 
> diff --git
> a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> index 6b7540a5eab9..4e0919ea2c79 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
> @@ -67,7 +67,8 @@ [Pcd]
>## SOMETIMES_PRODUCES
>gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateDataPtr
>## CONSUMES
>## SOMETIMES_PRODUCES
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateSmmDataPtr
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptRuntimeTableReservePa
> geNumber   ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable 
>##
> CONSUMES
> 
> diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> index e7d2a2492e84..d954503f864d 100644
> --- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> +++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
> @@ -434,12 +434,16 @@ S3BootScriptLibInitialize (
>EFI_SMM_BASE2_PROTOCOL *SmmBase2;
>BOOLEANInSmm;
>EFI_SMM_SYSTEM_TABLE2  *Smst;
>EFI_PHYSICAL_ADDRESS   Buffer;
>EFI_EVENT  Event;
> 
> +  if (!PcdGetBool (PcdAcpiS3Enable)) {
> +return RETURN_SUCCESS;
> +  }
> +
>S3TablePtr =
> (SCRIPT_TABLE_PRIVATE_DATA*)(UINTN)PcdGet64(PcdS3BootScriptTablePriv
> ateDataPtr);
>//
>// The Boot script private data is not be initialized. create it
>//
>if (S3TablePtr == 0) {
>  Buffer = SIZE_4GB - 1;
> --
> 1.8.3.1
> 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> 

[edk2] [PATCH 2/3] OvmfPkg: PlatformBdsLib: lock down SMM in PlatformBdsInit()

[Laszlo Ersek]

OVMF's PlatformBdsLib currently makes SMM vulnerable to the following
attack:

(1) a malicious guest OS copies a UEFI driver module to the EFI system
partition,

(2) the OS adds the driver as a Driver option, and references it from
DriverOrder,

(3) at next boot, the BdsEntry() function in
"IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c" processes
Driver and DriverOrder between the calls to PlatformBdsInit() and
PlatformBdsPolicyBehavior(),

(4) OVMF locks down SMM only in PlatformBdsPolicyBehavior(), hence the
driver runs with SMM unlocked.

The BdsEntry() function of the MdeModulePkg BDS driver (in file
"MdeModulePkg/Universal/BdsDxe/BdsEntry.c") recommends to "Signal
ReadyToLock event" in PlatformBootManagerBeforeConsole() -- which
corresponds to PlatformBdsInit() --, not in
PlatformBootManagerAfterConsole() -- which corresponds to
PlatformBdsPolicyBehavior().

Albeit an independent question, but it's worth mentioning: this patch also
brings OvmfPkg's PlatformBdsInit() closer to ArmVirtPkg's. Namely, the
latter signals End-of-Dxe in PlatformBdsInit() already.

Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 64 
 1 file changed, 39 insertions(+), 25 deletions(-)

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c 
b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index 0bc02baf0371..b22f2a74a9d8 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -85,12 +85,26 @@ VisitAllPciInstancesOfProtocol (
 
 VOID
 InstallDevicePathCallback (
   VOID
   );
 
+EFI_STATUS
+EFIAPI
+ConnectRootBridge (
+  IN EFI_HANDLE  RootBridgeHandle,
+  IN VOID*Instance,
+  IN VOID*Context
+  );
+
+STATIC
+VOID
+SaveS3BootScript (
+  VOID
+  );
+
 //
 // BDS Platform Functions
 //
 VOID
 EFIAPI
 PlatformBdsInit (
@@ -110,12 +124,37 @@ Returns:
   None.
 
 --*/
 {
   DEBUG ((EFI_D_INFO, "PlatformBdsInit\n"));
   InstallDevicePathCallback ();
+
+  VisitAllInstancesOfProtocol (,
+ConnectRootBridge, NULL);
+
+  //
+  // Signal the ACPI platform driver that it can download QEMU ACPI tables.
+  //
+  EfiEventGroupSignal ();
+
+  //
+  // We can't signal End-of-Dxe earlier than this. Namely, End-of-Dxe triggers
+  // the preparation of S3 system information. That logic has a hard dependency
+  // on the presence of the FACS ACPI table. Since our ACPI tables are only
+  // installed after PCI enumeration completes, we must not trigger the S3 save
+  // earlier, hence we can't signal End-of-Dxe earlier.
+  //
+  EfiEventGroupSignal ();
+
+  if (QemuFwCfgS3Enabled ()) {
+//
+// Save the boot script too. Note that this requires/includes emitting the
+// DxeSmmReadyToLock event, which in turn locks down SMM.
+//
+SaveS3BootScript ();
+  }
 }
 
 
 EFI_STATUS
 EFIAPI
 ConnectRootBridge (
@@ -1239,37 +1278,12 @@ Returns:
 {
   EFI_STATUS Status;
   EFI_BOOT_MODE  BootMode;
 
   DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior\n"));
 
-  VisitAllInstancesOfProtocol (,
-ConnectRootBridge, NULL);
-
-  //
-  // Signal the ACPI platform driver that it can download QEMU ACPI tables.
-  //
-  EfiEventGroupSignal ();
-
-  //
-  // We can't signal End-of-Dxe earlier than this. Namely, End-of-Dxe triggers
-  // the preparation of S3 system information. That logic has a hard dependency
-  // on the presence of the FACS ACPI table. Since our ACPI tables are only
-  // installed after PCI enumeration completes, we must not trigger the S3 save
-  // earlier, hence we can't signal End-of-Dxe earlier.
-  //
-  EfiEventGroupSignal ();
-
-  if (QemuFwCfgS3Enabled ()) {
-//
-// Save the boot script too. Note that this requires/includes emitting the
-// DxeSmmReadyToLock event, which in turn locks down SMM.
-//
-SaveS3BootScript ();
-  }
-
   if (PcdGetBool (PcdOvmfFlashVariablesEnable)) {
 DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior: not restoring NvVars "
   "from disk since flash variables appear to be supported.\n"));
   } else {
 //
 // Try to restore variables from the hard disk early so
-- 
1.8.3.1


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

[edk2] [PATCH 3/3] OvmfPkg: PlatformBdsLib: lock down SMM regardless of S3

[Laszlo Ersek]

At the moment, the EFI_DXE_SMM_READY_TO_LOCK_PROTOCOL is only installed if
S3 is enabled -- at the end of SaveS3BootScript().

While a runtime OS is never booted with SMM unlocked (because the SMM IPL
locks down SMM as a last resort:

> SMM IPL!  DXE SMM Ready To Lock Protocol not installed before Ready To
> Boot signal
> SmmInstallProtocolInterface: [EfiSmmReadyToLockProtocol] 0
> Patch page table start ...
> Patch page table done!
> SMM IPL locked SMRAM window

), we shouldn't allow UEFI drivers and applications either to mess with
SMM just because S3 is disabled. So install
EFI_DXE_SMM_READY_TO_LOCK_PROTOCOL in PlatformBdsInit() unconditionally.

Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 29 +++-
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c 
b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
index b22f2a74a9d8..8354f31ac2fe 100644
--- a/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c
@@ -122,12 +122,15 @@ Arguments:
 Returns:
 
   None.
 
 --*/
 {
+  EFI_HANDLE Handle;
+  EFI_STATUS Status;
+
   DEBUG ((EFI_D_INFO, "PlatformBdsInit\n"));
   InstallDevicePathCallback ();
 
   VisitAllInstancesOfProtocol (,
 ConnectRootBridge, NULL);
 
@@ -144,17 +147,26 @@ Returns:
   // earlier, hence we can't signal End-of-Dxe earlier.
   //
   EfiEventGroupSignal ();
 
   if (QemuFwCfgS3Enabled ()) {
 //
-// Save the boot script too. Note that this requires/includes emitting the
-// DxeSmmReadyToLock event, which in turn locks down SMM.
+// Save the boot script too. Note that this will require us to emit the
+// DxeSmmReadyToLock event just below, which in turn locks down SMM.
 //
 SaveS3BootScript ();
   }
+
+  //
+  // Prevent further changes to LockBoxes or SMRAM.
+  //
+  Handle = NULL;
+  Status = gBS->InstallProtocolInterface (,
+  , EFI_NATIVE_INTERFACE,
+  NULL);
+  ASSERT_EFI_ERROR (Status);
 }
 
 
 EFI_STATUS
 EFIAPI
 ConnectRootBridge (
@@ -1203,26 +1215,23 @@ Returns:
 }
 
 
 /**
   Save the S3 boot script.
 
-  Note that we trigger DxeSmmReadyToLock here -- otherwise the script wouldn't
-  be saved actually. Triggering this protocol installation event in turn locks
-  down SMM, so no further changes to LockBoxes or SMRAM are possible
-  afterwards.
+  Note that DxeSmmReadyToLock must be signaled after this function returns;
+  otherwise the script wouldn't be saved actually.
 **/
 STATIC
 VOID
 SaveS3BootScript (
   VOID
   )
 {
   EFI_STATUS Status;
   EFI_S3_SAVE_STATE_PROTOCOL *BootScript;
-  EFI_HANDLE Handle;
   STATIC CONST UINT8 Info[] = { 0xDE, 0xAD, 0xBE, 0xEF };
 
   Status = gBS->LocateProtocol (, NULL,
   (VOID **) );
   ASSERT_EFI_ERROR (Status);
 
@@ -1232,18 +1241,12 @@ SaveS3BootScript (
   // than storing just a pointer to runtime or NVS storage.
   //
   Status = BootScript->Write(BootScript, EFI_BOOT_SCRIPT_INFORMATION_OPCODE,
  (UINT32) sizeof Info,
  (EFI_PHYSICAL_ADDRESS)(UINTN) );
   ASSERT_EFI_ERROR (Status);
-
-  Handle = NULL;
-  Status = gBS->InstallProtocolInterface (,
-  , EFI_NATIVE_INTERFACE,
-  NULL);
-  ASSERT_EFI_ERROR (Status);
 }
 
 
 VOID
 EFIAPI
 PlatformBdsPolicyBehavior (
-- 
1.8.3.1

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

[edk2] [PATCH 1/3] MdeModulePkg: PiDxeS3BootScriptLib: honor PcdAcpiS3Enable

[Laszlo Ersek]

In the edk2 tree, there are currently four drivers that consume
PcdAcpiS3Enable:

  IntelFrameworkModulePkg/Universal/Acpi/AcpiS3SaveDxe/AcpiS3SaveDxe.inf
  MdeModulePkg/Universal/Acpi/BootScriptExecutorDxe/BootScriptExecutorDxe.inf
  MdeModulePkg/Universal/Acpi/S3SaveStateDxe/S3SaveStateDxe.inf
  MdeModulePkg/Universal/Acpi/SmmS3SaveState/SmmS3SaveState.inf

>From these, AcpiS3SaveDxe is the only one that isn't also a client of the
S3BootScriptLib class; all the others (BootScriptExecutorDxe,
S3SaveStateDxe, SmmS3SaveState) are clients of the S3BootScriptLib class.

In turn, the edk2 tree contains only one non-Null instance of the
S3BootScriptLib class:

  MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf

Therefore we can safely state that BootScriptExecutorDxe, S3SaveStateDxe,
and SmmS3SaveState are all linked against PiDxeS3BootScriptLib.

Now, if PcdAcpiS3Enable is FALSE when either of BootScriptExecutorDxe,
SmmS3SaveState, or SmmS3SaveState is dispatched, then the following
happens:

- The constructor of PiDxeS3BootScriptLib, function
  S3BootScriptLibInitialize(), registers a protocol installation callback
  for gEfiDxeSmmReadyToLockProtocolGuid. Namely, the function
  S3BootScriptEventCallBack().

- The driver immediately exits with EFI_UNSUPPORTED from its entry point
  function, upon seeing PcdAcpiS3Enable == FALSE. (See commits
  800c02fbe2da6, 125e093876414, and d2d38610603f6.)

- This leaves a dangling callback pointer in the DXE core.

- When Platform BDS installs gEfiDxeSmmReadyToLockProtocolGuid (which is a
  valid thing to do for locking down SMM, even in the absence of S3
  support!), things blow up.

Fix this issue by returning immediately from S3BootScriptLibInitialize()
if PcdAcpiS3Enable is FALSE -- it is useless to initialize the library
instance if the containing driver module exits first thing in its entry
point.

Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf | 1 +
 MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   | 4 
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
index 6b7540a5eab9..4e0919ea2c79 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf
@@ -67,7 +67,8 @@ [Pcd]
   ## SOMETIMES_PRODUCES
   gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateDataPtr
   ## CONSUMES
   ## SOMETIMES_PRODUCES
   gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptTablePrivateSmmDataPtr
   gEfiMdeModulePkgTokenSpaceGuid.PcdS3BootScriptRuntimeTableReservePageNumber  
 ## CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable   
 ## CONSUMES
 
diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
index e7d2a2492e84..d954503f864d 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
@@ -434,12 +434,16 @@ S3BootScriptLibInitialize (
   EFI_SMM_BASE2_PROTOCOL *SmmBase2;
   BOOLEANInSmm;
   EFI_SMM_SYSTEM_TABLE2  *Smst;
   EFI_PHYSICAL_ADDRESS   Buffer;
   EFI_EVENT  Event;
 
+  if (!PcdGetBool (PcdAcpiS3Enable)) {
+return RETURN_SUCCESS;
+  }
+
   S3TablePtr = 
(SCRIPT_TABLE_PRIVATE_DATA*)(UINTN)PcdGet64(PcdS3BootScriptTablePrivateDataPtr);
   //
   // The Boot script private data is not be initialized. create it
   //
   if (S3TablePtr == 0) {
 Buffer = SIZE_4GB - 1;
-- 
1.8.3.1


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

[edk2] [PATCH 0/3] OvmfPkg: tighten the SMM lockdown

[Laszlo Ersek]

The first patch (for MdeModulePkg) fixes a bug that is exposed
(triggered) by the third patch.

The second and third patches fix a security vulnerability in OVMF that I
reported to the UEFI SRT more than three weeks ago:

  To: secur...@uefi.org, secal...@redhat.com [...]
  From: Laszlo Ersek 
  Subject: OVMF PlatformBds allows circumvention of SMM
  Message-ID: <5701256d.8010...@redhat.com>
  Date: Sun, 3 Apr 2016 16:15:09 +0200

I have not received any response thus far.

As can be seen above, I also reported the issue to the Red Hat SRT.
While I received acknowledgement about my report, there has been no
technical feedback either.

Now, this issue has very low impact in my opinion:

- Configurations (that is, (host kernel, QEMU, OVMF firmware) triplets)
  on which the issue being fixed is *actually* a vulnerability count as
  "very recent" and "sporadic" at best. I'm not aware of any deployments
  where such a configuration is put to use in a production environment.

- If Secure Boot is enabled, then the attacker's job is much harder: he
  cannot install just any UEFI driver in DriverOrder (see the second
  patch for more explanation), he must instead exploit a bug in an
  already signed UEFI driver, before that driver is blacklisted in DBX.

Independently, Ray's work for porting OvmfPkg to MdeModulePkg/BDS
includes a patch, namely

  [edk2] [Patch v3 11/23] OvmfPkg/PlatformBds: Initialize console
  variables in *BeforeConsole()
  http://thread.gmane.org/gmane.comp.bios.edk2.devel/10859/focus=11039

that needs to connect the PCI root bridges on the call stack of
PlatformBdsInit(), not the current PlatformBdsPolicyBehavior(). Since
patch #2 in this series implements a superset of that requirement, and
given the low impact of the security issue (and the unresponsiveness of
the USRT), it makes sense for me to post this small series first, and
for Ray to rebase his work on top second.

I tested these changes in OVMF, with

  { S3 enabled, S3 disabled } x { SMM enabled, SMM disabled },

using Fedora guests.

Public branch: .

Cc: Feng Tian 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Ruiyu Ni 
Cc: Star Zeng 

Thanks
Laszlo

Laszlo Ersek (3):
  MdeModulePkg: PiDxeS3BootScriptLib: honor PcdAcpiS3Enable
  OvmfPkg: PlatformBdsLib: lock down SMM in PlatformBdsInit()
  OvmfPkg: PlatformBdsLib: lock down SMM regardless of S3

 MdeModulePkg/Library/PiDxeS3BootScriptLib/DxeS3BootScriptLib.inf |  1 +
 MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c   |  4 +
 OvmfPkg/Library/PlatformBdsLib/BdsPlatform.c | 89 

 3 files changed, 58 insertions(+), 36 deletions(-)

-- 
1.8.3.1

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

Re: [edk2] HMAC calculation clarification for TPM 1.2

[David Van Arnem]

On 04/26/2016 09:46 PM, Long, Qin wrote:
> David,
> 
> I think your original understanding should be correct. (I am not 100% sure. 
> Still need to double-confirm.)
> For one OSAP session, 
>   sharedSecret = HMAC(key.usageAuth, nonceEvenOSAP, nonceOddOSAP)
> The shared secret will be calculated by HMACing the TPM OSAP nonce 
> (nonceEven) and the caller OSAP nonce (nonceOdd). The HMAC key will be 
> key.usageAuth. The common process should look like:
>   HMAC_Init (Ctx, key.usageAuth);
>   HMAC_Update (Ctx, nonceEvenOSAP);
>   HMAC_Update (Ctx, nonceOddOSAP);
>   HMAC_Final (Ctx, sharedSecret)

Hi Qin,

Thanks for the response!  Seeing the HMAC parameters in the functions
makes a lot more sense -- and I foolishly didn't realize EDK2 had
functions for performing the HMAC calculations (and was doing the
padding/XORing manually..)

I suspect the errors in calculation have to do with either me doing
something wrong in my manual calculations, or the value I'm using for
key.usageAuth (I'm using the SRK, but key.usageAuth is not just the SRK
password as I originally thought).  Using the HMAC* functions should
help me figure out my errors more quickly.

Thanks,
David

> 
> So the concatenation should be right here. 
> 
> Not sure if any other issues for the wrong result, e.g. data order, key auth 
> data, etc.  I just ever validated some other TPM 1.2 commands with auth 
> requirements before. 
> 
> 
> Best Regards & Thanks,
> LONG, Qin
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>> David Van Arnem
>> Sent: Wednesday, April 27, 2016 5:57 AM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] HMAC calculation clarification for TPM 1.2
>>
>> Hi all,
>>
>> I don't know if this is the correct place for this question, but I'm hoping
>> someone can provide insight.
>>
>> I'm trying to calculate authorization HMACs for authenticated commands
>> with a TPM 1.2 from an EFI application, but I have not been able to get the
>> correct values the TPM expects.  The TPM uses RFC 2104 for its HMAC
>> calculations:
>>
>> HMAC = H(K XOR opad, H(K XOR ipad, text))
>>
>> where
>>
>> H - SHA1
>> K - key or AuthData
>> ipad - B bytes of 0x36, where B is the block length, 64 opad - B bytes of 
>> 0x5C
>>
>> When the TPM 1.2 Spec, Part 1 Design Principles (page 75), says:
>>
>> sharedSecret = HMAC(key.usageAuth, nonceEvenOSAP, nonceOddOSAP)
>>
>> How is this translated to the RFC 2104 equation, specifically the "text"
>> variable?  I tried:
>>
>> text = nonceEvenOSAP || nonceOddOSAP (|| concatenation)
>>
>> but that did not work.  I'm wondering if instead it should be:
>>
>> text = SHA1(nonceEvenOSAP) || nonceOddOSAP
>>
>> which more closely matches the format for AuthData HMACs associated with
>> an OSAP/OIAP session.  I would like clarification on this before I implement 
>> it,
>> since it is the key for other HMACs.
>>
>> Thanks in advance!
>>
>> --
>> Thanks,
>> David
>> ___
>> 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
> 


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

Re: [edk2] [PATCH] ShellPkg: Fix typos and EDK2 coding style issues

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of El-
> Haj-Mahmoud, Samer
> Sent: Wednesday, April 27, 2016 8:11 AM
> To: Shah, Tapan ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben 
> Subject: Re: [edk2] [PATCH] ShellPkg: Fix typos and EDK2 coding style issues
> Importance: High
> 
> Reviewed-by: Samer El-Haj-Mahmoud 
> 
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Shah, Tapan
> Sent: Wednesday, April 27, 2016 9:56 AM
> To: edk2-devel@lists.01.org
> Cc: jaben.car...@intel.com
> Subject: [edk2] [PATCH] ShellPkg: Fix typos and EDK2 coding style issues
> 
> Fixing typos and EDK2 coding style issues found from previous submit
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Tapan Shah 
> ---
>  .../UefiHandleParsingLib/UefiHandleParsingLib.c  | 20 
> ++--
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> index efafe6f..58f1814 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> @@ -348,7 +348,7 @@ GraphicsOutputProtocolDumpInformation(
>@param[in] TheHandle  The handle that has LoadedImage installed.
>@param[in] VerboseTRUE for additional information, FALSE otherwise.
> 
> -  @retval A poitner to a string containing the information.
> +  @retval A pointer to a string containing the information.
>  **/
>  CHAR16*
>  EFIAPI
> @@ -364,7 +364,7 @@ EdidDiscoveredProtocolDumpInformation (
>CHAR16*TempRetVal;
> 
>if (!Verbose) {
> -return (CatSPrint(NULL, L"EDIDDiscovered"));
> +return (CatSPrint (NULL, L"EDIDDiscovered"));
>}
> 
>Status = gBS->OpenProtocol (
> @@ -380,7 +380,7 @@ EdidDiscoveredProtocolDumpInformation (
>  return NULL;
>}
> 
> -  Temp = HiiGetString (mHandleParsingHiiHandle,
> STRING_TOKEN(STR_EDID_DISCOVERED_MAIN), NULL);
> +  Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN
> (STR_EDID_DISCOVERED_MAIN), NULL);
>if (Temp == NULL) {
>  return NULL;
>}
> @@ -388,8 +388,8 @@ EdidDiscoveredProtocolDumpInformation (
>RetVal = CatSPrint (NULL, Temp, EdidDiscovered->SizeOfEdid);
>SHELL_FREE_NON_NULL (Temp);
> 
> -  if(EdidDiscovered->SizeOfEdid != 0) {
> -Temp = HiiGetString (mHandleParsingHiiHandle,
> STRING_TOKEN(STR_EDID_DISCOVERED_DATA), NULL);
> +  if (EdidDiscovered->SizeOfEdid != 0) {
> +Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN
> (STR_EDID_DISCOVERED_DATA), NULL);
>  if (Temp == NULL) {
>SHELL_FREE_NON_NULL (RetVal);
>return NULL;
> @@ -412,7 +412,7 @@ EdidDiscoveredProtocolDumpInformation (
>@param[in] TheHandle  The handle that has LoadedImage installed.
>@param[in] VerboseTRUE for additional information, FALSE otherwise.
> 
> -  @retval A poitner to a string containing the information.
> +  @retval A pointer to a string containing the information.
>  **/
>  CHAR16*
>  EFIAPI
> @@ -428,7 +428,7 @@ EdidActiveProtocolDumpInformation (
>CHAR16*TempRetVal;
> 
>if (!Verbose) {
> -return (CatSPrint(NULL, L"EDIDActive"));
> +return (CatSPrint (NULL, L"EDIDActive"));
>}
> 
>Status = gBS->OpenProtocol (
> @@ -444,7 +444,7 @@ EdidActiveProtocolDumpInformation (
>  return NULL;
>}
> 
> -  Temp = HiiGetString (mHandleParsingHiiHandle,
> STRING_TOKEN(STR_EDID_ACTIVE_MAIN), NULL);
> +  Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN
> (STR_EDID_ACTIVE_MAIN), NULL);
>if (Temp == NULL) {
>  return NULL;
>}
> @@ -452,8 +452,8 @@ EdidActiveProtocolDumpInformation (
>RetVal = CatSPrint (NULL, Temp, EdidActive->SizeOfEdid);
>SHELL_FREE_NON_NULL (Temp);
> 
> -  if(EdidActive->SizeOfEdid != 0) {
> -Temp = HiiGetString (mHandleParsingHiiHandle,
> STRING_TOKEN(STR_EDID_ACTIVE_DATA), NULL);
> +  if (EdidActive->SizeOfEdid != 0) {
> +Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN
> (STR_EDID_ACTIVE_DATA), NULL);
>  if (Temp == NULL) {
>SHELL_FREE_NON_NULL (RetVal);
>return NULL;
> --
> 2.6.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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPlatformPkg: implement CpuIo2 protocol driver specific for PCI

[Leif Lindholm]

On Wed, Apr 27, 2016 at 05:48:55PM +0200, Ard Biesheuvel wrote:
> >> So we can implement this protocol using MMIO operations only, and take the
> >> PCI I/O translation offset into account when performing I/O port accesses.
> >
> > Some of these lines look a bit long?
> 
> Will fix that

Thanks.

> > First thing, the diff against the UefiCpuPkg original (thanks Laszlo
> > for saving me the effort of figuring out myself :) is pretty tiny.
> > Unless someone is planning to rename UefiCpuPkg to something
> > Xnn-specific, maybe the best thing to do would be to merge this into
> > the existing code? To consider for future consolidation if nothing
> > else.
> >
> 
> Well, the concern is that the I/O space does not exist natively on
> ARM, and so it will always be tied to the PCI implementation (and we
> only need it because PciHostBridgeDxe invokes the protocol in its
> implementation of the I/O accessors it exposes to PCI devices
> drivers). This is different from UefiCpuPkg (of which I don't quite
> understand the reason for existing in the first place tbh)

I'm with you on that one.

> 
> >>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c   | 559 
> >> 
> >>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.inf |  53 ++
> >
> > Don't you mean ArmPkg/Drivers?
> >
> 
> No. The IoTranslation PCD is defined by ArmPlatformPkg, so I didn't
> think it was appropriate.

The PCD can move too.

ArmPlatformPkg is a historic mistake which should over time disperse
into ArmPkg,  and .

I really don't like new stuff being added there, apart from maybe
primecell device drivers pending .

> Also, the ARM architecture does not have the
> notion of I/O ports.

No, but this isn't about I/O ports, it is about providing a
compatibility layer for software (and hardware?) that expects I/O
ports. Which will be generally required across very many ARM*
platforms.

It may well be that where it _should_ live is in MdePkg.

> Also, I should probably rename the dir to match the .inf/.c names
> 
> > Other than that, I'm happy with this addition.
> >
> 
> Thanks.
> 
> 
> >>  2 files changed, 612 insertions(+)
> >>
> >> diff --git a/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c 
> >> b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
> >> new file mode 100644
> >> index ..fecf6a87adce
> >> --- /dev/null
> >> +++ b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
> >> @@ -0,0 +1,559 @@
> >> +/** @file
> >> +  Produces the CPU I/O 2 Protocol.
> >> +
> >> +Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
> >> +Copyright (c) 2016, Linaro Ltd. All rights reserved.
> >> +
> >> +This program and the accompanying materials
> >> +are licensed and made available under the terms and conditions of the BSD 
> >> License
> >> +which accompanies this distribution.  The full text of the license may be 
> >> found at
> >> +http://opensource.org/licenses/bsd-license.php
> >> +
> >> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> >> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> >> IMPLIED.
> >> +
> >> +**/
> >> +
> >> +#include 
> >> +
> >> +#include 
> >> +
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +#include 
> >> +
> >> +#define MAX_IO_PORT_ADDRESS   0x
> >> +
> >> +//
> >> +// Handle for the CPU I/O 2 Protocol
> >> +//
> >> +STATIC EFI_HANDLE  mHandle = NULL;
> >> +
> >> +//
> >> +// Lookup table for increment values based on transfer widths
> >> +//
> >> +STATIC CONST UINT8 mInStride[] = {
> >> +  1, // EfiCpuIoWidthUint8
> >> +  2, // EfiCpuIoWidthUint16
> >> +  4, // EfiCpuIoWidthUint32
> >> +  8, // EfiCpuIoWidthUint64
> >> +  0, // EfiCpuIoWidthFifoUint8
> >> +  0, // EfiCpuIoWidthFifoUint16
> >> +  0, // EfiCpuIoWidthFifoUint32
> >> +  0, // EfiCpuIoWidthFifoUint64
> >> +  1, // EfiCpuIoWidthFillUint8
> >> +  2, // EfiCpuIoWidthFillUint16
> >> +  4, // EfiCpuIoWidthFillUint32
> >> +  8  // EfiCpuIoWidthFillUint64
> >> +};
> >> +
> >> +//
> >> +// Lookup table for increment values based on transfer widths
> >> +//
> >> +STATIC CONST UINT8 mOutStride[] = {
> >> +  1, // EfiCpuIoWidthUint8
> >> +  2, // EfiCpuIoWidthUint16
> >> +  4, // EfiCpuIoWidthUint32
> >> +  8, // EfiCpuIoWidthUint64
> >> +  1, // EfiCpuIoWidthFifoUint8
> >> +  2, // EfiCpuIoWidthFifoUint16
> >> +  4, // EfiCpuIoWidthFifoUint32
> >> +  8, // EfiCpuIoWidthFifoUint64
> >> +  0, // EfiCpuIoWidthFillUint8
> >> +  0, // EfiCpuIoWidthFillUint16
> >> +  0, // EfiCpuIoWidthFillUint32
> >> +  0  // EfiCpuIoWidthFillUint64
> >> +};
> >> +
> >> +/**
> >> +  Check parameters to a CPU I/O 2 Protocol service request.
> >> +
> >> +  The I/O operations are carried out exactly as requested. The caller is 
> >> responsible
> >> +  for satisfying any alignment and I/O width restrictions that a PI 
> >> System on a
> >> +  platform might require. For example on some platforms, width requests of
> >> +  EfiCpuIoWidthUint64 do not work. Misaligned buffers, 

Re: [edk2] [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

[Ard Biesheuvel]

On 27 April 2016 at 08:40, Liming Gao  wrote:
> Use 128 bytes as the start size region to be same to previous one.
>
> 64 bytes is small as the first range. On X64 arch, POOL_OVERHEAD
> takes 40 bytes, the pool data less than 24 bytes can be fit into
> it. But, the real allocation is few that can't reduce its free pool
> link list. And, the second range (64~128) has more allocation
> that also increases the free pool link list of the first range.
> Then, the link list will become longer and longer. When LinkList
> check enable in DEBUG tip, the long link list will bring the
> additional overhead and bad performance. Here is the performance
> data collected in our X64 platform with DEBUG enable.
> 64  byte: 22 seconds in BDS phase
> 128 byte: 19.6 seconds in BDS phase
>
> Cc: Michael Kinney 
> Cc: Ard Biesheuvel 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Liming Gao 
> ---
>  MdeModulePkg/Core/Dxe/Mem/Pool.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c 
> b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> index 5eced88..934b4ca 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
> @@ -52,7 +52,7 @@ typedef struct {
>  // as we would in a strict power-of-2 sequence
>  //
>  STATIC CONST UINT16 mPoolSizeTable[] = {
> -  64, 128, 192, 320, 512, 832, 1344, 2176, 3520, 5696, 9216, 14912, 24128
> +  128, 256, 384, 640, 1024, 1664, 2688, 5352, 7040, 11392, 18432, 29824

5352 should be 4352

Other than that, this change looks fine to me

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

Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial IO mode

[Kinney, Michael D]

Heyi,

1) A couple ways to measure.
  a) Loop calling gBS->Stall() and counting number of stall calls between 
 2 periodic timer event notifications.  The accuracy of the measurement
 depends on the granularity and accuracy of the Stall service.  For your 
 use case, using a Stall() period between 10 uS and 100 uS should work 
 well.
  b) Use the EFI_TIMESTAMP_PROTOCOL to get a performance counter value in
 2 successive timer event notifications and use GetProperties() to convert
 performance counter ticks to time.  This protocol should provide more 
 accurate information, but it is optional, so the code that detects the
 tick period has to check to see if this protocol is present and can use
 it if it is present, and fall back to Stall() if it is not present.

2) I agree.  Ignore time to first notification.  Measure time between 2nd and 
   3rd notification.

3,4) I agree that setting TriggerTime to 0 makes the most sense.

Mike


> -Original Message-
> From: Heyi Guo [mailto:heyi@linaro.org]
> Sent: Wednesday, April 27, 2016 8:21 AM
> To: Kinney, Michael D ; edk2-devel@lists.01.org
> Cc: Tian, Feng ; Zeng, Star 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by 
> serial IO
> mode
> 
> Hi Michael,
> 
> I still have questions about measuring timer tick by event:
> 
> 1. How can we measure the time elapsed? TimerLib is not in UEFI spec,
> and shall we use gBS->Stall in a loop to measure the time?
> 2. By using a periodic timer event, I think we need to drop the 1st
> trigger as it is random and determined by the time when event is
> created, isn't it?
> 3. For the timer event with period of 1*100ns, I think it might be
> triggered twice in one timer tick by below code:
> Timer.c, line #143:
>Event->Timer.TriggerTime = Event->Timer.TriggerTime +
> Event->Timer.Period;
> 
>//
>// If that's before now, then reset the timer to start from now
>//
>if (Event->Timer.TriggerTime <= SystemTime) {
>  Event->Timer.TriggerTime = SystemTime;
>  CoreSignalEvent (mEfiCheckTimerEvent);
>}
> 4. How about setting TriggerTime=0 when calling SetTimer so that the
> period of event will be exactly the timer tick, as stated in UEFI spec?
> 
> Thanks and regards.
> 
> Heyi
> 
> 
> On 04/27/2016 12:21 AM, Kinney, Michael D wrote:
> > Heyi,
> >
> > I agree the source code required to detect the current tick rate using only 
> > UEFI
> services
> > is more complex.
> >
> > However, a UEFI driver (especially ones on an add-in devices such as a PCI 
> > adapter)
> should not
> > use a PCD for the system tick rate because the add-in card can be used in 
> > systems
> with different
> > system tick rates.
> >
> > If you are concerned about complexity, we could consider adding a new lib 
> > function to
> the
> > UefiLib that returns the current system tick rate using UEFI services to 
> > detect it.
> This way,
> > A UEFI driver can be kept simple and we move the complexity into a single 
> > new lib
> function.
> >
> > Best regards,
> >
> > Mike
> >
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> >> Heyi Guo
> >> Sent: Tuesday, April 26, 2016 8:14 AM
> >> To: Kinney, Michael D ; edk2-devel@lists.01.org
> >> Cc: Tian, Feng ; Zeng, Star 
> >> Subject: Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by 
> >> serial IO
> >> mode
> >>
> >> Hi Michael,
> >>
> >> It seems we are making the implementation more and more complicated. How
> >> about just creating a PCD for polling rate which can be set freely by
> >> platforms?
> >>
> >> Regards.
> >>
> >> Heyi
> >>
> >> On 04/24/2016 12:11 AM, Kinney, Michael D wrote:
> >>> Heyi Guo,
> >>>
> >>> The TerminalDxe driver is intended to be a UEFI Driver.  The Timer Arch 
> >>> Protocol is
> >>> a PI Protocol that is intended to be used by the PI DXE Core.  In order to
> determine
> >>> the timer rate in a UEFI way, create a periodic timer event with a period 
> >>> of 1,
> 100ns
> >>> unit, and then measure the time between timer event notification 
> >>> functions.
> >>>
> >>> Mike
> >>>
>  -Original Message-
>  From: Heyi Guo [mailto:heyi@linaro.org]
>  Sent: Saturday, April 23, 2016 1:54 AM
>  To: edk2-devel@lists.01.org
>  Cc: Heyi Guo ; Tian, Feng ; 
>  Zeng, Star
>  ; Kinney, Michael D 
>  Subject: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by 
>  serial IO
> mode
> 
>  Calculate serial input polling rate according to parameters from
>  serial IO mode as below, to fix potential input truncation.
> 
>  Polling interval (100ns) =
>  FifoDepth * (StartBit + DataBits + 

Re: [edk2] [PATCH] ArmPlatformPkg: implement CpuIo2 protocol driver specific for PCI

[Ard Biesheuvel]

On 27 April 2016 at 17:44, Leif Lindholm  wrote:
> On Wed, Apr 27, 2016 at 02:58:29PM +0200, Ard Biesheuvel wrote:
>> The CpuIo2 protocol is required by the generic PciHostBridgeDxe driver,
>> which relies on it to back its own I/O and MMIO operations.
>>
>> Since ARM has no native I/O port equivalent, such accesses can only originate
>> from PCI drivers, and the PCI I/O space is translated to MMIO in this case.
>>
>> So we can implement this protocol using MMIO operations only, and take the
>> PCI I/O translation offset into account when performing I/O port accesses.
>
> Some of these lines look a bit long?
>

Will fix that

>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>
>> We will need this on Seattle to support PCI devices with I/O BARs when we 
>> move
>> to the generic PciHostBridgeDxe driver.
>
> So, I'm mostly happy with this.
>
> First thing, the diff against the UefiCpuPkg original (thanks Laszlo
> for saving me the effort of figuring out myself :) is pretty tiny.
> Unless someone is planning to rename UefiCpuPkg to something
> Xnn-specific, maybe the best thing to do would be to merge this into
> the existing code? To consider for future consolidation if nothing
> else.
>

Well, the concern is that the I/O space does not exist natively on
ARM, and so it will always be tied to the PCI implementation (and we
only need it because PciHostBridgeDxe invokes the protocol in its
implementation of the I/O accessors it exposes to PCI devices
drivers). This is different from UefiCpuPkg (of which I don't quite
understand the reason for existing in the first place tbh)

>>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c   | 559 
>> 
>>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.inf |  53 ++
>
> Don't you mean ArmPkg/Drivers?
>

No. The IoTranslation PCD is defined by ArmPlatformPkg, so I didn't
think it was appropriate. Also, the ARM architecture does not have the
notion of I/O ports.

Also, I should probably rename the dir to match the .inf/.c names

> Other than that, I'm happy with this addition.
>

Thanks.


>>  2 files changed, 612 insertions(+)
>>
>> diff --git a/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c 
>> b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
>> new file mode 100644
>> index ..fecf6a87adce
>> --- /dev/null
>> +++ b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
>> @@ -0,0 +1,559 @@
>> +/** @file
>> +  Produces the CPU I/O 2 Protocol.
>> +
>> +Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
>> +Copyright (c) 2016, Linaro Ltd. All rights reserved.
>> +
>> +This program and the accompanying materials
>> +are licensed and made available under the terms and conditions of the BSD 
>> License
>> +which accompanies this distribution.  The full text of the license may be 
>> found at
>> +http://opensource.org/licenses/bsd-license.php
>> +
>> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> IMPLIED.
>> +
>> +**/
>> +
>> +#include 
>> +
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define MAX_IO_PORT_ADDRESS   0x
>> +
>> +//
>> +// Handle for the CPU I/O 2 Protocol
>> +//
>> +STATIC EFI_HANDLE  mHandle = NULL;
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths
>> +//
>> +STATIC CONST UINT8 mInStride[] = {
>> +  1, // EfiCpuIoWidthUint8
>> +  2, // EfiCpuIoWidthUint16
>> +  4, // EfiCpuIoWidthUint32
>> +  8, // EfiCpuIoWidthUint64
>> +  0, // EfiCpuIoWidthFifoUint8
>> +  0, // EfiCpuIoWidthFifoUint16
>> +  0, // EfiCpuIoWidthFifoUint32
>> +  0, // EfiCpuIoWidthFifoUint64
>> +  1, // EfiCpuIoWidthFillUint8
>> +  2, // EfiCpuIoWidthFillUint16
>> +  4, // EfiCpuIoWidthFillUint32
>> +  8  // EfiCpuIoWidthFillUint64
>> +};
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths
>> +//
>> +STATIC CONST UINT8 mOutStride[] = {
>> +  1, // EfiCpuIoWidthUint8
>> +  2, // EfiCpuIoWidthUint16
>> +  4, // EfiCpuIoWidthUint32
>> +  8, // EfiCpuIoWidthUint64
>> +  1, // EfiCpuIoWidthFifoUint8
>> +  2, // EfiCpuIoWidthFifoUint16
>> +  4, // EfiCpuIoWidthFifoUint32
>> +  8, // EfiCpuIoWidthFifoUint64
>> +  0, // EfiCpuIoWidthFillUint8
>> +  0, // EfiCpuIoWidthFillUint16
>> +  0, // EfiCpuIoWidthFillUint32
>> +  0  // EfiCpuIoWidthFillUint64
>> +};
>> +
>> +/**
>> +  Check parameters to a CPU I/O 2 Protocol service request.
>> +
>> +  The I/O operations are carried out exactly as requested. The caller is 
>> responsible
>> +  for satisfying any alignment and I/O width restrictions that a PI System 
>> on a
>> +  platform might require. For example on some platforms, width requests of
>> +  EfiCpuIoWidthUint64 do not work. Misaligned buffers, on the other hand, 
>> will
>> +  be handled by the driver.
>> +
>> +  @param[in] 

Re: [edk2] [PATCH] ArmPlatformPkg: implement CpuIo2 protocol driver specific for PCI

[Laszlo Ersek]

On 04/27/16 17:44, Leif Lindholm wrote:
> On Wed, Apr 27, 2016 at 02:58:29PM +0200, Ard Biesheuvel wrote:
>> The CpuIo2 protocol is required by the generic PciHostBridgeDxe driver,
>> which relies on it to back its own I/O and MMIO operations.
>>
>> Since ARM has no native I/O port equivalent, such accesses can only originate
>> from PCI drivers, and the PCI I/O space is translated to MMIO in this case.
>>
>> So we can implement this protocol using MMIO operations only, and take the
>> PCI I/O translation offset into account when performing I/O port accesses.
> 
> Some of these lines look a bit long?
> 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>
>> We will need this on Seattle to support PCI devices with I/O BARs when we 
>> move
>> to the generic PciHostBridgeDxe driver.
> 
> So, I'm mostly happy with this.
> 
> First thing, the diff against the UefiCpuPkg original (thanks Laszlo
> for saving me the effort of figuring out myself :) is pretty tiny.
> Unless someone is planning to rename UefiCpuPkg to something
> Xnn-specific, maybe the best thing to do would be to merge this into
> the existing code? To consider for future consolidation if nothing
> else.

As far as I remember, opinions differ whether a single PCD is flexible
enough, in the general case, to control this mapping.

Since the protocol produced by this driver is supposed to be platform
and/or CPU dependent, I think it's fine to keep under ArmPlatformPkg,
separately. But that's just my opinion. :)

Thanks
Laszlo

> 
>>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c   | 559 
>> 
>>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.inf |  53 ++
> 
> Don't you mean ArmPkg/Drivers?
> 
> Other than that, I'm happy with this addition.
> 
>>  2 files changed, 612 insertions(+)
>>
>> diff --git a/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c 
>> b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
>> new file mode 100644
>> index ..fecf6a87adce
>> --- /dev/null
>> +++ b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
>> @@ -0,0 +1,559 @@
>> +/** @file
>> +  Produces the CPU I/O 2 Protocol.
>> +
>> +Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
>> +Copyright (c) 2016, Linaro Ltd. All rights reserved.
>> +
>> +This program and the accompanying materials
>> +are licensed and made available under the terms and conditions of the BSD 
>> License
>> +which accompanies this distribution.  The full text of the license may be 
>> found at
>> +http://opensource.org/licenses/bsd-license.php
>> +
>> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
>> IMPLIED.
>> +
>> +**/
>> +
>> +#include 
>> +
>> +#include 
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +
>> +#define MAX_IO_PORT_ADDRESS   0x
>> +
>> +//
>> +// Handle for the CPU I/O 2 Protocol
>> +//
>> +STATIC EFI_HANDLE  mHandle = NULL;
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths
>> +//
>> +STATIC CONST UINT8 mInStride[] = {
>> +  1, // EfiCpuIoWidthUint8
>> +  2, // EfiCpuIoWidthUint16
>> +  4, // EfiCpuIoWidthUint32
>> +  8, // EfiCpuIoWidthUint64
>> +  0, // EfiCpuIoWidthFifoUint8
>> +  0, // EfiCpuIoWidthFifoUint16
>> +  0, // EfiCpuIoWidthFifoUint32
>> +  0, // EfiCpuIoWidthFifoUint64
>> +  1, // EfiCpuIoWidthFillUint8
>> +  2, // EfiCpuIoWidthFillUint16
>> +  4, // EfiCpuIoWidthFillUint32
>> +  8  // EfiCpuIoWidthFillUint64
>> +};
>> +
>> +//
>> +// Lookup table for increment values based on transfer widths
>> +//
>> +STATIC CONST UINT8 mOutStride[] = {
>> +  1, // EfiCpuIoWidthUint8
>> +  2, // EfiCpuIoWidthUint16
>> +  4, // EfiCpuIoWidthUint32
>> +  8, // EfiCpuIoWidthUint64
>> +  1, // EfiCpuIoWidthFifoUint8
>> +  2, // EfiCpuIoWidthFifoUint16
>> +  4, // EfiCpuIoWidthFifoUint32
>> +  8, // EfiCpuIoWidthFifoUint64
>> +  0, // EfiCpuIoWidthFillUint8
>> +  0, // EfiCpuIoWidthFillUint16
>> +  0, // EfiCpuIoWidthFillUint32
>> +  0  // EfiCpuIoWidthFillUint64
>> +};
>> +
>> +/**
>> +  Check parameters to a CPU I/O 2 Protocol service request.
>> +
>> +  The I/O operations are carried out exactly as requested. The caller is 
>> responsible
>> +  for satisfying any alignment and I/O width restrictions that a PI System 
>> on a
>> +  platform might require. For example on some platforms, width requests of
>> +  EfiCpuIoWidthUint64 do not work. Misaligned buffers, on the other hand, 
>> will
>> +  be handled by the driver.
>> +
>> +  @param[in] MmioOperation  TRUE for an MMIO operation, FALSE for I/O Port 
>> operation.
>> +  @param[in] Width  Signifies the width of the I/O or Memory 
>> operation.
>> +  @param[in] AddressThe base address of the I/O operation.
>> +  @param[in] Count  The number of I/O operations to perform. The 
>> number of
>> +

Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial IO mode

[Heyi Guo]

Hi Michael,

I still have questions about measuring timer tick by event:

1. How can we measure the time elapsed? TimerLib is not in UEFI spec, 
and shall we use gBS->Stall in a loop to measure the time?
2. By using a periodic timer event, I think we need to drop the 1st 
trigger as it is random and determined by the time when event is 
created, isn't it?
3. For the timer event with period of 1*100ns, I think it might be 
triggered twice in one timer tick by below code:

Timer.c, line #143:
  Event->Timer.TriggerTime = Event->Timer.TriggerTime + 
Event->Timer.Period;


  //
  // If that's before now, then reset the timer to start from now
  //
  if (Event->Timer.TriggerTime <= SystemTime) {
Event->Timer.TriggerTime = SystemTime;
CoreSignalEvent (mEfiCheckTimerEvent);
  }
4. How about setting TriggerTime=0 when calling SetTimer so that the 
period of event will be exactly the timer tick, as stated in UEFI spec?


Thanks and regards.

Heyi


On 04/27/2016 12:21 AM, Kinney, Michael D wrote:

Heyi,

I agree the source code required to detect the current tick rate using only 
UEFI services
is more complex.

However, a UEFI driver (especially ones on an add-in devices such as a PCI 
adapter) should not
use a PCD for the system tick rate because the add-in card can be used in 
systems with different
system tick rates.

If you are concerned about complexity, we could consider adding a new lib 
function to the
UefiLib that returns the current system tick rate using UEFI services to detect 
it.  This way,
A UEFI driver can be kept simple and we move the complexity into a single new 
lib function.

Best regards,

Mike


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Heyi Guo
Sent: Tuesday, April 26, 2016 8:14 AM
To: Kinney, Michael D ; edk2-devel@lists.01.org
Cc: Tian, Feng ; Zeng, Star 
Subject: Re: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by 
serial IO
mode

Hi Michael,

It seems we are making the implementation more and more complicated. How
about just creating a PCD for polling rate which can be set freely by
platforms?

Regards.

Heyi

On 04/24/2016 12:11 AM, Kinney, Michael D wrote:

Heyi Guo,

The TerminalDxe driver is intended to be a UEFI Driver.  The Timer Arch 
Protocol is
a PI Protocol that is intended to be used by the PI DXE Core.  In order to 
determine
the timer rate in a UEFI way, create a periodic timer event with a period of 1, 
100ns
unit, and then measure the time between timer event notification functions.

Mike


-Original Message-
From: Heyi Guo [mailto:heyi@linaro.org]
Sent: Saturday, April 23, 2016 1:54 AM
To: edk2-devel@lists.01.org
Cc: Heyi Guo ; Tian, Feng ; Zeng, Star
; Kinney, Michael D 
Subject: [edk2] [PATCH] MdeModulePkg/TerminalDxe: Set polling rate by serial IO 
mode

Calculate serial input polling rate according to parameters from
serial IO mode as below, to fix potential input truncation.

Polling interval (100ns) =
FifoDepth * (StartBit + DataBits + StopBits + ParityBits) * 10,000,000
/ BaudRate
(StopBits is assumed to be 1 and ParityBits is ignored for simplicity.

However, as the event is time sensitive, we need to align the interval
to timer interrupt period to make sure the event will be triggered at
the expected rate. E.g. if the interval is 2.7ms and timer interrupt
period is 1ms, the event will be triggered by time slice sequence as
below:
3ms 3ms 2ms 3ms 3ms 2ms...

In such case we will adjust the polling interval to be 2ms.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Heyi Guo 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Michael D Kinney 
---
   .../Universal/Console/TerminalDxe/Terminal.c   |  5 +-
   .../Universal/Console/TerminalDxe/Terminal.h   | 28 ++-
   .../Universal/Console/TerminalDxe/TerminalConIn.c  | 92 
++
   .../Universal/Console/TerminalDxe/TerminalDxe.inf  |  1 +
   4 files changed, 123 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
index 6fde3b2..2944707 100644
--- a/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
+++ b/MdeModulePkg/Universal/Console/TerminalDxe/Terminal.c
@@ -71,6 +71,7 @@ TERMINAL_DEV  mTerminalDevTemplate = {
 },
 NULL, // TerminalConsoleModeData
 0,  // SerialInTimeOut
+  0,  // KeyboardTimerInterval

 NULL, // RawFifo
 NULL, // UnicodeFiFo
@@ -984,10 +985,12 @@ TerminalDriverBindingStart (
   );
   ASSERT_EFI_ERROR (Status);

+TerminalDevice->KeyboardTimerInterval = GetKeyboardTimerInterval (Mode);
+
   Status = gBS->SetTimer (
   

Re: [edk2] [PATCH] ShellPkg: Fix typos and EDK2 coding style issues

[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 Shah, 
Tapan
Sent: Wednesday, April 27, 2016 9:56 AM
To: edk2-devel@lists.01.org
Cc: jaben.car...@intel.com
Subject: [edk2] [PATCH] ShellPkg: Fix typos and EDK2 coding style issues

Fixing typos and EDK2 coding style issues found from previous submit

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tapan Shah 
---
 .../UefiHandleParsingLib/UefiHandleParsingLib.c  | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index efafe6f..58f1814 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -348,7 +348,7 @@ GraphicsOutputProtocolDumpInformation(
   @param[in] TheHandle  The handle that has LoadedImage installed.
   @param[in] VerboseTRUE for additional information, FALSE otherwise.
 
-  @retval A poitner to a string containing the information.
+  @retval A pointer to a string containing the information.
 **/
 CHAR16*
 EFIAPI
@@ -364,7 +364,7 @@ EdidDiscoveredProtocolDumpInformation (
   CHAR16*TempRetVal;
 
   if (!Verbose) {
-return (CatSPrint(NULL, L"EDIDDiscovered"));
+return (CatSPrint (NULL, L"EDIDDiscovered"));
   }
 
   Status = gBS->OpenProtocol (
@@ -380,7 +380,7 @@ EdidDiscoveredProtocolDumpInformation (
 return NULL;
   }
 
-  Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_DISCOVERED_MAIN), NULL);
+  Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_DISCOVERED_MAIN), NULL);
   if (Temp == NULL) {
 return NULL;
   }
@@ -388,8 +388,8 @@ EdidDiscoveredProtocolDumpInformation (
   RetVal = CatSPrint (NULL, Temp, EdidDiscovered->SizeOfEdid);
   SHELL_FREE_NON_NULL (Temp);
 
-  if(EdidDiscovered->SizeOfEdid != 0) {
-Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_DISCOVERED_DATA), NULL);
+  if (EdidDiscovered->SizeOfEdid != 0) {
+Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_DISCOVERED_DATA), NULL);
 if (Temp == NULL) {
   SHELL_FREE_NON_NULL (RetVal);
   return NULL;
@@ -412,7 +412,7 @@ EdidDiscoveredProtocolDumpInformation (
   @param[in] TheHandle  The handle that has LoadedImage installed.
   @param[in] VerboseTRUE for additional information, FALSE otherwise.
 
-  @retval A poitner to a string containing the information.
+  @retval A pointer to a string containing the information.
 **/
 CHAR16*
 EFIAPI
@@ -428,7 +428,7 @@ EdidActiveProtocolDumpInformation (
   CHAR16*TempRetVal;
 
   if (!Verbose) {
-return (CatSPrint(NULL, L"EDIDActive"));
+return (CatSPrint (NULL, L"EDIDActive"));
   }
 
   Status = gBS->OpenProtocol (
@@ -444,7 +444,7 @@ EdidActiveProtocolDumpInformation (
 return NULL;
   }
 
-  Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_ACTIVE_MAIN), NULL);
+  Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_ACTIVE_MAIN), NULL);
   if (Temp == NULL) {
 return NULL;
   }
@@ -452,8 +452,8 @@ EdidActiveProtocolDumpInformation (
   RetVal = CatSPrint (NULL, Temp, EdidActive->SizeOfEdid);
   SHELL_FREE_NON_NULL (Temp);
 
-  if(EdidActive->SizeOfEdid != 0) {
-Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_ACTIVE_DATA), NULL);
+  if (EdidActive->SizeOfEdid != 0) {
+Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_ACTIVE_DATA), NULL);
 if (Temp == NULL) {
   SHELL_FREE_NON_NULL (RetVal);
   return NULL;
-- 
2.6.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

Re: [edk2] [PATCH] ArmPlatformPkg: implement CpuIo2 protocol driver specific for PCI

[Laszlo Ersek]

On 04/27/16 14:58, Ard Biesheuvel wrote:
> The CpuIo2 protocol is required by the generic PciHostBridgeDxe driver,
> which relies on it to back its own I/O and MMIO operations.
> 
> Since ARM has no native I/O port equivalent, such accesses can only originate
> from PCI drivers, and the PCI I/O space is translated to MMIO in this case.
> 
> So we can implement this protocol using MMIO operations only, and take the
> PCI I/O translation offset into account when performing I/O port accesses.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel 
> ---
> 
> We will need this on Seattle to support PCI devices with I/O BARs when we move
> to the generic PciHostBridgeDxe driver.

I welcome this patch. I've been silently waiting / hoping for it :)

It will also help rebasing ArmVirtPkg to the generic PciHostBridgeDxe
driver.

I assume you started with a copy of UefiCpuPkg/CpuIo2Dxe? (Just so Leif
can compare the drivers, for an easier review.)

Thanks!
Laszlo

>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c   | 559 
> 
>  ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.inf |  53 ++
>  2 files changed, 612 insertions(+)
> 
> diff --git a/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c 
> b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
> new file mode 100644
> index ..fecf6a87adce
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
> @@ -0,0 +1,559 @@
> +/** @file
> +  Produces the CPU I/O 2 Protocol.
> +
> +Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
> +Copyright (c) 2016, Linaro Ltd. All rights reserved.
> +
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD 
> License
> +which accompanies this distribution.  The full text of the license may be 
> found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include 
> +
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define MAX_IO_PORT_ADDRESS   0x
> +
> +//
> +// Handle for the CPU I/O 2 Protocol
> +//
> +STATIC EFI_HANDLE  mHandle = NULL;
> +
> +//
> +// Lookup table for increment values based on transfer widths
> +//
> +STATIC CONST UINT8 mInStride[] = {
> +  1, // EfiCpuIoWidthUint8
> +  2, // EfiCpuIoWidthUint16
> +  4, // EfiCpuIoWidthUint32
> +  8, // EfiCpuIoWidthUint64
> +  0, // EfiCpuIoWidthFifoUint8
> +  0, // EfiCpuIoWidthFifoUint16
> +  0, // EfiCpuIoWidthFifoUint32
> +  0, // EfiCpuIoWidthFifoUint64
> +  1, // EfiCpuIoWidthFillUint8
> +  2, // EfiCpuIoWidthFillUint16
> +  4, // EfiCpuIoWidthFillUint32
> +  8  // EfiCpuIoWidthFillUint64
> +};
> +
> +//
> +// Lookup table for increment values based on transfer widths
> +//
> +STATIC CONST UINT8 mOutStride[] = {
> +  1, // EfiCpuIoWidthUint8
> +  2, // EfiCpuIoWidthUint16
> +  4, // EfiCpuIoWidthUint32
> +  8, // EfiCpuIoWidthUint64
> +  1, // EfiCpuIoWidthFifoUint8
> +  2, // EfiCpuIoWidthFifoUint16
> +  4, // EfiCpuIoWidthFifoUint32
> +  8, // EfiCpuIoWidthFifoUint64
> +  0, // EfiCpuIoWidthFillUint8
> +  0, // EfiCpuIoWidthFillUint16
> +  0, // EfiCpuIoWidthFillUint32
> +  0  // EfiCpuIoWidthFillUint64
> +};
> +
> +/**
> +  Check parameters to a CPU I/O 2 Protocol service request.
> +
> +  The I/O operations are carried out exactly as requested. The caller is 
> responsible
> +  for satisfying any alignment and I/O width restrictions that a PI System 
> on a
> +  platform might require. For example on some platforms, width requests of
> +  EfiCpuIoWidthUint64 do not work. Misaligned buffers, on the other hand, 
> will
> +  be handled by the driver.
> +
> +  @param[in] MmioOperation  TRUE for an MMIO operation, FALSE for I/O Port 
> operation.
> +  @param[in] Width  Signifies the width of the I/O or Memory 
> operation.
> +  @param[in] AddressThe base address of the I/O operation.
> +  @param[in] Count  The number of I/O operations to perform. The 
> number of
> +bytes moved is Width size * Count, starting at 
> Address.
> +  @param[in] Buffer For read operations, the destination buffer to 
> store the results.
> +For write operations, the source buffer from 
> which to write data.
> +
> +  @retval EFI_SUCCESSThe parameters for this request pass the 
> checks.
> +  @retval EFI_INVALID_PARAMETER  Width is invalid for this PI system.
> +  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
> +  @retval EFI_UNSUPPORTEDThe Buffer is not aligned for the given 
> Width.
> +  @retval EFI_UNSUPPORTEDThe address range specified by Address, 
> Width,
> + and Count is not valid for this PI system.
> +
> +**/
> +STATIC
> 

Re: [edk2] [Patch] ShellPkg: Enhance ping6 to select the interface automatically

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Wu, Jiaxin
> Sent: Tuesday, April 26, 2016 6:21 PM
> To: Bhupesh Sharma ; edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ye, Ting ;
> Fu, Siyuan 
> Subject: RE: [edk2] [Patch] ShellPkg: Enhance ping6 to select the interface
> automatically
> Importance: High
> 
> Jaben and Bhupesh,
> 
> Any comments for this patch?
> 
> Thanks.
> Jiaxin
> 
> > -Original Message-
> > From: Wu, Jiaxin
> > Sent: Friday, April 22, 2016 11:01 AM
> > To: Bhupesh Sharma ; Wu, Jiaxin
> > ; edk2-devel@lists.01.org
> > Cc: Carsey, Jaben ; Ye, Ting ;
> > Fu, Siyuan 
> > Subject: RE: [edk2] [Patch] ShellPkg: Enhance ping6 to select the interface
> > automatically
> >
> > Hello Bhupesh,
> >
> > Can you also help to verify the ping6 command for the same case as ping?
> >
> > Thanks and Best Regards!
> > Jiaxin
> >
> > > -Original Message-
> > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > > Jiaxin Wu
> > > Sent: Friday, April 22, 2016 10:57 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Carsey, Jaben ; Ye, Ting
> > > ; Fu, Siyuan 
> > > Subject: [edk2] [Patch] ShellPkg: Enhance ping6 to select the
> > > interface automatically
> > >
> > > This patch is used to support no source IP specified case while
> > > multiple NICs existed in the platform. The command will select the
> > > first both connected and configured interface automatically.
> > >
> > > Cc: Bhupesh Sharma 
> > > Cc: Jaben Carsey 
> > > Cc: Ye Ting 
> > > Cc: Fu Siyuan 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Jiaxin Wu 
> > > ---
> > >  .../Library/UefiShellNetwork2CommandsLib/Ping6.c   | 166
> > --
> > > ---
> > >  1 file changed, 95 insertions(+), 71 deletions(-)
> > >
> > > diff --git a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> > > b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> > > index af7d08f..f129612 100644
> > > --- a/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> > > +++ b/ShellPkg/Library/UefiShellNetwork2CommandsLib/Ping6.c
> > > @@ -661,22 +661,26 @@ Ping6CreateIpInstance (  {
> > >EFI_STATUS   Status;
> > >UINTNHandleIndex;
> > >UINTNHandleNum;
> > >EFI_HANDLE   *HandleBuffer;
> > > +  BOOLEAN  UnspecifiedSrc;
> > > +  BOOLEAN  MediaPresent;
> > >EFI_SERVICE_BINDING_PROTOCOL *Ip6Sb;
> > >EFI_IP6_CONFIG_PROTOCOL  *Ip6Cfg;
> > >EFI_IP6_CONFIG_DATA  Ip6Config;
> > >EFI_IP6_CONFIG_INTERFACE_INFO*IfInfo;
> > >UINTNIfInfoSize;
> > >EFI_IPv6_ADDRESS *Addr;
> > >UINTNAddrIndex;
> > >
> > > -  HandleBuffer = NULL;
> > > -  Ip6Sb= NULL;
> > > -  IfInfo   = NULL;
> > > -  IfInfoSize   = 0;
> > > +  HandleBuffer  = NULL;
> > > +  UnspecifiedSrc= FALSE;
> > > +  MediaPresent  = TRUE;
> > > +  Ip6Sb = NULL;
> > > +  IfInfo= NULL;
> > > +  IfInfoSize= 0;
> > >
> > >//
> > >// Locate all the handles with ip6 service binding protocol.
> > >//
> > >Status = gBS->LocateHandleBuffer (
> > > @@ -687,115 +691,135 @@ Ping6CreateIpInstance (
> > >
> > >);
> > >if (EFI_ERROR (Status) || (HandleNum == 0)) {
> > >  return EFI_ABORTED;
> > >}
> > > +
> > > +  if (NetIp6IsUnspecifiedAddr (>SrcAddress)) {
> > > +//
> > > +// SrcAddress is unspecified. So, both connected and configured
> > > + interface
> > > will be automatic selected.
> > > +//
> > > +UnspecifiedSrc = TRUE;
> > > +  }
> > > +
> > >//
> > >// Source address is required when pinging a link-local address on 
> > > multi-
> > >// interfaces host.
> > >//
> > >if (NetIp6IsLinkLocalAddr (>DstAddress) &&
> > > -  NetIp6IsUnspecifiedAddr (>SrcAddress) &&
> > > -  (HandleNum > 1)) {
> > > +  UnspecifiedSrc && (HandleNum > 1)) {
> > >  ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > > (STR_PING6_INVALID_SOURCE), gShellNetwork2HiiHandle);
> > >  Status = EFI_INVALID_PARAMETER;
> > >  goto ON_ERROR;
> > >}
> > > +
> > >//
> > >// For each ip6 protocol, check interface addresses list.
> > >//
> > >for (HandleIndex = 0; HandleIndex < HandleNum; HandleIndex++) {
> > >
> > >  Ip6Sb  = NULL;
> > >  IfInfo = NULL;
> > >  IfInfoSize = 0;
> > >
> > > +   

Re: [edk2] [Patch v3 11/23] OvmfPkg/PlatformBds: Initialize console variables in *BeforeConsole()

[Laszlo Ersek]

On 04/21/16 08:58, Ruiyu Ni wrote:
> The major difference between IntelFrameworkModulePkg/BDS and
> MdeModulePkg/BDS is the latter connects the consoles in core
> code while the former connects in platform code.
> The change initializes the console variables in
> PlatformBootManagerBeforeConsole() and removes the console
> connection code.
> It also removes unused functions: PlatformBdsNoConsoleAction()
> and LockKeyboards().
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 99 
> ++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   | 31 +++
>  .../Library/PlatformBootManagerLib/PlatformData.c  |  2 +-
>  3 files changed, 23 insertions(+), 109 deletions(-)

The patch is good.

There is one thing about it that I strongly dislike:

> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index a4008fd..7fc2dd5 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -113,6 +113,7 @@ Returns:
>  {
>DEBUG ((EFI_D_INFO, "PlatformBootManagerBeforeConsole\n"));
>InstallDevicePathCallback ();
> +  PlatformInitializeConsole (gPlatformConsole);
>  }
>  
>  
> @@ -613,14 +614,17 @@ DetectAndPreparePlatformPciDevicePaths (
>BOOLEAN DetectVgaOnly
>)
>  {
> +  VisitAllInstancesOfProtocol (,
> +ConnectRootBridge, NULL);
> +

This right here. I understand why it is necessary, but it's not the
right thing to do.

Instead, here's what I propose. I'll make an attempt to move the
existing connection of root bridges -- and other stuff that depends on
it directly -- to PlatformBdsInit(), from the top of
PlatformBdsPolicyBehavior().

I have two patches in mind. Once those are merged, please rebase your
series on top.

Note that this rebase of yours will involve manually redoing the
following patch:

  [Patch v3 05/23] OvmfPkg: Duplicate PlatformBdsLib to
   PlatformBootManagerLib

That is, you will have to duplicate "OvmfPkg/Library/PlatformBdsLib"
afresh. And then adapt / rework the rest of your patches as well (I
expect minimal differences -- just context differences, basically).

Then, regarding this patch, the above hunk will become unnecessary. The
rest of this patch is fine.

Thanks!
Laszlo


>mDetectVgaOnly = DetectVgaOnly;
>return VisitAllPciInstances (DetectAndPreparePlatformPciDevicePath);
>  }
>  
>  
> -EFI_STATUS
> -PlatformBdsConnectConsole (
> -  IN BDS_CONSOLE_CONNECT_ENTRY   *PlatformConsole
> +VOID
> +PlatformInitializeConsole (
> +  IN PLATFORM_CONSOLE_CONNECT_ENTRY   *PlatformConsole
>)
>  /*++
>  
> @@ -632,37 +636,17 @@ Routine Description:
>  Arguments:
>  
>PlatformConsole - Predfined platform default console device array.
> -
> -Returns:
> -
> -  EFI_SUCCESS - Success connect at least one ConIn and ConOut
> -device, there must have one ConOut device is
> -active vga device.
> -
> -  EFI_STATUS  - Return the status of
> -BdsLibConnectAllDefaultConsoles ()
> -
>  --*/
>  {
> -  EFI_STATUS Status;
>UINTN  Index;
>EFI_DEVICE_PATH_PROTOCOL   *VarConout;
>EFI_DEVICE_PATH_PROTOCOL   *VarConin;
> -  UINTN  DevicePathSize;
>  
>//
>// Connect RootBridge
>//
> -  VarConout = BdsLibGetVariableAndSize (
> -VarConsoleOut,
> -,
> -
> -);
> -  VarConin = BdsLibGetVariableAndSize (
> -   VarConsoleInp,
> -   ,
> -   
> -   );
> +  GetEfiGlobalVariable2 (EFI_CON_OUT_VARIABLE_NAME, (VOID **) , 
> NULL);
> +  GetEfiGlobalVariable2 (EFI_CON_IN_VARIABLE_NAME, (VOID **) , 
> NULL);
>  
>if (VarConout == NULL || VarConin == NULL) {
>  //
> @@ -695,16 +679,6 @@ Returns:
>  //
>  DetectAndPreparePlatformPciDevicePaths (TRUE);
>}
> -
> -  //
> -  // Connect the all the default console with current cosole variable
> -  //
> -  Status = BdsLibConnectAllDefaultConsoles ();
> -  if (EFI_ERROR (Status)) {
> -return Status;
> -  }
> -
> -  return EFI_SUCCESS;
>  }
>  
>  
> @@ -1277,16 +1251,6 @@ Routine Description:
>// Notes: this part code can be change with the table policy
>//
>ASSERT (BootMode == BOOT_WITH_FULL_CONFIGURATION);
> -  //
> -  // Connect platform console
> -  //
> -  Status = PlatformBdsConnectConsole (gPlatformConsole);
> -  if (EFI_ERROR (Status)) {
> -//
> -// Here OEM/IBV can customize with defined action
> -//
> -PlatformBdsNoConsoleAction ();
> -  }
>  
>//
>// Memory test and Logo 

[edk2] [PATCH] ShellPkg: Fix typos and EDK2 coding style issues

[Tapan Shah]

Fixing typos and EDK2 coding style issues found from previous submit

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Tapan Shah 
---
 .../UefiHandleParsingLib/UefiHandleParsingLib.c  | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index efafe6f..58f1814 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -348,7 +348,7 @@ GraphicsOutputProtocolDumpInformation(
   @param[in] TheHandle  The handle that has LoadedImage installed.
   @param[in] VerboseTRUE for additional information, FALSE otherwise.
 
-  @retval A poitner to a string containing the information.
+  @retval A pointer to a string containing the information.
 **/
 CHAR16*
 EFIAPI
@@ -364,7 +364,7 @@ EdidDiscoveredProtocolDumpInformation (
   CHAR16*TempRetVal;
 
   if (!Verbose) {
-return (CatSPrint(NULL, L"EDIDDiscovered"));
+return (CatSPrint (NULL, L"EDIDDiscovered"));
   }
 
   Status = gBS->OpenProtocol (
@@ -380,7 +380,7 @@ EdidDiscoveredProtocolDumpInformation (
 return NULL;
   }
 
-  Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_DISCOVERED_MAIN), NULL);
+  Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_DISCOVERED_MAIN), NULL);
   if (Temp == NULL) {
 return NULL;
   }
@@ -388,8 +388,8 @@ EdidDiscoveredProtocolDumpInformation (
   RetVal = CatSPrint (NULL, Temp, EdidDiscovered->SizeOfEdid);
   SHELL_FREE_NON_NULL (Temp);
 
-  if(EdidDiscovered->SizeOfEdid != 0) {
-Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_DISCOVERED_DATA), NULL);
+  if (EdidDiscovered->SizeOfEdid != 0) {
+Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_DISCOVERED_DATA), NULL);
 if (Temp == NULL) {
   SHELL_FREE_NON_NULL (RetVal);
   return NULL;
@@ -412,7 +412,7 @@ EdidDiscoveredProtocolDumpInformation (
   @param[in] TheHandle  The handle that has LoadedImage installed.
   @param[in] VerboseTRUE for additional information, FALSE otherwise.
 
-  @retval A poitner to a string containing the information.
+  @retval A pointer to a string containing the information.
 **/
 CHAR16*
 EFIAPI
@@ -428,7 +428,7 @@ EdidActiveProtocolDumpInformation (
   CHAR16*TempRetVal;
 
   if (!Verbose) {
-return (CatSPrint(NULL, L"EDIDActive"));
+return (CatSPrint (NULL, L"EDIDActive"));
   }
 
   Status = gBS->OpenProtocol (
@@ -444,7 +444,7 @@ EdidActiveProtocolDumpInformation (
 return NULL;
   }
 
-  Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_ACTIVE_MAIN), NULL);
+  Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_ACTIVE_MAIN), NULL);
   if (Temp == NULL) {
 return NULL;
   }
@@ -452,8 +452,8 @@ EdidActiveProtocolDumpInformation (
   RetVal = CatSPrint (NULL, Temp, EdidActive->SizeOfEdid);
   SHELL_FREE_NON_NULL (Temp);
 
-  if(EdidActive->SizeOfEdid != 0) {
-Temp = HiiGetString (mHandleParsingHiiHandle, 
STRING_TOKEN(STR_EDID_ACTIVE_DATA), NULL);
+  if (EdidActive->SizeOfEdid != 0) {
+Temp = HiiGetString (mHandleParsingHiiHandle, STRING_TOKEN 
(STR_EDID_ACTIVE_DATA), NULL);
 if (Temp == NULL) {
   SHELL_FREE_NON_NULL (RetVal);
   return NULL;
-- 
2.6.2.windows.1

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

Re: [edk2] [Patch v3 10/23] OvmfPkg/PlatformBds: Use ConvertDevicePathToText in DevicePathLib

[Laszlo Ersek]

On 04/21/16 08:58, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c  | 6 +++---
>  OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index dbbc579..a4008fd 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -200,7 +200,7 @@ Returns:
>//
>// Print Device Path
>//
> -  DevPathStr = DevicePathToStr(DevicePath);
> +  DevPathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
>if (DevPathStr != NULL) {
>  DEBUG((
>EFI_D_INFO,
> @@ -229,7 +229,7 @@ Returns:
>//
>// Print Device Path
>//
> -  DevPathStr = DevicePathToStr(DevicePath);
> +  DevPathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
>if (DevPathStr != NULL) {
>  DEBUG((
>EFI_D_INFO,
> @@ -940,7 +940,7 @@ ConnectRecursivelyIfPciMassStorage (
>  //
>  // Print Device Path
>  //
> -DevPathStr = DevicePathToStr (DevicePath);
> +DevPathStr = ConvertDevicePathToText (DevicePath, FALSE, FALSE);
>  if (DevPathStr != NULL) {
>DEBUG((
>  EFI_D_INFO,
> diff --git 
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 18cd987..00a7583 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -46,6 +46,7 @@ [LibraryClasses]
>DebugLib
>PcdLib
>UefiBootManagerLib
> +  DevicePathLib
>PciLib
>NvVarsFileLib
>QemuFwCfgLib
> 

The DevicePathToStr() function (in
"IntelFrameworkModulePkg/Library/GenericBdsLib/DevicePath.c") is a
simple wrapper around ConvertDevicePathToText().

DevicePathToStr() passes DisplayOnly=TRUE and AllowShortcuts=TRUE to
ConvertDevicePathToText(), whereas in this patch, both parameters are
flipped to FALSE.

The formatted devpaths are used only for debugging purposes, so this
change is safe.

... Can you please add the above three paragraphs to the commit message?
Then:

Reviewed-by: Laszlo Ersek 

(BTW, I too prefer the full textual rendering.)

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

Re: [edk2] [Patch v3 09/23] OvmfPkg/PlatformBds: link to UefiBootManagerLib

[Laszlo Ersek]

On 04/21/16 08:57, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h  | 2 +-
>  OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> index cb72596..3234066 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> @@ -39,7 +39,7 @@ Abstract:
>  #include 
>  #include 
>  #include 
> -#include 
> +#include 
>  #include 
>  #include 
>  #include 
> diff --git 
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 8cbbd12..18cd987 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -45,7 +45,7 @@ [LibraryClasses]
>BaseMemoryLib
>DebugLib
>PcdLib
> -  GenericBdsLib
> +  UefiBootManagerLib
>PciLib
>NvVarsFileLib
>QemuFwCfgLib
> 

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

Re: [edk2] [Patch v3 08/23] OvmfPkg/PlatformBds: use EfiBootManagerUpdateConsoleVariable

[Laszlo Ersek]

On 04/21/16 08:57, Ruiyu Ni wrote:
> Call EfiBootManagerUpdateConsoleVariable in UefiBootManagerLib
> instead of BdsLibUpdateConsoleVariable in GenericBdsLib.
> 
> Still cannot pass build.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 32 
> +++---
>  1 file changed, 16 insertions(+), 16 deletions(-)

I see that the macros VarConsoleOut, VarConsoleInp, etc, from
"BdsPlatform.h", remain in use after this patch (until patch #11), so
not removing them in this patch is fine.

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 53b277d..dbbc579 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -185,7 +185,7 @@ Returns:
>//
>DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL 
> *));
>  
> -  BdsLibUpdateConsoleVariable (VarConsoleInp, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
>  
>//
>// Register COM1
> @@ -212,9 +212,9 @@ Returns:
>  FreePool(DevPathStr);
>}
>  
> -  BdsLibUpdateConsoleVariable (VarConsoleOut, DevicePath, NULL);
> -  BdsLibUpdateConsoleVariable (VarConsoleInp, DevicePath, NULL);
> -  BdsLibUpdateConsoleVariable (VarErrorOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
>  
>//
>// Register COM2
> @@ -241,9 +241,9 @@ Returns:
>  FreePool(DevPathStr);
>}
>  
> -  BdsLibUpdateConsoleVariable (VarConsoleOut, DevicePath, NULL);
> -  BdsLibUpdateConsoleVariable (VarConsoleInp, DevicePath, NULL);
> -  BdsLibUpdateConsoleVariable (VarErrorOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
>  
>return EFI_SUCCESS;
>  }
> @@ -323,8 +323,8 @@ GetGopDevicePath (
>  // Delete the PCI device's path that added by 
> GetPlugInPciVgaDevicePath()
>  // Add the integrity GOP device path.
>  //
> -BdsLibUpdateConsoleVariable (VarConsoleOutDev, NULL, PciDevicePath);
> -BdsLibUpdateConsoleVariable (VarConsoleOutDev, TempDevicePath, NULL);
> +EfiBootManagerUpdateConsoleVariable (ConOutDev, NULL, PciDevicePath);
> +EfiBootManagerUpdateConsoleVariable (ConOutDev, TempDevicePath, 
> NULL);
>}
>  }
>  gBS->FreePool (GopHandleBuffer);
> @@ -373,7 +373,7 @@ Returns:
>GetGopDevicePath (DevicePath, );
>DevicePath = GopDevicePath;
>  
> -  BdsLibUpdateConsoleVariable (VarConsoleOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
>  
>return EFI_SUCCESS;
>  }
> @@ -416,9 +416,9 @@ Returns:
>DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL 
> *));
>DevicePath = AppendDevicePathNode (DevicePath, (EFI_DEVICE_PATH_PROTOCOL 
> *));
>  
> -  BdsLibUpdateConsoleVariable (VarConsoleOut, DevicePath, NULL);
> -  BdsLibUpdateConsoleVariable (VarConsoleInp, DevicePath, NULL);
> -  BdsLibUpdateConsoleVariable (VarErrorOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConOut, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ConIn, DevicePath, NULL);
> +  EfiBootManagerUpdateConsoleVariable (ErrOut, DevicePath, NULL);
>  
>return EFI_SUCCESS;
>  }
> @@ -680,13 +680,13 @@ Returns:
>// Update the console variable with the connect type
>//
>if ((PlatformConsole[Index].ConnectType & CONSOLE_IN) == CONSOLE_IN) {
> -BdsLibUpdateConsoleVariable (VarConsoleInp, 
> PlatformConsole[Index].DevicePath, NULL);
> +EfiBootManagerUpdateConsoleVariable (ConIn, 
> PlatformConsole[Index].DevicePath, NULL);
>}
>if ((PlatformConsole[Index].ConnectType & CONSOLE_OUT) == CONSOLE_OUT) 
> {
> -BdsLibUpdateConsoleVariable (VarConsoleOut, 
> PlatformConsole[Index].DevicePath, NULL);
> +EfiBootManagerUpdateConsoleVariable (ConOut, 
> PlatformConsole[Index].DevicePath, NULL);
>}
>if ((PlatformConsole[Index].ConnectType & STD_ERROR) == STD_ERROR) {
> -BdsLibUpdateConsoleVariable (VarErrorOut, 
> PlatformConsole[Index].DevicePath, NULL);
> +EfiBootManagerUpdateConsoleVariable (ErrOut, 
> PlatformConsole[Index].DevicePath, NULL);
>}
>  }
>} else {
> 

___
edk2-devel mailing list
edk2-devel@lists.01.org

[edk2] [PATCH] ArmPlatformPkg: implement CpuIo2 protocol driver specific for PCI

[Ard Biesheuvel]

The CpuIo2 protocol is required by the generic PciHostBridgeDxe driver,
which relies on it to back its own I/O and MMIO operations.

Since ARM has no native I/O port equivalent, such accesses can only originate
from PCI drivers, and the PCI I/O space is translated to MMIO in this case.

So we can implement this protocol using MMIO operations only, and take the
PCI I/O translation offset into account when performing I/O port accesses.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel 
---

We will need this on Seattle to support PCI devices with I/O BARs when we move
to the generic PciHostBridgeDxe driver.

 ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c   | 559 
 ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.inf |  53 ++
 2 files changed, 612 insertions(+)

diff --git a/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c 
b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
new file mode 100644
index ..fecf6a87adce
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/CpuIo2Dxe/ArmPciCpuIo2Dxe.c
@@ -0,0 +1,559 @@
+/** @file
+  Produces the CPU I/O 2 Protocol.
+
+Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+Copyright (c) 2016, Linaro Ltd. All rights reserved.
+
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD 
License
+which accompanies this distribution.  The full text of the license may be 
found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include 
+
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MAX_IO_PORT_ADDRESS   0x
+
+//
+// Handle for the CPU I/O 2 Protocol
+//
+STATIC EFI_HANDLE  mHandle = NULL;
+
+//
+// Lookup table for increment values based on transfer widths
+//
+STATIC CONST UINT8 mInStride[] = {
+  1, // EfiCpuIoWidthUint8
+  2, // EfiCpuIoWidthUint16
+  4, // EfiCpuIoWidthUint32
+  8, // EfiCpuIoWidthUint64
+  0, // EfiCpuIoWidthFifoUint8
+  0, // EfiCpuIoWidthFifoUint16
+  0, // EfiCpuIoWidthFifoUint32
+  0, // EfiCpuIoWidthFifoUint64
+  1, // EfiCpuIoWidthFillUint8
+  2, // EfiCpuIoWidthFillUint16
+  4, // EfiCpuIoWidthFillUint32
+  8  // EfiCpuIoWidthFillUint64
+};
+
+//
+// Lookup table for increment values based on transfer widths
+//
+STATIC CONST UINT8 mOutStride[] = {
+  1, // EfiCpuIoWidthUint8
+  2, // EfiCpuIoWidthUint16
+  4, // EfiCpuIoWidthUint32
+  8, // EfiCpuIoWidthUint64
+  1, // EfiCpuIoWidthFifoUint8
+  2, // EfiCpuIoWidthFifoUint16
+  4, // EfiCpuIoWidthFifoUint32
+  8, // EfiCpuIoWidthFifoUint64
+  0, // EfiCpuIoWidthFillUint8
+  0, // EfiCpuIoWidthFillUint16
+  0, // EfiCpuIoWidthFillUint32
+  0  // EfiCpuIoWidthFillUint64
+};
+
+/**
+  Check parameters to a CPU I/O 2 Protocol service request.
+
+  The I/O operations are carried out exactly as requested. The caller is 
responsible
+  for satisfying any alignment and I/O width restrictions that a PI System on a
+  platform might require. For example on some platforms, width requests of
+  EfiCpuIoWidthUint64 do not work. Misaligned buffers, on the other hand, will
+  be handled by the driver.
+
+  @param[in] MmioOperation  TRUE for an MMIO operation, FALSE for I/O Port 
operation.
+  @param[in] Width  Signifies the width of the I/O or Memory operation.
+  @param[in] AddressThe base address of the I/O operation.
+  @param[in] Count  The number of I/O operations to perform. The 
number of
+bytes moved is Width size * Count, starting at 
Address.
+  @param[in] Buffer For read operations, the destination buffer to 
store the results.
+For write operations, the source buffer from which 
to write data.
+
+  @retval EFI_SUCCESSThe parameters for this request pass the 
checks.
+  @retval EFI_INVALID_PARAMETER  Width is invalid for this PI system.
+  @retval EFI_INVALID_PARAMETER  Buffer is NULL.
+  @retval EFI_UNSUPPORTEDThe Buffer is not aligned for the given Width.
+  @retval EFI_UNSUPPORTEDThe address range specified by Address, Width,
+ and Count is not valid for this PI system.
+
+**/
+STATIC
+EFI_STATUS
+CpuIoCheckParameter (
+  IN BOOLEANMmioOperation,
+  IN EFI_CPU_IO_PROTOCOL_WIDTH  Width,
+  IN UINT64 Address,
+  IN UINTN  Count,
+  IN VOID   *Buffer
+  )
+{
+  UINT64  MaxCount;
+  UINT64  Limit;
+
+  //
+  // Check to see if Buffer is NULL
+  //
+  if (Buffer == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Check to see if Width is in the valid range
+  //
+  if ((UINT32)Width >= EfiCpuIoWidthMaximum) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // For FIFO type, the target address 

Re: [edk2] [Patch v3 07/23] OvmfPkg/PlatformBds: Follow PlatformBootManagerLib interfaces

[Laszlo Ersek]

On 04/21/16 08:57, Ruiyu Ni wrote:
> Change the function name to follow new library class
> PlatformBootManagerLib interfaces.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> ---
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 42 
> ++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  3 +-
>  .../PlatformBootManagerLib.inf |  8 ++---
>  3 files changed, 24 insertions(+), 29 deletions(-)

I commented on this patch (in advance) while reviewing patch #5. Beyond
those requests, I have one question:

> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 0bc02ba..53b277d 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -1,7 +1,7 @@
>  /** @file
>Platform BDS customizations.
>  
> -  Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.
> +  Copyright (c) 2004 - 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
> @@ -93,7 +93,7 @@ InstallDevicePathCallback (
>  //
>  VOID
>  EFIAPI
> -PlatformBdsInit (
> +PlatformBootManagerBeforeConsole (
>VOID
>)
>  /*++
> @@ -111,7 +111,7 @@ Returns:
>  
>  --*/
>  {
> -  DEBUG ((EFI_D_INFO, "PlatformBdsInit\n"));
> +  DEBUG ((EFI_D_INFO, "PlatformBootManagerBeforeConsole\n"));
>InstallDevicePathCallback ();
>  }
>  
> @@ -1207,11 +1207,8 @@ SaveS3BootScript (
>  
>  VOID
>  EFIAPI
> -PlatformBdsPolicyBehavior (
> -  IN OUT LIST_ENTRY  *DriverOptionList,
> -  IN OUT LIST_ENTRY  *BootOptionList,
> -  IN PROCESS_CAPSULESProcessCapsules,
> -  IN BASEM_MEMORY_TEST   BaseMemoryTest
> +PlatformBootManagerAfterConsole (
> +  VOID
>)
>  /*++
>  
> @@ -1221,26 +1218,12 @@ Routine Description:
>is driven by boot mode. IBV/OEM can customize this code for their specific
>policy action.
>  
> -Arguments:
> -
> -  DriverOptionList - The header of the driver option link list
> -
> -  BootOptionList   - The header of the boot option link list
> -
> -  ProcessCapsules  - A pointer to ProcessCapsules()
> -
> -  BaseMemoryTest   - A pointer to BaseMemoryTest()
> -
> -Returns:
> -
> -  None.
> -
>  --*/
>  {
>EFI_STATUS Status;
>EFI_BOOT_MODE  BootMode;
>  
> -  DEBUG ((EFI_D_INFO, "PlatformBdsPolicyBehavior\n"));
> +  DEBUG ((EFI_D_INFO, "PlatformBootManagerAfterConsole\n"));
>  
>VisitAllInstancesOfProtocol (,
>  ConnectRootBridge, NULL);
> @@ -1554,6 +1537,19 @@ InstallDevicePathCallback (
>  }
>  
>  /**
> +  This function is called each second during the boot manager waits the 
> timeout.
> +
> +  @param TimeoutRemain  The remaining timeout.
> +**/
> +VOID
> +EFIAPI
> +PlatformBootManagerWaitCallback (
> +  UINT16  TimeoutRemain
> +  )
> +{
> +}

I located the call to this function in
"MdeModulePkg/Universal/BdsDxe/BdsEntry.c", function BdsWait().

Am I correct to think that with MdeModulePkg BDS, we will have no
progress bar (unlike with IntelFrameworkModulePkg BDS)? That is, unless
we implement one ourselves, in PlatformBootManagerWaitCallback()?

If so: I don't think it's a big deal (definitely not a "deal breaker"
for this series), but the disappearance of the progress bar should be
mentioned in the commit message.

The patch looks good otherwise (with my comments in patch #5 addressed).

Thanks!
Laszlo

> +
> +/**
>Lock the ConsoleIn device in system table. All key
>presses will be ignored until the Password is typed in. The only way to
>disable the password is to type it in to a ConIn device.
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h 
> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> index 6ba0d48..cb72596 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h
> @@ -1,7 +1,7 @@
>  /** @file
>Platform BDS customizations include file.
>  
> -  Copyright (c) 2006 - 2007, 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
> @@ -40,7 +40,6 @@ Abstract:
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>  #include 
> diff --git 
> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
> 

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Blank Field]

> Yes, they are /var/lib/libvirt/qemu/nvram/${vmname}_VARS.fd.
So the VM is(should be) configured properly - one r/o .fd with the
OVMF_CODE, one r/w .fd with vars per VM.

> Your NvVars file could be very old, maybe a remnant of your earlier
Will check the date, but i doubt about this. Maybe there was no problem at
all and i just need to create a normal BootOrder entry.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Laszlo Ersek]

On 04/27/16 12:43, Blank Field wrote:
>> Because your VM configuration is incorrect (you didn't configure two
>> pflash chips, one for storing the UEFI variables, another (r/o) for
>> presenting the firmware binary). Hence OVMF fell back to the historical
>> hack where it stored (some of) the UEFI variables in a file called
>> NvVars on the ESP.

> I thought libvirt creates a separate $vmname-VARS.fd file somewhere in
> in /var/lib/libvirt... when using ovmf properly.

Yes, they are /var/lib/libvirt/qemu/nvram/${vmname}_VARS.fd.

> That is strange if it doesn't, because i remember it did. Will check that.
> That way it won't touch the host system's efivars (it does not anyway)
> nor store the efivars on the ESP at all and i'd be able to create a new
> BootOrder entry. Right?

Your NvVars file could be very old, maybe a remnant of your earlier
experiments.

>> No, that's not right.
> What if qemu just doesn't respond to SIGTERM for some bizarre reason and
> libvirt is too shy to do SIGKILL?

Libvirt is not too shy; see the "--graceful" option in the documentation
of "virsh destroy" (--graceful is not the default).

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

Re: [edk2] OVMF broken under Xen (in PCI initialisation)

[Laszlo Ersek]

On 04/27/16 11:50, Ni, Ruiyu wrote:
> Copying Mike.
> 
> Regards,
> Ray
> 
>> -Original Message-
>> From: Ni, Ruiyu
>> Sent: Wednesday, April 27, 2016 5:49 PM
>> To: 'Gary Lin' 
>> Cc: edk2-devel@lists.01.org; Xen Devel ; Laszlo 
>> Ersek 
>> Subject: RE: [edk2] OVMF broken under Xen (in PCI initialisation)
>>
>> Gary,
>> Thanks for the try.
>>
>> I now understand why the issue happens.
>> 1. PciEnumeratorLight() depends on the MinBus returned from
>> PciRootBridgeIo.Configuration().
>> 2. PciRootBridgeIo.Configuration() returns the resource descriptors
>> initialized by PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources).
>> 3. PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
>> is not called when PcdPciDisableBusEnumeration is TRUE
>>
>> I am thinking about let PciRootBridgeIo.Configuration() returns
>> the correct resource consumption information even when
>> PcdPciDisableBusEnumeration is TRUE.
>>
>> I can add a new field to PCI_ROOT_BRIDGE structure defined in
>> MdeModulePkg/Include/Library/PciHostBridgeLib.h to indicate
>> that the resources filled in PCI_ROOT_BRIDGE.Bus/Io/Mem/
>> MemAbove4G/PMem/PMemAbove4G are already assigned
>> to PCI bus. So that PciRootBridgeIo.Configuration() can return
>> these value even when
>> PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
>> is not called due to PcdPciDisableBusEnumeration is TRUE.

Can you use PcdPciDisableBusEnumeration directly for this?

(If you don't like the idea, we could certainly set the new field in
PCI_ROOT_BRIDGE from the PCD too, in OvmfPkg/Library/PciHostBridgeLib.)

>> Do you know whether Xen passes the PCI device resource
>> information to firmware?

I don't think so, no.

But, given that the previous PciHostBridgeDxe driver was working on Xen,
can we perhaps emulate that behavior in
"OvmfPkg/Library/PciHostBridgeLib" somehow?

Do you think that step (2) above behaves differently between the old and
the new PCI host bridge driver? (Steps (1) and (3) should be identical,
they are initiated by the PCI Bus driver.)

Thanks
Laszlo

>>
>> Copying Laszlo.
>>
>> Regards,
>> Ray
>>
>>> -Original Message-
>>> From: Gary Lin [mailto:g...@suse.com]
>>> Sent: Wednesday, April 27, 2016 4:27 PM
>>> To: Ni, Ruiyu 
>>> Cc: edk2-devel@lists.01.org; Xen Devel 
>>> Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>>>
>>> On Wed, Apr 27, 2016 at 07:18:21AM +, Ni, Ruiyu wrote:
 Gary,
 In PciEnumeratorLight(), please directly assign 0 to MinBus when
 Configuration() returns error, instead of calling PciGetBusRange() to
 extract MinBus from the descriptors returned from Configuration().

>>> OK, I ignored PciGetBusRange() and the check of the return status of
>>> Configuration(). Since MinBus is already initialized, I don't bother to
>>> change the value anyway. The change is attached.
>>>
>>> And yes, this time the shell showed.
>>>
>>> Thanks,
>>>
>>> Gary Lin
>>>
 Regards,
 Ray

> -Original Message-
> From: Gary Lin [mailto:g...@suse.com]
> Sent: Wednesday, April 27, 2016 2:54 PM
> To: Ni, Ruiyu 
> Cc: edk2-devel@lists.01.org; Xen Devel 
> Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>
> On Wed, Apr 27, 2016 at 05:39:40AM +, Ni, Ruiyu wrote:
>>
>>
>> Regards,
>> Ray
>>
>>> -Original Message-
>>> From: Gary Lin [mailto:g...@suse.com]
>>> Sent: Wednesday, April 27, 2016 12:29 PM
>>> To: Ni, Ruiyu 
>>> Cc: edk2-devel@lists.01.org; Xen Devel 
>>> Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>>>
>>> On Tue, Apr 26, 2016 at 09:40:42AM +, Ni, Ruiyu wrote:
 Gary,
 I can reproduce the issue and have debugged to get the reason.
 In MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c:
 PciEnumeratorLight() calls PciRootBridgeIo->Configuration()
 while the Configuration returns EFI_UNSUPPORTED resulting
 the PciBus driver exits earlier.
 You could try to manually set MinBus to 0 and MaxBus to 0xFF.

>>> Do you mean to set MinBus and MaxBus in PciGetBusRange() ?
>>> Should I also keep the EFI_UNSUPPORTED code in Configuration() ?
>> Keep returning EFI_UNSUPPORTED in Configuration() and set
>> MinBus/MaxBus in PciGetBusRange().
>>
>>>
 The second change you need to make is in PciIo.c:
 Change GetMmioAddressTranslationOffset() to return 0 instead
 of -1 when error occurs

>>>
>>> Should I also remove "ASSERT (FALSE);" ?
>>
>>
>> ASSERT() won't be hit.
>>
> It didn't work (see my attached patch) :(
> BTW, if Configuration() already returns EFI_UNSUPPORTED, then there is
> no 

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Laszlo Ersek]

On 04/27/16 11:52, Blank Field wrote:

> And i've thought that efivars on a physical platform are located
> somewhere in a separate chip on the motherboard. Why i've found it on
> the ESP?

Because your VM configuration is incorrect (you didn't configure two
pflash chips, one for storing the UEFI variables, another (r/o) for
presenting the firmware binary). Hence OVMF fell back to the historical
hack where it stored (some of) the UEFI variables in a file called
NvVars on the ESP.

Since your ESP was a physical one, your VM created the NvVars file on
the physical ESP.

Your motherboard most definitely has a physical flash chip (or maybe
several of them). Also, if you configure your OVMF VM correctly, it will
have to virtual pflash chips as well (and it won't use the NvVars file
hack).

>> SMM is not that far ahead in OVMF, and it's
>> pretty much out of scope as well, IMO.
> I've see  a lot of patches and even an entirely new branch.

The patches you may have seen are now upstream; OVMF does support SMM to
an extent (see OvmfPkg/README, section "SMM support"). The scope of SMM
support in OVMF is limited by design / deliberately; the README file
describes it. (The OVMF whitepaper, dated before SMM, also speaks about
what SMM would be / should be used for.)

So, SMM is not generally out of scope. It is the handling of ACPI events
in SMM that is out of scope for OVMF.

> I should manually connect to the qemu monitor and then send the
> system_poweroff command to see if it is qemu's fault or not.

No, that's not right.

First, the command is called "system_powerdown" (in the human monitor),
not "system_poweroff".

Second, "system_powerdown" is precisely the one that does the ACPI thing
(set the PM event status bit, raise the SCI, let the guest act upon it).
It is not a forcible poweroff (which is akin to yanking the power cable).

For that, you need "quit" (HMP), or "virsh destroy" (makes libvirtd send
a SIGTERM to QEMU), or select "Force Off" in virt-manager (should be the
same as "virsh destroy").

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

Re: [edk2] [Patch v3 00/23] Use MdeModulePkg/BDS in OVMF platform

[Laszlo Ersek]

On 04/27/16 11:53, Michael Zimmermann wrote:
> well all ARM targets are still using Intel's BDS and iirc this even is
> the "new" one for them because previously they were using the ARM bds.

Once this series converges for OvmfPkg, we intend to port ArmVirtPkg as
well, using the lessons learned here.

Thanks
Laszlo

> On Wed, Apr 27, 2016 at 3:32 AM, Andrew Fish  > wrote:
> 
> 
> > On Apr 26, 2016, at 8:58 AM, Laszlo Ersek  > wrote:
> >
> > On 04/26/16 17:38, Michael Zimmermann wrote:
> >> is Intel's bds deprecated now
> >
> > Yes, it is. I seem to remember that Ray said he wouldn't implement new
> > UEFI spec features for the BDS driver in IntelFrameworkModulePkg, only
> > the new one in MdeModulePkg.
> >
> > IIRC the main reason for the new BDS driver (in MdeModulePkg) is better
> > separation of responsibilities -- there is now a BDS driver providing
> > the BDS arch protocol, and a standalone UEFI application that is
> > responsible for UI etc. As far as I recall anyway.
> >
> 
> The IntelFrameworkModulePkg/IntelFrameworkPkg use definitions from
> the Intel(r) Platform Innovation Framework for EFI set of
> specifications, and I don't think those specs have been updated
> since 2004-2005 ish? Most of that functionality was moved over to
> the UEFI Forum owned Platform Initialization (PI) specification. The
> MdePkg/MdeModulePkg support the industry standards and the UEFI
> Forum specifications. So it makes sense the Intel Framework stuff is
> hitting end of support.
> 
> Thanks,
> 
> Andrew Fish
> 
> > Thanks
> > Laszlo
> >
> >> or what is this switch about?
> >>
> >> Michael
> >>
> >> On Thu, Apr 21, 2016 at 9:03 AM, Ni, Ruiyu  > wrote:
> >>
> >>> The changes set can be found in below GIT repos:
> >>> https://github.com/niruiyu/edk2/tree/Ovmf_Bds2
> >>>
> >>>
> >>> Regards,
> >>> Ray
> >>>
>  -Original Message-
>  From: Ni, Ruiyu
>  Sent: Thursday, April 21, 2016 2:58 PM
>  To: edk2-devel@lists.01.org 
>  Cc: Ni, Ruiyu >
>  Subject: [Patch v3 00/23] Use MdeModulePkg/BDS in OVMF platform
> 
>  The patch serials creates a flag USE_OLD_BDS and by default the
> value
>  of the flag is FALSE so that the new MdeModulePkg/BDS is used.
>  User can define USE_OLD_BDS as TRUE to force to use
> >>> IntelFrameworkModulePkg
>  /BDS.
> 
>  The v3 adopts comments for v1 and v2 to split the big changes to
>  small changes and also expose the
> EfiBootManagerGetLoadOptionBuffer().
> 
>  Ruiyu Ni (23):
>  MdeModulePkg/UefiBootManagerLib: Expose *GetLoadOptionBuffer() API
>  OvmfPkg/PlatformPei: Add memory above 4GB as tested
>  OvmfPkg: Duplicate QemuBootOrderLib to QemuNewBootOrderLib
>  OvmfPkg/QemuNewBootOrderLib: Build with UefiBootManagerLib
>  OvmfPkg: Duplicate PlatformBdsLib to PlatformBootManagerLib
>  OvmfPkg/PlatformBds: Rename INF file
>  OvmfPkg/PlatformBds: Follow PlatformBootManagerLib interfaces
>  OvmfPkg/PlatformBds: use EfiBootManagerUpdateConsoleVariable
>  OvmfPkg/PlatformBds: link to UefiBootManagerLib
>  OvmfPkg/PlatformBds: Use ConvertDevicePathToText in DevicePathLib
>  OvmfPkg/PlatformBds: Initialize console variables in
> *BeforeConsole()
>  OvmfPkg/PlatformBds: Do not launch Boot Manager Menu
>  OvmfPkg/PlatformBds: Register boot options and hot keys.
>  OvmfPkg/PlatformBds: Remove unused local functions.
>  OvmfPkg/PlatformBds: Change PlatformBdsConnectSequence()
>  OvmfPkg/PlatformBds: Use EfiBootManagerRefreshAllBootOption()
>  OvmfPkg/PlatformBds: Remove PlatformBdsGetDriverOption()
>  OvmfPkg/PlatformBds: Use GetBootModeHob() in HobLib
>  OvmfPkg/PlatformBds: Remove unnecessary memory test
>  OvmfPkg/PlatformBds: Remove unused variables and function
> prototypes.
>  OvmfPkg/PlatformBds: Add EnableQuietBoot and DisableQuietBoot
>  OvmfPkg/PlatformBds: Remove unused C structures definitions.
>  OvmfPkg: Use MdeModulePkg/BDS
> 
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |   23 +-
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   |   11 +-
>  .../Library/UefiBootManagerLib/BmLoadOption.c  |2 +-
>  .../Library/UefiBootManagerLib/InternalBm.h|   19 -
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1371
> ++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 +++
> 

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Blank Field]

>Since the ESP and the main partitions are also accessible from the
host(which would be almost impossible to achieve if the VM would be stored
in a file)...
...i was able to boot it bare-metal.
Sorry, lost a bit of text due to small screen size on a mobile client.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch v3 00/23] Use MdeModulePkg/BDS in OVMF platform

[Michael Zimmermann]

well all ARM targets are still using Intel's BDS and iirc this even is the
"new" one for them because previously they were using the ARM bds.

Michael

On Wed, Apr 27, 2016 at 3:32 AM, Andrew Fish  wrote:

>
> > On Apr 26, 2016, at 8:58 AM, Laszlo Ersek  wrote:
> >
> > On 04/26/16 17:38, Michael Zimmermann wrote:
> >> is Intel's bds deprecated now
> >
> > Yes, it is. I seem to remember that Ray said he wouldn't implement new
> > UEFI spec features for the BDS driver in IntelFrameworkModulePkg, only
> > the new one in MdeModulePkg.
> >
> > IIRC the main reason for the new BDS driver (in MdeModulePkg) is better
> > separation of responsibilities -- there is now a BDS driver providing
> > the BDS arch protocol, and a standalone UEFI application that is
> > responsible for UI etc. As far as I recall anyway.
> >
>
> The IntelFrameworkModulePkg/IntelFrameworkPkg use definitions from the
> Intel(r) Platform Innovation Framework for EFI set of specifications, and I
> don't think those specs have been updated since 2004-2005 ish? Most of that
> functionality was moved over to the UEFI Forum owned Platform
> Initialization (PI) specification. The MdePkg/MdeModulePkg support the
> industry standards and the UEFI Forum specifications. So it makes sense the
> Intel Framework stuff is hitting end of support.
>
> Thanks,
>
> Andrew Fish
>
> > Thanks
> > Laszlo
> >
> >> or what is this switch about?
> >>
> >> Michael
> >>
> >> On Thu, Apr 21, 2016 at 9:03 AM, Ni, Ruiyu  wrote:
> >>
> >>> The changes set can be found in below GIT repos:
> >>> https://github.com/niruiyu/edk2/tree/Ovmf_Bds2
> >>>
> >>>
> >>> Regards,
> >>> Ray
> >>>
>  -Original Message-
>  From: Ni, Ruiyu
>  Sent: Thursday, April 21, 2016 2:58 PM
>  To: edk2-devel@lists.01.org
>  Cc: Ni, Ruiyu 
>  Subject: [Patch v3 00/23] Use MdeModulePkg/BDS in OVMF platform
> 
>  The patch serials creates a flag USE_OLD_BDS and by default the value
>  of the flag is FALSE so that the new MdeModulePkg/BDS is used.
>  User can define USE_OLD_BDS as TRUE to force to use
> >>> IntelFrameworkModulePkg
>  /BDS.
> 
>  The v3 adopts comments for v1 and v2 to split the big changes to
>  small changes and also expose the EfiBootManagerGetLoadOptionBuffer().
> 
>  Ruiyu Ni (23):
>  MdeModulePkg/UefiBootManagerLib: Expose *GetLoadOptionBuffer() API
>  OvmfPkg/PlatformPei: Add memory above 4GB as tested
>  OvmfPkg: Duplicate QemuBootOrderLib to QemuNewBootOrderLib
>  OvmfPkg/QemuNewBootOrderLib: Build with UefiBootManagerLib
>  OvmfPkg: Duplicate PlatformBdsLib to PlatformBootManagerLib
>  OvmfPkg/PlatformBds: Rename INF file
>  OvmfPkg/PlatformBds: Follow PlatformBootManagerLib interfaces
>  OvmfPkg/PlatformBds: use EfiBootManagerUpdateConsoleVariable
>  OvmfPkg/PlatformBds: link to UefiBootManagerLib
>  OvmfPkg/PlatformBds: Use ConvertDevicePathToText in DevicePathLib
>  OvmfPkg/PlatformBds: Initialize console variables in *BeforeConsole()
>  OvmfPkg/PlatformBds: Do not launch Boot Manager Menu
>  OvmfPkg/PlatformBds: Register boot options and hot keys.
>  OvmfPkg/PlatformBds: Remove unused local functions.
>  OvmfPkg/PlatformBds: Change PlatformBdsConnectSequence()
>  OvmfPkg/PlatformBds: Use EfiBootManagerRefreshAllBootOption()
>  OvmfPkg/PlatformBds: Remove PlatformBdsGetDriverOption()
>  OvmfPkg/PlatformBds: Use GetBootModeHob() in HobLib
>  OvmfPkg/PlatformBds: Remove unnecessary memory test
>  OvmfPkg/PlatformBds: Remove unused variables and function prototypes.
>  OvmfPkg/PlatformBds: Add EnableQuietBoot and DisableQuietBoot
>  OvmfPkg/PlatformBds: Remove unused C structures definitions.
>  OvmfPkg: Use MdeModulePkg/BDS
> 
>  MdeModulePkg/Include/Library/UefiBootManagerLib.h  |   23 +-
>  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c   |   11 +-
>  .../Library/UefiBootManagerLib/BmLoadOption.c  |2 +-
>  .../Library/UefiBootManagerLib/InternalBm.h|   19 -
>  .../Library/PlatformBootManagerLib/BdsPlatform.c   | 1371
> ++
>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  246 +++
>  .../PlatformBootManagerLib.inf |   80 +
>  .../Library/PlatformBootManagerLib/PlatformData.c  |   41 +
>  .../Library/PlatformBootManagerLib/QemuKernel.c|  170 ++
>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c |  668 +++
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.c  |  313 
>  .../Library/QemuNewBootOrderLib/ExtraRootBusMap.h  |   40 +
>  .../Library/QemuNewBootOrderLib/QemuBootOrderLib.c | 1913
> >>> 
>  .../QemuNewBootOrderLib/QemuBootOrderLib.inf   |   68 +
>  OvmfPkg/OvmfPkgIa32.dsc|   45 +-
>  OvmfPkg/OvmfPkgIa32.fdf|5 +
> 

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Blank Field]

> I don't know what "windows exVM" means.
It is my fault not giving enough backstory, sorry.
The windows system was previously installed in a VM using host disk as a
storage.
Since the ESP and the main partitions are also accessible from the
host(which would be almost impossible to achieve if the VM would be stored
in a file).
> Also it's not clear if your host side F23 installation (the fresh one)
> is identical to "your fedora" that is booting in "pure EFI" -- is that a
> separate Fedora guest, or your host OS?
There are just two OSes installed, the fresh host fedora 23 installed
without CSM, booting from ESP on the harddrive with the root partition on
another disk, and the Windows, booting from the same ESP, which i am trying
to boot inside the VM.
> Ugh... I don't know why people are attracted to this (i.e., sharing
> system disk(s) between a physically installed OS and some guests). What
> is wrong with keeping your VM disk images in normal host-side files?
It is fragile, but it's the simplest way to be able to boot the VM on a
hardware platform for various comparisons, for example benchmarks. Also,
that way it is possible to debug buggy device drivers - i am fine about
resetting the hung up VM.
> Well, you have a UEFI variable store (with BootOrder and Boot in it)
> on your physical machine, and you have another one (an independent one,
> with separate BootOrder and Boot) in your virtual machine. However,
> the EFI system partition whose files the Boot variables point to is
> shared. Not good.
So, i should separate ESPs and that way i'll also separate the the EFIVARS.
To make a normal BootOrder i must either create a new one manually or copy
the entries via UEFIShell.
> I do know. Stop sharing your ESP between the host OS and guests.
Can i somehow override the choosing of EFIVARS? Make the VM somehow ignore
the storage located on ESP.
And i've thought that efivars on a physical platform are located somewhere
in a separate chip on the motherboard. Why i've found it on the ESP?
> <
https://www.happyassassin.net/2014/01/25/uefi-boot-how-does-that-actually-work-then/
>
Didn't read yet, sorry.
> SMM is not that far ahead in OVMF, and it's
> pretty much out of scope as well, IMO.
I've see  a lot of patches and even an entirely new branch.
> in virt-manager, while OVMF is "at rest", qemu will simply power off the
> VM, forcibly (the guest has no chance to interfere), and when it's all
> done, emit a QMP event for libvirt to consume. That's how libvirt knows
> the VM has been powered down.
Thx for the explanation.
> I use this all the time. I don't know why it doesn't work for you --
> maybe you should configure libvirtd to log all QMP traffic (commands for
> QEMU, and replies and events from QEMU), and ask the libvirt developers
> what's up.
I should manually connect to the qemu monitor and then send the
system_poweroff command to see if it is qemu's fault or not.
> The settings I use for QMP and guest agent traffic capture are, in
> "/etc/libvirt/libvirtd.conf":
>
>   log_filters="1:qemu_monitor_ 1:qemu_agent"
>   log_outputs="1:file:/var/log/libvirt/libvirtd.log"
Thanks a lot.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] OVMF broken under Xen (in PCI initialisation)

[Ni, Ruiyu]

Copying Mike.

Regards,
Ray

>-Original Message-
>From: Ni, Ruiyu
>Sent: Wednesday, April 27, 2016 5:49 PM
>To: 'Gary Lin' 
>Cc: edk2-devel@lists.01.org; Xen Devel ; Laszlo Ersek 
>
>Subject: RE: [edk2] OVMF broken under Xen (in PCI initialisation)
>
>Gary,
>Thanks for the try.
>
>I now understand why the issue happens.
>1. PciEnumeratorLight() depends on the MinBus returned from
>PciRootBridgeIo.Configuration().
>2. PciRootBridgeIo.Configuration() returns the resource descriptors
>initialized by PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources).
>3. PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
>is not called when PcdPciDisableBusEnumeration is TRUE
>
>I am thinking about let PciRootBridgeIo.Configuration() returns
>the correct resource consumption information even when
>PcdPciDisableBusEnumeration is TRUE.
>
>I can add a new field to PCI_ROOT_BRIDGE structure defined in
>MdeModulePkg/Include/Library/PciHostBridgeLib.h to indicate
>that the resources filled in PCI_ROOT_BRIDGE.Bus/Io/Mem/
>MemAbove4G/PMem/PMemAbove4G are already assigned
>to PCI bus. So that PciRootBridgeIo.Configuration() can return
>these value even when
>PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
>is not called due to PcdPciDisableBusEnumeration is TRUE.
>
>Do you know whether Xen passes the PCI device resource
>information to firmware?
>
>Copying Laszlo.
>
>Regards,
>Ray
>
>>-Original Message-
>>From: Gary Lin [mailto:g...@suse.com]
>>Sent: Wednesday, April 27, 2016 4:27 PM
>>To: Ni, Ruiyu 
>>Cc: edk2-devel@lists.01.org; Xen Devel 
>>Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>>
>>On Wed, Apr 27, 2016 at 07:18:21AM +, Ni, Ruiyu wrote:
>>> Gary,
>>> In PciEnumeratorLight(), please directly assign 0 to MinBus when
>>> Configuration() returns error, instead of calling PciGetBusRange() to
>>> extract MinBus from the descriptors returned from Configuration().
>>>
>>OK, I ignored PciGetBusRange() and the check of the return status of
>>Configuration(). Since MinBus is already initialized, I don't bother to
>>change the value anyway. The change is attached.
>>
>>And yes, this time the shell showed.
>>
>>Thanks,
>>
>>Gary Lin
>>
>>> Regards,
>>> Ray
>>>
>>> >-Original Message-
>>> >From: Gary Lin [mailto:g...@suse.com]
>>> >Sent: Wednesday, April 27, 2016 2:54 PM
>>> >To: Ni, Ruiyu 
>>> >Cc: edk2-devel@lists.01.org; Xen Devel 
>>> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>>> >
>>> >On Wed, Apr 27, 2016 at 05:39:40AM +, Ni, Ruiyu wrote:
>>> >>
>>> >>
>>> >> Regards,
>>> >> Ray
>>> >>
>>> >> >-Original Message-
>>> >> >From: Gary Lin [mailto:g...@suse.com]
>>> >> >Sent: Wednesday, April 27, 2016 12:29 PM
>>> >> >To: Ni, Ruiyu 
>>> >> >Cc: edk2-devel@lists.01.org; Xen Devel 
>>> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>>> >> >
>>> >> >On Tue, Apr 26, 2016 at 09:40:42AM +, Ni, Ruiyu wrote:
>>> >> >> Gary,
>>> >> >> I can reproduce the issue and have debugged to get the reason.
>>> >> >> In MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c:
>>> >> >> PciEnumeratorLight() calls PciRootBridgeIo->Configuration()
>>> >> >> while the Configuration returns EFI_UNSUPPORTED resulting
>>> >> >> the PciBus driver exits earlier.
>>> >> >> You could try to manually set MinBus to 0 and MaxBus to 0xFF.
>>> >> >>
>>> >> >Do you mean to set MinBus and MaxBus in PciGetBusRange() ?
>>> >> >Should I also keep the EFI_UNSUPPORTED code in Configuration() ?
>>> >> Keep returning EFI_UNSUPPORTED in Configuration() and set
>>> >> MinBus/MaxBus in PciGetBusRange().
>>> >>
>>> >> >
>>> >> >> The second change you need to make is in PciIo.c:
>>> >> >> Change GetMmioAddressTranslationOffset() to return 0 instead
>>> >> >> of -1 when error occurs
>>> >> >>
>>> >> >
>>> >> >Should I also remove "ASSERT (FALSE);" ?
>>> >>
>>> >>
>>> >> ASSERT() won't be hit.
>>> >>
>>> >It didn't work (see my attached patch) :(
>>> >BTW, if Configuration() already returns EFI_UNSUPPORTED, then there is
>>> >no way to invoke PciGetBusRange(). Besides, MaxBus was never used in
>>> >PciEnumeratorLight(), so it's meaningless to assign any value to it.
>>> >
>>> >Cheers,
>>> >
>>> >Gary Lin
>>> >
>>> >> >
>>> >> >Cheers,
>>> >> >
>>> >> >Gary Lin
>>> >> >
>>> >> >> With the above 2 changes, can you check whether the system can
>>> >> >> boot?
>>> >> >>
>>> >> >> If it can boot, then we need to think about what the proper fix is.
>>> >> >> For OVMF above Xen, the PCI resources are assigned by Xen other than
>>> >> >> PciBus driver. So PciRootBridgeIo->Configuration() doesn't know
>>> >> >> the resource assignment information.
>>> >> >> With old PciHostBridge driver, the Configuration() returns BUS 
>>> >> >> resource
>>> >> >> descriptor with 

Re: [edk2] OVMF broken under Xen (in PCI initialisation)

[Ni, Ruiyu]

Gary,
Thanks for the try.

I now understand why the issue happens.
1. PciEnumeratorLight() depends on the MinBus returned from
PciRootBridgeIo.Configuration().
2. PciRootBridgeIo.Configuration() returns the resource descriptors
initialized by PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources).
3. PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
is not called when PcdPciDisableBusEnumeration is TRUE

I am thinking about let PciRootBridgeIo.Configuration() returns
the correct resource consumption information even when
PcdPciDisableBusEnumeration is TRUE.

I can add a new field to PCI_ROOT_BRIDGE structure defined in
MdeModulePkg/Include/Library/PciHostBridgeLib.h to indicate
that the resources filled in PCI_ROOT_BRIDGE.Bus/Io/Mem/
MemAbove4G/PMem/PMemAbove4G are already assigned
to PCI bus. So that PciRootBridgeIo.Configuration() can return
these value even when
PciHostBridge.NotifyPhase(EfiPciHostBridgeAllocateResources)
is not called due to PcdPciDisableBusEnumeration is TRUE. 

Do you know whether Xen passes the PCI device resource
information to firmware?

Copying Laszlo.

Regards,
Ray

>-Original Message-
>From: Gary Lin [mailto:g...@suse.com]
>Sent: Wednesday, April 27, 2016 4:27 PM
>To: Ni, Ruiyu 
>Cc: edk2-devel@lists.01.org; Xen Devel 
>Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>
>On Wed, Apr 27, 2016 at 07:18:21AM +, Ni, Ruiyu wrote:
>> Gary,
>> In PciEnumeratorLight(), please directly assign 0 to MinBus when
>> Configuration() returns error, instead of calling PciGetBusRange() to
>> extract MinBus from the descriptors returned from Configuration().
>>
>OK, I ignored PciGetBusRange() and the check of the return status of
>Configuration(). Since MinBus is already initialized, I don't bother to
>change the value anyway. The change is attached.
>
>And yes, this time the shell showed.
>
>Thanks,
>
>Gary Lin
>
>> Regards,
>> Ray
>>
>> >-Original Message-
>> >From: Gary Lin [mailto:g...@suse.com]
>> >Sent: Wednesday, April 27, 2016 2:54 PM
>> >To: Ni, Ruiyu 
>> >Cc: edk2-devel@lists.01.org; Xen Devel 
>> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>> >
>> >On Wed, Apr 27, 2016 at 05:39:40AM +, Ni, Ruiyu wrote:
>> >>
>> >>
>> >> Regards,
>> >> Ray
>> >>
>> >> >-Original Message-
>> >> >From: Gary Lin [mailto:g...@suse.com]
>> >> >Sent: Wednesday, April 27, 2016 12:29 PM
>> >> >To: Ni, Ruiyu 
>> >> >Cc: edk2-devel@lists.01.org; Xen Devel 
>> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
>> >> >
>> >> >On Tue, Apr 26, 2016 at 09:40:42AM +, Ni, Ruiyu wrote:
>> >> >> Gary,
>> >> >> I can reproduce the issue and have debugged to get the reason.
>> >> >> In MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c:
>> >> >> PciEnumeratorLight() calls PciRootBridgeIo->Configuration()
>> >> >> while the Configuration returns EFI_UNSUPPORTED resulting
>> >> >> the PciBus driver exits earlier.
>> >> >> You could try to manually set MinBus to 0 and MaxBus to 0xFF.
>> >> >>
>> >> >Do you mean to set MinBus and MaxBus in PciGetBusRange() ?
>> >> >Should I also keep the EFI_UNSUPPORTED code in Configuration() ?
>> >> Keep returning EFI_UNSUPPORTED in Configuration() and set
>> >> MinBus/MaxBus in PciGetBusRange().
>> >>
>> >> >
>> >> >> The second change you need to make is in PciIo.c:
>> >> >> Change GetMmioAddressTranslationOffset() to return 0 instead
>> >> >> of -1 when error occurs
>> >> >>
>> >> >
>> >> >Should I also remove "ASSERT (FALSE);" ?
>> >>
>> >>
>> >> ASSERT() won't be hit.
>> >>
>> >It didn't work (see my attached patch) :(
>> >BTW, if Configuration() already returns EFI_UNSUPPORTED, then there is
>> >no way to invoke PciGetBusRange(). Besides, MaxBus was never used in
>> >PciEnumeratorLight(), so it's meaningless to assign any value to it.
>> >
>> >Cheers,
>> >
>> >Gary Lin
>> >
>> >> >
>> >> >Cheers,
>> >> >
>> >> >Gary Lin
>> >> >
>> >> >> With the above 2 changes, can you check whether the system can
>> >> >> boot?
>> >> >>
>> >> >> If it can boot, then we need to think about what the proper fix is.
>> >> >> For OVMF above Xen, the PCI resources are assigned by Xen other than
>> >> >> PciBus driver. So PciRootBridgeIo->Configuration() doesn't know
>> >> >> the resource assignment information.
>> >> >> With old PciHostBridge driver, the Configuration() returns BUS resource
>> >> >> descriptor with MinBus = MaxBus = 0 (default value), so that the PciBus
>> >> >> driver can detect all devices in BUS 0, but I guess devices in BUS 1+ 
>> >> >> cannot
>> >> >> be detected.
>> >> >>
>> >> >> Does Xen pass the resource assignment information through some memory
>> >> >> blob to firmware? If yes, we could think about let PciHostBridgeLib 
>> >> >> pass that
>> >> >> information to PciHostBridge driver. If no, we also could let 
>> >> >> 

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Laszlo Ersek]

On 04/27/16 11:13, Laszlo Ersek wrote:
> On 04/27/16 01:51, Blank Field wrote:
>> Hi. I am attempting to rebuild my system with a host EFI bootloader,
>> namely GRUB.
>> I did a fresh install of F23(i've found that way easier than figuring
>> out how to migrate to EFI), turned off my CSM in host UEFI setup, got
>> my windows exVM booting in pure EFI, got my fedora booting in pure
>> EFI.
> 
> I don't know what "windows exVM" means.

On a second read, maybe you mean the following setup:

- Your host computer used to run Windows exclusively, originally.

- Then you installed Fedora to a separate host-side partition, and in
your new Fedora host OS, you created a virtual machine that is supposed
to run your original Windows installation, directly from the host disk.

If this is the case, the only thing I can recommend is "don't do it".
Tools have been developed that move physically installed OSes into
virtual machines. I recommend that you use them.

http://libguestfs.org/virt-p2v.1.html
http://libguestfs.org/virt-v2v.1.html

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

Re: [edk2] Minor problems with dualboot VM. EFIVars?

[Laszlo Ersek]

On 04/27/16 01:51, Blank Field wrote:
> Hi. I am attempting to rebuild my system with a host EFI bootloader,
> namely GRUB.
> I did a fresh install of F23(i've found that way easier than figuring
> out how to migrate to EFI), turned off my CSM in host UEFI setup, got
> my windows exVM booting in pure EFI, got my fedora booting in pure
> EFI.

I don't know what "windows exVM" means.

Also it's not clear if your host side F23 installation (the fresh one)
is identical to "your fedora" that is booting in "pure EFI" -- is that a
separate Fedora guest, or your host OS?

> So, the host has it's ESP on the harddrive, which i am
> passing-through(hey, seems like that bug with MBR.c got fixed
> somehow).

Ugh... I don't know why people are attracted to this (i.e., sharing
system disk(s) between a physically installed OS and some guests). What
is wrong with keeping your VM disk images in normal host-side files?

This kind of sharing looks extremely brittle.

> Here are the contents of it:
> http://paste.fedoraproject.org/360038/71386414/
> As you can see, we have some weird stuff going on here(what's that
> GUID?), but anyway.
> 
> When booting the VM (dumpxml will be attached somewhere later in the
> timeline for the reasons described far below) i've got "System
> BootOrder not found. Initializing defaults" and then it hangs.

Well, you have a UEFI variable store (with BootOrder and Boot in it)
on your physical machine, and you have another one (an independent one,
with separate BootOrder and Boot) in your virtual machine. However,
the EFI system partition whose files the Boot variables point to is
shared. Not good.

> dumpxml:
> 
> I do not know how to fix this.

I do know. Stop sharing your ESP between the host OS and guests.

Dual-booting on EFI -- even when both of your OSes are physically
installed, i.e., when they share not just the ESP, but the UEFI
variables as well -- is not a universally solved issue, *in practice*.
Please see this blog post:
.

> My ideas:
> 1. copy the host EFIVARS to VM's EFIVARS storage somewhere in
> /var/lib/libvirt...

No can do. The internal representation of the varstore is firmware
specific. Even on a more abstract level (UEFI variable names and
contents), your host-side varstore can have variables in vendor-specific
namespaces (GUIDs), with special names and values.

In general, consider each varstore compatible only with its own firmware
binary.

> 2. try to choose the exact bootmgr64 or what's-it's-name windows
> bootloader directly using either console UI OVMF provides or UefiShell
> instead of just selecting the disk in the console UI.
> 3. (a really bad idea) add the second host drive where /boot/ relies,
> so i could boot grub and select the proper .efi.
> 4. (a tedious idea) move /boot and grub to the hard drive, so the VM
> could've access to it.
> 
> What should i do?

I recommend that you install your guests without sharing system disks /
partitions between any two guests, and between any guest and the host.

> P.S.
> Please note that i am not a developer of OVMF or edk2. It means that i
> am subscribed to this list, but i am not receiving messages from this
> list because endless PATCHes would fill up my mailbox.

That's fine. I'm also subscribed to some lists with mail delivery
disabled. That way I can post, receive direct replies, and access the
archive even if the archive needs authentication.

> P.P.S.
> The reason for me not providing the dumpxml is...
> Well, seems like the libvirtd on my system has a bug, which hangs it
> dead when force stopping the VM. It would be pretty good if OVMF would
> respond to regular libvirtd poweroff(i guess it emulates the acpi
> button action) like a physical UEFI does - actually turn the machine
> off.

Libvirt has multiple ways to signal a shutdown request to the guest (for
example, using a guest agent, or as you say, by asking QEMU to set an
ACPI event status bit and to raise an SCI).

A guest agent for OVMF is quite a stretch.

Handling an SCI in OVMF is also a stretch. UEFI is not interrupt-driven.
I vaguely believe that on physical machines, when you press the ACPI
power button and the machine powers off immediately, is connected to
either different hardware configuration (i.e., the machine goes down
without the firmware knowing about it at all), or else it is somehow
configured / handled in SMM. SMM is not that far ahead in OVMF, and it's
pretty much out of scope as well, IMO.

However, you shouldn't even need such tricks. When you click "force off"
in virt-manager, while OVMF is "at rest", qemu will simply power off the
VM, forcibly (the guest has no chance to interfere), and when it's all
done, emit a QMP event for libvirt to consume. That's how libvirt knows
the VM has been powered down.

I use this all the time. I don't know why it doesn't work for you --
maybe you should configure libvirtd to log all QMP traffic 

Re: [edk2] [Patch] NetworkPkg: Fix incorrect buffer free in HttpDxe

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: Wu, Jiaxin 
Sent: Wednesday, April 27, 2016 3:08 PM
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Fu, Siyuan ; Zhang, Lubo 

Subject: [Patch] NetworkPkg: Fix incorrect buffer free in HttpDxe

FragmentBuffer of each TcpWrap in HttpDxe should not be freed in 
HttpTcpTokenCleanup(). This buffer points to HttpMsg body actually, which is 
the responsibility of the caller to allocate a buffer for Body.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpProto.c | 44 +-
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c 
index 9b3c774..bd95c0d 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1796,49 +1796,49 @@ HttpTcpTokenCleanup (
   HttpInstance   = Wrap->HttpInstance;
   Rx4Token   = NULL;
   Rx6Token   = NULL;
   
   if (HttpInstance->LocalAddressIsIPv6) {
-if (Wrap->TcpWrap.Rx6Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (Wrap->TcpWrap.Rx6Token.CompletionToken.Event);
-}
-
 Rx6Token = >TcpWrap.Rx6Token;
-if (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
-  FreePool (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
-  Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
+
+if (Rx6Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx6Token->CompletionToken.Event);
+  Rx6Token->CompletionToken.Event = NULL;
 }
-FreePool (Wrap);
 
-if (HttpInstance->Rx6Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (HttpInstance->Rx6Token.CompletionToken.Event);
-  HttpInstance->Rx6Token.CompletionToken.Event = NULL;
-}
+FreePool (Wrap);
 
 Rx6Token = >Rx6Token;
+
+if (Rx6Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx6Token->CompletionToken.Event);
+  Rx6Token->CompletionToken.Event = NULL;
+}
+
 if (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
   FreePool (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
   Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
 }
 
   } else {
-if (Wrap->TcpWrap.Rx4Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (Wrap->TcpWrap.Rx4Token.CompletionToken.Event);
-}
 Rx4Token = >TcpWrap.Rx4Token;
-if (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
-  FreePool (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
-  Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
+  
+if (Rx4Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx4Token->CompletionToken.Event);
+  Rx4Token->CompletionToken.Event = NULL;
 }
+
 FreePool (Wrap);
 
-if (HttpInstance->Rx4Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (HttpInstance->Rx4Token.CompletionToken.Event);
-  HttpInstance->Rx4Token.CompletionToken.Event = NULL;
+Rx4Token = >Rx4Token;
+
+if (Rx4Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx4Token->CompletionToken.Event);
+  Rx4Token->CompletionToken.Event = NULL;
 }
 
-Rx4Token = >Rx4Token;
+
 if (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
   FreePool (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
   Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
 }
   }
--
1.9.5.msysgit.1

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

Re: [edk2] OVMF broken under Xen (in PCI initialisation)

[Gary Lin]

On Wed, Apr 27, 2016 at 07:18:21AM +, Ni, Ruiyu wrote:
> Gary,
> In PciEnumeratorLight(), please directly assign 0 to MinBus when
> Configuration() returns error, instead of calling PciGetBusRange() to
> extract MinBus from the descriptors returned from Configuration().
> 
OK, I ignored PciGetBusRange() and the check of the return status of
Configuration(). Since MinBus is already initialized, I don't bother to
change the value anyway. The change is attached.

And yes, this time the shell showed.

Thanks,

Gary Lin

> Regards,
> Ray
> 
> >-Original Message-
> >From: Gary Lin [mailto:g...@suse.com]
> >Sent: Wednesday, April 27, 2016 2:54 PM
> >To: Ni, Ruiyu 
> >Cc: edk2-devel@lists.01.org; Xen Devel 
> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >
> >On Wed, Apr 27, 2016 at 05:39:40AM +, Ni, Ruiyu wrote:
> >>
> >>
> >> Regards,
> >> Ray
> >>
> >> >-Original Message-
> >> >From: Gary Lin [mailto:g...@suse.com]
> >> >Sent: Wednesday, April 27, 2016 12:29 PM
> >> >To: Ni, Ruiyu 
> >> >Cc: edk2-devel@lists.01.org; Xen Devel 
> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >
> >> >On Tue, Apr 26, 2016 at 09:40:42AM +, Ni, Ruiyu wrote:
> >> >> Gary,
> >> >> I can reproduce the issue and have debugged to get the reason.
> >> >> In MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c:
> >> >> PciEnumeratorLight() calls PciRootBridgeIo->Configuration()
> >> >> while the Configuration returns EFI_UNSUPPORTED resulting
> >> >> the PciBus driver exits earlier.
> >> >> You could try to manually set MinBus to 0 and MaxBus to 0xFF.
> >> >>
> >> >Do you mean to set MinBus and MaxBus in PciGetBusRange() ?
> >> >Should I also keep the EFI_UNSUPPORTED code in Configuration() ?
> >> Keep returning EFI_UNSUPPORTED in Configuration() and set
> >> MinBus/MaxBus in PciGetBusRange().
> >>
> >> >
> >> >> The second change you need to make is in PciIo.c:
> >> >> Change GetMmioAddressTranslationOffset() to return 0 instead
> >> >> of -1 when error occurs
> >> >>
> >> >
> >> >Should I also remove "ASSERT (FALSE);" ?
> >>
> >>
> >> ASSERT() won't be hit.
> >>
> >It didn't work (see my attached patch) :(
> >BTW, if Configuration() already returns EFI_UNSUPPORTED, then there is
> >no way to invoke PciGetBusRange(). Besides, MaxBus was never used in
> >PciEnumeratorLight(), so it's meaningless to assign any value to it.
> >
> >Cheers,
> >
> >Gary Lin
> >
> >> >
> >> >Cheers,
> >> >
> >> >Gary Lin
> >> >
> >> >> With the above 2 changes, can you check whether the system can
> >> >> boot?
> >> >>
> >> >> If it can boot, then we need to think about what the proper fix is.
> >> >> For OVMF above Xen, the PCI resources are assigned by Xen other than
> >> >> PciBus driver. So PciRootBridgeIo->Configuration() doesn't know
> >> >> the resource assignment information.
> >> >> With old PciHostBridge driver, the Configuration() returns BUS resource
> >> >> descriptor with MinBus = MaxBus = 0 (default value), so that the PciBus
> >> >> driver can detect all devices in BUS 0, but I guess devices in BUS 1+ 
> >> >> cannot
> >> >> be detected.
> >> >>
> >> >> Does Xen pass the resource assignment information through some memory
> >> >> blob to firmware? If yes, we could think about let PciHostBridgeLib 
> >> >> pass that
> >> >> information to PciHostBridge driver. If no, we also could let 
> >> >> PciHostBridgeLib
> >> >> collect the information (IO/MEM/BUS resource assignment) and pass that
> >> >> to PciHostBridgeDxe driver.
> >> >>
> >> >> And we need to have a way to tell PciHostBridgeDxe the resource 
> >> >> information
> >> >> passed from PciHostBridgeLib is available resource to assign, or 
> >> >> already allocated.
> >> >> Maybe just depends on the PcdPciBusDisableEnumeration. still thinking.
> >> >>
> >> >>
> >> >> Regards,
> >> >> Ray
> >> >>
> >> >> >-Original Message-
> >> >> >From: Gary Lin [mailto:g...@suse.com]
> >> >> >Sent: Tuesday, April 26, 2016 4:40 PM
> >> >> >To: Ni, Ruiyu 
> >> >> >Cc: edk2-devel@lists.01.org; Xen Devel 
> >> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >> >
> >> >> >On Tue, Apr 26, 2016 at 08:19:49AM +, Ni, Ruiyu wrote:
> >> >> >> Gary,
> >> >> >> Maybe the system boots to Shell well but the video isn't initialized 
> >> >> >> properly.
> >> >> >> Can you build the firmware using "-D DEBUG_ON_SERIAL_PORT" switch and
> >> >> >> give me the boot log?
> >> >> >>
> >> >> >Hi Ray,
> >> >> >
> >> >> >I already did and it's how I make the firmware spit the log :-(
> >> >> >
> >> >> >Thanks,
> >> >> >
> >> >> >Gary Lin
> >> >> >
> >> >> >> Regards,
> >> >> >> Ray
> >> >> >>
> >> >> >> From: Gary Lin [mailto:g...@suse.com]
> >> >> >> Sent: Tuesday, April 26, 2016 3:35 PM
> >> >> >> To: Ni, Ruiyu 
> >> >> >> Cc: Anthony PERARD 

[edk2] [Patch] NetworkPkg: Fix incorrect buffer free in HttpDxe

[Jiaxin Wu]

FragmentBuffer of each TcpWrap in HttpDxe should not be
freed in HttpTcpTokenCleanup(). This buffer points to
HttpMsg body actually, which is the responsibility of the
caller to allocate a buffer for Body.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Zhang Lubo 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiaxin Wu 
---
 NetworkPkg/HttpDxe/HttpProto.c | 44 +-
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index 9b3c774..bd95c0d 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1796,49 +1796,49 @@ HttpTcpTokenCleanup (
   HttpInstance   = Wrap->HttpInstance;
   Rx4Token   = NULL;
   Rx6Token   = NULL;
   
   if (HttpInstance->LocalAddressIsIPv6) {
-if (Wrap->TcpWrap.Rx6Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (Wrap->TcpWrap.Rx6Token.CompletionToken.Event);
-}
-
 Rx6Token = >TcpWrap.Rx6Token;
-if (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
-  FreePool (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
-  Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
+
+if (Rx6Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx6Token->CompletionToken.Event);
+  Rx6Token->CompletionToken.Event = NULL;
 }
-FreePool (Wrap);
 
-if (HttpInstance->Rx6Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (HttpInstance->Rx6Token.CompletionToken.Event);
-  HttpInstance->Rx6Token.CompletionToken.Event = NULL;
-}
+FreePool (Wrap);
 
 Rx6Token = >Rx6Token;
+
+if (Rx6Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx6Token->CompletionToken.Event);
+  Rx6Token->CompletionToken.Event = NULL;
+}
+
 if (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
   FreePool (Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
   Rx6Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
 }
 
   } else {
-if (Wrap->TcpWrap.Rx4Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (Wrap->TcpWrap.Rx4Token.CompletionToken.Event);
-}
 Rx4Token = >TcpWrap.Rx4Token;
-if (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
-  FreePool (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
-  Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
+  
+if (Rx4Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx4Token->CompletionToken.Event);
+  Rx4Token->CompletionToken.Event = NULL;
 }
+
 FreePool (Wrap);
 
-if (HttpInstance->Rx4Token.CompletionToken.Event != NULL) {
-  gBS->CloseEvent (HttpInstance->Rx4Token.CompletionToken.Event);
-  HttpInstance->Rx4Token.CompletionToken.Event = NULL;
+Rx4Token = >Rx4Token;
+
+if (Rx4Token->CompletionToken.Event != NULL) {
+  gBS->CloseEvent (Rx4Token->CompletionToken.Event);
+  Rx4Token->CompletionToken.Event = NULL;
 }
 
-Rx4Token = >Rx4Token;
+
 if (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer != NULL) {
   FreePool (Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer);
   Rx4Token->Packet.RxData->FragmentTable[0].FragmentBuffer = NULL;
 }
   }
-- 
1.9.5.msysgit.1

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

Re: [edk2] OVMF broken under Xen (in PCI initialisation)

[Gary Lin]

On Wed, Apr 27, 2016 at 05:39:40AM +, Ni, Ruiyu wrote:
> 
> 
> Regards,
> Ray
> 
> >-Original Message-
> >From: Gary Lin [mailto:g...@suse.com]
> >Sent: Wednesday, April 27, 2016 12:29 PM
> >To: Ni, Ruiyu 
> >Cc: edk2-devel@lists.01.org; Xen Devel 
> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >
> >On Tue, Apr 26, 2016 at 09:40:42AM +, Ni, Ruiyu wrote:
> >> Gary,
> >> I can reproduce the issue and have debugged to get the reason.
> >> In MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c:
> >> PciEnumeratorLight() calls PciRootBridgeIo->Configuration()
> >> while the Configuration returns EFI_UNSUPPORTED resulting
> >> the PciBus driver exits earlier.
> >> You could try to manually set MinBus to 0 and MaxBus to 0xFF.
> >>
> >Do you mean to set MinBus and MaxBus in PciGetBusRange() ?
> >Should I also keep the EFI_UNSUPPORTED code in Configuration() ?
> Keep returning EFI_UNSUPPORTED in Configuration() and set
> MinBus/MaxBus in PciGetBusRange().
> 
> >
> >> The second change you need to make is in PciIo.c:
> >> Change GetMmioAddressTranslationOffset() to return 0 instead
> >> of -1 when error occurs
> >>
> >
> >Should I also remove "ASSERT (FALSE);" ?
> 
> 
> ASSERT() won't be hit.
> 
It didn't work (see my attached patch) :(
BTW, if Configuration() already returns EFI_UNSUPPORTED, then there is
no way to invoke PciGetBusRange(). Besides, MaxBus was never used in
PciEnumeratorLight(), so it's meaningless to assign any value to it.

Cheers,

Gary Lin

> >
> >Cheers,
> >
> >Gary Lin
> >
> >> With the above 2 changes, can you check whether the system can
> >> boot?
> >>
> >> If it can boot, then we need to think about what the proper fix is.
> >> For OVMF above Xen, the PCI resources are assigned by Xen other than
> >> PciBus driver. So PciRootBridgeIo->Configuration() doesn't know
> >> the resource assignment information.
> >> With old PciHostBridge driver, the Configuration() returns BUS resource
> >> descriptor with MinBus = MaxBus = 0 (default value), so that the PciBus
> >> driver can detect all devices in BUS 0, but I guess devices in BUS 1+ 
> >> cannot
> >> be detected.
> >>
> >> Does Xen pass the resource assignment information through some memory
> >> blob to firmware? If yes, we could think about let PciHostBridgeLib pass 
> >> that
> >> information to PciHostBridge driver. If no, we also could let 
> >> PciHostBridgeLib
> >> collect the information (IO/MEM/BUS resource assignment) and pass that
> >> to PciHostBridgeDxe driver.
> >>
> >> And we need to have a way to tell PciHostBridgeDxe the resource information
> >> passed from PciHostBridgeLib is available resource to assign, or already 
> >> allocated.
> >> Maybe just depends on the PcdPciBusDisableEnumeration. still thinking.
> >>
> >>
> >> Regards,
> >> Ray
> >>
> >> >-Original Message-
> >> >From: Gary Lin [mailto:g...@suse.com]
> >> >Sent: Tuesday, April 26, 2016 4:40 PM
> >> >To: Ni, Ruiyu 
> >> >Cc: edk2-devel@lists.01.org; Xen Devel 
> >> >Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >
> >> >On Tue, Apr 26, 2016 at 08:19:49AM +, Ni, Ruiyu wrote:
> >> >> Gary,
> >> >> Maybe the system boots to Shell well but the video isn't initialized 
> >> >> properly.
> >> >> Can you build the firmware using "-D DEBUG_ON_SERIAL_PORT" switch and
> >> >> give me the boot log?
> >> >>
> >> >Hi Ray,
> >> >
> >> >I already did and it's how I make the firmware spit the log :-(
> >> >
> >> >Thanks,
> >> >
> >> >Gary Lin
> >> >
> >> >> Regards,
> >> >> Ray
> >> >>
> >> >> From: Gary Lin [mailto:g...@suse.com]
> >> >> Sent: Tuesday, April 26, 2016 3:35 PM
> >> >> To: Ni, Ruiyu 
> >> >> Cc: Anthony PERARD ; 
> >> >> edk2-devel@lists.01.org; Xen Devel 
> >> >> Subject: Re: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >>
> >> >> On Tue, Apr 26, 2016 at 06:43:56AM +, Ni, Ruiyu wrote:
> >> >> >
> >> >> >
> >> >> > Regards,
> >> >> > Ray
> >> >> >
> >> >> > >-Original Message-
> >> >> > >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> >> >> > >Of Anthony PERARD
> >> >> > >Sent: Friday, April 22, 2016 10:48 PM
> >> >> > >To: edk2-devel@lists.01.org
> >> >> > >Cc: Xen Devel 
> >> >> > >>
> >> >> > >Subject: [edk2] OVMF broken under Xen (in PCI initialisation)
> >> >> > >
> >> >> > >Hi,
> >> >> > >
> >> >> > >Following the switch to MdeModulePkg/Bus/Pci/PciHostBridgeDxe, the 
> >> >> > >pci root
> >> >> > >bridge does not finish to initialize and breaks under Xen.
> >> >> > >
> >> >> > >There are several issue probably due to the use of
> >> >> > >PcdPciDisableBusEnumeration=TRUE.
> >> >> > >
> >> >> > >First one:
> >> >> > >ASSERT MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c(99): 
> >> 

[edk2] [Patch] MdeModulePkg: DxeCore MemoryPool Algorithm Update

[Liming Gao]

Use 128 bytes as the start size region to be same to previous one.

64 bytes is small as the first range. On X64 arch, POOL_OVERHEAD
takes 40 bytes, the pool data less than 24 bytes can be fit into
it. But, the real allocation is few that can't reduce its free pool
link list. And, the second range (64~128) has more allocation
that also increases the free pool link list of the first range.
Then, the link list will become longer and longer. When LinkList
check enable in DEBUG tip, the long link list will bring the
additional overhead and bad performance. Here is the performance
data collected in our X64 platform with DEBUG enable.
64  byte: 22 seconds in BDS phase
128 byte: 19.6 seconds in BDS phase

Cc: Michael Kinney 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Core/Dxe/Mem/Pool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/Pool.c b/MdeModulePkg/Core/Dxe/Mem/Pool.c
index 5eced88..934b4ca 100644
--- a/MdeModulePkg/Core/Dxe/Mem/Pool.c
+++ b/MdeModulePkg/Core/Dxe/Mem/Pool.c
@@ -52,7 +52,7 @@ typedef struct {
 // as we would in a strict power-of-2 sequence
 //
 STATIC CONST UINT16 mPoolSizeTable[] = {
-  64, 128, 192, 320, 512, 832, 1344, 2176, 3520, 5696, 9216, 14912, 24128
+  128, 256, 384, 640, 1024, 1664, 2688, 5352, 7040, 11392, 18432, 29824
 };
 
 #define SIZE_TO_LIST(a)   (GetPoolIndexFromSize (a))
-- 
2.8.0.windows.1

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

Re: [edk2] [PATCH] Vlv2TbltDevicePkg: Modify DSC file to ensure PlatformPkgIA32.dsc is sync with PlatformPkgX64.dsc.

[Wei, David]

Reviewed-by: David Wei 

Thanks,
David  

Intel SSG BIOS


-Original Message-
From: Guo, Mang 
Sent: Wednesday, April 27, 2016 10:38 AM
To: edk2-devel@lists.01.org
Cc: Wei, David 
Subject: [PATCH] Vlv2TbltDevicePkg: Modify DSC file to ensure 
PlatformPkgIA32.dsc is sync with PlatformPkgX64.dsc.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Mang Guo 
Reviewed-by: David Wei 
---
 Vlv2TbltDevicePkg/PlatformPkgIA32.dsc | 11 ++-
 Vlv2TbltDevicePkg/PlatformPkgX64.dsc  |  5 +++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
index cb0aae3..70ff2e0 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgIA32.dsc
@@ -1125,7 +1125,16 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
 !endif
   }
 
-
+!if $(DXE_ARCHITECTURE) == X64
+!if $(CAPSULE_ENABLE) == TRUE
+  MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf {
+
+PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
+
MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
+HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
+  }
+!endif
+!endif
 
   
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterSmm.inf
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf{
diff --git a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc 
b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
index 3432a13..a031208 100644
--- a/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
+++ b/Vlv2TbltDevicePkg/PlatformPkgX64.dsc
@@ -1125,7 +1125,8 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
   DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
 !endif
   }
-
+  
+!if $(DXE_ARCHITECTURE) == X64
 !if $(CAPSULE_ENABLE) == TRUE
   MdeModulePkg/Universal/CapsulePei/CapsuleX64.inf {
 
@@ -1134,6 +1135,7 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
 HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
   }
 !endif
+!endif
 
   
MdeModulePkg/Universal/ReportStatusCodeRouter/Smm/ReportStatusCodeRouterSmm.inf
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf{
@@ -1201,7 +1203,6 @@ 
$(PLATFORM_BINARY_PACKAGE)/$(DXE_ARCHITECTURE)$(TARGET)/IA32/fTPMInitPeim.inf
   !if $(ESRT_ENABLE) == TRUE
   
CapsuleLib|$(PLATFORM_PACKAGE)/Library/DxeEsrtCapsuleRtLib/DxeEsrtCapsuleRtLib.inf
   !endif
-  FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
   }
 
   
MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
-- 
2.8.1.windows.1

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