Re: [XEN PATCH v1 3/5] xen/arm: ffa: simplify ffa_handle_mem_share()

[Bertrand Marquis <Bertrand.Marquis@xxxxxxx>]

Hi Jens,

> On 9 Apr 2024, at 17:36, Jens Wiklander <jens.wiklander@xxxxxxxxxx> wrote:
> 
> Simplify ffa_handle_mem_share() by removing the start_page_idx and
> last_page_idx parameters from get_shm_pages() and check that the number
> of pages matches expectations at the end of get_shm_pages().
> 
> Signed-off-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Reviewed-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>

Cheers
Bertrand

> ---
> xen/arch/arm/tee/ffa_shm.c | 18 ++++++------------
> 1 file changed, 6 insertions(+), 12 deletions(-)
> 
> diff --git a/xen/arch/arm/tee/ffa_shm.c b/xen/arch/arm/tee/ffa_shm.c
> index 75a5b66aeb4c..370d83ec5cf8 100644
> --- a/xen/arch/arm/tee/ffa_shm.c
> +++ b/xen/arch/arm/tee/ffa_shm.c
> @@ -159,10 +159,9 @@ static int32_t ffa_mem_reclaim(uint32_t handle_lo, 
> uint32_t handle_hi,
>  */
> static int get_shm_pages(struct domain *d, struct ffa_shm_mem *shm,
>                          const struct ffa_address_range *range,
> -                         uint32_t range_count, unsigned int start_page_idx,
> -                         unsigned int *last_page_idx)
> +                         uint32_t range_count)
> {
> -    unsigned int pg_idx = start_page_idx;
> +    unsigned int pg_idx = 0;
>     gfn_t gfn;
>     unsigned int n;
>     unsigned int m;
> @@ -191,7 +190,9 @@ static int get_shm_pages(struct domain *d, struct 
> ffa_shm_mem *shm,
>         }
>     }
> 
> -    *last_page_idx = pg_idx;
> +    /* The ranges must add up */
> +    if ( pg_idx < shm->page_count )
> +            return FFA_RET_INVALID_PARAMETERS;
> 
>     return FFA_RET_OK;
> }
> @@ -460,7 +461,6 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
>     struct domain *d = current->domain;
>     struct ffa_ctx *ctx = d->arch.tee;
>     struct ffa_shm_mem *shm = NULL;
> -    unsigned int last_page_idx = 0;
>     register_t handle_hi = 0;
>     register_t handle_lo = 0;
>     int ret = FFA_RET_DENIED;
> @@ -570,15 +570,9 @@ void ffa_handle_mem_share(struct cpu_user_regs *regs)
>         goto out;
>     }
> 
> -    ret = get_shm_pages(d, shm, region_descr->address_range_array, 
> range_count,
> -                        0, &last_page_idx);
> +    ret = get_shm_pages(d, shm, region_descr->address_range_array, 
> range_count);
>     if ( ret )
>         goto out;
> -    if ( last_page_idx != shm->page_count )
> -    {
> -        ret = FFA_RET_INVALID_PARAMETERS;
> -        goto out;
> -    }
> 
>     /* Note that share_shm() uses our tx buffer */
>     spin_lock(&ffa_tx_buffer_lock);
> -- 
> 2.34.1
>