[PATCH] tpm_tis: use default timeout value if chip reports it as zero

[PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Maciej S. Szmigiero]

Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
longer works.
The initialization proceeds fine until we get and start using chip-reported
timeouts - and the chip reports C and D timeouts of zero.

It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
means to override the chip returned timeouts") we had actually let default
timeout values remain in this case, so let's bring back this behavior to
make chips like Atmel 3203 work again.

Use a common code that was introduced by that commit so a warning is
printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
timeouts aren't chip-original.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>

Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
Cc: stable@vger.kernel.org
---
This replaces "tpm_tis: override reported C and D timeouts for Atmel 3203"
submission.

 drivers/char/tpm/tpm-interface.c | 53 ++++++++++++++++++++++++----------------
 1 file changed, 32 insertions(+), 21 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index fecdd3fa8126..a3461cbdde5f 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -522,8 +522,7 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
 int tpm_get_timeouts(struct tpm_chip *chip)
 {
 	cap_t cap;
-	unsigned long new_timeout[4];
-	unsigned long old_timeout[4];
+	unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
 	ssize_t rc;
 
 	if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
@@ -564,11 +563,15 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 		return rc;
 	}
 
-	old_timeout[0] = be32_to_cpu(cap.timeout.a);
-	old_timeout[1] = be32_to_cpu(cap.timeout.b);
-	old_timeout[2] = be32_to_cpu(cap.timeout.c);
-	old_timeout[3] = be32_to_cpu(cap.timeout.d);
-	memcpy(new_timeout, old_timeout, sizeof(new_timeout));
+	timeout_old[0] = jiffies_to_usecs(chip->timeout_a);
+	timeout_old[1] = jiffies_to_usecs(chip->timeout_b);
+	timeout_old[2] = jiffies_to_usecs(chip->timeout_c);
+	timeout_old[3] = jiffies_to_usecs(chip->timeout_d);
+	timeout_chip[0] = be32_to_cpu(cap.timeout.a);
+	timeout_chip[1] = be32_to_cpu(cap.timeout.b);
+	timeout_chip[2] = be32_to_cpu(cap.timeout.c);
+	timeout_chip[3] = be32_to_cpu(cap.timeout.d);
+	memcpy(timeout_eff, timeout_chip, sizeof(timeout_eff));
 
 	/*
 	 * Provide ability for vendor overrides of timeout values in case
@@ -576,16 +579,24 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 	 */
 	if (chip->ops->update_timeouts != NULL)
 		chip->timeout_adjusted =
-			chip->ops->update_timeouts(chip, new_timeout);
+			chip->ops->update_timeouts(chip, timeout_eff);
 
 	if (!chip->timeout_adjusted) {
-		/* Don't overwrite default if value is 0 */
-		if (new_timeout[0] != 0 && new_timeout[0] < 1000) {
-			int i;
+		/* Restore default if chip reported 0 */
+		int i;
 
+		for (i = 0; i < ARRAY_SIZE(timeout_eff); i++) {
+			if (timeout_eff[i])
+				continue;
+
+			timeout_eff[i] = timeout_old[i];
+			chip->timeout_adjusted = true;
+		}
+
+		if (timeout_eff[0] != 0 && timeout_eff[0] < 1000) {
 			/* timeouts in msec rather usec */
-			for (i = 0; i != ARRAY_SIZE(new_timeout); i++)
-				new_timeout[i] *= 1000;
+			for (i = 0; i != ARRAY_SIZE(timeout_eff); i++)
+				timeout_eff[i] *= 1000;
 			chip->timeout_adjusted = true;
 		}
 	}
@@ -594,16 +605,16 @@ int tpm_get_timeouts(struct tpm_chip *chip)
 	if (chip->timeout_adjusted) {
 		dev_info(&chip->dev,
 			 HW_ERR "Adjusting reported timeouts: A %lu->%luus B %lu->%luus C %lu->%luus D %lu->%luus\n",
-			 old_timeout[0], new_timeout[0],
-			 old_timeout[1], new_timeout[1],
-			 old_timeout[2], new_timeout[2],
-			 old_timeout[3], new_timeout[3]);
+			 timeout_chip[0], timeout_eff[0],
+			 timeout_chip[1], timeout_eff[1],
+			 timeout_chip[2], timeout_eff[2],
+			 timeout_chip[3], timeout_eff[3]);
 	}
 
