From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752351AbdAYS7X (ORCPT ); Wed, 25 Jan 2017 13:59:23 -0500 Received: from mga11.intel.com ([192.55.52.93]:22726 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751513AbdAYS7W (ORCPT ); Wed, 25 Jan 2017 13:59:22 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,285,1477983600"; d="scan'208";a="1117778551" Date: Wed, 25 Jan 2017 20:59:19 +0200 From: Jarkko Sakkinen To: Andrey Pronin Cc: linux-kernel@vger.kernel.org, semenzato@chromium.org, tpmdd-devel@lists.sourceforge.net, groeck@chromium.org Subject: Re: [tpmdd-devel] [PATCH] tpm/tpm_i2c_infineon: ensure no ongoing commands on shutdown Message-ID: <20170125185919.jpd46e4774t65e5s@intel.com> References: <20170114000954.17728-1-apronin@chromium.org> <20170114002857.GA5851@obsidianresearch.com> <20170114004230.GA21035@apronin> <20170116093318.wmmr2z54pc6k7t2c@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170116093318.wmmr2z54pc6k7t2c@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.6.2-neo (2016-08-21) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jan 16, 2017 at 11:33:18AM +0200, Jarkko Sakkinen wrote: > On Fri, Jan 13, 2017 at 04:42:30PM -0800, Andrey Pronin wrote: > > 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. > > Why don't you fix the tpm-sysfs issue but rather misusing > tpm_chip_unregister? Ignore this. I wasn't following this thread properly. Only now had time read carefully through the discussion. /Jarkko