[PATCH 0/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[PATCH 0/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jiandi An]

TCG ACPI Specification Family "1.2" and "2.0" Version 1.2
Revision 8 introduces a new start method (type 11) for ARM64,
along with platform specific paramters for this new start
method.  This new start method invokes a Secure Monitor Call
to request the firmware to execute or cancel a TPM 2.0 command.
These 3 patches enables TPM CRB driver for ARM64 and implements
the new start method for ARM64 in the TPM CRB driver.

Jiandi An (3):
  ACPICA: Update TPM2 ACPI table
  tpm: Add start method for ARM Secure Monitor Call
  tpm/tpm_crb: Enable TPM CRB interface for ARM64

 drivers/char/tpm/Kconfig   |  2 +-
 drivers/char/tpm/tpm.h     | 29 +++++++++++++++++++++++++++++
 drivers/char/tpm/tpm_crb.c | 30 +++++++++++++++++++++++++++---
 drivers/char/tpm/tpm_tis.c |  6 +++++-
 include/acpi/actbl2.h      | 12 ++++++++++++
 5 files changed, 74 insertions(+), 5 deletions(-)

-- 
Jiandi An
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.


------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

[PATCH 1/3] ACPICA: Update TPM2 ACPI table

[Jiandi An]

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);
+
 	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);
+
 	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;
 };
 
 /* Values for start_method above */
