Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1695714 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  tpm: read burstcount from TPM_STS in one 32-bit transaction Michal Suchánek <msuchanek@suse.de> - 2017-07-25 15:10 +0200
    Re: [tpmdd-devel] tpm: read burstcount from TPM_STS in one 32-bit  transaction James Bottomley <jejb@linux.vnet.ibm.com> - 2017-07-25 19:40 +0200
      Re: [tpmdd-devel] tpm: read burstcount from TPM_STS in one 32-bit  transaction Michal Suchánek <msuchanek@suse.de> - 2017-07-25 20:20 +0200

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

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

[toc] | [next] | [standalone]

#1695968 — Re: [tpmdd-devel] tpm: read burstcount from TPM_STS in one 32-bit transaction

On Tue, 2017-07-25 at 15:04 +0200, Michal Suchánek wrote:
> 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

[toc] | [prev] | [next] | [standalone]

#1696002 — Re: [tpmdd-devel] tpm: read burstcount from TPM_STS in one 32-bit transaction

On Tue, 25 Jul 2017 10:36:11 -0700
James Bottomley <jejb@linux.vnet.ibm.com> wrote:

> On Tue, 2017-07-25 at 15:04 +0200, Michal Suchánek wrote:
> > 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.
> 

Yes, that makes sense.

Thanks for clarification.

Michal

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web