public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [RFC] TPM non-MMIO Access
@ 2018-11-09 22:37 Cohen, Eugene
  2018-11-09 23:11 ` Yao, Jiewen
  2018-11-09 23:17 ` Yao, Jiewen
  0 siblings, 2 replies; 8+ messages in thread
From: Cohen, Eugene @ 2018-11-09 22:37 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Chao Zhang, Jiewen Yao
  Cc: Bin, Sung-Uk (빈성욱), Chae, Matthew

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
  2018-11-09 22:37 [RFC] TPM non-MMIO Access Cohen, Eugene
@ 2018-11-09 23:11 ` Yao, Jiewen
  2018-11-09 23:17 ` Yao, Jiewen
  1 sibling, 0 replies; 8+ messages in thread
From: Yao, Jiewen @ 2018-11-09 23:11 UTC (permalink / raw)
  To: Cohen, Eugene, edk2-devel@lists.01.org, Zhang, Chao B
  Cc: Bin, Sung-Uk (???), Chae, Matthew

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; Zhang, Chao B <chao.b.zhang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Cc: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com>; Chae, Matthew <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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
  2018-11-09 22:37 [RFC] TPM non-MMIO Access Cohen, Eugene
  2018-11-09 23:11 ` Yao, Jiewen
@ 2018-11-09 23:17 ` Yao, Jiewen
       [not found]   ` <AT5PR8401MB0897901965373899E2D9351DF4C10@AT5PR8401MB0897.NAMPRD84.PROD.OUTLOOK.COM>
  1 sibling, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2018-11-09 23:17 UTC (permalink / raw)
  To: Cohen, Eugene, edk2-devel@lists.01.org, Zhang, Chao B
  Cc: Bin, Sung-Uk (???), Chae, Matthew

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>; edk2-devel@lists.01.org; Zhang, Chao B <chao.b.zhang@intel.com>
Cc: Bin, Sung-Uk (빈성욱) <sunguk-bin@hp.com>; Chae, Matthew <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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
       [not found]   ` <AT5PR8401MB0897901965373899E2D9351DF4C10@AT5PR8401MB0897.NAMPRD84.PROD.OUTLOOK.COM>
@ 2018-11-12  4:33     ` Yao, Jiewen
  2018-11-12 13:58       ` Cohen, Eugene
  0 siblings, 1 reply; 8+ messages in thread
From: Yao, Jiewen @ 2018-11-12  4:33 UTC (permalink / raw)
  To: sunguk-bin@hp.com, Cohen, Eugene, edk2-devel@lists.01.org,
	Zhang, Chao B
  Cc: Chae, Matthew

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>; 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

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
  2018-11-12  4:33     ` Yao, Jiewen
@ 2018-11-12 13:58       ` Cohen, Eugene
  2018-11-12 16:44         ` Kinney, Michael D
  0 siblings, 1 reply; 8+ messages in thread
From: Cohen, Eugene @ 2018-11-12 13:58 UTC (permalink / raw)
  To: Yao, Jiewen, Bin, Sung-Uk (빈성욱),
	edk2-devel@lists.01.org, Zhang, Chao B
  Cc: Chae, Matthew

[-- 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º&

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
  2018-11-12 13:58       ` Cohen, Eugene
@ 2018-11-12 16:44         ` Kinney, Michael D
  2018-11-12 20:23           ` Cohen, Eugene
  0 siblings, 1 reply; 8+ messages in thread
From: Kinney, Michael D @ 2018-11-12 16:44 UTC (permalink / raw)
  To: Cohen, Eugene, Yao, Jiewen, Bin, Sung-Uk (???),
	edk2-devel@lists.01.org, Zhang, Chao B, Kinney, Michael D
  Cc: Chae, Matthew

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] On Behalf Of Cohen, Eugene
> Sent: Monday, November 12, 2018 5:59 AM
> To: 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>
> Cc: Chae, Matthew <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>
> 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
> 
> 
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
  2018-11-12 16:44         ` Kinney, Michael D
@ 2018-11-12 20:23           ` Cohen, Eugene
  2018-11-13  2:13             ` Bin, Sung-Uk (빈성욱)
  0 siblings, 1 reply; 8+ messages in thread
From: Cohen, Eugene @ 2018-11-12 20:23 UTC (permalink / raw)
  To: Kinney, Michael D, Yao, Jiewen,
	Bin, Sung-Uk (빈성욱), edk2-devel@lists.01.org,
	Zhang, Chao B
  Cc: Chae, Matthew

[-- 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º&

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [RFC] TPM non-MMIO Access
  2018-11-12 20:23           ` Cohen, Eugene
@ 2018-11-13  2:13             ` Bin, Sung-Uk (빈성욱)
  0 siblings, 0 replies; 8+ messages in thread
From: Bin, Sung-Uk (빈성욱) @ 2018-11-13  2:13 UTC (permalink / raw)
  To: Cohen, Eugene, Kinney, Michael D, Yao, Jiewen,
	edk2-devel@lists.01.org, Zhang, Chao B
  Cc: Chae, Matthew

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

Mike,
I tested TPM command library with this approach and it works.

As far as I understand, TIS (TPM Interface Spec) has only 1 register set for FIFO interface.
And PTP(Platform TPM Profile) has another register set called CRB interface.
Both interface do not distinguish the hardware protocol (LPC or SPI) and TPM driver will use one of two interfaces.

**** I’m not sure this and this is just my understanding, if something wrong, Jiewn or other experts let me know. *****
Only “PC Client” TIS and PTP specification documents were found and MMIO access is only for PC (it has IO controller hub like southbridge).
When we access through MMIO then IO controller hub translate it to LPC or SPI.
In case of embedded device, there is no IO controller hub so it directly control LPC or SPI.
**** End of assumption ****

Thanks,
Bin


From: Cohen, Eugene
Sent: Tuesday, November 13, 2018 5:24 AM
To: Kinney, Michael D <michael.d.kinney@intel.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>
Cc: Chae, Matthew <matthew.chae@hp.com>
Subject: RE: [RFC] TPM non-MMIO Access

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<mailto:michael.d.kinney@intel.com>>
Sent: Monday, November 12, 2018 9:45 AM
To: Cohen, Eugene <eugene@hp.com<mailto:eugene@hp.com>>; 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>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Cc: Chae, Matthew <matthew.chae@hp.com<mailto: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º&

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-11-13  2:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-11-09 22:37 [RFC] TPM non-MMIO Access Cohen, Eugene
2018-11-09 23:11 ` Yao, Jiewen
2018-11-09 23:17 ` Yao, Jiewen
     [not found]   ` <AT5PR8401MB0897901965373899E2D9351DF4C10@AT5PR8401MB0897.NAMPRD84.PROD.OUTLOOK.COM>
2018-11-12  4:33     ` Yao, Jiewen
2018-11-12 13:58       ` Cohen, Eugene
2018-11-12 16:44         ` Kinney, Michael D
2018-11-12 20:23           ` Cohen, Eugene
2018-11-13  2:13             ` Bin, Sung-Uk (빈성욱)

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox