Re: [PATCH v2 1/2] ACPICA: Update TPM2 ACPI table

[Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>]

On 03/15/17 11:02, Jarkko Sakkinen wrote:
> On Tue, Mar 14, 2017 at 08:32:32PM -0500, Jiandi An wrote:
>> TCG ACPI Specification Family "1.2" and "2.0" Version 1.2
>> Revision 8 introduces new start method for ARM SMC.
>>
>> - Add new start method (type 11) for ARM SMC
>> - Add start method specific parameters for ARM SMC start method
>>
>> Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> ---
>>   drivers/char/tpm/tpm_crb.c |  6 +++++-
>>   drivers/char/tpm/tpm_tis.c |  6 +++++-
>>   include/acpi/actbl2.h      | 12 ++++++++++++
>>   3 files changed, 22 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
>> index cb6fb13..089fcf8 100644
>> --- a/drivers/char/tpm/tpm_crb.c
>> +++ b/drivers/char/tpm/tpm_crb.c
>> @@ -410,12 +410,16 @@ static int crb_acpi_add(struct acpi_device
> *device)
>>   	struct tpm_chip *chip;
>>   	struct device *dev = &device->dev;
>>   	acpi_status status;
>> +	u32 default_len;
>>   	u32 sm;
>>   	int rc;
>>
>> +	default_len = sizeof(struct acpi_table_tpm2) -
>> +		      sizeof(union platform_params);
>
> Maybe you should consider not putting struct crb_smc to actbl2.h. This
> makes tpm_crb.c a mess.

Will fix this in v3.
- Jiandi

>
>> +
>>   	status = acpi_get_table(ACPI_SIG_TPM2, 1,
>>   				(struct acpi_table_header **) &buf);
>> -	if (ACPI_FAILURE(status) || buf->header.length < sizeof(*buf)) {
>> +	if (ACPI_FAILURE(status) || buf->header.length < default_len) {
>>   		dev_err(dev, FW_BUG "failed to get TPM2 ACPI table\n");
>>   		return -EINVAL;
>>   	}
>> diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
>> index c7e1384..0e2e5f6 100644
>> --- a/drivers/char/tpm/tpm_tis.c
>> +++ b/drivers/char/tpm/tpm_tis.c
>> @@ -253,11 +253,15 @@ static int tpm_tis_acpi_init(struct acpi_device
> *acpi_dev)
>>   	acpi_status st;
>>   	struct list_head resources;
>>   	struct tpm_info tpm_info = {};
>> +	u32 default_len;
>>   	int ret;
>>
>> +	default_len = sizeof(struct acpi_table_tpm2) -
>> +		      sizeof(union platform_params);
>> +
>
> And more clutter.
>
>>   	st = acpi_get_table(ACPI_SIG_TPM2, 1,
>>   			    (struct acpi_table_header **) &tbl);
>> -	if (ACPI_FAILURE(st) || tbl->header.length < sizeof(*tbl)) {
>> +	if (ACPI_FAILURE(st) || tbl->header.length < default_len) {
>>   		dev_err(&acpi_dev->dev,
>>   			FW_BUG "failed to get TPM2 ACPI table\n");
>>   		return -EINVAL;
>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>> index 7aee9fb..9612049 100644
>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -1277,6 +1277,14 @@ struct acpi_table_tcpa_server {
>>    *
>>
> **************************************************************************
> ****/
>>
>> +struct tpm2_crb_smc {
>> +	u32 interrupt;
>> +	u8 interrupt_flags;
>> +	u8 op_flags;
>> +	u16 reserved2;
>> +	u32 smc_func_id;
>> +};
>> +
>>   struct acpi_table_tpm2 {
>>   	struct acpi_table_header header;	/* Common ACPI table
> header */
>>   	u16 platform_class;
>> @@ -1285,6 +1293,9 @@ struct acpi_table_tpm2 {
>>   	u32 start_method;
>>
>>   	/* Platform-specific data follows */
>> +	union platform_params {
>> +		struct tpm2_crb_smc smc_params;
>> +	} platform_data;
>
> Why the union type is not anonymous? ACPICA change should be its
> own commit.
>
> /Jarkko

Thanks. I realized it's not a good idea to change the size of 
acpi_table_tpm2.
I will address this in V3 to avoid clutters in tpm_crb and tpm_tis
driver and make ACPICA change its own commit.

- Jiandi

>
> --------------------------------------------------------------------------
> ----
> 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-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> https://lists.sourceforge.net/lists/listinfo/tpmdd-devel
>


-- 
Qualcomm Datacenter Technologies, Inc.
as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot