| Message ID | 1426659434-4003-1-git-send-email-jarkko.sakkinen@linux.intel.com |
|---|---|
| State | Accepted |
| Headers | show |
On Wed, 18 Mar 2015, Jarkko Sakkinen wrote: > I started to work with PPI interface so that it would be available > under character device sysfs directory and realized that chip > registeration was still too messy. > > In TPM 1.x in some rare scenarios (errors that almost never occur) > wrong order in deinitialization steps was taken in teardown. I > reproduced these scenarios by manually inserting error codes in the > place of the corresponding function calls. > > The key problem is that the teardown is messy with two separate code > paths (this was inherited when moving code from tpm-interface.c). > > Moved TPM 1.x specific register/unregister functionality to own helper > functions and added single code path for teardown in tpm_chip_register(). > Now the code paths have been fixed and it should be easier to review > later on this part of the code. > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> CONFIG_TCG_TPM=y CONFIG_TCG_TIS=y # CONFIG_TCG_TIS_I2C_ATMEL is not set # CONFIG_TCG_TIS_I2C_INFINEON is not set # CONFIG_TCG_TIS_I2C_NUVOTON is not set # CONFIG_TCG_NSC is not set # CONFIG_TCG_ATMEL is not set # CONFIG_TCG_INFINEON is not set # CONFIG_TCG_CRB is not set [0.236145] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16) [0.292769] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working, polling instead and suspend/resume continue to function. Tested-by: Scot Doyle <lkml14@scotdoyle.com> ------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
On Wed, 2015-03-18 at 16:05 +0000, Scot Doyle wrote: > On Wed, 18 Mar 2015, Jarkko Sakkinen wrote: > > I started to work with PPI interface so that it would be available > > under character device sysfs directory and realized that chip > > registeration was still too messy. > > > > In TPM 1.x in some rare scenarios (errors that almost never occur) > > wrong order in deinitialization steps was taken in teardown. I > > reproduced these scenarios by manually inserting error codes in the > > place of the corresponding function calls. > > > > The key problem is that the teardown is messy with two separate code > > paths (this was inherited when moving code from tpm-interface.c). > > > > Moved TPM 1.x specific register/unregister functionality to own helper > > functions and added single code path for teardown in tpm_chip_register(). > > Now the code paths have been fixed and it should be easier to review > > later on this part of the code. > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > > CONFIG_TCG_TPM=y > CONFIG_TCG_TIS=y > # CONFIG_TCG_TIS_I2C_ATMEL is not set > # CONFIG_TCG_TIS_I2C_INFINEON is not set > # CONFIG_TCG_TIS_I2C_NUVOTON is not set > # CONFIG_TCG_NSC is not set > # CONFIG_TCG_ATMEL is not set > # CONFIG_TCG_INFINEON is not set > # CONFIG_TCG_CRB is not set > > [0.236145] tpm_tis 00:08: 1.2 TPM (device-id 0xB, rev-id 16) > [0.292769] tpm_tis 00:08: [Firmware Bug]: TPM interrupt not working, polling instead > > and suspend/resume continue to function. > > Tested-by: Scot Doyle <lkml14@scotdoyle.com> Thanks! /Jarkko ------------------------------------------------------------------------------ Dive into the World of Parallel Programming The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net/
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index e096e9c..283f00a 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -170,6 +170,41 @@ static void tpm_dev_del_device(struct tpm_chip *chip) device_unregister(&chip->dev); } +static int tpm1_chip_register(struct tpm_chip *chip) +{ + int rc; + + if (chip->flags & TPM_CHIP_FLAG_TPM2) + return 0; + + rc = tpm_sysfs_add_device(chip); + if (rc) + return rc; + + rc = tpm_add_ppi(chip); + if (rc) { + tpm_sysfs_del_device(chip); + return rc; + } + + chip->bios_dir = tpm_bios_log_setup(chip->devname); + + return 0; +} + +static void tpm1_chip_unregister(struct tpm_chip *chip) +{ + if (chip->flags & TPM_CHIP_FLAG_TPM2) + return; + + if (chip->bios_dir) + tpm_bios_log_teardown(chip->bios_dir); + + tpm_remove_ppi(chip); + + tpm_sysfs_del_device(chip); +} + /* * tpm_chip_register() - create a character device for the TPM chip * @chip: TPM chip to use. @@ -185,22 +220,13 @@ int tpm_chip_register(struct tpm_chip *chip) { int rc; - /* Populate sysfs for TPM1 devices. */ - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { - rc = tpm_sysfs_add_device(chip); - if (rc) - goto del_misc; - - rc = tpm_add_ppi(chip); - if (rc) - goto del_sysfs; - - chip->bios_dir = tpm_bios_log_setup(chip->devname); - } + rc = tpm1_chip_register(chip); + if (rc) + return rc; rc = tpm_dev_add_device(chip); if (rc) - return rc; + goto out_err; /* Make the chip available. */ spin_lock(&driver_lock); @@ -210,10 +236,8 @@ int tpm_chip_register(struct tpm_chip *chip) chip->flags |= TPM_CHIP_FLAG_REGISTERED; return 0; -del_sysfs: - tpm_sysfs_del_device(chip); -del_misc: - tpm_dev_del_device(chip); +out_err: + tpm1_chip_unregister(chip); return rc; } EXPORT_SYMBOL_GPL(tpm_chip_register); @@ -238,13 +262,7 @@ void tpm_chip_unregister(struct tpm_chip *chip) spin_unlock(&driver_lock); synchronize_rcu(); - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { - if (chip->bios_dir) - tpm_bios_log_teardown(chip->bios_dir); - tpm_remove_ppi(chip); - tpm_sysfs_del_device(chip); - } - + tpm1_chip_unregister(chip); tpm_dev_del_device(chip); } EXPORT_SYMBOL_GPL(tpm_chip_unregister);
I started to work with PPI interface so that it would be available under character device sysfs directory and realized that chip registeration was still too messy. In TPM 1.x in some rare scenarios (errors that almost never occur) wrong order in deinitialization steps was taken in teardown. I reproduced these scenarios by manually inserting error codes in the place of the corresponding function calls. The key problem is that the teardown is messy with two separate code paths (this was inherited when moving code from tpm-interface.c). Moved TPM 1.x specific register/unregister functionality to own helper functions and added single code path for teardown in tpm_chip_register(). Now the code paths have been fixed and it should be easier to review later on this part of the code. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- drivers/char/tpm/tpm-chip.c | 66 ++++++++++++++++++++++++++++----------------- 1 file changed, 42 insertions(+), 24 deletions(-)