public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD
@ 2021-09-16  9:03 Corvin Köhne
  2021-09-17  3:23 ` Yao, Jiewen
  0 siblings, 1 reply; 4+ messages in thread
From: Corvin Köhne @ 2021-09-16  9:03 UTC (permalink / raw)
  To: devel
  Cc: ardb+tianocore, jiewen.yao, jordan.l.ljusten, kraxel, rebecca,
	grehan, Corvin Köhne

SEC checks in IsSevGuest if the PCD defined WorkAreaHeader size
matches the size of the WorkAreaHeader struct definition. Set a
default value for the PCD to avoid unnecessary DSC/FDF file
changes in all OVMF DSC/FDF files.

Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---
 OvmfPkg/Include/WorkArea.h     | 4 ++++
 OvmfPkg/OvmfPkg.dec            | 7 ++++++-
 OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ------
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
index c16030e3ac..0f9b442e43 100644
--- a/OvmfPkg/Include/WorkArea.h
+++ b/OvmfPkg/Include/WorkArea.h
@@ -25,6 +25,10 @@ typedef enum {
 // to the structure need to be kept in sync with the
 // PcdOvmfConfidentialComputingWorkAreaHeader.
 //
+// PcdOvmfConfidentialComputingWorkAreaHeader ==
+//   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
+//
+// PcdOvmfConfidentialComputingWorkAreaHeader defined in OvmfPkg/OvmfPkg.dec
 typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
   UINT8                   GuestType;
   UINT8                   Reserved1[3];
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index c37dafad49..a5119b1909 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -338,7 +338,12 @@
   # The size of this header is used early boot, and is provided through
   # a fixed PCD. It need to be kept in sync with any changes to the
   # header definition.
-  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader|0|UINT32|0x51
+  #
+  # PcdOvmfConfidentialComputingWorkAreaHeader ==
+  #   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
+  #
+  # CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER defined in OvmfPkg/Include/WorkArea.h
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader|4|UINT32|0x51


 [PcdsDynamic, PcdsDynamicEx]
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 3b5e452539..35fd454b97 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -82,12 +82,6 @@ SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize = $(BLOCK_SIZ
 SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
 SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = $(VARS_SPARE_SIZE)

-# The OVMF WorkArea contains a fixed size header followed by the actual data.
-# The size of header is accessed through a fixed PCD in the reset vector code.
-# The value need to be kept in sync with the any changes to the Confidential
-# Computing Work Area header defined in the Include/WorkArea.h
-SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader  = 4
-
 !if $(SMM_REQUIRE) == TRUE
 SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
 SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
--
2.11.0

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD
  2021-09-16  9:03 [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD Corvin Köhne
@ 2021-09-17  3:23 ` Yao, Jiewen
  2021-09-17  5:56   ` Corvin Köhne
  0 siblings, 1 reply; 4+ messages in thread
From: Yao, Jiewen @ 2021-09-17  3:23 UTC (permalink / raw)
  To: Köhne, Corvin, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, jordan.l.ljusten@intel.com,
	kraxel@redhat.com, rebecca@bsdio.com, grehan@freebsd.org,
	Köhne, Corvin

Hi Corvin
I try to apply this patch and merge. But I got error:

================
git.exe am --3way --ignore-space-change --keep-cr "C:\home\edkii\edk2\[PATCH-v3]-OvmfPkg-set-a-default-value-for-the-WorkAreaHeader-PCD-warn.patch"
Applying: OvmfPkg: set a default value for the WorkAreaHeader PCD
Patch failed at 0001 OvmfPkg: set a default value for the WorkAreaHeader PCD
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

error: corrupt patch at line 38
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch

Fail
=================

Would you please take a look?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Corvin Köhne <c.koehne@beckhoff.com>
> Sent: Thursday, September 16, 2021 5:03 PM
> To: devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; Yao, Jiewen <jiewen.yao@intel.com>;
> jordan.l.ljusten@intel.com; kraxel@redhat.com; rebecca@bsdio.com;
> grehan@freebsd.org; Köhne, Corvin <c.koehne@beckhoff.com>
> Subject: [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD
> 
> SEC checks in IsSevGuest if the PCD defined WorkAreaHeader size
> matches the size of the WorkAreaHeader struct definition. Set a
> default value for the PCD to avoid unnecessary DSC/FDF file
> changes in all OVMF DSC/FDF files.
> 
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  OvmfPkg/Include/WorkArea.h     | 4 ++++
>  OvmfPkg/OvmfPkg.dec            | 7 ++++++-
>  OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ------
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
> index c16030e3ac..0f9b442e43 100644
> --- a/OvmfPkg/Include/WorkArea.h
> +++ b/OvmfPkg/Include/WorkArea.h
> @@ -25,6 +25,10 @@ typedef enum {
>  // to the structure need to be kept in sync with the
>  // PcdOvmfConfidentialComputingWorkAreaHeader.
>  //
> +// PcdOvmfConfidentialComputingWorkAreaHeader ==
> +//   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> +//
> +// PcdOvmfConfidentialComputingWorkAreaHeader defined in
> OvmfPkg/OvmfPkg.dec
>  typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
>    UINT8                   GuestType;
>    UINT8                   Reserved1[3];
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index c37dafad49..a5119b1909 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -338,7 +338,12 @@
>    # The size of this header is used early boot, and is provided through
>    # a fixed PCD. It need to be kept in sync with any changes to the
>    # header definition.
> -
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r|0|UINT32|0x51
> +  #
> +  # PcdOvmfConfidentialComputingWorkAreaHeader ==
> +  #   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> +  #
> +  # CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER defined in
> OvmfPkg/Include/WorkArea.h
> +
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r|4|UINT32|0x51
> 
> 
>  [PcdsDynamic, PcdsDynamicEx]
> diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc
> b/OvmfPkg/OvmfPkgDefines.fdf.inc
> index 3b5e452539..35fd454b97 100644
> --- a/OvmfPkg/OvmfPkgDefines.fdf.inc
> +++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
> @@ -82,12 +82,6 @@ SET
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize =
> $(BLOCK_SIZ
>  SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize =
> $(VARS_SPARE_SIZE)
> 
> -# The OVMF WorkArea contains a fixed size header followed by the actual data.
> -# The size of header is accessed through a fixed PCD in the reset vector code.
> -# The value need to be kept in sync with the any changes to the Confidential
> -# Computing Work Area header defined in the Include/WorkArea.h
> -SET
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r  = 4
> -
>  !if $(SMM_REQUIRE) == TRUE
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
> --
> 2.11.0
> 
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans
> Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA
> 7075
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD
  2021-09-17  3:23 ` Yao, Jiewen
@ 2021-09-17  5:56   ` Corvin Köhne
  2021-09-17  9:39     ` [edk2-devel] " Yao, Jiewen
  0 siblings, 1 reply; 4+ messages in thread
From: Corvin Köhne @ 2021-09-17  5:56 UTC (permalink / raw)
  To: Yao, Jiewen, devel@edk2.groups.io
  Cc: ardb+tianocore@kernel.org, jordan.l.ljusten@intel.com,
	kraxel@redhat.com, rebecca@bsdio.com, grehan@freebsd.org

Hi Yao,

looks like the patch isn't formatted correctly. Could you try to apply this patch:

From 3c22fffad51e431fbf953809215eea7022168e81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Corvin=20K=C3=B6hne?= <c.koehne@beckhoff.com>
Date: Fri, 17 Sep 2021 07:37:24 +0200
Subject: [PATCH] OvmfPkg: set a default value for the WorkAreaHeader PCD
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

SEC checks in IsSevGuest if the PCD defined WorkAreaHeader size
matches the size of the WorkAreaHeader struct definition. Set a
default value for the PCD to avoid unnecessary DSC/FDF file
changes in all OVMF DSC/FDF files.

Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
---
 OvmfPkg/Include/WorkArea.h     | 4 ++++
 OvmfPkg/OvmfPkg.dec            | 7 ++++++-
 OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ------
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
index c16030e3ac..0f9b442e43 100644
--- a/OvmfPkg/Include/WorkArea.h
+++ b/OvmfPkg/Include/WorkArea.h
@@ -25,6 +25,10 @@ typedef enum {
 // to the structure need to be kept in sync with the
 // PcdOvmfConfidentialComputingWorkAreaHeader.
 //
+// PcdOvmfConfidentialComputingWorkAreaHeader ==
+//   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
+//
+// PcdOvmfConfidentialComputingWorkAreaHeader defined in OvmfPkg/OvmfPkg.dec
 typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
   UINT8                   GuestType;
   UINT8                   Reserved1[3];
diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index c37dafad49..a5119b1909 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -338,7 +338,12 @@
   # The size of this header is used early boot, and is provided through
   # a fixed PCD. It need to be kept in sync with any changes to the
   # header definition.
-  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader|0|UINT32|0x51
+  #
+  # PcdOvmfConfidentialComputingWorkAreaHeader ==
+  #   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
+  #
+  # CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER defined in OvmfPkg/Include/WorkArea.h
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader|4|UINT32|0x51


 [PcdsDynamic, PcdsDynamicEx]
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 3b5e452539..35fd454b97 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -82,12 +82,6 @@ SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize = $(BLOCK_SIZ
 SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
 SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = $(VARS_SPARE_SIZE)

-# The OVMF WorkArea contains a fixed size header followed by the actual data.
-# The size of header is accessed through a fixed PCD in the reset vector code.
-# The value need to be kept in sync with the any changes to the Confidential
-# Computing Work Area header defined in the Include/WorkArea.h
-SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader  = 4
-
 !if $(SMM_REQUIRE) == TRUE
 SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
 SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
--
2.11.0



Thanks
Corvin

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Friday, September 17, 2021 5:23 AM
To: Corvin Köhne <C.Koehne@beckhoff.com>; devel@edk2.groups.io
Cc: ardb+tianocore@kernel.org; jordan.l.ljusten@intel.com; kraxel@redhat.com; rebecca@bsdio.com; grehan@freebsd.org; Corvin Köhne <C.Koehne@beckhoff.com>
Subject: RE: [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD

CAUTION: External Email!!


Hi Corvin
I try to apply this patch and merge. But I got error:

================
git.exe am --3way --ignore-space-change --keep-cr "C:\home\edkii\edk2\[PATCH-v3]-OvmfPkg-set-a-default-value-for-the-WorkAreaHeader-PCD-warn.patch"
Applying: OvmfPkg: set a default value for the WorkAreaHeader PCD Patch failed at 0001 OvmfPkg: set a default value for the WorkAreaHeader PCD When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

error: corrupt patch at line 38
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch

Fail
=================

Would you please take a look?

Thank you
Yao Jiewen


> -----Original Message-----
> From: Corvin Köhne <c.koehne@beckhoff.com>
> Sent: Thursday, September 16, 2021 5:03 PM
> To: devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; Yao, Jiewen <jiewen.yao@intel.com>;
> jordan.l.ljusten@intel.com; kraxel@redhat.com; rebecca@bsdio.com;
> grehan@freebsd.org; Köhne, Corvin <c.koehne@beckhoff.com>
> Subject: [PATCH v3] OvmfPkg: set a default value for the
> WorkAreaHeader PCD
>
> SEC checks in IsSevGuest if the PCD defined WorkAreaHeader size
> matches the size of the WorkAreaHeader struct definition. Set a
> default value for the PCD to avoid unnecessary DSC/FDF file changes in
> all OVMF DSC/FDF files.
>
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  OvmfPkg/Include/WorkArea.h     | 4 ++++
>  OvmfPkg/OvmfPkg.dec            | 7 ++++++-
>  OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ------
>  3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
> index c16030e3ac..0f9b442e43 100644
> --- a/OvmfPkg/Include/WorkArea.h
> +++ b/OvmfPkg/Include/WorkArea.h
> @@ -25,6 +25,10 @@ typedef enum {
>  // to the structure need to be kept in sync with the  //
> PcdOvmfConfidentialComputingWorkAreaHeader.
>  //
> +// PcdOvmfConfidentialComputingWorkAreaHeader ==
> +//   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> +//
> +// PcdOvmfConfidentialComputingWorkAreaHeader defined in
> OvmfPkg/OvmfPkg.dec
>  typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
>    UINT8                   GuestType;
>    UINT8                   Reserved1[3];
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
> c37dafad49..a5119b1909 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -338,7 +338,12 @@
>    # The size of this header is used early boot, and is provided through
>    # a fixed PCD. It need to be kept in sync with any changes to the
>    # header definition.
> -
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r|0|UINT32|0x51
> +  #
> +  # PcdOvmfConfidentialComputingWorkAreaHeader ==
> +  #   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> +  #
> +  # CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER defined in
> OvmfPkg/Include/WorkArea.h
> +
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r|4|UINT32|0x51
>
>
>  [PcdsDynamic, PcdsDynamicEx]
> diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc
> b/OvmfPkg/OvmfPkgDefines.fdf.inc index 3b5e452539..35fd454b97 100644
> --- a/OvmfPkg/OvmfPkgDefines.fdf.inc
> +++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
> @@ -82,12 +82,6 @@ SET
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize =
> $(BLOCK_SIZ  SET
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize =
> $(VARS_SPARE_SIZE)
>
> -# The OVMF WorkArea contains a fixed size header followed by the actual data.
> -# The size of header is accessed through a fixed PCD in the reset vector code.
> -# The value need to be kept in sync with the any changes to the
> Confidential -# Computing Work Area header defined in the
> Include/WorkArea.h -SET
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r  = 4
> -
>  !if $(SMM_REQUIRE) == TRUE
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
> --
> 2.11.0
>
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys.
> Hans Beckhoff Registered office: Verl, Germany | Register court:
> Guetersloh HRA
> 7075
>

Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA 7075


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD
  2021-09-17  5:56   ` Corvin Köhne
@ 2021-09-17  9:39     ` Yao, Jiewen
  0 siblings, 0 replies; 4+ messages in thread
From: Yao, Jiewen @ 2021-09-17  9:39 UTC (permalink / raw)
  To: devel@edk2.groups.io, Köhne, Corvin
  Cc: ardb+tianocore@kernel.org, jordan.l.ljusten@intel.com,
	kraxel@redhat.com, rebecca@bsdio.com, grehan@freebsd.org

Hi Corvin
Bad news: CI fails.
https://github.com/tianocore/edk2/pull/1990

Would you please take a look?

You are welcome to submit to CI by yourself. (It won't be pushed.)

Thank you
Yao Jiewen


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Corvin
> Köhne
> Sent: Friday, September 17, 2021 1:56 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; jordan.l.ljusten@intel.com; kraxel@redhat.com;
> rebecca@bsdio.com; grehan@freebsd.org
> Subject: Re: [edk2-devel] [PATCH v3] OvmfPkg: set a default value for the
> WorkAreaHeader PCD
> 
> Hi Yao,
> 
> looks like the patch isn't formatted correctly. Could you try to apply this patch:
> 
> From 3c22fffad51e431fbf953809215eea7022168e81 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Corvin=20K=C3=B6hne?= <c.koehne@beckhoff.com>
> Date: Fri, 17 Sep 2021 07:37:24 +0200
> Subject: [PATCH] OvmfPkg: set a default value for the WorkAreaHeader PCD
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> SEC checks in IsSevGuest if the PCD defined WorkAreaHeader size
> matches the size of the WorkAreaHeader struct definition. Set a
> default value for the PCD to avoid unnecessary DSC/FDF file
> changes in all OVMF DSC/FDF files.
> 
> Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> ---
>  OvmfPkg/Include/WorkArea.h     | 4 ++++
>  OvmfPkg/OvmfPkg.dec            | 7 ++++++-
>  OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ------
>  3 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
> index c16030e3ac..0f9b442e43 100644
> --- a/OvmfPkg/Include/WorkArea.h
> +++ b/OvmfPkg/Include/WorkArea.h
> @@ -25,6 +25,10 @@ typedef enum {
>  // to the structure need to be kept in sync with the
>  // PcdOvmfConfidentialComputingWorkAreaHeader.
>  //
> +// PcdOvmfConfidentialComputingWorkAreaHeader ==
> +//   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> +//
> +// PcdOvmfConfidentialComputingWorkAreaHeader defined in
> OvmfPkg/OvmfPkg.dec
>  typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
>    UINT8                   GuestType;
>    UINT8                   Reserved1[3];
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index c37dafad49..a5119b1909 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -338,7 +338,12 @@
>    # The size of this header is used early boot, and is provided through
>    # a fixed PCD. It need to be kept in sync with any changes to the
>    # header definition.
> -
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r|0|UINT32|0x51
> +  #
> +  # PcdOvmfConfidentialComputingWorkAreaHeader ==
> +  #   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> +  #
> +  # CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER defined in
> OvmfPkg/Include/WorkArea.h
> +
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r|4|UINT32|0x51
> 
> 
>  [PcdsDynamic, PcdsDynamicEx]
> diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc
> b/OvmfPkg/OvmfPkgDefines.fdf.inc
> index 3b5e452539..35fd454b97 100644
> --- a/OvmfPkg/OvmfPkgDefines.fdf.inc
> +++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
> @@ -82,12 +82,6 @@ SET
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize =
> $(BLOCK_SIZ
>  SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase +
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize =
> $(VARS_SPARE_SIZE)
> 
> -# The OVMF WorkArea contains a fixed size header followed by the actual data.
> -# The size of header is accessed through a fixed PCD in the reset vector code.
> -# The value need to be kept in sync with the any changes to the Confidential
> -# Computing Work Area header defined in the Include/WorkArea.h
> -SET
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> r  = 4
> -
>  !if $(SMM_REQUIRE) == TRUE
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
> --
> 2.11.0
> 
> 
> 
> Thanks
> Corvin
> 
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Friday, September 17, 2021 5:23 AM
> To: Corvin Köhne <C.Koehne@beckhoff.com>; devel@edk2.groups.io
> Cc: ardb+tianocore@kernel.org; jordan.l.ljusten@intel.com; kraxel@redhat.com;
> rebecca@bsdio.com; grehan@freebsd.org; Corvin Köhne
> <C.Koehne@beckhoff.com>
> Subject: RE: [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader
> PCD
> 
> CAUTION: External Email!!
> 
> 
> Hi Corvin
> I try to apply this patch and merge. But I got error:
> 
> ================
> git.exe am --3way --ignore-space-change --keep-cr
> "C:\home\edkii\edk2\[PATCH-v3]-OvmfPkg-set-a-default-value-for-the-
> WorkAreaHeader-PCD-warn.patch"
> Applying: OvmfPkg: set a default value for the WorkAreaHeader PCD Patch
> failed at 0001 OvmfPkg: set a default value for the WorkAreaHeader PCD When
> you have resolved this problem, run "git am --continue".
> If you prefer to skip this patch, run "git am --skip" instead.
> To restore the original branch and stop patching, run "git am --abort".
> 
> error: corrupt patch at line 38
> error: could not build fake ancestor
> hint: Use 'git am --show-current-patch=diff' to see the failed patch
> 
> Fail
> =================
> 
> Would you please take a look?
> 
> Thank you
> Yao Jiewen
> 
> 
> > -----Original Message-----
> > From: Corvin Köhne <c.koehne@beckhoff.com>
> > Sent: Thursday, September 16, 2021 5:03 PM
> > To: devel@edk2.groups.io
> > Cc: ardb+tianocore@kernel.org; Yao, Jiewen <jiewen.yao@intel.com>;
> > jordan.l.ljusten@intel.com; kraxel@redhat.com; rebecca@bsdio.com;
> > grehan@freebsd.org; Köhne, Corvin <c.koehne@beckhoff.com>
> > Subject: [PATCH v3] OvmfPkg: set a default value for the
> > WorkAreaHeader PCD
> >
> > SEC checks in IsSevGuest if the PCD defined WorkAreaHeader size
> > matches the size of the WorkAreaHeader struct definition. Set a
> > default value for the PCD to avoid unnecessary DSC/FDF file changes in
> > all OVMF DSC/FDF files.
> >
> > Signed-off-by: Corvin Köhne <c.koehne@beckhoff.com>
> > Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
> > ---
> >  OvmfPkg/Include/WorkArea.h     | 4 ++++
> >  OvmfPkg/OvmfPkg.dec            | 7 ++++++-
> >  OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ------
> >  3 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
> > index c16030e3ac..0f9b442e43 100644
> > --- a/OvmfPkg/Include/WorkArea.h
> > +++ b/OvmfPkg/Include/WorkArea.h
> > @@ -25,6 +25,10 @@ typedef enum {
> >  // to the structure need to be kept in sync with the  //
> > PcdOvmfConfidentialComputingWorkAreaHeader.
> >  //
> > +// PcdOvmfConfidentialComputingWorkAreaHeader ==
> > +//   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> > +//
> > +// PcdOvmfConfidentialComputingWorkAreaHeader defined in
> > OvmfPkg/OvmfPkg.dec
> >  typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
> >    UINT8                   GuestType;
> >    UINT8                   Reserved1[3];
> > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
> > c37dafad49..a5119b1909 100644
> > --- a/OvmfPkg/OvmfPkg.dec
> > +++ b/OvmfPkg/OvmfPkg.dec
> > @@ -338,7 +338,12 @@
> >    # The size of this header is used early boot, and is provided through
> >    # a fixed PCD. It need to be kept in sync with any changes to the
> >    # header definition.
> > -
> >
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> > r|0|UINT32|0x51
> > +  #
> > +  # PcdOvmfConfidentialComputingWorkAreaHeader ==
> > +  #   sizeof (CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER)
> > +  #
> > +  # CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER defined in
> > OvmfPkg/Include/WorkArea.h
> > +
> >
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> > r|4|UINT32|0x51
> >
> >
> >  [PcdsDynamic, PcdsDynamicEx]
> > diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc
> > b/OvmfPkg/OvmfPkgDefines.fdf.inc index 3b5e452539..35fd454b97 100644
> > --- a/OvmfPkg/OvmfPkgDefines.fdf.inc
> > +++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
> > @@ -82,12 +82,6 @@ SET
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize =
> > $(BLOCK_SIZ  SET
> > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase =
> > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase +
> > gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> >  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize =
> > $(VARS_SPARE_SIZE)
> >
> > -# The OVMF WorkArea contains a fixed size header followed by the actual
> data.
> > -# The size of header is accessed through a fixed PCD in the reset vector code.
> > -# The value need to be kept in sync with the any changes to the
> > Confidential -# Computing Work Area header defined in the
> > Include/WorkArea.h -SET
> >
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeade
> > r  = 4
> > -
> >  !if $(SMM_REQUIRE) == TRUE
> >  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 =
> > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
> >  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
> =
> > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
> > --
> > 2.11.0
> >
> > Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys.
> > Hans Beckhoff Registered office: Verl, Germany | Register court:
> > Guetersloh HRA
> > 7075
> >
> 
> Beckhoff Automation GmbH & Co. KG | Managing Director: Dipl. Phys. Hans
> Beckhoff Registered office: Verl, Germany | Register court: Guetersloh HRA
> 7075
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-09-17  9:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-09-16  9:03 [PATCH v3] OvmfPkg: set a default value for the WorkAreaHeader PCD Corvin Köhne
2021-09-17  3:23 ` Yao, Jiewen
2021-09-17  5:56   ` Corvin Köhne
2021-09-17  9:39     ` [edk2-devel] " Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox