[edk2-devel] [PATCH 4/4] OvmfPkg/AmdSev: Expose the Sev Secret area using a configuration table

James Bottomley posted 4 patches 5 years, 2 months ago
There is a newer version of this series
[edk2-devel] [PATCH 4/4] OvmfPkg/AmdSev: Expose the Sev Secret area using a configuration table
Posted by James Bottomley 5 years, 2 months ago
This is to allow the boot loader (grub) to pick up the secret area.
The Configuration Table simply points to the base and size (in
physical memory) and this area is covered by a Boot time HOB, meaning
that the secret will be freed after ExitBootServices, by which time it
should be consumed anyway.

Signed-off-by: James Bottomley <jejb@linux.ibm.com>
---
 OvmfPkg/AmdSev/AmdSevX64.dsc                  |  3 ++
 OvmfPkg/AmdSev/AmdSevX64.fdf                  |  3 ++
 .../SevLaunchSecret/SecretDxe/SecretDxe.inf   | 38 +++++++++++++++
 .../SevLaunchSecret/SecretPei/SecretPei.inf   | 46 +++++++++++++++++++
 .../SevLaunchSecret/SecretDxe/SecretDxe.c     | 29 ++++++++++++
 .../SevLaunchSecret/SecretPei/SecretPei.c     | 26 +++++++++++
 6 files changed, 145 insertions(+)
 create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
 create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
 create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c
 create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 7d3663150e..eb8cc9d60a 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -698,6 +698,7 @@
   OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
   UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+  OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
 
 !if $(TPM_ENABLE) == TRUE
   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -1007,6 +1008,8 @@
   }
 !endif
 
+  OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
+
   #
   # TPM support
   #
diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
index 1fd38b3fe2..65ee4d993b 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.fdf
+++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
@@ -146,6 +146,7 @@ INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
 !endif
 INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
+INF  OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
 
 !if $(TPM_ENABLE) == TRUE
 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -290,6 +291,8 @@ INF  ShellPkg/Application/Shell/Shell.inf
 
 INF MdeModulePkg/Logo/LogoDxe.inf
 
+INF OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
+
 #
 # Network modules
 #
diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
new file mode 100644
index 0000000000..085162e5c4
--- /dev/null
+++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
@@ -0,0 +1,38 @@
+## @file
+#  Sev Secret configuration Table installer
+#
+#  Copyright (C) 2020 James Bottomley, IBM Corporation.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = SecretDxe
+  FILE_GUID                      = 6e2b9619-8810-4e9d-a177-d432bb9abeda
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = InitializeSecretDxe
+
+[Sources]
+  SecretDxe.c
+
+[Packages]
+  OvmfPkg/OvmfPkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  UefiBootServicesTableLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Guids]
+  gSevLaunchSecretGuid
+
+[FixedPcd]
+  gSevLaunchSecretGuid.PcdSevLaunchSecretBase
+  gSevLaunchSecretGuid.PcdSevLaunchSecretSize
+
+[Depex]
+  TRUE
diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
new file mode 100644
index 0000000000..b154dcc74e
--- /dev/null
+++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
@@ -0,0 +1,46 @@
+## @file
+#  PEI support for SEV Secrets
+#
+#  Copyright (C) 2020 James Bottomley, IBM Corporation.
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = SecretPei
+  FILE_GUID                      = 45260dde-0c3c-4b41-a226-ef3803fac7d4
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = InitializeSecretPei
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 EBC
+#
+
+[Sources]
+  SecretPei.c
+
+[Packages]
+  OvmfPkg/OvmfPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  HobLib
+  PeiServicesLib
+  PeiServicesTablePointerLib
+  PeimEntryPoint
+  PcdLib
+
+[FixedPcd]
+  gSevLaunchSecretGuid.PcdSevLaunchSecretBase
+  gSevLaunchSecretGuid.PcdSevLaunchSecretSize
+
+[Depex]
+  TRUE
diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c
new file mode 100644
index 0000000000..b40bbe1eb9
--- /dev/null
+++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c
@@ -0,0 +1,29 @@
+/** @file
+  SEV Secret configuration table constructor
+
+  Copyright (C) 2020 James Bottomley, IBM Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <PiDxe.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiDriverEntryPoint.h>
+#include <Library/UefiBootServicesTableLib.h>
+
+struct {
+  UINT32        base;
+  UINT32        size;
+} secretDxeTable = {
+  FixedPcdGet32(PcdSevLaunchSecretBase),
+  FixedPcdGet32(PcdSevLaunchSecretSize),
+};
+
+EFI_STATUS
+EFIAPI
+InitializeSecretDxe(
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_SYSTEM_TABLE     *SystemTable
+  )
+{
+  return gBS->InstallConfigurationTable (&gSevLaunchSecretGuid,
+                                         &secretDxeTable);
+}
diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c
new file mode 100644
index 0000000000..16b49792ad
--- /dev/null
+++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c
@@ -0,0 +1,26 @@
+/** @file
+  SEV Secret boot time HOB placement
+
+  Copyright (C) 2020 James Bottomley, IBM Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <PiPei.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/PcdLib.h>
+
+EFI_STATUS
+EFIAPI
+InitializeSecretPei (
+  IN       EFI_PEI_FILE_HANDLE  FileHandle,
+  IN CONST EFI_PEI_SERVICES     **PeiServices
+  )
+{
+  BuildMemoryAllocationHob (
+    PcdGet32 (PcdSevLaunchSecretBase),
+    PcdGet32 (PcdSevLaunchSecretSize),
+    EfiBootServicesData);
+
+  return EFI_SUCCESS;
+}
-- 
2.26.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67343): https://edk2.groups.io/g/devel/message/67343
Mute This Topic: https://groups.io/mt/78198621/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 4/4] OvmfPkg/AmdSev: Expose the Sev Secret area using a configuration table
Posted by Laszlo Ersek 5 years, 2 months ago
On 11/12/20 01:13, James Bottomley wrote:
> This is to allow the boot loader (grub) to pick up the secret area.
> The Configuration Table simply points to the base and size (in
> physical memory) and this area is covered by a Boot time HOB, meaning
> that the secret will be freed after ExitBootServices, by which time it
> should be consumed anyway.
>
> Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> ---
>  OvmfPkg/AmdSev/AmdSevX64.dsc                  |  3 ++
>  OvmfPkg/AmdSev/AmdSevX64.fdf                  |  3 ++
>  .../SevLaunchSecret/SecretDxe/SecretDxe.inf   | 38 +++++++++++++++
>  .../SevLaunchSecret/SecretPei/SecretPei.inf   | 46 +++++++++++++++++++
>  .../SevLaunchSecret/SecretDxe/SecretDxe.c     | 29 ++++++++++++
>  .../SevLaunchSecret/SecretPei/SecretPei.c     | 26 +++++++++++
>  6 files changed, 145 insertions(+)
>  create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
>  create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
>  create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c
>  create mode 100644 OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c
>
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
> index 7d3663150e..eb8cc9d60a 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc
> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
> @@ -698,6 +698,7 @@
>    OvmfPkg/SmmAccess/SmmAccessPei.inf
>  !endif
>    UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +  OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
>
>  !if $(TPM_ENABLE) == TRUE
>    OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -1007,6 +1008,8 @@
>    }
>  !endif
>
> +  OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
> +
>    #
>    # TPM support
>    #
> diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fdf
> index 1fd38b3fe2..65ee4d993b 100644
> --- a/OvmfPkg/AmdSev/AmdSevX64.fdf
> +++ b/OvmfPkg/AmdSev/AmdSevX64.fdf
> @@ -146,6 +146,7 @@ INF  MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>  INF  OvmfPkg/SmmAccess/SmmAccessPei.inf
>  !endif
>  INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
> +INF  OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
>
>  !if $(TPM_ENABLE) == TRUE
>  INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -290,6 +291,8 @@ INF  ShellPkg/Application/Shell/Shell.inf
>
>  INF MdeModulePkg/Logo/LogoDxe.inf
>
> +INF OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
> +
>  #
>  # Network modules
>  #

(1) Please split the SecretDxe-related hunks and new files to a separate
patch.

(1.a) Patch#4 should be called

  OvmfPkg/AmdSev: assign and protect the Sev Secret area

It should contain the PEI phase-related changes.

(1.b) Patch#5 should inherit the present subject ("OvmfPkg/AmdSev:
Expose the Sev Secret area using a configuration table"), and should
contain the DXE phase-related hunks and new files. I'll comment on
patch#5 more, below.


(2) In both the DSC and the FDF files, please place the

  OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf

just above

  OvmfPkg/AmdSev/Grub/Grub.inf

(Not a functional difference, it just improves consistency between
the DSC and the FDF, plus this placement appears quite logical too.)


(3) I suggest unnesting both SecretPei and SecretDxe from the
SevLaunchSecret subdirectory, and removing that subdirectory. To me the
following pathnames seem fine:

- OvmfPkg/AmdSev/SecretPei/
- OvmfPkg/AmdSev/SecretDxe/


> diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
> new file mode 100644
> index 0000000000..b154dcc74e
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.inf
> @@ -0,0 +1,46 @@
> +## @file
> +#  PEI support for SEV Secrets
> +#
> +#  Copyright (C) 2020 James Bottomley, IBM Corporation.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = SecretPei
> +  FILE_GUID                      = 45260dde-0c3c-4b41-a226-ef3803fac7d4
> +  MODULE_TYPE                    = PEIM
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = InitializeSecretPei
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 EBC
> +#

(4) Please either drop VALID_ARCHITECTURES, or restrict it to X64 only.


> +
> +[Sources]
> +  SecretPei.c
> +
> +[Packages]
> +  OvmfPkg/OvmfPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

(5) MdeModulePkg seems unnecessary


> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  HobLib
> +  PeiServicesLib
> +  PeiServicesTablePointerLib
> +  PeimEntryPoint
> +  PcdLib

(6) only HobLib, PcdLib, and PeimEntryPoint appear necessary


> +
> +[FixedPcd]
> +  gSevLaunchSecretGuid.PcdSevLaunchSecretBase
> +  gSevLaunchSecretGuid.PcdSevLaunchSecretSize
> +
> +[Depex]
> +  TRUE

> diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c
> new file mode 100644
> index 0000000000..16b49792ad
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretPei/SecretPei.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  SEV Secret boot time HOB placement
> +
> +  Copyright (C) 2020 James Bottomley, IBM Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +#include <PiPei.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/PcdLib.h>

(7) From Library/, only HobLib and PcdLib look required.

After removing the other lib class headers, you may have to include
<Uefi/UefiMultiPhase.h> separately, for EfiBootServicesData.


> +
> +EFI_STATUS
> +EFIAPI
> +InitializeSecretPei (
> +  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> +  IN CONST EFI_PEI_SERVICES     **PeiServices
> +  )
> +{
> +  BuildMemoryAllocationHob (
> +    PcdGet32 (PcdSevLaunchSecretBase),
> +    PcdGet32 (PcdSevLaunchSecretSize),
> +    EfiBootServicesData);

(8) Please break the closing paren to a new line:

  BuildMemoryAllocationHob (
    PcdGet32 (PcdSevLaunchSecretBase),
    PcdGet32 (PcdSevLaunchSecretSize),
    EfiBootServicesData
    );

(Note that the indentation of the closing paren is not a mistake, it is
intentional.)


> +
> +  return EFI_SUCCESS;
> +}
>

> diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
> new file mode 100644
> index 0000000000..085162e5c4
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.inf
> @@ -0,0 +1,38 @@
> +## @file
> +#  Sev Secret configuration Table installer
> +#
> +#  Copyright (C) 2020 James Bottomley, IBM Corporation.
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = SecretDxe
> +  FILE_GUID                      = 6e2b9619-8810-4e9d-a177-d432bb9abeda
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  ENTRY_POINT                    = InitializeSecretDxe
> +
> +[Sources]
> +  SecretDxe.c
> +
> +[Packages]
> +  OvmfPkg/OvmfPkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  UefiBootServicesTableLib
> +  UefiDriverEntryPoint
> +  UefiLib

(9) UefiLib looks superfluous


> +
> +[Guids]
> +  gSevLaunchSecretGuid
> +
> +[FixedPcd]
> +  gSevLaunchSecretGuid.PcdSevLaunchSecretBase
> +  gSevLaunchSecretGuid.PcdSevLaunchSecretSize
> +
> +[Depex]
> +  TRUE


> diff --git a/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c
> new file mode 100644
> index 0000000000..b40bbe1eb9
> --- /dev/null
> +++ b/OvmfPkg/AmdSev/SevLaunchSecret/SecretDxe/SecretDxe.c
> @@ -0,0 +1,29 @@
> +/** @file
> +  SEV Secret configuration table constructor
> +
> +  Copyright (C) 2020 James Bottomley, IBM Corporation.
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +#include <PiDxe.h>
> +#include <Library/UefiLib.h>

(10) UefiLib looks superfluous.


> +#include <Library/UefiDriverEntryPoint.h>

(11) Entry point libraries must be listed in the INF files'
[LibraryClasses] sections, but should not be #included in the C source
files.


> +#include <Library/UefiBootServicesTableLib.h>
> +
> +struct {
> +  UINT32        base;
> +  UINT32        size;
> +} secretDxeTable = {
> +  FixedPcdGet32(PcdSevLaunchSecretBase),
> +  FixedPcdGet32(PcdSevLaunchSecretSize),
> +};
> +

(12) This is an interface between (theoretically) independent
applications, so we should place the structure definition in a dedicated
header file.

(12.a) Please move the following hunk, from patch#3:

> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index 3fbf7a0ee1a4..b00f08341713 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -117,6 +117,7 @@ [Guids]
>    gLinuxEfiInitrdMediaGuid              = {0x5568e427, 0x68fc, 0x4f3d, {0xac, 0x74, 0xca, 0x55, 0x52, 0x31, 0xcc, 0x68}}
>    gQemuKernelLoaderFsMediaGuid          = {0x1428f772, 0xb64a, 0x441e, {0xb8, 0xc3, 0x9e, 0xbd, 0xd7, 0xf8, 0x93, 0xc7}}
>    gGrubFileGuid                         = {0xb5ae312c, 0xbc8a, 0x43b1, {0x9c, 0x62, 0xeb, 0xb8, 0x26, 0xdd, 0x5d, 0x07}}
> +  gSevLaunchSecretGuid                  = {0xadf956ad, 0xe98c, 0x484c, {0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47}}
>
>  [Ppis]
>    # PPI whose presence in the PPI database signals that the TPM base address

to patch#5 (i.e., the patch that's going to introduce SecretDxe).

(12.b) In patch #5, please create another new header file:

  OvmfPkg/Include/Guid/SevLaunchSecret.h

with the following contents (fix up the (C) notice):

> /** @file
>   UEFI Configuration Table for exposing the SEV Launch Secret location to UEFI
>   applications (boot loaders).
>
>   Copyright (C) <FIXME>
>   SPDX-License-Identifier: BSD-2-Clause-Patent
> **/
>
> #ifndef SEV_LAUNCH_SECRET_H_
> #define SEV_LAUNCH_SECRET_H_
>
> #include <Uefi/UefiBaseType.h>
>
> #define SEV_LAUNCH_SECRET_GUID                          \
>   { 0xadf956ad,                                         \
>     0xe98c,                                             \
>     0x484c,                                             \
>     { 0xae, 0x11, 0xb5, 0x1c, 0x7d, 0x33, 0x64, 0x47 }, \
>   }
>
> typedef struct {
>   UINT32 Base;
>   UINT32 Size;
> } SEV_LAUNCH_SECRET_LOCATION;
>
> extern EFI_GUID gSevLaunchSecretGuid;
>
> #endif // SEV_LAUNCH_SECRET_H_

