date:20210406

回复: [edk2-devel] [PATCH] RedfishPkg/RedfishConfigHandler: EDKII RedfishConfigHandler Protocol

2021-04-06 Thread gaoliming

Abner: 
  This is a new feature. Can you submit one BZ for it?

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Abner Chang
> 发送时间: 2021年3月26日 13:55
> 收件人: devel@edk2.groups.io
> 抄送: Nickle Wang 
> 主题: [edk2-devel] [PATCH] RedfishPkg/RedfishConfigHandler: EDKII
> RedfishConfigHandler Protocol
> 
> The driver is used to manage EDK2 Redfish Configuration Handler
> Protocol installed by EDK2 Redfish feature drivers.
> This is the EDK2 Redfish client driver written based on the EDK2
> Redfish foundation to initialize EDK2 Redfish feature drivers.
> 
> EDK2 Redfish feature drivers are used to provision/consume/update
> the firmware owns Redfish properties during system power on
> initialization.
> 
> RedfishConfigHandlerCommon.c has the common code for the driver
> instances used in different EDK2 boot phases or used by different
> driver models in the future contribution.
> 
> Signed-off-by: Jiaxin Wu 
> Signed-off-by: Siyuan Fu 
> Signed-off-by: Fan Wang 
> Signed-off-by: Abner Chang 
> 
> Cc: Nickle Wang 
> ---
>  RedfishPkg/RedfishComponents.dsc.inc  |   3 +-
>  RedfishPkg/Redfish.fdf.inc|   3 +-
>  .../RedfishConfigHandlerDriver.inf|  60 ++
>  .../RedfishConfigHandlerCommon.h  | 101 +++
>  .../RedfishConfigHandlerDriver.h  | 159 +
>  .../RedfishConfigHandlerCommon.c  | 265 
>  .../RedfishConfigHandlerDriver.c  | 587
> ++
>  7 files changed, 1176 insertions(+), 2 deletions(-)
>  create mode 100644
> RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
>  create mode 100644
> RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerCommon.h
>  create mode 100644
> RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.h
>  create mode 100644
> RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerCommon.c
>  create mode 100644
> RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.c
> 
> diff --git a/RedfishPkg/RedfishComponents.dsc.inc
> b/RedfishPkg/RedfishComponents.dsc.inc
> index 08f1d3bc32..d0116f065c 100644
> --- a/RedfishPkg/RedfishComponents.dsc.inc
> +++ b/RedfishPkg/RedfishComponents.dsc.inc
> @@ -6,7 +6,7 @@
>  # of EDKII Redfish drivers according to the value of flags described in
>  # "RedfishDefines.dsc.inc".
>  #
> -# (C) Copyright 2020 Hewlett Packard Enterprise Development LP
> +# (C) Copyright 2021 Hewlett Packard Enterprise Development LP
>  #
>  #SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -17,4 +17,5 @@
>RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
>RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
>RedfishPkg/RedfishCredentialDxe/RedfishCredentialDxe.inf
> +  RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
>  !endif
> diff --git a/RedfishPkg/Redfish.fdf.inc b/RedfishPkg/Redfish.fdf.inc
> index a64fd119a9..9673246b3f 100644
> --- a/RedfishPkg/Redfish.fdf.inc
> +++ b/RedfishPkg/Redfish.fdf.inc
> @@ -5,7 +5,7 @@
>  # by using "!include RedfishPkg/RedfisLibs.fdf.inc" to specify the module
> instances
>  # to be built in the firmware volume.
>  #
> -# (C) Copyright 2020 Hewlett Packard Enterprise Development LP
> +# (C) Copyright 2021 Hewlett Packard Enterprise Development LP
>  #
>  #SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -15,4 +15,5 @@
>INF RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
>INF RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
>INF RedfishPkg/RedfishCredentialDxe/RedfishCredentialDxe.inf
> +  INF RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
>  !endif
> diff --git
a/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
> b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
> new file mode 100644
> index 00..def91c7531
> --- /dev/null
> +++ b/RedfishPkg/RedfishConfigHandler/RedfishConfigHandlerDriver.inf
> @@ -0,0 +1,60 @@
> +## @file
> +#  INF file for the UEFI driver model Redfish Configuration Handler
> +#  Driver.
> +#
> +#  Copyright (c) 2019, Intel Corporation. All rights reserved.
> +#  (C) Copyright 2021 Hewlett Packard Enterprise Development LP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION   = 0x0001000b
> +  BASE_NAME = RedfishConfigHandlerDriver
> +  FILE_GUID =
> 6e881000-5749-11e8-9bf0-8cdcd426c973
> +  MODULE_TYPE   = UEFI_DRIVER
> +  VERSION_STRING= 1.0
> +  ENTRY_POINT   = RedfishConfigHandlerDriverEntryPoint
> +  UNLOAD_IMAGE  = RedfishConfigHandlerDriverUnload
> +
> +#
> +#  VALID_ARCHITECTURES  = IA32 X64 ARM AARCH64 RISCV64
> +#
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  NetworkPkg/NetworkPkg.dec
> +  RedfishPkg/RedfishPkg.dec
> +
> +[Sources]
> +  RedfishConfigHandlerDriver.h
> +  RedfishConfigHandlerDriver.c
> +  RedfishConfigHandlerCommon.h
> +  RedfishConfigHandlerCommon.c
>

回复: [edk2-devel] [PATCH] RedfishPkg/RedfishDiscoverDxe: EFI Redfish Discover Protocol

2021-04-06 Thread gaoliming

