diff mbox series

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

Message ID 20200213202329.898607-3-stefanb@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show
Series Enable vTPM 2.0 for the IBM vTPM driver | expand

Commit Message

Stefan Berger Feb. 13, 2020, 8:23 p.m. UTC 
From: Stefan Berger <stefanb@linux.ibm.com>

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 <stefanb@linux.ibm.com>
---
 drivers/char/tpm/tpm_ibmvtpm.c | 9 +++++++++
 drivers/char/tpm/tpm_ibmvtpm.h | 1 +
 2 files changed, 10 insertions(+)

Comments

Jarkko Sakkinen Feb. 25, 2020, 4:57 p.m. UTC | #1 
On Thu, Feb 13, 2020 at 03:23:27PM -0500, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
> 
> 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?

> 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 Feb. 25, 2020, 6:14 p.m. UTC | #2 
On 2/25/20 11:57 AM, Jarkko Sakkinen wrote:
> On Thu, Feb 13, 2020 at 03:23:27PM -0500, Stefan Berger wrote:
>> From: Stefan Berger <stefanb@linux.ibm.com>
>>
>> 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.


>
>> 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


>
> /Jarkko
Jarkko Sakkinen Feb. 26, 2020, 3 p.m. UTC | #3 
On Tue, Feb 25, 2020 at 01:14:32PM -0500, Stefan Berger wrote:
> On 2/25/20 11:57 AM, Jarkko Sakkinen wrote:
> > On Thu, Feb 13, 2020 at 03:23:27PM -0500, Stefan Berger wrote:
> > > From: Stefan Berger <stefanb@linux.ibm.com>
> > > 
> > > 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.
> 
> 
> > 
> > > 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
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 78cc52690177..eee566eddb35 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -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 {
diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index 7983f1a33267..b92aa7d3e93e 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h
@@ -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 {