diff mbox

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

Message ID 20180503174311.d7bejzqjclvdcfyn@cantor (mailing list archive)
State New, archived
Headers show

Commit Message

Jerry Snitselaar May 3, 2018, 5:43 p.m. UTC
On Thu May 03 18, Laurent Bigonville wrote:
>Le 15/03/18 à 17:24, Jarkko Sakkinen a écrit :
>>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

Comments

Jarkko Sakkinen May 4, 2018, 8:20 a.m. UTC | #1
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 à 17:24, Jarkko Sakkinen a écrit :
> > > 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
diff mbox

Patch

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));
+       }
        return 0;
 }