Re: [tpmdd-devel] tpm device not showing up in /dev anymore

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Thu, May 03, 2018 at 10:43:11AM -0700, Jerry Snitselaar wrote:
> On Thu May 03 18, Laurent Bigonville wrote:
> > Le 15/03/18 a 17:24, Jarkko Sakkinen a ecrit :
> > > On Fri, 2018-03-09 at 18:24 +0100, Laurent Bigonville wrote:
> > > > The duration that that was in your patch seems to work, cannot this be
> > > > implemented?
> > > > 
> > > > I'm quite surprised I'm the only one impacted by this...
> > > Sorry it took so long time response but I had forgotten so too many
> > > details.
> > > 
> > > After re-reading (PHEW!) the whole thread I'm thinking if we could
> > > use section 5.5.2.4 as a reference to select a timeout of TIMEOUT_A
> > > (albeit it is for request locality) and use Jerry's suggestion to
> > > put wait_for_tpm_stat() there? Opinions?
> > Hey,
> > 
> > Sorry to bother you again, but are there any progress on getting this
> > mainlined?
> > 
> > Kind regards,
> > 
> > Laurent Bigonville
> 
> Jarkko would something like this work? Building it to test on tpm2.0 system right now.
> I don't have access to a system with a tpm1.2 tis device at the moment. I tested a
> patch similar to this based on the slightly older code that is currently in the
> rhel kernel and it seemed work fine. release_locality was still a void function
> back then, so might have to think more about the return values here and checking
> them. Also wondering if release_locality, check_locality, and wait_for_tpm_stat
> can be refactored since they would all have basically the same chunk of code.
> 
> Jerry
> 
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 5a1f47b43947..31ee154450eb 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -143,12 +143,56 @@ static bool check_locality(struct tpm_chip *chip, int l)
>        return false;
> }
> 
> +static bool locality_inactive(struct tpm_chip *chip, int l)
> +{
> +       struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> +       int rc;
> +       u8 access;
> +
> +       rc = tpm_tis_read8(priv, TPM_ACCESS(l), &access);
> +       if (rc < 0)
> +               return false;
> +
> +       if ((access & (TPM_ACCESS_VALID | TPM_ACCESS_ACTIVE_LOCALITY)) == TPM_ACCESS_VALID)
> +               return true;
> +
> +       return false;
> +}
> +
> static int release_locality(struct tpm_chip *chip, int l)
> {
>        struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
> +       unsigned long stop, timeout;
> +       long rc;
> 
>        tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
> 
> +       stop = jiffies + chip->timeout_a;
> +
> +       if (chip->flags & TPM_CHIP_FLAG_IRQ) {
> +again:
> +               timeout = stop - jiffies;
> +               if ((long)timeout <= 0)
> +                       return -1;
> +
> +               rc = wait_event_interruptible_timeout(priv->int_queue,
> +                                                     (locality_inactive(chip, l)),
> +                                                     timeout);
> +
> +               if (rc > 0)
> +                       return rc;
> +
> +               if (rc == -ERESTARTSYS && freezing(current)) {
> +                       clear_thread_flag(TIF_SIGPENDING);
> +                       goto again;
> +               }
> +       } else {
> +               do {
> +                       if (locality_inactive(chip, l))
> +                               return 0;
> +                       tpm_msleep(TPM_TIMEOUT);
> +               } while (time_before(jiffies, stop));
> +       }

Maybe have the second branch first with a return statement and
convert the thing going on in the first branch as a loop?

/Jarkko