Enable vTPM 2.0 for the IBM vTPM driver

PATCH Superseded Stefan Berger 3 participants 16 messages Feb 13, 2020 – Feb 26, 2020

Subsystems (3)

PowerPC Keys-trusted TPM device driver

lore

Share & tools

lore

Stefan Berger

author

2020-02-13 20:23 UTC #0 lore raw

v2 cover Enable vTPM 2.0 for the IBM vTPM driver

From: Stefan Berger <[email protected]>

QEMU 5.0 will support the PAPR vTPM device model for TPM 1.2 and TPM 2.0. This series of patches enables vTPM 2.0 support for the IBM vTPM driver.

Regards,
Stefan

- v1->v2:
- Addressed comments to v1; added patch 3 to handle case when TPM_OPS_AUTO_STARTUP is not set

Stefan Berger (4):
tpm: of: Handle IBM,vtpm20 case when getting log parameters
tpm: ibmvtpm: Wait for buffer to be set before proceeding
tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set
tpm: ibmvtpm: Add support for TPM 2

 drivers/char/tpm/eventlog/of.c   |  8 +++++++-
 drivers/char/tpm/tpm-interface.c |  5 ++++-
 drivers/char/tpm/tpm.h           |  1 +
 drivers/char/tpm/tpm2-cmd.c      | 14 ++++++++++++++
 drivers/char/tpm/tpm_ibmvtpm.c   | 15 ++++++++++++++-
 drivers/char/tpm/tpm_ibmvtpm.h   |  1 +
 6 files changed, 41 insertions(+), 3 deletions(-)

Signature · 1 line

2.23.0

Stefan Berger

author

2020-02-13 20:23 UTC #1 lore raw

PATCH v2 1/4 tpm: of: Handle IBM,vtpm20 case when getting log parameters

From: Stefan Berger <[email protected]>

A vTPM 2.0 is identified by 'IBM,vtpm20' in the 'compatible' node in the device tree. Handle it in the same way as 'IBM,vtpm'.

The vTPM 2.0's log is written in little endian format so that for this aspect we can rely on existing code.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/eventlog/of.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

drivers/char/tpm/eventlog/of.c +7 -1

index af347c190819..a31a625ad44e 100644
@@ -17,6 +17,12 @@
 #include "../tpm.h"
 #include "common.h"
 