Abner:
  This is new feature. Can you submit one BZ for it?

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Abner Chang
> 发送时间: 2021年3月26日 12:54
> 收件人: devel@edk2.groups.io
> 抄送: Nickle Wang ; Jiaxin Wu
> ; Siyuan Fu ; Fan Wang
> ; Jiewen Yao 
> 主题: [edk2-devel] [PATCH] RedfishPkg/RedfishDiscoverDxe: EFI Redfish
> Discover Protocol
> 
> EDK2 EFI Redfish Discover Protocol implementation. Refer to UEFI spec 2.9
> section 31.1.
> 
> Signed-off-by: Abner Chang 
> Cc: Nickle Wang 
> Cc: Jiaxin Wu 
> Cc: Siyuan Fu 
> Cc: Fan Wang 
> Cc: Jiewen Yao 
> ---
>  RedfishPkg/RedfishComponents.dsc.inc  |3 +-
>  RedfishPkg/Redfish.fdf.inc|3 +-
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.inf |   55 +
>  .../RedfishDiscoverInternal.h |  234 ++
>  RedfishPkg/RedfishDiscoverDxe/ComponentName.c |  218 ++
>  .../RedfishDiscoverDxe/RedfishDiscoverDxe.c   | 1910
> +
>  .../RedfishSmbiosHostInterface.c  |  118 +
>  7 files changed, 2539 insertions(+), 2 deletions(-)
>  create mode 100644 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.inf
>  create mode 100644
> RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverInternal.h
>  create mode 100644 RedfishPkg/RedfishDiscoverDxe/ComponentName.c
>  create mode 100644 RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.c
>  create mode 100644
> RedfishPkg/RedfishDiscoverDxe/RedfishSmbiosHostInterface.c
> 
> diff --git a/RedfishPkg/RedfishComponents.dsc.inc
> b/RedfishPkg/RedfishComponents.dsc.inc
> index 08f1d3bc32..6f3b055aba 100644
> --- a/RedfishPkg/RedfishComponents.dsc.inc
> +++ b/RedfishPkg/RedfishComponents.dsc.inc
> @@ -6,7 +6,7 @@
>  # of EDKII Redfish drivers according to the value of flags described in
>  # "RedfishDefines.dsc.inc".
>  #
> -# (C) Copyright 2020 Hewlett Packard Enterprise Development LP
> +# (C) Copyright 2021 Hewlett Packard Enterprise Development LP
>  #
>  #SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -17,4 +17,5 @@
>RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
>RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
>RedfishPkg/RedfishCredentialDxe/RedfishCredentialDxe.inf
> +  RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.inf
>  !endif
> diff --git a/RedfishPkg/Redfish.fdf.inc b/RedfishPkg/Redfish.fdf.inc
> index a64fd119a9..205c3101c0 100644
> --- a/RedfishPkg/Redfish.fdf.inc
> +++ b/RedfishPkg/Redfish.fdf.inc
> @@ -5,7 +5,7 @@
>  # by using "!include RedfishPkg/RedfisLibs.fdf.inc" to specify the module
> instances
>  # to be built in the firmware volume.
>  #
> -# (C) Copyright 2020 Hewlett Packard Enterprise Development LP
> +# (C) Copyright 2021 Hewlett Packard Enterprise Development LP
>  #
>  #SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -15,4 +15,5 @@
>INF RedfishPkg/RedfishHostInterfaceDxe/RedfishHostInterfaceDxe.inf
>INF RedfishPkg/RedfishRestExDxe/RedfishRestExDxe.inf
>INF RedfishPkg/RedfishCredentialDxe/RedfishCredentialDxe.inf
> +  INF RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.inf
>  !endif
> diff --git a/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.inf
> b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.inf
> new file mode 100644
> index 00..345bacf44d
> --- /dev/null
> +++ b/RedfishPkg/RedfishDiscoverDxe/RedfishDiscoverDxe.inf
> @@ -0,0 +1,55 @@
> +## @file
> +#  Implementation of EFI_REDFISH_DISCOVER_PROTOCOL interfaces.
> +#
> +#  (C) Copyright 2021 Hewlett Packard Enterprise Development LP
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION   = 0x0001001b
> +  BASE_NAME = RedfishDiscoverDxe
> +  FILE_GUID =
> 28A76FE5-43D7-48A3-9714-C1B7BDD6DFB6
> +  MODULE_TYPE   = UEFI_DRIVER
> +  VERSION_STRING= 1.0
> +  ENTRY_POINT   = RedfishDiscoverEntryPoint
> +  UNLOAD_IMAGE  = RedfishDiscoverUnload
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  NetworkPkg/NetworkPkg.dec
> +  RedfishPkg/RedfishPkg.dec
> +
> +[Sources]
> +  ComponentName.c
> +  RedfishDiscoverDxe.c
> +  RedfishSmbiosHostInterface.c
> +  RedfishDiscoverInternal.h
> +
> +[LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
> +  DebugLib
> +  MemoryAllocationLib
> +  PrintLib
> +  RestExLib
> +  UefiLib
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +
> +[Protocols]
> +  gEfiRestExServiceBindingProtocolGuid## Consuming
> +  gEfiRestExProtocolGuid  ## Consuming
> +  gEfiTcp4ServiceBindingProtocolGuid  ## Consuming
> +  gEfiTcp4ProtocolGuid## Consuming
> +  gEfiTcp6ServiceBindingProtocolGuid  ## Consuming
> +  gEfiTcp6ProtocolGuid## Consuming
> +  gEfiRedfishDiscoverProtocolGuid ## Prodcuing
> +  gEfiSmbiosProtocolGuid  ## Consuming
> +  gEfiDriverBindingProtocolGuid

Re: [edk2-devel] [edk2-platforms] [PATCH v4 0/4] Add Large Variable Libraries

2021-04-06 Thread Chiu, Chasel



Change looks good to me, just one comment, would you also add new library class 
into MinPlatformPkg.dec file for reference?

Thanks,
Chasel


> -Original Message-
> From: Desimone, Nathaniel L 
> Sent: Wednesday, April 7, 2021 11:04 AM
> To: devel@edk2.groups.io
> Cc: Chiu, Chasel ; Liming Gao
> ; Dong, Eric ; Michael
> Kubacki ; Oram, Isaac W
> 
> Subject: [edk2-platforms] [PATCH v4 0/4] Add Large Variable Libraries
> 
> Changes from V3:
>  - Added header guards
>  - Documented the possibility of returning EFI_UNSUPPORTED
>  - Added MM_CORE_STANDALONE to the StandaloneMm library
>implementations
>  - Documented library constructor return status
>  - Moved LargeVariableReadLib and LargeVariableWriteLib into
>a single BaseLargeVariableLib folder
>  - Added LargeVariableCommon.h for shared macro definitions
>  - Converted some debug macros from DEBUG_INFO to DEBUG_VERBOSE
> 
> Changes from V2:
>  - Added comment to LargeVariableLib INF and header files
>describing the usage for drivers that cannot assume that
>PcdMaxVariableSize has been set to a certain minimum value.
> 
> Changes from V1:
>  - Changed prefix from "Min" to "VarLib"
>  - Better comments
>  - Added more whitespace for readability
>  - Removed unused INF sections
>  - Better debug messages
> 
> This patch series introduces libaries that enable large data sets to be stored
> using the UEFI Variable Services. At present, most UEFI Variable Services
> implementations have a maximum variable size of <=64KB. The exact value
> varies depending on platform.
> 
> These libaries enable a data set to use as much space as needed, up to the
> remaining space in the UEFI Variable non-volatile storage.
> 
> To implement this, I have broken the problem down into two parts:
> 
>  1. Phase angostic UEFI Variable access.
>  2. Storage of data across multiple UEFI Variables.
> 
> For the first part, I have created two new LibraryClasses:
> VariableReadLib and VariableWriteLib. I have provided implementation instances
> of VariableReadLib for PEI, DXE, and SMM.
> For VariableWriteLib, I have provided implementation instances for DXE and
> SMM. This enables code that accesses UEFI variables to be written in a matter
> than is phase agnostic, so the same code can be used in PEI, DXE, or SMM
> without modification.
> 
> The second part involves another two new LibaryClasses:
> LargeVariableReadLib and LargeVariableWriteLib. Only one BASE
> implementation is needed for both of these as the phase dependent code was
> seperated out in the first piece. These libraries provide logic to calculate 
> the
> maximum size of an individual UEFI variable and split the data into as many
> smaller pieces as needed to store the entire data set in the UEFI Variable 
> storage.
> They also provide the ability to stitch the data back together when it is 
> read.
> Deleting the data will delete all variables used to store it.
> 
> Cc: Chasel Chiu 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Michael Kubacki 
> Cc: Isaac Oram 
> Signed-off-by: Nate DeSimone 
> 
> Nate DeSimone (4):
>   MinPlatformPkg: Add VariableReadLib
>   MinPlatformPkg: Add VariableWriteLib
>   MinPlatformPkg: Add LargeVariableReadLib
>   MinPlatformPkg: Add LargeVariableWriteLib
> 
>  .../Include/Dsc/CoreCommonLib.dsc |   6 +-
>  .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  12 +-
>  .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
>  .../Include/Library/LargeVariableReadLib.h|  61 +++
>  .../Include/Library/LargeVariableWriteLib.h   |  69 +++
>  .../Include/Library/VariableReadLib.h |  94 
>  .../Include/Library/VariableWriteLib.h| 138 ++
>  .../BaseLargeVariableReadLib.inf  |  51 ++
>  .../BaseLargeVariableWriteLib.inf |  51 ++
>  .../LargeVariableCommon.h |  47 ++
>  .../LargeVariableReadLib.c| 176 +++
>  .../LargeVariableWriteLib.c   | 450 ++
>  .../DxeRuntimeVariableReadLib.c   | 117 +
>  .../DxeRuntimeVariableReadLib.inf |  41 ++
>  .../DxeRuntimeVariableWriteLib.c  | 265 +++
>  .../DxeRuntimeVariableWriteLib.inf|  49 ++
>  .../PeiVariableReadLib/PeiVariableReadLib.c   | 155 ++
>  .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 ++
>  .../SmmVariableReadCommon.c   | 116 +
>  .../StandaloneMmVariableReadLib.inf   |  50 ++
>  .../StandaloneMmVariableReadLibConstructor.c  |  51 ++
>  .../TraditionalMmVariableReadLib.inf  |  49 ++
>  .../TraditionalMmVariableReadLibConstructor.c |  51 ++
>  .../SmmVariableWriteCommon.c  | 171 +++
>  .../StandaloneMmVariableWriteLib.inf  |  45 ++
>  .../StandaloneMmVariableWriteLibConstructor.c |  51 ++
>  .../TraditionalMmVariableWriteLib.inf |  44 ++
>  ...TraditionalMmVariableWriteLibConstructor.c |  51 ++
>  .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   4 +-
>  29

Re: [edk2-devel] [edk2-platforms] [PATCH v4 0/4] Add Large Variable Libraries

2021-04-06 Thread Michael Kubacki


For the series:
Reviewed-by: Michael Kubacki 

On 4/6/2021 8:04 PM, Nate DeSimone wrote:

Changes from V3:
  - Added header guards
  - Documented the possibility of returning EFI_UNSUPPORTED
  - Added MM_CORE_STANDALONE to the StandaloneMm library
implementations
  - Documented library constructor return status
  - Moved LargeVariableReadLib and LargeVariableWriteLib into
a single BaseLargeVariableLib folder
  - Added LargeVariableCommon.h for shared macro definitions
  - Converted some debug macros from DEBUG_INFO to DEBUG_VERBOSE

Changes from V2:
  - Added comment to LargeVariableLib INF and header files
describing the usage for drivers that cannot assume that
PcdMaxVariableSize has been set to a certain minimum value.

Changes from V1:
  - Changed prefix from "Min" to "VarLib"
  - Better comments
  - Added more whitespace for readability
  - Removed unused INF sections
  - Better debug messages

This patch series introduces libaries that enable large data sets
to be stored using the UEFI Variable Services. At present, most
UEFI Variable Services implementations have a maximum variable
size of <=64KB. The exact value varies depending on platform.

These libaries enable a data set to use as much space as needed,
up to the remaining space in the UEFI Variable non-volatile storage.

To implement this, I have broken the problem down into two parts:

  1. Phase angostic UEFI Variable access.
  2. Storage of data across multiple UEFI Variables.

For the first part, I have created two new LibraryClasses:
VariableReadLib and VariableWriteLib. I have provided
implementation instances of VariableReadLib for PEI, DXE, and SMM.
For VariableWriteLib, I have provided implementation instances for
DXE and SMM. This enables code that accesses UEFI variables
to be written in a matter than is phase agnostic, so the same
code can be used in PEI, DXE, or SMM without modification.

The second part involves another two new LibaryClasses:
LargeVariableReadLib and LargeVariableWriteLib. Only one BASE
implementation is needed for both of these as the phase dependent
code was seperated out in the first piece. These libraries provide
logic to calculate the maximum size of an individual UEFI variable
and split the data into as many smaller pieces as needed to store
the entire data set in the UEFI Variable storage. They also provide
the ability to stitch the data back together when it is read.
Deleting the data will delete all variables used to store it.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 

Nate DeSimone (4):
   MinPlatformPkg: Add VariableReadLib
   MinPlatformPkg: Add VariableWriteLib
   MinPlatformPkg: Add LargeVariableReadLib
   MinPlatformPkg: Add LargeVariableWriteLib

  .../Include/Dsc/CoreCommonLib.dsc |   6 +-
  .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  12 +-
  .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
  .../Include/Library/LargeVariableReadLib.h|  61 +++
  .../Include/Library/LargeVariableWriteLib.h   |  69 +++
  .../Include/Library/VariableReadLib.h |  94 
  .../Include/Library/VariableWriteLib.h| 138 ++
  .../BaseLargeVariableReadLib.inf  |  51 ++
  .../BaseLargeVariableWriteLib.inf |  51 ++
  .../LargeVariableCommon.h |  47 ++
  .../LargeVariableReadLib.c| 176 +++
  .../LargeVariableWriteLib.c   | 450 ++
  .../DxeRuntimeVariableReadLib.c   | 117 +
  .../DxeRuntimeVariableReadLib.inf |  41 ++
  .../DxeRuntimeVariableWriteLib.c  | 265 +++
  .../DxeRuntimeVariableWriteLib.inf|  49 ++
  .../PeiVariableReadLib/PeiVariableReadLib.c   | 155 ++
  .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 ++
  .../SmmVariableReadCommon.c   | 116 +
  .../StandaloneMmVariableReadLib.inf   |  50 ++
  .../StandaloneMmVariableReadLibConstructor.c  |  51 ++
  .../TraditionalMmVariableReadLib.inf  |  49 ++
  .../TraditionalMmVariableReadLibConstructor.c |  51 ++
  .../SmmVariableWriteCommon.c  | 171 +++
  .../StandaloneMmVariableWriteLib.inf  |  45 ++
  .../StandaloneMmVariableWriteLibConstructor.c |  51 ++
  .../TraditionalMmVariableWriteLib.inf |  44 ++
  ...TraditionalMmVariableWriteLibConstructor.c |  51 ++
  .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   4 +-
  29 files changed, 2506 insertions(+), 10 deletions(-)
  create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
  create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
  create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
  create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
  create mode 100644

[edk2-devel] [edk2-platforms] [PATCH v4 0/4] Add Large Variable Libraries

2021-04-06 Thread Nate DeSimone

Changes from V3:
 - Added header guards
 - Documented the possibility of returning EFI_UNSUPPORTED
 - Added MM_CORE_STANDALONE to the StandaloneMm library
   implementations
 - Documented library constructor return status
 - Moved LargeVariableReadLib and LargeVariableWriteLib into
   a single BaseLargeVariableLib folder
 - Added LargeVariableCommon.h for shared macro definitions
 - Converted some debug macros from DEBUG_INFO to DEBUG_VERBOSE

Changes from V2:
 - Added comment to LargeVariableLib INF and header files
   describing the usage for drivers that cannot assume that
   PcdMaxVariableSize has been set to a certain minimum value.

Changes from V1:
 - Changed prefix from "Min" to "VarLib"
 - Better comments
 - Added more whitespace for readability
 - Removed unused INF sections
 - Better debug messages

This patch series introduces libaries that enable large data sets
to be stored using the UEFI Variable Services. At present, most
UEFI Variable Services implementations have a maximum variable
size of <=64KB. The exact value varies depending on platform.

These libaries enable a data set to use as much space as needed,
up to the remaining space in the UEFI Variable non-volatile storage.

To implement this, I have broken the problem down into two parts:

 1. Phase angostic UEFI Variable access.
 2. Storage of data across multiple UEFI Variables.

For the first part, I have created two new LibraryClasses:
VariableReadLib and VariableWriteLib. I have provided
implementation instances of VariableReadLib for PEI, DXE, and SMM.
For VariableWriteLib, I have provided implementation instances for
DXE and SMM. This enables code that accesses UEFI variables
to be written in a matter than is phase agnostic, so the same
code can be used in PEI, DXE, or SMM without modification.

The second part involves another two new LibaryClasses:
LargeVariableReadLib and LargeVariableWriteLib. Only one BASE
implementation is needed for both of these as the phase dependent
code was seperated out in the first piece. These libraries provide
logic to calculate the maximum size of an individual UEFI variable
and split the data into as many smaller pieces as needed to store
the entire data set in the UEFI Variable storage. They also provide
the ability to stitch the data back together when it is read.
Deleting the data will delete all variables used to store it.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 

Nate DeSimone (4):
  MinPlatformPkg: Add VariableReadLib
  MinPlatformPkg: Add VariableWriteLib
  MinPlatformPkg: Add LargeVariableReadLib
  MinPlatformPkg: Add LargeVariableWriteLib

 .../Include/Dsc/CoreCommonLib.dsc |   6 +-
 .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  12 +-
 .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
 .../Include/Library/LargeVariableReadLib.h|  61 +++
 .../Include/Library/LargeVariableWriteLib.h   |  69 +++
 .../Include/Library/VariableReadLib.h |  94 
 .../Include/Library/VariableWriteLib.h| 138 ++
 .../BaseLargeVariableReadLib.inf  |  51 ++
 .../BaseLargeVariableWriteLib.inf |  51 ++
 .../LargeVariableCommon.h |  47 ++
 .../LargeVariableReadLib.c| 176 +++
 .../LargeVariableWriteLib.c   | 450 ++
 .../DxeRuntimeVariableReadLib.c   | 117 +
 .../DxeRuntimeVariableReadLib.inf |  41 ++
 .../DxeRuntimeVariableWriteLib.c  | 265 +++
 .../DxeRuntimeVariableWriteLib.inf|  49 ++
 .../PeiVariableReadLib/PeiVariableReadLib.c   | 155 ++
 .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 ++
 .../SmmVariableReadCommon.c   | 116 +
 .../StandaloneMmVariableReadLib.inf   |  50 ++
 .../StandaloneMmVariableReadLibConstructor.c  |  51 ++
 .../TraditionalMmVariableReadLib.inf  |  49 ++
 .../TraditionalMmVariableReadLibConstructor.c |  51 ++
 .../SmmVariableWriteCommon.c  | 171 +++
 .../StandaloneMmVariableWriteLib.inf  |  45 ++
 .../StandaloneMmVariableWriteLibConstructor.c |  51 ++
 .../TraditionalMmVariableWriteLib.inf |  44 ++
 ...TraditionalMmVariableWriteLibConstructor.c |  51 ++
 .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   4 +-
 29 files changed, 2506 insertions(+), 10 deletions(-)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf
 create mode

[edk2-devel] [edk2-platforms] [PATCH v4 2/4] MinPlatformPkg: Add VariableWriteLib

2021-04-06 Thread Nate DeSimone

VariableWriteLib is a phase agnostic library for writing
to UEFI Variables. This library provides the MinSetVariable(),
MinQueryVariableInfo(), MinIsVariableRequestToLockSupported(),
and MinVariableRequestToLock() APIs which are usable in DXE
and SMM.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |   2 +
 .../Include/Library/VariableWriteLib.h| 138 +
 .../DxeRuntimeVariableWriteLib.c  | 265 ++
 .../DxeRuntimeVariableWriteLib.inf|  49 
 .../SmmVariableWriteCommon.c  | 171 +++
 .../StandaloneMmVariableWriteLib.inf  |  45 +++
 .../StandaloneMmVariableWriteLibConstructor.c |  51 
 .../TraditionalMmVariableWriteLib.inf |  44 +++
 ...TraditionalMmVariableWriteLibConstructor.c |  51 
 .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   1 +
 10 files changed, 817 insertions(+)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index 0db1250ab7..57847e6a4d 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -48,6 +48,7 @@
 
 [LibraryClasses.common.DXE_CORE, LibraryClasses.common.DXE_DRIVER, 
LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER, 
LibraryClasses.common.UEFI_APPLICATION]
   
VariableReadLib|MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
+  
VariableWriteLib|MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -93,6 +94,7 @@
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
   
VariableReadLib|MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
+  
VariableWriteLib|MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
 
 [LibraryClasses.common.SMM_CORE]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
new file mode 100644
index 00..fab87f2e48
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
@@ -0,0 +1,138 @@
+/** @file
+  Variable Write Lib
+
+  This library provides phase agnostic access to the UEFI Variable Services.
+  This is done by implementing a wrapper on top of the phase specific mechanism
+  for writing to UEFI variables. For example, the DXE implementation accesses
+  the UEFI Runtime Services Table, and the SMM implementation uses
+  EFI_SMM_VARIABLE_PROTOCOL.
+
+  Using this library allows code to be written in a generic manner that can be
+  used in DXE or SMM without modification.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _VARIABLE_WRITE_LIB_H_
+#define _VARIABLE_WRITE_LIB_H_
+
+#include 
+
+/**
+  Sets the value of a variable.
+
+  @param[in]  VariableName   A Null-terminated string that is the name of 
the vendor's variable.
+ Each VariableName is unique for each 
VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName 
is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+  @param[in]  VendorGuid A unique identifier for the vendor.
+  @param[in]  Attributes Attributes bitmask to set for the variable.
+  @param[in]  DataSize   The size in bytes of the Data buffer. Unless 
the EFI_VARIABLE_APPEND_WRITE or
+ 
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute is set, a size of 
zero
+

[edk2-devel] [edk2-platforms] [PATCH v4 4/4] MinPlatformPkg: Add LargeVariableWriteLib

2021-04-06 Thread Nate DeSimone

LargeVariableWriteLib is used to store large data sets using
the UEFI Variable Services. At time of writting, most UEFI
Variable Services implementations to not allow more than 64KB
of data to be stored in a single UEFI variable. This library
will split data sets across multiple variables as needed.

It adds the SetLargeVariable() API to provide this service.

The primary use for this library is to create binary compatible
drivers and OpROMs which need to work both with TianoCore and
other UEFI PI implementations. When customizing and recompiling
the platform firmware image is possible, adjusting the value of
PcdMaxVariableSize may provide a simpler solution to this
problem.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../Include/Dsc/CoreCommonLib.dsc |   1 +
 .../Include/Library/LargeVariableWriteLib.h   |  69 +++
 .../BaseLargeVariableWriteLib.inf |  51 ++
 .../LargeVariableWriteLib.c   | 450 ++
 4 files changed, 571 insertions(+)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableWriteLib.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index 9bc5ff1e3d..5c1d2cf4aa 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -143,6 +143,7 @@
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
   
LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableReadLib.inf
+  
LargeVariableWriteLib|MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.inf
 
   #
   # CryptLib
diff --git 
a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
new file mode 100644
index 00..c847d7f152
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
@@ -0,0 +1,69 @@
+/** @file
+  Large Variable Write Lib
+
+  This library is used to store large data sets using the UEFI Variable 
Services.
+  At time of writting, most UEFI Variable Services implementations do not allow
+  more than 64KB of data to be stored in a single UEFI variable. This library
+  will split data sets across multiple variables as needed.
+
+  In the case where more than one variable is needed to store the data, an
+  integer number will be added to the end of the variable name. This number
+  will be incremented for each variable as needed to store the entire data set.
+
+  The primary use for this library is to create binary compatible drivers
+  and OpROMs which need to work both with TianoCore and other UEFI PI
+  implementations. When customizing and recompiling the platform firmware image
+  is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
+  solution to this problem.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _LARGE_VARIABLE_WRITE_LIB_H_
+#define _LARGE_VARIABLE_WRITE_LIB_H_
+
+#include 
+
+/**
+  Sets the value of a large variable.
+
+  @param[in]  VariableName   A Null-terminated string that is the name of 
the vendor's variable.
+ Each VariableName is unique for each 
VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName 
is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+  @param[in]  VendorGuid A unique identifier for the vendor.
+  @param[in]  LockVariable   If TRUE, any further writes to the variable 
will be prevented until the next reset.
+ Note: LockVariable must be FALSE when running 
in SMM or after ExitBootServices.
+  @param[in]  DataSize   The size in bytes of the Data buffer. A size 
of zero causes the variable to be deleted.
+ If DataSize is zero, then LockVariable must 
be FALSE since a variable that does not
+ exist cannot be locked.
+  @param[in]  Data   The contents for the variable.
+
+  @retval EFI_SUCCESSThe firmware has successfully stored the 
variable and its data as
+ defined by the Attributes.
+  @retval EFI_INVALID_PARAMETER  An invalid combination of LockVariable, name, 
and GUID was supplied, or the
+ DataSize exceeds the maximum allowed.
+  @retval

[edk2-devel] [edk2-platforms] [PATCH v4 1/4] MinPlatformPkg: Add VariableReadLib

2021-04-06 Thread Nate DeSimone

VariableReadLib is a phase agnostic libary for reading UEFI
Variables. This library provides the
MinGetVariable() and MinGetNextVariableName() APIs which
are usable PEI, DXE, and SMM.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  10 +-
 .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
 .../Include/Library/VariableReadLib.h |  94 +++
 .../DxeRuntimeVariableReadLib.c   | 117 +
 .../DxeRuntimeVariableReadLib.inf |  41 +
 .../PeiVariableReadLib/PeiVariableReadLib.c   | 155 ++
 .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 +
 .../SmmVariableReadCommon.c   | 116 +
 .../StandaloneMmVariableReadLib.inf   |  50 ++
 .../StandaloneMmVariableReadLibConstructor.c  |  51 ++
 .../TraditionalMmVariableReadLib.inf  |  49 ++
 .../TraditionalMmVariableReadLibConstructor.c |  51 ++
 .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   3 +-
 13 files changed, 780 insertions(+), 8 deletions(-)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/SmmVariableReadCommon.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLibConstructor.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLibConstructor.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index fa9098d525..0db1250ab7 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  Platform description.
 #
-# Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -11,7 +11,7 @@
   #
   # Generic EDKII Lib
   #
-  
+
   #
   # DXE phase common
   #
@@ -23,7 +23,7 @@
   
ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
 
   HstiLib|MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
-  
+
   LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
 
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
@@ -46,6 +46,9 @@
 
   VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
 
+[LibraryClasses.common.DXE_CORE, LibraryClasses.common.DXE_DRIVER, 
LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER, 
LibraryClasses.common.UEFI_APPLICATION]
+  
VariableReadLib|MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
+
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
   
MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
@@ -89,6 +92,7 @@
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+  
VariableReadLib|MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
 
 [LibraryClasses.common.SMM_CORE]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
index 2bcaed05a1..d64873ac6d 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  Platform description.
 #
-# Copyright (c) 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -10,11 +10,11 @@
   #
   # Generic EDKII Lib
   #
-  
+
   #
   # PEI phase common
   #
-  
+
 
[LibraryClasses.common.SEC,LibraryClasses.common.PEI_CORE,LibraryClasses.common.PEIM]

[edk2-devel] [edk2-platforms] [PATCH v4 3/4] MinPlatformPkg: Add LargeVariableReadLib

2021-04-06 Thread Nate DeSimone

LargeVariableReadLib is used to retrieve large data sets using
the UEFI Variable Services. At time of writting, most UEFI
Variable Services implementations to not allow more than 64KB
of data to be stored in a single UEFI variable. This library
will split data sets across multiple variables as needed.

It adds the GetLargeVariable() API to provide this service.

The primary use for this library is to create binary compatible
drivers and OpROMs which need to work both with TianoCore and
other UEFI PI implementations. When customizing and recompiling
the platform firmware image is possible, adjusting the value of
PcdMaxVariableSize may provide a simpler solution to this
problem.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../Include/Dsc/CoreCommonLib.dsc |   5 +-
 .../Include/Library/LargeVariableReadLib.h|  61 ++
 .../BaseLargeVariableReadLib.inf  |  51 +
 .../LargeVariableCommon.h |  47 +
 .../LargeVariableReadLib.c| 176 ++
 5 files changed, 338 insertions(+), 2 deletions(-)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableCommon.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableLib/LargeVariableReadLib.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index cf2940cf02..9bc5ff1e3d 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -135,13 +135,14 @@
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   
PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
-  
+
 !if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
 !endif
 
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
+  
LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableReadLib.inf
 
   #
   # CryptLib
@@ -165,4 +166,4 @@
 
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
-  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
\ No newline at end of file
+  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
diff --git 
a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
new file mode 100644
index 00..df5020b95f
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
@@ -0,0 +1,61 @@
+/** @file
+  Large Variable Read Lib
+
+  This library is used to retrieve large data sets using the UEFI Variable
+  Services. At time of writing, most UEFI Variable Services implementations do
+  not allow more than 64KB of data to be stored in a single UEFI variable. This
+  library will split data sets across multiple variables as needed.
+
+  In the case where more than one variable is needed to store the data, an
+  integer number will be added to the end of the variable name. This number
+  will be incremented for each variable as needed to retrieve the entire data
+  set.
+
+  The primary use for this library is to create binary compatible drivers
+  and OpROMs which need to work both with TianoCore and other UEFI PI
+  implementations. When customizing and recompiling the platform firmware image
+  is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
+  solution to this problem.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _LARGE_VARIABLE_READ_LIB_H_
+#define _LARGE_VARIABLE_READ_LIB_H_
+
+#include 
+
+/**
+  Returns the value of a large variable.
+
+  @param[in]   VariableName  A Null-terminated string that is the name of 
the vendor's
+ variable.
+  @param[in]   VendorGuidA unique identifier for the vendor.
+  @param[in, out]  DataSize  On input, the size in bytes of the return 
Data buffer.
+ On output the size of data returned in Data.
+  @param[out]  Data  The buffer to return the contents of the 
variable. May be NULL
+ with a zero

Re: [edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

2021-04-06 Thread Nate DeSimone

Hi Michael,

Thank you for the great feedback! I believe I have all of it addressed in V4.

Thanks,
Nate

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael Kubacki
Sent: Tuesday, April 6, 2021 3:01 PM
To: devel@edk2.groups.io; Desimone, Nathaniel L 
Cc: Chiu, Chasel ; Liming Gao 
; Dong, Eric ; Michael Kubacki 
; Oram, Isaac W 
Subject: Re: [edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add 
LargeVariableReadLib

Hi Nate,

Feedback is inline.

Please carry over applicable feedback to LargeVariableWriteLib, I did not 
duplicate the response there.

Thanks,
Michael

On 4/6/2021 12:24 PM, Nate DeSimone wrote:
> LargeVariableReadLib is used to retrieve large data sets using the 
> UEFI Variable Services. At time of writting, most UEFI Variable 
> Services implementations to not allow more than 64KB of data to be 
> stored in a single UEFI variable. This library will split data sets 
> across multiple variables as needed.
> 
> It adds the GetLargeVariable() API to provide this service.
> 
> The primary use for this library is to create binary compatible 
> drivers and OpROMs which need to work both with TianoCore and other 
> UEFI PI implementations. When customizing and recompiling the platform 
> firmware image is possible, adjusting the value of PcdMaxVariableSize 
> may provide a simpler solution to this problem.
> 
> Cc: Chasel Chiu 
> Cc: Liming Gao 
> Cc: Eric Dong 
> Cc: Michael Kubacki 
> Cc: Isaac Oram 
> Signed-off-by: Nate DeSimone 
> Reviewed-by: Isaac Oram 
> ---
>   .../Include/Dsc/CoreCommonLib.dsc |   5 +-
>   .../Include/Library/LargeVariableReadLib.h|  56 +
>   .../BaseLargeVariableReadLib.inf  |  50 +
>   .../LargeVariableReadLib.c| 199 ++
>   4 files changed, 308 insertions(+), 2 deletions(-)
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
>   create mode 100644 
> Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVa
> riableReadLib.c
> 
> diff --git 
> a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc 
> b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> index cf2940cf02..5f2ad3f0f0 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
> @@ -135,13 +135,14 @@
> VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> 
> PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> 
> AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableL
> ibNull.inf
> -
> +
>   !if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
> AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
>   !endif
>   
> SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> 
> BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib
> .inf
> +  
> + LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableReadLib
> + /BaseLargeVariableReadLib.inf
>   
> #
> # CryptLib
> @@ -165,4 +166,4 @@
>   
> SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
> 
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolic
> yLib.inf
> -  
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V
> ariablePolicyHelperLib.inf
> \ No newline at end of file
> +  
> + VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib
> + /VariablePolicyHelperLib.inf
> diff --git 
> a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h 
> b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
> new file mode 100644
> index 00..5579492727
> --- /dev/null
> +++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadL
> +++ ib.h
> @@ -0,0 +1,56 @@
> +/** @file
> +  Large Variable Read Lib
> +
> +  This library is used to retrieve large data sets using the UEFI 
> + Variable  Services. At time of writing, most UEFI Variable Services 
> + implementations to  not allow more than 64KB of data to be stored in 
> + a single UEFI variable. This  library will split data sets across multiple 
> variables as needed.
> +
> +  In the case where more than one variable is needed to store the 
> + data, an  integer number will be added to the end of the variable 
> + name. This number  will be incremented for each variable as needed 
> + to retrieve the entire data  set.
> +
> +  The primary use for this library is to create binary compatible 
> + drivers  and OpROMs which need to work both with TianoCore and other 
> + UEFI PI  implementations. When customizing and recompiling the 
> + platform firmware image  is possible, adjusting the value of 
> + PcdMaxVariableSize may provide a simpler  solution to this problem.
> +
> +

Re: [edk2-devel] [PATCH 0/4] UefiCpuPkg: Add MicrocodeLib for loading microcode

2021-04-06 Thread Ni, Ray

On Tue, Apr  6, 2021 at 08:03 PM, Laszlo Ersek wrote:

>
> (1) I think we should use a new TianoCore feature request BZ for this
> feature, and the commit messages should link it. (I understand the
> library only factors out existent logic, but still.)

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

> 
> (2) As I understand it, a platform can provide microcode in three ways:
> - via the "microcode patch" GUIDed HOB (PEI and DXE phases both),
> - via the "shadow microcode" PPI (PEI phase only),
> - via the PcdCpuMicrocodePatch* PCDs (PEI and DXE phases both).
> 
> If a platform uses none of these methods (for example, OVMF does not),
> then I think it would benefit from a Null instance of the new
> MicrocodeLib class.
> 
> Would you consider introducing a Null instance too, and using that one
> in the OVMF DSC files?
>

No. I don't think it's necessary for a NULL instance.
Because:
1. MicrocodePatch GUIDed HOB is only produced when "shadow microcode" PPI or 
"PcdCpuMicrocodePatch *" exists.
I will further simplify today's MpInitLib to skip loading microcode when 
this HOB exists (because DXE re-load is unnecessary).
This is captured by https://bugzilla.tianocore.org/show_bug.cgi?id=3155.

2. Today's logic only calls the MicrocodeLib API when "shadow microcode" PPI or 
"PcdCpuMicrocodePatch *" exists.
Even NULL instance is provided, it's not called.

3. MicrocodeLib calls ASSERT() when the supplied microcode binary is NULL.
If the logic in MpInitLib is changed by accident to allow the call to 
MicrocodeLib even in OVMF, the assertion can catch this.

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73753): https://edk2.groups.io/g/devel/message/73753
Mute This Topic: https://groups.io/mt/81796730/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] UEFI spec version in system table, shall we update it to 2.8?

2021-04-06 Thread gaoliming

Nhi:
  So far, I don’t receive edk2 feature planning on the latest UEFI 2.8 & 2.
9 spec in edk2 community except for Redfish. 

  So, I suggest to keep current UEFI version 2.7 in system table.

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Nhi Pham via
> groups.io
> 发送时间: 2021年4月6日 20:13
> 收件人: devel@edk2.groups.io; derek.l...@hpe.com;
> gaolim...@byosoft.com.cn
> 主题: Re: [edk2-devel] UEFI spec version in system table, shall we update
it
> to 2.8?
> 
> + Liming Gao
> 
> I'm also interested in the plan for the edk2 in compliance with UEFI 2.8
> specification.
> 
> -Nhi
> 
> 
> From: devel@edk2.groups.io  on behalf of Lin, Derek
> (HPS SW) via groups.io 
> Sent: Wednesday, March 31, 2021 12:52 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] UEFI spec version in system table, shall we update
it to
> 2.8?
> 
> Hi,
> 
> In UefiSpec.h, we define the UEFI version 2.7 in system table.
> 
> 
> #define EFI_2_70_SYSTEM_TABLE_REVISION  ((2 << 16) | (70))
> #define EFI_SYSTEM_TABLE_REVISION
> EFI_2_70_SYSTEM_TABLE_REVISION
> #define EFI_SPECIFICATION_VERSION   EFI_SYSTEM_TABLE_REVISION
> #define EFI_RUNTIME_SERVICES_REVISION
> EFI_SPECIFICATION_VERSION
> 
> In Linux dmesg
> [0.00] efi: EFI v2.70 by EDK II
> 
> Shall it be updated to v2.8 to align with UEFI spec?
> 
> Thanks,
> Derek
> 
> 
> 
> 
> 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73752): https://edk2.groups.io/g/devel/message/73752
Mute This Topic: https://groups.io/mt/81906348/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

回复: [edk2-devel] [Patch 1/1] UnitTestFrameworkPkg: Use TianoCore mirror of cmocka repository

2021-04-06 Thread gaoliming

Mike:
  I agree this change. Reviewed-by: Liming Gao 

Thanks
Liming
> -邮件原件-
> 发件人: devel@edk2.groups.io  代表 Michael D
> Kinney
> 发送时间: 2021年4月7日 4:12
> 收件人: devel@edk2.groups.io
> 抄送: Sean Brogan ; Bret Barkelew
> ; Liming Gao ;
> Andrew Fish ; Laszlo Ersek ; Leif
> Lindholm 
> 主题: [edk2-devel] [Patch 1/1] UnitTestFrameworkPkg: Use TianoCore mirror
> of cmocka repository
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3301
> 
> The cmocka repository https://git.cryptomilk.org/projects/cmocka.git
> has gone down a few times in past year.  When it is down, it blocks
> EDK II CI.  A mirror of this repository has been created in the
> TianoCore organization at https://github.com/tianocore/edk2-cmocka.git
> and uses a GitHub Action to auto-sync changes from
> https://git.cryptomilk.org/projects/cmocka.git.
> 
> * Update .gitmodules to use https://github.com/tianocore/edk2-cmocka.git
>   instead of https://git.cryptomilk.org/projects/cmocka.git.
> 
> * Update README.rst to reference the COPYING file in
>   https://github.com/tianocore/edk2-cmocka.git with the cmocka license.
> 
> * Update Azure Pipelines YML files to remove a temporary workaround that
>   used an alternate GitHub mirror of cmocka.  With the workaround
> removed,
>   EDK II CI always uses the TianoCore mirror of cmocka.
> 
> Cc: Sean Brogan 
> Cc: Bret Barkelew 
> Cc: Liming Gao 
> Cc: Andrew Fish 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Signed-off-by: Michael D Kinney 
> ---
>  .azurepipelines/templates/platform-build-run-steps.yml | 3 ---
>  .azurepipelines/templates/pr-gate-steps.yml| 3 ---
>  .gitmodules| 2 +-
>  ReadMe.rst | 2 +-
>  4 files changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/.azurepipelines/templates/platform-build-run-steps.yml
> b/.azurepipelines/templates/platform-build-run-steps.yml
> index b712e2fafafb..97e7faa26682 100644
> --- a/.azurepipelines/templates/platform-build-run-steps.yml
> +++ b/.azurepipelines/templates/platform-build-run-steps.yml
> @@ -51,9 +51,6 @@ steps:
>  # Set default
>  - bash: echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}"
> 
> -# Use altername cmocka repo
> -- bash: git config --global
> url.https://github.com/neverware-mirrors/cmocka.git.insteadOf
> https://git.cryptomilk.org/projects/cmocka.git
> -
>  # Fetch the target branch so that pr_eval can diff them.
>  # Seems like azure pipelines/github changed checkout process in nov 2020.
>  - script: git fetch origin $(System.PullRequest.targetBranch)
> diff --git a/.azurepipelines/templates/pr-gate-steps.yml
> b/.azurepipelines/templates/pr-gate-steps.yml
> index 28edb453bddc..70c19a462194 100644
> --- a/.azurepipelines/templates/pr-gate-steps.yml
> +++ b/.azurepipelines/templates/pr-gate-steps.yml
> @@ -31,9 +31,6 @@ steps:
>  echo "##vso[task.setvariable
> variable=pkgs_to_build]${{ parameters.build_pkgs }}"
>  echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}"
> 
> -# Use altername cmocka repo
> -- bash: git config --global
> url.https://github.com/neverware-mirrors/cmocka.git.insteadOf
> https://git.cryptomilk.org/projects/cmocka.git
> -
>  # Fetch the target branch so that pr_eval can diff them.
>  # Seems like azure pipelines/github changed checkout process in nov 2020.
>  - script: git fetch origin $(System.PullRequest.targetBranch)
> diff --git a/.gitmodules b/.gitmodules
> index 0f06c09a29a1..b845c9ee3ff0 100644
> --- a/.gitmodules
> +++ b/.gitmodules
> @@ -6,7 +6,7 @@
>   url = https://github.com/ucb-bar/berkeley-softfloat-3.git
>  [submodule "UnitTestFrameworkPkg/Library/CmockaLib/cmocka"]
>   path = UnitTestFrameworkPkg/Library/CmockaLib/cmocka
> - url = https://git.cryptomilk.org/projects/cmocka.git
> + url = https://github.com/tianocore/edk2-cmocka.git
>  [submodule "MdeModulePkg/Universal/RegularExpressionDxe/oniguruma"]
>   path = MdeModulePkg/Universal/RegularExpressionDxe/oniguruma
>   url = https://github.com/kkos/oniguruma
> diff --git a/ReadMe.rst b/ReadMe.rst
> index b754378646db..8f5db11281bf 100644
> --- a/ReadMe.rst
> +++ b/ReadMe.rst
> @@ -92,7 +92,7 @@ that are covered by additional licenses.
>  -  `CryptoPkg/Library/OpensslLib/openssl
>  2f56f1ffb72/LICENSE>`__
>  -  `MdeModulePkg/Library/BrotliCustomDecompressLib/brotli
>  bdd12cc8d/LICENSE>`__
>  -  `MdeModulePkg/Universal/RegularExpressionDxe/oniguruma
>  06975678f0d/COPYING>`__
> --  `UnitTestFrameworkPkg/Library/CmockaLib/cmocka
>  1.5=f5e2cd77c88d9f792562888d2b70c5a396bfbf7a>`__
> +-  `UnitTestFrameworkPkg/Library/CmockaLib/cmocka
>

Re: [edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

2021-04-06 Thread James Bottomley

On Wed, 2021-04-07 at 00:21 +, Xu, Min M wrote:
> Hi, Laszlo
> 
> For Intel TDX supported guest, all processors start in 32-bit
> protected
> mode, while for Non-Td guest, it starts in 16-bit real mode. To make
> the
> ResetVector work on both Td-guest and Non-Td guest, ResetVector are
> updated as below:
> --
>   ALIGN   16
>   resetVector:
>   ;
>   ; Reset Vector
>   ;
>   ; This is where the processor will begin execution
>   ;
>   nop
>   nop
>   smswax
>   testal, 1
>   jnz EarlyBspPmEntry
>   jmp EarlyBspInitReal16

Well, then use the rel8 jump like the compiler would in this situation:

  smswax
  testal, 1
  jz  1f
  jmp EarlyBspPmEntry
1:
  jmp EarlyBspInitReal16

So now both entries can be 32k away.

James




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73750): https://edk2.groups.io/g/devel/message/73750
Mute This Topic: https://groups.io/mt/81584577/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

2021-04-06 Thread James Bottomley

On Tue, 2021-04-06 at 14:16 +0200, Laszlo Ersek wrote:
> On 04/06/21 10:11, Xu, Min M wrote:
> > Hi, Singh
> > I have a concern about the sevSnpBlock in ResetVectorVtf0.asm.
> > Actually
> > SEV has inserted 3 blocks in ResetVectorVtf0.asm and the total
> > bytes are
> > (26 + 22 + 20 = 68 bytes). If sevSnpBlock is added, then the total
> > bytes
> > will be (68 +26 = 94 bytes).
> > 
> > I am not sure whether there will be more blocks added in
> > ResetVectorVtf0.asm in the future. But I don't think
> > ResetVectorVtf0.asm
> > is a good place to add these data blobs. Can these data be packed
> > into a
> > single file, for example, SevMetadata.asm, then a pointer is
> > inserted in
> > ResetVectorVtf0.asm which then points to the SevMetadata. In this
> > way we
> > can keep ResetVectorVtf0.asm clean, small and straight forward.
> > 
> > Another reason is that I am working on the Intel TDX which will
> > update the ResetVectorVtf0.asm as well. My change depends on the
> > assumption that the distance between ResetVector(0xfff0) and
> > EarlyBspInitReal16 is less than 128 bytes. The blocks in
> > ResetVectorVtf0.asm make it impossible.

Why?  I think the short jump can cover 32k as an offset.

> That's a problem. These info blocks are placed in the reset vector
> because then they can be found by QEMU easily -- they are not
> compressed, and they appear at a known location in the guest physical
> address space. (More precisely, a GUID-ed structure chain starts at a
> known location, and then QEMU can traverse the chain of structures,
> for learning various bits of information about the firmware.)
> 
> Do we absolutely need a short jump?

Yes, I explained this before:

https://listman.redhat.com/archives/edk2-devel-archive/2020-November/msg01063.html

Sorry, it's buried in the middle about why we can only have 32k worth
of entries.  However, the restriction is 32k not 128 bytes unless Intel
is doing something strange.

James




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73749): https://edk2.groups.io/g/devel/message/73749
Mute This Topic: https://groups.io/mt/81584577/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

2021-04-06 Thread Min Xu

Hi, Laszlo

For Intel TDX supported guest, all processors start in 32-bit protected
mode, while for Non-Td guest, it starts in 16-bit real mode. To make the
ResetVector work on both Td-guest and Non-Td guest, ResetVector are
updated as below:
--
  ALIGN   16
  resetVector:
  ;
  ; Reset Vector
  ;
  ; This is where the processor will begin execution
  ;
  nop
  nop
  smswax
  testal, 1
  jnz EarlyBspPmEntry
  jmp EarlyBspInitReal16

  ALIGN   16
  fourGigabytes:
--

For Non-Td guest, jmp to EarlyBspInitReal16 in 16-bit real mode is ok.

For Td-guest, first jmp to EarlyBspPmEntry in 32-bit protected mode, then
in EarlyBspPmEntry jmp to MainTd which is the the main entry for Td-guest.
This requires the distance between ResetVector and EarlyBspPmEntry less
than 128 bytes.

Intel TDX also has metadata which is consumed by QEMU. We put the metadata
in a single file (TdxMetadata.asm) and put it at the end of ResetVectorVtf0.
Then a pointer is placed in a known location in ResetVector.nasm. In this way
QEMU can easily read the Metadata by the pointer.
--
ALIGN   8
;
; TDX Virtual Firmware injects metadata in VTF0.
; The address of the metadata is injected in this location (0xffe8)
;
DD  (OVMF_IMAGE_SIZE_IN_KB * 1024 - (fourGigabytes - TdxMetadataGuid - 
16))
;
; The VTF signature
;
; VTF-0 means that the VTF (Volume Top File) code does not require
; any fixups.
;
vtfSignature:
DB  'V', 'T', 'F', 0
--

The space in ResetVector is very precious and we all want a known location so 
that QEMU
can find the metadata easily. Putting the metadata in a single file give the 
developers
more flexible (They can put anything they want). So I think a pointer (point to 
a metadata
file) in a known location maybe a better solution.

Thanks!

> -Original Message-
> From: Laszlo Ersek 
> Sent: Tuesday, April 6, 2021 8:17 PM
> To: Xu, Min M ; Brijesh Singh
> ; devel@edk2.groups.io
> Cc: James Bottomley ; Yao, Jiewen
> ; Tom Lendacky ;
> Justen, Jordan L ; Ard Biesheuvel
> 
> Subject: Re: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page
> for the SEV-SNP guest
> 
> On 04/06/21 10:11, Xu, Min M wrote:
> > Hi, Singh
> > I have a concern about the sevSnpBlock in ResetVectorVtf0.asm.
> > Actually SEV has inserted 3 blocks in ResetVectorVtf0.asm and the
> > total bytes are
> > (26 + 22 + 20 = 68 bytes). If sevSnpBlock is added, then the total
> > bytes will be (68 +26 = 94 bytes).
> >
> > I am not sure whether there will be more blocks added in
> > ResetVectorVtf0.asm in the future. But I don't think
> > ResetVectorVtf0.asm is a good place to add these data blobs. Can these
> > data be packed into a single file, for example, SevMetadata.asm, then
> > a pointer is inserted in ResetVectorVtf0.asm which then points to the
> > SevMetadata. In this way we can keep ResetVectorVtf0.asm clean, small
> and straight forward.
> >
> > Another reason is that I am working on the Intel TDX which will update
> > the ResetVectorVtf0.asm as well. My change depends on the assumption
> > that the distance between ResetVector(0xfff0) and
> > EarlyBspInitReal16 is less than 128 bytes. The blocks in
> ResetVectorVtf0.asm make it impossible.
> 
> That's a problem. These info blocks are placed in the reset vector because
> then they can be found by QEMU easily -- they are not compressed, and they
> appear at a known location in the guest physical address space. (More
> precisely, a GUID-ed structure chain starts at a known location, and then
> QEMU can traverse the chain of structures, for learning various bits of
> information about the firmware.)
> 
> Do we absolutely need a short jump?
> 
> Thanks
> Laszlo
> 
> >
> > Thanks!
> >
> >> -Original Message-
> >> From: Brijesh Singh 
> >> Sent: Wednesday, March 24, 2021 11:32 PM
> >> To: devel@edk2.groups.io
> >> Cc: Brijesh Singh ; James Bottomley
> >> ; Xu, Min M ; Yao, Jiewen
> >> ; Tom Lendacky ;
> >> Justen, Jordan L ; Ard Biesheuvel
> >> ; Laszlo Ersek 
> >> Subject: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid
> >> page for the SEV-SNP guest
> >>
> >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> >>
> >> During the SEV-SNP guest launch sequence, two special pages need to
> >> be inserted, the secrets page and cpuid page. The secrets page,
> >> contain the VM platform communication keys. The guest BIOS and OS can
> >> use this key to communicate with the SEV firmware to get the
> >> attestation report. The Cpuid page, contain the CPUIDs entries filtered
> through the AMD-SEV firmware.
> >>
> >> The VMM will locate the secrets and cpuid page addresses through a
> >> fixed GUID and pass them to SEV firmware to populate further.
> >> For more

Re: [edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

2021-04-06 Thread Michael Kubacki


Hi Nate,

Feedback is inline.

Please carry over applicable feedback to LargeVariableWriteLib, I did 
not duplicate the response there.


Thanks,
Michael

On 4/6/2021 12:24 PM, Nate DeSimone wrote:

LargeVariableReadLib is used to retrieve large data sets using
the UEFI Variable Services. At time of writting, most UEFI
Variable Services implementations to not allow more than 64KB
of data to be stored in a single UEFI variable. This library
will split data sets across multiple variables as needed.

It adds the GetLargeVariable() API to provide this service.

The primary use for this library is to create binary compatible
drivers and OpROMs which need to work both with TianoCore and
other UEFI PI implementations. When customizing and recompiling
the platform firmware image is possible, adjusting the value of
PcdMaxVariableSize may provide a simpler solution to this
problem.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
  .../Include/Dsc/CoreCommonLib.dsc |   5 +-
  .../Include/Library/LargeVariableReadLib.h|  56 +
  .../BaseLargeVariableReadLib.inf  |  50 +
  .../LargeVariableReadLib.c| 199 ++
  4 files changed, 308 insertions(+), 2 deletions(-)
  create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index cf2940cf02..5f2ad3f0f0 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -135,13 +135,14 @@
VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf

PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf

AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
-
+
  !if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
  !endif
  
SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf

BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
+  
LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
  
#

# CryptLib
@@ -165,4 +166,4 @@
  
SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf


VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
-  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
\ No newline at end of file
+  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
diff --git 
a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
new file mode 100644
index 00..5579492727
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
@@ -0,0 +1,56 @@
+/** @file
+  Large Variable Read Lib
+
+  This library is used to retrieve large data sets using the UEFI Variable
+  Services. At time of writing, most UEFI Variable Services implementations to
+  not allow more than 64KB of data to be stored in a single UEFI variable. This
+  library will split data sets across multiple variables as needed.
+
+  In the case where more than one variable is needed to store the data, an
+  integer number will be added to the end of the variable name. This number
+  will be incremented for each variable as needed to retrieve the entire data
+  set.
+
+  The primary use for this library is to create binary compatible drivers
+  and OpROMs which need to work both with TianoCore and other UEFI PI
+  implementations. When customizing and recompiling the platform firmware image
+  is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
+  solution to this problem.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+


Similar comment to VariableReadLib/VariableWriteLib, header guard is 
missing.



+#include 
+
+/**
+  Returns the value of a large variable.
+
+  @param[in]   VariableName  A Null-terminated string that is the name of 
the vendor's
+ variable.
+  @param[in]   VendorGuidA unique identifier for the vendor.
+  @param[in, out]  DataSize  On input, the size in bytes of the return 
Data buffer.
+ On output the size of data returned in Data.
+  @param[out]  Data  The buffer to return the

Re: [edk2-devel] [edk2-platforms] [PATCH v3 1/4] MinPlatformPkg: Add VariableReadLib

2021-04-06 Thread Michael Kubacki


Hi Nate,

Feedback is inline.

Most of the items carry over to the VariableWriteLib patch as well but I 
didn't duplicate the response to that patch.


Thanks,
Michael

On 4/6/2021 12:24 PM, Nate DeSimone wrote:

VariableReadLib is a phase agnostic libary for reading UEFI
Variables. This library provides the
MinGetVariable() and MinGetNextVariableName() APIs which
are usable PEI, DXE, and SMM.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
  .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  10 +-
  .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
  .../Include/Library/VariableReadLib.h |  87 ++
  .../DxeRuntimeVariableReadLib.c   | 115 +
  .../DxeRuntimeVariableReadLib.inf |  41 +
  .../PeiVariableReadLib/PeiVariableReadLib.c   | 153 ++
  .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 +
  .../SmmVariableReadCommon.c   | 114 +
  .../StandaloneMmVariableReadLib.inf   |  50 ++
  .../StandaloneMmVariableReadLibConstructor.c  |  48 ++
  .../TraditionalMmVariableReadLib.inf  |  49 ++
  .../TraditionalMmVariableReadLibConstructor.c |  48 ++
  .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   3 +-
  13 files changed, 761 insertions(+), 8 deletions(-)
  create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.c
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.inf
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/SmmVariableReadCommon.c
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLib.inf
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLibConstructor.c
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
  create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLibConstructor.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index fa9098d525..0db1250ab7 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -1,7 +1,7 @@
  ## @file
  #  Platform description.
  #
-# Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
  #
  # SPDX-License-Identifier: BSD-2-Clause-Patent
  #
@@ -11,7 +11,7 @@
#
# Generic EDKII Lib
#
-
+
#
# DXE phase common
#
@@ -23,7 +23,7 @@

ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
  
HstiLib|MdePkg/Library/DxeHstiLib/DxeHstiLib.inf

-
+
LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
  
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf

@@ -46,6 +46,9 @@
  
VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
  
+[LibraryClasses.common.DXE_CORE, LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION]

+  
VariableReadLib|MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
+
  [LibraryClasses.common.DXE_CORE]
HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf

MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
@@ -89,6 +92,7 @@

CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf

Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+  
VariableReadLib|MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
  
  [LibraryClasses.common.SMM_CORE]

PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
index 2bcaed05a1..d64873ac6d 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
@@ -1,7 +1,7 @@
  ## @file
  #  Platform description.
  #
-# Copyright (c) 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017 - 2021, Intel Corporation.

Re: [edk2-devel] [RFC] UnitTestFrameworkPkg cmocka submodule alternatives

2021-04-06 Thread Michael D Kinney

Hi Rebecca,

Thank you for the reminder.  I have entered a BZ and sent a patch for review

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

https://edk2.groups.io/g/devel/message/73744

Mike

> -Original Message-
> From: Rebecca Cran 
> Sent: Sunday, February 14, 2021 4:43 PM
> To: devel@edk2.groups.io; Kinney, Michael D ; 
> r...@edk2.groups.io; 'Bret Barkelew'
> ; Laszlo Ersek ; Gao, Liming 
> 
> Subject: Re: [edk2-devel] [RFC] UnitTestFrameworkPkg cmocka submodule 
> alternatives
> 
> On 12/19/20 11:58 AM, Michael D Kinney wrote:
> > Hello,
> >
> > There have been a few suggestions to create a mirror of cmocka in TianoCore
> > org in GitHub.
> >
> > I have found a GitHub action that can do a repo sync.
> >
> >  https://github.com/marketplace/actions/github-repo-sync
> >
> > I have created a temporary mirror of cmocka in my personal GitHub area that
> > uses this GitHub action to sync all branches and all tags once a day.
> >
> >  https://github.com/mdkinney/mirror-cmocka
> >
> > Here is the GitHub workflow file.  It must be in the default branch for the
> > repo using a branch name that is not present in the repo being mirrored.
> > In this case, I used a branch name of 'repo-sync'.
> >
> >  
> > https://github.com/mdkinney/mirror-cmocka/blob/repo-sync/.github/workflows/repo-sync.yml
> >
> > Please provide feedback on this approach.  If we like this approach, then
> > I suggest we create a new repo in TianoCore called edk2-cmocka that is a
> > mirror that is synced once a day and we update the cmocka submodule in the
> > edk2 repo to use edk2-cmocka.
> 
> I just noticed this never got committed:
> https://github.com/tianocore/edk2/blob/master/.gitmodules still shows
> the old URL.
> 
> --
> Rebecca Cran



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73745): https://edk2.groups.io/g/devel/message/73745
Mute This Topic: https://groups.io/mt/79018115/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [Patch 1/1] UnitTestFrameworkPkg: Use TianoCore mirror of cmocka repository

2021-04-06 Thread Michael D Kinney

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

The cmocka repository https://git.cryptomilk.org/projects/cmocka.git
has gone down a few times in past year.  When it is down, it blocks
EDK II CI.  A mirror of this repository has been created in the
TianoCore organization at https://github.com/tianocore/edk2-cmocka.git
and uses a GitHub Action to auto-sync changes from
https://git.cryptomilk.org/projects/cmocka.git.

* Update .gitmodules to use https://github.com/tianocore/edk2-cmocka.git
  instead of https://git.cryptomilk.org/projects/cmocka.git.

* Update README.rst to reference the COPYING file in
  https://github.com/tianocore/edk2-cmocka.git with the cmocka license.

* Update Azure Pipelines YML files to remove a temporary workaround that
  used an alternate GitHub mirror of cmocka.  With the workaround removed,
  EDK II CI always uses the TianoCore mirror of cmocka.

Cc: Sean Brogan 
Cc: Bret Barkelew 
Cc: Liming Gao 
Cc: Andrew Fish 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Signed-off-by: Michael D Kinney 
---
 .azurepipelines/templates/platform-build-run-steps.yml | 3 ---
 .azurepipelines/templates/pr-gate-steps.yml| 3 ---
 .gitmodules| 2 +-
 ReadMe.rst | 2 +-
 4 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/.azurepipelines/templates/platform-build-run-steps.yml 
b/.azurepipelines/templates/platform-build-run-steps.yml
index b712e2fafafb..97e7faa26682 100644
--- a/.azurepipelines/templates/platform-build-run-steps.yml
+++ b/.azurepipelines/templates/platform-build-run-steps.yml
@@ -51,9 +51,6 @@ steps:
 # Set default
 - bash: echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}"
 
-# Use altername cmocka repo
-- bash: git config --global 
url.https://github.com/neverware-mirrors/cmocka.git.insteadOf 
https://git.cryptomilk.org/projects/cmocka.git
-
 # Fetch the target branch so that pr_eval can diff them.
 # Seems like azure pipelines/github changed checkout process in nov 2020.
 - script: git fetch origin $(System.PullRequest.targetBranch)
diff --git a/.azurepipelines/templates/pr-gate-steps.yml 
b/.azurepipelines/templates/pr-gate-steps.yml
index 28edb453bddc..70c19a462194 100644
--- a/.azurepipelines/templates/pr-gate-steps.yml
+++ b/.azurepipelines/templates/pr-gate-steps.yml
@@ -31,9 +31,6 @@ steps:
 echo "##vso[task.setvariable variable=pkgs_to_build]${{ 
parameters.build_pkgs }}"
 echo "##vso[task.setvariable variable=pkg_count]${{ 1 }}"
 
-# Use altername cmocka repo
-- bash: git config --global 
url.https://github.com/neverware-mirrors/cmocka.git.insteadOf 
https://git.cryptomilk.org/projects/cmocka.git
-
 # Fetch the target branch so that pr_eval can diff them.
 # Seems like azure pipelines/github changed checkout process in nov 2020.
 - script: git fetch origin $(System.PullRequest.targetBranch)
diff --git a/.gitmodules b/.gitmodules
index 0f06c09a29a1..b845c9ee3ff0 100644
--- a/.gitmodules
+++ b/.gitmodules
@@ -6,7 +6,7 @@
url = https://github.com/ucb-bar/berkeley-softfloat-3.git
 [submodule "UnitTestFrameworkPkg/Library/CmockaLib/cmocka"]
path = UnitTestFrameworkPkg/Library/CmockaLib/cmocka
-   url = https://git.cryptomilk.org/projects/cmocka.git
+   url = https://github.com/tianocore/edk2-cmocka.git
 [submodule "MdeModulePkg/Universal/RegularExpressionDxe/oniguruma"]
path = MdeModulePkg/Universal/RegularExpressionDxe/oniguruma
url = https://github.com/kkos/oniguruma
diff --git a/ReadMe.rst b/ReadMe.rst
index b754378646db..8f5db11281bf 100644
--- a/ReadMe.rst
+++ b/ReadMe.rst
@@ -92,7 +92,7 @@ that are covered by additional licenses.
 -  `CryptoPkg/Library/OpensslLib/openssl 
`__
 -  `MdeModulePkg/Library/BrotliCustomDecompressLib/brotli 
`__
 -  `MdeModulePkg/Universal/RegularExpressionDxe/oniguruma 
`__
--  `UnitTestFrameworkPkg/Library/CmockaLib/cmocka 
`__
+-  `UnitTestFrameworkPkg/Library/CmockaLib/cmocka 
`__
 -  `RedfishPkg/Library/JsonLib/jansson 
`__
 
 The EDK II Project is composed of packages. The maintainers for each package
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73744): https://edk2.groups.io/g/devel/message/73744
Mute This Topic: https://groups.io/mt/81900091/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe:

Re: [edk2-devel] [Patch 1/1] MdeModulePkg/PiDxeS3BootScriptLib: Rename mAcpiS3Enable to avoid dup symbol

2021-04-06 Thread Lohr, Paul A

Looks good to me Mike.  Thank you!

Paul A. Lohr - Server Firmware Enabling
512.239.9073 (cell)
512.794.5044 (work)

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Michael D Kinney
Sent: Monday, April 5, 2021 5:31 PM
To: devel@edk2.groups.io
Cc: Wang, Jian J ; Wu, Hao A ; Dong, 
Eric 
Subject: [edk2-devel] [Patch 1/1] MdeModulePkg/PiDxeS3BootScriptLib: Rename 
mAcpiS3Enable to avoid dup symbol

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

Rename the global variable mAcpiS3Enable to mS3BootScriptAcpiS3Enable to avoid 
duplicate symbol errors from CLANGPDB tool change when PiDxeS3BootScriptLib 
from the MdeModulePkg is linked with PiSmmCpuDxeSmm from the UefiCpuPkg.

Cc: Jian J Wang 
Cc: Hao A Wu 
Cc: Eric Dong 
Signed-off-by: Michael D Kinney 
---
 .../Library/PiDxeS3BootScriptLib/BootScriptSave.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c 
b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
index 9315fc9f0188..9cdf47552162 100644
--- a/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
+++ b/MdeModulePkg/Library/PiDxeS3BootScriptLib/BootScriptSave.c
@@ -1,7 +1,7 @@
 /** @file
   Save the S3 data to S3 boot script.
 
-  Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2021, Intel Corporation. All rights 
+ reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -124,7 +124,7 @@ VOID 
*mRegistrationSmmReadyToLock = NULL;
 BOOLEAN  mS3BootScriptTableAllocated = FALSE;
 BOOLEAN  mS3BootScriptTableSmmAllocated = FALSE;
 EFI_SMM_SYSTEM_TABLE2*mBootScriptSmst = NULL;
-BOOLEAN  mAcpiS3Enable = TRUE;
+BOOLEAN  mS3BootScriptAcpiS3Enable = TRUE;
 
 /**
   This is an internal function to add a terminate node the entry, recalculate 
the table @@ -438,7 +438,7 @@ S3BootScriptLibInitialize (
   EFI_PHYSICAL_ADDRESS   Buffer;
 
   if (!PcdGetBool (PcdAcpiS3Enable)) {
-mAcpiS3Enable = FALSE;
+mS3BootScriptAcpiS3Enable = FALSE;
 DEBUG ((DEBUG_INFO, "%a: Skip S3BootScript because ACPI S3 disabled.\n", 
gEfiCallerBaseName));
 return RETURN_SUCCESS;
   }
@@ -569,7 +569,7 @@ S3BootScriptLibDeinitialize (  {
   EFI_STATUSStatus;
 
-  if (!mAcpiS3Enable) {
+  if (!mS3BootScriptAcpiS3Enable) {
 return RETURN_SUCCESS;
   }
 
@@ -821,7 +821,7 @@ S3BootScriptGetEntryAddAddress (  {
   UINT8* NewEntryPtr;
 
-  if (!mAcpiS3Enable) {
+  if (!mS3BootScriptAcpiS3Enable) {
 return NULL;
   }
 
@@ -2410,4 +2410,3 @@ S3BootScriptCompare (
 
   return EFI_SUCCESS;
 }
-
--
2.31.1.windows.1








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73743): https://edk2.groups.io/g/devel/message/73743
Mute This Topic: https://groups.io/mt/81876778/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 0/1] Add buffer size check before save state read

2021-04-06 Thread Kun Qin

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

This change is validated on two different physical platforms.

v2 mainly focus on feedback for v1 patche, including:
a. Adding "Reviewed-by" tags;
b. Updated return code description for modified function;

Patch v2 branch: https://github.com/kuqin12/edk2/tree/svst_width_v2

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Kun Qin (1):
  UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing

 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 9 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

-- 
2.31.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73741): https://edk2.groups.io/g/devel/message/73741
Mute This Topic: https://groups.io/mt/81899610/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing

2021-04-06 Thread Kun Qin

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

Current SMM Save State routine does not check the number of bytes to be
read, when it comse to read IO_INFO, before casting the incoming buffer
to EFI_SMM_SAVE_STATE_IO_INFO. This could potentially cause memory
corruption due to extra bytes are written out of buffer boundary.

This change adds a width check before copying IoInfo into output buffer.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
Reviewed-by: Ray Ni 
Reviewed-by: Laszlo Ersek 
---

Notes:
v2:
- Update return code description [Laszlo]

 UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 9 -
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 2 +-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
index 661cc51f361a..fc418c2500a9 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
@@ -343,7 +343,7 @@ ReadSaveStateRegisterByIndex (
 
   @retval EFI_SUCCESS   The register was read from Save State.
   @retval EFI_NOT_FOUND The register is not defined for the Save State 
of Processor.
-  @retval EFI_INVALID_PARAMETER  This or Buffer is NULL.
+  @retval EFI_INVALID_PARAMETER Buffer is NULL, or Width does not meet 
requirement per Register type.
 
 **/
 EFI_STATUS
@@ -418,6 +418,13 @@ ReadSaveStateRegister (
   return EFI_NOT_FOUND;
 }
 
+//
+// Make sure the incoming buffer is large enough to hold IoInfo before 
accessing
+//
+if (Width < sizeof (EFI_SMM_SAVE_STATE_IO_INFO)) {
+  return EFI_INVALID_PARAMETER;
+}
+
 //
 // Zero the IoInfo structure that will be returned in Buffer
 //
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index b8aa9e1769d3..2248a8c5ee66 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -337,7 +337,7 @@ This function supports reading a CPU Save State register in 
SMBase relocation ha
 
 @retval EFI_SUCCESS   The register was read from Save State.
 @retval EFI_NOT_FOUND The register is not defined for the Save State 
of Processor.
-@retval EFI_INVALID_PARAMETER  This or Buffer is NULL.
+@retval EFI_INVALID_PARAMETER Buffer is NULL, or Width does not meet 
requirement per Register type.
 
 **/
 EFI_STATUS
-- 
2.31.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73742): https://edk2.groups.io/g/devel/message/73742
Mute This Topic: https://groups.io/mt/81899611/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [edk2-platforms] [PATCH v3 0/4] Add Large Variable Libraries

