Date   

Re: [Patch V3] NetworkPkg: Making the HTTP IO timeout value programmable with PCD

Maciej Rabeda
 

Change merged: https://github.com/tianocore/edk2/pull/1840

Corrections applied:
1. NetworkPkg.dsc: [PdcsDynamic] -> [PdcsDynamicDefault]
2. NetworkPkg.dsc: PcdHttpTimeout -> PcdHttpIoTimeout

On 28-Jul-21 15:59, Maciej Rabeda wrote:
Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 28-Jul-21 14:16, Heng Luo wrote:
From: Zachary Clark-Williams <zachary.clark-williams@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3507

HTTP boot has a default set forced timeout value of 5 seconds
for getting the recovery image from a remote source.
This change allows the HTTP boot flow to get the IO timeout value
from the PcdHttpIoTimeout.
PcdHttpIoTimeout value is set in platform code.

Signed-off-by: Zachary Clark-Williams <zachary.clark-williams@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
---
  NetworkPkg/HttpBootDxe/HttpBootClient.c | 12 +++++++++---
  NetworkPkg/HttpBootDxe/HttpBootClient.h |  7 +------
  NetworkPkg/HttpBootDxe/HttpBootDxe.inf  |  3 ++-
  NetworkPkg/HttpDxe/HttpDxe.inf          |  3 ++-
  NetworkPkg/HttpDxe/HttpImpl.c           | 17 ++++++++++++++---
  NetworkPkg/HttpDxe/HttpProto.h          |  3 +--
  NetworkPkg/NetworkPkg.dec               |  8 ++++----
  NetworkPkg/NetworkPkg.dsc               |  3 +++
  NetworkPkg/NetworkPkg.uni               |  8 +++++++-
  9 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 8f21f7766e..aa0a153019 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -1,7 +1,7 @@
  /** @file
    Implementation of the boot file download function.
  -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
  SPDX-License-Identifier: BSD-2-Clause-Patent
  @@ -596,19 +596,25 @@ HttpBootCreateHttpIo (
    HTTP_IO_CONFIG_DATA          ConfigData;
    EFI_STATUS                   Status;
    EFI_HANDLE                   ImageHandle;
+  UINT32                       TimeoutValue;
      ASSERT (Private != NULL);
  +  //
+  // Get HTTP timeout value
+  //
+  TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
    ZeroMem (&ConfigData, sizeof (HTTP_IO_CONFIG_DATA));
    if (!Private->UsingIpv6) {
      ConfigData.Config4.HttpVersion    = HttpVersion11;
-    ConfigData.Config4.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;
+    ConfigData.Config4.RequestTimeOut = TimeoutValue;
      IP4_COPY_ADDRESS (&ConfigData.Config4.LocalIp, &Private->StationIp.v4);
      IP4_COPY_ADDRESS (&ConfigData.Config4.SubnetMask, &Private->SubnetMask.v4);
      ImageHandle = Private->Ip4Nic->ImageHandle;
    } else {
      ConfigData.Config6.HttpVersion    = HttpVersion11;
-    ConfigData.Config6.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;
+    ConfigData.Config6.RequestTimeOut = TimeoutValue;
      IP6_COPY_ADDRESS (&ConfigData.Config6.LocalIp, &Private->StationIp.v6);
      ImageHandle = Private->Ip6Nic->ImageHandle;
    }
diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h b/NetworkPkg/HttpBootDxe/HttpBootClient.h
index 971b2dc800..3a98f0f557 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
@@ -1,7 +1,7 @@
  /** @file
    Declaration of the boot file download function.
  -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
  SPDX-License-Identifier: BSD-2-Clause-Patent
  @@ -10,12 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  #ifndef __EFI_HTTP_BOOT_HTTP_H__
  #define __EFI_HTTP_BOOT_HTTP_H__
  -#define HTTP_BOOT_REQUEST_TIMEOUT            5000      // 5 seconds in uints of millisecond.
-#define HTTP_BOOT_RESPONSE_TIMEOUT           5000      // 5 seconds in uints of millisecond.
  #define HTTP_BOOT_BLOCK_SIZE                 1500
-
-
-
  #define HTTP_USER_AGENT_EFI_HTTP_BOOT "UefiHttpBoot/1.0"
    //
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
index a27a561722..cffa642a4b 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
@@ -1,7 +1,7 @@
  ## @file
  #  This modules produce the Load File Protocol for UEFI HTTP boot.
  #
-#  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
  #  (C) Copyright 2020 Hewlett-Packard Development Company, L.P.<BR>
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -96,6 +96,7 @@
    [Pcd]
    gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
+  gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout              ## CONSUMES
    [UserExtensions.TianoCore."ExtraFiles"]
    HttpBootDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 35fe31af18..8dd3838793 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,7 +1,7 @@
  ## @file
  #  Implementation of EFI HTTP protocol interfaces.
  #
-#  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
  #
  #  SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -73,6 +73,7 @@
    [Pcd]
    gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
+  gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout              ## CONSUMES
    [UserExtensions.TianoCore."ExtraFiles"]
    HttpDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 5a6ecbc9d9..8790e9b4e0 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1,7 +1,7 @@
  /** @file
    Implementation of EFI_HTTP_PROTOCOL protocol interfaces.
  -  Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
    (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
      SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -983,6 +983,7 @@ HttpResponseWorker (
    HTTP_TOKEN_WRAP               *ValueInItem;
    UINTN                         HdrLen;
    NET_FRAGMENT                  Fragment;
+  UINT32                        TimeoutValue;
      if (Wrap == NULL || Wrap->HttpInstance == NULL) {
      return EFI_INVALID_PARAMETER;
@@ -1052,10 +1053,15 @@ HttpResponseWorker (
        }
      }
  +    //
+    // Get HTTP timeout value
+    //
+    TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
      //
      // Start the timer, and wait Timeout seconds to receive the header packet.
      //
-    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, TimeoutValue * TICKS_PER_MS);
      if (EFI_ERROR (Status)) {
        goto Error;
      }
@@ -1329,10 +1335,15 @@ HttpResponseWorker (
        }
      }
  +    //
+    // Get HTTP timeout value
+    //
+    TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
      //
      // Start the timer, and wait Timeout seconds to receive the body packet.
      //
-    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+    Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, TimeoutValue * TICKS_PER_MS);
      if (EFI_ERROR (Status)) {
        goto Error2;
      }
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 00ba26aca4..6b3e49090e 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -1,7 +1,7 @@
  /** @file
    The header files of miscellaneous routines for HttpDxe driver.
  -Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
  SPDX-License-Identifier: BSD-2-Clause-Patent
  @@ -41,7 +41,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
  #define HTTP_BUFFER_SIZE_DEAULT      65535
  #define HTTP_MAX_SYN_BACK_LOG        5
  #define HTTP_CONNECTION_TIMEOUT      60
-#define HTTP_RESPONSE_TIMEOUT        5
  #define HTTP_DATA_RETRIES            12
  #define HTTP_FIN_TIMEOUT             2
  #define HTTP_KEEP_ALIVE_PROBES       6
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index b81f10ef6e..3e1f5c101d 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -97,10 +97,6 @@
    # @Prompt Max size of total HTTP chunk transfer. the default value is 12MB.
gEfiNetworkPkgTokenSpaceGuid.PcdMaxHttpChunkTransfer|0x0C00000|UINT32|0x0000000E
  -  ## The Timeout value of HTTP IO.
-  # @Prompt The Timeout value of HTTP Io. Default value is 5000.
- gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F
-
  [PcdsFixedAtBuild, PcdsPatchableInModule]
    ## Indicates whether HTTP connections (i.e., unsecured) are permitted or not.
    # TRUE  - HTTP connections are allowed. Both the "https://" and "http://" URI schemes are permitted.
@@ -160,5 +156,9 @@
    # 0x00 = PXE Disabled
gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01|UINT8|0x1000000a
  +  ## The Timeout value of HTTP IO.
+  # @Prompt The Timeout value of HTTP Io. Default value is 5000.
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F
+
  [UserExtensions.TianoCore."ExtraFiles"]
    NetworkPkgExtra.uni
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
index 5e6619ad85..04685a844d 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -87,6 +87,9 @@
    gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000000
  +[PcdsDynamic]
+  gEfiNetworkPkgTokenSpaceGuid.PcdHttpTimeout|5000
+
###################################################################################################
  #
  # Components Section - list of the modules and components that will be processed by compilation
diff --git a/NetworkPkg/NetworkPkg.uni b/NetworkPkg/NetworkPkg.uni
index 328d8cb54a..6d0fa67c6f 100644
--- a/NetworkPkg/NetworkPkg.uni
+++ b/NetworkPkg/NetworkPkg.uni
@@ -3,7 +3,7 @@
  //
  // This package provides network modules that conform to UEFI 2.4 specification.
  //
-// Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
  //
  // SPDX-License-Identifier: BSD-2-Clause-Patent
  //
@@ -105,3 +105,9 @@
  #string STR_gEfiNetworkPkgTokenSpaceGuid_PcdTftpBlockSize_HELP #language en-US "This setting can override the default TFTP block size. A value of 0 computes "
"the default from MTU information. A non-zero value will be used as block size "
"in bytes."
+
+#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_PROMPT #language en-US "HTTP Boot Image Request and Response Timeout"
+
+#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_HELP #language en-US "This value is used to configure the request and response timeout when getting "
+ "the recovery image from the remote source during an HTTP recovery boot."
+ "The default value set is 5 seconds."




Re: [PATCH v5 00/11] Measured SEV boot with kernel/initrd/cmdline

Yao, Jiewen
 

For OvmfPkg, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
For ArmVirtPkg, acked-by: Jiewen Yao <Jiewen.yao@intel.com>

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Dov Murik
Sent: Wednesday, July 28, 2021 3:07 AM
To: devel@edk2.groups.io
Cc: Dov Murik <dovmurik@linux.ibm.com>; Tobin Feldman-Fitzthum
<tobin@linux.ibm.com>; Tobin Feldman-Fitzthum <tobin@ibm.com>; Jim
Cadden <jcadden@ibm.com>; James Bottomley <jejb@linux.ibm.com>;
Hubertus Franke <frankeh@us.ibm.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ashish Kalra <ashish.kalra@amd.com>; Brijesh Singh <brijesh.singh@amd.com>;
Erdem Aktas <erdemaktas@google.com>; Yao, Jiewen <jiewen.yao@intel.com>;
Xu, Min M <min.m.xu@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Leif Lindholm <leif@nuviainc.com>; Sami
Mujawar <sami.mujawar@arm.com>
Subject: [edk2-devel] [PATCH v5 00/11] Measured SEV boot with
kernel/initrd/cmdline

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3457

Booting with SEV prevented the loading of kernel, initrd, and kernel
command-line via QEMU fw_cfg interface because they arrive from the VMM
which is untrusted in SEV.

However, in some cases the kernel, initrd, and cmdline are not secret
but should not be modified by the host. In such a case, we want to
verify inside the trusted VM that the kernel, initrd, and cmdline are
indeed the ones expected by the Guest Owner, and only if that is the
case go on and boot them up (removing the need for grub inside OVMF in
that mode).

This patch series reserves an area in MEMFD (previously the last 1KB of
the launch secret page) which will contain the hashes of these three
blobs (kernel, initrd, cmdline), each under its own GUID entry. This
tables of hashes is populated by QEMU before launch, and encrypted as
part of the initial VM memory; this makes sure these hashes are part of
the SEV measurement (which has to be approved by the Guest Owner for
secret injection, for example). Note that populating the hashes table
requires QEMU support [1].

OVMF parses the table of hashes populated by QEMU (patch 10), and as it
reads the fw_cfg blobs from QEMU, it will verify each one against the
expected hash. This is all done inside the trusted VM context. If all
the hashes are correct, boot of the kernel is allowed to continue.

Any attempt by QEMU to modify the kernel, initrd, cmdline (including
dropping one of them), or to modify the OVMF code that verifies those
hashes, will cause the initial SEV measurement to change and therefore
will be detectable by the Guest Owner during launch before secret
injection.

Relevant part of OVMF serial log during boot with AmdSevX86 build and
QEMU with -kernel/-initrd/-append:

...
BlobVerifierLibSevHashesConstructor: Found injected hashes table in secure
location
Select Item: 0x17
Select Item: 0x8
FetchBlob: loading 7379328 bytes for "kernel"
Select Item: 0x18
Select Item: 0x11
VerifyBlob: Found GUID 4DE79437-ABD2-427F-B835-D5B172D2045B in table
VerifyBlob: Hash comparison succeeded for "kernel"
Select Item: 0xB
FetchBlob: loading 12483878 bytes for "initrd"
Select Item: 0x12
VerifyBlob: Found GUID 44BAF731-3A2F-4BD7-9AF1-41E29169781D in table
VerifyBlob: Hash comparison succeeded for "initrd"
Select Item: 0x14
FetchBlob: loading 86 bytes for "cmdline"
Select Item: 0x15
VerifyBlob: Found GUID 97D02DD8-BD20-4C94-AA78-E7714D36AB2A in table
VerifyBlob: Hash comparison succeeded for "cmdline"
...

The patch series is organized as follows:

1: Simple comment fix in adjacent area in the code.
2: Use GenericQemuLoadImageLib to gain one location for fw_cfg blob
fetching.
3: Allow the (previously blocked) usage of -kernel in AmdSevX64.
4-7: Add BlobVerifierLib with null implementation and use it in the correct
location in QemuKernelLoaderFsDxe.
8-9: Reserve memory for hashes table, declare this area in the reset vector.
10-11: Add the secure implementation BlobVerifierLibSevHashes and use it in
AmdSevX64 builds.

[1] https://lore.kernel.org/qemu-devel/20210624102040.2015280-1-
dovmurik@linux.ibm.com/

Code is at
https://github.com/confidential-containers-demo/edk2/tree/sev-hashes-v5

v5 changes:
- rename the null implementation dir to OvmfPkg/Library/BlobVerifierLibNull
(note that I didn't remove the R-b tags on these patches; please let me
know if I should have acted otherwise)
- move the SevHashes implementation to
OvmfPkg/AmdSev/BlobVerifierLibSevHashes
- BlobVerifierLib.h: fix #ifndef according to ECC warnings
- separate variable declaration and assignment in
BlobVerifierLibSevHashesConstructor (ECC warning)

v4: https://edk2.groups.io/g/devel/message/78075
v4 changes:
- BlobVerifierSevHashes (patch 10): more comprehensive overflow tests
when parsing the SEV hashes table structure

v3: https://edk2.groups.io/g/devel/message/77955
v3 changes:
- Rename to BlobVerifierLibNull, use decimal INF_VERSION, remove unused
DebugLib reference, fix doxygen comments, add missing IN attribute
- Rename to BlobVerifierLibSevHashes, use decimal INF_VERSION, fix
doxygen comments, add missing IN attribute,
calculate buffer hash only when the guid is found in hashes table
- SecretPei: use ALIGN_VALUE to round the hob size
- Coding style fixes
- Add missing 'Ref:' in patch 1 commit message
- Fix phrasing and typos in commit messages
- Remove Cc: Laszlo from series

v2: https://edk2.groups.io/g/devel/message/77505
v2 changes:
- Use the last 1KB of the existing SEV launch secret page for hashes table
(instead of reserving a whole new MEMFD page).
- Build on top of commit cf203024745f ("OvmfPkg/GenericQemuLoadImageLib:
Read
cmdline from QemuKernelLoaderFs", 2021-06-28) to have a single location in
which all of kernel/initrd/cmdline are fetched from QEMU.
- Use static linking of the two BlobVerifierLib implemenatations.
- Reorganize series.

v1: https://edk2.groups.io/g/devel/message/75567

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ashish Kalra <ashish.kalra@amd.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Erdem Aktas <erdemaktas@google.com>
Cc: James Bottomley <jejb@linux.ibm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Sami Mujawar <sami.mujawar@arm.com>

Dov Murik (8):
OvmfPkg/AmdSev: use GenericQemuLoadImageLib in AmdSev builds
OvmfPkg: add library class BlobVerifierLib with null implementation
OvmfPkg: add BlobVerifierLibNull to DSC
ArmVirtPkg: add BlobVerifierLibNull to DSC
OvmfPkg/QemuKernelLoaderFsDxe: call VerifyBlob after fetch from fw_cfg
OvmfPkg/AmdSev/SecretPei: build hob for full page
OvmfPkg/AmdSev: add BlobVerifierLibSevHashes
OvmfPkg/AmdSev: Enforce hash verification of kernel blobs

James Bottomley (3):
OvmfPkg/AmdSev/SecretDxe: fix header comment to generic naming
OvmfPkg: PlatformBootManagerLibGrub: Allow executing kernel via fw_cfg
OvmfPkg/AmdSev: reserve MEMFD space for for firmware config hashes

OvmfPkg/OvmfPkg.dec | 9 +
ArmVirtPkg/ArmVirtQemu.dsc | 5 +-
ArmVirtPkg/ArmVirtQemuKernel.dsc | 5 +-
OvmfPkg/AmdSev/AmdSevX64.dsc | 9 +-
OvmfPkg/OvmfPkgIa32.dsc | 5 +-
OvmfPkg/OvmfPkgIa32X64.dsc | 5 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/AmdSev/AmdSevX64.fdf | 5 +-
OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierLibSevHashes.inf
| 37 ++++
OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf | 24
+++

OvmfPkg/Library/PlatformBootManagerLibGrub/PlatformBootManagerLibGrub.
inf | 2 +
OvmfPkg/ResetVector/ResetVector.inf | 2 +
OvmfPkg/Include/Library/BlobVerifierLib.h | 38 ++++
OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.h |
11 ++
OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
| 202 ++++++++++++++++++++
OvmfPkg/AmdSev/SecretDxe/SecretDxe.c | 2 +-
OvmfPkg/AmdSev/SecretPei/SecretPei.c | 3 +-
OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c | 33
++++
OvmfPkg/Library/PlatformBootManagerLibGrub/BdsPlatform.c |
5 +
OvmfPkg/Library/{PlatformBootManagerLib =>
PlatformBootManagerLibGrub}/QemuKernel.c | 0
OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.c
| 9 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 20 ++
OvmfPkg/ResetVector/ResetVector.nasmb | 2 +
23 files changed, 428 insertions(+), 10 deletions(-)
create mode 100644
OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierLibSevHashes.inf
create mode 100644
OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierLibNull.inf
create mode 100644 OvmfPkg/Include/Library/BlobVerifierLib.h
create mode 100644
OvmfPkg/AmdSev/BlobVerifierLibSevHashes/BlobVerifierSevHashes.c
create mode 100644 OvmfPkg/Library/BlobVerifierLibNull/BlobVerifierNull.c
copy OvmfPkg/Library/{PlatformBootManagerLib =>
PlatformBootManagerLibGrub}/QemuKernel.c (100%)

--
2.25.1





Re: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP) support

Yao, Jiewen
 

Sounds good. Thank you to confirm that.

I will send my feedback.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 11:22 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
Cc: brijesh.singh@amd.com; James Bottomley <jejb@linux.ibm.com>; Xu, Min M
<min.m.xu@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Laszlo Ersek <lersek@redhat.com>; Erdem Aktas
<erdemaktas@google.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
<ray.ni@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Kinney, Michael
D <michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Liu,
Zhiguang <zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>
Subject: Re: [edk2-devel] [RFC PATCH v4 00/27] Add AMD Secure Nested Paging
(SEV-SNP) support

Hi Yao Jiewen,

On 7/28/21 3:16 AM, Yao, Jiewen wrote:
Hi Brijesh
I reviewed the patch set. I have some basic questions.
Please help me understand before I post my comment

If a platform supports SEV-SNP, can we assume SEV-ES is supported?
The SEV-SNP depends on SEV and SEV-ES support.

The SEV-ES depends on the SEV support.


Or is it a valid case that SecSnp==YES, SevEs==NO?
Nope.


I am trying to understand how many cases we need support.
I think we want to support below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 0 | 0 |
| 1 | 0 | 0 |
| 1 | 1 | 0 |
| 1 | 1 | 1 |
+------------------------+
Yes, the above looks correct.


Any other combination we need support? Such as below:
The below cases are not applicable.

+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 1 | 0 |
| 0 | 0 | 1 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
+------------------------+


Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Tuesday, June 29, 2021 1:42 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M
<min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Laszlo Ersek
<lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>; Dong, Eric
<eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<rahul1.kumar@intel.com>; Kinney, Michael D
<michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
Singh <brijesh.singh@amd.com>
Subject: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP)
support

BZ:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.
tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&amp;data=04%7C01%7Cbrijesh.
singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe48
84e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%
7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL
CJXVCI6Mn0%3D%7C1000&amp;sdata=BqKBPTm4RQFXsekHTH2ktc2YmZMwazn
9bZy8G8%2BWSTA%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-
SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the
available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is
validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-
&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a
808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6376
30571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJ
QIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7p5Ap
%2FHMiSXgxxMI35SYWcZaUcx5VjNt1wnpV9kbT6c%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&amp;data=04%7C01%7
Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8
961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnk
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h5ZrpTSwjBVhw9Bdh%2FvcZVGK
%2BaxgHre42B8evZuTkKQ%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.c
om%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-
4&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53
a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637
630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=MwXz
gykRRjT0QCp%2B77zJG1nH44478OzH4HtCQJbpHLc%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdevelop
er.amd.com%2Fwp-
content%2Fresources%2F56421.pdf&amp;data=04%7C01%7Cbrijesh.singh%40a
md.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11
a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZ
sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
%3D%7C1000&amp;sdata=jU2LPonK9rQUjKQsRijBNU6uk1eN%2B7uuqYiXKvz7r4
w%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.a
md.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&amp;data=04%7C01%7
Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8
961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnk
nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
aWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6xiPHnAMKyJy6b%2B9trUlukxKYA
pH%2FncYM8Qg0r9%2BWlA%3D&amp;reserved=0

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm} (67%)

--
2.17.1



Re: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

Yao, Jiewen
 

I would say it is NOT the best software practice to define 2 enable fields to indicate one type.

What if some software error, set both TdxEnable and SevEnable at same time?
How do you detect such error?

If some code may check the SEV only, and some code may check TDX only, then both code can run. That will be a disaster. The code is hard to maintain and hard to debug.

Another consideration is: what if someone wants to set the third type?
Do we want to reserve the 3rd byte? To indicate the third type? It is not scalable.

The best software practice it to just define one field as enumeration. So any software can only set Tdx or Sev. The result is consistent, no matter you check the SEV at first or TDX at first.
If there is 3rd bytes, we just need assign the 3rd value to it, without impact any other field.


I think we can add "must zero" in the comment. But it does not mean there will be no error during development.

UNION is not a type safe structure. Usually, the consumer of UNION shall refer to a common field to know what the type of union is - I treat that as header.

Since we are defining a common data structure, I think we can do some clean up.
Or if you have concern to change SEC_SEV_ES_WORK_AREA, we can define a new CC WORK_AREA without touching SEV_SEV_ES_WORKING_AREA.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 11:59 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Xu, Min M
<min.m.xu@intel.com>
Cc: brijesh.singh@amd.com; Ard Biesheuvel <ardb+tianocore@kernel.org>;
Justen, Jordan L <jordan.l.justen@intel.com>; Erdem Aktas
<erdemaktas@google.com>; James Bottomley <jejb@linux.ibm.com>; Tom
Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
ResetVector

Hi Yao Jiewen,

I guess I am still trying to figure out why we need the header in the
work area. Can't we do something like this:

typedef struct {
UINT8 SevEsEnabled;

// If Es is enabled then this field must be zeroed
UINT8 MustBeZero;

UINT8 Reserved1[6];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

typedef struct {
// If Tdx is enabled then it must be zeroed
UINT8 MustBeZero

UINT8 TdxEnabled;

UINT8 Reserved2[6];
....

} TX_WORK_AREA;

typedef union {
SEC_SEV_ES_WORK_AREA SevEsWorkArea;
TDX_WORK_AREA TdxWorkArea;
} CC_WORK_AREA;

I am trying to minimize the changes to the existing code. The SEV and
TDX probe logic should ensure that if the feature is detected, then it
must clear the MustBeZero'ed field.

Basically, we already have a 64-bit value reserved in the SevEsWork area
and currently only one byte is used and second byte can be detected for
the TDX. Basically the which encryption technology is active the
definition of the work area will change.

Am I missing something ?

Thanks

On 7/28/21 10:22 AM, Yao, Jiewen wrote:
Hi Brijesh
Thanks!

I think if we want to reuse this, we need rename the data structure.

First, we should use a generic name.

Second, I don’t think it is good idea to define two *enable* fields. Since only
one of them should be enabled, we should use 1 field as enumeration.

Third, we should hide the SEV specific and TDX specific definition in CC
common work area.

If we agree to use a common work area, I recommend below:

typedef struct {
UINT8 HeaderVersion; // 0
UINT8 HeadLength; // 4
UINT8 Type; // 0 - legacy, 1 - SEV, 2 - TDX
UINT8 SubType; // Type specific sub type, if needed.
} CC_COMMON_WORK_AREA_HEADER;

typedef struct {
CC_COMMON_WORK_AREA_HEADER Header;
// reset is valid if Type == 1
UINT8 Reserved1[4];
UINT64 RandomData;
UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

typedef struct {
CC_COMMON_WORK_AREA_HEADER Header;
// reset is valid if Type == 2
UINT8 TdxSpecific[]; // TBD
} TDX_WORK_AREA;

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 10:34 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Xu, Min M <min.m.xu@intel.com>
Cc: brijesh.singh@amd.com; devel@edk2.groups.io; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
Erdem Aktas <erdemaktas@google.com>; James Bottomley
<jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
ResetVector

Hi Jiewen and Min,

See my comments below.


On 7/28/21 2:54 AM, Yao, Jiewen wrote:
Yes. I am thinking the same thing.

[CC Flag memory location]
1) A general purpose register, such as EBP.

2) A global variable, such as
.data
TeeFlags: DD 0

3) A fixed region in stack, such as
dword[STACK_TOP - 4]

4) A new CC common fixed region, such as
dword[CC_COMMON_FLAGS]

5) A fixed region piggyback on existing CC working area, such as
dword[CC_WORKING_AREA]

Hi Brijesh/Min
Any preference?

[CC Indicator Flags]
Proposal: UINT8[4]

Byte [0] Version: 0
byte [1] Length: 4
byte [2] Type:
0: legacy
1: SEV
2: TDX
byte [3] Sub Type:
If Type is 0 (legacy), then
0: legacy
If Type is 1 (SEV), then
0: SEV
1: SEV-ES
2: SEV-SNP
If Type is 2 (TDX), then
0: TDX 1.0

Thank you
Yao Jiewen


-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com>
Sent: Wednesday, July 28, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L
<jordan.l.justen@intel.com>; Erdem Aktas <erdemaktas@google.com>;
James
Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>
Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

On July 28, 2021 2:05 PM, Yao, Jiewen wrote:
It does not necessary to be a working area.

We just need a common TEE flag to indicate if the system run in legacy,
SEV,
or
TDX, right?
Right. We need somewhere to store this flag, either in a Register or in
Memory.
If it is memory, then in Tdx the memory region should be initialized by host
VMM.

thank you!
Yao, Jiewen


在 2021年7月28日,下午1:07,Xu, Min M <min.m.xu@intel.com>

道:

On July 27, 2021 8:46 PM, Yao, Jiewen wrote:
HI Min
I agree with Brijesh.

The basic rule is: SEV file shall never refer to TDX data structure.
TDX file shall never refer to SEV data structure.
These code should be isolated clearly.

Do we still need that logic if we follow the new pattern?
I have replied to Brijesh's mail about the concern of the new pattern.

I have some concern in the current pattern:
====================
OneTimeCall PreMainFunctionHookSev
OneTimeCall PreMainFunctionHookTdx
MainFunction:
XXXXXX
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
The TEE function need implement a TEE check function (such as IsSev, or
IsTdx).

Tdx call CPUID(0x21) to determine if it is tdx guest in the very
beginning of ResetVector. Then 'TDXG' is set in TDX_WORK_AREA. SEV
does
the similar work which call CheckSevFeatures to set SEV_ES_WORK_AREA
to
1.

After that both TDX and SEV read the above WORK_AREA to check if it is
TDX
or SEV or legacy guest.

In Tdx the access to SEV_ES_WORK_AREA will trigger error because
SEV_ES_WORK_AREA is *NOT* initialized by host VMM.
In SEV-SNP I am afraid the access to TDX_WORK_AREA will trigger error
too.

I am wondering if TDX and SEV can use the same memory region (for
example,
TEE_WORK_AREA) as the work area?
So that this work area is guaranteed to be initialized in both TDX and
SEV. Structure of the TEE_WORK_AREA may look like this:
typedef struct {
UINT8 Flag[4]; 'TDXG' or 'SEVG' or all-0
UINT8 Others[];
} TEE_WORK_AREA;
Are we reserving a new page for the TDX_WORK_AREA ? I am wondering why
can't we use the SEV_ES_WORK_AREA instead of wasting space in the
MEMFD.

The SEV_ES_WORK_AREA layout looks like this:

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 Reserved1[7];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

There is reserved bit after the SevEsEnabled and one byte can be used
for the TdxEnabled;

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 TdxEnabled;
UINT8 Reserved2[6];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

The SEV_ES_WORK_AREA can be treated as a TEE_WORK_AREA and we can
be
pull out from MemEncrypSevLib.h to CcWorkAreaLib.h and rename the
structure (if needed).

Both the SEV-SNP and TEE host-VMM accepts the TEE_WORK_AREA before
booting the guest to ensure that its safe to access the memory without
going through the accept/validation process.

In case of the TDX, the reset vector code sets the TdxEnabled on the
entry. In case of the SEV, the workarea is valid from SEC to PEI phase
only and it gets reused for other purposes. The PEI phase set the Pcd's
(such as SevEsEnabled or SevEnabled etc) so that Dxe or other EDK2 core
does not need to know anything about the workarea and they simply can
read the PCDs.

-Brijesh






Re: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

Brijesh Singh
 

Hi Yao Jiewen,

I guess I am still trying to figure out why we need the header in the work area. Can't we do something like this:

typedef struct {
UINT8 SevEsEnabled;

// If Es is enabled then this field must be zeroed
UINT8 MustBeZero;

UINT8 Reserved1[6];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

typedef struct {
// If Tdx is enabled then it must be zeroed
UINT8 MustBeZero

UINT8 TdxEnabled;

UINT8 Reserved2[6];
....

} TX_WORK_AREA;

typedef union {
SEC_SEV_ES_WORK_AREA SevEsWorkArea;
TDX_WORK_AREA TdxWorkArea;
} CC_WORK_AREA;

I am trying to minimize the changes to the existing code. The SEV and TDX probe logic should ensure that if the feature is detected, then it must clear the MustBeZero'ed field.

Basically, we already have a 64-bit value reserved in the SevEsWork area and currently only one byte is used and second byte can be detected for the TDX. Basically the which encryption technology is active the definition of the work area will change.

Am I missing something ?

Thanks

On 7/28/21 10:22 AM, Yao, Jiewen wrote:
Hi Brijesh
Thanks!
I think if we want to reuse this, we need rename the data structure.
First, we should use a generic name.
Second, I don’t think it is good idea to define two *enable* fields. Since only one of them should be enabled, we should use 1 field as enumeration.
Third, we should hide the SEV specific and TDX specific definition in CC common work area.
If we agree to use a common work area, I recommend below:
typedef struct {
UINT8 HeaderVersion; // 0
UINT8 HeadLength; // 4
UINT8 Type; // 0 - legacy, 1 - SEV, 2 - TDX
UINT8 SubType; // Type specific sub type, if needed.
} CC_COMMON_WORK_AREA_HEADER;
typedef struct {
CC_COMMON_WORK_AREA_HEADER Header;
// reset is valid if Type == 1
UINT8 Reserved1[4];
UINT64 RandomData;
UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;
typedef struct {
CC_COMMON_WORK_AREA_HEADER Header;
// reset is valid if Type == 2
UINT8 TdxSpecific[]; // TBD
} TDX_WORK_AREA;
Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 10:34 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Xu, Min M <min.m.xu@intel.com>
Cc: brijesh.singh@amd.com; devel@edk2.groups.io; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
Erdem Aktas <erdemaktas@google.com>; James Bottomley
<jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
ResetVector

Hi Jiewen and Min,

See my comments below.


On 7/28/21 2:54 AM, Yao, Jiewen wrote:
Yes. I am thinking the same thing.

[CC Flag memory location]
1) A general purpose register, such as EBP.

2) A global variable, such as
.data
TeeFlags: DD 0

3) A fixed region in stack, such as
dword[STACK_TOP - 4]

4) A new CC common fixed region, such as
dword[CC_COMMON_FLAGS]

5) A fixed region piggyback on existing CC working area, such as
dword[CC_WORKING_AREA]

Hi Brijesh/Min
Any preference?

[CC Indicator Flags]
Proposal: UINT8[4]

Byte [0] Version: 0
byte [1] Length: 4
byte [2] Type:
0: legacy
1: SEV
2: TDX
byte [3] Sub Type:
If Type is 0 (legacy), then
0: legacy
If Type is 1 (SEV), then
0: SEV
1: SEV-ES
2: SEV-SNP
If Type is 2 (TDX), then
0: TDX 1.0

Thank you
Yao Jiewen


-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com>
Sent: Wednesday, July 28, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L
<jordan.l.justen@intel.com>; Erdem Aktas <erdemaktas@google.com>;
James
Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>
Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

On July 28, 2021 2:05 PM, Yao, Jiewen wrote:
It does not necessary to be a working area.

We just need a common TEE flag to indicate if the system run in legacy, SEV,
or
TDX, right?
Right. We need somewhere to store this flag, either in a Register or in
Memory.
If it is memory, then in Tdx the memory region should be initialized by host
VMM.

thank you!
Yao, Jiewen


在 2021年7月28日,下午1:07,Xu, Min M <min.m.xu@intel.com> 写
道:

On July 27, 2021 8:46 PM, Yao, Jiewen wrote:
HI Min
I agree with Brijesh.

The basic rule is: SEV file shall never refer to TDX data structure.
TDX file shall never refer to SEV data structure.
These code should be isolated clearly.

Do we still need that logic if we follow the new pattern?
I have replied to Brijesh's mail about the concern of the new pattern.

I have some concern in the current pattern:
====================
OneTimeCall PreMainFunctionHookSev
OneTimeCall PreMainFunctionHookTdx
MainFunction:
XXXXXX
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
The TEE function need implement a TEE check function (such as IsSev, or
IsTdx).

Tdx call CPUID(0x21) to determine if it is tdx guest in the very
beginning of ResetVector. Then 'TDXG' is set in TDX_WORK_AREA. SEV
does
the similar work which call CheckSevFeatures to set SEV_ES_WORK_AREA to
1.

After that both TDX and SEV read the above WORK_AREA to check if it is
TDX
or SEV or legacy guest.

In Tdx the access to SEV_ES_WORK_AREA will trigger error because
SEV_ES_WORK_AREA is *NOT* initialized by host VMM.
In SEV-SNP I am afraid the access to TDX_WORK_AREA will trigger error
too.

I am wondering if TDX and SEV can use the same memory region (for
example,
TEE_WORK_AREA) as the work area?
So that this work area is guaranteed to be initialized in both TDX and
SEV. Structure of the TEE_WORK_AREA may look like this:
typedef struct {
UINT8 Flag[4]; 'TDXG' or 'SEVG' or all-0
UINT8 Others[];
} TEE_WORK_AREA;
Are we reserving a new page for the TDX_WORK_AREA ? I am wondering why
can't we use the SEV_ES_WORK_AREA instead of wasting space in the MEMFD.

The SEV_ES_WORK_AREA layout looks like this:

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 Reserved1[7];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

There is reserved bit after the SevEsEnabled and one byte can be used
for the TdxEnabled;

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 TdxEnabled;
UINT8 Reserved2[6];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

The SEV_ES_WORK_AREA can be treated as a TEE_WORK_AREA and we can be
pull out from MemEncrypSevLib.h to CcWorkAreaLib.h and rename the
structure (if needed).

Both the SEV-SNP and TEE host-VMM accepts the TEE_WORK_AREA before
booting the guest to ensure that its safe to access the memory without
going through the accept/validation process.

In case of the TDX, the reset vector code sets the TdxEnabled on the
entry. In case of the SEV, the workarea is valid from SEC to PEI phase
only and it gets reused for other purposes. The PEI phase set the Pcd's
(such as SevEsEnabled or SevEnabled etc) so that Dxe or other EDK2 core
does not need to know anything about the workarea and they simply can
read the PCDs.

-Brijesh



Re: "StdLibPkg" branch on edk2-staging

Rebecca Cran
 

I think it's mostly been abandoned, and people are expected to use native UEFI functions instead. Personally I'd like to see it continue to be maintained.


--
Rebecca Cran

On 7/28/21 9:45 AM, Maciej Rabeda wrote:
Naturally. Since it is there, why is it not used for brotli, openssl in CryptoPkg or jansson in RedfishPkg?

On 28-Jul-21 17:34, Rebecca Cran wrote:
Are you aware of the edk2-libc project at https://github.com/tianocore/edk2-libc ?





Re: "StdLibPkg" branch on edk2-staging

Maciej Rabeda
 

Naturally. Since it is there, why is it not used for brotli, openssl in CryptoPkg or jansson in RedfishPkg?

On 28-Jul-21 17:34, Rebecca Cran wrote:
Are you aware of the edk2-libc project at https://github.com/tianocore/edk2-libc ?


[edk2-platforms PATCH v1 1/1] Platform/Intel/WhitleyOpenBoardPkg: Fix build error of WilsonCityRvp

 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3511

The PCD PcdDebugLoadImageMethod, which is consumed by
PeCoffExtraActionLibDebug.inf is only for the DEBUG build.

Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Signed-off-by: Takuto Naito <naitaku@gmail.com>
---
Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc | 2 ++
1 file changed, 2 insertions(+)

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
index 41dc55a14d..fa41ae923d 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
+++ b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
@@ -148,7 +148,9 @@

gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2F # Enable asserts, prints, code, clear memory, and deadloops on asserts.
gEfiMdePkgTokenSpaceGuid.PcdFixedDebugPrintErrorLevel|0x80200047 # Built in messages: Error, MTRR, info, load, warn, init
+!if $(TARGET) == "DEBUG"
gEfiSourceLevelDebugPkgTokenSpaceGuid.PcdDebugLoadImageMethod|0x2 # This is set to INT3 (0x2) for Simics source level debugging
+!endif

gEfiMdeModulePkgTokenSpaceGuid.PcdLoadModuleAtFixAddressEnable|0
gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize|0x0
--
2.25.1


[edk2-platforms PATCH v1 0/1] Platform/Intel/WhitleyOpenBoardPkg: Fix build error of WilsonCityRvp

 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3511

https://github.com/naitaku/edk2-platforms/tree/FixWilsonCityRvpRelease_v1

Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>

Takuto Naito (1):
Platform/Intel/WhitleyOpenBoardPkg: Fix build error of WilsonCityRvp

Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc | 2 ++
1 file changed, 2 insertions(+)

--
2.25.1


Re: "StdLibPkg" branch on edk2-staging

Rebecca Cran
 

Are you aware of the edk2-libc project at https://github.com/tianocore/edk2-libc ?


--
Rebecca Cran

On 7/27/21 9:48 AM, Maciej Rabeda wrote:
Hi,

I have submitted a new edk2-staging branch named "StdLibPkg".
https://github.com/tianocore/edk2-staging/tree/StdLibPkg

Branch contains initial implementation of C standard library and intrinsic library for UEFI to smoothen open-source submodule integration (instead of implementing those functions in each new package introducing an external submodule dependent on C stdlib).

More details on the package, its contents and feature scope are placed in that branch, in StdLibPkg/Readme.md file.
https://github.com/tianocore/edk2-staging/blob/StdLibPkg/StdLibPkg/Readme.md

Feel free to play around. Any and all feedback is welcome.

Thanks,
Maciej




Re: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP) support

Brijesh Singh
 

Hi Yao Jiewen,

On 7/28/21 3:16 AM, Yao, Jiewen wrote:
Hi Brijesh
I reviewed the patch set. I have some basic questions.
Please help me understand before I post my comment
If a platform supports SEV-SNP, can we assume SEV-ES is supported?
The SEV-SNP depends on SEV and SEV-ES support.

The SEV-ES depends on the SEV support.


Or is it a valid case that SecSnp==YES, SevEs==NO?
Nope.

I am trying to understand how many cases we need support.
I think we want to support below:
+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 0 | 0 |
| 1 | 0 | 0 |
| 1 | 1 | 0 |
| 1 | 1 | 1 |
+------------------------+
Yes, the above looks correct.

Any other combination we need support? Such as below:
The below cases are not applicable.

+------------------------+
| SEV | SEV_ES | SEV_SNP |
+------------------------+
| 0 | 1 | 0 |
| 0 | 0 | 1 |
| 0 | 1 | 1 |
| 1 | 0 | 1 |
+------------------------+
Thank you
Yao Jiewen

-----Original Message-----
From: Brijesh Singh <brijesh.singh@amd.com>
Sent: Tuesday, June 29, 2021 1:42 AM
To: devel@edk2.groups.io
Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
<thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
Ard Biesheuvel <ardb+tianocore@kernel.org>; Laszlo Ersek
<lersek@redhat.com>; Erdem Aktas <erdemaktas@google.com>; Dong, Eric
<eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1
<rahul1.kumar@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
Liming Gao <gaoliming@byosoft.com.cn>; Liu, Zhiguang
<zhiguang.liu@intel.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
Singh <brijesh.singh@amd.com>
Subject: [RFC PATCH v4 00/27] Add AMD Secure Nested Paging (SEV-SNP)
support

BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=BqKBPTm4RQFXsekHTH2ktc2YmZMwazn9bZy8G8%2BWSTA%3D&amp;reserved=0

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is validated
before it is made available to the EDK2 core.

This series does not implements the following SEV-SNP features yet:

* CPUID filtering
* Lazy validation
* Interrupt security

Additional resources
---------------------
SEV-SNP whitepaper
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2FSEV-SNP-strengthening-vm-&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=7p5Ap%2FHMiSXgxxMI35SYWcZaUcx5VjNt1wnpV9kbT6c%3D&amp;reserved=0
isolation-with-integrity-protection-and-more.pdf

APM 2: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F24593.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=h5ZrpTSwjBVhw9Bdh%2FvcZVGK%2BaxgHre42B8evZuTkKQ%3D&amp;reserved=0 (section 15.36)

The complete source is available at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsev-snp-rfc-4&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=MwXzgykRRjT0QCp%2B77zJG1nH44478OzH4HtCQJbpHLc%3D&amp;reserved=0

GHCB spec:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.amd.com%2Fwp-content%2Fresources%2F56421.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=jU2LPonK9rQUjKQsRijBNU6uk1eN%2B7uuqYiXKvz7r4w%3D&amp;reserved=0

SEV-SNP firmware specification:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.amd.com%2Fsystem%2Ffiles%2FTechDocs%2F56860.pdf&;data=04%7C01%7Cbrijesh.singh%40amd.com%7C6bbdbdbb0ac8400b53a808d951a00e10%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637630571069893367%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=6xiPHnAMKyJy6b%2B9trUlukxKYApH%2FncYM8Qg0r9%2BWlA%3D&amp;reserved=0

Brijesh Singh (26):
OvmfPkg/ResetVector: move SEV specific code in a separate file
OvmfPkg/ResetVector: add the macro to invoke MSR protocol based
VMGEXIT
OvmfPkg/ResetVector: add the macro to request guest termination
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page for SEV-SNP
OvmfPkg/ResetVector: introduce SEV-SNP boot block GUID
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/ResetVector: invalidate the GHCB page
UefiCpuPkg: Define the SEV-SNP specific dynamic PCDs
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: pre-validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
OvmfPkg/PlatformPei: set the SEV-SNP enabled PCD
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

OvmfPkg/OvmfPkg.dec | 24 +
UefiCpuPkg/UefiCpuPkg.dec | 11 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 5 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 1 +
OvmfPkg/OvmfPkgIa32X64.dsc | 6 +-
OvmfPkg/OvmfPkgX64.dsc | 5 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 14 +-
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 8 +
OvmfPkg/ResetVector/ResetVector.inf | 6 +
OvmfPkg/Sec/SecMain.inf | 3 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSecret.h | 18 +
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 ++
.../X64/SnpPageStateChange.h | 31 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 19 +
UefiCpuPkg/Library/MpInitLib/MpLib.h | 19 +
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 126 ++++++
.../X64/SecSnpSystemRamValidate.c | 36 ++
.../X64/SnpPageStateChangeInternal.c | 295 +++++++++++++
OvmfPkg/PlatformPei/AmdSev.c | 192 ++++++++
OvmfPkg/PlatformPei/MemDetect.c | 21 +
OvmfPkg/Sec/SecMain.c | 111 +++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 11 +-
.../MpInitLib/Ia32/SevSnpRmpAdjustInternal.c | 31 ++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 275 +++++++++++-
.../MpInitLib/X64/SevSnpRmpAdjustInternal.c | 44 ++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 27 ++
.../Ia32/{PageTables64.asm => AmdSev.asm} | 415 +++++++++---------
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 404 +----------------
OvmfPkg/ResetVector/ResetVector.nasmb | 7 +
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 1 +
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 51 +++
48 files changed, 1978 insertions(+), 630 deletions(-)
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/Ia32/SevSnpRmpAdjustInternal.c
create mode 100644
UefiCpuPkg/Library/MpInitLib/X64/SevSnpRmpAdjustInternal.c
copy OvmfPkg/ResetVector/Ia32/{PageTables64.asm => AmdSev.asm} (67%)

--
2.17.1


Re: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

Yao, Jiewen
 

Hi Brijesh
Thanks!

I think if we want to reuse this, we need rename the data structure.

First, we should use a generic name.

Second, I don’t think it is good idea to define two *enable* fields. Since only one of them should be enabled, we should use 1 field as enumeration.

Third, we should hide the SEV specific and TDX specific definition in CC common work area.

If we agree to use a common work area, I recommend below:

typedef struct {
UINT8 HeaderVersion; // 0
UINT8 HeadLength; // 4
UINT8 Type; // 0 - legacy, 1 - SEV, 2 - TDX
UINT8 SubType; // Type specific sub type, if needed.
} CC_COMMON_WORK_AREA_HEADER;

typedef struct {
CC_COMMON_WORK_AREA_HEADER Header;
// reset is valid if Type == 1
UINT8 Reserved1[4];
UINT64 RandomData;
UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

typedef struct {
CC_COMMON_WORK_AREA_HEADER Header;
// reset is valid if Type == 2
UINT8 TdxSpecific[]; // TBD
} TDX_WORK_AREA;

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Wednesday, July 28, 2021 10:34 PM
To: Yao, Jiewen <jiewen.yao@intel.com>; Xu, Min M <min.m.xu@intel.com>
Cc: brijesh.singh@amd.com; devel@edk2.groups.io; Ard Biesheuvel
<ardb+tianocore@kernel.org>; Justen, Jordan L <jordan.l.justen@intel.com>;
Erdem Aktas <erdemaktas@google.com>; James Bottomley
<jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in
ResetVector

Hi Jiewen and Min,

See my comments below.


On 7/28/21 2:54 AM, Yao, Jiewen wrote:
Yes. I am thinking the same thing.

[CC Flag memory location]
1) A general purpose register, such as EBP.

2) A global variable, such as
.data
TeeFlags: DD 0

3) A fixed region in stack, such as
dword[STACK_TOP - 4]

4) A new CC common fixed region, such as
dword[CC_COMMON_FLAGS]

5) A fixed region piggyback on existing CC working area, such as
dword[CC_WORKING_AREA]

Hi Brijesh/Min
Any preference?

[CC Indicator Flags]
Proposal: UINT8[4]

Byte [0] Version: 0
byte [1] Length: 4
byte [2] Type:
0: legacy
1: SEV
2: TDX
byte [3] Sub Type:
If Type is 0 (legacy), then
0: legacy
If Type is 1 (SEV), then
0: SEV
1: SEV-ES
2: SEV-SNP
If Type is 2 (TDX), then
0: TDX 1.0

Thank you
Yao Jiewen


-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com>
Sent: Wednesday, July 28, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L
<jordan.l.justen@intel.com>; Erdem Aktas <erdemaktas@google.com>;
James
Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>
Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

On July 28, 2021 2:05 PM, Yao, Jiewen wrote:
It does not necessary to be a working area.

We just need a common TEE flag to indicate if the system run in legacy, SEV,
or
TDX, right?
Right. We need somewhere to store this flag, either in a Register or in
Memory.
If it is memory, then in Tdx the memory region should be initialized by host
VMM.

thank you!
Yao, Jiewen


在 2021年7月28日,下午1:07,Xu, Min M <min.m.xu@intel.com> 写
道:

On July 27, 2021 8:46 PM, Yao, Jiewen wrote:
HI Min
I agree with Brijesh.

The basic rule is: SEV file shall never refer to TDX data structure.
TDX file shall never refer to SEV data structure.
These code should be isolated clearly.

Do we still need that logic if we follow the new pattern?
I have replied to Brijesh's mail about the concern of the new pattern.

I have some concern in the current pattern:
====================
OneTimeCall PreMainFunctionHookSev
OneTimeCall PreMainFunctionHookTdx
MainFunction:
XXXXXX
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
The TEE function need implement a TEE check function (such as IsSev, or
IsTdx).

Tdx call CPUID(0x21) to determine if it is tdx guest in the very
beginning of ResetVector. Then 'TDXG' is set in TDX_WORK_AREA. SEV
does
the similar work which call CheckSevFeatures to set SEV_ES_WORK_AREA to
1.

After that both TDX and SEV read the above WORK_AREA to check if it is
TDX
or SEV or legacy guest.

In Tdx the access to SEV_ES_WORK_AREA will trigger error because
SEV_ES_WORK_AREA is *NOT* initialized by host VMM.
In SEV-SNP I am afraid the access to TDX_WORK_AREA will trigger error
too.

I am wondering if TDX and SEV can use the same memory region (for
example,
TEE_WORK_AREA) as the work area?
So that this work area is guaranteed to be initialized in both TDX and
SEV. Structure of the TEE_WORK_AREA may look like this:
typedef struct {
UINT8 Flag[4]; 'TDXG' or 'SEVG' or all-0
UINT8 Others[];
} TEE_WORK_AREA;
Are we reserving a new page for the TDX_WORK_AREA ? I am wondering why
can't we use the SEV_ES_WORK_AREA instead of wasting space in the MEMFD.

The SEV_ES_WORK_AREA layout looks like this:

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 Reserved1[7];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

There is reserved bit after the SevEsEnabled and one byte can be used
for the TdxEnabled;

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 TdxEnabled;
UINT8 Reserved2[6];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

The SEV_ES_WORK_AREA can be treated as a TEE_WORK_AREA and we can be
pull out from MemEncrypSevLib.h to CcWorkAreaLib.h and rename the
structure (if needed).

Both the SEV-SNP and TEE host-VMM accepts the TEE_WORK_AREA before
booting the guest to ensure that its safe to access the memory without
going through the accept/validation process.

In case of the TDX, the reset vector code sets the TdxEnabled on the
entry. In case of the SEV, the workarea is valid from SEC to PEI phase
only and it gets reused for other purposes. The PEI phase set the Pcd's
(such as SevEsEnabled or SevEnabled etc) so that Dxe or other EDK2 core
does not need to know anything about the workarea and they simply can
read the PCDs.

-Brijesh




Re: [EXTERNAL] [edk2-devel] Missing TPM 2 related call to Tpm2HierarchyChangeAuth

Stefan Berger
 

I now filed this bug:

https://bugzilla.tianocore.org/show_bug.cgi?id=3510

    Stefan

On 7/28/21 10:38 AM, Michael Kubacki wrote:
The main commit of the series Bret mentioned (in edk2-platforms) is here:

https://github.com/tianocore/edk2-platforms/commit/bfabeef4c9a63374784bd19f18a869aa2769e011

Regards,
Michael

On 7/27/2021 12:25 PM, Yao, Jiewen wrote:
Oops. Sorry for late response.

The code is NOT in EDKII, but EDKII-platform as example. https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Tcg <https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Tcg>

We allow a platform having its own implementation. That is why it is NOT in EDKII.

Thank you

Yao Jiewen

*From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Bret Barkelew via groups.io
*Sent:* Wednesday, July 28, 2021 12:11 AM
*To:* devel@edk2.groups.io; stefanb@linux.ibm.com; Yao, Jiewen <jiewen.yao@intel.com>; Jeremiah Cox <jerecox@microsoft.com>; Michael Kubacki <Michael.Kubacki@microsoft.com>
*Cc:* Marc-André Lureau <marcandre.lureau@redhat.com>
*Subject:* Re: [EXTERNAL] [edk2-devel] Missing TPM 2 related call to Tpm2HierarchyChangeAuth

Adding @Jeremiah <mailto:jerecox@microsoft.com>…

Jeremiah, weren’t you or @Michael <mailto:Michael.Kubacki@microsoft.com> shopping this change to MinPlatform?

- Bret

*From: *Stefan Berger via groups.io <mailto:stefanb=linux.ibm.com@groups.io>
*Sent: *Monday, July 26, 2021 7:48 AM
*To: *Yao, Jiewen <mailto:jiewen.yao@intel.com>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
*Cc: *Marc-André Lureau <mailto:marcandre.lureau@redhat.com>
*Subject: *[EXTERNAL] [edk2-devel] Missing TPM 2 related call to Tpm2HierarchyChangeAuth

Hello!

    The TPM 2 code in EDK2 is missing an important call to
Tpm2HierarchyChangeAuth for the platform hierarchy. We have to set the
password of that hierarchy and discard the password. See also specs
section 11:
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrustedcomputinggroup.org%2Fwp-content%2Fuploads%2FTCG_PCClient_PFP_r1p05_v22_02dec2020.pdf&;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cf2a2262eee2c44b3760c08d95044601a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637629077356686202%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=N7VQIw87rHqUAFQ54TvhNwcsPFEwJzdZQ9JZrmX1S4E%3D&amp;reserved=0 <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrustedcomputinggroup.org%2Fwp-content%2Fuploads%2FTCG_PCClient_PFP_r1p05_v22_02dec2020.pdf&;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cf2a2262eee2c44b3760c08d95044601a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637629077356686202%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=N7VQIw87rHqUAFQ54TvhNwcsPFEwJzdZQ9JZrmX1S4E%3D&amp;reserved=0>

"Platform Firmware MUST protect access to the Platform Hierarchy and
prevent access to the platform hierarchy by
non-manufacturer-controlled components.  "

I was wondering where we could put that call so it's invoked after the
user has possibly interacted with the menu and before passing control to
the next stage such as boot loader.

Regards,

    Stefan








Re: [EXTERNAL] [edk2-devel] Missing TPM 2 related call to Tpm2HierarchyChangeAuth

Michael Kubacki
 

The main commit of the series Bret mentioned (in edk2-platforms) is here:

https://github.com/tianocore/edk2-platforms/commit/bfabeef4c9a63374784bd19f18a869aa2769e011

Regards,
Michael

On 7/27/2021 12:25 PM, Yao, Jiewen wrote:
Oops. Sorry for late response.
The code is NOT in EDKII, but EDKII-platform as example. https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Tcg <https://github.com/tianocore/edk2-platforms/tree/master/Platform/Intel/MinPlatformPkg/Tcg>
We allow a platform having its own implementation. That is why it is NOT in EDKII.
Thank you
Yao Jiewen
*From:* devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf Of *Bret Barkelew via groups.io
*Sent:* Wednesday, July 28, 2021 12:11 AM
*To:* devel@edk2.groups.io; stefanb@linux.ibm.com; Yao, Jiewen <jiewen.yao@intel.com>; Jeremiah Cox <jerecox@microsoft.com>; Michael Kubacki <Michael.Kubacki@microsoft.com>
*Cc:* Marc-André Lureau <marcandre.lureau@redhat.com>
*Subject:* Re: [EXTERNAL] [edk2-devel] Missing TPM 2 related call to Tpm2HierarchyChangeAuth
Adding @Jeremiah <mailto:jerecox@microsoft.com>…
Jeremiah, weren’t you or @Michael <mailto:Michael.Kubacki@microsoft.com> shopping this change to MinPlatform?
- Bret
*From: *Stefan Berger via groups.io <mailto:stefanb=linux.ibm.com@groups.io>
*Sent: *Monday, July 26, 2021 7:48 AM
*To: *Yao, Jiewen <mailto:jiewen.yao@intel.com>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
*Cc: *Marc-André Lureau <mailto:marcandre.lureau@redhat.com>
*Subject: *[EXTERNAL] [edk2-devel] Missing TPM 2 related call to Tpm2HierarchyChangeAuth
Hello!
   The TPM 2 code in EDK2 is missing an important call to
Tpm2HierarchyChangeAuth for the platform hierarchy. We have to set the
password of that hierarchy and discard the password. See also specs
section 11:
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrustedcomputinggroup.org%2Fwp-content%2Fuploads%2FTCG_PCClient_PFP_r1p05_v22_02dec2020.pdf&;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cf2a2262eee2c44b3760c08d95044601a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637629077356686202%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=N7VQIw87rHqUAFQ54TvhNwcsPFEwJzdZQ9JZrmX1S4E%3D&amp;reserved=0 <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Ftrustedcomputinggroup.org%2Fwp-content%2Fuploads%2FTCG_PCClient_PFP_r1p05_v22_02dec2020.pdf&;data=04%7C01%7Cbret.barkelew%40microsoft.com%7Cf2a2262eee2c44b3760c08d95044601a%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637629077356686202%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C2000&amp;sdata=N7VQIw87rHqUAFQ54TvhNwcsPFEwJzdZQ9JZrmX1S4E%3D&amp;reserved=0>
"Platform Firmware MUST protect access to the Platform Hierarchy and
prevent access to the platform hierarchy by
non-manufacturer-controlled components.  "
I was wondering where we could put that call so it's invoked after the
user has possibly interacted with the menu and before passing control to
the next stage such as boot loader.
Regards,
   Stefan


Re: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

Brijesh Singh
 

Hi Jiewen and Min,

See my comments below.


On 7/28/21 2:54 AM, Yao, Jiewen wrote:
Yes. I am thinking the same thing.
[CC Flag memory location]
1) A general purpose register, such as EBP.
2) A global variable, such as
.data
TeeFlags: DD 0
3) A fixed region in stack, such as
dword[STACK_TOP - 4]
4) A new CC common fixed region, such as
dword[CC_COMMON_FLAGS]
5) A fixed region piggyback on existing CC working area, such as
dword[CC_WORKING_AREA]
Hi Brijesh/Min
Any preference?
[CC Indicator Flags]
Proposal: UINT8[4]
Byte [0] Version: 0
byte [1] Length: 4
byte [2] Type:
0: legacy
1: SEV
2: TDX
byte [3] Sub Type:
If Type is 0 (legacy), then
0: legacy
If Type is 1 (SEV), then
0: SEV
1: SEV-ES
2: SEV-SNP
If Type is 2 (TDX), then
0: TDX 1.0
Thank you
Yao Jiewen

-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com>
Sent: Wednesday, July 28, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L
<jordan.l.justen@intel.com>; Erdem Aktas <erdemaktas@google.com>; James
Bottomley <jejb@linux.ibm.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

On July 28, 2021 2:05 PM, Yao, Jiewen wrote:
It does not necessary to be a working area.

We just need a common TEE flag to indicate if the system run in legacy, SEV,
or
TDX, right?
Right. We need somewhere to store this flag, either in a Register or in Memory.
If it is memory, then in Tdx the memory region should be initialized by host VMM.

thank you!
Yao, Jiewen


在 2021年7月28日,下午1:07,Xu, Min M <min.m.xu@intel.com> 写道:

On July 27, 2021 8:46 PM, Yao, Jiewen wrote:
HI Min
I agree with Brijesh.

The basic rule is: SEV file shall never refer to TDX data structure.
TDX file shall never refer to SEV data structure.
These code should be isolated clearly.

Do we still need that logic if we follow the new pattern?
I have replied to Brijesh's mail about the concern of the new pattern.

I have some concern in the current pattern:
====================
OneTimeCall PreMainFunctionHookSev
OneTimeCall PreMainFunctionHookTdx
MainFunction:
XXXXXX
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
The TEE function need implement a TEE check function (such as IsSev, or
IsTdx).

Tdx call CPUID(0x21) to determine if it is tdx guest in the very
beginning of ResetVector. Then 'TDXG' is set in TDX_WORK_AREA. SEV does
the similar work which call CheckSevFeatures to set SEV_ES_WORK_AREA to 1.

After that both TDX and SEV read the above WORK_AREA to check if it is TDX
or SEV or legacy guest.

In Tdx the access to SEV_ES_WORK_AREA will trigger error because
SEV_ES_WORK_AREA is *NOT* initialized by host VMM.
In SEV-SNP I am afraid the access to TDX_WORK_AREA will trigger error too.

I am wondering if TDX and SEV can use the same memory region (for
example,
TEE_WORK_AREA) as the work area?
So that this work area is guaranteed to be initialized in both TDX and
SEV. Structure of the TEE_WORK_AREA may look like this:
typedef struct {
UINT8 Flag[4]; 'TDXG' or 'SEVG' or all-0
UINT8 Others[];
} TEE_WORK_AREA;
Are we reserving a new page for the TDX_WORK_AREA ? I am wondering why can't we use the SEV_ES_WORK_AREA instead of wasting space in the MEMFD.

The SEV_ES_WORK_AREA layout looks like this:

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 Reserved1[7];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

There is reserved bit after the SevEsEnabled and one byte can be used for the TdxEnabled;

typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 TdxEnabled;
UINT8 Reserved2[6];

UINT64 RandomData;

UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

The SEV_ES_WORK_AREA can be treated as a TEE_WORK_AREA and we can be pull out from MemEncrypSevLib.h to CcWorkAreaLib.h and rename the structure (if needed).

Both the SEV-SNP and TEE host-VMM accepts the TEE_WORK_AREA before booting the guest to ensure that its safe to access the memory without going through the accept/validation process.

In case of the TDX, the reset vector code sets the TdxEnabled on the entry. In case of the SEV, the workarea is valid from SEC to PEI phase only and it gets reused for other purposes. The PEI phase set the Pcd's
(such as SevEsEnabled or SevEnabled etc) so that Dxe or other EDK2 core does not need to know anything about the workarea and they simply can read the PCDs.

-Brijesh


Re: [Patch V3] NetworkPkg: Making the HTTP IO timeout value programmable with PCD

Maciej Rabeda
 

Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 28-Jul-21 14:16, Heng Luo wrote:
From: Zachary Clark-Williams <zachary.clark-williams@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3507

HTTP boot has a default set forced timeout value of 5 seconds
for getting the recovery image from a remote source.
This change allows the HTTP boot flow to get the IO timeout value
from the PcdHttpIoTimeout.
PcdHttpIoTimeout value is set in platform code.

Signed-off-by: Zachary Clark-Williams <zachary.clark-williams@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
---
NetworkPkg/HttpBootDxe/HttpBootClient.c | 12 +++++++++---
NetworkPkg/HttpBootDxe/HttpBootClient.h | 7 +------
NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 3 ++-
NetworkPkg/HttpDxe/HttpDxe.inf | 3 ++-
NetworkPkg/HttpDxe/HttpImpl.c | 17 ++++++++++++++---
NetworkPkg/HttpDxe/HttpProto.h | 3 +--
NetworkPkg/NetworkPkg.dec | 8 ++++----
NetworkPkg/NetworkPkg.dsc | 3 +++
NetworkPkg/NetworkPkg.uni | 8 +++++++-
9 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 8f21f7766e..aa0a153019 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -1,7 +1,7 @@
/** @file
Implementation of the boot file download function.
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -596,19 +596,25 @@ HttpBootCreateHttpIo (
HTTP_IO_CONFIG_DATA ConfigData;
EFI_STATUS Status;
EFI_HANDLE ImageHandle;
+ UINT32 TimeoutValue;
ASSERT (Private != NULL);
+ //
+ // Get HTTP timeout value
+ //
+ TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
ZeroMem (&ConfigData, sizeof (HTTP_IO_CONFIG_DATA));
if (!Private->UsingIpv6) {
ConfigData.Config4.HttpVersion = HttpVersion11;
- ConfigData.Config4.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;
+ ConfigData.Config4.RequestTimeOut = TimeoutValue;
IP4_COPY_ADDRESS (&ConfigData.Config4.LocalIp, &Private->StationIp.v4);
IP4_COPY_ADDRESS (&ConfigData.Config4.SubnetMask, &Private->SubnetMask.v4);
ImageHandle = Private->Ip4Nic->ImageHandle;
} else {
ConfigData.Config6.HttpVersion = HttpVersion11;
- ConfigData.Config6.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;
+ ConfigData.Config6.RequestTimeOut = TimeoutValue;
IP6_COPY_ADDRESS (&ConfigData.Config6.LocalIp, &Private->StationIp.v6);
ImageHandle = Private->Ip6Nic->ImageHandle;
}
diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h b/NetworkPkg/HttpBootDxe/HttpBootClient.h
index 971b2dc800..3a98f0f557 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
@@ -1,7 +1,7 @@
/** @file
Declaration of the boot file download function.
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,12 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef __EFI_HTTP_BOOT_HTTP_H__
#define __EFI_HTTP_BOOT_HTTP_H__
-#define HTTP_BOOT_REQUEST_TIMEOUT 5000 // 5 seconds in uints of millisecond.
-#define HTTP_BOOT_RESPONSE_TIMEOUT 5000 // 5 seconds in uints of millisecond.
#define HTTP_BOOT_BLOCK_SIZE 1500
-
-
-
#define HTTP_USER_AGENT_EFI_HTTP_BOOT "UefiHttpBoot/1.0"
//
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
index a27a561722..cffa642a4b 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
@@ -1,7 +1,7 @@
## @file
# This modules produce the Load File Protocol for UEFI HTTP boot.
#
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2020 Hewlett-Packard Development Company, L.P.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -96,6 +96,7 @@
[Pcd]
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout ## CONSUMES
[UserExtensions.TianoCore."ExtraFiles"]
HttpBootDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 35fe31af18..8dd3838793 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,7 +1,7 @@
## @file
# Implementation of EFI HTTP protocol interfaces.
#
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -73,6 +73,7 @@
[Pcd]
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout ## CONSUMES
[UserExtensions.TianoCore."ExtraFiles"]
HttpDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 5a6ecbc9d9..8790e9b4e0 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1,7 +1,7 @@
/** @file
Implementation of EFI_HTTP_PROTOCOL protocol interfaces.
- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -983,6 +983,7 @@ HttpResponseWorker (
HTTP_TOKEN_WRAP *ValueInItem;
UINTN HdrLen;
NET_FRAGMENT Fragment;
+ UINT32 TimeoutValue;
if (Wrap == NULL || Wrap->HttpInstance == NULL) {
return EFI_INVALID_PARAMETER;
@@ -1052,10 +1053,15 @@ HttpResponseWorker (
}
}
+ //
+ // Get HTTP timeout value
+ //
+ TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
//
// Start the timer, and wait Timeout seconds to receive the header packet.
//
- Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+ Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, TimeoutValue * TICKS_PER_MS);
if (EFI_ERROR (Status)) {
goto Error;
}
@@ -1329,10 +1335,15 @@ HttpResponseWorker (
}
}
+ //
+ // Get HTTP timeout value
+ //
+ TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
//
// Start the timer, and wait Timeout seconds to receive the body packet.
//
- Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+ Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, TimeoutValue * TICKS_PER_MS);
if (EFI_ERROR (Status)) {
goto Error2;
}
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 00ba26aca4..6b3e49090e 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -1,7 +1,7 @@
/** @file
The header files of miscellaneous routines for HttpDxe driver.
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -41,7 +41,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define HTTP_BUFFER_SIZE_DEAULT 65535
#define HTTP_MAX_SYN_BACK_LOG 5
#define HTTP_CONNECTION_TIMEOUT 60
-#define HTTP_RESPONSE_TIMEOUT 5
#define HTTP_DATA_RETRIES 12
#define HTTP_FIN_TIMEOUT 2
#define HTTP_KEEP_ALIVE_PROBES 6
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index b81f10ef6e..3e1f5c101d 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -97,10 +97,6 @@
# @Prompt Max size of total HTTP chunk transfer. the default value is 12MB.
gEfiNetworkPkgTokenSpaceGuid.PcdMaxHttpChunkTransfer|0x0C00000|UINT32|0x0000000E
- ## The Timeout value of HTTP IO.
- # @Prompt The Timeout value of HTTP Io. Default value is 5000.
- gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F
-
[PcdsFixedAtBuild, PcdsPatchableInModule]
## Indicates whether HTTP connections (i.e., unsecured) are permitted or not.
# TRUE - HTTP connections are allowed. Both the "https://" and "http://" URI schemes are permitted.
@@ -160,5 +156,9 @@
# 0x00 = PXE Disabled
gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01|UINT8|0x1000000a
+ ## The Timeout value of HTTP IO.
+ # @Prompt The Timeout value of HTTP Io. Default value is 5000.
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F
+
[UserExtensions.TianoCore."ExtraFiles"]
NetworkPkgExtra.uni
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
index 5e6619ad85..04685a844d 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -87,6 +87,9 @@
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000000
+[PcdsDynamic]
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpTimeout|5000
+
###################################################################################################
#
# Components Section - list of the modules and components that will be processed by compilation
diff --git a/NetworkPkg/NetworkPkg.uni b/NetworkPkg/NetworkPkg.uni
index 328d8cb54a..6d0fa67c6f 100644
--- a/NetworkPkg/NetworkPkg.uni
+++ b/NetworkPkg/NetworkPkg.uni
@@ -3,7 +3,7 @@
//
// This package provides network modules that conform to UEFI 2.4 specification.
//
-// Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
//
// SPDX-License-Identifier: BSD-2-Clause-Patent
//
@@ -105,3 +105,9 @@
#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdTftpBlockSize_HELP #language en-US "This setting can override the default TFTP block size. A value of 0 computes "
"the default from MTU information. A non-zero value will be used as block size "
"in bytes."
+
+#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_PROMPT #language en-US "HTTP Boot Image Request and Response Timeout"
+
+#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_HELP #language en-US "This value is used to configure the request and response timeout when getting "
+ "the recovery image from the remote source during an HTTP recovery boot."
+ "The default value set is 5 seconds."


Re: [Patch V3] NetworkPkg: Add HTTP Additional Event Notifications

Maciej Rabeda
 

Reviewed-by: Maciej Rabeda <maciej.rabeda@linux.intel.com>

On 28-Jul-21 13:58, Heng Luo wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3496

Add new EDKII_HTTP_CALLBACK_PROTOCOL in NetworkPkg,
Send HTTP Events via EDKII_HTTP_CALLBACK_PROTOCOL
when Dns/ConnectTcp/TlsConnectSession/InitSession
occurs.

Signed-off-by: Heng Luo <heng.luo@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
---

NetworkPkg/HttpDxe/HttpDriver.h | 3 ++-
NetworkPkg/HttpDxe/HttpDxe.inf | 3 ++-
NetworkPkg/HttpDxe/HttpImpl.c | 4 +++-
NetworkPkg/HttpDxe/HttpProto.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
NetworkPkg/HttpDxe/HttpProto.h | 15 ++++++++++++++-
NetworkPkg/Include/Protocol/HttpCallback.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
NetworkPkg/NetworkPkg.dec | 3 +++
7 files changed, 166 insertions(+), 5 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 5fe8c5b5e9..b701b80858 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -1,7 +1,7 @@
/** @file
The header files of the driver binding and service binding protocol for HttpDxe driver.
- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -47,6 +47,7 @@
#include <Protocol/Ip6Config.h>
#include <Protocol/Tls.h>
#include <Protocol/TlsConfig.h>
+#include <Protocol/HttpCallback.h>
#include <Guid/ImageAuthentication.h>
//
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 35fe31af18..23fb9ec394 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,7 +1,7 @@
## @file
# Implementation of EFI HTTP protocol interfaces.
#
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -65,6 +65,7 @@
gEfiTlsServiceBindingProtocolGuid ## SOMETIMES_CONSUMES
gEfiTlsProtocolGuid ## SOMETIMES_CONSUMES
gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
+ gEdkiiHttpCallbackProtocolGuid ## SOMETIMES_CONSUMES
[Guids]
gEfiTlsCaCertificateGuid ## SOMETIMES_CONSUMES ## Variable:L"TlsCaCertificate"
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 5a6ecbc9d9..97f15d229f 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1,7 +1,7 @@
/** @file
Implementation of EFI_HTTP_PROTOCOL protocol interfaces.
- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -527,6 +527,7 @@ EfiHttpRequest (
} else {
Status = HttpDns6 (HttpInstance, HostNameStr, &HttpInstance->RemoteIpv6Addr);
}
+ HttpNotify (HttpEventDns, Status);
FreePool (HostNameStr);
if (EFI_ERROR (Status)) {
@@ -588,6 +589,7 @@ EfiHttpRequest (
Configure || ReConfigure,
TlsConfigure
);
+ HttpNotify (HttpEventInitSession, Status);
if (EFI_ERROR (Status)) {
goto Error2;
}
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index afc7db5a72..affa916bd6 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1,7 +1,7 @@
/** @file
Miscellaneous routines for HttpDxe driver.
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -966,6 +966,7 @@ HttpCreateConnection (
HttpInstance->IsTcp4ConnDone = FALSE;
HttpInstance->Tcp4ConnToken.CompletionToken.Status = EFI_NOT_READY;
Status = HttpInstance->Tcp4->Connect (HttpInstance->Tcp4, &HttpInstance->Tcp4ConnToken);
+ HttpNotify (HttpEventConnectTcp, Status);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "HttpCreateConnection: Tcp4->Connect() = %r\n", Status));
return Status;
@@ -981,6 +982,7 @@ HttpCreateConnection (
HttpInstance->IsTcp6ConnDone = FALSE;
HttpInstance->Tcp6ConnToken.CompletionToken.Status = EFI_NOT_READY;
Status = HttpInstance->Tcp6->Connect (HttpInstance->Tcp6, &HttpInstance->Tcp6ConnToken);
+ HttpNotify (HttpEventConnectTcp, Status);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "HttpCreateConnection: Tcp6->Connect() = %r\n", Status));
return Status;
@@ -1277,6 +1279,7 @@ HttpConnectTcp4 (
}
Status = TlsConnectSession (HttpInstance, HttpInstance->TimeoutEvent);
+ HttpNotify (HttpEventTlsConnectSession, Status);
gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
@@ -1369,6 +1372,7 @@ HttpConnectTcp6 (
}
Status = TlsConnectSession (HttpInstance, HttpInstance->TimeoutEvent);
+ HttpNotify (HttpEventTlsConnectSession, Status);
gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);
@@ -2195,3 +2199,55 @@ HttpTcpTokenCleanup (
}
}
+
+/**
+ Send Events via EDKII_HTTP_CALLBACK_PROTOCOL.
+
+ @param[in] Event The event that occurs in the current state.
+ @param[in] EventStatus The Status of Event, EFI_SUCCESS or other errors.
+
+**/
+VOID
+HttpNotify (
+ IN EDKII_HTTP_CALLBACK_EVENT Event,
+ IN EFI_STATUS EventStatus
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE *Handles;
+ UINTN Index;
+ UINTN HandleCount;
+ EFI_HANDLE Handle;
+ EDKII_HTTP_CALLBACK_PROTOCOL *HttpCallback;
+
+ DEBUG ((DEBUG_INFO, "HttpNotify: Event - %d, EventStatus - %r\n", Event, EventStatus));
+
+ Handles = NULL;
+ HandleCount = 0;
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEdkiiHttpCallbackProtocolGuid,
+ NULL,
+ &HandleCount,
+ &Handles
+ );
+ if (Status == EFI_SUCCESS) {
+ for (Index = 0; Index < HandleCount; Index++) {
+ Handle = Handles[Index];
+ Status = gBS->HandleProtocol (
+ Handle,
+ &gEdkiiHttpCallbackProtocolGuid,
+ (VOID **) &HttpCallback
+ );
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "HttpNotify: Notifying %p\n", HttpCallback));
+ HttpCallback->Callback (
+ HttpCallback,
+ Event,
+ EventStatus
+ );
+ }
+ }
+ FreePool (Handles);
+ }
+}
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 00ba26aca4..5b90a6b074 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -1,7 +1,7 @@
/** @file
The header files of miscellaneous routines for HttpDxe driver.
-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -609,4 +609,17 @@ HttpResponseWorker (
IN HTTP_TOKEN_WRAP *Wrap
);
+/**
+ Send Events via EDKII_HTTP_CALLBACK_PROTOCOL.
+
+ @param[in] Event The event that occurs in the current state.
+ @param[in] EventStatus The Status of Event, EFI_SUCCESS or other errors.
+
+**/
+VOID
+HttpNotify (
+ IN EDKII_HTTP_CALLBACK_EVENT Event,
+ IN EFI_STATUS EventStatus
+ );
+
#endif
diff --git a/NetworkPkg/Include/Protocol/HttpCallback.h b/NetworkPkg/Include/Protocol/HttpCallback.h
new file mode 100644
index 0000000000..d036a8a4be
--- /dev/null
+++ b/NetworkPkg/Include/Protocol/HttpCallback.h
@@ -0,0 +1,85 @@
+/** @file
+ This file defines the EDKII HTTP Callback Protocol interface.
+
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __EDKII_HTTP_CALLBACK_H__
+#define __EDKII_HTTP_CALLBACK_H__
+
+#define EDKII_HTTP_CALLBACK_PROTOCOL_GUID \
+ { \
+ 0x611114f1, 0xa37b, 0x4468, {0xa4, 0x36, 0x5b, 0xdd, 0xa1, 0x6a, 0xa2, 0x40} \
+ }
+
+typedef struct _EDKII_HTTP_CALLBACK_PROTOCOL EDKII_HTTP_CALLBACK_PROTOCOL;
+
+///
+/// EDKII_HTTP_CALLBACK_EVENT
+///
+typedef enum {
+ ///
+ /// The Status of DNS Event to retrieve the host address.
+ /// EventStatus:
+ /// EFI_SUCCESS Operation succeeded.
+ /// EFI_OUT_OF_RESOURCES Failed to allocate needed resources.
+ /// EFI_DEVICE_ERROR An unexpected network error occurred.
+ /// Others Other errors as indicated.
+ ///
+ HttpEventDns,
+
+ ///
+ /// The Status of Event to initiate a nonblocking TCP connection request.
+ /// EventStatus:
+ /// EFI_SUCCESS The connection request is successfully initiated.
+ /// EFI_NOT_STARTED This EFI TCP Protocol instance has not been configured.
+ /// EFI_DEVICE_ERROR An unexpected system or network error occurred.
+ /// Others Other errors as indicated.
+ ///
+ HttpEventConnectTcp,
+
+ ///
+ /// The Status of Event to connect one TLS session by finishing the TLS handshake process.
+ /// EventStatus:
+ /// EFI_SUCCESS The TLS session is established.
+ /// EFI_OUT_OF_RESOURCES Can't allocate memory resources.
+ /// EFI_ABORTED TLS session state is incorrect.
+ /// Others Other error as indicated.
+ ///
+ HttpEventTlsConnectSession,
+
+ ///
+ /// The Status of Event to initialize Http session
+ /// EventStatus:
+ /// EFI_SUCCESS The initialization of session is done.
+ /// Others Other error as indicated.
+ ///
+ HttpEventInitSession
+} EDKII_HTTP_CALLBACK_EVENT;
+
+/**
+ Callback function that is invoked when HTTP event occurs.
+
+ @param[in] This Pointer to the EDKII_HTTP_CALLBACK_PROTOCOL instance.
+ @param[in] Event The event that occurs in the current state.
+ @param[in] EventStatus The Status of Event, EFI_SUCCESS or other errors.
+**/
+typedef
+VOID
+(EFIAPI * EDKII_HTTP_CALLBACK) (
+ IN EDKII_HTTP_CALLBACK_PROTOCOL *This,
+ IN EDKII_HTTP_CALLBACK_EVENT Event,
+ IN EFI_STATUS EventStatus
+ );
+
+///
+/// EFI HTTP Callback Protocol is invoked when HTTP event occurs.
+///
+struct _EDKII_HTTP_CALLBACK_PROTOCOL {
+ EDKII_HTTP_CALLBACK Callback;
+};
+
+extern EFI_GUID gEdkiiHttpCallbackProtocolGuid;
+
+#endif
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index b81f10ef6e..0f9f7bb15e 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -88,6 +88,9 @@
## Include/Protocol/Dpc.h
gEfiDpcProtocolGuid = {0x480f8ae9, 0xc46, 0x4aa9, { 0xbc, 0x89, 0xdb, 0x9f, 0xba, 0x61, 0x98, 0x6 }}
+ ## Include/Protocol/HttpCallback.h
+ gEdkiiHttpCallbackProtocolGuid = {0x611114f1, 0xa37b, 0x4468, {0xa4, 0x36, 0x5b, 0xdd, 0xa1, 0x6a, 0xa2, 0x40}}
+
[PcdsFixedAtBuild]
## The max attempt number will be created by iSCSI driver.
# @Prompt Max attempt number.


