From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tamas K Lengyel Subject: [PATCH V9 4/6] xen/vm_event: Relocate memop checks Date: Thu, 9 Apr 2015 16:32:51 +0200 Message-ID: <1428589973-24438-5-git-send-email-tamas.lengyel@zentific.com> References: <1428589973-24438-1-git-send-email-tamas.lengyel@zentific.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1428589973-24438-1-git-send-email-tamas.lengyel@zentific.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org Cc: kevin.tian@intel.com, wei.liu2@citrix.com, ian.campbell@citrix.com, steve@zentific.com, stefano.stabellini@eu.citrix.com, jun.nakajima@intel.com, tim@xen.org, ian.jackson@eu.citrix.com, eddie.dong@intel.com, andres@lagarcavilla.org, jbeulich@suse.com, Tamas K Lengyel , rshriram@cs.ubc.ca, keir@xen.org, dgdegra@tycho.nsa.gov, yanghy@cn.fujitsu.com, rcojocaru@bitdefender.com List-Id: xen-devel@lists.xenproject.org The memop handler function for paging/sharing responsible for calling XSM doesn't really have anything to do with vm_event, thus in this patch we relocate it into mem_paging_memop and mem_sharing_memop. This has already been the approach in mem_access_memop, so in this patch we just make it consistent. Signed-off-by: Tamas K Lengyel Reviewed-by: Andrew Cooper Reviewed-by: Tim Deegan --- v7: Minor fixes with returning error codes on rcu lock failure v6: Don't pass superfluous cmd to the memops. Unlock rcu's in sharing/paging Style fixes --- xen/arch/x86/mm/mem_paging.c | 41 ++++++++++--- xen/arch/x86/mm/mem_sharing.c | 125 +++++++++++++++++++++++++------------- xen/arch/x86/x86_64/compat/mm.c | 26 +------- xen/arch/x86/x86_64/mm.c | 24 +------- xen/common/vm_event.c | 43 ------------- xen/include/asm-x86/mem_paging.h | 2 +- xen/include/asm-x86/mem_sharing.h | 3 +- xen/include/xen/vm_event.h | 1 - 8 files changed, 124 insertions(+), 141 deletions(-) diff --git a/xen/arch/x86/mm/mem_paging.c b/xen/arch/x86/mm/mem_paging.c index e63d8c1..17d2319 100644 --- a/xen/arch/x86/mm/mem_paging.c +++ b/xen/arch/x86/mm/mem_paging.c @@ -22,27 +22,45 @@ #include -#include +#include +#include - -int mem_paging_memop(struct domain *d, xen_mem_paging_op_t *mpo) +int mem_paging_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_paging_op_t) arg) { - int rc = -ENODEV; - if ( unlikely(!d->vm_event->paging.ring_page) ) + int rc; + xen_mem_paging_op_t mpo; + struct domain *d; + bool_t copyback = 0; + + if ( copy_from_guest(&mpo, arg, 1) ) + return -EFAULT; + + rc = rcu_lock_live_remote_domain_by_id(mpo.domain, &d); + if ( rc ) return rc; - switch( mpo->op ) + rc = xsm_vm_event_op(XSM_DM_PRIV, d, XENMEM_paging_op); + if ( rc ) + goto out; + + rc = -ENODEV; + if ( unlikely(!d->vm_event->paging.ring_page) ) + goto out; + + switch( mpo.op ) { case XENMEM_paging_op_nominate: - rc = p2m_mem_paging_nominate(d, mpo->gfn); + rc = p2m_mem_paging_nominate(d, mpo.gfn); break; case XENMEM_paging_op_evict: - rc = p2m_mem_paging_evict(d, mpo->gfn); + rc = p2m_mem_paging_evict(d, mpo.gfn); break; case XENMEM_paging_op_prep: - rc = p2m_mem_paging_prep(d, mpo->gfn, mpo->buffer); + rc = p2m_mem_paging_prep(d, mpo.gfn, mpo.buffer); + if ( !rc ) + copyback = 1; break; default: @@ -50,6 +68,11 @@ int mem_paging_memop(struct domain *d, xen_mem_paging_op_t *mpo) break; } + if ( copyback && __copy_to_guest(arg, &mpo, 1) ) + rc = -EFAULT; + +out: + rcu_unlock_domain(d); return rc; } diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index 4959407..ff01378 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -1293,39 +1294,66 @@ int relinquish_shared_pages(struct domain *d) return rc; } -int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec) +int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg) { - int rc = 0; + int rc; + xen_mem_sharing_op_t mso; + struct domain *d; + + rc = -EFAULT; + if ( copy_from_guest(&mso, arg, 1) ) + return rc; + + if ( mso.op == XENMEM_sharing_op_audit ) + return mem_sharing_audit(); + + rc = rcu_lock_live_remote_domain_by_id(mso.domain, &d); + if ( rc ) + return rc; + + rc = xsm_vm_event_op(XSM_DM_PRIV, d, XENMEM_sharing_op); + if ( rc ) + goto out; /* Only HAP is supported */ + rc = -ENODEV; if ( !hap_enabled(d) || !d->arch.hvm_domain.mem_sharing_enabled ) - return -ENODEV; + goto out; - switch(mec->op) + rc = -ENODEV; + if ( unlikely(!d->vm_event->share.ring_page) ) + goto out; + + switch ( mso.op ) { case XENMEM_sharing_op_nominate_gfn: { - unsigned long gfn = mec->u.nominate.u.gfn; + unsigned long gfn = mso.u.nominate.u.gfn; shr_handle_t handle; + + rc = -EINVAL; if ( !mem_sharing_enabled(d) ) - return -EINVAL; + goto out; + rc = mem_sharing_nominate_page(d, gfn, 0, &handle); - mec->u.nominate.handle = handle; + mso.u.nominate.handle = handle; } break; case XENMEM_sharing_op_nominate_gref: { - grant_ref_t gref = mec->u.nominate.u.grant_ref; + grant_ref_t gref = mso.u.nominate.u.grant_ref; unsigned long gfn; shr_handle_t handle; + rc = -EINVAL; if ( !mem_sharing_enabled(d) ) - return -EINVAL; + goto out; if ( mem_sharing_gref_to_gfn(d, gref, &gfn) < 0 ) - return -EINVAL; + goto out; + rc = mem_sharing_nominate_page(d, gfn, 3, &handle); - mec->u.nominate.handle = handle; + mso.u.nominate.handle = handle; } break; @@ -1335,57 +1363,61 @@ int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec) struct domain *cd; shr_handle_t sh, ch; + rc = -EINVAL; if ( !mem_sharing_enabled(d) ) - return -EINVAL; + goto out; - rc = rcu_lock_live_remote_domain_by_id(mec->u.share.client_domain, + rc = rcu_lock_live_remote_domain_by_id(mso.u.share.client_domain, &cd); if ( rc ) - return rc; + goto out; - rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mec->op); + rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mso.op); if ( rc ) { rcu_unlock_domain(cd); - return rc; + goto out; } if ( !mem_sharing_enabled(cd) ) { rcu_unlock_domain(cd); - return -EINVAL; + rc = -EINVAL; + goto out; } - if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mec->u.share.source_gfn) ) + if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.source_gfn) ) { grant_ref_t gref = (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( - mec->u.share.source_gfn)); + mso.u.share.source_gfn)); if ( mem_sharing_gref_to_gfn(d, gref, &sgfn) < 0 ) { rcu_unlock_domain(cd); - return -EINVAL; + rc = -EINVAL; + goto out; } } else { - sgfn = mec->u.share.source_gfn; + sgfn = mso.u.share.source_gfn; } - if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mec->u.share.client_gfn) ) + if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.client_gfn) ) { grant_ref_t gref = (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( - mec->u.share.client_gfn)); + mso.u.share.client_gfn)); if ( mem_sharing_gref_to_gfn(cd, gref, &cgfn) < 0 ) { rcu_unlock_domain(cd); - return -EINVAL; + rc = -EINVAL; + goto out; } } else { - cgfn = mec->u.share.client_gfn; + cgfn = mso.u.share.client_gfn; } - sh = mec->u.share.source_handle; - ch = mec->u.share.client_handle; + sh = mso.u.share.source_handle; + ch = mso.u.share.client_handle; rc = mem_sharing_share_pages(d, sgfn, sh, cd, cgfn, ch); @@ -1399,37 +1431,40 @@ int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec) struct domain *cd; shr_handle_t sh; + rc = -EINVAL; if ( !mem_sharing_enabled(d) ) - return -EINVAL; + goto out; - rc = rcu_lock_live_remote_domain_by_id(mec->u.share.client_domain, + rc = rcu_lock_live_remote_domain_by_id(mso.u.share.client_domain, &cd); if ( rc ) - return rc; + goto out; - rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mec->op); + rc = xsm_mem_sharing_op(XSM_DM_PRIV, d, cd, mso.op); if ( rc ) { rcu_unlock_domain(cd); - return rc; + goto out; } if ( !mem_sharing_enabled(cd) ) { rcu_unlock_domain(cd); - return -EINVAL; + rc = -EINVAL; + goto out; } - if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mec->u.share.source_gfn) ) + if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.source_gfn) ) { /* Cannot add a gref to the physmap */ rcu_unlock_domain(cd); - return -EINVAL; + rc = -EINVAL; + goto out; } - sgfn = mec->u.share.source_gfn; - sh = mec->u.share.source_handle; - cgfn = mec->u.share.client_gfn; + sgfn = mso.u.share.source_gfn; + sh = mso.u.share.source_handle; + cgfn = mso.u.share.client_gfn; rc = mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn); @@ -1440,7 +1475,10 @@ int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec) case XENMEM_sharing_op_resume: { if ( !mem_sharing_enabled(d) ) - return -EINVAL; + { + rc = -EINVAL; + goto out; + } vm_event_resume(d, &d->vm_event->share); } @@ -1448,14 +1486,14 @@ int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec) case XENMEM_sharing_op_debug_gfn: { - unsigned long gfn = mec->u.debug.u.gfn; + unsigned long gfn = mso.u.debug.u.gfn; rc = mem_sharing_debug_gfn(d, gfn); } break; case XENMEM_sharing_op_debug_gref: { - grant_ref_t gref = mec->u.debug.u.gref; + grant_ref_t gref = mso.u.debug.u.gref; rc = mem_sharing_debug_gref(d, gref); } break; @@ -1465,6 +1503,11 @@ int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec) break; } + if ( !rc && __copy_to_guest(arg, &mso, 1) ) + rc = -EFAULT; + +out: + rcu_unlock_domain(d); return rc; } diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c index d5eb920..d034bd0 100644 --- a/xen/arch/x86/x86_64/compat/mm.c +++ b/xen/arch/x86/x86_64/compat/mm.c @@ -1,9 +1,9 @@ #include -#include #include #include #include #include +#include #include int compat_set_gdt(XEN_GUEST_HANDLE_PARAM(uint) frame_list, unsigned int entries) @@ -187,30 +187,10 @@ int compat_arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return mem_sharing_get_nr_shared_mfns(); case XENMEM_paging_op: - { - xen_mem_paging_op_t mpo; - - if ( copy_from_guest(&mpo, arg, 1) ) - return -EFAULT; - rc = do_vm_event_op(cmd, mpo.domain, &mpo); - if ( !rc && __copy_to_guest(arg, &mpo, 1) ) - return -EFAULT; - break; - } + return mem_paging_memop(guest_handle_cast(arg, xen_mem_paging_op_t)); case XENMEM_sharing_op: - { - xen_mem_sharing_op_t mso; - - if ( copy_from_guest(&mso, arg, 1) ) - return -EFAULT; - if ( mso.op == XENMEM_sharing_op_audit ) - return mem_sharing_audit(); - rc = do_vm_event_op(cmd, mso.domain, &mso); - if ( !rc && __copy_to_guest(arg, &mso, 1) ) - return -EFAULT; - break; - } + return mem_sharing_memop(guest_handle_cast(arg, xen_mem_sharing_op_t)); default: rc = -ENOSYS; diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c index 4953dcd..de95c38 100644 --- a/xen/arch/x86/x86_64/mm.c +++ b/xen/arch/x86/x86_64/mm.c @@ -26,7 +26,6 @@ #include #include #include -#include #include #include #include @@ -37,6 +36,7 @@ #include #include #include +#include #include #include @@ -984,28 +984,10 @@ long subarch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg) return mem_sharing_get_nr_shared_mfns(); case XENMEM_paging_op: - { - xen_mem_paging_op_t mpo; - if ( copy_from_guest(&mpo, arg, 1) ) - return -EFAULT; - rc = do_vm_event_op(cmd, mpo.domain, &mpo); - if ( !rc && __copy_to_guest(arg, &mpo, 1) ) - return -EFAULT; - break; - } + return mem_paging_memop(guest_handle_cast(arg, xen_mem_paging_op_t)); case XENMEM_sharing_op: - { - xen_mem_sharing_op_t mso; - if ( copy_from_guest(&mso, arg, 1) ) - return -EFAULT; - if ( mso.op == XENMEM_sharing_op_audit ) - return mem_sharing_audit(); - rc = do_vm_event_op(cmd, mso.domain, &mso); - if ( !rc && __copy_to_guest(arg, &mso, 1) ) - return -EFAULT; - break; - } + return mem_sharing_memop(guest_handle_cast(arg, xen_mem_sharing_op_t)); default: rc = -ENOSYS; diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c index d276bd8..1eb9825 100644 --- a/xen/common/vm_event.c +++ b/xen/common/vm_event.c @@ -27,15 +27,6 @@ #include #include #include - -#ifdef HAS_MEM_PAGING -#include -#endif - -#ifdef HAS_MEM_SHARING -#include -#endif - #include /* for public/io/ring.h macros */ @@ -513,40 +504,6 @@ static void mem_sharing_notification(struct vcpu *v, unsigned int port) } #endif -int do_vm_event_op(int op, uint32_t domain, void *arg) -{ - int ret; - struct domain *d; - - ret = rcu_lock_live_remote_domain_by_id(domain, &d); - if ( ret ) - return ret; - - ret = xsm_vm_event_op(XSM_DM_PRIV, d, op); - if ( ret ) - goto out; - - switch (op) - { -#ifdef HAS_MEM_PAGING - case XENMEM_paging_op: - ret = mem_paging_memop(d, arg); - break; -#endif -#ifdef HAS_MEM_SHARING - case XENMEM_sharing_op: - ret = mem_sharing_memop(d, arg); - break; -#endif - default: - ret = -ENOSYS; - } - - out: - rcu_unlock_domain(d); - return ret; -} - /* Clean up on domain destruction */ void vm_event_cleanup(struct domain *d) { diff --git a/xen/include/asm-x86/mem_paging.h b/xen/include/asm-x86/mem_paging.h index 5f0f91b..4179eb7 100644 --- a/xen/include/asm-x86/mem_paging.h +++ b/xen/include/asm-x86/mem_paging.h @@ -23,7 +23,7 @@ #ifndef __ASM_X86_MEM_PAGING_H__ #define __ASM_X86_MEM_PAGING_H__ -int mem_paging_memop(struct domain *d, xen_mem_paging_op_t *mpo); +int mem_paging_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_paging_op_t) arg); #endif /*__ASM_X86_MEM_PAGING_H__ */ diff --git a/xen/include/asm-x86/mem_sharing.h b/xen/include/asm-x86/mem_sharing.h index da99d46..e4a1024 100644 --- a/xen/include/asm-x86/mem_sharing.h +++ b/xen/include/asm-x86/mem_sharing.h @@ -90,8 +90,7 @@ static inline int mem_sharing_unshare_page(struct domain *d, */ int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, bool_t allow_sleep); -int mem_sharing_memop(struct domain *d, - xen_mem_sharing_op_t *mec); +int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg); int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec); int mem_sharing_audit(void); diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h index 960beb2..51cbe8b 100644 --- a/xen/include/xen/vm_event.h +++ b/xen/include/xen/vm_event.h @@ -69,7 +69,6 @@ int vm_event_get_response(struct domain *d, struct vm_event_domain *ved, void vm_event_resume(struct domain *d, struct vm_event_domain *ved); -int do_vm_event_op(int op, uint32_t domain, void *arg); int vm_event_domctl(struct domain *d, xen_domctl_vm_event_op_t *vec, XEN_GUEST_HANDLE_PARAM(void) u_domctl); -- 2.1.4