thr3ads.net - Xen devel - [PATCH 3/18 V2]: PVH xen: create domctl_memory

If this information is useful, please help other people find it:
Share via:

Mukesh Rathor

2013-Mar-16 00:26 UTC

[PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function

In this patch, XEN_DOMCTL_memory_mapping code is put into a function so
it can be shared later for PVH. There is no change in it''s
functionality.

Changes in V2:
  - Remove PHYSDEVOP_map_iomem sub hypercall, and the code supporting it
    as the IO region is mapped transparently now.

Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
---
 xen/arch/x86/domctl.c    |  119 ++++++++++++++++++++++++---------------------
 xen/include/xen/domain.h |    2 +
 2 files changed, 65 insertions(+), 56 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index 31937e0..ecc8240 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -46,6 +46,68 @@ static int gdbsx_guest_mem_io(
     return (iop->remain ? -EFAULT : 0);
 }
 
+long domctl_memory_mapping(struct domain *d, unsigned long gfn,
+                           unsigned long mfn, unsigned long nr_mfns,
+                           int add_map)
+{
+    int i;
+    long ret = -EINVAL;
+
+    if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
+         ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT)) ||
+         (gfn + nr_mfns - 1) < gfn ) /* wrap? */
+        return ret;
+
+    ret = xsm_iomem_permission(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add_map);
+    if ( ret )
+        return ret;
+
+    if ( add_map )
+    {
+        printk(XENLOG_G_INFO
+               "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+               d->domain_id, gfn, mfn, nr_mfns);
+
+        ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
+        if ( !ret && paging_mode_translate(d) )
+        {
+            for ( i = 0; !ret && i < nr_mfns; i++ )
+                if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
+                    ret = -EIO;
+            if ( ret )
+            {
+                printk(XENLOG_G_WARNING
+                       "memory_map:fail: dom%d gfn=%lx mfn=%lx\n",
+                       d->domain_id, gfn + i, mfn + i);
+                while ( i-- )
+                    clear_mmio_p2m_entry(d, gfn + i);
+                if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1) &&
+                     IS_PRIV(current->domain) )
+                    printk(XENLOG_ERR
+                           "memory_map: failed to deny dom%d access to
[%lx,%lx]\n",
+                           d->domain_id, mfn, mfn + nr_mfns - 1);
+            }
+        }
+    } else {
+        printk(XENLOG_G_INFO
+               "memory_map:remove: dom%d gfn=%lx mfn=%lx nr=%lx\n",
+               d->domain_id, gfn, mfn, nr_mfns);
+
+        if ( paging_mode_translate(d) )
+            for ( i = 0; i < nr_mfns; i++ )
+                add_map |= !clear_mmio_p2m_entry(d, gfn + i);
+        ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
+        if ( !ret && add_map )
+            ret = -EIO;
+        if ( ret && IS_PRIV(current->domain) )
+            printk(XENLOG_ERR
+                   "memory_map: error %ld %s dom%d access to
[%lx,%lx]\n",
+                   ret, add_map ? "removing" : "denying",
d->domain_id,
+                   mfn, mfn + nr_mfns - 1);
+    }
+    return ret;
+}
+
 long arch_do_domctl(
     struct xen_domctl *domctl, struct domain *d,
     XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
@@ -628,68 +690,13 @@ long arch_do_domctl(
         unsigned long mfn = domctl->u.memory_mapping.first_mfn;
         unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
         int add = domctl->u.memory_mapping.add_mapping;
-        unsigned long i;
-
-        ret = -EINVAL;
-        if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
-             ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits - PAGE_SHIFT))
||
-             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
-            break;
 
         ret = -EPERM;
         if ( !IS_PRIV(current->domain) &&
              !iomem_access_permitted(current->domain, mfn, mfn + nr_mfns -
1) )
             break;
 
