From: Stefan Berger <[email protected]>
Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
Signed-off-by: Stefan Berger <[email protected]>
Cc: Nayna Jain <[email protected]>
Cc: George Wilson <[email protected]>
---
drivers/char/tpm/tpm_ibmvtpm.c | 15 ++++++++-------
drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 7a9eca5768f8..5d795866b483 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -215,11 +215,12 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
return -EIO;
}
- if (ibmvtpm->tpm_processing_cmd) {
+ if ((ibmvtpm->tpm_status & TPM_STATUS_BUSY)) {
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);
+ sig = wait_event_interruptible(ibmvtpm->wq,
+ (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
if (sig)
return -EINTR;
}
@@ -232,7 +233,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
* 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;
+ ibmvtpm->tpm_status |= TPM_STATUS_BUSY;
again:
rc = ibmvtpm_send_crq(ibmvtpm->vdev,
@@ -250,7 +251,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
goto again;
}
dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
- ibmvtpm->tpm_processing_cmd = false;
+ ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
}
spin_unlock(&ibmvtpm->rtce_lock);
@@ -266,7 +267,7 @@ static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
{
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
- return ibmvtpm->tpm_processing_cmd;
+ return ibmvtpm->tpm_status;
}
/**
@@ -454,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
.send = tpm_ibmvtpm_send,
.cancel = tpm_ibmvtpm_cancel,
.status = tpm_ibmvtpm_status,
- .req_complete_mask = true,
+ .req_complete_mask = TPM_STATUS_BUSY,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
@@ -547,7 +548,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;
+ ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
wake_up_interruptible(&ibmvtpm->wq);
return;
default:
diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index 51198b137461..252f1cccdfc5 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h
@@ -41,7 +41,8 @@ struct ibmvtpm_dev {
wait_queue_head_t wq;
u16 res_len;
u32 vtpm_version;
- u8 tpm_processing_cmd;
+ u8 tpm_status;
+#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
};
#define CRQ_RES_BUF_SIZE PAGE_SIZE
--
2.31.1
On Mon, Aug 09, 2021 at 03:21:59PM -0400, Stefan Berger wrote:
> From: Stefan Berger <[email protected]>
>
> Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
> the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>
> Signed-off-by: Stefan Berger <[email protected]>
> Cc: Nayna Jain <[email protected]>
> Cc: George Wilson <[email protected]>
> ---
> drivers/char/tpm/tpm_ibmvtpm.c | 15 ++++++++-------
> drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 7a9eca5768f8..5d795866b483 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -215,11 +215,12 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> return -EIO;
> }
>
> - if (ibmvtpm->tpm_processing_cmd) {
> + if ((ibmvtpm->tpm_status & TPM_STATUS_BUSY)) {
> 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);
> + sig = wait_event_interruptible(ibmvtpm->wq,
> + (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
> if (sig)
> return -EINTR;
> }
> @@ -232,7 +233,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> * 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;
> + ibmvtpm->tpm_status |= TPM_STATUS_BUSY;
>
> again:
> rc = ibmvtpm_send_crq(ibmvtpm->vdev,
> @@ -250,7 +251,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> goto again;
> }
> dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
> - ibmvtpm->tpm_processing_cmd = false;
> + ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
> }
>
> spin_unlock(&ibmvtpm->rtce_lock);
> @@ -266,7 +267,7 @@ static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
> {
> struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
>
> - return ibmvtpm->tpm_processing_cmd;
> + return ibmvtpm->tpm_status;
> }
>
> /**
> @@ -454,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
> .send = tpm_ibmvtpm_send,
> .cancel = tpm_ibmvtpm_cancel,
> .status = tpm_ibmvtpm_status,
> - .req_complete_mask = true,
> + .req_complete_mask = TPM_STATUS_BUSY,
> .req_complete_val = 0,
> .req_canceled = tpm_ibmvtpm_req_canceled,
> };
> @@ -547,7 +548,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;
> + ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
> wake_up_interruptible(&ibmvtpm->wq);
> return;
> default:
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> index 51198b137461..252f1cccdfc5 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.h
> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
> wait_queue_head_t wq;
> u16 res_len;
> u32 vtpm_version;
> - u8 tpm_processing_cmd;
> + u8 tpm_status;
> +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
Declare this already in the fix, and just leave the rename here.
> };
>
> #define CRQ_RES_BUF_SIZE PAGE_SIZE
> --
> 2.31.1
>
>
Otherwise, these look fine.
/Jarkko
On 8/10/21 1:58 PM, Jarkko Sakkinen wrote:
> On Mon, Aug 09, 2021 at 03:21:59PM -0400, Stefan Berger wrote:
>> From: Stefan Berger <[email protected]>
>>
>> Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
>> the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>>
>>
>> default:
>> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
>> index 51198b137461..252f1cccdfc5 100644
>> --- a/drivers/char/tpm/tpm_ibmvtpm.h
>> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
>> @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
>> wait_queue_head_t wq;
>> u16 res_len;
>> u32 vtpm_version;
>> - u8 tpm_processing_cmd;
>> + u8 tpm_status;
>> +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
> Declare this already in the fix, and just leave the rename here.
You mean the fix patch does not use 'true' anymore but uses the
TPM_STATUS_BUSY flag already but the name is still tpm_processing_cmd?
And literally only the renaming of this field is done in the 2nd patch?
Stefan
>
>> };
>>
>> #define CRQ_RES_BUF_SIZE PAGE_SIZE
>> --
>> 2.31.1
>>
>>
> Otherwise, these look fine.
>
> /Jarkko
On Tue, Aug 10, 2021 at 09:50:55PM -0400, Stefan Berger wrote:
>
> On 8/10/21 1:58 PM, Jarkko Sakkinen wrote:
> > On Mon, Aug 09, 2021 at 03:21:59PM -0400, Stefan Berger wrote:
> > > From: Stefan Berger <[email protected]>
> > >
> > > Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
> > > the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
> > >
> > >
> > > default:
> > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> > > index 51198b137461..252f1cccdfc5 100644
> > > --- a/drivers/char/tpm/tpm_ibmvtpm.h
> > > +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> > > @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
> > > wait_queue_head_t wq;
> > > u16 res_len;
> > > u32 vtpm_version;
> > > - u8 tpm_processing_cmd;
> > > + u8 tpm_status;
> > > +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
> > Declare this already in the fix, and just leave the rename here.
>
> You mean the fix patch does not use 'true' anymore but uses the
> TPM_STATUS_BUSY flag already but the name is still tpm_processing_cmd? And
> literally only the renaming of this field is done in the 2nd patch?
I can fixup these patches, and use '1', instead of true. No need to send
new ones.
Acked-by: Jarkko Sakkinen <[email protected]>
/Jarkko
On 8/10/21 10:10 PM, Jarkko Sakkinen wrote:
> On Tue, Aug 10, 2021 at 09:50:55PM -0400, Stefan Berger wrote:
>> On 8/10/21 1:58 PM, Jarkko Sakkinen wrote:
>>> On Mon, Aug 09, 2021 at 03:21:59PM -0400, Stefan Berger wrote:
>>>> From: Stefan Berger <[email protected]>
>>>>
>>>> Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
>>>> the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>>>>
>>>>
>>>> default:
>>>> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
>>>> index 51198b137461..252f1cccdfc5 100644
>>>> --- a/drivers/char/tpm/tpm_ibmvtpm.h
>>>> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
>>>> @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
>>>> wait_queue_head_t wq;
>>>> u16 res_len;
>>>> u32 vtpm_version;
>>>> - u8 tpm_processing_cmd;
>>>> + u8 tpm_status;
>>>> +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
>>> Declare this already in the fix, and just leave the rename here.
>> You mean the fix patch does not use 'true' anymore but uses the
>> TPM_STATUS_BUSY flag already but the name is still tpm_processing_cmd? And
>> literally only the renaming of this field is done in the 2nd patch?
> I can fixup these patches, and use '1', instead of true. No need to send
> new ones.
>
> Acked-by: Jarkko Sakkinen <[email protected]>
THANKS!
Stefan
>
> /Jarkko
On Wed, Aug 11, 2021 at 08:15:14AM -0400, Stefan Berger wrote:
>
> On 8/10/21 10:10 PM, Jarkko Sakkinen wrote:
> > On Tue, Aug 10, 2021 at 09:50:55PM -0400, Stefan Berger wrote:
> > > On 8/10/21 1:58 PM, Jarkko Sakkinen wrote:
> > > > On Mon, Aug 09, 2021 at 03:21:59PM -0400, Stefan Berger wrote:
> > > > > From: Stefan Berger <[email protected]>
> > > > >
> > > > > Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
> > > > > the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
> > > > >
> > > > >
> > > > > default:
> > > > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> > > > > index 51198b137461..252f1cccdfc5 100644
> > > > > --- a/drivers/char/tpm/tpm_ibmvtpm.h
> > > > > +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> > > > > @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
> > > > > wait_queue_head_t wq;
> > > > > u16 res_len;
> > > > > u32 vtpm_version;
> > > > > - u8 tpm_processing_cmd;
> > > > > + u8 tpm_status;
> > > > > +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
> > > > Declare this already in the fix, and just leave the rename here.
> > > You mean the fix patch does not use 'true' anymore but uses the
> > > TPM_STATUS_BUSY flag already but the name is still tpm_processing_cmd? And
> > > literally only the renaming of this field is done in the 2nd patch?
> > I can fixup these patches, and use '1', instead of true. No need to send
> > new ones.
> >
> > Acked-by: Jarkko Sakkinen <[email protected]>
I applied the first. If you have only one flag that you even
document as "processing the command" in the inline comment,
it makes absolutely no sense to rename it, as the current
name perfectly documents what it exactly is.
/Jarkko