Re: [PATCH 3/4] SecurityPkg: add TpmIoLibMmio instance

["Kinney, Michael D" <michael.d.kinney@intel.com>]

Eugene,

It appears you are expecting the Address parameter
to be the full MMIO address for DTPM.  If you are
wanting this lib class to abstract the access for
different bus types, shouldn’t the Address be the
relative address from a base?

Or are you using the full DTPM MMIO address on 
purpose and you expect non DTPM MMIO implementations
to translate from the DTPM MMIO address to the
equivalent register access in on an alternate bus
type?

Wouldn't it be better to have an abstraction for
different TPM registers and the DTPM implementation
would convert to a full MMIO address in the lib
implementation?

Thanks,

Mike


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Cohen, Eugene
> Sent: Tuesday, November 13, 2018 2:13 PM
> To: edk2-devel@lists.01.org; Yao, Jiewen
> <jiewen.yao@intel.com>; Zhang, Chao B
> <chao.b.zhang@intel.com>
> Cc: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com>
> Subject: [edk2] [PATCH 3/4] SecurityPkg: add
> TpmIoLibMmio instance
> 
> SecurityPkg: add TpmIoLibMmio instance
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Cc: Chao Zhang <chao.b.zhang@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Signed-off-by: Eugene Cohen <eugene@hp.com>
> ---
>  SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.c   |
> 221 ++++++++++++++++++++
>  SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.inf |
> 41 ++++
>  2 files changed, 262 insertions(+)
> 
> diff --git
> a/SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.c
> b/SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.c
> new file mode 100644
> index 0000000..56f3187
> --- /dev/null
> +++ b/SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.c
> @@ -0,0 +1,221 @@
> +/** @file
> +  This library is to abstract TPM2 register accesses
> so that a common
> +  interface can be used for multiple underlying busses
> such as TPM,
> +  SPI, or I2C access.
> +
> +Copyright (c) 2018 HP Development Company, L.P.
> +This program and the accompanying materials
> +are licensed and made available under the terms and
> conditions of the BSD License
> +which accompanies this distribution.  The full text of
> the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN
> "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <Base.h>
> +
> +#include <Library/TpmIoLib.h>
> +#include <Library/IoLib.h>
> +
> +
> +
> +/**
> +  Reads an 8-bit TPM register.
> +
> +  Reads the 8-bit TPM register specified by Address.
> The 8-bit read value is
> +  returned. This function must guarantee that all TPM
> read and write
> +  operations are serialized.
> +
> +  If 8-bit TPM register operations are not supported,
> then ASSERT().
> +
> +  @param  Address The TPM register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT8
> +EFIAPI
> +TpmRead8 (
> +  IN      UINTN                     Address
> +  )
> +{
> +  return MmioRead8 (Address);
> +}
> +
> +/**
> +  Writes an 8-bit TPM register.
> +
> +  Writes the 8-bit TPM register specified by Address
> with the value specified
> +  by Value and returns Value. This function must
> guarantee that all TPM read
> +  and write operations are serialized.
> +
> +  If 8-bit TPM register operations are not supported,
> then ASSERT().
> +
> +  @param  Address The TPM register to write.
> +  @param  Value   The value to write to the TPM
> register.
> +
> +  @return Value.
> +
> +**/
> +UINT8
> +EFIAPI
> +TpmWrite8 (
> +  IN      UINTN                     Address,
> +  IN      UINT8                     Value
> +  )
> +{
> +  return MmioWrite8 (Address, Value);
> +}
> +
> +
> +/**
> +  Reads a 16-bit TPM register.
> +
> +  Reads the 16-bit TPM register specified by Address.
> The 16-bit read value is
> +  returned. This function must guarantee that all TPM
> read and write
> +  operations are serialized.
> +
> +  If 16-bit TPM register operations are not supported,
> then ASSERT().
> +  If Address is not aligned on a 16-bit boundary, then
> ASSERT().
> +
> +  @param  Address The TPM register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT16
> +EFIAPI
> +TpmRead16 (
> +  IN      UINTN                     Address
> +  )
> +{
> +  return MmioRead16 (Address);
> +}
> +
> +
> +/**
> +  Writes a 16-bit TPM register.
> +
> +  Writes the 16-bit TPM register specified by Address
> with the value specified
> +  by Value and returns Value. This function must
> guarantee that all TPM read
> +  and write operations are serialized.
> +
> +  If 16-bit TPM register operations are not supported,
> then ASSERT().
> +  If Address is not aligned on a 16-bit boundary, then
> ASSERT().
> +
> +  @param  Address The TPM register to write.
> +  @param  Value   The value to write to the TPM
> register.
> +
> +  @return Value.
> +
> +**/
> +UINT16
> +EFIAPI
> +TpmWrite16 (
> +  IN      UINTN                     Address,
> +  IN      UINT16                    Value
> +  )
> +{
> +  return MmioWrite16 (Address, Value);
> +}
> +
> +/**
> +  Reads a 32-bit TPM register.
> +
> +  Reads the 32-bit TPM register specified by Address.
> The 32-bit read value is
> +  returned. This function must guarantee that all TPM
> read and write
> +  operations are serialized.
> +
> +  If 32-bit TPM register operations are not supported,
> then ASSERT().
> +  If Address is not aligned on a 32-bit boundary, then
> ASSERT().
> +
> +  @param  Address The TPM register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT32
> +EFIAPI
> +TpmRead32 (
> +  IN      UINTN                     Address
> +  )
> +{
> +  return MmioRead32 (Address);
> +}
> +
> +/**
> +  Writes a 32-bit TPM register.
> +
> +  Writes the 32-bit TPM register specified by Address
> with the value specified
> +  by Value and returns Value. This function must
> guarantee that all TPM read
> +  and write operations are serialized.
> +
> +  If 32-bit TPM register operations are not supported,
> then ASSERT().
> +  If Address is not aligned on a 32-bit boundary, then
> ASSERT().
> +
> +  @param  Address The TPM register to write.
> +  @param  Value   The value to write to the TPM
> register.
> +
> +  @return Value.
> +
> +**/
> +UINT32
> +EFIAPI
> +TpmWrite32 (
> +  IN      UINTN                     Address,
> +  IN      UINT32                    Value
> +  )
> +{
> +  return MmioWrite32 (Address, Value);
> +}
> +
> +
> +/**
> +  Reads a 64-bit TPM register.
> +
> +  Reads the 64-bit TPM register specified by Address.
> The 64-bit read value is
> +  returned. This function must guarantee that all TPM
> read and write
> +  operations are serialized.
> +
> +  If 64-bit TPM register operations are not supported,
> then ASSERT().
> +  If Address is not aligned on a 64-bit boundary, then
> ASSERT().
> +
> +  @param  Address The TPM register to read.
> +
> +  @return The value read.
> +
> +**/
> +UINT64
> +EFIAPI
> +TpmRead64 (
> +  IN      UINTN                     Address
> +  )
> +{
> +  return MmioRead64 (Address);
> +}
> +
> +/**
> +  Writes a 64-bit TPM register.
> +
> +  Writes the 64-bit TPM register specified by Address
> with the value specified
> +  by Value and returns Value. This function must
> guarantee that all TPM read
> +  and write operations are serialized.
> +
> +  If 64-bit TPM register operations are not supported,
> then ASSERT().
> +  If Address is not aligned on a 64-bit boundary, then
> ASSERT().
> +
> +  @param  Address The TPM register to write.
> +  @param  Value   The value to write to the TPM
> register.
> +
> +**/
> +UINT64
> +EFIAPI
> +TpmWrite64 (
> +  IN      UINTN                     Address,
> +  IN      UINT64                    Value
> +  )
> +{
> +  return MmioWrite64 (Address, Value);
> +}
> diff --git
> a/SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.inf
> b/SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.inf
> new file mode 100644
> index 0000000..a40f90e
> --- /dev/null
> +++ b/SecurityPkg/Library/TpmIoLibMmio/TpmIoLibMmio.inf
> @@ -0,0 +1,41 @@
> +## @file
> +#  Provides BaseCrypto SHA1 hash service
> +#
> +#  This library is to abstract TPM2 register accesses
> so that a common
> +#  interface can be used for multiple underlying
> busses such as TPM,
> +#  SPI, or I2C access.
> +#
> +# Copyright (c) 2018 HP Development Company, L.P.
> +# This program and the accompanying materials
> +# are licensed and made available under the terms and
> conditions of the BSD License
> +# which accompanies this distribution. The full text
> of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = TpmIoLibMmio
> +  FILE_GUID                      = B51DE0C6-8A48-4AF9-
> BF88-5A46CC853FC8
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = NULL
> +
> +#
> +# The following information is for reference only and
> not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 ARM
> AARCH64
> +#
> +
> +[Sources]
> +  TpmIoLibMmio.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  SecurityPkg/SecurityPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  IoLib
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel