Re: [Xen-devel] [PATCH v2] xen: arm: process XENMEM_add_to_physmap_range forwards not backwards.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.12.13 at 14:39, Ian Campbell <ian.campbell@xxxxxxxxxx> wrote:
> Jan points out that processing the list backwards is rather counter intuitive
> and that the effect of the hypercall can differ between forwards and 
> backwards
> processing (e.g. in the presence of duplicate idx or gpfn, which would be
> unusualy but as Jan says, users are a creative bunch)
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

> Cc: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> ---
> v2: Remember to crank the errs array too.
> 
>     Resending despite abuse of overwriting the IN paramters, since coming to
>     rely on backwards processing is a harder hole to get out of than going
>     from modifying the strict to not.
> 
> Release: subtle ABI change, should go in to 4.4 before people rely on it 
> (they
> are not relying on it today TTBOMK and it seems unlikely but lets not risk 
> it)
> ---
>  xen/arch/arm/mm.c |   13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index e235364..f1b7099 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1062,21 +1062,18 @@ static int xenmem_add_to_physmap_range(struct domain 
> *d,
>  {
>      int rc;
>  
> -    /* Process entries in reverse order to allow continuations */
>      while ( xatpr->size > 0 )
>      {
>          xen_ulong_t idx;
>          xen_pfn_t gpfn;
>  
> -        if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs,
> -                                             xatpr->size-1, 1)) )
> +        if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs, 0, 1)) )
>          {
>              rc = -EFAULT;
>              goto out;
>          }
>  
> -        if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns,
> -                                             xatpr->size-1, 1)) )
> +        if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns, 0, 1)) )
>          {
>              rc = -EFAULT;
>              goto out;
> @@ -1086,8 +1083,7 @@ static int xenmem_add_to_physmap_range(struct domain 
> *d,
>                                         xatpr->foreign_domid,
>                                         idx, gpfn);
>  
> -        if ( unlikely(copy_to_guest_offset(xatpr->errs,
> -                                           xatpr->size-1, &rc, 1)) )
> +        if ( unlikely(copy_to_guest_offset(xatpr->errs, 0, &rc, 1)) )
>          {
>              rc = -EFAULT;
>              goto out;
> @@ -1096,6 +1092,9 @@ static int xenmem_add_to_physmap_range(struct domain 
> *d,
>          if ( rc < 0 )
>              goto out;
>  
> +        guest_handle_add_offset(xatpr->idxs, 1);
> +        guest_handle_add_offset(xatpr->gpfns, 1);
> +        guest_handle_add_offset(xatpr->errs, 1);
>          xatpr->size--;
>  
>          /* Check for continuation if it's not the last interation */
> -- 
> 1.7.10.4




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel