[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 3/4] move XENMEM_add_to_physmap_range handling framework to common code



On Wed, 2013-12-18 at 14:35 +0000, Jan Beulich wrote:
> There's really nothing really architecture specific here; the
> architecture specific handling is limited to
> xenmem_add_to_physmap_one().
> 
> Note the restriction of XENMAPSPACE_gmfn_range only being supported
> through XENMEM_add_to_physmap is being dropped as being pointless.

Note that this will still not work on ARM where add_to_physmap_one()
does not support the gmfn_range and will still return ENOSYS.

If you are dropping this restriction then xen/include/public/memory.h
will need updating (likewise the first patch).

Personally I think that having two redundant ways of doing the same
thing is the more pointless of the two. Especially given the need to
think a bit carefully about what XENMEM_add_to_physmap_range
XENMAPSPACE_gmfn_range might mean...

> This further eliminates the erroneous bailing from
> xenmem_add_to_physmap_range() if xenmem_add_to_physmap_one() fails.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1056,103 +1056,10 @@ int xenmem_add_to_physmap_one(
>      return rc;
>  }
>  
> -static int xenmem_add_to_physmap_range(struct domain *d,
> -                                       struct xen_add_to_physmap_range 
> *xatpr)
> -{
> -    int rc;
> -
> -    while ( xatpr->size > 0 )
> -    {
> -        xen_ulong_t idx;
> -        xen_pfn_t gpfn;
> -
> -        if ( unlikely(copy_from_guest_offset(&idx, xatpr->idxs, 0, 1)) )
> -        {
> -            rc = -EFAULT;
> -            goto out;
> -        }
> -
> -        if ( unlikely(copy_from_guest_offset(&gpfn, xatpr->gpfns, 0, 1)) )
> -        {
> -            rc = -EFAULT;
> -            goto out;
> -        }
> -
> -        rc = xenmem_add_to_physmap_one(d, xatpr->space,
> -                                       xatpr->foreign_domid,
> -                                       idx, gpfn);
> -
> -        if ( unlikely(copy_to_guest_offset(xatpr->errs, 0, &rc, 1)) )
> -        {
> -            rc = -EFAULT;
> -            goto out;
> -        }
> -
> -        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 */
> -        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)
>  {
> -    int rc;
> -
>      switch ( op )
>      {
> -    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;
> -
> -        /* This mapspace is redundant for this hypercall */
> -        if ( xatpr.space == XENMAPSPACE_gmfn_range )
> -            return -EINVAL;
> -
> -        d = rcu_lock_domain_by_any_id(xatpr.domid);
> -        if ( d == NULL )
> -            return -ESRCH;
> -
> -        rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d);
> -        if ( rc )
> -        {
> -            rcu_unlock_domain(d);
> -            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;
> -    }
>      /* XXX: memsharing not working yet */
>      case XENMEM_get_sharing_shared_pages:
>      case XENMEM_get_sharing_freed_pages:
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -598,6 +598,57 @@ static int xenmem_add_to_physmap(struct 
>      return rc;
>  }
>  
> +static int xenmem_add_to_physmap_range(struct domain *d,
> +                                       struct xen_add_to_physmap_range 
> *xatpr)
> +{
> +    int rc;
> +
> +    while ( xatpr->size > 0 )
> +    {
> +        xen_ulong_t idx;
> +        xen_pfn_t gpfn;
> +
> +        if ( unlikely(__copy_from_guest_offset(&idx, xatpr->idxs, 0, 1)) )
> +        {
> +            rc = -EFAULT;
> +            goto out;
> +        }
> +
> +        if ( unlikely(__copy_from_guest_offset(&gpfn, xatpr->gpfns, 0, 1)) )
> +        {
> +            rc = -EFAULT;
> +            goto out;
> +        }
> +
> +        rc = xenmem_add_to_physmap_one(d, xatpr->space,
> +                                       xatpr->foreign_domid,
> +                                       idx, gpfn);
> +
> +        if ( unlikely(__copy_to_guest_offset(xatpr->errs, 0, &rc, 1)) )
> +        {
> +            rc = -EFAULT;
> +            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. */
> +        if ( xatpr->size > 0 && hypercall_preempt_check() )
> +        {
> +            rc = -EAGAIN;
> +            goto out;
> +        }
> +    }
> +
> +    rc = 0;
> +
> +out:
> +    return rc;
> +}
> +
>  long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      struct domain *d;
> @@ -763,6 +814,44 @@ long do_memory_op(unsigned long cmd, XEN
>          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) ||
> +             !guest_handle_okay(xatpr.idxs, xatpr.size) ||
> +             !guest_handle_okay(xatpr.gpfns, xatpr.size) ||
> +             !guest_handle_okay(xatpr.errs, xatpr.size) )
> +            return -EFAULT;
> +
> +        d = rcu_lock_domain_by_any_id(xatpr.domid);
> +        if ( d == NULL )
> +            return -ESRCH;
> +
> +        rc = xsm_add_to_physmap(XSM_TARGET, current->domain, d);
> +        if ( rc )
> +        {
> +            rcu_unlock_domain(d);
> +            return rc;
> +        }
> +
> +        rc = xenmem_add_to_physmap_range(d, &xatpr);
> +
> +        rcu_unlock_domain(d);
> +
> +        if ( rc == -EAGAIN )
> +        {
> +            if ( !__copy_to_guest(arg, &xatpr, 1) )
> +                rc = hypercall_create_continuation(
> +                    __HYPERVISOR_memory_op, "ih", op, arg);
> +            else
> +                rc = -EFAULT;
> +        }
> +
> +        return rc;
> +    }
> +
>      case XENMEM_remove_from_physmap:
>      {
>          struct xen_remove_from_physmap xrfp;
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel



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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.