2021-04-06 Thread Nate DeSimone

Changes from V2:
 - Added comment to LargeVariableLib INF and header files
   describing the usage for drivers that cannot assume that
   PcdMaxVariableSize has been set to a certain minimum value.

Changes from V1:
 - Changed prefix from "Min" to "VarLib"
 - Better comments
 - Added more whitespace for readability
 - Removed unused INF sections
 - Better debug messages

This patch series introduces libaries that enable large data sets
to be stored using the UEFI Variable Services. At present, most
UEFI Variable Services implementations have a maximum variable
size of <=64KB. The exact value varies depending on platform.

These libaries enable a data set to use as much space as needed,
up to the remaining space in the UEFI Variable non-volatile storage.

To implement this, I have broken the problem down into two parts:

 1. Phase angostic UEFI Variable access.
 2. Storage of data across multiple UEFI Variables.

For the first part, I have created two new LibraryClasses:
VariableReadLib and VariableWriteLib. I have provided
implementation instances of VariableReadLib for PEI, DXE, and SMM.
For VariableWriteLib, I have provided implementation instances for
DXE and SMM. This enables code that accesses UEFI variables
to be written in a matter than is phase agnostic, so the same
code can be used in PEI, DXE, or SMM without modification.

The second part involves another two new LibaryClasses:
LargeVariableReadLib and LargeVariableWriteLib. Only one BASE
implementation is needed for both of these as the phase dependent
code was seperated out in the first piece. These libraries provide
logic to calculate the maximum size of an individual UEFI variable
and split the data into as many smaller pieces as needed to store
the entire data set in the UEFI Variable storage. They also provide
the ability to stitch the data back together when it is read.
Deleting the data will delete all variables used to store it.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 

