Re: [PATCH 1/1] driver: Tpm hardware enablement --updated version

[Nish Aravamudan <nish.aravamudan@gmail.com>]

On Mon, 20 Dec 2004 11:50:00 -0600 (CST), Kylene Hall <kjhall@us.ibm.com> wrote:
> This patch is a device driver to enable new hardware.  The new hardware is
> the TPM chip as described by specifications at 
> <http://www.trustedcomputinggroup.org>.  The TPM chip will enable you to
> use hardware to securely store and protect your keys and personal data.
> To use the chip according to the specification, you will need the Trusted
> Software Stack (TSS) of which an implementation for Linux is available at:
> <http://sourceforge.net/projects/trousers>.
> 
> > > Updates include: splitting out the vendor specific code into separated
> > > drivers from the base TPM functionality, fixed busy waiting loops,
> > > concerns about timer handling and rmmod, buffer name.
> 
> > Updates include a consolodated buffer mutex, numerous naming improvements,
> > better placement of pci_driver and file_operations structure
> > initializations, change to use of EXPORT_SYMBOL_GPL, improvement of
> > Kconfig help texts and creation of a tpm directory
> 
> Updates include moving the pci_ids table into the individual vendor
> specific .c files and including a tpm description file in the
> Documentation directory.
> 
> Thanks,
> Kylene
> 
> Signed-off-by: Leendert van Doorn <leendert@watson.ibm.com>
> Signed-off-by: Reiner Sailer <sailer@watson.ibm.com>
> Signed-off-by: Dave Safford <safford@watson.ibm.com>
> Signed-off-by: Kylene Hall <kjhall@us.ibm.com>

<snip>

> diff -uprN linux-2.6.9/drivers/char/tpm/tpm.c linux-2.6.9-tpm/drivers/char/tpm/tpm.c
> --- linux-2.6.9/drivers/char/tpm/tpm.c  1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.9-tpm/drivers/char/tpm/tpm.c      2004-12-17 17:40:21.000000000 -0600
> @@ -0,0 +1,508 @@
> +/*
> + * Copyright (C) 2004 IBM Corporation
> + *
> + * Authors:
> + * Leendert van Doorn <leendert@watson.ibm.com>
> + * Reiner Sailer <sailer@watson.ibm.com>
> + * Dave Safford <safford@watson.ibm.com>
> + * Kylene Hall <kjhall@us.ibm.com>
> + *
> + * Maintained by: <tpmdd_devel@lists.sourceforge.net>
> + *
> + * Device driver for TCG/TCPA TPM (trusted platform module).
> + * Specifications at www.trustedcomputinggroup.org
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation, version 2 of the
> + * License.
> + *
> + * Note, the TPM chip is not interrupt driven (only polling)
> + * and can have very long timeouts (minutes!). Hence the unusual
> + * calls to schedule_timeout.

<snip>

> +       do {
> +               u8 status = inb(chip->vendor->base + 1);
> +               if ((status & chip->vendor->req_complete_mask) ==
> +                   chip->vendor->req_complete_val) {
> +                       down(&chip->timer_manipulation_mutex);
> +                       del_singleshot_timer_sync(&chip->device_timer);
> +                       up(&chip->timer_manipulation_mutex);
> +                       goto out_recv;
> +               }
> +               schedule_timeout(TPM_TIMEOUT);
> +               rmb();
> +       } while (!chip->time_expired);

All of these calls to schedule_timeout() are broken. You must set the
state appropriately before calling schedule_timeout() or it will
return immediately. For your reference here is the difference between
the two states with respects to calling schedule_timeout() directly:

TASK_INTERRUPTIBLE: sleep, waking up early on both signals and waitqueue events
TASK_UNINTERRUPTIBLE: sleep, waking up early on watiqueue events

Keep in mind that if TASK_INTERRUPTIBLE is used, you *must* be
prepared to handle signals (check return value, indicating an early
return, and signals_pending(current), indicating whether the cause of
early return is either a signal or waitqueue event).

Also, as a final consideration, because of the way the loadavg is
calculated, a TASK_UNINTERRUPTIBLE sleep will factor in as a 1 to the
load.

-Nish