(12.c) In the "SecretDxe.c" file, please use the following syntax, for
defining the table:

> #include <Guid/SevLaunchSecret.h>
>
> STATIC CONST SEV_LAUNCH_SECRET_LOCATION mSecretDxeTable = {
>   FixedPcdGet32 (PcdSevLaunchSecretBase),
>   FixedPcdGet32 (PcdSevLaunchSecretSize),
> };

(12.d) Note the space character just after "FixedPcdGet32", and the "m"
prefix in the global variable's name.


Back to your patch:

On 11/12/20 01:13, James Bottomley wrote:
> +EFI_STATUS
> +EFIAPI
> +InitializeSecretDxe(
> +  IN EFI_HANDLE           ImageHandle,
> +  IN EFI_SYSTEM_TABLE     *SystemTable
> +  )
> +{
> +  return gBS->InstallConfigurationTable (&gSevLaunchSecretGuid,
> +                                         &secretDxeTable);
> +}

(13) There are two styles to indent this idiomatically:

(13.a) the official edk2 one:

  return gBS->InstallConfigurationTable (
                &gSevLaunchSecretGuid,
                &mSecretDxeTable
                );

- each argument *and* the closing paren on a new line,

- each argument *and* the closing paren indented 2 spaces relative to
  "InstallConfigurationTable" (not relative to "gBS")

(13.b) or the "condensed" style:

  return gBS->InstallConfigurationTable (&gSevLaunchSecretGuid,
                &mSecretDxeTable);

The only difference from (13.a) is that a new source line is only
started when we'd exceed the recommended line length (80).

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67623): https://edk2.groups.io/g/devel/message/67623
Mute This Topic: https://groups.io/mt/78198621/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-