Nate DeSimone (4):
  MinPlatformPkg: Add VariableReadLib
  MinPlatformPkg: Add VariableWriteLib
  MinPlatformPkg: Add LargeVariableReadLib
  MinPlatformPkg: Add LargeVariableWriteLib

 .../Include/Dsc/CoreCommonLib.dsc |   6 +-
 .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  12 +-
 .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
 .../Include/Library/LargeVariableReadLib.h|  56 ++
 .../Include/Library/LargeVariableWriteLib.h   |  64 +++
 .../Include/Library/VariableReadLib.h |  87 
 .../Include/Library/VariableWriteLib.h| 129 +
 .../BaseLargeVariableReadLib.inf  |  50 ++
 .../LargeVariableReadLib.c| 199 
 .../BaseLargeVariableWriteLib.inf |  50 ++
 .../LargeVariableWriteLib.c   | 479 ++
 .../DxeRuntimeVariableReadLib.c   | 115 +
 .../DxeRuntimeVariableReadLib.inf |  41 ++
 .../DxeRuntimeVariableWriteLib.c  | 256 ++
 .../DxeRuntimeVariableWriteLib.inf|  49 ++
 .../PeiVariableReadLib/PeiVariableReadLib.c   | 153 ++
 .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 ++
 .../SmmVariableReadCommon.c   | 114 +
 .../StandaloneMmVariableReadLib.inf   |  50 ++
 .../StandaloneMmVariableReadLibConstructor.c  |  48 ++
 .../TraditionalMmVariableReadLib.inf  |  49 ++
 .../TraditionalMmVariableReadLibConstructor.c |  48 ++
 .../SmmVariableWriteCommon.c  | 167 ++
 .../StandaloneMmVariableWriteLib.inf  |  45 ++
 .../StandaloneMmVariableWriteLibConstructor.c |  48 ++
 .../TraditionalMmVariableWriteLib.inf |  44 ++
 ...TraditionalMmVariableWriteLibConstructor.c |  48 ++
 .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   4 +-
 28 files changed, 2452 insertions(+), 10 deletions(-)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableWriteLib/BaseLargeVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableWriteLib/LargeVariableWriteLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
 create mode 100644

[edk2-devel] [edk2-platforms] [PATCH v3 2/4] MinPlatformPkg: Add VariableWriteLib

2021-04-06 Thread Nate DeSimone

VariableWriteLib is a phase agnostic library for writing
to UEFI Variables. This library provides the MinSetVariable(),
MinQueryVariableInfo(), MinIsVariableRequestToLockSupported(),
and MinVariableRequestToLock() APIs which are usable in DXE
and SMM.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |   2 +
 .../Include/Library/VariableWriteLib.h| 129 +
 .../DxeRuntimeVariableWriteLib.c  | 256 ++
 .../DxeRuntimeVariableWriteLib.inf|  49 
 .../SmmVariableWriteCommon.c  | 167 
 .../StandaloneMmVariableWriteLib.inf  |  45 +++
 .../StandaloneMmVariableWriteLibConstructor.c |  48 
 .../TraditionalMmVariableWriteLib.inf |  44 +++
 ...TraditionalMmVariableWriteLibConstructor.c |  48 
 .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   1 +
 10 files changed, 789 insertions(+)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/SmmVariableWriteCommon.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/StandaloneMmVariableWriteLibConstructor.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLibConstructor.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index 0db1250ab7..57847e6a4d 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -48,6 +48,7 @@
 
 [LibraryClasses.common.DXE_CORE, LibraryClasses.common.DXE_DRIVER, 
LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER, 
LibraryClasses.common.UEFI_APPLICATION]
   
VariableReadLib|MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
+  
VariableWriteLib|MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRuntimeVariableWriteLib.inf
 
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
@@ -93,6 +94,7 @@
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
   
VariableReadLib|MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
+  
VariableWriteLib|MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMmVariableWriteLib.inf
 
 [LibraryClasses.common.SMM_CORE]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
