2020-02-13 20:24:14

by Stefan Berger

[permalink] [raw]
Subject: [PATCH v2 0/4] 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(-)

--
2.23.0


2020-02-13 20:24:56

by Stefan Berger

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/char/tpm/eventlog/of.c b/drivers/char/tpm/eventlog/of.c
index af347c190819..a31a625ad44e 100644
--- a/drivers/char/tpm/eventlog/of.c
+++ b/drivers/char/tpm/eventlog/of.c
@@ -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 {
--
2.23.0

2020-02-13 20:25:32

by Stefan Berger

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

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(+)

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

2020-02-19 19:24:14

by Stefan Berger

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable vTPM 2.0 for the IBM vTPM driver

On 2/13/20 3:23 PM, Stefan Berger wrote:
> 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


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

2020-02-20 20:00:00

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable vTPM 2.0 for the IBM vTPM driver

On Wed, Feb 19, 2020 at 02:23:29PM -0500, Stefan Berger wrote:
> On 2/13/20 3:23 PM, Stefan Berger wrote:
> > 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

2020-02-20 20:02:39

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v2 0/4] Enable vTPM 2.0 for the IBM vTPM driver

On Thu, Feb 20, 2020 at 09:59:06PM +0200, Jarkko Sakkinen wrote:
> On Wed, Feb 19, 2020 at 02:23:29PM -0500, Stefan Berger wrote:
> > On 2/13/20 3:23 PM, Stefan Berger wrote:
> > > 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.

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

/Jarkko

2020-02-25 17:13:02

by Jarkko Sakkinen

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

On Thu, Feb 13, 2020 at 03:23:26PM -0500, Stefan Berger wrote:
> 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

2020-02-25 17:16:19

by Jarkko Sakkinen

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

On Thu, Feb 13, 2020 at 03:23:27PM -0500, Stefan Berger wrote:
> 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?

> 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

2020-02-25 18:15:16

by Stefan Berger

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

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


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


2020-02-26 15:02:07

by Jarkko Sakkinen

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

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