diff mbox series

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

Message ID 20230325043751.3559591-3-ninad@linux.ibm.com
State New
Headers show
Series Add support for TPM devices over I2C bus | expand

Commit Message

Ninad Palsule March 25, 2023, 4:37 a.m. UTC 
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(+)

Comments

Stefan Berger March 25, 2023, 4:12 p.m. UTC | #1 
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'.

    Stefan

>   void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev);
>   
>   #endif /* CONFIG_TPM */
Ninad Palsule March 26, 2023, 1:16 a.m. UTC | #2 
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 */
>
>
diff mbox series

Patch

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
+
 void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev);
 
 #endif /* CONFIG_TPM */