new file mode 100644
index 00..c52aec93ec
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/VariableWriteLib.h
@@ -0,0 +1,129 @@
+/** @file
+  Variable Write Lib
+
+  This library provides phase agnostic access to the UEFI Variable Services.
+  This is done by implementing a wrapper on top of the phase specific mechanism
+  for writing to UEFI variables. For example, the DXE implementation accesses
+  the UEFI Runtime Services Table, and the SMM implementation uses
+  EFI_SMM_VARIABLE_PROTOCOL.
+
+  Using this library allows code to be written in a generic manner that can be
+  used in DXE or SMM without modification.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+/**
+  Sets the value of a variable.
+
+  @param[in]  VariableName   A Null-terminated string that is the name of 
the vendor's variable.
+ Each VariableName is unique for each 
VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName 
is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+  @param[in]  VendorGuid A unique identifier for the vendor.
+  @param[in]  Attributes Attributes bitmask to set for the variable.
+  @param[in]  DataSize   The size in bytes of the Data buffer. Unless 
the EFI_VARIABLE_APPEND_WRITE or
+ 
EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS attribute is set, a size of 
zero
+ causes the variable to be deleted. When the

[edk2-devel] [edk2-platforms] [PATCH v3 3/4] MinPlatformPkg: Add LargeVariableReadLib

2021-04-06 Thread Nate DeSimone

LargeVariableReadLib is used to retrieve large data sets using
the UEFI Variable Services. At time of writting, most UEFI
Variable Services implementations to not allow more than 64KB
of data to be stored in a single UEFI variable. This library
will split data sets across multiple variables as needed.

It adds the GetLargeVariable() API to provide this service.

The primary use for this library is to create binary compatible
drivers and OpROMs which need to work both with TianoCore and
other UEFI PI implementations. When customizing and recompiling
the platform firmware image is possible, adjusting the value of
PcdMaxVariableSize may provide a simpler solution to this
problem.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../Include/Dsc/CoreCommonLib.dsc |   5 +-
 .../Include/Library/LargeVariableReadLib.h|  56 +
 .../BaseLargeVariableReadLib.inf  |  50 +
 .../LargeVariableReadLib.c| 199 ++
 4 files changed, 308 insertions(+), 2 deletions(-)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableReadLib/LargeVariableReadLib.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index cf2940cf02..5f2ad3f0f0 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -135,13 +135,14 @@
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   
PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
   
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
-  
+
 !if gMinPlatformPkgTokenSpaceGuid.PcdUefiSecureBootEnable == TRUE
   AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
 !endif
 
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
+  
LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
 
   #
   # CryptLib
@@ -165,4 +166,4 @@
 
   SmbusLib|MdePkg/Library/BaseSmbusLibNull/BaseSmbusLibNull.inf
   
VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf
-  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
\ No newline at end of file
+  
VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
diff --git 
a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
new file mode 100644
index 00..5579492727
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableReadLib.h
@@ -0,0 +1,56 @@
+/** @file
+  Large Variable Read Lib
+
+  This library is used to retrieve large data sets using the UEFI Variable
+  Services. At time of writing, most UEFI Variable Services implementations to
+  not allow more than 64KB of data to be stored in a single UEFI variable. This
+  library will split data sets across multiple variables as needed.
+
+  In the case where more than one variable is needed to store the data, an
+  integer number will be added to the end of the variable name. This number
+  will be incremented for each variable as needed to retrieve the entire data
+  set.
+
+  The primary use for this library is to create binary compatible drivers
+  and OpROMs which need to work both with TianoCore and other UEFI PI
+  implementations. When customizing and recompiling the platform firmware image
+  is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
+  solution to this problem.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+/**
+  Returns the value of a large variable.
+
+  @param[in]   VariableName  A Null-terminated string that is the name of 
the vendor's
+ variable.
+  @param[in]   VendorGuidA unique identifier for the vendor.
+  @param[in, out]  DataSize  On input, the size in bytes of the return 
Data buffer.
+ On output the size of data returned in Data.
+  @param[out]  Data  The buffer to return the contents of the 
variable. May be NULL
+ with a zero DataSize in order to determine 
the size buffer needed.
+
+  @retval EFI_SUCCESSThe function completed successfully.
+  @retval EFI_NOT_FOUND  The variable was not found.
+  @retval EFI_BUFFER_TOO_SMALL   The

[edk2-devel] [edk2-platforms] [PATCH v3 1/4] MinPlatformPkg: Add VariableReadLib

2021-04-06 Thread Nate DeSimone

VariableReadLib is a phase agnostic libary for reading UEFI
Variables. This library provides the
MinGetVariable() and MinGetNextVariableName() APIs which
are usable PEI, DXE, and SMM.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc |  10 +-
 .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc |   9 +-
 .../Include/Library/VariableReadLib.h |  87 ++
 .../DxeRuntimeVariableReadLib.c   | 115 +
 .../DxeRuntimeVariableReadLib.inf |  41 +
 .../PeiVariableReadLib/PeiVariableReadLib.c   | 153 ++
 .../PeiVariableReadLib/PeiVariableReadLib.inf |  42 +
 .../SmmVariableReadCommon.c   | 114 +
 .../StandaloneMmVariableReadLib.inf   |  50 ++
 .../StandaloneMmVariableReadLibConstructor.c  |  48 ++
 .../TraditionalMmVariableReadLib.inf  |  49 ++
 .../TraditionalMmVariableReadLibConstructor.c |  48 ++
 .../Intel/MinPlatformPkg/MinPlatformPkg.dsc   |   3 +-
 13 files changed, 761 insertions(+), 8 deletions(-)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/VariableReadLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/SmmVariableReadCommon.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/StandaloneMmVariableReadLibConstructor.c
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLibConstructor.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
index fa9098d525..0db1250ab7 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  Platform description.
 #
-# Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -11,7 +11,7 @@
   #
   # Generic EDKII Lib
   #
-  
+
   #
   # DXE phase common
   #
@@ -23,7 +23,7 @@
   
ExtractGuidedSectionLib|MdePkg/Library/DxeExtractGuidedSectionLib/DxeExtractGuidedSectionLib.inf
 
   HstiLib|MdePkg/Library/DxeHstiLib/DxeHstiLib.inf
-  
+
   LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.inf
 
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
@@ -46,6 +46,9 @@
 
   VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
 
+[LibraryClasses.common.DXE_CORE, LibraryClasses.common.DXE_DRIVER, 
LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER, 
LibraryClasses.common.UEFI_APPLICATION]
+  
VariableReadLib|MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRuntimeVariableReadLib.inf
+
 [LibraryClasses.common.DXE_CORE]
   HobLib|MdePkg/Library/DxeCoreHobLib/DxeCoreHobLib.inf
   
MemoryAllocationLib|MdeModulePkg/Library/DxeCoreMemoryAllocationLib/DxeCoreMemoryAllocationLib.inf
@@ -89,6 +92,7 @@
   
CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
   
Tcg2PhysicalPresenceLib|SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.inf
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+  
VariableReadLib|MinPlatformPkg/Library/SmmVariableReadLib/TraditionalMmVariableReadLib.inf
 
 [LibraryClasses.common.SMM_CORE]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
index 2bcaed05a1..d64873ac6d 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
@@ -1,7 +1,7 @@
 ## @file
 #  Platform description.
 #
-# Copyright (c) 2017, Intel Corporation. All rights reserved.
+# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.
 #
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -10,11 +10,11 @@
   #
   # Generic EDKII Lib
   #
-  
+
   #
   # PEI phase common
   #
-  
+
 
[LibraryClasses.common.SEC,LibraryClasses.common.PEI_CORE,LibraryClasses.common.PEIM]

[edk2-devel] [edk2-platforms] [PATCH v3 4/4] MinPlatformPkg: Add LargeVariableWriteLib

2021-04-06 Thread Nate DeSimone

LargeVariableWriteLib is used to store large data sets using
the UEFI Variable Services. At time of writting, most UEFI
Variable Services implementations to not allow more than 64KB
of data to be stored in a single UEFI variable. This library
will split data sets across multiple variables as needed.

It adds the SetLargeVariable() API to provide this service.

The primary use for this library is to create binary compatible
drivers and OpROMs which need to work both with TianoCore and
other UEFI PI implementations. When customizing and recompiling
the platform firmware image is possible, adjusting the value of
PcdMaxVariableSize may provide a simpler solution to this
problem.

Cc: Chasel Chiu 
Cc: Liming Gao 
Cc: Eric Dong 
Cc: Michael Kubacki 
Cc: Isaac Oram 
Signed-off-by: Nate DeSimone 
Reviewed-by: Isaac Oram 
---
 .../Include/Dsc/CoreCommonLib.dsc |   1 +
 .../Include/Library/LargeVariableWriteLib.h   |  64 +++
 .../BaseLargeVariableWriteLib.inf |  50 ++
 .../LargeVariableWriteLib.c   | 479 ++
 4 files changed, 594 insertions(+)
 create mode 100644 
Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableWriteLib/BaseLargeVariableWriteLib.inf
 create mode 100644 
Platform/Intel/MinPlatformPkg/Library/BaseLargeVariableWriteLib/LargeVariableWriteLib.c

diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc 
b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
index 5f2ad3f0f0..78d66b9072 100644
--- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
+++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreCommonLib.dsc
@@ -143,6 +143,7 @@
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
   BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
   
LargeVariableReadLib|MinPlatformPkg/Library/BaseLargeVariableReadLib/BaseLargeVariableReadLib.inf
+  
LargeVariableWriteLib|MinPlatformPkg/Library/BaseLargeVariableWriteLib/BaseLargeVariableWriteLib.inf
 
   #
   # CryptLib
diff --git 
a/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h 
b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
new file mode 100644
index 00..6d597447f1
--- /dev/null
+++ b/Platform/Intel/MinPlatformPkg/Include/Library/LargeVariableWriteLib.h
@@ -0,0 +1,64 @@
+/** @file
+  Large Variable Write Lib
+
+  This library is used to store large data sets using the UEFI Variable 
Services.
+  At time of writting, most UEFI Variable Services implementations to not allow
+  more than 64KB of data to be stored in a single UEFI variable. This library
+  will split data sets across multiple variables as needed.
+
+  In the case where more than one variable is needed to store the data, an
+  integer number will be added to the end of the variable name. This number
+  will be incremented for each variable as needed to store the entire data set.
+
+  The primary use for this library is to create binary compatible drivers
+  and OpROMs which need to work both with TianoCore and other UEFI PI
+  implementations. When customizing and recompiling the platform firmware image
+  is possible, adjusting the value of PcdMaxVariableSize may provide a simpler
+  solution to this problem.
+
+  Copyright (c) 2021, Intel Corporation. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include 
+
+/**
+  Sets the value of a large variable.
+
+  @param[in]  VariableName   A Null-terminated string that is the name of 
the vendor's variable.
+ Each VariableName is unique for each 
VendorGuid. VariableName must
+ contain 1 or more characters. If VariableName 
is an empty string,
+ then EFI_INVALID_PARAMETER is returned.
+  @param[in]  VendorGuid A unique identifier for the vendor.
+  @param[in]  LockVariable   If TRUE, any further writes to the variable 
will be prevented until the next reset.
+ Note: LockVariable must be FALSE when running 
in SMM or after ExitBootServices.
+  @param[in]  DataSize   The size in bytes of the Data buffer. A size 
of zero causes the variable to be deleted.
+ If DataSize is zero, then LockVariable must 
be FALSE since a variable that does not
+ exist cannot be locked.
+  @param[in]  Data   The contents for the variable.
+
+  @retval EFI_SUCCESSThe firmware has successfully stored the 
variable and its data as
+ defined by the Attributes.
+  @retval EFI_INVALID_PARAMETER  An invalid combination of LockVariable, name, 
and GUID was supplied, or the
+ DataSize exceeds the maximum allowed.
+  @retval EFI_INVALID_PARAMETER  VariableName is an empty string.
+

Re: [edk2-devel] GSoC 2021 (MinPlatform, Ext2, ACPICA, etc)

2021-04-06 Thread Pedro Falcato

Hi Nate, Andrew,

Sounds great! I'll finish up my proposal and submit it tonight.
Thank you all for your advice!

Hopefully we'll see each other again in the summer :D

Thanks,

Pedro


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73735): https://edk2.groups.io/g/devel/message/73735
Mute This Topic: https://groups.io/mt/81290134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing

2021-04-06 Thread Kun Qin


Hi Laszlo,

Thanks for the feedback. I will update the description in v2.

Regards,
Kun

On 04/06/2021 05:09, Laszlo Ersek wrote:

On 03/27/21 00:41, Kun Qin wrote:

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

Current SMM Save State routine does not check the number of bytes to be
read, when it comse to read IO_INFO, before casting the incoming buffer
to EFI_SMM_SAVE_STATE_IO_INFO. This could potentially cause memory
corruption due to extra bytes are written out of buffer boundary.

This change adds a width check before copying IoInfo into output buffer.

Cc: Eric Dong 
Cc: Ray Ni 
Cc: Laszlo Ersek 
Cc: Rahul Kumar 

Signed-off-by: Kun Qin 
---
  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
index 661cc51f361a..ec760e4c37ca 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
@@ -418,6 +418,13 @@ ReadSaveStateRegister (
return EFI_NOT_FOUND;
  }
  
+//

+// Make sure the incoming buffer is large enough to hold IoInfo before 
accessing
+//
+if (Width < sizeof (EFI_SMM_SAVE_STATE_IO_INFO)) {
+  return EFI_INVALID_PARAMETER;
+}
+
  //
  // Zero the IoInfo structure that will be returned in Buffer
  //



Please update the description of the EFI_INVALID_PARAMETER return code
in the function's leading comment as well.

With that:

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73734): https://edk2.groups.io/g/devel/message/73734
Mute This Topic: https://groups.io/mt/81642500/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] SecurityPkg/Tcg2Smm: Initialize local Status variable

2021-04-06 Thread Michael Kubacki


Hi Laszlo,

I believe this is an actual bug. I updated the commit message to clarify 
this in a v2 patch: https://edk2.groups.io/g/devel/message/73732


Thanks,
Michael

On 4/6/2021 7:29 AM, Laszlo Ersek wrote:

On 03/26/21 01:42, Michael Kubacki wrote:

From: Michael Kubacki 

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

Initializes the Status variable in TcgMmReadyToLock().

Fixes a Clang build failure:
Tcg2Smm.c - SecurityPkg\Tcg\Tcg2Smm\Tcg2Smm.c:254:7: error:
variable 'Status' is used uninitialized whenever 'if'
condition is false [-Werror,-Wsometimes-uninitialized]

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 
Cc: Kun Qin 
Signed-off-by: Michael Kubacki 
---
  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 589c08794bcf..f49eccb0bdf4 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -253,6 +253,8 @@ TcgMmReadyToLock (
  {
EFI_STATUS Status;
  
+  Status = EFI_SUCCESS;

+
if (mReadyToLockHandle != NULL) {
  Status = gMmst->MmiHandlerUnRegister (mReadyToLockHandle);
  mReadyToLockHandle = NULL;



Is this an actual bug in the code, or a clang code analyzer wart?

The commit message should document which one it is.

Furthermore, if the patch is for suppressing an invalid compiler
warning, then please add a comment of the format seen e.g. in commit
aa7596534987 ("MdeModulePkg: Initialize local variable value before they
are used", 2021-03-25).

Thanks
Laszlo








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73733): https://edk2.groups.io/g/devel/message/73733
Mute This Topic: https://groups.io/mt/81617668/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2 1/1] SecurityPkg/Tcg2Smm: Initialize local Status variable

2021-04-06 Thread Michael Kubacki

From: Michael Kubacki 

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

Initializes the Status variable in TcgMmReadyToLock().

Fixes a Clang build failure:
Tcg2Smm.c - SecurityPkg\Tcg\Tcg2Smm\Tcg2Smm.c:254:7: error:
variable 'Status' is used uninitialized whenever 'if'
condition is false [-Werror,-Wsometimes-uninitialized]

Initializing this variable is required to address a practical
scenario in which the return value of TcgMmReadyToLock() is
undefined based on conditional evaluation in the function.

Cc: Jiewen Yao 
Cc: Jian J Wang 
Cc: Qi Zhang 
Cc: Rahul Kumar 
Cc: Kun Qin 
Signed-off-by: Michael Kubacki 
Reviewed-by: Jiewen Yao 
---

Notes:
V2 change:

Clarify in commit message that the issue reported by Clang is not
solely a false positive.

 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 589c08794bcf..f49eccb0bdf4 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -253,6 +253,8 @@ TcgMmReadyToLock (
 {
   EFI_STATUS Status;
 
+  Status = EFI_SUCCESS;
+
   if (mReadyToLockHandle != NULL) {
 Status = gMmst->MmiHandlerUnRegister (mReadyToLockHandle);
 mReadyToLockHandle = NULL;
-- 
2.28.0.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73732): https://edk2.groups.io/g/devel/message/73732
Mute This Topic: https://groups.io/mt/81896951/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [EXTERNAL] Re: [edk2-devel] [GSoC proposal] Secure Image Loader

2021-04-06 Thread Bret Barkelew via groups.io

You’ve got my vote! This sounds amazing!

- Bret

From: Marvin Häuser via groups.io
Sent: Tuesday, April 6, 2021 3:10 AM
To: devel@edk2.groups.io; Desimone, Nathaniel 
L; Laszlo 
Ersek; Andrew Fish; Kinney, 
Michael D
Subject: [EXTERNAL] Re: [edk2-devel] [GSoC proposal] Secure Image Loader

Good day Nate,

Comments are inline.

Best regards,
Marvin

On 06.04.21 11:41, Nate DeSimone wrote:
> Hi Marvin,
>
> Great to meet you and welcome back! Glad you hear you are interested! 
> Completing a formal verification of a PE/COFF loader is certainly impressive. 
> Was this done with some sort of automated theorem proving? It would seem a 
> rather arduous task doing an inductive proof for an algorithm like that by 
> hand!

I would call it "semi-automated", a great deal of intermediate goals
(preconditions, postconditions, invariants, assertions, ...) were
required to show all interesting properties. But yes, the actual proof
steps are automated by common SMT solvers. It was done using the
AstraVer Toolset and ACSL, latter basically a language to express logic
statements with C-like syntax.

> I completely agree with you that getting a formally verified PE/COFF loader 
> into mainline is undoubtably valuable and would pay security dividends for 
> years to come.

I'm glad to hear that. :)

> Admittedly, this is an area of computer science that I don't have a great 
> deal of experience with. The furthest I have gone on this topic is writing 
> out proofs for simple algorithms on exams in my Algorithms class in college. 
> Regardless you have a much better idea of what the current status is of the 
> work that you and Vitaly have done. I guess my only question is do you think 
> there is sufficient work remaining to fill the 10 week GSoC development 
> window?

Please don't get me wrong, but I would be surprised if the UEFI
specification changes I'd like to discuss alone would be completed
within 10 weeks, let alone implementation throughout the codebase. While
I think the plain amount of code may be a bit less than say a
MinPlatform port, the changes are much deeper and require much more
caution to avoid regressions (e.g. by invalidating undocumented
assertions). This sadly is not a matter of just replacing the underlying
library implementation or "plug-in and play" at all. It furthermore
affects many parts of the stack, the core dispatchers used for all
platforms, image emulation (EBC), UEFI userland emulation (EmuPkg), and
so on. I was rather worried the scope is too broad time-wise, but it can
be narrowed/widened as you see fit really. This is one of *the* core
components used on millions of device, and many package maintainers need
to review and validate the changes, this must really be done right the
first try. :)

> Certainly we can use some of that time to perform the code reviews you 
> mention and write up formal ECRs for the UEFI spec changes that you believe 
> are needed.

I believed that was part of the workload, yes, but even without it I
think there is plenty to do.

> Thank you for sending the application and alerting us to the great work you 
> and Vitaly have done! I'll read your paper more closely and come back with 
> any questions I still have.

Thank you, I will gladly explain anything unclear. Just try to not give
Laszlo too many flashbacks. :)

>
> With Best Regards,
> Nate
>
>> -Original Message-
>> From: devel@edk2.groups.io  On Behalf Of Marvin
>> Häuser
>> Sent: Sunday, April 4, 2021 4:02 PM
>> To: devel@edk2.groups.io; Laszlo Ersek ; Andrew Fish
>> ; Kinney, Michael D 
>> Subject: [edk2-devel] [GSoC proposal] Secure Image Loader
>>
>> Good day everyone,
>>
>> I'll keep the introduction brief because I've been around for a while now. 
>> :) I'm
>> Marvin Häuser, a third-year Computer Science student from TU Kaiserslautern,
>> Germany. Late last year, my colleague Vitaly from ISP RAS and me introduced a
>> formally verified Image Loader for UEFI usage at ISP RAS Open[1] due to 
>> various
>> defects we outlined in the corresponding paper. Thank you once again Laszlo
>> for your *incredible* review work on the publication part.
>>
>> I now want to make an effort to mainline it, preferably as part of the 
>> current
>> Google Summer of Code event. To be clear, my internship at ISP RAS has
>> concluded, and while Vitaly will be available for design discussion, he has 
>> other
>> priorities at the moment and the practical part will be on me. I have 
>> previously
>> submitted a proposal via the GSoC website for your review.
>>
>> There are many things to consider:
>> 1. The Image Loader is a core component, and there needs to be a significant
>> level of quality and security assurance.
>> 2. Being consumed by many packages, the proposed patch set will take a lot of
>> time to review and

Re: [edk2-devel] VirtIO Sound Driver (GSoC 2021)

2021-04-06 Thread Ethin Probst

I'll attach the bug for the build tools to the BZ shortly.
Laszlo, thanks for that. I don't know their email addresses though.
And yes, I was going to make it device independent, as the majority
(if not all) of UEFI protocols are.

On 4/6/21, Laszlo Ersek  wrote:
> On 03/31/21 08:41, Nate DeSimone wrote:
>> Another option is to put the protocol definition in MdeModulePkg and
>> mark it with the EDKII_ prefix. For my last “code first” UEFI spec
>> contribution I did this with the PPI that added up getting added.
>
> The new audio protocol should be generic, only its implementation in
> question should be virtio specific.
>
> Please include Gerd Hoffmann (CC'd) in the protocol design, as well as
> the developers of the virtio-sound device model in QEMU.
>
> Thanks
> Laszlo
>
>
>>
>>
>>
>> Thanks,
>>
>> Nate
>>
>>
>>
>> *From: * on behalf of "Andrew Fish via groups.io"
>> 
>> *Reply-To: *"devel@edk2.groups.io" ,
>> "af...@apple.com" 
>> *Date: *Tuesday, March 30, 2021 at 10:54 PM
>> *To: *edk2-devel-groups-io ,
>> "harlydavid...@gmail.com" 
>> *Cc: *Rafael Rodrigues Machado 
>> *Subject: *Re: [edk2-devel] VirtIO Sound Driver (GSoC 2021)
>>
>>
>>
>>
>>
>> On Mar 30, 2021, at 5:01 PM, Ethin Probst > > wrote:
>>
>>
>>
>> I'm wondering where exactly I should add the VirtIO sound protocol. I
>> just familiarized myself with the build system and am about to test
>> it
>> by building OVMF if possible, but I'm wondering where I should
>> actually put the protocol and all that stuff. Maybe there's
>> documentation I've missed as well.
>>
>>
>>
>> Ethin,
>>
>>
>>
>> For the driver I’d match the patter of OVMF [1] and use
>> OvmfPkg/VirtioSoundDxe/. Maybe even use one of the other drivers as a
>> template.
>>
>>
>>
>> The protocol is more of a public thing. I think eventually we would like
>> to publish the protocol in the UEFI Spec (I can help with that part) and
>> that would mean we put the Protocol definition in
>> MdePkg/Include/Protocol, but we don’t want to do that before it is
>> standardized as that causes compatibility issues. So this is a “code
>> first project” (code prototype and then contribute to the UEFI Forum for
>> inclusion in the specification) so we need to follow some code first
>> rules that I don’t remember of the top of my head? So why not start out
>> the protocol definition OvmfPkg/Include/Protocol. You can also add a
>> test application looks like you can just use the root [2] of OVMF for
>> that. That way the project is not blocked.
>>
>>
>>
>> We can have a conversation on the mailing list about better places to
>> put stuff, and it should be easy enough to move stuff around if
>> everything else is working.
>>
>>
>>
>> [1] find OvmfPkg  -iname '*Virtio*.inf'
>>
>> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
>>
>> OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
>>
>> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
>>
>> OvmfPkg/Library/VirtioLib/VirtioLib.inf
>>
>> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
>>
>> OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
>>
>> OvmfPkg/Virtio10Dxe/Virtio10.inf
>>
>> OvmfPkg/VirtioNetDxe/VirtioNet.inf
>>
>> OvmfPkg/VirtioRngDxe/VirtioRng.inf
>>
>>
>>
>> [2] /Volumes/Case/edk2-github/OvmfPkg>git grep APPLICATION -- *.inf |
>> grep MODULE_TYPE
>>
>> EnrollDefaultKeys/EnrollDefaultKeys.inf:13:  MODULE_TYPE
>>     = UEFI_APPLICATION
>>
>>
>>
>> Thanks,
>>
>>
>>
>> Andrew Fish
>>
>>
>>
>>
>>
>>
>> On 3/30/21, Ethin Probst via groups.io 
>> > > wrote:
>>
>> I agree. Plus, it gives me a chance to finally learn the EDK2
>> build
>> system and how it works! I've been working on a hobby OS as a
>> side
>> project and, though learning from other code examples from OSes
>> is
>> fun, I have to say that learning from the firmware code like from
>> SeaBIOS has been some of the most enlightening and interesting
>> times
>> thus far.
>> Thanks for the link to your code, Rafael; once I get virtIO
>> support
>> in, I can work on HDA support, though I might tackle USB support
>> second and HDA third. We'll see, but VirtIO definitely is coming
>> first.
>>
>> As I said before, I look forward to working with all of you
>> wonderful
>> people!
>>
>> On 3/30/21, Rafael Rodrigues Machado
>> > >
>> wrote:
>>
>> This would be amazing so people can continue my work related
>> to
>> accessibility at BIOS. Something desired by the blind people
>> since the
>> 90's
>> Just for reference, this is what I have done:
>>
>>
>> https://github.com/RafaelRMachado/Msc_UefiHda_PreOs_Accessibility
>>
>> Thanks
>> Rafael
>>
>> Em seg, 29 de mar de 2021 20:24, Ethin Probst
>> 
>>

Re: [edk2-devel] [PATCH v1 1/1] SecurityPkg/Tcg2Smm: Initialize local Status variable

2021-04-06 Thread Laszlo Ersek

On 03/26/21 01:42, Michael Kubacki wrote:
> From: Michael Kubacki 
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3277
> 
> Initializes the Status variable in TcgMmReadyToLock().
> 
> Fixes a Clang build failure:
> Tcg2Smm.c - SecurityPkg\Tcg\Tcg2Smm\Tcg2Smm.c:254:7: error:
> variable 'Status' is used uninitialized whenever 'if'
> condition is false [-Werror,-Wsometimes-uninitialized]
> 
> Cc: Jiewen Yao 
> Cc: Jian J Wang 
> Cc: Qi Zhang 
> Cc: Rahul Kumar 
> Cc: Kun Qin 
> Signed-off-by: Michael Kubacki 
> ---
>  SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
> b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> index 589c08794bcf..f49eccb0bdf4 100644
> --- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> +++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
> @@ -253,6 +253,8 @@ TcgMmReadyToLock (
>  {
>EFI_STATUS Status;
>  
> +  Status = EFI_SUCCESS;
> +
>if (mReadyToLockHandle != NULL) {
>  Status = gMmst->MmiHandlerUnRegister (mReadyToLockHandle);
>  mReadyToLockHandle = NULL;
> 

Is this an actual bug in the code, or a clang code analyzer wart?

The commit message should document which one it is.

Furthermore, if the patch is for suppressing an invalid compiler
warning, then please add a comment of the format seen e.g. in commit
aa7596534987 ("MdeModulePkg: Initialize local variable value before they
are used", 2021-03-25).

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73729): https://edk2.groups.io/g/devel/message/73729
Mute This Topic: https://groups.io/mt/81617668/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM shadow stack overflow

2021-04-06 Thread Laszlo Ersek

Ray,

On 03/29/21 07:13, Yao, Jiewen wrote:
> Thank you very much!
> 
> Reviewed-by: Jiewen Yao 

can you please review and merge this patch? You were the UefiCpuPkg
reviewer on the following two commits as well:

3eb69b081c68 ("UefiCpuPkg/PiSmmCpu: Add Shadow Stack Support for X86
SMM.", 2019-02-28)

ef91b07388e1 ("UefiCpuPkg/PiSmmCpuDxeSmm: Fix SMM stack offset is not
correct", 2021-03-02)

Thanks
Laszlo

> 
>> -Original Message-
>> From: Sheng, W 
>> Sent: Friday, March 26, 2021 2:33 PM
>> To: Yao, Jiewen ; devel@edk2.groups.io
>> Cc: Dong, Eric ; Ni, Ray ; Laszlo 
>> Ersek
>> ; Kumar, Rahul1 ; Feng, Roger
>> 
>> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
>> shadow stack overflow
>>
>> Hi Jiewen,
>> In current code, if SMM stack guard is enabled, there is a guard page at the 
>> top
>> of SMM shadow stack.
>> If SMM shadow stack overflow Happens, it will touch the guard page, and
>> trigger the #PF exception.
>> In this patch, I will check the PFAddress in SmiPFHandler(), if it belongs 
>> to the
>> range of SMM shadow stack guard page, I will show the error message.
>>
>> unit test:
>> I use recursive function to do the test. In each function call, it will push 
>> the
>> return address to the SMM shadow stack.
>> When the loop reaches to a certain amount, it will finally touch the guard 
>> page,
>> and trigger #PF exception.
>>
>> Thank you
>> BR
>> Sheng Wei
>>
>>> -Original Message-
>>> From: Yao, Jiewen 
>>> Sent: 2021年3月26日 14:14
>>> To: Sheng, W ; devel@edk2.groups.io
>>> Cc: Dong, Eric ; Ni, Ray ; Laszlo
>>> Ersek ; Kumar, Rahul1 ;
>>> Feng, Roger 
>>> Subject: RE: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
>>> shadow stack overflow
>>>
>>> Hi
>>> Would you please share the info on how you do unit test for the new added
>>> code?
>>>
>>> Thank you
>>>
 -Original Message-
 From: Sheng, W 
 Sent: Friday, March 26, 2021 2:04 PM
 To: devel@edk2.groups.io
 Cc: Dong, Eric ; Ni, Ray ;
 Laszlo Ersek ; Kumar, Rahul1
 ; Yao, Jiewen ; Feng,
 Roger 
 Subject: [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: Support detect SMM
>>> shadow
 stack overflow

 Use SMM stack guard feature to detect SMM shadow stack overflow.

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

 Signed-off-by: Sheng Wei 
 Cc: Eric Dong 
 Cc: Ray Ni 
 Cc: Laszlo Ersek 
 Cc: Rahul Kumar 
 Cc: Jiewen Yao 
 Cc: Roger Feng 
 ---
  UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
 b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
 index 07e7ea70de..6902584b1f 100644
 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
 +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
 @@ -1016,6 +1016,7 @@ SmiPFHandler (
  {
UINTN PFAddress;
UINTN GuardPageAddress;
 +  UINTN ShadowStackGuardPageAddress;
UINTN CpuIndex;

ASSERT (InterruptType == EXCEPT_IA32_PAGE_FAULT); @@ -1032,7
 +1033,7 @@ SmiPFHandler (
}

//
 -  // If a page fault occurs in SMRAM range, it might be in a SMM
 stack guard page,
 +  // If a page fault occurs in SMRAM range, it might be in a SMM
 + stack/shadow
 stack guard page,
// or SMM page protection violation.
//
if ((PFAddress >= mCpuHotPlugData.SmrrBase) && @@ -1040,10 +1041,16
 @@ SmiPFHandler (
  DumpCpuContext (InterruptType, SystemContext);
  CpuIndex = GetCpuIndex ();
  GuardPageAddress = (mSmmStackArrayBase + EFI_PAGE_SIZE +
>>> CpuIndex
 * (mSmmStackSize + mSmmShadowStackSize));
 +ShadowStackGuardPageAddress = (mSmmStackArrayBase +
>>> mSmmStackSize
 + EFI_PAGE_SIZE + CpuIndex * (mSmmStackSize +
>>> mSmmShadowStackSize));
  if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
  (PFAddress >= GuardPageAddress) &&
  (PFAddress < (GuardPageAddress + EFI_PAGE_SIZE))) {
DEBUG ((DEBUG_ERROR, "SMM stack overflow!\n"));
 +} else if ((FeaturePcdGet (PcdCpuSmmStackGuard)) &&
 +(mSmmShadowStackSize > 0) &&
 +(PFAddress >= ShadowStackGuardPageAddress) &&
 +(PFAddress < (ShadowStackGuardPageAddress + EFI_PAGE_SIZE))) {
 +  DEBUG ((DEBUG_ERROR, "SMM shadow stack overflow!\n"));
  } else {
if ((SystemContext.SystemContextX64->ExceptionData &
 IA32_PF_EC_ID) !=
 0) {
  DEBUG ((DEBUG_ERROR, "SMM exception at execution (0x%lx)\n",
 PFAddress));
 --
 2.16.2.windows.1
> 
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73728): https://edk2.groups.io/g/devel/message/73728
Mute This Topic: https://groups.io/mt/81621994/21656
Group Owner: devel+ow...@edk2.groups.io

Re: [edk2-devel] Enabling Secure boot

2021-04-06 Thread Laszlo Ersek

On 04/01/21 11:03, sent...@gmail.com wrote:
> Hi,
>  I have enable the secure boot for CorebootPayloadPkg in EDK 2017 and
> got the secure boot configuration in the boot menu. But the problem is
> Attempt secure boot is disabled. Also when I changed from standard mode
> to custom mode to add vmware key in the db, after reset its not getting
> saved. This may due to NVRAM support is not there.
> 
> How to make "Attempt secure boot" to be enabled?
> If NVRAM is not there, how i will add vmware keys in db database?
> Can i hardcode the keys in the edk2 source and secure boot? If so where
> to modify it?

Secure boot is based on authenticated non-volatile UEFI variables that
are described by the UEFI spec. If you don't have functional,
tamper-proof storage on your platform (virtual or otherwise) for said
non-volatile UEFI variables, secure boot will either not work, or will
not be secure in fact. (By "tamper-proof", I mean that e.g. the
operating system must be prevented from modifying said variables, unless
it invokes the appropriate UEFI runtime services.)

I don't know how this specifically applies to CorebootPayloadPkg though.

Thanks
Laszlo

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73727): https://edk2.groups.io/g/devel/message/73727
Mute This Topic: https://groups.io/mt/81789296/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] VirtIO Sound Driver (GSoC 2021)

2021-04-06 Thread Laszlo Ersek

On 03/31/21 08:41, Nate DeSimone wrote:
> Another option is to put the protocol definition in MdeModulePkg and
> mark it with the EDKII_ prefix. For my last “code first” UEFI spec
> contribution I did this with the PPI that added up getting added.

The new audio protocol should be generic, only its implementation in
question should be virtio specific.

Please include Gerd Hoffmann (CC'd) in the protocol design, as well as
the developers of the virtio-sound device model in QEMU.

Thanks
Laszlo


> 
>  
> 
> Thanks,
> 
> Nate
> 
>  
> 
> *From: * on behalf of "Andrew Fish via groups.io"
> 
> *Reply-To: *"devel@edk2.groups.io" ,
> "af...@apple.com" 
> *Date: *Tuesday, March 30, 2021 at 10:54 PM
> *To: *edk2-devel-groups-io ,
> "harlydavid...@gmail.com" 
> *Cc: *Rafael Rodrigues Machado 
> *Subject: *Re: [edk2-devel] VirtIO Sound Driver (GSoC 2021)
> 
>  
> 
> 
> 
> On Mar 30, 2021, at 5:01 PM, Ethin Probst  > wrote:
> 
>  
> 
> I'm wondering where exactly I should add the VirtIO sound protocol. I
> just familiarized myself with the build system and am about to test it
> by building OVMF if possible, but I'm wondering where I should
> actually put the protocol and all that stuff. Maybe there's
> documentation I've missed as well.
> 
>  
> 
> Ethin,
> 
>  
> 
> For the driver I’d match the patter of OVMF [1] and use
> OvmfPkg/VirtioSoundDxe/. Maybe even use one of the other drivers as a
> template. 
> 
>  
> 
> The protocol is more of a public thing. I think eventually we would like
> to publish the protocol in the UEFI Spec (I can help with that part) and
> that would mean we put the Protocol definition in
> MdePkg/Include/Protocol, but we don’t want to do that before it is
> standardized as that causes compatibility issues. So this is a “code
> first project” (code prototype and then contribute to the UEFI Forum for
> inclusion in the specification) so we need to follow some code first
> rules that I don’t remember of the top of my head? So why not start out
> the protocol definition OvmfPkg/Include/Protocol. You can also add a
> test application looks like you can just use the root [2] of OVMF for
> that. That way the project is not blocked. 
> 
>  
> 
> We can have a conversation on the mailing list about better places to
> put stuff, and it should be easy enough to move stuff around if
> everything else is working.  
> 
>  
> 
> [1] find OvmfPkg  -iname '*Virtio*.inf'
> 
> OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf
> 
> OvmfPkg/VirtioScsiDxe/VirtioScsi.inf
> 
> OvmfPkg/Library/VirtioMmioDeviceLib/VirtioMmioDeviceLib.inf
> 
> OvmfPkg/Library/VirtioLib/VirtioLib.inf
> 
> OvmfPkg/VirtioGpuDxe/VirtioGpu.inf
> 
> OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
> 
> OvmfPkg/Virtio10Dxe/Virtio10.inf
> 
> OvmfPkg/VirtioNetDxe/VirtioNet.inf
> 
> OvmfPkg/VirtioRngDxe/VirtioRng.inf
> 
> 
> 
> [2] /Volumes/Case/edk2-github/OvmfPkg>git grep APPLICATION -- *.inf |
> grep MODULE_TYPE
> 
> EnrollDefaultKeys/EnrollDefaultKeys.inf:13:  MODULE_TYPE               
>     = UEFI_APPLICATION
> 
>  
> 
> Thanks,
> 
>  
> 
> Andrew Fish
> 
>  
> 
> 
> 
> 
> On 3/30/21, Ethin Probst via groups.io 
>  > wrote:
> 
> I agree. Plus, it gives me a chance to finally learn the EDK2 build
> system and how it works! I've been working on a hobby OS as a side
> project and, though learning from other code examples from OSes is
> fun, I have to say that learning from the firmware code like from
> SeaBIOS has been some of the most enlightening and interesting times
> thus far.
> Thanks for the link to your code, Rafael; once I get virtIO support
> in, I can work on HDA support, though I might tackle USB support
> second and HDA third. We'll see, but VirtIO definitely is coming
> first.
> 
> As I said before, I look forward to working with all of you
> wonderful
> people!
> 
> On 3/30/21, Rafael Rodrigues Machado
>  >
> wrote:
> 
> This would be amazing so people can continue my work related to
> accessibility at BIOS. Something desired by the blind people
> since the
> 90's
> Just for reference, this is what I have done:
> 
> https://github.com/RafaelRMachado/Msc_UefiHda_PreOs_Accessibility
> 
> Thanks
> Rafael
> 
> Em seg, 29 de mar de 2021 20:24, Ethin Probst
> 
> escreveu:
> 
> 
> Hello everyone,
> 
> This is the first time I've ever contributed to EDK2. As
> part of GSoC
> 2021, I have submitted a proposal to implement a UEFI
> audio output
> protocol that will utilize the VirtIO sound driver. I've
>

[edk2-devel] [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi: Fix mini UART clock divisor calculation

2021-04-06 Thread Mario Bălănică

The VPU clock divisor has changed in this commit:
https://github.com/raspberrypi/firmware/commit/1e5456a,
thus breaking the mini UART clock divisor calculation on the Pi 4.

Fix this by reading the core clock from the mailbox instead.

Signed-off-by: Mario Bălănică 
---
 Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c   | 15 
+++
 Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S |  4 
 Platform/RaspberryPi/RPi4/RPi4.dsc   |  6 
--
 3 files changed, 7 insertions(+), 18 deletions(-)

diff --git a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c 
b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
index 5e83bbf022eb..d2f983bf0a9f 100644
--- a/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
+++ b/Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.c
@@ -34,25 +34,16 @@ SerialPortGetDivisor (
   UINT32  SerialBaudRate

 )

 {

-  UINT64  BaseClockRate;

+  UINT32  BaseClockRate;

   UINT32  Divisor;

 

-  //

-  // On the Raspberry Pi, the clock to use for the 16650-compatible UART

-  // is the base clock divided by the 12.12 fixed point VPU clock divisor.

-  //

-  BaseClockRate = (UINT64)PcdGet32 (PcdSerialClockRate);

-#if (RPI_MODEL == 4)

-  Divisor = MmioRead32(BCM2836_CM_BASE + BCM2836_CM_VPU_CLOCK_DIVISOR) & 
0xFF;

-  if (Divisor != 0)

-BaseClockRate = (BaseClockRate << 12) / Divisor;

-#endif

+  BaseClockRate = PcdGet32 (PcdSerialClockRate);

 

   //

   // As per the BCM2xxx datasheets:

   // baudrate = system_clock_freq / (8 * (divisor + 1)).

   //

-  Divisor = (UINT32)BaseClockRate / (SerialBaudRate * 8);

+  Divisor = BaseClockRate / (SerialBaudRate * 8);

   if (Divisor != 0) {

 Divisor--;

   }

diff --git 
a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S 
b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
index 58351e4fb8cc..7008aaf8aa4c 100644
--- a/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
+++ b/Platform/RaspberryPi/Library/PlatformLib/AArch64/RaspberryPiHelper.S
@@ -85,13 +85,11 @@ ASM_FUNC (ArmPlatformPeiBootAction)
 adr x2, mBoardRevision

 str w0, [x2]

 

-#if (RPI_MODEL == 3)

 run .Lclkinfo_buffer

 

 ldr w0, .Lfrequency

 adr x2, _gPcd_BinaryPatch_PcdSerialClockRate

 str w0, [x2]

-#endif

 

 ret

 

@@ -135,7 +133,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
 .long   0   // end tag

 .set.Lrevinfo_size, . - .Lrevinfo_buffer

 

-#if (RPI_MODEL == 3)

 .align  4

 .Lclkinfo_buffer:

 .long   .Lclkinfo_size

@@ -148,7 +145,6 @@ ASM_FUNC (ArmPlatformPeiBootAction)
 .long   0   // frequency

 .long   0   // end tag

 .set.Lclkinfo_size, . - .Lclkinfo_buffer

-#endif

 

 //UINTN

 //ArmPlatformGetPrimaryCoreMpId (

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc 
b/Platform/RaspberryPi/RPi4/RPi4.dsc
index 2c05c31118d2..ff802d8347ea 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -429,7 +429,6 @@ [PcdsFixedAtBuild.common]
   gArmPlatformTokenSpaceGuid.PL011UartClkInHz|4800

   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseMmio|TRUE

   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterStride|4

-  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|10

   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialFifoControl|0x27

   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialExtendedTxFifoSize|8

   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200

@@ -465,6 +464,9 @@ [PcdsFixedAtBuild.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor|L"EDK2"

   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE

 

+[PcdsPatchableInModule]

+  gEfiMdeModulePkgTokenSpaceGuid.PcdSerialClockRate|5

+

 [PcdsDynamicHii.common.DEFAULT]

 

   #

@@ -621,7 +623,7 @@ [Components.common]
   MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf

   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf {

 

-  
SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortLib.inf

+  
SerialPortLib|Platform/RaspberryPi/Library/DualSerialPortLib/DualSerialPortDxeLib.inf

   }

   Platform/RaspberryPi/Drivers/DisplayDxe/DisplayDxe.inf

   EmbeddedPkg/Drivers/ConsolePrefDxe/ConsolePrefDxe.inf

-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73725): https://edk2.groups.io/g/devel/message/73725
Mute This Topic: https://groups.io/mt/81890065/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH V2 0/2] Enable SMMUv3 for Arm SGI/RD platforms

2021-04-06 Thread Sami Mujawar

Hi Vivek,

Thanks for this patch series.
Pushed as: bc8a8b16bd4b..7fe9704893f1

Regards,

Sami Mujawar


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73724): https://edk2.groups.io/g/devel/message/73724
Mute This Topic: https://groups.io/mt/81102021/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

2021-04-06 Thread Laszlo Ersek

On 04/06/21 10:11, Xu, Min M wrote:
> Hi, Singh
> I have a concern about the sevSnpBlock in ResetVectorVtf0.asm. Actually
> SEV has inserted 3 blocks in ResetVectorVtf0.asm and the total bytes are
> (26 + 22 + 20 = 68 bytes). If sevSnpBlock is added, then the total bytes
> will be (68 +26 = 94 bytes).
> 
> I am not sure whether there will be more blocks added in
> ResetVectorVtf0.asm in the future. But I don't think ResetVectorVtf0.asm
> is a good place to add these data blobs. Can these data be packed into a
> single file, for example, SevMetadata.asm, then a pointer is inserted in
> ResetVectorVtf0.asm which then points to the SevMetadata. In this way we
> can keep ResetVectorVtf0.asm clean, small and straight forward.
> 
> Another reason is that I am working on the Intel TDX which will update
> the ResetVectorVtf0.asm as well. My change depends on the assumption that
> the distance between ResetVector(0xfff0) and EarlyBspInitReal16 is
> less than 128 bytes. The blocks in ResetVectorVtf0.asm make it impossible.

That's a problem. These info blocks are placed in the reset vector
because then they can be found by QEMU easily -- they are not
compressed, and they appear at a known location in the guest physical
address space. (More precisely, a GUID-ed structure chain starts at a
known location, and then QEMU can traverse the chain of structures, for
learning various bits of information about the firmware.)

Do we absolutely need a short jump?

Thanks
Laszlo

> 
> Thanks!
> 
>> -Original Message-
>> From: Brijesh Singh 
>> Sent: Wednesday, March 24, 2021 11:32 PM
>> To: devel@edk2.groups.io
>> Cc: Brijesh Singh ; James Bottomley
>> ; Xu, Min M ; Yao, Jiewen
>> ; Tom Lendacky ;
>> Justen, Jordan L ; Ard Biesheuvel
>> ; Laszlo Ersek 
>> Subject: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for
>> the SEV-SNP guest
>>
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
>>
>> During the SEV-SNP guest launch sequence, two special pages need to be
>> inserted, the secrets page and cpuid page. The secrets page, contain the VM
>> platform communication keys. The guest BIOS and OS can use this key to
>> communicate with the SEV firmware to get the attestation report. The Cpuid
>> page, contain the CPUIDs entries filtered through the AMD-SEV firmware.
>>
>> The VMM will locate the secrets and cpuid page addresses through a fixed
>> GUID and pass them to SEV firmware to populate further.
>> For more information about the page content, see the SEV-SNP spec.
>>
>> To simplify the pre-validation range calculation in the next patch, the CPUID
>> and Secrets pages are moved to the start of the MEMFD_BASE_ADDRESS.
>>
>> Cc: James Bottomley 
>> Cc: Min Xu 
>> Cc: Jiewen Yao 
>> Cc: Tom Lendacky 
>> Cc: Jordan Justen 
>> Cc: Ard Biesheuvel 
>> Cc: Laszlo Ersek 
>> Signed-off-by: Brijesh Singh 
>> ---
>>  OvmfPkg/OvmfPkg.dec  |  8 +++
>>  OvmfPkg/OvmfPkgX64.fdf   | 24 
>>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 19 
>>  OvmfPkg/ResetVector/ResetVector.inf  |  4 
>>  OvmfPkg/ResetVector/ResetVector.nasmb|  2 ++
>>  5 files changed, 48 insertions(+), 9 deletions(-)
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
>> 4348bb45c6..062926772d 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -317,6 +317,14 @@
>>gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
>>gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
>>
>> +  ## The base address of the CPUID page used by SEV-SNP
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|0|UINT32|0x48
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize|0|UINT32|0x49
>> +
>> +  ## The base address of the Secrets page used by SEV-SNP
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|0|UINT32|0x50
>> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize|0|UINT32|0x51
>> +
>>  [PcdsDynamic, PcdsDynamicEx]
>>gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>>
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN
>> |0x10
>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index
>> d519f85328..ea214600be 100644
>> --- a/OvmfPkg/OvmfPkgX64.fdf
>> +++ b/OvmfPkg/OvmfPkgX64.fdf
>> @@ -67,27 +67,33 @@ ErasePolarity = 1
>>  BlockSize = 0x1
>>  NumBlocks = 0xD0
>>
>> -0x00|0x006000
>> +0x00|0x001000
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|gUefiOvmfPkgTokenS
>> paceGu
>> +id.PcdOvmfSnpCpuidSize
>> +
>> +0x001000|0x001000
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgToken
>> Space
>> +Guid.PcdOvmfSnpSecretsSize
>> +
>> +0x002000|0x006000
>>
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTok
>> enSpaceGuid.PcdOvmfSecPageTablesSize
>>
>> -0x006000|0x001000
>> +0x008000|0x001000
>>
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTo
>>

Re: [edk2-devel] UEFI spec version in system table, shall we update it to 2.8?

2021-04-06 Thread Nhi Pham via groups.io

+ Liming Gao

I'm also interested in the plan for the edk2 in compliance with UEFI 2.8 
specification.

-Nhi


From: devel@edk2.groups.io  on behalf of Lin, Derek (HPS 
SW) via groups.io 
Sent: Wednesday, March 31, 2021 12:52 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] UEFI spec version in system table, shall we update it to 
2.8?

Hi,

In UefiSpec.h, we define the UEFI version 2.7 in system table.


#define EFI_2_70_SYSTEM_TABLE_REVISION  ((2 << 16) | (70))
#define EFI_SYSTEM_TABLE_REVISION   EFI_2_70_SYSTEM_TABLE_REVISION
#define EFI_SPECIFICATION_VERSION   EFI_SYSTEM_TABLE_REVISION
#define EFI_RUNTIME_SERVICES_REVISION   EFI_SPECIFICATION_VERSION

In Linux dmesg
[0.00] efi: EFI v2.70 by EDK II

Shall it be updated to v2.8 to align with UEFI spec?

Thanks,
Derek



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73722): https://edk2.groups.io/g/devel/message/73722
Mute This Topic: https://groups.io/mt/81743707/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v1 1/1] UefiCpuPkg: PiSmmCpuDxeSmm: Check buffer size before accessing

2021-04-06 Thread Laszlo Ersek

On 03/27/21 00:41, Kun Qin wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3283
> 
> Current SMM Save State routine does not check the number of bytes to be
> read, when it comse to read IO_INFO, before casting the incoming buffer
> to EFI_SMM_SAVE_STATE_IO_INFO. This could potentially cause memory
> corruption due to extra bytes are written out of buffer boundary.
> 
> This change adds a width check before copying IoInfo into output buffer.
> 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Laszlo Ersek 
> Cc: Rahul Kumar 
> 
> Signed-off-by: Kun Qin 
> ---
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c 
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> index 661cc51f361a..ec760e4c37ca 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmramSaveState.c
> @@ -418,6 +418,13 @@ ReadSaveStateRegister (
>return EFI_NOT_FOUND;
>  }
>  
> +//
> +// Make sure the incoming buffer is large enough to hold IoInfo before 
> accessing
> +//
> +if (Width < sizeof (EFI_SMM_SAVE_STATE_IO_INFO)) {
> +  return EFI_INVALID_PARAMETER;
> +}
> +
>  //
>  // Zero the IoInfo structure that will be returned in Buffer
>  //
> 

Please update the description of the EFI_INVALID_PARAMETER return code
in the function's leading comment as well.

With that:

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73721): https://edk2.groups.io/g/devel/message/73721
Mute This Topic: https://groups.io/mt/81642500/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH 0/4] UefiCpuPkg: Add MicrocodeLib for loading microcode

2021-04-06 Thread Laszlo Ersek

Hi Ray,

On 04/02/21 07:58, Ni, Ray wrote:
> The patch set creates a new MicrocodeLib for loading microcode.
> Then updates all platforms to include this lib in DSC.
> Then updates the MpInitLib to consume this lib.
> 
> Edk2-platforms change will be sent out in a separate patch set.
> 
> 
> Ray Ni (4):
>   UefiCpuPkg: Add MicrocodeLib for loading microcode
>   OvmfPkg: Add MicrocodeLib in DSC files.
>   UefiPayloadPkg/UefiPayloadPkg.dsc: Consume MicrocodeLib
>   UefiCpuPkg/MpInitLib: Consume MicrocodeLib to remove duplicated code
> 
>  OvmfPkg/AmdSev/AmdSevX64.dsc  |   1 +
>  OvmfPkg/Bhyve/BhyveX64.dsc|   1 +
>  OvmfPkg/OvmfPkgIa32.dsc   |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc|   1 +
>  OvmfPkg/OvmfPkgX64.dsc|   1 +
>  OvmfPkg/OvmfXen.dsc   |   1 +
>  UefiCpuPkg/Include/Library/MicrocodeLib.h | 120 +
>  .../Library/MicrocodeLib/MicrocodeLib.c   | 322 
>  .../Library/MicrocodeLib/MicrocodeLib.inf |  32 ++
>  UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |   1 +
>  UefiCpuPkg/Library/MpInitLib/Microcode.c  | 484 --
>  UefiCpuPkg/Library/MpInitLib/MpLib.h  |   1 +
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |   1 +
>  UefiCpuPkg/UefiCpuPkg.dec |   5 +-
>  UefiCpuPkg/UefiCpuPkg.dsc |   1 +
>  UefiPayloadPkg/UefiPayloadPkg.dsc |   1 +
>  16 files changed, 582 insertions(+), 392 deletions(-)
>  create mode 100644 UefiCpuPkg/Include/Library/MicrocodeLib.h
>  create mode 100644 UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.c
>  create mode 100644 UefiCpuPkg/Library/MicrocodeLib/MicrocodeLib.inf
> 

(1) I think we should use a new TianoCore feature request BZ for this
feature, and the commit messages should link it. (I understand the
library only factors out existent logic, but still.)

(2) As I understand it, a platform can provide microcode in three ways:
- via the "microcode patch" GUIDed HOB (PEI and DXE phases both),
- via the "shadow microcode" PPI (PEI phase only),
- via the PcdCpuMicrocodePatch* PCDs (PEI and DXE phases both).

If a platform uses none of these methods (for example, OVMF does not),
then I think it would benefit from a Null instance of the new
MicrocodeLib class.

Would you consider introducing a Null instance too, and using that one
in the OVMF DSC files?

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73720): https://edk2.groups.io/g/devel/message/73720
Mute This Topic: https://groups.io/mt/81796730/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [edk2-platforms][PATCH 1/1] Platform/RaspberryPi: Fix mini UART baud divisor calculation

2021-04-06 Thread Pete Batard


Hi Mario,

Please re-send a v2 with a commit message that explains the issue that 
is being fixed. Thanks.


For the record, I ran some tests that show that the divisor was changed 
from 1 to 2 with the latest Pi Foundation firmware (which is confirmed 
by the fact that if you switch your baudrate from 115200 to 57600, 
you'll get the expected output).


I'm not sure what game the Pi Foundation are playing at this stage, coz 
they sure don't seem to be following any official logic with how they 
actually set the serial clock, and our attempts at second guessing that 
logic are no longer working. So I guess the best we can do is go with 
whatever gets us output for the latest firmware, and hope they're not 
going to break that anytime soon (which is still better than the TF-A 
approach, where they got so fed up with this cat and mouse game that 
they dropped UART initialization altogether [1])


Regards,

/Pete

[1] 
https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/commit/plat/rpi?h=v2.3=0eda713b9bd65222155900aacf3a67805351f88f


On 2021.04.03 23:45, Mario Bălănică wrote:
Okay, I've tested the patch with the firmware from the commit mentioned 
above and it doesn't work. The VPU clock divisor bit needs 
PcdSerialClockRate to be set to 1 GHz (so that the quotient becomes 
equal to the core clock).


The mailbox on the other hand always returns 200 MHz for the core clock. 
This is definitely a bug, which got fixed it in a later commit.


There isn't any reliable way to support the firmwares with a broken 
mailbox interface as well in this case, and I really doubt anyone would 
want to use them with the UEFI in the first place. Your build scripts 
always include the latest version.


I can resend the patch with a more detailed commit message if you wish.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73719): https://edk2.groups.io/g/devel/message/73719
Mute This Topic: https://groups.io/mt/81808942/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [GSoC proposal] Secure Image Loader

2021-04-06 Thread Marvin Häuser


Good day Nate,

Comments are inline.

Best regards,
Marvin

On 06.04.21 11:41, Nate DeSimone wrote:

Hi Marvin,

Great to meet you and welcome back! Glad you hear you are interested! 
Completing a formal verification of a PE/COFF loader is certainly impressive. 
Was this done with some sort of automated theorem proving? It would seem a 
rather arduous task doing an inductive proof for an algorithm like that by hand!


I would call it "semi-automated", a great deal of intermediate goals 
(preconditions, postconditions, invariants, assertions, ...) were 
required to show all interesting properties. But yes, the actual proof 
steps are automated by common SMT solvers. It was done using the 
AstraVer Toolset and ACSL, latter basically a language to express logic 
statements with C-like syntax.



I completely agree with you that getting a formally verified PE/COFF loader 
into mainline is undoubtably valuable and would pay security dividends for 
years to come.


I'm glad to hear that. :)


Admittedly, this is an area of computer science that I don't have a great deal 
of experience with. The furthest I have gone on this topic is writing out 
proofs for simple algorithms on exams in my Algorithms class in college. 
Regardless you have a much better idea of what the current status is of the 
work that you and Vitaly have done. I guess my only question is do you think 
there is sufficient work remaining to fill the 10 week GSoC development window?


Please don't get me wrong, but I would be surprised if the UEFI 
specification changes I'd like to discuss alone would be completed 
within 10 weeks, let alone implementation throughout the codebase. While 
I think the plain amount of code may be a bit less than say a 
MinPlatform port, the changes are much deeper and require much more 
caution to avoid regressions (e.g. by invalidating undocumented 
assertions). This sadly is not a matter of just replacing the underlying 
library implementation or "plug-in and play" at all. It furthermore 
affects many parts of the stack, the core dispatchers used for all 
platforms, image emulation (EBC), UEFI userland emulation (EmuPkg), and 
so on. I was rather worried the scope is too broad time-wise, but it can 
be narrowed/widened as you see fit really. This is one of *the* core 
components used on millions of device, and many package maintainers need 
to review and validate the changes, this must really be done right the 
first try. :)



Certainly we can use some of that time to perform the code reviews you mention 
and write up formal ECRs for the UEFI spec changes that you believe are needed.


I believed that was part of the workload, yes, but even without it I 
think there is plenty to do.



Thank you for sending the application and alerting us to the great work you and 
Vitaly have done! I'll read your paper more closely and come back with any 
questions I still have.


Thank you, I will gladly explain anything unclear. Just try to not give 
Laszlo too many flashbacks. :)




With Best Regards,
Nate


-Original Message-
From: devel@edk2.groups.io  On Behalf Of Marvin
Häuser
Sent: Sunday, April 4, 2021 4:02 PM
To: devel@edk2.groups.io; Laszlo Ersek ; Andrew Fish
; Kinney, Michael D 
Subject: [edk2-devel] [GSoC proposal] Secure Image Loader

Good day everyone,

I'll keep the introduction brief because I've been around for a while now. :) 
I'm
Marvin Häuser, a third-year Computer Science student from TU Kaiserslautern,
Germany. Late last year, my colleague Vitaly from ISP RAS and me introduced a
formally verified Image Loader for UEFI usage at ISP RAS Open[1] due to various
defects we outlined in the corresponding paper. Thank you once again Laszlo
for your *incredible* review work on the publication part.

I now want to make an effort to mainline it, preferably as part of the current
Google Summer of Code event. To be clear, my internship at ISP RAS has
concluded, and while Vitaly will be available for design discussion, he has 
other
priorities at the moment and the practical part will be on me. I have previously
submitted a proposal via the GSoC website for your review.

There are many things to consider:
1. The Image Loader is a core component, and there needs to be a significant
level of quality and security assurance.
2. Being consumed by many packages, the proposed patch set will take a lot of
time to review and integrate.
3. During my initial exploration, I discovered defective PPIs and protocols 
(e.g.
returning data with no corresponding size) originating from the UEFI PI and
UEFI specifications. Changes need to be discussed, settled on, and submitted to
the UEFI Forum.
4. Some UEFI APIs like the Security Architecture protocols are inconveniently
abstract, see 5.
5. Some of the current code does not use the existing context, or accesses it
outside of the exposed APIs. The control flow of the dispatchers may need to be
adapted to make the context available to appropriate APIs.

But obviously there are

Re: [edk2-devel] [GSoC proposal] Secure Image Loader

2021-04-06 Thread Nate DeSimone

Hi Marvin,

Great to meet you and welcome back! Glad you hear you are interested! 
Completing a formal verification of a PE/COFF loader is certainly impressive. 
Was this done with some sort of automated theorem proving? It would seem a 
rather arduous task doing an inductive proof for an algorithm like that by 
hand! I completely agree with you that getting a formally verified PE/COFF 
loader into mainline is undoubtably valuable and would pay security dividends 
for years to come.

Admittedly, this is an area of computer science that I don't have a great deal 
of experience with. The furthest I have gone on this topic is writing out 
proofs for simple algorithms on exams in my Algorithms class in college. 
Regardless you have a much better idea of what the current status is of the 
work that you and Vitaly have done. I guess my only question is do you think 
there is sufficient work remaining to fill the 10 week GSoC development window? 
Certainly we can use some of that time to perform the code reviews you mention 
and write up formal ECRs for the UEFI spec changes that you believe are needed.

Thank you for sending the application and alerting us to the great work you and 
Vitaly have done! I'll read your paper more closely and come back with any 
questions I still have.

With Best Regards,
Nate

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Marvin
> Häuser
> Sent: Sunday, April 4, 2021 4:02 PM
> To: devel@edk2.groups.io; Laszlo Ersek ; Andrew Fish
> ; Kinney, Michael D 
> Subject: [edk2-devel] [GSoC proposal] Secure Image Loader
> 
> Good day everyone,
> 
> I'll keep the introduction brief because I've been around for a while now. :) 
> I'm
> Marvin Häuser, a third-year Computer Science student from TU Kaiserslautern,
> Germany. Late last year, my colleague Vitaly from ISP RAS and me introduced a
> formally verified Image Loader for UEFI usage at ISP RAS Open[1] due to 
> various
> defects we outlined in the corresponding paper. Thank you once again Laszlo
> for your *incredible* review work on the publication part.
> 
> I now want to make an effort to mainline it, preferably as part of the current
> Google Summer of Code event. To be clear, my internship at ISP RAS has
> concluded, and while Vitaly will be available for design discussion, he has 
> other
> priorities at the moment and the practical part will be on me. I have 
> previously
> submitted a proposal via the GSoC website for your review.
> 
> There are many things to consider:
> 1. The Image Loader is a core component, and there needs to be a significant
> level of quality and security assurance.
> 2. Being consumed by many packages, the proposed patch set will take a lot of
> time to review and integrate.
> 3. During my initial exploration, I discovered defective PPIs and protocols 
> (e.g.
> returning data with no corresponding size) originating from the UEFI PI and
> UEFI specifications. Changes need to be discussed, settled on, and submitted 
> to
> the UEFI Forum.
> 4. Some UEFI APIs like the Security Architecture protocols are inconveniently
> abstract, see 5.
> 5. Some of the current code does not use the existing context, or accesses it
> outside of the exposed APIs. The control flow of the dispatchers may need to 
> be
> adapted to make the context available to appropriate APIs.
> 
> But obviously there are not only unpleasant considerations:
> A. The Image Loader is mostly formally verified, and only very few changes 
> will
> be required from the last proven state. This gives a lot of trust in its 
> correctness
> and safety.
> B. All outlined defects that are of critical nature have been fixed 
> successfully.
> C. The Image Loader has been tested with real-world code loading real-world
> OSes on thousands of machines in the past few months, including rejecting
> malformed images (configurable by PCD).
> D. The new APIs will centralise everything PE, reducing code duplication and
> potentially unsafe operations.
> E. Centralising and reduced parse duplication may improve overall boot
> performance.
> F. The code has been coverage-tested to not contain dead code.
> G. The code has been fuzz-tested including sanitizers to not invoke undefined
> behaviour.
> H. I already managed to identify a malformed image in OVMF with its help
> (incorrectly reported section alignment of an Intel IPXE driver). A fix will 
> be
> submitted shortly.
> I. I plan to support PE section permissions, allowing for read-only data
> segments when enabled.
> 
> There are likely more points for both lists, but I hope this gives a decent
> starting point for discussion. What are your thoughts on the matter? I 
> strongly
> encourage everyone to read the section regarding defects of our publication[2]
> to better understand the motivation. The vague points above can of course be
> elaborated in due time, as you see fit.
> 
> Thank you for your time!
> 
> Best regards,
> Marvin
> 
> 
> [1]

Re: [edk2-devel] [PATCH] IntelFsp2Pkg: Fix multibyte array data issue

2021-04-06 Thread Chiu, Chasel



Hi Tung Lun,

The fix looks good, but please help to re-send with correct patch format.

Thanks,
Chasel

1. remove Change-id
2. cc all reviewers/maintainers
3. add bugzilla link to describe the issue you are fixing.
4. enrich commit message to describe the fix.





> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Tung Lun
> Sent: Friday, April 2, 2021 10:32 AM
> To: devel@edk2.groups.io
> Cc: Loo, Tung Lun 
> Subject: [edk2-devel] [PATCH] IntelFsp2Pkg: Fix multibyte array data issue
> 
> Change-Id: Iad50d68a93ea1da13c2264fac7229f2d540531f0
> Signed-off-by: Loo Tung Lun 
> ---
>  IntelFsp2Pkg/Tools/FspDscBsf2Yaml.py | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/IntelFsp2Pkg/Tools/FspDscBsf2Yaml.py
> b/IntelFsp2Pkg/Tools/FspDscBsf2Yaml.py
> index cad9b60e73..5e94e86db5 100644
> --- a/IntelFsp2Pkg/Tools/FspDscBsf2Yaml.py
> +++ b/IntelFsp2Pkg/Tools/FspDscBsf2Yaml.py
> @@ -291,7 +291,6 @@ class CFspDsc2Yaml():
>  raise Exception('DSC variable creation error !') 
> else: raise
> Exception('Unsupported file "%s" !' % file_name)-
> gen_cfg_data.UpdateDefaultValue() self.gen_cfg_data = gen_cfg_data
> def print_dsc_line(self):--
> 2.28.0.windows.1
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#73635): https://edk2.groups.io/g/devel/message/73635
> Mute This Topic: https://groups.io/mt/81794616/1777047
> Group Owner: devel+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [chasel.c...@intel.com] -=-
> =-=-=-=-=
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73716): https://edk2.groups.io/g/devel/message/73716
Mute This Topic: https://groups.io/mt/81794616/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] Cancelled Event: TianoCore Bug Triage - APAC / NAMO - Tuesday, 6 April 2021 #cal-cancelled

2021-04-06 Thread devel@edk2.groups.io Calendar

BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Groups.io Inc//Groups.io Calendar//EN
METHOD:CANCELLED
CALSCALE:GREGORIAN
BEGIN:VTIMEZONE
TZID:America/Los_Angeles
LAST-MODIFIED:20201011T015911Z
TZURL:http://tzurl.org/zoneinfo-outlook/America/Los_Angeles
X-LIC-LOCATION:America/Los_Angeles
BEGIN:DAYLIGHT
TZNAME:PDT
TZOFFSETFROM:-0800
TZOFFSETTO:-0700
DTSTART:19700308T02
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=2SU
END:DAYLIGHT
BEGIN:STANDARD
TZNAME:PST
TZOFFSETFROM:-0700
TZOFFSETTO:-0800
DTSTART:19701101T02
RRULE:FREQ=YEARLY;BYMONTH=11;BYDAY=1SU
END:STANDARD
END:VTIMEZONE
BEGIN:VEVENT
X-GIOIDS:Event:1057058 
UID:1057058.mlda.1580078539586725120.r...@groups.io
DTSTAMP:20210406T090318Z
ORGANIZER;CN=Liming Gao:mailto:gaolim...@byosoft.com.cn
DTSTART:20210407T013000Z
DTEND:20210407T023000Z
SUMMARY:TianoCore Bug Triage - APAC / NAMO
DESCRIPTION:TianoCore Bug Triage - APAC / NAMO\n\nHosted by Liming Gao\n\
 nJoin on your computer or mobile app\nClick here to join the meeting ( ht
 tps://teams.microsoft.com/l/meetup-join/19%3ameeting_YmU5MzNjZTQtMzQwNC00
 ZmI5LWEyZmMtNzJjODRjN2YyNjhm%40thread.v2/0?context=%7b%22Tid%22%3a%223f29
 ed5e-8d29-429d-a771-6b1ae4150dd7%22%2c%22Oid%22%3a%22634e9251-1a6d-4e35-8
 269-856651a5178f%22%7d )
LOCATION:https://teams.microsoft.com/l/meetup-join/19%3ameeting_YmU5MzNjZ
 TQtMzQwNC00ZmI5LWEyZmMtNzJjODRjN2YyNjhm%40thread.v2/0?context=%7b%22Tid%2
 2%3a%223f29ed5e-8d29-429d-a771-6b1ae4150dd7%22%2c%22Oid%22%3a%22634e9251-
 1a6d-4e35-8269-856651a5178f%22%7d
RECURRENCE-ID:20210407T013000Z
SEQUENCE:999
STATUS:CANCELLED
END:VEVENT
END:VCALENDAR


invite.ics
Description: application/ics

回复: [edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 04/06/2021 6:30pm-7:30pm #cal-reminder

2021-04-06 Thread gaoliming

Hi, all

 Few issues are reported this week. Let’s cancel this week TianoCore Bug Triage 
meeting. 

 


  2180

EDK2

Tools

michael.tur...@microsoft.com

UNCO

GenFw always zeros the timestamp 
 

04:59:41

michael.tur...@microsoft.com


  3296

EDK2

Code

unassig...@tianocore.org

UNCO

EFI_ACPI_MAX_NUM_TABLES hardcoded to 20 was too small for our platform. 
 

Fri 19:56

af...@apple.com


  3295

EDK2

Code

unassig...@tianocore.org

UNCO

Promote device security protocol to PI spec 
 

Fri 03:35

jiewen@intel.com


  3294

EDK2 Pla

MinPlatf

unassig...@tianocore.org

UNCO

RebaseFspBinBaseAddress - python path not verified leading to exception 
 

Thu 17:40

samuel.moff...@gmail.com


  3289

EDK2

Code

unassig...@tianocore.org

UNCO

VfrCompile fails when attempting to compile DriverSampleDxe 
 

Wed 15:53

harlydavid...@gmail.com

 

Thanks

Liming

发件人: devel@edk2.groups.io  
发送时间: 2021年4月6日 9:30
收件人: devel@edk2.groups.io
主题: [edk2-devel] TianoCore Bug Triage - APAC / NAMO - Tue, 04/06/2021 
6:30pm-7:30pm #cal-reminder

 

Reminder: TianoCore Bug Triage - APAC / NAMO

When: Tuesday, 6 April 2021, 6:30pm to 7:30pm, (GMT-07:00) America/Los Angeles 

Where:https://teams.microsoft.com/l/meetup-join/19%3ameeting_YmU5MzNjZTQtMzQwNC00ZmI5LWEyZmMtNzJjODRjN2YyNjhm%40thread.v2/0?context=%7b%22Tid%22%3a%223f29ed5e-8d29-429d-a771-6b1ae4150dd7%22%2c%22Oid%22%3a%22634e9251-1a6d-4e35-8269-856651a5178f%22%7d

View Event  

Organizer: Liming Gao gaolim...@byosoft.com.cn 


Description: 

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao


Join on your computer or mobile app
Click here to join the meeting 

 





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73714): https://edk2.groups.io/g/devel/message/73714
Mute This Topic: https://groups.io/mt/81885306/21656
Mute #cal-reminder:https://edk2.groups.io/g/devel/mutehashtag/cal-reminder
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] MinPlatform Board port (GSoC 2021)

2021-04-06 Thread Nate DeSimone

On Sat, Apr 3, 2021 at 09:45 PM, Benjamin Doron wrote:

> Yes, Boot Guard is disabled on this laptop (and fused, which is a slight 
> shame). While SPI PRRs cover most of flash and BIOS control bits are 
> partially enabled by the vendor firmware (perhaps the lock wasn't set by 
> default, defeating the point), after one modification to the setup's (HII) 
> IFR, these can be disabled. Flashing externally is fairly straightforward - 
> after the keyboard comes off the flash chip is visible - but I try to avoid 
> it because I've started having issues with the keyboard connector. It's fine 
> once it's in and closed again...

As long as Boot Guard is disabled we can do work on it! As far as laptop 
designs go, sounds like one of the more straightforward ones which is great.

> ...but this might not be relevant to anyone after all. :-)
> 
> Is that how this normally works? Who would organise it? (It sounds needlessly 
> expensive, for someone... *shrug*)

I think it is a little harsh to say this is not a relevant project. Sure, not 
many people have an exact Acer Aspire VN7-572G. In fact, the large OEMs like 
Acer on average produce 60 new laptop designs every 6 months. So yes it is true 
that keeping up with every single laptop design out there in the wild isn't 
really feasible. However, the important thing to keep in mind is that while 
there may be ~100 Acer laptop designs with Sky Lake, they are all mostly 
identical. They differ in screen sizes, keyboards, colors, amount of RAM, etc. 
but they are all built on common templates. A typical OEM will make maybe 1-5 
motherboard designs for a given chipset (say Sky Lake-U) and then ship a ton of 
minor variations. Sometimes those variations will show up with different brand 
names on the lid. The important thing to keep in mind is that while you may be 
currently focused on the VN7-572G, it is extremely likely that your work would 
be 95% of what is needed to get 30-50% of Acer's Sky Lake laptops working.

To be completely honest with you, this is the first time I have done GSoC as 
well so this is a learning experience for me as well. I'll look into the 
feasibility of acquiring a second VN7-572G.

> Thanks for these pointers and references, I will look at them more.
> 
> As I understand it, the objective of dispatch mode was to remove code 
> duplication in flash and possibly save boot time by minimising phase 
> transitions. But it makes the PeiCore module non-updateable. From a board 
> initialisation perspective, they're probably the same, so this can be figured 
> out later.

You are talking to the guy that invented FSP dispatch mode, so I can 
definitively say that yes those were all goals for dispatch mode. However, by 
far the biggest goal was to make FSP integrate nicely with EDK II based 
firmware. There are two ways to use dispatch mode actually. The first way as 
you mention uses the PeiCore included with FSP-M. The other method uses a 
PeiCore provided by the platform firmware. This method is described in Section 
7.2.3 - "Alternate Boot Flow Description" of the FSP v2.2 specification. While 
it does result in 2 copies of PeiCore, it keeps PeiCore updatable. It is still 
better than API mode because even though there are 2 copies of PeiCore, only 1 
of them is actually used. So you only have a single instance of PEI at runtime, 
whereas in API mode you have two separate instances of PEI in memory at 
runtime. My understanding is that most OEMs choose to use FSP dispatch mode in 
this way. From a board initialization perspective, they are mostly the same but 
there are some differences. Specifically, in dispatch mode one does not use FSP 
UPDs at all. Instead, one passes the native configuration policy data 
structures that FSP uses internally, in the case of Kaby Lake that is Config 
Blocks stored inside a PPI.

> Understood. So, there will be two similar commits of board initialisation? I 
> doubt I can truly dual-license the code, "BSD+GPL" might be a contradiction. 
> However, I'm not a lawyer.

Actually I have talked with some lawyers about this very subject. So the 
BSD+Patent license is considered "GPL compatible" from a legal standpoint. What 
that means is that if you release some code under BSD+Patent, then anyone else 
can take that code and re-license it under the GPL without having to ask you. 
So in effect, BSD+Patent is already a BSD+GPL dual-license! My guess is there 
will be two commits of board initialization for technical reasons, but if one 
licenses their code under BSD+Patent then there are zero legal barriers to 
using that code in both projects.

> Regarding your second email, yes, that makes sense. I was just nervous about 
> getting it wrong.

Completely understand, thanks for double checking!

With Best Regards,
Nate

> 
> Best regards,
> Benjamin Doron

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73713): https://edk2.groups.io/g/devel/message/73713
Mute

Re: [edk2-devel] [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for the SEV-SNP guest

2021-04-06 Thread Min Xu

Hi, Singh
I have a concern about the sevSnpBlock in ResetVectorVtf0.asm. Actually
SEV has inserted 3 blocks in ResetVectorVtf0.asm and the total bytes are
(26 + 22 + 20 = 68 bytes). If sevSnpBlock is added, then the total bytes
will be (68 +26 = 94 bytes).

I am not sure whether there will be more blocks added in
ResetVectorVtf0.asm in the future. But I don't think ResetVectorVtf0.asm
is a good place to add these data blobs. Can these data be packed into a
single file, for example, SevMetadata.asm, then a pointer is inserted in
ResetVectorVtf0.asm which then points to the SevMetadata. In this way we
can keep ResetVectorVtf0.asm clean, small and straight forward.

Another reason is that I am working on the Intel TDX which will update
the ResetVectorVtf0.asm as well. My change depends on the assumption that
the distance between ResetVector(0xfff0) and EarlyBspInitReal16 is
less than 128 bytes. The blocks in ResetVectorVtf0.asm make it impossible.

Thanks!

> -Original Message-
> From: Brijesh Singh 
> Sent: Wednesday, March 24, 2021 11:32 PM
> To: devel@edk2.groups.io
> Cc: Brijesh Singh ; James Bottomley
> ; Xu, Min M ; Yao, Jiewen
> ; Tom Lendacky ;
> Justen, Jordan L ; Ard Biesheuvel
> ; Laszlo Ersek 
> Subject: [RFC PATCH 01/19] OvmfPkg: Reserve the Secrets and Cpuid page for
> the SEV-SNP guest
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3275
> 
> During the SEV-SNP guest launch sequence, two special pages need to be
> inserted, the secrets page and cpuid page. The secrets page, contain the VM
> platform communication keys. The guest BIOS and OS can use this key to
> communicate with the SEV firmware to get the attestation report. The Cpuid
> page, contain the CPUIDs entries filtered through the AMD-SEV firmware.
> 
> The VMM will locate the secrets and cpuid page addresses through a fixed
> GUID and pass them to SEV firmware to populate further.
> For more information about the page content, see the SEV-SNP spec.
> 
> To simplify the pre-validation range calculation in the next patch, the CPUID
> and Secrets pages are moved to the start of the MEMFD_BASE_ADDRESS.
> 
> Cc: James Bottomley 
> Cc: Min Xu 
> Cc: Jiewen Yao 
> Cc: Tom Lendacky 
> Cc: Jordan Justen 
> Cc: Ard Biesheuvel 
> Cc: Laszlo Ersek 
> Signed-off-by: Brijesh Singh 
> ---
>  OvmfPkg/OvmfPkg.dec  |  8 +++
>  OvmfPkg/OvmfPkgX64.fdf   | 24 
>  OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 19 
>  OvmfPkg/ResetVector/ResetVector.inf  |  4 
>  OvmfPkg/ResetVector/ResetVector.nasmb|  2 ++
>  5 files changed, 48 insertions(+), 9 deletions(-)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
> 4348bb45c6..062926772d 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -317,6 +317,14 @@
>gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase|0x0|UINT32|0x42
>gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretSize|0x0|UINT32|0x43
> 
> +  ## The base address of the CPUID page used by SEV-SNP
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|0|UINT32|0x48
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidSize|0|UINT32|0x49
> +
> +  ## The base address of the Secrets page used by SEV-SNP
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|0|UINT32|0x50
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsSize|0|UINT32|0x51
> +
>  [PcdsDynamic, PcdsDynamicEx]
>gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN
> |0x10
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf index
> d519f85328..ea214600be 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -67,27 +67,33 @@ ErasePolarity = 1
>  BlockSize = 0x1
>  NumBlocks = 0xD0
> 
> -0x00|0x006000
> +0x00|0x001000
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpCpuidBase|gUefiOvmfPkgTokenS
> paceGu
> +id.PcdOvmfSnpCpuidSize
> +
> +0x001000|0x001000
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSnpSecretsBase|gUefiOvmfPkgToken
> Space
> +Guid.PcdOvmfSnpSecretsSize
> +
> +0x002000|0x006000
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase|gUefiOvmfPkgTok
> enSpaceGuid.PcdOvmfSecPageTablesSize
> 
> -0x006000|0x001000
> +0x008000|0x001000
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTo
> kenSpaceGuid.PcdOvmfLockBoxStorageSize
> 
> -0x007000|0x001000
> +0x009000|0x001000
> 
> gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvm
> fPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
> 
> -0x008000|0x001000
> +0x00A000|0x001000
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkg
> TokenSpaceGuid.PcdOvmfSecGhcbPageTableSize
> 
> -0x009000|0x002000
> +0x00B000|0x002000
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpa
> ceGuid.PcdOvmfSecGhcbSize
> 
> -0x00B000|0x001000
> -
> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpac
>

Re: [edk2-devel] GSoC 2021 (MinPlatform, Ext2, ACPICA, etc)

2021-04-06 Thread Nate DeSimone

Hi Pedro,

I agree with Andrew Fish. I’d say for the first half try to have an initial 
(but probably broken) implementation of the driver done. Then your second half 
can be fixing bugs, writing tests, and documenting your work.

Thanks,
Nate

From: Pedro Falcato 
Date: Sunday, April 4, 2021 at 3:50 AM
To: "Desimone, Nathaniel L" , 
"devel@edk2.groups.io" 
Subject: Re: [edk2-devel] GSoC 2021 (MinPlatform, Ext2, ACPICA, etc)

Hi,

Sounds great! You may be right about that, although I believe I read somewhere 
that you need to set a milestone for each evaluation? If so, I'm out of ideas 
as to what can be tangible enough to be a milestone; if not, specifying a 
read-only driver as project's objective and having write support as a stretch 
goal is certainly the way to go.

Thanks,
Pedro 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#73711): https://edk2.groups.io/g/devel/message/73711
Mute This Topic: https://groups.io/mt/81290134/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

53 matches

Mail list logo