Re: [edk2] [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when polling OCR Reg

2017-03-12 Thread Wu, Hao A
> -Original Message-
> From: Tian, Feng
> Sent: Monday, March 13, 2017 11:24 AM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A
> Subject: [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when
> polling OCR Reg
> 
> At worst case, OCR register may always not set BIT31. It will cause
> original code enter to dead loop. Adding a break for such case.
> 
> Cc: Hao Wu 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Feng Tian 
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 -
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 8 
>  MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c| 9 -
>  MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c| 9 -
>  4 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index 9dbec10..6653a87 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -1109,6 +1109,7 @@ EmmcIdentification (
>EFI_SD_MMC_PASS_THRU_PROTOCOL  *PassThru;
>UINT32 Ocr;
>UINT16 Rca;
> +  UINTN  Retry;
> 
>PciIo= Private->PciIo;
>PassThru = >PassThru;
> @@ -1119,7 +1120,8 @@ EmmcIdentification (
>  return Status;
>}
> 
> -  Ocr = 0;
> +  Ocr   = 0;
> +  Retry = 0;
>do {
>  Status = EmmcGetOcr (PassThru, Slot, );
>  if (EFI_ERROR (Status)) {
> @@ -1127,6 +1129,11 @@ EmmcIdentification (
>return Status;
>  }
>  Ocr |= BIT30;
> +
> +if (Retry++ == 100) {

Hi Feng,

Maybe a debug message can be added here to indicate the type of error,
like other error handling logics in this function.

Apart from that, the patch is good to me.
Reviewed-by: Hao Wu 

Best Regards,
Hao Wu

> +  return EFI_DEVICE_ERROR;
> +}
> +gBS->Stall(10 * 1000);
>} while ((Ocr & BIT31) == 0);
> 
>Status = EmmcGetAllCid (PassThru, Slot);
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 9122848..1da0849 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1017,6 +1017,7 @@ SdCardIdentification (
>UINT8  PowerCtrl;
>UINT32 PresentState;
>UINT8  HostCtrl2;
> +  UINTN  Retry;
> 
>PciIo= Private->PciIo;
>PassThru = >PassThru;
> @@ -1097,12 +1098,19 @@ SdCardIdentification (
>//Note here we only support the cards complied with SD physical
>//layer simplified spec version 2.0 and version 3.0 and above.
>//
> +  Ocr   = 0;
> +  Retry = 0;
>do {
>  Status = SdCardSendOpCond (PassThru, Slot, 0, Ocr, S18r, Xpc, TRUE, 
> );
>  if (EFI_ERROR (Status)) {
>DEBUG ((DEBUG_ERROR, "SdCardIdentification: SdCardSendOpCond fails
> with %r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
>return EFI_DEVICE_ERROR;
>  }
> +
> +if (Retry++ == 100) {
> +  return EFI_DEVICE_ERROR;
> +}
> +gBS->Stall(10 * 1000);
>} while ((Ocr & BIT31) == 0);
> 
>//
> diff --git a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> index 2c0baca..5cb20a3 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
> @@ -2827,6 +2827,7 @@ EmmcPeimIdentification (
>EFI_STATUS Status;
>UINT32 Ocr;
>UINT32 Rca;
> +  UINTN  Retry;
> 
>Status = EmmcPeimReset (Slot);
>if (EFI_ERROR (Status)) {
> @@ -2834,13 +2835,19 @@ EmmcPeimIdentification (
>  return Status;
>}
> 
> -  Ocr = 0;
> +  Ocr   = 0;
> +  Retry = 0;
>do {
>  Status = EmmcPeimGetOcr (Slot, );
>  if (EFI_ERROR (Status)) {
>DEBUG ((EFI_D_ERROR, "EmmcPeimIdentification: EmmcPeimGetOcr fails
> with %r\n", Status));
>return Status;
>  }
> +
> +if (Retry++ == 100) {
> +  return EFI_DEVICE_ERROR;
> +}
> +MicroSecondDelay (10 * 1000);
>} while ((Ocr & BIT31) == 0);
> 
>Status = EmmcPeimGetAllCid (Slot);
> diff --git a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> index 23e6563..81076ba 100644
> --- a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> +++ b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
> @@ -2754,7 +2754,7 @@ SdPeimIdentification (
>UINT32 PresentState;
>UINT8  HostCtrl2;
>SD_HC_SLOT_CAP Capability;
> -
> +  UINTN  Retry;
>//
>// 1. Send Cmd0 to the device
>//
> @@ -2842,12 +2842,19 @@ SdPeimIdentification (
>

[edk2] [patch] MdeModulePkg/SdMmc: Add break to avoid dead loop when polling OCR Reg

2017-03-12 Thread Feng Tian
At worst case, OCR register may always not set BIT31. It will cause
original code enter to dead loop. Adding a break for such case.

Cc: Hao Wu 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Feng Tian 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 -
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 8 
 MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c| 9 -
 MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c| 9 -
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 9dbec10..6653a87 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -1109,6 +1109,7 @@ EmmcIdentification (
   EFI_SD_MMC_PASS_THRU_PROTOCOL  *PassThru;
   UINT32 Ocr;
   UINT16 Rca;
+  UINTN  Retry;
 
   PciIo= Private->PciIo;
   PassThru = >PassThru;
@@ -1119,7 +1120,8 @@ EmmcIdentification (
 return Status;
   }
 
-  Ocr = 0;
+  Ocr   = 0;
+  Retry = 0;
   do {
 Status = EmmcGetOcr (PassThru, Slot, );
 if (EFI_ERROR (Status)) {
@@ -1127,6 +1129,11 @@ EmmcIdentification (
   return Status;
 }
 Ocr |= BIT30;
+
+if (Retry++ == 100) {
+  return EFI_DEVICE_ERROR; 
+}
+gBS->Stall(10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   Status = EmmcGetAllCid (PassThru, Slot);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 9122848..1da0849 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -1017,6 +1017,7 @@ SdCardIdentification (
   UINT8  PowerCtrl;
   UINT32 PresentState;
   UINT8  HostCtrl2;
+  UINTN  Retry;
 
   PciIo= Private->PciIo;
   PassThru = >PassThru;
@@ -1097,12 +1098,19 @@ SdCardIdentification (
   //Note here we only support the cards complied with SD physical
   //layer simplified spec version 2.0 and version 3.0 and above.
   //
+  Ocr   = 0;
+  Retry = 0;
   do {
 Status = SdCardSendOpCond (PassThru, Slot, 0, Ocr, S18r, Xpc, TRUE, );
 if (EFI_ERROR (Status)) {
   DEBUG ((DEBUG_ERROR, "SdCardIdentification: SdCardSendOpCond fails with 
%r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
   return EFI_DEVICE_ERROR;
 }
+
+if (Retry++ == 100) {
+  return EFI_DEVICE_ERROR; 
+}
+gBS->Stall(10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   //
diff --git a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c 
b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
index 2c0baca..5cb20a3 100644
--- a/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
+++ b/MdeModulePkg/Bus/Sd/EmmcBlockIoPei/EmmcHci.c
@@ -2827,6 +2827,7 @@ EmmcPeimIdentification (
   EFI_STATUS Status;
   UINT32 Ocr;
   UINT32 Rca;
+  UINTN  Retry;
 
   Status = EmmcPeimReset (Slot);
   if (EFI_ERROR (Status)) {
@@ -2834,13 +2835,19 @@ EmmcPeimIdentification (
 return Status;
   }
 
-  Ocr = 0;
+  Ocr   = 0;
+  Retry = 0;
   do {
 Status = EmmcPeimGetOcr (Slot, );
 if (EFI_ERROR (Status)) {
   DEBUG ((EFI_D_ERROR, "EmmcPeimIdentification: EmmcPeimGetOcr fails with 
%r\n", Status));
   return Status;
 }
+
+if (Retry++ == 100) {
+  return EFI_DEVICE_ERROR; 
+}
+MicroSecondDelay (10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   Status = EmmcPeimGetAllCid (Slot);
diff --git a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c 
b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
index 23e6563..81076ba 100644
--- a/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
+++ b/MdeModulePkg/Bus/Sd/SdBlockIoPei/SdHci.c
@@ -2754,7 +2754,7 @@ SdPeimIdentification (
   UINT32 PresentState;
   UINT8  HostCtrl2;
   SD_HC_SLOT_CAP Capability;
-
+  UINTN  Retry;
   //
   // 1. Send Cmd0 to the device
   //
@@ -2842,12 +2842,19 @@ SdPeimIdentification (
   //Note here we only support the cards complied with SD physical
   //layer simplified spec version 2.0 and version 3.0 and above.
   //
+  Ocr   = 0;
+  Retry = 0;
   do {
 Status = SdPeimSendOpCond (Slot, 0, Ocr, S18r, Xpc, TRUE, );
 if (EFI_ERROR (Status)) {
   DEBUG ((EFI_D_ERROR, "SdPeimIdentification: SdPeimSendOpCond fails with 
%r Ocr %x, S18r %x, Xpc %x\n", Status, Ocr, S18r, Xpc));
   return EFI_DEVICE_ERROR;
 }
+
+if (Retry++ == 100) {
+  return EFI_DEVICE_ERROR; 
+}
+MicroSecondDelay (10 * 1000);
   } while ((Ocr & BIT31) == 0);
 
   //
-- 
2.7.1.windows.2

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

Re: [edk2] [PATCH 0/3] Add Context in SmiHandlerProfileUnregister.

2017-03-12 Thread Tian, Feng
Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: Fan, Jeff 
Sent: Monday, March 13, 2017 10:06 AM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Tian, Feng ; Zeng, Star ; Bret 
Barkelew 
Subject: RE: [PATCH 0/3] Add Context in SmiHandlerProfileUnregister.

Reviewed-by: Jeff Fan 

-Original Message-
From: Yao, Jiewen 
Sent: Friday, March 10, 2017 3:35 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Tian, Feng; Zeng, Star; Bret Barkelew
Subject: [PATCH 0/3] Add Context in SmiHandlerProfileUnregister.

This issue is reported by bret.barke...@microsoft.com.

We observe that a platform may use same Handler for different context.

In order to support Unregister such handler, we have to input context 
information as well.


The patch does not impact any platform with SmiHandlerProfile disabled.

Unit tests below:
1) register same handler with different context, and unregister each.
2) register and unregister UsbContext.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 

Jiewen Yao (3):
  MdePkg/SmiHandlerProfile: Add Context support in Unregister
  MdeModulePkg/SmiHandlerProfile: Add Context support in Unregister
  MdeModulePkg/SmmCore: Add Context in SmiHandlerProfileUnregister.

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   8 +-
 MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c| 103 

 MdeModulePkg/Include/Guid/SmiHandlerProfile.h  |  41 
+++-
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c |  10 +-
 MdePkg/Include/Library/SmiHandlerProfileLib.h  |   8 +-
 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c |   8 +-
 6 files changed, 151 insertions(+), 27 deletions(-)

--
2.7.4.windows.1

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


Re: [edk2] [PATCH v2 2/2] MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT, X_DSDT} mutual exclusion

2017-03-12 Thread Fan, Jeff
Laszlo,

We found one Windows Server 2012 R2 blue screen issue with ACPI 6.1 FADT table.

We did the following configuration test with DSDT under 4GB.
.DSDT .X_DSDT Window Server 2012 R2
--      ---
setclear Failed// current implementation
clear setSucceed
setsetSucceed

Thanks!
Jeff

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Thursday, March 09, 2017 3:59 AM
To: edk2-devel-01
Cc: Tian, Feng; Michael Tsirkin; Ard Biesheuvel; Phil Dennis-Jordan; Leo Duran; 
Yao, Jiewen; Al Stone; Zeng, Star
Subject: [edk2] [PATCH v2 2/2] MdeModulePkg/AcpiTableDxe: improve FADT.{DSDT, 
X_DSDT} mutual exclusion

The ACPI specification, up to and including revision 5.1 Errata A, allows the 
DSDT and X_DSDT fields to be both set in the FADT. (Obviously, this only makes 
sense if the DSDT address is representable in 4 bytes.)

Starting with 5.1 Errata B, specifically for Mantis 1393 
, the spec requires at most 
one of DSDT and X_DSDT to be set to a nonzero value.

MdeModulePkg/AcpiTableDxe handles this mutual exclusion somewhat inconsistently.

- If the caller of EFI_ACPI_TABLE_PROTOCOL.InstallAcpiTable() installs the
  tables in "DSDT, FADT" order, then we enforce the exclusion between the
  DSDT and X_DSDT fields:

  DSDT under 4GB  FADT.DSDT  FADT.X_DSDT[VARIANT B]
  --  -  ---
  yes setclear
  no  clear  set

  This behavior conforms to 5.1 Errata B. (And it's not required by
  earlier versions of the spec.)

- If the caller passes in the tables in "FADT, DSDT" relative order, then
  we do not enforce the exclusion:

  DSDT under 4GB  FADT.DSDT  FADT.X_DSDT[VARIANT A]
  --  -  ---
  yes setset
  no  clear  set

  This satisfies 5.1 Errata A and earlier, but breaks 5.1 Errata B and
  later.

Unify the handling of both relative orders. In particular, check the major and 
minor version numbers in the FADT. If the FADT version is strictly before 5.1, 
then implement [VARIANT A]. If the FADT version is equal to or larger than 5.1, 
then implement [VARIANT B].

We make three observations:

- We can't check the FADT table version precisely against "5.1 Errata B";
  erratum levels are not captured in the table. We err in the safe
  direction, namely we enforce the exclusion for "5.1" and "5.1 Errata A".

- The same applies to "6.0" versus "6.0 Errata A". Because we cannot
  distinguish these two, we consider "6.0" to be "equal to or larger than
  5.1", and apply [VARIANT B], enforcing the exclusion.

- While a blanket [VARIANT B] would be simpler, there is a significant
  benefit to [VARIANT A], under the spec versions that permit it:
  compatibility with a wider range of OSPMs (typically, older ones).

  For example, Igor reported about a "DELL R430 system with rev4 FADT
  where DSDT and X_DSDT are pointing to the same address". Michael also
  reported about several systems that exhibit the same.

Regression tested with the following KVM guests (QEMU built at ata0def594286d, 
"Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
staging", 2017-01-30):

- OVMF: boot and S3 suspend/resume
  - Ia32, Q35, SMM
- Fedlet 20141209
  - Ia32X64, Q35, SMM
- Fedora 22
- Windows 7
- Windows 8.1
- Windows 10
- Windows Server 2008 R2
- Windows Server 2012 R2
- Windows Server 2016 Tech Preview 4
  - X64, I440FX, no SMM
- Fedora 24
- RHEL-6.7
- RHEL-7.2-ish
- ArmVirtQemu: boot test with virtio-gpu
  - AARCH64
- Fedora 24
- RHELSA-7.3
- openSUSE Tumbleweed (4.8.4-based)

This change is connected to ASWG ticket
, which is now closed/fixed.

Cc: Al Stone 
Cc: Ard Biesheuvel 
Cc: Feng Tian 
Cc: Igor Mammedov 
Cc: Jiewen Yao 
Cc: Leo Duran 
Cc: Michael Tsirkin 
Cc: Phil Dennis-Jordan 
Cc: Star Zeng 
Reported-by: Phil Dennis-Jordan 
Suggested-by: Igor Mammedov 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek 
Reviewed-by: Phil Dennis-Jordan 
---

Notes:
v2:
- simplify logic in RequireDsdtXDsdtExclusion() [Jiewen]
- pick up Phil's R-b nonetheless (the above change is a minimal
  reformulation of code, with no behavioral difference)
- add reference to Mantis#1757 to the commit message

v1:
NOTE for people on the CC list:

If you are not presently subscribed to edk2-devel and wish to comment 

Re: [edk2] [PATCH 0/3] Refine the comment for AsciiValueToStringS

2017-03-12 Thread Gao, Liming
Reviewed-by: Liming Gao 

>-Original Message-
>From: Wu, Hao A
>Sent: Tuesday, March 07, 2017 4:16 PM
>To: edk2-devel@lists.01.org
>Cc: Wu, Hao A ; Yao, Jiewen ;
>Gao, Liming ; Kinney, Michael D
>
>Subject: [PATCH 0/3] Refine the comment for AsciiValueToStringS
>
>The series will refine the comment description for:
>1. PrintLib API AsciiValueToStringS
>2. EFI_PRINT2S_PROTOCOL service ASCII_VALUE_TO_STRING_S
>
>They will not ASSERT when the input/output parameter 'Buffer' is not
>aligned on a 16-bit boundary.
>
>Cc: Jiewen Yao 
>Cc: Liming Gao 
>Cc: Michael Kinney 
>
>Hao Wu (3):
>  MdePkg/BasePrintLib: Refine the comment for AsciiValueToStringS API
>  MdeModulePkg/PrintLib: Refine the comment for AsciiValueToStringS API
>  MdeModulePkg/Print2: Refine the comment for
>ASCII_VALUE_TO_STRING_S
>
> MdeModulePkg/Include/Protocol/Print2.h| 3 +--
> MdeModulePkg/Library/DxePrintLibPrint2Protocol/PrintLib.c | 3 +--
> MdePkg/Include/Library/PrintLib.h | 3 +--
> MdePkg/Library/BasePrintLib/PrintLib.c| 3 +--
> 4 files changed, 4 insertions(+), 8 deletions(-)
>
>--
>2.12.0.windows.1

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


Re: [edk2] Do edk2 support PcdGetEx64 in library

2017-03-12 Thread Gao, Liming
Xiaofeng:
  Thanks for your reporting. This is a tool issue. Please submit one bug in 
bugzillar. We will fix it. 

Thanks
Liming
>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>wang xiaofeng
>Sent: Thursday, March 09, 2017 10:46 AM
>To: edk2-devel@lists.01.org
>Subject: [edk2] Do edk2 support PcdGetEx64 in library
>
>Hi ,
>   Anyone knows whether edk2 support PcdGetEx64 in library?
>
>   First I declare a PcdsDynamicEx in dec file
>[PcdsDynamicEx]
>   gEfixxxTokenSpaceGuid.PcdPasswordToken|0|UINT64|0x1009
>
>  Then I tried to use PcdGetEx64 in a library construct function
>   OldToken = PcdGetEx64((CONST GUID
>*),PcdPasswordToken);
>
>
>   There is a compile error:
> warning C4013: '_PCD_TOKEN_EX_PcdPasswordToken' undefined; assuming
>extern returning int
>NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio
>12.0\Vc\bin\x86_amd64\cl.exe"' : return code '0x2'
>
>
>  But I can use PcdGetEx64 or PcdSetEx64S in DXE drivers, so is there a
>limitation to set/get PcdsDynamicEx with PcdGetEx64/ PcdSetEx64S  in library?
>
>
>
>
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for NVME

2017-03-12 Thread Wang, Sunny (HPS SW)
Never mind. I overlooked something. Thanks for clarifying this. 
Looks good to me. 
Reviewed-by: Sunny Wang 

-Original Message-
From: Ni, Ruiyu [mailto:ruiyu...@intel.com] 
Sent: Monday, March 13, 2017 10:24 AM
To: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org
Cc: Tian, Feng ; Haskell, Darrell 
Subject: RE: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
description for NVME
Importance: High

I didn't find this library in EDKII.
When I firstly saw your comments, I was a little bit unwilling to change 
UefiBootManagerLib to depend on UefiNvmExpressLib. Because it would require all 
platforms to include UefiNvmExpressLib in DSC.
But since now I find no such library, I feel much better

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Sunny (HPS SW)
> Sent: Friday, March 10, 2017 6:11 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Tian, Feng ; Haskell, Darrell 
> 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib:
> Generate boot description for NVME
> 
> Hi Ray,
> Just a question out of curiosity. Why don't we use UefiNvmExpressLib's
> NvmExpressIdentify() for sending ADMIN_IDENTIFY command to NVME 
> controller?
> Others look good to me.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Friday, March 10, 2017 3:58 PM
> To: edk2-devel@lists.01.org
> Cc: Feng Tian 
> Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
> description for NVME
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Feng Tian 
> ---
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 104
> -
>  .../Library/UefiBootManagerLib/InternalBm.h|   4 +-
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
>  3 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> index 050647d..501a0cc 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> @@ -1,7 +1,7 @@
>  /** @file
>Library functions which relate with boot option description.
> 
> -Copyright (c) 2011 - 2016, Intel Corporation. All rights 
> reserved.
> +Copyright (c) 2011 - 2017, Intel Corporation. All rights 
> +reserved.
>  (C) Copyright 2015 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 @@ -501,6 
> +501,107 @@ BmGetLoadFileDescription (  }
> 
>  /**
> +  Return the boot description for NVME boot device.
> +
> +  @param HandleController handle.
> +
> +  @return  The description string.
> +**/
> +CHAR16 *
> +BmGetNvmeDescription (
> +  IN EFI_HANDLE  Handle
> +  )
> +{
> +  EFI_STATUS   Status;
> +  EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL   *NvmePassthru;
> +  EFI_DEV_PATH_PTR DevicePath;
> +  EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET CommandPacket;
> +  EFI_NVM_EXPRESS_COMMAND  Command;
> +  EFI_NVM_EXPRESS_COMPLETION   Completion;
> +  NVME_ADMIN_CONTROLLER_DATA   ControllerData;
> +  CHAR16   *Description;
> +  CHAR16   *Char;
> +  UINTNIndex;
> +
> +  Status = gBS->HandleProtocol (Handle, , 
> + (VOID **) );  if (EFI_ERROR (Status)) {
> +return NULL;
> +  }
> +
> +  Status = gBS->LocateDevicePath 
> + (,
> + , );  if (EFI_ERROR (Status) ||
> +  (DevicePathType (DevicePath.DevPath) != MESSAGING_DEVICE_PATH)
> ||
> +  (DevicePathSubType (DevicePath.DevPath) !=
> MSG_NVME_NAMESPACE_DP)) {
> +//
> +// Do not return description when the Handle is not a child of 
> + NVME
> controller.
> +//
> +return NULL;
> +  }
> +
> +  //
> +  // Send ADMIN_IDENTIFY command to NVME controller to get the model
> and serial number.
> +  //
> +  Status = gBS->HandleProtocol (Handle, 
> + , (VOID **) ); 
> + ASSERT_EFI_ERROR (Status);
> +
> +  ZeroMem (,
> + sizeof(EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET));
> +  ZeroMem (, sizeof(EFI_NVM_EXPRESS_COMMAND));
> ZeroMem
> + (, sizeof(EFI_NVM_EXPRESS_COMPLETION));
> +
> +  Command.Cdw0.Opcode = NVME_ADMIN_IDENTIFY_CMD;  //  //
> According to
> + Nvm Express 1.1 spec Figure 38, When not used, the field shall be 
> + cleared
> to 0h.
> +  // For the Identify command, the Namespace Identifier is only used 
> + for the
> Namespace 

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for NVME

2017-03-12 Thread Tian, Feng
There is no such library class/instance. It should be HPE-specific stuff.

Reviewed-by: Feng Tian 

Thanks
Feng

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, March 13, 2017 10:24 AM
To: Wang, Sunny (HPS SW) ; edk2-devel@lists.01.org
Cc: Tian, Feng ; Haskell, Darrell 
Subject: RE: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
description for NVME

I didn't find this library in EDKII.
When I firstly saw your comments, I was a little bit unwilling to change 
UefiBootManagerLib to depend on UefiNvmExpressLib. Because it would require all 
platforms to include UefiNvmExpressLib in DSC.
But since now I find no such library, I feel much better

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Sunny (HPS SW)
> Sent: Friday, March 10, 2017 6:11 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Tian, Feng ; Haskell, Darrell 
> 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib:
> Generate boot description for NVME
> 
> Hi Ray,
> Just a question out of curiosity. Why don't we use UefiNvmExpressLib's
> NvmExpressIdentify() for sending ADMIN_IDENTIFY command to NVME 
> controller?
> Others look good to me.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ruiyu Ni
> Sent: Friday, March 10, 2017 3:58 PM
> To: edk2-devel@lists.01.org
> Cc: Feng Tian 
> Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot 
> description for NVME
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Feng Tian 
> ---
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 104
> -
>  .../Library/UefiBootManagerLib/InternalBm.h|   4 +-
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
>  3 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> index 050647d..501a0cc 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> @@ -1,7 +1,7 @@
>  /** @file
>Library functions which relate with boot option description.
> 
> -Copyright (c) 2011 - 2016, Intel Corporation. All rights 
> reserved.
> +Copyright (c) 2011 - 2017, Intel Corporation. All rights 
> +reserved.
>  (C) Copyright 2015 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 @@ -501,6 
> +501,107 @@ BmGetLoadFileDescription (  }
> 
>  /**
> +  Return the boot description for NVME boot device.
> +
> +  @param HandleController handle.
> +
> +  @return  The description string.
> +**/
> +CHAR16 *
> +BmGetNvmeDescription (
> +  IN EFI_HANDLE  Handle
> +  )
> +{
> +  EFI_STATUS   Status;
> +  EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL   *NvmePassthru;
> +  EFI_DEV_PATH_PTR DevicePath;
> +  EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET CommandPacket;
> +  EFI_NVM_EXPRESS_COMMAND  Command;
> +  EFI_NVM_EXPRESS_COMPLETION   Completion;
> +  NVME_ADMIN_CONTROLLER_DATA   ControllerData;
> +  CHAR16   *Description;
> +  CHAR16   *Char;
> +  UINTNIndex;
> +
> +  Status = gBS->HandleProtocol (Handle, , 
> + (VOID **) );  if (EFI_ERROR (Status)) {
> +return NULL;
> +  }
> +
> +  Status = gBS->LocateDevicePath 
> + (,
> + , );  if (EFI_ERROR (Status) ||
> +  (DevicePathType (DevicePath.DevPath) != MESSAGING_DEVICE_PATH)
> ||
> +  (DevicePathSubType (DevicePath.DevPath) !=
> MSG_NVME_NAMESPACE_DP)) {
> +//
> +// Do not return description when the Handle is not a child of 
> + NVME
> controller.
> +//
> +return NULL;
> +  }
> +
> +  //
> +  // Send ADMIN_IDENTIFY command to NVME controller to get the model
> and serial number.
> +  //
> +  Status = gBS->HandleProtocol (Handle, 
> + , (VOID **) ); 
> + ASSERT_EFI_ERROR (Status);
> +
> +  ZeroMem (,
> + sizeof(EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET));
> +  ZeroMem (, sizeof(EFI_NVM_EXPRESS_COMMAND));
> ZeroMem
> + (, sizeof(EFI_NVM_EXPRESS_COMPLETION));
> +
> +  Command.Cdw0.Opcode = NVME_ADMIN_IDENTIFY_CMD;  //  //
> According to
> + Nvm Express 1.1 spec Figure 38, When not used, the field shall be 
> + cleared
> to 0h.
> +  // For the Identify command, the Namespace Identifier is only used 
> + for the
> Namespace data structure.
> +  //
> +  Command.Nsid 

Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate boot description for NVME

2017-03-12 Thread Ni, Ruiyu
I didn't find this library in EDKII.
When I firstly saw your comments, I was a little bit unwilling to change
UefiBootManagerLib to depend on UefiNvmExpressLib. Because it would
require all platforms to include UefiNvmExpressLib in DSC.
But since now I find no such library, I feel much better

Thanks/Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Wang, Sunny (HPS SW)
> Sent: Friday, March 10, 2017 6:11 PM
> To: Ni, Ruiyu ; edk2-devel@lists.01.org
> Cc: Tian, Feng ; Haskell, Darrell
> 
> Subject: Re: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib:
> Generate boot description for NVME
> 
> Hi Ray,
> Just a question out of curiosity. Why don't we use UefiNvmExpressLib's
> NvmExpressIdentify() for sending ADMIN_IDENTIFY command to NVME
> controller?
> Others look good to me.
> 
> Regards,
> Sunny Wang
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Friday, March 10, 2017 3:58 PM
> To: edk2-devel@lists.01.org
> Cc: Feng Tian 
> Subject: [edk2] [PATCH] MdeModulePkg/UefiBootManagerLib: Generate
> boot description for NVME
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni 
> Cc: Feng Tian 
> ---
>  .../Library/UefiBootManagerLib/BmBootDescription.c | 104
> -
>  .../Library/UefiBootManagerLib/InternalBm.h|   4 +-
>  .../UefiBootManagerLib/UefiBootManagerLib.inf  |   1 +
>  3 files changed, 107 insertions(+), 2 deletions(-)
> 
> diff --git
> a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> index 050647d..501a0cc 100644
> --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBootDescription.c
> @@ -1,7 +1,7 @@
>  /** @file
>Library functions which relate with boot option description.
> 
> -Copyright (c) 2011 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2011 - 2017, Intel Corporation. All rights reserved.
>  (C) Copyright 2015 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 @@ -501,6 +501,107 @@
> BmGetLoadFileDescription (  }
> 
>  /**
> +  Return the boot description for NVME boot device.
> +
> +  @param HandleController handle.
> +
> +  @return  The description string.
> +**/
> +CHAR16 *
> +BmGetNvmeDescription (
> +  IN EFI_HANDLE  Handle
> +  )
> +{
> +  EFI_STATUS   Status;
> +  EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL   *NvmePassthru;
> +  EFI_DEV_PATH_PTR DevicePath;
> +  EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET CommandPacket;
> +  EFI_NVM_EXPRESS_COMMAND  Command;
> +  EFI_NVM_EXPRESS_COMPLETION   Completion;
> +  NVME_ADMIN_CONTROLLER_DATA   ControllerData;
> +  CHAR16   *Description;
> +  CHAR16   *Char;
> +  UINTNIndex;
> +
> +  Status = gBS->HandleProtocol (Handle, ,
> + (VOID **) );  if (EFI_ERROR (Status)) {
> +return NULL;
> +  }
> +
> +  Status = gBS->LocateDevicePath (,
> + , );  if (EFI_ERROR (Status) ||
> +  (DevicePathType (DevicePath.DevPath) != MESSAGING_DEVICE_PATH)
> ||
> +  (DevicePathSubType (DevicePath.DevPath) !=
> MSG_NVME_NAMESPACE_DP)) {
> +//
> +// Do not return description when the Handle is not a child of NVME
> controller.
> +//
> +return NULL;
> +  }
> +
> +  //
> +  // Send ADMIN_IDENTIFY command to NVME controller to get the model
> and serial number.
> +  //
> +  Status = gBS->HandleProtocol (Handle,
> + , (VOID **) );
> + ASSERT_EFI_ERROR (Status);
> +
> +  ZeroMem (,
> + sizeof(EFI_NVM_EXPRESS_PASS_THRU_COMMAND_PACKET));
> +  ZeroMem (, sizeof(EFI_NVM_EXPRESS_COMMAND));
> ZeroMem
> + (, sizeof(EFI_NVM_EXPRESS_COMPLETION));
> +
> +  Command.Cdw0.Opcode = NVME_ADMIN_IDENTIFY_CMD;  //  //
> According to
> + Nvm Express 1.1 spec Figure 38, When not used, the field shall be cleared
> to 0h.
> +  // For the Identify command, the Namespace Identifier is only used for the
> Namespace data structure.
> +  //
> +  Command.Nsid= 0;
> +  CommandPacket.NvmeCmd= 
> +  CommandPacket.NvmeCompletion = 
> + CommandPacket.TransferBuffer = 
> + CommandPacket.TransferLength = sizeof (ControllerData);
> + CommandPacket.CommandTimeout = EFI_TIMER_PERIOD_SECONDS (5);
> +  CommandPacket.QueueType  = NVME_ADMIN_QUEUE;
> +  //
> +  // Set bit 0 (Cns bit) to 1 to identify a controller  //
> +  Command.Cdw10= 1;
> +  Command.Flags= CDW10_VALID;
> +
> +  Status = NvmePassthru->PassThru (
> 

Re: [edk2] overriding variables from cmdline used in fdf broken?

2017-03-12 Thread Gao, Liming
Michael:
  I agree this is a bug. Could you enter it into bugzillar?

Thanks
Liming
> -Original Message-
> From: Michael Zimmermann [mailto:sigmaepsilo...@gmail.com]
> Sent: Sunday, March 12, 2017 12:44 AM
> To: edk2-devel@lists.01.org 
> Cc: Zhu, Yonghong ; Gao, Liming 
> Subject: Re: [edk2] overriding variables from cmdline used in fdf broken?
> 
> I forgot to CC the maintainers back then so I'm doing this now.
> 
> Thanks
> Michael
> 
> On Thu, May 12, 2016 at 11:13 AM, Michael Zimmermann
>  wrote:
> > Hi,
> >
> > when I override or set a variable using -DNAME=VAL from the build cmdline
> > these are visible to the DSC file only, the FDF continues using the
> > old/undefined values - at least if you use them as a value, i.e.:
> > BaseAddress   = $(FD_BASE)|gArmTokenSpaceGuid.PcdFdBaseAddress
> >
> > is this intended behavior or a bug?
> >
> > Michael
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 0/3] Add Context in SmiHandlerProfileUnregister.

2017-03-12 Thread Fan, Jeff
Reviewed-by: Jeff Fan 

-Original Message-
From: Yao, Jiewen 
Sent: Friday, March 10, 2017 3:35 PM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff; Tian, Feng; Zeng, Star; Bret Barkelew
Subject: [PATCH 0/3] Add Context in SmiHandlerProfileUnregister.

This issue is reported by bret.barke...@microsoft.com.

We observe that a platform may use same Handler for different context.

In order to support Unregister such handler, we have to input context 
information as well.


The patch does not impact any platform with SmiHandlerProfile disabled.

Unit tests below:
1) register same handler with different context, and unregister each.
2) register and unregister UsbContext.

Cc: Jeff Fan 
Cc: Feng Tian 
Cc: Star Zeng 
Cc: Bret Barkelew 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 

Jiewen Yao (3):
  MdePkg/SmiHandlerProfile: Add Context support in Unregister
  MdeModulePkg/SmiHandlerProfile: Add Context support in Unregister
  MdeModulePkg/SmmCore: Add Context in SmiHandlerProfileUnregister.

 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h|   8 +-
 MdeModulePkg/Core/PiSmmCore/SmiHandlerProfile.c| 103 

 MdeModulePkg/Include/Guid/SmiHandlerProfile.h  |  41 
+++-
 MdeModulePkg/Library/SmmSmiHandlerProfileLib/SmmSmiHandlerProfileLib.c |  10 +-
 MdePkg/Include/Library/SmiHandlerProfileLib.h  |   8 +-
 MdePkg/Library/SmiHandlerProfileLibNull/SmiHandlerProfileLibNull.c |   8 +-
 6 files changed, 151 insertions(+), 27 deletions(-)

--
2.7.4.windows.1

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


Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute setting.

2017-03-12 Thread Fan, Jeff
Anthony,

It's very interesting.  Some INTEL IA-32 architectural MSR (like 0x1A0, 
IA32_MISC_ENABLE) is not supported by AMD processors.

For this case, I think reading 0x1A0 is not necessary. Reading 
CPUID.8001H:EDK[20] is enough to check if XD feature is supported or not.

I will create the patch to remove the following check.

  if ((AsmReadMsr64 (0x01A0) & BIT34) == 0) {
// XD enabled

I suggest to review such issue case by case when we encounter unsupported 
architectural MSRs on AMD processors in the future.

Thanks!
Jeff

-Original Message-
From: Anthony PERARD [mailto:anthony.per...@citrix.com] 
Sent: Friday, March 10, 2017 11:22 PM
To: Fan, Jeff
Cc: Yao, Jiewen; edk2-devel@lists.01.org; Kinney, Michael D
Subject: Re: [edk2] [PATCH V4 1/3] UefiCpuPkg/CpuDxe: Add memory attribute 
setting.

On Fri, Mar 10, 2017 at 01:02:22AM +, Fan, Jeff wrote:
> Anthony,
> 
> MSR 0x1A0 is architectural MSR defined in IA32 SDM.
> 
> Have you tried AMD real platform?

Yes, I did, and unless I've made a mistake, trying to read msr 0x1a0 does 
generate a GPF.

Also, I've now take a look into the AMD manuals[1] and more specificaly
"AMD64 Architecture Programmer’s Manual Volume 2: System Programming"[2], there 
is no sign of MSR 0x1a0 (in Appendix A).

As for the XD bit on AMD, (well NX actually), it is describe at 5.6.3, in the 
"Page Translation and Protection" chapter.


About MSR 0x1a0 been architectural defined, I guess that applies only to Intel 
CPUs, all of them.

[1]: http://developer.amd.com/resources/developer-guides-manuals/
[2]: http://support.amd.com/TechDocs/24593.pdf

Thanks,

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