Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752966AbcJCRai (ORCPT ); Mon, 3 Oct 2016 13:30:38 -0400 Received: from quartz.orcorp.ca ([184.70.90.242]:49374 "EHLO quartz.orcorp.ca" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751559AbcJCRab (ORCPT ); Mon, 3 Oct 2016 13:30:31 -0400 Date: Mon, 3 Oct 2016 11:30:28 -0600 From: Jason Gunthorpe To: "Winkler, Tomas" Cc: Jarkko Sakkinen , "tpmdd-devel@lists.sourceforge.net" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] tpm: don't destroy chip device prematurely Message-ID: <20161003173028.GF6801@obsidianresearch.com> References: <1475393971-12715-1-git-send-email-tomas.winkler@intel.com> <20161002101755.GA25844@intel.com> <20161002102455.GA27464@intel.com> <20161002212126.GA25872@obsidianresearch.com> <5B8DA87D05A7694D9FA63FD143655C1B542F466B@hasmsx108.ger.corp.intel.com> <20161003160008.GE5722@obsidianresearch.com> <5B8DA87D05A7694D9FA63FD143655C1B542F474C@hasmsx108.ger.corp.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5B8DA87D05A7694D9FA63FD143655C1B542F474C@hasmsx108.ger.corp.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-Broken-Reverse-DNS: no host name found for IP address 10.0.0.151 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1475 Lines: 39 On Mon, Oct 03, 2016 at 05:16:18PM +0000, Winkler, Tomas wrote: > > > Please be more specific regarding flows you think will be wrong with > > > this patch, you must agree that the current code is broken even w/o > > > runtime pm. > > > > No, I don't agree. Accessing dev->name is OK after the device_del. > > But you cannot assume that just dev->name is accessed and and > runtime_pm breaks this assumption.. It was built around that assumption. Our chip methods have been built to not require the device to be registered. We call many of them before we even do device registration, for instance. The pm patches can't break that. What is the actual problem anyhow? > > What devicde_del does is fence off all sorts of ways to access the device, eg > > sysfs files, bus registrations, etc, etc. That absolutely must be done before > > calling tpm_suspend. > > But tpm2_shutdown is acceded via tpm_chip , we cannot call > device_del before, this is just wrong from the Linux device mode > perspective, we have to use other means to close the access to the > device. device_del is the means to close access - that it what it does - unregister the device from the system. The tpm_chip must be operational independent of a *registered* device. chip methods can only assume that the device is *initialized* This is a basic pattern followed by other subsystems. This is why it is OK to look at dev->name, but not okay to muck with other stuff under a chip method. Jason