Re: [XEN PATCH v7 19/20] xen/arm: ffa: add support to reclaim shared memory

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jens,

> On 22 Feb 2023, at 16:33, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Adds support to reclaim memory previously shared with FFA_MEM_SHARE.
> 
> Adds a check that the SP supports the needed FF-A feature
> FFA_MEM_RECLAIM.
> 
> Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>
Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Just one thing: we need to discuss what you do with the feature check because
i would expect to return unsupported if a feature is not supported but instead 
here
we make it mandatory for all features we have to be supported to have FFA 
working.
I am not convince that this was the aim of this and we might need to 
distinguish between
features we do need to do something and features which would just make some
functionalities not available.

I will try to check how other implementations are handling this.

Cheers
Bertrand

> ---
> xen/arch/arm/tee/ffa.c | 53 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 53 insertions(+)
> 
> diff --git a/xen/arch/arm/tee/ffa.c b/xen/arch/arm/tee/ffa.c
> index cdc286caf62c..3557edc455d0 100644
> --- a/xen/arch/arm/tee/ffa.c
> +++ b/xen/arch/arm/tee/ffa.c
> @@ -512,6 +512,12 @@ static int32_t ffa_mem_share(uint32_t tot_len, uint32_t 
> frag_len,
>     }
> }
> 
> +static int32_t ffa_mem_reclaim(uint32_t handle_lo, uint32_t handle_hi,
> +                               uint32_t flags)
> +{
> +    return ffa_simple_call(FFA_MEM_RECLAIM, handle_lo, handle_hi, flags, 0);
> +}
> +
> static int32_t ffa_direct_req_send_vm(uint16_t sp_id, uint16_t vm_id,
>                                       uint8_t msg)
> {
> @@ -1238,6 +1244,43 @@ out_set_ret:
>             set_regs_error(regs, ret);
> }
> 
> +static int handle_mem_reclaim(uint64_t handle, uint32_t flags)
> +{
> +    struct domain *d = current->domain;
> +    struct ffa_ctx *ctx = d->arch.tee;
> +    struct ffa_shm_mem *shm;
> +    register_t handle_hi;
> +    register_t handle_lo;
> +    int ret;
> +
> +    spin_lock(&ctx->lock);
> +    list_for_each_entry(shm, &ctx->shm_list, list)
> +    {
> +        if ( shm->handle == handle )
> +            goto found_it;
> +    }
> +    shm = NULL;
> +    ret = FFA_RET_INVALID_PARAMETERS;
> +    goto out;
> +found_it:
> +
> +    uint64_to_regpair(&handle_hi, &handle_lo, handle);
> +    ret = ffa_mem_reclaim(handle_lo, handle_hi, flags);
> +    if ( ret )
> +    {
> +        shm = NULL;
> +        goto out;
> +    }
> +
> +    list_del(&shm->list);
> +
> +out:
> +    free_ffa_shm_mem(ctx, shm);
> +    spin_unlock(&ctx->lock);
> +
> +    return ret;
> +}
> +
> static bool ffa_handle_call(struct cpu_user_regs *regs)
> {
>     uint32_t fid = get_user_reg(regs, 0);
> @@ -1305,6 +1348,15 @@ static bool ffa_handle_call(struct cpu_user_regs *regs)
> #endif
>         handle_mem_share(regs);
>         return true;
> +    case FFA_MEM_RECLAIM:
> +        e = handle_mem_reclaim(regpair_to_uint64(get_user_reg(regs, 2),
> +                                                 get_user_reg(regs, 1)),
> +                               get_user_reg(regs, 3));
> +        if ( e )
> +            set_regs_error(regs, e);
> +        else
> +            set_regs_success(regs, 0, 0);
> +        return true;
> 
>     default:
>         gprintk(XENLOG_ERR, "ffa: unhandled fid 0x%x\n", fid);
> @@ -1508,6 +1560,7 @@ static bool ffa_probe(void)
> #endif
>          !check_mandatory_feature(FFA_RXTX_UNMAP) ||
>          !check_mandatory_feature(FFA_MEM_SHARE_32) ||
> +         !check_mandatory_feature(FFA_MEM_RECLAIM) ||
>          !check_mandatory_feature(FFA_MSG_SEND_DIRECT_REQ_32) )
>         return false;
> 
> -- 
> 2.34.1
>