On 11/17/2016 01:10 PM, Jason Gunthorpe wrote: > 1;2802;0cOn Thu, Nov 17, 2016 at 07:35:05AM -0500, Stefan Berger wrote: >> I ran the vtpm driver test suite (with -j32) a few times at that patch and >> it didn't crash. It crashes severely with later patches applied. Here's the >> current experimental patch that fixes these problems: > I can't see how setting owner has any bearing on this.. I also don't > see why it should ever fail at all... It would be great to get a root > cause here - could it be memory corruption???? > > Getting a really bad feeling from this :( > >> iff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c >> index 0cb43ef..a73295a 100644 >> +++ b/drivers/char/tpm/tpm_acpi.c >> @@ -56,6 +56,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip) >> >> log = &chip->log; >> >> + if (!chip->acpi_dev_handle) >> + return 0; >> + >> >> // So ACPI is not supported on this device, but ACPI support is compiled in. >> I am returning 0 here, assuming it's not an OF device and the corresponding >> OF function need not be called (see below). > Return -ENODEV > >> + if (!(chip->flags & TPM_CHIP_FLAG_VIRTUAL)) >> + rc = tpm_read_log_of(chip); >> >> // I am not sure how to handle this case, in case we get here, which would >> only be on an OF device (following 'return 0;' above), but we don't want to >> attempt to read the log there, either. I think the most straight-forward way >> would be to gate this whole function with a flag that only the vtpm proxy >> driver has: TPM_CHIP_FLAG_NO_FIRMWARE_LOG. > OF is already fine, it checks chip->dev.parent->of_node so it will > exit properly for vtpm, no need for this.
In the case of x86, tpm_read_log_of() is a stub return -ENODEV, which in turn fails the whole device: http://git.infradead.org/users/jjs/linux-tpmdd.git/blob/4d388433e85f8257f5a9344a7acf6f499ba2b29e:/drivers/char/tpm/tpm_eventlog.h#l87 http://git.infradead.org/users/jjs/linux-tpmdd.git/blob/4d388433e85f8257f5a9344a7acf6f499ba2b29e:/drivers/char/tpm/tpm-chip.c#l348 So we must not run into that or handle -ENODEV differently or return a different error code in the stub function. I think the OF log reading code will also need to check for chip->dev.parent being NULL. Further, I had the impression that the error unwinding following -ENODEV has an issue related to sysfs. Stefan > > Jason > ------------------------------------------------------------------------------ _______________________________________________ tpmdd-devel mailing list tpmdd-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