-        ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add);
-        if ( ret )
-            break;
-
-        if ( add )
-        {
-            printk(XENLOG_G_INFO
-                   "memory_map:add: dom%d gfn=%lx mfn=%lx nr=%lx\n",
-                   d->domain_id, gfn, mfn, nr_mfns);
-
-            ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
-            if ( !ret && paging_mode_translate(d) )
-            {
-                for ( i = 0; !ret && i < nr_mfns; i++ )
-                    if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
-                        ret = -EIO;
-                if ( ret )
-                {
-                    printk(XENLOG_G_WARNING
-                           "memory_map:fail: dom%d gfn=%lx
mfn=%lx\n",
-                           d->domain_id, gfn + i, mfn + i);
-                    while ( i-- )
-                        clear_mmio_p2m_entry(d, gfn + i);
-                    if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1)
&&
-                         IS_PRIV(current->domain) )
-                        printk(XENLOG_ERR
-                               "memory_map: failed to deny dom%d access to
[%lx,%lx]\n",
-                               d->domain_id, mfn, mfn + nr_mfns - 1);
-                }
-            }
-        }
-        else
-        {
-            printk(XENLOG_G_INFO
-                   "memory_map:remove: dom%d gfn=%lx mfn=%lx
nr=%lx\n",
-                   d->domain_id, gfn, mfn, nr_mfns);
-
-            if ( paging_mode_translate(d) )
-                for ( i = 0; i < nr_mfns; i++ )
-                    add |= !clear_mmio_p2m_entry(d, gfn + i);
-            ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
-            if ( !ret && add )
-                ret = -EIO;
-            if ( ret && IS_PRIV(current->domain) )
-                printk(XENLOG_ERR
-                       "memory_map: error %ld %s dom%d access to
[%lx,%lx]\n",
-                       ret, add ? "removing" : "denying",
d->domain_id,
-                       mfn, mfn + nr_mfns - 1);
-        }
+        ret = domctl_memory_mapping(d, gfn, mfn, nr_mfns, add);
     }
     break;
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index d4ac50f..a7b4c34 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -86,4 +86,6 @@ extern unsigned int xen_processor_pmbits;
 
 extern bool_t opt_dom0_vcpus_pin;
 
+extern long domctl_memory_mapping(struct domain *d, unsigned long gfn,
+                    unsigned long mfn, unsigned long nr_mfns, int add_map);
 #endif /* __XEN_DOMAIN_H__ */
-- 
1.7.2.3

Tim Deegan

2013-Mar-21 15:04 UTC

head link