+static const char * const compatibles[] = {
+	"IBM,vtpm",
+	"IBM,vtpm20",
+	NULL
+};
+
 int tpm_read_log_of(struct tpm_chip *chip)
 {
 	struct device_node *np;

@@ -51,7 +57,7 @@ int tpm_read_log_of(struct tpm_chip *chip)
 	 * endian format. For this reason, vtpm doesn't need conversion
 	 * but physical tpm needs the conversion.
 	 */
-	if (of_property_match_string(np, "compatible", "IBM,vtpm") < 0) {
+	if (!of_device_compatible_match(np, compatibles)) {
 		size = be32_to_cpup((__force __be32 *)sizep);
 		base = be64_to_cpup((__force __be64 *)basep);
 	} else {

Signature · 1 line

2.23.0

Jarkko Sakkinen

Acked-by on v2 1/4

2020-02-25 16:54 UTC #2 lore raw

Re: [PATCH v2 1/4] tpm: of: Handle IBM,vtpm20 case when getting log parameters

Quoted 9 lines · Stefan Berger

From: Stefan Berger <[email protected]>
A vTPM 2.0 is identified by 'IBM,vtpm20' in the 'compatible' node in the device tree. Handle it in the same way as 'IBM,vtpm'.
The vTPM 2.0's log is written in little endian format so that for this aspect we can rely on existing code.
Signed-off-by Stefan Berger <[email protected]>

Acked-by Jarkko Sakkinen <[email protected]>

/Jarkko

Stefan Berger

author

2020-02-13 20:23 UTC #3 lore raw

PATCH v2 2/4 tpm: ibmvtpm: Wait for buffer to be set before proceeding

In reply to Stefan Berger

From: Stefan Berger <[email protected]>

Synchronize with the results from the CRQs before continuing with the initialization. This avoids trying to send TPM commands while the rtce buffer has not been allocated, yet.

This patch fixes an existing race condition that may occurr if the hypervisor does not quickly respond to the VTPM_GET_RTCE_BUFFER_SIZE request sent during initialization and therefore the ibmvtpm->rtce_buf has not been allocated at the time the first TPM command is sent.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm_ibmvtpm.c | 9 +++++++++
 drivers/char/tpm/tpm_ibmvtpm.h | 1 +
 2 files changed, 10 insertions(+)

drivers/char/tpm/tpm_ibmvtpm.c +9 -0

index 78cc52690177..eee566eddb35 100644
@@ -571,6 +571,7 @@ static irqreturn_t ibmvtpm_interrupt(int irq, void *vtpm_instance)
 	 */
 	while ((crq = ibmvtpm_crq_get_next(ibmvtpm)) != NULL) {
 		ibmvtpm_crq_process(crq, ibmvtpm);
+		wake_up_interruptible(&ibmvtpm->crq_queue.wq);
 		crq->valid = 0;
 		smp_wmb();
 	}

@@ -618,6 +619,7 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
 	}
 
 	crq_q->num_entry = CRQ_RES_BUF_SIZE / sizeof(*crq_q->crq_addr);
+	init_waitqueue_head(&crq_q->wq);
 	ibmvtpm->crq_dma_handle = dma_map_single(dev, crq_q->crq_addr,
 						 CRQ_RES_BUF_SIZE,
 						 DMA_BIDIRECTIONAL);

@@ -670,6 +672,13 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
 	if (rc)
 		goto init_irq_cleanup;
 
+	if (!wait_event_timeout(ibmvtpm->crq_queue.wq,
+				ibmvtpm->rtce_buf != NULL,
+				HZ)) {
+		dev_err(dev, "Initialization failed\n");
+		goto init_irq_cleanup;
+	}
+
 	return tpm_chip_register(chip);
 init_irq_cleanup:
 	do {

drivers/char/tpm/tpm_ibmvtpm.h +1 -0

index 7983f1a33267..b92aa7d3e93e 100644
@@ -26,6 +26,7 @@ struct ibmvtpm_crq_queue {
 	struct ibmvtpm_crq *crq_addr;
 	u32 index;
 	u32 num_entry;
+	wait_queue_head_t wq;
 };
 
 struct ibmvtpm_dev {

Signature · 1 line

2.23.0

Jarkko Sakkinen

2020-02-25 16:57 UTC #4 lore raw

Re: [PATCH v2 2/4] tpm: ibmvtpm: Wait for buffer to be set before proceeding

Quoted 5 lines · Stefan Berger

From: Stefan Berger <[email protected]>
Synchronize with the results from the CRQs before continuing with the initialization. This avoids trying to send TPM commands while the rtce buffer has not been allocated, yet.

What is CRQ anyway an acronym of?

Quoted 4 lines

This patch fixes an existing race condition that may occurr if the hypervisor does not quickly respond to the VTPM_GET_RTCE_BUFFER_SIZE request sent during initialization and therefore the ibmvtpm->rtce_buf has not been allocated at the time the first TPM command is sent.

If it fixes a race condition, why doesn't it have a fixes tag?

/Jarkko

Stefan Berger

2020-02-25 18:14 UTC #5 lore raw

Re: [PATCH v2 2/4] tpm: ibmvtpm: Wait for buffer to be set before proceeding

Quoted 7 lines · Jarkko Sakkinen

Quoted 5 lines · Stefan Berger
From: Stefan Berger <[email protected]>
Synchronize with the results from the CRQs before continuing with the initialization. This avoids trying to send TPM commands while the rtce buffer has not been allocated, yet.
What is CRQ anyway an acronym of?

Command request queue.

Quoted 6 lines

Quoted 5 lines
This patch fixes an existing race condition that may occurr if the hypervisor does not quickly respond to the VTPM_GET_RTCE_BUFFER_SIZE request sent during initialization and therefore the ibmvtpm->rtce_buf has not been allocated at the time the first TPM command is sent.
If it fixes a race condition, why doesn't it have a fixes tag?

Which commit should I mention?

Stefan

Quoted 2 lines

/Jarkko

Jarkko Sakkinen

2020-02-26 15:00 UTC #6 lore raw

Re: [PATCH v2 2/4] tpm: ibmvtpm: Wait for buffer to be set before proceeding

Quoted 13 lines · Stefan Berger

Quoted 7 lines · Jarkko Sakkinen
Quoted 5 lines · Stefan Berger
From: Stefan Berger <[email protected]>
Synchronize with the results from the CRQs before continuing with the initialization. This avoids trying to send TPM commands while the rtce buffer has not been allocated, yet.
What is CRQ anyway an acronym of?
Command request queue.
Quoted 1 line

Quoted 7 lines

Quoted 5 lines
Quoted 4 lines
This patch fixes an existing race condition that may occurr if the hypervisor does not quickly respond to the VTPM_GET_RTCE_BUFFER_SIZE request sent during initialization and therefore the ibmvtpm->rtce_buf has not been allocated at the time the first TPM command is sent.
If it fixes a race condition, why doesn't it have a fixes tag?
Which commit should I mention?

The one that introduced the race condition if there is such.

/Jarkko

Stefan Berger

author

2020-02-13 20:23 UTC #7 lore raw

PATCH v2 3/4 tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set

In reply to Stefan Berger

From: Stefan Berger <[email protected]>

Implement tpm2_init() that gets the TPM 2 timeouts and command durations and command code attributes. This function is to be called in case the TPM_OPS_AUTO_STARTUP flag is not set and therefore tpm2_auto_startup() is not called.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm-interface.c |  5 ++++-
 drivers/char/tpm/tpm.h           |  1 +
 drivers/char/tpm/tpm2-cmd.c      | 14 ++++++++++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm-interface.c +4 -1

index a438b1206fcb..cca0e542172d 100644
@@ -371,8 +371,11 @@ int tpm_auto_startup(struct tpm_chip *chip)
 {
 	int rc;
 
-	if (!(chip->ops->flags & TPM_OPS_AUTO_STARTUP))
+	if (!(chip->ops->flags & TPM_OPS_AUTO_STARTUP)) {
+		if (chip->flags & TPM_CHIP_FLAG_TPM2)
+			return tpm2_init(chip);
 		return 0;
+	}
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2)
 		rc = tpm2_auto_startup(chip);

drivers/char/tpm/tpm.h +1 -0

index 5620747da0cf..895f8a1e17d2 100644
@@ -222,6 +222,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
 
 ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
+int tpm2_init(struct tpm_chip *chip);
 int tpm2_auto_startup(struct tpm_chip *chip);
 void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
 unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);

drivers/char/tpm/tpm2-cmd.c +14 -0

index 13696deceae8..d5b4b0449c23 100644
@@ -709,6 +709,20 @@ static int tpm2_startup(struct tpm_chip *chip)
 	return rc;
 }
 
+/**
+ * tpm2_init - Get timeouts and command code attributes
+ *             in case tpm2_auto_startup is not used.
+ * @chip: TPM chip to use
+ *
+ * Return 0 on success, < 0 in case of fatal error.
+ */
+int tpm2_init(struct tpm_chip *chip)
+{
+	tpm2_get_timeouts(chip);
+
+	return tpm2_get_cc_attrs_tbl(chip);
+}
+
 /**
  * tpm2_auto_startup - Perform the standard automatic TPM initialization
  *                     sequence

Signature · 1 line

2.23.0

Jarkko Sakkinen

2020-02-25 17:00 UTC #8 lore raw

Re: [PATCH v2 3/4] tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set

Quoted 8 lines · Stefan Berger

From: Stefan Berger <[email protected]>
Implement tpm2_init() that gets the TPM 2 timeouts and command durations and command code attributes. This function is to be called in case the TPM_OPS_AUTO_STARTUP flag is not set and therefore tpm2_auto_startup() is not called.
Signed-off-by Stefan Berger <[email protected]>

The commit makes zero effort trying to explain what the heck tpm_init() is and when it should be used and why the function name tpm2_init().

/Jarkko

Stefan Berger

2020-02-25 18:20 UTC #9 lore raw

Re: [PATCH v2 3/4] tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set

Quoted 11 lines · Jarkko Sakkinen

Quoted 8 lines · Stefan Berger
From: Stefan Berger <[email protected]>
Implement tpm2_init() that gets the TPM 2 timeouts and command durations and command code attributes. This function is to be called in case the TPM_OPS_AUTO_STARTUP flag is not set and therefore tpm2_auto_startup() is not called.
Signed-off-by Stefan Berger <[email protected]>
The commit makes zero effort trying to explain what the heck tpm_init() is and when it should be used and why the function name tpm2_init().

Are you saying the explanation of when to use tpm2_init above is not enough? 'bviously we are trying to cover the case of using the TPM 2 by a driver that doesn't use the TPM_OPS_AUTO_STARTUP flag and therefore the TPM 2 timeouts and command durations and command code attributes are not set as they would be if tpm2_auto_startup() was to be called and tpm2_init() is the alternative to call. I didn't like tpm2_init() either... any suggestions for a better name?

Stefan

Quoted 2 lines

/Jarkko

Jarkko Sakkinen

2020-02-26 15:03 UTC #10 lore raw

Re: [PATCH v2 3/4] tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set

Quoted 13 lines · Stefan Berger

Quoted 11 lines · Jarkko Sakkinen
Quoted 8 lines · Stefan Berger
From: Stefan Berger <[email protected]>
Implement tpm2_init() that gets the TPM 2 timeouts and command durations and command code attributes. This function is to be called in case the TPM_OPS_AUTO_STARTUP flag is not set and therefore tpm2_auto_startup() is not called.
Signed-off-by Stefan Berger <[email protected]>
The commit makes zero effort trying to explain what the heck tpm_init() is and when it should be used and why the function name tpm2_init().

Quoted 7 lines

Are you saying the explanation of when to use tpm2_init above is not enough? 'bviously we are trying to cover the case of using the TPM 2 by a driver that doesn't use the TPM_OPS_AUTO_STARTUP flag and therefore the TPM 2 timeouts and command durations and command code attributes are not set as they would be if tpm2_auto_startup() was to be called and tpm2_init() is the alternative to call. I didn't like tpm2_init() either... any suggestions for a better name?

I'm not getting what this commit is trying to do in the first place.

/Jarkko

Stefan Berger

2020-02-26 17:36 UTC #11 lore raw

Re: [PATCH v2 3/4] tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set

Quoted 13 lines · Jarkko Sakkinen

Quoted 12 lines · Stefan Berger
Quoted 11 lines · Jarkko Sakkinen
Quoted 8 lines · Stefan Berger
From: Stefan Berger <[email protected]>
Implement tpm2_init() that gets the TPM 2 timeouts and command durations and command code attributes. This function is to be called in case the TPM_OPS_AUTO_STARTUP flag is not set and therefore tpm2_auto_startup() is not called.
Signed-off-by Stefan Berger <[email protected]>
The commit makes zero effort trying to explain what the heck tpm_init() is and when it should be used and why the function name tpm2_init().

Quoted 8 lines

Quoted 7 lines
Are you saying the explanation of when to use tpm2_init above is not enough? 'bviously we are trying to cover the case of using the TPM 2 by a driver that doesn't use the TPM_OPS_AUTO_STARTUP flag and therefore the TPM 2 timeouts and command durations and command code attributes are not set as they would be if tpm2_auto_startup() was to be called and tpm2_init() is the alternative to call. I didn't like tpm2_init() either... any suggestions for a better name?
I'm not getting what this commit is trying to do in the first place.

I sent out v3, maybe the text there is better.

Quoted 2 lines

/Jarkko

Stefan Berger

author

2020-02-13 20:23 UTC #12 lore raw

PATCH v2 4/4 tpm: ibmvtpm: Add support for TPM 2

In reply to Stefan Berger

From: Stefan Berger <[email protected]>

Support TPM 2 in the IBM vTPM driver. The hypervisor tells us what version of TPM is connected through the vio_device_id.

In case a TPM 2 is found, we set the TPM_OPS_AUTO_STARTUP flag to have properly initialize the TPM and driver.

Signed-off-by Stefan Berger <[email protected]>

---
 drivers/char/tpm/tpm_ibmvtpm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

drivers/char/tpm/tpm_ibmvtpm.c +5 -1

index eee566eddb35..25cd256c7c3d 100644
@@ -29,6 +29,7 @@ static const char tpm_ibmvtpm_driver_name[] = "tpm_ibmvtpm";
 
 static const struct vio_device_id tpm_ibmvtpm_device_table[] = {
 	{ "IBM,vtpm", "IBM,vtpm"},
+	{ "IBM,vtpm", "IBM,vtpm20"},
 	{ "", "" }
 };
 MODULE_DEVICE_TABLE(vio, tpm_ibmvtpm_device_table);

@@ -443,7 +444,7 @@ static bool tpm_ibmvtpm_req_canceled(struct tpm_chip *chip, u8 status)
 	return (status == 0);
 }
 
-static const struct tpm_class_ops tpm_ibmvtpm = {
+const static struct tpm_class_ops tpm_ibmvtpm = {
 	.recv = tpm_ibmvtpm_recv,
 	.send = tpm_ibmvtpm_send,
 	.cancel = tpm_ibmvtpm_cancel,

@@ -672,6 +673,9 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
 	if (rc)
 		goto init_irq_cleanup;
 
+	if (!strcmp(id->compat, "IBM,vtpm20"))
+		chip->flags |= TPM_CHIP_FLAG_TPM2;
+
 	if (!wait_event_timeout(ibmvtpm->crq_queue.wq,
 				ibmvtpm->rtce_buf != NULL,
 				HZ)) {

Signature · 1 line

2.23.0

Stefan Berger

2020-02-19 19:23 UTC #13 lore raw

In reply to Stefan Berger

Quoted 4 lines · Stefan Berger

From: Stefan Berger <[email protected]>
QEMU 5.0 will support the PAPR vTPM device model for TPM 1.2 and TPM 2.0. This series of patches enables vTPM 2.0 support for the IBM vTPM driver.

If there are no more comments to this series, maybe Jarkko can queue it?

Stefan

Quoted 22 lines

Regards,
Stefan
- v1->v2:
- Addressed comments to v1; added patch 3 to handle case when TPM_OPS_AUTO_STARTUP is not set
Stefan Berger (4):
tpm: of: Handle IBM,vtpm20 case when getting log parameters
tpm: ibmvtpm: Wait for buffer to be set before proceeding
tpm: Implement tpm2_init to call when TPM_OPS_AUTO_STARTUP is not set
tpm: ibmvtpm: Add support for TPM 2
  drivers/char/tpm/eventlog/of.c   |  8 +++++++-
  drivers/char/tpm/tpm-interface.c |  5 ++++-
  drivers/char/tpm/tpm.h           |  1 +
  drivers/char/tpm/tpm2-cmd.c      | 14 ++++++++++++++
  drivers/char/tpm/tpm_ibmvtpm.c   | 15 ++++++++++++++-
  drivers/char/tpm/tpm_ibmvtpm.h   |  1 +
  6 files changed, 41 insertions(+), 3 deletions(-)

Jarkko Sakkinen

2020-02-20 19:59 UTC #14 lore raw

Quoted 8 lines · Stefan Berger

Quoted 4 lines · Stefan Berger
From: Stefan Berger <[email protected]>
QEMU 5.0 will support the PAPR vTPM device model for TPM 1.2 and TPM 2.0. This series of patches enables vTPM 2.0 support for the IBM vTPM driver.
If there are no more comments to this series, maybe Jarkko can queue it?

Do not recall seeing this series before. Probably have missed it. I'll look into it next week.

/Jarkkko

Jarkko Sakkinen

2020-02-20 20:00 UTC #15 lore raw

Quoted 4 lines · Jarkko Sakkinen

Quoted 3 lines · Stefan Berger
Quoted 2 lines · Stefan Berger
From: Stefan Berger <[email protected]>

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
QEMU 5.0 will support the PAPR vTPM device model for TPM 1.2 and TPM 2.0. This series of patches enables vTPM 2.0 support for the IBM vTPM driver.
If there are no more comments to this series, maybe Jarkko can queue it?
Do not recall seeing this series before. Probably have missed it. I'll look into it next week.

Yup, did not have CC to me. If you have rush to get something queued, then should at least CC to series.

/Jarkko