On 8/9/21 6:55 AM, Jarkko Sakkinen wrote: > On Fri, Aug 06, 2021 at 04:18:08PM +0200, Borys Movchan wrote: >> If something went wrong during the TPM firmware upgrade, like power >> failure or the firmware image file get corrupted, the TPM might end >> up in Upgrade or Failure mode upon the next start. The state is >> persistent between the TPM power cycle/restart. >> >> According to TPM specification: >> * If the TPM is in Upgrade mode, it will answer with TPM2_RC_UPGRADE >> to all commands except Field Upgrade related ones. >> * If the TPM is in Failure mode, it will allow performing TPM >> initialization but will not provide any crypto operations. >> Will happily respond to Field Upgrade calls. >> >> Change the behavior of the tpm2_auto_startup(), so it detects the active >> running mode of the TPM. It is easy to determine that TPM is in Upgrade >> mode by relying on the fact that tpm2_do_selftest() will return >> TPM2_RC_UPGRADE. In such a case, there is no point to finish the >> start-up procedure as the TPM will not accept any commands, except >> firmware upgrade related. >> >> On the other hand, if the TPM is in Failure mode, it will successfully >> respond to both tpm2_do_selftest() and tpm2_startup() calls. Although, >> will fail to answer to tpm2_get_cc_attrs_tbl(). Use this fact to >> conclude that TPM is in Failure mode. >> >> If the chip is in the Upgrade or Failure mode, the function returns -EIO >> error code. >> >> The return value is checked in the tpm_chip_register() call to determine >> the state of the TPM. If the TPM is not in normal operation mode, set >> the `limited_mode` flag. If the flag is set then the TPM is not able to > Nit: do not use hyphens for limited mode. 'limited_mode' is fine. I'm > also fine with just limited_mode. Done >> provide any crypto functionality. Correspondignly, the calls to >> tpm2_get_cc_attrs_tbl(), tpm_add_hwrng() and tpm_get_pcr_allocation() >> will fail. Use the flag to exclude them from the initialization >> sequence. > This is blacklisting. E.g. I'm not sure why all of the sysfs attributes > would still be exported. Some of them use TPM commands. That was just > one random example I came up with. > > It's easy to come up other examples, like, why you provide still tpmrm0, > which is dependent on a TPM running normal mode? > > This misses completely the rationale for ever acking this change: which > parts of the uapi are export and *why*. > > Please whitelist the things that should still work. Even the obvious > ones like /dev/tpm0 (because of TPM_RC_UPGRADE). > > This is clearly a faulty and incomplete patch in its current form. I expected something like this. In new patch version I tried to disable all functionality which should not work in Upgrade/Failure mode. Basically, I am interested in getting /dev/tpm0 working when TPM is in limited mode. In this state, the TPM is not capable to provide any other functionality except firmware upgrade/recovery. So the rest of features should be disabled/removed. Physical reset of the TPM is mandatory part of the firmware upgrade/recovery. Which will lead to driver rebind/reload and reappearance of all interfaces applicable to normal operational mode. > /Jarkko > Kind regards, Borys