-	chip->timeout_a = usecs_to_jiffies(new_timeout[0]);
-	chip->timeout_b = usecs_to_jiffies(new_timeout[1]);
-	chip->timeout_c = usecs_to_jiffies(new_timeout[2]);
-	chip->timeout_d = usecs_to_jiffies(new_timeout[3]);
+	chip->timeout_a = usecs_to_jiffies(timeout_eff[0]);
+	chip->timeout_b = usecs_to_jiffies(timeout_eff[1]);
+	chip->timeout_c = usecs_to_jiffies(timeout_eff[2]);
+	chip->timeout_d = usecs_to_jiffies(timeout_eff[3]);
 
 	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, &cap,
 			"attempting to determine the durations");

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> longer works.
> The initialization proceeds fine until we get and start using chip-reported
> timeouts - and the chip reports C and D timeouts of zero.
> 
> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> means to override the chip returned timeouts") we had actually let default
> timeout values remain in this case, so let's bring back this behavior to
> make chips like Atmel 3203 work again.
> 
> Use a common code that was introduced by that commit so a warning is
> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> timeouts aren't chip-original.
> 
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> 
> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> Cc: stable@vger.kernel.org

Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

/Jarkko

> ---
> This replaces "tpm_tis: override reported C and D timeouts for Atmel 3203"
> submission.
> 
>  drivers/char/tpm/tpm-interface.c | 53 ++++++++++++++++++++++++----------------
>  1 file changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index fecdd3fa8126..a3461cbdde5f 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -522,8 +522,7 @@ static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
>  int tpm_get_timeouts(struct tpm_chip *chip)
>  {
>  	cap_t cap;
> -	unsigned long new_timeout[4];
> -	unsigned long old_timeout[4];
> +	unsigned long timeout_old[4], timeout_chip[4], timeout_eff[4];
>  	ssize_t rc;
>  
>  	if (chip->flags & TPM_CHIP_FLAG_HAVE_TIMEOUTS)
> @@ -564,11 +563,15 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  		return rc;
>  	}
>  
> -	old_timeout[0] = be32_to_cpu(cap.timeout.a);
> -	old_timeout[1] = be32_to_cpu(cap.timeout.b);
> -	old_timeout[2] = be32_to_cpu(cap.timeout.c);
> -	old_timeout[3] = be32_to_cpu(cap.timeout.d);
> -	memcpy(new_timeout, old_timeout, sizeof(new_timeout));
> +	timeout_old[0] = jiffies_to_usecs(chip->timeout_a);
> +	timeout_old[1] = jiffies_to_usecs(chip->timeout_b);
> +	timeout_old[2] = jiffies_to_usecs(chip->timeout_c);
> +	timeout_old[3] = jiffies_to_usecs(chip->timeout_d);
> +	timeout_chip[0] = be32_to_cpu(cap.timeout.a);
> +	timeout_chip[1] = be32_to_cpu(cap.timeout.b);
> +	timeout_chip[2] = be32_to_cpu(cap.timeout.c);
> +	timeout_chip[3] = be32_to_cpu(cap.timeout.d);
> +	memcpy(timeout_eff, timeout_chip, sizeof(timeout_eff));
>  
>  	/*
>  	 * Provide ability for vendor overrides of timeout values in case
> @@ -576,16 +579,24 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  	 */
>  	if (chip->ops->update_timeouts != NULL)
>  		chip->timeout_adjusted =
> -			chip->ops->update_timeouts(chip, new_timeout);
> +			chip->ops->update_timeouts(chip, timeout_eff);
>  
>  	if (!chip->timeout_adjusted) {
> -		/* Don't overwrite default if value is 0 */
> -		if (new_timeout[0] != 0 && new_timeout[0] < 1000) {
> -			int i;
> +		/* Restore default if chip reported 0 */
> +		int i;
>  
> +		for (i = 0; i < ARRAY_SIZE(timeout_eff); i++) {
> +			if (timeout_eff[i])
> +				continue;
> +
> +			timeout_eff[i] = timeout_old[i];
> +			chip->timeout_adjusted = true;
> +		}
> +
> +		if (timeout_eff[0] != 0 && timeout_eff[0] < 1000) {
>  			/* timeouts in msec rather usec */
> -			for (i = 0; i != ARRAY_SIZE(new_timeout); i++)
> -				new_timeout[i] *= 1000;
> +			for (i = 0; i != ARRAY_SIZE(timeout_eff); i++)
> +				timeout_eff[i] *= 1000;
>  			chip->timeout_adjusted = true;
>  		}
>  	}
> @@ -594,16 +605,16 @@ int tpm_get_timeouts(struct tpm_chip *chip)
>  	if (chip->timeout_adjusted) {
>  		dev_info(&chip->dev,
>  			 HW_ERR "Adjusting reported timeouts: A %lu->%luus B %lu->%luus C %lu->%luus D %lu->%luus\n",
> -			 old_timeout[0], new_timeout[0],
> -			 old_timeout[1], new_timeout[1],
> -			 old_timeout[2], new_timeout[2],
> -			 old_timeout[3], new_timeout[3]);
> +			 timeout_chip[0], timeout_eff[0],
> +			 timeout_chip[1], timeout_eff[1],
> +			 timeout_chip[2], timeout_eff[2],
> +			 timeout_chip[3], timeout_eff[3]);
>  	}
>  
> -	chip->timeout_a = usecs_to_jiffies(new_timeout[0]);
> -	chip->timeout_b = usecs_to_jiffies(new_timeout[1]);
> -	chip->timeout_c = usecs_to_jiffies(new_timeout[2]);
> -	chip->timeout_d = usecs_to_jiffies(new_timeout[3]);
> +	chip->timeout_a = usecs_to_jiffies(timeout_eff[0]);
> +	chip->timeout_b = usecs_to_jiffies(timeout_eff[1]);
> +	chip->timeout_c = usecs_to_jiffies(timeout_eff[2]);
> +	chip->timeout_d = usecs_to_jiffies(timeout_eff[3]);
>  
>  	rc = tpm_getcap(chip, TPM_CAP_PROP_TIS_DURATION, &cap,
>  			"attempting to determine the durations");
> 

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> > Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> > access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> > longer works.
> > The initialization proceeds fine until we get and start using chip-reported
> > timeouts - and the chip reports C and D timeouts of zero.
> > 
> > It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> > means to override the chip returned timeouts") we had actually let default
> > timeout values remain in this case, so let's bring back this behavior to
> > make chips like Atmel 3203 work again.
> > 
> > Use a common code that was introduced by that commit so a warning is
> > printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> > timeouts aren't chip-original.
> > 
> > Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> > 
> > Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> > Cc: stable@vger.kernel.org
> 
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

