2020-03-31 11:47:05

by Amir Mizinski

[permalink] [raw]
Subject: [PATCH v4 3/7] tpm: tpm_tis: rewrite "tpm_tis_req_canceled()"

From: Amir Mizinski <[email protected]>

Using this function while read/write data resulted in aborted operation.
After investigating according to TCG TPM Profile (PTP) Specifications,
i found cancel should happen only if TPM_STS.commandReady bit is lit and
couldn't find a case when the current condition is valid.
Also only cmdReady bit need to be compared instead of the full lower status
register byte.

Signed-off-by: Amir Mizinski <[email protected]>
---
drivers/char/tpm/tpm_tis_core.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 6c4f232..18b9dc4 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -710,17 +710,7 @@ static int probe_itpm(struct tpm_chip *chip)

static bool tpm_tis_req_canceled(struct tpm_chip *chip, u8 status)
{
- struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
-
- switch (priv->manufacturer_id) {
- case TPM_VID_WINBOND:
- return ((status == TPM_STS_VALID) ||
- (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY)));
- case TPM_VID_STM:
- return (status == (TPM_STS_VALID | TPM_STS_COMMAND_READY));
- default:
- return (status == TPM_STS_COMMAND_READY);
- }
+ return ((status & TPM_STS_COMMAND_READY) == TPM_STS_COMMAND_READY);
}

static irqreturn_t tis_int_handler(int dummy, void *dev_id)
--
2.7.4



===========================================================================================
The privileged confidential information contained in this email is intended for use only by the addressees as indicated by the original sender of this email. If you are not the addressee indicated in this email or are not responsible for delivery of the email to such a person, please kindly reply to the sender indicating this fact and delete all copies of it from your computer and network server immediately. Your cooperation is highly appreciated. It is advised that any unauthorized use of confidential information of Nuvoton is strictly prohibited; and any information in this email irrelevant to the official business of Nuvoton shall be deemed as neither given nor endorsed by Nuvoton.


2020-03-31 12:15:54

by Jarkko Sakkinen

[permalink] [raw]
Subject: Re: [PATCH v4 3/7] tpm: tpm_tis: rewrite "tpm_tis_req_canceled()"

On Tue, Mar 31, 2020 at 02:32:03PM +0300, [email protected] wrote:
> From: Amir Mizinski <[email protected]>
>
> Using this function while read/write data resulted in aborted operation.
> After investigating according to TCG TPM Profile (PTP) Specifications,
> i found cancel should happen only if TPM_STS.commandReady bit is lit and
> couldn't find a case when the current condition is valid.
> Also only cmdReady bit need to be compared instead of the full lower status
> register byte.
>
> Signed-off-by: Amir Mizinski <[email protected]>

We don't care about spec's. We care about hardware and not all hardware
follows specifications.

Please fix the exact thing you want to fix (and please provide a fixes
tag).

/Jarkko

2020-03-31 13:55:08

by Ken Goldman

[permalink] [raw]
Subject: Re: [PATCH v4 3/7] tpm: tpm_tis: rewrite "tpm_tis_req_canceled()"

On 3/31/2020 8:13 AM, Jarkko Sakkinen wrote:
> On Tue, Mar 31, 2020 at 02:32:03PM +0300, [email protected] wrote:
>> From: Amir Mizinski <[email protected]>
>>
>> Using this function while read/write data resulted in aborted operation.
>> After investigating according to TCG TPM Profile (PTP) Specifications,
>> i found cancel should happen only if TPM_STS.commandReady bit is lit and
>> couldn't find a case when the current condition is valid.
>> Also only cmdReady bit need to be compared instead of the full lower status
>> register byte.
>>
>> Signed-off-by: Amir Mizinski <[email protected]>
>
> We don't care about spec's. We care about hardware and not all hardware
> follows specifications.
>
> Please fix the exact thing you want to fix (and please provide a fixes
> tag).

I edit the TPM main spec, not the PTP. As I discover TPMs that don't
meet the spec, or where the spec has changed over time, I add
informative comments to guide developers.

If you know of TPM hardware that does not meet the PTP specification,
let me know the specifics. I can bring it to the PTP work group and try
to get comments added.

I do not need to know the TPM vendor. That information would not go
into the specification anyway.