Bug 16256

Summary: tpm_tis breaks suspend/hibernate on kernels > 2.6.34
Product: Power Management Reporter: Helmut Schaa (helmut.schaa)
Component: Hibernation/SuspendAssignee: power-management_other
Status: CLOSED CODE_FIX    
Severity: normal CC: maciej.rutecki, rjw, srajiv, thomas
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 2.6.35-rc3-wl Subsystem:
Regression: Yes Bisected commit-id:
Bug Depends on:    
Bug Blocks: 7216, 16055    

Description Helmut Schaa 2010-06-20 11:15:48 UTC
I recently switched to 2.6.35-rcX-wl (wireless-testing) for development and discovered that only the first issued suspend/hibernate was successful, every subsequent suspend/hibernate failed with the following in dmesg:

legacy_suspend(): pnp_bus_suspend+0x0/0x6a returns 28
PM: Device 00:02 failed to suspend: error 28
PM: Some devices failed to suspend

Device 00:02 is owned by the tpm_tis driver. Unloading tpm_tis before suspend fixes this issue for me.

I didn't have such problems with kernels <2.6.35 and it seems the culprit is
the patch "TPM: workaround to enforce PCR updates across suspends" which
introduced a return value in tpm_pm_suspend if transmit_cmd fails:

[...]
-       tpm_transmit(chip, savestate, sizeof(savestate));
-       return 0;
+       /* for buggy tpm, flush pcrs with extend to selected dummy */
+       if (tpm_suspend_pcr) {
+               cmd.header.in = pcrextend_header;
+               cmd.params.pcrextend_in.pcr_idx = cpu_to_be32(tpm_suspend_pcr);
+               memcpy(cmd.params.pcrextend_in.hash, dummy_hash,
+                      TPM_DIGEST_SIZE);
+               rc = transmit_cmd(chip, &cmd, EXTEND_PCR_RESULT_SIZE,
+                                 "extending dummy pcr before suspend");
+       }
+
+       /* now do the actual savestate */
+       cmd.header.in = savestate_header;
+       rc = transmit_cmd(chip, &cmd, SAVESTATE_RESULT_SIZE,
+                         "sending savestate before suspend");
+       return rc;
[...]

In the meantime I was able to find a workaround for this issue and sent a PATCH to the tpmdd-devel list [1] but unfortunately got no reply yet. Since I have really _no_ knowledge at all about tpm stuff I'd like to get the patch reviewed first from somebody working on the tpm stuff. 

Thanks,
Helmut

[1] http://sourceforge.net/mailarchive/forum.php?thread_name=201006091511.09810.helmut.schaa%40googlemail.com&forum_name=tpmdd-devel
Comment 1 Rafael J. Wysocki 2010-06-21 22:20:39 UTC
Caused by:

commit 225a9be24d799aa16d543c31fb09f0c9ed1d9caa
Author: Rajiv Andrade <srajiv@linux.vnet.ibm.com>
Date:   Thu Mar 25 00:55:32 2010 -0300

    TPM: workaround to enforce PCR updates across suspends
    
    Add a workaround for TPM's which fail to flush last written
    PCR values in a TPM_SaveState, in preparation for suspend.
    
    Signed-off-by: David Safford <safford@watson.ibm.com>
    Acked-by: Rajiv Andrade <srajiv@linux.vnet.ibm.com>
    Signed-off-by: James Morris <jmorris@namei.org>

First-Bad-Commit : 225a9be24d799aa16d543c31fb09f0c9ed1d9caa
Comment 2 Rajiv Andrade 2010-06-22 03:13:56 UTC
The commit actually did a good job showing us the TPM wasn't saving its state given the non successful TPM_ORD_SaveState command in the second suspend attempt. Just acked and forwared to LKML the patch Helmut submitted to tpmdd-devel that solves this.

Rajiv
Comment 3 Rafael J. Wysocki 2010-06-22 09:00:10 UTC
Can you give us a pointer to the patch, please?
Comment 4 Helmut Schaa 2010-06-22 10:21:51 UTC
Here you are: http://lkml.org/lkml/2010/6/21/463
Comment 5 Rafael J. Wysocki 2010-06-22 10:28:49 UTC
Thanks!

Patch : http://lkml.org/lkml/2010/6/21/463
Handled-By : Helmut Schaa <helmut.schaa@googlemail.com>
Comment 6 Rafael J. Wysocki 2010-06-22 10:36:58 UTC
Patch : http://marc.info/?l=tpmdd-devel&m=127609160616162&w=2
Ignore-Patch : http://lkml.org/lkml/2010/6/21/463
Comment 7 Rafael J. Wysocki 2010-08-01 13:39:58 UTC
Fixed by commit 59f6fbe4291fcc078ba26ce4edf8373a7620a13a .