diff mbox series

[XEN,v8,08/22] xen/arm: ffa: add support for FFA_ID_GET

Message ID 20230413071424.3273490-9-jens.wiklander@linaro.org (mailing list archive)
State New, archived
Headers show
Series Xen FF-A mediator | expand

Commit Message

Jens Wiklander April 13, 2023, 7:14 a.m. UTC
Adds support for the FF-A function FFA_ID_GET to return the ID of the
calling client.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
---
 xen/arch/arm/tee/ffa.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Henry Wang April 13, 2023, 11:07 a.m. UTC | #1
Hi Jens,

> -----Original Message-----
> Subject: [XEN PATCH v8 08/22] xen/arm: ffa: add support for FFA_ID_GET
> 
> Adds support for the FF-A function FFA_ID_GET to return the ID of the
> calling client.
> 
> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
> ---
>  xen/arch/arm/tee/ffa.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index 90ed71cbfda3..f129879c5b81 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -181,6 +181,12 @@ static bool ffa_get_version(uint32_t *vers)
>      return true;
>  }
> 
> +static uint16_t get_vm_id(const struct domain *d)
> +{
> +    /* +1 since 0 is reserved for the hypervisor in FF-A */
> +    return d->domain_id + 1;

Since here you want 0 to be reserved, I think maybe you can use
the "d->arch.p2m.vmid"? According to the logic in p2m_alloc_vmid(),
the "d->arch.p2m.vmid" is also a per-domain u16 value that starts
from 1.

Kind regards,
Henry
Julien Grall April 13, 2023, 12:36 p.m. UTC | #2
On 13/04/2023 12:07, Henry Wang wrote:
> Hi Jens,
> 
>> -----Original Message-----
>> Subject: [XEN PATCH v8 08/22] xen/arm: ffa: add support for FFA_ID_GET
>>
>> Adds support for the FF-A function FFA_ID_GET to return the ID of the
>> calling client.
>>
>> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
>> ---
>>   xen/arch/arm/tee/ffa.c | 21 +++++++++++++++++++++
>>   1 file changed, 21 insertions(+)
>>
>> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
>> index 90ed71cbfda3..f129879c5b81 100644
>> --- a/xen/arch/arm/tee/ffa.c
>> +++ b/xen/arch/arm/tee/ffa.c
>> @@ -181,6 +181,12 @@ static bool ffa_get_version(uint32_t *vers)
>>       return true;
>>   }
>>
>> +static uint16_t get_vm_id(const struct domain *d)
>> +{
>> +    /* +1 since 0 is reserved for the hypervisor in FF-A */
>> +    return d->domain_id + 1;
> 
> Since here you want 0 to be reserved, I think maybe you can use
> the "d->arch.p2m.vmid"? According to the logic in p2m_alloc_vmid(),
> the "d->arch.p2m.vmid" is also a per-domain u16 value that starts
> from 1.

I would rather not do that for a few reasons:
  1) This is assuming the P2M code is initialized first
  2) We should not rely on the VMID to be fixed. We may need to change 
that if we want to run more VMs than the number of VMIDs (we may even 
need multiple VMIDs per domain...).

Cheers,
Henry Wang April 13, 2023, 12:40 p.m. UTC | #3
Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien@xen.org>
> Subject: Re: [XEN PATCH v8 08/22] xen/arm: ffa: add support for FFA_ID_GET
> >> +static uint16_t get_vm_id(const struct domain *d)
> >> +{
> >> +    /* +1 since 0 is reserved for the hypervisor in FF-A */
> >> +    return d->domain_id + 1;
> >
> > Since here you want 0 to be reserved, I think maybe you can use
> > the "d->arch.p2m.vmid"? According to the logic in p2m_alloc_vmid(),
> > the "d->arch.p2m.vmid" is also a per-domain u16 value that starts
> > from 1.
> 
> I would rather not do that for a few reasons:
>   1) This is assuming the P2M code is initialized first
>   2) We should not rely on the VMID to be fixed. We may need to change
> that if we want to run more VMs than the number of VMIDs (we may even
> need multiple VMIDs per domain...).

Yeah these arguments are reasonable. Forget about my comments then.

Kind regards,
Henry

> 
> Cheers,
> 
> --
> Julien Grall
diff mbox series

Patch

diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
index 90ed71cbfda3..f129879c5b81 100644
--- a/xen/arch/arm/tee/ffa.c
+++ b/xen/arch/arm/tee/ffa.c
@@ -181,6 +181,12 @@  static bool ffa_get_version(uint32_t *vers)
     return true;
 }
 
+static uint16_t get_vm_id(const struct domain *d)
+{
+    /* +1 since 0 is reserved for the hypervisor in FF-A */
+    return d->domain_id + 1;
+}
+
 static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
                      register_t v2, register_t v3, register_t v4, register_t v5,
                      register_t v6, register_t v7)
@@ -195,6 +201,12 @@  static void set_regs(struct cpu_user_regs *regs, register_t v0, register_t v1,
         set_user_reg(regs, 7, v7);
 }
 
+static void set_regs_success(struct cpu_user_regs *regs, uint32_t w2,
+                             uint32_t w3)
+{
+    set_regs(regs, FFA_SUCCESS_32, 0, w2, w3, 0, 0, 0, 0);
+}
+
 static void handle_version(struct cpu_user_regs *regs)
 {
     struct domain *d = current->domain;
@@ -224,6 +236,9 @@  static bool ffa_handle_call(struct cpu_user_regs *regs)
     case FFA_VERSION:
         handle_version(regs);
         return true;
+    case FFA_ID_GET:
+        set_regs_success(regs, get_vm_id(d), 0);
+        return true;
 
     default:
         gprintk(XENLOG_ERR, "ffa: unhandled fid 0x%x\n", fid);
@@ -237,6 +252,12 @@  static int ffa_domain_init(struct domain *d)
 
     if ( !ffa_version )
         return -ENODEV;
+     /*
+      * We can't use that last possible domain ID or get_vm_id() would cause
+      * an overflow.
+      */
+    if ( d->domain_id >= UINT16_MAX)
+        return -ERANGE;
 
     ctx = xzalloc(struct ffa_ctx);
     if ( !ctx )