Re: [PATCH 1/4] OvmfPkg/Tcg2ConfigPei: introduce a signalling PPI to depex on

["Laszlo Ersek" <lersek@redhat.com>]

On 01/07/20 10:47, Ard Biesheuvel wrote:
> On ARM systems, the TPM does not live at a fixed address, and so we
> need the platform to discover it first. So introduce a PPI that signals
> that the TPM address has been discovered and recorded in the appropriate
> PCD, and make Tcg2ConfigPei depex on it when built for ARM or AARCH64.

I got briefly confused about this model, but after reviewing the commit
message of 6cf1880fb5b6 ("OvmfPkg: add customized Tcg2ConfigPei clone",
2018-03-09), I agree the above is a good addition / good fit.

> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  OvmfPkg/OvmfPkg.dec                      | 4 ++++
>  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 6 +++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
> index d5fee805ef4a..10a2b714c1b4 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -101,6 +101,10 @@ [Protocols]
>    gEfiLegacyInterruptProtocolGuid     = {0x31ce593d, 0x108a, 0x485d, {0xad, 0xb2, 0x78, 0xf2, 0x1f, 0x29, 0x66, 0xbe}}
>    gEfiVgaMiniPortProtocolGuid         = {0xc7735a2f, 0x88f5, 0x4882, {0xae, 0x63, 0xfa, 0xac, 0x8c, 0x8b, 0x86, 0xb3}}
>  
> +[Ppis]
> +  # PPI whose presence in the PPI database signals that the TPM base address has been discovered and recorded
> +  gOvmfTpmDiscoveredPpiGuid           = {0xb9a61ad0, 0x2802, 0x41f3, {0xb5, 0x13, 0x96, 0x51, 0xce, 0x6b, 0xd5, 0x75}}
> +

(1) Please rewrap the comment to 80 characters. (The DEC file is
consistent in that, so I'd like to stick with it.)

(2) If you agree, I'd suggest moving the [Ppis] section above the
[Protocols] section, but still below the [Guids] section -- somehow, for
me, that seems to match the "level of abstraction" in PPIs. But, again,
this is optional.

With the above addressed/considered:

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

Thanks,
Laszlo

>  [PcdsFixedAtBuild]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|0x0|UINT32|0
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize|0x0|UINT32|1
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> index e34cd6210611..55684ba045b3 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -25,6 +25,7 @@ [Sources]
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> +  OvmfPkg/OvmfPkg.dec
>    SecurityPkg/SecurityPkg.dec
>  
>  [LibraryClasses]
> @@ -43,5 +44,8 @@ [Ppis]
>  [Pcd]
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid                 ## PRODUCES
>  
> -[Depex]
> +[Depex.IA32, Depex.X64]
>    TRUE
> +
> +[Depex.ARM, Depex.AARCH64]
> +  gOvmfTpmDiscoveredPpiGuid
>