From: Jerry Snitselaar <jsnitsel@redhat.com> To: David Gibson <david@gibson.dropbear.id.au> Cc: jarkko.sakkinen@linux.intel.com, stefanb@linux.ibm.com, mpe@ellerman.id.au, peterhuewe@gmx.de, jgg@ziepe.ca, nayna@linux.ibm.com, paulus@samba.org, linuxppc-dev@lists.ozlabs.org, linux-integrity@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCHv2] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes Date: Thu, 18 Jun 2020 20:42:40 -0700 [thread overview] Message-ID: <20200619034240.xcl56oboxurym4jm@cantor> (raw) In-Reply-To: <20200619033040.121412-1-david@gibson.dropbear.id.au> On Fri Jun 19 20, David Gibson wrote: >The tpm2_get_cc_attrs_tbl() call will result in TPM commands being issued, >which will need the use of the internal command/response buffer. But, >we're issuing this *before* we've waited to make sure that buffer is >allocated. > >This can result in intermittent failures to probe if the hypervisor / TPM >implementation doesn't respond quickly enough. I find it fails almost >every time with an 8 vcpu guest under KVM with software emulated TPM. > >To fix it, just move the tpm2_get_cc_attrs_tlb() call after the >existing code to wait for initialization, which will ensure the buffer >is allocated. > >Fixes: 18b3670d79ae9 ("tpm: ibmvtpm: Add support for TPM2") >Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >--- Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com> > >Changes from v1: > * Fixed a formatting error in the commit message > * Added some more detail to the commit message > >drivers/char/tpm/tpm_ibmvtpm.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > >diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c >index 09fe45246b8cc..994385bf37c0c 100644 >--- a/drivers/char/tpm/tpm_ibmvtpm.c >+++ b/drivers/char/tpm/tpm_ibmvtpm.c >@@ -683,13 +683,6 @@ 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; >- rc = tpm2_get_cc_attrs_tbl(chip); >- if (rc) >- goto init_irq_cleanup; >- } >- > if (!wait_event_timeout(ibmvtpm->crq_queue.wq, > ibmvtpm->rtce_buf != NULL, > HZ)) { >@@ -697,6 +690,13 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > goto init_irq_cleanup; > } > >+ if (!strcmp(id->compat, "IBM,vtpm20")) { >+ chip->flags |= TPM_CHIP_FLAG_TPM2; >+ rc = tpm2_get_cc_attrs_tbl(chip); >+ if (rc) >+ goto init_irq_cleanup; >+ } >+ > return tpm_chip_register(chip); > init_irq_cleanup: > do { >-- >2.26.2 >
WARNING: multiple messages have this Message-ID (diff)
From: Jerry Snitselaar <jsnitsel@redhat.com> To: David Gibson <david@gibson.dropbear.id.au> Cc: nayna@linux.ibm.com, linux-kernel@vger.kernel.org, jarkko.sakkinen@linux.intel.com, jgg@ziepe.ca, paulus@samba.org, peterhuewe@gmx.de, linuxppc-dev@lists.ozlabs.org, linux-integrity@vger.kernel.org, stefanb@linux.ibm.com Subject: Re: [PATCHv2] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes Date: Thu, 18 Jun 2020 20:42:40 -0700 [thread overview] Message-ID: <20200619034240.xcl56oboxurym4jm@cantor> (raw) In-Reply-To: <20200619033040.121412-1-david@gibson.dropbear.id.au> On Fri Jun 19 20, David Gibson wrote: >The tpm2_get_cc_attrs_tbl() call will result in TPM commands being issued, >which will need the use of the internal command/response buffer. But, >we're issuing this *before* we've waited to make sure that buffer is >allocated. > >This can result in intermittent failures to probe if the hypervisor / TPM >implementation doesn't respond quickly enough. I find it fails almost >every time with an 8 vcpu guest under KVM with software emulated TPM. > >To fix it, just move the tpm2_get_cc_attrs_tlb() call after the >existing code to wait for initialization, which will ensure the buffer >is allocated. > >Fixes: 18b3670d79ae9 ("tpm: ibmvtpm: Add support for TPM2") >Signed-off-by: David Gibson <david@gibson.dropbear.id.au> >--- Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com> > >Changes from v1: > * Fixed a formatting error in the commit message > * Added some more detail to the commit message > >drivers/char/tpm/tpm_ibmvtpm.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > >diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c >index 09fe45246b8cc..994385bf37c0c 100644 >--- a/drivers/char/tpm/tpm_ibmvtpm.c >+++ b/drivers/char/tpm/tpm_ibmvtpm.c >@@ -683,13 +683,6 @@ 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; >- rc = tpm2_get_cc_attrs_tbl(chip); >- if (rc) >- goto init_irq_cleanup; >- } >- > if (!wait_event_timeout(ibmvtpm->crq_queue.wq, > ibmvtpm->rtce_buf != NULL, > HZ)) { >@@ -697,6 +690,13 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev, > goto init_irq_cleanup; > } > >+ if (!strcmp(id->compat, "IBM,vtpm20")) { >+ chip->flags |= TPM_CHIP_FLAG_TPM2; >+ rc = tpm2_get_cc_attrs_tbl(chip); >+ if (rc) >+ goto init_irq_cleanup; >+ } >+ > return tpm_chip_register(chip); > init_irq_cleanup: > do { >-- >2.26.2 >
next prev parent reply other threads:[~2020-06-19 3:42 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-06-19 3:30 [PATCHv2] tpm: ibmvtpm: Wait for ready buffer before probing for TPM2 attributes David Gibson 2020-06-19 3:30 ` David Gibson 2020-06-19 3:42 ` Jerry Snitselaar [this message] 2020-06-19 3:42 ` Jerry Snitselaar 2020-06-23 0:48 ` Jarkko Sakkinen 2020-06-23 0:48 ` Jarkko Sakkinen
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200619034240.xcl56oboxurym4jm@cantor \ --to=jsnitsel@redhat.com \ --cc=david@gibson.dropbear.id.au \ --cc=jarkko.sakkinen@linux.intel.com \ --cc=jgg@ziepe.ca \ --cc=linux-integrity@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linuxppc-dev@lists.ozlabs.org \ --cc=mpe@ellerman.id.au \ --cc=nayna@linux.ibm.com \ --cc=paulus@samba.org \ --cc=peterhuewe@gmx.de \ --cc=stefanb@linux.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.