diff mbox series

[v2] tpm_ffa_crb: access tpm service over FF-A direct message request v2

Message ID 20250411090856.1417021-1-yeoreum.yun@arm.com (mailing list archive)
State New
Headers show
Series [v2] tpm_ffa_crb: access tpm service over FF-A direct message request v2 | expand

Commit Message

Yeoreum Yun April 11, 2025, 9:08 a.m. UTC
For secure partition with multi service, tpm_ffa_crb can access tpm
service with direct message request v2 interface according to chapter 3.3,
TPM Service Command Response Buffer Interface Over FF-A specification [0].

This patch reflects this spec to access tpm service over
FF-A direct message request v2 ABI.

Link: https://developer.arm.com/documentation/den0138/latest/ [0]
Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
---
Since v1:
    - Fix indentation.
    - https://lore.kernel.org/all/20250410110701.1244965-1-yeoreum.yun@arm.com/
---
 drivers/char/tpm/tpm_crb_ffa.c | 55 ++++++++++++++++++++++++----------
 1 file changed, 40 insertions(+), 15 deletions(-)

--
LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}

Comments

Sudeep Holla April 11, 2025, 9:17 a.m. UTC | #1
On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> For secure partition with multi service, tpm_ffa_crb can access tpm
> service with direct message request v2 interface according to chapter 3.3,
> TPM Service Command Response Buffer Interface Over FF-A specification [0].
> 
> This patch reflects this spec to access tpm service over
> FF-A direct message request v2 ABI.
> 

From FF-A interface usage perspective,

Acked-by: Sudeep Holla <sudeep.holla@arm.com>
Jarkko Sakkinen April 11, 2025, 10:37 a.m. UTC | #2
On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> For secure partition with multi service, tpm_ffa_crb can access tpm
> service with direct message request v2 interface according to chapter 3.3,
> TPM Service Command Response Buffer Interface Over FF-A specification [0].
> 
> This patch reflects this spec to access tpm service over
> FF-A direct message request v2 ABI.
> 
> Link: https://developer.arm.com/documentation/den0138/latest/ [0]

Sorry, did not notice in the first round:

1. Does not have "[0]" postfix.
2. Only for lore links:
   https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org 

