[edk2] [Patch 2/2] MdeModulePkg V3: Addressing TCP Window Retraction when window scale factor is used.

[Fu Siyuan]

Change compare with V2 patch:
TcpRetransmit(): Use TCP_SEQ_BETWEEN instead of TCP_SEQ_GT to guarantee
  Tcb->SndWl2 + Tcb->SndWnd <= Seq <= Tcb->SndWl2 + Tcb->SndWnd + 
2^Rcv.Wind.Shift

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC3723 to address the TCP window retraction problem
when a non-zero scale factor is used.

Cc: Wu Jiaxin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c  |  5 +--
 .../Universal/Network/Tcp4Dxe/Tcp4Output.c | 42 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h |  8 -
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
index 1a7c41a..892d19b 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
@@ -1,7 +1,7 @@
 /** @file
   Misc support routines for tcp.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -78,7 +78,8 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
-
+  Tcb->RetxmitSeqMax = 0;
+  
   Tcb->ProbeTimerOn = FALSE;
 }
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
index 0eec8f0..5a3d837 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -671,17 +671,39 @@ TcpRetransmit (
   // 2. must in the current send window
   // 3. will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
-DEBUG ((EFI_D_WARN, "TcpRetransmit: retransmission cancelled "
-  "because send window too small for TCB %p\n", Tcb));
+
+  //
+  // Handle the Window Retraction if TCP window scale is enabled according to 
RFC7323:
+  //   On first retransmission, or if the sequence number is out of
+  //   window by less than 2^Rcv.Wind.Shift, then do normal
+  //   retransmission(s) without regard to the receiver window as long
+  //   as the original segment was in window when it was sent.
+  //
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_BETWEEN (Tcb->SndWl2 + 
Tcb->SndWnd, Seq, Tcb->SndWl2 + Tcb->SndWnd + (1 << Tcb->SndWndScale {
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n",
+  Tcb)
+  );
 
 return 0;
   }
+  
+  Len = MIN (Len, Tcb->SndMss);
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
-
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -691,6 +713,10 @@ TcpRetransmit (
   if (TcpTransmitSegment (Tcb, Nbuf) != 0) {
 goto OnError;
   }
+  
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
 
   //
   // The retransmitted buffer may be on the SndQue,
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
index 01d6034..49d8a1d 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
@@ -1,7 +1,7 @@
 /** @file
   Tcp Protocol header file.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -251,6 +251,12 @@ struct _TCP_CB {
   UINT32

[edk2] [Patch 0/2] Addressing TCP Window Retraction when window scale factor is used.

[Fu Siyuan]

Fu Siyuan (2):
  NetworkPkg V3: Addressing TCP Window Retraction when window scale
factor is used.
  MdeModulePkg V3: Addressing TCP Window Retraction when window scale
factor is used.

 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c  |  5 +--
 .../Universal/Network/Tcp4Dxe/Tcp4Output.c | 42 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h |  8 -
 NetworkPkg/TcpDxe/TcpMisc.c|  3 +-
 NetworkPkg/TcpDxe/TcpOutput.c  | 33 ++---
 NetworkPkg/TcpDxe/TcpProto.h   |  8 -
 6 files changed, 81 insertions(+), 18 deletions(-)

-- 
1.9.5.msysgit.1

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

[edk2] [Patch 1/2] NetworkPkg V3: Addressing TCP Window Retraction when window scale factor is used.

[Fu Siyuan]

Change compare with V2 patch:
TcpRetransmit(): Use TCP_SEQ_BETWEEN instead of TCP_SEQ_GT to guarantee
  Tcb->SndWl2 + Tcb->SndWnd <= Seq <= Tcb->SndWl2 + Tcb->SndWnd + 
2^Rcv.Wind.Shift

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC3723 to address the TCP window retraction problem
when a non-zero scale factor is used.

Cc: Wu Jiaxin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/TcpDxe/TcpMisc.c   |  3 ++-
 NetworkPkg/TcpDxe/TcpOutput.c | 33 -
 NetworkPkg/TcpDxe/TcpProto.h  |  8 +++-
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c
index a8592c9..4435036 100644
--- a/NetworkPkg/TcpDxe/TcpMisc.c
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
@@ -2,7 +2,7 @@
   Misc support routines for TCP driver.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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
@@ -86,6 +86,7 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
+  Tcb->RetxmitSeqMax = 0;
 
   Tcb->ProbeTimerOn = FALSE;
 }
diff --git a/NetworkPkg/TcpDxe/TcpOutput.c b/NetworkPkg/TcpDxe/TcpOutput.c
index a46cb60..a7e59f0 100644
--- a/NetworkPkg/TcpDxe/TcpOutput.c
+++ b/NetworkPkg/TcpDxe/TcpOutput.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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
@@ -664,7 +664,27 @@ TcpRetransmit (
   // 2. Must in the current send window
   // 3. Will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+
+  //
+  // Handle the Window Retraction if TCP window scale is enabled according to 
RFC7323:
+  //   On first retransmission, or if the sequence number is out of
+  //   window by less than 2^Rcv.Wind.Shift, then do normal
+  //   retransmission(s) without regard to the receiver window as long
+  //   as the original segment was in window when it was sent.
+  //
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_BETWEEN (Tcb->SndWl2 + 
Tcb->SndWnd, Seq, Tcb->SndWl2 + Tcb->SndWnd + (1 << Tcb->SndWndScale {
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
 DEBUG (
   (EFI_D_WARN,
   "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n",
@@ -674,10 +694,9 @@ TcpRetransmit (
 return 0;
   }
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
+  Len = MIN (Len, Tcb->SndMss);
 
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -688,6 +707,10 @@ TcpRetransmit (
 goto OnError;
   }
 
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
+
   //
   // The retransmitted buffer may be on the SndQue,
   // trim TCP head because all the buffers on SndQue
diff --git a/NetworkPkg/TcpDxe/TcpProto.h b/NetworkPkg/TcpDxe/TcpProto.h
index ee35134..81397d7 100644
--- a/NetworkPkg/TcpDxe/TcpProto.h
+++ b/NetworkPkg/TcpDxe/TcpProto.h
@@ -1,7 +1,7 @@
 /** @file
   TCP protocol header file.
 
-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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
@@ -316,6 +316,12 @@ struct _TCP_CONTROL_BLOCK {
   TCP_SEQNO LossRecover;  ///< Recover point for retxmit.
 
   //
+  // RFC7323
+  // Addressing Window Retraction for TCP Window Scale Option.
+  //
+  TCP_SEQNO RetxmitSeqMax;   ///< Max Seq number in previous 
retransmission.
+
+  //
   // configuration parameters, for EFI_TCP4_PROTOCOL specification
   //
   UINT32KeepAliveIdle;   ///< Idle time before sending first probe.
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch] NetworkPkg: Update package version to 0.97.

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin 


> -Original Message-
> From: Fu, Siyuan
> Sent: Wednesday, May 3, 2017 3:54 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Jiaxin ; Ye, Ting 
> Subject: [Patch] NetworkPkg: Update package version to 0.97.
> 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Fu Siyuan 
> ---
>  NetworkPkg/NetworkPkg.dec | 2 +-
>  NetworkPkg/NetworkPkg.dsc | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
> index 28558d3..b502f90 100644
> --- a/NetworkPkg/NetworkPkg.dec
> +++ b/NetworkPkg/NetworkPkg.dec
> @@ -20,7 +20,7 @@
>DEC_SPECIFICATION  = 0x00010005
>PACKAGE_NAME   = NetworkPkg
>PACKAGE_GUID   = 947988BE-8D5C-471a-893D-AD181C46BEBB
> -  PACKAGE_VERSION= 0.96
> +  PACKAGE_VERSION= 0.97
>PACKAGE_UNI_FILE   = NetworkPkg.uni
> 
>  [Includes]
> diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
> index b8e071b..56a1a6b 100644
> --- a/NetworkPkg/NetworkPkg.dsc
> +++ b/NetworkPkg/NetworkPkg.dsc
> @@ -2,7 +2,7 @@
>  # UEFI 2.4 Network Module Package for All Architectures
>  #
>  # (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
> -# Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2009 - 2017, 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
> @@ -17,7 +17,7 @@
>  [Defines]
>PLATFORM_NAME  = NetworkPkg
>PLATFORM_GUID  = 3FD34E9B-E90C-44e1-B510-1F632A509F10
> -  PLATFORM_VERSION   = 0.96
> +  PLATFORM_VERSION   = 0.97
>DSC_SPECIFICATION  = 0x00010005
>OUTPUT_DIRECTORY   = Build/NetworkPkg
>SUPPORTED_ARCHITECTURES= IA32|IPF|X64|EBC|ARM|AARCH64
> --
> 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] NetworkPkg: Fix issue in dns driver when building DHCP packet.

[Wu, Jiaxin]

Reviewed-by: Wu Jiaxin 



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Zhang Lubo
> Sent: Thursday, May 4, 2017 5:36 PM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting ; Fu, Siyuan ; Wu, Jiaxin
> 
> Subject: [edk2] [patch] NetworkPkg: Fix issue in dns driver when building DHCP
> packet.
> 
> Currently, DNS driver configure the dhcp message type to inform
> when building dhcp packet to get dns info from, but it not works
> with dhcp server deployed on linux system. However it works well
> when changed to request type.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Zhang Lubo 
> Cc: Wu Jiaxin 
> Cc: Ye Ting 
> Cc: Fu Siyuan 
> ---
>  NetworkPkg/DnsDxe/DnsDhcp.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/NetworkPkg/DnsDxe/DnsDhcp.c b/NetworkPkg/DnsDxe/DnsDhcp.c
> index c4add70..93779be 100644
> --- a/NetworkPkg/DnsDxe/DnsDhcp.c
> +++ b/NetworkPkg/DnsDxe/DnsDhcp.c
> @@ -1,9 +1,9 @@
>  /** @file
>  Functions implementation related with DHCPv4/v6 for DNS driver.
> 
> -Copyright (c) 2015, Intel Corporation. All rights reserved.
> +Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions of the BSD
> License
>  which accompanies this distribution.  The full text of the license may be 
> found
> at
>  http://opensource.org/licenses/bsd-license.php
> 
> @@ -460,11 +460,11 @@ GetDns4ServerFromDhcp4 (
>  goto ON_EXIT;
>}
> 
>ParaList[0]->OpCode  = DHCP4_TAG_TYPE;
>ParaList[0]->Length  = 1;
> -  ParaList[0]->Data[0] = DHCP4_MSG_INFORM;
> +  ParaList[0]->Data[0] = DHCP4_MSG_REQUEST;
> 
>ParaList[1] = AllocateZeroPool (sizeof (EFI_DHCP4_PACKET_OPTION));
>if (ParaList[1] == NULL) {
>  Status = EFI_OUT_OF_RESOURCES;
>  goto ON_EXIT;
> --
> 1.9.5.msysgit.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch V2] MdeModulePkg: Addressing TCP Window Retraction when window scale factor is used.

[Ye, Ting]

Reviewed-by: Ye Ting   

-Original Message-
From: Fu, Siyuan 
Sent: Friday, May 05, 2017 11:01 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Andrey Tepin ; Ye, 
Ting 
Subject: [Patch V2] MdeModulePkg: Addressing TCP Window Retraction when window 
scale factor is used.

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC7323 to address the TCP window retraction 
problem when a non-zero scale factor is used.
The changes has been test in high packet loss rate network by using HTTP boot 
and iSCSI file read/write.

Cc: Wu Jiaxin 
Cc: Andrey Tepin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c  |  5 +--
 .../Universal/Network/Tcp4Dxe/Tcp4Output.c | 41 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h |  8 -
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
index 1a7c41a..892d19b 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
@@ -1,7 +1,7 @@
 /** @file
   Misc support routines for tcp.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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 @@ -78,7 +78,8 @@ 
TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
-
+  Tcb->RetxmitSeqMax = 0;
+  
   Tcb->ProbeTimerOn = FALSE;
 }
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
index 0eec8f0..ed71f97 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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 @@ -671,17 +671,38 
@@ TcpRetransmit (
   // 2. must in the current send window
   // 3. will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
-DEBUG ((EFI_D_WARN, "TcpRetransmit: retransmission cancelled "
-  "because send window too small for TCB %p\n", Tcb));
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_GT (Tcb->SndWl2 + 
Tcb->SndWnd + (1 << Tcb->SndWndScale), Seq))) {
+//
+// Handle the Window Retraction if TCP window scale is enabled according 
to RFC7323:
+//   On first retransmission, or if the sequence number is out of
+//   window by less than 2^Rcv.Wind.Shift, then do normal
+//   retransmission(s) without regard to the receiver window as long
+//   as the original segment was in window when it was sent.
+//
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n",
+  Tcb)
+  );
 
 return 0;
   }
+  
+  Len = MIN (Len, Tcb->SndMss);
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
-
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -691,6 +712,10 @@ TcpRetransmit (
   if (TcpTransmitSegment (Tcb, Nbuf) != 0) {
 goto OnError;
   }
+  
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
 
   //
   // The retransmitted buffer may be on the SndQue, diff --git 
a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
index 01d6034..49d8a1d 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
@@ -1,7 +1,7 @@
 /** @file
   Tcp Protocol header file.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, Intel 

Re: [edk2] [Patch V2] NetworkPkg: Addressing TCP Window Retraction when window scale factor is used.

[Ye, Ting]

Reviewed-by: Ye Ting 


-Original Message-
From: Fu, Siyuan 
Sent: Friday, May 05, 2017 9:56 AM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Andrey Tepin ; Ye, 
Ting 
Subject: [Patch V2] NetworkPkg: Addressing TCP Window Retraction when window 
scale factor is used.

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC7323 to address the TCP window retraction 
problem when a non-zero scale factor is used.
The changes has been test in high packet loss rate network by using HTTP boot 
and iSCSI file read/write.

Cc: Wu Jiaxin 
Cc: Andrey Tepin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/TcpDxe/TcpMisc.c   |  3 ++-
 NetworkPkg/TcpDxe/TcpOutput.c | 33 -  
NetworkPkg/TcpDxe/TcpProto.h  |  8 +++-
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c index 
a8592c9..4435036 100644
--- a/NetworkPkg/TcpDxe/TcpMisc.c
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
@@ -2,7 +2,7 @@
   Misc support routines for TCP driver.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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 @@ -86,6 +86,7 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
+  Tcb->RetxmitSeqMax = 0;
 
   Tcb->ProbeTimerOn = FALSE;
 }
diff --git a/NetworkPkg/TcpDxe/TcpOutput.c b/NetworkPkg/TcpDxe/TcpOutput.c 
index a46cb60..f3dacf3 100644
--- a/NetworkPkg/TcpDxe/TcpOutput.c
+++ b/NetworkPkg/TcpDxe/TcpOutput.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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 @@ -664,7 +664,27 @@ TcpRetransmit (
   // 2. Must in the current send window
   // 3. Will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+  
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_GT (Tcb->SndWl2 + 
Tcb->SndWnd + (1 << Tcb->SndWndScale), Seq))) {
+//
+// Handle the Window Retraction if TCP window scale is enabled according 
to RFC7323:
+//   On first retransmission, or if the sequence number is out of
+//   window by less than 2^Rcv.Wind.Shift, then do normal
+//   retransmission(s) without regard to the receiver window as long
+//   as the original segment was in window when it was sent.
+//
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
 DEBUG (
   (EFI_D_WARN,
   "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n", @@ -674,10 +694,9 @@ TcpRetransmit (
 return 0;
   }
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
+  Len = MIN (Len, Tcb->SndMss);
 
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -688,6 +707,10 @@ TcpRetransmit (
 goto OnError;
   }
 
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
+
   //
   // The retransmitted buffer may be on the SndQue,
   // trim TCP head because all the buffers on SndQue diff --git 
a/NetworkPkg/TcpDxe/TcpProto.h b/NetworkPkg/TcpDxe/TcpProto.h index 
ee35134..81397d7 100644
--- a/NetworkPkg/TcpDxe/TcpProto.h
+++ b/NetworkPkg/TcpDxe/TcpProto.h
@@ -1,7 +1,7 @@
 /** @file
   TCP protocol header file.
 
-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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 @@ -316,6 +316,12 @@ struct _TCP_CONTROL_BLOCK {
   TCP_SEQNO LossRecover;  ///< Recover point for retxmit.
 
   //
+  // RFC7323
+  // Addressing Window Retraction for TCP Window Scale Option.
+  //
+  TCP_SEQNO RetxmitSeqMax;   ///< Max Seq number in previous 
retransmission.
+
+  //
   // configuration parameters, for 

[edk2] How to build Open UEFI for Intel Xeon Server v3

[Dhanasekar J]

Hi All,

I am trying to build BIOS for Inte Xeon Server v3. But I am not seeing
PlatformPkg for v3 in https://github.com/tianocore/edk2.


Where can I get PlatformPkg for v3?

Is it possible to boot Intel Server by open UEFI source without BIOS vendor
help?.

Is Intel providing FSP  support Xeon server v3?.

Can I able to boot xeon sever v3 with FSP?


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

Re: [edk2] [PATCH v2 3/5] OvmfPkg: introduce 4MB flash image (mainly) for Windows HCK

[Laszlo Ersek]

On 05/03/17 23:39, Laszlo Ersek wrote:
> The "Confirm64KilobytesOfUnauthenticatedVariableStorage" test case of the
> Secure Boot Logo Test ("Microsoft.UefiSecureBootLogo.Tests") suite in the
> Microsoft Hardware Certification Kit expects to be able to populate the
> variable store up to roughly 64 KB, with a series of 1 KB sized,
> unauthenticated variables. OVMF's current live varstore area is too small
> for this: 56 KB.
> 
> Introduce the FD_SIZE_4MB build macro (equivalently, FD_SIZE_IN_KB=4096),
> which
> 
> - enlarges the full flash image to 4MB -- QEMU supports up to 8MB, see
>   FLASH_MAP_BASE_MIN in "hw/i386/pc_sysfw.c" --,
> 
> - inside that, grows the varstore area / pflash chip to 528 KB, and within
>   it, the live area from 56 KB to 256 KB.
> 
> Importantly, a firmware binary built with -D FD_SIZE_4MB will *not* be
> compatible with a variable store that originates from a variable store
> template built *without* -D FD_SIZE_4MB. This is the reason for the large
> increase, as every such change breaks compatibility between a new firmware
> binary and old varstore files.
> 
> Enlarging the varstore does not impact the performance of normal
> operations, as we keep the varstore block size 4KB. The performance of
> reclaim is affected, but that is expected (since reclaim has to rework the
> full live area). And, reclaim occurs proportionally less frequently.
> 
> While at it, the FVMAIN_COMPACT volume (with the compressed FFS file in
> it) is also enlarged significantly, so that we have plenty of room for
> future DXEFV (and perhaps PEIFV) increments -- DXEFV has been growing
> steadily, and that increase shows through compression too. Right now the
> PEIFV and DXEFV volumes need no resizing.
> 
> Here's a summary:
> 
>   DescriptionCompression typeSize [KB]
>   -  -  --
>   Non-volatile data storage  open-coded binary128 ->   528 ( +400)
>data
> Variable store 56 ->   256 ( +200)
> Event log   4 -> 4 (   +0)
> Working block   4 -> 4 (   +0)
> Spare area 64 ->   264 ( +200)
> 
>   FVMAIN_COMPACT uncompressed1712 ->  3360 (+1648)
> FV FFS file  LZMA compressed
>   PEIFV  uncompressed 896 ->   896 (   +0)
> individual PEI   uncompressed
>   modules
>   DXEFV  uncompressed   10240 -> 10240 (   +0)
> individual DXE   uncompressed
>   modules
> 
>   SECFV  uncompressed 208 ->   208 (   +0)
> SEC driver
> reset vector code
> 
> For now, the 2MB flash image remains the default.
> 
> Cc: Gary Ching-Pang Lin 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> v2:
> - use $(FD_SIZE_IN_KB) in conditional statements
> 
> - Raise VARS_LIVE_SIZE by 8KB to 256KB, VARS_SPARE_SIZE by 8KB to 264KB,

Unfortunately, we have a terrible regression here. :(

It has to do with the emulated variable store.

The OvmfPkg/EmuVariableFvbRuntimeDxe driver exposes a two-block flash
device (FVB protocol) that uses the NV spare area size as internal block
size. And thereby covers the full NV area with just two blocks in total.

In addition, that driver keeps cutting corners everywhere, exploiting
very heavily that there are just two blocks. This assumption is
open-coded all over it.

This is also why we have the exact allocation logic in
ReserveEmuVariableNvStore() that we have, in PlatformPei.

The AllocateAlignedRuntimePages() call in ReserveEmuVariableNvStore()
depends on the NV spare area size being a power of two, because only
such values are accepted as Alignment.

And the FTW driver that consumes the FVB from EmuVariableFvbRuntimeDxe
also expects the FVB block size to be a power of two, which again
translates to the NV spare area size having to be a power of two.
(Because that's what EmuVariableFvbRuntimeDxe exposes as FVB block size,
for "all two" of its blocks.)

Changing the spare area size to 264KB breaks this extremely.

Booting the 4MB image with -bios is completely broken. I've been working
for a few hours on it now, but it's not just about refining the
granularity of the internal block size of the EmuVariableFvbRuntimeDxe
driver, to 4KB. As I said, the driver open-codes the two-block
assumption everywhere. For example, in the FvbProtocolEraseBlocks()
function, a complete loop is missing. :(

Adapting EmuVariableFvbRuntimeDxe to a non-power-of-two NV spare area
size is hopeless. (Definitely hopeless for the time frame & resources
I'm looking at.)

Worse, even -pflash is broken in the 4MB build, actually. The

[edk2] How to build Open UEFI for Intel Xeon Server v3

[Dhanasekar Jaganathan]

Hi All,
I am trying to build BIOS for Inte Xeon Server v3. But I am not seeing
PlatformPkg for v3 in https://github.com/tianocore/edk2.
Where can I get PlatformPkg for v3?

Is it possible to boot Intel Server by open UEFI source without BIOS vendor
help?.
Is Intel providing FSP  support Xeon server v3?.
Can I able to boot xeon sever v3 with FSP?

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

Re: [edk2] [Patch] Nt32Pkg/SnpNt32Dxe: Fix hang issue when multiple network interfaces existed

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Jiaxin Wu
Sent: 2017年5月5日 11:31
To: edk2-devel@lists.01.org
Cc: Ye, Ting ; Ni, Ruiyu ; Fu, Siyuan 
; Wu, Jiaxin 
Subject: [edk2] [Patch] Nt32Pkg/SnpNt32Dxe: Fix hang issue when multiple 
network interfaces existed

Currently all the network interfaces share the one recycled transmit buffer 
array, which is used to store the recycled buffer address. However, those 
recycled buffers are allocated by the different MNP interface if the multiple 
network interfaces existed. Then, SNP GetStatus may return one recycled 
transmit buffer address to the another MNP interface, which may result in the 
MNP driver hang after 'reconnect -r' operation.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 Nt32Pkg/SnpNt32Dxe/SnpNt32.c | 50 ++--
 Nt32Pkg/SnpNt32Dxe/SnpNt32.h | 31 ++-
 2 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c index 
9018800..c1ef324 100644
--- a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
+++ b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials  are licensed and made available 
under the terms and conditions of the BSD License  which accompanies this 
distribution.  The full text of the license may be found at  
http://opensource.org/licenses/bsd-license.php
 
@@ -42,13 +42,10 @@ SNPNT32_GLOBAL_DATA gSnpNt32GlobalData = {
   {
 0,
 0,
 EfiLockUninitialized
   },  //  Lock
-  NULL,   //  RecycledTxBuf
-  0,  //  RecycledTxBufCount
-  32, //  MaxRecycledTxBuf
   //
   //  Private functions
   //
   SnpNt32InitializeGlobalData,//  InitializeGlobalData
   SnpNt32InitializeInstanceData,  //  InitializeInstanceData
@@ -394,10 +391,13 @@ SNPNT32_INSTANCE_DATA gSnpNt32InstanceTemplate = {
   SNP_NT32_INSTANCE_SIGNATURE,//  Signature
   {
 NULL,
 NULL
   },  //  Entry
+  NULL,   //  RecycledTxBuf
+  0,  //  RecycledTxBufCount
+  32, //  MaxRecycledTxBuf
   NULL,   //  GlobalData
   NULL,   //  DeviceHandle
   NULL,   //  DevicePath
   {   //  Snp
 EFI_SIMPLE_NETWORK_PROTOCOL_REVISION, //  Revision @@ -863,20 +863,17 @@ 
SnpNt32GetStatus (
   OUT UINT32 *InterruptStatus,
   OUT VOID   **TxBuffer
   )
 {
   SNPNT32_INSTANCE_DATA *Instance;
-  SNPNT32_GLOBAL_DATA   *GlobalData;
 
   Instance= SNP_NT32_INSTANCE_DATA_FROM_SNP_THIS (This);
 
-  GlobalData  = Instance->GlobalData;
-
   if (TxBuffer != NULL) {
-if (GlobalData->RecycledTxBufCount != 0) {
-  GlobalData->RecycledTxBufCount --;
-  *((UINT8 **) TxBuffer)= (UINT8 *) 
(UINTN)GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount];
+if (Instance->RecycledTxBufCount != 0) {
+  Instance->RecycledTxBufCount --;
+  *((UINT8 **) TxBuffer)= (UINT8 *) 
(UINTN)Instance->RecycledTxBuf[Instance->RecycledTxBufCount];
 } else {
   *((UINT8 **) TxBuffer)= NULL;
 }
   }
 
@@ -959,26 +956,26 @@ SnpNt32Transmit (
   EfiReleaseLock (>Lock);
 
   if (ReturnValue < 0) {
 return EFI_DEVICE_ERROR;
   } else {
-if ((GlobalData->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= 
SNP_MAX_TX_BUFFER_NUM) {
+if ((Instance->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= 
+ SNP_MAX_TX_BUFFER_NUM) {
   return EFI_NOT_READY;
 }
 
-if (GlobalData->RecycledTxBufCount < GlobalData->MaxRecycledTxBuf) {
-  GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount] = (UINT64) 
Buffer;
-  GlobalData->RecycledTxBufCount ++;
+if (Instance->RecycledTxBufCount < Instance->MaxRecycledTxBuf) {
+  Instance->RecycledTxBuf[Instance->RecycledTxBufCount] = (UINT64) Buffer;
+  Instance->RecycledTxBufCount ++;
 } else {
-  Tmp = AllocatePool (sizeof (UINT64) * (GlobalData->MaxRecycledTxBuf + 
SNP_TX_BUFFER_INCREASEMENT));
+  Tmp = AllocatePool (sizeof (UINT64) * (Instance->MaxRecycledTxBuf 
+ + SNP_TX_BUFFER_INCREASEMENT));
   if (Tmp == NULL) {
 return EFI_DEVICE_ERROR;
   }
-  CopyMem (Tmp, GlobalData->RecycledTxBuf, sizeof (UINT64) 

[edk2] [Patch] Nt32Pkg/SnpNt32Dxe: Fix hang issue when multiple network interfaces existed

[Jiaxin Wu]

Currently all the network interfaces share the one recycled transmit buffer
array, which is used to store the recycled buffer address. However, those
recycled buffers are allocated by the different MNP interface if the multiple
network interfaces existed. Then, SNP GetStatus may return one recycled transmit
buffer address to the another MNP interface, which may result in the MNP driver
hang after 'reconnect -r' operation.

Cc: Ye Ting 
Cc: Fu Siyuan 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin 
---
 Nt32Pkg/SnpNt32Dxe/SnpNt32.c | 50 ++--
 Nt32Pkg/SnpNt32Dxe/SnpNt32.h | 31 ++-
 2 files changed, 41 insertions(+), 40 deletions(-)

diff --git a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
index 9018800..c1ef324 100644
--- a/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
+++ b/Nt32Pkg/SnpNt32Dxe/SnpNt32.c
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -42,13 +42,10 @@ SNPNT32_GLOBAL_DATA gSnpNt32GlobalData = {
   {
 0,
 0,
 EfiLockUninitialized
   },  //  Lock
-  NULL,   //  RecycledTxBuf
-  0,  //  RecycledTxBufCount
-  32, //  MaxRecycledTxBuf
   //
   //  Private functions
   //
   SnpNt32InitializeGlobalData,//  InitializeGlobalData
   SnpNt32InitializeInstanceData,  //  InitializeInstanceData
@@ -394,10 +391,13 @@ SNPNT32_INSTANCE_DATA gSnpNt32InstanceTemplate = {
   SNP_NT32_INSTANCE_SIGNATURE,//  Signature
   {
 NULL,
 NULL
   },  //  Entry
+  NULL,   //  RecycledTxBuf
+  0,  //  RecycledTxBufCount
+  32, //  MaxRecycledTxBuf
   NULL,   //  GlobalData
   NULL,   //  DeviceHandle
   NULL,   //  DevicePath
   {   //  Snp
 EFI_SIMPLE_NETWORK_PROTOCOL_REVISION, //  Revision
@@ -863,20 +863,17 @@ SnpNt32GetStatus (
   OUT UINT32 *InterruptStatus,
   OUT VOID   **TxBuffer
   )
 {
   SNPNT32_INSTANCE_DATA *Instance;
-  SNPNT32_GLOBAL_DATA   *GlobalData;
 
   Instance= SNP_NT32_INSTANCE_DATA_FROM_SNP_THIS (This);
 
-  GlobalData  = Instance->GlobalData;
-
   if (TxBuffer != NULL) {
-if (GlobalData->RecycledTxBufCount != 0) {
-  GlobalData->RecycledTxBufCount --;
-  *((UINT8 **) TxBuffer)= (UINT8 *) 
(UINTN)GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount];
+if (Instance->RecycledTxBufCount != 0) {
+  Instance->RecycledTxBufCount --;
+  *((UINT8 **) TxBuffer)= (UINT8 *) 
(UINTN)Instance->RecycledTxBuf[Instance->RecycledTxBufCount];
 } else {
   *((UINT8 **) TxBuffer)= NULL;
 }
   }
 
@@ -959,26 +956,26 @@ SnpNt32Transmit (
   EfiReleaseLock (>Lock);
 
   if (ReturnValue < 0) {
 return EFI_DEVICE_ERROR;
   } else {
-if ((GlobalData->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= 
SNP_MAX_TX_BUFFER_NUM) {
+if ((Instance->MaxRecycledTxBuf + SNP_TX_BUFFER_INCREASEMENT) >= 
SNP_MAX_TX_BUFFER_NUM) {
   return EFI_NOT_READY;
 }
 
-if (GlobalData->RecycledTxBufCount < GlobalData->MaxRecycledTxBuf) {
-  GlobalData->RecycledTxBuf[GlobalData->RecycledTxBufCount] = (UINT64) 
Buffer;
-  GlobalData->RecycledTxBufCount ++;
+if (Instance->RecycledTxBufCount < Instance->MaxRecycledTxBuf) {
+  Instance->RecycledTxBuf[Instance->RecycledTxBufCount] = (UINT64) Buffer;
+  Instance->RecycledTxBufCount ++;
 } else {
-  Tmp = AllocatePool (sizeof (UINT64) * (GlobalData->MaxRecycledTxBuf + 
SNP_TX_BUFFER_INCREASEMENT));
+  Tmp = AllocatePool (sizeof (UINT64) * (Instance->MaxRecycledTxBuf + 
SNP_TX_BUFFER_INCREASEMENT));
   if (Tmp == NULL) {
 return EFI_DEVICE_ERROR;
   }
-  CopyMem (Tmp, GlobalData->RecycledTxBuf, sizeof (UINT64) * 
GlobalData->RecycledTxBufCount);
-  FreePool (GlobalData->RecycledTxBuf);
-  GlobalData->RecycledTxBuf=  Tmp;
-  GlobalData->MaxRecycledTxBuf += SNP_TX_BUFFER_INCREASEMENT;
+  CopyMem (Tmp, Instance->RecycledTxBuf, sizeof (UINT64) * 
Instance->RecycledTxBufCount);
+  FreePool (Instance->RecycledTxBuf);
+  Instance->RecycledTxBuf=  Tmp;
+  Instance->MaxRecycledTxBuf += SNP_TX_BUFFER_INCREASEMENT;
 }
   }
 
  

[edk2] [Patch V2] MdeModulePkg: Addressing TCP Window Retraction when window scale factor is used.

[Fu Siyuan]

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC7323 to address the TCP window retraction problem
when a non-zero scale factor is used.
The changes has been test in high packet loss rate network by using HTTP boot 
and 
iSCSI file read/write.

Cc: Wu Jiaxin 
Cc: Andrey Tepin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c  |  5 +--
 .../Universal/Network/Tcp4Dxe/Tcp4Output.c | 41 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h |  8 -
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
index 1a7c41a..892d19b 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
@@ -1,7 +1,7 @@
 /** @file
   Misc support routines for tcp.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -78,7 +78,8 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
-
+  Tcb->RetxmitSeqMax = 0;
+  
   Tcb->ProbeTimerOn = FALSE;
 }
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
index 0eec8f0..ed71f97 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -671,17 +671,38 @@ TcpRetransmit (
   // 2. must in the current send window
   // 3. will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
-DEBUG ((EFI_D_WARN, "TcpRetransmit: retransmission cancelled "
-  "because send window too small for TCB %p\n", Tcb));
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_GT (Tcb->SndWl2 + 
Tcb->SndWnd + (1 << Tcb->SndWndScale), Seq))) {
+//
+// Handle the Window Retraction if TCP window scale is enabled according 
to RFC7323:
+//   On first retransmission, or if the sequence number is out of
+//   window by less than 2^Rcv.Wind.Shift, then do normal
+//   retransmission(s) without regard to the receiver window as long
+//   as the original segment was in window when it was sent.
+//
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n",
+  Tcb)
+  );
 
 return 0;
   }
+  
+  Len = MIN (Len, Tcb->SndMss);
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
-
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -691,6 +712,10 @@ TcpRetransmit (
   if (TcpTransmitSegment (Tcb, Nbuf) != 0) {
 goto OnError;
   }
+  
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
 
   //
   // The retransmitted buffer may be on the SndQue,
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
index 01d6034..49d8a1d 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
@@ -1,7 +1,7 @@
 /** @file
   Tcp Protocol header file.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -251,6 +251,12 @@ struct _TCP_CB {
   UINT32ConnectTimeout;  ///< The connect establishment time out
 
   

Re: [edk2] [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info

[Ni, Ruiyu]

Jeff,
Firstly great thanks for filling the gap between EDK shell and UEFI shell.
There are many behavior differences between the two,  and EDK Shell does carry 
much more information that
are very helpful for developer debugging.

Before I check in your patch, could you please kindly review whether the 
additional change is ok to you.
One change is to fix the ECC failure: "python 
BaseTools\Source\Python\Ecc\Ecc.py -t ShellPkg\Library\UefiHandleParsingLib -s"

ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272): 
[3002]Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, 
<=) Predicate Expression: FileName
ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272): [3003]A 
comparison of any pointer to zero must be done via the NULL type Predicate 
Expression: FileName

patch content===
.../UefiHandleParsingLib/UefiHandleParsingLib.c| 39 --
 1 file changed, 14 insertions(+), 25 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index 74934f8617..d3ee068eba 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -16,14 +16,14 @@
 
 #include "UefiHandleParsingLib.h"
 #include "IndustryStandard/Acpi10.h"
-#include "Pi/PiFirmwareFile.h"
-#include "Pi/PiFirmwareVolume.h"
-#include "Protocol/FirmwareVolume2.h"
+#include 
+#include 
 
 EFI_HANDLEmHandleParsingHiiHandle = NULL;
 HANDLE_INDEX_LIST mHandleList = {{{NULL,NULL},0,0},0};
 GUID_INFO_BLOCK   *mGuidList;
 UINTN mGuidListCount;
+
 /**
   Function to find the file name associated with a LoadedImageProtocol.
 
@@ -44,37 +44,26 @@ FindLoadedImageFileName (
   UINTN  BufferSize;
   UINT32 AuthenticationStatus;
 
-  //
-  // Only subtype MEDIA_PIWG_FW_FILE_DP of type MEDIA_DEVICE_PATH is supported.
-  //
-  if (   (LoadedImage == NULL)
-  || (LoadedImage->FilePath == NULL)
-  || (LoadedImage->FilePath->Type != MEDIA_DEVICE_PATH)
-  || (LoadedImage->FilePath->SubType != MEDIA_PIWG_FW_FILE_DP)
-   ) {
-return (NULL);
+  if ((LoadedImage == NULL) || (LoadedImage->FilePath == NULL)) {
+return NULL;
   }
 
   NameGuid = 
EfiGetNameGuidFromFwVolDevicePathNode((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH 
*)LoadedImage->FilePath);
 
   if (NameGuid == NULL) {
-return (NULL);
+return NULL;
   }
 
   //
-  // Get the FirmwareVolume2Protocol of the device handle that this image was
-  // loaded from.
+  // Get the FirmwareVolume2Protocol of the device handle that this image was 
loaded from.
   //
-  Status = gBS->HandleProtocol(
-LoadedImage->DeviceHandle,
-,
-(VOID**));
+  Status = gBS->HandleProtocol (LoadedImage->DeviceHandle, 
, (VOID**) );
 
   //
   // FirmwareVolume2Protocol is PI, and is not required to be available.
   //
-  if (EFI_ERROR(Status)) {
-return (NULL);
+  if (EFI_ERROR (Status)) {
+return NULL;
   }
 
   //
@@ -83,15 +72,15 @@ FindLoadedImageFileName (
   Buffer = NULL;
   Status = Fv->ReadSection(Fv, NameGuid, EFI_SECTION_USER_INTERFACE, 0, 
, , );
 
-  if (EFI_ERROR(Status)) {
-return (NULL);
+  if (EFI_ERROR (Status)) {
+return NULL;
   }
 
   //
   // ReadSection returns just the section data, without any section header. For
   // a user interface section, the only data is the file name.
   //
-  return (Buffer);
+  return Buffer;
 }
 
 /**
@@ -269,7 +258,7 @@ LoadedImageProtocolDumpInformation(
   FileName = FindLoadedImageFileName(LoadedImage);
 
   RetVal = NULL;
-  if (FileName) {
+  if (FileName != NULL) {
 Temp = HiiGetString(mHandleParsingHiiHandle, 
STRING_TOKEN(STR_LI_DUMP_NAME), NULL);
 
 if (Temp != NULL) {

Thanks/Ray

> -Original Message-
> From: Jeff Westfahl [mailto:jeff.westf...@ni.com]
> Sent: Friday, May 5, 2017 5:53 AM
> To: edk2-devel@lists.01.org
> Cc: Jeff Westfahl ; Ni, Ruiyu ;
> Carsey, Jaben 
> Subject: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol
> info
> 
> In the old shell, 'dh -v' showed some useful information about the file path
> associated with a LoadedImageProtocol that is no longer shown in the modern
> shell.
> 
> For example, with the old shell:
> 
> Handle D3 (3A552218)
> Image (3A54C918)   File:MicrocodeUpdate
> ParentHandle..: 3A666398
> SystemTable...: 3D2A8F18
> DeviceHandle..: 3B1C8098
> FilePath..: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
> ImageBase.: 3D65 - 3D655540
> ImageSize.: 5540
> CodeType..: RT_code
> DataType..: RT_data
> 
> compared to the new shell:
> 
> D3: 3A552218
> LoadedImage
> Revision..: 0x1000
> ParentHandle..: 3A666398
> SystemTable...: 3D2A8F18
> DeviceHandle..: 3B1C8098

Re: [edk2] [PATCH v2 0/2] Add wnd scale check before shrinking window

[Fu, Siyuan]

Reviewed-by: Fu Siyuan 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
ate...@kraftway.ru
Sent: 2017年4月20日 17:31
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2 0/2] Add wnd scale check before shrinking window

This is an updated patch. Previous patch can be found in thread named "Fix 
unconditional window shrinking" here:
https://lists.01.org/pipermail/edk2-devel/2017-March/009129.html

Andrey Tepin (2):
  NetworkPkg/TcpDxe: Add wnd scale check before shrinking window
  MdeModulePkg/Tcp4Dxe: Add wnd scale check before shrinking window

 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Input.c | 15 ++-
 NetworkPkg/TcpDxe/TcpInput.c   | 15 ++-
 2 files changed, 28 insertions(+), 2 deletions(-)

--
2.7.4
___
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] UefiCpuPkg/MtrrLib: Don't report OutOfResource when MTRR is enough

[Fan, Jeff]

Reviewed-by: Jeff Fan 

-Original Message-
From: Ni, Ruiyu 
Sent: Friday, May 05, 2017 10:16 AM
To: edk2-devel@lists.01.org
Cc: Fan, Jeff
Subject: [PATCH] UefiCpuPkg/MtrrLib: Don't report OutOfResource when MTRR is 
enough

The MTRR calculation algorithm contains a bug that when left subtraction cannot 
produce better MTRR solution, it forgets to restore the BaseAddress/Length so 
that MtrrLibGetMtrrNumber() returns bigger value of actual required MTRR 
numbers.
As a result, the MtrrLib reports OutOfResource but actually the MTRR is enough.

MEMORY_RANGE mC[] = {
  0, 0x10, CacheUncacheable,
  0x10, 0x89F0, CacheWriteBack,
  0x8A00, 0x7500, CacheUncacheable,
  0xFF00, 0x0100, CacheWriteProtected,
  0x1, 0x7F, CacheUncacheable,
  0xFC24, 0x2000, CacheWriteCombining // <-- trigger the error };

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jeff Fan 
---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 9d1927262a..cf1af29936 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -851,6 +851,8 @@ MtrrLibGetMtrrNumber (
   UINT64  SubtractiveLength;
   UINT64  BaseAlignment;
   UINT32  Index;
+  UINT64  OriginalBaseAddress;
+  UINT64  OriginalLength;
 
   *SubLeft = 0;
   *SubRight = 0;
@@ -861,6 +863,9 @@ MtrrLibGetMtrrNumber (
   // Get the optimal left subtraction solution.
   //
   if (BaseAddress != 0) {
+
+OriginalBaseAddress= BaseAddress;
+OriginalLength = Length;
 SubtractiveBaseAddress = 0;
 SubtractiveLength  = 0;
 //
@@ -915,7 +920,10 @@ MtrrLibGetMtrrNumber (
 //
 if (*SubLeft != 0) {
   BaseAddress = SubtractiveBaseAddress;
-  Length = SubtractiveLength;
+  Length  = SubtractiveLength;
+} else {
+  BaseAddress = OriginalBaseAddress;
+  Length  = OriginalLength;
 }
   }
 
--
2.12.2.windows.2

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

[edk2] [PATCH] UefiCpuPkg/MtrrLib: Don't report OutOfResource when MTRR is enough

[Ruiyu Ni]

The MTRR calculation algorithm contains a bug that when left
subtraction cannot produce better MTRR solution, it forgets
to restore the BaseAddress/Length so that MtrrLibGetMtrrNumber()
returns bigger value of actual required MTRR numbers.
As a result, the MtrrLib reports OutOfResource but actually the
MTRR is enough.

MEMORY_RANGE mC[] = {
  0, 0x10, CacheUncacheable,
  0x10, 0x89F0, CacheWriteBack,
  0x8A00, 0x7500, CacheUncacheable,
  0xFF00, 0x0100, CacheWriteProtected,
  0x1, 0x7F, CacheUncacheable,
  0xFC24, 0x2000, CacheWriteCombining // <-- trigger the error
};

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni 
Cc: Jeff Fan 
---
 UefiCpuPkg/Library/MtrrLib/MtrrLib.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c 
b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
index 9d1927262a..cf1af29936 100644
--- a/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
+++ b/UefiCpuPkg/Library/MtrrLib/MtrrLib.c
@@ -851,6 +851,8 @@ MtrrLibGetMtrrNumber (
   UINT64  SubtractiveLength;
   UINT64  BaseAlignment;
   UINT32  Index;
+  UINT64  OriginalBaseAddress;
+  UINT64  OriginalLength;
 
   *SubLeft = 0;
   *SubRight = 0;
@@ -861,6 +863,9 @@ MtrrLibGetMtrrNumber (
   // Get the optimal left subtraction solution.
   //
   if (BaseAddress != 0) {
+
+OriginalBaseAddress= BaseAddress;
+OriginalLength = Length;
 SubtractiveBaseAddress = 0;
 SubtractiveLength  = 0;
 //
@@ -915,7 +920,10 @@ MtrrLibGetMtrrNumber (
 //
 if (*SubLeft != 0) {
   BaseAddress = SubtractiveBaseAddress;
-  Length = SubtractiveLength;
+  Length  = SubtractiveLength;
+} else {
+  BaseAddress = OriginalBaseAddress;
+  Length  = OriginalLength;
 }
   }
 
-- 
2.12.2.windows.2

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

[edk2] [Patch V2] MdeModulePkg: Addressing TCP Window Retraction when window scale factor is used.

[Fu Siyuan]

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC7323 to address the TCP window retraction problem
when a non-zero scale factor is used.

Cc: Wu Jiaxin 
Cc: Andrey Tepin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c  |  5 +--
 .../Universal/Network/Tcp4Dxe/Tcp4Output.c | 41 +-
 MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h |  8 -
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
index 1a7c41a..892d19b 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Misc.c
@@ -1,7 +1,7 @@
 /** @file
   Misc support routines for tcp.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -78,7 +78,8 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
-
+  Tcb->RetxmitSeqMax = 0;
+  
   Tcb->ProbeTimerOn = FALSE;
 }
 
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
index 0eec8f0..ed71f97 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Output.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-Copyright (c) 2005 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -671,17 +671,38 @@ TcpRetransmit (
   // 2. must in the current send window
   // 3. will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
-DEBUG ((EFI_D_WARN, "TcpRetransmit: retransmission cancelled "
-  "because send window too small for TCB %p\n", Tcb));
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_GT (Tcb->SndWl2 + 
Tcb->SndWnd + (1 << Tcb->SndWndScale), Seq))) {
+//
+// Handle the Window Retraction if TCP window scale is enabled according 
to RFC7323:
+//   On first retransmission, or if the sequence number is out of
+//   window by less than 2^Rcv.Wind.Shift, then do normal
+//   retransmission(s) without regard to the receiver window as long
+//   as the original segment was in window when it was sent.
+//
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n",
+  Tcb)
+  );
 
 return 0;
   }
+  
+  Len = MIN (Len, Tcb->SndMss);
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
-
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -691,6 +712,10 @@ TcpRetransmit (
   if (TcpTransmitSegment (Tcb, Nbuf) != 0) {
 goto OnError;
   }
+  
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
 
   //
   // The retransmitted buffer may be on the SndQue,
diff --git a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h 
b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
index 01d6034..49d8a1d 100644
--- a/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
+++ b/MdeModulePkg/Universal/Network/Tcp4Dxe/Tcp4Proto.h
@@ -1,7 +1,7 @@
 /** @file
   Tcp Protocol header file.
 
-Copyright (c) 2005 - 2010, Intel Corporation. All rights reserved.
+Copyright (c) 2005 - 2017, 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
@@ -251,6 +251,12 @@ struct _TCP_CB {
   UINT32ConnectTimeout;  ///< The connect establishment time out
 
   //
+  // RFC7323
+  // Addressing Window Retraction for TCP Window Scale Option.
+  //
+  TCP_SEQNO 

[edk2] [Patch V2] NetworkPkg: Addressing TCP Window Retraction when window scale factor is used.

[Fu Siyuan]

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC7323 to address the TCP window retraction problem
when a non-zero scale factor is used.
The changes has been test in high packet loss rate network by using HTTP boot 
and 
iSCSI file read/write.

Cc: Wu Jiaxin 
Cc: Andrey Tepin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/TcpDxe/TcpMisc.c   |  3 ++-
 NetworkPkg/TcpDxe/TcpOutput.c | 33 -
 NetworkPkg/TcpDxe/TcpProto.h  |  8 +++-
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c
index a8592c9..4435036 100644
--- a/NetworkPkg/TcpDxe/TcpMisc.c
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
@@ -2,7 +2,7 @@
   Misc support routines for TCP driver.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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
@@ -86,6 +86,7 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
+  Tcb->RetxmitSeqMax = 0;
 
   Tcb->ProbeTimerOn = FALSE;
 }
diff --git a/NetworkPkg/TcpDxe/TcpOutput.c b/NetworkPkg/TcpDxe/TcpOutput.c
index a46cb60..f3dacf3 100644
--- a/NetworkPkg/TcpDxe/TcpOutput.c
+++ b/NetworkPkg/TcpDxe/TcpOutput.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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
@@ -664,7 +664,27 @@ TcpRetransmit (
   // 2. Must in the current send window
   // 3. Will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+  
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_GT (Tcb->SndWl2 + 
Tcb->SndWnd + (1 << Tcb->SndWndScale), Seq))) {
+//
+// Handle the Window Retraction if TCP window scale is enabled according 
to RFC7323:
+//   On first retransmission, or if the sequence number is out of
+//   window by less than 2^Rcv.Wind.Shift, then do normal
+//   retransmission(s) without regard to the receiver window as long
+//   as the original segment was in window when it was sent.
+//
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
 DEBUG (
   (EFI_D_WARN,
   "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n",
@@ -674,10 +694,9 @@ TcpRetransmit (
 return 0;
   }
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
+  Len = MIN (Len, Tcb->SndMss);
 
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -688,6 +707,10 @@ TcpRetransmit (
 goto OnError;
   }
 
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
+
   //
   // The retransmitted buffer may be on the SndQue,
   // trim TCP head because all the buffers on SndQue
diff --git a/NetworkPkg/TcpDxe/TcpProto.h b/NetworkPkg/TcpDxe/TcpProto.h
index ee35134..81397d7 100644
--- a/NetworkPkg/TcpDxe/TcpProto.h
+++ b/NetworkPkg/TcpDxe/TcpProto.h
@@ -1,7 +1,7 @@
 /** @file
   TCP protocol header file.
 
-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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
@@ -316,6 +316,12 @@ struct _TCP_CONTROL_BLOCK {
   TCP_SEQNO LossRecover;  ///< Recover point for retxmit.
 
   //
+  // RFC7323
+  // Addressing Window Retraction for TCP Window Scale Option.
+  //
+  TCP_SEQNO RetxmitSeqMax;   ///< Max Seq number in previous 
retransmission.
+
+  //
   // configuration parameters, for EFI_TCP4_PROTOCOL specification
   //
   UINT32KeepAliveIdle;   ///< Idle time before sending first probe.
-- 
1.9.5.msysgit.1

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

Re: [edk2] [Patch] SecurityPkg/Pkcs7VerifyDxe: Add format check in DB list contents

[Zhang, Chao B]

Reviewed-by: Chao Zhang 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Qin Long
Sent: Wednesday, May 3, 2017 5:29 PM
To: Zhang, Chao B 
Cc: edk2-devel@lists.01.org
Subject: [edk2] [Patch] SecurityPkg/Pkcs7VerifyDxe: Add format check in DB list 
contents

Add the size check for invalid format detection in AllowedDb, RevokedDb and 
TimeStampDb list contents.

Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 .../Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c| 66 --
 1 file changed, 60 insertions(+), 6 deletions(-)

diff --git a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c 
b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
index 07fdf552be..3776f903d4 100644
--- a/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
+++ b/SecurityPkg/Pkcs7Verify/Pkcs7VerifyDxe/Pkcs7VerifyDxe.c
@@ -5,7 +5,7 @@
   verify data signed using PKCS7 structure. The PKCS7 data to be verified must
   be ASN.1 (DER) encoded.
 
-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2017, 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 @@ -801,11 +801,13 
@@ VerifyBuffer (
   IN OUT UINTN*ContentSize
   )
 {
-  EFI_STATUS  Status;
-  UINT8   *AttachedData;
-  UINTN   AttachedDataSize;
-  UINT8   *DataPtr;
-  UINTN   DataSize;
+  EFI_STATUS  Status;
+  EFI_SIGNATURE_LIST  *SigList;
+  UINTN   Index;
+  UINT8   *AttachedData;
+  UINTN   AttachedDataSize;
+  UINT8   *DataPtr;
+  UINTN   DataSize;
 
   //
   // Parameters Checking
@@ -818,6 +820,58 @@ VerifyBuffer (
   }
 
   //
+  // Check if any invalid entry format in AllowedDb list contents  //  
+ for (Index = 0; ; Index++) {
+SigList = (EFI_SIGNATURE_LIST *)(AllowedDb[Index]);
+
+if (SigList == NULL) {
+  break;
+}
+if (SigList->SignatureListSize < sizeof (EFI_SIGNATURE_LIST) +
+ SigList->SignatureHeaderSize +
+ SigList->SignatureSize) {
+  return EFI_ABORTED;
+}
+  }
+
+  //
+  // Check if any invalid entry format in RevokedDb list contents  //  
+ if (RevokedDb != NULL) {
+for (Index = 0; ; Index++) {
+  SigList = (EFI_SIGNATURE_LIST *)(RevokedDb[Index]);
+
+  if (SigList == NULL) {
+break;
+  }
+  if (SigList->SignatureListSize < sizeof (EFI_SIGNATURE_LIST) +
+   SigList->SignatureHeaderSize +
+   SigList->SignatureSize) {
+return EFI_ABORTED;
+  }
+}
+  }
+
+  //
+  // Check if any invalid entry format in TimeStampDb list contents  //  
+ if (TimeStampDb != NULL) {
+for (Index = 0; ; Index++) {
+  SigList = (EFI_SIGNATURE_LIST *)(TimeStampDb[Index]);
+
+  if (SigList == NULL) {
+break;
+  }
+  if (SigList->SignatureListSize < sizeof (EFI_SIGNATURE_LIST) +
+   SigList->SignatureHeaderSize +
+   SigList->SignatureSize) {
+return EFI_ABORTED;
+  }
+}
+  }
+
+  //
   // Try to retrieve the attached content from PKCS7 signedData
   //
   AttachedData = NULL;
--
2.12.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 v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

[Laszlo Ersek]

On 05/04/17 18:52, Laszlo Ersek wrote:
> On 05/04/17 18:36, Jordan Justen wrote:
>> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
>>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
>>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
>>> Certification Kit sets a 32 KB large non-authenticated variable.
>>
>> According to
>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
>>
>> "The maximum supported variable size must be at least 64kB"
>>
>> Should we just bump the size to match this? We should be able to make
>> this change later once it is in a test/spec, but for some reason I
>> thought the requirement was already 64k.
> 
> The 32KB requirement comes from the most recent Secure Boot Logo Test. I
> installed both the Windows Server 2008 R2 SP1 test controller and the
> Windows 2016 Server test client just the other day, together with the
> most recent filters, using the following descriptions:
> 
> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM
> 
> Given that this limit can be bumped without breaking compatibility, as
> you say, I'd like to remain frugal with it, same as we were in James's
> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
> Authenticated variables", 2016-03-24).
> 
> I don't understand why the plugfest presentation and the SB Logo Test
> require different limits... But, I'm certain our QE will find out in
> short order once the SB Logo Test catches up with the presentation, and
> I expect I'll submit the corresponding patch soon after.
> 
> I dislike the speculation in this series, but breaking compatibility is
> even worse. (A lot worse, to me at least.) So I consider the varstore
> restructuring the smaller of two wrongs. However, wrt.
> PcdMaxVariableSize, it seems we're not being forced to either of those
> wrongs (i.e., breaking compat or speculation), so we can delay the increase.
> 
>>
>> Aside from this question:
>>
>> Series Reviewed-by: Jordan Justen 
> 
> Thanks a lot!
> 
> I'll await your ACK for the above argument before pushing the series.

Based on the ack I got from Jordan on IRC, I pushed the series. Commit
range 4bf3b994e8b2..bba8dfbec3bb.

Thanks!
Laszlo


>>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
>>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
>>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
>>> for the variable name and attributes as well.
>>>
>>> Cc: Gary Ching-Pang Lin 
>>> Cc: Jordan Justen 
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>> Signed-off-by: Laszlo Ersek 
>>> ---
>>>
>>> Notes:
>>> v2:
>>> - adjust to FD_SIZE_IN_KB
>>> - update commit msg to state 256 KB for the varstore [Jordan]
>>>
>>>  OvmfPkg/OvmfPkgIa32.dsc| 6 ++
>>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++
>>>  OvmfPkg/OvmfPkgX64.dsc | 6 ++
>>>  3 files changed, 18 insertions(+)
>>>
>>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>>> index 26b807dde9fa..e0779ddaa426 100644
>>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>>>gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>>  !endif
>>>  !if $(SMM_REQUIRE) == TRUE
>>>gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>>  !endif
>>>  
>>>  [PcdsFixedAtBuild]
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>>
>>> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>>> +!endif
>>> +!if $(FD_SIZE_IN_KB) == 4096
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>>> +!endif
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>>  
>>>gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>>  
>>>gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>>  
>>># DEBUG_INIT  0x0001  // Initialization
>>># DEBUG_WARN  0x0002  // Warnings
>>># DEBUG_LOAD  0x0004  // Load events
>>># DEBUG_FS0x0008  // EFI File system
>>># DEBUG_POOL  0x0010  // Alloc & Free (pool)
>>># DEBUG_PAGE  0x0020  // 

[edk2] [PATCH 2/3] ShellPkg/HandleParsingLib: Open LoadedImageProtocol first

[Jeff Westfahl]

This patch changes the order of operations to make sure we can open the
LoadedImageProtocol before getting the format string. This should not
affect functionality, and makes the next patch easier to review.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
 .../Library/UefiHandleParsingLib/UefiHandleParsingLib.c   | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index 2db8a3a..c96f6dd 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -177,13 +177,6 @@ LoadedImageProtocolDumpInformation(
 return (CatSPrint(NULL, L"LoadedImage"));
   }
 
-  HandleParsingHiiInit();
-
-  Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_MAIN), 
NULL);
-  if (Temp == NULL) {
-return NULL;
-  }
-
   Status = gBS->OpenProtocol (
 TheHandle,
 ,
@@ -194,7 +187,13 @@ LoadedImageProtocolDumpInformation(
);
 
   if (EFI_ERROR (Status)) {
-SHELL_FREE_NON_NULL (Temp);
+return NULL;
+  }
+
+  HandleParsingHiiInit();
+
+  Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_MAIN), 
NULL);
+  if (Temp == NULL) {
 return NULL;
   }
 
-- 
2.7.4

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

[edk2] [PATCH 1/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text

[Jeff Westfahl]

This patch adds support for displaying a text representation of the file
path associated with a LoadedImageProtocol. This is a behavior that was
present in the old shell but has been lost in the new shell.

For example, using 'dh -v' in the old shell:

FilePath..: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
FilePath..: \EFI\BOOT\BOOTX64.EFI

vs. the new shell:

FilePath..: 3A539018
FilePath..: 3A728718

This seems like useful information for the shell to display.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
 ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c   | 6 +-
 ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni | 2 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index da1d92f..2db8a3a 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -169,6 +169,7 @@ LoadedImageProtocolDumpInformation(
   EFI_STATUSStatus;
   CHAR16*RetVal;
   CHAR16*Temp;
+  CHAR16*FilePath;
   CHAR16*CodeType;
   CHAR16*DataType;
 
@@ -197,6 +198,8 @@ LoadedImageProtocolDumpInformation(
 return NULL;
   }
 
+  FilePath = ConvertDevicePathToText(LoadedImage->FilePath, TRUE, TRUE);
+
   DataType = ConvertMemoryType(LoadedImage->ImageDataType);
   CodeType = ConvertMemoryType(LoadedImage->ImageCodeType);
 
@@ -207,7 +210,7 @@ LoadedImageProtocolDumpInformation(
  LoadedImage->ParentHandle,
  LoadedImage->SystemTable,
  LoadedImage->DeviceHandle,
- LoadedImage->FilePath,
+ FilePath,
  LoadedImage->LoadOptionsSize,
  LoadedImage->LoadOptions,
  LoadedImage->ImageBase,
@@ -219,6 +222,7 @@ LoadedImageProtocolDumpInformation(
 
 
   SHELL_FREE_NON_NULL(Temp);
+  SHELL_FREE_NON_NULL(FilePath);
   SHELL_FREE_NON_NULL(CodeType);
   SHELL_FREE_NON_NULL(DataType);
 
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
index 273a420..7b3711d 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
@@ -352,7 +352,7 @@
   " ParentHandle..: 
%%H%x%%N\r\n"
   " SystemTable...: 
%%H%x%%N\r\n"
   " DeviceHandle..: 
%%H%x%%N\r\n"
-  " FilePath..: 
%%H%x%%N\r\n"
+  " FilePath..: 
%%H%s%%N\r\n"
   " OptionsSize...: 
%%H%x%%N\r\n"
   " LoadOptions...: 
%%H%x%%N\r\n"
   " ImageBase.: 
%%H%x%%N\r\n"
-- 
2.7.4

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

[edk2] [PATCH 3/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name

[Jeff Westfahl]

This patch adds support for showing the file name associated with a
LoadedImageProtocol file path. This is a behavior that was present in
the old shell but has been lost in the new shell.

For example, using 'dh -v' in the old shell:

Handle D3 (3A552218)
Image (3A54C918)   File:MicrocodeUpdate
ParentHandle..: 3A666398

vs. the new shell:

D3: 3A552218
LoadedImage
Revision..: 0x1000
ParentHandle..: 3A666398

Here's what the output of 'dh -v' looks like after this patch:

D3: 3A552218
LoadedImage
Name..: MicrocodeUpdate
Revision..: 0x1000
ParentHandle..: 3A666398

This seems like useful information for the shell to display.

Cc: Ruiyu Ni 
Cc: Jaben Carsey 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl 
---
 .../UefiHandleParsingLib/UefiHandleParsingLib.c| 90 +-
 .../UefiHandleParsingLib/UefiHandleParsingLib.uni  |  2 +
 2 files changed, 91 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c 
b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index c96f6dd..74934f8 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -16,12 +16,85 @@
 
 #include "UefiHandleParsingLib.h"
 #include "IndustryStandard/Acpi10.h"
+#include "Pi/PiFirmwareFile.h"
+#include "Pi/PiFirmwareVolume.h"
+#include "Protocol/FirmwareVolume2.h"
 
 EFI_HANDLEmHandleParsingHiiHandle = NULL;
 HANDLE_INDEX_LIST mHandleList = {{{NULL,NULL},0,0},0};
 GUID_INFO_BLOCK   *mGuidList;
 UINTN mGuidListCount;
 /**
+  Function to find the file name associated with a LoadedImageProtocol.
+
+  @param[in] LoadedImage An instance of LoadedImageProtocol.
+
+  @retvalA string representation of the file name 
associated
+ with LoadedImage, or NULL if no name can be found.
+**/
+CHAR16*
+FindLoadedImageFileName (
+  IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage
+  )
+{
+  EFI_GUID   *NameGuid;
+  EFI_STATUS Status;
+  EFI_FIRMWARE_VOLUME2_PROTOCOL  *Fv;
+  VOID   *Buffer;
+  UINTN  BufferSize;
+  UINT32 AuthenticationStatus;
+
+  //
+  // Only subtype MEDIA_PIWG_FW_FILE_DP of type MEDIA_DEVICE_PATH is supported.
+  //
+  if (   (LoadedImage == NULL)
+  || (LoadedImage->FilePath == NULL)
+  || (LoadedImage->FilePath->Type != MEDIA_DEVICE_PATH)
+  || (LoadedImage->FilePath->SubType != MEDIA_PIWG_FW_FILE_DP)
+   ) {
+return (NULL);
+  }
+
+  NameGuid = 
EfiGetNameGuidFromFwVolDevicePathNode((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH 
*)LoadedImage->FilePath);
+
+  if (NameGuid == NULL) {
+return (NULL);
+  }
+
+  //
+  // Get the FirmwareVolume2Protocol of the device handle that this image was
+  // loaded from.
+  //
+  Status = gBS->HandleProtocol(
+LoadedImage->DeviceHandle,
+,
+(VOID**));
+
+  //
+  // FirmwareVolume2Protocol is PI, and is not required to be available.
+  //
+  if (EFI_ERROR(Status)) {
+return (NULL);
+  }
+
+  //
+  // Read the user interface section of the image.
+  //
+  Buffer = NULL;
+  Status = Fv->ReadSection(Fv, NameGuid, EFI_SECTION_USER_INTERFACE, 0, 
, , );
+
+  if (EFI_ERROR(Status)) {
+return (NULL);
+  }
+
+  //
+  // ReadSection returns just the section data, without any section header. For
+  // a user interface section, the only data is the file name.
+  //
+  return (Buffer);
+}
+
+/**
   Function to translate the EFI_MEMORY_TYPE into a string.
 
   @param[in] Memory The memory type.
@@ -169,6 +242,7 @@ LoadedImageProtocolDumpInformation(
   EFI_STATUSStatus;
   CHAR16*RetVal;
   CHAR16*Temp;
+  CHAR16*FileName;
   CHAR16*FilePath;
   CHAR16*CodeType;
   CHAR16*DataType;
@@ -192,6 +266,20 @@ LoadedImageProtocolDumpInformation(
 
   HandleParsingHiiInit();
 
+  FileName = FindLoadedImageFileName(LoadedImage);
+
+  RetVal = NULL;
+  if (FileName) {
+Temp = HiiGetString(mHandleParsingHiiHandle, 
STRING_TOKEN(STR_LI_DUMP_NAME), NULL);
+
+if (Temp != NULL) {
+  RetVal = CatSPrint(NULL, Temp, FileName);
+}
+
+SHELL_FREE_NON_NULL(Temp);
+SHELL_FREE_NON_NULL(FileName);
+  }
+
   Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_MAIN), 
NULL);
   if (Temp == NULL) {
 return NULL;
@@ -203,7 +291,7 @@ LoadedImageProtocolDumpInformation(
   CodeType = ConvertMemoryType(LoadedImage->ImageCodeType);
 
   RetVal = CatSPrint(
- NULL,
+ RetVal,
  Temp,
  LoadedImage->Revision,
 

[edk2] [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info

[Jeff Westfahl]

In the old shell, 'dh -v' showed some useful information about the file path
associated with a LoadedImageProtocol that is no longer shown in the modern
shell.

For example, with the old shell:

Handle D3 (3A552218)
Image (3A54C918)   File:MicrocodeUpdate
ParentHandle..: 3A666398
SystemTable...: 3D2A8F18
DeviceHandle..: 3B1C8098
FilePath..: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
ImageBase.: 3D65 - 3D655540
ImageSize.: 5540
CodeType..: RT_code   
DataType..: RT_data   

compared to the new shell:

D3: 3A552218
LoadedImage
Revision..: 0x1000
ParentHandle..: 3A666398
SystemTable...: 3D2A8F18
DeviceHandle..: 3B1C8098
FilePath..: 3A552298
OptionsSize...: 0
LoadOptions...: 0
ImageBase.: 3D65
ImageSize.: 5540
CodeType..: EfiRuntimeServicesCode
DataType..: EfiRuntimeServicesData
Unload: 0

Here is the output for the same handle with this series applied:

D3: 3A552218
LoadedImage
Name..: MicrocodeUpdate
Revision..: 0x1000
ParentHandle..: 3A666398
SystemTable...: 3D2A8F18
DeviceHandle..: 3B1C8098
FilePath..: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
OptionsSize...: 0
LoadOptions...: 0
ImageBase.: 3D65
ImageSize.: 5540
CodeType..: EfiRuntimeServicesCode
DataType..: EfiRuntimeServicesData
Unload: 0

Cc: Ruiyu Ni 
Cc: Jaben Carsey 

Jeff Westfahl (3):
  ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text
  ShellPkg/HandleParsingLib: Open LoadedImageProtocol first
  ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name

 .../UefiHandleParsingLib/UefiHandleParsingLib.c| 111 +++--
 .../UefiHandleParsingLib/UefiHandleParsingLib.uni  |   4 +-
 2 files changed, 104 insertions(+), 11 deletions(-)

-- 
2.7.4

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

Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

[Laszlo Ersek]

On 05/04/17 21:27, Laszlo Ersek wrote:
> On 05/04/17 20:50, Jordan Justen wrote:
>> On 2017-05-04 09:52:48, Laszlo Ersek wrote:
>>> On 05/04/17 18:36, Jordan Justen wrote:
 On 2017-05-03 14:39:46, Laszlo Ersek wrote:
> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
> Certification Kit sets a 32 KB large non-authenticated variable.

 According to
 http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf

 "The maximum supported variable size must be at least 64kB"

 Should we just bump the size to match this? We should be able to make
 this change later once it is in a test/spec, but for some reason I
 thought the requirement was already 64k.
>>>
>>> The 32KB requirement comes from the most recent Secure Boot Logo Test. I
>>
>> If the limit is 32k, why go with 33k? Does the test fail with a 32k
>> limit?
> 
> Yes, it does. First I tried setting the PCDs to 0x8000 sharp, and then
> the test failed.
> 
> Then I remembered that the PCDs present a limit on data, name, and
> attributes together:
> 
> (1) In "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", function
> InitNonVolatileVariableStore(), we stash the PCDs like this:
> 
>>   mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
>>   mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 
>> (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : 
>> mVariableModuleGlobal->MaxVariableSize);
> 
> (2) In the same file, function VariableServiceSetVariable(), we have the
> following size checks:
> 
>>   //
>>   //  The size of the VariableName, including the Unicode Null in bytes plus
>>   //  the DataSize is limited to maximum size of PcdGet32 
>> (PcdMaxHardwareErrorVariableSize)
>>   //  bytes for HwErrRec variable.
>>   //
>>   if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == 
>> EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
>> if (StrSize (VariableName) + PayloadSize > PcdGet32 
>> (PcdMaxHardwareErrorVariableSize) - GetVariableHeaderSize ()) {
>>   return EFI_INVALID_PARAMETER;
>> }
>>   } else {
>> //
>> //  The size of the VariableName, including the Unicode Null in bytes 
>> plus
>> //  the DataSize is limited to maximum size of Max(Auth)VariableSize 
>> bytes.
>> //
>> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
>>   if (StrSize (VariableName) + PayloadSize > 
>> mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
>> return EFI_INVALID_PARAMETER;
>>   }
>> } else {
>>   if (StrSize (VariableName) + PayloadSize > 
>> mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
>> return EFI_INVALID_PARAMETER;
>>   }
>> }
>>   }
> 
> That is, the PCDs limit the sum of:
> - the variable name (including the terminating L'\0'),
> - the variable contents,
> - the internal header (which carries the attributes as well). In our
>   case that's AUTHENTICATED_VARIABLE_HEADER, because we always format
>   the auth varstore GUID into the template.

(NB I mentioned this in the commit message, just not in such detail.)

Thanks
Laszlo

> 
> Furthermore, the 32KB data size for the contents is not based on
> "reverse engineering"; the Secure Boot Logo Test tells you exactly what
> it is trying to do. (In fact, there is no other way to fix failures in
> any of the tests; you can only rely on the messages in order to see what
> the test attempted.)
> 
> In this case, we have (see
> ):
> 
>> NOTE: if this testcase fails, try restarting the machine and then
>> running this test case on its own, for example: "te.exe
>> UefiSecureBootLogo.dll
>> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>>
>> Test creation of 1 large variable of size 32768 bytes.
>>
>> Test creation of testLargeVariable Unexpected Error - SetVariable
>> failed with status 0xC00D VariableName "testLargeVariable"
>> VendorGuid 77fa9abd-0359-4d32-bd60-28f4e78f784b Attributes 0x0007
>> DataSize 0x8000
> 
> It reports all the parameters of the gRT->SetVariable() call that
> failed, except "Data".
> 
> In the successful test (with this patch applied), the messages are (see
> ):
> 
>> NOTE: if this testcase fails, try restarting the machine and then
>> running this test case on its own, for example: "te.exe
>> UefiSecureBootLogo.dll
>> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>>
>> Test creation of 1 large variable of size 32768 bytes.
>>
>> AreEqual(7, 7) - testLargeVariable: Attributes from GetVariable()
>> match SetVariable()
>>
>> AreEqual(32768, 32768) - testLargeVariable: DataSize from
>> GetVariable() matches SetVariable()
> 
> Thanks
> Laszlo
> 
>>
>>> installed both the Windows Server 2008 R2 

Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

[Laszlo Ersek]

On 05/04/17 20:50, Jordan Justen wrote:
> On 2017-05-04 09:52:48, Laszlo Ersek wrote:
>> On 05/04/17 18:36, Jordan Justen wrote:
>>> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
 The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
 ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
 Certification Kit sets a 32 KB large non-authenticated variable.
>>>
>>> According to
>>> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
>>>
>>> "The maximum supported variable size must be at least 64kB"
>>>
>>> Should we just bump the size to match this? We should be able to make
>>> this change later once it is in a test/spec, but for some reason I
>>> thought the requirement was already 64k.
>>
>> The 32KB requirement comes from the most recent Secure Boot Logo Test. I
>
> If the limit is 32k, why go with 33k? Does the test fail with a 32k
> limit?

Yes, it does. First I tried setting the PCDs to 0x8000 sharp, and then
the test failed.

Then I remembered that the PCDs present a limit on data, name, and
attributes together:

(1) In "MdeModulePkg/Universal/Variable/RuntimeDxe/Variable.c", function
InitNonVolatileVariableStore(), we stash the PCDs like this:

>   mVariableModuleGlobal->MaxVariableSize = PcdGet32 (PcdMaxVariableSize);
>   mVariableModuleGlobal->MaxAuthVariableSize = ((PcdGet32 
> (PcdMaxAuthVariableSize) != 0) ? PcdGet32 (PcdMaxAuthVariableSize) : 
> mVariableModuleGlobal->MaxVariableSize);

(2) In the same file, function VariableServiceSetVariable(), we have the
following size checks:

>   //
>   //  The size of the VariableName, including the Unicode Null in bytes plus
>   //  the DataSize is limited to maximum size of PcdGet32 
> (PcdMaxHardwareErrorVariableSize)
>   //  bytes for HwErrRec variable.
>   //
>   if ((Attributes & EFI_VARIABLE_HARDWARE_ERROR_RECORD) == 
> EFI_VARIABLE_HARDWARE_ERROR_RECORD) {
> if (StrSize (VariableName) + PayloadSize > PcdGet32 
> (PcdMaxHardwareErrorVariableSize) - GetVariableHeaderSize ()) {
>   return EFI_INVALID_PARAMETER;
> }
>   } else {
> //
> //  The size of the VariableName, including the Unicode Null in bytes plus
> //  the DataSize is limited to maximum size of Max(Auth)VariableSize 
> bytes.
> //
> if ((Attributes & VARIABLE_ATTRIBUTE_AT_AW) != 0) {
>   if (StrSize (VariableName) + PayloadSize > 
> mVariableModuleGlobal->MaxAuthVariableSize - GetVariableHeaderSize ()) {
> return EFI_INVALID_PARAMETER;
>   }
> } else {
>   if (StrSize (VariableName) + PayloadSize > 
> mVariableModuleGlobal->MaxVariableSize - GetVariableHeaderSize ()) {
> return EFI_INVALID_PARAMETER;
>   }
> }
>   }

That is, the PCDs limit the sum of:
- the variable name (including the terminating L'\0'),
- the variable contents,
- the internal header (which carries the attributes as well). In our
  case that's AUTHENTICATED_VARIABLE_HEADER, because we always format
  the auth varstore GUID into the template.

Furthermore, the 32KB data size for the contents is not based on
"reverse engineering"; the Secure Boot Logo Test tells you exactly what
it is trying to do. (In fact, there is no other way to fix failures in
any of the tests; you can only rely on the messages in order to see what
the test attempted.)

In this case, we have (see
):

> NOTE: if this testcase fails, try restarting the machine and then
> running this test case on its own, for example: "te.exe
> UefiSecureBootLogo.dll
> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>
> Test creation of 1 large variable of size 32768 bytes.
>
> Test creation of testLargeVariable Unexpected Error - SetVariable
> failed with status 0xC00D VariableName "testLargeVariable"
> VendorGuid 77fa9abd-0359-4d32-bd60-28f4e78f784b Attributes 0x0007
> DataSize 0x8000

It reports all the parameters of the gRT->SetVariable() call that
failed, except "Data".

In the successful test (with this patch applied), the messages are (see
):

> NOTE: if this testcase fails, try restarting the machine and then
> running this test case on its own, for example: "te.exe
> UefiSecureBootLogo.dll
> /name:Microsoft.UefiSecureBootLogo.Tests.ConfirmSetOfLargeVariable"
>
> Test creation of 1 large variable of size 32768 bytes.
>
> AreEqual(7, 7) - testLargeVariable: Attributes from GetVariable()
> match SetVariable()
>
> AreEqual(32768, 32768) - testLargeVariable: DataSize from
> GetVariable() matches SetVariable()

Thanks
Laszlo

>
>> installed both the Windows Server 2008 R2 SP1 test controller and the
>> Windows 2016 Server test client just the other day, together with the
>> most recent filters, using the following descriptions:
>>
>> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
>> 

Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

[Jordan Justen]

On 2017-05-04 09:52:48, Laszlo Ersek wrote:
> On 05/04/17 18:36, Jordan Justen wrote:
> > On 2017-05-03 14:39:46, Laszlo Ersek wrote:
> >> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
> >> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
> >> Certification Kit sets a 32 KB large non-authenticated variable.
> > 
> > According to
> > http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
> > 
> > "The maximum supported variable size must be at least 64kB"
> > 
> > Should we just bump the size to match this? We should be able to make
> > this change later once it is in a test/spec, but for some reason I
> > thought the requirement was already 64k.
> 
> The 32KB requirement comes from the most recent Secure Boot Logo Test. I

If the limit is 32k, why go with 33k? Does the test fail with a 32k
limit?

-Jordan

> installed both the Windows Server 2008 R2 SP1 test controller and the
> Windows 2016 Server test client just the other day, together with the
> most recent filters, using the following descriptions:
> 
> https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
> https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM
> 
> Given that this limit can be bumped without breaking compatibility, as
> you say, I'd like to remain frugal with it, same as we were in James's
> commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
> Authenticated variables", 2016-03-24).
> 
> I don't understand why the plugfest presentation and the SB Logo Test
> require different limits... But, I'm certain our QE will find out in
> short order once the SB Logo Test catches up with the presentation, and
> I expect I'll submit the corresponding patch soon after.
> 
> I dislike the speculation in this series, but breaking compatibility is
> even worse. (A lot worse, to me at least.) So I consider the varstore
> restructuring the smaller of two wrongs. However, wrt.
> PcdMaxVariableSize, it seems we're not being forced to either of those
> wrongs (i.e., breaking compat or speculation), so we can delay the increase.
> 
> > 
> > Aside from this question:
> > 
> > Series Reviewed-by: Jordan Justen 
> 
> Thanks a lot!
> 
> I'll await your ACK for the above argument before pushing the series.
> 
> Thanks,
> Laszlo
> 
> >> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
> >> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
> >> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
> >> for the variable name and attributes as well.
> >>
> >> Cc: Gary Ching-Pang Lin 
> >> Cc: Jordan Justen 
> >> Contributed-under: TianoCore Contribution Agreement 1.0
> >> Signed-off-by: Laszlo Ersek 
> >> ---
> >>
> >> Notes:
> >> v2:
> >> - adjust to FD_SIZE_IN_KB
> >> - update commit msg to state 256 KB for the varstore [Jordan]
> >>
> >>  OvmfPkg/OvmfPkgIa32.dsc| 6 ++
> >>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++
> >>  OvmfPkg/OvmfPkgX64.dsc | 6 ++
> >>  3 files changed, 18 insertions(+)
> >>
> >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> >> index 26b807dde9fa..e0779ddaa426 100644
> >> --- a/OvmfPkg/OvmfPkgIa32.dsc
> >> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> >> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
> >>gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
> >>  !endif
> >>  !if $(SMM_REQUIRE) == TRUE
> >>gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
> >>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
> >>  !endif
> >>  
> >>  [PcdsFixedAtBuild]
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
> >>
> >> gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
> >>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> >> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> >> +!endif
> >> +!if $(FD_SIZE_IN_KB) == 4096
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> >> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> >> +!endif
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
> >>  
> >>gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
> >>  
> >>gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
> >>  
> >># DEBUG_INIT  0x0001  // Initialization
> >># DEBUG_WARN  0x0002  // Warnings
> >># DEBUG_LOAD  0x0004  // Load events
> >># DEBUG_FS0x0008  // EFI File system
> >># DEBUG_POOL  

Re: [edk2] [PATCH 3/3] ArmPlatformPkg, ArmVirtPkg: delete redundant PL031 functions

[Ard Biesheuvel]

On 3 May 2017 at 22:38, Leif Lindholm  wrote:
> Remove the functions now provided by EfiTimeBaseLib from
> PL031RealTimeClockLib. Add EfiTimeBaseLib resolution to ArmVirtPkg
> in same commit to prevent breakage.
>
> Cc: Laszlo Ersek 
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leif Lindholm 

I'd rather you'd call it something that doesn't start with 'Efi' given
that we usually reserve that for protocols and other prototypes that
are defined by the UEFI spec.

Other than that, this series looks fine to me

Reviewed-by: Ard Biesheuvel 

> ---
>  .../Include/Drivers/PL031RealTimeClock.h   |  11 --
>  .../PL031RealTimeClockLib/PL031RealTimeClockLib.c  | 127 
> +
>  .../PL031RealTimeClockLib.inf  |   1 +
>  ArmVirtPkg/ArmVirt.dsc.inc |   1 +
>  4 files changed, 5 insertions(+), 135 deletions(-)
>
> diff --git a/ArmPlatformPkg/Include/Drivers/PL031RealTimeClock.h 
> b/ArmPlatformPkg/Include/Drivers/PL031RealTimeClock.h
> index 76fbd0eb82..812cd9b397 100644
> --- a/ArmPlatformPkg/Include/Drivers/PL031RealTimeClock.h
> +++ b/ArmPlatformPkg/Include/Drivers/PL031RealTimeClock.h
> @@ -42,15 +42,4 @@
>
>  #define PL031_COUNTS_PER_SECOND 1
>
> -// Define EPOCH (1970-JANUARY-01) in the Julian Date representation
> -#define EPOCH_JULIAN_DATE   2440588
> -
> -// Seconds per unit
> -#define SEC_PER_MIN ((UINTN)60)
> -#define SEC_PER_HOUR((UINTN)  3600)
> -#define SEC_PER_DAY ((UINTN) 86400)
> -
> -#define SEC_PER_MONTH   ((UINTN)  2,592,000)
> -#define SEC_PER_YEAR((UINTN) 31,536,000)
> -
>  #endif
> diff --git 
> a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c 
> b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
> index 4aa448b528..9f115d383c 100644
> --- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
> +++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.c
> @@ -36,6 +36,8 @@
>
>  #include 
>
> +#include 
> +
>  #include 
>
>  STATIC CONST CHAR16   mTimeZoneVariableName[] = L"PL031RtcTimeZone";
> @@ -110,129 +112,6 @@ InitializePL031 (
>  }
>
>  /**
> -  Converts Epoch seconds (elapsed since 1970 JANUARY 01, 00:00:00 UTC) to 
> EFI_TIME
> - **/
> -VOID
> -EpochToEfiTime (
> -  IN  UINTN EpochSeconds,
> -  OUT EFI_TIME  *Time
> -  )
> -{
> -  UINTN a;
> -  UINTN b;
> -  UINTN c;
> -  UINTN d;
> -  UINTN g;
> -  UINTN j;
> -  UINTN m;
> -  UINTN y;
> -  UINTN da;
> -  UINTN db;
> -  UINTN dc;
> -  UINTN dg;
> -  UINTN hh;
> -  UINTN mm;
> -  UINTN ss;
> -  UINTN J;
> -
> -  J  = (EpochSeconds / 86400) + 2440588;
> -  j  = J + 32044;
> -  g  = j / 146097;
> -  dg = j % 146097;
> -  c  = (((dg / 36524) + 1) * 3) / 4;
> -  dc = dg - (c * 36524);
> -  b  = dc / 1461;
> -  db = dc % 1461;
> -  a  = (((db / 365) + 1) * 3) / 4;
> -  da = db - (a * 365);
> -  y  = (g * 400) + (c * 100) + (b * 4) + a;
> -  m  = (((da * 5) + 308) / 153) - 2;
> -  d  = da - (((m + 4) * 153) / 5) + 122;
> -
> -  Time->Year  = y - 4800 + ((m + 2) / 12);
> -  Time->Month = ((m + 2) % 12) + 1;
> -  Time->Day   = d + 1;
> -
> -  ss = EpochSeconds % 60;
> -  a  = (EpochSeconds - ss) / 60;
> -  mm = a % 60;
> -  b = (a - mm) / 60;
> -  hh = b % 24;
> -
> -  Time->Hour= hh;
> -  Time->Minute  = mm;
> -  Time->Second  = ss;
> -  Time->Nanosecond  = 0;
> -
> -}
> -
> -/**
> -  Converts EFI_TIME to Epoch seconds (elapsed since 1970 JANUARY 01, 
> 00:00:00 UTC)
> - **/
> -UINTN
> -EfiTimeToEpoch (
> -  IN  EFI_TIME  *Time
> -  )
> -{
> -  UINTN a;
> -  UINTN y;
> -  UINTN m;
> -  UINTN JulianDate;  // Absolute Julian Date representation of the supplied 
> Time
> -  UINTN EpochDays;   // Number of days elapsed since EPOCH_JULIAN_DAY
> -  UINTN EpochSeconds;
> -
> -  a = (14 - Time->Month) / 12 ;
> -  y = Time->Year + 4800 - a;
> -  m = Time->Month + (12*a) - 3;
> -
> -  JulianDate = Time->Day + ((153*m + 2)/5) + (365*y) + (y/4) - (y/100) + 
> (y/400) - 32045;
> -
> -  ASSERT (JulianDate >= EPOCH_JULIAN_DATE);
> -  EpochDays = JulianDate - EPOCH_JULIAN_DATE;
> -
> -  EpochSeconds = (EpochDays * SEC_PER_DAY) + ((UINTN)Time->Hour * 
> SEC_PER_HOUR) + (Time->Minute * SEC_PER_MIN) + Time->Second;
> -
> -  return EpochSeconds;
> -}
> -
> -BOOLEAN
> -IsLeapYear (
> -  IN EFI_TIME   *Time
> -  )
> -{
> -  if (Time->Year % 4 == 0) {
> -if (Time->Year % 100 == 0) {
> -  if (Time->Year % 400 == 0) {
> -return TRUE;
> -  } else {
> -return FALSE;
> -  }
> -} else {
> -

Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

[Laszlo Ersek]

On 05/04/17 18:36, Jordan Justen wrote:
> On 2017-05-03 14:39:46, Laszlo Ersek wrote:
>> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
>> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
>> Certification Kit sets a 32 KB large non-authenticated variable.
> 
> According to
> http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf
> 
> "The maximum supported variable size must be at least 64kB"
> 
> Should we just bump the size to match this? We should be able to make
> this change later once it is in a test/spec, but for some reason I
> thought the requirement was already 64k.

The 32KB requirement comes from the most recent Secure Boot Logo Test. I
installed both the Windows Server 2008 R2 SP1 test controller and the
Windows 2016 Server test client just the other day, together with the
most recent filters, using the following descriptions:

https://msdn.microsoft.com/en-us/library/windows/hardware/jj123537.aspx
https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Controller_VM
https://github.com/daynix/VirtHCK/wiki#Checklist_for_a_New_Client_VM

Given that this limit can be bumped without breaking compatibility, as
you say, I'd like to remain frugal with it, same as we were in James's
commit f5404a3eba1d ("OvmfPkg: Increase the maximum size for
Authenticated variables", 2016-03-24).

I don't understand why the plugfest presentation and the SB Logo Test
require different limits... But, I'm certain our QE will find out in
short order once the SB Logo Test catches up with the presentation, and
I expect I'll submit the corresponding patch soon after.

I dislike the speculation in this series, but breaking compatibility is
even worse. (A lot worse, to me at least.) So I consider the varstore
restructuring the smaller of two wrongs. However, wrt.
PcdMaxVariableSize, it seems we're not being forced to either of those
wrongs (i.e., breaking compat or speculation), so we can delay the increase.

> 
> Aside from this question:
> 
> Series Reviewed-by: Jordan Justen 

Thanks a lot!

I'll await your ACK for the above argument before pushing the series.

Thanks,
Laszlo

>> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
>> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
>> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
>> for the variable name and attributes as well.
>>
>> Cc: Gary Ching-Pang Lin 
>> Cc: Jordan Justen 
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek 
>> ---
>>
>> Notes:
>> v2:
>> - adjust to FD_SIZE_IN_KB
>> - update commit msg to state 256 KB for the varstore [Jordan]
>>
>>  OvmfPkg/OvmfPkgIa32.dsc| 6 ++
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++
>>  OvmfPkg/OvmfPkgX64.dsc | 6 ++
>>  3 files changed, 18 insertions(+)
>>
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 26b807dde9fa..e0779ddaa426 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>>gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>>  !endif
>>  !if $(SMM_REQUIRE) == TRUE
>>gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>>  !endif
>>  
>>  [PcdsFixedAtBuild]
>>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>>gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
>> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
>> +!endif
>> +!if $(FD_SIZE_IN_KB) == 4096
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
>> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
>> +!endif
>>gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>>  
>>gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>>  
>>gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>>  
>># DEBUG_INIT  0x0001  // Initialization
>># DEBUG_WARN  0x0002  // Warnings
>># DEBUG_LOAD  0x0004  // Load events
>># DEBUG_FS0x0008  // EFI File system
>># DEBUG_POOL  0x0010  // Alloc & Free (pool)
>># DEBUG_PAGE  0x0020  // Alloc & Free (page)
>># DEBUG_INFO  0x0040  // Informational debug messages
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 41f06a6b6a66..bbe26e2cf452 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -409,28 

Re: [edk2] [PATCH v2 4/5] OvmfPkg: raise max variable size (auth & non-auth) to 33KB for FD_SIZE_4MB

[Jordan Justen]

On 2017-05-03 14:39:46, Laszlo Ersek wrote:
> The "ConfirmSetOfLargeVariable" test case of the Secure Boot Logo Test
> ("Microsoft.UefiSecureBootLogo.Tests") suite in the Microsoft Hardware
> Certification Kit sets a 32 KB large non-authenticated variable.

According to
http://www.uefi.org/sites/default/files/resources/UEFI_Plugfest_Security_Microsoft_Fall_2016.pdf

"The maximum supported variable size must be at least 64kB"

Should we just bump the size to match this? We should be able to make
this change later once it is in a test/spec, but for some reason I
thought the requirement was already 64k.

Aside from this question:

Series Reviewed-by: Jordan Justen 

> In the FD_SIZE_4MB build, our live varstore is now 256 KB big, so we can
> accommodate this. Set both PcdMaxVariableSize and PcdMaxAuthVariableSize
> to 0x8400 -- beyond DataSize=0x8000 from the HCK test, we need some room
> for the variable name and attributes as well.
> 
> Cc: Gary Ching-Pang Lin 
> Cc: Jordan Justen 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> v2:
> - adjust to FD_SIZE_IN_KB
> - update commit msg to state 256 KB for the varstore [Jordan]
> 
>  OvmfPkg/OvmfPkgIa32.dsc| 6 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++
>  OvmfPkg/OvmfPkgX64.dsc | 6 ++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 26b807dde9fa..e0779ddaa426 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -404,28 +404,34 @@ [PcdsFeatureFlag]
>gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>  !endif
>  !if $(SMM_REQUIRE) == TRUE
>gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>  !endif
>  
>  [PcdsFixedAtBuild]
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +!endif
> +!if $(FD_SIZE_IN_KB) == 4096
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> +!endif
>gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>  
>gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>  
>gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>  
># DEBUG_INIT  0x0001  // Initialization
># DEBUG_WARN  0x0002  // Warnings
># DEBUG_LOAD  0x0004  // Load events
># DEBUG_FS0x0008  // EFI File system
># DEBUG_POOL  0x0010  // Alloc & Free (pool)
># DEBUG_PAGE  0x0020  // Alloc & Free (page)
># DEBUG_INFO  0x0040  // Informational debug messages
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 41f06a6b6a66..bbe26e2cf452 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -409,28 +409,34 @@ [PcdsFeatureFlag]
>gUefiOvmfPkgTokenSpaceGuid.PcdSecureBootEnable|TRUE
>  !endif
>  !if $(SMM_REQUIRE) == TRUE
>gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire|TRUE
>gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
>  !endif
>  
>  [PcdsFixedAtBuild]
>gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
>gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
>gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10
>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxFvSupported|6
>gEfiMdeModulePkgTokenSpaceGuid.PcdPeiCoreMaxPeimPerFv|32
> +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048)
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
>gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +!endif
> +!if $(FD_SIZE_IN_KB) == 4096
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400
> +!endif
>gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000
>  
>gEfiMdeModulePkgTokenSpaceGuid.PcdVpdBaseAddress|0x0
>  
>gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
>  
># DEBUG_INIT  0x0001  // Initialization
># DEBUG_WARN  0x0002  // Warnings
># DEBUG_LOAD  0x0004  // Load events
># DEBUG_FS0x0008  // EFI File system
># DEBUG_POOL  0x0010  // Alloc & Free (pool)
># DEBUG_PAGE  0x0020  // Alloc & Free (page)
># DEBUG_INFO  0x0040  // Informational debug messages
> 

Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance

[Tim Lewis]

Star --

I understand your use case. We use the same behavior for other resources 
besides PCD. 

All we are asking is that this data that is used by the build tools is also 
available in some form at runtime. 

Thanks,

Tim
 


-Original Message-
From: Zeng, Star [mailto:star.z...@intel.com] 
Sent: Thursday, May 04, 2017 6:42 AM
To: Tim Lewis ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Gao, Liming ; Zeng, Star 
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Tim,

To avoid misunderstanding, I think I need to clarify more about this RFC. :)

This RFC is NOT to propose building real relationship (like parent-child) 
between non-DEFAULT SKUs, it seems easy to bring confusion from the proposed 
syntax " SkuValue|SkuName[|ParentSkuName]", especially the word 'Parent', sorry 
for any confusion.
The syntax extension proposed by this RFC is just to simplify the PCDs 
configuring for multiple SKUs in DSC, I want to emphasize it is in DSC, it is 
to cover the case that some non-DEFAULT SKU has much PCD values equal with 
another non-DEFAULT SKU, but less PCD values equal with DEFAULT SKU.
This RFC has no any change to boot time behavior and does not break the rule 
that DEFAULT SKU PCD value will be returned if the specified SKU PCD value is 
not configured in DSC, the code logic is below.

PcdPeim Service.c:
  //
  // Find the current system's SKU ID entry in SKU ID table.
  //
  FoundSku = FALSE;
  for (Index = 0; Index < SkuIdTable[0]; Index++) {
if (PeiPcdDb->SystemSkuId == SkuIdTable[Index + 1]) {
  FoundSku = TRUE;
  break;
}
  }

  //
  // Find the default SKU ID entry in SKU ID table.
  //
  if(!FoundSku) {
for (Index = 0; Index < SkuIdTable[0]; Index++) {
  if (0 == SkuIdTable[Index + 1]) {
break;
  }
}
  }


The usage case you provided for other resources beyond PCD could be like below?

CurrentSkuId = PcdGetSkuId();

Resource = NULL;
if (!GetResourceForSkuId(CurrentSkuId, ) {
  // try DEFAULT SKU
  GetResourceForSkuId(0, );
}
If (Resource == NULL) {
  // error, no resource found for any of the SKU IDs }



Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tim Lewis
Sent: Thursday, May 4, 2017 2:55 AM
To: Zeng, Star ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance

We use SKU IDs for working with 15 different key resources in our product. Some 
are related to code execution, but most are related to selection of resources. 
I mentioned logo components and HII strings already.

I am asking that the information about SKU relationships be output in some form 
so that it can be used at runtime, by whatever component. Today, there is an 
implied relationship between a SKU X and the SKU DEFAULT for PCDs. We use this 
rule in many places, because it expresses a useful relationship.

So, in the Project's .dsc file (forgive my syntax, which may not match yours 
exactly)

// Select which SKUs
SKUID_IDENTIFIER = Sku2 Sku1 DEFAULT

[SkuIds]
  0|DEFAULT
  1|Sku1
  2|Sku2 Sku1
  3|Sku3

In (proposed) AutoGen.h. This gives enough data for runtime usage. For other 
build tools to use, maybe something else is required.

// Output as an array initializer. DEFAULT is always last.
// Only output for SKUs listed in SKUID_IDENTIFIER // 2->1->0 // 1->0 // 0 
#define SKUID_IDENTIFIER_DEFAULT 2,1,0,1,0,0

// In actual code:

UINT32 SkuIdDefaults[] = {SKUID_IDENTIFIER_DEFAULT};
UINT32 *SkuId;
UINT32 CurrentSkuId;
BOOLEAN Found;
VOID *Resource;

CurrentSkuId = PcdGetSkuId();

SkuId = SkuIdDefaults;
Found = (CurrentSkuId == 0); // so that DEFAULT will also return TRUE; while 
(*SkuId != 0) {
  if (*SkuId == CurrentSkuId) {
Found = TRUE;
Break;
  }
  while (*SkuId != 0) {
 SkuId++;
  }
  SkuId++;
}
if (!Found) {
  //error, because SKU ID was not found in array }

Resource = NULL;
while (!GetResourceForSkuId(*SkuId, ) && *SkuId == 0) {
  *SkuId++;
}
If (Resource == NULL) {
  // error, no resource found for any of the SKU IDs }






From: Zeng, Star [mailto:star.z...@intel.com]
Sent: Wednesday, May 03, 2017 7:03 AM
To: Tim Lewis ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Gao, Liming ; Zeng, Star 
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Tim,

Could you help share and explain the use cases in your code base that is using 
[SkuIds] section and SKUID_IDENTIFIER for others beyond PCD?
Then we can consider more in [RFC] PCD: Extended SKU support. Or do you have 
detailed propose in this RFC or new RFC about your use cases?

Thanks,
Star
From: Kinney, Michael D
Sent: Friday, April 28, 2017 11:26 AM
To: Tim Lewis 

[edk2] [PATCH V5 3/3] MdeModulePkg/PciBus: Add IOMMU support.

[Jiewen Yao]

If IOMMU protocol is installed, PciBus need call IOMMU
to set access attribute for the PCI device in Map/Ummap.

Only after the access attribute is set, the PCI device can
access the DMA memory.

Cc: Ruiyu Ni 
Cc: Leo Duran 
Cc: Brijesh Singh 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c  |  9 
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h  |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf |  1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c   | 47 ++--
 4 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
index f3be47a..950cacc 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c
@@ -42,6 +42,7 @@ UINT64gAllZero
 = 0;
 
 EFI_PCI_PLATFORM_PROTOCOL *gPciPlatformProtocol;
 EFI_PCI_OVERRIDE_PROTOCOL *gPciOverrideProtocol;
+EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
 
 
 GLOBAL_REMOVE_IF_UNREFERENCED EFI_PCI_HOTPLUG_REQUEST_PROTOCOL 
mPciHotPlugRequest = {
@@ -284,6 +285,14 @@ PciBusDriverBindingStart (
   );
   }  
 
+  if (mIoMmuProtocol == NULL) {
+gBS->LocateProtocol (
+  ,
+  NULL,
+  (VOID **) 
+  );
+  }
+
   if (PcdGetBool (PcdPciDisableBusEnumeration)) {
 gFullEnumeration = FALSE;
   } else {
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 39ba8b9..3bcc134 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
index a3ab11f..5da094f 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
@@ -95,6 +95,7 @@
   gEfiPciRootBridgeIoProtocolGuid ## TO_START
   gEfiIncompatiblePciDeviceSupportProtocolGuid## SOMETIMES_CONSUMES
   gEfiLoadFile2ProtocolGuid   ## SOMETIMES_PRODUCES
+  gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
 
 [FeaturePcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPciBusHotplugDeviceSupport  ## CONSUMES
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index f72598d..3b3b53a 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -14,6 +14,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "PciBus.h"
 
+extern EDKII_IOMMU_PROTOCOL  *mIoMmuProtocol;
+
 //
 // Pci Io Protocol Interface
 //
@@ -965,8 +967,10 @@ PciIoMap (
   OUTVOID   **Mapping
   )
 {
-  EFI_STATUSStatus;
-  PCI_IO_DEVICE *PciIoDevice;
+  EFI_STATUS Status;
+  PCI_IO_DEVICE  *PciIoDevice;
+  UINT64 IoMmuAttribute;
+  EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION  RootBridgeIoOperation;
 
   PciIoDevice = PCI_IO_DEVICE_FROM_PCI_IO_THIS (This);
 
@@ -978,13 +982,14 @@ PciIoMap (
 return EFI_INVALID_PARAMETER;
   }
 
+  RootBridgeIoOperation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION)Operation;
   if ((PciIoDevice->Attributes & EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE) != 
0) {
-Operation = (EFI_PCI_IO_PROTOCOL_OPERATION) (Operation + 
EfiPciOperationBusMasterRead64);
+RootBridgeIoOperation = 
(EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION)(Operation + 
EfiPciOperationBusMasterRead64);
   }
 
   Status = PciIoDevice->PciRootBridgeIo->Map (
   PciIoDevice->PciRootBridgeIo,
-  
(EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) Operation,
+  RootBridgeIoOperation,
   HostAddress,
   NumberOfBytes,
   DeviceAddress,
@@ -999,6 +1004,31 @@ PciIoMap (
   );
   }
 
+  if (mIoMmuProtocol != NULL) {
+if (!EFI_ERROR (Status)) {
+  switch (Operation) {
+  case EfiPciIoOperationBusMasterRead:
+IoMmuAttribute = EDKII_IOMMU_ACCESS_READ;
+break;
+  case EfiPciIoOperationBusMasterWrite:
+IoMmuAttribute = EDKII_IOMMU_ACCESS_WRITE;
+break;
+  case EfiPciIoOperationBusMasterCommonBuffer:
+IoMmuAttribute = EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE;
+

[edk2] [PATCH V5 2/3] MdeModulePkg/PciHostBridge: Add IOMMU support.

[Jiewen Yao]

If IOMMU protocol is installed, PciHostBridge just calls
IOMMU AllocateBuffer/FreeBuffer/Map/Unmap.

PciHostBridge does not set IOMMU access attribute,
because it does not know which device request the DMA.
This work is done by PciBus driver.

Cc: Ruiyu Ni 
Cc: Leo Duran 
Cc: Brijesh Singh 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  | 37 
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf |  2 +
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h  |  2 +
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c| 61 

 4 files changed, 102 insertions(+)

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 9005dee..70726a6 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -28,6 +28,10 @@ GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mPciResourceTypeStr[] 
= {
   L"I/O", L"Mem", L"PMem", L"Mem64", L"PMem64", L"Bus"
 };
 
+EDKII_IOMMU_PROTOCOL*mIoMmuProtocol;
+EFI_EVENT   mIoMmuEvent;
+VOID*mIoMmuRegistration;
+
 /**
   Ensure the compatibility of an IO space descriptor with the IO aperture.
 
@@ -313,6 +317,28 @@ FreeMemorySpaceMap:
 }
 
 /**
+  Event notification that is fired when IOMMU protocol is installed.
+
+  @param  Event The Event that is being processed.
+  @param  Context   Event Context.
+
+**/
+VOID
+EFIAPI
+IoMmuProtocolCallback (
+  IN  EFI_EVENT   Event,
+  IN  VOID*Context
+  )
+{
+  EFI_STATUS   Status;
+
+  Status = gBS->LocateProtocol (, NULL, (VOID 
**));
+  if (!EFI_ERROR(Status)) {
+gBS->CloseEvent (mIoMmuEvent);
+  }
+}
+
+/**
 
   Entry point of this driver.
 
@@ -489,6 +515,17 @@ InitializePciHostBridge (
 ASSERT_EFI_ERROR (Status);
   }
   PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
+
+  if (!EFI_ERROR (Status)) {
+mIoMmuEvent = EfiCreateProtocolNotifyEvent (
+,
+TPL_CALLBACK,
+IoMmuProtocolCallback,
+NULL,
+
+);
+  }
+
   return Status;
 }
 
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
index d8b0439..42bd8a2 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
@@ -41,6 +41,7 @@
   BaseMemoryLib
   BaseLib
   PciSegmentLib
+  UefiLib
   PciHostBridgeLib
 
 [Protocols]
@@ -49,6 +50,7 @@
   gEfiDevicePathProtocolGuid  ## BY_START
   gEfiPciRootBridgeIoProtocolGuid ## BY_START
   gEfiPciHostBridgeResourceAllocationProtocolGuid ## BY_START
+  gEdkiiIoMmuProtocolGuid ## SOMETIMES_CONSUMES
 
 [Depex]
   gEfiCpuIo2ProtocolGuid AND
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
index 13185b4..1fec88b 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h
@@ -27,6 +27,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -34,6 +35,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include "PciHostResource.h"
 
 
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
index 8af131b..068295b 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c
@@ -17,6 +17,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include "PciRootBridge.h"
 #include "PciHostResource.h"
 
+extern EDKII_IOMMU_PROTOCOL*mIoMmuProtocol;
+
 #define NO_MAPPING  (VOID *) (UINTN) -1
 
 //
@@ -1072,6 +1074,26 @@ RootBridgeIoMap (
 
   RootBridge = ROOT_BRIDGE_FROM_THIS (This);
 
+  if (mIoMmuProtocol != NULL) {
+if (!RootBridge->DmaAbove4G) {
+  //
+  // Clear 64bit support
+  //
+  if (Operation > EfiPciOperationBusMasterCommonBuffer) {
+Operation = (EFI_PCI_ROOT_BRIDGE_IO_PROTOCOL_OPERATION) (Operation - 
EfiPciOperationBusMasterRead64);
+  }
+}
+Status = mIoMmuProtocol->Map (
+   mIoMmuProtocol,
+   Operation,
+   HostAddress,
+   NumberOfBytes,
+

[edk2] [PATCH V5 0/3] Add IOMMU support.

[Jiewen Yao]

 V5 ==
Minor update from V4.

1) Remove unused SetAttribute() API in IOMMU protocol.
(Feedback from Ruiyu and Ard)
2) Rename SetMappingAttribute() to SetAttribute().
(Feedback from Ruiyu)
3) Fix the bug in PciBus driver for Operation
(Thanks to Ard to catch it)

V4:
Tested-by: Brijesh Singh 
With the issue in 3/3 addressed:
Tested-by: Ard Biesheuvel 

 V4 ==
Refine the EDKII_IOMMU_PROTOCOL.

1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
They are similar to DmaLib in EmbeddedPkg and
similar to the previous BmDmaLib (by leo.du...@amd.com).

These APIs are invoked by PciHostBridge driver
to allocate DMA memory.

The PciHostBridge driver (IOMMU consumer) is simplified:
It uses IOMMU, if IOMMU protocol is present.
Else it uses original logic.

2) Add SetMappingAttribute() API.
It is similar to SetAttribute() API in V1.

This API is invoked by PciBus driver to set DMA
access attribute (read/write) for device.

The PciBus driver (IOMMU consumer) is simplified:
It sets access attribute in Map/Unmap,
if IOMMU protocol is present.

3) Remove SetRemapAddress/GetRemapAddress() API.
Because PciHostBridge/PciBus can call the APIs defined
above, there is no need to provide remap capability.

-- Sample producer drivers:
1) The sample VTd driver (IOMMU producer)
is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe

It is added to show the concept. It is not fully implemented yet.
It will not be checked in in this patch.

2) The sample AMD SEV driver (IOMMU producer)
is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSevDxe
(code is borrowed from leo.du...@amd.com and brijesh.si...@amd.com)

This is not a right place to put this driver.

It is added to show the concept.
It is not fully implemented. It will not be checked in.
Please do not use it directly.

3) The sample STYX driver (IOMMU producer)
is at https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDxe
(code is borrowed from ard.biesheu...@linaro.org)

This is not a right place to put this driver.

It is added to show the concept.
It is not fully implemented. It will not be checked in.
Please do not use it directly.


 V3 ==
1) Add Remap capability (from Ard Biesheuvel)
Add EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.

NOTE: The code is not fully validated yet.
The purpose is to collect feedback to decide the next step.

 V2 ==
1) Enhance Unmap() in PciIo (From Ruiyu Ni)
Maintain a local list of MapInfo and match it in Unmap.

2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran)
Fix a bug in V1 that copy mem for read happen before SetAttribute,
which will break AMD SEV solution.

 V1 ==

This patch series adds IOMMU protocol and updates the consumer
to support IOMMU based DMA access in UEFI.

This patch series can support the BmDmaLib request for AMD SEV.
submitted by Duran, Leo  and Brijesh Singh 
.
https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
and clear SEV in IOMMU->SetAttribute().

This patch series can also support Intel VTd based DMA protection,
requested by Jiewen Yao , discussed in
https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
We can have an Intel VTd specific IOMMU driver to produce IOMMU protocol,
and update VTd engine to grant or deny access in IOMMU->SetAttribute().

This patch series does not provide a full Intel VTd driver, which
will be provide in other patch in the future.

The purpose of this patch series to review if this IOMMU protocol design
can meet all DMA access and management requirement.

Cc: Ruiyu Ni 
Cc: Leo Duran 
Cc: Brijesh Singh 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 

Jiewen Yao (3):
  MdeModulePkg/Include: Add IOMMU protocol definition.
  MdeModulePkg/PciHostBridge: Add IOMMU support.
  MdeModulePkg/PciBus: Add IOMMU support.

 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.c|   9 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h|   1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf   |   1 +
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c |  47 +++-
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c  |  37 +++
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf |   2 +
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridge.h  |   2 +
 MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c|  61 +
 MdeModulePkg/Include/Protocol/IoMmu.h  | 259 

[edk2] [PATCH V5 1/3] MdeModulePkg/Include: Add IOMMU protocol definition.

[Jiewen Yao]

This protocol is to abstract DMA access from IOMMU.
1) Intel "DMAR" ACPI table.
2) AMD "IVRS" ACPI table
3) ARM "IORT" ACPI table.

There might be multiple IOMMU engines on one platform.
For example, one for graphic and one for rest PCI devices
(such as ATA/USB).
All IOMMU engines are reported by one ACPI table.

All IOMMU protocol provider should be based upon ACPI table.
This single IOMMU protocol can handle multiple IOMMU engines on one system.

This IOMMU protocol provider can use UEFI device path to distinguish
if the device is graphic or ATA/USB, and find out corresponding
IOMMU engine.

The IOMMU protocol provides 2 capabilities:
A) Set DMA access attribute - such as write/read control.
B) Remap DMA memory - such as remap above 4GiB system memory address
to below 4GiB device address.
It provides AllocateBuffer/FreeBuffer/Map/Unmap for DMA memory.
The remapping can be static (fixed at build time) or dynamic (allocate
at runtime).

4) AMD "SEV" feature.
We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
and manage SEV bit.

Cc: Ruiyu Ni 
Cc: Leo Duran 
Cc: Brijesh Singh 
Cc: Ard Biesheuvel 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Include/Protocol/IoMmu.h | 259 
 MdeModulePkg/MdeModulePkg.dec |   3 +
 2 files changed, 262 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/IoMmu.h 
b/MdeModulePkg/Include/Protocol/IoMmu.h
new file mode 100644
index 000..9d25c17
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/IoMmu.h
@@ -0,0 +1,259 @@
+/** @file
+  EFI IOMMU Protocol.
+
+Copyright (c) 2017, Intel Corporation. All rights reserved.
+This program and the accompanying materials are licensed and made available 
under
+the terms and conditions of the BSD License that accompanies this distribution.
+The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php.
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+
+#ifndef __IOMMU_H__
+#define __IOMMU_H__
+
+//
+// IOMMU Protocol GUID value
+//
+#define EDKII_IOMMU_PROTOCOL_GUID \
+{ \
+  0x4e939de9, 0xd948, 0x4b0f, { 0x88, 0xed, 0xe6, 0xe1, 0xce, 0x51, 0x7c, 
0x1e } \
+}
+
+//
+// Forward reference for pure ANSI compatability
+//
+typedef struct _EDKII_IOMMU_PROTOCOL  EDKII_IOMMU_PROTOCOL;
+
+//
+// Revision The revision to which the IOMMU interface adheres.
+//  All future revisions must be backwards compatible.
+//  If a future version is not back wards compatible it is not the 
same GUID.
+//
+#define EDKII_IOMMU_PROTOCOL_REVISION 0x0001
+
+//
+// IOMMU Access for SetAttribute
+//
+// These types can be "ORed" together as needed.
+// Any undefined bits are reserved and must be zero.
+//
+#define EDKII_IOMMU_ACCESS_READ   0x1
+#define EDKII_IOMMU_ACCESS_WRITE  0x2
+
+//
+// IOMMU Operation for Map
+//
+typedef enum {
+  ///
+  /// A read operation from system memory by a bus master that is not capable 
of producing
+  /// PCI dual address cycles.
+  ///
+  EdkiiIoMmuOperationBusMasterRead,
+  ///
+  /// A write operation from system memory by a bus master that is not capable 
of producing
+  /// PCI dual address cycles.
+  ///
+  EdkiiIoMmuOperationBusMasterWrite,
+  ///
+  /// Provides both read and write access to system memory by both the 
processor and a bus
+  /// master that is not capable of producing PCI dual address cycles.
+  ///
+  EdkiiIoMmuOperationBusMasterCommonBuffer,
+  ///
+  /// A read operation from system memory by a bus master that is capable of 
producing PCI
+  /// dual address cycles.
+  ///
+  EdkiiIoMmuOperationBusMasterRead64,
+  ///
+  /// A write operation to system memory by a bus master that is capable of 
producing PCI
+  /// dual address cycles.
+  ///
+  EdkiiIoMmuOperationBusMasterWrite64,
+  ///
+  /// Provides both read and write access to system memory by both the 
processor and a bus
+  /// master that is capable of producing PCI dual address cycles.
+  ///
+  EdkiiIoMmuOperationBusMasterCommonBuffer64,
+  EdkiiIoMmuOperationMaximum
+} EDKII_IOMMU_OPERATION;
+
+//
+// IOMMU attribute for AllocateBuffer
+// Any undefined bits are reserved and must be zero.
+//
+#define EDKII_IOMMU_ATTRIBUTE_MEMORY_WRITE_COMBINE0x0080
+#define EDKII_IOMMU_ATTRIBUTE_MEMORY_CACHED   0x0800
+#define EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE  0x8000
+
+#define EDKII_IOMMU_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER   
(EDKII_IOMMU_ATTRIBUTE_MEMORY_WRITE_COMBINE | 
EDKII_IOMMU_ATTRIBUTE_MEMORY_CACHED | EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
+
+#define EDKII_IOMMU_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER 
(~EDKII_IOMMU_ATTRIBUTE_VALID_FOR_ALLOCATE_BUFFER)
+
+/**
+  Set IOMMU attribute for a system memory.
+
+  If the IOMMU 

Re: [edk2] ShellPkg: Difference in behavior of 'dh' between old shell and new

[Carsey, Jaben]

Pretty much. 

The shell can absolutely perform better by using them, but it must not generate 
errors due to not finding them.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jeff Westfahl
> Sent: Thursday, May 04, 2017 8:00 AM
> To: Carsey, Jaben 
> Cc: Ni, Ruiyu ; edk2-devel@lists.01.org
> Subject: Re: [edk2] ShellPkg: Difference in behavior of 'dh' between old shell
> and new
> Importance: High
> 
> Hi Jaben,
> 
> No PI protocols can be required means that I can try to use them? And
> gracefully handle the case where they aren't found?
> 
> Regards,
> Jeff
> 
> On Wed, 19 Apr 2017, Carsey, Jaben wrote:
> 
> > That would be welcome; I don't know how it was missed.  A caveat is that
> the shell should use UEFI protocols only.  No PI protocols can be required
> (optional is ok).
> >
> > Only the specific output controlled by "-sfo" is severely constrained by the
> spec.
> >
> >> -Original Message-
> >> From: Jeff Westfahl [mailto:jeff.westf...@ni.com]
> >> Sent: Wednesday, April 19, 2017 10:36 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: Carsey, Jaben ; Ni, Ruiyu
> 
> >> Subject: [edk2] ShellPkg: Difference in behavior of 'dh' between old shell
> and
> >> new
> >> Importance: High
> >>
> >> I noticed a difference in behavior of 'dh' between the old shell and the
> new shell.
> >> With the old shell, 'dh -v' for a LoadedImage handle shows the
> >> following:
> >>
> >> Handle D3 (3A537F98)
> >> Image (3A532818)   File:MicrocodeUpdate
> >>   ParentHandle..: 3A64F118
> >>   SystemTable...: 3D2A8F18
> >>   DeviceHandle..: 3B1B2098
> >>   FilePath..: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
> >>   ImageBase.: 3D65 - 3D655540
> >>   ImageSize.: 5540
> >>   CodeType..: RT_code
> >>   DataType..: RT_data
> >>
> >> With the new shell, I get this for the same LoadedImage handle:
> >>
> >> D3: 3A537F98
> >> LoadedImage
> >>   Revision..: 0x1000
> >>   ParentHandle..: 3A64F118
> >>   SystemTable...: 3D2A8F18
> >>   DeviceHandle..: 3B1B2098
> >>   FilePath..: 3A539018
> >>   OptionsSize...: 0
> >>   LoadOptions...: 0
> >>   ImageBase.: 3D65
> >>   ImageSize.: 5540
> >>   CodeType..: EfiRuntimeServicesCode
> >>   DataType..: EfiRuntimeServicesData
> >>   Unload: 0
> >>
> >> The old shell shows the name of the file associated with the
> LoadedImage,
> >> which seems like useful information. Is this omission intentional or an
> oversight?
> >> Would a patch adding the file name to ShellPkg be welcomed?
> >>
> >> Regards,
> >> Jeff
> >
> ___
> 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] SMRAM sizes on large hosts

[Laszlo Ersek]

On 05/04/17 16:52, Gerd Hoffmann wrote:
>   Hi,
> 
>> If we invent such a new register, it should be in a location that is
>> either read-only, or zeroed-on-reset, in current QEMU. Otherwise, new
>> firmware running on old QEMU could be misled by a guest OS that writes
>> to this register, and then either reboots or enters S3.
> 
> Good point, we need to be quite careful here to not open security holes.
> Current state is that pretty much all pci config space is writable and
> not cleared on reset.  So no easy way out.
> 
>> ... With this in mind, I don't oppose "having to write somewhere to read
>> back the result", but then let's please make that write access as well
>> to the same new qemu-specific register, and not to MCH_ESMRAMC.
> 
> That should work, yes.  Write '1' to the register, then read back.  If
> it is still '1' -> no big tseg support.  Otherwise it returns the tseg
> size in some form, and "11b" in ESMRAMC can be used to pick that.

My thoughts exactly!

Thank you,
Laszlo

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

Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.

[Andrew Fish]

Amit,

In regards to AVX/AVX2 performance how are you doing the measuring?

In EFI it is hard to measure wall clock time for things that take a long time. 
Basically there is no scheduler in EFI and no threads, but there are events. 
The events can preempt your App while it is running and the time spent in 
events would look to you like time spent in your App. 

Generally the time spent in events should be constant (hot plugging USB or 
other changes like that may have a noticeable impact). If the goal of the 
performance measurement is to make the system boot faster you care more about 
the delta, than the absolute time (so the event overhead does not matter). 

If you are just doing a computation that does not do any IO then you may be 
able to raise the TPL to prevent events from being part of your measurement. 

Thanks,

Andrew Fish

PS I assume your are measuring the RELEASE code since you are turning off 
optimization on the DEBUG code. 

> On May 4, 2017, at 5:22 AM, Amit kumar  wrote:
> 
> Here are the compiler flags
> [BuildOptions]
>  MSFT:DEBUG_*_*_CC_FLAGS = /Od /FAsc /GL-
>  MSFT:RELEASE_*_*_CC_FLAGS = /FAsc /D MDEPKG_NDEBUG
>  MSFT:RELEASE_*_*_DLINK_FLAGS = /BASE:0x1  /ALIGN:4096 /FILEALIGN:4096
> 
> 
> 
> From: Amit kumar 
> Sent: Thursday, May 4, 2017 5:48:11 PM
> To: Andrew Fish
> Cc: Mike Kinney; edk2-devel@lists.01.org
> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
> 
> 
> Yes am aligning the data at 32 byte boundary while allocating memory in both 
> environments.
> 
> in windows using  _alligned_malloc(size,32);
> 
> in UEFI
> 
> Offset = (UINTN)src & 0xFF;
> 
> src = (CHAR8 *)((UINTN) src - Offset + 0x20);
> 
> 
> Thanks
> 
> Amit
> 
> 
> From: af...@apple.com  on behalf of Andrew Fish 
> 
> Sent: Thursday, May 4, 2017 5:02:55 PM
> To: Amit kumar
> Cc: Mike Kinney; edk2-devel@lists.01.org
> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
> 
> 
>> On May 4, 2017, at 4:13 AM, Amit kumar  wrote:
>> 
>> Hi,
>> 
>> 
>> Even after using AVX2 instruction my code shown no performance improvement 
>> in UEFI although there is substantial improvement when i run the similar 
>> code in windows .
>> 
>> Am i missing something ?
>> 
> 
> Is the data aligned the same in both environments?
> 
> Thanks,
> 
> Andrew Fish
> 
>> Using MSVC compiler and the codes written in ASM.
>> 
>> Thanks And Regards
>> 
>> Amit
>> 
>> 
>> From: edk2-devel  on behalf of Amit kumar 
>> 
>> Sent: Wednesday, May 3, 2017 11:18:39 AM
>> To: Kinney, Michael D; Andrew Fish
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>> 
>> Thank you Michael and Andrew
>> 
>> 
>> Regards
>> 
>> Amit
>> 
>> 
>> From: Kinney, Michael D 
>> Sent: Tuesday, May 2, 2017 10:33:45 PM
>> To: Andrew Fish; Amit kumar; Kinney, Michael D
>> Cc: edk2-devel@lists.01.org
>> Subject: RE: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>> 
>> Amit,
>> 
>> The information from Andrew is correct.
>> 
>> The document that covers this topic is the
>> Intel(r) 64 and IA-32 Architectures Software Developer Manuals
>> 
>> https://software.intel.com/en-us/articles/intel-sdm
>> 
>> Volume 1, Section 13.5.3 describes the AVX State.  There are
>> More details about detecting and enabling different AVX features
>> in that document.
>> 
>> If the CPU supports AVX, then the basic assembly instructions
>> required to use AVX instructions are the following that sets
>> bits 0, 1, 2 of XCR0.
>> 
>>   mov rcx, 0
>>   xgetbv
>>   or  rax, 0007h
>>   xsetbv
>> 
>> One additional item you need to be aware of is that UEFI firmware only
>> saves/Restores CPU registers required for the UEFI ABI calling convention
>> when a timer interrupt or exception is processed.
>> 
>> This means CPU state such as the YMM registers are not saved/restored
>> across an interrupt and may be modified if code in interrupt context
>> also uses YMM registers.
>> 
>> When you enable the use of extended registers, interrupts should be
>> saved/disabled and restored around the extended register usage.
>> 
>> You can use the following functions from MdePkg BaseLib to do this
>> 
>> /**
>> Disables CPU interrupts and returns the interrupt state prior to the disable
>> operation.
>> 
>> @retval TRUE  CPU interrupts were enabled on entry to this call.
>> @retval FALSE CPU interrupts were disabled on entry to this call.
>> 
>> **/
>> BOOLEAN
>> EFIAPI
>> SaveAndDisableInterrupts (
>> VOID
>> );
>> 
>> /**
>> Set the current CPU interrupt state.
>> 
>> Sets the current CPU interrupt state to the state specified by
>> InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
>> InterruptState is FALSE, 

Re: [edk2] SMRAM sizes on large hosts

[Laszlo Ersek]

On 05/04/17 16:50, Igor Mammedov wrote:
> On Thu, 04 May 2017 16:41:00 +0200
> Gerd Hoffmann  wrote:
> 
>>   Hi,
>>
>>> There is another thing to consider here, when vm is migrated to newer
>>> qemu(with newer firmware version) then it might not boot on the next
>>> restart due to hitting old set limit.  
>>
>> Firmware is part of the migration data, so you have the same version on
>> both ends.
> true, until VM is shut down. After that it loads new firmware
> from target host. So user will have see error that tells what is wrong
> and manually fix limit value if vm startup config to another value
> that would work for this specific fw/config combo.

I've been generally promoting the idea that firmware logs should be
captured by libvirt by default, and exposed to the user similarly to
QEMU error messages / error logs... Some rate limiting and/or log
rotation would be necessary, so that guest code cannot flood the host
system with infinite logs, but that wouldn't prevent saving error
messages from non-malicious firmware.

Laszlo

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

Re: [edk2] ShellPkg: Difference in behavior of 'dh' between old shell and new

[Jeff Westfahl]

Hi Jaben,

No PI protocols can be required means that I can try to use them? And 
gracefully handle the case where they aren't found?


Regards,
Jeff

On Wed, 19 Apr 2017, Carsey, Jaben wrote:


That would be welcome; I don't know how it was missed.  A caveat is that the 
shell should use UEFI protocols only.  No PI protocols can be required 
(optional is ok).

Only the specific output controlled by "-sfo" is severely constrained by the 
spec.


-Original Message-
From: Jeff Westfahl [mailto:jeff.westf...@ni.com]
Sent: Wednesday, April 19, 2017 10:36 AM
To: edk2-devel@lists.01.org
Cc: Carsey, Jaben ; Ni, Ruiyu 
Subject: [edk2] ShellPkg: Difference in behavior of 'dh' between old shell and
new
Importance: High

I noticed a difference in behavior of 'dh' between the old shell and the new 
shell.
With the old shell, 'dh -v' for a LoadedImage handle shows the
following:

Handle D3 (3A537F98)
Image (3A532818)   File:MicrocodeUpdate
  ParentHandle..: 3A64F118
  SystemTable...: 3D2A8F18
  DeviceHandle..: 3B1B2098
  FilePath..: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
  ImageBase.: 3D65 - 3D655540
  ImageSize.: 5540
  CodeType..: RT_code
  DataType..: RT_data

With the new shell, I get this for the same LoadedImage handle:

D3: 3A537F98
LoadedImage
  Revision..: 0x1000
  ParentHandle..: 3A64F118
  SystemTable...: 3D2A8F18
  DeviceHandle..: 3B1B2098
  FilePath..: 3A539018
  OptionsSize...: 0
  LoadOptions...: 0
  ImageBase.: 3D65
  ImageSize.: 5540
  CodeType..: EfiRuntimeServicesCode
  DataType..: EfiRuntimeServicesData
  Unload: 0

The old shell shows the name of the file associated with the LoadedImage,
which seems like useful information. Is this omission intentional or an 
oversight?
Would a patch adding the file name to ShellPkg be welcomed?

Regards,
Jeff



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

Re: [edk2] SMRAM sizes on large hosts

[Gerd Hoffmann]

  Hi,

> If we invent such a new register, it should be in a location that is
> either read-only, or zeroed-on-reset, in current QEMU. Otherwise, new
> firmware running on old QEMU could be misled by a guest OS that writes
> to this register, and then either reboots or enters S3.

Good point, we need to be quite careful here to not open security holes.
Current state is that pretty much all pci config space is writable and
not cleared on reset.  So no easy way out.

> ... With this in mind, I don't oppose "having to write somewhere to read
> back the result", but then let's please make that write access as well
> to the same new qemu-specific register, and not to MCH_ESMRAMC.

That should work, yes.  Write '1' to the register, then read back.  If
it is still '1' -> no big tseg support.  Otherwise it returns the tseg
size in some form, and "11b" in ESMRAMC can be used to pick that.

cheers,
  Gerd

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

Re: [edk2] SMRAM sizes on large hosts

[Igor Mammedov]

On Thu, 04 May 2017 16:41:00 +0200
Gerd Hoffmann  wrote:

>   Hi,
> 
> > There is another thing to consider here, when vm is migrated to newer
> > qemu(with newer firmware version) then it might not boot on the next
> > restart due to hitting old set limit.  
> 
> Firmware is part of the migration data, so you have the same version on
> both ends.
true, until VM is shut down. After that it loads new firmware
from target host. So user will have see error that tells what is wrong
and manually fix limit value if vm startup config to another value
that would work for this specific fw/config combo.

> 
> cheers,
>   Gerd
> 

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

Re: [edk2] ArmPlatformPkg: LcdGraphicsOutputDxe, PL111, and HdLcd rejig

[Evan Lloyd]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
...
> >
> > The real improvement is the reduction in code duplication.
> >
>
> Perhaps we shoukd move to the UEFI driver model at the same time?
>
[[Evan Lloyd]] That would certainly be the "proper" way to do it.
The reasons not to are:
1. So that early output can be seen on the display.
(I confess that argument would be stronger were ConSplitterDxe not 
itself UEFI driver model)  :-{
2. The theoretical case of a boot logo, which, I think, should flash up as 
early as possible.

However, we don't feel strongly about either.
...
Regards,
Evan

IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium. Thank you.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] SMRAM sizes on large hosts

[Gerd Hoffmann]

  Hi,

> There is another thing to consider here, when vm is migrated to newer
> qemu(with newer firmware version) then it might not boot on the next
> restart due to hitting old set limit.

Firmware is part of the migration data, so you have the same version on
both ends.

cheers,
  Gerd

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

Re: [edk2] SMRAM sizes on large hosts

[Igor Mammedov]

On Thu, 4 May 2017 13:34:13 +0200
Laszlo Ersek  wrote:

> On 05/04/17 10:23, Igor Mammedov wrote:
> > On Thu, 4 May 2017 00:33:27 +0200
> > Laszlo Ersek  wrote:
> > 
> > [...]   
> >> If we invented a read-only, side-effect-free PCI config space register
> >> that gave me this value plain and simple (similarly to how a new fw_cfg
> >> file would do), that would be a lot cleaner for me.  
> > Just a thought,
> > have you considered firmware setting size it needs explicitly?
> > That way we won't have to bump that value on qemu side when
> > qemu dictated size becomes too small and won't need compat cruft
> > around it.  
> 
> The problem is that I don't know what size to set. The per-VCPU SMRAM
> demand varies (mostly, grows) over time as edk2's SMM stack gets new
> features and/or is refactored occasionally. The size hint would have to
> come from OvmfPkg (platform code) while the overwhelming majority of the
> SMM stack lives outside of OvmfPkg.
> 
> Also, it's not just data that is allocated from SMRAM, it's also the SMM
> driver binaries themselves. The size of those varies even with the
> compiler toolchain that you use to build OVMF -- for example, with
> gcc-5, link-time optimization is enabled in edk2, which results in
> significantly smaller binaries --, and whether you build OVMF for NOOPT
> / DEBUG / RELEASE. This kind of difference is likely not significant per
> se, but it could be the difference between working with N*100 VCPUs, or
> only with N*100-5 VCPUs.
looks complicated, but still it would be the best option.
Anyways, I don't insist.
 
> So I continue to think of SMRAM size as another board property, like
> plain RAM size. If the guest payload doesn't fit, make QEMU provide more
> of it, be it disk space, normal RAM, or SMRAM. In fact I think the SMRAM
> size property should not be an X-* property but something that users
> could *validly* override on the command line, if they wanted to. Even
> exposing it on the libvirt level wouldn't be wrong, I believe; the same
> way we already expose whether SMM emulation is enabled at all.
Agreed, if it's a public property set by management layers and
firmware will crash with clear message it would work as well.

There is another thing to consider here, when vm is migrated to newer
qemu(with newer firmware version) then it might not boot on the next
restart due to hitting old set limit.

> Thanks
> Laszlo

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

Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance

[Zeng, Star]

Tim,

To avoid misunderstanding, I think I need to clarify more about this RFC. :)

This RFC is NOT to propose building real relationship (like parent-child) 
between non-DEFAULT SKUs, it seems easy to bring confusion from the proposed 
syntax " SkuValue|SkuName[|ParentSkuName]", especially the word 'Parent', sorry 
for any confusion.
The syntax extension proposed by this RFC is just to simplify the PCDs 
configuring for multiple SKUs in DSC, I want to emphasize it is in DSC, it is 
to cover the case that some non-DEFAULT SKU has much PCD values equal with 
another non-DEFAULT SKU, but less PCD values equal with DEFAULT SKU.
This RFC has no any change to boot time behavior and does not break the rule 
that DEFAULT SKU PCD value will be returned if the specified SKU PCD value is 
not configured in DSC, the code logic is below.

PcdPeim Service.c:
  //
  // Find the current system's SKU ID entry in SKU ID table.
  //
  FoundSku = FALSE;
  for (Index = 0; Index < SkuIdTable[0]; Index++) {
if (PeiPcdDb->SystemSkuId == SkuIdTable[Index + 1]) {
  FoundSku = TRUE;
  break;
}
  }

  //
  // Find the default SKU ID entry in SKU ID table.
  //
  if(!FoundSku) {
for (Index = 0; Index < SkuIdTable[0]; Index++) {
  if (0 == SkuIdTable[Index + 1]) {
break;
  }
}
  }


The usage case you provided for other resources beyond PCD could be like below?

CurrentSkuId = PcdGetSkuId();

Resource = NULL;
if (!GetResourceForSkuId(CurrentSkuId, ) {
  // try DEFAULT SKU
  GetResourceForSkuId(0, );
}
If (Resource == NULL) {
  // error, no resource found for any of the SKU IDs }



Thanks,
Star
-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Tim Lewis
Sent: Thursday, May 4, 2017 2:55 AM
To: Zeng, Star ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: Re: [edk2] [RFC] PCD: Extended SKU support 1 - inheritance

We use SKU IDs for working with 15 different key resources in our product. Some 
are related to code execution, but most are related to selection of resources. 
I mentioned logo components and HII strings already.

I am asking that the information about SKU relationships be output in some form 
so that it can be used at runtime, by whatever component. Today, there is an 
implied relationship between a SKU X and the SKU DEFAULT for PCDs. We use this 
rule in many places, because it expresses a useful relationship.

So, in the Project's .dsc file (forgive my syntax, which may not match yours 
exactly)

// Select which SKUs
SKUID_IDENTIFIER = Sku2 Sku1 DEFAULT

[SkuIds]
  0|DEFAULT
  1|Sku1
  2|Sku2 Sku1
  3|Sku3

In (proposed) AutoGen.h. This gives enough data for runtime usage. For other 
build tools to use, maybe something else is required.

// Output as an array initializer. DEFAULT is always last.
// Only output for SKUs listed in SKUID_IDENTIFIER // 2->1->0 // 1->0 // 0 
#define SKUID_IDENTIFIER_DEFAULT 2,1,0,1,0,0

// In actual code:

UINT32 SkuIdDefaults[] = {SKUID_IDENTIFIER_DEFAULT};
UINT32 *SkuId;
UINT32 CurrentSkuId;
BOOLEAN Found;
VOID *Resource;

CurrentSkuId = PcdGetSkuId();

SkuId = SkuIdDefaults;
Found = (CurrentSkuId == 0); // so that DEFAULT will also return TRUE; while 
(*SkuId != 0) {
  if (*SkuId == CurrentSkuId) {
Found = TRUE;
Break;
  }
  while (*SkuId != 0) {
 SkuId++;
  }
  SkuId++;
}
if (!Found) {
  //error, because SKU ID was not found in array }

Resource = NULL;
while (!GetResourceForSkuId(*SkuId, ) && *SkuId == 0) {
  *SkuId++;
}
If (Resource == NULL) {
  // error, no resource found for any of the SKU IDs }






From: Zeng, Star [mailto:star.z...@intel.com]
Sent: Wednesday, May 03, 2017 7:03 AM
To: Tim Lewis ; Kinney, Michael D 
; edk2-devel@lists.01.org
Cc: Gao, Liming ; Zeng, Star 
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Tim,

Could you help share and explain the use cases in your code base that is using 
[SkuIds] section and SKUID_IDENTIFIER for others beyond PCD?
Then we can consider more in [RFC] PCD: Extended SKU support. Or do you have 
detailed propose in this RFC or new RFC about your use cases?

Thanks,
Star
From: Kinney, Michael D
Sent: Friday, April 28, 2017 11:26 AM
To: Tim Lewis >; Zeng, Star 
>; 
edk2-devel@lists.01.org; Kinney, Michael D 
>
Cc: Gao, Liming >; Zeng, Star 
>
Subject: RE: [RFC] PCD: Extended SKU support 1 - inheritance

Hi Star,

The 2nd RFC for SKUs add some autogen to support setting the SKU from a 
platform PEIM.

We should consider additional autogen that allows a module to 

Re: [edk2] ArmPlatformPkg: LcdGraphicsOutputDxe, PL111, and HdLcd rejig

[Ard Biesheuvel]

> On 4 May 2017, at 12:14, Leif Lindholm  wrote:
> 
> Hi Girish,
> 
>> On Wed, May 03, 2017 at 04:58:33PM +, Girish Pathak wrote:
>> With a view to an upcoming change, we have been examining the current 
>> Graphics
>> Output Protocol implementation. Currently for, ARM platforms, the UEFI 
>> Graphics
>> Output Protocol is implemented using a platform specific Library
>> (PL111LcdArmVExpressLib/HdLcdArmVExpressLib) and a DXE driver
>> (PL111LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe). The platform specific
>> library handles platform specific variations such as platform supported 
>> display
>> modes, memory management of the frame buffer, and clock/mux setting. The DXE
>> driver implements the GOP protocol and manages the respective display IP.
>> Although this implementation works fine for current platforms, we think the 
>> way
>> the current DXE driver sources are linked is sub-optimal and can be improved 
>> to
>> meet recommendation of the EDKII Module Writer's Guide.
> 
> The real improvement is the reduction in code duplication.
> 

Perhaps we shoukd move to the UEFI driver model at the same time?

>> The DXE driver source contains three source files per driver. The files
>> LcdGraphicsOutputDxe.c and LcdGraphicsOutputBlt.c implement common
>> functionality whereas HdLcd.c/PL111Lcd.c  implement display IP specific part 
>> of
>> the DXE. The problem is, there are two .inf files for HdLcdGraphicsOutputDxe
>> and PL111LcdGraphicsOutputDxe and both link common code 
>> LcdGraphicsOutputDxe.c
>> and LcdGraphicsOutputBlt.c with display IP source instead of a library 
>> instance
>> , which seems incorrect and can be improved. We propose to separate HdLcd.c
>> and PL111Lcd.c and create independent libraries managing only respective
>> display IP which can then be instantiated as LcdHwLib and linked in a common
>> LcdGraphicsOutputDxe DXE driver. This will help to clearly partition
>> implementation of the Graphics Output Protocol into three separate 
>> components,
>> a platform specific component for the display IP, a display IP specific
>> component and GOP common code.
> 
> This all sounds imminently sensible. One request for clarifiation below.
> 
>> So instead of the current structure in ArmPlatformPkg for display DXE:
>> 
>>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
>> (HDLCD GOP DXE)
>>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf   
>>  (PL111 GOP DXE)
>> 
>> We propose a structure like:
>> 
>>  ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
>> (GOP DXE independent of hardware/platform)
> 
> Are we expecting this one to be truly independent, or simply common
> between most/all ARM Ltd. display controllers?
> 
>>  ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf(Library code managing HDLCD HW 
>> IP)
>>  ArmPlatformPkg/Drivers/PL111/PL11Lcd.inf(Library code managing PL111 HW 
>> IP)
>> 
>> LcdGraphicsOutputDxe.inf will link to LcdPlatformLib and LcdHwLib which can 
>> be
>> selected for the platform in the platform specific .dsc file.
>> 
>> e.g.
>> 
>> Under LibraryClasses we might have:
>>
>> LcdPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
>>LcdHwLib|ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf
>> 
>> And Under Components:
>>ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf  
>> (Common GOP DXE code for ARM Platforms)
>> 
>> This is a significant change and we would like to invite viewpoints before we
>> proceed with implementing these. Since the change would only be with respect 
>> to
>> display aspects of ArmPlatformPkg we don't foresee any impact on any other
>> functionality.
>> 
>> Please reply if you feel this intended change might impact you and why ?
>> Unless objections are raised, we will soon submit the patches for review.
> 
> Sounds good to me.
> 
> Regards,
> 
> Leif
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [RFC] [PATCH V4 0/3] Add IOMMU support.

[Yao, Jiewen]

Thanks. I agree.

From: Ni, Ruiyu
Sent: Thursday, May 4, 2017 3:02 PM
To: Yao, Jiewen ; edk2-devel@lists.01.org
Cc: Leo Duran ; Brijesh Singh ; Ard 
Biesheuvel 
Subject: RE: [RFC] [PATCH V4 0/3] Add IOMMU support.

Jiewen,
All 3 patches are good to me.
When you post V5 to retire one of the SetxxxAttribute() interfaces from IO_MMU 
protocol,
I suggest we keep the Mapping version, but rename the function name to 
SetAttribute().
As the 2 changes in below:

typedef
EFI_STATUS
(EFIAPI *EDKII_IOMMU_SET_ATTRIBUTE)( // <-- 1. Remove the _MAPPING
  IN EDKII_IOMMU_PROTOCOL  *This,
  IN EFI_HANDLEDeviceHandle,
  IN VOID  *Mapping,
  IN UINT64IoMmuAccess
  );

///
/// IOMMU Protocol structure.
///
struct _EDKII_IOMMU_PROTOCOL {
  UINT64  Revision;
  EDKII_IOMMU_SET_ATTRIBUTE   SetAttribute;
  EDKII_IOMMU_MAP Map;
  EDKII_IOMMU_UNMAP   Unmap;
  EDKII_IOMMU_ALLOCATE_BUFFER AllocateBuffer;
  EDKII_IOMMU_FREE_BUFFER FreeBuffer;
  // <-- 2. Remove the SetMappingAttribute interface.
};

Thanks/Ray

> -Original Message-
> From: Yao, Jiewen
> Sent: Saturday, April 29, 2017 9:48 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu >; Leo Duran 
> >; Brijesh
> Singh >; Ard Biesheuvel 
> >
> Subject: [RFC] [PATCH V4 0/3] Add IOMMU support.
>
>  V4 ==
> Refine the EDKII_IOMMU_PROTOCOL.
>
> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
> They are similar to DmaLib in EmbeddedPkg and similar to the previous
> BmDmaLib (by leo.du...@amd.com).
>
> These APIs are invoked by PciHostBridge driver to allocate DMA memory.
>
> The PciHostBridge driver (IOMMU consumer) is simplified:
> It uses IOMMU, if IOMMU protocol is present.
> Else it uses original logic.
>
> 2) Add SetMappingAttribute() API.
> It is similar to SetAttribute() API in V1.
>
> This API is invoked by PciBus driver to set DMA access attribute (read/write) 
> for
> device.
>
> The PciBus driver (IOMMU consumer) is simplified:
> It sets access attribute in Map/Unmap,
> if IOMMU protocol is present.
>
> 3) Remove SetRemapAddress/GetRemapAddress() API.
> Because PciHostBridge/PciBus can call the APIs defined above, there is no need
> to provide remap capability.
>
> -- Sample producer drivers:
> 1) The sample VTd driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
>
> It is added to show the concept. It is not fully implemented yet.
> It will not be checked in in this patch.
>
> 2) The sample AMD SEV driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSevDx
> e
> (code is borrowed from leo.du...@amd.com and 
> brijesh.si...@amd.com)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
> 3) The sample STYX driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDxe
> (code is borrowed from 
> ard.biesheu...@linaro.org)
>
> This is not a right place to put this driver.
>
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
>
>
>  V3 ==
> 1) Add Remap capability (from Ard Biesheuvel) Add
> EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
>
> NOTE: The code is not fully validated yet.
> The purpose is to collect feedback to decide the next step.
>
>  V2 ==
> 1) Enhance Unmap() in PciIo (From Ruiyu Ni) Maintain a local list of MapInfo 
> and
> match it in Unmap.
>
> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran) Fix a bug
> in V1 that copy mem for read happen before SetAttribute, which will break AMD
> SEV solution.
>
>  V1 ==
>
> This patch series adds IOMMU protocol and updates the consumer to support
> IOMMU based DMA access in UEFI.
>
> This patch series can support the BmDmaLib request for AMD SEV.
> submitted by Duran, Leo > and 
> Brijesh Singh
> >.
> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
> We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
> and clear SEV in IOMMU->SetAttribute().
>
> This patch series can also 

Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.

[Amit kumar]

Here are the compiler flags
[BuildOptions]
  MSFT:DEBUG_*_*_CC_FLAGS = /Od /FAsc /GL-
  MSFT:RELEASE_*_*_CC_FLAGS = /FAsc /D MDEPKG_NDEBUG
  MSFT:RELEASE_*_*_DLINK_FLAGS = /BASE:0x1  /ALIGN:4096 /FILEALIGN:4096



From: Amit kumar 
Sent: Thursday, May 4, 2017 5:48:11 PM
To: Andrew Fish
Cc: Mike Kinney; edk2-devel@lists.01.org
Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.


Yes am aligning the data at 32 byte boundary while allocating memory in both 
environments.

in windows using  _alligned_malloc(size,32);

in UEFI

Offset = (UINTN)src & 0xFF;

src = (CHAR8 *)((UINTN) src - Offset + 0x20);


Thanks

Amit


From: af...@apple.com  on behalf of Andrew Fish 

Sent: Thursday, May 4, 2017 5:02:55 PM
To: Amit kumar
Cc: Mike Kinney; edk2-devel@lists.01.org
Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.


> On May 4, 2017, at 4:13 AM, Amit kumar  wrote:
>
> Hi,
>
>
> Even after using AVX2 instruction my code shown no performance improvement in 
> UEFI although there is substantial improvement when i run the similar code in 
> windows .
>
> Am i missing something ?
>

Is the data aligned the same in both environments?

Thanks,

Andrew Fish

> Using MSVC compiler and the codes written in ASM.
>
> Thanks And Regards
>
> Amit
>
> 
> From: edk2-devel  on behalf of Amit kumar 
> 
> Sent: Wednesday, May 3, 2017 11:18:39 AM
> To: Kinney, Michael D; Andrew Fish
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>
> Thank you Michael and Andrew
>
>
> Regards
>
> Amit
>
> 
> From: Kinney, Michael D 
> Sent: Tuesday, May 2, 2017 10:33:45 PM
> To: Andrew Fish; Amit kumar; Kinney, Michael D
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>
> Amit,
>
> The information from Andrew is correct.
>
> The document that covers this topic is the
> Intel(r) 64 and IA-32 Architectures Software Developer Manuals
>
> https://software.intel.com/en-us/articles/intel-sdm
>
> Volume 1, Section 13.5.3 describes the AVX State.  There are
> More details about detecting and enabling different AVX features
> in that document.
>
> If the CPU supports AVX, then the basic assembly instructions
> required to use AVX instructions are the following that sets
> bits 0, 1, 2 of XCR0.
>
>mov rcx, 0
>xgetbv
>or  rax, 0007h
>xsetbv
>
> One additional item you need to be aware of is that UEFI firmware only
> saves/Restores CPU registers required for the UEFI ABI calling convention
> when a timer interrupt or exception is processed.
>
> This means CPU state such as the YMM registers are not saved/restored
> across an interrupt and may be modified if code in interrupt context
> also uses YMM registers.
>
> When you enable the use of extended registers, interrupts should be
> saved/disabled and restored around the extended register usage.
>
> You can use the following functions from MdePkg BaseLib to do this
>
> /**
>  Disables CPU interrupts and returns the interrupt state prior to the disable
>  operation.
>
>  @retval TRUE  CPU interrupts were enabled on entry to this call.
>  @retval FALSE CPU interrupts were disabled on entry to this call.
>
> **/
> BOOLEAN
> EFIAPI
> SaveAndDisableInterrupts (
>  VOID
>  );
>
> /**
>  Set the current CPU interrupt state.
>
>  Sets the current CPU interrupt state to the state specified by
>  InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
>  InterruptState is FALSE, then interrupts are disabled. InterruptState is
>  returned.
>
>  @param  InterruptState  TRUE if interrupts should enabled. FALSE if
>  interrupts should be disabled.
>
>  @return InterruptState
>
> **/
> BOOLEAN
> EFIAPI
> SetInterruptState (
>  IN  BOOLEAN   InterruptState
>  );
>
> Algorithm:
> 
> {
>  BOOLEAN  InterruptState;
>
>  InterruptState = SaveAndDisableInterrupts();
>
>  // Enable use of AVX/AVX2 instructions
>
>  // Use AVX/AVX2 instructions
>
>  SetInterruptState (InterruptState);
> }
>
> Best regards,
>
> Mike
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Andrew Fish
>> Sent: Tuesday, May 2, 2017 8:12 AM
>> To: Amit kumar 
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>>
>>
>>> On May 2, 2017, at 6:57 AM, Amit kumar  wrote:
>>>
>>> Hi,
>>>
>>> Am trying to optimize an application using AVX/AVX2, but my code hangs 
>>> while trying
>> to access YMM registers.
>>> The instruction where my code hangs is :
>>>
>>>
>>> vmovups ymm0, YMMWORD PTR [rax]
>>>
>>>
>>> I have verified the cpuid in OS 

Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.

[Amit kumar]

Yes am aligning the data at 32 byte boundary while allocating memory in both 
environments.

in windows using  _alligned_malloc(size,32);

in UEFI

Offset = (UINTN)src & 0xFF;

src = (CHAR8 *)((UINTN) src - Offset + 0x20);


Thanks

Amit


From: af...@apple.com  on behalf of Andrew Fish 

Sent: Thursday, May 4, 2017 5:02:55 PM
To: Amit kumar
Cc: Mike Kinney; edk2-devel@lists.01.org
Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.


> On May 4, 2017, at 4:13 AM, Amit kumar  wrote:
>
> Hi,
>
>
> Even after using AVX2 instruction my code shown no performance improvement in 
> UEFI although there is substantial improvement when i run the similar code in 
> windows .
>
> Am i missing something ?
>

Is the data aligned the same in both environments?

Thanks,

Andrew Fish

> Using MSVC compiler and the codes written in ASM.
>
> Thanks And Regards
>
> Amit
>
> 
> From: edk2-devel  on behalf of Amit kumar 
> 
> Sent: Wednesday, May 3, 2017 11:18:39 AM
> To: Kinney, Michael D; Andrew Fish
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>
> Thank you Michael and Andrew
>
>
> Regards
>
> Amit
>
> 
> From: Kinney, Michael D 
> Sent: Tuesday, May 2, 2017 10:33:45 PM
> To: Andrew Fish; Amit kumar; Kinney, Michael D
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>
> Amit,
>
> The information from Andrew is correct.
>
> The document that covers this topic is the
> Intel(r) 64 and IA-32 Architectures Software Developer Manuals
>
> https://software.intel.com/en-us/articles/intel-sdm
>
> Volume 1, Section 13.5.3 describes the AVX State.  There are
> More details about detecting and enabling different AVX features
> in that document.
>
> If the CPU supports AVX, then the basic assembly instructions
> required to use AVX instructions are the following that sets
> bits 0, 1, 2 of XCR0.
>
>mov rcx, 0
>xgetbv
>or  rax, 0007h
>xsetbv
>
> One additional item you need to be aware of is that UEFI firmware only
> saves/Restores CPU registers required for the UEFI ABI calling convention
> when a timer interrupt or exception is processed.
>
> This means CPU state such as the YMM registers are not saved/restored
> across an interrupt and may be modified if code in interrupt context
> also uses YMM registers.
>
> When you enable the use of extended registers, interrupts should be
> saved/disabled and restored around the extended register usage.
>
> You can use the following functions from MdePkg BaseLib to do this
>
> /**
>  Disables CPU interrupts and returns the interrupt state prior to the disable
>  operation.
>
>  @retval TRUE  CPU interrupts were enabled on entry to this call.
>  @retval FALSE CPU interrupts were disabled on entry to this call.
>
> **/
> BOOLEAN
> EFIAPI
> SaveAndDisableInterrupts (
>  VOID
>  );
>
> /**
>  Set the current CPU interrupt state.
>
>  Sets the current CPU interrupt state to the state specified by
>  InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
>  InterruptState is FALSE, then interrupts are disabled. InterruptState is
>  returned.
>
>  @param  InterruptState  TRUE if interrupts should enabled. FALSE if
>  interrupts should be disabled.
>
>  @return InterruptState
>
> **/
> BOOLEAN
> EFIAPI
> SetInterruptState (
>  IN  BOOLEAN   InterruptState
>  );
>
> Algorithm:
> 
> {
>  BOOLEAN  InterruptState;
>
>  InterruptState = SaveAndDisableInterrupts();
>
>  // Enable use of AVX/AVX2 instructions
>
>  // Use AVX/AVX2 instructions
>
>  SetInterruptState (InterruptState);
> }
>
> Best regards,
>
> Mike
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Andrew Fish
>> Sent: Tuesday, May 2, 2017 8:12 AM
>> To: Amit kumar 
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>>
>>
>>> On May 2, 2017, at 6:57 AM, Amit kumar  wrote:
>>>
>>> Hi,
>>>
>>> Am trying to optimize an application using AVX/AVX2, but my code hangs 
>>> while trying
>> to access YMM registers.
>>> The instruction where my code hangs is :
>>>
>>>
>>> vmovups ymm0, YMMWORD PTR [rax]
>>>
>>>
>>> I have verified the cpuid in OS and it supports AVX and AVX2 instruction. 
>>> Processor
>> i7 6th gen.
>>> Can somebody help me out here ? Is there a way to enable YMM registers ?
>>>
>>
>> Amit,
>>
>> I think these instructions will generate an illegal instruction fault until 
>> you enable
>> AVX. You need to check the Cpu ID bits in your code, then write BIT18 of 
>> CR4. After
>> that XGETBV/XSETBV instructions are enabled and you can or in the lower 2 
>> bits of
>> 

Re: [edk2] SMRAM sizes on large hosts

[Laszlo Ersek]

On 05/04/17 10:23, Igor Mammedov wrote:
> On Thu, 4 May 2017 00:33:27 +0200
> Laszlo Ersek  wrote:
> 
> [...] 
>> If we invented a read-only, side-effect-free PCI config space register
>> that gave me this value plain and simple (similarly to how a new fw_cfg
>> file would do), that would be a lot cleaner for me.
> Just a thought,
> have you considered firmware setting size it needs explicitly?
> That way we won't have to bump that value on qemu side when
> qemu dictated size becomes too small and won't need compat cruft
> around it.

The problem is that I don't know what size to set. The per-VCPU SMRAM
demand varies (mostly, grows) over time as edk2's SMM stack gets new
features and/or is refactored occasionally. The size hint would have to
come from OvmfPkg (platform code) while the overwhelming majority of the
SMM stack lives outside of OvmfPkg.

Also, it's not just data that is allocated from SMRAM, it's also the SMM
driver binaries themselves. The size of those varies even with the
compiler toolchain that you use to build OVMF -- for example, with
gcc-5, link-time optimization is enabled in edk2, which results in
significantly smaller binaries --, and whether you build OVMF for NOOPT
/ DEBUG / RELEASE. This kind of difference is likely not significant per
se, but it could be the difference between working with N*100 VCPUs, or
only with N*100-5 VCPUs.

So I continue to think of SMRAM size as another board property, like
plain RAM size. If the guest payload doesn't fit, make QEMU provide more
of it, be it disk space, normal RAM, or SMRAM. In fact I think the SMRAM
size property should not be an X-* property but something that users
could *validly* override on the command line, if they wanted to. Even
exposing it on the libvirt level wouldn't be wrong, I believe; the same
way we already expose whether SMM emulation is enabled at all.

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

Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.

[Andrew Fish]

> On May 4, 2017, at 4:13 AM, Amit kumar  wrote:
> 
> Hi,
> 
> 
> Even after using AVX2 instruction my code shown no performance improvement in 
> UEFI although there is substantial improvement when i run the similar code in 
> windows .
> 
> Am i missing something ?
> 

Is the data aligned the same in both environments?

Thanks,

Andrew Fish

> Using MSVC compiler and the codes written in ASM.
> 
> Thanks And Regards
> 
> Amit
> 
> 
> From: edk2-devel  on behalf of Amit kumar 
> 
> Sent: Wednesday, May 3, 2017 11:18:39 AM
> To: Kinney, Michael D; Andrew Fish
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
> 
> Thank you Michael and Andrew
> 
> 
> Regards
> 
> Amit
> 
> 
> From: Kinney, Michael D 
> Sent: Tuesday, May 2, 2017 10:33:45 PM
> To: Andrew Fish; Amit kumar; Kinney, Michael D
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] Accessing AVX/AVX2 instruction in UEFI.
> 
> Amit,
> 
> The information from Andrew is correct.
> 
> The document that covers this topic is the
> Intel(r) 64 and IA-32 Architectures Software Developer Manuals
> 
> https://software.intel.com/en-us/articles/intel-sdm
> 
> Volume 1, Section 13.5.3 describes the AVX State.  There are
> More details about detecting and enabling different AVX features
> in that document.
> 
> If the CPU supports AVX, then the basic assembly instructions
> required to use AVX instructions are the following that sets
> bits 0, 1, 2 of XCR0.
> 
>mov rcx, 0
>xgetbv
>or  rax, 0007h
>xsetbv
> 
> One additional item you need to be aware of is that UEFI firmware only
> saves/Restores CPU registers required for the UEFI ABI calling convention
> when a timer interrupt or exception is processed.
> 
> This means CPU state such as the YMM registers are not saved/restored
> across an interrupt and may be modified if code in interrupt context
> also uses YMM registers.
> 
> When you enable the use of extended registers, interrupts should be
> saved/disabled and restored around the extended register usage.
> 
> You can use the following functions from MdePkg BaseLib to do this
> 
> /**
>  Disables CPU interrupts and returns the interrupt state prior to the disable
>  operation.
> 
>  @retval TRUE  CPU interrupts were enabled on entry to this call.
>  @retval FALSE CPU interrupts were disabled on entry to this call.
> 
> **/
> BOOLEAN
> EFIAPI
> SaveAndDisableInterrupts (
>  VOID
>  );
> 
> /**
>  Set the current CPU interrupt state.
> 
>  Sets the current CPU interrupt state to the state specified by
>  InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
>  InterruptState is FALSE, then interrupts are disabled. InterruptState is
>  returned.
> 
>  @param  InterruptState  TRUE if interrupts should enabled. FALSE if
>  interrupts should be disabled.
> 
>  @return InterruptState
> 
> **/
> BOOLEAN
> EFIAPI
> SetInterruptState (
>  IN  BOOLEAN   InterruptState
>  );
> 
> Algorithm:
> 
> {
>  BOOLEAN  InterruptState;
> 
>  InterruptState = SaveAndDisableInterrupts();
> 
>  // Enable use of AVX/AVX2 instructions
> 
>  // Use AVX/AVX2 instructions
> 
>  SetInterruptState (InterruptState);
> }
> 
> Best regards,
> 
> Mike
> 
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
>> Andrew Fish
>> Sent: Tuesday, May 2, 2017 8:12 AM
>> To: Amit kumar 
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>> 
>> 
>>> On May 2, 2017, at 6:57 AM, Amit kumar  wrote:
>>> 
>>> Hi,
>>> 
>>> Am trying to optimize an application using AVX/AVX2, but my code hangs 
>>> while trying
>> to access YMM registers.
>>> The instruction where my code hangs is :
>>> 
>>> 
>>> vmovups ymm0, YMMWORD PTR [rax]
>>> 
>>> 
>>> I have verified the cpuid in OS and it supports AVX and AVX2 instruction. 
>>> Processor
>> i7 6th gen.
>>> Can somebody help me out here ? Is there a way to enable YMM registers ?
>>> 
>> 
>> Amit,
>> 
>> I think these instructions will generate an illegal instruction fault until 
>> you enable
>> AVX. You need to check the Cpu ID bits in your code, then write BIT18 of 
>> CR4. After
>> that XGETBV/XSETBV instructions are enabled and you can or in the lower 2 
>> bits of
>> XCR0. This basic operation is in the Intel Docs, it is just hard to find. 
>> Usually the
>> OS has done this for the programmer and all the code needs to do is check 
>> the CPU ID.
>> 
>> Thanks,
>> 
>> Andrew Fish
>> 
>>> 
>>> Thanks And Regards
>>> Amit Kumar
>>> 
>>> ___
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> 
>> 

Re: [edk2] ArmPlatformPkg: LcdGraphicsOutputDxe, PL111, and HdLcd rejig

[Leif Lindholm]

Hi Girish,

On Wed, May 03, 2017 at 04:58:33PM +, Girish Pathak wrote:
> With a view to an upcoming change, we have been examining the current Graphics
> Output Protocol implementation. Currently for, ARM platforms, the UEFI 
> Graphics
> Output Protocol is implemented using a platform specific Library
> (PL111LcdArmVExpressLib/HdLcdArmVExpressLib) and a DXE driver
> (PL111LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe). The platform specific
> library handles platform specific variations such as platform supported 
> display
> modes, memory management of the frame buffer, and clock/mux setting. The DXE
> driver implements the GOP protocol and manages the respective display IP.
> Although this implementation works fine for current platforms, we think the 
> way
> the current DXE driver sources are linked is sub-optimal and can be improved 
> to
> meet recommendation of the EDKII Module Writer's Guide.

The real improvement is the reduction in code duplication.

> The DXE driver source contains three source files per driver. The files
> LcdGraphicsOutputDxe.c and LcdGraphicsOutputBlt.c implement common
> functionality whereas HdLcd.c/PL111Lcd.c  implement display IP specific part 
> of
> the DXE. The problem is, there are two .inf files for HdLcdGraphicsOutputDxe
> and PL111LcdGraphicsOutputDxe and both link common code LcdGraphicsOutputDxe.c
> and LcdGraphicsOutputBlt.c with display IP source instead of a library 
> instance
> , which seems incorrect and can be improved. We propose to separate HdLcd.c
> and PL111Lcd.c and create independent libraries managing only respective
> display IP which can then be instantiated as LcdHwLib and linked in a common
> LcdGraphicsOutputDxe DXE driver. This will help to clearly partition
> implementation of the Graphics Output Protocol into three separate components,
> a platform specific component for the display IP, a display IP specific
> component and GOP common code.

This all sounds imminently sensible. One request for clarifiation below.

> So instead of the current structure in ArmPlatformPkg for display DXE:
> 
>   ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/HdLcdGraphicsOutputDxe.inf
> (HDLCD GOP DXE)
>   ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/PL111LcdGraphicsOutputDxe.inf   
>  (PL111 GOP DXE)
> 
> We propose a structure like:
> 
>   ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf
> (GOP DXE independent of hardware/platform)

Are we expecting this one to be truly independent, or simply common
between most/all ARM Ltd. display controllers?

>   ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf(Library code managing HDLCD HW 
> IP)
>   ArmPlatformPkg/Drivers/PL111/PL11Lcd.inf(Library code managing PL111 HW 
> IP)
> 
> LcdGraphicsOutputDxe.inf will link to LcdPlatformLib and LcdHwLib which can be
> selected for the platform in the platform specific .dsc file.
> 
> e.g.
> 
> Under LibraryClasses we might have:
> 
> LcdPlatformLib|ArmPlatformPkg/ArmVExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExpressLib.inf
> LcdHwLib|ArmPlatformPkg/Drivers/HdLcd/HdLcd.inf
> 
> And Under Components:
> ArmPlatformPkg/Drivers/LcdGraphicsOutputDxe/LcdGraphicsOutputDxe.inf  
> (Common GOP DXE code for ARM Platforms)
> 
> This is a significant change and we would like to invite viewpoints before we
> proceed with implementing these. Since the change would only be with respect 
> to
> display aspects of ArmPlatformPkg we don't foresee any impact on any other
> functionality.
> 
> Please reply if you feel this intended change might impact you and why ?
> Unless objections are raised, we will soon submit the patches for review.

Sounds good to me.

Regards,

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

Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.

[Amit kumar]

Hi,


Even after using AVX2 instruction my code shown no performance improvement in 
UEFI although there is substantial improvement when i run the similar code in 
windows .

Am i missing something ?

Using MSVC compiler and the codes written in ASM.

Thanks And Regards

Amit


From: edk2-devel  on behalf of Amit kumar 

Sent: Wednesday, May 3, 2017 11:18:39 AM
To: Kinney, Michael D; Andrew Fish
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.

Thank you Michael and Andrew


Regards

Amit


From: Kinney, Michael D 
Sent: Tuesday, May 2, 2017 10:33:45 PM
To: Andrew Fish; Amit kumar; Kinney, Michael D
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] Accessing AVX/AVX2 instruction in UEFI.

Amit,

The information from Andrew is correct.

The document that covers this topic is the
Intel(r) 64 and IA-32 Architectures Software Developer Manuals

https://software.intel.com/en-us/articles/intel-sdm

Volume 1, Section 13.5.3 describes the AVX State.  There are
More details about detecting and enabling different AVX features
in that document.

If the CPU supports AVX, then the basic assembly instructions
required to use AVX instructions are the following that sets
bits 0, 1, 2 of XCR0.

mov rcx, 0
xgetbv
or  rax, 0007h
xsetbv

One additional item you need to be aware of is that UEFI firmware only
saves/Restores CPU registers required for the UEFI ABI calling convention
when a timer interrupt or exception is processed.

This means CPU state such as the YMM registers are not saved/restored
across an interrupt and may be modified if code in interrupt context
also uses YMM registers.

When you enable the use of extended registers, interrupts should be
saved/disabled and restored around the extended register usage.

You can use the following functions from MdePkg BaseLib to do this

/**
  Disables CPU interrupts and returns the interrupt state prior to the disable
  operation.

  @retval TRUE  CPU interrupts were enabled on entry to this call.
  @retval FALSE CPU interrupts were disabled on entry to this call.

**/
BOOLEAN
EFIAPI
SaveAndDisableInterrupts (
  VOID
  );

/**
  Set the current CPU interrupt state.

  Sets the current CPU interrupt state to the state specified by
  InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
  InterruptState is FALSE, then interrupts are disabled. InterruptState is
  returned.

  @param  InterruptState  TRUE if interrupts should enabled. FALSE if
  interrupts should be disabled.

  @return InterruptState

**/
BOOLEAN
EFIAPI
SetInterruptState (
  IN  BOOLEAN   InterruptState
  );

Algorithm:

{
  BOOLEAN  InterruptState;

  InterruptState = SaveAndDisableInterrupts();

  // Enable use of AVX/AVX2 instructions

  // Use AVX/AVX2 instructions

  SetInterruptState (InterruptState);
}

Best regards,

Mike

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Andrew 
> Fish
> Sent: Tuesday, May 2, 2017 8:12 AM
> To: Amit kumar 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Accessing AVX/AVX2 instruction in UEFI.
>
>
> > On May 2, 2017, at 6:57 AM, Amit kumar  wrote:
> >
> > Hi,
> >
> > Am trying to optimize an application using AVX/AVX2, but my code hangs 
> > while trying
> to access YMM registers.
> > The instruction where my code hangs is :
> >
> >
> >  vmovups ymm0, YMMWORD PTR [rax]
> >
> >
> > I have verified the cpuid in OS and it supports AVX and AVX2 instruction. 
> > Processor
> i7 6th gen.
> > Can somebody help me out here ? Is there a way to enable YMM registers ?
> >
>
> Amit,
>
> I think these instructions will generate an illegal instruction fault until 
> you enable
> AVX. You need to check the Cpu ID bits in your code, then write BIT18 of CR4. 
> After
> that XGETBV/XSETBV instructions are enabled and you can or in the lower 2 
> bits of
> XCR0. This basic operation is in the Intel Docs, it is just hard to find. 
> Usually the
> OS has done this for the programmer and all the code needs to do is check the 
> CPU ID.
>
> Thanks,
>
> Andrew Fish
>
> >
> > Thanks And Regards
> > Amit Kumar
> >
> > ___
> > 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] EmbeddedPkg/AndroidFastbootApp: support sparse image

[Leif Lindholm]

On Fri, Apr 14, 2017 at 05:55:32PM +0800, Haojian Zhuang wrote:
> Support sparse image that is reformatted from large raw image and
> splitted into a few smaller images. The sparse image follows the
> rule of Android Sparse Image format.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Haojian Zhuang 
> ---
>  .../AndroidFastboot/AndroidFastbootApp.c   | 127 
> +++--
>  .../Include/Protocol/AndroidFastbootPlatform.h |  23 
>  2 files changed, 142 insertions(+), 8 deletions(-)
> 
> diff --git a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c 
> b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> index 61abc67..4883a77 100644
> --- a/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> +++ b/EmbeddedPkg/Application/AndroidFastboot/AndroidFastbootApp.c
> @@ -25,7 +25,34 @@
>  #include 
>  #include 
>  
> -#define ANDROID_FASTBOOT_VERSION "0.4"
> +#define ANDROID_FASTBOOT_VERSION "0.5"

Could this #define, as well as the ones below, and the typedefs, and
the macros, all be moved into AndroidFastbootApp.h?

Actually, I'm confused. In current upstream EDK2, this #define _is_ in
AndroidFastbootApp.h. What is this patch against?

> +
> +#define SPARSE_HEADER_MAGIC 0xED26FF3A
> +#define CHUNK_TYPE_RAW  0xCAC1
> +#define CHUNK_TYPE_FILL 0xCAC2
> +#define CHUNK_TYPE_DONT_CARE0xCAC3
> +#define CHUNK_TYPE_CRC320xCAC4
> +
> +#define FILL_BUF_SIZE   1024

Is this meeting some specification limit, or an arbitrary value?

> +
> +typedef struct _SPARSE_HEADER {
> +  UINT32   Magic;
> +  UINT16   MajorVersion;
> +  UINT16   MinorVersion;
> +  UINT16   FileHeaderSize;
> +  UINT16   ChunkHeaderSize;
> +  UINT32   BlockSize;
> +  UINT32   TotalBlocks;
> +  UINT32   TotalChunks;
> +  UINT32   ImageChecksum;
> +} SPARSE_HEADER;
> +
> +typedef struct _CHUNK_HEADER {
> +  UINT16   ChunkType;
> +  UINT16   Reserved1;
> +  UINT32   ChunkSize;
> +  UINT32   TotalSize;
> +} CHUNK_HEADER;
>  
>  /*
>   * UEFI Application using the FASTBOOT_TRANSPORT_PROTOCOL and
> @@ -139,13 +166,83 @@ HandleDownload (
>  }
>  
>  STATIC
> +EFI_STATUS
> +FlashSparseImage (
> +  IN CHAR8 *PartitionName,
> +  IN SPARSE_HEADER *SparseHeader
> +  )
> +{
> +  EFI_STATUSStatus;
> +  UINTN Chunk, Offset = 0, Index;
> +  VOID *Image;
> +  CHUNK_HEADER *ChunkHeader;
> +  UINT32FillBuf[FILL_BUF_SIZE];
> +  CHAR16OutputString[FASTBOOT_STRING_MAX_LENGTH];
> +
> +  Image = (VOID *)SparseHeader;
> +  Image += SparseHeader->FileHeaderSize;
> +  for (Chunk = 0; Chunk < SparseHeader->TotalChunks; Chunk++) {
> +ChunkHeader = (CHUNK_HEADER *)Image;
> +DEBUG ((DEBUG_INFO, "Chunk #%d - Type: 0x%x Size: %d TotalSize: %d 
> Offset %d\n",
> +(Chunk+1), ChunkHeader->ChunkType, ChunkHeader->ChunkSize,
> +ChunkHeader->TotalSize, Offset));
> +Image += sizeof (CHUNK_HEADER);
> +switch (ChunkHeader->ChunkType) {
> +case CHUNK_TYPE_RAW:
> +  Status = mPlatform->FlashPartitionEx (
> +PartitionName,
> +Offset,
> +ChunkHeader->ChunkSize * SparseHeader->BlockSize,
> +Image
> +);
> +  if (EFI_ERROR (Status)) {
> +return Status;
> +  }
> +  Image += ChunkHeader->ChunkSize * SparseHeader->BlockSize;
> +  Offset += ChunkHeader->ChunkSize * SparseHeader->BlockSize;
> +  break;
> +case CHUNK_TYPE_FILL:
> +  SetMem32 (FillBuf, FILL_BUF_SIZE * sizeof (UINT32), *(UINT32 *)Image);

It's in scope, "sizeof (FillBuf)" would be more clear (and less error prone).

> +  Image += sizeof (UINT32);
> +  for (Index = 0; Index < ChunkHeader->ChunkSize; Index++) {
> +Status = mPlatform->FlashPartitionEx (
> +  PartitionName,
> +  Offset,
> +  SparseHeader->BlockSize,
> +  FillBuf
> +  );
> +if (EFI_ERROR (Status)) {
> +  return Status;
> +}
> +Offset += SparseHeader->BlockSize;
> +  }
> +  break;
> +case CHUNK_TYPE_DONT_CARE:
> +  Offset += ChunkHeader->ChunkSize * SparseHeader->BlockSize;
> +  break;
> +default:
> +  UnicodeSPrint (
> +OutputString,
> +sizeof (OutputString),
> +L"Unsupported Chunk Type:0x%x\n",
> +ChunkHeader->ChunkType
> +);
> +  mTextOut->OutputString (mTextOut, OutputString);
> +  break;
> +}
> +  }
> +  return Status;
> +}
> +
> +STATIC
>  VOID
>  HandleFlash (
>IN CHAR8 *PartitionName
>)
>  {
> -  EFI_STATUS  Status;
> -  CHAR16  OutputString[FASTBOOT_STRING_MAX_LENGTH];
> +  EFI_STATUS

[edk2] [patch] NetworkPkg: Fix issue in dns driver when building DHCP packet.

[Zhang Lubo]

Currently, DNS driver configure the dhcp message type to inform
when building dhcp packet to get dns info from, but it not works
with dhcp server deployed on linux system. However it works well
when changed to request type.

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

diff --git a/NetworkPkg/DnsDxe/DnsDhcp.c b/NetworkPkg/DnsDxe/DnsDhcp.c
index c4add70..93779be 100644
--- a/NetworkPkg/DnsDxe/DnsDhcp.c
+++ b/NetworkPkg/DnsDxe/DnsDhcp.c
@@ -1,9 +1,9 @@
 /** @file
 Functions implementation related with DHCPv4/v6 for DNS driver.
 
-Copyright (c) 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2015 - 2017, Intel Corporation. All rights reserved.
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -460,11 +460,11 @@ GetDns4ServerFromDhcp4 (
 goto ON_EXIT;
   }
   
   ParaList[0]->OpCode  = DHCP4_TAG_TYPE;
   ParaList[0]->Length  = 1;
-  ParaList[0]->Data[0] = DHCP4_MSG_INFORM;
+  ParaList[0]->Data[0] = DHCP4_MSG_REQUEST;
   
   ParaList[1] = AllocateZeroPool (sizeof (EFI_DHCP4_PACKET_OPTION));
   if (ParaList[1] == NULL) {
 Status = EFI_OUT_OF_RESOURCES;
 goto ON_EXIT;
-- 
1.9.5.msysgit.1

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

Re: [edk2] [PATCH] CorebootModulePkg, EmulatorPkg, Vlv2TbltDevicePkg: .inf whitespace fixes

[Leif Lindholm]

Thanks, David - will do.

Still no response from either of the listed CorebootModulePkg
maintainers. Whoops, looks like I left Maurice out of the cc list.
Maurice, Prince, any comments?

Regards,

Leif

On Tue, May 02, 2017 at 09:37:25AM +, Wei, David wrote:
> Only one comment: Please also update the year of  copyright to 2017, such as 
> changing "Copyright (c) 2006  - 2014" to  "Copyright (c) 2006  - 2017". 
> 
> Reviewed-by: zwei4 
> 
> Thanks,
> David  Wei 
> 
> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
> Sent: Saturday, April 29, 2017 12:39 AM
> To: edk2-devel@lists.01.org
> Cc: Andrew Fish ; Agyeman, Prince 
> ; Wei, David 
> Subject: Re: [edk2] [PATCH] CorebootModulePkg, EmulatorPkg, 
> Vlv2TbltDevicePkg: .inf whitespace fixes
> 
> Andrew, Prince/David - any comments?
> 
> On Mon, Apr 24, 2017 at 10:49:48AM -0700, Jordan Justen wrote:
> > On 2017-04-24 02:43:08, Leif Lindholm wrote:
> > > Incorrect line endings, trailing spaces and missing line break at end
> > > of file.
> > > 
> > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > Signed-off-by: Leif Lindholm 
> > > ---
> > > 
> > > This addresses only issues found when hacking on some scripts for
> > > sanity checking .inf files.
> > > 
> > > Since trivial[1], would prefer not to split into one patch per
> > > package.
> > > 
> > > [1] git diff -w  --word-diff-regex=[^[:space:]] HEAD~1
> > > does not generate any output.
> > 
> > Also: git diff --ignore-space-at-eol HEAD~
> > 
> > Reviewed-by: Jordan Justen 
> > 
> > > 
> > >  
> > > CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> > >  |  96 
> > >  
> > > CorebootModulePkg/Library/CbPlatformSupportLibNull/CbPlatformSupportLibNull.inf
> > >  |  70 +-
> > >  CorebootModulePkg/SataControllerDxe/SataControllerDxe.inf
> > >|  98 
> > >  
> > > EmulatorPkg/Library/DxeEmuPeCoffExtraActionLib/DxeEmuPeCoffExtraActionLib.inf
> > >|  96 
> > >  
> > > EmulatorPkg/Library/PeiEmuPeCoffExtraActionLib/PeiEmuPeCoffExtraActionLib.inf
> > >|  98 
> > >  EmulatorPkg/Library/SmbiosLib/SmbiosLib.inf  
> > >|  94 
> > >  Vlv2TbltDevicePkg/PlatformInitPei/PlatformInitPei.inf
> > >|  18 ++---
> > >  Vlv2TbltDevicePkg/SmBiosMiscDxe/SmBiosMiscDxe.inf
> > >| 290 
> > > 
> > >  8 files changed, 430 insertions(+), 430 deletions(-)
> > > 
> > > diff --git 
> > > a/CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> > >  
> > > b/CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> > > index cd758ae4bf..77075ccc95 100644
> > > --- 
> > > a/CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> > > +++ 
> > > b/CorebootModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> > > @@ -1,48 +1,48 @@
> > > -## @file
> > > -#  SerialPortLib instance for 16550 UART.
> > > -#
> > > -#  Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> > > -#  This program and the accompanying materials
> > > -#  are licensed and made available under the terms and conditions of the 
> > > BSD License
> > > -#  which accompanies this distribution.  The full text of the license 
> > > may be found at
> > > -#  http://opensource.org/licenses/bsd-license.php
> > > -#
> > > -#  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> > > -#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> > > IMPLIED.
> > > -#
> > > -##
> > > -
> > > -[Defines]
> > > -  INF_VERSION= 0x00010005
> > > -  BASE_NAME  = BaseSerialPortLib16550
> > > -  MODULE_UNI_FILE= BaseSerialPortLib16550.uni
> > > -  FILE_GUID  = 9E7C00CF-355A-4d4e-BF60-0428CFF95540
> > > -  MODULE_TYPE= BASE
> > > -  VERSION_STRING = 1.1
> > > -  LIBRARY_CLASS  = SerialPortLib
> > > -
> > > -[Packages]
> > > -  MdePkg/MdePkg.dec
> > > -  MdeModulePkg/MdeModulePkg.dec
> > > -
> > > -[LibraryClasses]
> > > -  PcdLib
> > > -  IoLib
> > > -  PlatformHookLib
> > > -  PciLib
> > > -
> > > -[Sources]
> > > -  BaseSerialPortLib16550.c
> > > -
> > > -[Pcd]
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio ## 
> > > CONSUMES
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl  ## 
> > > CONSUMES
> > > -  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialDetectCable ## 
> > > SOMETIMES_CONSUMES
> > > -  

Re: [edk2] [Patch] NetworkPkg: Addressing TCP Window Retraction when window scale factor is used.

[Ye, Ting]

Hi Siyuan,

Maybe we could add some notes to the check-in log to record the test cases 
covered for validating this patch? And please fix RFC3723 in the log also.

Best Regards,
Ting

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, May 03, 2017 3:40 PM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Andrey Tepin ; Ye, 
Ting 
Subject: [Patch] NetworkPkg: Addressing TCP Window Retraction when window scale 
factor is used.

The RFC1323 which defines the TCP window scale option has been obsoleted by 
RFC7323.
This patch is to follow the RFC3723 to address the TCP window retraction 
problem when a non-zero scale factor is used.

Cc: Wu Jiaxin 
Cc: Andrey Tepin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/TcpDxe/TcpMisc.c   |  3 ++-
 NetworkPkg/TcpDxe/TcpOutput.c | 33 -  
NetworkPkg/TcpDxe/TcpProto.h  |  8 +++-
 3 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/NetworkPkg/TcpDxe/TcpMisc.c b/NetworkPkg/TcpDxe/TcpMisc.c index 
a8592c9..4435036 100644
--- a/NetworkPkg/TcpDxe/TcpMisc.c
+++ b/NetworkPkg/TcpDxe/TcpMisc.c
@@ -2,7 +2,7 @@
   Misc support routines for TCP driver.
 
   (C) Copyright 2014 Hewlett-Packard Development Company, L.P.
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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 @@ -86,6 +86,7 @@ TcpInitTcbLocal (
   // First window size is never scaled
   //
   Tcb->RcvWndScale  = 0;
+  Tcb->RetxmitSeqMax = 0;
 
   Tcb->ProbeTimerOn = FALSE;
 }
diff --git a/NetworkPkg/TcpDxe/TcpOutput.c b/NetworkPkg/TcpDxe/TcpOutput.c 
index a46cb60..f3dacf3 100644
--- a/NetworkPkg/TcpDxe/TcpOutput.c
+++ b/NetworkPkg/TcpDxe/TcpOutput.c
@@ -1,7 +1,7 @@
 /** @file
   TCP output process routines.
 
-  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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 @@ -664,7 +664,27 @@ TcpRetransmit (
   // 2. Must in the current send window
   // 3. Will not change the boundaries of queued segments.
   //
-  if (TCP_SEQ_LT (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+  
+  if ((Tcb->SndWndScale != 0) &&
+  (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax) || TCP_SEQ_GT (Tcb->SndWl2 + 
Tcb->SndWnd + (1 << Tcb->SndWndScale), Seq))) {
+//
+// Handle the Window Retraction if TCP window scale is enabled according 
to RFC7323:
+//   On first retransmission, or if the sequence number is out of
+//   window by less than 2^Rcv.Wind.Shift, then do normal
+//   retransmission(s) without regard to the receiver window as long
+//   as the original segment was in window when it was sent.
+//
+Len = TCP_SUB_SEQ (Tcb->SndNxt, Seq);
+DEBUG (
+  (EFI_D_WARN,
+  "TcpRetransmit: retransmission without regard to the receiver window for 
TCB %p\n",
+  Tcb)
+  );
+
+  } else if (TCP_SEQ_GEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq)) {
+Len = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
+
+  } else {
 DEBUG (
   (EFI_D_WARN,
   "TcpRetransmit: retransmission cancelled because send window too small 
for TCB %p\n", @@ -674,10 +694,9 @@ TcpRetransmit (
 return 0;
   }
 
-  Len   = TCP_SUB_SEQ (Tcb->SndWl2 + Tcb->SndWnd, Seq);
-  Len   = MIN (Len, Tcb->SndMss);
+  Len = MIN (Len, Tcb->SndMss);
 
-  Nbuf  = TcpGetSegmentSndQue (Tcb, Seq, Len);
+  Nbuf = TcpGetSegmentSndQue (Tcb, Seq, Len);
   if (Nbuf == NULL) {
 return -1;
   }
@@ -688,6 +707,10 @@ TcpRetransmit (
 goto OnError;
   }
 
+  if (TCP_SEQ_GT (Seq, Tcb->RetxmitSeqMax)) {
+Tcb->RetxmitSeqMax = Seq;
+  }
+
   //
   // The retransmitted buffer may be on the SndQue,
   // trim TCP head because all the buffers on SndQue diff --git 
a/NetworkPkg/TcpDxe/TcpProto.h b/NetworkPkg/TcpDxe/TcpProto.h index 
ee35134..81397d7 100644
--- a/NetworkPkg/TcpDxe/TcpProto.h
+++ b/NetworkPkg/TcpDxe/TcpProto.h
@@ -1,7 +1,7 @@
 /** @file
   TCP protocol header file.
 
-  Copyright (c) 2009 - 2012, Intel Corporation. All rights reserved.
+  Copyright (c) 2009 - 2017, 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 @@ -316,6 +316,12 @@ struct _TCP_CONTROL_BLOCK {
   TCP_SEQNO LossRecover;  ///< Recover point for retxmit.
 
   //
+  // RFC7323
+  // Addressing Window Retraction for TCP Window Scale Option.
+  //
+  TCP_SEQNO RetxmitSeqMax;   ///< Max Seq number in previous 
retransmission.
+
+  //
   // 

Re: [edk2] [Patch] NetworkPkg: Update package version to 0.97.

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: Fu, Siyuan 
Sent: Wednesday, May 03, 2017 3:54 PM
To: edk2-devel@lists.01.org
Cc: Wu, Jiaxin ; Ye, Ting 
Subject: [Patch] NetworkPkg: Update package version to 0.97.

Cc: Wu Jiaxin 
Cc: Ye Ting 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Fu Siyuan 
---
 NetworkPkg/NetworkPkg.dec | 2 +-
 NetworkPkg/NetworkPkg.dsc | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec index 
28558d3..b502f90 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -20,7 +20,7 @@
   DEC_SPECIFICATION  = 0x00010005
   PACKAGE_NAME   = NetworkPkg
   PACKAGE_GUID   = 947988BE-8D5C-471a-893D-AD181C46BEBB
-  PACKAGE_VERSION= 0.96
+  PACKAGE_VERSION= 0.97
   PACKAGE_UNI_FILE   = NetworkPkg.uni
 
 [Includes]
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index 
b8e071b..56a1a6b 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -2,7 +2,7 @@
 # UEFI 2.4 Network Module Package for All Architectures  #  # (C) Copyright 
2014 Hewlett-Packard Development Company, L.P. -# Copyright (c) 2009 - 
2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2009 - 2017, 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
@@ -17,7 +17,7 @@
 [Defines]
   PLATFORM_NAME  = NetworkPkg
   PLATFORM_GUID  = 3FD34E9B-E90C-44e1-B510-1F632A509F10
-  PLATFORM_VERSION   = 0.96
+  PLATFORM_VERSION   = 0.97
   DSC_SPECIFICATION  = 0x00010005
   OUTPUT_DIRECTORY   = Build/NetworkPkg
   SUPPORTED_ARCHITECTURES= IA32|IPF|X64|EBC|ARM|AARCH64
--
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] CryptoPkg: Update package version to 0.97

[Ye, Ting]

Reviewed-by: Ye Ting  

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Long Qin
Sent: Thursday, May 04, 2017 2:40 PM
To: Ye, Ting 
Cc: edk2-devel@lists.01.org; Long, Qin 
Subject: [edk2] [PATCH] CryptoPkg: Update package version to 0.97

Update package version of CryptoPkg to 0.97.

Cc: Ting Ye 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/CryptoPkg.dec | 2 +-
 CryptoPkg/CryptoPkg.dsc | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec index 
b2fae6142a..afeb723211 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -20,7 +20,7 @@
   PACKAGE_NAME   = CryptoPkg
   PACKAGE_UNI_FILE   = CryptoPkg.uni
   PACKAGE_GUID   = 36470E80-36F2-4ba0-8CC8-937C7D9FF888
-  PACKAGE_VERSION= 0.96
+  PACKAGE_VERSION= 0.97
 
 [Includes]
   Include
diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc index 
468e60b5b1..07ff42c5b7 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  Cryptographic Library Package for UEFI Security Implementation.
 #
-#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2009 - 2017, 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 @@ 
-20,7 +20,7 @@  [Defines]
   PLATFORM_NAME  = CryptoPkg
   PLATFORM_GUID  = E1063286-6C8C-4c25-AEF0-67A9A5B6E6B6
-  PLATFORM_VERSION   = 0.96
+  PLATFORM_VERSION   = 0.97
   DSC_SPECIFICATION  = 0x00010005
   OUTPUT_DIRECTORY   = Build/CryptoPkg
   SUPPORTED_ARCHITECTURES= IA32|X64|IPF|ARM|AARCH64
--
2.12.2.windows.2

___
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] SMRAM sizes on large hosts

[Igor Mammedov]

On Thu, 4 May 2017 00:33:27 +0200
Laszlo Ersek  wrote:

[...] 
> If we invented a read-only, side-effect-free PCI config space register
> that gave me this value plain and simple (similarly to how a new fw_cfg
> file would do), that would be a lot cleaner for me.
Just a thought,
have you considered firmware setting size it needs explicitly?
That way we won't have to bump that value on qemu side when
qemu dictated size becomes too small and won't need compat cruft
around it.

> 
> Thanks!
> Laszlo
> ___
> 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] [RFC] [PATCH V4 0/3] Add IOMMU support.

[Ni, Ruiyu]

Jiewen,
All 3 patches are good to me.
When you post V5 to retire one of the SetxxxAttribute() interfaces from IO_MMU 
protocol,
I suggest we keep the Mapping version, but rename the function name to 
SetAttribute().
As the 2 changes in below:

typedef
EFI_STATUS
(EFIAPI *EDKII_IOMMU_SET_ATTRIBUTE)( // <-- 1. Remove the _MAPPING
  IN EDKII_IOMMU_PROTOCOL  *This,
  IN EFI_HANDLEDeviceHandle,
  IN VOID  *Mapping,
  IN UINT64IoMmuAccess
  );

///
/// IOMMU Protocol structure.
///
struct _EDKII_IOMMU_PROTOCOL {
  UINT64  Revision;
  EDKII_IOMMU_SET_ATTRIBUTE   SetAttribute;
  EDKII_IOMMU_MAP Map;
  EDKII_IOMMU_UNMAP   Unmap;
  EDKII_IOMMU_ALLOCATE_BUFFER AllocateBuffer;
  EDKII_IOMMU_FREE_BUFFER FreeBuffer;
  // <-- 2. Remove the SetMappingAttribute interface.
};

Thanks/Ray

> -Original Message-
> From: Yao, Jiewen
> Sent: Saturday, April 29, 2017 9:48 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Leo Duran ; Brijesh
> Singh ; Ard Biesheuvel 
> Subject: [RFC] [PATCH V4 0/3] Add IOMMU support.
> 
>  V4 ==
> Refine the EDKII_IOMMU_PROTOCOL.
> 
> 1) Add AllocateBuffer/FreeBuffer/Map/Unmap() API.
> They are similar to DmaLib in EmbeddedPkg and similar to the previous
> BmDmaLib (by leo.du...@amd.com).
> 
> These APIs are invoked by PciHostBridge driver to allocate DMA memory.
> 
> The PciHostBridge driver (IOMMU consumer) is simplified:
> It uses IOMMU, if IOMMU protocol is present.
> Else it uses original logic.
> 
> 2) Add SetMappingAttribute() API.
> It is similar to SetAttribute() API in V1.
> 
> This API is invoked by PciBus driver to set DMA access attribute (read/write) 
> for
> device.
> 
> The PciBus driver (IOMMU consumer) is simplified:
> It sets access attribute in Map/Unmap,
> if IOMMU protocol is present.
> 
> 3) Remove SetRemapAddress/GetRemapAddress() API.
> Because PciHostBridge/PciBus can call the APIs defined above, there is no need
> to provide remap capability.
> 
> -- Sample producer drivers:
> 1) The sample VTd driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/IntelVTdDxe
> 
> It is added to show the concept. It is not fully implemented yet.
> It will not be checked in in this patch.
> 
> 2) The sample AMD SEV driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleAmdSevDx
> e
> (code is borrowed from leo.du...@amd.com and brijesh.si...@amd.com)
> 
> This is not a right place to put this driver.
> 
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
> 
> 3) The sample STYX driver (IOMMU producer) is at
> https://github.com/jyao1/edk2/tree/dma_v4/IntelSiliconPkg/SampleStyxDxe
> (code is borrowed from ard.biesheu...@linaro.org)
> 
> This is not a right place to put this driver.
> 
> It is added to show the concept.
> It is not fully implemented. It will not be checked in.
> Please do not use it directly.
> 
> 
>  V3 ==
> 1) Add Remap capability (from Ard Biesheuvel) Add
> EDKII_IOMMU_REMAP_ADDRESS API in IOMMU_PROTOCOL.
> 
> NOTE: The code is not fully validated yet.
> The purpose is to collect feedback to decide the next step.
> 
>  V2 ==
> 1) Enhance Unmap() in PciIo (From Ruiyu Ni) Maintain a local list of MapInfo 
> and
> match it in Unmap.
> 
> 2) CopyMem for ReadOperation in PciIo after SetAttribute (Leo Duran) Fix a bug
> in V1 that copy mem for read happen before SetAttribute, which will break AMD
> SEV solution.
> 
>  V1 ==
> 
> This patch series adds IOMMU protocol and updates the consumer to support
> IOMMU based DMA access in UEFI.
> 
> This patch series can support the BmDmaLib request for AMD SEV.
> submitted by Duran, Leo  and Brijesh Singh
> .
> https://lists.01.org/pipermail/edk2-devel/2017-March/008109.html, and
> https://lists.01.org/pipermail/edk2-devel/2017-March/008820.html.
> We can have an AMD SEV specific IOMMU driver to produce IOMMU protocol,
> and clear SEV in IOMMU->SetAttribute().
> 
> This patch series can also support Intel VTd based DMA protection, requested 
> by
> Jiewen Yao , discussed in
> https://lists.01.org/pipermail/edk2-devel/2017-March/008157.html.
> We can have an Intel VTd specific IOMMU driver to produce IOMMU protocol,
> and update VTd engine to grant or deny access in IOMMU->SetAttribute().
> 
> This patch series does not provide a full Intel VTd driver, which will be 
> provide in
> other patch in the future.
> 
> The purpose of this patch series to review if this IOMMU protocol design can
> meet all DMA access and management requirement.
> 
> Cc: Ruiyu Ni 
> Cc: 

[edk2] [PATCH] CryptoPkg: Update package version to 0.97

[Long Qin]

Update package version of CryptoPkg to 0.97.

Cc: Ting Ye 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long 
---
 CryptoPkg/CryptoPkg.dec | 2 +-
 CryptoPkg/CryptoPkg.dsc | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/CryptoPkg/CryptoPkg.dec b/CryptoPkg/CryptoPkg.dec
index b2fae6142a..afeb723211 100644
--- a/CryptoPkg/CryptoPkg.dec
+++ b/CryptoPkg/CryptoPkg.dec
@@ -20,7 +20,7 @@
   PACKAGE_NAME   = CryptoPkg
   PACKAGE_UNI_FILE   = CryptoPkg.uni
   PACKAGE_GUID   = 36470E80-36F2-4ba0-8CC8-937C7D9FF888
-  PACKAGE_VERSION= 0.96
+  PACKAGE_VERSION= 0.97
 
 [Includes]
   Include
diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index 468e60b5b1..07ff42c5b7 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  Cryptographic Library Package for UEFI Security Implementation.
 #
-#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+#  Copyright (c) 2009 - 2017, 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
@@ -20,7 +20,7 @@
 [Defines]
   PLATFORM_NAME  = CryptoPkg
   PLATFORM_GUID  = E1063286-6C8C-4c25-AEF0-67A9A5B6E6B6
-  PLATFORM_VERSION   = 0.96
+  PLATFORM_VERSION   = 0.97
   DSC_SPECIFICATION  = 0x00010005
   OUTPUT_DIRECTORY   = Build/CryptoPkg
   SUPPORTED_ARCHITECTURES= IA32|X64|IPF|ARM|AARCH64
-- 
2.12.2.windows.2

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

Re: [edk2] SMRAM sizes on large hosts

[Gerd Hoffmann]

  Hi,

> > The problem with this is that I need the TSEG size in another module as
> > well, namely PlatformPei. The dispatch order between PlatformPei and
> > SmmAccessPei is unspecified (they have both TRUE for DEPEX). If
> > PlatformPei gets dispatched second, I really wouldn't want to write to
> > MCH_ESMRAMC again, just to query MCH_TSEGMB. (I couldn't just read
> > MCH_TSEGMB because if PlatformPei were dispatched first, then MCH_TSEGMB
> > would still be unset).
> > 
> > In other words, I wouldn't like to write a register just to receive the
> > information; I need the information in two places whose relative
> > ordering is unspecified, and one of those already writes the register in
> > question, genuinely. I guess I could hack it around with another dynamic
> > PCD, but that's kind of ugly.

Hmm, this unspecified ordering works nicely today because tseg size is a
compile time constant.  If we make this dynamic we either need a pcd or
must probe tseg size twice (no matter whenever this is some pci cfg
space register or fw_cfg file), which is kind of ugly too ...

> > If we invented a read-only, side-effect-free PCI config space register
> > that gave me this value plain and simple (similarly to how a new fw_cfg
> > file would do), that would be a lot cleaner for me.

That makes the "probe twice" thing easier indeed.

> > I think this would
> > match your earlier alternative where you wrote "Alternatively we could
> > add some qemu-specific register".

Yes.

cheers,
  Gerd


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