Skip to contentSkip to conversation

tpm: read burstcount from TPM_STS in one 32-bit transaction

DISCUSSION Michal Suchánek 4 participants 5 messages Jul 25, 2017 – Aug 1, 2017

lore
Share & tools
lore
#0 lore raw

Hello,

in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one 32-bit transaction") you change reading of two 8-bit values to one 32bit read. This is obviously wrong wrt endianess unless the underlying tpm_tis_read32 does endian conversion.

Looking at the implementation
static inline int tpm_tis_read32(struct tpm_tis_data *data, u32 addr,

                                 u32 *result)
{
        return data->phy_ops->read32(data, addr, result);
}

it calls read32 which has two implementations:

static const struct tpm_tis_phy_ops tpm_tcg = {
	.read32 = tpm_tcg_read32,

static int tpm_tcg_read32(struct tpm_tis_data *data, u32 addr, u32

*result) {
        struct tpm_tis_tcg_phy *phy = to_tpm_tis_tcg_phy(data);

        *result = ioread32(phy->iobase + addr);
       return 0;
}
static const struct tpm_tis_phy_ops tpm_spi_phy_ops = {
	.read32 = tpm_tis_spi_read32,

static int tpm_tis_spi_read32(struct tpm_tis_data *data, u32 addr, u32

*result) {
        int rc;

        rc = data->phy_ops->read_bytes(data, addr, sizeof(u32), (u8
        *)result); if (!rc)
                *result = le32_to_cpu(*result);
        return rc;
}

meaning that unless you are on LE where le32_to_cpu is a noop these functions do completely different thing. So presumably this is completely broken on BE.

Presumably only the SPI variant can be actually used with TPM devices bolted on after the fact so it is more likely correct for obscure hardware. Conseqently tpm_tcg_read32 should use le32_to_cpu(ioread32(phy->iobase + addr)) in case somebody manages to map a TPM into io-space on a BE machine.

Thanks

Michal

#1 lore raw
Quoted 6 lines · Michal Suchánek

Hello,

in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one 32-bit transaction") you change reading of two 8-bit values to one 32bit read. This is obviously wrong wrt endianess unless the underlying tpm_tis_read32 does endian conversion.

Some of the bus read primitives do do endianness conversions.  The problem is with the SPI attachment, which has unclear endianness.  A standard PCI bus attachment uses ioread32() which automatically transforms from a little endian bus to the cpu endianness, however SPI is forced to transfer the bytes one at a time over the serial bus and then transform.  The assumption seems to be that the TIS TPM is replying in little endian format when SPI connected.

We can probably get the PPC people to confirm this, I believe they have a SPI attached TPM.

James

#2 lore raw
Quoted 8 lines · On Tue, 25 Jul 2017 10:36:11 -0700 James Bottomley <[email protected]>…
Quoted 6 lines · Michal Suchánek

Hello,

in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one 32-bit transaction") you change reading of two 8-bit values to one 32bit read. This is obviously wrong wrt endianess unless the underlying tpm_tis_read32 does endian conversion.

Quoted 8 lines

Some of the bus read primitives do do endianness conversions.  The problem is with the SPI attachment, which has unclear endianness.  A standard PCI bus attachment uses ioread32() which automatically transforms from a little endian bus to the cpu endianness, however SPI is forced to transfer the bytes one at a time over the serial bus and then transform.  The assumption seems to be that the TIS TPM is replying in little endian format when SPI connected.

Yes, that makes sense.

Thanks for clarification.

Michal

#3 lore raw
Quoted 20 lines · Michal Suchánek
Quoted 8 lines · On Tue, 25 Jul 2017 10:36:11 -0700 James Bottomley…
Quoted 6 lines · Michal Suchánek

Hello,

in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one 32-bit transaction") you change reading of two 8-bit values to one 32bit read. This is obviously wrong wrt endianess unless the underlying tpm_tis_read32 does endian conversion.

Quoted 8 lines

Some of the bus read primitives do do endianness conversions.  The problem is with the SPI attachment, which has unclear endianness.  A standard PCI bus attachment uses ioread32() which automatically transforms from a little endian bus to the cpu endianness, however SPI is forced to transfer the bytes one at a time over the serial bus and then transform.  The assumption seems to be that the TIS TPM is replying in little endian format when SPI connected.

Quoted 5 lines

Yes, that makes sense.

Thanks for clarification.

Michal

Thank you for reporting this and thanks James for explaining this.

I do not have access to PPC hardware with SPI-TPM.

/Jarkko

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot

#4 lore raw
In reply to James Bottomley
Quoted 16 lines · James Bottomley
Quoted 6 lines · Michal Suchánek

Hello,

in commit 9754d45e9970 ("tpm: read burstcount from TPM_STS in one 32-bit transaction") you change reading of two 8-bit values to one 32bit read. This is obviously wrong wrt endianess unless the underlying tpm_tis_read32 does endian conversion.

Some of the bus read primitives do do endianness conversions.  The problem is with the SPI attachment, which has unclear endianness.  A standard PCI bus attachment uses ioread32() which automatically transforms from a little endian bus to the cpu endianness, however SPI is forced to transfer the bytes one at a time over the serial bus and then transform.  The assumption seems to be that the TIS TPM is replying in little endian format when SPI connected.

Quoted 2 lines

We can probably get the PPC people to confirm this, I believe they have a SPI attached TPM.

All the current OpenPOWER hardware designs I'm aware of have the TPM on I2C. Trusted Computing support in OpenPOWER firmware depends on it being on I2C.

Quoted 11 lines

James

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot