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

[Xen-devel] [PATCH] consolidate xsm processing within domain control hypercall



Consolidate all the 15 xsm calls from within do_domctl a single
routine that is only called in one place, xsm_domctl:

int xsm_domctl (struct xen_domctl *domctl);

The parameter to domctl is a pointer to the xen_domctl structure that
contains a union of all sub operational parameters.

The benefits of this patch include:

(1) Easier to maintain because there is one place in the entire
hypercall to check with the xsm, instead of 15 or more.

(2) New sub-operations don't also need to add a corresponding xsm function.

(3) Removes 178 lines of code.

(4) Enhanced security because of 1-4.

Signed-off-by: Mike D. Day <ncmike@xxxxxxxxxx>
---

When updating my scheduling group patches I started looking at the xsm
code in domctl.c. This patch makes it easier for me to update my
scheduling group patches, but it should be considered on its own.

 common/domctl.c   |   95 ++------------------------------------------
 include/xsm/xsm.h |  115 +++++-------------------------------------------------
 2 files changed, 16 insertions(+), 194 deletions(-)

diff -r 26e766e0c628 xen/common/domctl.c
--- a/xen/common/domctl.c       Mon Nov 26 22:28:48 2007 +0000
+++ b/xen/common/domctl.c       Thu Nov 29 07:54:42 2007 -0500
@@ -187,6 +187,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
 
     spin_lock(&domctl_lock);
 
+    if ( xsm_domctl(op) )
+        goto domctl_out;
+
     switch ( op->cmd )
     {
 
@@ -200,10 +203,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( d == NULL )
             break;
-
-        ret = xsm_setvcpucontext(d);
-        if ( ret )
-            goto svc_out;
 
         ret = -EINVAL;
         if ( (vcpu >= MAX_VIRT_CPUS) || ((v = d->vcpu[vcpu]) == NULL) )
@@ -252,17 +251,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( d != NULL )
         {
-            ret = xsm_pausedomain(d);
-            if ( ret )
-                goto pausedomain_out;
-
             ret = -EINVAL;
             if ( d != current->domain )
             {
                 domain_pause_by_systemcontroller(d);
                 ret = 0;
             }
-        pausedomain_out:
             rcu_unlock_domain(d);
         }
     }
@@ -275,14 +269,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( d == NULL )
             break;
-
-        ret = xsm_unpausedomain(d);
-        if ( ret )
-        {
-            rcu_unlock_domain(d);
-            break;
-        }
-
         domain_unpause_by_systemcontroller(d);
         rcu_unlock_domain(d);
         ret = 0;
@@ -296,13 +282,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( d == NULL )
             break;
-
-        ret = xsm_resumedomain(d);
-        if ( ret )
-        {
-            rcu_unlock_domain(d);
-            break;
-        }
 
         domain_resume(d);
         rcu_unlock_domain(d);
@@ -379,13 +358,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
             break;
 
-        ret = xsm_max_vcpus(d);
-        if ( ret )
-        {
-            rcu_unlock_domain(d);
-            break;
-        }
-
         /* Needed, for example, to ensure writable p.t. state is synced. */
         domain_pause(d);
 
@@ -422,7 +394,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( d != NULL )
         {
-            ret = xsm_destroydomain(d) ? : domain_kill(d);
+            domain_kill(d);
             rcu_unlock_domain(d);
         }
     }
@@ -440,10 +412,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( d == NULL )
             break;
 
-        ret = xsm_vcpuaffinity(op->cmd, d);
-        if ( ret )
-            goto vcpuaffinity_out;
-
         ret = -EINVAL;
         if ( op->u.vcpuaffinity.vcpu >= MAX_VIRT_CPUS )
             goto vcpuaffinity_out;
@@ -478,15 +446,10 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
             break;
 
-        ret = xsm_scheduler(d);
-        if ( ret )
-            goto scheduler_op_out;
-
         ret = sched_adjust(d, &op->u.scheduler_op);
         if ( copy_to_guest(u_domctl, op, 1) )
             ret = -EFAULT;
 
-    scheduler_op_out:
         rcu_unlock_domain(d);
     }
     break;
@@ -509,17 +472,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
             break;
         }
 
-        ret = xsm_getdomaininfo(d);
-        if ( ret )
-            goto getdomaininfo_out;
-
         getdomaininfo(d, &op->u.getdomaininfo);
 
         op->domain = op->u.getdomaininfo.domain;
         if ( copy_to_guest(u_domctl, op, 1) )
             ret = -EFAULT;
 
-    getdomaininfo_out:
         rcu_read_unlock(&domlist_read_lock);
     }
     break;
@@ -533,10 +491,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         ret = -ESRCH;
         if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
             break;
-
-        ret = xsm_getvcpucontext(d);
-        if ( ret )
-            goto getvcpucontext_out;
 
         ret = -EINVAL;
         if ( op->u.vcpucontext.vcpu >= MAX_VIRT_CPUS )
@@ -594,10 +548,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( (d = rcu_lock_domain_by_id(op->domain)) == NULL )
             break;
 
-        ret = xsm_getvcpuinfo(d);
-        if ( ret )
-            goto getvcpuinfo_out;
-
         ret = -EINVAL;
         if ( op->u.getvcpuinfo.vcpu >= MAX_VIRT_CPUS )
             goto getvcpuinfo_out;
@@ -633,10 +583,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( d == NULL )
             break;
 
-        ret = xsm_setdomainmaxmem(d);
-        if ( ret )
-            goto max_mem_out;
-
         ret = -EINVAL;
         new_max = op->u.max_mem.max_memkb >> (PAGE_SHIFT-10);
 
@@ -651,7 +597,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         }
         spin_unlock(&d->page_alloc_lock);
 
-    max_mem_out:
         rcu_unlock_domain(d);
     }
     break;
@@ -664,13 +609,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         d = rcu_lock_domain_by_id(op->domain);
         if ( d == NULL )
             break;
-
-        ret = xsm_setdomainhandle(d);
-        if ( ret )
-        {
-            rcu_unlock_domain(d);
-            break;
-        }
 
         memcpy(d->handle, op->u.setdomainhandle.handle,
                sizeof(xen_domain_handle_t));
@@ -687,13 +625,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         d = rcu_lock_domain_by_id(op->domain);
         if ( d == NULL )
             break;
-
-        ret = xsm_setdebugging(d);
-        if ( ret )
-        {
-            rcu_unlock_domain(d);
-            break;
-        }
 
         domain_pause(d);
         d->debugger_attached = !!op->u.setdebugging.enable;
@@ -717,16 +648,11 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         if ( d == NULL )
             break;
 
-        ret = xsm_irq_permission(d, pirq, op->u.irq_permission.allow_access);
-        if ( ret )
-            goto irq_permission_out;
-        
         if ( op->u.irq_permission.allow_access )
             ret = irq_permit_access(d, pirq);
         else
             ret = irq_deny_access(d, pirq);
 
-    irq_permission_out:
         rcu_unlock_domain(d);
     }
     break;
@@ -745,17 +671,12 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         d = rcu_lock_domain_by_id(op->domain);
         if ( d == NULL )
             break;
-
-        ret = xsm_iomem_permission(d, mfn, 
op->u.iomem_permission.allow_access);
-        if ( ret )
-            goto iomem_permission_out;
 
         if ( op->u.iomem_permission.allow_access )
             ret = iomem_permit_access(d, mfn, mfn + nr_mfns - 1);
         else
             ret = iomem_deny_access(d, mfn, mfn + nr_mfns - 1);
 
-    iomem_permission_out:
         rcu_unlock_domain(d);
     }
     break;
@@ -768,13 +689,6 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         d = rcu_lock_domain_by_id(op->domain);
         if ( d != NULL )
         {
-            ret = xsm_domain_settime(d);
-            if ( ret )
-            {
-                rcu_unlock_domain(d);
-                break;
-            }
-
             d->time_offset_seconds = op->u.settimeoffset.time_offset_seconds;
             rcu_unlock_domain(d);
             ret = 0;
@@ -787,6 +701,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domc
         break;
     }
 
+domctl_out:
     spin_unlock(&domctl_lock);
 
     return ret;
diff -r 26e766e0c628 xen/include/xsm/xsm.h
--- a/xen/include/xsm/xsm.h     Mon Nov 26 22:28:48 2007 +0000
+++ b/xen/include/xsm/xsm.h     Thu Nov 29 07:54:42 2007 -0500
@@ -50,34 +50,18 @@ extern xsm_initcall_t __xsm_initcall_sta
 
 struct xsm_operations {
     void (*security_domaininfo) (struct domain *d,
-                                        struct xen_domctl_getdomaininfo *info);
-    int (*setvcpucontext) (struct domain *d);
-    int (*pausedomain) (struct domain *d);
-    int (*unpausedomain) (struct domain *d);
-    int (*resumedomain) (struct domain *d);
+                                 struct xen_domctl_getdomaininfo *info);
+    int (*domctl) (struct xen_domctl *domctl);
     int (*domain_create) (struct domain *d, u32 ssidref);
-    int (*max_vcpus) (struct domain *d);
-    int (*destroydomain) (struct domain *d);
-    int (*vcpuaffinity) (int cmd, struct domain *d);
-    int (*scheduler) (struct domain *d);
     int (*getdomaininfo) (struct domain *d);