It's now applied to my master branch so if someone wants to
test it, it should be fairly easy.

/Jarkko

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
> > On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> > > Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> > > access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> > > longer works.
> > > The initialization proceeds fine until we get and start using chip-reported
> > > timeouts - and the chip reports C and D timeouts of zero.
> > > 
> > > It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> > > means to override the chip returned timeouts") we had actually let default
> > > timeout values remain in this case, so let's bring back this behavior to
> > > make chips like Atmel 3203 work again.
> > > 
> > > Use a common code that was introduced by that commit so a warning is
> > > printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> > > timeouts aren't chip-original.
> > > 
> > > Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> > > 
> > > Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> > > Cc: stable@vger.kernel.org
> > 
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> It's now applied to my master branch so if someone wants to
> test it, it should be fairly easy.
> 
> /Jarkko

And I decided to squash the rename commit to it.

/Jarkko

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Maciej S. Szmigiero]

On 16.01.2017 14:55, Jarkko Sakkinen wrote:
> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
>>>> longer works.
>>>> The initialization proceeds fine until we get and start using chip-reported
>>>> timeouts - and the chip reports C and D timeouts of zero.
>>>>
>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
>>>> means to override the chip returned timeouts") we had actually let default
>>>> timeout values remain in this case, so let's bring back this behavior to
>>>> make chips like Atmel 3203 work again.
>>>>
>>>> Use a common code that was introduced by that commit so a warning is
>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
>>>> timeouts aren't chip-original.
>>>>
>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
>>>>
>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
>>>> Cc: stable@vger.kernel.org
>>>
>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>
>> It's now applied to my master branch so if someone wants to
>> test it, it should be fairly easy.
> 
> And I decided to squash the rename commit to it.

