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: 10413 bytes --]

Mike, thanks.


Ø  Do we know for sure with a change like this that

Ø  MMIO and non-MMIO TPMs support the same registers

Ø  and require the exact same sequence of register

Ø  access to function correctly?

I'm checking with our TPM experts - I was under the impression that the interface was the same regardless of transport.  Bin and Jiewen, please offer your guidance.

Ø  Also, some of the non-MMIO bus types have much

Ø  different performance characteristics, which may

Ø  require slightly different interactions for best

Ø  performance.

Agreed.  That being said duplicating most of the logic in the device lib may be excessive - on quick glance it looks like there is a lot of logic that we might want to keep in both cases.

Thanks,

Eugene

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

Eugene,

I understand the intent.

Do we know for sure with a change like this that
MMIO and non-MMIO TPMs support the same registers
and require the exact same sequence of register
access to function correctly? If not, then the
value of adding this new lib class is small.

Also, some of the non-MMIO bus types have much
different performance characteristics, which may
require slightly different interactions for best
performance.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org<mailto:bounces@lists.01.org>] On Behalf Of Cohen, Eugene
> Sent: Monday, November 12, 2018 5:59 AM
> To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Bin, Sung-Uk
> (빈성욱) <sunguk-bin@hp.com<mailto:sunguk-bin@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: [edk2] [RFC] TPM non-MMIO Access
>
> 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<mailto:jiewen.yao@intel.com>>
> Sent: Sunday, November 11, 2018 9:33 PM
> To: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com<mailto:sunguk-bin@hp.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
>
> 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<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>;
> Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com<mailto:eugene@hp.com%3cmailto:eugene@hp.com>>>;
> edk2-devel@lists.01.org<mailto:edk2-<mailto:edk2-devel@lists.01.org%3cmailto:edk2->
> devel@lists.01.org<mailto:devel@lists.01.org>>; Zhang, Chao B
> <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
> Cc: Chae, Matthew
> <matthew.chae@hp.com<mailto:matthew.chae@hp.com<mailto:matthew.chae@hp.com%3cmailto: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<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>
> Sent: Saturday, November 10, 2018 8:18 AM
> To: Cohen, Eugene
> <eugene@hp.com<mailto:eugene@hp.com<mailto:eugene@hp.com%3cmailto:eugene@hp.com>>>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>;
> Zhang, Chao B
> <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
> Cc: Bin, Sung-Uk (빈성욱) <sunguk-
> bin@hp.com<mailto:sunguk-bin@hp.com<mailto:bin@hp.com%3cmailto:sunguk-bin@hp.com>>>; Chae, Matthew
> <matthew.chae@hp.com<mailto:matthew.chae@hp.com<mailto:matthew.chae@hp.com%3cmailto: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<mailto:eugene@hp.com%3cmailto:eugene@hp.com>>>; edk2-
> devel@lists.01.org<mailto:edk2-devel@lists.01.org<mailto:devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>>;
> Zhang, Chao B
> <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
> Cc: Bin, Sung-Uk (빈성욱) <sunguk-
> bin@hp.com<mailto:sunguk-bin@hp.com<mailto:bin@hp.com%3cmailto:sunguk-bin@hp.com>>>; Chae, Matthew
> <matthew.chae@hp.com<mailto:matthew.chae@hp.com<mailto:matthew.chae@hp.com%3cmailto: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-<mailto:edk2-devel@lists.01.org%3cmailto:edk2->
> devel@lists.01.org<mailto:devel@lists.01.org>>; Zhang, Chao B
> <chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com<mailto:chao.b.zhang@intel.com%3cmailto:chao.b.zhang@intel.com>>>
> ; Yao, Jiewen
> <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>
> Cc: Bin, Sung-Uk (빈성욱) <sunguk-
> bin@hp.com<mailto:sunguk-bin@hp.com<mailto:bin@hp.com%3cmailto:sunguk-bin@hp.com>>>; Chae, Matthew
> <matthew.chae@hp.com<mailto:matthew.chae@hp.com<mailto:matthew.chae@hp.com%3cmailto: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
>
>
>
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel<https://lists.01.org/mailman/listinfo/edk2-devel>
\x16º&