* tpm device not showing up in /dev anymore
@ 2017-08-28 11:28 Laurent Bigonville
[not found] ` <f9526f55-df96-64fc-a4d6-877ce04e7156-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
0 siblings, 1 reply; 23+ messages in thread
From: Laurent Bigonville @ 2017-08-28 11:28 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
Hello,
Since the version 4.12 (I also tested with 4.13-rc5) of the kernel, the
tpm device is not showing up in /dev/. In dmesg I can see the following
lines:
> [ 1.772153] tpm_tis 00:06: 1.2 TPM (device-id 0x6871, rev-id 1)
> [ 1.788106] tpm tpm0: tpm_transmit: tpm_send: error -5
> [ 1.788146] tpm tpm0: A TPM error (-5) occurred attempting to
> determine the timeouts
> [ 1.788194] tpm_tis: probe of 00:06 failed with error -5
> [ 1.796865] ima: No TPM chip found, activating TPM-bypass! (rc=-19)
> [ 10.085245] tpm_inf_pnp 00:06: Found TPM with ID IFX0102
If I'm reverting to 4.11, everything is working fine.
An idea how to troubleshoot this?
Regards,
Laurent Bigonville
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
^ permalink raw reply [flat|nested] 23+ messages in thread[parent not found: <f9526f55-df96-64fc-a4d6-877ce04e7156-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <f9526f55-df96-64fc-a4d6-877ce04e7156-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-08-29 16:00 ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w [not found] ` <dcad0104c46d4d5f88e642862bdb42c2-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w @ 2017-08-29 16:00 UTC (permalink / raw) To: bigon-8fiUuRrzOP0dnm+yROfE0A; +Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Hi Laurent, > Since the version 4.12 (I also tested with 4.13-rc5) of the kernel, the tpm > device is not showing up in /dev/. In dmesg I can see the following > lines: Do you know what TPM you are using exactly (model number, firmware version, etc.)? > > [ 1.772153] tpm_tis 00:06: 1.2 TPM (device-id 0x6871, rev-id 1) [ > > 1.788106] tpm tpm0: tpm_transmit: tpm_send: error -5 [ 1.788146] > > tpm tpm0: A TPM error (-5) occurred attempting to determine the > > timeouts [ 1.788194] tpm_tis: probe of 00:06 failed with error -5 [ > > 1.796865] ima: No TPM chip found, activating TPM-bypass! (rc=-19) [ > > 10.085245] tpm_inf_pnp 00:06: Found TPM with ID IFX0102 > > If I'm reverting to 4.11, everything is working fine. Error -5 is EIO, which is as far as I can tell only used in few places (related to expect flag checks) in the tpm_transmit code path that first reports that error. If this is what causes the problem, then it only tells us that the TPM did not understand the command correctly (it received not enough/too much data). I cannot see any obvious changes between 4.11 and 4.12 that might affect the behavior in that region. > An idea how to troubleshoot this? Can you run git bisect on the changes between 4.11 and 4.12, so that we find the offending commit? It is probably sufficient to limit the search to commits that touch something in drivers/char/tpm. Alexander ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <dcad0104c46d4d5f88e642862bdb42c2-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <dcad0104c46d4d5f88e642862bdb42c2-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org> @ 2017-08-29 16:35 ` Laurent Bigonville [not found] ` <47c4300b-8701-79a6-1c58-3a5853f4c5e3-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Laurent Bigonville @ 2017-08-29 16:35 UTC (permalink / raw) To: Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : > Hi Laurent, Hello Alexander, >> Since the version 4.12 (I also tested with 4.13-rc5) of the kernel, the tpm >> device is not showing up in /dev/. In dmesg I can see the following >> lines: > Do you know what TPM you are using exactly (model number, firmware version, etc.)? tpm_version shows: # tpm_version xK�\x13N TPM 1.2 Version Info: Chip Version: 1.2.3.6 Spec Level: 2 Errata Revision: 0 TPM Vendor ID: SNS TPM Version: 01010000 Manufacturer Info: 534e5300 > >>> [ 1.772153] tpm_tis 00:06: 1.2 TPM (device-id 0x6871, rev-id 1) [ >>> 1.788106] tpm tpm0: tpm_transmit: tpm_send: error -5 [ 1.788146] >>> tpm tpm0: A TPM error (-5) occurred attempting to determine the >>> timeouts [ 1.788194] tpm_tis: probe of 00:06 failed with error -5 [ >>> 1.796865] ima: No TPM chip found, activating TPM-bypass! (rc=-19) [ >>> 10.085245] tpm_inf_pnp 00:06: Found TPM with ID IFX0102 >> If I'm reverting to 4.11, everything is working fine. > Error -5 is EIO, which is as far as I can tell only used in few places (related to expect flag checks) in the tpm_transmit code path that first reports that error. If this is what causes the problem, then it only tells us that the TPM did not understand the command correctly (it received not enough/too much data). I cannot see any obvious changes between 4.11 and 4.12 that might affect the behavior in that region. > >> An idea how to troubleshoot this? > Can you run git bisect on the changes between 4.11 and 4.12, so that we find the offending commit? It is probably sufficient to limit the search to commits that touch something in drivers/char/tpm. I'll try and keep you posted. Thanks for your answer. Laurent Bigonville > > Alexander ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <47c4300b-8701-79a6-1c58-3a5853f4c5e3-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <47c4300b-8701-79a6-1c58-3a5853f4c5e3-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-08-29 17:39 ` Peter Huewe 2017-08-29 18:55 ` Laurent Bigonville 1 sibling, 0 replies; 23+ messages in thread From: Peter Huewe @ 2017-08-29 17:39 UTC (permalink / raw) To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Laurent Bigonville, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w Hi Am 29. August 2017 18:35:26 MESZ schrieb Laurent Bigonville <bigon@debian.org>: >Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : >> Hi Laurent, > >Hello Alexander, > >>> Since the version 4.12 (I also tested with 4.13-rc5) of the kernel, >the tpm >>> device is not showing up in /dev/. In dmesg I can see the following >>> lines: >> Do you know what TPM you are using exactly (model number, firmware >version, etc.)? >tpm_version shows: > ># tpm_version >xK�\x13N TPM 1.2 Version Info: > Chip Version: 1.2.3.6 > Spec Level: 2 > Errata Revision: 0 > TPM Vendor ID: SNS > TPM Version: 01010000 > Manufacturer Info: 534e5300 > > >> >>>> [ 1.772153] tpm_tis 00:06: 1.2 TPM (device-id 0x6871, rev-id 1) >[ >>>> 1.788106] tpm tpm0: tpm_transmit: tpm_send: error -5 [ 1.788146] >>>> tpm tpm0: A TPM error (-5) occurred attempting to determine the >>>> timeouts [ 1.788194] tpm_tis: probe of 00:06 failed with error >-5 [ >>>> 1.796865] ima: No TPM chip found, activating TPM-bypass! (rc=-19) [ >>>> 10.085245] tpm_inf_pnp 00:06: Found TPM with ID IFX0102 >>> If I'm reverting to 4.11, everything is working fine. >> Error -5 is EIO, which is as far as I can tell only used in few >places (related to expect flag checks) in the tpm_transmit code path >that first reports that error. If this is what causes the problem, then >it only tells us that the TPM did not understand the command correctly >(it received not enough/too much data). I cannot see any obvious >changes between 4.11 and 4.12 that might affect the behavior in that >region. >> >>> An idea how to troubleshoot this? >> Can you run git bisect on the changes between 4.11 and 4.12, so that >we find the offending commit? It is probably sufficient to limit the >search to commits that touch something in drivers/char/tpm. > >I'll try and keep you posted. > >Thanks for your answer. Can you also maybe post the dmesg output on 4.11 for reference? Peter > >Laurent Bigonville > >> >> Alexander > > >------------------------------------------------------------------------------ >Check out the vibrant tech community on one of the world's most >engaging tech sites, Slashdot.org! http://sdm.link/slashdot >_______________________________________________ >tpmdd-devel mailing list >tpmdd-devel@lists.sourceforge.net >https://lists.sourceforge.net/lists/listinfo/tpmdd-devel -- Sent from my mobile ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore [not found] ` <47c4300b-8701-79a6-1c58-3a5853f4c5e3-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 2017-08-29 17:39 ` Peter Huewe @ 2017-08-29 18:55 ` Laurent Bigonville [not found] ` <595efb25-8d87-f39d-037f-9c9a98462339-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 1 sibling, 1 reply; 23+ messages in thread From: Laurent Bigonville @ 2017-08-29 18:55 UTC (permalink / raw) To: Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Le 29/08/17 à 18:35, Laurent Bigonville a écrit : > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : >>> An idea how to troubleshoot this? >> Can you run git bisect on the changes between 4.11 and 4.12, so that >> we find the offending commit? It is probably sufficient to limit the >> search to commits that touch something in drivers/char/tpm. > > I'll try and keep you posted. OK I've been able to bisect the problem and the bad commit is: e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 Author: Jerry Snitselaar <jsnitsel@redhat.com> Date: Mon Mar 27 08:46:04 2017 -0700 tpm_tis: convert to using locality callbacks This patch converts tpm_tis to use of the new tpm class ops request_locality, and relinquish_locality. With the move to using the callbacks, release_locality is changed so that we now release the locality even if there is no request pending. This required some changes to the tpm_tis_core_init code path to make sure locality is requested when needed: - tpm2_probe code path will end up calling request/release through callbacks, so request_locality prior to tpm2_probe not needed. - probe_itpm makes calls to tpm_tis_send_data which no longer calls request_locality, so add request_locality prior to tpm_tis_send_data calls. Also drop release_locality call in middleof probe_itpm, and keep locality until release_locality called at end of probe_itpm. Cc: Peter Huewe <peterhuewe@gmx.de> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> Cc: Marcel Selhorst <tpmdd@selhorst.net> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 72f21b446e45ea1003de75902b0553deb99157fd M drivers ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <595efb25-8d87-f39d-037f-9c9a98462339-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <595efb25-8d87-f39d-037f-9c9a98462339-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-08-31 12:10 ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w [not found] ` <857106e4bb864bb8a68b1381fffc8f50-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w @ 2017-08-31 12:10 UTC (permalink / raw) To: bigon-8fiUuRrzOP0dnm+yROfE0A, jsnitsel-H+wXaHxf7aLQT0dZR+AlfA Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : > > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : > >>> An idea how to troubleshoot this? > >> Can you run git bisect on the changes between 4.11 and 4.12, so that > >> we find the offending commit? It is probably sufficient to limit the > >> search to commits that touch something in drivers/char/tpm. > > > > I'll try and keep you posted. > > OK I've been able to bisect the problem and the bad commit is: > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 > Author: Jerry Snitselaar <jsnitsel@redhat.com> > Date: Mon Mar 27 08:46:04 2017 -0700 > > tpm_tis: convert to using locality callbacks > > This patch converts tpm_tis to use of the new tpm class ops > request_locality, and relinquish_locality. > > With the move to using the callbacks, release_locality is changed so > that we now release the locality even if there is no request pending. > > This required some changes to the tpm_tis_core_init code path to > make sure locality is requested when needed: > > - tpm2_probe code path will end up calling request/release through > callbacks, so request_locality prior to tpm2_probe not needed. > > - probe_itpm makes calls to tpm_tis_send_data which no longer calls > request_locality, so add request_locality prior to tpm_tis_send_data > calls. Also drop release_locality call in middleof probe_itpm, and > keep locality until release_locality called at end of probe_itpm. > > Cc: Peter Huewe <peterhuewe@gmx.de> > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Cc: Marcel Selhorst <tpmdd@selhorst.net> > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 > 72f21b446e45ea1003de75902b0553deb99157fd M drivers > I've looked again at the code in question, but could not find anything that is obviously wrong there. Locality is now requested/released at slightly different points in the process than before, but that's it. It does not seem to cause problems with the majority of TPMs, since you are the first to report any, so maybe it is a quirk that only affects this device. Perhaps Jerry can help, since this is his change? Alexander ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <857106e4bb864bb8a68b1381fffc8f50-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <857106e4bb864bb8a68b1381fffc8f50-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org> @ 2017-08-31 16:40 ` Jerry Snitselaar 2017-09-01 12:10 ` Laurent Bigonville 0 siblings, 1 reply; 23+ messages in thread From: Jerry Snitselaar @ 2017-08-31 16:40 UTC (permalink / raw) To: Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jarkko Sakkinen On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >> Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >> > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >> >>> An idea how to troubleshoot this? >> >> Can you run git bisect on the changes between 4.11 and 4.12, so that >> >> we find the offending commit? It is probably sufficient to limit the >> >> search to commits that touch something in drivers/char/tpm. >> > >> > I'll try and keep you posted. >> >> OK I've been able to bisect the problem and the bad commit is: >> >> e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >> commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >> Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> Date: Mon Mar 27 08:46:04 2017 -0700 >> >> tpm_tis: convert to using locality callbacks >> >> This patch converts tpm_tis to use of the new tpm class ops >> request_locality, and relinquish_locality. >> >> With the move to using the callbacks, release_locality is changed so >> that we now release the locality even if there is no request pending. >> >> This required some changes to the tpm_tis_core_init code path to >> make sure locality is requested when needed: >> >> - tpm2_probe code path will end up calling request/release through >> callbacks, so request_locality prior to tpm2_probe not needed. >> >> - probe_itpm makes calls to tpm_tis_send_data which no longer calls >> request_locality, so add request_locality prior to tpm_tis_send_data >> calls. Also drop release_locality call in middleof probe_itpm, and >> keep locality until release_locality called at end of probe_itpm. >> >> Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >> Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> Cc: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> >> Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> >> Signed-off-by: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> >> :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >> 72f21b446e45ea1003de75902b0553deb99157fd M drivers >> > >I've looked again at the code in question, but could not find anything that is obviously wrong there. Locality is now requested/released at slightly different points in the process than before, but that's it. It does not seem to cause problems with the majority of TPMs, since you are the first to report any, so maybe it is a quirk that only affects this device. > >Perhaps Jerry can help, since this is his change? > >Alexander Getting some caffeine in me, and starting to take a look. Adding Jarkko as well since this might involve the general locality changes. Laurent, if I send you a patch with some debugging code added, would you be able to run it on that system? I wasn't running into issues on the system I had with a 1.2 device, but I no longer have access to it. I'll see if I can find one in our labs and reproduce it there. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore 2017-08-31 16:40 ` Jerry Snitselaar @ 2017-09-01 12:10 ` Laurent Bigonville [not found] ` <0d9be244-ace0-030d-6ff9-c4e94c63b7e9-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Laurent Bigonville @ 2017-09-01 12:10 UTC (permalink / raw) To: Jerry Snitselaar, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jarkko Sakkinen Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : > On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote: >>> Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>> > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : >>> >>> An idea how to troubleshoot this? >>> >> Can you run git bisect on the changes between 4.11 and 4.12, so that >>> >> we find the offending commit? It is probably sufficient to limit the >>> >> search to commits that touch something in drivers/char/tpm. >>> > >>> > I'll try and keep you posted. >>> >>> OK I've been able to bisect the problem and the bad commit is: >>> >>> e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >>> commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>> Author: Jerry Snitselaar <jsnitsel@redhat.com> >>> Date: Mon Mar 27 08:46:04 2017 -0700 >>> >>> tpm_tis: convert to using locality callbacks >>> >>> This patch converts tpm_tis to use of the new tpm class ops >>> request_locality, and relinquish_locality. >>> >>> With the move to using the callbacks, release_locality is >>> changed so >>> that we now release the locality even if there is no request >>> pending. >>> >>> This required some changes to the tpm_tis_core_init code path to >>> make sure locality is requested when needed: >>> >>> - tpm2_probe code path will end up calling request/release >>> through >>> callbacks, so request_locality prior to tpm2_probe not needed. >>> >>> - probe_itpm makes calls to tpm_tis_send_data which no longer >>> calls >>> request_locality, so add request_locality prior to >>> tpm_tis_send_data >>> calls. Also drop release_locality call in middleof >>> probe_itpm, and >>> keep locality until release_locality called at end of >>> probe_itpm. >>> >>> Cc: Peter Huewe <peterhuewe@gmx.de> >>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>> Cc: Marcel Selhorst <tpmdd@selhorst.net> >>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> >>> :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>> 72f21b446e45ea1003de75902b0553deb99157fd M drivers >>> >> >> I've looked again at the code in question, but could not find >> anything that is obviously wrong there. Locality is now >> requested/released at slightly different points in the process than >> before, but that's it. It does not seem to cause problems with the >> majority of TPMs, since you are the first to report any, so maybe it >> is a quirk that only affects this device. >> >> Perhaps Jerry can help, since this is his change? >> >> Alexander > > Getting some caffeine in me, and starting to take a look. Adding > Jarkko as well since this might involve the general locality changes. > > Laurent, if I send you a patch with some debugging code added, would > you be able to run it on that system? I wasn't running into issues > on the system I had with a 1.2 device, but I no longer have access > to it. I'll see if I can find one in our labs and reproduce it there. Yes I should be able to do that ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <0d9be244-ace0-030d-6ff9-c4e94c63b7e9-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <0d9be244-ace0-030d-6ff9-c4e94c63b7e9-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-09-06 4:05 ` Jarkko Sakkinen [not found] ` <20170906040555.fqedhmo5277sd6fq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Jarkko Sakkinen @ 2017-09-06 4:05 UTC (permalink / raw) To: Laurent Bigonville; +Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : > > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : > > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : > > > > >>> An idea how to troubleshoot this? > > > > >> Can you run git bisect on the changes between 4.11 and 4.12, so that > > > > >> we find the offending commit? It is probably sufficient to limit the > > > > >> search to commits that touch something in drivers/char/tpm. > > > > > > > > > > I'll try and keep you posted. > > > > > > > > OK I've been able to bisect the problem and the bad commit is: > > > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 > > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > > > Date: Mon Mar 27 08:46:04 2017 -0700 > > > > > > > > tpm_tis: convert to using locality callbacks > > > > > > > > This patch converts tpm_tis to use of the new tpm class ops > > > > request_locality, and relinquish_locality. > > > > > > > > With the move to using the callbacks, release_locality is > > > > changed so > > > > that we now release the locality even if there is no > > > > request pending. > > > > > > > > This required some changes to the tpm_tis_core_init code path to > > > > make sure locality is requested when needed: > > > > > > > > - tpm2_probe code path will end up calling > > > > request/release through > > > > callbacks, so request_locality prior to tpm2_probe not needed. > > > > > > > > - probe_itpm makes calls to tpm_tis_send_data which no > > > > longer calls > > > > request_locality, so add request_locality prior to > > > > tpm_tis_send_data > > > > calls. Also drop release_locality call in middleof > > > > probe_itpm, and > > > > keep locality until release_locality called at end of > > > > probe_itpm. > > > > > > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > Cc: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> > > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> > > > > Signed-off-by: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1560@public.gmane.orgtel.com> > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563N0uC3ymp8PA@public.gmane.orgl.com> > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers > > > > > > > > > > I've looked again at the code in question, but could not find > > > anything that is obviously wrong there. Locality is now > > > requested/released at slightly different points in the process than > > > before, but that's it. It does not seem to cause problems with the > > > majority of TPMs, since you are the first to report any, so maybe it > > > is a quirk that only affects this device. > > > > > > Perhaps Jerry can help, since this is his change? > > > > > > Alexander > > > > Getting some caffeine in me, and starting to take a look. Adding > > Jarkko as well since this might involve the general locality changes. > > > > Laurent, if I send you a patch with some debugging code added, would > > you be able to run it on that system? I wasn't running into issues > > on the system I had with a 1.2 device, but I no longer have access > > to it. I'll see if I can find one in our labs and reproduce it there. > > Yes I should be able to do that Any findings? /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <20170906040555.fqedhmo5277sd6fq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <20170906040555.fqedhmo5277sd6fq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-10-14 8:13 ` Jerry Snitselaar 2017-10-21 8:53 ` Laurent Bigonville 0 siblings, 1 reply; 23+ messages in thread From: Jerry Snitselaar @ 2017-10-14 8:13 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Wed Sep 06 17, Jarkko Sakkinen wrote: >On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >> Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >> > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >> > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >> > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >> > > > >>> An idea how to troubleshoot this? >> > > > >> Can you run git bisect on the changes between 4.11 and 4.12, so that >> > > > >> we find the offending commit? It is probably sufficient to limit the >> > > > >> search to commits that touch something in drivers/char/tpm. >> > > > > >> > > > > I'll try and keep you posted. >> > > > >> > > > OK I've been able to bisect the problem and the bad commit is: >> > > > >> > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >> > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >> > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> > > > Date: Mon Mar 27 08:46:04 2017 -0700 >> > > > >> > > > tpm_tis: convert to using locality callbacks >> > > > >> > > > This patch converts tpm_tis to use of the new tpm class ops >> > > > request_locality, and relinquish_locality. >> > > > >> > > > With the move to using the callbacks, release_locality is >> > > > changed so >> > > > that we now release the locality even if there is no >> > > > request pending. >> > > > >> > > > This required some changes to the tpm_tis_core_init code path to >> > > > make sure locality is requested when needed: >> > > > >> > > > - tpm2_probe code path will end up calling >> > > > request/release through >> > > > callbacks, so request_locality prior to tpm2_probe not needed. >> > > > >> > > > - probe_itpm makes calls to tpm_tis_send_data which no >> > > > longer calls >> > > > request_locality, so add request_locality prior to >> > > > tpm_tis_send_data >> > > > calls. Also drop release_locality call in middleof >> > > > probe_itpm, and >> > > > keep locality until release_locality called at end of >> > > > probe_itpm. >> > > > >> > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >> > > > Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> > > > Cc: Jason Gunthorpe <jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org> >> > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> >> > > > Signed-off-by: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1561eoWH0uzbU5w@public.gmane.orgel.com> >> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> > > > >> > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >> > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers >> > > > >> > > >> > > I've looked again at the code in question, but could not find >> > > anything that is obviously wrong there. Locality is now >> > > requested/released at slightly different points in the process than >> > > before, but that's it. It does not seem to cause problems with the >> > > majority of TPMs, since you are the first to report any, so maybe it >> > > is a quirk that only affects this device. >> > > >> > > Perhaps Jerry can help, since this is his change? >> > > >> > > Alexander >> > >> > Getting some caffeine in me, and starting to take a look. Adding >> > Jarkko as well since this might involve the general locality changes. >> > >> > Laurent, if I send you a patch with some debugging code added, would >> > you be able to run it on that system? I wasn't running into issues >> > on the system I had with a 1.2 device, but I no longer have access >> > to it. I'll see if I can find one in our labs and reproduce it there. >> >> Yes I should be able to do that > >Any findings? > >/Jarkko Okay, finally getting back to this. Looking at the code it isn't clear to me why the change is causing this. So while I stare at this some more Laurent could you reproduce it with this patch so I can see what the status and access registers look like? Does anyone else on here happen to have a Sinosun tpm device? The systems I have access to with TPM1.2 devices don't have this issue. --8<-- diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c index fdde971bc810..7d60a7e4b50a 100644 --- a/drivers/char/tpm/tpm_tis_core.c +++ b/drivers/char/tpm/tpm_tis_core.c @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) int rc, status, burstcnt; size_t count = 0; bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; + u8 access; status = tpm_tis_status(chip); if ((status & TPM_STS_COMMAND_READY) == 0) { @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) } status = tpm_tis_status(chip); if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); + if (rc < 0) + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read failure TPM_ACCESS(%d)\n", priv->locality); + else + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: locality: %d status: %x access: %x\n", priv->locality, status, access); rc = -EIO; goto out_err; } @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len) } status = tpm_tis_status(chip); if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); + if (rc < 0) + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read failure TPM_ACCESS(%d)\n", priv->locality); + else + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: %d status: %x access: %x\n", priv->locality, status, access); rc = -EIO; goto out_err; } -- 2.15.0.rc0 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore 2017-10-14 8:13 ` Jerry Snitselaar @ 2017-10-21 8:53 ` Laurent Bigonville [not found] ` <b63b765d-2477-d9fe-9d80-c2ea7e582bce-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Laurent Bigonville @ 2017-10-21 8:53 UTC (permalink / raw) To: Jerry Snitselaar, Jarkko Sakkinen Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f [-- Attachment #1: Type: text/plain, Size: 7032 bytes --] Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : > On Wed Sep 06 17, Jarkko Sakkinen wrote: >> On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >>> Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>> > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >>> > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>> > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >>> > > > >>> An idea how to troubleshoot this? >>> > > > >> Can you run git bisect on the changes between 4.11 and >>> 4.12, so that >>> > > > >> we find the offending commit? It is probably sufficient to >>> limit the >>> > > > >> search to commits that touch something in drivers/char/tpm. >>> > > > > >>> > > > > I'll try and keep you posted. >>> > > > >>> > > > OK I've been able to bisect the problem and the bad commit is: >>> > > > >>> > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >>> > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>> > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>> > > > Date: Mon Mar 27 08:46:04 2017 -0700 >>> > > > >>> > > > tpm_tis: convert to using locality callbacks >>> > > > >>> > > > This patch converts tpm_tis to use of the new tpm class ops >>> > > > request_locality, and relinquish_locality. >>> > > > >>> > > > With the move to using the callbacks, release_locality is >>> > > > changed so >>> > > > that we now release the locality even if there is no >>> > > > request pending. >>> > > > >>> > > > This required some changes to the tpm_tis_core_init code >>> path to >>> > > > make sure locality is requested when needed: >>> > > > >>> > > > - tpm2_probe code path will end up calling >>> > > > request/release through >>> > > > callbacks, so request_locality prior to tpm2_probe >>> not needed. >>> > > > >>> > > > - probe_itpm makes calls to tpm_tis_send_data which no >>> > > > longer calls >>> > > > request_locality, so add request_locality prior to >>> > > > tpm_tis_send_data >>> > > > calls. Also drop release_locality call in middleof >>> > > > probe_itpm, and >>> > > > keep locality until release_locality called at end of >>> > > > probe_itpm. >>> > > > >>> > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >>> > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>> > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONeeDGRHsOpWV0g@public.gmane.orgt> >>> > > > Signed-off-by: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>> > > > Reviewed-by: Jarkko Sakkinen >>> <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>> > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>> > > > Signed-off-by: Jarkko Sakkinen >>> <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>> > > > >>> > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>> > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers >>> > > > >>> > > >>> > > I've looked again at the code in question, but could not find >>> > > anything that is obviously wrong there. Locality is now >>> > > requested/released at slightly different points in the process than >>> > > before, but that's it. It does not seem to cause problems with the >>> > > majority of TPMs, since you are the first to report any, so >>> maybe it >>> > > is a quirk that only affects this device. >>> > > >>> > > Perhaps Jerry can help, since this is his change? >>> > > >>> > > Alexander >>> > >>> > Getting some caffeine in me, and starting to take a look. Adding >>> > Jarkko as well since this might involve the general locality changes. >>> > >>> > Laurent, if I send you a patch with some debugging code added, would >>> > you be able to run it on that system? I wasn't running into issues >>> > on the system I had with a 1.2 device, but I no longer have access >>> > to it. I'll see if I can find one in our labs and reproduce it there. >>> >>> Yes I should be able to do that >> >> Any findings? >> >> /Jarkko > > Okay, finally getting back to this. Looking at the code it isn't clear > to me > why the change is causing this. So while I stare at this some more > Laurent > could you reproduce it with this patch so I can see what the status and > access registers look like? Does anyone else on here happen to have a > Sinosun > tpm device? The systems I have access to with TPM1.2 devices don't > have this > issue. > > --8<-- > > diff --git a/drivers/char/tpm/tpm_tis_core.c > b/drivers/char/tpm/tpm_tis_core.c > index fdde971bc810..7d60a7e4b50a 100644 > --- a/drivers/char/tpm/tpm_tis_core.c > +++ b/drivers/char/tpm/tpm_tis_core.c > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip > *chip, const u8 *buf, size_t len) > int rc, status, burstcnt; > size_t count = 0; > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; > + u8 access; > > status = tpm_tis_status(chip); > if ((status & TPM_STS_COMMAND_READY) == 0) { > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip > *chip, const u8 *buf, size_t len) > } > status = tpm_tis_status(chip); > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), > &access); > + if (rc < 0) > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read > failure TPM_ACCESS(%d)\n", priv->locality); > + else > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: > locality: %d status: %x access: %x\n", priv->locality, status, access); > rc = -EIO; > goto out_err; > } > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip > *chip, const u8 *buf, size_t len) > } > status = tpm_tis_status(chip); > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); > + if (rc < 0) > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read > failure TPM_ACCESS(%d)\n", priv->locality); > + else > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: > %d status: %x access: %x\n", priv->locality, status, access); > rc = -EIO; > goto out_err; > } Please find here the dmesg output of the patched kernel [-- Attachment #2: dmesg.txt.gz --] [-- Type: application/gzip, Size: 20410 bytes --] [-- Attachment #3: Type: text/plain, Size: 202 bytes --] ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot [-- Attachment #4: Type: text/plain, Size: 192 bytes --] _______________________________________________ tpmdd-devel mailing list tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <b63b765d-2477-d9fe-9d80-c2ea7e582bce-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <b63b765d-2477-d9fe-9d80-c2ea7e582bce-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-10-23 13:23 ` Jarkko Sakkinen [not found] ` <20171023132346.jbqgokwv3ah2oqjo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Jarkko Sakkinen @ 2017-10-23 13:23 UTC (permalink / raw) To: Laurent Bigonville Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: > Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : > > On Wed Sep 06 17, Jarkko Sakkinen wrote: > > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: > > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : > > > > > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: > > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : > > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : > > > > > > > >>> An idea how to troubleshoot this? > > > > > > > >> Can you run git bisect on the changes between 4.11 and > > > > 4.12, so that > > > > > > > >> we find the offending commit? It is probably sufficient > > > > to limit the > > > > > > > >> search to commits that touch something in drivers/char/tpm. > > > > > > > > > > > > > > > > I'll try and keep you posted. > > > > > > > > > > > > > > OK I've been able to bisect the problem and the bad commit is: > > > > > > > > > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit > > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 > > > > > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > > > > > > Date: Mon Mar 27 08:46:04 2017 -0700 > > > > > > > > > > > > > > tpm_tis: convert to using locality callbacks > > > > > > > > > > > > > > This patch converts tpm_tis to use of the new tpm class ops > > > > > > > request_locality, and relinquish_locality. > > > > > > > > > > > > > > With the move to using the callbacks, release_locality is > > > > > > > changed so > > > > > > > that we now release the locality even if there is no > > > > > > > request pending. > > > > > > > > > > > > > > This required some changes to the tpm_tis_core_init > > > > code path to > > > > > > > make sure locality is requested when needed: > > > > > > > > > > > > > > - tpm2_probe code path will end up calling > > > > > > > request/release through > > > > > > > callbacks, so request_locality prior to > > > > tpm2_probe not needed. > > > > > > > > > > > > > > - probe_itpm makes calls to tpm_tis_send_data which no > > > > > > > longer calls > > > > > > > request_locality, so add request_locality prior to > > > > > > > tpm_tis_send_data > > > > > > > calls. Also drop release_locality call in middleof > > > > > > > probe_itpm, and > > > > > > > keep locality until release_locality called at end of > > > > > > > probe_itpm. > > > > > > > > > > > > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1560/9W2qTSuDoA@public.gmane.org.com> > > > > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> > > > > > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> > > > > > > > Reviewed-by: Jarkko Sakkinen > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > Signed-off-by: Jarkko Sakkinen > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 > > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers > > > > > > > > > > > > > > > > > > > I've looked again at the code in question, but could not find > > > > > > anything that is obviously wrong there. Locality is now > > > > > > requested/released at slightly different points in the process than > > > > > > before, but that's it. It does not seem to cause problems with the > > > > > > majority of TPMs, since you are the first to report any, so > > > > maybe it > > > > > > is a quirk that only affects this device. > > > > > > > > > > > > Perhaps Jerry can help, since this is his change? > > > > > > > > > > > > Alexander > > > > > > > > > > Getting some caffeine in me, and starting to take a look. Adding > > > > > Jarkko as well since this might involve the general locality changes. > > > > > > > > > > Laurent, if I send you a patch with some debugging code added, would > > > > > you be able to run it on that system? I wasn't running into issues > > > > > on the system I had with a 1.2 device, but I no longer have access > > > > > to it. I'll see if I can find one in our labs and reproduce it there. > > > > > > > > Yes I should be able to do that > > > > > > Any findings? > > > > > > /Jarkko > > > > Okay, finally getting back to this. Looking at the code it isn't clear > > to me > > why the change is causing this. So while I stare at this some more > > Laurent > > could you reproduce it with this patch so I can see what the status and > > access registers look like? Does anyone else on here happen to have a > > Sinosun > > tpm device? The systems I have access to with TPM1.2 devices don't have > > this > > issue. > > > > --8<-- > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c > > b/drivers/char/tpm/tpm_tis_core.c > > index fdde971bc810..7d60a7e4b50a 100644 > > --- a/drivers/char/tpm/tpm_tis_core.c > > +++ b/drivers/char/tpm/tpm_tis_core.c > > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > const u8 *buf, size_t len) > > int rc, status, burstcnt; > > size_t count = 0; > > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; > > + u8 access; > > > > status = tpm_tis_status(chip); > > if ((status & TPM_STS_COMMAND_READY) == 0) { > > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > const u8 *buf, size_t len) > > } > > status = tpm_tis_status(chip); > > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), > > &access); > > + if (rc < 0) > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read > > failure TPM_ACCESS(%d)\n", priv->locality); > > + else > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: > > locality: %d status: %x access: %x\n", priv->locality, status, access); > > rc = -EIO; > > goto out_err; > > } > > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > const u8 *buf, size_t len) > > } > > status = tpm_tis_status(chip); > > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); > > + if (rc < 0) > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read > > failure TPM_ACCESS(%d)\n", priv->locality); > > + else > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: > > %d status: %x access: %x\n", priv->locality, status, access); > > rc = -EIO; > > goto out_err; > > } > > Please find here the dmesg output of the patched kernel At least 0xff is corrupted value in senseful way. CPU fills the read with ones for example for unaligned bus read. See table 19 in PC client spec. This can happen when you do unaligned read for example. Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but would probably make sense to compare that 0x81 to table 18 in the same spec. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <20171023132346.jbqgokwv3ah2oqjo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <20171023132346.jbqgokwv3ah2oqjo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-10-23 13:45 ` Jerry Snitselaar 2017-10-23 13:48 ` Laurent Bigonville 2017-10-24 13:51 ` Jarkko Sakkinen 0 siblings, 2 replies; 23+ messages in thread From: Jerry Snitselaar @ 2017-10-23 13:45 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Mon Oct 23 17, Jarkko Sakkinen wrote: >On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >> Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >> > On Wed Sep 06 17, Jarkko Sakkinen wrote: >> > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >> > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >> > > > > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >> > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >> > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >> > > > > > > >>> An idea how to troubleshoot this? >> > > > > > > >> Can you run git bisect on the changes between 4.11 and >> > > > 4.12, so that >> > > > > > > >> we find the offending commit? It is probably sufficient >> > > > to limit the >> > > > > > > >> search to commits that touch something in drivers/char/tpm. >> > > > > > > > >> > > > > > > > I'll try and keep you posted. >> > > > > > > >> > > > > > > OK I've been able to bisect the problem and the bad commit is: >> > > > > > > >> > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >> > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >> > > > > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> > > > > > > Date: Mon Mar 27 08:46:04 2017 -0700 >> > > > > > > >> > > > > > > tpm_tis: convert to using locality callbacks >> > > > > > > >> > > > > > > This patch converts tpm_tis to use of the new tpm class ops >> > > > > > > request_locality, and relinquish_locality. >> > > > > > > >> > > > > > > With the move to using the callbacks, release_locality is >> > > > > > > changed so >> > > > > > > that we now release the locality even if there is no >> > > > > > > request pending. >> > > > > > > >> > > > > > > This required some changes to the tpm_tis_core_init >> > > > code path to >> > > > > > > make sure locality is requested when needed: >> > > > > > > >> > > > > > > - tpm2_probe code path will end up calling >> > > > > > > request/release through >> > > > > > > callbacks, so request_locality prior to >> > > > tpm2_probe not needed. >> > > > > > > >> > > > > > > - probe_itpm makes calls to tpm_tis_send_data which no >> > > > > > > longer calls >> > > > > > > request_locality, so add request_locality prior to >> > > > > > > tpm_tis_send_data >> > > > > > > calls. Also drop release_locality call in middleof >> > > > > > > probe_itpm, and >> > > > > > > keep locality until release_locality called at end of >> > > > > > > probe_itpm. >> > > > > > > >> > > > > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >> > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563N0uC3ymp8PA@public.gmane.orgl.com> >> > > > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> > > > > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> >> > > > > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >> > > > > > > Reviewed-by: Jarkko Sakkinen >> > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> > > > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> > > > > > > Signed-off-by: Jarkko Sakkinen >> > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> > > > > > > >> > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >> > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers >> > > > > > > >> > > > > > >> > > > > > I've looked again at the code in question, but could not find >> > > > > > anything that is obviously wrong there. Locality is now >> > > > > > requested/released at slightly different points in the process than >> > > > > > before, but that's it. It does not seem to cause problems with the >> > > > > > majority of TPMs, since you are the first to report any, so >> > > > maybe it >> > > > > > is a quirk that only affects this device. >> > > > > > >> > > > > > Perhaps Jerry can help, since this is his change? >> > > > > > >> > > > > > Alexander >> > > > > >> > > > > Getting some caffeine in me, and starting to take a look. Adding >> > > > > Jarkko as well since this might involve the general locality changes. >> > > > > >> > > > > Laurent, if I send you a patch with some debugging code added, would >> > > > > you be able to run it on that system? I wasn't running into issues >> > > > > on the system I had with a 1.2 device, but I no longer have access >> > > > > to it. I'll see if I can find one in our labs and reproduce it there. >> > > > >> > > > Yes I should be able to do that >> > > >> > > Any findings? >> > > >> > > /Jarkko >> > >> > Okay, finally getting back to this. Looking at the code it isn't clear >> > to me >> > why the change is causing this. So while I stare at this some more >> > Laurent >> > could you reproduce it with this patch so I can see what the status and >> > access registers look like? Does anyone else on here happen to have a >> > Sinosun >> > tpm device? The systems I have access to with TPM1.2 devices don't have >> > this >> > issue. >> > >> > --8<-- >> > >> > diff --git a/drivers/char/tpm/tpm_tis_core.c >> > b/drivers/char/tpm/tpm_tis_core.c >> > index fdde971bc810..7d60a7e4b50a 100644 >> > --- a/drivers/char/tpm/tpm_tis_core.c >> > +++ b/drivers/char/tpm/tpm_tis_core.c >> > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >> > const u8 *buf, size_t len) >> > int rc, status, burstcnt; >> > size_t count = 0; >> > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >> > + u8 access; >> > >> > status = tpm_tis_status(chip); >> > if ((status & TPM_STS_COMMAND_READY) == 0) { >> > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >> > const u8 *buf, size_t len) >> > } >> > status = tpm_tis_status(chip); >> > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >> > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >> > &access); >> > + if (rc < 0) >> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read >> > failure TPM_ACCESS(%d)\n", priv->locality); >> > + else >> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >> > locality: %d status: %x access: %x\n", priv->locality, status, access); >> > rc = -EIO; >> > goto out_err; >> > } >> > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >> > const u8 *buf, size_t len) >> > } >> > status = tpm_tis_status(chip); >> > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >> > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); >> > + if (rc < 0) >> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >> > failure TPM_ACCESS(%d)\n", priv->locality); >> > + else >> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: >> > %d status: %x access: %x\n", priv->locality, status, access); >> > rc = -EIO; >> > goto out_err; >> > } >> >> Please find here the dmesg output of the patched kernel > >At least 0xff is corrupted value in senseful way. CPU fills the read >with ones for example for unaligned bus read. See table 19 in PC client >spec. This can happen when you do unaligned read for example. > >Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but >would probably make sense to compare that 0x81 to table 18 in the same >spec. > >/Jarkko 0x81 is saying the access register status is valid, and the locality is not active. That first bit means A Dynamic OS has not been previously established on the platform. Normally we would see 0xa1, which would mean valid register status, and the locality is active. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore 2017-10-23 13:45 ` Jerry Snitselaar @ 2017-10-23 13:48 ` Laurent Bigonville 2017-10-24 13:51 ` Jarkko Sakkinen 1 sibling, 0 replies; 23+ messages in thread From: Laurent Bigonville @ 2017-10-23 13:48 UTC (permalink / raw) To: Jerry Snitselaar, Jarkko Sakkinen Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Le 23/10/17 à 15:45, Jerry Snitselaar a écrit : > On Mon Oct 23 17, Jarkko Sakkinen wrote: >> On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >>> Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >>> > On Wed Sep 06 17, Jarkko Sakkinen wrote: >>> > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >>> > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>> > > > > On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote: >>> > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>> > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a >>> écrit : >>> > > > > > > >>> An idea how to troubleshoot this? >>> > > > > > > >> Can you run git bisect on the changes between 4.11 and >>> > > > 4.12, so that >>> > > > > > > >> we find the offending commit? It is probably sufficient >>> > > > to limit the >>> > > > > > > >> search to commits that touch something in >>> drivers/char/tpm. >>> > > > > > > > >>> > > > > > > > I'll try and keep you posted. >>> > > > > > > >>> > > > > > > OK I've been able to bisect the problem and the bad >>> commit is: >>> > > > > > > >>> > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first >>> bad commit >>> > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>> > > > > > > Author: Jerry Snitselaar <jsnitsel@redhat.com> >>> > > > > > > Date: Mon Mar 27 08:46:04 2017 -0700 >>> > > > > > > >>> > > > > > > tpm_tis: convert to using locality callbacks >>> > > > > > > >>> > > > > > > This patch converts tpm_tis to use of the new tpm >>> class ops >>> > > > > > > request_locality, and relinquish_locality. >>> > > > > > > >>> > > > > > > With the move to using the callbacks, >>> release_locality is >>> > > > > > > changed so >>> > > > > > > that we now release the locality even if there is no >>> > > > > > > request pending. >>> > > > > > > >>> > > > > > > This required some changes to the tpm_tis_core_init >>> > > > code path to >>> > > > > > > make sure locality is requested when needed: >>> > > > > > > >>> > > > > > > - tpm2_probe code path will end up calling >>> > > > > > > request/release through >>> > > > > > > callbacks, so request_locality prior to >>> > > > tpm2_probe not needed. >>> > > > > > > >>> > > > > > > - probe_itpm makes calls to tpm_tis_send_data >>> which no >>> > > > > > > longer calls >>> > > > > > > request_locality, so add request_locality prior to >>> > > > > > > tpm_tis_send_data >>> > > > > > > calls. Also drop release_locality call in middleof >>> > > > > > > probe_itpm, and >>> > > > > > > keep locality until release_locality called at >>> end of >>> > > > > > > probe_itpm. >>> > > > > > > >>> > > > > > > Cc: Peter Huewe <peterhuewe@gmx.de> >>> > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>> > > > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>> > > > > > > Cc: Marcel Selhorst <tpmdd@selhorst.net> >>> > > > > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>> > > > > > > Reviewed-by: Jarkko Sakkinen >>> > > > <jarkko.sakkinen@linux.intel.com> >>> > > > > > > Tested-by: Jarkko Sakkinen >>> <jarkko.sakkinen@linux.intel.com> >>> > > > > > > Signed-off-by: Jarkko Sakkinen >>> > > > <jarkko.sakkinen@linux.intel.com> >>> > > > > > > >>> > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>> > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers >>> > > > > > > >>> > > > > > >>> > > > > > I've looked again at the code in question, but could not find >>> > > > > > anything that is obviously wrong there. Locality is now >>> > > > > > requested/released at slightly different points in the >>> process than >>> > > > > > before, but that's it. It does not seem to cause problems >>> with the >>> > > > > > majority of TPMs, since you are the first to report any, so >>> > > > maybe it >>> > > > > > is a quirk that only affects this device. >>> > > > > > >>> > > > > > Perhaps Jerry can help, since this is his change? >>> > > > > > >>> > > > > > Alexander >>> > > > > >>> > > > > Getting some caffeine in me, and starting to take a look. >>> Adding >>> > > > > Jarkko as well since this might involve the general locality >>> changes. >>> > > > > >>> > > > > Laurent, if I send you a patch with some debugging code >>> added, would >>> > > > > you be able to run it on that system? I wasn't running into >>> issues >>> > > > > on the system I had with a 1.2 device, but I no longer have >>> access >>> > > > > to it. I'll see if I can find one in our labs and reproduce >>> it there. >>> > > > >>> > > > Yes I should be able to do that >>> > > >>> > > Any findings? >>> > > >>> > > /Jarkko >>> > >>> > Okay, finally getting back to this. Looking at the code it isn't >>> clear >>> > to me >>> > why the change is causing this. So while I stare at this some more >>> > Laurent >>> > could you reproduce it with this patch so I can see what the >>> status and >>> > access registers look like? Does anyone else on here happen to have a >>> > Sinosun >>> > tpm device? The systems I have access to with TPM1.2 devices don't >>> have >>> > this >>> > issue. >>> > >>> > --8<-- >>> > >>> > diff --git a/drivers/char/tpm/tpm_tis_core.c >>> > b/drivers/char/tpm/tpm_tis_core.c >>> > index fdde971bc810..7d60a7e4b50a 100644 >>> > --- a/drivers/char/tpm/tpm_tis_core.c >>> > +++ b/drivers/char/tpm/tpm_tis_core.c >>> > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip >>> *chip, >>> > const u8 *buf, size_t len) >>> > int rc, status, burstcnt; >>> > size_t count = 0; >>> > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >>> > + u8 access; >>> > >>> > status = tpm_tis_status(chip); >>> > if ((status & TPM_STS_COMMAND_READY) == 0) { >>> > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip >>> *chip, >>> > const u8 *buf, size_t len) >>> > } >>> > status = tpm_tis_status(chip); >>> > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >>> > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>> > &access); >>> > + if (rc < 0) >>> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read >>> > failure TPM_ACCESS(%d)\n", priv->locality); >>> > + else >>> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >>> > locality: %d status: %x access: %x\n", priv->locality, status, >>> access); >>> > rc = -EIO; >>> > goto out_err; >>> > } >>> > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip >>> *chip, >>> > const u8 *buf, size_t len) >>> > } >>> > status = tpm_tis_status(chip); >>> > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >>> > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>> &access); >>> > + if (rc < 0) >>> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >>> > failure TPM_ACCESS(%d)\n", priv->locality); >>> > + else >>> > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: >>> locality: >>> > %d status: %x access: %x\n", priv->locality, status, access); >>> > rc = -EIO; >>> > goto out_err; >>> > } >>> >>> Please find here the dmesg output of the patched kernel >> >> At least 0xff is corrupted value in senseful way. CPU fills the read >> with ones for example for unaligned bus read. See table 19 in PC client >> spec. This can happen when you do unaligned read for example. >> >> Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but >> would probably make sense to compare that 0x81 to table 18 in the same >> spec. >> >> /Jarkko > > 0x81 is saying the access register status is valid, and the locality > is not active. That first bit means A Dynamic OS has not been previously > established on the platform. Normally we would see 0xa1, which would > mean valid register status, and the locality is active. FTR, the ownership has been claimed using tpm-tools on linux, but I configured windows 10 (dual-boot) on the machine to do its magic to recognize the tpm chip by entering the owner password manually there, not sure if that matters. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore 2017-10-23 13:45 ` Jerry Snitselaar 2017-10-23 13:48 ` Laurent Bigonville @ 2017-10-24 13:51 ` Jarkko Sakkinen [not found] ` <20171024135123.uqail7olnespun4k-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 1 sibling, 1 reply; 23+ messages in thread From: Jarkko Sakkinen @ 2017-10-24 13:51 UTC (permalink / raw) To: Jerry Snitselaar Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: > On Mon Oct 23 17, Jarkko Sakkinen wrote: > > On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: > > > Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : > > > > On Wed Sep 06 17, Jarkko Sakkinen wrote: > > > > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: > > > > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : > > > > > > > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: > > > > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : > > > > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : > > > > > > > > > >>> An idea how to troubleshoot this? > > > > > > > > > >> Can you run git bisect on the changes between 4.11 and > > > > > > 4.12, so that > > > > > > > > > >> we find the offending commit? It is probably sufficient > > > > > > to limit the > > > > > > > > > >> search to commits that touch something in drivers/char/tpm. > > > > > > > > > > > > > > > > > > > > I'll try and keep you posted. > > > > > > > > > > > > > > > > > > OK I've been able to bisect the problem and the bad commit is: > > > > > > > > > > > > > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit > > > > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 > > > > > > > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > > > > > > > > Date: Mon Mar 27 08:46:04 2017 -0700 > > > > > > > > > > > > > > > > > > tpm_tis: convert to using locality callbacks > > > > > > > > > > > > > > > > > > This patch converts tpm_tis to use of the new tpm class ops > > > > > > > > > request_locality, and relinquish_locality. > > > > > > > > > > > > > > > > > > With the move to using the callbacks, release_locality is > > > > > > > > > changed so > > > > > > > > > that we now release the locality even if there is no > > > > > > > > > request pending. > > > > > > > > > > > > > > > > > > This required some changes to the tpm_tis_core_init > > > > > > code path to > > > > > > > > > make sure locality is requested when needed: > > > > > > > > > > > > > > > > > > - tpm2_probe code path will end up calling > > > > > > > > > request/release through > > > > > > > > > callbacks, so request_locality prior to > > > > > > tpm2_probe not needed. > > > > > > > > > > > > > > > > > > - probe_itpm makes calls to tpm_tis_send_data which no > > > > > > > > > longer calls > > > > > > > > > request_locality, so add request_locality prior to > > > > > > > > > tpm_tis_send_data > > > > > > > > > calls. Also drop release_locality call in middleof > > > > > > > > > probe_itpm, and > > > > > > > > > keep locality until release_locality called at end of > > > > > > > > > probe_itpm. > > > > > > > > > > > > > > > > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> > > > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > > > > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> > > > > > > > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> > > > > > > > > > Reviewed-by: Jarkko Sakkinen > > > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > > Signed-off-by: Jarkko Sakkinen > > > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > > > > > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 > > > > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers > > > > > > > > > > > > > > > > > > > > > > > > > I've looked again at the code in question, but could not find > > > > > > > > anything that is obviously wrong there. Locality is now > > > > > > > > requested/released at slightly different points in the process than > > > > > > > > before, but that's it. It does not seem to cause problems with the > > > > > > > > majority of TPMs, since you are the first to report any, so > > > > > > maybe it > > > > > > > > is a quirk that only affects this device. > > > > > > > > > > > > > > > > Perhaps Jerry can help, since this is his change? > > > > > > > > > > > > > > > > Alexander > > > > > > > > > > > > > > Getting some caffeine in me, and starting to take a look. Adding > > > > > > > Jarkko as well since this might involve the general locality changes. > > > > > > > > > > > > > > Laurent, if I send you a patch with some debugging code added, would > > > > > > > you be able to run it on that system? I wasn't running into issues > > > > > > > on the system I had with a 1.2 device, but I no longer have access > > > > > > > to it. I'll see if I can find one in our labs and reproduce it there. > > > > > > > > > > > > Yes I should be able to do that > > > > > > > > > > Any findings? > > > > > > > > > > /Jarkko > > > > > > > > Okay, finally getting back to this. Looking at the code it isn't clear > > > > to me > > > > why the change is causing this. So while I stare at this some more > > > > Laurent > > > > could you reproduce it with this patch so I can see what the status and > > > > access registers look like? Does anyone else on here happen to have a > > > > Sinosun > > > > tpm device? The systems I have access to with TPM1.2 devices don't have > > > > this > > > > issue. > > > > > > > > --8<-- > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c > > > > b/drivers/char/tpm/tpm_tis_core.c > > > > index fdde971bc810..7d60a7e4b50a 100644 > > > > --- a/drivers/char/tpm/tpm_tis_core.c > > > > +++ b/drivers/char/tpm/tpm_tis_core.c > > > > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > > > const u8 *buf, size_t len) > > > > int rc, status, burstcnt; > > > > size_t count = 0; > > > > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; > > > > + u8 access; > > > > > > > > status = tpm_tis_status(chip); > > > > if ((status & TPM_STS_COMMAND_READY) == 0) { > > > > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > > > const u8 *buf, size_t len) > > > > } > > > > status = tpm_tis_status(chip); > > > > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { > > > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), > > > > &access); > > > > + if (rc < 0) > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read > > > > failure TPM_ACCESS(%d)\n", priv->locality); > > > > + else > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: > > > > locality: %d status: %x access: %x\n", priv->locality, status, access); > > > > rc = -EIO; > > > > goto out_err; > > > > } > > > > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > > > const u8 *buf, size_t len) > > > > } > > > > status = tpm_tis_status(chip); > > > > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { > > > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); > > > > + if (rc < 0) > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read > > > > failure TPM_ACCESS(%d)\n", priv->locality); > > > > + else > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: > > > > %d status: %x access: %x\n", priv->locality, status, access); > > > > rc = -EIO; > > > > goto out_err; > > > > } > > > > > > Please find here the dmesg output of the patched kernel > > > > At least 0xff is corrupted value in senseful way. CPU fills the read > > with ones for example for unaligned bus read. See table 19 in PC client > > spec. This can happen when you do unaligned read for example. > > > > Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but > > would probably make sense to compare that 0x81 to table 18 in the same > > spec. > > > > /Jarkko > > 0x81 is saying the access register status is valid, and the locality > is not active. That first bit means A Dynamic OS has not been previously > established on the platform. Normally we would see 0xa1, which would > mean valid register status, and the locality is active. I think the important thing to note here is that STS has bits set that should never be set. So we can conclude that TPM might be either 1. Powered off 2. In some transition state? /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <20171024135123.uqail7olnespun4k-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <20171024135123.uqail7olnespun4k-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-10-24 14:57 ` Jerry Snitselaar 2017-10-24 16:07 ` Jarkko Sakkinen 2017-10-25 8:04 ` Laurent Bigonville 1 sibling, 1 reply; 23+ messages in thread From: Jerry Snitselaar @ 2017-10-24 14:57 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Tue Oct 24 17, Jarkko Sakkinen wrote: >On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: >> On Mon Oct 23 17, Jarkko Sakkinen wrote: >> > On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >> > > Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >> > > > On Wed Sep 06 17, Jarkko Sakkinen wrote: >> > > > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >> > > > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >> > > > > > > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >> > > > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >> > > > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >> > > > > > > > > >>> An idea how to troubleshoot this? >> > > > > > > > > >> Can you run git bisect on the changes between 4.11 and >> > > > > > 4.12, so that >> > > > > > > > > >> we find the offending commit? It is probably sufficient >> > > > > > to limit the >> > > > > > > > > >> search to commits that touch something in drivers/char/tpm. >> > > > > > > > > > >> > > > > > > > > > I'll try and keep you posted. >> > > > > > > > > >> > > > > > > > > OK I've been able to bisect the problem and the bad commit is: >> > > > > > > > > >> > > > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >> > > > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >> > > > > > > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> > > > > > > > > Date: Mon Mar 27 08:46:04 2017 -0700 >> > > > > > > > > >> > > > > > > > > tpm_tis: convert to using locality callbacks >> > > > > > > > > >> > > > > > > > > This patch converts tpm_tis to use of the new tpm class ops >> > > > > > > > > request_locality, and relinquish_locality. >> > > > > > > > > >> > > > > > > > > With the move to using the callbacks, release_locality is >> > > > > > > > > changed so >> > > > > > > > > that we now release the locality even if there is no >> > > > > > > > > request pending. >> > > > > > > > > >> > > > > > > > > This required some changes to the tpm_tis_core_init >> > > > > > code path to >> > > > > > > > > make sure locality is requested when needed: >> > > > > > > > > >> > > > > > > > > - tpm2_probe code path will end up calling >> > > > > > > > > request/release through >> > > > > > > > > callbacks, so request_locality prior to >> > > > > > tpm2_probe not needed. >> > > > > > > > > >> > > > > > > > > - probe_itpm makes calls to tpm_tis_send_data which no >> > > > > > > > > longer calls >> > > > > > > > > request_locality, so add request_locality prior to >> > > > > > > > > tpm_tis_send_data >> > > > > > > > > calls. Also drop release_locality call in middleof >> > > > > > > > > probe_itpm, and >> > > > > > > > > keep locality until release_locality called at end of >> > > > > > > > > probe_itpm. >> > > > > > > > > >> > > > > > > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >> > > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> > > > > > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >> > > > > > > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> >> > > > > > > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >> > > > > > > > > Reviewed-by: Jarkko Sakkinen >> > > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> > > > > > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >> > > > > > > > > Signed-off-by: Jarkko Sakkinen >> > > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >> > > > > > > > > >> > > > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >> > > > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers >> > > > > > > > > >> > > > > > > > >> > > > > > > > I've looked again at the code in question, but could not find >> > > > > > > > anything that is obviously wrong there. Locality is now >> > > > > > > > requested/released at slightly different points in the process than >> > > > > > > > before, but that's it. It does not seem to cause problems with the >> > > > > > > > majority of TPMs, since you are the first to report any, so >> > > > > > maybe it >> > > > > > > > is a quirk that only affects this device. >> > > > > > > > >> > > > > > > > Perhaps Jerry can help, since this is his change? >> > > > > > > > >> > > > > > > > Alexander >> > > > > > > >> > > > > > > Getting some caffeine in me, and starting to take a look. Adding >> > > > > > > Jarkko as well since this might involve the general locality changes. >> > > > > > > >> > > > > > > Laurent, if I send you a patch with some debugging code added, would >> > > > > > > you be able to run it on that system? I wasn't running into issues >> > > > > > > on the system I had with a 1.2 device, but I no longer have access >> > > > > > > to it. I'll see if I can find one in our labs and reproduce it there. >> > > > > > >> > > > > > Yes I should be able to do that >> > > > > >> > > > > Any findings? >> > > > > >> > > > > /Jarkko >> > > > >> > > > Okay, finally getting back to this. Looking at the code it isn't clear >> > > > to me >> > > > why the change is causing this. So while I stare at this some more >> > > > Laurent >> > > > could you reproduce it with this patch so I can see what the status and >> > > > access registers look like? Does anyone else on here happen to have a >> > > > Sinosun >> > > > tpm device? The systems I have access to with TPM1.2 devices don't have >> > > > this >> > > > issue. >> > > > >> > > > --8<-- >> > > > >> > > > diff --git a/drivers/char/tpm/tpm_tis_core.c >> > > > b/drivers/char/tpm/tpm_tis_core.c >> > > > index fdde971bc810..7d60a7e4b50a 100644 >> > > > --- a/drivers/char/tpm/tpm_tis_core.c >> > > > +++ b/drivers/char/tpm/tpm_tis_core.c >> > > > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >> > > > const u8 *buf, size_t len) >> > > > int rc, status, burstcnt; >> > > > size_t count = 0; >> > > > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >> > > > + u8 access; >> > > > >> > > > status = tpm_tis_status(chip); >> > > > if ((status & TPM_STS_COMMAND_READY) == 0) { >> > > > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >> > > > const u8 *buf, size_t len) >> > > > } >> > > > status = tpm_tis_status(chip); >> > > > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >> > > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >> > > > &access); >> > > > + if (rc < 0) >> > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read >> > > > failure TPM_ACCESS(%d)\n", priv->locality); >> > > > + else >> > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >> > > > locality: %d status: %x access: %x\n", priv->locality, status, access); >> > > > rc = -EIO; >> > > > goto out_err; >> > > > } >> > > > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >> > > > const u8 *buf, size_t len) >> > > > } >> > > > status = tpm_tis_status(chip); >> > > > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >> > > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); >> > > > + if (rc < 0) >> > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >> > > > failure TPM_ACCESS(%d)\n", priv->locality); >> > > > + else >> > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: >> > > > %d status: %x access: %x\n", priv->locality, status, access); >> > > > rc = -EIO; >> > > > goto out_err; >> > > > } >> > > >> > > Please find here the dmesg output of the patched kernel >> > >> > At least 0xff is corrupted value in senseful way. CPU fills the read >> > with ones for example for unaligned bus read. See table 19 in PC client >> > spec. This can happen when you do unaligned read for example. >> > >> > Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but >> > would probably make sense to compare that 0x81 to table 18 in the same >> > spec. >> > >> > /Jarkko >> >> 0x81 is saying the access register status is valid, and the locality >> is not active. That first bit means A Dynamic OS has not been previously >> established on the platform. Normally we would see 0xa1, which would >> mean valid register status, and the locality is active. > >I think the important thing to note here is that STS has bits set that >should never be set. So we can conclude that TPM might be either > >1. Powered off >2. In some transition state? > >/Jarkko If it was powered off would we be getting a valid read from the access register? ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore 2017-10-24 14:57 ` Jerry Snitselaar @ 2017-10-24 16:07 ` Jarkko Sakkinen [not found] ` <20171024160725.r6kj452jdzpkbb6o-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Jarkko Sakkinen @ 2017-10-24 16:07 UTC (permalink / raw) To: Jerry Snitselaar Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f On Tue, Oct 24, 2017 at 07:57:06AM -0700, Jerry Snitselaar wrote: > On Tue Oct 24 17, Jarkko Sakkinen wrote: > > On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: > > > On Mon Oct 23 17, Jarkko Sakkinen wrote: > > > > On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: > > > > > Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : > > > > > > On Wed Sep 06 17, Jarkko Sakkinen wrote: > > > > > > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: > > > > > > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : > > > > > > > > > On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: > > > > > > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit : > > > > > > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : > > > > > > > > > > > >>> An idea how to troubleshoot this? > > > > > > > > > > > >> Can you run git bisect on the changes between 4.11 and > > > > > > > > 4.12, so that > > > > > > > > > > > >> we find the offending commit? It is probably sufficient > > > > > > > > to limit the > > > > > > > > > > > >> search to commits that touch something in drivers/char/tpm. > > > > > > > > > > > > > > > > > > > > > > > > I'll try and keep you posted. > > > > > > > > > > > > > > > > > > > > > > OK I've been able to bisect the problem and the bad commit is: > > > > > > > > > > > > > > > > > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit > > > > > > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 > > > > > > > > > > > Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > > > > > > > > > > > Date: Mon Mar 27 08:46:04 2017 -0700 > > > > > > > > > > > > > > > > > > > > > > tpm_tis: convert to using locality callbacks > > > > > > > > > > > > > > > > > > > > > > This patch converts tpm_tis to use of the new tpm class ops > > > > > > > > > > > request_locality, and relinquish_locality. > > > > > > > > > > > > > > > > > > > > > > With the move to using the callbacks, release_locality is > > > > > > > > > > > changed so > > > > > > > > > > > that we now release the locality even if there is no > > > > > > > > > > > request pending. > > > > > > > > > > > > > > > > > > > > > > This required some changes to the tpm_tis_core_init > > > > > > > > code path to > > > > > > > > > > > make sure locality is requested when needed: > > > > > > > > > > > > > > > > > > > > > > - tpm2_probe code path will end up calling > > > > > > > > > > > request/release through > > > > > > > > > > > callbacks, so request_locality prior to > > > > > > > > tpm2_probe not needed. > > > > > > > > > > > > > > > > > > > > > > - probe_itpm makes calls to tpm_tis_send_data which no > > > > > > > > > > > longer calls > > > > > > > > > > > request_locality, so add request_locality prior to > > > > > > > > > > > tpm_tis_send_data > > > > > > > > > > > calls. Also drop release_locality call in middleof > > > > > > > > > > > probe_itpm, and > > > > > > > > > > > keep locality until release_locality called at end of > > > > > > > > > > > probe_itpm. > > > > > > > > > > > > > > > > > > > > > > Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> > > > > > > > > > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > > > > > > > > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > > > > > > > > > > Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> > > > > > > > > > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> > > > > > > > > > > > Reviewed-by: Jarkko Sakkinen > > > > > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > > > Signed-off-by: Jarkko Sakkinen > > > > > > > > <jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> > > > > > > > > > > > > > > > > > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 > > > > > > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I've looked again at the code in question, but could not find > > > > > > > > > > anything that is obviously wrong there. Locality is now > > > > > > > > > > requested/released at slightly different points in the process than > > > > > > > > > > before, but that's it. It does not seem to cause problems with the > > > > > > > > > > majority of TPMs, since you are the first to report any, so > > > > > > > > maybe it > > > > > > > > > > is a quirk that only affects this device. > > > > > > > > > > > > > > > > > > > > Perhaps Jerry can help, since this is his change? > > > > > > > > > > > > > > > > > > > > Alexander > > > > > > > > > > > > > > > > > > Getting some caffeine in me, and starting to take a look. Adding > > > > > > > > > Jarkko as well since this might involve the general locality changes. > > > > > > > > > > > > > > > > > > Laurent, if I send you a patch with some debugging code added, would > > > > > > > > > you be able to run it on that system? I wasn't running into issues > > > > > > > > > on the system I had with a 1.2 device, but I no longer have access > > > > > > > > > to it. I'll see if I can find one in our labs and reproduce it there. > > > > > > > > > > > > > > > > Yes I should be able to do that > > > > > > > > > > > > > > Any findings? > > > > > > > > > > > > > > /Jarkko > > > > > > > > > > > > Okay, finally getting back to this. Looking at the code it isn't clear > > > > > > to me > > > > > > why the change is causing this. So while I stare at this some more > > > > > > Laurent > > > > > > could you reproduce it with this patch so I can see what the status and > > > > > > access registers look like? Does anyone else on here happen to have a > > > > > > Sinosun > > > > > > tpm device? The systems I have access to with TPM1.2 devices don't have > > > > > > this > > > > > > issue. > > > > > > > > > > > > --8<-- > > > > > > > > > > > > diff --git a/drivers/char/tpm/tpm_tis_core.c > > > > > > b/drivers/char/tpm/tpm_tis_core.c > > > > > > index fdde971bc810..7d60a7e4b50a 100644 > > > > > > --- a/drivers/char/tpm/tpm_tis_core.c > > > > > > +++ b/drivers/char/tpm/tpm_tis_core.c > > > > > > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > > > > > const u8 *buf, size_t len) > > > > > > int rc, status, burstcnt; > > > > > > size_t count = 0; > > > > > > bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; > > > > > > + u8 access; > > > > > > > > > > > > status = tpm_tis_status(chip); > > > > > > if ((status & TPM_STS_COMMAND_READY) == 0) { > > > > > > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > > > > > const u8 *buf, size_t len) > > > > > > } > > > > > > status = tpm_tis_status(chip); > > > > > > if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { > > > > > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), > > > > > > &access); > > > > > > + if (rc < 0) > > > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read > > > > > > failure TPM_ACCESS(%d)\n", priv->locality); > > > > > > + else > > > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: > > > > > > locality: %d status: %x access: %x\n", priv->locality, status, access); > > > > > > rc = -EIO; > > > > > > goto out_err; > > > > > > } > > > > > > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, > > > > > > const u8 *buf, size_t len) > > > > > > } > > > > > > status = tpm_tis_status(chip); > > > > > > if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { > > > > > > + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); > > > > > > + if (rc < 0) > > > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read > > > > > > failure TPM_ACCESS(%d)\n", priv->locality); > > > > > > + else > > > > > > + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: > > > > > > %d status: %x access: %x\n", priv->locality, status, access); > > > > > > rc = -EIO; > > > > > > goto out_err; > > > > > > } > > > > > > > > > > Please find here the dmesg output of the patched kernel > > > > > > > > At least 0xff is corrupted value in senseful way. CPU fills the read > > > > with ones for example for unaligned bus read. See table 19 in PC client > > > > spec. This can happen when you do unaligned read for example. > > > > > > > > Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but > > > > would probably make sense to compare that 0x81 to table 18 in the same > > > > spec. > > > > > > > > /Jarkko > > > > > > 0x81 is saying the access register status is valid, and the locality > > > is not active. That first bit means A Dynamic OS has not been previously > > > established on the platform. Normally we would see 0xa1, which would > > > mean valid register status, and the locality is active. > > > > I think the important thing to note here is that STS has bits set that > > should never be set. So we can conclude that TPM might be either > > > > 1. Powered off > > 2. In some transition state? > > > > /Jarkko > > If it was powered off would we be getting a valid read from the access > register? I think there is no universal answer to that :-) Maybe adding a extra delay would be next test to make? If for random reason it is in-between states... /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <20171024160725.r6kj452jdzpkbb6o-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <20171024160725.r6kj452jdzpkbb6o-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> @ 2017-11-09 0:04 ` Laurent Bigonville [not found] ` <8f4df9a9-c8cd-832f-4c3f-5305fabab7a8-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Laurent Bigonville @ 2017-11-09 0:04 UTC (permalink / raw) To: Jarkko Sakkinen, Jerry Snitselaar Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Le 24/10/17 à 18:07, Jarkko Sakkinen a écrit : > On Tue, Oct 24, 2017 at 07:57:06AM -0700, Jerry Snitselaar wrote: >> On Tue Oct 24 17, Jarkko Sakkinen wrote: >>> On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: >>>> On Mon Oct 23 17, Jarkko Sakkinen wrote: >>>>> On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >>>>>> Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >>>>>>> On Wed Sep 06 17, Jarkko Sakkinen wrote: >>>>>>>> On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >>>>>>>>> Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>>>>>>>>> On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote: >>>>>>>>>>>> Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>>>>>>>>>>>> Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : >>>>>>>>>>>>>>> An idea how to troubleshoot this? >>>>>>>>>>>>>> Can you run git bisect on the changes between 4.11 and >>>>>>>>> 4.12, so that >>>>>>>>>>>>>> we find the offending commit? It is probably sufficient >>>>>>>>> to limit the >>>>>>>>>>>>>> search to commits that touch something in drivers/char/tpm. >>>>>>>>>>>>> I'll try and keep you posted. >>>>>>>>>>>> OK I've been able to bisect the problem and the bad commit is: >>>>>>>>>>>> >>>>>>>>>>>> e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >>>>>>>>>>>> commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>>>>>>>>>>> Author: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>>>> Date: Mon Mar 27 08:46:04 2017 -0700 >>>>>>>>>>>> >>>>>>>>>>>> tpm_tis: convert to using locality callbacks >>>>>>>>>>>> >>>>>>>>>>>> This patch converts tpm_tis to use of the new tpm class ops >>>>>>>>>>>> request_locality, and relinquish_locality. >>>>>>>>>>>> >>>>>>>>>>>> With the move to using the callbacks, release_locality is >>>>>>>>>>>> changed so >>>>>>>>>>>> that we now release the locality even if there is no >>>>>>>>>>>> request pending. >>>>>>>>>>>> >>>>>>>>>>>> This required some changes to the tpm_tis_core_init >>>>>>>>> code path to >>>>>>>>>>>> make sure locality is requested when needed: >>>>>>>>>>>> >>>>>>>>>>>> - tpm2_probe code path will end up calling >>>>>>>>>>>> request/release through >>>>>>>>>>>> callbacks, so request_locality prior to >>>>>>>>> tpm2_probe not needed. >>>>>>>>>>>> - probe_itpm makes calls to tpm_tis_send_data which no >>>>>>>>>>>> longer calls >>>>>>>>>>>> request_locality, so add request_locality prior to >>>>>>>>>>>> tpm_tis_send_data >>>>>>>>>>>> calls. Also drop release_locality call in middleof >>>>>>>>>>>> probe_itpm, and >>>>>>>>>>>> keep locality until release_locality called at end of >>>>>>>>>>>> probe_itpm. >>>>>>>>>>>> >>>>>>>>>>>> Cc: Peter Huewe <peterhuewe@gmx.de> >>>>>>>>>>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>>>>>>>>>>> Cc: Marcel Selhorst <tpmdd@selhorst.net> >>>>>>>>>>>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>>>> Reviewed-by: Jarkko Sakkinen >>>>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>> Signed-off-by: Jarkko Sakkinen >>>>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>> :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>>>>>>>>>>> 72f21b446e45ea1003de75902b0553deb99157fd M drivers >>>>>>>>>>>> >>>>>>>>>>> I've looked again at the code in question, but could not find >>>>>>>>>>> anything that is obviously wrong there. Locality is now >>>>>>>>>>> requested/released at slightly different points in the process than >>>>>>>>>>> before, but that's it. It does not seem to cause problems with the >>>>>>>>>>> majority of TPMs, since you are the first to report any, so >>>>>>>>> maybe it >>>>>>>>>>> is a quirk that only affects this device. >>>>>>>>>>> >>>>>>>>>>> Perhaps Jerry can help, since this is his change? >>>>>>>>>>> >>>>>>>>>>> Alexander >>>>>>>>>> Getting some caffeine in me, and starting to take a look. Adding >>>>>>>>>> Jarkko as well since this might involve the general locality changes. >>>>>>>>>> >>>>>>>>>> Laurent, if I send you a patch with some debugging code added, would >>>>>>>>>> you be able to run it on that system? I wasn't running into issues >>>>>>>>>> on the system I had with a 1.2 device, but I no longer have access >>>>>>>>>> to it. I'll see if I can find one in our labs and reproduce it there. >>>>>>>>> Yes I should be able to do that >>>>>>>> Any findings? >>>>>>>> >>>>>>>> /Jarkko >>>>>>> Okay, finally getting back to this. Looking at the code it isn't clear >>>>>>> to me >>>>>>> why the change is causing this. So while I stare at this some more >>>>>>> Laurent >>>>>>> could you reproduce it with this patch so I can see what the status and >>>>>>> access registers look like? Does anyone else on here happen to have a >>>>>>> Sinosun >>>>>>> tpm device? The systems I have access to with TPM1.2 devices don't have >>>>>>> this >>>>>>> issue. >>>>>>> >>>>>>> --8<-- >>>>>>> >>>>>>> diff --git a/drivers/char/tpm/tpm_tis_core.c >>>>>>> b/drivers/char/tpm/tpm_tis_core.c >>>>>>> index fdde971bc810..7d60a7e4b50a 100644 >>>>>>> --- a/drivers/char/tpm/tpm_tis_core.c >>>>>>> +++ b/drivers/char/tpm/tpm_tis_core.c >>>>>>> @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >>>>>>> const u8 *buf, size_t len) >>>>>>> int rc, status, burstcnt; >>>>>>> size_t count = 0; >>>>>>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >>>>>>> + u8 access; >>>>>>> >>>>>>> status = tpm_tis_status(chip); >>>>>>> if ((status & TPM_STS_COMMAND_READY) == 0) { >>>>>>> @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >>>>>>> const u8 *buf, size_t len) >>>>>>> } >>>>>>> status = tpm_tis_status(chip); >>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >>>>>>> + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>>>>>> &access); >>>>>>> + if (rc < 0) >>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read >>>>>>> failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>> + else >>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >>>>>>> locality: %d status: %x access: %x\n", priv->locality, status, access); >>>>>>> rc = -EIO; >>>>>>> goto out_err; >>>>>>> } >>>>>>> @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >>>>>>> const u8 *buf, size_t len) >>>>>>> } >>>>>>> status = tpm_tis_status(chip); >>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >>>>>>> + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); >>>>>>> + if (rc < 0) >>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >>>>>>> failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>> + else >>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: >>>>>>> %d status: %x access: %x\n", priv->locality, status, access); >>>>>>> rc = -EIO; >>>>>>> goto out_err; >>>>>>> } >>>>>> Please find here the dmesg output of the patched kernel >>>>> At least 0xff is corrupted value in senseful way. CPU fills the read >>>>> with ones for example for unaligned bus read. See table 19 in PC client >>>>> spec. This can happen when you do unaligned read for example. >>>>> >>>>> Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but >>>>> would probably make sense to compare that 0x81 to table 18 in the same >>>>> spec. >>>>> >>>>> /Jarkko >>>> 0x81 is saying the access register status is valid, and the locality >>>> is not active. That first bit means A Dynamic OS has not been previously >>>> established on the platform. Normally we would see 0xa1, which would >>>> mean valid register status, and the locality is active. >>> I think the important thing to note here is that STS has bits set that >>> should never be set. So we can conclude that TPM might be either >>> >>> 1. Powered off >>> 2. In some transition state? >>> >>> /Jarkko >> If it was powered off would we be getting a valid read from the access >> register? > I think there is no universal answer to that :-) > > Maybe adding a extra delay would be next test to make? If for random > reason it is in-between states... Any more ideas? The chip is definitely in a weird state :/ I tried several ways to reset the chip (windows, tpm-tools,...). I've been able to reset the chip via the bios (which now shows unowned) but chip is still locked apparently. But still with < 4.12 I'm able to get /some/ information Public EK, PCR,... out of the chip so it's not completely broken... ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <8f4df9a9-c8cd-832f-4c3f-5305fabab7a8-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <8f4df9a9-c8cd-832f-4c3f-5305fabab7a8-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-11-09 19:58 ` Laurent Bigonville [not found] ` <0a6e4771-f871-b3ca-b5b0-26dbd9efa8b1-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Laurent Bigonville @ 2017-11-09 19:58 UTC (permalink / raw) To: Jarkko Sakkinen, Jerry Snitselaar Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Le 09/11/17 à 01:04, Laurent Bigonville a écrit : > > > Le 24/10/17 à 18:07, Jarkko Sakkinen a écrit : >> On Tue, Oct 24, 2017 at 07:57:06AM -0700, Jerry Snitselaar wrote: >>> On Tue Oct 24 17, Jarkko Sakkinen wrote: >>>> On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: >>>>> On Mon Oct 23 17, Jarkko Sakkinen wrote: >>>>>> On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >>>>>>> Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >>>>>>>> On Wed Sep 06 17, Jarkko Sakkinen wrote: >>>>>>>>> On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville >>>>>>>>> wrote: >>>>>>>>>> Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>>>>>>>>>> On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote: >>>>>>>>>>>>> Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>>>>>>>>>>>>> Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a >>>>>>>>>>>>>> écrit : >>>>>>>>>>>>>>>> An idea how to troubleshoot this? >>>>>>>>>>>>>>> Can you run git bisect on the changes between 4.11 and >>>>>>>>>> 4.12, so that >>>>>>>>>>>>>>> we find the offending commit? It is probably sufficient >>>>>>>>>> to limit the >>>>>>>>>>>>>>> search to commits that touch something in drivers/char/tpm. >>>>>>>>>>>>>> I'll try and keep you posted. >>>>>>>>>>>>> OK I've been able to bisect the problem and the bad commit >>>>>>>>>>>>> is: >>>>>>>>>>>>> >>>>>>>>>>>>> e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad >>>>>>>>>>>>> commit >>>>>>>>>>>>> commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>>>>>>>>>>>> Author: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>>>>> Date: Mon Mar 27 08:46:04 2017 -0700 >>>>>>>>>>>>> >>>>>>>>>>>>> tpm_tis: convert to using locality callbacks >>>>>>>>>>>>> >>>>>>>>>>>>> This patch converts tpm_tis to use of the new tpm >>>>>>>>>>>>> class ops >>>>>>>>>>>>> request_locality, and relinquish_locality. >>>>>>>>>>>>> >>>>>>>>>>>>> With the move to using the callbacks, >>>>>>>>>>>>> release_locality is >>>>>>>>>>>>> changed so >>>>>>>>>>>>> that we now release the locality even if there is no >>>>>>>>>>>>> request pending. >>>>>>>>>>>>> >>>>>>>>>>>>> This required some changes to the tpm_tis_core_init >>>>>>>>>> code path to >>>>>>>>>>>>> make sure locality is requested when needed: >>>>>>>>>>>>> >>>>>>>>>>>>> - tpm2_probe code path will end up calling >>>>>>>>>>>>> request/release through >>>>>>>>>>>>> callbacks, so request_locality prior to >>>>>>>>>> tpm2_probe not needed. >>>>>>>>>>>>> - probe_itpm makes calls to tpm_tis_send_data >>>>>>>>>>>>> which no >>>>>>>>>>>>> longer calls >>>>>>>>>>>>> request_locality, so add request_locality prior to >>>>>>>>>>>>> tpm_tis_send_data >>>>>>>>>>>>> calls. Also drop release_locality call in middleof >>>>>>>>>>>>> probe_itpm, and >>>>>>>>>>>>> keep locality until release_locality called at >>>>>>>>>>>>> end of >>>>>>>>>>>>> probe_itpm. >>>>>>>>>>>>> >>>>>>>>>>>>> Cc: Peter Huewe <peterhuewe@gmx.de> >>>>>>>>>>>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>>>>>>>>>>>> Cc: Marcel Selhorst <tpmdd@selhorst.net> >>>>>>>>>>>>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>>>>> Reviewed-by: Jarkko Sakkinen >>>>>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>>> Tested-by: Jarkko Sakkinen >>>>>>>>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>>> Signed-off-by: Jarkko Sakkinen >>>>>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>>> :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>>>>>>>>>>>> 72f21b446e45ea1003de75902b0553deb99157fd M drivers >>>>>>>>>>>>> >>>>>>>>>>>> I've looked again at the code in question, but could not find >>>>>>>>>>>> anything that is obviously wrong there. Locality is now >>>>>>>>>>>> requested/released at slightly different points in the >>>>>>>>>>>> process than >>>>>>>>>>>> before, but that's it. It does not seem to cause problems >>>>>>>>>>>> with the >>>>>>>>>>>> majority of TPMs, since you are the first to report any, so >>>>>>>>>> maybe it >>>>>>>>>>>> is a quirk that only affects this device. >>>>>>>>>>>> >>>>>>>>>>>> Perhaps Jerry can help, since this is his change? >>>>>>>>>>>> >>>>>>>>>>>> Alexander >>>>>>>>>>> Getting some caffeine in me, and starting to take a look. >>>>>>>>>>> Adding >>>>>>>>>>> Jarkko as well since this might involve the general locality >>>>>>>>>>> changes. >>>>>>>>>>> >>>>>>>>>>> Laurent, if I send you a patch with some debugging code >>>>>>>>>>> added, would >>>>>>>>>>> you be able to run it on that system? I wasn't running into >>>>>>>>>>> issues >>>>>>>>>>> on the system I had with a 1.2 device, but I no longer have >>>>>>>>>>> access >>>>>>>>>>> to it. I'll see if I can find one in our labs and reproduce >>>>>>>>>>> it there. >>>>>>>>>> Yes I should be able to do that >>>>>>>>> Any findings? >>>>>>>>> >>>>>>>>> /Jarkko >>>>>>>> Okay, finally getting back to this. Looking at the code it >>>>>>>> isn't clear >>>>>>>> to me >>>>>>>> why the change is causing this. So while I stare at this some more >>>>>>>> Laurent >>>>>>>> could you reproduce it with this patch so I can see what the >>>>>>>> status and >>>>>>>> access registers look like? Does anyone else on here happen to >>>>>>>> have a >>>>>>>> Sinosun >>>>>>>> tpm device? The systems I have access to with TPM1.2 devices >>>>>>>> don't have >>>>>>>> this >>>>>>>> issue. >>>>>>>> >>>>>>>> --8<-- >>>>>>>> >>>>>>>> diff --git a/drivers/char/tpm/tpm_tis_core.c >>>>>>>> b/drivers/char/tpm/tpm_tis_core.c >>>>>>>> index fdde971bc810..7d60a7e4b50a 100644 >>>>>>>> --- a/drivers/char/tpm/tpm_tis_core.c >>>>>>>> +++ b/drivers/char/tpm/tpm_tis_core.c >>>>>>>> @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct >>>>>>>> tpm_chip *chip, >>>>>>>> const u8 *buf, size_t len) >>>>>>>> int rc, status, burstcnt; >>>>>>>> size_t count = 0; >>>>>>>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >>>>>>>> + u8 access; >>>>>>>> >>>>>>>> status = tpm_tis_status(chip); >>>>>>>> if ((status & TPM_STS_COMMAND_READY) == 0) { >>>>>>>> @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct >>>>>>>> tpm_chip *chip, >>>>>>>> const u8 *buf, size_t len) >>>>>>>> } >>>>>>>> status = tpm_tis_status(chip); >>>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >>>>>>>> + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>>>>>>> &access); >>>>>>>> + if (rc < 0) >>>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == >>>>>>>> 0: read >>>>>>>> failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>>> + else >>>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >>>>>>>> locality: %d status: %x access: %x\n", priv->locality, status, >>>>>>>> access); >>>>>>>> rc = -EIO; >>>>>>>> goto out_err; >>>>>>>> } >>>>>>>> @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct >>>>>>>> tpm_chip *chip, >>>>>>>> const u8 *buf, size_t len) >>>>>>>> } >>>>>>>> status = tpm_tis_status(chip); >>>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >>>>>>>> + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>>>>>>> &access); >>>>>>>> + if (rc < 0) >>>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >>>>>>>> failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>>> + else >>>>>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: >>>>>>>> locality: >>>>>>>> %d status: %x access: %x\n", priv->locality, status, access); >>>>>>>> rc = -EIO; >>>>>>>> goto out_err; >>>>>>>> } >>>>>>> Please find here the dmesg output of the patched kernel >>>>>> At least 0xff is corrupted value in senseful way. CPU fills the read >>>>>> with ones for example for unaligned bus read. See table 19 in PC >>>>>> client >>>>>> spec. This can happen when you do unaligned read for example. >>>>>> >>>>>> Maybe TPM is unreachable i.e. powered off. Bit busy with stuff >>>>>> ATM but >>>>>> would probably make sense to compare that 0x81 to table 18 in the >>>>>> same >>>>>> spec. >>>>>> >>>>>> /Jarkko >>>>> 0x81 is saying the access register status is valid, and the locality >>>>> is not active. That first bit means A Dynamic OS has not been >>>>> previously >>>>> established on the platform. Normally we would see 0xa1, which would >>>>> mean valid register status, and the locality is active. >>>> I think the important thing to note here is that STS has bits set that >>>> should never be set. So we can conclude that TPM might be either >>>> >>>> 1. Powered off >>>> 2. In some transition state? >>>> >>>> /Jarkko >>> If it was powered off would we be getting a valid read from the access >>> register? >> I think there is no universal answer to that :-) >> >> Maybe adding a extra delay would be next test to make? If for random >> reason it is in-between states... > Any more ideas? > > The chip is definitely in a weird state :/ I tried several ways to > reset the chip (windows, tpm-tools,...). > > I've been able to reset the chip via the bios (which now shows > unowned) but chip is still locked apparently. > > But still with < 4.12 I'm able to get /some/ information Public EK, > PCR,... out of the chip so it's not completely broken... OK correction the TPM is now unlocked (I let the computer running for more than 24h with nothing accessing the TPM) and with 4.9 I've been able to take the ownership again. Under 4.12 I still have the same errors as mentioned originally ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
[parent not found: <0a6e4771-f871-b3ca-b5b0-26dbd9efa8b1-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <0a6e4771-f871-b3ca-b5b0-26dbd9efa8b1-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org> @ 2017-11-09 23:50 ` Jerry Snitselaar [not found] ` <20171109235005.pmklfw6zkjirahfa-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org> 0 siblings, 1 reply; 23+ messages in thread From: Jerry Snitselaar @ 2017-11-09 23:50 UTC (permalink / raw) To: Laurent Bigonville Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jarkko Sakkinen On Thu Nov 09 17, Laurent Bigonville wrote: >Le 09/11/17 à 01:04, Laurent Bigonville a écrit : >> >> >>Le 24/10/17 à 18:07, Jarkko Sakkinen a écrit : >>>On Tue, Oct 24, 2017 at 07:57:06AM -0700, Jerry Snitselaar wrote: >>>>On Tue Oct 24 17, Jarkko Sakkinen wrote: >>>>>On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: >>>>>>On Mon Oct 23 17, Jarkko Sakkinen wrote: >>>>>>>On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >>>>>>>>Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >>>>>>>>>On Wed Sep 06 17, Jarkko Sakkinen wrote: >>>>>>>>>>On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent >>>>>>>>>>Bigonville wrote: >>>>>>>>>>>Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>>>>>>>>>>>On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >>>>>>>>>>>>>>Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>>>>>>>>>>>>>>Le 29/08/17 à 18:00, >>>>>>>>>>>>>>>Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >>>>>>>>>>>>>>>>>An idea how to troubleshoot this? >>>>>>>>>>>>>>>>Can you run git bisect on the changes between 4.11 and >>>>>>>>>>>4.12, so that >>>>>>>>>>>>>>>>we find the offending commit? It is probably sufficient >>>>>>>>>>>to limit the >>>>>>>>>>>>>>>>search to commits that touch something in drivers/char/tpm. >>>>>>>>>>>>>>>I'll try and keep you posted. >>>>>>>>>>>>>>OK I've been able to bisect the problem and >>>>>>>>>>>>>>the bad commit is: >>>>>>>>>>>>>> >>>>>>>>>>>>>>e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is >>>>>>>>>>>>>>the first bad commit >>>>>>>>>>>>>>commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>>>>>>>>>>>>>Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>>>>>>>>>>>>>Date: Mon Mar 27 08:46:04 2017 -0700 >>>>>>>>>>>>>> >>>>>>>>>>>>>> tpm_tis: convert to using locality callbacks >>>>>>>>>>>>>> >>>>>>>>>>>>>> This patch converts tpm_tis to use of >>>>>>>>>>>>>>the new tpm class ops >>>>>>>>>>>>>> request_locality, and relinquish_locality. >>>>>>>>>>>>>> >>>>>>>>>>>>>> With the move to using the callbacks, >>>>>>>>>>>>>>release_locality is >>>>>>>>>>>>>>changed so >>>>>>>>>>>>>> that we now release the locality even if there is no >>>>>>>>>>>>>>request pending. >>>>>>>>>>>>>> >>>>>>>>>>>>>> This required some changes to the tpm_tis_core_init >>>>>>>>>>>code path to >>>>>>>>>>>>>> make sure locality is requested when needed: >>>>>>>>>>>>>> >>>>>>>>>>>>>> - tpm2_probe code path will end up calling >>>>>>>>>>>>>>request/release through >>>>>>>>>>>>>> callbacks, so request_locality prior to >>>>>>>>>>>tpm2_probe not needed. >>>>>>>>>>>>>> - probe_itpm makes calls to >>>>>>>>>>>>>>tpm_tis_send_data which no >>>>>>>>>>>>>>longer calls >>>>>>>>>>>>>> request_locality, so add request_locality prior to >>>>>>>>>>>>>>tpm_tis_send_data >>>>>>>>>>>>>> calls. Also drop release_locality call in middleof >>>>>>>>>>>>>>probe_itpm, and >>>>>>>>>>>>>> keep locality until >>>>>>>>>>>>>>release_locality called at end of >>>>>>>>>>>>>>probe_itpm. >>>>>>>>>>>>>> >>>>>>>>>>>>>> Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >>>>>>>>>>>>>> Cc: Jarkko Sakkinen <jarkko.sakkinen-VuQAYsv1560@public.gmane.orgtel.com> >>>>>>>>>>>>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>>>>>>>>>>>>> Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> >>>>>>>>>>>>>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>>>>>> Reviewed-by: Jarkko Sakkinen >>>>>>>>>>><jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>>>>>>>>>>>> Tested-by: Jarkko Sakkinen >>>>>>>>>>>>>><jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>>>>>>>>>>>> Signed-off-by: Jarkko Sakkinen >>>>>>>>>>><jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>>>>>>>>>>>>:040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>>>>>>>>>>>>>72f21b446e45ea1003de75902b0553deb99157fd M drivers >>>>>>>>>>>>>> >>>>>>>>>>>>>I've looked again at the code in question, but could not find >>>>>>>>>>>>>anything that is obviously wrong there. Locality is now >>>>>>>>>>>>>requested/released at slightly different >>>>>>>>>>>>>points in the process than >>>>>>>>>>>>>before, but that's it. It does not seem to >>>>>>>>>>>>>cause problems with the >>>>>>>>>>>>>majority of TPMs, since you are the first to report any, so >>>>>>>>>>>maybe it >>>>>>>>>>>>>is a quirk that only affects this device. >>>>>>>>>>>>> >>>>>>>>>>>>>Perhaps Jerry can help, since this is his change? >>>>>>>>>>>>> >>>>>>>>>>>>>Alexander >>>>>>>>>>>>Getting some caffeine in me, and starting to >>>>>>>>>>>>take a look. Adding >>>>>>>>>>>>Jarkko as well since this might involve the >>>>>>>>>>>>general locality changes. >>>>>>>>>>>> >>>>>>>>>>>>Laurent, if I send you a patch with some >>>>>>>>>>>>debugging code added, would >>>>>>>>>>>>you be able to run it on that system? I wasn't >>>>>>>>>>>>running into issues >>>>>>>>>>>>on the system I had with a 1.2 device, but I no >>>>>>>>>>>>longer have access >>>>>>>>>>>>to it. I'll see if I can find one in our labs >>>>>>>>>>>>and reproduce it there. >>>>>>>>>>>Yes I should be able to do that >>>>>>>>>>Any findings? >>>>>>>>>> >>>>>>>>>>/Jarkko >>>>>>>>>Okay, finally getting back to this. Looking at the >>>>>>>>>code it isn't clear >>>>>>>>>to me >>>>>>>>>why the change is causing this. So while I stare at this some more >>>>>>>>>Laurent >>>>>>>>>could you reproduce it with this patch so I can see >>>>>>>>>what the status and >>>>>>>>>access registers look like? Does anyone else on here >>>>>>>>>happen to have a >>>>>>>>>Sinosun >>>>>>>>>tpm device? The systems I have access to with TPM1.2 >>>>>>>>>devices don't have >>>>>>>>>this >>>>>>>>>issue. >>>>>>>>> >>>>>>>>>--8<-- >>>>>>>>> >>>>>>>>>diff --git a/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>b/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>index fdde971bc810..7d60a7e4b50a 100644 >>>>>>>>>--- a/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>+++ b/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>@@ -258,6 +258,7 @@ static int >>>>>>>>>tpm_tis_send_data(struct tpm_chip *chip, >>>>>>>>>const u8 *buf, size_t len) >>>>>>>>> int rc, status, burstcnt; >>>>>>>>> size_t count = 0; >>>>>>>>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >>>>>>>>>+ u8 access; >>>>>>>>> >>>>>>>>> status = tpm_tis_status(chip); >>>>>>>>> if ((status & TPM_STS_COMMAND_READY) == 0) { >>>>>>>>>@@ -292,6 +293,11 @@ static int >>>>>>>>>tpm_tis_send_data(struct tpm_chip *chip, >>>>>>>>>const u8 *buf, size_t len) >>>>>>>>> } >>>>>>>>> status = tpm_tis_status(chip); >>>>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >>>>>>>>>+ rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>>>>>>>>&access); >>>>>>>>>+ if (rc < 0) >>>>>>>>>+ dev_info(&chip->dev, >>>>>>>>>"TPM_STS_DATA_EXPECT == 0: read >>>>>>>>>failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>>>>+ else >>>>>>>>>+ dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >>>>>>>>>locality: %d status: %x access: %x\n", priv->locality, >>>>>>>>>status, access); >>>>>>>>> rc = -EIO; >>>>>>>>> goto out_err; >>>>>>>>> } >>>>>>>>>@@ -309,6 +315,11 @@ static int >>>>>>>>>tpm_tis_send_data(struct tpm_chip *chip, >>>>>>>>>const u8 *buf, size_t len) >>>>>>>>> } >>>>>>>>> status = tpm_tis_status(chip); >>>>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >>>>>>>>>+ rc = tpm_tis_read8(priv, >>>>>>>>>TPM_ACCESS(priv->locality), &access); >>>>>>>>>+ if (rc < 0) >>>>>>>>>+ dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >>>>>>>>>failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>>>>+ else >>>>>>>>>+ dev_info(&chip->dev, "TPM_STS_DATA_EXPECT >>>>>>>>>!= 0: locality: >>>>>>>>>%d status: %x access: %x\n", priv->locality, status, access); >>>>>>>>> rc = -EIO; >>>>>>>>> goto out_err; >>>>>>>>> } >>>>>>>>Please find here the dmesg output of the patched kernel >>>>>>>At least 0xff is corrupted value in senseful way. CPU fills the read >>>>>>>with ones for example for unaligned bus read. See table 19 >>>>>>>in PC client >>>>>>>spec. This can happen when you do unaligned read for example. >>>>>>> >>>>>>>Maybe TPM is unreachable i.e. powered off. Bit busy with >>>>>>>stuff ATM but >>>>>>>would probably make sense to compare that 0x81 to table 18 >>>>>>>in the same >>>>>>>spec. >>>>>>> >>>>>>>/Jarkko >>>>>>0x81 is saying the access register status is valid, and the locality >>>>>>is not active. That first bit means A Dynamic OS has not >>>>>>been previously >>>>>>established on the platform. Normally we would see 0xa1, which would >>>>>>mean valid register status, and the locality is active. >>>>>I think the important thing to note here is that STS has bits set that >>>>>should never be set. So we can conclude that TPM might be either >>>>> >>>>>1. Powered off >>>>>2. In some transition state? >>>>> >>>>>/Jarkko >>>>If it was powered off would we be getting a valid read from the access >>>>register? >>>I think there is no universal answer to that :-) >>> >>>Maybe adding a extra delay would be next test to make? If for random >>>reason it is in-between states... >>Any more ideas? >> >>The chip is definitely in a weird state :/ I tried several ways to >>reset the chip (windows, tpm-tools,...). >> >>I've been able to reset the chip via the bios (which now shows >>unowned) but chip is still locked apparently. >> >>But still with < 4.12 I'm able to get /some/ information Public EK, >>PCR,... out of the chip so it's not completely broken... > >OK correction the TPM is now unlocked (I let the computer running for >more than 24h with nothing accessing the TPM) and with 4.9 I've been >able to take the ownership again. > >Under 4.12 I still have the same errors as mentioned originally Still thinking about how to attack this. I don't think extending the timeout would change anything because it is reading the register, and thinking the status is valid because it reads 0xff. So it would still have that problem, right? Maybe a sanity check could be added to wait_for_tpm_stat() to first check that it didn't read 0xff before checking for the value it is looking for. Would that make sense Jarkko? diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c index 1d6729be4cd6..8b77fa2003ce 100644 --- a/drivers/char/tpm/tpm-interface.c +++ b/drivers/char/tpm/tpm-interface.c @@ -1087,7 +1087,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout, do { tpm_msleep(TPM_TIMEOUT); status = chip->ops->status(chip); - if ((status & mask) == mask) + if ((status != 0xff) && (status & mask) == mask) return 0; } while (time_before(jiffies, stop)); } ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply related [flat|nested] 23+ messages in thread
[parent not found: <20171109235005.pmklfw6zkjirahfa-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>]
* Re: tpm device not showing up in /dev anymore [not found] ` <20171109235005.pmklfw6zkjirahfa-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org> @ 2017-11-10 2:19 ` Jerry Snitselaar 0 siblings, 0 replies; 23+ messages in thread From: Jerry Snitselaar @ 2017-11-10 2:19 UTC (permalink / raw) To: Laurent Bigonville Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f, Jarkko Sakkinen On Thu Nov 09 17, Jerry Snitselaar wrote: >On Thu Nov 09 17, Laurent Bigonville wrote: >>Le 09/11/17 à 01:04, Laurent Bigonville a écrit : >>> >>> >>>Le 24/10/17 à 18:07, Jarkko Sakkinen a écrit : >>>>On Tue, Oct 24, 2017 at 07:57:06AM -0700, Jerry Snitselaar wrote: >>>>>On Tue Oct 24 17, Jarkko Sakkinen wrote: >>>>>>On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: >>>>>>>On Mon Oct 23 17, Jarkko Sakkinen wrote: >>>>>>>>On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >>>>>>>>>Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >>>>>>>>>>On Wed Sep 06 17, Jarkko Sakkinen wrote: >>>>>>>>>>>On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent >>>>>>>>>>>Bigonville wrote: >>>>>>>>>>>>Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>>>>>>>>>>>>On Thu Aug 31 17, Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org wrote: >>>>>>>>>>>>>>>Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>>>>>>>>>>>>>>>Le 29/08/17 à 18:00, >>>>>>>>>>>>>>>>Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w@public.gmane.org a écrit : >>>>>>>>>>>>>>>>>>An idea how to troubleshoot this? >>>>>>>>>>>>>>>>>Can you run git bisect on the changes between 4.11 and >>>>>>>>>>>>4.12, so that >>>>>>>>>>>>>>>>>we find the offending commit? It is probably sufficient >>>>>>>>>>>>to limit the >>>>>>>>>>>>>>>>>search to commits that touch something in drivers/char/tpm. >>>>>>>>>>>>>>>>I'll try and keep you posted. >>>>>>>>>>>>>>>OK I've been able to bisect the problem >>>>>>>>>>>>>>>and the bad commit is: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>>e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>>>>>>>>>>>>>>is the first bad commit >>>>>>>>>>>>>>>commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>>>>>>>>>>>>>>Author: Jerry Snitselaar <jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >>>>>>>>>>>>>>>Date: Mon Mar 27 08:46:04 2017 -0700 >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> tpm_tis: convert to using locality callbacks >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This patch converts tpm_tis to use >>>>>>>>>>>>>>>of the new tpm class ops >>>>>>>>>>>>>>> request_locality, and relinquish_locality. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> With the move to using the >>>>>>>>>>>>>>>callbacks, release_locality is >>>>>>>>>>>>>>>changed so >>>>>>>>>>>>>>> that we now release the locality even if there is no >>>>>>>>>>>>>>>request pending. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This required some changes to the tpm_tis_core_init >>>>>>>>>>>>code path to >>>>>>>>>>>>>>> make sure locality is requested when needed: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> - tpm2_probe code path will end up calling >>>>>>>>>>>>>>>request/release through >>>>>>>>>>>>>>> callbacks, so request_locality prior to >>>>>>>>>>>>tpm2_probe not needed. >>>>>>>>>>>>>>> - probe_itpm makes calls to >>>>>>>>>>>>>>>tpm_tis_send_data which no >>>>>>>>>>>>>>>longer calls >>>>>>>>>>>>>>> request_locality, so add request_locality prior to >>>>>>>>>>>>>>>tpm_tis_send_data >>>>>>>>>>>>>>> calls. Also drop release_locality call in middleof >>>>>>>>>>>>>>>probe_itpm, and >>>>>>>>>>>>>>> keep locality until >>>>>>>>>>>>>>>release_locality called at end of >>>>>>>>>>>>>>>probe_itpm. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Cc: Peter Huewe <peterhuewe-Mmb7MZpHnFY@public.gmane.org> >>>>>>>>>>>>>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>>>>>>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>>>>>>>>>>>>>> Cc: Marcel Selhorst <tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org> >>>>>>>>>>>>>>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>>>>>>> Reviewed-by: Jarkko Sakkinen >>>>>>>>>>>><jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>>>>>>>>>>>>> Tested-by: Jarkko Sakkinen >>>>>>>>>>>>>>><jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>>>>>>>>>>>>> Signed-off-by: Jarkko Sakkinen >>>>>>>>>>>><jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> >>>>>>>>>>>>>>>:040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>>>>>>>>>>>>>>72f21b446e45ea1003de75902b0553deb99157fd M drivers >>>>>>>>>>>>>>> >>>>>>>>>>>>>>I've looked again at the code in question, but could not find >>>>>>>>>>>>>>anything that is obviously wrong there. Locality is now >>>>>>>>>>>>>>requested/released at slightly different >>>>>>>>>>>>>>points in the process than >>>>>>>>>>>>>>before, but that's it. It does not seem to >>>>>>>>>>>>>>cause problems with the >>>>>>>>>>>>>>majority of TPMs, since you are the first to report any, so >>>>>>>>>>>>maybe it >>>>>>>>>>>>>>is a quirk that only affects this device. >>>>>>>>>>>>>> >>>>>>>>>>>>>>Perhaps Jerry can help, since this is his change? >>>>>>>>>>>>>> >>>>>>>>>>>>>>Alexander >>>>>>>>>>>>>Getting some caffeine in me, and starting to >>>>>>>>>>>>>take a look. Adding >>>>>>>>>>>>>Jarkko as well since this might involve the >>>>>>>>>>>>>general locality changes. >>>>>>>>>>>>> >>>>>>>>>>>>>Laurent, if I send you a patch with some >>>>>>>>>>>>>debugging code added, would >>>>>>>>>>>>>you be able to run it on that system? I wasn't >>>>>>>>>>>>>running into issues >>>>>>>>>>>>>on the system I had with a 1.2 device, but I >>>>>>>>>>>>>no longer have access >>>>>>>>>>>>>to it. I'll see if I can find one in our labs >>>>>>>>>>>>>and reproduce it there. >>>>>>>>>>>>Yes I should be able to do that >>>>>>>>>>>Any findings? >>>>>>>>>>> >>>>>>>>>>>/Jarkko >>>>>>>>>>Okay, finally getting back to this. Looking at the >>>>>>>>>>code it isn't clear >>>>>>>>>>to me >>>>>>>>>>why the change is causing this. So while I stare at this some more >>>>>>>>>>Laurent >>>>>>>>>>could you reproduce it with this patch so I can see >>>>>>>>>>what the status and >>>>>>>>>>access registers look like? Does anyone else on here >>>>>>>>>>happen to have a >>>>>>>>>>Sinosun >>>>>>>>>>tpm device? The systems I have access to with TPM1.2 >>>>>>>>>>devices don't have >>>>>>>>>>this >>>>>>>>>>issue. >>>>>>>>>> >>>>>>>>>>--8<-- >>>>>>>>>> >>>>>>>>>>diff --git a/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>>b/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>>index fdde971bc810..7d60a7e4b50a 100644 >>>>>>>>>>--- a/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>>+++ b/drivers/char/tpm/tpm_tis_core.c >>>>>>>>>>@@ -258,6 +258,7 @@ static int >>>>>>>>>>tpm_tis_send_data(struct tpm_chip *chip, >>>>>>>>>>const u8 *buf, size_t len) >>>>>>>>>> int rc, status, burstcnt; >>>>>>>>>> size_t count = 0; >>>>>>>>>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >>>>>>>>>>+ u8 access; >>>>>>>>>> >>>>>>>>>> status = tpm_tis_status(chip); >>>>>>>>>> if ((status & TPM_STS_COMMAND_READY) == 0) { >>>>>>>>>>@@ -292,6 +293,11 @@ static int >>>>>>>>>>tpm_tis_send_data(struct tpm_chip *chip, >>>>>>>>>>const u8 *buf, size_t len) >>>>>>>>>> } >>>>>>>>>> status = tpm_tis_status(chip); >>>>>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >>>>>>>>>>+ rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>>>>>>>>>&access); >>>>>>>>>>+ if (rc < 0) >>>>>>>>>>+ dev_info(&chip->dev, >>>>>>>>>>"TPM_STS_DATA_EXPECT == 0: read >>>>>>>>>>failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>>>>>+ else >>>>>>>>>>+ dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >>>>>>>>>>locality: %d status: %x access: %x\n", >>>>>>>>>>priv->locality, status, access); >>>>>>>>>> rc = -EIO; >>>>>>>>>> goto out_err; >>>>>>>>>> } >>>>>>>>>>@@ -309,6 +315,11 @@ static int >>>>>>>>>>tpm_tis_send_data(struct tpm_chip *chip, >>>>>>>>>>const u8 *buf, size_t len) >>>>>>>>>> } >>>>>>>>>> status = tpm_tis_status(chip); >>>>>>>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >>>>>>>>>>+ rc = tpm_tis_read8(priv, >>>>>>>>>>TPM_ACCESS(priv->locality), &access); >>>>>>>>>>+ if (rc < 0) >>>>>>>>>>+ dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >>>>>>>>>>failure TPM_ACCESS(%d)\n", priv->locality); >>>>>>>>>>+ else >>>>>>>>>>+ dev_info(&chip->dev, >>>>>>>>>>"TPM_STS_DATA_EXPECT != 0: locality: >>>>>>>>>>%d status: %x access: %x\n", priv->locality, status, access); >>>>>>>>>> rc = -EIO; >>>>>>>>>> goto out_err; >>>>>>>>>> } >>>>>>>>>Please find here the dmesg output of the patched kernel >>>>>>>>At least 0xff is corrupted value in senseful way. CPU fills the read >>>>>>>>with ones for example for unaligned bus read. See table >>>>>>>>19 in PC client >>>>>>>>spec. This can happen when you do unaligned read for example. >>>>>>>> >>>>>>>>Maybe TPM is unreachable i.e. powered off. Bit busy with >>>>>>>>stuff ATM but >>>>>>>>would probably make sense to compare that 0x81 to table >>>>>>>>18 in the same >>>>>>>>spec. >>>>>>>> >>>>>>>>/Jarkko >>>>>>>0x81 is saying the access register status is valid, and the locality >>>>>>>is not active. That first bit means A Dynamic OS has not >>>>>>>been previously >>>>>>>established on the platform. Normally we would see 0xa1, which would >>>>>>>mean valid register status, and the locality is active. >>>>>>I think the important thing to note here is that STS has bits set that >>>>>>should never be set. So we can conclude that TPM might be either >>>>>> >>>>>>1. Powered off >>>>>>2. In some transition state? >>>>>> >>>>>>/Jarkko >>>>>If it was powered off would we be getting a valid read from the access >>>>>register? >>>>I think there is no universal answer to that :-) >>>> >>>>Maybe adding a extra delay would be next test to make? If for random >>>>reason it is in-between states... >>>Any more ideas? >>> >>>The chip is definitely in a weird state :/ I tried several ways to >>>reset the chip (windows, tpm-tools,...). >>> >>>I've been able to reset the chip via the bios (which now shows >>>unowned) but chip is still locked apparently. >>> >>>But still with < 4.12 I'm able to get /some/ information Public >>>EK, PCR,... out of the chip so it's not completely broken... >> >>OK correction the TPM is now unlocked (I let the computer running >>for more than 24h with nothing accessing the TPM) and with 4.9 I've >>been able to take the ownership again. >> >>Under 4.12 I still have the same errors as mentioned originally > > >Still thinking about how to attack this. I don't think extending the timeout >would change anything because it is reading the register, and thinking the >status is valid because it reads 0xff. So it would still have that problem, >right? > >Maybe a sanity check could be added to wait_for_tpm_stat() to >first check that it didn't read 0xff before checking for the value it >is looking for. Would that make sense Jarkko? > >diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c >index 1d6729be4cd6..8b77fa2003ce 100644 >--- a/drivers/char/tpm/tpm-interface.c >+++ b/drivers/char/tpm/tpm-interface.c >@@ -1087,7 +1087,7 @@ int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask, unsigned long timeout, > do { > tpm_msleep(TPM_TIMEOUT); > status = chip->ops->status(chip); >- if ((status & mask) == mask) >+ if ((status != 0xff) && (status & mask) == mask) > return 0; > } while (time_before(jiffies, stop)); > } > Actually I'm reading through the tpm profile spec again, and 6.1 FIFO Interface Locality Usage per Register has a table that I think explains what is going on. Since the access register is showing 0x81, which means access register bits are valid and the locality is not active. Table 39 in 6.1 says in the case where active locality is not set, or set for another locality, TPM_STS_x register reads return FFh. ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore [not found] ` <20171024135123.uqail7olnespun4k-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> 2017-10-24 14:57 ` Jerry Snitselaar @ 2017-10-25 8:04 ` Laurent Bigonville 1 sibling, 0 replies; 23+ messages in thread From: Laurent Bigonville @ 2017-10-25 8:04 UTC (permalink / raw) To: Jarkko Sakkinen, Jerry Snitselaar Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA, tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f Le 24/10/17 à 15:51, Jarkko Sakkinen a écrit : > On Mon, Oct 23, 2017 at 06:45:15AM -0700, Jerry Snitselaar wrote: >> On Mon Oct 23 17, Jarkko Sakkinen wrote: >>> On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote: >>>> Le 14/10/17 à 10:13, Jerry Snitselaar a écrit : >>>>> On Wed Sep 06 17, Jarkko Sakkinen wrote: >>>>>> On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote: >>>>>>> Le 31/08/17 à 18:40, Jerry Snitselaar a écrit : >>>>>>>> On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote: >>>>>>>>>> Le 29/08/17 à 18:35, Laurent Bigonville a écrit : >>>>>>>>>>> Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit : >>>>>>>>>>>>> An idea how to troubleshoot this? >>>>>>>>>>>> Can you run git bisect on the changes between 4.11 and >>>>>>> 4.12, so that >>>>>>>>>>>> we find the offending commit? It is probably sufficient >>>>>>> to limit the >>>>>>>>>>>> search to commits that touch something in drivers/char/tpm. >>>>>>>>>>> I'll try and keep you posted. >>>>>>>>>> OK I've been able to bisect the problem and the bad commit is: >>>>>>>>>> >>>>>>>>>> e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit >>>>>>>>>> commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 >>>>>>>>>> Author: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>> Date: Mon Mar 27 08:46:04 2017 -0700 >>>>>>>>>> >>>>>>>>>> tpm_tis: convert to using locality callbacks >>>>>>>>>> >>>>>>>>>> This patch converts tpm_tis to use of the new tpm class ops >>>>>>>>>> request_locality, and relinquish_locality. >>>>>>>>>> >>>>>>>>>> With the move to using the callbacks, release_locality is >>>>>>>>>> changed so >>>>>>>>>> that we now release the locality even if there is no >>>>>>>>>> request pending. >>>>>>>>>> >>>>>>>>>> This required some changes to the tpm_tis_core_init >>>>>>> code path to >>>>>>>>>> make sure locality is requested when needed: >>>>>>>>>> >>>>>>>>>> - tpm2_probe code path will end up calling >>>>>>>>>> request/release through >>>>>>>>>> callbacks, so request_locality prior to >>>>>>> tpm2_probe not needed. >>>>>>>>>> - probe_itpm makes calls to tpm_tis_send_data which no >>>>>>>>>> longer calls >>>>>>>>>> request_locality, so add request_locality prior to >>>>>>>>>> tpm_tis_send_data >>>>>>>>>> calls. Also drop release_locality call in middleof >>>>>>>>>> probe_itpm, and >>>>>>>>>> keep locality until release_locality called at end of >>>>>>>>>> probe_itpm. >>>>>>>>>> >>>>>>>>>> Cc: Peter Huewe <peterhuewe@gmx.de> >>>>>>>>>> Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>>> Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> >>>>>>>>>> Cc: Marcel Selhorst <tpmdd@selhorst.net> >>>>>>>>>> Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com> >>>>>>>>>> Reviewed-by: Jarkko Sakkinen >>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>> Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> >>>>>>>>>> Signed-off-by: Jarkko Sakkinen >>>>>>> <jarkko.sakkinen@linux.intel.com> >>>>>>>>>> :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44 >>>>>>>>>> 72f21b446e45ea1003de75902b0553deb99157fd M drivers >>>>>>>>>> >>>>>>>>> I've looked again at the code in question, but could not find >>>>>>>>> anything that is obviously wrong there. Locality is now >>>>>>>>> requested/released at slightly different points in the process than >>>>>>>>> before, but that's it. It does not seem to cause problems with the >>>>>>>>> majority of TPMs, since you are the first to report any, so >>>>>>> maybe it >>>>>>>>> is a quirk that only affects this device. >>>>>>>>> >>>>>>>>> Perhaps Jerry can help, since this is his change? >>>>>>>>> >>>>>>>>> Alexander >>>>>>>> Getting some caffeine in me, and starting to take a look. Adding >>>>>>>> Jarkko as well since this might involve the general locality changes. >>>>>>>> >>>>>>>> Laurent, if I send you a patch with some debugging code added, would >>>>>>>> you be able to run it on that system? I wasn't running into issues >>>>>>>> on the system I had with a 1.2 device, but I no longer have access >>>>>>>> to it. I'll see if I can find one in our labs and reproduce it there. >>>>>>> Yes I should be able to do that >>>>>> Any findings? >>>>>> >>>>>> /Jarkko >>>>> Okay, finally getting back to this. Looking at the code it isn't clear >>>>> to me >>>>> why the change is causing this. So while I stare at this some more >>>>> Laurent >>>>> could you reproduce it with this patch so I can see what the status and >>>>> access registers look like? Does anyone else on here happen to have a >>>>> Sinosun >>>>> tpm device? The systems I have access to with TPM1.2 devices don't have >>>>> this >>>>> issue. >>>>> >>>>> --8<-- >>>>> >>>>> diff --git a/drivers/char/tpm/tpm_tis_core.c >>>>> b/drivers/char/tpm/tpm_tis_core.c >>>>> index fdde971bc810..7d60a7e4b50a 100644 >>>>> --- a/drivers/char/tpm/tpm_tis_core.c >>>>> +++ b/drivers/char/tpm/tpm_tis_core.c >>>>> @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >>>>> const u8 *buf, size_t len) >>>>> int rc, status, burstcnt; >>>>> size_t count = 0; >>>>> bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND; >>>>> + u8 access; >>>>> >>>>> status = tpm_tis_status(chip); >>>>> if ((status & TPM_STS_COMMAND_READY) == 0) { >>>>> @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >>>>> const u8 *buf, size_t len) >>>>> } >>>>> status = tpm_tis_status(chip); >>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) { >>>>> + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), >>>>> &access); >>>>> + if (rc < 0) >>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read >>>>> failure TPM_ACCESS(%d)\n", priv->locality); >>>>> + else >>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: >>>>> locality: %d status: %x access: %x\n", priv->locality, status, access); >>>>> rc = -EIO; >>>>> goto out_err; >>>>> } >>>>> @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip *chip, >>>>> const u8 *buf, size_t len) >>>>> } >>>>> status = tpm_tis_status(chip); >>>>> if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) { >>>>> + rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), &access); >>>>> + if (rc < 0) >>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read >>>>> failure TPM_ACCESS(%d)\n", priv->locality); >>>>> + else >>>>> + dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: locality: >>>>> %d status: %x access: %x\n", priv->locality, status, access); >>>>> rc = -EIO; >>>>> goto out_err; >>>>> } >>>> Please find here the dmesg output of the patched kernel >>> At least 0xff is corrupted value in senseful way. CPU fills the read >>> with ones for example for unaligned bus read. See table 19 in PC client >>> spec. This can happen when you do unaligned read for example. >>> >>> Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but >>> would probably make sense to compare that 0x81 to table 18 in the same >>> spec. >>> >>> /Jarkko >> 0x81 is saying the access register status is valid, and the locality >> is not active. That first bit means A Dynamic OS has not been previously >> established on the platform. Normally we would see 0xa1, which would >> mean valid register status, and the locality is active. > I think the important thing to note here is that STS has bits set that > should never be set. So we can conclude that TPM might be either > > 1. Powered off > 2. In some transition state? MMhhhh It's actually possible that windows has blocked the chip after trying to enter automatically a wrong owner passphrase. But that doesn't explain device was showing up with previous kernel versions ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: tpm device not showing up in /dev anymore
@ 2017-09-06 5:09 Jerry Snitselaar
0 siblings, 0 replies; 23+ messages in thread
From: Jerry Snitselaar @ 2017-09-06 5:09 UTC (permalink / raw)
To: Jarkko Sakkinen, Laurent Bigonville
Cc: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
[-- Attachment #1.1: Type: text/plain, Size: 4653 bytes --]
I haven't had a chance to dive into more. My wife had surgery on Friday and I've been tied up with that. My thought is to dump a couple registers and look at the state when it hits this. Was hoping to poke at it tomorrow, but back at the hospital tonight so it might be a couple more days before I have a chance. It should have locality when it hits that point, but wondering if it doesn't.
Sent via the Samsung Galaxy S® 6, an AT&T 4G LTE smartphone
-------- Original message --------From: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> Date: 9/5/17 9:06 PM (GMT-07:00) To: Laurent Bigonville <bigon@debian.org> Cc: Jerry Snitselaar <jsnitsel@redhat.com>, Alexander.Steffen@infineon.com, tpmdd-devel@lists.sourceforge.net Subject: Re: [tpmdd-devel] tpm device not showing up in /dev anymore
On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote:
> Le 31/08/17 à 18:40, Jerry Snitselaar a écrit :
> > On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote:
> > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit :
> > > > > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a écrit :
> > > > >>> An idea how to troubleshoot this?
> > > > >> Can you run git bisect on the changes between 4.11 and 4.12, so that
> > > > >> we find the offending commit? It is probably sufficient to limit the
> > > > >> search to commits that touch something in drivers/char/tpm.
> > > > >
> > > > > I'll try and keep you posted.
> > > >
> > > > OK I've been able to bisect the problem and the bad commit is:
> > > >
> > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first bad commit
> > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3
> > > > Author: Jerry Snitselaar <jsnitsel@redhat.com>
> > > > Date: Mon Mar 27 08:46:04 2017 -0700
> > > >
> > > > tpm_tis: convert to using locality callbacks
> > > >
> > > > This patch converts tpm_tis to use of the new tpm class ops
> > > > request_locality, and relinquish_locality.
> > > >
> > > > With the move to using the callbacks, release_locality is
> > > > changed so
> > > > that we now release the locality even if there is no
> > > > request pending.
> > > >
> > > > This required some changes to the tpm_tis_core_init code path to
> > > > make sure locality is requested when needed:
> > > >
> > > > - tpm2_probe code path will end up calling
> > > > request/release through
> > > > callbacks, so request_locality prior to tpm2_probe not needed.
> > > >
> > > > - probe_itpm makes calls to tpm_tis_send_data which no
> > > > longer calls
> > > > request_locality, so add request_locality prior to
> > > > tpm_tis_send_data
> > > > calls. Also drop release_locality call in middleof
> > > > probe_itpm, and
> > > > keep locality until release_locality called at end of
> > > > probe_itpm.
> > > >
> > > > Cc: Peter Huewe <peterhuewe@gmx.de>
> > > > Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
> > > > Cc: Marcel Selhorst <tpmdd@selhorst.net>
> > > > Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
> > > > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > > >
> > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44
> > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers
> > > >
> > >
> > > I've looked again at the code in question, but could not find
> > > anything that is obviously wrong there. Locality is now
> > > requested/released at slightly different points in the process than
> > > before, but that's it. It does not seem to cause problems with the
> > > majority of TPMs, since you are the first to report any, so maybe it
> > > is a quirk that only affects this device.
> > >
> > > Perhaps Jerry can help, since this is his change?
> > >
> > > Alexander
> >
> > Getting some caffeine in me, and starting to take a look. Adding
> > Jarkko as well since this might involve the general locality changes.
> >
> > Laurent, if I send you a patch with some debugging code added, would
> > you be able to run it on that system? I wasn't running into issues
> > on the system I had with a 1.2 device, but I no longer have access
> > to it. I'll see if I can find one in our labs and reproduce it there.
>
> Yes I should be able to do that
Any findings?
/Jarkko
[-- Attachment #1.2: Type: text/html, Size: 6691 bytes --]
[-- Attachment #2: Type: text/plain, Size: 202 bytes --]
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
[-- Attachment #3: Type: text/plain, Size: 192 bytes --]
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
^ permalink raw reply [flat|nested] 23+ messages in threadend of thread, other threads:[~2017-11-10 2:19 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-28 11:28 tpm device not showing up in /dev anymore Laurent Bigonville
[not found] ` <f9526f55-df96-64fc-a4d6-877ce04e7156-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-08-29 16:00 ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
[not found] ` <dcad0104c46d4d5f88e642862bdb42c2-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>
2017-08-29 16:35 ` Laurent Bigonville
[not found] ` <47c4300b-8701-79a6-1c58-3a5853f4c5e3-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-08-29 17:39 ` Peter Huewe
2017-08-29 18:55 ` Laurent Bigonville
[not found] ` <595efb25-8d87-f39d-037f-9c9a98462339-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-08-31 12:10 ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
[not found] ` <857106e4bb864bb8a68b1381fffc8f50-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>
2017-08-31 16:40 ` Jerry Snitselaar
2017-09-01 12:10 ` Laurent Bigonville
[not found] ` <0d9be244-ace0-030d-6ff9-c4e94c63b7e9-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-09-06 4:05 ` Jarkko Sakkinen
[not found] ` <20170906040555.fqedhmo5277sd6fq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-10-14 8:13 ` Jerry Snitselaar
2017-10-21 8:53 ` Laurent Bigonville
[not found] ` <b63b765d-2477-d9fe-9d80-c2ea7e582bce-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-10-23 13:23 ` Jarkko Sakkinen
[not found] ` <20171023132346.jbqgokwv3ah2oqjo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-10-23 13:45 ` Jerry Snitselaar
2017-10-23 13:48 ` Laurent Bigonville
2017-10-24 13:51 ` Jarkko Sakkinen
[not found] ` <20171024135123.uqail7olnespun4k-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-10-24 14:57 ` Jerry Snitselaar
2017-10-24 16:07 ` Jarkko Sakkinen
[not found] ` <20171024160725.r6kj452jdzpkbb6o-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-11-09 0:04 ` Laurent Bigonville
[not found] ` <8f4df9a9-c8cd-832f-4c3f-5305fabab7a8-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-11-09 19:58 ` Laurent Bigonville
[not found] ` <0a6e4771-f871-b3ca-b5b0-26dbd9efa8b1-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-11-09 23:50 ` Jerry Snitselaar
[not found] ` <20171109235005.pmklfw6zkjirahfa-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-11-10 2:19 ` Jerry Snitselaar
2017-10-25 8:04 ` Laurent Bigonville
2017-09-06 5:09 Jerry Snitselaar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).