Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jason Gunthorpe
On Thu, Oct 06, 2016 at 12:43:13AM +, Winkler, Tomas wrote: > > You keep asserting that, but it just isn't true at all. > > Okay, let's rephrase, that calling device_del before tpm_transmit is not sane > when using runtime_pm. Maybe, but I haven't heard an explanation from you why that is t

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Winkler, Tomas
> On Wed, Oct 05, 2016 at 08:09:17PM +, Winkler, Tomas wrote: > > > It could, but that patch was not merged yet, and I believe even if the > > issue is exposed only with runtime_pm currently, we have a bug in > > design even w/o runtime pm. > > Please don't make changes without any justifica

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jason Gunthorpe
On Wed, Oct 05, 2016 at 08:09:17PM +, Winkler, Tomas wrote: > It could, but that patch was not merged yet, and I believe even if > the issue is exposed only with runtime_pm currently, we have a bug > in design even w/o runtime pm. Please don't make changes without any justification :( > > Th

Re: [tpmdd-devel] [PATCH v2 1/2] Documentation: tpm: add the IBM Virtual TPM device tree binding documentation

2016-10-05 Thread Nayna
On 09/29/2016 04:34 PM, Jarkko Sakkinen wrote: > On Wed, Sep 28, 2016 at 04:30:40AM -0400, Nayna Jain wrote: >> Virtual TPM, which is being used on IBM POWER7+ and POWER8 systems running >> POWERVM, is currently supported by tpm device driver but lacks the >> documentation. This patch adds the mi

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Winkler, Tomas
> > On Wed, Oct 05, 2016 at 07:48:59AM +, Winkler, Tomas wrote: > > > > down_write(&chip->ops_sem); > > > > chip->ops = NULL; > > > > up_write(&chip->ops_sem); > > > > > > No, that is wrong as well, another thread can issue a TPM command > > > between the device_del and the ops = NULL. Presum

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jason Gunthorpe
On Wed, Oct 05, 2016 at 07:48:59AM +, Winkler, Tomas wrote: > > > down_write(&chip->ops_sem); > > > chip->ops = NULL; > > > up_write(&chip->ops_sem); > > > > No, that is wrong as well, another thread can issue a TPM command between > > the device_del and the ops = NULL. Presumably that will fa

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jason Gunthorpe
On Wed, Oct 05, 2016 at 06:15:26PM +0300, Jarkko Sakkinen wrote: > The important thing is to notice that runtime PM requires the device > to be "alive" and in the device hierarchy. It's a constraint... There are two devices. The chip->dev and the chip->dev.parent (aka the acpi_device) Runtime P

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jason Gunthorpe
On Wed, Oct 05, 2016 at 01:02:34PM +0300, Jarkko Sakkinen wrote: > I'll repeat my question: what worse can happen than returning -EPIPE? I > though the whole rw lock scheme was introduced just for this purpose. I thought I explained this, if device_del is moved after ops = null then if sysfs loo

Re: [tpmdd-devel] [PATCH] tpm/tpm_crb: revamp starting method setting.

2016-10-05 Thread Winkler, Tomas
> On Wed, Oct 05, 2016 at 04:12:08PM +0300, Tomas Winkler wrote: > > Encapsulate the start method parsing in a single function and add > > needed debug printouts. > > It eliminates small issue with useless double checking for > > ACPI_TPM2_MEMORY_MAPPED. > > Start method is trival to check from TP

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jarkko Sakkinen
On Wed, Oct 05, 2016 at 07:48:59AM +, Winkler, Tomas wrote: > > > > > > > On Tue, Oct 04, 2016 at 08:19:46AM +0300, Jarkko Sakkinen wrote: > > > > > > > > > > Make the driver uncallable first. The worst race that can happen > > > > > > is that open("/dev/tpm0", ...) returns -EPIPE. I do not con

Re: [tpmdd-devel] [PATCH] tpm/tpm_crb: revamp starting method setting.

2016-10-05 Thread Jarkko Sakkinen
On Wed, Oct 05, 2016 at 04:12:08PM +0300, Tomas Winkler wrote: > Encapsulate the start method parsing in a single function > and add needed debug printouts. > It eliminates small issue with useless double checking for > ACPI_TPM2_MEMORY_MAPPED. Start method is trival to check from TPM2 dump. I'm n

[tpmdd-devel] [PATCH] tpm/tpm_crb: revamp starting method setting.

2016-10-05 Thread Tomas Winkler
Encapsulate the start method parsing in a single function and add needed debug printouts. It eliminates small issue with useless double checking for ACPI_TPM2_MEMORY_MAPPED. Signed-off-by: Tomas Winkler --- drivers/char/tpm/tpm_crb.c | 65 +++--- 1 file ch

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jarkko Sakkinen
On Tue, Oct 04, 2016 at 10:47:38AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 04, 2016 at 08:19:46AM +0300, Jarkko Sakkinen wrote: > > > > Make the driver uncallable first. The worst race that can happen is that > > > open("/dev/tpm0", ...) returns -EPIPE. I do not consider this fatal at > > > al

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Jarkko Sakkinen
On Tue, Oct 04, 2016 at 10:47:38AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 04, 2016 at 08:19:46AM +0300, Jarkko Sakkinen wrote: > > > > Make the driver uncallable first. The worst race that can happen is that > > > open("/dev/tpm0", ...) returns -EPIPE. I do not consider this fatal at > > > al

Re: [tpmdd-devel] [PATCH v4 3/8] tpm: validate event log access before tpm_bios_log_setup

2016-10-05 Thread Jarkko Sakkinen
On Tue, Oct 04, 2016 at 11:12:31AM -0600, Jason Gunthorpe wrote: > On Tue, Oct 04, 2016 at 08:26:51AM +0300, Jarkko Sakkinen wrote: > > On Mon, Oct 03, 2016 at 03:11:29PM -0600, Jason Gunthorpe wrote: > > > On Mon, Oct 03, 2016 at 11:22:30PM +0300, Jarkko Sakkinen wrote: > > > > > > > > Sort of, t

Re: [tpmdd-devel] [PATCH] tpm: don't destroy chip device prematurely

2016-10-05 Thread Winkler, Tomas
> > > > > On Tue, Oct 04, 2016 at 08:19:46AM +0300, Jarkko Sakkinen wrote: > > > > > > > > Make the driver uncallable first. The worst race that can happen > > > > > is that open("/dev/tpm0", ...) returns -EPIPE. I do not consider > > > > > this fatal at all. > > > > > > > > No responses for this r