[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4D6C0814.5020901@linux.vnet.ibm.com>
Date: Mon, 28 Feb 2011 15:39:48 -0500
From: Stefan Berger <stefanb@...ux.vnet.ibm.com>
To: Rajiv Andrade <srajiv@...ux.vnet.ibm.com>,
James Morris <jmorris@...ei.org>
CC: Jiri Slaby <jirislaby@...il.com>,
Linux kernel mailing list <linux-kernel@...r.kernel.org>,
Guillaume Chazarain <guichaz@...il.com>,
akpm@...ux-foundation.org
Subject: Re: [GIT PULL] TPM timeouts and resume fixes
On 02/28/2011 08:46 AM, Rajiv Andrade wrote:
> Hi James,
>
> Please pull from:
> git://tpmdd.git.sourceforge.net/gitroot/tpmdd/tpmdd/ for-james
Unless it already happened, please don't pull. I would like to post a
revised version of the patch that re-enables interrupts after a resume.
Stefan
>
> The first change corrects the logic used to extract timeouts values
> from TPMs.
> This second version includes an adjustment of such values from msecs
> to usecs,
> since some chips return such values in the first form, causing some
> commands
> to fail due the resulting short timeouts.
>
> The second change re-enables TPM interrupts after a resume, making sure
> that the TPM interrupts are enabled when running in the interrupt mode.
> The TPM's interrupt enable register maye have been cleared by
> the TPM's TIS loosing its state during device sleep in ACPI S3 (suspend)
> or by the BIOS, which upon resume sends a TPM_Startup() command to the
> TPM, and may run the TPM in polling mode and leave the TIS interrupts
> disabled once it transfers control to the OS again.
>
> Stefan Berger (2):
> tpm_tis: Use timeouts returned from TPM
> tpm_tis: Re-enable interrupts upon resume
>
> drivers/char/tpm/tpm.c | 40
> +++++++++++++++++++++++++++++++---------
> drivers/char/tpm/tpm.h | 3 +++
> drivers/char/tpm/tpm_tis.c | 21 ++++++++++++++++++++-
> 3 files changed, 54 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 36e0fa1..a92254e 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -577,23 +577,30 @@ duration:
> if (rc)
> return;
>
> - if (be32_to_cpu(tpm_cmd.header.out.return_code)
> - != 3 * sizeof(u32))
> + if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
> + be32_to_cpu(tpm_cmd.header.out.length)
> + != sizeof(tpm_cmd.header.out) + sizeof(u32) + 3 * sizeof(u32))
> return;
> +
> duration_cap =&tpm_cmd.params.getcap_out.cap.duration;
> chip->vendor.duration[TPM_SHORT] =
> usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_short));
> - /* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the
> above
> - * value wrong and apparently reports msecs rather than usecs. So we
> - * fix up the resulting too-small TPM_SHORT value to make things
> work.
> - */
> - if (chip->vendor.duration[TPM_SHORT]< (HZ/100))
> - chip->vendor.duration[TPM_SHORT] = HZ;
> -
> chip->vendor.duration[TPM_MEDIUM] =
> usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_medium));
> chip->vendor.duration[TPM_LONG] =
> usecs_to_jiffies(be32_to_cpu(duration_cap->tpm_long));
> +
> + /* The Broadcom BCM0102 chipset in a Dell Latitude D820 gets the
> above
> + * value wrong and apparently reports msecs rather than usecs. So we
> + * fix up the resulting too-small TPM_SHORT value to make things
> work.
> + */
> + if (chip->vendor.duration[TPM_SHORT]< (HZ/100)) {
> + chip->vendor.duration[TPM_SHORT] = HZ;
> + chip->vendor.duration[TPM_MEDIUM] *= 1000;
> + chip->vendor.duration[TPM_LONG] *= 1000;
> + chip->vendor.duration_adjusted = true;
> + dev_info(chip->dev, "Adjusting TPM timeout parameters.");
> + }
> }
> EXPORT_SYMBOL_GPL(tpm_get_timeouts);
> @@ -939,6 +946,21 @@ ssize_t tpm_show_caps_1_2(struct device * dev,
> }
> EXPORT_SYMBOL_GPL(tpm_show_caps_1_2);
> +ssize_t tpm_show_timeouts(struct device *dev, struct device_attribute
> *attr,
> + char *buf)
> +{
> + struct tpm_chip *chip = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%d %d %d [%s]\n",
> + jiffies_to_usecs(chip->vendor.duration[TPM_SHORT]),
> + jiffies_to_usecs(chip->vendor.duration[TPM_MEDIUM]),
> + jiffies_to_usecs(chip->vendor.duration[TPM_LONG]),
> + chip->vendor.duration_adjusted
> + ? "adjusted"
> + : "original");
> +}
> +EXPORT_SYMBOL_GPL(tpm_show_timeouts);
> +
> ssize_t tpm_store_cancel(struct device *dev, struct device_attribute
> *attr,
> const char *buf, size_t count)
> {
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 72ddb03..85e47af 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -56,6 +56,8 @@ extern ssize_t tpm_show_owned(struct device *,
> struct device_attribute *attr,
> char *);
> extern ssize_t tpm_show_temp_deactivated(struct device *,
> struct device_attribute *attr, char *);
> +extern ssize_t tpm_show_timeouts(struct device *,
> + struct device_attribute *attr, char *);
> struct tpm_chip;
> @@ -82,6 +84,7 @@ struct tpm_vendor_specific {
> int locality;
> unsigned long timeout_a, timeout_b, timeout_c, timeout_d; /*
> jiffies */
> unsigned long duration[3]; /* jiffies */
> + bool duration_adjusted;
> wait_queue_head_t read_queue;
> wait_queue_head_t int_queue;
> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
> index dd21df5..38040bd 100644
> --- a/drivers/char/tpm/tpm_tis.c
> +++ b/drivers/char/tpm/tpm_tis.c
> @@ -376,6 +376,7 @@ static DEVICE_ATTR(temp_deactivated, S_IRUGO,
> tpm_show_temp_deactivated,
> NULL);
> static DEVICE_ATTR(caps, S_IRUGO, tpm_show_caps_1_2, NULL);
> static DEVICE_ATTR(cancel, S_IWUSR | S_IWGRP, NULL, tpm_store_cancel);
> +static DEVICE_ATTR(timeouts, S_IRUGO, tpm_show_timeouts, NULL);
> static struct attribute *tis_attrs[] = {
> &dev_attr_pubek.attr,
> @@ -385,7 +386,8 @@ static struct attribute *tis_attrs[] = {
> &dev_attr_owned.attr,
> &dev_attr_temp_deactivated.attr,
> &dev_attr_caps.attr,
> -&dev_attr_cancel.attr, NULL,
> +&dev_attr_cancel.attr,
> +&dev_attr_timeouts.attr, NULL,
> };
> static struct attribute_group tis_attr_grp = {
> @@ -649,6 +651,23 @@ static int tpm_tis_pnp_resume(struct pnp_dev *dev)
> {
> struct tpm_chip *chip = pnp_get_drvdata(dev);
> int ret;
> + u32 intmask;
> +
> + if (chip->vendor.irq) {
> + /* reenable interrupts that device may have lost or
> + BIOS/firmware may have disabled */
> + intmask =
> + ioread32(chip->vendor.iobase +
> + TPM_INT_ENABLE(chip->vendor.locality));
> +
> + intmask |= TPM_INTF_CMD_READY_INT
> + | TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_DATA_AVAIL_INT
> + | TPM_INTF_STS_VALID_INT | TPM_GLOBAL_INT_ENABLE;
> +
> + iowrite32(intmask,
> + chip->vendor.iobase +
> + TPM_INT_ENABLE(chip->vendor.locality));
> + }
> ret = tpm_pm_resume(&dev->dev);
> if (!ret)
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists