Re: [Xen-devel] [PATCH 02/18] PVH xen: add XENMEM_add_to_physmap_range

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.05.13 at 03:25, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
>  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)

The patch could be a bit smaller afaict if you used the otherwise
unused here domain ID field in xatp for passing the domain ID you
care about here (I hinted at that in the last round already, where
I also asked Stefano why we have three domains here in the first
place).

While I won't object the patch to remain the way it is, I'm also
not sure I'd want to ack it in the less efficient shape.

> +        if ( xsm_add_to_physmap(XSM_TARGET, current->domain, d) )
> +        {
> +            rcu_unlock_domain(d);
> +            return -EPERM;
> +        }

I realize there's another such bogus use of the function in the same
file, but you shouldn't propagate that mistake: The function has a
proper return value, and that's what should be used here instead
of forcing it to be -EPERM.

I also vaguely recall having pointed out in a much earlier review
that this functionality is lacking a counterpart in
compat_arch_memory_op().

Jan


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