Re: [Xen-devel] [PATCH 03/18] PVH xen: create domctl_memory_mapping() function

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.05.13 at 03:25, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> Changes in V5:
>   - Move iomem_access_permitted check to case statment from the function
>     as current doesn't point to dom0 during construct_dom0.
> 
> Changes in V6:
>   - Move iomem_access_permitted back to domctl_memory_mapping() as it should
>     be after the sanity and wrap checks of mfns/gfns.

So you're undoing what you previously did? How can it work then?
Or is the whole patch perhaps no longer needed with Dom0 code
dropped for the time being?

Otherwise, rather than undoing what you did in v5, I'd think you'd
want to keep the sanity/wrap checks in the caller as well, since if
you need the function to be split out just for Dom0 building, you
can assume the inputs to be sane. And if so, the XSM check could
remain in the caller too. That would also make sure there really is
no change in functionality:

> +long domctl_memory_mapping(struct domain *d, unsigned long gfn,
> +                           unsigned long mfn, unsigned long nr_mfns,
> +                           bool_t add_map)
> +{
> +    unsigned long i;
> +    long ret;
> +
> +    if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
> +         ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
> +         (gfn + nr_mfns - 1) < gfn ) /* wrap? */
> +        return -EINVAL;
> +
> +    /* caller construct_dom0() runs on idle vcpu */
> +    if ( !is_idle_vcpu(current) &&

This check is new, so this is not pure code motion.

> +         !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns - 1) )
> +        return -EPERM;
> +
> +    ret = xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add_map);

And this was xsm_iomem_mapping() in the original code. What's going
on here?

Jan


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