* [PATCH v10 01/14] tpm, tpm_tis: Avoid cache incoherency in test for interrupts
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 02/14] tpm, tpm_tis: Claim locality before writing TPM_INT_ENABLE register Lino Sanfilippo
` (12 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
The interrupt handler that sets the boolean variable irq_tested may run on
another CPU as the thread that checks irq_tested as part of the irq test in
tpm_tis_send().
Since nothing guarantees cache coherency between CPUs for unsynchronized
accesses to boolean variables the testing thread might not perceive the
value change done in the interrupt handler.
Avoid this issue by setting the bit TPM_TIS_IRQ_TESTED in the flags field
of the tpm_tis_data struct and by accessing this field with the bit
manipulating functions that provide cache coherency.
Also convert all other existing sites to use the proper macros when
accessing this bitfield.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
---
drivers/char/tpm/tpm_tis.c | 2 +-
drivers/char/tpm/tpm_tis_core.c | 21 +++++++++++----------
drivers/char/tpm/tpm_tis_core.h | 2 +-
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index bcff6429e0b4..ce43412eb398 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -226,7 +226,7 @@ static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info)
irq = tpm_info->irq;
if (itpm || is_itpm(ACPI_COMPANION(dev)))
- phy->priv.flags |= TPM_TIS_ITPM_WORKAROUND;
+ set_bit(TPM_TIS_ITPM_WORKAROUND, &phy->priv.flags);
return tpm_tis_core_init(dev, &phy->priv, irq, &tpm_tcg,
ACPI_HANDLE(dev));
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 757623bacfd5..c0008efb95dc 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -351,7 +351,7 @@ static int tpm_tis_send_data(struct tpm_chip *chip, const u8 *buf, size_t len)
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
int rc, status, burstcnt;
size_t count = 0;
- bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
+ bool itpm = test_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags);
status = tpm_tis_status(chip);
if ((status & TPM_STS_COMMAND_READY) == 0) {
@@ -484,7 +484,8 @@ 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 (!(chip->flags & TPM_CHIP_FLAG_IRQ) ||
+ test_bit(TPM_TIS_IRQ_TESTED, &priv->flags))
return tpm_tis_send_main(chip, buf, len);
/* Verify receipt of the expected IRQ */
@@ -494,11 +495,11 @@ 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 (!test_bit(TPM_TIS_IRQ_TESTED, &priv->flags))
tpm_msleep(1);
- if (!priv->irq_tested)
+ if (!test_bit(TPM_TIS_IRQ_TESTED, &priv->flags))
disable_interrupts(chip);
- priv->irq_tested = true;
+ set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
return rc;
}
@@ -641,7 +642,7 @@ static int probe_itpm(struct tpm_chip *chip)
size_t len = sizeof(cmd_getticks);
u16 vendor;
- if (priv->flags & TPM_TIS_ITPM_WORKAROUND)
+ if (test_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags))
return 0;
rc = tpm_tis_read16(priv, TPM_DID_VID(0), &vendor);
@@ -661,13 +662,13 @@ static int probe_itpm(struct tpm_chip *chip)
tpm_tis_ready(chip);
- priv->flags |= TPM_TIS_ITPM_WORKAROUND;
+ set_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags);
rc = tpm_tis_send_data(chip, cmd_getticks, len);
if (rc == 0)
dev_info(&chip->dev, "Detected an iTPM.\n");
else {
- priv->flags &= ~TPM_TIS_ITPM_WORKAROUND;
+ clear_bit(TPM_TIS_ITPM_WORKAROUND, &priv->flags);
rc = -EFAULT;
}
@@ -707,7 +708,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
if (interrupt == 0)
return IRQ_NONE;
- priv->irq_tested = true;
+ set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
if (interrupt & TPM_INTF_DATA_AVAIL_INT)
wake_up_interruptible(&priv->read_queue);
if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
@@ -793,7 +794,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
if (rc < 0)
return rc;
- priv->irq_tested = false;
+ clear_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
/* Generate an interrupt by having the core call through to
* tpm_tis_send
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 66a5a13cd1df..695a2516dce0 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -86,13 +86,13 @@ enum tis_defaults {
enum tpm_tis_flags {
TPM_TIS_ITPM_WORKAROUND = BIT(0),
TPM_TIS_INVALID_STATUS = BIT(1),
+ TPM_TIS_IRQ_TESTED = BIT(2),
};
struct tpm_tis_data {
u16 manufacturer_id;
int locality;
int irq;
- bool irq_tested;
unsigned long flags;
void __iomem *ilb_base_addr;
u16 clkrun_enabled;
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 02/14] tpm, tpm_tis: Claim locality before writing TPM_INT_ENABLE register
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 01/14] tpm, tpm_tis: Avoid cache incoherency in test for interrupts Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 03/14] tpm, tpm_tis: Disable interrupts if tpm_tis_probe_irq() failed Lino Sanfilippo
` (11 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
In disable_interrupts() the TPM_GLOBAL_INT_ENABLE bit is unset in the
TPM_INT_ENABLE register to shut the interrupts off. However modifying the
register is only possible with a held locality. So claim the locality
before disable_interrupts() is called.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
---
drivers/char/tpm/tpm_tis_core.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index c0008efb95dc..d2c9c9d76893 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1098,7 +1098,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
+ rc = request_locality(chip, 0);
+ if (rc < 0)
+ goto out_err;
disable_interrupts(chip);
+ release_locality(chip, 0);
}
} else {
tpm_tis_probe_irq(chip, intmask);
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 03/14] tpm, tpm_tis: Disable interrupts if tpm_tis_probe_irq() failed
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 01/14] tpm, tpm_tis: Avoid cache incoherency in test for interrupts Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 02/14] tpm, tpm_tis: Claim locality before writing TPM_INT_ENABLE register Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 04/14] tpm, tpm_tis: Do not skip reset of original interrupt vector Lino Sanfilippo
` (10 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Both functions tpm_tis_probe_irq_single() and tpm_tis_probe_irq() may setup
the interrupts and then return with an error. This case is indicated by a
missing TPM_CHIP_FLAG_IRQ flag in chip->flags.
Currently the interrupt setup is only undone if tpm_tis_probe_irq_single()
fails. Undo the setup also if tpm_tis_probe_irq() fails.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
---
drivers/char/tpm/tpm_tis_core.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d2c9c9d76893..603b82ca56da 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1091,21 +1091,21 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
goto out_err;
}
- if (irq) {
+ if (irq)
tpm_tis_probe_irq_single(chip, intmask, IRQF_SHARED,
irq);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
- dev_err(&chip->dev, FW_BUG
+ else
+ tpm_tis_probe_irq(chip, intmask);
+
+ if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
+ dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
- rc = request_locality(chip, 0);
- if (rc < 0)
- goto out_err;
- disable_interrupts(chip);
- release_locality(chip, 0);
- }
- } else {
- tpm_tis_probe_irq(chip, intmask);
+ rc = request_locality(chip, 0);
+ if (rc < 0)
+ goto out_err;
+ disable_interrupts(chip);
+ release_locality(chip, 0);
}
}
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 04/14] tpm, tpm_tis: Do not skip reset of original interrupt vector
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (2 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 03/14] tpm, tpm_tis: Disable interrupts if tpm_tis_probe_irq() failed Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-24 6:58 ` Jarkko Sakkinen
2022-11-20 13:31 ` [PATCH v10 05/14] tpm, tpm_tis: Claim locality before writing interrupt registers Lino Sanfilippo
` (9 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
If in tpm_tis_probe_irq_single() an error occurs after the original
interrupt vector has been read, restore the interrupts before the error is
returned.
Since the caller does not check the error value, return -1 in any case that
the TPM_CHIP_FLAG_IRQ flag is not set. Since the return value of function
tpm_tis_gen_interrupt() is not longer used, make it a void function.
Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
drivers/char/tpm/tpm_tis_core.c | 33 +++++++++++++--------------------
1 file changed, 13 insertions(+), 20 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 603b82ca56da..1eac1279594d 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -729,7 +729,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
return IRQ_HANDLED;
}
-static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
+static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
{
const char *desc = "attempting to generate an interrupt";
u32 cap2;
@@ -738,16 +738,14 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
ret = request_locality(chip, 0);
if (ret < 0)
- return ret;
+ return;
if (chip->flags & TPM_CHIP_FLAG_TPM2)
- ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
+ tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
else
- ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
+ tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
release_locality(chip, 0);
-
- return ret;
}
/* Register the IRQ and issue a command that will cause an interrupt. If an
@@ -777,42 +775,37 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
rc = tpm_tis_write8(priv, TPM_INT_VECTOR(priv->locality), irq);
if (rc < 0)
- return rc;
+ goto restore_irqs;
rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &int_status);
if (rc < 0)
- return rc;
+ goto restore_irqs;
/* Clear all existing */
rc = tpm_tis_write32(priv, TPM_INT_STATUS(priv->locality), int_status);
if (rc < 0)
- return rc;
-
+ goto restore_irqs;
/* Turn on */
rc = tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality),
intmask | TPM_GLOBAL_INT_ENABLE);
if (rc < 0)
- return rc;
+ goto restore_irqs;
clear_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
/* Generate an interrupt by having the core call through to
* tpm_tis_send
*/
- rc = tpm_tis_gen_interrupt(chip);
- if (rc < 0)
- return rc;
+ tpm_tis_gen_interrupt(chip);
+restore_irqs:
/* tpm_tis_send will either confirm the interrupt is working or it
* will call disable_irq which undoes all of the above.
*/
if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
- rc = tpm_tis_write8(priv, original_int_vec,
- TPM_INT_VECTOR(priv->locality));
- if (rc < 0)
- return rc;
-
- return 1;
+ tpm_tis_write8(priv, original_int_vec,
+ TPM_INT_VECTOR(priv->locality));
+ return -1;
}
return 0;
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v10 04/14] tpm, tpm_tis: Do not skip reset of original interrupt vector
2022-11-20 13:31 ` [PATCH v10 04/14] tpm, tpm_tis: Do not skip reset of original interrupt vector Lino Sanfilippo
@ 2022-11-24 6:58 ` Jarkko Sakkinen
2022-11-24 13:25 ` Lino Sanfilippo
0 siblings, 1 reply; 19+ messages in thread
From: Jarkko Sakkinen @ 2022-11-24 6:58 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: peterhuewe, jgg, stefanb, linux, linux-integrity, linux-kernel,
jandryuk, pmenzel, l.sanfilippo, lukas, p.rosenberger
On Sun, Nov 20, 2022 at 02:31:24PM +0100, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>
> If in tpm_tis_probe_irq_single() an error occurs after the original
> interrupt vector has been read, restore the interrupts before the error is
> returned.
>
> Since the caller does not check the error value, return -1 in any case that
> the TPM_CHIP_FLAG_IRQ flag is not set. Since the return value of function
> tpm_tis_gen_interrupt() is not longer used, make it a void function.
>
> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 33 +++++++++++++--------------------
> 1 file changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 603b82ca56da..1eac1279594d 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -729,7 +729,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
> return IRQ_HANDLED;
> }
>
> -static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
> +static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
> {
> const char *desc = "attempting to generate an interrupt";
> u32 cap2;
> @@ -738,16 +738,14 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
>
> ret = request_locality(chip, 0);
> if (ret < 0)
> - return ret;
> + return;
>
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> + tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
In a successive patch:
- tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
+ ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
`
BR, Jarkko
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: [PATCH v10 04/14] tpm, tpm_tis: Do not skip reset of original interrupt vector
2022-11-24 6:58 ` Jarkko Sakkinen
@ 2022-11-24 13:25 ` Lino Sanfilippo
0 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-24 13:25 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: peterhuewe, jgg, stefanb, linux, linux-integrity, linux-kernel,
jandryuk, pmenzel, l.sanfilippo, lukas, p.rosenberger
On 24.11.22 at 07:58, Jarkko Sakkinen wrote:
> On Sun, Nov 20, 2022 at 02:31:24PM +0100, Lino Sanfilippo wrote:
>> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>>
>> If in tpm_tis_probe_irq_single() an error occurs after the original
>> interrupt vector has been read, restore the interrupts before the error is
>> returned.
>>
>> Since the caller does not check the error value, return -1 in any case that
>> the TPM_CHIP_FLAG_IRQ flag is not set. Since the return value of function
>> tpm_tis_gen_interrupt() is not longer used, make it a void function.
>>
>> Fixes: 1107d065fdf1 ("tpm_tis: Introduce intermediate layer for TPM access")
>> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>> ---
>> drivers/char/tpm/tpm_tis_core.c | 33 +++++++++++++--------------------
>> 1 file changed, 13 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
>> index 603b82ca56da..1eac1279594d 100644
>> --- a/drivers/char/tpm/tpm_tis_core.c
>> +++ b/drivers/char/tpm/tpm_tis_core.c
>> @@ -729,7 +729,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
>> return IRQ_HANDLED;
>> }
>>
>> -static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
>> +static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
>> {
>> const char *desc = "attempting to generate an interrupt";
>> u32 cap2;
>> @@ -738,16 +738,14 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
>>
>> ret = request_locality(chip, 0);
>> if (ret < 0)
>> - return ret;
>> + return;
>>
>> if (chip->flags & TPM_CHIP_FLAG_TPM2)
>> - ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
>> + tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
>
> In a successive patch:
>
> - tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> + ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> `
>
> BR, Jarkko
>
If it was a single patch it would be IMHO correct to remove ret, since at this point it
is not needed any more. But as part of a series it is admittedly a bit odd to remove the value only
to re-add it in a later patch of the same series. I will fix this and send a v11.
Regards,
Lino
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v10 05/14] tpm, tpm_tis: Claim locality before writing interrupt registers
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (3 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 04/14] tpm, tpm_tis: Do not skip reset of original interrupt vector Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-24 0:55 ` Jarkko Sakkinen
2022-11-20 13:31 ` [PATCH v10 06/14] tpm, tpm_tis: Only handle supported interrupts Lino Sanfilippo
` (8 subsequent siblings)
13 siblings, 1 reply; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
In tpm_tis_probe_single_irq() interrupt registers TPM_INT_VECTOR,
TPM_INT_STATUS and TPM_INT_ENABLE are modified to setup the interrupts.
Currently these modifications are done without holding a locality thus they
have no effect. Fix this by claiming the (default) locality before the
registers are written.
Since now tpm_tis_gen_interrupt() is called with the locality already
claimed remove locality request and release from this function.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
drivers/char/tpm/tpm_tis_core.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 1eac1279594d..58a53ec534aa 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -734,18 +734,11 @@ static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
const char *desc = "attempting to generate an interrupt";
u32 cap2;
cap_t cap;
- int ret;
-
- ret = request_locality(chip, 0);
- if (ret < 0)
- return;
if (chip->flags & TPM_CHIP_FLAG_TPM2)
tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
else
tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
-
- release_locality(chip, 0);
}
/* Register the IRQ and issue a command that will cause an interrupt. If an
@@ -768,10 +761,16 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
}
priv->irq = irq;
+ rc = request_locality(chip, 0);
+ if (rc < 0)
+ return rc;
+
rc = tpm_tis_read8(priv, TPM_INT_VECTOR(priv->locality),
&original_int_vec);
- if (rc < 0)
+ if (rc < 0) {
+ release_locality(chip, priv->locality);
return rc;
+ }
rc = tpm_tis_write8(priv, TPM_INT_VECTOR(priv->locality), irq);
if (rc < 0)
@@ -805,10 +804,12 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
tpm_tis_write8(priv, original_int_vec,
TPM_INT_VECTOR(priv->locality));
- return -1;
+ rc = -1;
}
- return 0;
+ release_locality(chip, priv->locality);
+
+ return rc;
}
/* Try to find the IRQ the TPM is using. This is for legacy x86 systems that
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v10 05/14] tpm, tpm_tis: Claim locality before writing interrupt registers
2022-11-20 13:31 ` [PATCH v10 05/14] tpm, tpm_tis: Claim locality before writing interrupt registers Lino Sanfilippo
@ 2022-11-24 0:55 ` Jarkko Sakkinen
0 siblings, 0 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2022-11-24 0:55 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: peterhuewe, jgg, stefanb, linux, linux-integrity, linux-kernel,
jandryuk, pmenzel, l.sanfilippo, lukas, p.rosenberger
On Sun, Nov 20, 2022 at 02:31:25PM +0100, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>
> In tpm_tis_probe_single_irq() interrupt registers TPM_INT_VECTOR,
> TPM_INT_STATUS and TPM_INT_ENABLE are modified to setup the interrupts.
> Currently these modifications are done without holding a locality thus they
> have no effect. Fix this by claiming the (default) locality before the
> registers are written.
>
> Since now tpm_tis_gen_interrupt() is called with the locality already
> claimed remove locality request and release from this function.
>
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 1eac1279594d..58a53ec534aa 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -734,18 +734,11 @@ static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
> const char *desc = "attempting to generate an interrupt";
> u32 cap2;
> cap_t cap;
> - int ret;
> -
> - ret = request_locality(chip, 0);
> - if (ret < 0)
> - return;
>
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
> else
> tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
> -
> - release_locality(chip, 0);
> }
>
> /* Register the IRQ and issue a command that will cause an interrupt. If an
> @@ -768,10 +761,16 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
> }
> priv->irq = irq;
>
> + rc = request_locality(chip, 0);
> + if (rc < 0)
> + return rc;
> +
> rc = tpm_tis_read8(priv, TPM_INT_VECTOR(priv->locality),
> &original_int_vec);
> - if (rc < 0)
> + if (rc < 0) {
> + release_locality(chip, priv->locality);
> return rc;
> + }
>
> rc = tpm_tis_write8(priv, TPM_INT_VECTOR(priv->locality), irq);
> if (rc < 0)
> @@ -805,10 +804,12 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
> if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
> tpm_tis_write8(priv, original_int_vec,
> TPM_INT_VECTOR(priv->locality));
> - return -1;
> + rc = -1;
> }
>
> - return 0;
> + release_locality(chip, priv->locality);
> +
> + return rc;
> }
>
> /* Try to find the IRQ the TPM is using. This is for legacy x86 systems that
> --
> 2.36.1
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v10 06/14] tpm, tpm_tis: Only handle supported interrupts
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (4 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 05/14] tpm, tpm_tis: Claim locality before writing interrupt registers Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 07/14] tpm, tpm_tis: Move interrupt mask checks into own function Lino Sanfilippo
` (7 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
According to the TPM Interface Specification (TIS) support for "stsValid"
and "commandReady" interrupts is only optional.
This has to be taken into account when handling the interrupts in functions
like wait_for_tpm_stat(). To determine the supported interrupts use the
capability query.
Also adjust wait_for_tpm_stat() to only wait for interrupt reported status
changes. After that process all the remaining status changes by polling
the status register.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm_tis_core.c | 120 +++++++++++++++++++-------------
drivers/char/tpm/tpm_tis_core.h | 1 +
2 files changed, 73 insertions(+), 48 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 58a53ec534aa..fbad92b18788 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -53,41 +53,63 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
long rc;
u8 status;
bool canceled = false;
+ u8 sts_mask = 0;
+ int ret = 0;
/* check current status */
status = chip->ops->status(chip);
if ((status & mask) == mask)
return 0;
- stop = jiffies + timeout;
+ /* check what status changes can be handled by irqs */
+ if (priv->int_mask & TPM_INTF_STS_VALID_INT)
+ sts_mask |= TPM_STS_VALID;
- if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ if (priv->int_mask & TPM_INTF_DATA_AVAIL_INT)
+ sts_mask |= TPM_STS_DATA_AVAIL;
+
+ if (priv->int_mask & TPM_INTF_CMD_READY_INT)
+ sts_mask |= TPM_STS_COMMAND_READY;
+
+ sts_mask &= mask;
+
+ stop = jiffies + timeout;
+ /* process status changes with irq support */
+ if (sts_mask) {
+ ret = -ETIME;
again:
timeout = stop - jiffies;
if ((long)timeout <= 0)
return -ETIME;
rc = wait_event_interruptible_timeout(*queue,
- wait_for_tpm_stat_cond(chip, mask, check_cancel,
+ wait_for_tpm_stat_cond(chip, sts_mask, check_cancel,
&canceled),
timeout);
if (rc > 0) {
if (canceled)
return -ECANCELED;
- return 0;
+ ret = 0;
}
if (rc == -ERESTARTSYS && freezing(current)) {
clear_thread_flag(TIF_SIGPENDING);
goto again;
}
- } else {
- do {
- usleep_range(priv->timeout_min,
- priv->timeout_max);
- status = chip->ops->status(chip);
- if ((status & mask) == mask)
- return 0;
- } while (time_before(jiffies, stop));
}
+
+ if (ret)
+ return ret;
+
+ mask &= ~sts_mask;
+ if (!mask) /* all done */
+ return 0;
+ /* process status changes without irq support */
+ do {
+ status = chip->ops->status(chip);
+ if ((status & mask) == mask)
+ return 0;
+ usleep_range(priv->timeout_min,
+ priv->timeout_max);
+ } while (time_before(jiffies, stop));
return -ETIME;
}
@@ -1000,8 +1022,40 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
if (rc < 0)
goto out_err;
- intmask |= TPM_INTF_CMD_READY_INT | TPM_INTF_LOCALITY_CHANGE_INT |
- TPM_INTF_DATA_AVAIL_INT | TPM_INTF_STS_VALID_INT;
+ /* Figure out the capabilities */
+ rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
+ if (rc < 0)
+ goto out_err;
+
+ dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
+ intfcaps);
+ if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
+ dev_dbg(dev, "\tBurst Count Static\n");
+ if (intfcaps & TPM_INTF_CMD_READY_INT) {
+ intmask |= TPM_INTF_CMD_READY_INT;
+ dev_dbg(dev, "\tCommand Ready Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
+ dev_dbg(dev, "\tInterrupt Edge Falling\n");
+ if (intfcaps & TPM_INTF_INT_EDGE_RISING)
+ dev_dbg(dev, "\tInterrupt Edge Rising\n");
+ if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
+ dev_dbg(dev, "\tInterrupt Level Low\n");
+ if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
+ dev_dbg(dev, "\tInterrupt Level High\n");
+ if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT) {
+ intmask |= TPM_INTF_LOCALITY_CHANGE_INT;
+ dev_dbg(dev, "\tLocality Change Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_STS_VALID_INT) {
+ intmask |= TPM_INTF_STS_VALID_INT;
+ dev_dbg(dev, "\tSts Valid Int Support\n");
+ }
+ if (intfcaps & TPM_INTF_DATA_AVAIL_INT) {
+ intmask |= TPM_INTF_DATA_AVAIL_INT;
+ dev_dbg(dev, "\tData Avail Int Support\n");
+ }
+
intmask &= ~TPM_GLOBAL_INT_ENABLE;
rc = request_locality(chip, 0);
@@ -1035,32 +1089,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
goto out_err;
}
- /* Figure out the capabilities */
- rc = tpm_tis_read32(priv, TPM_INTF_CAPS(priv->locality), &intfcaps);
- if (rc < 0)
- goto out_err;
-
- dev_dbg(dev, "TPM interface capabilities (0x%x):\n",
- intfcaps);
- if (intfcaps & TPM_INTF_BURST_COUNT_STATIC)
- dev_dbg(dev, "\tBurst Count Static\n");
- if (intfcaps & TPM_INTF_CMD_READY_INT)
- dev_dbg(dev, "\tCommand Ready Int Support\n");
- if (intfcaps & TPM_INTF_INT_EDGE_FALLING)
- dev_dbg(dev, "\tInterrupt Edge Falling\n");
- if (intfcaps & TPM_INTF_INT_EDGE_RISING)
- dev_dbg(dev, "\tInterrupt Edge Rising\n");
- if (intfcaps & TPM_INTF_INT_LEVEL_LOW)
- dev_dbg(dev, "\tInterrupt Level Low\n");
- if (intfcaps & TPM_INTF_INT_LEVEL_HIGH)
- dev_dbg(dev, "\tInterrupt Level High\n");
- if (intfcaps & TPM_INTF_LOCALITY_CHANGE_INT)
- dev_dbg(dev, "\tLocality Change Int Support\n");
- if (intfcaps & TPM_INTF_STS_VALID_INT)
- dev_dbg(dev, "\tSts Valid Int Support\n");
- if (intfcaps & TPM_INTF_DATA_AVAIL_INT)
- dev_dbg(dev, "\tData Avail Int Support\n");
-
/* INTERRUPT Setup */
init_waitqueue_head(&priv->read_queue);
init_waitqueue_head(&priv->int_queue);
@@ -1091,7 +1119,9 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
else
tpm_tis_probe_irq(chip, intmask);
- if (!(chip->flags & TPM_CHIP_FLAG_IRQ)) {
+ if (chip->flags & TPM_CHIP_FLAG_IRQ) {
+ priv->int_mask = intmask;
+ } else {
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
@@ -1138,13 +1168,7 @@ static void tpm_tis_reenable_interrupts(struct tpm_chip *chip)
if (rc < 0)
goto out;
- rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
- if (rc < 0)
- goto out;
-
- 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;
+ intmask = priv->int_mask | TPM_GLOBAL_INT_ENABLE;
tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 695a2516dce0..2deef11c88db 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -93,6 +93,7 @@ struct tpm_tis_data {
u16 manufacturer_id;
int locality;
int irq;
+ unsigned int int_mask;
unsigned long flags;
void __iomem *ilb_base_addr;
u16 clkrun_enabled;
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 07/14] tpm, tpm_tis: Move interrupt mask checks into own function
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (5 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 06/14] tpm, tpm_tis: Only handle supported interrupts Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 08/14] tpm, tpm_tis: do not check for the active locality in interrupt handler Lino Sanfilippo
` (6 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Clean up wait_for_tpm_stat() by moving multiple similar interrupt mask
checks into an own function.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Suggested-by: Jarkko Sakkinen <jarkko@kernel.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm_tis_core.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index fbad92b18788..5ff4ca5fb936 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -44,6 +44,20 @@ static bool wait_for_tpm_stat_cond(struct tpm_chip *chip, u8 mask,
return false;
}
+static u8 tpm_tis_filter_sts_mask(u8 int_mask, u8 sts_mask)
+{
+ if (!(int_mask & TPM_INTF_STS_VALID_INT))
+ sts_mask &= ~TPM_STS_VALID;
+
+ if (!(int_mask & TPM_INTF_DATA_AVAIL_INT))
+ sts_mask &= ~TPM_STS_DATA_AVAIL;
+
+ if (!(int_mask & TPM_INTF_CMD_READY_INT))
+ sts_mask &= ~TPM_STS_COMMAND_READY;
+
+ return sts_mask;
+}
+
static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
unsigned long timeout, wait_queue_head_t *queue,
bool check_cancel)
@@ -53,7 +67,7 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
long rc;
u8 status;
bool canceled = false;
- u8 sts_mask = 0;
+ u8 sts_mask;
int ret = 0;
/* check current status */
@@ -61,17 +75,10 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
if ((status & mask) == mask)
return 0;
+ sts_mask = mask & (TPM_STS_VALID | TPM_STS_DATA_AVAIL |
+ TPM_STS_COMMAND_READY);
/* check what status changes can be handled by irqs */
- if (priv->int_mask & TPM_INTF_STS_VALID_INT)
- sts_mask |= TPM_STS_VALID;
-
- if (priv->int_mask & TPM_INTF_DATA_AVAIL_INT)
- sts_mask |= TPM_STS_DATA_AVAIL;
-
- if (priv->int_mask & TPM_INTF_CMD_READY_INT)
- sts_mask |= TPM_STS_COMMAND_READY;
-
- sts_mask &= mask;
+ sts_mask = tpm_tis_filter_sts_mask(priv->int_mask, sts_mask);
stop = jiffies + timeout;
/* process status changes with irq support */
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 08/14] tpm, tpm_tis: do not check for the active locality in interrupt handler
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (6 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 07/14] tpm, tpm_tis: Move interrupt mask checks into own function Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 09/14] tpm, tpm: Implement usage counter for locality Lino Sanfilippo
` (5 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
After driver initialization tpm_tis_data->locality may only be modified in
case of a LOCALITY CHANGE interrupt. In this case the interrupt handler
iterates over all localities only to assign the active one to
tpm_tis_data->locality.
However this information is never used any more, so the assignment is not
needed.
Furthermore without the assignment tpm_tis_data->locality cannot change any
more at driver runtime, and thus no protection against concurrent
modification is required when the variable is read at other places.
So remove this iteration entirely.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm_tis_core.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 5ff4ca5fb936..d0e5acd6b769 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -728,7 +728,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
struct tpm_chip *chip = dev_id;
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
u32 interrupt;
- int i, rc;
+ int rc;
rc = tpm_tis_read32(priv, TPM_INT_STATUS(priv->locality), &interrupt);
if (rc < 0)
@@ -740,10 +740,7 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
set_bit(TPM_TIS_IRQ_TESTED, &priv->flags);
if (interrupt & TPM_INTF_DATA_AVAIL_INT)
wake_up_interruptible(&priv->read_queue);
- if (interrupt & TPM_INTF_LOCALITY_CHANGE_INT)
- for (i = 0; i < 5; i++)
- if (check_locality(chip, i))
- break;
+
if (interrupt &
(TPM_INTF_LOCALITY_CHANGE_INT | TPM_INTF_STS_VALID_INT |
TPM_INTF_CMD_READY_INT))
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 09/14] tpm, tpm: Implement usage counter for locality
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (7 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 08/14] tpm, tpm_tis: do not check for the active locality in interrupt handler Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 10/14] tpm, tpm_tis: Request threaded interrupt handler Lino Sanfilippo
` (4 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Implement a usage counter for the (default) locality used by the TPM TIS
driver:
Request the locality from the TPM if it has not been claimed yet, otherwise
only increment the counter. Also release the locality if the counter is 0
otherwise only decrement the counter. Since in case of SPI the register
accesses are locked by means of the SPI bus mutex use a sleepable lock
(i.e. also a mutex) to ensure thread-safety of the counter which may be
accessed by both a userspace thread and the interrupt handler.
By doing this refactor the names of the amended functions to use a more
appropriate prefix.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
---
drivers/char/tpm/tpm_tis_core.c | 63 +++++++++++++++++++++++----------
drivers/char/tpm/tpm_tis_core.h | 2 ++
2 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d0e5acd6b769..9aed9c26d2d5 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -165,16 +165,27 @@ static bool check_locality(struct tpm_chip *chip, int l)
return false;
}
-static int release_locality(struct tpm_chip *chip, int l)
+static int __tpm_tis_relinquish_locality(struct tpm_tis_data *priv, int l)
+{
+ tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
+
+ return 0;
+}
+
+static int tpm_tis_relinquish_locality(struct tpm_chip *chip, int l)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
- tpm_tis_write8(priv, TPM_ACCESS(l), TPM_ACCESS_ACTIVE_LOCALITY);
+ mutex_lock(&priv->locality_count_mutex);
+ priv->locality_count--;
+ if (priv->locality_count == 0)
+ __tpm_tis_relinquish_locality(priv, l);
+ mutex_unlock(&priv->locality_count_mutex);
return 0;
}
-static int request_locality(struct tpm_chip *chip, int l)
+static int __tpm_tis_request_locality(struct tpm_chip *chip, int l)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
unsigned long stop, timeout;
@@ -215,6 +226,20 @@ static int request_locality(struct tpm_chip *chip, int l)
return -1;
}
+static int tpm_tis_request_locality(struct tpm_chip *chip, int l)
+{
+ struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+ int ret = 0;
+
+ mutex_lock(&priv->locality_count_mutex);
+ if (priv->locality_count == 0)
+ ret = __tpm_tis_request_locality(chip, l);
+ if (!ret)
+ priv->locality_count++;
+ mutex_unlock(&priv->locality_count_mutex);
+ return ret;
+}
+
static u8 tpm_tis_status(struct tpm_chip *chip)
{
struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
@@ -682,7 +707,7 @@ static int probe_itpm(struct tpm_chip *chip)
if (vendor != TPM_VID_INTEL)
return 0;
- if (request_locality(chip, 0) != 0)
+ if (tpm_tis_request_locality(chip, 0) != 0)
return -EBUSY;
rc = tpm_tis_send_data(chip, cmd_getticks, len);
@@ -703,7 +728,7 @@ static int probe_itpm(struct tpm_chip *chip)
out:
tpm_tis_ready(chip);
- release_locality(chip, priv->locality);
+ tpm_tis_relinquish_locality(chip, priv->locality);
return rc;
}
@@ -787,14 +812,14 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
}
priv->irq = irq;
- rc = request_locality(chip, 0);
+ rc = tpm_tis_request_locality(chip, 0);
if (rc < 0)
return rc;
rc = tpm_tis_read8(priv, TPM_INT_VECTOR(priv->locality),
&original_int_vec);
if (rc < 0) {
- release_locality(chip, priv->locality);
+ tpm_tis_relinquish_locality(chip, priv->locality);
return rc;
}
@@ -833,7 +858,7 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
rc = -1;
}
- release_locality(chip, priv->locality);
+ tpm_tis_relinquish_locality(chip, priv->locality);
return rc;
}
@@ -949,8 +974,8 @@ static const struct tpm_class_ops tpm_tis = {
.req_complete_mask = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_complete_val = TPM_STS_DATA_AVAIL | TPM_STS_VALID,
.req_canceled = tpm_tis_req_canceled,
- .request_locality = request_locality,
- .relinquish_locality = release_locality,
+ .request_locality = tpm_tis_request_locality,
+ .relinquish_locality = tpm_tis_relinquish_locality,
.clk_enable = tpm_tis_clkrun_enable,
};
@@ -984,6 +1009,8 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
priv->timeout_min = TPM_TIMEOUT_USECS_MIN;
priv->timeout_max = TPM_TIMEOUT_USECS_MAX;
priv->phy_ops = phy_ops;
+ priv->locality_count = 0;
+ mutex_init(&priv->locality_count_mutex);
dev_set_drvdata(&chip->dev, priv);
@@ -1062,14 +1089,14 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
intmask &= ~TPM_GLOBAL_INT_ENABLE;
- rc = request_locality(chip, 0);
+ rc = tpm_tis_request_locality(chip, 0);
if (rc < 0) {
rc = -ENODEV;
goto out_err;
}
tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
- release_locality(chip, 0);
+ tpm_tis_relinquish_locality(chip, 0);
rc = tpm_chip_start(chip);
if (rc)
@@ -1103,13 +1130,13 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
* proper timeouts for the driver.
*/
- rc = request_locality(chip, 0);
+ rc = tpm_tis_request_locality(chip, 0);
if (rc < 0)
goto out_err;
rc = tpm_get_timeouts(chip);
- release_locality(chip, 0);
+ tpm_tis_relinquish_locality(chip, 0);
if (rc) {
dev_err(dev, "Could not get TPM timeouts and durations\n");
@@ -1129,11 +1156,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
dev_err(&chip->dev, FW_BUG
"TPM interrupt not working, polling instead\n");
- rc = request_locality(chip, 0);
+ rc = tpm_tis_request_locality(chip, 0);
if (rc < 0)
goto out_err;
disable_interrupts(chip);
- release_locality(chip, 0);
+ tpm_tis_relinquish_locality(chip, 0);
}
}
@@ -1200,13 +1227,13 @@ int tpm_tis_resume(struct device *dev)
* an error code but for unknown reason it isn't handled.
*/
if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
- ret = request_locality(chip, 0);
+ ret = tpm_tis_request_locality(chip, 0);
if (ret < 0)
return ret;
tpm1_do_selftest(chip);
- release_locality(chip, 0);
+ tpm_tis_relinquish_locality(chip, 0);
}
return 0;
diff --git a/drivers/char/tpm/tpm_tis_core.h b/drivers/char/tpm/tpm_tis_core.h
index 2deef11c88db..13bdcf38e56f 100644
--- a/drivers/char/tpm/tpm_tis_core.h
+++ b/drivers/char/tpm/tpm_tis_core.h
@@ -91,6 +91,8 @@ enum tpm_tis_flags {
struct tpm_tis_data {
u16 manufacturer_id;
+ struct mutex locality_count_mutex;
+ unsigned int locality_count;
int locality;
int irq;
unsigned int int_mask;
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 10/14] tpm, tpm_tis: Request threaded interrupt handler
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (8 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 09/14] tpm, tpm: Implement usage counter for locality Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 11/14] tpm, tpm_tis: Claim locality in " Lino Sanfilippo
` (3 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
The TIS interrupt handler at least has to read and write the interrupt
status register. In case of SPI both operations result in a call to
tpm_tis_spi_transfer() which uses the bus_lock_mutex of the spi device
and thus must only be called from a sleepable context.
To ensure this request a threaded interrupt handler.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm_tis_core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 9aed9c26d2d5..6c765a4406bc 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -804,8 +804,11 @@ static int tpm_tis_probe_irq_single(struct tpm_chip *chip, u32 intmask,
int rc;
u32 int_status;
- if (devm_request_irq(chip->dev.parent, irq, tis_int_handler, flags,
- dev_name(&chip->dev), chip) != 0) {
+
+ rc = devm_request_threaded_irq(chip->dev.parent, irq, NULL,
+ tis_int_handler, IRQF_ONESHOT | flags,
+ dev_name(&chip->dev), chip);
+ if (rc) {
dev_info(&chip->dev, "Unable to request irq: %d for probe\n",
irq);
return -1;
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 11/14] tpm, tpm_tis: Claim locality in interrupt handler
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (9 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 10/14] tpm, tpm_tis: Request threaded interrupt handler Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 12/14] tpm, tpm_tis: Claim locality when interrupts are reenabled on resume Lino Sanfilippo
` (2 subsequent siblings)
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Writing the TPM_INT_STATUS register in the interrupt handler to clear the
interrupts only has effect if a locality is held. Since this is not
guaranteed at the time the interrupt is fired, claim the locality
explicitly in the handler.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
---
drivers/char/tpm/tpm_tis_core.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 6c765a4406bc..4312c5cc13da 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -772,7 +772,9 @@ static irqreturn_t tis_int_handler(int dummy, void *dev_id)
wake_up_interruptible(&priv->int_queue);
/* Clear interrupts handled with TPM_EOI */
+ tpm_tis_request_locality(chip, 0);
rc = tpm_tis_write32(priv, TPM_INT_STATUS(priv->locality), interrupt);
+ tpm_tis_relinquish_locality(chip, 0);
if (rc < 0)
return IRQ_NONE;
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 12/14] tpm, tpm_tis: Claim locality when interrupts are reenabled on resume
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (10 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 11/14] tpm, tpm_tis: Claim locality in " Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-24 0:32 ` Jarkko Sakkinen
2022-11-20 13:31 ` [PATCH v10 13/14] tpm, tpm_tis: startup chip before testing for interrupts Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 14/14] tpm, tpm_tis: Enable interrupt test Lino Sanfilippo
13 siblings, 1 reply; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
In tpm_tis_resume() make sure that the locality has been claimed when
tpm_tis_reenable_interrupts() is called. Otherwise the writings to the
register might not have any effect.
Fixes: 45baa1d1fa39 ("tpm_tis: Re-enable interrupts upon (S3) resume")
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
drivers/char/tpm/tpm_tis_core.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 4312c5cc13da..2514e60f6778 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1220,28 +1220,27 @@ int tpm_tis_resume(struct device *dev)
struct tpm_chip *chip = dev_get_drvdata(dev);
int ret;
+ ret = tpm_tis_request_locality(chip, 0);
+ if (ret < 0)
+ return ret;
+
if (chip->flags & TPM_CHIP_FLAG_IRQ)
tpm_tis_reenable_interrupts(chip);
ret = tpm_pm_resume(dev);
if (ret)
- return ret;
+ goto out;
/*
* TPM 1.2 requires self-test on resume. This function actually returns
* an error code but for unknown reason it isn't handled.
*/
- if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
- ret = tpm_tis_request_locality(chip, 0);
- if (ret < 0)
- return ret;
-
+ if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
tpm1_do_selftest(chip);
+out:
+ tpm_tis_relinquish_locality(chip, 0);
- tpm_tis_relinquish_locality(chip, 0);
- }
-
- return 0;
+ return ret;
}
EXPORT_SYMBOL_GPL(tpm_tis_resume);
#endif
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* Re: [PATCH v10 12/14] tpm, tpm_tis: Claim locality when interrupts are reenabled on resume
2022-11-20 13:31 ` [PATCH v10 12/14] tpm, tpm_tis: Claim locality when interrupts are reenabled on resume Lino Sanfilippo
@ 2022-11-24 0:32 ` Jarkko Sakkinen
0 siblings, 0 replies; 19+ messages in thread
From: Jarkko Sakkinen @ 2022-11-24 0:32 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: peterhuewe, jgg, stefanb, linux, linux-integrity, linux-kernel,
jandryuk, pmenzel, l.sanfilippo, lukas, p.rosenberger
On Sun, Nov 20, 2022 at 02:31:32PM +0100, Lino Sanfilippo wrote:
> From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
>
> In tpm_tis_resume() make sure that the locality has been claimed when
> tpm_tis_reenable_interrupts() is called. Otherwise the writings to the
> register might not have any effect.
>
> Fixes: 45baa1d1fa39 ("tpm_tis: Re-enable interrupts upon (S3) resume")
> Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
> ---
> drivers/char/tpm/tpm_tis_core.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
> index 4312c5cc13da..2514e60f6778 100644
> --- a/drivers/char/tpm/tpm_tis_core.c
> +++ b/drivers/char/tpm/tpm_tis_core.c
> @@ -1220,28 +1220,27 @@ int tpm_tis_resume(struct device *dev)
> struct tpm_chip *chip = dev_get_drvdata(dev);
> int ret;
>
> + ret = tpm_tis_request_locality(chip, 0);
> + if (ret < 0)
> + return ret;
> +
> if (chip->flags & TPM_CHIP_FLAG_IRQ)
> tpm_tis_reenable_interrupts(chip);
>
> ret = tpm_pm_resume(dev);
> if (ret)
> - return ret;
> + goto out;
>
> /*
> * TPM 1.2 requires self-test on resume. This function actually returns
> * an error code but for unknown reason it isn't handled.
> */
> - if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) {
> - ret = tpm_tis_request_locality(chip, 0);
> - if (ret < 0)
> - return ret;
> -
> + if (!(chip->flags & TPM_CHIP_FLAG_TPM2))
> tpm1_do_selftest(chip);
> +out:
> + tpm_tis_relinquish_locality(chip, 0);
>
> - tpm_tis_relinquish_locality(chip, 0);
> - }
> -
> - return 0;
> + return ret;
> }
> EXPORT_SYMBOL_GPL(tpm_tis_resume);
> #endif
> --
> 2.36.1
>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
BR, Jarkko
^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v10 13/14] tpm, tpm_tis: startup chip before testing for interrupts
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (11 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 12/14] tpm, tpm_tis: Claim locality when interrupts are reenabled on resume Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
2022-11-20 13:31 ` [PATCH v10 14/14] tpm, tpm_tis: Enable interrupt test Lino Sanfilippo
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
In tpm_tis_gen_interrupt() a request for a property value is sent to the
TPM to test if interrupts are generated. However after a power cycle the
TPM responds with TPM_RC_INITIALIZE which indicates that the TPM is not
yet properly initialized.
Fix this by first starting the TPM up before the request is sent. For this
the startup implementation is removed from tpm_chip_register() and put
into the new function tpm_chip_startup() which is called before the
interrupts are tested.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
---
drivers/char/tpm/tpm-chip.c | 38 +++++++++++++++++++++------------
drivers/char/tpm/tpm.h | 1 +
drivers/char/tpm/tpm_tis_core.c | 5 +++++
3 files changed, 30 insertions(+), 14 deletions(-)
diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 783d65fc71f0..370aa1f529f2 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -543,6 +543,30 @@ static int tpm_get_pcr_allocation(struct tpm_chip *chip)
return rc;
}
+/*
+ * tpm_chip_startup() - performs auto startup and allocates the PCRs
+ * @chip: TPM chip to use.
+ */
+int tpm_chip_startup(struct tpm_chip *chip)
+{
+ int rc;
+
+ rc = tpm_chip_start(chip);
+ if (rc)
+ return rc;
+
+ rc = tpm_auto_startup(chip);
+ if (rc)
+ goto stop;
+
+ rc = tpm_get_pcr_allocation(chip);
+stop:
+ tpm_chip_stop(chip);
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(tpm_chip_startup);
+
/*
* tpm_chip_register() - create a character device for the TPM chip
* @chip: TPM chip to use.
@@ -558,20 +582,6 @@ int tpm_chip_register(struct tpm_chip *chip)
{
int rc;
- rc = tpm_chip_start(chip);
- if (rc)
- return rc;
- rc = tpm_auto_startup(chip);
- if (rc) {
- tpm_chip_stop(chip);
- return rc;
- }
-
- rc = tpm_get_pcr_allocation(chip);
- tpm_chip_stop(chip);
- if (rc)
- return rc;
-
tpm_sysfs_add_device(chip);
tpm_bios_log_setup(chip);
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 24ee4e1cc452..919bb0b88b12 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -190,6 +190,7 @@ static inline void tpm_msleep(unsigned int delay_msec)
delay_msec * 1000);
};
+int tpm_chip_startup(struct tpm_chip *chip);
int tpm_chip_start(struct tpm_chip *chip);
void tpm_chip_stop(struct tpm_chip *chip);
struct tpm_chip *tpm_find_get_ops(struct tpm_chip *chip);
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 2514e60f6778..b97cb325ccb7 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1128,6 +1128,11 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
/* INTERRUPT Setup */
init_waitqueue_head(&priv->read_queue);
init_waitqueue_head(&priv->int_queue);
+
+ rc = tpm_chip_startup(chip);
+ if (rc)
+ goto out_err;
+
if (irq != -1) {
/*
* Before doing irq testing issue a command to the TPM in polling mode
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread* [PATCH v10 14/14] tpm, tpm_tis: Enable interrupt test
2022-11-20 13:31 [PATCH v10 00/14] TPM IRQ fixes Lino Sanfilippo
` (12 preceding siblings ...)
2022-11-20 13:31 ` [PATCH v10 13/14] tpm, tpm_tis: startup chip before testing for interrupts Lino Sanfilippo
@ 2022-11-20 13:31 ` Lino Sanfilippo
13 siblings, 0 replies; 19+ messages in thread
From: Lino Sanfilippo @ 2022-11-20 13:31 UTC (permalink / raw)
To: peterhuewe, jarkko, jgg
Cc: stefanb, linux, linux-integrity, linux-kernel, jandryuk, pmenzel,
l.sanfilippo, LinoSanfilippo, lukas, p.rosenberger
From: Lino Sanfilippo <l.sanfilippo@kunbus.com>
The test for interrupts in tpm_tis_send() is skipped if the flag
TPM_CHIP_FLAG_IRQ is not set. Since the current code never sets the flag
initially the test is never executed.
Fix this by setting the flag in tpm_tis_gen_interrupt() right after
interrupts have been enabled and before the test is executed.
Signed-off-by: Lino Sanfilippo <l.sanfilippo@kunbus.com>
Tested-by: Michael Niewöhner <linux@mniewoehner.de>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
drivers/char/tpm/tpm_tis_core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index b97cb325ccb7..602ca4bb8e2f 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -787,11 +787,17 @@ static void tpm_tis_gen_interrupt(struct tpm_chip *chip)
const char *desc = "attempting to generate an interrupt";
u32 cap2;
cap_t cap;
+ int ret;
+
+ chip->flags |= TPM_CHIP_FLAG_IRQ;
if (chip->flags & TPM_CHIP_FLAG_TPM2)
- tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
+ ret = tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
else
- tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
+ ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
+
+ if (ret)
+ chip->flags &= ~TPM_CHIP_FLAG_IRQ;
}
/* Register the IRQ and issue a command that will cause an interrupt. If an
--
2.36.1
^ permalink raw reply related [flat|nested] 19+ messages in thread