> Signed-off-by: Yeoreum Yun <yeoreum.yun@arm.com>
> ---
> Since v1:
>     - Fix indentation.
>     - https://lore.kernel.org/all/20250410110701.1244965-1-yeoreum.yun@arm.com/
> ---
>  drivers/char/tpm/tpm_crb_ffa.c | 55 ++++++++++++++++++++++++----------
>  1 file changed, 40 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
> index 3169a87a56b6..fed775cf53ab 100644
> --- a/drivers/char/tpm/tpm_crb_ffa.c
> +++ b/drivers/char/tpm/tpm_crb_ffa.c
> @@ -105,7 +105,10 @@ struct tpm_crb_ffa {
>  	u16 minor_version;
>  	/* lock to protect sending of FF-A messages: */
>  	struct mutex msg_data_lock;
> -	struct ffa_send_direct_data direct_msg_data;
> +	union {
> +		struct ffa_send_direct_data direct_msg_data;
> +		struct ffa_send_direct_data2 direct_msg_data2;
> +	};
>  };
> 
>  static struct tpm_crb_ffa *tpm_crb_ffa;
> @@ -185,18 +188,34 @@ static int __tpm_crb_ffa_send_recieve(unsigned long func_id,
> 
>  	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;
> 
> -	memset(&tpm_crb_ffa->direct_msg_data, 0x00,
> -	       sizeof(struct ffa_send_direct_data));
> -
> -	tpm_crb_ffa->direct_msg_data.data1 = func_id;
> -	tpm_crb_ffa->direct_msg_data.data2 = a0;
> -	tpm_crb_ffa->direct_msg_data.data3 = a1;
> -	tpm_crb_ffa->direct_msg_data.data4 = a2;
> +	if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
> +		memset(&tpm_crb_ffa->direct_msg_data2, 0x00,
> +		       sizeof(struct ffa_send_direct_data2));
> +
> +		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
> +		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
> +		tpm_crb_ffa->direct_msg_data2.data[2] = a1;
> +		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
> +
> +		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
> +				&tpm_crb_ffa->direct_msg_data2);
> +		if (!ret)
> +			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data2.data[0]);
> +	} else {
> +		memset(&tpm_crb_ffa->direct_msg_data, 0x00,
> +		       sizeof(struct ffa_send_direct_data));
> +
> +		tpm_crb_ffa->direct_msg_data.data1 = func_id;
> +		tpm_crb_ffa->direct_msg_data.data2 = a0;
> +		tpm_crb_ffa->direct_msg_data.data3 = a1;
> +		tpm_crb_ffa->direct_msg_data.data4 = a2;
> +
> +		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
> +				&tpm_crb_ffa->direct_msg_data);
> +		if (!ret)
> +			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
> +	}
> 
> -	ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
> -			&tpm_crb_ffa->direct_msg_data);
> -	if (!ret)
> -		ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
> 
>  	return ret;
>  }
> @@ -231,8 +250,13 @@ int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor)
> 
>  	rc = __tpm_crb_ffa_send_recieve(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
>  	if (!rc) {
> -		*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
> -		*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
> +		if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
> +			*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data2.data[1]);
> +			*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa->direct_msg_data2.data[1]);
> +		} else {
> +			*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
> +			*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
> +		}
>  	}
> 
>  	return rc;
> @@ -277,7 +301,8 @@ static int tpm_crb_ffa_probe(struct ffa_device *ffa_dev)
> 
>  	tpm_crb_ffa = ERR_PTR(-ENODEV); // set tpm_crb_ffa so we can detect probe failure
> 
> -	if (!ffa_partition_supports_direct_recv(ffa_dev)) {
> +	if (!ffa_partition_supports_direct_recv(ffa_dev) &&
> +	    !ffa_partition_supports_direct_req2_recv(ffa_dev)) {
>  		pr_err("TPM partition doesn't support direct message receive.\n");
>  		return -EINVAL;
>  	}
> --
> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
> 

BR, Jarkko
Sudeep Holla April 11, 2025, 10:43 a.m. UTC | #3
On Fri, Apr 11, 2025 at 01:37:31PM +0300, Jarkko Sakkinen wrote:
> On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> > For secure partition with multi service, tpm_ffa_crb can access tpm
> > service with direct message request v2 interface according to chapter 3.3,
> > TPM Service Command Response Buffer Interface Over FF-A specification [0].
> > 
> > This patch reflects this spec to access tpm service over
> > FF-A direct message request v2 ABI.
> > 
> > Link: https://developer.arm.com/documentation/den0138/latest/ [0]
> 
> Sorry, did not notice in the first round:
> 
> 1. Does not have "[0]" postfix.
> 2. Only for lore links:
>    https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org 
> 

I was about to comment on the presence of link itself but left it to
the maintainer. It was part of the first commit log from Stuart. If it
is so important that it requires mention in each commit, it better me
made part of the file itself to avoid having to mention again and again.
Just my opinion, I leave it to the maintainers.
Stefano Garzarella April 11, 2025, 11:04 a.m. UTC | #4
On Fri, Apr 11, 2025 at 11:43:24AM +0100, Sudeep Holla wrote:
>On Fri, Apr 11, 2025 at 01:37:31PM +0300, Jarkko Sakkinen wrote:
>> On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
>> > For secure partition with multi service, tpm_ffa_crb can access tpm
>> > service with direct message request v2 interface according to chapter 3.3,
>> > TPM Service Command Response Buffer Interface Over FF-A specification [0].
>> >
>> > This patch reflects this spec to access tpm service over
>> > FF-A direct message request v2 ABI.
>> >
>> > Link: https://developer.arm.com/documentation/den0138/latest/ [0]
>>
>> Sorry, did not notice in the first round:
>>
>> 1. Does not have "[0]" postfix.
>> 2. Only for lore links:
>>    https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org
>>
>
>I was about to comment on the presence of link itself but left it to
>the maintainer. It was part of the first commit log from Stuart. If it
>is so important that it requires mention in each commit, it better me
>made part of the file itself to avoid having to mention again and again.
>Just my opinion, I leave it to the maintainers.

I agree on this.
Also, are these links assured to be stable? Could we just mention the 
title and version?

e.g. "TPM Service Command Response Buffer Interface Over FF-A"
      Document version: v1.0 BET

Thanks,
Stefano
Sudeep Holla April 11, 2025, 11:09 a.m. UTC | #5
On Fri, Apr 11, 2025 at 01:04:32PM +0200, Stefano Garzarella wrote:
> On Fri, Apr 11, 2025 at 11:43:24AM +0100, Sudeep Holla wrote:
> > On Fri, Apr 11, 2025 at 01:37:31PM +0300, Jarkko Sakkinen wrote:
> > > On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> > > > For secure partition with multi service, tpm_ffa_crb can access tpm
> > > > service with direct message request v2 interface according to chapter 3.3,
> > > > TPM Service Command Response Buffer Interface Over FF-A specification [0].
> > > >
> > > > This patch reflects this spec to access tpm service over
> > > > FF-A direct message request v2 ABI.
> > > >
> > > > Link: https://developer.arm.com/documentation/den0138/latest/ [0]
> > > 
> > > Sorry, did not notice in the first round:
> > > 
> > > 1. Does not have "[0]" postfix.
> > > 2. Only for lore links:
> > >    https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org
> > > 
> > 
> > I was about to comment on the presence of link itself but left it to
> > the maintainer. It was part of the first commit log from Stuart. If it
> > is so important that it requires mention in each commit, it better me
> > made part of the file itself to avoid having to mention again and again.
> > Just my opinion, I leave it to the maintainers.
> 
> I agree on this.
> Also, are these links assured to be stable?

The latest version as used above should be stable(we got some assurance
on that by the team maintaining those)

> Could we just mention the title and version?
> 
> e.g. "TPM Service Command Response Buffer Interface Over FF-A"
>      Document version: v1.0 BET
> 

Yes that must suffice especially if you are referring to a specific version
of the spec as latest will always point to the latest version.
Jarkko Sakkinen April 12, 2025, 12:39 a.m. UTC | #6
On Fri, Apr 11, 2025 at 11:43:24AM +0100, Sudeep Holla wrote:
> On Fri, Apr 11, 2025 at 01:37:31PM +0300, Jarkko Sakkinen wrote:
> > On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> > > For secure partition with multi service, tpm_ffa_crb can access tpm
> > > service with direct message request v2 interface according to chapter 3.3,
> > > TPM Service Command Response Buffer Interface Over FF-A specification [0].
> > > 
> > > This patch reflects this spec to access tpm service over
> > > FF-A direct message request v2 ABI.
> > > 
> > > Link: https://developer.arm.com/documentation/den0138/latest/ [0]
> > 
> > Sorry, did not notice in the first round:
> > 
> > 1. Does not have "[0]" postfix.
> > 2. Only for lore links:
> >    https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org 
> > 
> 
> I was about to comment on the presence of link itself but left it to
> the maintainer. It was part of the first commit log from Stuart. If it
> is so important that it requires mention in each commit, it better me
> made part of the file itself to avoid having to mention again and again.
> Just my opinion, I leave it to the maintainers.

Sure, this does make sense to me.

> 
> -- 
> Regards,
> Sudeep

BR, Jarkko
Jarkko Sakkinen April 12, 2025, 12:41 a.m. UTC | #7
On Fri, Apr 11, 2025 at 01:04:32PM +0200, Stefano Garzarella wrote:
> On Fri, Apr 11, 2025 at 11:43:24AM +0100, Sudeep Holla wrote:
> > On Fri, Apr 11, 2025 at 01:37:31PM +0300, Jarkko Sakkinen wrote:
> > > On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> > > > For secure partition with multi service, tpm_ffa_crb can access tpm
> > > > service with direct message request v2 interface according to chapter 3.3,
> > > > TPM Service Command Response Buffer Interface Over FF-A specification [0].
> > > >
> > > > This patch reflects this spec to access tpm service over
> > > > FF-A direct message request v2 ABI.
> > > >
> > > > Link: https://developer.arm.com/documentation/den0138/latest/ [0]
> > > 
> > > Sorry, did not notice in the first round:
> > > 
> > > 1. Does not have "[0]" postfix.
> > > 2. Only for lore links:
> > >    https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org
> > > 
> > 
> > I was about to comment on the presence of link itself but left it to
> > the maintainer. It was part of the first commit log from Stuart. If it
> > is so important that it requires mention in each commit, it better me
> > made part of the file itself to avoid having to mention again and again.
> > Just my opinion, I leave it to the maintainers.
> 
> I agree on this.
> Also, are these links assured to be stable? Could we just mention the title
> and version?

I don't actually care in the end of the day if it is valid forever as
long as it is valid for "reasonable amount of time".

> 
> e.g. "TPM Service Command Response Buffer Interface Over FF-A"
>      Document version: v1.0 BET
> 
> Thanks,
> Stefano
> 

BR, Jarkko
Yeoreum Yun April 12, 2025, 5:41 a.m. UTC | #8
> On Fri, Apr 11, 2025 at 11:43:24AM +0100, Sudeep Holla wrote:
> > On Fri, Apr 11, 2025 at 01:37:31PM +0300, Jarkko Sakkinen wrote:
> > > On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> > > > For secure partition with multi service, tpm_ffa_crb can access tpm
> > > > service with direct message request v2 interface according to chapter 3.3,
> > > > TPM Service Command Response Buffer Interface Over FF-A specification [0].
> > > >
> > > > This patch reflects this spec to access tpm service over
> > > > FF-A direct message request v2 ABI.
> > > >
> > > > Link: https://developer.arm.com/documentation/den0138/latest/ [0]
> > >
> > > Sorry, did not notice in the first round:
> > >
> > > 1. Does not have "[0]" postfix.
> > > 2. Only for lore links:
> > >    https://www.kernel.org/doc/html/v6.12/maintainer/configure-git.html#creating-commit-links-to-lore-kernel-org
> > >
> >
> > I was about to comment on the presence of link itself but left it to
> > the maintainer. It was part of the first commit log from Stuart. If it
> > is so important that it requires mention in each commit, it better me
> > made part of the file itself to avoid having to mention again and again.
> > Just my opinion, I leave it to the maintainers.
>
> Sure, this does make sense to me.

Sorry for late answer.
I think in the file already mention the proper document number,
Here It doesn't need to mention the link but spec name and version only.
I'll send again.

Thanks.

--
Sincerely,
Yeoreum Yun
Yeoreum Yun April 12, 2025, 5:42 a.m. UTC | #9
On Fri, Apr 11, 2025 at 10:17:49AM +0100, Sudeep Holla wrote:
> On Fri, Apr 11, 2025 at 10:08:56AM +0100, Yeoreum Yun wrote:
> > For secure partition with multi service, tpm_ffa_crb can access tpm
> > service with direct message request v2 interface according to chapter 3.3,
> > TPM Service Command Response Buffer Interface Over FF-A specification [0].
> >
> > This patch reflects this spec to access tpm service over
> > FF-A direct message request v2 ABI.
> >
>
> From FF-A interface usage perspective,
>
> Acked-by: Sudeep Holla <sudeep.holla@arm.com>
>
>
> --
> Regards,
> Sudeep

Thanks!

--
Sincerely,
Yeoreum Yun
diff mbox series

Patch

diff --git a/drivers/char/tpm/tpm_crb_ffa.c b/drivers/char/tpm/tpm_crb_ffa.c
index 3169a87a56b6..fed775cf53ab 100644
--- a/drivers/char/tpm/tpm_crb_ffa.c
+++ b/drivers/char/tpm/tpm_crb_ffa.c
@@ -105,7 +105,10 @@  struct tpm_crb_ffa {
 	u16 minor_version;
 	/* lock to protect sending of FF-A messages: */
 	struct mutex msg_data_lock;
-	struct ffa_send_direct_data direct_msg_data;
+	union {
+		struct ffa_send_direct_data direct_msg_data;
+		struct ffa_send_direct_data2 direct_msg_data2;
+	};
 };

 static struct tpm_crb_ffa *tpm_crb_ffa;
@@ -185,18 +188,34 @@  static int __tpm_crb_ffa_send_recieve(unsigned long func_id,

 	msg_ops = tpm_crb_ffa->ffa_dev->ops->msg_ops;

-	memset(&tpm_crb_ffa->direct_msg_data, 0x00,
-	       sizeof(struct ffa_send_direct_data));
-
-	tpm_crb_ffa->direct_msg_data.data1 = func_id;
-	tpm_crb_ffa->direct_msg_data.data2 = a0;
-	tpm_crb_ffa->direct_msg_data.data3 = a1;
-	tpm_crb_ffa->direct_msg_data.data4 = a2;
+	if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
+		memset(&tpm_crb_ffa->direct_msg_data2, 0x00,
+		       sizeof(struct ffa_send_direct_data2));
+
+		tpm_crb_ffa->direct_msg_data2.data[0] = func_id;
+		tpm_crb_ffa->direct_msg_data2.data[1] = a0;
+		tpm_crb_ffa->direct_msg_data2.data[2] = a1;
+		tpm_crb_ffa->direct_msg_data2.data[3] = a2;
+
+		ret = msg_ops->sync_send_receive2(tpm_crb_ffa->ffa_dev,
+				&tpm_crb_ffa->direct_msg_data2);
+		if (!ret)
+			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data2.data[0]);
+	} else {
+		memset(&tpm_crb_ffa->direct_msg_data, 0x00,
+		       sizeof(struct ffa_send_direct_data));
+
+		tpm_crb_ffa->direct_msg_data.data1 = func_id;
+		tpm_crb_ffa->direct_msg_data.data2 = a0;
+		tpm_crb_ffa->direct_msg_data.data3 = a1;
+		tpm_crb_ffa->direct_msg_data.data4 = a2;
+
+		ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
+				&tpm_crb_ffa->direct_msg_data);
+		if (!ret)
+			ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);
+	}