[Patch V3] NetworkPkg: Making the HTTP IO timeout value programmable with PCD

Heng Luo
 

From: Zachary Clark-Williams <zachary.clark-williams@intel.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3507

HTTP boot has a default set forced timeout value of 5 seconds
for getting the recovery image from a remote source.
This change allows the HTTP boot flow to get the IO timeout value
from the PcdHttpIoTimeout.
PcdHttpIoTimeout value is set in platform code.

Signed-off-by: Zachary Clark-Williams <zachary.clark-williams@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
---
NetworkPkg/HttpBootDxe/HttpBootClient.c | 12 +++++++++---
NetworkPkg/HttpBootDxe/HttpBootClient.h | 7 +------
NetworkPkg/HttpBootDxe/HttpBootDxe.inf | 3 ++-
NetworkPkg/HttpDxe/HttpDxe.inf | 3 ++-
NetworkPkg/HttpDxe/HttpImpl.c | 17 ++++++++++++++---
NetworkPkg/HttpDxe/HttpProto.h | 3 +--
NetworkPkg/NetworkPkg.dec | 8 ++++----
NetworkPkg/NetworkPkg.dsc | 3 +++
NetworkPkg/NetworkPkg.uni | 8 +++++++-
9 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.c b/NetworkPkg/HttpBootDxe/HttpBootClient.c
index 8f21f7766e..aa0a153019 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.c
@@ -1,7 +1,7 @@
/** @file
Implementation of the boot file download function.

-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -596,19 +596,25 @@ HttpBootCreateHttpIo (
HTTP_IO_CONFIG_DATA ConfigData;
EFI_STATUS Status;
EFI_HANDLE ImageHandle;
+ UINT32 TimeoutValue;

ASSERT (Private != NULL);

+ //
+ // Get HTTP timeout value
+ //
+ TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
ZeroMem (&ConfigData, sizeof (HTTP_IO_CONFIG_DATA));
if (!Private->UsingIpv6) {
ConfigData.Config4.HttpVersion = HttpVersion11;
- ConfigData.Config4.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;
+ ConfigData.Config4.RequestTimeOut = TimeoutValue;
IP4_COPY_ADDRESS (&ConfigData.Config4.LocalIp, &Private->StationIp.v4);
IP4_COPY_ADDRESS (&ConfigData.Config4.SubnetMask, &Private->SubnetMask.v4);
ImageHandle = Private->Ip4Nic->ImageHandle;
} else {
ConfigData.Config6.HttpVersion = HttpVersion11;
- ConfigData.Config6.RequestTimeOut = HTTP_BOOT_REQUEST_TIMEOUT;
+ ConfigData.Config6.RequestTimeOut = TimeoutValue;
IP6_COPY_ADDRESS (&ConfigData.Config6.LocalIp, &Private->StationIp.v6);
ImageHandle = Private->Ip6Nic->ImageHandle;
}
diff --git a/NetworkPkg/HttpBootDxe/HttpBootClient.h b/NetworkPkg/HttpBootDxe/HttpBootClient.h
index 971b2dc800..3a98f0f557 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootClient.h
+++ b/NetworkPkg/HttpBootDxe/HttpBootClient.h
@@ -1,7 +1,7 @@
/** @file
Declaration of the boot file download function.

-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -10,12 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#ifndef __EFI_HTTP_BOOT_HTTP_H__
#define __EFI_HTTP_BOOT_HTTP_H__

-#define HTTP_BOOT_REQUEST_TIMEOUT 5000 // 5 seconds in uints of millisecond.
-#define HTTP_BOOT_RESPONSE_TIMEOUT 5000 // 5 seconds in uints of millisecond.
#define HTTP_BOOT_BLOCK_SIZE 1500
-
-
-
#define HTTP_USER_AGENT_EFI_HTTP_BOOT "UefiHttpBoot/1.0"

//
diff --git a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
index a27a561722..cffa642a4b 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
+++ b/NetworkPkg/HttpBootDxe/HttpBootDxe.inf
@@ -1,7 +1,7 @@
## @file
# This modules produce the Load File Protocol for UEFI HTTP boot.
#
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
# (C) Copyright 2020 Hewlett-Packard Development Company, L.P.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -96,6 +96,7 @@

[Pcd]
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout ## CONSUMES

[UserExtensions.TianoCore."ExtraFiles"]
HttpBootDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 35fe31af18..8dd3838793 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,7 +1,7 @@
## @file
# Implementation of EFI HTTP protocol interfaces.
#
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -73,6 +73,7 @@

[Pcd]
gEfiNetworkPkgTokenSpaceGuid.PcdAllowHttpConnections ## CONSUMES
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout ## CONSUMES

[UserExtensions.TianoCore."ExtraFiles"]
HttpDxeExtra.uni
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 5a6ecbc9d9..8790e9b4e0 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1,7 +1,7 @@
/** @file
Implementation of EFI_HTTP_PROTOCOL protocol interfaces.

- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -983,6 +983,7 @@ HttpResponseWorker (
HTTP_TOKEN_WRAP *ValueInItem;
UINTN HdrLen;
NET_FRAGMENT Fragment;
+ UINT32 TimeoutValue;

if (Wrap == NULL || Wrap->HttpInstance == NULL) {
return EFI_INVALID_PARAMETER;
@@ -1052,10 +1053,15 @@ HttpResponseWorker (
}
}

+ //
+ // Get HTTP timeout value
+ //
+ TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
//
// Start the timer, and wait Timeout seconds to receive the header packet.
//
- Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+ Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, TimeoutValue * TICKS_PER_MS);
if (EFI_ERROR (Status)) {
goto Error;
}
@@ -1329,10 +1335,15 @@ HttpResponseWorker (
}
}

+ //
+ // Get HTTP timeout value
+ //
+ TimeoutValue = PcdGet32 (PcdHttpIoTimeout);
+
//
// Start the timer, and wait Timeout seconds to receive the body packet.
//
- Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, HTTP_RESPONSE_TIMEOUT * TICKS_PER_SECOND);
+ Status = gBS->SetTimer (HttpInstance->TimeoutEvent, TimerRelative, TimeoutValue * TICKS_PER_MS);
if (EFI_ERROR (Status)) {
goto Error2;
}
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 00ba26aca4..6b3e49090e 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -1,7 +1,7 @@
/** @file
The header files of miscellaneous routines for HttpDxe driver.

-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -41,7 +41,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define HTTP_BUFFER_SIZE_DEAULT 65535
#define HTTP_MAX_SYN_BACK_LOG 5
#define HTTP_CONNECTION_TIMEOUT 60
-#define HTTP_RESPONSE_TIMEOUT 5
#define HTTP_DATA_RETRIES 12
#define HTTP_FIN_TIMEOUT 2
#define HTTP_KEEP_ALIVE_PROBES 6
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index b81f10ef6e..3e1f5c101d 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -97,10 +97,6 @@
# @Prompt Max size of total HTTP chunk transfer. the default value is 12MB.
gEfiNetworkPkgTokenSpaceGuid.PcdMaxHttpChunkTransfer|0x0C00000|UINT32|0x0000000E

- ## The Timeout value of HTTP IO.
- # @Prompt The Timeout value of HTTP Io. Default value is 5000.
- gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F
-
[PcdsFixedAtBuild, PcdsPatchableInModule]
## Indicates whether HTTP connections (i.e., unsecured) are permitted or not.
# TRUE - HTTP connections are allowed. Both the "https://" and "http://" URI schemes are permitted.
@@ -160,5 +156,9 @@
# 0x00 = PXE Disabled
gEfiNetworkPkgTokenSpaceGuid.PcdIPv6PXESupport|0x01|UINT8|0x1000000a

+ ## The Timeout value of HTTP IO.
+ # @Prompt The Timeout value of HTTP Io. Default value is 5000.
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpIoTimeout|5000|UINT32|0x0000000F
+
[UserExtensions.TianoCore."ExtraFiles"]
NetworkPkgExtra.uni
diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc
index 5e6619ad85..04685a844d 100644
--- a/NetworkPkg/NetworkPkg.dsc
+++ b/NetworkPkg/NetworkPkg.dsc
@@ -87,6 +87,9 @@
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0x2f
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x80000000

+[PcdsDynamic]
+ gEfiNetworkPkgTokenSpaceGuid.PcdHttpTimeout|5000
+
###################################################################################################
#
# Components Section - list of the modules and components that will be processed by compilation
diff --git a/NetworkPkg/NetworkPkg.uni b/NetworkPkg/NetworkPkg.uni
index 328d8cb54a..6d0fa67c6f 100644
--- a/NetworkPkg/NetworkPkg.uni
+++ b/NetworkPkg/NetworkPkg.uni
@@ -3,7 +3,7 @@
//
// This package provides network modules that conform to UEFI 2.4 specification.
//
-// Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2009 - 2021, Intel Corporation. All rights reserved.<BR>
//
// SPDX-License-Identifier: BSD-2-Clause-Patent
//
@@ -105,3 +105,9 @@
#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdTftpBlockSize_HELP #language en-US "This setting can override the default TFTP block size. A value of 0 computes "
"the default from MTU information. A non-zero value will be used as block size "
"in bytes."
+
+#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_PROMPT #language en-US "HTTP Boot Image Request and Response Timeout"
+
+#string STR_gEfiNetworkPkgTokenSpaceGuid_PcdHttpIoTimeout_HELP #language en-US "This value is used to configure the request and response timeout when getting "
+ "the recovery image from the remote source during an HTTP recovery boot."
+ "The default value set is 5 seconds."
--
2.31.1.windows.1


[Patch V3] NetworkPkg: Add HTTP Additional Event Notifications

Heng Luo
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3496

Add new EDKII_HTTP_CALLBACK_PROTOCOL in NetworkPkg,
Send HTTP Events via EDKII_HTTP_CALLBACK_PROTOCOL
when Dns/ConnectTcp/TlsConnectSession/InitSession
occurs.

Signed-off-by: Heng Luo <heng.luo@intel.com>
Cc: Maciej Rabeda <maciej.rabeda@linux.intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Siyuan Fu <siyuan.fu@intel.com>
---

NetworkPkg/HttpDxe/HttpDriver.h | 3 ++-
NetworkPkg/HttpDxe/HttpDxe.inf | 3 ++-
NetworkPkg/HttpDxe/HttpImpl.c | 4 +++-
NetworkPkg/HttpDxe/HttpProto.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
NetworkPkg/HttpDxe/HttpProto.h | 15 ++++++++++++++-
NetworkPkg/Include/Protocol/HttpCallback.h | 85 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
NetworkPkg/NetworkPkg.dec | 3 +++
7 files changed, 166 insertions(+), 5 deletions(-)

diff --git a/NetworkPkg/HttpDxe/HttpDriver.h b/NetworkPkg/HttpDxe/HttpDriver.h
index 5fe8c5b5e9..b701b80858 100644
--- a/NetworkPkg/HttpDxe/HttpDriver.h
+++ b/NetworkPkg/HttpDxe/HttpDriver.h
@@ -1,7 +1,7 @@
/** @file
The header files of the driver binding and service binding protocol for HttpDxe driver.

- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -47,6 +47,7 @@
#include <Protocol/Ip6Config.h>
#include <Protocol/Tls.h>
#include <Protocol/TlsConfig.h>
+#include <Protocol/HttpCallback.h>

#include <Guid/ImageAuthentication.h>
//
diff --git a/NetworkPkg/HttpDxe/HttpDxe.inf b/NetworkPkg/HttpDxe/HttpDxe.inf
index 35fe31af18..23fb9ec394 100644
--- a/NetworkPkg/HttpDxe/HttpDxe.inf
+++ b/NetworkPkg/HttpDxe/HttpDxe.inf
@@ -1,7 +1,7 @@
## @file
# Implementation of EFI HTTP protocol interfaces.
#
-# Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -65,6 +65,7 @@
gEfiTlsServiceBindingProtocolGuid ## SOMETIMES_CONSUMES
gEfiTlsProtocolGuid ## SOMETIMES_CONSUMES
gEfiTlsConfigurationProtocolGuid ## SOMETIMES_CONSUMES
+ gEdkiiHttpCallbackProtocolGuid ## SOMETIMES_CONSUMES

[Guids]
gEfiTlsCaCertificateGuid ## SOMETIMES_CONSUMES ## Variable:L"TlsCaCertificate"
diff --git a/NetworkPkg/HttpDxe/HttpImpl.c b/NetworkPkg/HttpDxe/HttpImpl.c
index 5a6ecbc9d9..97f15d229f 100644
--- a/NetworkPkg/HttpDxe/HttpImpl.c
+++ b/NetworkPkg/HttpDxe/HttpImpl.c
@@ -1,7 +1,7 @@
/** @file
Implementation of EFI_HTTP_PROTOCOL protocol interfaces.

- Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -527,6 +527,7 @@ EfiHttpRequest (
} else {
Status = HttpDns6 (HttpInstance, HostNameStr, &HttpInstance->RemoteIpv6Addr);
}
+ HttpNotify (HttpEventDns, Status);

FreePool (HostNameStr);
if (EFI_ERROR (Status)) {
@@ -588,6 +589,7 @@ EfiHttpRequest (
Configure || ReConfigure,
TlsConfigure
);
+ HttpNotify (HttpEventInitSession, Status);
if (EFI_ERROR (Status)) {
goto Error2;
}
diff --git a/NetworkPkg/HttpDxe/HttpProto.c b/NetworkPkg/HttpDxe/HttpProto.c
index afc7db5a72..affa916bd6 100644
--- a/NetworkPkg/HttpDxe/HttpProto.c
+++ b/NetworkPkg/HttpDxe/HttpProto.c
@@ -1,7 +1,7 @@
/** @file
Miscellaneous routines for HttpDxe driver.

-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -966,6 +966,7 @@ HttpCreateConnection (
HttpInstance->IsTcp4ConnDone = FALSE;
HttpInstance->Tcp4ConnToken.CompletionToken.Status = EFI_NOT_READY;
Status = HttpInstance->Tcp4->Connect (HttpInstance->Tcp4, &HttpInstance->Tcp4ConnToken);
+ HttpNotify (HttpEventConnectTcp, Status);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "HttpCreateConnection: Tcp4->Connect() = %r\n", Status));
return Status;
@@ -981,6 +982,7 @@ HttpCreateConnection (
HttpInstance->IsTcp6ConnDone = FALSE;
HttpInstance->Tcp6ConnToken.CompletionToken.Status = EFI_NOT_READY;
Status = HttpInstance->Tcp6->Connect (HttpInstance->Tcp6, &HttpInstance->Tcp6ConnToken);
+ HttpNotify (HttpEventConnectTcp, Status);
if (EFI_ERROR (Status)) {
DEBUG ((EFI_D_ERROR, "HttpCreateConnection: Tcp6->Connect() = %r\n", Status));
return Status;
@@ -1277,6 +1279,7 @@ HttpConnectTcp4 (
}

Status = TlsConnectSession (HttpInstance, HttpInstance->TimeoutEvent);
+ HttpNotify (HttpEventTlsConnectSession, Status);

gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);

@@ -1369,6 +1372,7 @@ HttpConnectTcp6 (
}

Status = TlsConnectSession (HttpInstance, HttpInstance->TimeoutEvent);
+ HttpNotify (HttpEventTlsConnectSession, Status);

gBS->SetTimer (HttpInstance->TimeoutEvent, TimerCancel, 0);

@@ -2195,3 +2199,55 @@ HttpTcpTokenCleanup (
}

}
+
+/**
+ Send Events via EDKII_HTTP_CALLBACK_PROTOCOL.
+
+ @param[in] Event The event that occurs in the current state.
+ @param[in] EventStatus The Status of Event, EFI_SUCCESS or other errors.
+
+**/
+VOID
+HttpNotify (
+ IN EDKII_HTTP_CALLBACK_EVENT Event,
+ IN EFI_STATUS EventStatus
+ )
+{
+ EFI_STATUS Status;
+ EFI_HANDLE *Handles;
+ UINTN Index;
+ UINTN HandleCount;
+ EFI_HANDLE Handle;
+ EDKII_HTTP_CALLBACK_PROTOCOL *HttpCallback;
+
+ DEBUG ((DEBUG_INFO, "HttpNotify: Event - %d, EventStatus - %r\n", Event, EventStatus));
+
+ Handles = NULL;
+ HandleCount = 0;
+ Status = gBS->LocateHandleBuffer (
+ ByProtocol,
+ &gEdkiiHttpCallbackProtocolGuid,
+ NULL,
+ &HandleCount,
+ &Handles
+ );
+ if (Status == EFI_SUCCESS) {
+ for (Index = 0; Index < HandleCount; Index++) {
+ Handle = Handles[Index];
+ Status = gBS->HandleProtocol (
+ Handle,
+ &gEdkiiHttpCallbackProtocolGuid,
+ (VOID **) &HttpCallback
+ );
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "HttpNotify: Notifying %p\n", HttpCallback));
+ HttpCallback->Callback (
+ HttpCallback,
+ Event,
+ EventStatus
+ );
+ }
+ }
+ FreePool (Handles);
+ }
+}
diff --git a/NetworkPkg/HttpDxe/HttpProto.h b/NetworkPkg/HttpDxe/HttpProto.h
index 00ba26aca4..5b90a6b074 100644
--- a/NetworkPkg/HttpDxe/HttpProto.h
+++ b/NetworkPkg/HttpDxe/HttpProto.h
@@ -1,7 +1,7 @@
/** @file
The header files of miscellaneous routines for HttpDxe driver.

-Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
(C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

@@ -609,4 +609,17 @@ HttpResponseWorker (
IN HTTP_TOKEN_WRAP *Wrap
);

+/**
+ Send Events via EDKII_HTTP_CALLBACK_PROTOCOL.
+
+ @param[in] Event The event that occurs in the current state.
+ @param[in] EventStatus The Status of Event, EFI_SUCCESS or other errors.
+
+**/
+VOID
+HttpNotify (
+ IN EDKII_HTTP_CALLBACK_EVENT Event,
+ IN EFI_STATUS EventStatus
+ );
+
#endif
diff --git a/NetworkPkg/Include/Protocol/HttpCallback.h b/NetworkPkg/Include/Protocol/HttpCallback.h
new file mode 100644
index 0000000000..d036a8a4be
--- /dev/null
+++ b/NetworkPkg/Include/Protocol/HttpCallback.h
@@ -0,0 +1,85 @@
+/** @file
+ This file defines the EDKII HTTP Callback Protocol interface.
+
+ Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef __EDKII_HTTP_CALLBACK_H__
+#define __EDKII_HTTP_CALLBACK_H__
+
+#define EDKII_HTTP_CALLBACK_PROTOCOL_GUID \
+ { \
+ 0x611114f1, 0xa37b, 0x4468, {0xa4, 0x36, 0x5b, 0xdd, 0xa1, 0x6a, 0xa2, 0x40} \
+ }
+
+typedef struct _EDKII_HTTP_CALLBACK_PROTOCOL EDKII_HTTP_CALLBACK_PROTOCOL;
+
+///
+/// EDKII_HTTP_CALLBACK_EVENT
+///
+typedef enum {
+ ///
+ /// The Status of DNS Event to retrieve the host address.
+ /// EventStatus:
+ /// EFI_SUCCESS Operation succeeded.
+ /// EFI_OUT_OF_RESOURCES Failed to allocate needed resources.
+ /// EFI_DEVICE_ERROR An unexpected network error occurred.
+ /// Others Other errors as indicated.
+ ///
+ HttpEventDns,
+
+ ///
+ /// The Status of Event to initiate a nonblocking TCP connection request.
+ /// EventStatus:
+ /// EFI_SUCCESS The connection request is successfully initiated.
+ /// EFI_NOT_STARTED This EFI TCP Protocol instance has not been configured.
+ /// EFI_DEVICE_ERROR An unexpected system or network error occurred.
+ /// Others Other errors as indicated.
+ ///
+ HttpEventConnectTcp,
+
+ ///
+ /// The Status of Event to connect one TLS session by finishing the TLS handshake process.
+ /// EventStatus:
+ /// EFI_SUCCESS The TLS session is established.
+ /// EFI_OUT_OF_RESOURCES Can't allocate memory resources.
+ /// EFI_ABORTED TLS session state is incorrect.
+ /// Others Other error as indicated.
+ ///
+ HttpEventTlsConnectSession,
+
+ ///
+ /// The Status of Event to initialize Http session
+ /// EventStatus:
+ /// EFI_SUCCESS The initialization of session is done.
+ /// Others Other error as indicated.
+ ///
+ HttpEventInitSession
+} EDKII_HTTP_CALLBACK_EVENT;
+
+/**
+ Callback function that is invoked when HTTP event occurs.
+
+ @param[in] This Pointer to the EDKII_HTTP_CALLBACK_PROTOCOL instance.
+ @param[in] Event The event that occurs in the current state.
+ @param[in] EventStatus The Status of Event, EFI_SUCCESS or other errors.
+**/
+typedef
+VOID
+(EFIAPI * EDKII_HTTP_CALLBACK) (
+ IN EDKII_HTTP_CALLBACK_PROTOCOL *This,
+ IN EDKII_HTTP_CALLBACK_EVENT Event,
+ IN EFI_STATUS EventStatus
+ );
+
+///
+/// EFI HTTP Callback Protocol is invoked when HTTP event occurs.
+///
+struct _EDKII_HTTP_CALLBACK_PROTOCOL {
+ EDKII_HTTP_CALLBACK Callback;
+};
+
+extern EFI_GUID gEdkiiHttpCallbackProtocolGuid;
+
+#endif
diff --git a/NetworkPkg/NetworkPkg.dec b/NetworkPkg/NetworkPkg.dec
index b81f10ef6e..0f9f7bb15e 100644
--- a/NetworkPkg/NetworkPkg.dec
+++ b/NetworkPkg/NetworkPkg.dec
@@ -88,6 +88,9 @@
## Include/Protocol/Dpc.h
gEfiDpcProtocolGuid = {0x480f8ae9, 0xc46, 0x4aa9, { 0xbc, 0x89, 0xdb, 0x9f, 0xba, 0x61, 0x98, 0x6 }}