Wouldn't it be better to squash the rename commit into "fix iTPM probe via
probe_itpm() function" patch (if it isn't too late), since they touch the
same functionality?

> 
> /Jarkko
> 

Maciej

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
> > On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
> >> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
> >>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> >>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> >>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> >>>> longer works.
> >>>> The initialization proceeds fine until we get and start using chip-reported
> >>>> timeouts - and the chip reports C and D timeouts of zero.
> >>>>
> >>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> >>>> means to override the chip returned timeouts") we had actually let default
> >>>> timeout values remain in this case, so let's bring back this behavior to
> >>>> make chips like Atmel 3203 work again.
> >>>>
> >>>> Use a common code that was introduced by that commit so a warning is
> >>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> >>>> timeouts aren't chip-original.
> >>>>
> >>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> >>>>
> >>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> >>>> Cc: stable@vger.kernel.org
> >>>
> >>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >>
> >> It's now applied to my master branch so if someone wants to
> >> test it, it should be fairly easy.
> > 
> > And I decided to squash the rename commit to it.
> 
> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
> probe_itpm() function" patch (if it isn't too late), since they touch the
> same functionality?

It can be renamed, modified and even dropped as long as it is in my
master branch and I haven't sent pull request to James Morris.

/Jarkkok

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Maciej S. Szmigiero]

On 16.01.2017 17:39, Jarkko Sakkinen wrote:
> On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
>> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
>>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
>>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
>>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
>>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
>>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
>>>>>> longer works.
>>>>>> The initialization proceeds fine until we get and start using chip-reported
>>>>>> timeouts - and the chip reports C and D timeouts of zero.
>>>>>>
>>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
>>>>>> means to override the chip returned timeouts") we had actually let default
>>>>>> timeout values remain in this case, so let's bring back this behavior to
>>>>>> make chips like Atmel 3203 work again.
>>>>>>
>>>>>> Use a common code that was introduced by that commit so a warning is
>>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
>>>>>> timeouts aren't chip-original.
>>>>>>
>>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
>>>>>>
>>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
>>>>>> Cc: stable@vger.kernel.org
>>>>>
>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>>
>>>> It's now applied to my master branch so if someone wants to
>>>> test it, it should be fairly easy.
>>>
>>> And I decided to squash the rename commit to it.
>>
>> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
>> probe_itpm() function" patch (if it isn't too late), since they touch the
>> same functionality?
> 
> It can be renamed, modified and even dropped as long as it is in my
> master branch and I haven't sent pull request to James Morris.

I see that "fix iTPM probe via probe_itpm() function" patch isn't present
in your pull request for 4.11.

What I meant in previous message was that you squashed and "rename
TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
value if chip reports it as zero" patch while it was logically connected with
"fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
at all in the tree).
Sorry if it wasn't 100% clear.

> /Jarkkok

