From: Tim Harvey <tharvey@gateworks.com>
To: linux-integrity@vger.kernel.org, Peter Huewe <peterhuewe@gmx.de>,
Jarkko Sakkinen <jarkko@kernel.org>,
Lino Sanfilippo <l.sanfilippo@kunbus.com>,
Linux ARM Mailing List <linux-arm-kernel@lists.infradead.org>
Subject: TPM error path on probe
Date: Wed, 28 Feb 2024 16:46:58 -0800 [thread overview]
Message-ID: <CAJ+vNU2irDtEn=VD7N2ySofr5wTeko6OHy-QD_SLLZudOwWj0A@mail.gmail.com> (raw)
Greetings,
Commit 481c2d14627d ("tpm,tpm_tis: Disable interrupts after 1000
unhandled IRQs") introduced a kernel warning for boards that do not
have a TPM loaded but have one defined in the device-tree (ie a
subloaded board assembly):
[ 2.434431] WARNING: CPU: 3 PID: 55 at kernel/workqueue.c:3397
__flush_work.isra.0+0x2ac/0x2d8
[ 2.443069] Modules linked in:
[ 2.446133] CPU: 3 PID: 55 Comm: kworker/u8:3 Not tainted
6.6.8-00021-g232153790aa6-dirty #434
[ 2.454758] Hardware name: Gateworks Venice GW73xx-2x i.MX8MP
Development Kit (DT)
[ 2.462337] Workqueue: events_unbound async_run_entry_fn
[ 2.467669] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 2.474647] pc : __flush_work.isra.0+0x2ac/0x2d8
[ 2.479278] lr : flush_work+0x10/0x1c
[ 2.482950] sp : ffff80008270ba40
[ 2.486271] x29: ffff80008270ba40 x28: 00000000ffffffff x27: 00000000ffffffff
[ 2.493425] x26: ffff000000293000 x25: ffff80008218aff8 x24: ffff800080a1c6a8
[ 2.500578] x23: 00000000ffffffff x22: ffff80008213ca18 x21: 0000000000000001
[ 2.507731] x20: ffff000000e142c0 x19: ffff000000e14280 x18: 0000000000000000
[ 2.514884] x17: 0000000000000000 x16: 000000000000001c x15: 0000000000000000
[ 2.522038] x14: ffff00003fd94100 x13: 0000000000000002 x12: 0000000000000000
[ 2.529191] x11: 0000000000000400 x10: 0000000000000910 x9 : 00000000ffffffff
[ 2.536346] x8 : 0000000000000cc1 x7 : ffff8000804c76d8 x6 : 00000000090ee8ad
[ 2.543500] x5 : ffff00000026c380 x4 : 0000000000000000 x3 : 0000000000000000
[ 2.550655] x2 : 0000000000000000 x1 : ffff00000026c380 x0 : 0000000000000000
[ 2.557806] Call trace:
[ 2.560261] __flush_work.isra.0+0x2ac/0x2d8
[ 2.564544] flush_work+0x10/0x1c
[ 2.567869] tpm_tis_remove+0x8c/0xc4
[ 2.571545] tpm_tis_core_init+0x194/0x7b8
[ 2.575656] tpm_tis_spi_probe+0xa4/0xd4
[ 2.579593] tpm_tis_spi_driver_probe+0x34/0x64
[ 2.584136] spi_probe+0x84/0xe4
[ 2.587377] really_probe+0x148/0x2c0
[ 2.591052] __driver_probe_device+0x78/0x12c
[ 2.595417] driver_probe_device+0xd8/0x15c
[ 2.599609] __device_attach_driver+0xb8/0x134
[ 2.604064] bus_for_each_drv+0x84/0xe0
[ 2.607913] __device_attach_async_helper+0xac/0xd0
[ 2.612808] async_run_entry_fn+0x34/0xe0
[ 2.616832] process_one_work+0x138/0x260
[ 2.620851] worker_thread+0x32c/0x438
[ 2.624609] kthread+0x118/0x11c
[ 2.627844] ret_from_fork+0x10/0x20
[ 2.631428] ---[ end trace 0000000000000000 ]---
The issue is caused by tpm_tis_remove() calling flush_work() on the
work queue that has not yet been initialized. I can move the INIT_WORK
call added in this commit into the probe to fix this:
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d5ea22196b5b..e62294b3a437 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1133,6 +1133,8 @@ int tpm_tis_core_init(struct device *dev, struct
tpm_tis_data *priv, int irq,
dev_set_drvdata(&chip->dev, priv);
+ INIT_WORK(&priv->free_irq_work, tpm_tis_free_irq_func);
+
rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor);
if (rc < 0)
return rc;
@@ -1159,8 +1161,6 @@ int tpm_tis_core_init(struct device *dev, struct
tpm_tis_data *priv, int irq,
}
}
- INIT_WORK(&priv->free_irq_work, tpm_tis_free_irq_func);
-
if (chip->ops->clk_enable != NULL)
chip->ops->clk_enable(chip, true);
However I wonder if we should simply have error checking for an
invalid vendor of 0xffffffff. Is there any reason to not return an
-ENODEV if vendor == 0xffffffff?
diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index d5ea22196b5b..1bb1b3e1b2d0 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -1136,6 +1136,8 @@ int tpm_tis_core_init(struct device *dev, struct
tpm_tis_data *priv, int irq,
rc = tpm_tis_read32(priv, TPM_DID_VID(0), &vendor);
if (rc < 0)
return rc;
+ if (rc == 0xffffffff)
+ return -ENODEV;
priv->manufacturer_id = vendor;
I'm not sure if a hard coded 0xffffffff check is best here.
Best regards,
Tim
next reply other threads:[~2024-02-29 0:47 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-29 0:46 Tim Harvey [this message]
2024-02-29 7:08 ` TPM error path on probe Lino Sanfilippo
2024-02-29 16:02 ` Tim Harvey
2024-03-01 20:49 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJ+vNU2irDtEn=VD7N2ySofr5wTeko6OHy-QD_SLLZudOwWj0A@mail.gmail.com' \
--to=tharvey@gateworks.com \
--cc=jarkko@kernel.org \
--cc=l.sanfilippo@kunbus.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-integrity@vger.kernel.org \
--cc=peterhuewe@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).