Re: [edk2-devel] [PATCH 2/2] OvmfPkg/Tcg2Config: remove unused TPM 1.2 support

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

On 1/16/24 16:42, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf | 56 ---------------
>  OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c     | 83 -----------------------
>  2 files changed, 139 deletions(-)
>  delete mode 100644 OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
>  delete mode 100644 OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> 
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
> deleted file mode 100644
> index e8e0b88e6058..000000000000
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg12ConfigPei.inf
> +++ /dev/null
> @@ -1,56 +0,0 @@
> -## @file
> -# Set TPM device type - supports TPM 1.2 and 2.0
> -#
> -# In SecurityPkg, this module initializes the TPM device type based on a UEFI
> -# variable and/or hardware detection. In OvmfPkg, the module only performs TPM
> -# hardware detection.
> -#
> -# Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> -# Copyright (C) 2018, Red Hat, Inc.
> -#
> -# SPDX-License-Identifier: BSD-2-Clause-Patent
> -##
> -
> -[Defines]
> -  INF_VERSION                    = 0x00010005
> -  BASE_NAME                      = Tcg2ConfigPei
> -  FILE_GUID                      = 8AD3148F-945F-46B4-8ACD-71469EA73945
> -  MODULE_TYPE                    = PEIM
> -  VERSION_STRING                 = 1.0
> -  ENTRY_POINT                    = Tcg2ConfigPeimEntryPoint
> -
> -[Sources]
> -  Tcg2ConfigPeim.c
> -  Tpm12Support.h
> -  Tpm12Support.c
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
> -  OvmfPkg/OvmfPkg.dec
> -  SecurityPkg/SecurityPkg.dec
> -
> -[LibraryClasses]
> -  PeimEntryPoint
> -  DebugLib
> -  PeiServicesLib
> -  Tpm2DeviceLib
> -  BaseLib
> -  Tpm12DeviceLib
> -
> -[Guids]
> -  gEfiTpmDeviceSelectedGuid           ## PRODUCES ## GUID # Used as a PPI GUID
> -  gEfiTpmDeviceInstanceTpm20DtpmGuid  ## SOMETIMES_CONSUMES
> -  gEfiTpmDeviceInstanceTpm12Guid      ## SOMETIMES_CONSUMES
> -
> -[Ppis]
> -  gPeiTpmInitializationDonePpiGuid    ## SOMETIMES_PRODUCES
> -
> -[Pcd]
> -  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid                 ## PRODUCES
> -
> -[Depex.IA32, Depex.X64]
> -  gOvmfTpmMmioAccessiblePpiGuid
> -
> -[Depex.ARM, Depex.AARCH64]
> -  gOvmfTpmDiscoveredPpiGuid
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c b/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> deleted file mode 100644
> index c88da5758b44..000000000000
> --- a/OvmfPkg/Tcg/Tcg2Config/Tpm12Support.c
> +++ /dev/null
> @@ -1,83 +0,0 @@
> -/** @file
> -  Implement the InternalTpm12Detect() function on top of the Tpm12DeviceLib
> -  class.
> -
> -  Copyright (C) 2020, Red Hat, Inc.
> -
> -  SPDX-License-Identifier: BSD-2-Clause-Patent
> -**/
> -
> -#include <Library/BaseLib.h>
> -#include <Library/Tpm12DeviceLib.h>
> -
> -#include "Tpm12Support.h"
> -
> -#pragma pack (1)
> -typedef struct {
> -  TPM_RSP_COMMAND_HDR    Hdr;
> -  TPM_CURRENT_TICKS      CurrentTicks;
> -} TPM_RSP_GET_TICKS;
> -#pragma pack ()
> -
> -/**
> -  Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
> -
> -  Sending a TPM1.2 command to a TPM2 should return a TPM1.2
> -  header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
> -
> -  @retval EFI_SUCCESS  TPM version 1.2 probing successful.
> -
> -  @return              Error codes propagated from Tpm12SubmitCommand().
> -**/
> -STATIC
> -EFI_STATUS
> -TestTpm12 (
> -  )
> -{
> -  EFI_STATUS           Status;
> -  TPM_RQU_COMMAND_HDR  Command;
> -  TPM_RSP_GET_TICKS    Response;
> -  UINT32               Length;
> -
> -  Command.tag       = SwapBytes16 (TPM_TAG_RQU_COMMAND);
> -  Command.paramSize = SwapBytes32 (sizeof (Command));
> -  Command.ordinal   = SwapBytes32 (TPM_ORD_GetTicks);
> -
> -  Length = sizeof (Response);
> -  Status = Tpm12SubmitCommand (
> -             sizeof (Command),
> -             (UINT8 *)&Command,
> -             &Length,
> -             (UINT8 *)&Response
> -             );
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  return EFI_SUCCESS;
> -}
> -
> -/**
> -  Detect the presence of a TPM with interface version 1.2.
> -
> -  @retval EFI_SUCCESS      TPM-1.2 available. The Tpm12RequestUseTpm() and
> -                           Tpm12SubmitCommand(TPM_ORD_GetTicks) operations
> -                           (from the Tpm12DeviceLib class) have succeeded.
> -
> -  @return                  Error codes propagated from Tpm12RequestUseTpm() and
> -                           Tpm12SubmitCommand().
> -**/
> -EFI_STATUS
> -InternalTpm12Detect (
> -  VOID
> -  )
> -{
> -  EFI_STATUS  Status;
> -
> -  Status = Tpm12RequestUseTpm ();
> -  if (EFI_ERROR (Status)) {
> -    return Status;
> -  }
> -
> -  return TestTpm12 ();
> -}

This patch is good:

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

but the series shouldn't stop here. In "OvmfPkg/Tcg/Tcg2Config", we're
left with an INF file (Tcg2ConfigPei.inf) that still references
"Tpm12Support.h", and the common C source file "Tcg2ConfigPeim.c" still
calls the one API -- InternalTpm12Detect() -- declared in that header
file. The only remaining implementation of InternalTpm12Detect() is now
in "Tpm12SupportNull.c", and all it does is "return EFI_UNSUPPORTED".

Therefore, in a subsequent patch, "Tpm12SupportNull.c" and
"Tpm12Support.h" should be removed, both from the tree, and from the
remaining INF file. Furthermore, the InternalTpm12Detect() call in
"Tcg2ConfigPeim.c", and everything that depends on the success of that
call, now counts as dead code, and should be removed.

And *that* in turn means that we should also remove
"gEfiTpmDeviceInstanceTpm12Guid" from the [Guids] section of the
remaining INF file.

Thanks,
Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113940): https://edk2.groups.io/g/devel/message/113940
Mute This Topic: https://groups.io/mt/103764205/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-