A better approach than adding an ABI might be to land "workaround for Infineon TPM defensive timeout (v2)" upstream and then drop both the log message and the test from kernel_TPMPing.

Agreed with both C#0 and C#1 regarding:
1. landing the CL for now.
2. upstreaming the patch.

Can we fix the kernel version check in the test till #2 happens? Luigi, want to make that change?

Sure!

cl at https://chromium-review.googlesource.com/366133

for the record: kernel_TPMPing also needs to be updated for TPM2.0 systems (it uses tpm_version, which is a part of trousers package, which is 1.2-specific), but that's another bug - filed separately.

A little background:

The reason for having this weird test is that the actual functionality provided by the TPM driver change is difficult to test, since it only affects shutdown, and only if the shutdown happens within 30s of power-on, and only on certain TPM models.

In theory it could be a "server" test (autotest/files/server/site_tests) but it would be a pain to get right because of the timing.

We could just turn the test off.  We are careful about pulling patches.  In any case this one is definitely upstreamable---TPM commands are rare and it should be fine to wait for their completion even with less picky TPMs.

Ah, missed all those comments when I posted <https://chromium-review.googlesource.com/366135>.

In any case, either way is fine with me so long as we get this test passing in short order.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/845c95c12fa8cd42f3f9679cde48bc9eea1cab2f

commit 845c95c12fa8cd42f3f9679cde48bc9eea1cab2f
Author: Douglas Anderson <dianders@chromium.org>
Date: Thu Aug 04 19:49:22 2016

autotest: Disable kernel_TPMPing until it works more reliably

This test wasn't running on 3.10, 3.14, or 3.18 anyway due to an
incorrect version check.  Then it was failing on 4.4, but because of
missing log spam and not because of a missing feature.  We could fix the
version check and add the log spam to 4.4 but it sounds like folks would
like a better way to check this than extra kernel log spam anyway.  For
now we'll disable.

BUG=chromium:634341
TEST=emerge-${BOARD} -pv autotest-tests-tpm

Change-Id: Icbd1ed3d79e9d84fe490ca3037e0ebf7a4cbd4d2
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/366135
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/845c95c12fa8cd42f3f9679cde48bc9eea1cab2f/chromeos-base/autotest-tests-tpm/autotest-tests-tpm-9999.ebuild

Above avoids the error, but we should keep this bug open unless we actually want to delete the test.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/8dd8f10effe9e9b76cbe8f5f36a7702470d5e981

commit 8dd8f10effe9e9b76cbe8f5f36a7702470d5e981
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Mon Mar 10 20:55:53 2014

FIXUP: CHROMIUM: TPM: log "gentle shutdown" notice on all platforms.

We logged this on x86 only, thus making kernel_TPMPing fail on ARM.

BUG= chromium:351054 
TEST=none
BRANCH=none
Signed-off-by: Luigi Semenzato <semenzato@chromium.org>

Original-Change-Id: I0dab38bd87e6d925bda6b5302b8f8b1014e84a75
Originally-Reviewed-on: https://chromium-review.googlesource.com/189449
Tested-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Olof Johansson <olofj@chromium.org>
Commit-Queue: Luigi Semenzato <semenzato@chromium.org>

[This wasn't picked into v3.14, etc.; I guess no one is running
kernel_TPMPing ?]

TEST=kernel_TPMPing
BUG=chromium:634341

Change-Id: Ib71da0ffd6c88967ee0134dc595ea995bea3a9e9
Fixes: d479a0935525 ("CHROMIUM: workaround for Infineon TPM defensive timeout (v2)")
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
Reviewed-on: https://chromium-review.googlesource.com/366182
Commit-Ready: Brian Norris <briannorris@chromium.org>
Tested-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Reviewed-by: Douglas Anderson <dianders@chromium.org>

[modify] https://crrev.com/8dd8f10effe9e9b76cbe8f5f36a7702470d5e981/drivers/char/tpm/tpm_tis_core.c
[modify] https://crrev.com/8dd8f10effe9e9b76cbe8f5f36a7702470d5e981/drivers/char/tpm/tpm-chip.c

The way we ensure gentle shutdown upstream will depend on the outcome of https://patchwork.kernel.org/patch/9516631/. It will be through a bit different mechanism (setting chip->ops to NULL instead of holding the mutex) and may happen in (a) driver's shutdown handler, or (b) in newly introduced device class level shutdown handler, or (c) in reboot notifier hook. For now it's moved to shutdown handler: https://chromium-review.googlesource.com/#/c/428295/

Activating. Please assign to the right owner and the appropriate priority.

In kernel 4.12, TPM 2.0 got gentle shutdown equivalent from upstream. Adding the same for TPM 1.2 is tracked in issue 764024.

Kernel 4.12 no longer prints "gentle shutdown", so kernel_TPMPing should not run for 4.12.
https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/366133 shall be modified to check for the kernel to be in range [3.8 .. 4.4].

In 4.14, the Shutdown command is sent from the common class shutdown handler shared by all tpm drivers. This test is obsolete. Uploaded the updated https://crrev.com/c/366133 that limits the 'gentle shutdown' check to kernels in 3.8..4.4 range.

The test was there only to make sure we would remember to port the gentle shutdown to newer kernels.  If we're only using it on older kernels, it might not make sense, and maybe then it should just go?

Yes, can kill this check altogether if nobody sees any benefit in keeping it for 4.4 (and earlier). There is already another CL ready for that - https://crrev.com/c/368719 :-)