Maciej

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Mon, Jan 23, 2017 at 06:23:55PM +0100, Maciej S. Szmigiero wrote:
> On 16.01.2017 17:39, Jarkko Sakkinen wrote:
> > On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
> >> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
> >>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
> >>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
> >>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> >>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> >>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> >>>>>> longer works.
> >>>>>> The initialization proceeds fine until we get and start using chip-reported
> >>>>>> timeouts - and the chip reports C and D timeouts of zero.
> >>>>>>
> >>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> >>>>>> means to override the chip returned timeouts") we had actually let default
> >>>>>> timeout values remain in this case, so let's bring back this behavior to
> >>>>>> make chips like Atmel 3203 work again.
> >>>>>>
> >>>>>> Use a common code that was introduced by that commit so a warning is
> >>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> >>>>>> timeouts aren't chip-original.
> >>>>>>
> >>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> >>>>>>
> >>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> >>>>>> Cc: stable@vger.kernel.org
> >>>>>
> >>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >>>>
> >>>> It's now applied to my master branch so if someone wants to
> >>>> test it, it should be fairly easy.
> >>>
> >>> And I decided to squash the rename commit to it.
> >>
> >> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
> >> probe_itpm() function" patch (if it isn't too late), since they touch the
> >> same functionality?
> > 
> > It can be renamed, modified and even dropped as long as it is in my
> > master branch and I haven't sent pull request to James Morris.
> 
> I see that "fix iTPM probe via probe_itpm() function" patch isn't present
> in your pull request for 4.11.
> 
> What I meant in previous message was that you squashed and "rename
> TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
> value if chip reports it as zero" patch while it was logically connected with
> "fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
> at all in the tree).
> Sorry if it wasn't 100% clear.

I see.

I'll probably send later on pull request with fixes for release content
I can include that commit into that pull request. Does that work for
you?

/Jarkko

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Maciej S. Szmigiero]

On 24.01.2017 13:01, Jarkko Sakkinen wrote:
> On Mon, Jan 23, 2017 at 06:23:55PM +0100, Maciej S. Szmigiero wrote:
>> On 16.01.2017 17:39, Jarkko Sakkinen wrote:
>>> On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
>>>> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
>>>>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
>>>>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
>>>>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
>>>>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
>>>>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
>>>>>>>> longer works.
>>>>>>>> The initialization proceeds fine until we get and start using chip-reported
>>>>>>>> timeouts - and the chip reports C and D timeouts of zero.
>>>>>>>>
>>>>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
>>>>>>>> means to override the chip returned timeouts") we had actually let default
>>>>>>>> timeout values remain in this case, so let's bring back this behavior to
>>>>>>>> make chips like Atmel 3203 work again.
>>>>>>>>
>>>>>>>> Use a common code that was introduced by that commit so a warning is
>>>>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
>>>>>>>> timeouts aren't chip-original.
>>>>>>>>
>>>>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
>>>>>>>>
>>>>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>
>>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>>>>
>>>>>> It's now applied to my master branch so if someone wants to
>>>>>> test it, it should be fairly easy.
>>>>>
>>>>> And I decided to squash the rename commit to it.
>>>>
>>>> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
>>>> probe_itpm() function" patch (if it isn't too late), since they touch the
>>>> same functionality?
>>>
>>> It can be renamed, modified and even dropped as long as it is in my
>>> master branch and I haven't sent pull request to James Morris.
>>
>> I see that "fix iTPM probe via probe_itpm() function" patch isn't present
>> in your pull request for 4.11.
>>
>> What I meant in previous message was that you squashed and "rename
>> TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
>> value if chip reports it as zero" patch while it was logically connected with
>> "fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
>> at all in the tree).
>> Sorry if it wasn't 100% clear.
> 
> I see.
> 
> I'll probably send later on pull request with fixes for release content
> I can include that commit into that pull request. Does that work for
> you?

Yes, it would be fine, thanks.

> /Jarkko