-    int (*getvcpucontext) (struct domain *d);
-    int (*getvcpuinfo) (struct domain *d);
-    int (*domain_settime) (struct domain *d);
     int (*tbufcontrol) (void);
     int (*readconsole) (uint32_t clear);
     int (*sched_id) (void);
-    int (*setdomainmaxmem) (struct domain *d);
-    int (*setdomainhandle) (struct domain *d);
-    int (*setdebugging) (struct domain *d);
-    int (*irq_permission) (struct domain *d, uint8_t pirq, uint8_t access);
-    int (*iomem_permission) (struct domain *d, unsigned long mfn, 
-                                                                uint8_t 
access);
     int (*perfcontrol) (void);
 
     int (*evtchn_unbound) (struct domain *d, struct evtchn *chn, domid_t id2);
     int (*evtchn_interdomain) (struct domain *d1, struct evtchn *chn1,
-                                        struct domain *d2, struct evtchn 
*chn2);
+                               struct domain *d2, struct evtchn *chn2);
     void (*evtchn_close_post) (struct evtchn *chn);
     int (*evtchn_send) (struct domain *d, struct evtchn *chn);
     int (*evtchn_status) (struct domain *d, struct evtchn *chn);
@@ -149,69 +133,19 @@ static inline void xsm_security_domainin
     xsm_call(security_domaininfo(d, info));
 }
 
-static inline int xsm_setvcpucontext(struct domain *d)
-{
-    return xsm_call(setvcpucontext(d));
-}
-
-static inline int xsm_pausedomain (struct domain *d)
-{
-    return xsm_call(pausedomain(d));
-}
-
-static inline int xsm_unpausedomain (struct domain *d)
-{
-    return xsm_call(unpausedomain(d));
-}
-
-static inline int xsm_resumedomain (struct domain *d)
-{
-    return xsm_call(resumedomain(d));
-}
-
-static inline int xsm_domain_create (struct domain *d, u32 ssidref)
+static inline int xsm_domctl(struct xen_domctl *domctl)
+{
+    return xsm_call(domctl(domctl));
+}
+
+static inline int xsm_domain_create(struct domain *d, u32 ssidref)
 {
     return xsm_call(domain_create(d, ssidref));
 }
 
-static inline int xsm_max_vcpus(struct domain *d)
-{
-    return xsm_call(max_vcpus(d));
-}
-
-static inline int xsm_destroydomain (struct domain *d)
-{
-    return xsm_call(destroydomain(d));
-}
-
-static inline int xsm_vcpuaffinity (int cmd, struct domain *d)
-{
-    return xsm_call(vcpuaffinity(cmd, d));
-}
-
-static inline int xsm_scheduler (struct domain *d)
-{
-    return xsm_call(scheduler(d));
-}
-
-static inline int xsm_getdomaininfo (struct domain *d)
-{
-    return xsm_call(getdomaininfo(d));
-}
-
-static inline int xsm_getvcpucontext (struct domain *d)
-{
-    return xsm_call(getvcpucontext(d));
-}
-
-static inline int xsm_getvcpuinfo (struct domain *d)
-{
-    return xsm_call(getvcpuinfo(d));
-}
-
-static inline int xsm_domain_settime (struct domain *d)
-{
-    return xsm_call(domain_settime(d));
+static inline int xsm_getdomaininfo(struct domain *d)
+{
+    return xsm_call(domain_getdomaininfo(d));
 }
 
 static inline int xsm_tbufcontrol (void)
@@ -227,33 +161,6 @@ static inline int xsm_sched_id (void)
 static inline int xsm_sched_id (void)
 {
     return xsm_call(sched_id());
-}
-
-static inline int xsm_setdomainmaxmem (struct domain *d)
-{
-    return xsm_call(setdomainmaxmem(d));
-}
-
-static inline int xsm_setdomainhandle (struct domain *d)
-{
-    return xsm_call(setdomainhandle(d));
-}
-
-static inline int xsm_setdebugging (struct domain *d)
-{
-    return xsm_call(setdebugging(d));
-}
-
-static inline int xsm_irq_permission (struct domain *d, uint8_t pirq,
-                                                                uint8_t access)
-{
-    return xsm_call(irq_permission(d, pirq, access));
-} 
-
-static inline int xsm_iomem_permission (struct domain *d, unsigned long mfn,
-                                                                uint8_t access)
-{
-    return xsm_call(iomem_permission(d, mfn, access));
 }
 
 static inline int xsm_perfcontrol (void)

-- 
Mike D. Day
Virtualization Architect and Sr. Technical Staff Member, IBM LTC
Cell: 919 412-3900
ST: mdday@xxxxxxxxxx | AIM: ncmikeday | Yahoo IM: ultra.runner
PGP key: http://www.ncultra.org/ncmike/pubkey.asc

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

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