Re: [PATCH 2/18 V2]: PVH xen: add XENMEM_add_to_physmap_range

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Fri, Mar 15, 2013 at 05:20:18PM -0700, Mukesh Rathor wrote:
> In this patch we add a new function xenmem_add_to_physmap_range(), and
> change xenmem_add_to_physmap_once parameters so it can be called from
> xenmem_add_to_physmap_range. There is no PVH specific change here.
> 
> Changes in V2:
>    - Do not break parameter so xenmem_add_to_physmap_once() but pass in
>      struct xen_add_to_physmap.

Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

You could also add in the comment section here:

XENMAPSPACE_gmfn_foreign should never be used in XENMEM_add_to_physmap
hypercall. Before this patch we would wound down to
xenmem_add_to_physmap_once which would return -EINVAL (as mfn=0).

Now we do it straight away by returning -EINVAL.

> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  xen/arch/x86/mm.c |   82 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 79 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index d00d9a2..6603752 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4268,7 +4268,8 @@ static int handle_iomem_range(unsigned long s, unsigned long e, void *p)
>  
>  static int xenmem_add_to_physmap_once(
>      struct domain *d,
> -    const struct xen_add_to_physmap *xatp)
> +    const struct xen_add_to_physmap *xatp,
> +    domid_t foreign_domid)
>  {
>      struct page_info *page = NULL;
>      unsigned long gfn = 0; /* gcc ... */
> @@ -4395,7 +4396,7 @@ static int xenmem_add_to_physmap(struct domain *d,
>          start_xatp = *xatp;
>          while ( xatp->size > 0 )
>          {
> -            rc = xenmem_add_to_physmap_once(d, xatp);
> +            rc = xenmem_add_to_physmap_once(d, xatp, -1);
>              if ( rc < 0 )
>                  return rc;
>  
> @@ -4421,7 +4422,52 @@ static int xenmem_add_to_physmap(struct domain *d,
>          return rc;
>      }
>  
> -    return xenmem_add_to_physmap_once(d, xatp);
> +    return xenmem_add_to_physmap_once(d, xatp, -1);
> +}
> +
> +static noinline int xenmem_add_to_physmap_range(struct domain *d,
> +                                       struct xen_add_to_physmap_range *xatpr)
> +{
> +    int rc;
> +
> +    /* Process entries in reverse order to allow continuations */
> +    while ( xatpr->size > 0 )
> +    {
> +        xen_ulong_t idx;
> +        xen_pfn_t gpfn;
> +        struct xen_add_to_physmap xatp;
> +
> +        rc = copy_from_guest_offset(&idx, xatpr->idxs, xatpr->size-1, 1);
> +        if ( rc < 0 )
> +            goto out;
> +
> +        rc = copy_from_guest_offset(&gpfn, xatpr->gpfns, xatpr->size-1, 1);
> +        if ( rc < 0 )
> +            goto out;
> +
> +        xatp.space = xatpr->space;
> +        xatp.idx = idx;
> +        xatp.gpfn = gpfn;
> +        rc = xenmem_add_to_physmap_once(d, &xatp, xatpr->foreign_domid);
> +
> +        if (rc)
> +            goto out;
> +
> +        xatpr->size--;
> +
> +        /* Check for continuation if it's not the last interation */
> +        if ( xatpr->size > 0 && hypercall_preempt_check() )
> +        {
> +            rc = -EAGAIN;
> +            goto out;
> +        }
> +    }
> +
> +    rc = 0;
> +
> +out:
> +    return rc;
> +
>  }
>  
>  long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
> @@ -4438,6 +4484,10 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
>          if ( copy_from_guest(&xatp, arg, 1) )
>              return -EFAULT;
>  
> +        /* This one is only supported for add_to_physmap_range */
> +        if ( xatp.space == XENMAPSPACE_gmfn_foreign )
> +            return -EINVAL;
> +
>          d = rcu_lock_domain_by_any_id(xatp.domid);
>          if ( d == NULL )
>              return -ESRCH;
> @@ -4465,6 +4515,32 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE_PARAM(void) arg)
>          return rc;
>      }
>  
> +    case XENMEM_add_to_physmap_range:
> +    {
> +        struct xen_add_to_physmap_range xatpr;
> +        struct domain *d;
> +
> +        if ( copy_from_guest(&xatpr, arg, 1) )
> +            return -EFAULT;
> +
> +        rc = rcu_lock_target_domain_by_id(xatpr.domid, &d);
> +        if ( rc != 0 )
> +            return rc;
> +
> +        rc = xenmem_add_to_physmap_range(d, &xatpr);
> +
> +        rcu_unlock_domain(d);
> +
> +        if ( rc && copy_to_guest(arg, &xatpr, 1) )
> +            rc = -EFAULT;
> +
> +        if ( rc == -EAGAIN )
> +            rc = hypercall_create_continuation(
> +                __HYPERVISOR_memory_op, "ih", op, arg);
> +
> +        return rc;
> +    }
> +
>      case XENMEM_set_memory_map:
>      {
>          struct xen_foreign_memory_map fmap;
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>