Maciej

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Tue, Jan 24, 2017 at 02:42:29PM +0100, Maciej S. Szmigiero wrote:
> On 24.01.2017 13:01, Jarkko Sakkinen wrote:
> > On Mon, Jan 23, 2017 at 06:23:55PM +0100, Maciej S. Szmigiero wrote:
> >> On 16.01.2017 17:39, Jarkko Sakkinen wrote:
> >>> On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
> >>>> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
> >>>>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
> >>>>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
> >>>>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> >>>>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> >>>>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> >>>>>>>> longer works.
> >>>>>>>> The initialization proceeds fine until we get and start using chip-reported
> >>>>>>>> timeouts - and the chip reports C and D timeouts of zero.
> >>>>>>>>
> >>>>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> >>>>>>>> means to override the chip returned timeouts") we had actually let default
> >>>>>>>> timeout values remain in this case, so let's bring back this behavior to
> >>>>>>>> make chips like Atmel 3203 work again.
> >>>>>>>>
> >>>>>>>> Use a common code that was introduced by that commit so a warning is
> >>>>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> >>>>>>>> timeouts aren't chip-original.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> >>>>>>>>
> >>>>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> >>>>>>>> Cc: stable@vger.kernel.org
> >>>>>>>
> >>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >>>>>>
> >>>>>> It's now applied to my master branch so if someone wants to
> >>>>>> test it, it should be fairly easy.
> >>>>>
> >>>>> And I decided to squash the rename commit to it.
> >>>>
> >>>> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
> >>>> probe_itpm() function" patch (if it isn't too late), since they touch the
> >>>> same functionality?
> >>>
> >>> It can be renamed, modified and even dropped as long as it is in my
> >>> master branch and I haven't sent pull request to James Morris.
> >>
> >> I see that "fix iTPM probe via probe_itpm() function" patch isn't present
> >> in your pull request for 4.11.
> >>
> >> What I meant in previous message was that you squashed and "rename
> >> TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
> >> value if chip reports it as zero" patch while it was logically connected with
> >> "fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
> >> at all in the tree).
> >> Sorry if it wasn't 100% clear.
> > 
> > I see.
> > 
> > I'll probably send later on pull request with fixes for release content
> > I can include that commit into that pull request. Does that work for
> > you?
> 
> Yes, it would be fine, thanks.

It's now applied and pushed.

/Jarkko

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Maciej S. Szmigiero]

On 25.01.2017 21:09, Jarkko Sakkinen wrote:
> On Tue, Jan 24, 2017 at 02:42:29PM +0100, Maciej S. Szmigiero wrote:
>> On 24.01.2017 13:01, Jarkko Sakkinen wrote:
>>> On Mon, Jan 23, 2017 at 06:23:55PM +0100, Maciej S. Szmigiero wrote:
>>>> On 16.01.2017 17:39, Jarkko Sakkinen wrote:
>>>>> On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
>>>>>> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
>>>>>>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
>>>>>>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
>>>>>>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
>>>>>>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
>>>>>>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
>>>>>>>>>> longer works.
>>>>>>>>>> The initialization proceeds fine until we get and start using chip-reported
>>>>>>>>>> timeouts - and the chip reports C and D timeouts of zero.
>>>>>>>>>>
>>>>>>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
>>>>>>>>>> means to override the chip returned timeouts") we had actually let default
>>>>>>>>>> timeout values remain in this case, so let's bring back this behavior to
>>>>>>>>>> make chips like Atmel 3203 work again.
>>>>>>>>>>
>>>>>>>>>> Use a common code that was introduced by that commit so a warning is
>>>>>>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
>>>>>>>>>> timeouts aren't chip-original.
>>>>>>>>>>
>>>>>>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
>>>>>>>>>>
>>>>>>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
>>>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>>>
>>>>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>>>>>>
>>>>>>>> It's now applied to my master branch so if someone wants to
>>>>>>>> test it, it should be fairly easy.
>>>>>>>
>>>>>>> And I decided to squash the rename commit to it.
>>>>>>
>>>>>> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
>>>>>> probe_itpm() function" patch (if it isn't too late), since they touch the
>>>>>> same functionality?
>>>>>
>>>>> It can be renamed, modified and even dropped as long as it is in my
>>>>> master branch and I haven't sent pull request to James Morris.
>>>>
>>>> I see that "fix iTPM probe via probe_itpm() function" patch isn't present
>>>> in your pull request for 4.11.
>>>>
>>>> What I meant in previous message was that you squashed and "rename
>>>> TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
>>>> value if chip reports it as zero" patch while it was logically connected with
>>>> "fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
>>>> at all in the tree).
>>>> Sorry if it wasn't 100% clear.
>>>
>>> I see.
>>>
>>> I'll probably send later on pull request with fixes for release content
>>> I can include that commit into that pull request. Does that work for
>>> you?
>>
>> Yes, it would be fine, thanks.
> 
> It's now applied and pushed.

