| Message ID | d5469294-34e6-2e5e-43e8-9ff82579fc2a@maciej.szmigiero.name |
|---|---|
| State | New |
| Headers | show |
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"); > ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi
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 ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi
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 ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi
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 ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, SlashDot.org! http://sdm.link/slashdot
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");
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(-) ------------------------------------------------------------------------------ Developer Access Program for Intel Xeon Phi Processors Access to Intel Xeon Phi processor-based developer platforms. With one year of Intel Parallel Studio XE. Training and support from Colfax. Order your platform today. http://sdm.link/xeonphi