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@linaro.org>
---
 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

Hi Jens,

> On 9 Apr 2024, at 17:36, Jens Wiklander <jens.wiklander@linaro.org> 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@linaro.org>

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

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
>