Almost there: it looks like the last hunk of the patch is missing from
the commit.

> /Jarkko

Maciej

 

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Jarkko Sakkinen]

On Wed, Jan 25, 2017 at 10:26:44PM +0100, Maciej S. Szmigiero wrote:
> On 25.01.2017 21:09, Jarkko Sakkinen wrote:
> > On Tue, Jan 24, 2017 at 02:42:29PM +0100, Maciej S. Szmigiero wrote:
> >> On 24.01.2017 13:01, Jarkko Sakkinen wrote:
> >>> On Mon, Jan 23, 2017 at 06:23:55PM +0100, Maciej S. Szmigiero wrote:
> >>>> On 16.01.2017 17:39, Jarkko Sakkinen wrote:
> >>>>> On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
> >>>>>> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
> >>>>>>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
> >>>>>>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
> >>>>>>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
> >>>>>>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
> >>>>>>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
> >>>>>>>>>> longer works.
> >>>>>>>>>> The initialization proceeds fine until we get and start using chip-reported
> >>>>>>>>>> timeouts - and the chip reports C and D timeouts of zero.
> >>>>>>>>>>
> >>>>>>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
> >>>>>>>>>> means to override the chip returned timeouts") we had actually let default
> >>>>>>>>>> timeout values remain in this case, so let's bring back this behavior to
> >>>>>>>>>> make chips like Atmel 3203 work again.
> >>>>>>>>>>
> >>>>>>>>>> Use a common code that was introduced by that commit so a warning is
> >>>>>>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
> >>>>>>>>>> timeouts aren't chip-original.
> >>>>>>>>>>
> >>>>>>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> >>>>>>>>>>
> >>>>>>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> >>>>>>>>>> Cc: stable@vger.kernel.org
> >>>>>>>>>
> >>>>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >>>>>>>>
> >>>>>>>> It's now applied to my master branch so if someone wants to
> >>>>>>>> test it, it should be fairly easy.
> >>>>>>>
> >>>>>>> And I decided to squash the rename commit to it.
> >>>>>>
> >>>>>> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
> >>>>>> probe_itpm() function" patch (if it isn't too late), since they touch the
> >>>>>> same functionality?
> >>>>>
> >>>>> It can be renamed, modified and even dropped as long as it is in my
> >>>>> master branch and I haven't sent pull request to James Morris.
> >>>>
> >>>> I see that "fix iTPM probe via probe_itpm() function" patch isn't present
> >>>> in your pull request for 4.11.
> >>>>
> >>>> What I meant in previous message was that you squashed and "rename
> >>>> TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
> >>>> value if chip reports it as zero" patch while it was logically connected with
> >>>> "fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
> >>>> at all in the tree).
> >>>> Sorry if it wasn't 100% clear.
> >>>
> >>> I see.
> >>>
> >>> I'll probably send later on pull request with fixes for release content
> >>> I can include that commit into that pull request. Does that work for
> >>> you?
> >>
> >> Yes, it would be fine, thanks.
> > 
> > It's now applied and pushed.
> 
> Almost there: it looks like the last hunk of the patch is missing from
> the commit.
> 
> > /Jarkko
> 
> Maciej

Sorrya about that (too much multitasking lately). I had to do a bit of
manual work to get it there. Now it should be good.

/Jarkko

Re: [PATCH] tpm_tis: use default timeout value if chip reports it as zero

[Maciej S. Szmigiero]

