Re: [RFC] TPM non-MMIO Access

["Cohen, Eugene" <eugene@hp.com>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 6300 bytes --]

Jiewen,

We don't have a patch yet – we wanted to check with you first before going too far.


Ø  Or just a simple replace-all for MmioRead/Write->TpmMmioRead/Write.

Yes, basically – with a library class to support it.

We should not call it "TpmMmioRead8" since on some paths it is not MMIO at all.  We should call it TpmRead8 (or something like that) and then the MMIO instance of the library class would call Mmio functions.

Thanks,

Eugene

From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Sunday, November 11, 2018 9:33 PM
To: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com>; Cohen, Eugene <eugene@hp.com>; edk2-devel@lists.01.org; Zhang, Chao B <chao.b.zhang@intel.com>
Cc: Chae, Matthew <matthew.chae@hp.com>
Subject: RE: [RFC] TPM non-MMIO Access

OK. Thanks for the clarification.

If possible, would you please post your patch to somewhere?

Do you request to change any other logic in existing Tpm2DeviceLib ?
Or just a simple replace-all for MmioRead/Write->TpmMmioRead/Write.

Thank you
Yao Jiewen

From: Bin, Sung-Uk (빈성욱) [mailto:sunguk-bin@hp.com]
Sent: Monday, November 12, 2018 12:17 PM
To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
Cc: Chae, Matthew <matthew.chae@hp.com<mailto:matthew.chae@hp.com>>
Subject: RE: [RFC] TPM non-MMIO Access

Hi Jiewen

What Eugene proposes is not to make those another TPM device library instances.
Instead, we propose new “TpmIoLib” which can handle both MMIO and non-MMIO device.

è Currently Tpm12Tis.c, Tpm2Tis.c and Tpm2Ptp.c uses IoLib (CPU IO), and what we propose is to replace it with TpmIoLib.

For example, TpmIoLib can provide TpmMmioRead() and Tpm12Tis.c, Tpm2Tis.c and Tpm2Ptp.c can use TpmMmioRead() instead of MmioRead().

-          TpmIoLib for PC (default in SecurityPkg)
UINT8 TpmMmioRead8 (UINTN  Address )
{
                return MmioRead8(Address);
}


-          TpmIoLib for SPI (vendor creates new instance)
UINT8 TpmMmioRead8 (UINTN  Address )
{
                UINT8 Data, SpiCs;
                SpiCs = (Address & 0xFF0000) >> 16;
                TpmAddress = Address & 0xFFFF;

               /* Vendor specific SPI control for TPM */
                …
                SpiWrite(SpiCs, TpmAddress);
                …
                SpiRead(SpiCs, TpmAddress, &Data);
                return Data;
}

This proposal came from code maintanance,
when we need to update SecurityPkg, then in this case it’s more easy to update than making another Tpm2DeviceLibDTpm instance.

Thanks,
Bin

From: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Sent: Saturday, November 10, 2018 8:18 AM
To: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
Cc: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com<mailto:sunguk-bin@hp.com>>; Chae, Matthew <matthew.chae@hp.com<mailto:matthew.chae@hp.com>>
Subject: RE: [RFC] TPM non-MMIO Access

Hi Eugene
The TpmIoLib proposal is similar to the existing TpmDeviceLib.
We have I2C TPM instance as example. You may create Tpm12DeviceLibXXXSpi.

Please let us know if there is any gap to support your non-MMIO device.

Thank you
Yao Jiewen


From: Yao, Jiewen
Sent: Saturday, November 10, 2018 7:12 AM
To: 'Cohen, Eugene' <eugene@hp.com<mailto:eugene@hp.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>
Cc: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com<mailto:sunguk-bin@hp.com>>; Chae, Matthew <matthew.chae@hp.com<mailto:matthew.chae@hp.com>>
Subject: RE: [RFC] TPM non-MMIO Access

Hi Eugene
Complete agree.


1)       please ignore TpmCommLib. It is deprecated. ☺


2)       we did notice there is non-MMIO TPM device before and abstract the device access via Tpm2DeviceLib and Tpm12DeviceLib library class. The Tpm2DeviceLibDTpm and Tpm12DeviceLibDTpm are the library instance for MMIO TIS access.

We do have other instance such as QuarkPlatformPkg\Library\Tpm12DeviceLibAtmelI2c or QuarkPlatformPkg\Library\Tpm12DeviceLibInfineonI2c.


3)       Tcg2Config is also a TPM device oriented driver. It is optional.
You may want to take a look at QuarkPlatformPkg\Quark.dsc to see different TPM device is chosen.

  #
  # TPM 1.2 Hardware.  Options are [NONE, LPC, ATMEL_I2C, INFINEON_I2C]
  #
  DEFINE TPM_12_HARDWARE      = NONE

Thank you
Yao Jiewen

From: Cohen, Eugene [mailto:eugene@hp.com]
Sent: Saturday, November 10, 2018 6:38 AM
To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Zhang, Chao B <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Cc: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com<mailto:sunguk-bin@hp.com>>; Chae, Matthew <matthew.chae@hp.com<mailto:matthew.chae@hp.com>>
Subject: [RFC] TPM non-MMIO Access

Dear SecurityPkg maintainers,

We are trying to use the SecurityPkg TPM support (Tcg2Config, Tpm2DeviceLibDTpm, etc) and see that access to the TPM are using the MMIO routines such as MmioRead8().

Not all platforms support memory-mapped access to the TPM so we would like to propose that we create a library to abstract the TPM access, called the TpmIoLib.  One instance of the library would provide the Mmio (TpmIoLibMmio) method but others can use protocols like SPI (TpmIoLibSpi).

Before starting this work I wanted to check if you agree with the approach of replacing the Mmio calls with new TpmIoLib ones.  We can upstream the library header and mmio library instance (or you could do the work if you think it would be easier for you).

I counted up the number of instances of unique MMIO calls in the TPM libraries and got

Library/TpmCommLib/TisPc.c:7
Library/Tpm2DeviceLibDTpm/Tpm2Tis.c:13
Library/Tpm2DeviceLibDTpm/Tpm2Ptp.c:29
Library/Tpm12DeviceLibDTpm/Tpm12Tis.c:16

Please let me know if this works for you and how you'd like to structure the change.

Thanks,

Eugene




\x16º&