Update paging/sharing/access interfaces v2
PATCH 1/1 Update paging/sharing/access interfaces v2
i(Was switch from domctl to memops)
Changes from v1 posted Feb 2nd 2012
- Patches 1 & 2 Acked-by Tim Deegan on the hypervisor side
- Added patch 3 to clean up the enable domctl interface, based on discussion with Ian Campbell
Description from original post follows:
Per page operations in the paging, sharing, and access tracking subsystems are all implemented with domctls (e.g. a domctl to evict one page, or to share one page).
Under heavy load, the domctl path reveals a lack of scalability. The domctl lock serializes dom0's vcpus in the hypervisor. When performing thousands of per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of its other work-starved processes.
We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).
This is a backwards-incompatible ABI change. It's been floating on the list for a couple weeks now, with no nacks thus far.
tools/libxc/xc_mem_access.c | 12 +- tools/libxc/xc_mem_event.c | 23 +++- tools/libxc/xc_mem_paging.c | 44 ++++---- tools/libxc/xc_memshr.c | 182 ++++++++++++++++------------------ tools/libxc/xenctrl.h | 6 +- tools/memshr/interface.c | 4 +- tools/tests/mem-sharing/memshrtool.c | 4 +- xen/arch/x86/domctl.c | 1 - xen/arch/x86/mm/mem_access.c | 7 +- xen/arch/x86/mm/mem_event.c | 68 ++++++++++-- xen/arch/x86/mm/mem_paging.c | 13 +- xen/arch/x86/mm/mem_sharing.c | 99 +++++++++++------- xen/arch/x86/x86_64/compat/mm.c | 23 ++++ xen/arch/x86/x86_64/mm.c | 23 ++++ xen/include/asm-x86/mem_access.h | 3 +- xen/include/asm-x86/mem_event.h | 2 + xen/include/asm-x86/mem_paging.h | 3 +- xen/include/asm-x86/mem_sharing.h | 3 + xen/include/public/domctl.h | 90 +++------------- xen/include/public/memory.h | 87 ++++++++++++++++ tools/libxc/xc_memshr.c | 11 ++ tools/libxc/xenctrl.h | 1 + tools/tests/mem-sharing/memshrtool.c | 11 ++ xen/arch/x86/mm/mem_sharing.c | 13 +- xen/arch/x86/x86_64/compat/mm.c | 3 + xen/arch/x86/x86_64/mm.c | 2 + xen/include/asm-x86/mem_sharing.h | 3 +- xen/include/public/memory.h | 3 +- tools/libxc/xc_mem_access.c | 10 +- tools/libxc/xc_mem_event.c | 16 ++- tools/libxc/xc_mem_paging.c | 10 +- tools/libxc/xenctrl.h | 7 +- tools/tests/xen-access/xen-access.c | 56 +--------- tools/xenpaging/xenpaging.c | 33 +---- tools/xenpaging/xenpaging.h | 2 +- xen/arch/x86/mm/mem_event.c | 33 +----- xen/include/public/domctl.h | 4 +- xen/include/public/mem_event.h | 4 - xen/include/xen/sched.h | 2 - 39 files changed, 514 insertions(+), 407 deletions(-)
PATCH 1/1 Use memops for mem paging, sharing, and access, instead of domctls
tools/libxc/xc_mem_access.c | 12 +- tools/libxc/xc_mem_event.c | 23 +++- tools/libxc/xc_mem_paging.c | 44 ++++---- tools/libxc/xc_memshr.c | 182 ++++++++++++++++------------------ tools/libxc/xenctrl.h | 6 +- tools/memshr/interface.c | 4 +- tools/tests/mem-sharing/memshrtool.c | 4 +- xen/arch/x86/domctl.c | 1 - xen/arch/x86/mm/mem_access.c | 7 +- xen/arch/x86/mm/mem_event.c | 68 ++++++++++-- xen/arch/x86/mm/mem_paging.c | 13 +- xen/arch/x86/mm/mem_sharing.c | 99 +++++++++++------- xen/arch/x86/x86_64/compat/mm.c | 23 ++++ xen/arch/x86/x86_64/mm.c | 23 ++++ xen/include/asm-x86/mem_access.h | 3 +- xen/include/asm-x86/mem_event.h | 2 + xen/include/asm-x86/mem_paging.h | 3 +- xen/include/asm-x86/mem_sharing.h | 3 + xen/include/public/domctl.h | 90 +++------------- xen/include/public/memory.h | 87 ++++++++++++++++ 20 files changed, 421 insertions(+), 276 deletions(-)
Per page operations in the paging, sharing, and access tracking subsystems are all implemented with domctls (e.g. a domctl to evict one page, or to share one page).
Under heavy load, the domctl path reveals a lack of scalability. The domctl lock serializes dom0's vcpus in the hypervisor. When performing thousands of per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of its other work-starved processes.
We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the memory_op hypercalls (e.g XENMEM_*).
tools/libxc/xc_mem_access.c +6 -6
@@ -30,7 +30,7 @@ int xc_mem_access_enable(xc_interface *xreturn xc_mem_event_control (xch, domain_id,XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE ,XEN_DOMCTL_MEM_EVENT_OP_ACCESS , - shared_page, ring_page,INVALID_MFN ); + shared_page, ring_page); }int xc_mem_access_disable (xc_interface *xch,domid_t domain_id)
@@ -38,15 +38,15 @@ int xc_mem_access_disable(xc_interface *return xc_mem_event_control (xch, domain_id,XEN_DOMCTL_MEM_EVENT_OP_ACCESS_DISABLE ,XEN_DOMCTL_MEM_EVENT_OP_ACCESS , -NULL ,NULL ,INVALID_MFN ); +NULL ,NULL ); }int xc_mem_access_resume (xc_interface *xch,domid_t domain_id,unsigned long gfn) { -return xc_mem_event_control (xch, domain_id, -XEN_DOMCTL_MEM_EVENT_OP_ACCESS_RESUME , -XEN_DOMCTL_MEM_EVENT_OP_ACCESS , -NULL ,NULL , gfn); +return xc_mem_event_memop (xch, domain_id, + XENMEM_access_op_resume, + XENMEM_access_op, + gfn,NULL ); }/*
tools/libxc/xc_mem_event.c +18 -5
@@ -24,8 +24,7 @@#include "xc_private.h" int xc_mem_event_control (xc_interface *xch,domid_t domain_id,unsigned int op, -unsigned int mode,void *page, -void *ring_page,unsigned long gfn) +unsigned int mode,void *page,void *ring_page) {DECLARE_DOMCTL ;
@@ -34,11 +33,25 @@ int xc_mem_event_control(xc_interface *x domctl.u.mem_event_op.op = op; domctl.u.mem_event_op.mode = mode; - domctl.u.mem_event_op.u.shared_addr = (unsigned long )page; + domctl.u.mem_event_op.shared_addr = (unsigned long )page; domctl.u.mem_event_op.ring_addr = (unsigned long )ring_page; - - domctl.u.mem_event_op.gfn = gfn;return do_domctl (xch, &domctl); } +int xc_mem_event_memop (xc_interface *xch,domid_t domain_id, +unsigned int op,unsigned int mode, +uint64_t gfn,void *buffer) +{ +xen_mem_event_op_t meo; + +memset (&meo,0 ,sizeof (meo)); + + meo.op = op; + meo.domain = domain_id; + meo.gfn = gfn; + meo.buffer = (unsigned long ) buffer; + +return do_memory_op (xch, mode, &meo,sizeof (meo)); +} +
tools/libxc/xc_mem_paging.c +22 -22
80 lines not on this page · raw message
tools/libxc/xc_memshr.c +84 -98
265 lines not on this page · raw message
tools/libxc/xenctrl.h +4 -2
@@ -1888,8 +1888,10 @@ int xc_tmem_restore_extra(xc_interface ** mem_event operations */ int xc_mem_event_control (xc_interface *xch,domid_t domain_id,unsigned int op, -unsigned int mode,void *shared_page, -void *ring_page,unsigned long gfn); +unsigned int mode,void *shared_page,void *ring_page); +int xc_mem_event_memop (xc_interface *xch,domid_t domain_id, +unsigned int op,unsigned int mode, +uint64_t gfn,void *buffer);int xc_mem_paging_enable (xc_interface *xch,domid_t domain_id,void *shared_page,void *ring_page);
tools/memshr/interface.c +2 -2
@@ -186,12 +186,12 @@ int memshr_vbd_issue_ro_request(char *buremove the relevant ones from the map */ switch (ret) { -case XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID : +case XENMEM_SHARING_OP_S_HANDLE_INVALID : ret =blockshr_shrhnd_remove (memshr.blks, source_st,NULL );if (ret)DPRINTF ("Could not rm invl s_hnd: %u %" PRId64" %" PRId64"\n" , source_st.domain, source_st.frame, source_st.handle);break ; -case XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID : +case XENMEM_SHARING_OP_C_HANDLE_INVALID : ret =blockshr_shrhnd_remove (memshr.blks, client_st,NULL );if (ret)DPRINTF ("Could not rm invl c_hnd: %u %" PRId64" %" PRId64"\n" , client_st.domain, client_st.frame, client_st.handle);
tools/tests/mem-sharing/memshrtool.c +2 -2
@@ -34,9 +34,9 @@ static int usage(const char* prog)int rc = f; \if ( rc <0 ) { \ printf("error executing %s: %s\n", #f, \ - ((errno *-1 ) ==XEN_DOMCTL_MEM_SHARING_C_HANDLE_INVALID ) ? \ + ((errno *-1 ) ==XENMEM_SHARING_OP_S_HANDLE_INVALID ) ? \"problem with client handle" :\ - ((errno *-1 ) ==XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID ) ? \ + ((errno *-1 ) ==XENMEM_SHARING_OP_C_HANDLE_INVALID ) ? \"problem with source handle" :strerror (errno)); \return rc; \ } \
xen/arch/x86/domctl.c +0 -1
@@ -1463,7 +1463,6 @@ long arch_do_domctl(if ( !ret ) ret =mem_sharing_domctl (d, &domctl->u.mem_sharing_op);rcu_unlock_domain (d); -copy_to_guest (u_domctl, domctl,1 ); } }break ;
xen/arch/x86/mm/mem_access.c +3 -4
@@ -25,14 +25,13 @@#include <asm/mem_event.h> -int mem_access_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec, -XEN_GUEST_HANDLE (void ) u_domctl) +int mem_access_memop (struct domain *d,xen_mem_event_op_t *meo) {int rc; -switch ( mec->op ) +switch ( meo->op ) { -case XEN_DOMCTL_MEM_EVENT_OP_ACCESS_RESUME : +case XENMEM_access_op_resume: {p2m_mem_access_resume (d); rc =0 ;
xen/arch/x86/mm/mem_event.c +57 -11
101 lines not on this page · raw message
xen/arch/x86/mm/mem_paging.c +6 -7
@@ -25,33 +25,32 @@#include <asm/mem_event.h> -int mem_paging_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec, -XEN_GUEST_HANDLE (void ) u_domctl) +int mem_paging_memop (struct domain *d,xen_mem_event_op_t *mec) {switch ( mec->op ) { -case XEN_DOMCTL_MEM_EVENT_OP_PAGING_NOMINATE : +case XENMEM_paging_op_nominate: {unsigned long gfn = mec->gfn;return p2m_mem_paging_nominate (d, gfn); }break ; -case XEN_DOMCTL_MEM_EVENT_OP_PAGING_EVICT : +case XENMEM_paging_op_evict: {unsigned long gfn = mec->gfn;return p2m_mem_paging_evict (d, gfn); }break ; -case XEN_DOMCTL_MEM_EVENT_OP_PAGING_PREP : +case XENMEM_paging_op_prep: {unsigned long gfn = mec->gfn; -return p2m_mem_paging_prep (d, gfn, mec->u.buffer); +return p2m_mem_paging_prep (d, gfn, mec->buffer); }break ; -case XEN_DOMCTL_MEM_EVENT_OP_PAGING_RESUME : +case XENMEM_paging_op_resume: {p2m_mem_paging_resume (d);return 0 ;
xen/arch/x86/mm/mem_sharing.c +58 -41
254 lines not on this page · raw message
xen/arch/x86/x86_64/compat/mm.c +23 -0
@@ -2,6 +2,7 @@#include <xen/multicall.h> #include <compat/memory.h> #include <compat/xen.h> +#include <asm/mem_event.h> int compat_set_gdt (XEN_GUEST_HANDLE (uint) frame_list,unsigned int entries) {
@@ -211,6 +212,28 @@ int compat_arch_memory_op(int op, XEN_GUcase XENMEM_get_sharing_shared_pages:return mem_sharing_get_nr_shared_mfns (); +case XENMEM_paging_op: +case XENMEM_access_op: + { +xen_mem_event_op_t meo; +if (copy_from_guest (&meo, arg,1 ) ) +return -EFAULT ; + rc =do_mem_event_op (op, meo.domain, (void *) &meo); +if ( !rc &©_to_guest (arg, &meo,1 ) ) +return -EFAULT ; +break ; + } +case XENMEM_sharing_op: + { +xen_mem_sharing_op_t mso; +if (copy_from_guest (&mso, arg,1 ) ) +return -EFAULT ; + rc =do_mem_event_op (op, mso.domain, (void *) &mso); +if ( !rc &©_to_guest (arg, &mso,1 ) ) +return -EFAULT ; +break ; + } +default : rc = -ENOSYS ;break ;
xen/arch/x86/x86_64/mm.c +23 -0
@@ -34,6 +34,7 @@#include <asm/msr.h> #include <asm/setup.h> #include <asm/numa.h> +#include <asm/mem_event.h> #include <asm/mem_sharing.h> #include <public/memory.h>
@@ -1100,6 +1101,28 @@ long subarch_memory_op(int op, XEN_GUESTcase XENMEM_get_sharing_shared_pages:return mem_sharing_get_nr_shared_mfns (); +case XENMEM_paging_op: +case XENMEM_access_op: + { +xen_mem_event_op_t meo; +if (copy_from_guest (&meo, arg,1 ) ) +return -EFAULT ; + rc =do_mem_event_op (op, meo.domain, (void *) &meo); +if ( !rc &©_to_guest (arg, &meo,1 ) ) +return -EFAULT ; +break ; + } +case XENMEM_sharing_op: + { +xen_mem_sharing_op_t mso; +if (copy_from_guest (&mso, arg,1 ) ) +return -EFAULT ; + rc =do_mem_event_op (op, mso.domain, (void *) &mso); +if ( !rc &©_to_guest (arg, &mso,1 ) ) +return -EFAULT ; +break ; + } +default : rc = -ENOSYS ;break ;
xen/include/asm-x86/mem_access.h +1 -2
@@ -21,8 +21,7 @@*/ -int mem_access_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec, -XEN_GUEST_HANDLE (void ) u_domctl); +int mem_access_memop (struct domain *d,xen_mem_event_op_t *meo);int mem_access_send_req (struct domain *d,mem_event_request_t *req);
xen/include/asm-x86/mem_event.h +2 -0
@@ -42,6 +42,8 @@ void mem_event_put_request(struct domainint mem_event_get_response (struct domain *d,struct mem_event_domain *med,mem_event_response_t *rsp); +struct domain *get_mem_event_op_target (uint32_t domain,int *rc); +int do_mem_event_op (int op,uint32_t domain,void *arg);int mem_event_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec,XEN_GUEST_HANDLE (void ) u_domctl);
xen/include/asm-x86/mem_paging.h +1 -2
@@ -21,8 +21,7 @@*/ -int mem_paging_domctl (struct domain *d,xen_domctl_mem_event_op_t *mec, -XEN_GUEST_HANDLE (void ) u_domctl); +int mem_paging_memop (struct domain *d,xen_mem_event_op_t *meo);/*
xen/include/asm-x86/mem_sharing.h +3 -0
@@ -23,6 +23,7 @@#define __MEM_SHARING_H__#include <public/domctl.h> +#include <public/memory.h> /* Auditing of memory sharing code? */ #define MEM_SHARING_AUDIT 0
@@ -56,6 +57,8 @@ int mem_sharing_unshare_page(struct domaunsigned long gfn, uint16_t flags);int mem_sharing_sharing_resume (struct domain *d); +int mem_sharing_memop (struct domain *d, +xen_mem_sharing_op_t *mec);int mem_sharing_domctl (struct domain *d,xen_domctl_mem_sharing_op_t *mec);void mem_sharing_init (void );
xen/include/public/domctl.h +19 -71
129 lines not on this page · raw message
xen/include/public/memory.h +87 -0
99 lines not on this page · raw message
Re: [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls
Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls"):
Quoted 3 lines
We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the
memory_ophypercalls (e.g XENMEM_*).
The tools part of this looks sensible (and pretty formulaic) to me.
It'll need a hypervisor maintainer to approve the innards.
Ian.
Re: [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls
Quoted 11 lines
Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 1 of 3] Use memops for mem paging, sharing, and access, instead of domctls"):
Quoted 3 lines
We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the
memory_ophypercalls (e.g XENMEM_*).The tools part of this looks sensible (and pretty formulaic) to me.
Acked-by Ian Jackson <[email protected]>It'll need a hypervisor maintainer to approve the innards.
Both this and the next patch have been acked by Tim Deegan on the hypervisor side.
However, I have not tested these in isolation. I've always used them preceded by the "locking p2m series" posted earlier. I would suggest that you hold off applying this until the locking p2m series gets applied. I don't think there should be any problems, but I'm being conservative here.
Thanks!
Andres
Quoted 3 lines
Ian.
PATCH 1/1 x86/mm: New sharing audit memop
In reply to Andres Lagar-Cavillatools/libxc/xc_memshr.c | 11 +++++++++++ tools/libxc/xenctrl.h | 1 + tools/tests/mem-sharing/memshrtool.c | 11 +++++++++++ xen/arch/x86/mm/mem_sharing.c | 13 ++++++------- xen/arch/x86/x86_64/compat/mm.c | 3 +++ xen/arch/x86/x86_64/mm.c | 2 ++ xen/include/asm-x86/mem_sharing.h | 3 ++- xen/include/public/memory.h | 1 + 8 files changed, 37 insertions(+), 8 deletions(-)
Remove costly mem_sharting audits from the inline path, and instead make them callable as a memop.
Have the audit function return the number of errors detected.
Update memshrtool to be able to trigger audits.
Set sharing audits as enabled by default.
tools/libxc/xc_memshr.c +11 -0
@@ -211,6 +211,17 @@ int xc_memshr_debug_gref(xc_interface *xreturn xc_memshr_memop (xch, domid, &mso); } +int xc_memshr_audit (xc_interface *xch) +{ +xen_mem_sharing_op_t mso; + +memset (&mso,0 ,sizeof (mso)); + + mso.op = XENMEM_sharing_op_audit; + +return do_memory_op (xch, XENMEM_sharing_op, &mso,sizeof (mso)); +} +long xc_sharing_freed_pages (xc_interface *xch) {return do_memory_op (xch, XENMEM_get_sharing_freed_pages,NULL ,0 );
tools/libxc/xenctrl.h +1 -0
@@ -1956,6 +1956,7 @@ int xc_memshr_debug_mfn(xc_interface *xcint xc_memshr_debug_gref (xc_interface *xch,domid_t domid,grant_ref_t gref); +int xc_memshr_audit (xc_interface *xch);int xc_flask_load (xc_interface *xc_handle,char *buf,uint32_t size);int xc_flask_context_to_sid (xc_interface *xc_handle,char *buf,uint32_t size,uint32_t *sid);
tools/tests/mem-sharing/memshrtool.c +11 -0
@@ -27,6 +27,7 @@ static int usage(const char* prog)printf (" add-to-physmap <domid> <gfn> <source> <source-gfn> <source-handle>\n" );printf (" - Populate a page in a domain with a shared page.\n" );printf (" debug-gfn <domid> <gfn> - Debug a particular domain and gfn.\n" ); +printf (" audit - Audit the sharing subsytem in Xen.\n" );return 1 ; }
@@ -160,6 +161,16 @@ int main(int argc, const char** argv) gfn =strtol (argv[3 ],NULL ,0 );R (xc_memshr_debug_gfn (xch, domid, gfn)); } +else if ( !strcasecmp (cmd,"audit" ) ) + { +int rc =xc_memshr_audit (xch); +if ( rc <0 ) + { +printf ("error executing xc_memshr_audit: %s\n" ,strerror (errno)); +return rc; + } +printf ("Audit returned %d errors.\n" , rc); + }return 0 ; }
xen/arch/x86/mm/mem_sharing.c +6 -7
@@ -50,8 +50,6 @@ DEFINE_PER_CPU(pg_lock_data_t, __pld);#if MEM_SHARING_AUDIT -static void mem_sharing_audit (void ); -static struct list_head shr_audit_list;static spinlock_t shr_audit_lock;DEFINE_RCU_READ_LOCK (shr_audit_read_lock);
@@ -81,7 +79,10 @@ static inline void audit_del_list(struct#else -#define mem_sharing_audit () ((void)0) +int mem_sharing_audit (void ) +{ +return -ENOSYS ; +}#define audit_add_list (p) ((void)0)static inline void audit_del_list (struct page_info *page)
@@ -212,7 +213,7 @@ static struct page_info* mem_sharing_loo }#if MEM_SHARING_AUDIT -static void mem_sharing_audit (void ) +int mem_sharing_audit (void ) {int errors =0 ;unsigned long count_expected;
@@ -338,6 +339,7 @@ static void mem_sharing_audit(void) errors++; } +return errors; }#endif
@@ -914,7 +916,6 @@ int mem_sharing_unshare_page(struct domagfn_info_t *gfn_info =NULL ;struct list_head *le; -mem_sharing_audit (); mfn =get_gfn (d, gfn, &p2mt);/* Has someone already unshared it? */
@@ -1178,8 +1179,6 @@ int mem_sharing_memop(struct domain *d,break ; } -mem_sharing_audit (); -return rc; }
xen/arch/x86/x86_64/compat/mm.c +3 -0
@@ -3,6 +3,7 @@#include <compat/memory.h> #include <compat/xen.h> #include <asm/mem_event.h> +#include <asm/mem_sharing.h> int compat_set_gdt (XEN_GUEST_HANDLE (uint) frame_list,unsigned int entries) {
@@ -228,6 +229,8 @@ int compat_arch_memory_op(int op, XEN_GUxen_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_mem_event_op (op, mso.domain, (void *) &mso);if ( !rc &©_to_guest (arg, &mso,1 ) )return -EFAULT ;
xen/arch/x86/x86_64/mm.c +2 -0
@@ -1117,6 +1117,8 @@ long subarch_memory_op(int op, XEN_GUESTxen_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_mem_event_op (op, mso.domain, (void *) &mso);if ( !rc &©_to_guest (arg, &mso,1 ) )return -EFAULT ;
xen/include/asm-x86/mem_sharing.h +2 -1
@@ -26,7 +26,7 @@#include <public/memory.h> /* Auditing of memory sharing code? */ -#define MEM_SHARING_AUDIT 0 +#define MEM_SHARING_AUDIT 1typedef uint64_t shr_handle_t ;
@@ -61,6 +61,7 @@ int mem_sharing_memop(struct domain *d, xen_mem_sharing_op_t *mec);int mem_sharing_domctl (struct domain *d,xen_domctl_mem_sharing_op_t *mec); +int mem_sharing_audit (void );void mem_sharing_init (void );#else
xen/include/public/memory.h +1 -0
@@ -349,6 +349,7 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_event_op#define XENMEM_sharing_op_debug_mfn 5#define XENMEM_sharing_op_debug_gref 6#define XENMEM_sharing_op_add_physmap 7 +#define XENMEM_sharing_op_audit 8#define XENMEM_SHARING_OP_S_HANDLE_INVALID (-10)#define XENMEM_SHARING_OP_C_HANDLE_INVALID (-9)
Re: [PATCH 2 of 3] x86/mm: New sharing audit memop
Andres Lagar-Cavilla writes ("[Xen-devel] [PATCH 2 of 3] x86/mm: New sharing audit memop"):
Quoted 2 lines
Remove costly
mem_shartingaudits from the inline path, and instead make them callable as a memop.
For the tools parts,
But a hypervisor maintainer should have the final say.
Ian.
PATCH 1/1 Tools: Sanitize mem_event/access/paging interfaces
In reply to Andres Lagar-Cavillatools/libxc/xc_mem_access.c | 10 +++++- tools/libxc/xc_mem_event.c | 16 +++++++--- tools/libxc/xc_mem_paging.c | 10 +++++- tools/libxc/xenctrl.h | 7 ++- tools/tests/xen-access/xen-access.c | 56 +++++------------------------------- tools/xenpaging/xenpaging.c | 33 ++++++--------------- tools/xenpaging/xenpaging.h | 2 +- xen/arch/x86/mm/mem_event.c | 33 +------------------- xen/include/public/domctl.h | 4 +- xen/include/public/mem_event.h | 4 -- xen/include/xen/sched.h | 2 - 11 files changed, 56 insertions(+), 121 deletions(-)
Don't use the superfluous shared page, return the event channel directly as part of the domctl struct, instead.
Use hypercall buffers to manage the ring page.
In-tree consumers (xenpaging, xen-access) updated. This is an ABI/API change, so please voice any concerns.
Known pending issues:
- pager could die and its ring page could be used by some other process, yet Xen retains the mapping to it.
- use a saner interface for the paging_load buffer.
tools/libxc/xc_mem_access.c +8 -2
@@ -25,12 +25,18 @@int xc_mem_access_enable (xc_interface *xch,domid_t domain_id, -void *shared_page,void *ring_page) +uint32_t *port,xc_hypercall_buffer_t *ring_page) { +if ( !port ) + { + errno = -EINVAL ; +return -1 ; + } +return xc_mem_event_control (xch, domain_id,XEN_DOMCTL_MEM_EVENT_OP_ACCESS_ENABLE ,XEN_DOMCTL_MEM_EVENT_OP_ACCESS , - shared_page, ring_page); + port, ring_page); }int xc_mem_access_disable (xc_interface *xch,domid_t domain_id)
tools/libxc/xc_mem_event.c +11 -5
@@ -24,19 +24,25 @@#include "xc_private.h" int xc_mem_event_control (xc_interface *xch,domid_t domain_id,unsigned int op, -unsigned int mode,void *page,void *ring_page) +unsigned int mode,uint32_t *port, +xc_hypercall_buffer_t *ring_page) {DECLARE_DOMCTL ; +DECLARE_HYPERCALL_BUFFER_ARGUMENT (ring_page); +int rc; domctl.cmd = XEN_DOMCTL_mem_event_op; domctl.domain = domain_id; domctl.u.mem_event_op.op = op; domctl.u.mem_event_op.mode = mode; - - domctl.u.mem_event_op.shared_addr = (unsigned long )page; - domctl.u.mem_event_op.ring_addr = (unsigned long )ring_page; + domctl.u.mem_event_op.ring_addr = (ring_page) ? + (unsigned long long )HYPERCALL_BUFFER_AS_ARG (ring_page) :0 ; -return do_domctl (xch, &domctl); + errno =0 ; + rc =do_domctl (xch, &domctl); +if ( !rc && port ) + *port = domctl.u.mem_event_op.port; +return rc; } int xc_mem_event_memop(xc_interface *xch, domid_t domain_id,
tools/libxc/xc_mem_paging.c +8 -2
@@ -25,12 +25,18 @@int xc_mem_paging_enable (xc_interface *xch,domid_t domain_id, -void *shared_page,void *ring_page) +uint32_t *port,xc_hypercall_buffer_t *ring_page) { +if ( !port ) + { + errno = -EINVAL ; +return -1 ; + } +return xc_mem_event_control (xch, domain_id,XEN_DOMCTL_MEM_EVENT_OP_PAGING_ENABLE ,XEN_DOMCTL_MEM_EVENT_OP_PAGING , - shared_page, ring_page); + port, ring_page); }int xc_mem_paging_disable (xc_interface *xch,domid_t domain_id)
tools/libxc/xenctrl.h +4 -3
@@ -1888,13 +1888,14 @@ int xc_tmem_restore_extra(xc_interface ** mem_event operations */ int xc_mem_event_control (xc_interface *xch,domid_t domain_id,unsigned int op, -unsigned int mode,void *shared_page,void *ring_page); +unsigned int mode,uint32_t *port, +xc_hypercall_buffer_t *ring_page);int xc_mem_event_memop (xc_interface *xch,domid_t domain_id,unsigned int op,unsigned int mode,uint64_t gfn,void *buffer);int xc_mem_paging_enable (xc_interface *xch,domid_t domain_id, -void *shared_page,void *ring_page); +uint32_t *port,xc_hypercall_buffer_t *ring_page);int xc_mem_paging_disable (xc_interface *xch,domid_t domain_id);int xc_mem_paging_nominate (xc_interface *xch,domid_t domain_id,unsigned long gfn);
@@ -1906,7 +1907,7 @@ int xc_mem_paging_resume(xc_interface *x unsigned long gfn);int xc_mem_access_enable (xc_interface *xch,domid_t domain_id, -void *shared_page,void *ring_page); +uint32_t *port,xc_hypercall_buffer_t *ring_page);int xc_mem_access_disable (xc_interface *xch,domid_t domain_id);int xc_mem_access_resume (xc_interface *xch,domid_t domain_id,unsigned long gfn);
tools/tests/xen-access/xen-access.c +9 -47
100 lines not on this page · raw message
tools/xenpaging/xenpaging.c +10 -23
@@ -285,6 +285,7 @@ static struct xenpaging *xenpaging_init(xentoollog_logger *dbg =NULL ;char *p;int rc; +DECLARE_HYPERCALL_BUFFER (void, ring_page);/* Allocate memory */ paging =calloc (1 ,sizeof (struct xenpaging ));
@@ -341,16 +342,9 @@ static struct xenpaging *xenpaging_init( goto err; } -/* Initialise shared page */ - paging->mem_event.shared_page =init_page (); -if ( paging->mem_event.shared_page ==NULL ) - { -PERROR ("Error initialising shared page" ); - goto err; - } -/* Initialise ring page */ - paging->mem_event.ring_page =init_page (); + paging->mem_event.ring_page = +xc_hypercall_buffer_alloc_pages (paging->xc_handle, ring_page,1 );if ( paging->mem_event.ring_page ==NULL ) {PERROR ("Error initialising ring page" );
@@ -365,8 +359,8 @@ static struct xenpaging *xenpaging_init(/* Initialise Xen */ rc =xc_mem_paging_enable (xch, paging->mem_event.domain_id, - paging->mem_event.shared_page, - paging->mem_event.ring_page); + &paging->mem_event.evtchn_port, +HYPERCALL_BUFFER (ring_page));if ( rc !=0 ) {switch ( errno ) {
@@ -397,7 +391,7 @@ static struct xenpaging *xenpaging_init(/* Bind event notification */ rc =xc_evtchn_bind_interdomain (paging->mem_event.xce_handle, paging->mem_event.domain_id, - paging->mem_event.shared_page->port); + paging->mem_event.evtchn_port);if ( rc <0 ) {PERROR ("Failed to bind event channel" );
@@ -452,19 +446,12 @@ static struct xenpaging *xenpaging_init( {if ( paging->xs_handle )xs_close (paging->xs_handle); + +if ( paging->mem_event.ring_page ) +xc_hypercall_buffer_free_pages (xch, ring_page,1 ); +if ( xch )xc_interface_close (xch); -if ( paging->mem_event.shared_page ) - { -munlock (paging->mem_event.shared_page,PAGE_SIZE ); -free (paging->mem_event.shared_page); - } - -if ( paging->mem_event.ring_page ) - { -munlock (paging->mem_event.ring_page,PAGE_SIZE ); -free (paging->mem_event.ring_page); - }free (dom_path);free (watch_target_tot_pages);
tools/xenpaging/xenpaging.h +1 -1
@@ -36,7 +36,7 @@ struct mem_event {xc_evtchn *xce_handle;int port;mem_event_back_ring_t back_ring; -mem_event_shared_page_t *shared_page; +uint32_t evtchn_port;void *ring_page; };
xen/arch/x86/mm/mem_event.c +3 -30
@@ -50,12 +50,10 @@ static int mem_event_enable(struct domain *dom_mem_event = current->domain;struct vcpu *v = current;unsigned long ring_addr = mec->ring_addr; -unsigned long shared_addr = mec->shared_addr;l1_pgentry_t l1e; -unsigned long shared_gfn =0 , ring_gfn =0 ;/* gcc ... */ +unsigned long ring_gfn =0 ;/* gcc ... */ p2m_type_t p2mt;mfn_t ring_mfn; -mfn_t shared_mfn;/* Only one helper at a time. If the helper crashed, * the ring is in an undefined state and so is the guest.
@@ -66,10 +64,6 @@ static int mem_event_enable(/* Get MFN of ring page */ guest_get_eff_l1e (v, ring_addr, &l1e); ring_gfn =l1e_get_pfn (l1e); -/* We're grabbing these two in an order that could deadlock -* dom0 if 1. it were an hvm 2. there were two concurrent -* enables 3. the two gfn's in each enable criss-crossed -* 2MB regions. Duly noted.... */ ring_mfn =get_gfn (dom_mem_event, ring_gfn, &p2mt);if (unlikely (!mfn_valid (mfn_x (ring_mfn))) )
@@ -80,23 +74,9 @@ static int mem_event_enable(mem_event_ring_lock_init (med); -/* Get MFN of shared page */ -guest_get_eff_l1e (v, shared_addr, &l1e); - shared_gfn =l1e_get_pfn (l1e); - shared_mfn =get_gfn (dom_mem_event, shared_gfn, &p2mt); - -if (unlikely (!mfn_valid (mfn_x (shared_mfn))) ) - { -put_gfn (dom_mem_event, ring_gfn); -put_gfn (dom_mem_event, shared_gfn); -return -EINVAL ; - } - -/* Map ring and shared pages */ +/* Map ring page */ med->ring_page =map_domain_page (mfn_x (ring_mfn)); - med->shared_page =map_domain_page (mfn_x (shared_mfn));put_gfn (dom_mem_event, ring_gfn); -put_gfn (dom_mem_event, shared_gfn);/* Set the number of currently blocked vCPUs to 0. */ med->blocked =0 ;
@@ -108,8 +88,7 @@ static int mem_event_enable(if ( rc <0 ) goto err; - ((mem_event_shared_page_t *)med->shared_page)->port = rc; - med->xen_port = rc; + med->xen_port = mec->port = rc;/* Prepare ring buffer */ FRONT_RING_INIT(&med->front_ring,
@@ -125,9 +104,6 @@ static int mem_event_enable(return 0 ; err: -unmap_domain_page (med->shared_page); - med->shared_page =NULL ; -unmap_domain_page (med->ring_page); med->ring_page =NULL ;
@@ -246,9 +222,6 @@ static int mem_event_disable(struct domaunmap_domain_page (med->ring_page); med->ring_page =NULL ; -unmap_domain_page (med->shared_page); - med->shared_page =NULL ; -/* Unblock all vCPUs */ for_each_vcpu ( d, v ) {
xen/include/public/domctl.h +2 -2
@@ -746,8 +746,8 @@ struct xen_domctl_mem_event_op {uint32_t op;/* XEN_DOMCTL_MEM_EVENT_OP_*_* */ uint32_t mode;/* XEN_DOMCTL_MEM_EVENT_OP_* */ -uint64_aligned_t shared_addr;/* IN: Virtual address of shared page */ -uint64_aligned_t ring_addr;/* IN: Virtual address of ring page */ +uint32_t port;/* OUT: event channel for ring */ +uint64_aligned_t ring_addr;/* IN: Virtual address of ring page */ };typedef struct xen_domctl_mem_event_op xen_domctl_mem_event_op_t ;DEFINE_XEN_GUEST_HANDLE (xen_domctl_mem_event_op_t);
xen/include/public/mem_event.h +0 -4
@@ -51,10 +51,6 @@#define MEM_EVENT_REASON_INT3 5/* int3 was hit: gla/gfn are RIP */ #define MEM_EVENT_REASON_SINGLESTEP 6/* single step was invoked: gla/gfn are RIP */ -typedef struct mem_event_shared_page { -uint32_t port; -}mem_event_shared_page_t ; - typedef struct mem_event_st { uint16_t type; uint16_t flags;
xen/include/xen/sched.h +0 -2
@@ -190,8 +190,6 @@ struct mem_event_domain/* The ring has 64 entries */ unsigned char foreign_producers;unsigned char target_producers; -/* shared page */ -mem_event_shared_page_t *shared_page;/* shared ring page */ void *ring_page;/* front-end ring */
Quoted 24 lines · Andres Lagar-Cavilla
i(Was switch from domctl to memops)
Changes from v1 posted Feb 2nd 2012- Patches 1 & 2 Acked-by Tim Deegan on the hypervisor side
- Added patch 3 to clean up the enable domctl interface, based on discussion with Ian CampbellDescription from original post follows:
Per page operations in the paging, sharing, and access tracking subsystems are all implemented with domctls (e.g. a domctl to evict one page, or to share one page).
Under heavy load, the domctl path reveals a lack of scalability. The domctl lock serializes dom0's vcpus in the hypervisor. When performing thousands of per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of its other work-starved processes.
We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the
memory_ophypercalls (e.g XENMEM_*).
Quoted 5 lines
This is a backwards-incompatible ABI change. It's been floating on the list for a couple weeks now, with no nacks thus far.
Signed-off-by Andres Lagar-Cavilla <andres@lagarcavilla>Signed-off-by Adin Scannell <[email protected]>
Applied 1 and 2; thanks.
I'll leave patch 3 for others to comment -- I know there are out-of-tree users of the mem-access interface, and changing the hypercalls is less disruptive than changing the libxc interface.
Tim.
Quoted 35 lines
Quoted 34 lines · Andres Lagar-Cavilla
i(Was switch from domctl to memops)
Changes from v1 posted Feb 2nd 2012- Patches 1 & 2 Acked-by Tim Deegan on the hypervisor side
- Added patch 3 to clean up the enable domctl interface, based on discussion with Ian CampbellDescription from original post follows:
Per page operations in the paging, sharing, and access tracking subsystems are
all implemented with domctls (e.g. a domctl to evict one page, or to share one
page).Under heavy load, the domctl path reveals a lack of scalability. The domctl
lock serializes dom0's vcpus in the hypervisor. When performing thousands of
per-page operations on dozens of domains, these vcpus will spin in the hypervisor. Beyond the aggressive locking, an added inefficiency of blocking
vcpus in the domctl lock is that dom0 is prevented from re-scheduling any of
its other work-starved processes.We retain the domctl interface for setting up and tearing down paging/sharing/mem access for a domain. But we migrate all the per page operations to use the
memory_ophypercalls (e.g XENMEM_*).This is a backwards-incompatible ABI change. It's been floating on the list for
a couple weeks now, with no nacks thus far.
Quoted 8 lines
Quoted 2 lines
Signed-off-by Andres Lagar-Cavilla <andres@lagarcavilla>Signed-off-by Adin Scannell <[email protected]>Applied 1 and 2; thanks.
I'll leave patch 3 for others to comment -- I know there are out-of-tree users of the mem-access interface, and changing the hypercalls is less disruptive than changing the libxc interface.
Makes a lot of sense. Thanks.
I don't view this change as sine qua-non, yet, "it would be nice if"...
Is there a timeout mechanism if out-of-tree consumers are not on the ball?
Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process. This is dealt with in the qemu-dm case by stuffing the ring in an unused pfn (presumably somewhere in the mmio hole?)
Would that work? Is there a policy for parceling out these "magic pfn's"?
Andres
Quoted 3 lines
Tim.
Quoted 1 line · Andres Lagar-Cavilla
Is there a timeout mechanism if out-of-tree consumers are not on the ball?
I would guess they need an adjustment for a step from 4.1 to 4.2. Wasnt there already enough churn in the source in that area? And the (old) SONAME is gone as well, so they need to be recompiled anyway.
Quoted 7 lines
Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process. This is dealt with in the qemu-dm case by stuffing the ring in an unused pfn (presumably somewhere in the mmio hole?)Would that work? Is there a policy for parceling out these "magic pfn's"?
I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.
Olaf
Quoted 6 lines · Olaf Hering
Quoted 6 lines
Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process. This is dealt with in the qemu-dm case by stuffing the ring in an unused pfn (presumably somewhere in the mmio hole?)
Quoted 6 lines
Quoted 1 line
Would that work? Is there a policy for parceling out these "magic pfn's"?
I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.
Yes indeed. The gpfn could be allocated by the domain builder, or by hvmloader (which might be too late!).
-- Keir
Quoted 14 lines · Olaf Hering
Quoted 13 lines
Quoted 12 lines
Actually, this hiatus allows me to float a perhaps cleaner way to map the
ring: the known problem is that the pager may die abruptly, and Xen is still posting events to a page now belonging to some other dom0 process.
This is dealt with in the qemu-dm case by stuffing the ring in an unused
pfn (presumably somewhere in the mmio hole?)Would that work? Is there a policy for parceling out these "magic pfn's"?
Quoted 7 lines
Quoted 4 lines
I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.
Yes indeed. The gpfn could be allocated by the domain builder, or by hvmloader (which might be too late!).
Well, let's say the domain builder reserves three gpfns (paging, access, sharing). When helpers for each come up, the enable domctl allocates the actual frame. Or, we could have the frames allocated up front, it's not terrible wastage -- the enable domctl would init the ring.
My question was more along the lines of how to choose which guest pfns to reserve for this.
Andres
Quoted 6 lines
-- Keir
Quoted 8 lines · Andres Lagar-Cavilla
Quoted 7 lines
Quoted 4 lines
I was just thinking about this issue. The bug is that the ring lives in dom0, the page should belong to domU and should be destroyed along with it. And ring users in dom0 should request (and maybe initially allocate and setup) a certain gfn belonging to domU.
Yes indeed. The gpfn could be allocated by the domain builder, or by hvmloader (which might be too late!).
Quoted 7 lines
Well, let's say the domain builder reserves three gpfns (paging, access, sharing). When helpers for each come up, the enable domctl allocates the actual frame. Or, we could have the frames allocated up front, it's not terrible wastage -- the enable domctl would init the ring.
My question was more along the lines of how to choose which guest pfns to reserve for this.
Domain builder (xc_hvm_build.c) would be the sensible place to reserve, and push that info down to Xen in some way. Then allocate the pages during the enable domctl, via alloc_xenheap_page() (for a few arcane reasons, it's better to use this than alloc_domheap_page() for this particular purpose).
-- Keir