+ ## Include/Protocol/HttpCallback.h
+ gEdkiiHttpCallbackProtocolGuid = {0x611114f1, 0xa37b, 0x4468, {0xa4, 0x36, 0x5b, 0xdd, 0xa1, 0x6a, 0xa2, 0x40}}
+
[PcdsFixedAtBuild]
## The max attempt number will be created by iSCSI driver.
# @Prompt Max attempt number.
--
2.31.1.windows.1


[Patch] BaseTools: use shutil.copyfile instead shutil.copy2

Bob Feng
 

In Split tool, the copy file actions only need to
copy file content but not need to copy file metadata.

copy2() copies the file metadata that causes split
unit test failed under edk2-basetools CI environment.

So this patch changes the call of copy2() to copyfile().

Signed-off-by: Bob Feng <bob.c.feng@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Yuwei Chen <yuwei.chen@intel.com>
---
BaseTools/Source/Python/Split/Split.py | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/Python/Split/Split.py b/BaseTools/Source/Pyth=
on/Split/Split.py
index e223a72a94..e70d5c22c4 100644
--- a/BaseTools/Source/Python/Split/Split.py
+++ b/BaseTools/Source/Python/Split/Split.py
@@ -146,18 +146,18 @@ def splitFile(inputfile, position, outputdir=3DNone, =
outputfile1=3DNone, outputfile2
logger.error("Can't make dir: %s" % outputfolder)=0D
raise(e)=0D
=0D
if position <=3D 0:=0D
if outputfile2 !=3D os.path.abspath(inputfile):=0D
- shutil.copy2(os.path.abspath(inputfile), outputfile2)=0D
+ shutil.copyfile(os.path.abspath(inputfile), outputfile2)=0D
with open(outputfile1, "wb") as fout:=0D
fout.write(b'')=0D
else:=0D
inputfilesize =3D getFileSize(inputfile)=0D
if position >=3D inputfilesize:=0D
if outputfile1 !=3D os.path.abspath(inputfile):=0D
- shutil.copy2(os.path.abspath(inputfile), outputfile1)=0D
+ shutil.copyfile(os.path.abspath(inputfile), outputfile1)=0D
with open(outputfile2, "wb") as fout:=0D
fout.write(b'')=0D
else:=0D
try:=0D
tempdir =3D tempfile.mkdtemp()=0D
@@ -169,12 +169,12 @@ def splitFile(inputfile, position, outputdir=3DNone, =
outputfile1=3DNone, outputfile2
fout1.write(content1)=0D
=0D
content2 =3D fin.read(inputfilesize - position)=0D
with open(tempfile2, "wb") as fout2:=0D
fout2.write(content2)=0D
- shutil.copy2(tempfile1, outputfile1)=0D
- shutil.copy2(tempfile2, outputfile2)=0D
+ shutil.copyfile(tempfile1, outputfile1)=0D
+ shutil.copyfile(tempfile2, outputfile2)=0D
except Exception as e:=0D
logger.error("Split file failed")=0D
raise(e)=0D
finally:=0D
if os.path.exists(tempdir):=0D
--=20
2.29.1.windows.1

4041 - 4060 of 82266