qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v4 2/3] tpm: Extend common APIs to support TPM TIS I2C


From: Ninad Palsule
Subject: Re: [PATCH v4 2/3] tpm: Extend common APIs to support TPM TIS I2C
Date: Sat, 25 Mar 2023 20:16:27 -0500
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.8.0

Hi Stefan,

On 3/25/23 11:12 AM, Stefan Berger wrote:


On 3/25/23 00:37, Ninad Palsule wrote:
Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.

This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
   the I2C support. The checksum calculation is handled in the qemu
   common code.
- Added wrapper function for read and write data so that I2C code can
   call it without MMIO interface.

Signed-off-by: Ninad Palsule <ninad@linux.ibm.com>
---
V2:

Incorporated Stephen's comments.

- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
   i2c layer.

---
V3:
Incorporated review comments from Cedric and Stefan.

- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h

---
V4:

Incorporated review comments by Stefan

- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
  hw/tpm/tpm_tis.h        |  3 +++
  hw/tpm/tpm_tis_common.c | 28 ++++++++++++++++++++++++++++
  include/hw/acpi/tpm.h   | 28 ++++++++++++++++++++++++++++
  3 files changed, 59 insertions(+)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
  void tpm_tis_reset(TPMState *s);
  enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
  void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
    #endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c6d139943e 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
  #include "hw/irq.h"
  #include "hw/isa/isa.h"
  #include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
  #include "qemu/module.h"
    #include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr addr,
      return val;
  }
  +/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+    return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+    return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
  /*
   * Write a value to a register of the TIS interface
   * See specs pages 33-63 for description of the registers
@@ -767,6 +786,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
      }
  }
  +/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+    tpm_tis_mmio_write(s, addr, val, size);
+}
+
  const MemoryRegionOps tpm_tis_memory_ops = {
      .read = tpm_tis_mmio_read,
      .write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..4f2424e2fe 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
  #define TPM_TIS_CAP_DATA_TRANSFER_64B    (3 << 9)
  #define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
  #define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC   (1 << 8)
  #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is mandatory */
  #define TPM_TIS_CAPABILITIES_SUPPORTED1_3 \
      (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
@@ -209,6 +210,33 @@ REG32(CRB_DATA_BUFFER, 0x80)
  #define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
  #define TPM_PPI_FUNC_MASK                (7 << 0)
  +/* TPM TIS I2C registers */
+#define TPM_I2C_REG_LOC_SEL              0x00
+#define TPM_I2C_REG_ACCESS               0x04
+#define TPM_I2C_REG_INT_ENABLE           0x08
+#define TPM_I2C_REG_INT_CAPABILITY       0x14
+#define TPM_I2C_REG_STS                  0x18
+#define TPM_I2C_REG_DATA_FIFO            0x24
+#define TPM_I2C_REG_INTF_CAPABILITY      0x30
+#define TPM_I2C_REG_I2C_DEV_ADDRESS      0x38
+#define TPM_I2C_REG_DATA_CSUM_ENABLE     0x40
+#define TPM_I2C_REG_DATA_CSUM_GET        0x44
+#define TPM_I2C_REG_DID_VID              0x48
+#define TPM_I2C_REG_RID                  0x4c
+#define TPM_I2C_REG_UNKNOWN              0xff
+
+/* I2C specific interface capabilities */
+#define TPM_I2C_CAP_INTERFACE_TYPE     (0x2 << 0) /* FIFO interface */
+#define TPM_I2C_CAP_INTERFACE_VER      (0x0 << 4) /* TCG I2C intf 1.0 */
+#define TPM_I2C_CAP_TPM2_FAMILY        (0x1 << 7) /* TPM 2.0 family. */
+#define TPM_I2C_CAP_DEV_ADDR_CHANGE    (0x0 << 27) /* No dev addr chng */ +#define TPM_I2C_CAP_BURST_COUNT_STATIC (0x1 << 29) /* Burst count static */
+#define TPM_I2C_CAP_LOCALITY_CAP       (0x1 << 25) /* 0-5 locality */
+#define TPM_I2C_CAP_BUS_SPEED          (3   << 21) /* std and fast mode */
+
+/* TPM_STS mask for read bits 31:26 must be zero */
+#define TPM_I2C_STS_READ_MASK          0x03ffffff
+

I think you should define a bit here for the TPM_DATA_CSUM_ENABLE register and used instead of 'true'.

Done

Thanks for the review.

Ninad


   Stefan

  void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev);
    #endif /* CONFIG_TPM */





reply via email to

[Prev in Thread] Current Thread [Next in Thread]