public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content
@ 2020-01-08 14:38 Ard Biesheuvel
  2020-01-08 14:38 ` [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files Ard Biesheuvel
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 14:38 UTC (permalink / raw)
  To: devel; +Cc: philmd, lersek, Ard Biesheuvel

Clean up some issues that Laszlo spotted in OVMF while reviewing TPM2
support for ArmVirtQemu.

Ard Biesheuvel (2):
  OvmfPkg: reorganize TPM2 support in DSC/FDF files
  OvmfPkg: use HII type PCDs for TPM2 config related variables

 OvmfPkg/OvmfPkgIa32.dsc    | 15 ++++++++++++---
 OvmfPkg/OvmfPkgIa32X64.dsc | 15 ++++++++++++---
 OvmfPkg/OvmfPkgX64.dsc     |  9 +++++++++
 OvmfPkg/OvmfPkgIa32.fdf    |  3 +++
 OvmfPkg/OvmfPkgIa32X64.fdf |  3 +++
 OvmfPkg/OvmfPkgX64.fdf     |  3 +++
 6 files changed, 42 insertions(+), 6 deletions(-)

-- 
2.20.1


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

* [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files
  2020-01-08 14:38 [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Ard Biesheuvel
@ 2020-01-08 14:38 ` Ard Biesheuvel
  2020-01-09 12:38   ` Laszlo Ersek
  2020-01-08 14:38 ` [PATCH 2/2] OvmfPkg: use HII type PCDs for TPM2 config related variables Ard Biesheuvel
  2020-01-08 14:53 ` [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Philippe Mathieu-Daudé
  2 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 14:38 UTC (permalink / raw)
  To: devel; +Cc: philmd, lersek, Ard Biesheuvel

Put the TPM2 related DXE modules together in the DSC, and add a
TPM2 support header comment while at it.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 9 ++++++---
 OvmfPkg/OvmfPkgIa32X64.dsc | 9 ++++++---
 OvmfPkg/OvmfPkgX64.dsc     | 3 +++
 OvmfPkg/OvmfPkgIa32.fdf    | 3 +++
 OvmfPkg/OvmfPkgIa32X64.fdf | 3 +++
 OvmfPkg/OvmfPkgX64.fdf     | 3 +++
 6 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 9a60eb8fe2b0..f9e0b4b5bc54 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -632,9 +632,6 @@ [Components]
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
       NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
-!if $(TPM2_CONFIG_ENABLE) == TRUE
-  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
-!endif
 !endif
 
   #
@@ -902,6 +899,9 @@ [Components]
   }
 !endif
 
+  #
+  # TPM2 support
+  #
 !if $(TPM2_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
     <LibraryClasses>
@@ -914,4 +914,7 @@ [Components]
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
       NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+!endif
 !endif
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 1d1480b50b02..ee83bbaa5379 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -644,9 +644,6 @@ [Components.IA32]
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
       NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
-!if $(TPM2_CONFIG_ENABLE) == TRUE
-  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
-!endif
 !endif
 
 [Components.X64]
@@ -916,6 +913,9 @@ [Components.X64]
   }
 !endif
 
+  #
+  # TPM2 support
+  #
 !if $(TPM2_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
     <LibraryClasses>
@@ -928,4 +928,7 @@ [Components.X64]
       NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
       NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
   }
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+!endif
 !endif
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index c287a436f8ec..2b6106ff313f 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -914,6 +914,9 @@ [Components]
   }
 !endif
 
+  #
+  # TPM2 support
+  #
 !if $(TPM2_ENABLE) == TRUE
   SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
     <LibraryClasses>
diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 586bbff08585..63607551ed75 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -343,6 +343,9 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 !endif
 
+#
+# TPM2 support
+#
 !if $(TPM2_ENABLE) == TRUE
 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
 !if $(TPM2_CONFIG_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index e49adc425fce..0488e5d95ffe 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -350,6 +350,9 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 !endif
 
+#
+# TPM2 support
+#
 !if $(TPM2_ENABLE) == TRUE
 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
 !if $(TPM2_CONFIG_ENABLE) == TRUE
diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index e49adc425fce..0488e5d95ffe 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -350,6 +350,9 @@ [FV.DXEFV]
 INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 !endif
 
+#
+# TPM2 support
+#
 !if $(TPM2_ENABLE) == TRUE
 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
 !if $(TPM2_CONFIG_ENABLE) == TRUE
-- 
2.20.1


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

* [PATCH 2/2] OvmfPkg: use HII type PCDs for TPM2 config related variables
  2020-01-08 14:38 [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Ard Biesheuvel
  2020-01-08 14:38 ` [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files Ard Biesheuvel
@ 2020-01-08 14:38 ` Ard Biesheuvel
  2020-01-09 12:47   ` Laszlo Ersek
  2020-01-08 14:53 ` [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Philippe Mathieu-Daudé
  2 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2020-01-08 14:38 UTC (permalink / raw)
  To: devel; +Cc: philmd, lersek, Ard Biesheuvel

The HII pages that are part of Tcg2ConfigDxe expect the following PCDs
to be of dynamic HII type, so declare them as such.

  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer
  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev

Currently, the TPM2 ACPI table is not produced, since we do not
incorporate the Tcg2Smm module, which implements the SMI based
physical presence interface exposed to the OS.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
 OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
 OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index f9e0b4b5bc54..408da4cc19ac 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -575,6 +575,12 @@ [PcdsDynamicDefault]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 !endif
 
+[PcdsDynamicHii]
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index ee83bbaa5379..1ec94010c215 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -587,6 +587,12 @@ [PcdsDynamicDefault]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 !endif
 
+[PcdsDynamicHii]
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform.
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 2b6106ff313f..058ab00e69c6 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -586,6 +586,12 @@ [PcdsDynamicDefault]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
 !endif
 
+[PcdsDynamicHii]
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform.
-- 
2.20.1


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

* Re: [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content
  2020-01-08 14:38 [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Ard Biesheuvel
  2020-01-08 14:38 ` [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files Ard Biesheuvel
  2020-01-08 14:38 ` [PATCH 2/2] OvmfPkg: use HII type PCDs for TPM2 config related variables Ard Biesheuvel
@ 2020-01-08 14:53 ` Philippe Mathieu-Daudé
  2020-01-09 13:13   ` Ard Biesheuvel
  2 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-01-08 14:53 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: lersek, Marc-André Lureau, Stefan Berger

Cc'ing Marc-André & Stefan, maintainers of "OvmfPkg: TCG- and 
TPM2-related modules".

On 1/8/20 3:38 PM, Ard Biesheuvel wrote:
> Clean up some issues that Laszlo spotted in OVMF while reviewing TPM2
> support for ArmVirtQemu.
> 
> Ard Biesheuvel (2):
>    OvmfPkg: reorganize TPM2 support in DSC/FDF files
>    OvmfPkg: use HII type PCDs for TPM2 config related variables
> 
>   OvmfPkg/OvmfPkgIa32.dsc    | 15 ++++++++++++---
>   OvmfPkg/OvmfPkgIa32X64.dsc | 15 ++++++++++++---
>   OvmfPkg/OvmfPkgX64.dsc     |  9 +++++++++
>   OvmfPkg/OvmfPkgIa32.fdf    |  3 +++
>   OvmfPkg/OvmfPkgIa32X64.fdf |  3 +++
>   OvmfPkg/OvmfPkgX64.fdf     |  3 +++
>   6 files changed, 42 insertions(+), 6 deletions(-)
> 


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

* Re: [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files
  2020-01-08 14:38 ` [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files Ard Biesheuvel
@ 2020-01-09 12:38   ` Laszlo Ersek
  2020-01-09 12:40     ` Ard Biesheuvel
  2020-01-09 12:48     ` Laszlo Ersek
  0 siblings, 2 replies; 9+ messages in thread
From: Laszlo Ersek @ 2020-01-09 12:38 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: philmd

Hi Ard,

On 01/08/20 15:38, Ard Biesheuvel wrote:
> Put the TPM2 related DXE modules together in the DSC, and add a
> TPM2 support header comment while at it.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 9 ++++++---
>  OvmfPkg/OvmfPkgIa32X64.dsc | 9 ++++++---
>  OvmfPkg/OvmfPkgX64.dsc     | 3 +++

I think you forgot to stage some of the changes for "OvmfPkgX64.dsc"
before committing the patch:

>  OvmfPkg/OvmfPkgIa32.fdf    | 3 +++
>  OvmfPkg/OvmfPkgIa32X64.fdf | 3 +++
>  OvmfPkg/OvmfPkgX64.fdf     | 3 +++
>  6 files changed, 24 insertions(+), 6 deletions(-)


> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 9a60eb8fe2b0..f9e0b4b5bc54 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -632,9 +632,6 @@ [Components]
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
> -!if $(TPM2_CONFIG_ENABLE) == TRUE
> -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> -!endif
>  !endif
>  
>    #
> @@ -902,6 +899,9 @@ [Components]
>    }
>  !endif
>  
> +  #
> +  # TPM2 support
> +  #
>  !if $(TPM2_ENABLE) == TRUE
>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
>      <LibraryClasses>
> @@ -914,4 +914,7 @@ [Components]
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +!endif
>  !endif


> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 1d1480b50b02..ee83bbaa5379 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -644,9 +644,6 @@ [Components.IA32]
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
> -!if $(TPM2_CONFIG_ENABLE) == TRUE
> -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> -!endif
>  !endif
>  
>  [Components.X64]
> @@ -916,6 +913,9 @@ [Components.X64]
>    }
>  !endif
>  
> +  #
> +  # TPM2 support
> +  #
>  !if $(TPM2_ENABLE) == TRUE
>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
>      <LibraryClasses>
> @@ -928,4 +928,7 @@ [Components.X64]
>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>    }
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +!endif
>  !endif


> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index c287a436f8ec..2b6106ff313f 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -914,6 +914,9 @@ [Components]
>    }
>  !endif
>  
> +  #
> +  # TPM2 support
> +  #
>  !if $(TPM2_ENABLE) == TRUE
>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
>      <LibraryClasses>

The new comment is identical to the new comments in the other two DSC
files, but the moving around of
"SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf" (in two hunks) is missing
from "OvmfPkgX64.dsc".

Thank you for spending time on this!
Laszlo


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

* Re: [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files
  2020-01-09 12:38   ` Laszlo Ersek
@ 2020-01-09 12:40     ` Ard Biesheuvel
  2020-01-09 12:48     ` Laszlo Ersek
  1 sibling, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2020-01-09 12:40 UTC (permalink / raw)
  To: Laszlo Ersek; +Cc: edk2-devel-groups-io, Philippe Mathieu-Daudé

On Thu, 9 Jan 2020 at 13:38, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Hi Ard,
>
> On 01/08/20 15:38, Ard Biesheuvel wrote:
> > Put the TPM2 related DXE modules together in the DSC, and add a
> > TPM2 support header comment while at it.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  OvmfPkg/OvmfPkgIa32.dsc    | 9 ++++++---
> >  OvmfPkg/OvmfPkgIa32X64.dsc | 9 ++++++---
> >  OvmfPkg/OvmfPkgX64.dsc     | 3 +++
>
> I think you forgot to stage some of the changes for "OvmfPkgX64.dsc"
> before committing the patch:
>
> >  OvmfPkg/OvmfPkgIa32.fdf    | 3 +++
> >  OvmfPkg/OvmfPkgIa32X64.fdf | 3 +++
> >  OvmfPkg/OvmfPkgX64.fdf     | 3 +++
> >  6 files changed, 24 insertions(+), 6 deletions(-)
>
>
> > diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> > index 9a60eb8fe2b0..f9e0b4b5bc54 100644
> > --- a/OvmfPkg/OvmfPkgIa32.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32.dsc
> > @@ -632,9 +632,6 @@ [Components]
> >        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> >        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> > -!if $(TPM2_CONFIG_ENABLE) == TRUE
> > -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > -!endif
> >  !endif
> >
> >    #
> > @@ -902,6 +899,9 @@ [Components]
> >    }
> >  !endif
> >
> > +  #
> > +  # TPM2 support
> > +  #
> >  !if $(TPM2_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> >      <LibraryClasses>
> > @@ -914,4 +914,7 @@ [Components]
> >        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> >        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> > +!if $(TPM2_CONFIG_ENABLE) == TRUE
> > +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > +!endif
> >  !endif
>
>
> > diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> > index 1d1480b50b02..ee83bbaa5379 100644
> > --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> > +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> > @@ -644,9 +644,6 @@ [Components.IA32]
> >        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> >        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> > -!if $(TPM2_CONFIG_ENABLE) == TRUE
> > -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > -!endif
> >  !endif
> >
> >  [Components.X64]
> > @@ -916,6 +913,9 @@ [Components.X64]
> >    }
> >  !endif
> >
> > +  #
> > +  # TPM2 support
> > +  #
> >  !if $(TPM2_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> >      <LibraryClasses>
> > @@ -928,4 +928,7 @@ [Components.X64]
> >        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> >        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> >    }
> > +!if $(TPM2_CONFIG_ENABLE) == TRUE
> > +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> > +!endif
> >  !endif
>
>
> > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> > index c287a436f8ec..2b6106ff313f 100644
> > --- a/OvmfPkg/OvmfPkgX64.dsc
> > +++ b/OvmfPkg/OvmfPkgX64.dsc
> > @@ -914,6 +914,9 @@ [Components]
> >    }
> >  !endif
> >
> > +  #
> > +  # TPM2 support
> > +  #
> >  !if $(TPM2_ENABLE) == TRUE
> >    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> >      <LibraryClasses>
>
> The new comment is identical to the new comments in the other two DSC
> files, but the moving around of
> "SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf" (in two hunks) is missing
> from "OvmfPkgX64.dsc".
>

Ugh yes, I will fix up and resend.

> Thank you for spending time on this!
> Laszlo
>

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

* Re: [PATCH 2/2] OvmfPkg: use HII type PCDs for TPM2 config related variables
  2020-01-08 14:38 ` [PATCH 2/2] OvmfPkg: use HII type PCDs for TPM2 config related variables Ard Biesheuvel
@ 2020-01-09 12:47   ` Laszlo Ersek
  0 siblings, 0 replies; 9+ messages in thread
From: Laszlo Ersek @ 2020-01-09 12:47 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: philmd

On 01/08/20 15:38, Ard Biesheuvel wrote:
> The HII pages that are part of Tcg2ConfigDxe expect the following PCDs
> to be of dynamic HII type, so declare them as such.
> 
>   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer
>   gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev
> 
> Currently, the TPM2 ACPI table is not produced, since we do not
> incorporate the Tcg2Smm module, which implements the SMI based
> physical presence interface exposed to the OS.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 6 ++++++
>  OvmfPkg/OvmfPkgIa32X64.dsc | 6 ++++++
>  OvmfPkg/OvmfPkgX64.dsc     | 6 ++++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index f9e0b4b5bc54..408da4cc19ac 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -575,6 +575,12 @@ [PcdsDynamicDefault]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>  !endif
>  
> +[PcdsDynamicHii]
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
> +!endif
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform.
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index ee83bbaa5379..1ec94010c215 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -587,6 +587,12 @@ [PcdsDynamicDefault]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>  !endif
>  
> +[PcdsDynamicHii]
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
> +!endif
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform.
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 2b6106ff313f..058ab00e69c6 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -586,6 +586,12 @@ [PcdsDynamicDefault]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
>  !endif
>  
> +[PcdsDynamicHii]
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
> +!endif
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform.
> 

Please restrict the conditions as follows (in order to match the
condition that ultimately arises for the rest of the TPM2_CONFIG_ENABLE
stuff):

  !if $(TPM2_ENABLE) == TRUE && $(TPM2_CONFIG_ENABLE) == TRUE

With that:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thank you!
Laszlo


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

* Re: [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files
  2020-01-09 12:38   ` Laszlo Ersek
  2020-01-09 12:40     ` Ard Biesheuvel
@ 2020-01-09 12:48     ` Laszlo Ersek
  1 sibling, 0 replies; 9+ messages in thread
From: Laszlo Ersek @ 2020-01-09 12:48 UTC (permalink / raw)
  To: Ard Biesheuvel, devel; +Cc: philmd

On 01/09/20 13:38, Laszlo Ersek wrote:
> Hi Ard,
> 
> On 01/08/20 15:38, Ard Biesheuvel wrote:
>> Put the TPM2 related DXE modules together in the DSC, and add a
>> TPM2 support header comment while at it.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  OvmfPkg/OvmfPkgIa32.dsc    | 9 ++++++---
>>  OvmfPkg/OvmfPkgIa32X64.dsc | 9 ++++++---
>>  OvmfPkg/OvmfPkgX64.dsc     | 3 +++
> 
> I think you forgot to stage some of the changes for "OvmfPkgX64.dsc"
> before committing the patch:
> 
>>  OvmfPkg/OvmfPkgIa32.fdf    | 3 +++
>>  OvmfPkg/OvmfPkgIa32X64.fdf | 3 +++
>>  OvmfPkg/OvmfPkgX64.fdf     | 3 +++
>>  6 files changed, 24 insertions(+), 6 deletions(-)
> 
> 
>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
>> index 9a60eb8fe2b0..f9e0b4b5bc54 100644
>> --- a/OvmfPkg/OvmfPkgIa32.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32.dsc
>> @@ -632,9 +632,6 @@ [Components]
>>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>>    }
>> -!if $(TPM2_CONFIG_ENABLE) == TRUE
>> -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>> -!endif
>>  !endif
>>  
>>    #
>> @@ -902,6 +899,9 @@ [Components]
>>    }
>>  !endif
>>  
>> +  #
>> +  # TPM2 support
>> +  #
>>  !if $(TPM2_ENABLE) == TRUE
>>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
>>      <LibraryClasses>
>> @@ -914,4 +914,7 @@ [Components]
>>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>>    }
>> +!if $(TPM2_CONFIG_ENABLE) == TRUE
>> +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>> +!endif
>>  !endif
> 
> 
>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
>> index 1d1480b50b02..ee83bbaa5379 100644
>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
>> @@ -644,9 +644,6 @@ [Components.IA32]
>>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>>    }
>> -!if $(TPM2_CONFIG_ENABLE) == TRUE
>> -  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>> -!endif
>>  !endif
>>  
>>  [Components.X64]
>> @@ -916,6 +913,9 @@ [Components.X64]
>>    }
>>  !endif
>>  
>> +  #
>> +  # TPM2 support
>> +  #
>>  !if $(TPM2_ENABLE) == TRUE
>>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
>>      <LibraryClasses>
>> @@ -928,4 +928,7 @@ [Components.X64]
>>        NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
>>        NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
>>    }
>> +!if $(TPM2_CONFIG_ENABLE) == TRUE
>> +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
>> +!endif
>>  !endif
> 
> 
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index c287a436f8ec..2b6106ff313f 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -914,6 +914,9 @@ [Components]
>>    }
>>  !endif
>>  
>> +  #
>> +  # TPM2 support
>> +  #
>>  !if $(TPM2_ENABLE) == TRUE
>>    SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
>>      <LibraryClasses>
> 
> The new comment is identical to the new comments in the other two DSC
> files, but the moving around of
> "SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf" (in two hunks) is missing
> from "OvmfPkgX64.dsc".

With that fixed, you can add:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo


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

* Re: [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content
  2020-01-08 14:53 ` [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Philippe Mathieu-Daudé
@ 2020-01-09 13:13   ` Ard Biesheuvel
  0 siblings, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2020-01-09 13:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: edk2-devel-groups-io, Laszlo Ersek, Marc-André Lureau,
	Stefan Berger

On Wed, 8 Jan 2020 at 15:53, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Cc'ing Marc-André & Stefan, maintainers of "OvmfPkg: TCG- and
> TPM2-related modules".
>
> On 1/8/20 3:38 PM, Ard Biesheuvel wrote:
> > Clean up some issues that Laszlo spotted in OVMF while reviewing TPM2
> > support for ArmVirtQemu.
> >
> > Ard Biesheuvel (2):
> >    OvmfPkg: reorganize TPM2 support in DSC/FDF files
> >    OvmfPkg: use HII type PCDs for TPM2 config related variables
> >

Pushed with Laszlo's feedback incorporated and R-b's added.

Thanks,

> >   OvmfPkg/OvmfPkgIa32.dsc    | 15 ++++++++++++---
> >   OvmfPkg/OvmfPkgIa32X64.dsc | 15 ++++++++++++---
> >   OvmfPkg/OvmfPkgX64.dsc     |  9 +++++++++
> >   OvmfPkg/OvmfPkgIa32.fdf    |  3 +++
> >   OvmfPkg/OvmfPkgIa32X64.fdf |  3 +++
> >   OvmfPkg/OvmfPkgX64.fdf     |  3 +++
> >   6 files changed, 42 insertions(+), 6 deletions(-)
> >
>

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

end of thread, other threads:[~2020-01-09 13:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-01-08 14:38 [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Ard Biesheuvel
2020-01-08 14:38 ` [PATCH 1/2] OvmfPkg: reorganize TPM2 support in DSC/FDF files Ard Biesheuvel
2020-01-09 12:38   ` Laszlo Ersek
2020-01-09 12:40     ` Ard Biesheuvel
2020-01-09 12:48     ` Laszlo Ersek
2020-01-08 14:38 ` [PATCH 2/2] OvmfPkg: use HII type PCDs for TPM2 config related variables Ard Biesheuvel
2020-01-09 12:47   ` Laszlo Ersek
2020-01-08 14:53 ` [PATCH 0/2] OvmfPkg: clean up TPM2 related DSC/FDF content Philippe Mathieu-Daudé
2020-01-09 13:13   ` Ard Biesheuvel

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