On Fri, Apr 28, 2017 at 04:24:39PM -0600, Jason Gunthorpe wrote: > On Fri, Apr 28, 2017 at 03:07:02PM -0700, Josh Zimmerman wrote: > > On Fri, Apr 28, 2017 at 2:39 PM, Jason Gunthorpe > > <[email protected]> wrote: > > > On Fri, Apr 28, 2017 at 02:32:31PM -0700, Josh Zimmerman wrote: > > >> On Fri, Apr 28, 2017 at 11:43 AM, Jason Gunthorpe > > >> <[email protected]> wrote: > > >> > On Fri, Apr 28, 2017 at 11:27:16AM -0700, Josh Zimmerman wrote: > > >> >> It sounds like there are several intertwined issues here. I'd like to > > >> >> keep this patch relatively minimal, so here are my current thoughts on > > >> >> a path forward: > > >> >> > > >> >> * Refactor references to chip->ops to go through tpm_try_get_ops, as > > >> >> mentioned in the previous patch > > >> > > > >> > As I said, if you rely on the fact that sysfs does not exist for TPM2 > > >> > then this should already be done for the TPM2 case and an incremental > > >> > later patch could solve this problem in sysfs to turn shutdown on for > > >> > tpm1. As long as this logic is clearly documented it is probably an OK > > >> > step for now. > > >> > > >> This I'm not actually sure about: it seems that there are other places > > >> than sysfs where chip->ops is referenced without being guarded by > > >> tpm_try_get_ops. If we rely on just NULLing out chip->ops this could > > >> be dangerous. I'll look at sending a patch for this first. > > > > > > If that is the case then we have an existing bug.. > > > > > > The only other user entry point is via the cdev, and those call > > > try_get_ops around all calls down into the tpm code. > > > > > > Everything coming in via the kapi must call tpm_chip_find_get, which > > > does try_get_ops. > > > > OK, then I'm confused about the problem with sysfs. If everything > > is already grabbing a ref first, why can't we just grab a ref and > > NULL out chip->ops? > > > sysfs relies on an implicit ref that is held across the duration of > the sysfs file's lifetime. That ref is not given up until device_del > returns, which serializes everything. See the comment in > tpm_sysfs_add_device > > This is fine so long as ops is only ever null'd after device_del, but > this shutdown work proposes to change that invariant.. > > To compensate, the ops lock would have to be pushed down into each > sysfs method (for tpm1), or just not use sysfs (like tpm2 does) > > Jason
Ah. Ignore my last response. /Jarkko ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ tpmdd-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