-	ret = msg_ops->sync_send_receive(tpm_crb_ffa->ffa_dev,
-			&tpm_crb_ffa->direct_msg_data);
-	if (!ret)
-		ret = tpm_crb_ffa_to_linux_errno(tpm_crb_ffa->direct_msg_data.data1);

 	return ret;
 }
@@ -231,8 +250,13 @@  int tpm_crb_ffa_get_interface_version(u16 *major, u16 *minor)

 	rc = __tpm_crb_ffa_send_recieve(CRB_FFA_GET_INTERFACE_VERSION, 0x00, 0x00, 0x00);
 	if (!rc) {
-		*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
-		*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
+		if (ffa_partition_supports_direct_req2_recv(tpm_crb_ffa->ffa_dev)) {
+			*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data2.data[1]);
+			*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa->direct_msg_data2.data[1]);
+		} else {
+			*major = CRB_FFA_MAJOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
+			*minor = CRB_FFA_MINOR_VERSION(tpm_crb_ffa->direct_msg_data.data2);
+		}
 	}

 	return rc;
@@ -277,7 +301,8 @@  static int tpm_crb_ffa_probe(struct ffa_device *ffa_dev)

 	tpm_crb_ffa = ERR_PTR(-ENODEV); // set tpm_crb_ffa so we can detect probe failure

-	if (!ffa_partition_supports_direct_recv(ffa_dev)) {
+	if (!ffa_partition_supports_direct_recv(ffa_dev) &&
+	    !ffa_partition_supports_direct_req2_recv(ffa_dev)) {
 		pr_err("TPM partition doesn't support direct message receive.\n");
 		return -EINVAL;
 	}