lkml.org 
[lkml]   [2021]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 2/4] tpm: Simplify locality handling
    On Mon, Apr 26, 2021 at 01:47:18AM +0200, Lino Sanfilippo wrote:
    > Currently the TPM (default) locality is claimed and released for each
    > access to the TPM registers which require a claimed locality. This results
    > in locality claim/release combos at various code places. For interrupt
    > handling we also need such a combo in the interrupt handler (for clearing
    > the interrupts) which makes the locality handling even more complicated
    > since now we also have to synchronize concurrent accesses in process and in
    > interrupt context.
    >
    > Since currently the driver uses only one locality anyway, avoid the
    > increasing complexity by claiming it once at driver startup and only
    > releasing it at driver shutdown.
    >
    > Due to the simplifications the functions tpm_request_locality() and
    > tpm_relinquish_locality() are not needed any more an can be removed.
    >
    > As a side-effect these modifications fix a bug which results in the
    > following warning when using TPM 2:
    >
    > TPM returned invalid status
    > WARNING: CPU: 0 PID: 874 at drivers/char/tpm/tpm_tis_core.c:249 tpm_tis_status+0xbc/0xc8 [tpm_tis_core]
    > Modules linked in: tpm_tis_spi tpm_tis_core tpm sha256_generic cfg80211 rfkill 8021q garp stp llc spidev v3d gpu_sched vc4 bcm2835_codec(C) cec raspberrypi_hwmon drm_kms_helper drm bcm2835_isp(C) v4l2_mem2mem bcm2835_v4l2(C) bcm2835_mmal_vchiq(C) videobuf2_vmalloc videobuf2_dma_contig snd_bcm2835(C) videobuf2_memops drm_panel_orientation_quirks videobuf2_v4l2 videobuf2_common snd_soc_core snd_compress videodev snd_pcm_dmaengine spi_bcm2835 snd_pcm mc vc_sm_cma(C) snd_timer snd syscopyarea rpivid_mem sysfillrect sysimgblt fb_sys_fops backlight uio_pdrv_genirq uio ip_tables x_tables ipv6 [last unloaded: tpm]
    > CPU: 0 PID: 874 Comm: kworker/u8:1 Tainted: G WC 5.11.0-rc2-LS-HOME+ #1
    > Hardware name: Raspberry Pi Compute Module 4 Rev 1.0 (DT)
    > Workqueue: events_unbound async_run_entry_fn
    > pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
    > pc : tpm_tis_status+0xbc/0xc8 [tpm_tis_core]
    > lr : tpm_tis_status+0xbc/0xc8 [tpm_tis_core]
    > sp : ffffffc0127e38f0
    > x29: ffffffc0127e38f0 x28: ffffffc011238000
    > x27: 0000000000000016 x26: 000000000000017a
    > x25: 0000000000000014 x24: ffffff8047f60000
    > x23: 0000000000000000 x22: 0000000000000016
    > x21: ffffff8044e8a480 x20: 0000000000000000
    > x19: ffffffc011238000 x18: ffffffc011238948
    > x17: 0000000000000000 x16: 0000000000000000
    > x15: ffffffc01141be81 x14: ffffffffffffffff
    > x13: ffffffc01141be7e x12: ffffffffffffffff
    > x11: ffffff807fb797f0 x10: 00000000000019b0
    > x9 : ffffffc0127e38f0 x8 : 766e692064656e72
    > x7 : 0000000000000000 x6 : ffffffc011239000
    > x5 : ffffff807fb628b8 x4 : 0000000000000000
    > x3 : 0000000000000027 x2 : 0000000000000000
    > x1 : 6818b6f22fdef800 x0 : 0000000000000000
    > Call trace:
    > tpm_tis_status+0xbc/0xc8 [tpm_tis_core]
    > tpm_tis_send_data+0x58/0x250 [tpm_tis_core]
    > tpm_tis_send_main+0x50/0x128 [tpm_tis_core]
    > tpm_tis_send+0x4c/0xe0 [tpm_tis_core]
    > tpm_transmit+0xd0/0x350 [tpm]
    > tpm_transmit_cmd+0x3c/0xc0 [tpm]
    > tpm2_get_tpm_pt+0x124/0x1e8 [tpm]
    > tpm_tis_probe_irq_single+0x198/0x364 [tpm_tis_core]
    > tpm_tis_core_init+0x304/0x520 [tpm_tis_core]
    > tpm_tis_spi_init+0x5c/0x78 [tpm_tis_spi]
    > tpm_tis_spi_probe+0x80/0x98 [tpm_tis_spi]
    > tpm_tis_spi_driver_probe+0x4c/0x60 [tpm_tis_spi]
    > spi_probe+0x84/0xf0
    > really_probe+0x118/0x420
    > driver_probe_device+0x5c/0xc0
    > __driver_attach_async_helper+0x64/0x68
    > async_run_entry_fn+0x48/0x150
    > process_one_work+0x15c/0x4d0
    > worker_thread+0x50/0x490
    > kthread+0x118/0x150
    > ret_from_fork+0x10/0x1c
    >
    > The reason for this issue is that in case of TPM 2 function
    > tpm2_get_timeouts() which executes a TPM command is called without a
    > claimed locality. Since with this patch the locality is taken once at
    > driver startup and never released before shutdown the issue does not occur
    > any more.
    >
    > Fixes: a3fbfae82b4c ("tpm: take TPM chip power gating out of tpm_transmit()")
    > Signed-off-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>

    Hi, what hardware? Just if I could find something comparable lying around.

    > ---
    > drivers/char/tpm/tpm-chip.c | 40 ----------------------------------------
    > drivers/char/tpm/tpm_tis_core.c | 35 ++++++++++-------------------------
    > include/linux/tpm.h | 3 ---
    > 3 files changed, 10 insertions(+), 68 deletions(-)
    >
    > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
    > index ddaeceb..a09b652 100644
    > --- a/drivers/char/tpm/tpm-chip.c
    > +++ b/drivers/char/tpm/tpm-chip.c
    > @@ -32,35 +32,6 @@ struct class *tpm_class;
    > struct class *tpmrm_class;
    > dev_t tpm_devt;
    >
    > -static int tpm_request_locality(struct tpm_chip *chip)
    > -{
    > - int rc;
    > -
    > - if (!chip->ops->request_locality)
    > - return 0;
    > -
    > - rc = chip->ops->request_locality(chip, 0);
    > - if (rc < 0)
    > - return rc;
    > -
    > - chip->locality = rc;
    > - return 0;
    > -}
    > -
    > -static void tpm_relinquish_locality(struct tpm_chip *chip)
    > -{
    > - int rc;
    > -
    > - if (!chip->ops->relinquish_locality)
    > - return;
    > -
    > - rc = chip->ops->relinquish_locality(chip, chip->locality);
    > - if (rc)
    > - dev_err(&chip->dev, "%s: : error %d\n", __func__, rc);
    > -
    > - chip->locality = -1;
    > -}
    > -
    > static int tpm_cmd_ready(struct tpm_chip *chip)
    > {
    > if (!chip->ops->cmd_ready)
    > @@ -103,17 +74,8 @@ int tpm_chip_start(struct tpm_chip *chip)
    >
    > tpm_clk_enable(chip);
    >
    > - if (chip->locality == -1) {
    > - ret = tpm_request_locality(chip);
    > - if (ret) {
    > - tpm_clk_disable(chip);
    > - return ret;
    > - }
    > - }
    > -
    > ret = tpm_cmd_ready(chip);
    > if (ret) {
    > - tpm_relinquish_locality(chip);
    > tpm_clk_disable(chip);
    > return ret;
    > }
    > @@ -133,7 +95,6 @@ EXPORT_SYMBOL_GPL(tpm_chip_start);
    > void tpm_chip_stop(struct tpm_chip *chip)
    > {
    > tpm_go_idle(chip);
    > - tpm_relinquish_locality(chip);
    > tpm_clk_disable(chip);
    > }
    > EXPORT_SYMBOL_GPL(tpm_chip_stop);
    > @@ -392,7 +353,6 @@ struct tpm_chip *tpm_chip_alloc(struct device *pdev,
    > goto out;
    > }
    >
    > - chip->locality = -1;
    > return chip;
    >
    > out:
    > diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
    > index 0959559..a95daf8 100644
    > --- a/drivers/char/tpm/tpm_tis_core.c
    > +++ b/drivers/char/tpm/tpm_tis_core.c
    > @@ -670,9 +670,6 @@ static int probe_itpm(struct tpm_chip *chip)
    > if (vendor != TPM_VID_INTEL)
    > return 0;
    >
    > - if (request_locality(chip, 0) != 0)
    > - return -EBUSY;
    > -
    > rc = tpm_tis_send_data(chip, cmd_getticks, len);
    > if (rc == 0)
    > goto out;
    > @@ -691,7 +688,6 @@ static int probe_itpm(struct tpm_chip *chip)
    >
    > out:
    > tpm_tis_ready(chip);
    > - release_locality(chip, priv->locality);
    >
    > return rc;
    > }
    > @@ -751,22 +747,13 @@ static int tpm_tis_gen_interrupt(struct tpm_chip *chip)
    > const char *desc = "attempting to generate an interrupt";
    > u32 cap2;
    > cap_t cap;
    > - int ret;
    >
    > /* TPM 2.0 */
    > if (chip->flags & TPM_CHIP_FLAG_TPM2)
    > return tpm2_get_tpm_pt(chip, 0x100, &cap2, desc);
    >
    > /* TPM 1.2 */
    > - ret = request_locality(chip, 0);
    > - if (ret < 0)
    > - return ret;
    > -
    > - ret = tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
    > -
    > - release_locality(chip, 0);
    > -
    > - return ret;
    > + return tpm1_getcap(chip, TPM_CAP_PROP_TIS_TIMEOUT, &cap, desc, 0);
    > }
    >
    > /* Register the IRQ and issue a command that will cause an interrupt. If an
    > @@ -880,6 +867,7 @@ void tpm_tis_remove(struct tpm_chip *chip)
    >
    > tpm_tis_write32(priv, reg, ~TPM_GLOBAL_INT_ENABLE & interrupt);
    >
    > + release_locality(chip, 0);
    > tpm_tis_clkrun_enable(chip, false);
    >
    > if (priv->ilb_base_addr)
    > @@ -1007,6 +995,14 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
    > goto out_err;
    > }
    >
    > + rc = request_locality(chip, 0);
    > + if (rc)
    > + goto out_err;
    > +
    > + rc = tpm_chip_start(chip);
    > + if (rc)
    > + goto out_err;
    > +
    > /* Take control of the TPM's interrupt hardware and shut it off */
    > rc = tpm_tis_read32(priv, TPM_INT_ENABLE(priv->locality), &intmask);
    > if (rc < 0)
    > @@ -1017,9 +1013,6 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
    > intmask &= ~TPM_GLOBAL_INT_ENABLE;
    > tpm_tis_write32(priv, TPM_INT_ENABLE(priv->locality), intmask);
    >
    > - rc = tpm_chip_start(chip);
    > - if (rc)
    > - goto out_err;
    > rc = tpm2_probe(chip);
    > tpm_chip_stop(chip);
    > if (rc)
    > @@ -1080,15 +1073,7 @@ int tpm_tis_core_init(struct device *dev, struct tpm_tis_data *priv, int irq,
    > * to make sure it works. May as well use that command to set the
    > * proper timeouts for the driver.
    > */
    > -
    > - rc = request_locality(chip, 0);
    > - if (rc < 0)
    > - goto out_err;
    > -
    > rc = tpm_get_timeouts(chip);
    > -
    > - release_locality(chip, 0);
    > -
    > if (rc) {
    > dev_err(dev, "Could not get TPM timeouts and durations\n");
    > rc = -ENODEV;
    > diff --git a/include/linux/tpm.h b/include/linux/tpm.h
    > index 8f4ff39..55debe6 100644
    > --- a/include/linux/tpm.h
    > +++ b/include/linux/tpm.h
    > @@ -160,9 +160,6 @@ struct tpm_chip {
    > u32 last_cc;
    > u32 nr_commands;
    > u32 *cc_attrs_tbl;
    > -
    > - /* active locality */
    > - int locality;
    > };
    >
    > #define TPM_HEADER_SIZE 10
    > --
    > 2.7.4
    >

    /Jarkko

    \
     
     \ /
      Last update: 2021-04-28 01:49    [W:4.384 / U:0.104 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site