[PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing

Subsystems: the rest, tpm device driver

STALE2026d

5 messages, 2 authors, 2021-01-20 · open the first message on its own page

[PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing

From: Tianjia Zhang <hidden>
Date: 2021-01-13 12:01:44

In tpm_tis_core_init(), tpm2_probe() will be called first, this
function will eventually call tpm_tis_send(), and then
tpm_tis_probe_irq_single() will detect whether the interrupt is
normal, mainly the installation interrupted, set `priv->irq_tested`
to false. The logic will eventually be executed to tpm_tis_send()
to trigger an interrupt.

There is currently such a scenario, which will cause the IRQ probe
code to never be executed, so that the TPM device is in polling
mode: after setting irq_tested to false, an interrupt occurs
between entering the ttpm_tis_send() function, and the interrupt
will be first set irq_tested to true will cause the IRQ probe code
to never be executed.

It seems that this interrupt comes from tpm2_probe(). Although the
interrupt has not been installed when tpm2_probe() is called, the
interrupt of tpm2_probe() is only received after IRQ detection.

This patch solves this issue by introducing a new variable, which
is only used in interrupts, and irq_tested only marks whether the
interrupt test has been completed.

Signed-off-by: Tianjia Zhang <redacted>
---
 drivers/char/tpm/tpm_tis_core.c | 8 ++++----
 drivers/char/tpm/tpm_tis_core.h | 1 +
 2 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 92c51c6cfd1b..d7589b0b3e56 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -502,7 +502,7 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	int rc, irq;
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
 
-	if (!(chip->flags & TPM_CHIP_FLAG_IRQ) || priv->irq_tested)
+	if (priv->irq_tested)
 		return tpm_tis_send_main(chip, buf, len);
 
 	/* Verify receipt of the expected IRQ */
@@ -512,9 +512,9 @@ static int tpm_tis_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	rc = tpm_tis_send_main(chip, buf, len);
 	priv->irq = irq;
 	chip->flags |= TPM_CHIP_FLAG_IRQ;
-	if (!priv->irq_tested)
+	if (!priv->int_count)
 		tpm_msleep(1);
-	if (!priv->irq_tested)
+	if (!priv->int_count)
 		disable_interrupts(chip);
 	priv->irq_tested = true;
 	return rc;
@@ -725,7 +725,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
 	if (interrupt == 0)
 		return IRQ_NONE;
 
-	priv->irq_tested = true;
+	priv->int_count += 1;
 	if (interrupt & TPM_INTF_DATA_AVAIL_INT)
 		wake_up_interruptible(&priv->read_queue);
 	if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 9b2d32a59f67..c6845672f6f7 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -90,6 +90,7 @@ struct tpm_tis_data {
 	int locality;
 	int irq;
 	bool irq_tested;
+	unsigned int int_count;
 	unsigned int flags;
 	void __iomem *ilb_base_addr;
 	u16 clkrun_enabled;
-- 
2.19.1.3.ge56e4f7

Re: [PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing

From: Jarkko Sakkinen <jarkko@kernel.org>
Date: 2021-01-14 02:52:28

On Wed, Jan 13, 2021 at 08:00:21PM +0800, Tianjia Zhang wrote:
In tpm_tis_core_init(), tpm2_probe() will be called first, this
function will eventually call tpm_tis_send(), and then
tpm_tis_probe_irq_single() will detect whether the interrupt is
normal, mainly the installation interrupted, set `priv->irq_tested`
to false. The logic will eventually be executed to tpm_tis_send()
to trigger an interrupt.

There is currently such a scenario, which will cause the IRQ probe
code to never be executed, so that the TPM device is in polling
mode: after setting irq_tested to false, an interrupt occurs
between entering the ttpm_tis_send() function, and the interrupt
will be first set irq_tested to true will cause the IRQ probe code
to never be executed.
Can you describe the scenario more detail?
It seems that this interrupt comes from tpm2_probe(). Although the
interrupt has not been installed when tpm2_probe() is called, the
interrupt of tpm2_probe() is only received after IRQ detection.

This patch solves this issue by introducing a new variable, which
is only used in interrupts, and irq_tested only marks whether the
interrupt test has been completed.

Signed-off-by: Tianjia Zhang <redacted>
---
I'm not sure I understand this patch. TPM should be in polling
mode. This is also assumption before calling tpm_get_timeouts():

/* Before doing irq testing issue a command to the TPM in polling mode
 * to make sure it works. May as well use that command to set the
 * proper timeouts for the driver.
 */
if (tpm_get_timeouts(chip)) {
        dev_err(dev, "Could not get TPM timeouts and durations\n");
        rc = -ENODEV;
        goto out_err;
}

/Jarkko

Re: [PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing

From: Tianjia Zhang <hidden>
Date: 2021-01-14 04:13:20


On 1/14/21 10:51 AM, Jarkko Sakkinen wrote:
On Wed, Jan 13, 2021 at 08:00:21PM +0800, Tianjia Zhang wrote:
quoted
In tpm_tis_core_init(), tpm2_probe() will be called first, this
function will eventually call tpm_tis_send(), and then
tpm_tis_probe_irq_single() will detect whether the interrupt is
normal, mainly the installation interrupted, set `priv->irq_tested`
to false. The logic will eventually be executed to tpm_tis_send()
to trigger an interrupt.

There is currently such a scenario, which will cause the IRQ probe
code to never be executed, so that the TPM device is in polling
mode: after setting irq_tested to false, an interrupt occurs
between entering the ttpm_tis_send() function, and the interrupt
will be first set irq_tested to true will cause the IRQ probe code
to never be executed.
Can you describe the scenario more detail?
The problematic scenario we encountered is like this. The following 
figure shows the execution flow of tpm_tis_core_init(). An interrupt 
occurred before the IRQ probe. This interrupt was caused by 
tpm2_probe(), but it was triggered before the IRQ probe was executed, 
and the interrupt handler would set irq_tested to true, so the IRQ probe 
code can never execute, that is, the code marked 2 in the figure will 
never happen.

                                          IRQ
   tpm_tis_core_init()

     tpm2_probe()

       tpm_tis_send()           -----------+
                                           |
     tpm_tis_probe_irq_single()            |
                                           |
       devm_request_irq()                  | 1
       priv->irq_tested = false            |
       tpm_tis_gen_interrupt()             |
                                           |
         tpm_tis_send()                    |
                         irq_tested = true |
                        <------------------+
           if (priv->irq_tested)
             return tpm_tis_send_main()

           /* probe IRQ */
           tpm_tis_send_main()     --------+
                                           | 2
           chip->flags |= FLAG_IRQ <-------+
           priv->irq_tested = true

Best regards,
Tianjia

Re: [PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing

From: Jarkko Sakkinen <jarkko@kernel.org>
Date: 2021-01-15 09:24:50

On Thu, Jan 14, 2021 at 12:12:16PM +0800, Tianjia Zhang wrote:

On 1/14/21 10:51 AM, Jarkko Sakkinen wrote:
quoted
On Wed, Jan 13, 2021 at 08:00:21PM +0800, Tianjia Zhang wrote:
quoted
In tpm_tis_core_init(), tpm2_probe() will be called first, this
function will eventually call tpm_tis_send(), and then
tpm_tis_probe_irq_single() will detect whether the interrupt is
normal, mainly the installation interrupted, set `priv->irq_tested`
to false. The logic will eventually be executed to tpm_tis_send()
to trigger an interrupt.

There is currently such a scenario, which will cause the IRQ probe
code to never be executed, so that the TPM device is in polling
mode: after setting irq_tested to false, an interrupt occurs
between entering the ttpm_tis_send() function, and the interrupt
will be first set irq_tested to true will cause the IRQ probe code
to never be executed.
Can you describe the scenario more detail?
The problematic scenario we encountered is like this. The following figure
shows the execution flow of tpm_tis_core_init(). An interrupt occurred
before the IRQ probe. This interrupt was caused by tpm2_probe(), but it was
triggered before the IRQ probe was executed, and the interrupt handler would
set irq_tested to true, so the IRQ probe code can never execute, that is,
the code marked 2 in the figure will never happen.
TPM_INT_ENABLE is cleared on reset [*].

[*] Section 5.9.1
    https://trustedcomputinggroup.org/resource/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/

/Jarkko

Re: [PATCH] tpm/tpm_tis: Fix variable reset during IRQ probing

From: Tianjia Zhang <hidden>
Date: 2021-01-20 04:06:15


On 1/15/21 5:23 PM, Jarkko Sakkinen wrote:
On Thu, Jan 14, 2021 at 12:12:16PM +0800, Tianjia Zhang wrote:
quoted

On 1/14/21 10:51 AM, Jarkko Sakkinen wrote:
quoted
On Wed, Jan 13, 2021 at 08:00:21PM +0800, Tianjia Zhang wrote:
quoted
In tpm_tis_core_init(), tpm2_probe() will be called first, this
function will eventually call tpm_tis_send(), and then
tpm_tis_probe_irq_single() will detect whether the interrupt is
normal, mainly the installation interrupted, set `priv->irq_tested`
to false. The logic will eventually be executed to tpm_tis_send()
to trigger an interrupt.

There is currently such a scenario, which will cause the IRQ probe
code to never be executed, so that the TPM device is in polling
mode: after setting irq_tested to false, an interrupt occurs
between entering the ttpm_tis_send() function, and the interrupt
will be first set irq_tested to true will cause the IRQ probe code
to never be executed.
Can you describe the scenario more detail?
The problematic scenario we encountered is like this. The following figure
shows the execution flow of tpm_tis_core_init(). An interrupt occurred
before the IRQ probe. This interrupt was caused by tpm2_probe(), but it was
triggered before the IRQ probe was executed, and the interrupt handler would
set irq_tested to true, so the IRQ probe code can never execute, that is,
the code marked 2 in the figure will never happen.
TPM_INT_ENABLE is cleared on reset [*].

[*] Section 5.9.1
     https://trustedcomputinggroup.org/resource/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis/

/Jarkko
Hi,

I got it, this seems to be a firmware issue. Thanks for your reply.

Best regards,
Tianjia
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help