| Message ID | 1449669121-6101-1-git-send-email-stefanb@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 09, 2015 at 08:52:01AM -0500, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.vnet.ibm.com> > > When the TPM response reception is interrupted in the wait_event_interruptable > call, the TPM is still busy processing the command and will only deliver the > response later. So we have to wait for an outstanding response before sending > a new request to avoid trying to put a 2nd request into the CRQ. Also reset > the res_len before sending a command so we will end up in that > wait_event_interruptable() waiting for the response rather than reading the > command packet as a response. > > The easiest way to trigger the problem is to run the following > > cd /sys/device/vio/71000004 > > while :; cat pcrs >/dev/null; done > > And press Ctrl-C. This will then display an error > > tpm_ibmvtpm 71000004: tpm_transmit: tpm_recv: error -4 > > followed by several other errors once interaction with the TPM resumes. > > tpm_ibmvtpm 71000004: A TPM error (101) occurred attempting to determine the number of PCRS. > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > Tested-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > --- > drivers/char/tpm/tpm_ibmvtpm.c | 22 ++++++++++++++++++++-- > drivers/char/tpm/tpm_ibmvtpm.h | 1 + > 2 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > index 3e6a226..b0a9a9e 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.c > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > @@ -90,7 +90,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > return 0; > } > > - sig = wait_event_interruptible(ibmvtpm->wq, ibmvtpm->res_len != 0); > + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); > if (sig) > return -EINTR; > > @@ -125,7 +125,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > struct ibmvtpm_dev *ibmvtpm; > struct ibmvtpm_crq crq; > __be64 *word = (__be64 *)&crq; > - int rc; > + int rc, sig; > > ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > @@ -141,18 +141,35 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > return -EIO; > } > > + if (ibmvtpm->tpm_processing_cmd) { > + dev_info(ibmvtpm->dev, > + "Need to wait for TPM to finish\n"); > + /* wait for previous command to finish */ > + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); > + if (sig) > + return -EINTR; > + } > + > spin_lock(&ibmvtpm->rtce_lock); > + ibmvtpm->res_len = 0; > memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count); > crq.valid = (u8)IBMVTPM_VALID_CMD; > crq.msg = (u8)VTPM_TPM_COMMAND; > crq.len = cpu_to_be16(count); > crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle); > > + /* > + * set the processing flag before the Hcall, since we may get the > + * result (interrupt) before even being able to check rc. > + */ > + ibmvtpm->tpm_processing_cmd = true; > + > rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), > be64_to_cpu(word[1])); > if (rc != H_SUCCESS) { > dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc); > rc = 0; > + ibmvtpm->tpm_processing_cmd = false; > } else > rc = count; > > @@ -515,6 +532,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, > case VTPM_TPM_COMMAND_RES: > /* len of the data in rtce buffer */ > ibmvtpm->res_len = be16_to_cpu(crq->len); > + ibmvtpm->tpm_processing_cmd = false; > wake_up_interruptible(&ibmvtpm->wq); > return; > default: > diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h > index 6af9289..91dfe766 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.h > +++ b/drivers/char/tpm/tpm_ibmvtpm.h > @@ -45,6 +45,7 @@ struct ibmvtpm_dev { > wait_queue_head_t wq; > u16 res_len; > u32 vtpm_version; > + bool tpm_processing_cmd; > }; > > #define CRQ_RES_BUF_SIZE PAGE_SIZE > -- > 1.8.3.1 > > > ------------------------------------------------------------------------------ > _______________________________________________ > tpmdd-devel mailing list > tpmdd-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/tpmdd-devel /Jarkko ------------------------------------------------------------------------------
On Fri, Dec 11, 2015 at 09:31:55PM +0200, Jarkko Sakkinen wrote: > On Wed, Dec 09, 2015 at 08:52:01AM -0500, Stefan Berger wrote: > > From: Stefan Berger <stefanb@linux.vnet.ibm.com> > > > > When the TPM response reception is interrupted in the wait_event_interruptable > > call, the TPM is still busy processing the command and will only deliver the > > response later. So we have to wait for an outstanding response before sending > > a new request to avoid trying to put a 2nd request into the CRQ. Also reset > > the res_len before sending a command so we will end up in that > > wait_event_interruptable() waiting for the response rather than reading the > > command packet as a response. > > > > The easiest way to trigger the problem is to run the following > > > > cd /sys/device/vio/71000004 > > > > while :; cat pcrs >/dev/null; done > > > > And press Ctrl-C. This will then display an error > > > > tpm_ibmvtpm 71000004: tpm_transmit: tpm_recv: error -4 > > > > followed by several other errors once interaction with the TPM resumes. > > > > tpm_ibmvtpm 71000004: A TPM error (101) occurred attempting to determine the number of PCRS. > > > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > > Tested-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com> > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> You have to hold for a while that I get through fixing my trusted keys patches and test Jasons code but I will definitely apply this to my master and include to the 4.5 pull request. > > --- > > drivers/char/tpm/tpm_ibmvtpm.c | 22 ++++++++++++++++++++-- > > drivers/char/tpm/tpm_ibmvtpm.h | 1 + > > 2 files changed, 21 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > > index 3e6a226..b0a9a9e 100644 > > --- a/drivers/char/tpm/tpm_ibmvtpm.c > > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > > @@ -90,7 +90,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > > return 0; > > } > > > > - sig = wait_event_interruptible(ibmvtpm->wq, ibmvtpm->res_len != 0); > > + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); > > if (sig) > > return -EINTR; > > > > @@ -125,7 +125,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > > struct ibmvtpm_dev *ibmvtpm; > > struct ibmvtpm_crq crq; > > __be64 *word = (__be64 *)&crq; > > - int rc; > > + int rc, sig; > > > > ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > > > @@ -141,18 +141,35 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > > return -EIO; > > } > > > > + if (ibmvtpm->tpm_processing_cmd) { > > + dev_info(ibmvtpm->dev, > > + "Need to wait for TPM to finish\n"); > > + /* wait for previous command to finish */ > > + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); > > + if (sig) > > + return -EINTR; > > + } > > + > > spin_lock(&ibmvtpm->rtce_lock); > > + ibmvtpm->res_len = 0; > > memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count); > > crq.valid = (u8)IBMVTPM_VALID_CMD; > > crq.msg = (u8)VTPM_TPM_COMMAND; > > crq.len = cpu_to_be16(count); > > crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle); > > > > + /* > > + * set the processing flag before the Hcall, since we may get the > > + * result (interrupt) before even being able to check rc. > > + */ > > + ibmvtpm->tpm_processing_cmd = true; > > + > > rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), > > be64_to_cpu(word[1])); > > if (rc != H_SUCCESS) { > > dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc); > > rc = 0; > > + ibmvtpm->tpm_processing_cmd = false; > > } else > > rc = count; > > > > @@ -515,6 +532,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, > > case VTPM_TPM_COMMAND_RES: > > /* len of the data in rtce buffer */ > > ibmvtpm->res_len = be16_to_cpu(crq->len); > > + ibmvtpm->tpm_processing_cmd = false; > > wake_up_interruptible(&ibmvtpm->wq); > > return; > > default: > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h > > index 6af9289..91dfe766 100644 > > --- a/drivers/char/tpm/tpm_ibmvtpm.h > > +++ b/drivers/char/tpm/tpm_ibmvtpm.h > > @@ -45,6 +45,7 @@ struct ibmvtpm_dev { > > wait_queue_head_t wq; > > u16 res_len; > > u32 vtpm_version; > > + bool tpm_processing_cmd; > > }; > > > > #define CRQ_RES_BUF_SIZE PAGE_SIZE > > -- > > 1.8.3.1 > > > > > > ------------------------------------------------------------------------------ > > _______________________________________________ > > tpmdd-devel mailing list > > tpmdd-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/tpmdd-devel > > /Jarkko /Jarkko ------------------------------------------------------------------------------
On 12/12/2015 02:56 AM, Jarkko Sakkinen wrote: > On Fri, Dec 11, 2015 at 09:31:55PM +0200, Jarkko Sakkinen wrote: >> On Wed, Dec 09, 2015 at 08:52:01AM -0500, Stefan Berger wrote: >>> From: Stefan Berger <stefanb@linux.vnet.ibm.com> >>> >>> When the TPM response reception is interrupted in the wait_event_interruptable >>> call, the TPM is still busy processing the command and will only deliver the >>> response later. So we have to wait for an outstanding response before sending >>> a new request to avoid trying to put a 2nd request into the CRQ. Also reset >>> the res_len before sending a command so we will end up in that >>> wait_event_interruptable() waiting for the response rather than reading the >>> command packet as a response. >>> >>> The easiest way to trigger the problem is to run the following >>> >>> cd /sys/device/vio/71000004 >>> >>> while :; cat pcrs >/dev/null; done >>> >>> And press Ctrl-C. This will then display an error >>> >>> tpm_ibmvtpm 71000004: tpm_transmit: tpm_recv: error -4 >>> >>> followed by several other errors once interaction with the TPM resumes. >>> >>> tpm_ibmvtpm 71000004: A TPM error (101) occurred attempting to determine the number of PCRS. >>> >>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> >>> Tested-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com> >> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > You have to hold for a while that I get through fixing my trusted keys > patches and test Jasons code but I will definitely apply this to my > master and include to the 4.5 pull request. Thanks. Stefan ------------------------------------------------------------------------------
Looks good to me. Reviewed-by: Ashley Lai <ashley@ashleylai.com> Thanks, --Ashley Lai On 12/09/2015 07:52 AM, Stefan Berger wrote: > From: Stefan Berger <stefanb@linux.vnet.ibm.com> > > When the TPM response reception is interrupted in the wait_event_interruptable > call, the TPM is still busy processing the command and will only deliver the > response later. So we have to wait for an outstanding response before sending > a new request to avoid trying to put a 2nd request into the CRQ. Also reset > the res_len before sending a command so we will end up in that > wait_event_interruptable() waiting for the response rather than reading the > command packet as a response. > > The easiest way to trigger the problem is to run the following > > cd /sys/device/vio/71000004 > > while :; cat pcrs >/dev/null; done > > And press Ctrl-C. This will then display an error > > tpm_ibmvtpm 71000004: tpm_transmit: tpm_recv: error -4 > > followed by several other errors once interaction with the TPM resumes. > > tpm_ibmvtpm 71000004: A TPM error (101) occurred attempting to determine the number of PCRS. > > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > Tested-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com> > --- > drivers/char/tpm/tpm_ibmvtpm.c | 22 ++++++++++++++++++++-- > drivers/char/tpm/tpm_ibmvtpm.h | 1 + > 2 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c > index 3e6a226..b0a9a9e 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.c > +++ b/drivers/char/tpm/tpm_ibmvtpm.c > @@ -90,7 +90,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) > return 0; > } > > - sig = wait_event_interruptible(ibmvtpm->wq, ibmvtpm->res_len != 0); > + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); > if (sig) > return -EINTR; > > @@ -125,7 +125,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > struct ibmvtpm_dev *ibmvtpm; > struct ibmvtpm_crq crq; > __be64 *word = (__be64 *)&crq; > - int rc; > + int rc, sig; > > ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); > > @@ -141,18 +141,35 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) > return -EIO; > } > > + if (ibmvtpm->tpm_processing_cmd) { > + dev_info(ibmvtpm->dev, > + "Need to wait for TPM to finish\n"); > + /* wait for previous command to finish */ > + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); > + if (sig) > + return -EINTR; > + } > + > spin_lock(&ibmvtpm->rtce_lock); > + ibmvtpm->res_len = 0; > memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count); > crq.valid = (u8)IBMVTPM_VALID_CMD; > crq.msg = (u8)VTPM_TPM_COMMAND; > crq.len = cpu_to_be16(count); > crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle); > > + /* > + * set the processing flag before the Hcall, since we may get the > + * result (interrupt) before even being able to check rc. > + */ > + ibmvtpm->tpm_processing_cmd = true; > + > rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), > be64_to_cpu(word[1])); > if (rc != H_SUCCESS) { > dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc); > rc = 0; > + ibmvtpm->tpm_processing_cmd = false; > } else > rc = count; > > @@ -515,6 +532,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, > case VTPM_TPM_COMMAND_RES: > /* len of the data in rtce buffer */ > ibmvtpm->res_len = be16_to_cpu(crq->len); > + ibmvtpm->tpm_processing_cmd = false; > wake_up_interruptible(&ibmvtpm->wq); > return; > default: > diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h > index 6af9289..91dfe766 100644 > --- a/drivers/char/tpm/tpm_ibmvtpm.h > +++ b/drivers/char/tpm/tpm_ibmvtpm.h > @@ -45,6 +45,7 @@ struct ibmvtpm_dev { > wait_queue_head_t wq; > u16 res_len; > u32 vtpm_version; > + bool tpm_processing_cmd; > }; > > #define CRQ_RES_BUF_SIZE PAGE_SIZE ------------------------------------------------------------------------------
On Sat, Dec 12, 2015 at 11:21:24AM -0500, Stefan Berger wrote: > On 12/12/2015 02:56 AM, Jarkko Sakkinen wrote: > >On Fri, Dec 11, 2015 at 09:31:55PM +0200, Jarkko Sakkinen wrote: > >>On Wed, Dec 09, 2015 at 08:52:01AM -0500, Stefan Berger wrote: > >>>From: Stefan Berger <stefanb@linux.vnet.ibm.com> > >>> > >>>When the TPM response reception is interrupted in the wait_event_interruptable > >>>call, the TPM is still busy processing the command and will only deliver the > >>>response later. So we have to wait for an outstanding response before sending > >>>a new request to avoid trying to put a 2nd request into the CRQ. Also reset > >>>the res_len before sending a command so we will end up in that > >>>wait_event_interruptable() waiting for the response rather than reading the > >>>command packet as a response. > >>> > >>>The easiest way to trigger the problem is to run the following > >>> > >>>cd /sys/device/vio/71000004 > >>> > >>>while :; cat pcrs >/dev/null; done > >>> > >>>And press Ctrl-C. This will then display an error > >>> > >>>tpm_ibmvtpm 71000004: tpm_transmit: tpm_recv: error -4 > >>> > >>>followed by several other errors once interaction with the TPM resumes. > >>> > >>>tpm_ibmvtpm 71000004: A TPM error (101) occurred attempting to determine the number of PCRS. > >>> > >>>Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> > >>>Tested-by: Hon Ching(Vicky) Lo <honclo@linux.vnet.ibm.com> > >>Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > >You have to hold for a while that I get through fixing my trusted keys > >patches and test Jasons code but I will definitely apply this to my > >master and include to the 4.5 pull request. > > Thanks. Applied to https://github.com/jsakkine/linux-tpmdd/commits/master > Stefan /Jarkko ------------------------------------------------------------------------------
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c index 3e6a226..b0a9a9e 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.c +++ b/drivers/char/tpm/tpm_ibmvtpm.c @@ -90,7 +90,7 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count) return 0; } - sig = wait_event_interruptible(ibmvtpm->wq, ibmvtpm->res_len != 0); + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); if (sig) return -EINTR; @@ -125,7 +125,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) struct ibmvtpm_dev *ibmvtpm; struct ibmvtpm_crq crq; __be64 *word = (__be64 *)&crq; - int rc; + int rc, sig; ibmvtpm = (struct ibmvtpm_dev *)TPM_VPRIV(chip); @@ -141,18 +141,35 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count) return -EIO; } + if (ibmvtpm->tpm_processing_cmd) { + dev_info(ibmvtpm->dev, + "Need to wait for TPM to finish\n"); + /* wait for previous command to finish */ + sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd); + if (sig) + return -EINTR; + } + spin_lock(&ibmvtpm->rtce_lock); + ibmvtpm->res_len = 0; memcpy((void *)ibmvtpm->rtce_buf, (void *)buf, count); crq.valid = (u8)IBMVTPM_VALID_CMD; crq.msg = (u8)VTPM_TPM_COMMAND; crq.len = cpu_to_be16(count); crq.data = cpu_to_be32(ibmvtpm->rtce_dma_handle); + /* + * set the processing flag before the Hcall, since we may get the + * result (interrupt) before even being able to check rc. + */ + ibmvtpm->tpm_processing_cmd = true; + rc = ibmvtpm_send_crq(ibmvtpm->vdev, be64_to_cpu(word[0]), be64_to_cpu(word[1])); if (rc != H_SUCCESS) { dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc); rc = 0; + ibmvtpm->tpm_processing_cmd = false; } else rc = count; @@ -515,6 +532,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq, case VTPM_TPM_COMMAND_RES: /* len of the data in rtce buffer */ ibmvtpm->res_len = be16_to_cpu(crq->len); + ibmvtpm->tpm_processing_cmd = false; wake_up_interruptible(&ibmvtpm->wq); return; default: diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h index 6af9289..91dfe766 100644 --- a/drivers/char/tpm/tpm_ibmvtpm.h +++ b/drivers/char/tpm/tpm_ibmvtpm.h @@ -45,6 +45,7 @@ struct ibmvtpm_dev { wait_queue_head_t wq; u16 res_len; u32 vtpm_version; + bool tpm_processing_cmd; }; #define CRQ_RES_BUF_SIZE PAGE_SIZE