On 25.01.2017 23:58, Jarkko Sakkinen wrote:
> On Wed, Jan 25, 2017 at 10:26:44PM +0100, Maciej S. Szmigiero wrote:
>> On 25.01.2017 21:09, Jarkko Sakkinen wrote:
>>> On Tue, Jan 24, 2017 at 02:42:29PM +0100, Maciej S. Szmigiero wrote:
>>>> On 24.01.2017 13:01, Jarkko Sakkinen wrote:
>>>>> On Mon, Jan 23, 2017 at 06:23:55PM +0100, Maciej S. Szmigiero wrote:
>>>>>> On 16.01.2017 17:39, Jarkko Sakkinen wrote:
>>>>>>> On Mon, Jan 16, 2017 at 03:58:26PM +0100, Maciej S. Szmigiero wrote:
>>>>>>>> On 16.01.2017 14:55, Jarkko Sakkinen wrote:
>>>>>>>>> On Mon, Jan 16, 2017 at 03:46:12PM +0200, Jarkko Sakkinen wrote:
>>>>>>>>>> On Mon, Jan 16, 2017 at 11:42:02AM +0200, Jarkko Sakkinen wrote:
>>>>>>>>>>> On Fri, Jan 13, 2017 at 10:37:00PM +0100, Maciej S. Szmigiero wrote:
>>>>>>>>>>>> Since commit 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM
>>>>>>>>>>>> access") Atmel 3203 TPM on ThinkPad X61S (TPM firmware version 13.9) no
>>>>>>>>>>>> longer works.
>>>>>>>>>>>> The initialization proceeds fine until we get and start using chip-reported
>>>>>>>>>>>> timeouts - and the chip reports C and D timeouts of zero.
>>>>>>>>>>>>
>>>>>>>>>>>> It turns out that until commit 8e54caf407b98e ("tpm: Provide a generic
>>>>>>>>>>>> means to override the chip returned timeouts") we had actually let default
>>>>>>>>>>>> timeout values remain in this case, so let's bring back this behavior to
>>>>>>>>>>>> make chips like Atmel 3203 work again.
>>>>>>>>>>>>
>>>>>>>>>>>> Use a common code that was introduced by that commit so a warning is
>>>>>>>>>>>> printed in this case and /sys/class/tpm/tpm*/timeouts correctly says the
>>>>>>>>>>>> timeouts aren't chip-original.
>>>>>>>>>>>>
>>>>>>>>>>>> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
>>>>>>>>>>>>
>>>>>>>>>>>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
>>>>>>>>>>>> Cc: stable@vger.kernel.org
>>>>>>>>>>>
>>>>>>>>>>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>>>>>>>>>
>>>>>>>>>> It's now applied to my master branch so if someone wants to
>>>>>>>>>> test it, it should be fairly easy.
>>>>>>>>>
>>>>>>>>> And I decided to squash the rename commit to it.
>>>>>>>>
>>>>>>>> Wouldn't it be better to squash the rename commit into "fix iTPM probe via
>>>>>>>> probe_itpm() function" patch (if it isn't too late), since they touch the
>>>>>>>> same functionality?
>>>>>>>
>>>>>>> It can be renamed, modified and even dropped as long as it is in my
>>>>>>> master branch and I haven't sent pull request to James Morris.
>>>>>>
>>>>>> I see that "fix iTPM probe via probe_itpm() function" patch isn't present
>>>>>> in your pull request for 4.11.
>>>>>>
>>>>>> What I meant in previous message was that you squashed and "rename
>>>>>> TPM_TIS_ITPM_POSSIBLE to TPM_TIS_ITPM_WORKAROUND" patch into "use default timeout
>>>>>> value if chip reports it as zero" patch while it was logically connected with
>>>>>> "fix iTPM probe via probe_itpm() function" patch instead (which now isn't present
>>>>>> at all in the tree).
>>>>>> Sorry if it wasn't 100% clear.
>>>>>
>>>>> I see.
>>>>>
>>>>> I'll probably send later on pull request with fixes for release content
>>>>> I can include that commit into that pull request. Does that work for
>>>>> you?
>>>>
>>>> Yes, it would be fine, thanks.
>>>
>>> It's now applied and pushed.
>>
>> Almost there: it looks like the last hunk of the patch is missing from
>> the commit.
>>
>>> /Jarkko
>>
>> Maciej
> 
> Sorrya about that (too much multitasking lately). I had to do a bit of
> manual work to get it there. Now it should be good.

It looks right now, thanks.

> /Jarkko

Maciej