Re: [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function

At 17:26 -0700 on 15 Mar (1363368383), Mukesh Rathor
wrote:> In this patch, XEN_DOMCTL_memory_mapping code is put into a function so
> it can be shared later for PVH. There is no change in it''s
> functionality.
> 
> Changes in V2:
>   - Remove PHYSDEVOP_map_iomem sub hypercall, and the code supporting it
>     as the IO region is mapped transparently now.
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com>
> ---
>  xen/arch/x86/domctl.c    |  119
++++++++++++++++++++++++---------------------
>  xen/include/xen/domain.h |    2 +
>  2 files changed, 65 insertions(+), 56 deletions(-)
> 
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 31937e0..ecc8240 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -46,6 +46,68 @@ static int gdbsx_guest_mem_io(
>      return (iop->remain ? -EFAULT : 0);
>  }
>  
> +long domctl_memory_mapping(struct domain *d, unsigned long gfn,
> +                           unsigned long mfn, unsigned long nr_mfns,
> +                           int add_map)
> +{
> +    int i;
The loop variable at the source was ''unsigned long'', and the
loop limit
is an unsigned long, so ''int'' doesn''t seem right. 
> @@ -628,68 +690,13 @@ long arch_do_domctl(
>          unsigned long mfn = domctl->u.memory_mapping.first_mfn;
>          unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
>          int add = domctl->u.memory_mapping.add_mapping;
> -        unsigned long i;
> -
> -        ret = -EINVAL;
> -        if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
> -             ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits -
PAGE_SHIFT)) ||
> -             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
> -            break;
>  
>          ret = -EPERM;
>          if ( !IS_PRIV(current->domain) &&
>               !iomem_access_permitted(current->domain, mfn, mfn +
nr_mfns - 1) )
>              break;
Why is this privilege check not being moved along with the rest of it?
Execially given that the two other IS_PRIV()s below are taken?

Tim.
> -        ret = xsm_iomem_mapping(XSM_HOOK, d, mfn, mfn + nr_mfns - 1, add);
> -        if ( ret )
> -            break;
> -
> -        if ( add )
> -        {
> -            printk(XENLOG_G_INFO
> -                   "memory_map:add: dom%d gfn=%lx mfn=%lx
nr=%lx\n",
> -                   d->domain_id, gfn, mfn, nr_mfns);
> -
> -            ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
> -            if ( !ret && paging_mode_translate(d) )
> -            {
> -                for ( i = 0; !ret && i < nr_mfns; i++ )
> -                    if ( !set_mmio_p2m_entry(d, gfn + i, _mfn(mfn + i)) )
> -                        ret = -EIO;
> -                if ( ret )
> -                {
> -                    printk(XENLOG_G_WARNING
> -                           "memory_map:fail: dom%d gfn=%lx
mfn=%lx\n",
> -                           d->domain_id, gfn + i, mfn + i);
> -                    while ( i-- )
> -                        clear_mmio_p2m_entry(d, gfn + i);
> -                    if ( iomem_deny_access(d, mfn, mfn + nr_mfns - 1)
&&
> -                         IS_PRIV(current->domain) )
> -                        printk(XENLOG_ERR
> -                               "memory_map: failed to deny dom%d
access to [%lx,%lx]\n",
> -                               d->domain_id, mfn, mfn + nr_mfns - 1);
> -                }
> -            }
> -        }
> -        else
> -        {
> -            printk(XENLOG_G_INFO
> -                   "memory_map:remove: dom%d gfn=%lx mfn=%lx
nr=%lx\n",
> -                   d->domain_id, gfn, mfn, nr_mfns);
> -
> -            if ( paging_mode_translate(d) )
> -                for ( i = 0; i < nr_mfns; i++ )
> -                    add |= !clear_mmio_p2m_entry(d, gfn + i);
> -            ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
> -            if ( !ret && add )
> -                ret = -EIO;
> -            if ( ret && IS_PRIV(current->domain) )
> -                printk(XENLOG_ERR
> -                       "memory_map: error %ld %s dom%d access to
[%lx,%lx]\n",
> -                       ret, add ? "removing" :
"denying", d->domain_id,
> -                       mfn, mfn + nr_mfns - 1);
> -        }
> +        ret = domctl_memory_mapping(d, gfn, mfn, nr_mfns, add);
>      }
>      break;
>  
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index d4ac50f..a7b4c34 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -86,4 +86,6 @@ extern unsigned int xen_processor_pmbits;
>  
>  extern bool_t opt_dom0_vcpus_pin;
>  
> +extern long domctl_memory_mapping(struct domain *d, unsigned long gfn,
> +                    unsigned long mfn, unsigned long nr_mfns, int
add_map);
>  #endif /* __XEN_DOMAIN_H__ */
> -- 
> 1.7.2.3
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Mukesh Rathor

2013-Mar-22 21:02 UTC

head link

Re: [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function

On Thu, 21 Mar 2013 15:04:33 +0000
Tim Deegan <tim@xen.org> wrote:
> At 17:26 -0700 on 15 Mar (1363368383), Mukesh Rathor wrote:
> > In this patch, XEN_DOMCTL_memory_mapping code is put into a
> > function so it can be shared later for PVH. There is no change in
> > it''s functionality.
> > 
>limit is an unsigned long, so ''int'' doesn''t seem
right.
> 
> > @@ -628,68 +690,13 @@ long arch_do_domctl(
> >          unsigned long mfn = domctl->u.memory_mapping.first_mfn;
> >          unsigned long nr_mfns = domctl->u.memory_mapping.nr_mfns;
> >          int add = domctl->u.memory_mapping.add_mapping;
> > -        unsigned long i;
> > -
> > -        ret = -EINVAL;
> > -        if ( (mfn + nr_mfns - 1) < mfn || /* wrap? */
> > -             ((mfn | (mfn + nr_mfns - 1)) >> (paddr_bits -
> > PAGE_SHIFT)) ||
> > -             (gfn + nr_mfns - 1) < gfn ) /* wrap? */
> > -            break;
> >  
> >          ret = -EPERM;
> >          if ( !IS_PRIV(current->domain) &&
> >               !iomem_access_permitted(current->domain, mfn, mfn +
> > nr_mfns - 1) ) break;
> 
> Why is this privilege check not being moved along with the rest of it?
> Execially given that the two other IS_PRIV()s below are taken?
Because, the other caller is construct_dom0, so not really needed. But
I''ll move it anyways.

thanks,
Mukesh

Xen devel - Mar 2013 - [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function

[PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function

Re: [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function

Re: [PATCH 3/18 V2]: PVH xen: create domctl_memory_mapping() function