The test itself, as I understand, does the basic check of TPM communications from userland (calls tpm_version and checks that it does print the version obtained from the TPM). That still sounds useful (e.g. see b/71722449 where it failed before doing the gentle check), so the test should stay.

After the discussion on the CLs, the new approach re the 'gentle shutdown' check:
 - We don't need this check for TPM 2.0: for 4.4 we know we had it covered in cr50 drivers, in 4.14 the fix is available upstream.
 - The check for TPM 1.2 in 3.8..4.4 is optional. The drivers for the TPM chips we use have the 'gentle shutdown' patches.
 - The check for TPM 1.2 in 4.14+ is still required. The 1.2 case is not fixed upstream. The 1.2 drivers need to make use of the new class shutdown handler. The patch for that is to be submitted upstream (issue 819268).

Bottomline: The test needs to be modified to do the 'gentle shutdown' check for all kernels starting with 3.8 w/o an upper boundary (as it does now, though the version check code needs to be fixed, while we are on it), but only for 1.2 chips. For 2.0 chips we don't need the check.

Later, when the 1.2 case is fixed, the upper boundary for the tpm 1.2 check should be set.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/78eef7d5c05cff3444a3c79272d7d4cdc2bd1323

commit 78eef7d5c05cff3444a3c79272d7d4cdc2bd1323
Author: Luigi Semenzato <semenzato@chromium.org>
Date: Thu Mar 15 22:27:21 2018

kernel_TPMPing: fix conditions for gentle shutdown test

1) Fix version check. The comparison has been wrong all along.
2) Restrict running the gentle shutdown checks to TPM 1.2 chips:
   for TPM 2.0, on kernels >= 4.12 the shutdown fix is present
   upstream and doesn't require special 'gentle shutdown' handling.
3) Update the test description in the 'control' file.
4) Fix pylint warnings.

BUG=chromium:634341
BUG=b:74203647
TEST=checked that correctly skips the gentle shutdown check if
     kernel version or TPM spec version are not supported and
     runs it otherwise.

Change-Id: I3b3959a54faf64e6c916d3a682ac249d5ce9dd18
Signed-off-by: Andrey Pronin <apronin@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/366133
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Reviewed-by: Guenter Roeck <groeck@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>

[modify] https://crrev.com/78eef7d5c05cff3444a3c79272d7d4cdc2bd1323/client/site_tests/kernel_TPMPing/kernel_TPMPing.py
[modify] https://crrev.com/78eef7d5c05cff3444a3c79272d7d4cdc2bd1323/client/site_tests/kernel_TPMPing/control

Guys, this bug is in a bad state. It hasn't moved in months, I (sheriff) just wasted a decent chunk of time trying to figure out what's wrong with this test, and both I and the lab deputy where baffled as to why the test doesn't run in bvt-cq even though it seems to be clearly marked for that.

If the test is supposed to be disabled atm, it should be disabled by commenting out ATTRIBUTES = "suite:bvt-cq" in the control file. Hacking the autotest ebuild (like https://crosreview.com/366135) seems like a completely arcane and counter-intuitive way to control test execution. Nobody is going to look there to check whether the test is currently enabled, and people running test_that manually will still run the test (due to autotest_quickmerge, I think) and be very confused why they get a bvt failure from a test that apparently hasn't worked correctly on 3.18 for two years. This includes OEMs who use helper scripts like cros_run_bvt (see b:110971155).

Also, if this test actually was important to anyone, it should probably be fixed and reintroduced into the actual bvt at some point. The fact that it has been disabled for 2 years makes me think we should probably just delete it.

Sorry about this---I fixed the bug but didn't revert Doug's change in #8.

The test is supposed to be fixed---do you have any pointer to failures?

1) I know of one failure: http://b/110971155. Do we have something on CQ also? If so, looks like something has changed recently.

2) The test was fixed in https://crrev.com/c/366133 by running it only for TPM 1.2 and on kernels >= 3.8. We don't require spam in logs for future devices with 2.0, only for the old ones (where it was supposedly passing). The whole gentle shutdown situation was fixed upstream for TPM 2.0, but not for TPM 1.2 in general case (issue 819268). So, the test is still valid for devices with TPM 1.2, in the unlikely case we add a new one. Do we keep adding devices with TPM 1.2?

3) I haven't been aware of any previous issues on the already existing device on 3.8. From #21, turns out we just weren't running the test for (some of?) them before. 
But that doesn't explain why it fails, if run. From what I see, we *do* have gentle shutdown printouts in 3.8 drivers in src/third_party/kernel/v3.8/drivers/char/tpm/tpm.c. 

The test can be disabled for *all* future devices only once issue 819268 is fixed upstream and in chromeos. So, setting as dependency.

For #21: the M-70 target is for correctly enabling/disabling the kernel_TPMPing test. So, created a separate bug for that and set P1, M70 there: issue 874281.

Both the generic tpm driver, tpm_infineon and tpm_i2c_infineon drivers install correct shutdown handlers for 3.8, 3.10, 3.18, though not all paths print "gentle shutdown".

For 4.4, only tpm_i2c_infineon installs the right handler. For 4.14+ we need the fix from issue 819268. So, we can further restrict the check in kernel_TPMPing to "if kernel_version >= [4, 4] and spec_version != [2, 0]" to avoid false positives for older kernels.