Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1558809

Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on shutdown

Path csiph.com!aioe.org!bofh.it!news.nic.it!robomod
From Andrey Pronin <apronin@chromium.org>
Newsgroups linux.kernel
Subject Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on shutdown
Date Sat, 14 Jan 2017 01:50:01 +0100
Message-ID <sZkHD-7ID-1@gated-at.bofh.it> (permalink)
References <sZkeB-7yO-3@gated-at.bofh.it> <sZkxX-7Fv-5@gated-at.bofh.it>
Dkim-Signature v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=mCaTZLspUYWl8jdRYNYrvBxyTyA84uEhuahbKGJXpLY=; b=dG5TwoFb5LtRoJzOWbKi4lw1GgzF8OEU5pITvyV4Z+rPpOo9fsL3gOz39764H6lEGn KxZxz/1RcSV/ReyUX7jWYgjmbvuAmoTLKP+TSZlrzL32XFy8zJxJf9vr4eXnsX0h6pxf DCE/pWIqOXA76e1PCF5c0VOKJW2KB/ZEqbCOU=
X-Google-Dkim-Signature v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=mCaTZLspUYWl8jdRYNYrvBxyTyA84uEhuahbKGJXpLY=; b=R0skEe8tjydGN3kXvhHA3n5wOYgBEzoL7yclrRr2N14A+jRKIECYD+Ljp/jJHWtudy Ygg5RNtNBQFxdz7NFDovIEEkf4sIe3g7G/9bHYoQbuKu5kojDa201lvVhhs9/as5pNOf 3BV0OJ6Ysx0hRFIzf28de79zRwLifLepntjfHSXyXFmzmV1g+F4mtlsaiPXUI5whL3KF Rek08f9B9BuLKtIxGylCDvzXm8TsYR43bsG5IvSUnu9Qu1u3b9HbshiCE7Al8/hO4hkB y1DEB4g1DXx5qQ/xqlsk6P3SmvGEu3g6TlwZUA3+phc4sFnlOw0Kbd3ofm1PzXkJfPLT Yqyw==
X-Gm-Message-State AIkVDXJlwGSI5NJ6iRKuGNcxbZiZD7bHnfoSEvpZqTJI8uscAfFfYIEuMDqdAwOHmTTFOZd7
X-Received by 10.36.25.143 with SMTP id b137mr5307685itb.54.1484354553174; Fri, 13 Jan 2017 16:42:33 -0800 (PST)
MIME-Version 1.0
Content-Type text/plain; charset=us-ascii
Content-Disposition inline
User-Agent Mutt/1.5.21 (2010-09-15)
Sender robomod@news.nic.it
List-ID <linux-kernel.vger.kernel.org>
X-Mailing-List linux-kernel@vger.kernel.org
Approved robomod@news.nic.it
Lines 37
Organization linux.* mail to news gateway
X-Original-Cc Peter Huewe <peterhuewe@gmx.de>, Marcel Selhorst <tpmdd@selhorst.net>, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>, tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, semenzato@chromium.org, groeck@chromium.org
X-Original-Date Fri, 13 Jan 2017 16:42:30 -0800
X-Original-Message-ID <20170114004230.GA21035@apronin>
X-Original-References <20170114000954.17728-1-apronin@chromium.org> <20170114002857.GA5851@obsidianresearch.com>
X-Original-Sender linux-kernel-owner@vger.kernel.org
Xref csiph.com linux.kernel:1558809

Show key headers only | View raw


On Fri, Jan 13, 2017 at 05:28:57PM -0700, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2017 at 04:09:54PM -0800, Andrey Pronin wrote:
> > Resetting TPM while processing a command may lead to issues
> > on the next boot. Ensure that we don't have any ongoing
> > commands, and that no further commands can be sent to the chip
> > by unregistering the device in the shutdown handler.
> > tpm_chip_unregister() waits for the completion of an ongoing
> > command, if any, and then clears out chip->ops and unregisters
> > sysfs entities.
> 
> Unregistering in a shutdown handler seems very strange, it also waits
> for userspace things, so I wonder if it could be problematic?
> 
> Maybe just use
> 
>    down_write(&chip->ops_sem);
>    chip->ops = NULL;
>    up_write(&chip->ops_sem);
> 
> In the shutdown handler?

down_write(&chip->ops_sem) would still wait for completing the initiated
writes, since tpm_write() in tpm-dev.c calls tpm_try_get_ops().
Also, tpm-sysfs.c calls chip->ops directly, so sysfs should be
unregistered first.
And the last thing, this driver supports TPM 1.2, but if it was a 2.0
chip, it'd also need to send TPM2_Shutdown(CLEAR) from its shutdown
handler (or get an unorderly shutdown and DA counter increment).

All these things are handled by tpm_chip_unregister(). I thought about
creating a tpm_chip_shutdown routine that could be called from shutdown
handlers of the drivers that need it (and I'd do it for every driver,
especially in 2.0 case). But decided that it's better to reuse the
existing tpm_chip_unregister() that already does what's needed.

> 
> Jason

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on shutdown Andrey Pronin <apronin@chromium.org> - 2017-01-14 01:20 +0100
  Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on  shutdown Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-14 01:40 +0100
    Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on  shutdown Andrey Pronin <apronin@chromium.org> - 2017-01-14 01:50 +0100
      Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on  shutdown Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> - 2017-01-16 10:40 +0100
      Re: [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on  shutdown Jason Gunthorpe <jgunthorpe@obsidianresearch.com> - 2017-01-16 17:20 +0100

csiph-web