@@ -1294,6 +1305,7 @@ struct acpi_table_tpm2 {
 #define ACPI_TPM2_MEMORY_MAPPED                     6
 #define ACPI_TPM2_COMMAND_BUFFER                    7
 #define ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD  8
+#define ACPI_TPM2_COMMAND_BUFFER_WITH_SMC          11
 
 /*******************************************************************************
  *
-- 
Jiandi An
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.


------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

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

[Moore, Robert]

This appears to be the latest version on the TCG website:

*TCG ACPI Specification for Family 1.2 and 2.0, Level 00, Revision 00.37
December 19, 2014

Which is what ACPICA is using.

Please send me a link to a newer version if you have it.
Thanks,
Bob

> -----Original Message-----
> From: Jiandi An [mailto:anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org]
> Sent: Friday, March 10, 2017 1:58 AM
> To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Cc: peterhuewe-Mmb7MZpHnFY@public.gmane.org; tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org;
> jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org; jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org; Moore,
> Robert <robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Zheng, Lv <lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
> Wysocki, Rafael J <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Jiandi
> An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Subject: [PATCH 1/3] ACPICA: Update TPM2 ACPI table
> 
> 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);
> +
>  	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);
> +
>  	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;
>  };
> 
>  /* Values for start_method above */
> @@ -1294,6 +1305,7 @@ struct acpi_table_tpm2 {
>  #define ACPI_TPM2_MEMORY_MAPPED                     6
>  #define ACPI_TPM2_COMMAND_BUFFER                    7
>  #define ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD  8
> +#define ACPI_TPM2_COMMAND_BUFFER_WITH_SMC          11
> 
> 
> /***********************************************************************
> ********
>   *
> --
> Jiandi An
> 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.


------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

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

[Jiandi An]

On 03/10/17 09:35, Moore, Robert wrote:
> This appears to be the latest version on the TCG website:
>
> *TCG ACPI Specification for Family 1.2 and 2.0, Level 00, Revision 00.37
> December 19, 2014
>
> Which is what ACPICA is using.
>
> Please send me a link to a newer version if you have it.

Hi Bob,
Here is the link to "TCG ACPI Specification Family "1.2" and "2.0" 
Version 1.2 Revision 8".  Thanks.

https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification-Family-1.2-and-2.0-Ver1.2-Rev8_public-revie....pdf

https://trustedcomputinggroup.org/specifications-public-review/

> Thanks,
> Bob
>
>> -----Original Message-----
>> From: Jiandi An [mailto:anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org]
>> Sent: Friday, March 10, 2017 1:58 AM
>> To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
>> Cc: peterhuewe-Mmb7MZpHnFY@public.gmane.org; tpmdd-yWjUBOtONefk1uMJSBkQmQ@public.gmane.org;
>> jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org; jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org; Moore,
>> Robert <robert.moore-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Zheng, Lv <lv.zheng-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>;
>> Wysocki, Rafael J <rafael.j.wysocki-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; lenb-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Jiandi
>> An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>> Subject: [PATCH 1/3] ACPICA: Update TPM2 ACPI table
>>
>> 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);
>> +
>>   	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);
>> +
>>   	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;
>>   };
>>
>>   /* Values for start_method above */
>> @@ -1294,6 +1305,7 @@ struct acpi_table_tpm2 {
>>   #define ACPI_TPM2_MEMORY_MAPPED                     6
>>   #define ACPI_TPM2_COMMAND_BUFFER                    7
>>   #define ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD  8
>> +#define ACPI_TPM2_COMMAND_BUFFER_WITH_SMC          11
>>
>>
>> /***********************************************************************
>> ********
>>    *
>> --
>> Jiandi An
>> 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.
>


-- 
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.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

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

[Jarkko Sakkinen]

On Fri, Mar 10, 2017 at 03:58:07AM -0600, 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>

Could you briefly describe what SMC is? I don't know

1. What the abbrevation stands for.
2. What it is.

/Jarkko

> ---
>  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);
> +
>  	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);
> +
>  	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;
>  };
>  
>  /* Values for start_method above */
> @@ -1294,6 +1305,7 @@ struct acpi_table_tpm2 {
>  #define ACPI_TPM2_MEMORY_MAPPED                     6
>  #define ACPI_TPM2_COMMAND_BUFFER                    7
>  #define ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD  8
> +#define ACPI_TPM2_COMMAND_BUFFER_WITH_SMC          11
>  
>  /*******************************************************************************
>   *
> -- 
> Jiandi An
> 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.
> 

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

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

[Jiandi An]

On 03/11/17 02:19, Jarkko Sakkinen wrote:
> On Fri, Mar 10, 2017 at 03:58:07AM -0600, 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>
>
> Could you briefly describe what SMC is? I don't know
>
> 1. What the abbrevation stands for.
SMC stands for Secure Monitor Call.

> 2. What it is.
In ARM, TrustZone security extensions ennable a Secure software
environment with Secure Monitor mode.  A Secure Monitor Call (SMC) is
used to enter the Secure Monitor mode and perform a Secure Monitor
service call.  Software executing in the non-secure state and in the
secure state at exception levels lower than EL3 in ARM will request
runtime services using the Secure Monitor Call (SMC) instruction to
enter EL3 secure mode.

I'll include a brief description in commit message of patch as well
in next version of the patch.

Thanks.
- Jiandi

>
> /Jarkko
>
>> ---
>>   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);
>> +
>>   	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);
>> +
>>   	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;
>>   };
>>
>>   /* Values for start_method above */
>> @@ -1294,6 +1305,7 @@ struct acpi_table_tpm2 {
>>   #define ACPI_TPM2_MEMORY_MAPPED                     6
>>   #define ACPI_TPM2_COMMAND_BUFFER                    7
>>   #define ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD  8
>> +#define ACPI_TPM2_COMMAND_BUFFER_WITH_SMC          11
>>
>>   /*******************************************************************************
>>    *
>> --
>> Jiandi An
>> 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.
>>


-- 
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.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

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

[Jarkko Sakkinen]

On Sat, Mar 11, 2017 at 03:18:03PM -0600, Jiandi An wrote:
> On 03/11/17 02:19, Jarkko Sakkinen wrote:
> > On Fri, Mar 10, 2017 at 03:58:07AM -0600, 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>
> > 
> > Could you briefly describe what SMC is? I don't know
> > 
> > 1. What the abbrevation stands for.
> SMC stands for Secure Monitor Call.
> 
> > 2. What it is.
> In ARM, TrustZone security extensions ennable a Secure software
> environment with Secure Monitor mode.  A Secure Monitor Call (SMC) is
> used to enter the Secure Monitor mode and perform a Secure Monitor
> service call.  Software executing in the non-secure state and in the
> secure state at exception levels lower than EL3 in ARM will request
> runtime services using the Secure Monitor Call (SMC) instruction to
> enter EL3 secure mode.
> 
> I'll include a brief description in commit message of patch as well
> in next version of the patch.
> 
> Thanks.
> - Jiandi

Thank you. I got the idea but it still would be also good idea to
have this documented in the commit message so it's available in the
commit log if we need to backtrack something.

/Jarkko

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

[PATCH 2/3] tpm: Add start method for ARM Secure Monitor Call

[Jiandi An]

Add a TPM Command Response Buffer start method that invokes
a secure Monitor Call to request the firmware to execute or
cancel a TPM 2.0 command for ARM64.

Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/char/tpm/tpm.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 4937b56..4341594 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -35,6 +35,9 @@
 #include <linux/cdev.h>
 #include <linux/highmem.h>
 #include <crypto/hash_info.h>
+#ifdef CONFIG_ARM64
+#include <linux/arm-smccc.h>
+#endif
 
 enum tpm_const {
 	TPM_MINOR = 224,	/* officially assigned */
@@ -484,6 +487,32 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
 	tpm_buf_append(buf, (u8 *) &value2, 4);
 }
 
+#ifdef CONFIG_ARM64
+/*
+ * This is a TPM Command Response Buffer start method that invokes a
+ * Secure Monitor Call to requrest the firmware to execute or cancel
+ * a TPM 2.0 command.
+ */
+static inline int tpm_crb_smc_start(unsigned long func_id)
+{
+	struct arm_smccc_res res;
+
+	arm_smccc_smc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
+	if (res.a0 != 0) {
+		WARN(1, "tpm_crb_smc_start() returns res.a0 = 0x%lx\n", res.a0);
+		return -EIO;
+	}
+
+	return 0;
+}
+#else
+static inline int tpm_crb_smc_start(unsigned long func_id)
+{
+	WARN(1, "tpm_crb: incorrect start method\n");
+	return -EINVAL;
+}
+#endif
+
 extern struct class *tpm_class;
 extern dev_t tpm_devt;
 extern const struct file_operations tpm_fops;
-- 
Jiandi An
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.


------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 2/3] tpm: Add start method for ARM Secure Monitor Call

[Jason Gunthorpe]

On Fri, Mar 10, 2017 at 03:58:08AM -0600, Jiandi An wrote:

> +/*
> + * This is a TPM Command Response Buffer start method that invokes a
> + * Secure Monitor Call to requrest the firmware to execute or cancel
> + * a TPM 2.0 command.
> + */
> +static inline int tpm_crb_smc_start(unsigned long func_id)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> +	if (res.a0 != 0) {
> +		WARN(1, "tpm_crb_smc_start() returns res.a0 = 0x%lx\n", res.a0);
> +		return -EIO;

I don't think either of these WARN's are appropriate.

'dev_err(FIRMWARE_BUG'  would be better.

Jason

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 2/3] tpm: Add start method for ARM Secure Monitor Call

[Jiandi An]

On 03/10/17 11:00, Jason Gunthorpe wrote:
> On Fri, Mar 10, 2017 at 03:58:08AM -0600, Jiandi An wrote:
>
>> +/*
>> + * This is a TPM Command Response Buffer start method that invokes a
>> + * Secure Monitor Call to requrest the firmware to execute or cancel
>> + * a TPM 2.0 command.
>> + */
>> +static inline int tpm_crb_smc_start(unsigned long func_id)
>> +{
>> +	struct arm_smccc_res res;
>> +
>> +	arm_smccc_smc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
>> +	if (res.a0 != 0) {
>> +		WARN(1, "tpm_crb_smc_start() returns res.a0 = 0x%lx\n",
> res.a0);
>> +		return -EIO;
>
> I don't think either of these WARN's are appropriate.
>
> 'dev_err(FIRMWARE_BUG'  would be better.
>
> Jason

I will fix this in next version.  Waiting to see if there are
more comments from others.
Thanks
- Jiandi

>
> --------------------------------------------------------------------------
> ----
> Announcing the Oxford Dictionaries API! The API offers world-renowned
> dictionary content that is easy and intuitive to access. Sign up for an
> account today to start using our lexical data to power your apps and
> projects. Get started today and enter our developer competition.
> http://sdm.link/oxford
> _______________________________________________
> 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.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 2/3] tpm: Add start method for ARM Secure Monitor Call

[Jarkko Sakkinen]

On Fri, Mar 10, 2017 at 03:58:08AM -0600, Jiandi An wrote:
> Add a TPM Command Response Buffer start method that invokes
> a secure Monitor Call to request the firmware to execute or
> cancel a TPM 2.0 command for ARM64.
> 
> Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> ---

These are placed incorrectly. They should be in tpm_crb.c

/Jarkko

>  drivers/char/tpm/tpm.h | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 4937b56..4341594 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -35,6 +35,9 @@
>  #include <linux/cdev.h>
>  #include <linux/highmem.h>
>  #include <crypto/hash_info.h>
> +#ifdef CONFIG_ARM64
> +#include <linux/arm-smccc.h>
> +#endif
>  
>  enum tpm_const {
>  	TPM_MINOR = 224,	/* officially assigned */
> @@ -484,6 +487,32 @@ static inline void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value)
>  	tpm_buf_append(buf, (u8 *) &value2, 4);
>  }
>  
> +#ifdef CONFIG_ARM64
> +/*
> + * This is a TPM Command Response Buffer start method that invokes a
> + * Secure Monitor Call to requrest the firmware to execute or cancel
> + * a TPM 2.0 command.
> + */
> +static inline int tpm_crb_smc_start(unsigned long func_id)
> +{
> +	struct arm_smccc_res res;
> +
> +	arm_smccc_smc(func_id, 0, 0, 0, 0, 0, 0, 0, &res);
> +	if (res.a0 != 0) {
> +		WARN(1, "tpm_crb_smc_start() returns res.a0 = 0x%lx\n", res.a0);
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}
> +#else
> +static inline int tpm_crb_smc_start(unsigned long func_id)
> +{
> +	WARN(1, "tpm_crb: incorrect start method\n");
> +	return -EINVAL;
> +}
> +#endif
> +
>  extern struct class *tpm_class;
>  extern dev_t tpm_devt;
>  extern const struct file_operations tpm_fops;
> -- 
> Jiandi An
> 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.
> 

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

[PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jiandi An]

This enables TPM Command Response Buffer interface driver for
ARM64 and implements an ARM specific TPM CRB start method that
invokes a Secure Monitor Call to request the Firmware to execute
or cancel a TPM 2.0 command.

Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
---
 drivers/char/tpm/Kconfig   |  2 +-
 drivers/char/tpm/tpm_crb.c | 24 ++++++++++++++++++++++--
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
index d520ac5..9659f40 100644
--- a/drivers/char/tpm/Kconfig
+++ b/drivers/char/tpm/Kconfig
@@ -136,7 +136,7 @@ config TCG_XEN
 
 config TCG_CRB
 	tristate "TPM 2.0 CRB Interface"
-	depends on X86 && ACPI
+	depends on (X86 || ARM64) && ACPI
 	---help---
 	  If you have a TPM security chip that is compliant with the
 	  TCG CRB 2.0 TPM specification say Yes and it will be accessible
diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
index 089fcf8..d29a84a 100644
--- a/drivers/char/tpm/tpm_crb.c
+++ b/drivers/char/tpm/tpm_crb.c
@@ -73,6 +73,7 @@ enum crb_status {
 enum crb_flags {
 	CRB_FL_ACPI_START	= BIT(0),
 	CRB_FL_CRB_START	= BIT(1),
+	CRB_FL_CRB_SMC_START	= BIT(2),
 };
 
 struct crb_priv {
@@ -82,6 +83,7 @@ struct crb_priv {
 	u8 __iomem *cmd;
 	u8 __iomem *rsp;
 	u32 cmd_size;
+	u32 smc_func_id;
 };
 
 /**
@@ -101,7 +103,8 @@ struct crb_priv {
  */
 static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
 {
-	if (priv->flags & CRB_FL_ACPI_START)
+	if ((priv->flags & CRB_FL_ACPI_START) ||
+	    (priv->flags & CRB_FL_CRB_SMC_START))
 		return 0;
 
 	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
@@ -129,7 +132,8 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
 {
 	ktime_t stop, start;
 
-	if (priv->flags & CRB_FL_ACPI_START)
+	if ((priv->flags & CRB_FL_ACPI_START) ||
+	    (priv->flags & CRB_FL_CRB_SMC_START))
 		return 0;
 
 	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
@@ -229,6 +233,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
 	if (priv->flags & CRB_FL_ACPI_START)
 		rc = crb_do_acpi_start(chip);
 
+	if (priv->flags & CRB_FL_CRB_SMC_START) {
+		iowrite32(CRB_START_INVOKE, &priv->cca->start);
+		rc = tpm_crb_smc_start(priv->smc_func_id);
+	}
+
 	return rc;
 }
 
@@ -445,6 +454,17 @@ static int crb_acpi_add(struct acpi_device *device)
 	    sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
 		priv->flags |= CRB_FL_ACPI_START;
 
+	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_SMC) {
+		if ((buf->header.length - default_len) !=
+		    sizeof(struct tpm2_crb_smc)) {
+			dev_err(dev, "TPM2 ACPI table has wrong size %u for start method type %d\n",
+				buf->header.length, ACPI_TPM2_COMMAND_BUFFER_WITH_SMC);
+			return -EINVAL;
+		}
+		priv->flags |= CRB_FL_CRB_SMC_START;
+		priv->smc_func_id = buf->platform_data.smc_params.smc_func_id;
+	}
+
 	rc = crb_map_io(device, priv, buf);
 	if (rc)
 		return rc;
-- 
Jiandi An
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.


------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jason Gunthorpe]

On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:

> +	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_SMC) {
> +		if ((buf->header.length - default_len) !=
> +		    sizeof(struct tpm2_crb_smc)) {
> +			dev_err(dev, "TPM2 ACPI table has wrong size %u for start method type %d\n",
> +				buf->header.length,

This should be:

	dev_err(dev, FW_BUG "TPM2 ACPI table has wrong size %u for start method type %d\n",

Jason

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jiandi An]

On 03/10/17 11:01, Jason Gunthorpe wrote:
> On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:
>
>> +	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_SMC) {
>> +		if ((buf->header.length - default_len) !=
>> +		    sizeof(struct tpm2_crb_smc)) {
>> +			dev_err(dev, "TPM2 ACPI table has wrong size %u
> for start method type %d\n",
>> +				buf->header.length,
>
> This should be:
>
> 	dev_err(dev, FW_BUG "TPM2 ACPI table has wrong size %u for start
> method type %d\n",
>
> Jason

I will fix this in next version.  Waiting to see if there are
more comments from others.
Thanks
- Jiandi

>
> --------------------------------------------------------------------------
> ----
> Announcing the Oxford Dictionaries API! The API offers world-renowned
> dictionary content that is easy and intuitive to access. Sign up for an
> account today to start using our lexical data to power your apps and
> projects. Get started today and enter our developer competition.
> http://sdm.link/oxford
> _______________________________________________
> 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.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jason Gunthorpe]

On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:

>  	tristate "TPM 2.0 CRB Interface"
> -	depends on X86 && ACPI
> +	depends on (X86 || ARM64) && ACPI

Oh, and why is the X86 even here?

Surely it should just be 'depends on ACPI'? I don't remember anything
x86 specific in CRB?

Jason

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jiandi An]

On 03/10/17 11:02, Jason Gunthorpe wrote:
> On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:
>
>>   	tristate "TPM 2.0 CRB Interface"
>> -	depends on X86 && ACPI
>> +	depends on (X86 || ARM64) && ACPI
>
> Oh, and why is the X86 even here?
>
> Surely it should just be 'depends on ACPI'? I don't remember anything
> x86 specific in CRB?
>
> Jason
>

During testing of my patches on ARM64, I hit crb_cmd_ready() and
crb_go_idle() that fails.  Looking at the comment and commit history,
it was added for fixing Intel PTT hw bug in Skylake and Broxton.
The functions themselves do nothing if flag is CRB_FL_ACPI_START.
I added CRB_FL_CRB_SMC_START which is the new ARM64 start method for
those two functions to also do nothing.  So crb_cmd_ready() and
crb_go_idle() are only for when CRB_FL_ACPI_START.

And in crb_acpi_add(), there is also comments on what drives
CRB_FL_CRB_START to be set.  ACPI_TPM2_COMMAND_BUFFER or
ACPI_TPM2_MEMORY_MAPPED

/* The reason for the extra quirk is that the PTT in 4th Gen Core CPUs
  * report only ACPI start but in practice seems to require both
  * ACPI start and CRB start.
  */

I didn't have too much background on the CRB driver for X86.  I
thought to be safe just or in ARM64.  But if you see no issues,
I'll remove (X86 || ARM64) and just make it depend on ACPI.

Thanks.
- Jiandi

-- 
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.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jarkko Sakkinen]

On Fri, Mar 10, 2017 at 10:02:16AM -0700, Jason Gunthorpe wrote:
> On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:
> 
> >  	tristate "TPM 2.0 CRB Interface"
> > -	depends on X86 && ACPI
> > +	depends on (X86 || ARM64) && ACPI
> 
> Oh, and why is the X86 even here?
> 
> Surely it should just be 'depends on ACPI'? I don't remember anything
> x86 specific in CRB?
> 
> Jason

Not sure, maybe I though originally (back in 2014) the byte order
but if all ACPI platforms are little endian it is not needed.

/Jarkko

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jarkko Sakkinen]

On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:
> This enables TPM Command Response Buffer interface driver for
> ARM64 and implements an ARM specific TPM CRB start method that
> invokes a Secure Monitor Call to request the Firmware to execute
> or cancel a TPM 2.0 command.
> 
> Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>

Does look good to me. I might consider to squash this with the
two inline functions.

/Jarkko

> ---
>  drivers/char/tpm/Kconfig   |  2 +-
>  drivers/char/tpm/tpm_crb.c | 24 ++++++++++++++++++++++--
>  2 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
> index d520ac5..9659f40 100644
> --- a/drivers/char/tpm/Kconfig
> +++ b/drivers/char/tpm/Kconfig
> @@ -136,7 +136,7 @@ config TCG_XEN
>  
>  config TCG_CRB
>  	tristate "TPM 2.0 CRB Interface"
> -	depends on X86 && ACPI
> +	depends on (X86 || ARM64) && ACPI
>  	---help---
>  	  If you have a TPM security chip that is compliant with the
>  	  TCG CRB 2.0 TPM specification say Yes and it will be accessible
> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
> index 089fcf8..d29a84a 100644
> --- a/drivers/char/tpm/tpm_crb.c
> +++ b/drivers/char/tpm/tpm_crb.c
> @@ -73,6 +73,7 @@ enum crb_status {
>  enum crb_flags {
>  	CRB_FL_ACPI_START	= BIT(0),
>  	CRB_FL_CRB_START	= BIT(1),
> +	CRB_FL_CRB_SMC_START	= BIT(2),
>  };
>  
>  struct crb_priv {
> @@ -82,6 +83,7 @@ struct crb_priv {
>  	u8 __iomem *cmd;
>  	u8 __iomem *rsp;
>  	u32 cmd_size;
> +	u32 smc_func_id;
>  };
>  
>  /**
> @@ -101,7 +103,8 @@ struct crb_priv {
>   */
>  static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
>  {
> -	if (priv->flags & CRB_FL_ACPI_START)
> +	if ((priv->flags & CRB_FL_ACPI_START) ||
> +	    (priv->flags & CRB_FL_CRB_SMC_START))
>  		return 0;
>  
>  	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
> @@ -129,7 +132,8 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
>  {
>  	ktime_t stop, start;
>  
> -	if (priv->flags & CRB_FL_ACPI_START)
> +	if ((priv->flags & CRB_FL_ACPI_START) ||
> +	    (priv->flags & CRB_FL_CRB_SMC_START))
>  		return 0;
>  
>  	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
> @@ -229,6 +233,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
>  	if (priv->flags & CRB_FL_ACPI_START)
>  		rc = crb_do_acpi_start(chip);
>  
> +	if (priv->flags & CRB_FL_CRB_SMC_START) {
> +		iowrite32(CRB_START_INVOKE, &priv->cca->start);
> +		rc = tpm_crb_smc_start(priv->smc_func_id);
> +	}
> +
>  	return rc;
>  }
>  
> @@ -445,6 +454,17 @@ static int crb_acpi_add(struct acpi_device *device)
>  	    sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
>  		priv->flags |= CRB_FL_ACPI_START;
>  
> +	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_SMC) {
> +		if ((buf->header.length - default_len) !=
> +		    sizeof(struct tpm2_crb_smc)) {
> +			dev_err(dev, "TPM2 ACPI table has wrong size %u for start method type %d\n",
> +				buf->header.length, ACPI_TPM2_COMMAND_BUFFER_WITH_SMC);
> +			return -EINVAL;
> +		}
> +		priv->flags |= CRB_FL_CRB_SMC_START;
> +		priv->smc_func_id = buf->platform_data.smc_params.smc_func_id;
> +	}
> +
>  	rc = crb_map_io(device, priv, buf);
>  	if (rc)
>  		return rc;
> -- 
> Jiandi An
> 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.
> 

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jiandi An]

On 03/11/17 02:42, Jarkko Sakkinen wrote:
> On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:
>> This enables TPM Command Response Buffer interface driver for
>> ARM64 and implements an ARM specific TPM CRB start method that
>> invokes a Secure Monitor Call to request the Firmware to execute
>> or cancel a TPM 2.0 command.
>>
>> Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
>
> Does look good to me. I might consider to squash this with the
> two inline functions.
>
> /Jarkko

I'll squash the SMC inline function patch with this in next version
to address this.

Thanks.
- Jiandi

>
>> ---
>>   drivers/char/tpm/Kconfig   |  2 +-
>>   drivers/char/tpm/tpm_crb.c | 24 ++++++++++++++++++++++--
>>   2 files changed, 23 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig
>> index d520ac5..9659f40 100644
>> --- a/drivers/char/tpm/Kconfig
>> +++ b/drivers/char/tpm/Kconfig
>> @@ -136,7 +136,7 @@ config TCG_XEN
>>
>>   config TCG_CRB
>>   	tristate "TPM 2.0 CRB Interface"
>> -	depends on X86 && ACPI
>> +	depends on (X86 || ARM64) && ACPI
>>   	---help---
>>   	  If you have a TPM security chip that is compliant with the
>>   	  TCG CRB 2.0 TPM specification say Yes and it will be accessible
>> diff --git a/drivers/char/tpm/tpm_crb.c b/drivers/char/tpm/tpm_crb.c
>> index 089fcf8..d29a84a 100644
>> --- a/drivers/char/tpm/tpm_crb.c
>> +++ b/drivers/char/tpm/tpm_crb.c
>> @@ -73,6 +73,7 @@ enum crb_status {
>>   enum crb_flags {
>>   	CRB_FL_ACPI_START	= BIT(0),
>>   	CRB_FL_CRB_START	= BIT(1),
>> +	CRB_FL_CRB_SMC_START	= BIT(2),
>>   };
>>
>>   struct crb_priv {
>> @@ -82,6 +83,7 @@ struct crb_priv {
>>   	u8 __iomem *cmd;
>>   	u8 __iomem *rsp;
>>   	u32 cmd_size;
>> +	u32 smc_func_id;
>>   };
>>
>>   /**
>> @@ -101,7 +103,8 @@ struct crb_priv {
>>    */
>>   static int __maybe_unused crb_go_idle(struct device *dev, struct crb_priv *priv)
>>   {
>> -	if (priv->flags & CRB_FL_ACPI_START)
>> +	if ((priv->flags & CRB_FL_ACPI_START) ||
>> +	    (priv->flags & CRB_FL_CRB_SMC_START))
>>   		return 0;
>>
>>   	iowrite32(CRB_CTRL_REQ_GO_IDLE, &priv->cca->req);
>> @@ -129,7 +132,8 @@ static int __maybe_unused crb_cmd_ready(struct device *dev,
>>   {
>>   	ktime_t stop, start;
>>
>> -	if (priv->flags & CRB_FL_ACPI_START)
>> +	if ((priv->flags & CRB_FL_ACPI_START) ||
>> +	    (priv->flags & CRB_FL_CRB_SMC_START))
>>   		return 0;
>>
>>   	iowrite32(CRB_CTRL_REQ_CMD_READY, &priv->cca->req);
>> @@ -229,6 +233,11 @@ static int crb_send(struct tpm_chip *chip, u8 *buf, size_t len)
>>   	if (priv->flags & CRB_FL_ACPI_START)
>>   		rc = crb_do_acpi_start(chip);
>>
>> +	if (priv->flags & CRB_FL_CRB_SMC_START) {
>> +		iowrite32(CRB_START_INVOKE, &priv->cca->start);
>> +		rc = tpm_crb_smc_start(priv->smc_func_id);
>> +	}
>> +
>>   	return rc;
>>   }
>>
>> @@ -445,6 +454,17 @@ static int crb_acpi_add(struct acpi_device *device)
>>   	    sm == ACPI_TPM2_COMMAND_BUFFER_WITH_START_METHOD)
>>   		priv->flags |= CRB_FL_ACPI_START;
>>
>> +	if (sm == ACPI_TPM2_COMMAND_BUFFER_WITH_SMC) {
>> +		if ((buf->header.length - default_len) !=
>> +		    sizeof(struct tpm2_crb_smc)) {
>> +			dev_err(dev, "TPM2 ACPI table has wrong size %u for start method type %d\n",
>> +				buf->header.length, ACPI_TPM2_COMMAND_BUFFER_WITH_SMC);
>> +			return -EINVAL;
>> +		}
>> +		priv->flags |= CRB_FL_CRB_SMC_START;
>> +		priv->smc_func_id = buf->platform_data.smc_params.smc_func_id;
>> +	}
>> +
>>   	rc = crb_map_io(device, priv, buf);
>>   	if (rc)
>>   		return rc;
>> --
>> Jiandi An
>> 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.
>>


-- 
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.

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford

Re: [PATCH 3/3] tpm/tpm_crb: Enable TPM CRB interface for ARM64

[Jarkko Sakkinen]

On Sat, Mar 11, 2017 at 02:40:30PM -0600, Jiandi An wrote:
> On 03/11/17 02:42, Jarkko Sakkinen wrote:
> > On Fri, Mar 10, 2017 at 03:58:09AM -0600, Jiandi An wrote:
> > > This enables TPM Command Response Buffer interface driver for
> > > ARM64 and implements an ARM specific TPM CRB start method that
> > > invokes a Secure Monitor Call to request the Firmware to execute
> > > or cancel a TPM 2.0 command.
> > > 
> > > Signed-off-by: Jiandi An <anjiandi-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> > 
> > Does look good to me. I might consider to squash this with the
> > two inline functions.
> > 
> > /Jarkko
> 
> I'll squash the SMC inline function patch with this in next version
> to address this.
> 
> Thanks.
> - Jiandi

Thanks!

/Jarkko

------------------------------------------------------------------------------
Announcing the Oxford Dictionaries API! The API offers world-renowned
dictionary content that is easy and intuitive to access. Sign up for an
account today to start using our lexical data to power your apps and
projects. Get started today and enter our developer competition.
http://sdm.link/oxford