On 10/01/2016 12:35 AM, Jarkko Sakkinen wrote:
> On Wed, Sep 28, 2016 at 04:34:38AM -0400, Nayna Jain wrote:
>> Currently, read_log() has two implementations: one for ACPI platforms
>> and the other for OF platforms. The proper one is selected at compile
>> time using Kconfig and #ifdef in the Makefile, which is not the
>> recommended approach.
>>
>> This patch removes the #ifdef in the Makefile by defining a single
>> read_log() method, which checks for ACPI/OF event log properties at
>> runtime.
>>
>> Suggested-by: Jason Gunthorpe <jguntho...@obsidianresearch.com>
>> Signed-off-by: Nayna Jain <na...@linux.vnet.ibm.com>
>> Reviewed-by: Jason Gunthorpe <jguntho...@obsidianresearch.com>
>> ---
>>   drivers/char/tpm/Makefile       | 14 ++++----------
>>   drivers/char/tpm/tpm_acpi.c     |  9 ++-------
>>   drivers/char/tpm/tpm_eventlog.c | 18 ++++++++++++++++++
>>   drivers/char/tpm/tpm_eventlog.h | 22 +++++++++++++---------
>>   drivers/char/tpm/tpm_of.c       |  8 ++------
>>   5 files changed, 39 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/char/tpm/Makefile b/drivers/char/tpm/Makefile
>> index a385fb8..a05b1eb 100644
>> --- a/drivers/char/tpm/Makefile
>> +++ b/drivers/char/tpm/Makefile
>> @@ -2,16 +2,10 @@
>>   # Makefile for the kernel tpm device drivers.
>>   #
>>   obj-$(CONFIG_TCG_TPM) += tpm.o
>> -tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o
>> -tpm-$(CONFIG_ACPI) += tpm_ppi.o
>> -
>> -ifdef CONFIG_ACPI
>> -    tpm-y += tpm_eventlog.o tpm_acpi.o
>> -else
>> -ifdef CONFIG_TCG_IBMVTPM
>> -    tpm-y += tpm_eventlog.o tpm_of.o
>> -endif
>> -endif
>> +tpm-y := tpm-interface.o tpm-dev.o tpm-sysfs.o tpm-chip.o tpm2-cmd.o \
>> +            tpm_eventlog.o
>> +tpm-$(CONFIG_ACPI) += tpm_ppi.o tpm_acpi.o
>> +tpm-$(CONFIG_OF) += tpm_of.o
>>   obj-$(CONFIG_TCG_TIS_CORE) += tpm_tis_core.o
>>   obj-$(CONFIG_TCG_TIS) += tpm_tis.o
>>   obj-$(CONFIG_TCG_TIS_SPI) += tpm_tis_spi.o
>> diff --git a/drivers/char/tpm/tpm_acpi.c b/drivers/char/tpm/tpm_acpi.c
>> index 4d6c2d7..859bdba 100644
>> --- a/drivers/char/tpm/tpm_acpi.c
>> +++ b/drivers/char/tpm/tpm_acpi.c
>> @@ -6,6 +6,7 @@
>>    * Stefan Berger <stef...@us.ibm.com>
>>    * Reiner Sailer <sai...@watson.ibm.com>
>>    * Kylene Hall <kjh...@us.ibm.com>
>> + *  Nayna Jain <na...@linux.vnet.ibm.com>
>>    *
>>    * Maintained by: <tpmdd-devel@lists.sourceforge.net>
>>    *
>> @@ -45,7 +46,7 @@ struct acpi_tcpa {
>>   };
>>
>>   /* read binary bios log */
>> -int read_log(struct tpm_chip *chip)
>> +int read_log_acpi(struct tpm_chip *chip)
>>   {
>>      struct acpi_tcpa *buff;
>>      acpi_status status;
>> @@ -54,12 +55,6 @@ int read_log(struct tpm_chip *chip)
>>      struct tpm_bios_log *log;
>>
>>      log = &chip->log;
>> -    if (log->bios_event_log != NULL) {
>> -            printk(KERN_ERR
>> -                   "%s: ERROR - Eventlog already initialized\n",
>> -                   __func__);
>> -            return -EFAULT;
>> -    }
>>
>>      /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */
>>      status = acpi_get_table(ACPI_SIG_TCPA, 1,
>> diff --git a/drivers/char/tpm/tpm_eventlog.c 
>> b/drivers/char/tpm/tpm_eventlog.c
>> index a8cd4a1..c327089 100644
>> --- a/drivers/char/tpm/tpm_eventlog.c
>> +++ b/drivers/char/tpm/tpm_eventlog.c
>> @@ -346,6 +346,24 @@ static int is_bad(void *p)
>>      return 0;
>>   }
>>
>> +int read_log(struct tpm_chip *chip)
>> +{
>> +    int rc;
>> +
>> +    if (chip->log.bios_event_log != NULL) {
>> +            dev_dbg(&chip->dev, "%s: ERROR - Eventlog already 
>> initialized\n",
>> +                    __func__);
>> +            return -EFAULT;
>> +    }
>> +
>> +    rc = read_log_acpi(chip);
>> +    if ((rc == 0) || (rc == -ENOMEM))
>> +            return rc;
>> +    rc = read_log_of(chip);
>> +    return rc;
>> +
>> +}
>
> I'm wondering if it is a better idea to leverage tpm_class_ops? This
> would be kind of cool idea to implement this because then the decision
> to support event log could be leveraged to the driver level.

Can you please explain me this bit more ?

For eg, when we say driver level.. does that mean to the level of tis, 
nuvoton ?

And what type of decision ? I mean like decision to create securityfs 
setup for userspace to read eventlog ? or there are other things ?

>
> If the event_log pointer is NULL, then event log is not supported.

Yeah, sure will fix this.

Thanks & Regards,
     - Nayna
>
> /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
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

Reply via email to